UpdatingMap: fix synchronisation

Delegate to ServiceStateLogic.updateMapSensorEntry, to use a
`modify` operation.

Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/b91f03b7
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/b91f03b7
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/b91f03b7

Branch: refs/heads/master
Commit: b91f03b71283e751250beec7ec5e11cdf9f240c1
Parents: af44b6d
Author: Aled Sage <[email protected]>
Authored: Thu Aug 3 15:53:23 2017 +0100
Committer: Aled Sage <[email protected]>
Committed: Thu Aug 3 15:53:45 2017 +0100

----------------------------------------------------------------------
 .../brooklyn/enricher/stock/UpdatingMap.java    | 41 ++++-------
 .../enricher/stock/UpdatingMapTest.java         | 71 ++++++++++++++++++--
 2 files changed, 79 insertions(+), 33 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/b91f03b7/core/src/main/java/org/apache/brooklyn/enricher/stock/UpdatingMap.java
----------------------------------------------------------------------
diff --git 
a/core/src/main/java/org/apache/brooklyn/enricher/stock/UpdatingMap.java 
b/core/src/main/java/org/apache/brooklyn/enricher/stock/UpdatingMap.java
index c74423e..acd4dbd 100644
--- a/core/src/main/java/org/apache/brooklyn/enricher/stock/UpdatingMap.java
+++ b/core/src/main/java/org/apache/brooklyn/enricher/stock/UpdatingMap.java
@@ -31,7 +31,7 @@ import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.core.config.ConfigKeys;
 import org.apache.brooklyn.core.enricher.AbstractEnricher;
 import org.apache.brooklyn.core.entity.Entities;
-import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.core.entity.lifecycle.ServiceStateLogic;
 import org.apache.brooklyn.util.core.flags.SetFromFlag;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.slf4j.Logger;
@@ -70,16 +70,24 @@ public class UpdatingMap<S,TKey,TVal> extends 
AbstractEnricher implements Sensor
 
     @SetFromFlag("fromSensor")
     public static final ConfigKey<Sensor<?>> SOURCE_SENSOR = 
ConfigKeys.newConfigKey(new TypeToken<Sensor<?>>() {}, "enricher.sourceSensor");
+    
     @SetFromFlag("targetSensor")
     public static final ConfigKey<Sensor<?>> TARGET_SENSOR = 
ConfigKeys.newConfigKey(new TypeToken<Sensor<?>>() {}, "enricher.targetSensor");
+    
     @SetFromFlag("key")
     public static final ConfigKey<Object> KEY_IN_TARGET_SENSOR = 
ConfigKeys.newConfigKey(Object.class, "enricher.updatingMap.keyInTargetSensor",
         "Key to update in the target sensor map, defaulting to the name of the 
source sensor");
+
     @SetFromFlag("computing")
-    public static final ConfigKey<Function<?, ?>> COMPUTING = 
ConfigKeys.newConfigKey(new TypeToken<Function<?,?>>() {}, 
"enricher.updatingMap.computing");
+    public static final ConfigKey<Function<?, ?>> COMPUTING = 
ConfigKeys.newConfigKey(
+            new TypeToken<Function<?,?>>() {}, 
+            "enricher.updatingMap.computing");
+    
     @SetFromFlag("removingIfResultIsNull")
-    public static final ConfigKey<Boolean> REMOVING_IF_RESULT_IS_NULL = 
ConfigKeys.newBooleanConfigKey("enricher.updatingMap.removingIfResultIsNull", 
-        "Whether the key in the target map is removed if the result if the 
computation is null");
+    public static final ConfigKey<Boolean> REMOVING_IF_RESULT_IS_NULL = 
ConfigKeys.newBooleanConfigKey(
+            "enricher.updatingMap.removingIfResultIsNull", 
+            "Whether the key in the target map is removed if the result if the 
computation is null",
+            Boolean.TRUE);
 
     protected Entity producer;
     protected AttributeSensor<S> sourceSensor;
@@ -139,37 +147,16 @@ public class UpdatingMap<S,TKey,TVal> extends 
AbstractEnricher implements Sensor
     protected void onUpdated() {
         try {
             Object v = computing.apply(producer.getAttribute(sourceSensor));
-            if (v == null && !Boolean.FALSE.equals(removingIfResultIsNull)) {
+            if (v == null && Boolean.TRUE.equals(removingIfResultIsNull)) {
                 v = Entities.REMOVE;
             }
             if (v == Entities.UNCHANGED) {
                 // nothing
             } else {
-                // TODO check synchronization
                 TKey key = this.key;
                 if (key==null) key = (TKey) sourceSensor.getName();
-                
-                Map<TKey, TVal> map = entity.getAttribute(targetSensor);
 
-                boolean created = (map==null);
-                if (created) map = MutableMap.of();
-                
-                boolean changed;
-                if (v == Entities.REMOVE) {
-                    changed = map.containsKey(key);
-                    if (changed)
-                        map.remove(key);
-                } else {
-                    TVal oldV = map.get(key);
-                    if (oldV==null)
-                        changed = (v!=null || !map.containsKey(key));
-                    else
-                        changed = !oldV.equals(v);
-                    if (changed)
-                        map.put(key, (TVal)v);
-                }
-                if (changed || created)
-                    emit(targetSensor, map);
+                ServiceStateLogic.updateMapSensorEntry(entity, targetSensor, 
key, (TVal) v);
             }
         } catch (Throwable t) {
             LOG.warn("Error calculating map update for enricher "+this, t);

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/b91f03b7/core/src/test/java/org/apache/brooklyn/enricher/stock/UpdatingMapTest.java
----------------------------------------------------------------------
diff --git 
a/core/src/test/java/org/apache/brooklyn/enricher/stock/UpdatingMapTest.java 
b/core/src/test/java/org/apache/brooklyn/enricher/stock/UpdatingMapTest.java
index 801d4fa..c0e36e1 100644
--- a/core/src/test/java/org/apache/brooklyn/enricher/stock/UpdatingMapTest.java
+++ b/core/src/test/java/org/apache/brooklyn/enricher/stock/UpdatingMapTest.java
@@ -31,6 +31,7 @@ import org.apache.brooklyn.api.sensor.AttributeSensor;
 import org.apache.brooklyn.api.sensor.EnricherSpec;
 import org.apache.brooklyn.core.entity.Attributes;
 import org.apache.brooklyn.core.entity.EntityAsserts;
+import org.apache.brooklyn.core.entity.RecordingSensorEventListener;
 import org.apache.brooklyn.core.entity.lifecycle.Lifecycle;
 import org.apache.brooklyn.core.entity.lifecycle.ServiceStateLogic;
 import org.apache.brooklyn.core.sensor.Sensors;
@@ -42,12 +43,21 @@ import org.apache.brooklyn.util.collections.MutableMap;
 import org.testng.annotations.Test;
 
 import com.google.common.base.Functions;
+import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
 import com.google.common.reflect.TypeToken;
 
 public class UpdatingMapTest extends BrooklynAppUnitTestSupport {
 
+    protected AttributeSensor<Object> mySensor = 
Sensors.newSensor(Object.class, "mySensor");
+    
+    @SuppressWarnings("serial")
+    protected AttributeSensor<Map<String,Object>> mapSensor = 
Sensors.newSensor(
+            new TypeToken<Map<String,Object>>() {},
+            "mapSensor");
+
     @Test
     public void testUpdateServiceNotUpIndicator() throws Exception {
         AttributeSensor<Object> extraIsUpSensor = 
Sensors.newSensor(Object.class, "extraIsUp");
@@ -78,13 +88,7 @@ public class UpdatingMapTest extends 
BrooklynAppUnitTestSupport {
     }
     
     @Test
-    @SuppressWarnings("serial")
     public void testUpdateMapUsingDifferentProducer() throws Exception {
-        AttributeSensor<Object> mySensor = Sensors.newSensor(Object.class, 
"mySensor");
-        AttributeSensor<Map<String,Object>> mapSensor = Sensors.newSensor(
-                new TypeToken<Map<String,Object>>() {},
-                "mapSensor");
-        
         Entity producer = 
app.createAndManageChild(EntitySpec.create(TestEntity.class));
 
         Entity entity = 
app.createAndManageChild(EntitySpec.create(BasicApplication.class)
@@ -101,6 +105,61 @@ public class UpdatingMapTest extends 
BrooklynAppUnitTestSupport {
         EntityAsserts.assertAttributeEqualsEventually(entity, mapSensor, 
ImmutableMap.of("myKey", "valIsV1"));
     }
     
+    @Test
+    public void testUpdateMapEmitsEventOnChange() throws Exception {
+        Entity entity = 
app.createAndManageChild(EntitySpec.create(BasicApplication.class)
+                .enricher(EnricherSpec.create(UpdatingMap.class)
+                        .configure(UpdatingMap.SOURCE_SENSOR.getName(), 
mySensor.getName())
+                        .configure(UpdatingMap.TARGET_SENSOR, mapSensor)
+                        .configure(UpdatingMap.KEY_IN_TARGET_SENSOR, "myKey")
+                        .configure(UpdatingMap.COMPUTING, 
Functions.forMap(MutableMap.of("v1", "valIsV1", "v2", "valIsV2"), 
"myDefault"))));
+
+        EntityAsserts.assertAttributeEqualsEventually(entity, mapSensor, 
ImmutableMap.of("myKey", "myDefault"));
+        
+        RecordingSensorEventListener<Map<String, Object>> listener = new 
RecordingSensorEventListener<>();
+        app.subscriptions().subscribe(entity, mapSensor, listener);
+        
+        entity.sensors().set(mySensor, "v1");
+        EntityAsserts.assertAttributeEqualsEventually(entity, mapSensor, 
ImmutableMap.of("myKey", "valIsV1"));
+        
+        listener.assertHasEventEventually(Predicates.alwaysTrue());
+        assertEquals(Iterables.getOnlyElement(listener.getEventValues()), 
ImmutableMap.of("myKey", "valIsV1"));
+    }
+    
+    @Test
+    public void testRemovingIfResultIsNull() throws Exception {
+        Entity entity = 
app.createAndManageChild(EntitySpec.create(BasicApplication.class)
+                .enricher(EnricherSpec.create(UpdatingMap.class)
+                        .configure(UpdatingMap.SOURCE_SENSOR.getName(), 
mySensor.getName())
+                        .configure(UpdatingMap.TARGET_SENSOR, mapSensor)
+                        .configure(UpdatingMap.REMOVING_IF_RESULT_IS_NULL, 
true)
+                        .configure(UpdatingMap.COMPUTING, 
Functions.forMap(MutableMap.of("v1", "valIsV1"), null))));
+
+        
+        entity.sensors().set(mySensor, "v1");
+        EntityAsserts.assertAttributeEqualsEventually(entity, mapSensor, 
ImmutableMap.of("mySensor", "valIsV1"));
+
+        entity.sensors().set(mySensor, "different");
+        EntityAsserts.assertAttributeEqualsEventually(entity, mapSensor, 
ImmutableMap.of());
+    }
+
+    @Test
+    public void testNotRemovingIfResultIsNull() throws Exception {
+        Entity entity = 
app.createAndManageChild(EntitySpec.create(BasicApplication.class)
+                .enricher(EnricherSpec.create(UpdatingMap.class)
+                        .configure(UpdatingMap.SOURCE_SENSOR.getName(), 
mySensor.getName())
+                        .configure(UpdatingMap.TARGET_SENSOR, mapSensor)
+                        .configure(UpdatingMap.REMOVING_IF_RESULT_IS_NULL, 
false)
+                        .configure(UpdatingMap.COMPUTING, 
Functions.forMap(MutableMap.of("v1", "valIsV1"), null))));
+
+        
+        entity.sensors().set(mySensor, "v1");
+        EntityAsserts.assertAttributeEqualsEventually(entity, mapSensor, 
ImmutableMap.of("mySensor", "valIsV1"));
+
+        entity.sensors().set(mySensor, "different");
+        EntityAsserts.assertAttributeEqualsEventually(entity, mapSensor, 
MutableMap.of("mySensor", null));
+    }
+    
     private void assertMapSensorContainsEventually(Entity entity, 
AttributeSensor<? extends Map<?, ?>> mapSensor, Map<?, ?> expected) {
         Asserts.succeedsEventually(new Runnable() {
             @Override public void run() {

Reply via email to