Allow reading PortDescription from DeviceStore

DeviceManager had to reverse-engineer PortDescription in order to mark port off-line, etc. which required domain specific knowledge. (e.g., OpticalPortOperatorr#descriptionOf) required work for ONOS-4691 Change-Id: I954f5f2db2cb7db938f498ead4c8e3f84212a53f

Allow reading PortDescription from DeviceStore
DeviceManager had to reverse-engineer PortDescription in order to mark port off-line, etc. which required domain specific knowledge. (e.g., OpticalPortOperatorr#descriptionOf) required work for ONOS-4691 Change-Id: I954f5f2db2cb7db938f498ead4c8e3f84212a53f
Yuta HIGUCHI · Yuta HIGUCHI
Commit 6eb00cce43ff4cc714deff5f6e1f595c6dcd30cf 6eb00cce 1 parent 55562ea6
Showing 7 changed files with 137 additions and 118 deletions
core/api/src/main/java/org/onosproject/net/device/DeviceStore.java
core/api/src/test/java/org/onosproject/net/device/DeviceStoreAdapter.java
core/common/src/test/java/org/onosproject/store/trivial/SimpleDeviceStore.java
core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java
core/net/src/main/java/org/onosproject/net/device/impl/OpticalPortOperator.java
core/store/dist/src/main/java/org/onosproject/store/device/impl/ECDeviceStore.java
core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java
--- a/core/api/src/main/java/org/onosproject/net/device/DeviceStore.java
View file @6eb00cc
+++ b/core/api/src/main/java/org/onosproject/net/device/DeviceStore.java
View file @6eb00cc
@@ -24,6 +24,7 @@ import org.onosproject.store.Store;
 import java.util.Collection;
 import java.util.List;
+import java.util.stream.Stream;
 /**
  * Manages inventory of infrastructure devices; not intended for direct use.
@@ -115,6 +116,15 @@ public interface DeviceStore extends Store<DeviceEvent, DeviceStoreDelegate> {
     List<Port> getPorts(DeviceId deviceId);
     /**
+     * Returns the stream of port descriptions that belong to the specified device.
+     *
+     * @param providerId  provider identifier
+     * @param deviceId    device identifier
+     * @return stream of device portdescriptions
+     */
+    Stream<PortDescription> getPortDescriptions(ProviderId providerId, DeviceId deviceId);
+
+    /**
      * Updates the port statistics of the specified device using the give port
      * statistics.
      *
@@ -152,6 +162,16 @@ public interface DeviceStore extends Store<DeviceEvent, DeviceStoreDelegate> {
     Port getPort(DeviceId deviceId, PortNumber portNumber);
     /**
+     * Returns the specified device port description.
+     *
+     * @param providerId provider identifier
+     * @param deviceId   device identifier
+     * @param portNumber port number
+     * @return device port description
+     */
+    PortDescription getPortDescription(ProviderId providerId, DeviceId deviceId, PortNumber portNumber);
+
+    /**
      * Indicates whether the specified device is available/online.
      *
      * @param deviceId device identifier
--- a/core/api/src/test/java/org/onosproject/net/device/DeviceStoreAdapter.java
View file @6eb00cc
+++ b/core/api/src/test/java/org/onosproject/net/device/DeviceStoreAdapter.java
View file @6eb00cc
@@ -24,6 +24,7 @@ import org.onosproject.net.provider.ProviderId;
 import java.util.Collection;
 import java.util.List;
+import java.util.stream.Stream;
 /**
  * Test adapter for the DeviceStore API.
@@ -78,6 +79,12 @@ public class DeviceStoreAdapter implements DeviceStore {
     }
     @Override
+    public Stream<PortDescription> getPortDescriptions(ProviderId providerId,
+                                                       DeviceId deviceId) {
+        return Stream.empty();
+    }
+
+    @Override
     public DeviceEvent updatePortStatistics(ProviderId providerId, DeviceId deviceId,
                                             Collection<PortStatistics> portStats) {
         return null;
@@ -99,6 +106,13 @@ public class DeviceStoreAdapter implements DeviceStore {
     }
     @Override
+    public PortDescription getPortDescription(ProviderId providerId,
+                                              DeviceId deviceId,
+                                              PortNumber portNumber) {
+        return null;
+    }
+
+    @Override
     public boolean isAvailable(DeviceId deviceId) {
         return false;
     }
--- a/core/common/src/test/java/org/onosproject/store/trivial/SimpleDeviceStore.java
View file @6eb00cc
+++ b/core/common/src/test/java/org/onosproject/store/trivial/SimpleDeviceStore.java
View file @6eb00cc
@@ -59,11 +59,13 @@ import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.Stream;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
@@ -416,6 +418,15 @@ public class SimpleDeviceStore
     }
     @Override
+    public Stream<PortDescription> getPortDescriptions(ProviderId providerId,
+                                                       DeviceId deviceId) {
+        return Optional.ofNullable(deviceDescs.get(deviceId))
+                .map(m -> m.get(providerId))
+                .map(descs -> descs.portDescs.values().stream())
+                .orElse(Stream.empty());
+    }
+
+    @Override
     public DeviceEvent updatePortStatistics(ProviderId providerId, DeviceId deviceId,
                                             Collection<PortStatistics> newStatsCollection) {
@@ -480,6 +491,16 @@ public class SimpleDeviceStore
     }
     @Override
+    public PortDescription getPortDescription(ProviderId providerId,
+                                              DeviceId deviceId,
+                                              PortNumber portNumber) {
+        return Optional.ofNullable(deviceDescs.get(deviceId))
+                .map(m -> m.get(providerId))
+                .map(descs -> descs.getPortDesc(portNumber))
+                .orElse(null);
+    }
+
+    @Override
     public List<PortStatistics> getPortStatistics(DeviceId deviceId) {
         Map<PortNumber, PortStatistics> portStats = devicePortStats.get(deviceId);
         if (portStats == null) {
--- a/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java
View file @6eb00cc
+++ b/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java
View file @6eb00cc
@@ -86,6 +86,7 @@ import org.onosproject.net.optical.device.OmsPortHelper;
 import org.onosproject.net.optical.device.OtuPortHelper;
 import org.onosproject.net.provider.AbstractListenerProviderRegistry;
 import org.onosproject.net.provider.AbstractProviderService;
+import org.onosproject.net.provider.Provider;
 import org.slf4j.Logger;
 import com.google.common.util.concurrent.Futures;
@@ -367,6 +368,17 @@ public class DeviceManager
         }
+        private PortDescription ensurePortEnabledState(PortDescription desc, boolean enabled) {
+            if (desc.isEnabled() != enabled) {
+                return new DefaultPortDescription(desc.portNumber(),
+                                                  enabled,
+                                                  desc.type(),
+                                                  desc.portSpeed(),
+                                                  desc.annotations());
+            }
+            return desc;
+        }
+
         @Override
         public void deviceDisconnected(DeviceId deviceId) {
             checkNotNull(deviceId, DEVICE_ID_NULL);
@@ -374,17 +386,9 @@ public class DeviceManager
             log.info("Device {} disconnected from this node", deviceId);
-            List<Port> ports = store.getPorts(deviceId);
+            List<PortDescription> descs = store.getPortDescriptions(provider().id(), deviceId)
-            final Device device = getDevice(deviceId);
+                    .map(desc -> ensurePortEnabledState(desc, false))
-
+                    .collect(Collectors.toList());
-            List<PortDescription> descs = ports.stream().map(
-              port -> (!(Device.Type.ROADM.equals(device.type()) ||
-                        (Device.Type.OTN.equals(device.type())) ||
-                        (Device.Type.FIBER_SWITCH.equals(device.type())))) ?
-                  new DefaultPortDescription(port.number(), false,
-                          port.type(), port.portSpeed()) :
-                      OpticalPortOperator.descriptionOf(port, false)
-                 ).collect(Collectors.toList());
             store.updatePorts(this.provider().id(), deviceId, descs);
             try {
@@ -524,9 +528,12 @@ public class DeviceManager
             Device device = nullIsNotFound(getDevice(deviceId), "Device not found");
             if ((Device.Type.ROADM.equals(device.type())) ||
                 (Device.Type.OTN.equals(device.type()))) {
-                Port port = getPort(deviceId, portDescription.portNumber());
                 // FIXME This is ignoring all other info in portDescription given as input??
-                portDescription = OpticalPortOperator.descriptionOf(port, portDescription.isEnabled());
+                PortDescription storedPortDesc = store.getPortDescription(provider().id(),
+                                                          deviceId,
+                                                          portDescription.portNumber());
+                portDescription = ensurePortEnabledState(storedPortDesc,
+                                                         portDescription.isEnabled());
             }
             portDescription = consolidate(deviceId, portDescription);
@@ -847,13 +854,14 @@ public class DeviceManager
             if (event.configClass().equals(OpticalPortConfig.class)) {
                 ConnectPoint cpt = (ConnectPoint) event.subject();
                 DeviceId did = cpt.deviceId();
+                Provider provider = getProvider(did);
                 Port dpt = getPort(did, cpt.port());
-                if (dpt != null) {
+                if (dpt != null && provider != null) {
                     OpticalPortConfig opc = networkConfigService.getConfig(cpt, OpticalPortConfig.class);
-                    PortDescription desc = OpticalPortOperator.descriptionOf(dpt);
+                    PortDescription desc = store.getPortDescription(provider.id(), did, cpt.port());
                     desc = OpticalPortOperator.combine(opc, desc);
-                    if (desc != null && getProvider(did) != null) {
+                    if (desc != null) {
                         de = store.updatePortStatus(getProvider(did).id(), did, desc);
                     }
                 }
--- a/core/net/src/main/java/org/onosproject/net/device/impl/OpticalPortOperator.java
View file @6eb00cc
+++ b/core/net/src/main/java/org/onosproject/net/device/impl/OpticalPortOperator.java
View file @6eb00cc
@@ -20,13 +20,10 @@ import static org.onosproject.net.optical.device.OduCltPortHelper.oduCltPortDesc
 import static org.onosproject.net.optical.device.OmsPortHelper.omsPortDescription;
 import static org.onosproject.net.optical.device.OtuPortHelper.otuPortDescription;
 import static org.slf4j.LoggerFactory.getLogger;
-import static com.google.common.base.Preconditions.checkNotNull;
-
 import org.onosproject.net.config.ConfigOperator;
 import org.onosproject.net.config.basics.OpticalPortConfig;
 import org.onosproject.net.AnnotationKeys;
 import org.onosproject.net.DefaultAnnotations;
-import org.onosproject.net.Port;
 import org.onosproject.net.PortNumber;
 import org.onosproject.net.SparseAnnotations;
 import org.onosproject.net.device.DefaultPortDescription;
@@ -35,11 +32,6 @@ import org.onosproject.net.device.OduCltPortDescription;
 import org.onosproject.net.device.OmsPortDescription;
 import org.onosproject.net.device.OtuPortDescription;
 import org.onosproject.net.device.PortDescription;
-import org.onosproject.net.optical.OchPort;
-import org.onosproject.net.optical.OduCltPort;
-import org.onosproject.net.optical.OmsPort;
-import org.onosproject.net.optical.OpticalDevice;
-import org.onosproject.net.optical.OtuPort;
 import org.slf4j.Logger;
 /**
@@ -184,98 +176,4 @@ public final class OpticalPortOperator implements ConfigOperator {
         return DefaultAnnotations.union(an, b.build());
     }
-    /**
-     * Returns a description built from an existing port.
-     *
-     * @param port the device port
-     * @return a PortDescription based on the port
-     */
-    public static PortDescription descriptionOf(Port port) {
-        checkNotNull(port, "Must supply non-null Port");
-        final boolean isUp = port.isEnabled();
-        return descriptionOfPort(port, isUp);
-    }
-
-    /**
-     * Returns a description built from an existing port and reported status.
-     *
-     * @param port port
-     * @param isEnabled true if enabled
-     * @return a PortDescription based on the port
-     */
-    static PortDescription descriptionOf(Port port, boolean isEnabled) {
-        checkNotNull(port, "Must supply non-null Port");
-        final boolean isup = isEnabled;
-        return descriptionOfPort(port, isup);
-    }
-
-    private static PortDescription descriptionOfPort(Port port, final boolean isup) {
-        final PortNumber ptn = port.number();
-        final SparseAnnotations an = (SparseAnnotations) port.annotations();
-        switch (port.type()) {
-            case OMS:
-                if (port instanceof org.onosproject.net.OmsPort) {
-                    // remove if-block once deprecation is complete
-                    org.onosproject.net.OmsPort oms = (org.onosproject.net.OmsPort) port;
-                    return omsPortDescription(ptn, isup, oms.minFrequency(),
-                                              oms.maxFrequency(), oms.grid(), an);
-                }
-                if (port.element().is(OpticalDevice.class)) {
-                    OpticalDevice optDevice = port.element().as(OpticalDevice.class);
-                    if (optDevice.portIs(port, OmsPort.class)) {
-                        OmsPort oms = optDevice.portAs(port, OmsPort.class).get();
-                        return omsPortDescription(ptn, isup, oms.minFrequency(),
-                                                  oms.maxFrequency(), oms.grid(), an);
-                    }
-                }
-                return new DefaultPortDescription(ptn, isup, port.type(), port.portSpeed(), an);
-            case OCH:
-                if (port instanceof org.onosproject.net.OchPort) {
-                    // remove if-block once old OchPort deprecation is complete
-                    org.onosproject.net.OchPort och = (org.onosproject.net.OchPort) port;
-                    return ochPortDescription(ptn, isup, och.signalType(),
-                                              och.isTunable(), och.lambda(), an);
-                }
-                if (port.element().is(OpticalDevice.class)) {
-                    OpticalDevice optDevice = port.element().as(OpticalDevice.class);
-                    if (optDevice.portIs(port, OchPort.class)) {
-                        OchPort och = optDevice.portAs(port, OchPort.class).get();
-                        return ochPortDescription(ptn, isup, och.signalType(),
-                                                  och.isTunable(), och.lambda(), an);
-                    }
-                }
-                return new DefaultPortDescription(ptn, isup, port.type(), port.portSpeed(), an);
-
-            case ODUCLT:
-                if (port instanceof org.onosproject.net.OduCltPort) {
-                    // remove if-block once deprecation is complete
-                    org.onosproject.net.OduCltPort odu = (org.onosproject.net.OduCltPort) port;
-                    return oduCltPortDescription(ptn, isup, odu.signalType(), an);
-                }
-                if (port.element().is(OpticalDevice.class)) {
-                    OpticalDevice optDevice = port.element().as(OpticalDevice.class);
-                    if (optDevice.portIs(port, OduCltPort.class)) {
-                        OduCltPort odu = (OduCltPort) port;
-                        return oduCltPortDescription(ptn, isup, odu.signalType(), an);
-                    }
-                }
-                return new DefaultPortDescription(ptn, isup, port.type(), port.portSpeed(), an);
-            case OTU:
-                if (port instanceof org.onosproject.net.OtuPort) {
-                    // remove if-block once deprecation is complete
-                    org.onosproject.net.OtuPort otu = (org.onosproject.net.OtuPort) port;
-                    return otuPortDescription(ptn, isup, otu.signalType(), an);
-                }
-                if (port.element().is(OpticalDevice.class)) {
-                    OpticalDevice optDevice = port.element().as(OpticalDevice.class);
-                    if (optDevice.portIs(port, OtuPort.class)) {
-                        OtuPort otu = (OtuPort) port;
-                        return otuPortDescription(ptn, isup, otu.signalType(), an);
-                    }
-                }
-                return new DefaultPortDescription(ptn, isup, port.type(), port.portSpeed(), an);
-            default:
-                return new DefaultPortDescription(ptn, isup, port.type(), port.portSpeed(), an);
-        }
-    }
 }
--- a/core/store/dist/src/main/java/org/onosproject/store/device/impl/ECDeviceStore.java
View file @6eb00cc
+++ b/core/store/dist/src/main/java/org/onosproject/store/device/impl/ECDeviceStore.java
View file @6eb00cc
@@ -87,6 +87,7 @@ import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Verify.verify;
@@ -575,11 +576,27 @@ public class ECDeviceStore
     }
     @Override
+    public Stream<PortDescription> getPortDescriptions(ProviderId pid,
+                                                       DeviceId deviceId) {
+
+        return portDescriptions.entrySet().stream()
+                .filter(e -> e.getKey().providerId().equals(pid))
+                .map(Map.Entry::getValue);
+    }
+
+    @Override
     public Port getPort(DeviceId deviceId, PortNumber portNumber) {
         return devicePorts.getOrDefault(deviceId, Maps.newHashMap()).get(portNumber);
     }
     @Override
+    public PortDescription getPortDescription(ProviderId pid,
+                                              DeviceId deviceId,
+                                              PortNumber portNumber) {
+        return portDescriptions.get(new PortKey(pid, deviceId, portNumber));
+    }
+
+    @Override
     public DeviceEvent updatePortStatistics(ProviderId providerId,
             DeviceId deviceId,
             Collection<PortStatistics> newStatsCollection) {
--- a/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java
View file @6eb00cc
+++ b/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java
View file @6eb00cc
@@ -93,11 +93,13 @@ import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
+import java.util.stream.Stream;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Predicates.notNull;
@@ -835,6 +837,25 @@ public class GossipDeviceStore
     }
     @Override
+    public Stream<PortDescription> getPortDescriptions(ProviderId pid,
+                                                       DeviceId deviceId) {
+        Map<ProviderId, DeviceDescriptions> descs = this.deviceDescs.get(deviceId);
+        if (descs == null) {
+            return null;
+        }
+        // inner-Map(=descs) is HashMap, thus requires synchronization even for reads
+        final Optional<DeviceDescriptions> devDescs;
+        synchronized (descs) {
+            devDescs = Optional.ofNullable(descs.get(pid));
+        }
+        // DeviceDescriptions is concurrent access-safe
+        return devDescs
+            .map(dd -> dd.getPortDescs().values().stream()
+                                             .map(Timestamped::value))
+            .orElse(Stream.empty());
+    }
+
+    @Override
     public DeviceEvent updatePortStatistics(ProviderId providerId, DeviceId deviceId,
                                             Collection<PortStatistics> newStatsCollection) {
@@ -926,6 +947,26 @@ public class GossipDeviceStore
     }
     @Override
+    public PortDescription getPortDescription(ProviderId pid,
+                                              DeviceId deviceId,
+                                              PortNumber portNumber) {
+        Map<ProviderId, DeviceDescriptions> descs = this.deviceDescs.get(deviceId);
+        if (descs == null) {
+            return null;
+        }
+        // inner-Map(=descs) is HashMap, thus requires synchronization even for reads
+        final Optional<DeviceDescriptions> devDescs;
+        synchronized (descs) {
+            devDescs = Optional.ofNullable(descs.get(pid));
+        }
+        // DeviceDescriptions is concurrent access-safe
+        return devDescs
+                .map(deviceDescriptions -> deviceDescriptions.getPortDesc(portNumber))
+                .map(Timestamped::value)
+                .orElse(null);
+    }
+
+    @Override
     public boolean isAvailable(DeviceId deviceId) {
         return availableDevices.contains(deviceId);
     }