Managers refactored to use ConfigOperators:

o Device, Host, and LinkManagers now use respecive ConfigOperator implementations to combine Descriptions with Network Configs. This refactors work done in 3a0cdd57. o Touchups on ConfigOperator impls Change-Id: I735e7e6bfa0c47c8727433aab804cb2782eebfff

Managers refactored to use ConfigOperators:
o Device, Host, and LinkManagers now use respecive ConfigOperator implementations to combine Descriptions with Network Configs. This refactors work done in 3a0cdd57. o Touchups on ConfigOperator impls Change-Id: I735e7e6bfa0c47c8727433aab804cb2782eebfff
Ayaka Koshibe · Gerrit Code Review
Commit 5373e76dc1ad5c723e0ecc6a3d00b77de25c5668 5373e76d 1 parent 7c4d1966
Showing 5 changed files with 15 additions and 119 deletions
core/net/src/main/java/org/onosproject/net/device/impl/BasicDeviceOperator.java
core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java
core/net/src/main/java/org/onosproject/net/host/impl/HostManager.java
core/net/src/main/java/org/onosproject/net/link/impl/BasicLinkOperator.java
core/net/src/main/java/org/onosproject/net/link/impl/LinkManager.java
--- a/core/net/src/main/java/org/onosproject/net/device/impl/BasicDeviceOperator.java
View file @5373e76
+++ b/core/net/src/main/java/org/onosproject/net/device/impl/BasicDeviceOperator.java
View file @5373e76
@@ -17,6 +17,7 @@ package org.onosproject.net.device.impl;
 import static org.slf4j.LoggerFactory.getLogger;
+import org.onosproject.incubator.net.config.ConfigOperator;
 import org.onosproject.incubator.net.config.basics.BasicDeviceConfig;
 import org.onosproject.net.AnnotationKeys;
 import org.onosproject.net.DefaultAnnotations;
@@ -30,7 +31,7 @@ import org.slf4j.Logger;
  * Implementations of merge policies for various sources of device configuration
  * information. This includes applications, provides, and network configurations.
  */
-public final class BasicDeviceOperator {
+public final class BasicDeviceOperator implements ConfigOperator {
     protected static final double DEFAULT_COORD = -1.0;
     private static final Logger log = getLogger(BasicDeviceOperator.class);
--- a/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java
View file @5373e76
+++ b/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java
View file @5373e76
@@ -37,14 +37,12 @@ import org.onosproject.mastership.MastershipListener;
 import org.onosproject.mastership.MastershipService;
 import org.onosproject.mastership.MastershipTerm;
 import org.onosproject.mastership.MastershipTermService;
-import org.onosproject.net.DefaultAnnotations;
 import org.onosproject.net.Device;
 import org.onosproject.net.Device.Type;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.MastershipRole;
 import org.onosproject.net.Port;
 import org.onosproject.net.PortNumber;
-import org.onosproject.net.SparseAnnotations;
 import org.onosproject.net.device.DefaultDeviceDescription;
 import org.onosproject.net.device.DefaultPortDescription;
 import org.onosproject.net.device.DeviceAdminService;
@@ -334,17 +332,8 @@ public class DeviceManager
             BasicDeviceConfig cfg = networkConfigService.getConfig(deviceId, BasicDeviceConfig.class);
             checkState(cfg == null || cfg.isAllowed(), "Device " + deviceId + " is not allowed");
             log.info("Device {} connected", deviceId);
-            if (cfg != null) {
+
-                SparseAnnotations finalSparse = processAnnotations(cfg, deviceDescription, deviceId);
+            return BasicDeviceOperator.combine(cfg, deviceDescription);
-                if (cfg.type() != Type.SWITCH) {
-                    deviceDescription = new DefaultDeviceDescription(deviceDescription,
-                                                                     cfg.type(), finalSparse);
-                } else {
-                    deviceDescription = new DefaultDeviceDescription(deviceDescription,
-                                                                     deviceDescription.type(), finalSparse);
-                }
-            }
-            return deviceDescription;
         }
         @Override
@@ -500,37 +489,6 @@ public class DeviceManager
                                                            deviceId, portStatistics);
             post(event);
         }
-
-        // supplements or replaces deviceDescription annotations with
-        // BasicDeviceConfig annotations
-        private SparseAnnotations processAnnotations(BasicDeviceConfig cfg, DeviceDescription deviceDescription,
-                                                     DeviceId deviceId) {
-            SparseAnnotations originalAnnotations = deviceDescription.annotations();
-            DefaultAnnotations.Builder newBuilder = DefaultAnnotations.builder();
-            if (cfg.driver() != deviceId.toString()) {
-                newBuilder.set(cfg.DRIVER, cfg.driver());
-            }
-            if (cfg.type() != Type.SWITCH) {
-                newBuilder.set(cfg.TYPE, cfg.type().toString());
-            }
-            if (cfg.name() != null) {
-                newBuilder.set(cfg.NAME, cfg.name());
-            }
-            if (cfg.latitude() != -1) {
-                newBuilder.set(cfg.LATITUDE, Double.toString(cfg.latitude()));
-            }
-            if (cfg.longitude() != -1) {
-                newBuilder.set(cfg.LONGITUDE, Double.toString(cfg.longitude()));
-            }
-            if (cfg.rackAddress() != null) {
-                newBuilder.set(cfg.RACK_ADDRESS, cfg.rackAddress());
-            }
-            if (cfg.owner() != null) {
-                newBuilder.set(cfg.OWNER, cfg.owner());
-            }
-            DefaultAnnotations newAnnotations = newBuilder.build();
-            return DefaultAnnotations.union(originalAnnotations, newAnnotations);
-        }
     }
     // Applies the specified role to the device; ignores NONE
--- a/core/net/src/main/java/org/onosproject/net/host/impl/HostManager.java
View file @5373e76
+++ b/core/net/src/main/java/org/onosproject/net/host/impl/HostManager.java
View file @5373e76
@@ -31,13 +31,10 @@ import org.onosproject.incubator.net.config.NetworkConfigListener;
 import org.onosproject.incubator.net.config.NetworkConfigService;
 import org.onosproject.incubator.net.config.basics.BasicHostConfig;
 import org.onosproject.net.ConnectPoint;
-import org.onosproject.net.DefaultAnnotations;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.Host;
 import org.onosproject.net.HostId;
-import org.onosproject.net.SparseAnnotations;
 import org.onosproject.net.device.DeviceService;
-import org.onosproject.net.host.DefaultHostDescription;
 import org.onosproject.net.host.HostAdminService;
 import org.onosproject.net.host.HostDescription;
 import org.onosproject.net.host.HostEvent;
@@ -246,14 +243,8 @@ public class HostManager
         private HostDescription validateHost(HostDescription hostDescription, HostId hostId) {
             BasicHostConfig cfg = networkConfigService.getConfig(hostId, BasicHostConfig.class);
             checkState(cfg == null || cfg.isAllowed(), "Host {} is not allowed", hostId);
-            if (cfg != null) {
+
-                SparseAnnotations finalSparse = processAnnotations(cfg, hostDescription);
+            return BasicHostOperator.combine(cfg, hostDescription);
-                hostDescription = new DefaultHostDescription(hostId.mac(),
-                                                             hostDescription.vlan(),
-                                                             hostDescription.location(),
-                                                             finalSparse);
-            }
-            return hostDescription;
         }
         @Override
@@ -267,30 +258,6 @@ public class HostManager
         }
     }
-    // Supplements or replaces hostDescriptions's annotations with BasicHostConfig's
-    // annotations
-    private SparseAnnotations processAnnotations(BasicHostConfig cfg, HostDescription hostDescription) {
-        SparseAnnotations originalAnnotations = hostDescription.annotations();
-        DefaultAnnotations.Builder newBuilder = DefaultAnnotations.builder();
-        if (cfg.name() != null) {
-            newBuilder.set(cfg.NAME, cfg.name());
-        }
-        if (cfg.latitude() != -1) {
-            newBuilder.set(cfg.LATITUDE, Double.toString(cfg.latitude()));
-        }
-        if (cfg.longitude() != -1) {
-            newBuilder.set(cfg.LONGITUDE, Double.toString(cfg.longitude()));
-        }
-        if (cfg.rackAddress() != null) {
-            newBuilder.set(cfg.RACK_ADDRESS, cfg.rackAddress());
-        }
-        if (cfg.owner() != null) {
-            newBuilder.set(cfg.OWNER, cfg.owner());
-        }
-        DefaultAnnotations newAnnotations = newBuilder.build();
-        return DefaultAnnotations.union(originalAnnotations, newAnnotations);
-    }
-
     // Store delegate to re-post events emitted from the store.
     private class InternalStoreDelegate implements HostStoreDelegate {
         @Override
--- a/core/net/src/main/java/org/onosproject/net/link/impl/BasicLinkOperator.java
View file @5373e76
+++ b/core/net/src/main/java/org/onosproject/net/link/impl/BasicLinkOperator.java
View file @5373e76
@@ -20,6 +20,7 @@ import static org.slf4j.LoggerFactory.getLogger;
 import java.time.Duration;
 import org.onosproject.net.AnnotationKeys;
+import org.onosproject.incubator.net.config.ConfigOperator;
 import org.onosproject.incubator.net.config.basics.BasicLinkConfig;
 import org.onosproject.net.DefaultAnnotations;
 import org.onosproject.net.Link;
@@ -32,8 +33,10 @@ import org.slf4j.Logger;
  * Implementations of merge policies for various sources of link configuration
  * information. This includes applications, provides, and network configurations.
  */
-public final class BasicLinkOperator {
+public final class BasicLinkOperator implements ConfigOperator {
+    private static final long DEF_BANDWIDTH = -1L;
+    private static final Duration DEF_DURATION = Duration.ofNanos(-1L);
     private static final Logger log = getLogger(BasicLinkOperator.class);
     private BasicLinkOperator() {
@@ -72,10 +75,10 @@ public final class BasicLinkOperator {
      */
     public static SparseAnnotations combine(BasicLinkConfig cfg, SparseAnnotations an) {
         DefaultAnnotations.Builder b = DefaultAnnotations.builder();
-        if (cfg.latency() != Duration.ofNanos(-1)) {
+        if (cfg.latency() != DEF_DURATION) {
             b.set(AnnotationKeys.LATENCY, cfg.latency().toString());
         }
-        if (cfg.bandwidth() != -1) {
+        if (cfg.bandwidth() != DEF_BANDWIDTH) {
             b.set(AnnotationKeys.BANDWIDTH, String.valueOf(cfg.bandwidth()));
         }
         return DefaultAnnotations.union(an, b.build());
--- a/core/net/src/main/java/org/onosproject/net/link/impl/LinkManager.java
View file @5373e76
+++ b/core/net/src/main/java/org/onosproject/net/link/impl/LinkManager.java
View file @5373e76
@@ -31,17 +31,14 @@ import org.onosproject.incubator.net.config.NetworkConfigListener;
 import org.onosproject.incubator.net.config.NetworkConfigService;
 import org.onosproject.incubator.net.config.basics.BasicLinkConfig;
 import org.onosproject.net.ConnectPoint;
-import org.onosproject.net.DefaultAnnotations;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.Link;
 import org.onosproject.net.Link.State;
 import org.onosproject.net.LinkKey;
 import org.onosproject.net.MastershipRole;
-import org.onosproject.net.SparseAnnotations;
 import org.onosproject.net.device.DeviceEvent;
 import org.onosproject.net.device.DeviceListener;
 import org.onosproject.net.device.DeviceService;
-import org.onosproject.net.link.DefaultLinkDescription;
 import org.onosproject.net.link.LinkAdminService;
 import org.onosproject.net.link.LinkDescription;
 import org.onosproject.net.link.LinkEvent;
@@ -55,7 +52,6 @@ import org.onosproject.net.link.LinkStoreDelegate;
 import org.onosproject.net.provider.AbstractProviderService;
 import org.slf4j.Logger;
-import java.time.Duration;
 import java.util.Set;
 import static com.google.common.base.Preconditions.checkNotNull;
@@ -206,6 +202,7 @@ public class LinkManager
         removeLinks(getDeviceLinks(deviceId), false);
     }
+    @Override
     public void removeLink(ConnectPoint src, ConnectPoint dst) {
         post(store.removeLink(src, dst));
     }
@@ -264,38 +261,8 @@ public class LinkManager
             checkState(cfg == null || cfg.isAllowed(), "Link " + linkDescription.toString() + " is not allowed");
             checkState(cfgTwo == null || cfgTwo.isAllowed(), "Link " + linkDescription.toString() + " is not allowed");
-            if (cfg != null) {
+
-                SparseAnnotations finalSparse = processAnnotations(cfg, linkDescription);
+            return BasicLinkOperator.combine(cfg, linkDescription);
-                // check whether config has a specified type
-                if (cfg.type() != Link.Type.DIRECT) {
-                    linkDescription = new DefaultLinkDescription(linkDescription.src(),
-                                                                 linkDescription.dst(),
-                                                                 cfg.type(), finalSparse);
-                } else {
-                    linkDescription = new DefaultLinkDescription(linkDescription.src(),
-                                                                 linkDescription.dst(),
-                                                                 linkDescription.type(), finalSparse);
-                }
-            }
-            return linkDescription;
-        }
-
-        // supplements or replaces linkDescriptions's annotations with BasicLinkConfig's
-        // annotations
-        private SparseAnnotations processAnnotations(BasicLinkConfig cfg, LinkDescription linkDescription) {
-            SparseAnnotations originalAnnotations = linkDescription.annotations();
-            DefaultAnnotations.Builder newBuilder = DefaultAnnotations.builder();
-            if (cfg.type() != Link.Type.DIRECT) {
-                newBuilder.set(cfg.TYPE, cfg.type().toString());
-            }
-            if (cfg.latency() != Duration.ofNanos(-1)) {
-                newBuilder.set(cfg.LATENCY, cfg.latency().toString());
-            }
-            if (cfg.bandwidth() != -1) {
-                newBuilder.set(cfg.BANDWIDTH, String.valueOf(cfg.bandwidth()));
-            }
-            DefaultAnnotations newAnnotations = newBuilder.build();
-            return DefaultAnnotations.union(originalAnnotations, newAnnotations);
         }
         @Override