Unit tests for NetworkCongifLinksProvider

Also fixed a few bugs in the provider that were turned up by the new tests. Change-Id: Icafc945251b4d35ac3f285302af150c0a4d646d6

Unit tests for NetworkCongifLinksProvider
Also fixed a few bugs in the provider that were turned up by the new tests. Change-Id: Icafc945251b4d35ac3f285302af150c0a4d646d6
Ray Milkey · Gerrit Code Review
Commit cd6ab18c8c14a4a4116816a1454a707b2de54713 cd6ab18c 1 parent 483e29f9
Showing 9 changed files with 260 additions and 100 deletions
core/api/src/test/java/org/onosproject/net/config/NetworkConfigRegistryAdapter.java
core/api/src/test/java/org/onosproject/net/config/NetworkConfigServiceAdapter.java
core/api/src/test/java/org/onosproject/net/link/DefaultLinkDescriptionTest.java
core/api/src/test/java/org/onosproject/net/link/LinkProviderRegistryAdapter.java
core/api/src/test/java/org/onosproject/net/link/LinkProviderServiceAdapter.java
providers/lldp/src/test/java/org/onosproject/provider/lldp/impl/LldpLinkProviderTest.java
providers/netcfglinks/pom.xml
providers/netcfglinks/src/main/java/org/onosproject/provider/netcfglinks/NetworkConfigLinksProvider.java
providers/netcfglinks/src/test/java/org/onosproject/provider/netcfglinks/NetworkConfigLinksProviderTest.java
--- a/core/api/src/test/java/org/onosproject/net/config/NetworkConfigRegistryAdapter.java
View file @cd6ab18
+++ b/core/api/src/test/java/org/onosproject/net/config/NetworkConfigRegistryAdapter.java
View file @cd6ab18
@@ -17,6 +17,8 @@ package org.onosproject.net.config;
 import java.util.Set;
+import com.google.common.collect.ImmutableSet;
+
 /**
  * Test adapter for network configuration service registry.
  */
@@ -29,11 +31,11 @@ public class NetworkConfigRegistryAdapter extends NetworkConfigServiceAdapter im
     }
     public Set<ConfigFactory> getConfigFactories() {
-        return null;
+        return ImmutableSet.of();
     }
     public <S, C extends Config<S>> Set<ConfigFactory<S, C>> getConfigFactories(Class<S> subjectClass) {
-        return null;
+        return ImmutableSet.of();
     }
     public <S, C extends Config<S>> ConfigFactory<S, C> getConfigFactory(Class<C> configClass) {
--- a/core/api/src/test/java/org/onosproject/net/config/NetworkConfigServiceAdapter.java
View file @cd6ab18
+++ b/core/api/src/test/java/org/onosproject/net/config/NetworkConfigServiceAdapter.java
View file @cd6ab18
@@ -16,6 +16,7 @@
 package org.onosproject.net.config;
 import com.fasterxml.jackson.databind.JsonNode;
+import com.google.common.collect.ImmutableSet;
 import java.util.Set;
@@ -25,7 +26,7 @@ import java.util.Set;
 public class NetworkConfigServiceAdapter implements NetworkConfigService {
     @Override
     public Set<Class> getSubjectClasses() {
-        return null;
+        return ImmutableSet.of();
     }
     @Override
@@ -45,17 +46,17 @@ public class NetworkConfigServiceAdapter implements NetworkConfigService {
     @Override
     public <S> Set<S> getSubjects(Class<S> subjectClass) {
-        return null;
+        return ImmutableSet.of();
     }
     @Override
     public <S, C extends Config<S>> Set<S> getSubjects(Class<S> subjectClass, Class<C> configClass) {
-        return null;
+        return ImmutableSet.of();
     }
     @Override
     public <S> Set<? extends Config<S>> getConfigs(S subject) {
-        return null;
+        return ImmutableSet.of();
     }
     @Override
--- a/core/api/src/test/java/org/onosproject/net/link/DefaultLinkDescriptionTest.java
View file @cd6ab18
+++ b/core/api/src/test/java/org/onosproject/net/link/DefaultLinkDescriptionTest.java
View file @cd6ab18
@@ -16,10 +16,15 @@
 package org.onosproject.net.link;
 import org.junit.Test;
+import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.DefaultAnnotations;
 import org.onosproject.net.DeviceId;
+import org.onosproject.net.Link;
+import org.onosproject.net.NetTestTools;
 import org.onosproject.net.PortNumber;
+import com.google.common.testing.EqualsTester;
+
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 import static org.onosproject.net.DefaultLinkTest.cp;
@@ -44,7 +49,48 @@ public class DefaultLinkDescriptionTest {
         assertEquals("incorrect src", cp(DID1, P1), desc.src());
         assertEquals("incorrect dst", cp(DID2, P1), desc.dst());
         assertEquals("incorrect type", DIRECT, desc.type());
-        assertTrue("incorrect annotatios", desc.toString().contains("Key=Value"));
+        assertTrue("incorrect annotations", desc.toString().contains("Key=Value"));
     }
+    /**
+     * Tests the equals(), hashCode() and toString() methods.
+     */
+    @Test
+    public void testEquals() {
+        ConnectPoint connectPoint1 = NetTestTools.connectPoint("sw1", 1);
+        ConnectPoint connectPoint2 = NetTestTools.connectPoint("sw2", 2);
+        ConnectPoint connectPoint3 = NetTestTools.connectPoint("sw3", 3);
+
+        DefaultLinkDescription link1 =
+                new DefaultLinkDescription(connectPoint1, connectPoint2,
+                                           Link.Type.DIRECT);
+        DefaultLinkDescription sameAsLink1 =
+                new DefaultLinkDescription(connectPoint1, connectPoint2,
+                                           Link.Type.DIRECT);
+        DefaultLinkDescription link2 =
+                new DefaultLinkDescription(connectPoint1, connectPoint2,
+                                           Link.Type.INDIRECT);
+        DefaultLinkDescription link3 =
+                new DefaultLinkDescription(connectPoint1, connectPoint3,
+                                           Link.Type.DIRECT);
+        DefaultLinkDescription link4 =
+                new DefaultLinkDescription(connectPoint2, connectPoint3,
+                                           Link.Type.DIRECT);
+        DefaultLinkDescription link5 =
+                new DefaultLinkDescription(connectPoint1, connectPoint2,
+                                           Link.Type.DIRECT, false);
+        DefaultLinkDescription link6 =
+                new DefaultLinkDescription(connectPoint2, connectPoint3,
+                                           Link.Type.DIRECT, DA);
+
+        new EqualsTester()
+                .addEqualityGroup(link1, sameAsLink1)
+                .addEqualityGroup(link2)
+                .addEqualityGroup(link3)
+                .addEqualityGroup(link4)
+                .addEqualityGroup(link5)
+                .addEqualityGroup(link6)
+                .testEquals();
+
+    }
 }
--- a/core/api/src/test/java/org/onosproject/net/link/LinkProviderRegistryAdapter.java 0 → 100644
View file @cd6ab18
+++ b/core/api/src/test/java/org/onosproject/net/link/LinkProviderRegistryAdapter.java 0 → 100644
View file @cd6ab18
+/*
+ * Copyright 2016 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.link;
+
+import java.util.Set;
+
+import org.onosproject.net.provider.ProviderId;
+
+import com.google.common.collect.ImmutableSet;
+
+/**
+ * Testing adapter for the LinkProviderRegistry API.
+ */
+public class LinkProviderRegistryAdapter implements LinkProviderRegistry {
+
+    LinkProviderServiceAdapter providerService = null;
+
+    @Override
+    public LinkProviderService register(LinkProvider provider) {
+        providerService = new LinkProviderServiceAdapter(provider);
+        return providerService;
+    }
+
+    @Override
+    public void unregister(LinkProvider provider) {
+    }
+
+    @Override
+    public Set<ProviderId> getProviders() {
+        return ImmutableSet.of(providerService.provider().id());
+    }
+
+    public LinkProviderServiceAdapter registeredProvider() {
+        return providerService;
+    }
+}
--- a/core/api/src/test/java/org/onosproject/net/link/LinkProviderServiceAdapter.java 0 → 100644
View file @cd6ab18
+++ b/core/api/src/test/java/org/onosproject/net/link/LinkProviderServiceAdapter.java 0 → 100644
View file @cd6ab18
+/*
+ * Copyright 2016 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.link;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.onosproject.net.ConnectPoint;
+import org.onosproject.net.DeviceId;
+import org.onosproject.net.LinkKey;
+import org.onosproject.net.provider.AbstractProviderService;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+
+public class LinkProviderServiceAdapter
+        extends AbstractProviderService<LinkProvider>
+        implements LinkProviderService {
+
+    List<DeviceId> vanishedDpid = Lists.newLinkedList();
+    List<Long> vanishedPort = Lists.newLinkedList();
+    Map<DeviceId, DeviceId> discoveredLinks = Maps.newHashMap();
+    Map<LinkKey, LinkDescription> discoveredLinkDescriptions = new HashMap<>();
+
+    protected LinkProviderServiceAdapter(LinkProvider provider) {
+        super(provider);
+    }
+
+    @Override
+    public void linkDetected(LinkDescription linkDescription) {
+        LinkKey key = LinkKey.linkKey(linkDescription.src(), linkDescription.dst());
+        discoveredLinkDescriptions.put(key, linkDescription);
+        DeviceId sDid = linkDescription.src().deviceId();
+        DeviceId dDid = linkDescription.dst().deviceId();
+        discoveredLinks.put(sDid, dDid);
+    }
+
+    @Override
+    public void linkVanished(LinkDescription linkDescription) {
+        LinkKey key = LinkKey.linkKey(linkDescription.src(), linkDescription.dst());
+        discoveredLinkDescriptions.remove(key);
+    }
+
+    @Override
+    public void linksVanished(ConnectPoint connectPoint) {
+        vanishedPort.add(connectPoint.port().toLong());
+
+    }
+
+    @Override
+    public void linksVanished(DeviceId deviceId) {
+        vanishedDpid.add(deviceId);
+    }
+
+    public List<DeviceId> vanishedDpid() {
+        return vanishedDpid;
+    }
+
+    public List<Long> vanishedPort() {
+        return vanishedPort;
+    }
+
+    public Map<DeviceId, DeviceId> discoveredLinks() {
+        return discoveredLinks;
+    }
+
+    public Map<LinkKey, LinkDescription> discoveredLinkDescriptions() {
+        return discoveredLinkDescriptions;
+    }
+}
--- a/providers/lldp/src/test/java/org/onosproject/provider/lldp/impl/LldpLinkProviderTest.java
View file @cd6ab18
+++ b/providers/lldp/src/test/java/org/onosproject/provider/lldp/impl/LldpLinkProviderTest.java
View file @cd6ab18
@@ -15,13 +15,14 @@
  */
 package org.onosproject.provider.lldp.impl;
-import com.google.common.collect.ArrayListMultimap;
+import java.nio.ByteBuffer;
-import com.google.common.collect.ImmutableList;
+import java.util.Collections;
-import com.google.common.collect.ImmutableMap;
+import java.util.HashMap;
-import com.google.common.collect.ImmutableSet;
+import java.util.HashSet;
-import com.google.common.collect.Lists;
+import java.util.List;
-import com.google.common.collect.Maps;
+import java.util.Map;
-import com.google.common.collect.Sets;
+import java.util.Set;
+import java.util.concurrent.CompletableFuture;
 import org.junit.After;
 import org.junit.Before;
@@ -65,10 +66,8 @@ import org.onosproject.net.device.DeviceEvent;
 import org.onosproject.net.device.DeviceListener;
 import org.onosproject.net.device.DeviceServiceAdapter;
 import org.onosproject.net.flow.TrafficTreatment;
-import org.onosproject.net.link.LinkDescription;
+import org.onosproject.net.link.LinkProviderRegistryAdapter;
-import org.onosproject.net.link.LinkProvider;
+import org.onosproject.net.link.LinkProviderServiceAdapter;
-import org.onosproject.net.link.LinkProviderRegistry;
-import org.onosproject.net.link.LinkProviderService;
 import org.onosproject.net.link.LinkServiceAdapter;
 import org.onosproject.net.packet.DefaultInboundPacket;
 import org.onosproject.net.packet.InboundPacket;
@@ -76,27 +75,24 @@ import org.onosproject.net.packet.OutboundPacket;
 import org.onosproject.net.packet.PacketContext;
 import org.onosproject.net.packet.PacketProcessor;
 import org.onosproject.net.packet.PacketServiceAdapter;
-import org.onosproject.net.provider.AbstractProviderService;
 import org.onosproject.net.provider.ProviderId;
-import java.nio.ByteBuffer;
+import com.google.common.collect.ArrayListMultimap;
-import java.util.HashMap;
+import com.google.common.collect.ImmutableList;
-import java.util.HashSet;
+import com.google.common.collect.ImmutableMap;
-import java.util.List;
+import com.google.common.collect.ImmutableSet;
-import java.util.Map;
+import com.google.common.collect.Lists;
-import java.util.Set;
+import com.google.common.collect.Sets;
-import java.util.Collections;
-import java.util.concurrent.CompletableFuture;
 import static org.easymock.EasyMock.createMock;
 import static org.easymock.EasyMock.expect;
 import static org.easymock.EasyMock.replay;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.onosproject.provider.lldp.impl.LldpLinkProvider.DEFAULT_RULES;
-import static org.junit.Assert.assertFalse;
 public class LldpLinkProviderTest {
@@ -111,7 +107,7 @@ public class LldpLinkProviderTest {
     private static Port pd4;
     private final LldpLinkProvider provider = new LldpLinkProvider();
-    private final TestLinkRegistry linkRegistry = new TestLinkRegistry();
+    private final LinkProviderRegistryAdapter linkRegistry = new LinkProviderRegistryAdapter();
     private final TestLinkService linkService = new TestLinkService();
     private final TestPacketService packetService = new TestPacketService();
     private final TestDeviceService deviceService = new TestDeviceService();
@@ -119,7 +115,7 @@ public class LldpLinkProviderTest {
     private final TestNetworkConfigRegistry configRegistry = new TestNetworkConfigRegistry();
     private CoreService coreService;
-    private TestLinkProviderService providerService;
+    private LinkProviderServiceAdapter providerService;
     private PacketProcessor testProcessor;
     private DeviceListener deviceListener;
@@ -154,7 +150,10 @@ public class LldpLinkProviderTest {
         provider.providerRegistry = linkRegistry;
         provider.masterService = masterService;
         provider.clusterMetadataService = new TestMetadataService();
+
         provider.activate(null);
+
+        providerService = linkRegistry.registeredProvider();
     }
     @Test
@@ -521,7 +520,7 @@ public class LldpLinkProviderTest {
     private boolean vanishedDpid(DeviceId... dids) {
         for (int i = 0; i < dids.length; i++) {
-            if (!providerService.vanishedDpid.contains(dids[i])) {
+            if (!providerService.vanishedDpid().contains(dids[i])) {
                 return false;
             }
         }
@@ -530,7 +529,7 @@ public class LldpLinkProviderTest {
     private boolean vanishedPort(Long... ports) {
         for (int i = 0; i < ports.length; i++) {
-            if (!providerService.vanishedPort.contains(ports[i])) {
+            if (!providerService.vanishedPort().contains(ports[i])) {
                 return false;
             }
         }
@@ -538,9 +537,9 @@ public class LldpLinkProviderTest {
     }
     private boolean detectedLink(DeviceId src, DeviceId dst) {
-        for (DeviceId key : providerService.discoveredLinks.keySet()) {
+        for (DeviceId key : providerService.discoveredLinks().keySet()) {
             if (key.equals(src)) {
-                return providerService.discoveredLinks.get(src).equals(dst);
+                return providerService.discoveredLinks().get(src).equals(dst);
             }
         }
         return false;
@@ -631,65 +630,6 @@ public class LldpLinkProviderTest {
                 SuppressionConfig.class));
     }
-
-    private class TestLinkRegistry implements LinkProviderRegistry {
-
-        @Override
-        public LinkProviderService register(LinkProvider provider) {
-            providerService = new TestLinkProviderService(provider);
-            return providerService;
-        }
-
-        @Override
-        public void unregister(LinkProvider provider) {
-        }
-
-        @Override
-        public Set<ProviderId> getProviders() {
-            return null;
-        }
-
-    }
-
-    private class TestLinkProviderService
-            extends AbstractProviderService<LinkProvider>
-            implements LinkProviderService {
-
-        List<DeviceId> vanishedDpid = Lists.newLinkedList();
-        List<Long> vanishedPort = Lists.newLinkedList();
-        Map<DeviceId, DeviceId> discoveredLinks = Maps.newHashMap();
-
-        protected TestLinkProviderService(LinkProvider provider) {
-            super(provider);
-        }
-
-        @Override
-        public void linkDetected(LinkDescription linkDescription) {
-            DeviceId sDid = linkDescription.src().deviceId();
-            DeviceId dDid = linkDescription.dst().deviceId();
-            discoveredLinks.put(sDid, dDid);
-        }
-
-        @Override
-        public void linkVanished(LinkDescription linkDescription) {
-        }
-
-        @Override
-        public void linksVanished(ConnectPoint connectPoint) {
-            vanishedPort.add(connectPoint.port().toLong());
-
-        }
-
-        @Override
-        public void linksVanished(DeviceId deviceId) {
-            vanishedDpid.add(deviceId);
-        }
-
-
-    }
-
-
-
     private class TestPacketContext implements PacketContext {
         protected Device device;
--- a/providers/netcfglinks/pom.xml
View file @cd6ab18
+++ b/providers/netcfglinks/pom.xml
View file @cd6ab18
@@ -56,6 +56,15 @@
             <artifactId>easymock</artifactId>
             <scope>test</scope>
         </dependency>
+
+        <dependency>
+            <groupId>org.onosproject</groupId>
+            <artifactId>onos-api</artifactId>
+            <version>${project.version}</version>
+            <classifier>tests</classifier>
+            <scope>test</scope>
+        </dependency>
+
     </dependencies>
 </project>
--- a/providers/netcfglinks/src/main/java/org/onosproject/provider/netcfglinks/NetworkConfigLinksProvider.java
View file @cd6ab18
+++ b/providers/netcfglinks/src/main/java/org/onosproject/provider/netcfglinks/NetworkConfigLinksProvider.java
View file @cd6ab18
@@ -117,7 +117,7 @@ public class NetworkConfigLinksProvider
     private final InternalDeviceListener deviceListener = new InternalDeviceListener();
     private final InternalConfigListener cfgListener = new InternalConfigListener();
-    private Set<LinkKey> configuredLinks = new HashSet<>();
+    protected Set<LinkKey> configuredLinks = new HashSet<>();
     public NetworkConfigLinksProvider() {
         super(new ProviderId("lldp", PROVIDER_NAME));
@@ -259,6 +259,35 @@ public class NetworkConfigLinksProvider
     }
     /**
+     * Removes after stopping discovery helper for specified device.
+     * @param deviceId device to remove
+     */
+    private void removeDevice(final DeviceId deviceId) {
+        discoverers.computeIfPresent(deviceId, (did, ld) -> {
+            ld.stop();
+            return null;
+        });
+
+    }
+
+    /**
+     * Removes a port from the specified discovery helper.
+     * @param port the port
+     */
+    private void removePort(Port port) {
+        if (port.element() instanceof Device) {
+            Device d = (Device) port.element();
+            LinkDiscovery ld = discoverers.get(d.id());
+            if (ld != null) {
+                ld.removePort(port.number());
+            }
+        } else {
+            log.warn("Attempted to remove non-Device port", port);
+        }
+    }
+
+
+    /**
      * Processes incoming packets.
      */
     private class InternalPacketProcessor implements PacketProcessor {
@@ -355,21 +384,21 @@ public class NetworkConfigLinksProvider
                         updateDevice(device).ifPresent(ld -> updatePort(ld, port));
                     } else {
                         log.debug("Port down {}", port);
-                        //removePort(port);
+                        removePort(port);
                         providerService.linksVanished(new ConnectPoint(port.element().id(),
                                                                        port.number()));
                     }
                     break;
                 case PORT_REMOVED:
                     log.debug("Port removed {}", port);
-                    //removePort(port);
+                    removePort(port);
                     providerService.linksVanished(new ConnectPoint(port.element().id(),
                                                                    port.number()));
                     break;
                 case DEVICE_REMOVED:
                 case DEVICE_SUSPENDED:
                     log.debug("Device removed {}", deviceId);
-                    //removeDevice(deviceId);
+                    removeDevice(deviceId);
                     providerService.linksVanished(deviceId);
                     break;
                 case DEVICE_AVAILABILITY_CHANGED:
@@ -378,7 +407,7 @@ public class NetworkConfigLinksProvider
                         updateDevice(device).ifPresent(ld -> updatePorts(ld, deviceId));
                     } else {
                         log.debug("Device down {}", deviceId);
-                        //removeDevice(deviceId);
+                        removeDevice(deviceId);
                         providerService.linksVanished(deviceId);
                     }
                     break;
--- a/providers/netcfglinks/src/test/java/org/onosproject/provider/netcfglinks/NetworkConfigLinksProviderTest.java 0 → 100644
View file @cd6ab18
+++ b/providers/netcfglinks/src/test/java/org/onosproject/provider/netcfglinks/NetworkConfigLinksProviderTest.java 0 → 100644
View file @cd6ab18