From 1d6ec9abfaf5f2c72500eddc7739e48fd4794fb3 Mon Sep 17 00:00:00 2001 From: Keith Wall Date: Tue, 24 Feb 2015 13:34:32 +0000 Subject: QPID-6406: [Java Broker] ACO generates attribute set events even if the attribute value is not changed Merged from trunk with command: svn merge -c 1661531 https://svn.apache.org/repos/asf/qpid/trunk git-svn-id: https://svn.apache.org/repos/asf/qpid/branches/0.32@1661944 13f79535-47bb-0310-9956-ffa450edef68 --- .../server/model/AbstractConfiguredObject.java | 5 +- .../singleton/AbstractConfiguredObjectTest.java | 84 ++++++++++++++++++++++ 2 files changed, 87 insertions(+), 2 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 74982acb4b..05f980baca 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 @@ -1536,8 +1536,9 @@ public abstract class AbstractConfiguredObject> im { Object desired = attributes.get(name); Object expected = getAttribute(name); - if(((_attributes.get(name) != null && !_attributes.get(name).equals(attributes.get(name))) - || attributes.get(name) != null) + Object currentValue = _attributes.get(name); + if(((currentValue != null && !currentValue.equals(desired)) + || (currentValue == null && desired != null)) && changeAttribute(name, expected, desired)) { attributeSet(name, expected, desired); diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/singleton/AbstractConfiguredObjectTest.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/singleton/AbstractConfiguredObjectTest.java index 46f205116a..081fbe4edf 100644 --- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/singleton/AbstractConfiguredObjectTest.java +++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/singleton/AbstractConfiguredObjectTest.java @@ -22,14 +22,18 @@ import java.security.PrivilegedAction; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; import javax.security.auth.Subject; import org.apache.qpid.server.configuration.IllegalConfigurationException; import org.apache.qpid.server.model.AbstractConfiguredObject; +import org.apache.qpid.server.model.ConfigurationChangeListener; import org.apache.qpid.server.model.ConfiguredObject; import org.apache.qpid.server.model.Model; +import org.apache.qpid.server.model.State; import org.apache.qpid.server.store.ConfiguredObjectRecord; import org.apache.qpid.test.utils.QpidTestCase; @@ -476,4 +480,84 @@ public class AbstractConfiguredObjectTest extends QpidTestCase } + + public void testAttributeSetListenerFiring() + { + final String objectName = "listenerFiring"; + + Map attributes = new HashMap<>(); + attributes.put(ConfiguredObject.NAME, objectName); + attributes.put(TestSingleton.STRING_VALUE, "first"); + + final TestSingleton object = _model.getObjectFactory().create(TestSingleton.class, attributes); + + final AtomicInteger listenerCount = new AtomicInteger(); + final LinkedHashMap updates = new LinkedHashMap<>(); + object.addChangeListener(new NoopConfigurationChangeListener() + { + @Override + public void attributeSet(final ConfiguredObject object, + final String attributeName, + final Object oldAttributeValue, + final Object newAttributeValue) + { + listenerCount.incrementAndGet(); + String delta = String.valueOf(oldAttributeValue) + "=>" + String.valueOf(newAttributeValue); + updates.put(attributeName, delta); + } + }); + + // Set updated value (should cause listener to fire) + object.setAttributes(Collections.singletonMap(TestSingleton.STRING_VALUE, "second")); + + assertEquals(1, listenerCount.get()); + String delta = updates.remove(TestSingleton.STRING_VALUE); + assertEquals("first=>second", delta); + + // Set unchanged value (should not cause listener to fire) + object.setAttributes(Collections.singletonMap(TestSingleton.STRING_VALUE, "second")); + assertEquals(1, listenerCount.get()); + + // Set value to null (should cause listener to fire) + object.setAttributes(Collections.singletonMap(TestSingleton.STRING_VALUE, null)); + assertEquals(2, listenerCount.get()); + delta = updates.remove(TestSingleton.STRING_VALUE); + assertEquals("second=>null", delta); + + // Set to null again (should not cause listener to fire) + object.setAttributes(Collections.singletonMap(TestSingleton.STRING_VALUE, null)); + assertEquals(2, listenerCount.get()); + + // Set updated value (should cause listener to fire) + object.setAttributes(Collections.singletonMap(TestSingleton.STRING_VALUE, "third")); + assertEquals(3, listenerCount.get()); + delta = updates.remove(TestSingleton.STRING_VALUE); + assertEquals("null=>third", delta); + } + + private static class NoopConfigurationChangeListener implements ConfigurationChangeListener + { + @Override + public void stateChanged(final ConfiguredObject object, final State oldState, final State newState) + { + } + + @Override + public void childAdded(final ConfiguredObject object, final ConfiguredObject child) + { + } + + @Override + public void childRemoved(final ConfiguredObject object, final ConfiguredObject child) + { + } + + @Override + public void attributeSet(final ConfiguredObject object, + final String attributeName, + final Object oldAttributeValue, + final Object newAttributeValue) + { + } + } } -- cgit v1.2.1