diff --git a/core/store/primitives/src/main/java/org/onosproject/store/primitives/impl/ConsistentMapBackedJavaMap.java b/core/api/src/main/java/org/onosproject/store/primitives/ConsistentMapBackedJavaMap.java similarity index 98% rename from core/store/primitives/src/main/java/org/onosproject/store/primitives/impl/ConsistentMapBackedJavaMap.java rename to core/api/src/main/java/org/onosproject/store/primitives/ConsistentMapBackedJavaMap.java index a03b032154..0b67b620e3 100644 --- a/core/store/primitives/src/main/java/org/onosproject/store/primitives/impl/ConsistentMapBackedJavaMap.java +++ b/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; diff --git a/core/api/src/main/java/org/onosproject/store/service/MapEvent.java b/core/api/src/main/java/org/onosproject/store/service/MapEvent.java index 6e67135101..3462e21312 100644 --- a/core/api/src/main/java/org/onosproject/store/service/MapEvent.java +++ b/core/api/src/main/java/org/onosproject/store/service/MapEvent.java @@ -50,21 +50,24 @@ public class MapEvent { private final String name; private final Type type; private final K key; - private final Versioned value; + private final Versioned newValue; + private final Versioned 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 value) { + public MapEvent(String name, K key, Versioned currentValue, Versioned 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 { * the new value. * * @return the value + * @deprecated use {@link #newValue()} or {@link #oldValue()} instead. */ + @Deprecated public Versioned 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 newValue() { + return newValue; + } + + /** + * Returns the value associated with the key, before it was updated. + * + * @return previous value in map for the key + */ + public Versioned oldValue() { + return oldValue; } @Override @@ -115,12 +139,13 @@ public class MapEvent { 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 { .add("name", name) .add("type", type) .add("key", key) - .add("value", value) + .add("newValue", newValue) + .add("oldValue", oldValue) .toString(); } } diff --git a/core/api/src/test/java/org/onosproject/store/service/MapEventTest.java b/core/api/src/test/java/org/onosproject/store/service/MapEventTest.java index 47fba6c9e6..1015e1b2a6 100644 --- a/core/api/src/test/java/org/onosproject/store/service/MapEventTest.java +++ b/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 vStats = new Versioned<>(2, 1); + private final Versioned vStatsNew = new Versioned<>(2, 2); + private final Versioned vStatsOld = new Versioned<>(1, 1); - private final MapEvent stats1 = new MapEvent<>("a", MapEvent.Type.INSERT, "1", vStats); + private final MapEvent stats1 = new MapEvent<>("a", "1", vStatsNew, null); - private final MapEvent stats2 = new MapEvent<>("a", MapEvent.Type.REMOVE, "1", vStats); + private final MapEvent stats2 = new MapEvent<>("a", "1", null, vStatsOld); - private final MapEvent stats3 = new MapEvent<>("a", MapEvent.Type.UPDATE, "1", vStats); + private final MapEvent 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) 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) 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)); } /** diff --git a/core/api/src/test/java/org/onosproject/store/service/TestConsistentMap.java b/core/api/src/test/java/org/onosproject/store/service/TestConsistentMap.java index 4133095171..f7857adcef 100644 --- a/core/api/src/test/java/org/onosproject/store/service/TestConsistentMap.java +++ b/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 extends ConsistentMapAdapter { private final List> listeners; - private final Map map; + private final Map> map; private final String mapName; private final AtomicLong counter = new AtomicLong(0); @@ -54,9 +57,9 @@ public final class TestConsistentMap extends ConsistentMapAdapter { /** * Notify all listeners of an event. */ - private void notifyListeners(String mapName, MapEvent.Type type, - K key, Versioned value) { - MapEvent event = new MapEvent<>(mapName, type, key, value); + private void notifyListeners(String mapName, + K key, Versioned newvalue, Versioned oldValue) { + MapEvent event = new MapEvent<>(mapName, key, newvalue, oldValue); listeners.forEach( listener -> listener.event(event) ); @@ -84,71 +87,103 @@ public final class TestConsistentMap extends ConsistentMapAdapter { @Override public Versioned get(K key) { - V value = map.get(key); - if (value != null) { - return version(value); - } else { - return null; - } + return map.get(key); } @Override public Versioned computeIfAbsent(K key, Function mappingFunction) { - Versioned result = version(map.computeIfAbsent(key, mappingFunction)); - notifyListeners(mapName, INSERT, key, result); + AtomicBoolean updated = new AtomicBoolean(false); + Versioned 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 compute(K key, BiFunction remappingFunction) { - return version(map.compute(key, remappingFunction)); + AtomicBoolean updated = new AtomicBoolean(false); + AtomicReference> previousValue = new AtomicReference<>(); + Versioned 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 computeIfPresent(K key, BiFunction remappingFunction) { - return version(map.computeIfPresent(key, remappingFunction)); - } - - @Override - public Versioned computeIf(K key, Predicate condition, - BiFunction remappingFunction) { - return version(map.compute(key, (k, existingValue) -> { - if (condition.test(existingValue)) { - return remappingFunction.apply(k, existingValue); - } else { - return existingValue; + AtomicBoolean updated = new AtomicBoolean(false); + AtomicReference> previousValue = new AtomicReference<>(); + Versioned result = map.compute(key, (k, v) -> { + if (v != null) { + updated.set(true); + previousValue.set(v); + return version(remappingFunction.apply(k, v.value())); } - })); - } - - @Override - public Versioned put(K key, V value) { - Versioned result = version(value); - if (map.put(key, value) == null) { - notifyListeners(mapName, INSERT, key, result); - } else { - notifyListeners(mapName, UPDATE, key, result); + return v; + }); + if (updated.get()) { + notifyListeners(mapName, key, result, previousValue.get()); } return result; } @Override - public Versioned putAndGet(K key, V value) { - Versioned result = version(map.put(key, value)); - notifyListeners(mapName, UPDATE, key, result); + public Versioned computeIf(K key, Predicate condition, + BiFunction remappingFunction) { + AtomicBoolean updated = new AtomicBoolean(false); + AtomicReference> previousValue = new AtomicReference<>(); + Versioned 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 put(K key, V value) { + Versioned newValue = version(value); + Versioned previousValue = map.put(key, newValue); + notifyListeners(mapName, key, newValue, previousValue); + return previousValue; + } + + @Override + public Versioned putAndGet(K key, V value) { + Versioned newValue = version(value); + Versioned previousValue = map.put(key, newValue); + notifyListeners(mapName, key, newValue, previousValue); + return newValue; + } + @Override public Versioned remove(K key) { - Versioned result = version(map.remove(key)); - notifyListeners(mapName, REMOVE, key, result); + Versioned result = map.remove(key); + 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 extends ConsistentMapAdapter { @Override public Collection> values() { - return map - .values() + return map.values() .stream() - .map(this::version) .collect(Collectors.toList()); } @Override public Set>> entrySet() { - return super.entrySet(); + return map.entrySet(); } @Override public Versioned putIfAbsent(K key, V value) { - Versioned result = version(map.putIfAbsent(key, value)); - if (map.get(key).equals(value)) { - notifyListeners(mapName, INSERT, key, result); + Versioned newValue = version(value); + Versioned result = map.putIfAbsent(key, newValue); + if (result == null) { + notifyListeners(mapName, key, newValue, result); } return result; } @Override public boolean remove(K key, V value) { - boolean removed = map.remove(key, value); - if (removed) { - notifyListeners(mapName, REMOVE, key, null); + Versioned existingValue = map.get(key); + if (Objects.equal(Versioned.valueOrNull(existingValue), value)) { + 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); - if (removed) { - notifyListeners(mapName, REMOVE, key, null); + Versioned existingValue = map.get(key); + if (existingValue == null) { + return false; } - return removed; + if (existingValue.version() == version) { + map.remove(key); + notifyListeners(mapName, key, null, existingValue); + return true; + } + return false; } @Override public Versioned replace(K key, V value) { - Versioned result = version(map.replace(key, value)); - if (map.get(key).equals(value)) { - notifyListeners(mapName, UPDATE, key, result); + Versioned existingValue = map.get(key); + if (existingValue == null) { + return null; } + Versioned newValue = version(value); + Versioned 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); - if (replaced) { - notifyListeners(mapName, REMOVE, key, null); + Versioned existingValue = map.get(key); + if (existingValue == null || !existingValue.value().equals(oldValue)) { + return false; } - return replaced; + Versioned value = version(newValue); + Versioned 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); - if (replaced) { - notifyListeners(mapName, REMOVE, key, null); + Versioned existingValue = map.get(key); + if (existingValue == null || existingValue.version() != oldVersion) { + return false; } - return replaced; + Versioned value = version(newValue); + Versioned result = map.put(key, value); + notifyListeners(mapName, key, value, result); + return true; } @Override @@ -236,7 +286,7 @@ public final class TestConsistentMap extends ConsistentMapAdapter { @Override public Map asJavaMap() { - return map; + return new ConsistentMapBackedJavaMap<>(this); } public static Builder builder() { diff --git a/core/store/primitives/src/main/java/org/onosproject/store/primitives/impl/DefaultConsistentMap.java b/core/store/primitives/src/main/java/org/onosproject/store/primitives/impl/DefaultConsistentMap.java index e1af47ac74..569fc9025d 100644 --- a/core/store/primitives/src/main/java/org/onosproject/store/primitives/impl/DefaultConsistentMap.java +++ b/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; diff --git a/core/store/primitives/src/main/java/org/onosproject/store/primitives/impl/UpdateResult.java b/core/store/primitives/src/main/java/org/onosproject/store/primitives/impl/UpdateResult.java index cd943baf0f..6122b7980c 100644 --- a/core/store/primitives/src/main/java/org/onosproject/store/primitives/impl/UpdateResult.java +++ b/core/store/primitives/src/main/java/org/onosproject/store/primitives/impl/UpdateResult.java @@ -76,10 +76,7 @@ public class UpdateResult { if (!updated) { return null; } else { - MapEvent.Type eventType = oldValue == null ? - MapEvent.Type.INSERT : newValue == null ? MapEvent.Type.REMOVE : MapEvent.Type.UPDATE; - Versioned eventValue = eventType == MapEvent.Type.REMOVE ? oldValue : newValue; - return new MapEvent<>(mapName(), eventType, key(), eventValue); + return new MapEvent<>(mapName(), key(), newValue, oldValue); } } } \ No newline at end of file