Added newValue and oldValue methods to MapEvent.

Change-Id: Ibaffc8079de03b1f4623044ec53c949831ea8cd1

core/api/src/main/java/org/onosproject/store/primitives/ConsistentMapBackedJavaMap.java

@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.onosproject.store.primitives.impl;
+package org.onosproject.store.primitives;
 
 import java.util.Collection;
 import java.util.Map;

core/api/src/main/java/org/onosproject/store/service/MapEvent.java

@@ -50,21 +50,24 @@ public class MapEvent<K, V> {
     private final String name;
     private final Type type;
     private final K key;
-    private final Versioned<V> value;
+    private final Versioned<V> newValue;
+    private final Versioned<V> oldValue;
 
     /**
      * Creates a new event object.
      *
      * @param name map name
-     * @param type type of event
      * @param key key the event concerns
-     * @param value value key is mapped to
+     * @param currentValue new value key is mapped to
+     * @param previousValue value that was replaced
      */
-    public MapEvent(String name, Type type, K key, Versioned<V> value) {
+    public MapEvent(String name, K key, Versioned<V> currentValue, Versioned<V> previousValue) {
         this.name = name;
-        this.type = type;
         this.key = key;
-        this.value = value;
+        this.newValue = currentValue;
+        this.oldValue = previousValue;
+        this.type = currentValue != null ?
+                    previousValue != null ? Type.UPDATE : Type.INSERT : Type.REMOVE;
     }
 
     /**
@@ -100,9 +103,30 @@ public class MapEvent<K, V> {
      * the new value.
      *
      * @return the value
+     * @deprecated use {@link #newValue()} or {@link #oldValue()} instead.
      */
+    @Deprecated
     public Versioned<V> value() {
-        return value;
+        return type == Type.REMOVE ? oldValue() : newValue();
+    }
+
+    /**
+     * Returns the new value in the map associated with the key. If {@link #type()} returns {@code REMOVE},
+     * this method will return {@code null}.
+     *
+     * @return the new value for key
+     */
+    public Versioned<V> newValue() {
+        return newValue;
+    }
+
+    /**
+     * Returns the value associated with the key, before it was updated.
+     *
+     * @return previous value in map for the key
+     */
+    public Versioned<V> oldValue() {
+        return oldValue;
     }
 
     @Override
@@ -115,12 +139,13 @@ public class MapEvent<K, V> {
         return Objects.equals(this.name, that.name) &&
                 Objects.equals(this.type, that.type) &&
                 Objects.equals(this.key, that.key) &&
-                Objects.equals(this.value, that.value);
+                Objects.equals(this.newValue, that.newValue) &&
+                Objects.equals(this.oldValue, that.oldValue);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(name, type, key, value);
+        return Objects.hash(name, type, key, newValue, oldValue);
     }
 
     @Override
@@ -129,7 +154,8 @@ public class MapEvent<K, V> {
                 .add("name", name)
                 .add("type", type)
                 .add("key", key)
-                .add("value", value)
+                .add("newValue", newValue)
+                .add("oldValue", oldValue)
                 .toString();
     }
 }

core/api/src/test/java/org/onosproject/store/service/MapEventTest.java

@@ -16,6 +16,7 @@
 package org.onosproject.store.service;
 
 import com.google.common.testing.EqualsTester;
+
 import org.junit.Test;
 
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -26,13 +27,14 @@ import static org.hamcrest.Matchers.is;
  */
 public class MapEventTest {
 
-    private final Versioned<Integer> vStats = new Versioned<>(2, 1);
+    private final Versioned<Integer> vStatsNew = new Versioned<>(2, 2);
+    private final Versioned<Integer> vStatsOld = new Versioned<>(1, 1);
 
-    private final MapEvent<String, Integer> stats1 = new MapEvent<>("a", MapEvent.Type.INSERT, "1", vStats);
+    private final MapEvent<String, Integer> stats1 = new MapEvent<>("a", "1", vStatsNew, null);
 
-    private final MapEvent<String, Integer> stats2 = new MapEvent<>("a", MapEvent.Type.REMOVE, "1", vStats);
+    private final MapEvent<String, Integer> stats2 = new MapEvent<>("a", "1", null, vStatsOld);
 
-    private final MapEvent<String, Integer> stats3 = new MapEvent<>("a", MapEvent.Type.UPDATE, "1", vStats);
+    private final MapEvent<String, Integer> stats3 = new MapEvent<>("a", "1", vStatsNew, vStatsOld);
 
     /**
      * Tests the creation of the MapEvent object.
@@ -42,7 +44,23 @@ public class MapEventTest {
         assertThat(stats1.name(), is("a"));
         assertThat(stats1.type(), is(MapEvent.Type.INSERT));
         assertThat(stats1.key(), is("1"));
-        assertThat(stats1.value(), is(vStats));
+        assertThat(stats1.value(), is(vStatsNew));
+        assertThat(stats1.newValue(), is(vStatsNew));
+        assertThat(stats1.oldValue(), is((Versioned<Integer>) null));
+
+        assertThat(stats2.name(), is("a"));
+        assertThat(stats2.type(), is(MapEvent.Type.REMOVE));
+        assertThat(stats2.key(), is("1"));
+        assertThat(stats2.value(), is(vStatsOld));
+        assertThat(stats2.newValue(), is((Versioned<Integer>) null));
+        assertThat(stats2.oldValue(), is(vStatsOld));
+
+        assertThat(stats3.name(), is("a"));
+        assertThat(stats3.type(), is(MapEvent.Type.UPDATE));
+        assertThat(stats3.key(), is("1"));
+        assertThat(stats3.value(), is(vStatsNew));
+        assertThat(stats3.newValue(), is(vStatsNew));
+        assertThat(stats3.oldValue(), is(vStatsOld));
     }
 
     /**

core/api/src/test/java/org/onosproject/store/service/TestConsistentMap.java

@@ -21,15 +21,18 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.BiFunction;
 import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
 import org.onosproject.core.ApplicationId;
+import org.onosproject.store.primitives.ConsistentMapBackedJavaMap;
 
-import static org.onosproject.store.service.MapEvent.Type.*;
+import com.google.common.base.Objects;
 
 /**
  * Test implementation of the consistent map.
@@ -37,7 +40,7 @@ import static org.onosproject.store.service.MapEvent.Type.*;
 public final class TestConsistentMap<K, V> extends ConsistentMapAdapter<K, V> {
 
     private final List<MapEventListener<K, V>> listeners;
-    private final Map<K, V> map;
+    private final Map<K, Versioned<V>> map;
     private final String mapName;
     private final AtomicLong counter = new AtomicLong(0);
 
@@ -54,9 +57,9 @@ public final class TestConsistentMap<K, V> extends ConsistentMapAdapter<K, V> {
     /**
      * Notify all listeners of an event.
      */
-    private void notifyListeners(String mapName, MapEvent.Type type,
+    private void notifyListeners(String mapName,
-                                 K key, Versioned<V> value) {
+                                 K key, Versioned<V> newvalue, Versioned<V> oldValue) {
-        MapEvent<K, V> event = new MapEvent<>(mapName, type, key, value);
+        MapEvent<K, V> event = new MapEvent<>(mapName, key, newvalue, oldValue);
         listeners.forEach(
                 listener -> listener.event(event)
         );
@@ -84,71 +87,103 @@ public final class TestConsistentMap<K, V> extends ConsistentMapAdapter<K, V> {
 
     @Override
     public Versioned<V> get(K key) {
-        V value = map.get(key);
+        return map.get(key);
-        if (value != null) {
-            return version(value);
-        } else {
-            return null;
-        }
     }
 
     @Override
     public Versioned<V> computeIfAbsent(K key, Function<? super K, ? extends V> mappingFunction) {
-        Versioned<V> result = version(map.computeIfAbsent(key, mappingFunction));
+        AtomicBoolean updated = new AtomicBoolean(false);
-        notifyListeners(mapName, INSERT, key, result);
+        Versioned<V> result = map.compute(key, (k, v) -> {
+            if (v == null) {
+                updated.set(true);
+                return version(mappingFunction.apply(key));
+            }
+            return v;
+        });
+        if (updated.get()) {
+            notifyListeners(mapName, key, result, null);
+        }
         return result;
     }
 
     @Override
     public Versioned<V> compute(K key, BiFunction<? super K, ? super V, ? extends V> remappingFunction) {
-        return version(map.compute(key, remappingFunction));
+            AtomicBoolean updated = new AtomicBoolean(false);
+            AtomicReference<Versioned<V>> previousValue = new AtomicReference<>();
+            Versioned<V> result = map.compute(key, (k, v) -> {
+                    updated.set(true);
+                    previousValue.set(v);
+                    return version(remappingFunction.apply(k, Versioned.valueOrNull(v)));
+                });
+            if (updated.get()) {
+                notifyListeners(mapName, key, result, previousValue.get());
+            }
+            return result;
     }
 
     @Override
     public Versioned<V> computeIfPresent(K key, BiFunction<? super K, ? super V, ? extends V> remappingFunction) {
-        return version(map.computeIfPresent(key, remappingFunction));
+        AtomicBoolean updated = new AtomicBoolean(false);
-    }
+        AtomicReference<Versioned<V>> previousValue = new AtomicReference<>();
-
+        Versioned<V> result = map.compute(key, (k, v) -> {
-    @Override
+            if (v != null) {
-    public Versioned<V> computeIf(K key, Predicate<? super V> condition,
+                updated.set(true);
-                                  BiFunction<? super K, ? super V, ? extends V> remappingFunction) {
+                previousValue.set(v);
-        return version(map.compute(key, (k, existingValue) -> {
+                return version(remappingFunction.apply(k, v.value()));
-            if (condition.test(existingValue)) {
-                return remappingFunction.apply(k, existingValue);
-            } else {
-                return existingValue;
             }
-        }));
+            return v;
-    }
+        });
-
+        if (updated.get()) {
-    @Override
+            notifyListeners(mapName, key, result, previousValue.get());
-    public Versioned<V> put(K key, V value) {
-        Versioned<V> result = version(value);
-        if (map.put(key, value) == null) {
-            notifyListeners(mapName, INSERT, key, result);
-        } else {
-            notifyListeners(mapName, UPDATE, key, result);
         }
         return result;
     }
 
     @Override
-    public Versioned<V> putAndGet(K key, V value) {
+    public Versioned<V> computeIf(K key, Predicate<? super V> condition,
-        Versioned<V> result = version(map.put(key, value));
+                                  BiFunction<? super K, ? super V, ? extends V> remappingFunction) {
-        notifyListeners(mapName, UPDATE, key, result);
+        AtomicBoolean updated = new AtomicBoolean(false);
+        AtomicReference<Versioned<V>> previousValue = new AtomicReference<>();
+        Versioned<V> result = map.compute(key, (k, v) -> {
+            if (condition.test(Versioned.valueOrNull(v))) {
+                previousValue.set(v);
+                updated.set(true);
+                return version(remappingFunction.apply(k, Versioned.valueOrNull(v)));
+            }
+            return v;
+        });
+        if (updated.get()) {
+            notifyListeners(mapName, key, result, previousValue.get());
+        }
         return result;
     }
 
+    @Override
+    public Versioned<V> put(K key, V value) {
+        Versioned<V> newValue = version(value);
+        Versioned<V> previousValue = map.put(key, newValue);
+        notifyListeners(mapName, key, newValue, previousValue);
+        return previousValue;
+    }
+
+    @Override
+    public Versioned<V> putAndGet(K key, V value) {
+        Versioned<V> newValue = version(value);
+        Versioned<V> previousValue = map.put(key, newValue);
+        notifyListeners(mapName, key, newValue, previousValue);
+        return newValue;
+    }
+
     @Override
     public Versioned<V> remove(K key) {
-        Versioned<V> result = version(map.remove(key));
+        Versioned<V> result = map.remove(key);
-        notifyListeners(mapName, REMOVE, key, result);
+        notifyListeners(mapName, key, null, result);
         return result;
     }
 
     @Override
     public void clear() {
-        map.clear();
+        map.keySet().forEach(this::remove);
     }
 
     @Override
@@ -158,70 +193,85 @@ public final class TestConsistentMap<K, V> extends ConsistentMapAdapter<K, V> {
 
     @Override
     public Collection<Versioned<V>> values() {
-        return map
+        return map.values()
-                .values()
                 .stream()
-                .map(this::version)
                 .collect(Collectors.toList());
     }
 
     @Override
     public Set<Map.Entry<K, Versioned<V>>> entrySet() {
-        return super.entrySet();
+        return map.entrySet();
     }
 
     @Override
     public Versioned<V> putIfAbsent(K key, V value) {
-        Versioned<V> result =  version(map.putIfAbsent(key, value));
+        Versioned<V> newValue = version(value);
-        if (map.get(key).equals(value)) {
+        Versioned<V> result =  map.putIfAbsent(key, newValue);
-            notifyListeners(mapName, INSERT, key, result);
+        if (result == null) {
+            notifyListeners(mapName, key, newValue, result);
         }
         return result;
     }
 
     @Override
     public boolean remove(K key, V value) {
-        boolean removed = map.remove(key, value);
+        Versioned<V> existingValue = map.get(key);
-        if (removed) {
+        if (Objects.equal(Versioned.valueOrNull(existingValue), value)) {
-            notifyListeners(mapName, REMOVE, key, null);
+            map.remove(key);
+            notifyListeners(mapName, key, null, existingValue);
+            return true;
         }
-        return removed;
+        return false;
     }
 
     @Override
     public boolean remove(K key, long version) {
-        boolean removed =  map.remove(key, version);
+        Versioned<V> existingValue = map.get(key);
-        if (removed) {
+        if (existingValue == null) {
-            notifyListeners(mapName, REMOVE, key, null);
+            return false;
         }
-        return removed;
+        if (existingValue.version() == version) {
+            map.remove(key);
+            notifyListeners(mapName, key, null, existingValue);
+            return true;
+        }
+        return false;
     }
 
     @Override
     public Versioned<V> replace(K key, V value) {
-        Versioned<V> result = version(map.replace(key, value));
+        Versioned<V> existingValue = map.get(key);
-        if (map.get(key).equals(value)) {
+        if (existingValue == null) {
-            notifyListeners(mapName, UPDATE, key, result);
+            return null;
         }
+        Versioned<V> newValue = version(value);
+        Versioned<V> result = map.put(key, newValue);
+        notifyListeners(mapName, key, newValue, result);
         return result;
     }
 
     @Override
     public boolean replace(K key, V oldValue, V newValue) {
-        boolean replaced = map.replace(key, oldValue, newValue);
+        Versioned<V> existingValue = map.get(key);
-        if (replaced) {
+        if (existingValue == null || !existingValue.value().equals(oldValue)) {
-            notifyListeners(mapName, REMOVE, key, null);
+            return false;
         }
-        return replaced;
+        Versioned<V> value = version(newValue);
+        Versioned<V> result = map.put(key, value);
+        notifyListeners(mapName, key, value, result);
+        return true;
     }
 
     @Override
     public boolean replace(K key, long oldVersion, V newValue) {
-        boolean replaced =  map.replace(key, map.get(key), newValue);
+        Versioned<V> existingValue = map.get(key);
-        if (replaced) {
+        if (existingValue == null || existingValue.version() != oldVersion) {
-            notifyListeners(mapName, REMOVE, key, null);
+            return false;
         }
-        return replaced;
+        Versioned<V> value = version(newValue);
+        Versioned<V> result = map.put(key, value);
+        notifyListeners(mapName, key, value, result);
+        return true;
     }
 
     @Override
@@ -236,7 +286,7 @@ public final class TestConsistentMap<K, V> extends ConsistentMapAdapter<K, V> {
 
     @Override
     public Map<K, V> asJavaMap() {
-        return map;
+        return new ConsistentMapBackedJavaMap<>(this);
     }
 
     public static Builder builder() {

core/store/primitives/src/main/java/org/onosproject/store/primitives/impl/DefaultConsistentMap.java

@@ -28,6 +28,7 @@ import java.util.function.BiFunction;
 import java.util.function.Function;
 import java.util.function.Predicate;
 
+import org.onosproject.store.primitives.ConsistentMapBackedJavaMap;
 import org.onosproject.store.service.AsyncConsistentMap;
 import org.onosproject.store.service.ConsistentMap;
 import org.onosproject.store.service.ConsistentMapException;

core/store/primitives/src/main/java/org/onosproject/store/primitives/impl/UpdateResult.java

@@ -76,10 +76,7 @@ public class UpdateResult<K, V> {
         if (!updated) {
             return null;
         } else {
-            MapEvent.Type eventType = oldValue == null ?
+            return new MapEvent<>(mapName(), key(), newValue, oldValue);
-                    MapEvent.Type.INSERT : newValue == null ? MapEvent.Type.REMOVE : MapEvent.Type.UPDATE;
-            Versioned<V> eventValue = eventType == MapEvent.Type.REMOVE ? oldValue : newValue;
-            return new MapEvent<>(mapName(), eventType, key(), eventValue);
         }
     }
 }