Fixed annotation handling.
- Store as SparseAnnotations internally and convert to Annotations
when merging multiple provider supplied annotations.
Change-Id: I82fe159b536b3e7344a33e09792f6a3473fb3500
diff --git a/core/api/src/main/java/org/onlab/onos/net/DefaultAnnotations.java b/core/api/src/main/java/org/onlab/onos/net/DefaultAnnotations.java
index 0c0f375..b2e7e61 100644
--- a/core/api/src/main/java/org/onlab/onos/net/DefaultAnnotations.java
+++ b/core/api/src/main/java/org/onlab/onos/net/DefaultAnnotations.java
@@ -73,31 +73,63 @@
}
/**
- * Convert Annotations to DefaultAnnotations if needed and merges.
+ * Creates the union of two given SparseAnnotations.
+ * Unlike the {@link #merge(DefaultAnnotations, SparseAnnotations)} method,
+ * result will be {@link SparseAnnotations} instead of {@link Annotations}.
*
- * @see #merge(DefaultAnnotations, SparseAnnotations)
+ * A key tagged for removal will remain in the output SparseAnnotations,
+ * if the counterpart of the input does not contain the same key.
*
* @param annotations base annotations
* @param sparseAnnotations additional sparse annotations
* @return combined annotations or the original base annotations if there
* are not additional annotations
*/
- public static DefaultAnnotations merge(Annotations annotations,
- SparseAnnotations sparseAnnotations) {
- if (annotations instanceof DefaultAnnotations) {
- return merge((DefaultAnnotations) annotations, sparseAnnotations);
+ public static SparseAnnotations union(SparseAnnotations annotations,
+ SparseAnnotations sparseAnnotations) {
+
+ if (sparseAnnotations == null || sparseAnnotations.keys().isEmpty()) {
+ return annotations;
}
- DefaultAnnotations.Builder builder = DefaultAnnotations.builder();
- for (String key : annotations.keys()) {
- builder.set(key, annotations.value(key));
+ final HashMap<String, String> newMap;
+ if (annotations instanceof DefaultAnnotations) {
+ newMap = copy(((DefaultAnnotations) annotations).map);
+ } else {
+ newMap = new HashMap<>(annotations.keys().size() +
+ sparseAnnotations.keys().size());
+ putAllSparseAnnotations(newMap, annotations);
}
- return merge(builder.build(), sparseAnnotations);
+
+ putAllSparseAnnotations(newMap, sparseAnnotations);
+ return new DefaultAnnotations(newMap);
+ }
+
+ // adds the key-values contained in sparseAnnotations to
+ // newMap, if sparseAnnotations had a key tagged for removal,
+ // and corresponding key exist in newMap, entry will be removed.
+ // if corresponding key does not exist, removal tag will be added to
+ // the newMap.
+ private static void putAllSparseAnnotations(
+ final HashMap<String, String> newMap,
+ SparseAnnotations sparseAnnotations) {
+
+ for (String key : sparseAnnotations.keys()) {
+ if (sparseAnnotations.isRemoved(key)) {
+ if (newMap.containsKey(key)) {
+ newMap.remove(key);
+ } else {
+ newMap.put(key, Builder.REMOVED);
+ }
+ } else {
+ String value = sparseAnnotations.value(key);
+ newMap.put(key, value);
+ }
+ }
}
@Override
public Set<String> keys() {
- // TODO: unmodifiable to be removed after switching to ImmutableMap;
return Collections.unmodifiableSet(map.keySet());
}
@@ -115,7 +147,7 @@
@SuppressWarnings("unchecked")
private static HashMap<String, String> copy(Map<String, String> original) {
if (original instanceof HashMap) {
- return (HashMap) ((HashMap) original).clone();
+ return (HashMap<String, String>) ((HashMap<?, ?>) original).clone();
}
throw new IllegalArgumentException("Expecting HashMap instance");
}
diff --git a/core/api/src/test/java/org/onlab/onos/net/ConnectPointTest.java b/core/api/src/test/java/org/onlab/onos/net/ConnectPointTest.java
index 6d3e793..6b9f777 100644
--- a/core/api/src/test/java/org/onlab/onos/net/ConnectPointTest.java
+++ b/core/api/src/test/java/org/onlab/onos/net/ConnectPointTest.java
@@ -33,4 +33,4 @@
.addEqualityGroup(new ConnectPoint(DID2, P1), new ConnectPoint(DID2, P1))
.testEquals();
}
-}
\ No newline at end of file
+}
diff --git a/core/api/src/test/java/org/onlab/onos/net/DefaultAnnotationsTest.java b/core/api/src/test/java/org/onlab/onos/net/DefaultAnnotationsTest.java
index 274f4b8..9132126 100644
--- a/core/api/src/test/java/org/onlab/onos/net/DefaultAnnotationsTest.java
+++ b/core/api/src/test/java/org/onlab/onos/net/DefaultAnnotationsTest.java
@@ -36,6 +36,23 @@
}
@Test
+ public void union() {
+ annotations = builder().set("foo", "1").set("bar", "2").remove("buz").build();
+ assertEquals("incorrect keys", of("foo", "bar", "buz"), annotations.keys());
+
+ SparseAnnotations updates = builder().remove("foo").set("bar", "3").set("goo", "4").remove("fuzz").build();
+
+ SparseAnnotations result = DefaultAnnotations.union(annotations, updates);
+
+ assertTrue("remove instruction in original remains", result.isRemoved("buz"));
+ assertTrue("remove instruction in update remains", result.isRemoved("fuzz"));
+ assertEquals("incorrect keys", of("buz", "goo", "bar", "fuzz"), result.keys());
+ assertNull("incorrect value", result.value("foo"));
+ assertEquals("incorrect value", "3", result.value("bar"));
+ assertEquals("incorrect value", "4", result.value("goo"));
+ }
+
+ @Test
public void merge() {
annotations = builder().set("foo", "1").set("bar", "2").build();
assertEquals("incorrect keys", of("foo", "bar"), annotations.keys());
@@ -65,4 +82,4 @@
DefaultAnnotations.merge(null, null);
}
-}
\ No newline at end of file
+}
diff --git a/core/api/src/test/java/org/onlab/onos/net/DefaultDeviceTest.java b/core/api/src/test/java/org/onlab/onos/net/DefaultDeviceTest.java
index b4018e3..329e128 100644
--- a/core/api/src/test/java/org/onlab/onos/net/DefaultDeviceTest.java
+++ b/core/api/src/test/java/org/onlab/onos/net/DefaultDeviceTest.java
@@ -58,7 +58,5 @@
assertEquals("incorrect hw", HW, device.hwVersion());
assertEquals("incorrect sw", SW, device.swVersion());
assertEquals("incorrect serial", SN1, device.serialNumber());
- assertEquals("incorrect serial", SN1, device.serialNumber());
}
-
-}
\ No newline at end of file
+}
diff --git a/core/api/src/test/java/org/onlab/onos/net/topology/DefaultGraphDescriptionTest.java b/core/api/src/test/java/org/onlab/onos/net/topology/DefaultGraphDescriptionTest.java
index a968abf..5f7d47b 100644
--- a/core/api/src/test/java/org/onlab/onos/net/topology/DefaultGraphDescriptionTest.java
+++ b/core/api/src/test/java/org/onlab/onos/net/topology/DefaultGraphDescriptionTest.java
@@ -37,5 +37,4 @@
new DefaultGraphDescription(4321L, ImmutableSet.of(DEV1, DEV3),
ImmutableSet.of(L1, L2));
}
-
-}
\ No newline at end of file
+}
diff --git a/core/api/src/test/java/org/onlab/onos/net/topology/DefaultTopologyEdgeTest.java b/core/api/src/test/java/org/onlab/onos/net/topology/DefaultTopologyEdgeTest.java
index 6c3c112..5d64c83 100644
--- a/core/api/src/test/java/org/onlab/onos/net/topology/DefaultTopologyEdgeTest.java
+++ b/core/api/src/test/java/org/onlab/onos/net/topology/DefaultTopologyEdgeTest.java
@@ -50,5 +50,4 @@
new DefaultTopologyEdge(V2, V1, L2))
.testEquals();
}
-
-}
\ No newline at end of file
+}
diff --git a/core/api/src/test/java/org/onlab/onos/net/topology/DefaultTopologyVertexTest.java b/core/api/src/test/java/org/onlab/onos/net/topology/DefaultTopologyVertexTest.java
index 1284624..7210ff3 100644
--- a/core/api/src/test/java/org/onlab/onos/net/topology/DefaultTopologyVertexTest.java
+++ b/core/api/src/test/java/org/onlab/onos/net/topology/DefaultTopologyVertexTest.java
@@ -26,5 +26,4 @@
.addEqualityGroup(new DefaultTopologyVertex(D2),
new DefaultTopologyVertex(D2)).testEquals();
}
-
-}
\ No newline at end of file
+}
diff --git a/core/store/dist/src/main/java/org/onlab/onos/store/device/impl/GossipDeviceStore.java b/core/store/dist/src/main/java/org/onlab/onos/store/device/impl/GossipDeviceStore.java
index ac38e23..85d9b07 100644
--- a/core/store/dist/src/main/java/org/onlab/onos/store/device/impl/GossipDeviceStore.java
+++ b/core/store/dist/src/main/java/org/onlab/onos/store/device/impl/GossipDeviceStore.java
@@ -56,6 +56,7 @@
import static org.slf4j.LoggerFactory.getLogger;
import static org.apache.commons.lang3.concurrent.ConcurrentUtils.createIfAbsentUnchecked;
import static org.onlab.onos.net.DefaultAnnotations.merge;
+import static org.onlab.onos.net.DefaultAnnotations.union;
import static com.google.common.base.Verify.verify;
// TODO: implement remove event handling and call *Internal
@@ -603,7 +604,7 @@
Timestamped<DeviceDescription> oldOne = deviceDesc.get();
Timestamped<DeviceDescription> newOne = newDesc;
if (oldOne != null) {
- SparseAnnotations merged = merge(oldOne.value().annotations(),
+ SparseAnnotations merged = union(oldOne.value().annotations(),
newDesc.value().annotations());
newOne = new Timestamped<DeviceDescription>(
new DefaultDeviceDescription(newDesc.value(), merged),
@@ -622,7 +623,7 @@
Timestamped<PortDescription> oldOne = portDescs.get(newDesc.value().portNumber());
Timestamped<PortDescription> newOne = newDesc;
if (oldOne != null) {
- SparseAnnotations merged = merge(oldOne.value().annotations(),
+ SparseAnnotations merged = union(oldOne.value().annotations(),
newDesc.value().annotations());
newOne = new Timestamped<PortDescription>(
new DefaultPortDescription(newDesc.value(), merged),
diff --git a/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleDeviceStore.java b/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleDeviceStore.java
index 926c8a7..0880ac9 100644
--- a/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleDeviceStore.java
+++ b/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleDeviceStore.java
@@ -51,6 +51,7 @@
import static org.onlab.onos.net.device.DeviceEvent.Type.*;
import static org.slf4j.LoggerFactory.getLogger;
import static org.apache.commons.lang3.concurrent.ConcurrentUtils.createIfAbsentUnchecked;
+import static org.onlab.onos.net.DefaultAnnotations.union;
import static org.onlab.onos.net.DefaultAnnotations.merge;
/**
@@ -488,7 +489,7 @@
DeviceDescription oldOne = deviceDesc.get();
DeviceDescription newOne = newDesc;
if (oldOne != null) {
- SparseAnnotations merged = merge(oldOne.annotations(),
+ SparseAnnotations merged = union(oldOne.annotations(),
newDesc.annotations());
newOne = new DefaultDeviceDescription(newOne, merged);
}
@@ -505,7 +506,7 @@
PortDescription oldOne = portDescs.get(newDesc.portNumber());
PortDescription newOne = newDesc;
if (oldOne != null) {
- SparseAnnotations merged = merge(oldOne.annotations(),
+ SparseAnnotations merged = union(oldOne.annotations(),
newDesc.annotations());
newOne = new DefaultPortDescription(newOne, merged);
}
diff --git a/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleLinkStore.java b/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleLinkStore.java
index 1578384..daf28df 100644
--- a/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleLinkStore.java
+++ b/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleLinkStore.java
@@ -11,7 +11,6 @@
import org.apache.felix.scr.annotations.Component;
import org.apache.felix.scr.annotations.Deactivate;
import org.apache.felix.scr.annotations.Service;
-import org.onlab.onos.net.Annotations;
import org.onlab.onos.net.AnnotationsUtil;
import org.onlab.onos.net.ConnectPoint;
import org.onlab.onos.net.DefaultAnnotations;
@@ -39,6 +38,7 @@
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
+import static org.onlab.onos.net.DefaultAnnotations.union;
import static org.onlab.onos.net.DefaultAnnotations.merge;
import static org.onlab.onos.net.Link.Type.DIRECT;
import static org.onlab.onos.net.Link.Type.INDIRECT;
@@ -173,7 +173,7 @@
LinkDescription oldDesc = descs.get(providerId);
LinkDescription newDesc = linkDescription;
if (oldDesc != null) {
- SparseAnnotations merged = merge(oldDesc.annotations(),
+ SparseAnnotations merged = union(oldDesc.annotations(),
linkDescription.annotations());
newDesc = new DefaultLinkDescription(
linkDescription.src(),
@@ -268,7 +268,7 @@
ConnectPoint src = base.src();
ConnectPoint dst = base.dst();
Type type = base.type();
- Annotations annotations = DefaultAnnotations.builder().build();
+ DefaultAnnotations annotations = DefaultAnnotations.builder().build();
annotations = merge(annotations, base.annotations());
for (Entry<ProviderId, LinkDescription> e : descs.entrySet()) {