[SEBA-286] remove a subscriber only if it was provisioned

Change-Id: I9ad9205427d3276b088b15d4665d8f63a04c8bd1
diff --git a/app/pom.xml b/app/pom.xml
index 6a97b0a..468b321 100644
--- a/app/pom.xml
+++ b/app/pom.xml
@@ -77,6 +77,19 @@
             <groupId>org.apache.karaf.shell</groupId>
             <artifactId>org.apache.karaf.shell.console</artifactId>
         </dependency>
+
+        <dependency>
+            <groupId>junit</groupId>
+            <artifactId>junit</artifactId>
+            <scope>test</scope>
+        </dependency>
+
+        <dependency>
+            <groupId>org.onosproject</groupId>
+            <artifactId>onlab-junit</artifactId>
+            <version>${onos.version}</version>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>
diff --git a/app/src/main/java/org/opencord/olt/impl/Olt.java b/app/src/main/java/org/opencord/olt/impl/Olt.java
index f1236c9..e2fdc55 100644
--- a/app/src/main/java/org/opencord/olt/impl/Olt.java
+++ b/app/src/main/java/org/opencord/olt/impl/Olt.java
@@ -272,16 +272,12 @@
 
     @Override
     public boolean removeSubscriber(ConnectPoint port) {
-        // Get the subscriber connected to this port from Sadis
-        SubscriberAndDeviceInformation subscriber = getSubscriber(port);
+        // Get the subscriber connected to this port from the local cache
+        // as if we don't know about the subscriber there's no need to remove it
+        SubscriberAndDeviceInformation subscriber = programmedSubs.get(port);
         if (subscriber == null) {
-            log.warn("Subscriber on port {} not found in sadis .. checking "
-                    + "local cache", port);
-            subscriber = programmedSubs.get(port);
-            if (subscriber == null) {
-                log.warn("Subscriber on port {} was not previously programmed", port);
-                return false;
-            }
+            log.warn("Subscriber on port {} was not previously programmed, no need to remove it", port);
+            return true;
         }
 
         // Get the uplink port
@@ -1026,13 +1022,13 @@
     /**
      * Return the subscriber on a port.
      *
-     * @param port On which to find the subscriber
+     * @param cp ConnectPoint on which to find the subscriber
      * @return subscriber if found else null
      */
-    private SubscriberAndDeviceInformation getSubscriber(ConnectPoint port) {
-        String portName = deviceService.getPort(port).annotations()
-                .value(AnnotationKeys.PORT_NAME);
-
+    SubscriberAndDeviceInformation getSubscriber(ConnectPoint cp) {
+        Port port = deviceService.getPort(cp);
+        checkNotNull(port, "Invalid connect point");
+        String portName = port.annotations().value(AnnotationKeys.PORT_NAME);
         return subsService.get(portName);
     }
 
diff --git a/app/src/test/java/org/opencord/olt/impl/OltTest.java b/app/src/test/java/org/opencord/olt/impl/OltTest.java
new file mode 100644
index 0000000..1878c74
--- /dev/null
+++ b/app/src/test/java/org/opencord/olt/impl/OltTest.java
@@ -0,0 +1,204 @@
+/*
+ * Copyright 2016-present Open Networking Foundation
+ *
+ * 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.opencord.olt.impl;
+
+import static org.junit.Assert.assertEquals;
+
+import java.util.Set;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.onlab.packet.ChassisId;
+import org.onlab.packet.Ip4Address;
+import org.onlab.packet.MacAddress;
+import org.onlab.packet.VlanId;
+import org.onosproject.net.AnnotationKeys;
+import org.onosproject.net.Annotations;
+import org.onosproject.net.ConnectPoint;
+import org.onosproject.net.DefaultAnnotations;
+import org.onosproject.net.DefaultDevice;
+import org.onosproject.net.Device;
+import org.onosproject.net.DeviceId;
+import org.onosproject.net.Element;
+import org.onosproject.net.Port;
+import org.onosproject.net.PortNumber;
+import org.onosproject.net.device.DeviceServiceAdapter;
+import org.onosproject.net.provider.ProviderId;
+import org.opencord.sadis.SubscriberAndDeviceInformation;
+import org.opencord.sadis.SubscriberAndDeviceInformationService;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class OltTest {
+    private final Logger log = LoggerFactory.getLogger(getClass());
+    private Olt olt;
+
+    private static final VlanId CLIENT_C_TAG = VlanId.vlanId((short) 999);
+    private static final VlanId CLIENT_S_TAG = VlanId.vlanId((short) 111);
+    private static final String CLIENT_NAS_PORT_ID = "PON 1/1";
+    private static final String CLIENT_CIRCUIT_ID = "CIR-PON 1/1";
+
+    private static final String OLT_DEV_ID = "of:00000000000000aa";
+    private static final DeviceId DEVICE_ID_1 = DeviceId.deviceId(OLT_DEV_ID);
+    private static final String SCHEME_NAME = "olt";
+    private static final DefaultAnnotations DEVICE_ANNOTATIONS = DefaultAnnotations.builder()
+            .set(AnnotationKeys.PROTOCOL, SCHEME_NAME.toUpperCase()).build();
+
+    @Before
+    public void setUp() {
+        olt = new Olt();
+        olt.deviceService = new MockDeviceService();
+        olt.subsService = new MockSubService();
+    }
+
+    /**
+     * Tests that the getSubscriber method does throw a NullPointerException with a meaningful message.
+     */
+    @Test
+    public void testGetSubscriberError() {
+        ConnectPoint cp = ConnectPoint.deviceConnectPoint(OLT_DEV_ID + "/" + 1);
+        try {
+            olt.getSubscriber(cp);
+        } catch (NullPointerException e) {
+            assertEquals(e.getMessage(), "Invalid connect point");
+        }
+    }
+
+    /**
+     * Tests that the getSubscriber method returns Subscriber informations.
+     */
+    @Test
+    public void testGetSubscriber() {
+        ConnectPoint cp = ConnectPoint.deviceConnectPoint(OLT_DEV_ID + "/" + 2);
+
+        SubscriberAndDeviceInformation s =  olt.getSubscriber(cp);
+
+        assertEquals(s.circuitId(), CLIENT_CIRCUIT_ID);
+        assertEquals(s.cTag(), CLIENT_C_TAG);
+        assertEquals(s.sTag(), CLIENT_S_TAG);
+        assertEquals(s.nasPortId(), CLIENT_NAS_PORT_ID);
+    }
+
+    private class MockDevice extends DefaultDevice {
+
+        public MockDevice(ProviderId providerId, DeviceId id, Type type,
+                          String manufacturer, String hwVersion, String swVersion,
+                          String serialNumber, ChassisId chassisId, Annotations... annotations) {
+            super(providerId, id, type, manufacturer, hwVersion, swVersion, serialNumber,
+                  chassisId, annotations);
+        }
+    }
+
+    private class MockDeviceService extends DeviceServiceAdapter {
+
+        private ProviderId providerId = new ProviderId("of", "foo");
+        private final Device device1 = new MockDevice(providerId, DEVICE_ID_1, Device.Type.SWITCH,
+                                                      "foo.inc", "0", "0", OLT_DEV_ID, new ChassisId(),
+                                                      DEVICE_ANNOTATIONS);
+
+        @Override
+        public Device getDevice(DeviceId devId) {
+            return device1;
+
+        }
+
+        @Override
+        public Port getPort(ConnectPoint cp) {
+            log.info("Looking up port {}", cp.port().toString());
+            if (cp.port().toString().equals("1")) {
+                return null;
+            }
+            return new MockPort();
+        }
+    }
+
+    private class  MockPort implements Port {
+
+        @Override
+        public boolean isEnabled() {
+            return true;
+        }
+        @Override
+        public long portSpeed() {
+            return 1000;
+        }
+        @Override
+        public Element element() {
+            return null;
+        }
+        @Override
+        public PortNumber number() {
+            return null;
+        }
+        @Override
+        public Annotations annotations() {
+            return new MockAnnotations();
+        }
+        @Override
+        public Type type() {
+            return Port.Type.FIBER;
+        }
+
+        private class MockAnnotations implements Annotations {
+
+            @Override
+            public String value(String val) {
+                return "BRCM12345678";
+            }
+            @Override
+            public Set<String> keys() {
+                return null;
+            }
+        }
+    }
+
+    private class MockSubService implements SubscriberAndDeviceInformationService {
+        MockSubscriberAndDeviceInformation sub =
+                new MockSubscriberAndDeviceInformation(CLIENT_NAS_PORT_ID, CLIENT_C_TAG,
+                                                       CLIENT_S_TAG, CLIENT_NAS_PORT_ID, CLIENT_CIRCUIT_ID, null, null);
+        @Override
+        public SubscriberAndDeviceInformation get(String id) {
+            return  sub;
+        }
+
+        @Override
+        public void invalidateAll() {}
+        @Override
+        public void invalidateId(String id) {}
+        @Override
+        public SubscriberAndDeviceInformation getfromCache(String id) {
+            return null;
+        }
+    }
+
+    private class MockSubscriberAndDeviceInformation extends SubscriberAndDeviceInformation {
+
+        MockSubscriberAndDeviceInformation(String id, VlanId ctag,
+                                           VlanId stag, String nasPortId,
+                                           String circuitId, MacAddress hardId,
+                                           Ip4Address ipAddress) {
+            this.setCTag(ctag);
+            this.setHardwareIdentifier(hardId);
+            this.setId(id);
+            this.setIPAddress(ipAddress);
+            this.setSTag(stag);
+            this.setNasPortId(nasPortId);
+            this.setCircuitId(circuitId);
+        }
+    }
+
+}
\ No newline at end of file