[ONOS-4691] Refactoring OpticalPortOperator (2/3)

- Define ConfigOperator for a Port
- Refactor OpticalPortOperator as PortConfigOperator
- Add plug-in mechanism for PortConfigOperator on DeviceManager
- Move OpticalPortConfig, OpticalPortOperator to optical-model bundle

Change-Id: I5d416305b0c1b0e31e0ad64baa92d126303548bc

@@ -4,7 +4,7 @@ COMPILE_DEPS = [
 ]
 
 TEST_DEPS = [
-    '//lib:TEST',
+    '//lib:TEST_ADAPTERS',
 ]
 
 

@@ -47,6 +47,13 @@
 
         <dependency>
             <groupId>org.onosproject</groupId>
+            <artifactId>onos-api</artifactId>
+            <classifier>tests</classifier>
+            <scope>test</scope>
+        </dependency>
+
+        <dependency>
+            <groupId>org.onosproject</groupId>
             <artifactId>onlab-junit</artifactId>
             <scope>test</scope>
         </dependency>

@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.onosproject.net.config.basics;
+package org.onosproject.net.optical.config;
 
 import java.util.Optional;
 
@@ -30,6 +30,7 @@ import static org.onosproject.net.config.Config.FieldPresence.OPTIONAL;
  * Configurations for an optical port on a device.
  */
 public final class OpticalPortConfig extends Config<ConnectPoint> {
+
     // optical type {OMS, OCH, ODUClt, fiber}
     public static final String TYPE = "type";
 

@@ -13,17 +13,26 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.onosproject.net.device.impl;
+package org.onosproject.net.optical.config;
 
 import static org.onosproject.net.optical.device.OchPortHelper.ochPortDescription;
 import static org.onosproject.net.optical.device.OduCltPortHelper.oduCltPortDescription;
 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 org.onosproject.net.config.ConfigOperator;
-import org.onosproject.net.config.basics.OpticalPortConfig;
+
+import java.util.Set;
+
+import static com.google.common.base.MoreObjects.firstNonNull;
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import org.onosproject.net.config.NetworkConfigService;
+import org.onosproject.net.config.PortConfigOperator;
 import org.onosproject.net.AnnotationKeys;
+import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.DefaultAnnotations;
+import org.onosproject.net.Port;
+import org.onosproject.net.Port.Type;
 import org.onosproject.net.PortNumber;
 import org.onosproject.net.SparseAnnotations;
 import org.onosproject.net.device.DefaultPortDescription;
@@ -34,63 +43,85 @@ import org.onosproject.net.device.OtuPortDescription;
 import org.onosproject.net.device.PortDescription;
 import org.slf4j.Logger;
 
+import com.google.common.collect.Sets;
+
 /**
  * Implementations of merge policies for various sources of optical port
  * configuration information. This includes applications, provides, and network
  * configurations.
  */
-public final class OpticalPortOperator implements ConfigOperator {
+public final class OpticalPortOperator implements PortConfigOperator {
 
     private static final Logger log = getLogger(OpticalPortOperator.class);
 
-    private OpticalPortOperator() {
+    /**
+     * Port.Type this PortConfigOperator reacts on.
+     */
+    private final Set<Port.Type> optical = Sets.immutableEnumSet(Port.Type.ODUCLT,
+                                                                 Port.Type.OMS,
+                                                                 Port.Type.OCH,
+                                                                 Port.Type.OTU,
+                                                                 Port.Type.FIBER,
+                                                                 Port.Type.PACKET);
+
+    private NetworkConfigService networkConfigService;
+
+
+    public OpticalPortOperator() {
+    }
+
+    @Override
+    public void bindService(NetworkConfigService networkConfigService) {
+        this.networkConfigService = networkConfigService;
+    }
+
+    private OpticalPortConfig lookupConfig(ConnectPoint cp) {
+        if (networkConfigService == null) {
+            return null;
+        }
+        return networkConfigService.getConfig(cp, OpticalPortConfig.class);
     }
 
     /**
      * Generates a PortDescription containing fields from a PortDescription and
      * an OpticalPortConfig.
      *
-     * @param opc the port config entity from network config
-     * @param descr a PortDescription
-     * @return PortDescription based on both sources
+     * @param cp {@link ConnectPoint} representing the port.
+     * @param descr input {@link PortDescription}
+     * @return Combined {@link PortDescription}
      */
-    public static PortDescription combine(OpticalPortConfig opc, PortDescription descr) {
+    @Override
+    public PortDescription combine(ConnectPoint cp, PortDescription descr) {
+        checkNotNull(cp);
+
+        // short-circuit for non-optical ports
+        // must be removed if we need type override
+        if (descr != null && !optical.contains(descr.type())) {
+            return descr;
+        }
+
+        OpticalPortConfig opc = lookupConfig(cp);
         if (opc == null) {
             return descr;
         }
 
-        PortNumber port = descr.portNumber();
-        final String name = opc.name();
-        final String numName = opc.numberName();
-        // if the description is null, or the current description port name != config name,
-        // create a new PortNumber.
-        PortNumber newPort = null;
-        if (port == null) {
-            // try to get the portNumber from the numName.
-            if (!numName.isEmpty()) {
-                final long pn = Long.parseLong(numName);
-                newPort = (!name.isEmpty()) ? PortNumber.portNumber(pn, name) : PortNumber.portNumber(pn);
-            } else {
-                // we don't have defining info (a port number value)
-                throw new RuntimeException("Possible misconfig, bailing on handling for: \n\t" + descr);
-            }
-        } else if ((!name.isEmpty()) && !name.equals(port.name())) {
-            final long pn = (numName.isEmpty()) ? port.toLong() : Long.parseLong(numName);
-            newPort = PortNumber.portNumber(pn, name);
+        PortNumber number = descr.portNumber();
+        // handle PortNumber "name" portion
+        if (!opc.name().isEmpty()) {
+            number = PortNumber.portNumber(descr.portNumber().toLong(), opc.name());
         }
 
-        // Port type won't change unless we're overwriting a port completely.
-        // Watch out for overwrites to avoid class cast craziness.
-        boolean noOwrite = opc.type() == descr.type();
+        // handle additional annotations
+        SparseAnnotations annotations = combine(opc, descr.annotations());
 
-        SparseAnnotations sa = combine(opc, descr.annotations());
-        if (noOwrite) {
-            return updateDescription((newPort == null) ? port : newPort, sa, descr);
-        } else {
-            // TODO: must reconstruct a different type of PortDescription.
-            log.info("Type rewrite from {} to {} required", descr.type(), opc.type());
+        // (Future work) handle type overwrite?
+        Type type = firstNonNull(opc.type(), descr.type());
+        if (type != descr.type()) {
+            // TODO: Do we need to be able to overwrite Port.Type?
+            log.warn("Port type overwrite requested for {}. Ignoring.", cp);
         }
-        return descr;
+
+        return updateDescription(number, annotations, descr);
     }
 
     // updates a port description whose port type has not changed.
@@ -161,8 +192,9 @@ public final class OpticalPortOperator implements ConfigOperator {
      * @param an the annotation
      * @return annotation combining both sources
      */
-    public static SparseAnnotations combine(OpticalPortConfig opc, SparseAnnotations an) {
+    private static SparseAnnotations combine(OpticalPortConfig opc, SparseAnnotations an) {
         DefaultAnnotations.Builder b = DefaultAnnotations.builder();
+        b.putAll(an);
         if (!opc.staticPort().isEmpty()) {
             b.set(AnnotationKeys.STATIC_PORT, opc.staticPort());
         }
@@ -173,7 +205,8 @@ public final class OpticalPortOperator implements ConfigOperator {
         if (!opc.name().isEmpty()) {
             b.set(AnnotationKeys.PORT_NAME, opc.name());
         }
-        return DefaultAnnotations.union(an, b.build());
+        return b.build();
     }
 
+
 }

+/*
+ * Copyright 2016-present Open Networking Laboratory
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+/**
+ * Various optical model related configurations.
+ */
+package org.onosproject.net.optical.config;

+/*
+ * Copyright 2016-present Open Networking Laboratory
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.onosproject.net.optical.internal;
+
+import static org.onosproject.net.config.basics.SubjectFactories.CONNECT_POINT_SUBJECT_FACTORY;
+
+import org.apache.felix.scr.annotations.Activate;
+import org.apache.felix.scr.annotations.Component;
+import org.apache.felix.scr.annotations.Deactivate;
+import org.apache.felix.scr.annotations.Reference;
+import org.apache.felix.scr.annotations.ReferenceCardinality;
+import org.onosproject.net.ConnectPoint;
+import org.onosproject.net.config.ConfigFactory;
+import org.onosproject.net.config.NetworkConfigRegistry;
+import org.onosproject.net.config.PortConfigOperatorRegistry;
+import org.onosproject.net.optical.config.OpticalPortConfig;
+import org.onosproject.net.optical.config.OpticalPortOperator;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Loader which registers optical model related config, etc.
+ */
+@Component(immediate = true)
+public class OpticalModelLoader {
+
+    private final Logger log = LoggerFactory.getLogger(getClass());
+
+    @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
+    protected PortConfigOperatorRegistry portOperatorRegistry;
+
+    @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
+    protected NetworkConfigRegistry netcfgRegistry;
+
+
+    private OpticalPortOperator opticalPortOp;
+
+    private ConfigFactory<ConnectPoint, OpticalPortConfig>
+        opticalPortConfigFactory = new ConfigFactory<ConnectPoint, OpticalPortConfig>(CONNECT_POINT_SUBJECT_FACTORY,
+                                                       OpticalPortConfig.class,
+                                                       "optical") {
+        @Override
+        public OpticalPortConfig createConfig() {
+            return new OpticalPortConfig();
+        }
+    };
+
+    @Activate
+    protected void activate() {
+        netcfgRegistry.registerConfigFactory(opticalPortConfigFactory);
+
+        opticalPortOp = new OpticalPortOperator();
+        portOperatorRegistry.registerPortConfigOperator(opticalPortOp,
+                                                        OpticalPortConfig.class);
+
+        log.info("Started");
+    }
+
+    @Deactivate
+    protected void deactivate() {
+        portOperatorRegistry.unregisterPortConfigOperator(opticalPortOp);
+
+        netcfgRegistry.unregisterConfigFactory(opticalPortConfigFactory);
+        log.info("Stopped");
+    }
+}

+/*
+ * Copyright 2016-present Open Networking Laboratory
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+/**
+ * Internal tools for optical model.
+ */
+package org.onosproject.net.optical.internal;

@@ -13,15 +13,15 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.onosproject.incubator.net.config.basics;
+package org.onosproject.net.optical.config;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
-import static org.onosproject.net.config.basics.OpticalPortConfig.TYPE;
-import static org.onosproject.net.config.basics.OpticalPortConfig.NAME;
-import static org.onosproject.net.config.basics.OpticalPortConfig.PORT;
-import static org.onosproject.net.config.basics.OpticalPortConfig.STATIC_LAMBDA;
-import static org.onosproject.net.config.basics.OpticalPortConfig.STATIC_PORT;
+import static org.onosproject.net.optical.config.OpticalPortConfig.NAME;
+import static org.onosproject.net.optical.config.OpticalPortConfig.PORT;
+import static org.onosproject.net.optical.config.OpticalPortConfig.STATIC_LAMBDA;
+import static org.onosproject.net.optical.config.OpticalPortConfig.STATIC_PORT;
+import static org.onosproject.net.optical.config.OpticalPortConfig.TYPE;
 
 import java.io.IOException;
 import java.util.Iterator;
@@ -41,7 +41,6 @@ import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.node.JsonNodeFactory;
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.google.common.collect.Lists;
-import org.onosproject.net.config.basics.OpticalPortConfig;
 
 public class OpticalPortConfigTest {
     private static final String FIELD = "ports";

@@ -13,14 +13,14 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.onosproject.net.device.impl;
+package org.onosproject.net.optical.config;
 
 import org.junit.Before;
 import org.junit.Test;
 import org.onosproject.net.CltSignalType;
 import org.onosproject.net.config.Config;
 import org.onosproject.net.config.ConfigApplyDelegate;
-import org.onosproject.net.config.basics.OpticalPortConfig;
+import org.onosproject.net.config.NetworkConfigServiceAdapter;
 import org.onosproject.net.AnnotationKeys;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.DefaultAnnotations;
@@ -29,6 +29,7 @@ import org.onosproject.net.Port;
 import org.onosproject.net.PortNumber;
 import org.onosproject.net.SparseAnnotations;
 import org.onosproject.net.device.PortDescription;
+
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.node.JsonNodeFactory;
 
@@ -62,27 +63,33 @@ public class OpticalPortOperatorTest {
 
     private static final ConnectPoint CP = new ConnectPoint(DID, UNNAMED);
 
-    private static final OpticalPortConfig OPC = new OpticalPortConfig();
+    private OpticalPortConfig opc;
+
+    private OpticalPortOperator oper;
 
     @Before
     public void setUp() {
-        OPC.init(CP, CFG_KEY, JsonNodeFactory.instance.objectNode(), mapper, delegate);
+        opc = new OpticalPortConfig();
+        opc.init(CP, CFG_KEY, JsonNodeFactory.instance.objectNode(), mapper, delegate);
+
+        oper = new OpticalPortOperator();
+        oper.bindService(new MockNetworkConfigService());
     }
 
     @Test
     public void testConfigPortName() {
-        OPC.portType(Port.Type.ODUCLT)
+        opc.portType(Port.Type.ODUCLT)
             .portNumberName(PORT_NUMBER)
             .portName(CFG_PORT_NAME);
 
         PortDescription res;
         // full desc + opc with name
-        res = OpticalPortOperator.combine(OPC, N_DESC);
+        res = oper.combine(CP, N_DESC);
         assertEquals("Configured port name expected",
                      CFG_PORT_NAME, res.portNumber().name());
         assertEquals(DESC_STATIC_PORT, res.annotations().value(AnnotationKeys.STATIC_PORT));
 
-        res = OpticalPortOperator.combine(OPC, U_DESC);
+        res = oper.combine(CP, U_DESC);
         assertEquals("Configured port name expected",
                      CFG_PORT_NAME, res.portNumber().name());
         assertEquals(DESC_STATIC_PORT, res.annotations().value(AnnotationKeys.STATIC_PORT));
@@ -90,12 +97,12 @@ public class OpticalPortOperatorTest {
 
     @Test
     public void testConfigAddStaticLambda() {
-        OPC.portType(Port.Type.ODUCLT)
+        opc.portType(Port.Type.ODUCLT)
             .portNumberName(PORT_NUMBER)
             .staticLambda(CFG_STATIC_LAMBDA);
 
         PortDescription res;
-        res = OpticalPortOperator.combine(OPC, N_DESC);
+        res = oper.combine(CP, N_DESC);
         assertEquals("Original port name expected",
                      DESC_PORT_NAME, res.portNumber().name());
         assertEquals(DESC_STATIC_PORT, res.annotations().value(AnnotationKeys.STATIC_PORT));
@@ -105,17 +112,31 @@ public class OpticalPortOperatorTest {
 
     @Test
     public void testEmptyConfig() {
-        OPC.portType(Port.Type.ODUCLT)
+        opc.portType(Port.Type.ODUCLT)
             .portNumberName(PORT_NUMBER);
 
         PortDescription res;
-        res = OpticalPortOperator.combine(OPC, N_DESC);
+        res = oper.combine(CP, N_DESC);
         assertEquals("Configured port name expected",
                      DESC_PORT_NAME, res.portNumber().name());
         assertEquals(DESC_STATIC_PORT, res.annotations().value(AnnotationKeys.STATIC_PORT));
     }
 
 
+    private class MockNetworkConfigService
+            extends NetworkConfigServiceAdapter {
+
+        @Override
+        public <S, C extends Config<S>> C getConfig(S subject,
+                                                    Class<C> configClass) {
+            if (configClass == OpticalPortConfig.class) {
+                return (C) opc;
+            }
+            return null;
+        }
+    }
+
+
     private class MockCfgDelegate implements ConfigApplyDelegate {
 
         @Override

+/*
+ * Copyright 2016-present Open Networking Laboratory
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.onosproject.net.config;
+
+import org.onosproject.net.ConnectPoint;
+import org.onosproject.net.DeviceId;
+import org.onosproject.net.device.PortDescription;
+
+import com.google.common.annotations.Beta;
+
+/**
+ * {@link ConfigOperator} for Port.
+ * <p>
+ * Note: We currently assumes {@link PortConfigOperator}s are commutative.
+ */
+@Beta
+public interface PortConfigOperator extends ConfigOperator {
+
+    /**
+     * Binds {@link NetworkConfigService} to use for retrieving configuration.
+     *
+     * @param networkConfigService the service to use
+     */
+    void bindService(NetworkConfigService networkConfigService);
+
+
+    /**
+     * Generates a PortDescription containing fields from a PortDescription and
+     * configuration.
+     *
+     * @param cp {@link ConnectPoint} representing the port.
+     * @param descr input {@link PortDescription}
+     * @return Combined {@link PortDescription}
+     */
+    PortDescription combine(ConnectPoint cp, PortDescription descr);
+
+    /**
+     * Generates a PortDescription containing fields from a PortDescription and
+     * configuration.
+     *
+     * @param did DeviceId which the port described by {@code descr} resides.
+     * @param descr input {@link PortDescription}
+     * @return Combined {@link PortDescription}
+     */
+    default PortDescription combine(DeviceId did, PortDescription descr) {
+        return combine(new ConnectPoint(did, descr.portNumber()), descr);
+    }
+
+}

+/*
+ * Copyright 2016-present Open Networking Laboratory
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.onosproject.net.config;
+
+import org.onosproject.net.ConnectPoint;
+
+import com.google.common.annotations.Beta;
+
+/**
+ * Abstraction of a port operator registry.
+ */
+@Beta
+public interface PortConfigOperatorRegistry {
+
+    /**
+     * Registers {@link PortConfigOperator} instance.
+     *
+     * @param portOp {@link PortConfigOperator} instance.
+     * @param configs {@link Config} class for a Port referred by {@code portOp}
+     */
+    void registerPortConfigOperator(PortConfigOperator portOp,
+                                    Class<? extends Config<ConnectPoint>>... configs);
+
+    /**
+     * Unregisters {@link PortConfigOperator} instance.
+     *
+     * @param portOp {@link PortConfigOperator} instance.
+     */
+    void unregisterPortConfigOperator(PortConfigOperator portOp);
+
+}

@@ -105,9 +105,7 @@
         </dependency>
 
         <!-- TODO Remove after decoupling optical -->
-        <!-- - DeviceManager.InternalDeviceProviderService#ensureGeneric -->
         <!-- - OpticalCompilers x4 -->
-        <!-- - OpticalPortOperator -->
         <dependency>
             <groupId>org.onosproject</groupId>
             <artifactId>onos-optical-model</artifactId>

@@ -34,7 +34,6 @@ import org.onosproject.net.config.NetworkConfigRegistry;
 import org.onosproject.net.config.basics.BasicDeviceConfig;
 import org.onosproject.net.config.basics.BasicHostConfig;
 import org.onosproject.net.config.basics.BasicLinkConfig;
-import org.onosproject.net.config.basics.OpticalPortConfig;
 import org.onosproject.net.config.basics.SubjectFactories;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -85,15 +84,6 @@ public class BasicNetworkConfigs implements BasicNetworkConfigService {
                 public BasicLinkConfig createConfig() {
                     return new BasicLinkConfig();
                 }
-            },
-            // TODO move this optical specific configuration out to optical app
-            new ConfigFactory<ConnectPoint, OpticalPortConfig>(CONNECT_POINT_SUBJECT_FACTORY,
-                                                               OpticalPortConfig.class,
-                                                               "optical") {
-                @Override
-                public OpticalPortConfig createConfig() {
-                    return new OpticalPortConfig();
-                }
             }
     );