You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by rg...@apache.org on 2014/08/21 22:33:43 UTC
svn commit: r1619568 - in /qpid/trunk/qpid/java/broker-core/src:
main/java/org/apache/qpid/server/model/
test/java/org/apache/qpid/server/model/
test/java/org/apache/qpid/server/model/testmodel/
test/java/org/apache/qpid/server/security/auth/manager/
Author: rgodfrey
Date: Thu Aug 21 20:33:43 2014
New Revision: 1619568
URL: http://svn.apache.org/r1619568
Log:
QPID-6019 : [Java Broker] Move configured object registration with parents until after resolution, and provide clearer error messages on attribute resolution failure
Modified:
qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AttributeValueConverter.java
qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ConfiguredObjectAttribute.java
qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java
qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestChildCategory.java
qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestChildCategoryImpl.java
qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManagerFactoryTest.java
Modified: qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java?rev=1619568&r1=1619567&r2=1619568&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java (original)
+++ qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java Thu Aug 21 20:33:43 2014
@@ -469,7 +469,6 @@ public abstract class AbstractConfigured
{
if(_dynamicState.compareAndSet(DynamicState.UNINIT, DynamicState.OPENED))
{
- registerWithParents();
final AuthenticatedPrincipal currentUser = SecurityManager.getCurrentUser();
if(currentUser != null)
{
@@ -487,6 +486,9 @@ public abstract class AbstractConfigured
doResolution(true);
doValidation(true);
+
+ registerWithParents();
+
doCreation(true);
doOpening(true);
doAttainState();
Modified: qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AttributeValueConverter.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AttributeValueConverter.java?rev=1619568&r1=1619567&r2=1619568&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AttributeValueConverter.java (original)
+++ qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AttributeValueConverter.java Thu Aug 21 20:33:43 2014
@@ -89,7 +89,15 @@ abstract class AttributeValueConverter<T
}
else if(value instanceof String)
{
- return Long.valueOf(AbstractConfiguredObject.interpolate(object, (String) value));
+ String interpolated = AbstractConfiguredObject.interpolate(object, (String) value);
+ try
+ {
+ return Long.valueOf(interpolated);
+ }
+ catch(NumberFormatException e)
+ {
+ throw new IllegalArgumentException("Cannot convert string '" + interpolated + "'",e);
+ }
}
else if(value == null)
{
@@ -117,7 +125,15 @@ abstract class AttributeValueConverter<T
}
else if(value instanceof String)
{
- return Integer.valueOf(AbstractConfiguredObject.interpolate(object, (String) value));
+ String interpolated = AbstractConfiguredObject.interpolate(object, (String) value);
+ try
+ {
+ return Integer.valueOf(interpolated);
+ }
+ catch(NumberFormatException e)
+ {
+ throw new IllegalArgumentException("Cannot convert string '" + interpolated + "'",e);
+ }
}
else if(value == null)
{
@@ -145,7 +161,15 @@ abstract class AttributeValueConverter<T
}
else if(value instanceof String)
{
- return Short.valueOf(AbstractConfiguredObject.interpolate(object, (String) value));
+ String interpolated = AbstractConfiguredObject.interpolate(object, (String) value);
+ try
+ {
+ return Short.valueOf(interpolated);
+ }
+ catch(NumberFormatException e)
+ {
+ throw new IllegalArgumentException("Cannot convert string '" + interpolated + "'",e);
+ }
}
else if(value == null)
{
Modified: qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ConfiguredObjectAttribute.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ConfiguredObjectAttribute.java?rev=1619568&r1=1619567&r2=1619568&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ConfiguredObjectAttribute.java (original)
+++ qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ConfiguredObjectAttribute.java Thu Aug 21 20:33:43 2014
@@ -21,11 +21,10 @@
package org.apache.qpid.server.model;
import java.lang.reflect.Method;
+import java.lang.reflect.Type;
public abstract class ConfiguredObjectAttribute<C extends ConfiguredObject, T> extends ConfiguredObjectAttributeOrStatistic<C,T>
{
-
-
ConfiguredObjectAttribute(Class<C> clazz,
final Method getter)
{
@@ -48,6 +47,20 @@ public abstract class ConfiguredObjectAt
public T convert(final Object value, C object)
{
- return getConverter().convert(value, object);
+ final AttributeValueConverter<T> converter = getConverter();
+ try
+ {
+ return converter.convert(value, object);
+ }
+ catch (IllegalArgumentException iae)
+ {
+ Type returnType = getGetter().getGenericReturnType();
+ String simpleName = returnType instanceof Class ? ((Class) returnType).getSimpleName() : returnType.toString();
+
+ throw new IllegalArgumentException("Cannot convert '" + value
+ + "' into a " + simpleName
+ + " for attribute " + getName()
+ + " (" + iae.getMessage() + ")", iae);
+ }
}
}
Modified: qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java?rev=1619568&r1=1619567&r2=1619568&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java (original)
+++ qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java Thu Aug 21 20:33:43 2014
@@ -26,6 +26,7 @@ import java.util.Map;
import junit.framework.TestCase;
+import org.apache.qpid.server.model.testmodel.TestChildCategory;
import org.apache.qpid.server.model.testmodel.TestModel;
import org.apache.qpid.server.model.testmodel.TestRootCategory;
import org.apache.qpid.server.store.ConfiguredObjectRecord;
@@ -187,4 +188,73 @@ public class AbstractConfiguredObjectTes
assertEquals("myValue", object1.getStringValue());
}
-}
\ No newline at end of file
+ public void testCreationOfObjectWithInvalidInterpolatedValues()
+ {
+ final String parentName = "parent";
+ TestRootCategory parent =
+ _model.getObjectFactory().create(TestRootCategory.class,
+ Collections.<String, Object>singletonMap(ConfiguredObject.NAME,
+ parentName)
+ );
+
+ parent.setAttributes(Collections.singletonMap(ConfiguredObject.CONTEXT,
+ Collections.singletonMap("contextVal", "foo")));
+
+ final Map<String, Object> attributes = new HashMap<>();
+ attributes.put("intValue", "${contextVal}");
+ attributes.put("name", "child");
+ attributes.put("integerSet", "[ ]");
+ attributes.put(ConfiguredObject.TYPE, "test");
+
+ try
+ {
+ _model.getObjectFactory().create(TestChildCategory.class, attributes, parent);
+ fail("creation of child object should have failed due to invalid value");
+ }
+ catch (IllegalArgumentException e)
+ {
+ // PASS
+ String message = e.getMessage();
+ assertTrue("Message does not contain the attribute name", message.contains("intValue"));
+ assertTrue("Message does not contain the non-interpolated value", message.contains("contextVal"));
+ assertTrue("Message does not contain the interpolated value", message.contains("foo"));
+
+ }
+
+ assertTrue("Child should not have been registered with parent",
+ parent.getChildren(TestChildCategory.class).isEmpty());
+ }
+
+ public void testCreationOfObjectWithInvalidDefaultValues()
+ {
+ final String parentName = "parent";
+ TestRootCategory parent =
+ _model.getObjectFactory().create(TestRootCategory.class,
+ Collections.<String, Object>singletonMap(ConfiguredObject.NAME,
+ parentName)
+ );
+
+ final Map<String, Object> attributes = new HashMap<>();
+ attributes.put("intValue", "1");
+ attributes.put("name", "child");
+ attributes.put(ConfiguredObject.TYPE, "test");
+
+ try
+ {
+ _model.getObjectFactory().create(TestChildCategory.class, attributes, parent);
+ fail("creation of child object should have failed due to invalid value");
+ }
+ catch (IllegalArgumentException e)
+ {
+ // PASS
+ String message = e.getMessage();
+ assertTrue("Message does not contain the attribute name", message.contains("integerSet"));
+ assertTrue("Message does not contain the error value", message.contains("foo"));
+
+ }
+
+ assertTrue("Child should not have been registered with parent",
+ parent.getChildren(TestChildCategory.class).isEmpty());
+ }
+
+}
Modified: qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestChildCategory.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestChildCategory.java?rev=1619568&r1=1619567&r2=1619568&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestChildCategory.java (original)
+++ qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestChildCategory.java Thu Aug 21 20:33:43 2014
@@ -20,6 +20,8 @@
*/
package org.apache.qpid.server.model.testmodel;
+import java.util.Set;
+
import org.apache.qpid.server.model.ConfiguredObject;
import org.apache.qpid.server.model.ManagedAttribute;
import org.apache.qpid.server.model.ManagedObject;
@@ -30,6 +32,12 @@ public interface TestChildCategory<X ext
String NON_INTERPOLATED_VALID_VALUE = "${file.separator}";
- @ManagedAttribute(validValues = { NON_INTERPOLATED_VALID_VALUE })
+ @ManagedAttribute(validValues = { NON_INTERPOLATED_VALID_VALUE }, defaultValue = "")
String getValidValueNotInterpolated();
+
+ @ManagedAttribute( defaultValue = "3" )
+ int getIntValue();
+
+ @ManagedAttribute( defaultValue = "[ \"1\", \"2\", \"foo\" ]" )
+ Set<Integer> getIntegerSet();
}
Modified: qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestChildCategoryImpl.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestChildCategoryImpl.java?rev=1619568&r1=1619567&r2=1619568&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestChildCategoryImpl.java (original)
+++ qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestChildCategoryImpl.java Thu Aug 21 20:33:43 2014
@@ -21,6 +21,7 @@
package org.apache.qpid.server.model.testmodel;
import java.util.Map;
+import java.util.Set;
import org.apache.qpid.server.model.AbstractConfiguredObject;
import org.apache.qpid.server.model.ManagedAttributeField;
@@ -37,6 +38,12 @@ public class TestChildCategoryImpl
@ManagedAttributeField
private String _validValueNotInterpolated;
+ @ManagedAttributeField
+ private int _intValue;
+
+ @ManagedAttributeField
+ private Set<Integer> _integerSet;
+
@ManagedObjectFactoryConstructor
public TestChildCategoryImpl(final Map<String, Object> attributes, TestRootCategory<?> parent)
@@ -57,4 +64,16 @@ public class TestChildCategoryImpl
{
return _validValueNotInterpolated;
}
+
+ @Override
+ public int getIntValue()
+ {
+ return _intValue;
+ }
+
+ @Override
+ public Set<Integer> getIntegerSet()
+ {
+ return _integerSet;
+ }
}
Modified: qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManagerFactoryTest.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManagerFactoryTest.java?rev=1619568&r1=1619567&r2=1619568&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManagerFactoryTest.java (original)
+++ qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManagerFactoryTest.java Thu Aug 21 20:33:43 2014
@@ -36,7 +36,6 @@ import org.apache.qpid.server.model.Brok
import org.apache.qpid.server.model.BrokerModel;
import org.apache.qpid.server.model.ConfiguredObjectFactory;
import org.apache.qpid.server.model.TrustStore;
-import org.apache.qpid.server.model.UnknownConfiguredObjectException;
import org.apache.qpid.server.util.BrokerTestHelper;
public class SimpleLDAPAuthenticationManagerFactoryTest extends TestCase
@@ -108,10 +107,12 @@ public class SimpleLDAPAuthenticationMan
_factory.create(AuthenticationProvider.class, _configuration, _broker);
fail("Exception not thrown");
}
- catch(UnknownConfiguredObjectException e)
+ catch(IllegalArgumentException e)
{
- assertEquals(e.getCategory(), TrustStore.class);
- assertEquals(e.getName(), "notfound");
+ // PASS
+ assertTrue("Message does not include underlying issue", e.getMessage().contains("name 'notfound'"));
+ assertTrue("Message does not include the attribute name", e.getMessage().contains("trustStore"));
+ assertTrue("Message does not include the expected type", e.getMessage().contains("TrustStore"));
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org