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: [email protected]
For additional commands, e-mail: [email protected]
