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

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<X extends ConfiguredObject<X>> 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<String, Object> 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<String, String> 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)
+        {
+        }
+    }
 }