Fixed remove behavior for Device and Link Store
Change-Id: I2d6c6a48f9b92136c2f0734d0216f9f3b05b4d8c
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 996dfa7..ac38e23 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
@@ -33,6 +33,7 @@
import org.onlab.onos.store.ClockService;
import org.onlab.onos.store.Timestamp;
import org.onlab.onos.store.common.impl.Timestamped;
+import org.onlab.util.NewConcurrentHashMap;
import org.slf4j.Logger;
import java.util.ArrayList;
@@ -136,8 +137,7 @@
// Collection of DeviceDescriptions for a Device
ConcurrentMap<ProviderId, DeviceDescriptions> providerDescs
- = createIfAbsentUnchecked(deviceDescs, deviceId,
- new InitConcurrentHashMap<ProviderId, DeviceDescriptions>());
+ = getDeviceDescriptions(deviceId);
DeviceDescriptions descs
@@ -223,8 +223,7 @@
@Override
public DeviceEvent markOffline(DeviceId deviceId) {
ConcurrentMap<ProviderId, DeviceDescriptions> providerDescs
- = createIfAbsentUnchecked(deviceDescs, deviceId,
- new InitConcurrentHashMap<ProviderId, DeviceDescriptions>());
+ = getDeviceDescriptions(deviceId);
// locking device
synchronized (providerDescs) {
@@ -358,7 +357,13 @@
// exist, it creates and registers a new one.
private ConcurrentMap<PortNumber, Port> getPortMap(DeviceId deviceId) {
return createIfAbsentUnchecked(devicePorts, deviceId,
- new InitConcurrentHashMap<PortNumber, Port>());
+ NewConcurrentHashMap.<PortNumber, Port>ifNeeded());
+ }
+
+ private ConcurrentMap<ProviderId, DeviceDescriptions> getDeviceDescriptions(
+ DeviceId deviceId) {
+ return createIfAbsentUnchecked(deviceDescs, deviceId,
+ NewConcurrentHashMap.<ProviderId, DeviceDescriptions>ifNeeded());
}
@Override
@@ -438,10 +443,16 @@
@Override
public DeviceEvent removeDevice(DeviceId deviceId) {
- Device device = devices.remove(deviceId);
- // FIXME: should we be removing deviceDescs also?
- return device == null ? null :
- new DeviceEvent(DEVICE_REMOVED, device, null);
+ ConcurrentMap<ProviderId, DeviceDescriptions> descs = getDeviceDescriptions(deviceId);
+ synchronized (descs) {
+ Device device = devices.remove(deviceId);
+ // should DEVICE_REMOVED carry removed ports?
+ devicePorts.get(deviceId).clear();
+ availableDevices.remove(deviceId);
+ descs.clear();
+ return device == null ? null :
+ new DeviceEvent(DEVICE_REMOVED, device, null);
+ }
}
/**
@@ -546,14 +557,6 @@
return fallBackPrimary;
}
- private static final class InitConcurrentHashMap<K, V> implements
- ConcurrentInitializer<ConcurrentMap<K, V>> {
- @Override
- public ConcurrentMap<K, V> get() throws ConcurrentException {
- return new ConcurrentHashMap<>();
- }
- }
-
public static final class InitDeviceDescs
implements ConcurrentInitializer<DeviceDescriptions> {
diff --git a/core/store/dist/src/test/java/org/onlab/onos/store/device/impl/GossipDeviceStoreTest.java b/core/store/dist/src/test/java/org/onlab/onos/store/device/impl/GossipDeviceStoreTest.java
index 09c3098..94de9b2 100644
--- a/core/store/dist/src/test/java/org/onlab/onos/store/device/impl/GossipDeviceStoreTest.java
+++ b/core/store/dist/src/test/java/org/onlab/onos/store/device/impl/GossipDeviceStoreTest.java
@@ -116,17 +116,19 @@
deviceClockManager.deactivate();
}
- private void putDevice(DeviceId deviceId, String swVersion) {
+ private void putDevice(DeviceId deviceId, String swVersion,
+ SparseAnnotations... annotations) {
DeviceDescription description =
new DefaultDeviceDescription(deviceId.uri(), SWITCH, MFR,
- HW, swVersion, SN);
+ HW, swVersion, SN, annotations);
deviceStore.createOrUpdateDevice(PID, deviceId, description);
}
- private void putDeviceAncillary(DeviceId deviceId, String swVersion) {
+ private void putDeviceAncillary(DeviceId deviceId, String swVersion,
+ SparseAnnotations... annotations) {
DeviceDescription description =
new DefaultDeviceDescription(deviceId.uri(), SWITCH, MFR,
- HW, swVersion, SN);
+ HW, swVersion, SN, annotations);
deviceStore.createOrUpdateDevice(PIDA, deviceId, description);
}
@@ -448,16 +450,37 @@
@Test
public final void testRemoveDevice() {
- putDevice(DID1, SW1);
+ putDevice(DID1, SW1, A1);
+ List<PortDescription> pds = Arrays.<PortDescription>asList(
+ new DefaultPortDescription(P1, true, A2)
+ );
+ deviceStore.updatePorts(PID, DID1, pds);
putDevice(DID2, SW1);
assertEquals(2, deviceStore.getDeviceCount());
+ assertEquals(1, deviceStore.getPorts(DID1).size());
+ assertAnnotationsEquals(deviceStore.getDevice(DID1).annotations(), A1);
+ assertAnnotationsEquals(deviceStore.getPort(DID1, P1).annotations(), A2);
DeviceEvent event = deviceStore.removeDevice(DID1);
assertEquals(DEVICE_REMOVED, event.type());
assertDevice(DID1, SW1, event.subject());
assertEquals(1, deviceStore.getDeviceCount());
+ assertEquals(0, deviceStore.getPorts(DID1).size());
+
+ // putBack Device, Port w/o annotation
+ putDevice(DID1, SW1);
+ List<PortDescription> pds2 = Arrays.<PortDescription>asList(
+ new DefaultPortDescription(P1, true)
+ );
+ deviceStore.updatePorts(PID, DID1, pds2);
+
+ // annotations should not survive
+ assertEquals(2, deviceStore.getDeviceCount());
+ assertEquals(1, deviceStore.getPorts(DID1).size());
+ assertAnnotationsEquals(deviceStore.getDevice(DID1).annotations());
+ assertAnnotationsEquals(deviceStore.getPort(DID1, P1).annotations());
}
// If Delegates should be called only on remote events,