Removing Rest and Netconf devices when the providers are disabled

Change-Id: Icac7146fea1295c11972ae4cbf87f8ef9689c671

@@ -20,6 +20,7 @@ import org.onlab.packet.IpAddress;
 import org.onosproject.net.DeviceId;
 
 import java.util.Map;
+import java.util.Set;
 
 /**
  * Abstraction of an NETCONF controller. Serves as a one stop shop for obtaining
@@ -52,7 +53,14 @@ public interface NetconfController {
     NetconfDevice connectDevice(NetconfDeviceInfo deviceInfo) throws NetconfException;
 
     /**
-     * Removes a Netconf device.
+     * Disconnects a Netconf device and removes it from the core.
+     *
+     * @param deviceInfo info about the device to remove
+     */
+    void disconnectDevice(NetconfDeviceInfo deviceInfo);
+
+    /**
+     * Removes a Netconf device from the core.
      *
      * @param deviceInfo info about the device to remove
      */
@@ -62,10 +70,18 @@ public interface NetconfController {
      * Gets all the nodes information.
      *
      * @return map of devices
+     *
      */
     Map<DeviceId, NetconfDevice> getDevicesMap();
 
     /**
+     * Gets all Netconf Devices.
+     *
+     * @return List of all the NetconfDevices Ids
+     */
+    Set<DeviceId> getNetconfDevices();
+
+    /**
      * Gets a Netconf Device by node identifier.
      *
      * @param deviceInfo node identifier

@@ -108,7 +108,7 @@ public class NetconfControllerImpl implements NetconfController {
     }
 
     @Override
-    public void removeDevice(NetconfDeviceInfo deviceInfo) {
+    public void disconnectDevice(NetconfDeviceInfo deviceInfo) {
         if (!netconfDeviceMap.containsKey(deviceInfo.getDeviceId())) {
             log.warn("Device {} is not present", deviceInfo);
         } else {
@@ -116,6 +116,18 @@ public class NetconfControllerImpl implements NetconfController {
         }
     }
 
+    @Override
+    public void removeDevice(NetconfDeviceInfo deviceInfo) {
+        if (!netconfDeviceMap.containsKey(deviceInfo.getDeviceId())) {
+            log.warn("Device {} is not present", deviceInfo);
+        } else {
+            netconfDeviceMap.remove(deviceInfo.getDeviceId());
+            for (NetconfDeviceListener l : netconfDeviceListeners) {
+                l.deviceRemoved(deviceInfo);
+            }
+        }
+    }
+
     private NetconfDevice createDevice(NetconfDeviceInfo deviceInfo) throws NetconfException {
         NetconfDevice netconfDevice = deviceFactory.createNetconfDevice(deviceInfo);
         for (NetconfDeviceListener l : netconfDeviceListeners) {
@@ -138,6 +150,11 @@ public class NetconfControllerImpl implements NetconfController {
         return netconfDeviceMap;
     }
 
+    @Override
+    public Set<DeviceId> getNetconfDevices() {
+        return netconfDeviceMap.keySet();
+    }
+
     //Device factory for the specific NetconfDeviceImpl
     private class DefaultNetconfDeviceFactory implements NetconfDeviceFactory {
 

@@ -182,6 +182,7 @@ public class NetconfStreamThread extends Thread implements NetconfStreamHandler
                                     null, null, Optional.of(-1), netconfDeviceInfo);
                             netconfDeviceEventListeners.forEach(
                                     listener -> listener.event(event));
+                            this.interrupt();
                         } else {
                             deviceReply = deviceReply.replace(END_PATTERN, "");
                             if (deviceReply.contains(RPC_REPLY) ||

@@ -32,8 +32,10 @@ import org.onosproject.netconf.NetconfException;
 import org.onosproject.netconf.NetconfSession;
 
 import java.lang.reflect.Field;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
 
 import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.*;
@@ -134,6 +136,14 @@ public class NetconfControllerImplTest {
     }
 
     @Test
+    public void testGetNetconfDevices() {
+        Set<DeviceId> devices = new HashSet<>();
+        devices.add(deviceId1);
+        devices.add(deviceId2);
+        assertTrue("Incorrect devices", ctrl.getNetconfDevices().containsAll(devices));
+    }
+
+    @Test
     public void testGetNetconfDevice() {
         NetconfDevice fetchedDevice1 = ctrl.getNetconfDevice(deviceId1);
         assertThat("Incorrect device fetched", fetchedDevice1, is(device1));
@@ -192,7 +202,16 @@ public class NetconfControllerImplTest {
     }
 
     /**
-     * Check for removeDevice exception.
+     * Check that disconnectDevice actually disconnects the device and removes it.
+     */
+    @Test
+    public void testDisconnectDevice() throws Exception {
+        ctrl.disconnectDevice(deviceInfo1);
+        assertFalse("Incorrect device removal", ctrl.getDevicesMap().containsKey(deviceId1));
+    }
+
+    /**
+     * Checks that disconnectDevice actually disconnects the device and removes it.
      */
     @Test
     public void testRemoveDevice() throws Exception {

@@ -62,9 +62,9 @@ public interface RestSBController {
     /**
      * Removes the device from the devices map.
      *
-     * @param device to be removed
+     * @param deviceId to be removed
      */
-    void removeDevice(RestSBDevice device);
+    void removeDevice(DeviceId deviceId);
 
     /**
      * Does a REST POST request with specified parameters to the device.
@@ -91,8 +91,8 @@ public interface RestSBController {
     /**
      * Does a REST GET request with specified parameters to the device.
      *
-     * @param device  device to make the request to
-     * @param request url of the request
+     * @param device    device to make the request to
+     * @param request   url of the request
      * @param mediaType format to retrieve the content in
      * @return an inputstream of data from the reply.
      */
@@ -101,8 +101,8 @@ public interface RestSBController {
     /**
      * Does a REST PATCH request with specified parameters to the device.
      *
-     * @param device  device to make the request to
-     * @param request url of the request
+     * @param device    device to make the request to
+     * @param request   url of the request
      * @param payload   payload of the request as an InputStream
      * @param mediaType format to retrieve the content in
      * @return true if operation returned 200, 201, 202, false otherwise

@@ -109,8 +109,8 @@ public class RestSBControllerImpl implements RestSBController {
     }
 
     @Override
-    public void removeDevice(RestSBDevice device) {
-        deviceMap.remove(device.deviceId());
+    public void removeDevice(DeviceId deviceId) {
+        deviceMap.remove(deviceId);
     }
 
     @Override

@@ -53,7 +53,7 @@ public class RestSBControllerImplTest {
         assertEquals("Incorrect Get Device by IP, Port", controller.getDevice(device1.ip(), device1.port()), device1);
         controller.addDevice(device2);
         assertTrue("Device2 non added", controller.getDevices().containsValue(device2));
-        controller.removeDevice(device2);
+        controller.removeDevice(device2.deviceId());
         assertFalse("Device2 not removed", controller.getDevices().containsValue(device2));
     }
 }
\ No newline at end of file

@@ -126,10 +126,13 @@ public class NetconfDeviceProvider extends AbstractProvider
 
     @Deactivate
     public void deactivate() {
+        controller.removeDeviceListener(innerNodeListener);
+        controller.getNetconfDevices().forEach(id ->
+            controller.removeDevice(controller.getDevicesMap().get(id)
+                                            .getDeviceInfo()));
         providerRegistry.unregister(this);
         providerService = null;
         cfgService.unregisterConfigFactory(factory);
-        controller.removeDeviceListener(innerNodeListener);
         log.info("Stopped");
     }
 

@@ -135,13 +135,13 @@ public class RestDeviceProvider extends AbstractProvider
         log.info("Started");
     }
 
-
     @Deactivate
     public void deactivate() {
+        cfgService.removeListener(cfgLister);
+        controller.getDevices().keySet().forEach(this::deviceRemoved);
         providerRegistry.unregister(this);
         providerService = null;
         cfgService.unregisterConfigFactory(factory);
-        cfgService.removeListener(cfgLister);
         log.info("Stopped");
     }
 
@@ -195,12 +195,10 @@ public class RestDeviceProvider extends AbstractProvider
         addedDevices.add(deviceId);
     }
 
-    //when do I call it ?
-    public void deviceRemoved(RestSBDevice nodeId) {
-        Preconditions.checkNotNull(nodeId, ISNOTNULL);
-        DeviceId deviceId = nodeId.deviceId();
+    private void deviceRemoved(DeviceId deviceId) {
+        Preconditions.checkNotNull(deviceId, ISNOTNULL);
         providerService.deviceDisconnected(deviceId);
-        controller.removeDevice(nodeId);
+        controller.removeDevice(deviceId);
     }
 
     private void connectDevices() {
@@ -217,7 +215,7 @@ public class RestDeviceProvider extends AbstractProvider
                             deviceAdded(device);
                         });
                 //Removing devices not wanted anymore
-                toBeRemoved.stream().forEach(device -> deviceRemoved(device));
+                toBeRemoved.stream().forEach(device -> deviceRemoved(device.deviceId()));
 
             }
         } catch (ConfigException e) {