qpid-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kw...@apache.org
Subject svn commit: r1661531 - in /qpid/trunk/qpid/java/broker-core/src: main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java test/java/org/apache/qpid/server/model/testmodels/singleton/AbstractConfiguredObjectTest.java
Date Sun, 22 Feb 2015 19:11:22 GMT
Author: kwall
Date: Sun Feb 22 19:11:22 2015
New Revision: 1661531

URL: http://svn.apache.org/r1661531
Log:
QPID-6406: [Java Broker] Prevent the spurious firing of the attribute listener for attribute
where no value change is made

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/testmodels/singleton/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=1661531&r1=1661530&r2=1661531&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
Sun Feb 22 19:11:22 2015
@@ -1536,8 +1536,9 @@ public abstract class AbstractConfigured
             {
                 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);

Modified: qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/singleton/AbstractConfiguredObjectTest.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/singleton/AbstractConfiguredObjectTest.java?rev=1661531&r1=1661530&r2=1661531&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/singleton/AbstractConfiguredObjectTest.java
(original)
+++ qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/singleton/AbstractConfiguredObjectTest.java
Sun Feb 22 19:11:22 2015
@@ -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 AbstractConfiguredObjectTes
 
 
     }
+
+    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)
+        {
+        }
+    }
 }



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org


Mime
View raw message