QPID-5721: [Java Broker] Improve test coverage for default attributes, and attributes whose values are computed from context variables.

Also fixed defect so that a context attributes are resolved first, so that other attribute values may be calculated from context variables
set on the same object.

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

6 files changed, 196 insertions, 25 deletions

diff --git a/qpid/java/broker-codegen/src/main/java/org/apache/qpid/server/model/validation/AttributeAnnotationValidator.java b/qpid/java/broker-codegen/src/main/java/org/apache/qpid/server/model/validation/AttributeAnnotationValidator.java
index 1422a52449..a831e1ebd9 100644
--- a/qpid/java/broker-codegen/src/main/java/org/apache/qpid/server/model/validation/AttributeAnnotationValidator.java
+++ b/qpid/java/broker-codegen/src/main/java/org/apache/qpid/server/model/validation/AttributeAnnotationValidator.java
@@ -310,6 +310,10 @@ public class AttributeAnnotationValidator extends AbstractProcessor
         if(typeUtils.isSameType(erasedType, getErasure("java.util.Map")))
         {
             List<? extends TypeMirror> args = ((DeclaredType) type).getTypeArguments();
+            if (args.size() != 2)
+            {
+                throw new IllegalArgumentException("Map types " + type + " must have exactly two type arguments");
+            }
             return isValidType(args.get(0)) && (isValidType(args.get(1)) || typeUtils.isSameType(args.get(1), getErasure("java.lang.Object")));
         }
 
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 812ffc618d..f94f669dfc 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
@@ -586,25 +586,36 @@ public abstract class AbstractConfiguredObject<X extends ConfiguredObject<X>> im
 
     protected void onResolve()
     {
+        // If there is a context attribute, resolve it first, so that other attribute values
+        // may support values containing references to context keys.
+        ConfiguredObjectAttribute<?, ?> contextAttribute = _attributeTypes.get("context");
+        if (contextAttribute != null && contextAttribute.isAutomated())
+        {
+            resolveAutomatedAttribute((ConfiguredAutomatedAttribute<?, ?>) contextAttribute);
+        }
+
         for (ConfiguredObjectAttribute<?, ?> attr : _attributeTypes.values())
         {
-            String attrName = attr.getName();
-            if (attr.isAutomated())
+            if (attr != contextAttribute && attr.isAutomated())
             {
-                ConfiguredAutomatedAttribute<?,?> autoAttr = (ConfiguredAutomatedAttribute<?,?>)attr;
-                if (_attributes.containsKey(attrName))
-                {
-                    automatedSetValue(attrName, _attributes.get(attrName));
-                }
-                else if (!"".equals(autoAttr.defaultValue()))
-                {
-                    automatedSetValue(attrName, autoAttr.defaultValue());
-                }
-
+                resolveAutomatedAttribute((ConfiguredAutomatedAttribute<?, ?>) attr);
             }
         }
     }
 
+    private void resolveAutomatedAttribute(final ConfiguredAutomatedAttribute<?, ?> autoAttr)
+    {
+        String attrName = autoAttr.getName();
+        if (_attributes.containsKey(attrName))
+        {
+            automatedSetValue(attrName, _attributes.get(attrName));
+        }
+        else if (!"".equals(autoAttr.defaultValue()))
+        {
+            automatedSetValue(attrName, autoAttr.defaultValue());
+        }
+    }
+
     private void attainStateIfResolved()
     {
         if(_openComplete)
@@ -1316,7 +1327,7 @@ public abstract class AbstractConfiguredObject<X extends ConfiguredObject<X>> im
     public final <T> T getContextValue(Class<T> clazz, String propertyName)
     {
         AttributeValueConverter<T> converter = AttributeValueConverter.getConverter(clazz, clazz);
-        return converter.convert("${"+propertyName+"}", this);
+        return converter.convert("${" + propertyName + "}", this);
     }
 
     private OwnAttributeResolver getOwnAttributeResolver()
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AttributeValueConverter.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AttributeValueConverter.java
index d2f90378cf..d6940655a3 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AttributeValueConverter.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AttributeValueConverter.java
@@ -204,7 +204,7 @@ abstract class AttributeValueConverter<T>
                 }
                 catch (IOException e)
                 {
-                    throw new IllegalArgumentException("Cannot convert String " + value + " to a List");
+                    throw new IllegalArgumentException("Cannot convert String " + interpolated + " to a List");
                 }
             }
             else if(value == null)
@@ -241,7 +241,7 @@ abstract class AttributeValueConverter<T>
                 }
                 catch (IOException e)
                 {
-                    throw new IllegalArgumentException("Cannot convert String " + value + " to a List");
+                    throw new IllegalArgumentException("Cannot convert String " + interpolated + " to a List");
                 }
             }
             else if(value == null)
@@ -278,7 +278,7 @@ abstract class AttributeValueConverter<T>
                 }
                 catch (IOException e)
                 {
-                    throw new IllegalArgumentException("Cannot convert String " + value + " to a Collection");
+                    throw new IllegalArgumentException("Cannot convert String " + interpolated + " to a Collection");
                 }
             }
             else if(value == null)
@@ -324,7 +324,7 @@ abstract class AttributeValueConverter<T>
                 }
                 catch (IOException e)
                 {
-                    throw new IllegalArgumentException("Cannot convert String " + value + " to a Map");
+                    throw new IllegalArgumentException("Cannot convert String " + interpolated + " to a Map");
                 }
             }
             else
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 8167f915e9..7382a20022 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
@@ -32,17 +32,16 @@ import org.apache.qpid.server.store.ConfiguredObjectRecord;
 
 public class AbstractConfiguredObjectTest extends TestCase
 {
+    private final Model _model = TestModel.getInstance();
 
-
-    public void testNonPersistAttributes()
+    public void testAttributePersistence()
     {
-        Model model = TestModel.getInstance();
-
         final String objectName = "testNonPersistAttributes";
         TestRootCategory object =
-                model.getObjectFactory().create(TestRootCategory.class,
+                _model.getObjectFactory().create(TestRootCategory.class,
                                                 Collections.<String, Object>singletonMap(ConfiguredObject.NAME,
-                                                                                         objectName));
+                                                                                         objectName)
+                                               );
 
         assertEquals(objectName, object.getName());
         assertNull(object.getAutomatedNonPersistedValue());
@@ -56,7 +55,7 @@ public class AbstractConfiguredObjectTest extends TestCase
         assertFalse(record.getAttributes().containsKey(TestRootCategory.AUTOMATED_NONPERSISTED_VALUE));
 
 
-        Map<String,Object> updatedAttributes = new HashMap<>();
+        Map<String, Object> updatedAttributes = new HashMap<>();
 
         final String newValue = "newValue";
 
@@ -74,4 +73,118 @@ public class AbstractConfiguredObjectTest extends TestCase
         assertFalse(record.getAttributes().containsKey(TestRootCategory.AUTOMATED_NONPERSISTED_VALUE));
 
     }
-}
+
+    public void testDefaultedAttributeValue()
+    {
+        final String objectName = "myName";
+
+        Map<String, Object> attributes = Collections.<String, Object>singletonMap(ConfiguredObject.NAME, objectName);
+
+        TestRootCategory object1 = _model.getObjectFactory().create(TestRootCategory.class,
+                                                                   attributes);
+
+        assertEquals(objectName, object1.getName());
+        assertEquals(TestRootCategory.DEFAULTED_VALUE_DEFAULT, object1.getDefaultedValue());
+    }
+
+    public void testOverriddenDefaultedAttributeValue()
+    {
+        final String objectName = "myName";
+
+        Map<String, Object> attributes = new HashMap<>();
+        attributes.put(ConfiguredObject.NAME, objectName);
+        attributes.put(TestRootCategory.DEFAULTED_VALUE, "override");
+
+        TestRootCategory object2 = _model.getObjectFactory().create(TestRootCategory.class,
+                                                                   attributes);
+
+        assertEquals(objectName, object2.getName());
+        assertEquals("override", object2.getDefaultedValue());
+    }
+
+    public void testStringAttributeValueFromContextVariableProvidedBySystemProperty()
+    {
+        String sysPropertyName = "testStringAttributeValueFromContextVariableProvidedBySystemProperty";
+        String contextToken = "${" + sysPropertyName + "}";
+
+        System.setProperty(sysPropertyName, "myValue");
+
+        final String objectName = "myName";
+
+        Map<String, Object> attributes = new HashMap<>();
+        attributes.put(ConfiguredObject.NAME, objectName);
+        attributes.put(TestRootCategory.STRING_VALUE, contextToken);
+
+        TestRootCategory object1 = _model.getObjectFactory().create(TestRootCategory.class,
+                                                                    attributes);
+
+        assertEquals(objectName, object1.getName());
+        assertEquals("myValue", object1.getStringValue());
+
+        // System property set empty string
+
+        System.setProperty(sysPropertyName, "");
+        TestRootCategory object2 = _model.getObjectFactory().create(TestRootCategory.class,
+                                                                    attributes);
+
+        assertEquals("", object2.getStringValue());
+
+        // System property not set
+        System.clearProperty(sysPropertyName);
+
+        TestRootCategory object3 = _model.getObjectFactory().create(TestRootCategory.class,
+                                                                    attributes);
+
+        // yields the unexpanded token - not sure if this is really useful behaviour?
+        assertEquals(contextToken, object3.getStringValue());
+    }
+
+    public void testMapAttributeValueFromContextVariableProvidedBySystemProperty()
+    {
+        String sysPropertyName = "testMapAttributeValueFromContextVariableProvidedBySystemProperty";
+        String contextToken = "${" + sysPropertyName + "}";
+
+        Map<String,String> expectedMap = new HashMap<>();
+        expectedMap.put("field1", "value1");
+        expectedMap.put("field2", "value2");
+
+        System.setProperty(sysPropertyName, "{ \"field1\" : \"value1\", \"field2\" : \"value2\"}");
+
+        final String objectName = "myName";
+
+        Map<String, Object> attributes = new HashMap<>();
+        attributes.put(ConfiguredObject.NAME, objectName);
+        attributes.put(TestRootCategory.MAP_VALUE, contextToken);
+
+        TestRootCategory object1 = _model.getObjectFactory().create(TestRootCategory.class,
+                                                                    attributes);
+
+        assertEquals(objectName, object1.getName());
+        assertEquals(expectedMap, object1.getMapValue());
+
+        // System property not set
+        System.clearProperty(sysPropertyName);
+    }
+
+    public void testStringAttributeValueFromContextVariableProvidedObjectsContext()
+    {
+        String contextToken = "${myReplacement}";
+
+        final String objectName = "myName";
+
+        Map<String, Object> attributes = new HashMap<>();
+        attributes.put(ConfiguredObject.NAME, objectName);
+        attributes.put("context", Collections.singletonMap("myReplacement", "myValue"));
+        attributes.put(TestRootCategory.STRING_VALUE, contextToken);
+
+        TestRootCategory object1 = _model.getObjectFactory().create(TestRootCategory.class,
+                                                                    attributes);
+        // Check the object's context itself
+        assertTrue(object1.getContext().containsKey("myReplacement"));
+        assertEquals("myValue", object1.getContext().get("myReplacement"));
+
+        assertEquals(objectName, object1.getName());
+        assertEquals("myValue", object1.getStringValue());
+    }
+
+}
\ No newline at end of file
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 3fce26823e..954fe4dcb1 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
@@ -20,6 +20,8 @@
  */
 package org.apache.qpid.server.model.testmodel;
 
+import java.util.Map;
+
 import org.apache.qpid.server.model.ConfiguredObject;
 import org.apache.qpid.server.model.ManagedAttribute;
 import org.apache.qpid.server.model.ManagedObject;
@@ -29,10 +31,24 @@ public interface TestRootCategory<X extends TestRootCategory<X>> extends Configu
 {
     String AUTOMATED_PERSISTED_VALUE = "automatedPersistedValue";
     String AUTOMATED_NONPERSISTED_VALUE = "automatedNonPersistedValue";
+    String DEFAULTED_VALUE = "defaultedValue";
+    String STRING_VALUE = "stringValue";
+    String MAP_VALUE = "mapValue";
 
     @ManagedAttribute
     String getAutomatedPersistedValue();
 
     @ManagedAttribute( persist = false )
     String getAutomatedNonPersistedValue();
+
+    String DEFAULTED_VALUE_DEFAULT = "myDefaultVar";
+    @ManagedAttribute( defaultValue = DEFAULTED_VALUE_DEFAULT)
+    String getDefaultedValue();
+
+    @ManagedAttribute
+    String getStringValue();
+
+    @ManagedAttribute
+    Map<String,String> getMapValue();
+
 }
diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestRootCategoryImpl.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestRootCategoryImpl.java
index 91bc00c33c..d549086686 100644
--- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestRootCategoryImpl.java
+++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestRootCategoryImpl.java
@@ -40,6 +40,15 @@ public class TestRootCategoryImpl extends AbstractConfiguredObject<TestRootCateg
     @ManagedAttributeField
     private String _automatedNonPersistedValue;
 
+    @ManagedAttributeField
+    private String _defaultedValue;
+
+    @ManagedAttributeField
+    private String _stringValue;
+
+    @ManagedAttributeField
+    private Map<String,String> _mapValue;
+
     @ManagedObjectFactoryConstructor
     public TestRootCategoryImpl(final Map<String, Object> attributes)
     {
@@ -78,6 +87,24 @@ public class TestRootCategoryImpl extends AbstractConfiguredObject<TestRootCateg
     }
 
     @Override
+    public String getDefaultedValue()
+    {
+        return _defaultedValue;
+    }
+
+    @Override
+    public String getStringValue()
+    {
+        return _stringValue;
+    }
+
+    @Override
+    public Map<String, String> getMapValue()
+    {
+        return _mapValue;
+    }
+
+    @Override
     public State getState()
     {
         return null;