You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by kw...@apache.org on 2014/10/21 00:32:19 UTC
svn commit: r1633244 - in /qpid/trunk/qpid/java/broker-core/src:
main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java
Author: kwall
Date: Mon Oct 20 22:32:19 2014
New Revision: 1633244
URL: http://svn.apache.org/r1633244
Log:
QPID-6168: [Java Broker] Move valid value check into the create/change validation logic
* avoid the possibility that a failing many attribute update leaves an object part updated.
Modified:
qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.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=1633244&r1=1633243&r2=1633244&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 Mon Oct 20 22:32:19 2014
@@ -384,20 +384,6 @@ public abstract class AbstractConfigured
}
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)
@@ -757,6 +743,28 @@ public abstract class AbstractConfigured
public void onValidate()
{
+ for(ConfiguredObjectAttribute<?,?> attr : _attributeTypes.values())
+ {
+ if (attr.isAutomated())
+ {
+ ConfiguredAutomatedAttribute autoAttr = (ConfiguredAutomatedAttribute) attr;
+ if (autoAttr.hasValidValues())
+ {
+ Object desiredValueOrDefault = autoAttr.getValue(this);
+
+ if (desiredValueOrDefault != null && !checkValidValues(autoAttr, desiredValueOrDefault))
+ {
+ throw new IllegalConfigurationException("Attribute '" + autoAttr.getName()
+ + "' of instance of "+ getClass().getName()
+ + " named '" + getName() + "'"
+ + " cannot have value '" + desiredValueOrDefault + "'"
+ + ". Valid values are: "
+ + autoAttr.validValues());
+ }
+ }
+
+ }
+ }
}
protected void setEncrypter(final ConfigurationSecretEncrypter encrypter)
@@ -1163,6 +1171,7 @@ public abstract class AbstractConfigured
}
}
+ // TODO setAttribute does not validate. Do we need this method?
public Object setAttribute(final String name, final Object expected, final Object desired)
throws IllegalStateException, AccessControlException, IllegalArgumentException
{
@@ -1187,6 +1196,7 @@ public abstract class AbstractConfigured
});
}
+
protected boolean changeAttribute(final String name, final Object expected, final Object desired)
{
synchronized (_attributes)
@@ -1524,6 +1534,30 @@ public abstract class AbstractConfigured
{
throw new IllegalConfigurationException("Cannot change existing configured object id");
}
+
+ for(ConfiguredObjectAttribute<?,?> attr : _attributeTypes.values())
+ {
+ if (attr.isAutomated() && changedAttributes.contains(attr.getName()))
+ {
+ ConfiguredAutomatedAttribute autoAttr = (ConfiguredAutomatedAttribute) attr;
+ if (autoAttr.hasValidValues())
+ {
+ Object desiredValue = autoAttr.getValue(proxyForValidation);
+
+ if (!checkValidValues(autoAttr, desiredValue))
+ {
+ throw new IllegalConfigurationException("Attribute '" + autoAttr.getName()
+ + "' of instance of "+ getClass().getName()
+ + " named '" + getName() + "'"
+ + " cannot have value '" + desiredValue + "'"
+ + ". Valid values are: "
+ + autoAttr.validValues());
+ }
+ }
+
+ }
+ }
+
}
private ConfiguredObject<?> createProxyForValidation(final Map<String, Object> attributes)
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=1633244&r1=1633243&r2=1633244&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 Mon Oct 20 22:32:19 2014
@@ -539,7 +539,7 @@ public class AbstractConfiguredObjectTes
try
{
- object.setAttribute(TestRootCategory.VALID_VALUE, TestRootCategory.VALID_VALUE2, "illegal");
+ object.setAttributes(Collections.singletonMap(TestRootCategory.VALID_VALUE, "illegal"));
fail("Exception not thrown");
}
catch (IllegalConfigurationException iae)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org