QPID-6168: [Java Broker] Enforce that an attribute's value falls within the domain of values described by valid value set

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1633184 13f79535-47bb-0310-9956-ffa450edef68

6 files changed, 97 insertions, 9 deletions

diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
index 4edfbe0331..d60064b722 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
@@ -382,7 +382,23 @@ public abstract class AbstractConfiguredObject<X extends ConfiguredObject<X>> im
             {
                 field.getPreSettingAction().invoke(this);
             }
-            field.getField().set(this, attribute.convert(value, this));
+
+            Object desiredValue = attribute.convert(value, this);
+
+            if (attribute.hasValidValues())
+            {
+                if (!checkValidValues(attribute, desiredValue))
+                {
+                    throw new IllegalConfigurationException("Attribute '" + attribute.getName()
+                                                       + "' of instance of "+ getClass().getName()
+                                                       + " named '" + getName() + "'"
+                                                       + " cannot have value '" + desiredValue + "'"
+                                                       + ". Valid values are: "
+                                                       + attribute.validValues());
+                }
+            }
+
+            field.getField().set(this, desiredValue);
 
             if(field.getPostSettingAction() != null)
             {
@@ -403,6 +419,21 @@ public abstract class AbstractConfiguredObject<X extends ConfiguredObject<X>> im
         }
     }
 
+    private boolean checkValidValues(final ConfiguredAutomatedAttribute attribute, final Object desiredValue)
+    {
+        for (Object validValue : attribute.validValues())
+        {
+            Object convertedValidValue = attribute.getConverter().convert(validValue, this);
+
+            if (convertedValidValue.equals(desiredValue))
+            {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     @Override
     public final void open()
     {
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ConfiguredAutomatedAttribute.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ConfiguredAutomatedAttribute.java
index 4ef1d315dd..24a62de61c 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ConfiguredAutomatedAttribute.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ConfiguredAutomatedAttribute.java
@@ -142,7 +142,15 @@ public class ConfiguredAutomatedAttribute<C extends ConfiguredObject, T>  extend
                 return Collections.emptySet();
             }
         }
-        return Arrays.asList(_annotation.validValues());
+        else
+        {
+            return Arrays.asList(_annotation.validValues());
+        }
     }
 
+    /** Returns true iff this attribute has valid values defined */
+    public boolean hasValidValues()
+    {
+        return validValues() != null && validValues().size() > 0;
+    }
 }
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/port/RmiPort.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/port/RmiPort.java
index 8fad90ada3..e8a7d039a3 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/port/RmiPort.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/port/RmiPort.java
@@ -32,7 +32,7 @@ import org.apache.qpid.server.model.Transport;
 public interface RmiPort<X extends RmiPort<X>> extends Port<X>
 {
 
-    @ManagedAttribute( defaultValue = "RMI", validValues = { "[ \"RMI\"]"} )
+    @ManagedAttribute( defaultValue = "RMI", validValues = { "[ \"RMI\" ]" } )
     Set<Protocol> getProtocols();
 
     @ManagedAttribute( defaultValue = "TCP",
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/port/RmiPortImpl.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/port/RmiPortImpl.java
index 82e68d75c8..e7dff77bbf 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/port/RmiPortImpl.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/port/RmiPortImpl.java
@@ -48,12 +48,6 @@ public class RmiPortImpl extends AbstractPort<RmiPortImpl> implements RmiPort<Rm
         super.onValidate();
 
         validateOnlyOneInstance();
-
-        if (getTransports().contains(Transport.SSL))
-        {
-            throw new IllegalConfigurationException("Can't create RMI registry port which requires SSL");
-        }
-
     }
 
     public void setPortManager(PortManager manager)
diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java
index 54a059e067..9b4826a383 100644
--- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java
+++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java
@@ -464,4 +464,58 @@ public class AbstractConfiguredObjectTest extends QpidTestCase
         assertFalse("Child1 should not be opened", child1.isOpened());
         assertEquals("Unexpected child1 state", State.ERRORED, child1.getState());
     }
+
+    public void testConstructionEnforcesAttributeValidValues() throws Exception
+    {
+        final String objectName = getName();
+        Map<String, Object> illegalCreateAttributes = new HashMap<>();
+        illegalCreateAttributes.put(ConfiguredObject.NAME, objectName);
+        illegalCreateAttributes.put(TestRootCategory.VALID_VALUE, "illegal");
+
+        try
+        {
+            _model.getObjectFactory().create(TestRootCategory.class, illegalCreateAttributes);
+            fail("Exception not thrown");
+        }
+        catch (IllegalConfigurationException ice)
+        {
+            // PASS
+        }
+
+        Map<String, Object> legalCreateAttributes = new HashMap<>();
+        legalCreateAttributes.put(ConfiguredObject.NAME, objectName);
+        legalCreateAttributes.put(TestRootCategory.VALID_VALUE, TestRootCategory.VALID_VALUE1);
+
+        _model.getObjectFactory().create(TestRootCategory.class, legalCreateAttributes);
+        // PASS
+
+    }
+
+    public void testChangeEnforcesAttributeValidValues() throws Exception
+    {
+        final String objectName = getName();
+        Map<String, Object> legalCreateAttributes = new HashMap<>();
+        legalCreateAttributes.put(ConfiguredObject.NAME, objectName);
+        legalCreateAttributes.put(TestRootCategory.VALID_VALUE, TestRootCategory.VALID_VALUE1);
+
+        TestRootCategory object = _model.getObjectFactory().create(TestRootCategory.class, legalCreateAttributes);
+        assertEquals(TestRootCategory.VALID_VALUE1, object.getValidValue());
+
+        object.setAttribute(TestRootCategory.VALID_VALUE, TestRootCategory.VALID_VALUE1, TestRootCategory.VALID_VALUE2);
+        assertEquals(TestRootCategory.VALID_VALUE2, object.getValidValue());
+
+        try
+        {
+            object.setAttribute(TestRootCategory.VALID_VALUE, TestRootCategory.VALID_VALUE2, "illegal");
+            fail("Exception not thrown");
+        }
+        catch (IllegalConfigurationException iae)
+        {
+            // PASS
+        }
+
+        assertEquals(TestRootCategory.VALID_VALUE2, object.getValidValue());
+
+    }
+
 }
diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestRootCategory.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestRootCategory.java
index 2359a93b43..69f429da36 100644
--- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestRootCategory.java
+++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestRootCategory.java
@@ -35,6 +35,7 @@ public interface TestRootCategory<X extends TestRootCategory<X>> extends Configu
     String DEFAULTED_VALUE = "defaultedValue";
     String STRING_VALUE = "stringValue";
     String MAP_VALUE = "mapValue";
+    String VALID_VALUE = "validValue";
 
 
     String TEST_CONTEXT_DEFAULT = "TEST_CONTEXT_DEFAULT";