RestSBDevices with explicit proxy field

This patch allows to create RestSBDevices with explicit
isProxy (boolean) field.
This is important because previously, the RestDeviceProvider
did not allow this, resulting in new REST-based devices
being hidden behind other devices (thus not showing up in ONOS),
although a driver for these devices was available.

I also took the opportunity to slightly refactor the RestDeviceProvider.

Change-Id: I7c8ddf95ead357a35d54bc0bb6d36a2e4ca66f7a
Signed-off-by: Georgios Katsikas <katsikas.gp@gmail.com>
diff --git a/protocols/rest/api/src/main/java/org/onosproject/protocol/rest/DefaultRestSBDevice.java b/protocols/rest/api/src/main/java/org/onosproject/protocol/rest/DefaultRestSBDevice.java
index 10daabc..18ed537 100644
--- a/protocols/rest/api/src/main/java/org/onosproject/protocol/rest/DefaultRestSBDevice.java
+++ b/protocols/rest/api/src/main/java/org/onosproject/protocol/rest/DefaultRestSBDevice.java
@@ -79,13 +79,34 @@
                 Optional.empty() : Optional.ofNullable(swVersion);
         this.testUrl = StringUtils.isEmpty(testUrl) ?
                 Optional.empty() : Optional.ofNullable(testUrl);
-        if (this.manufacturer.isPresent()
-                && this.hwVersion.isPresent()
-                && this.swVersion.isPresent()) {
-            this.isProxy = true;
-        } else {
-            this.isProxy = false;
-        }
+        this.isProxy = false;
+    }
+
+    public DefaultRestSBDevice(IpAddress ip, int port, String name, String password,
+                               String protocol, String url, boolean isActive, String testUrl, String manufacturer,
+                               String hwVersion, String swVersion, boolean isProxy,
+                               AuthenticationScheme authenticationScheme, String token) {
+        Preconditions.checkNotNull(ip, "IP address cannot be null");
+        Preconditions.checkArgument(port > 0, "Port address cannot be negative");
+        Preconditions.checkNotNull(protocol, "protocol address cannot be null");
+        this.ip = ip;
+        this.port = port;
+        this.username = name;
+        this.password = StringUtils.isEmpty(password) ? null : password;
+        this.isActive = isActive;
+        this.protocol = protocol;
+        this.url = StringUtils.isEmpty(url) ? null : url;
+        this.authenticationScheme = authenticationScheme;
+        this.token = token;
+        this.manufacturer = StringUtils.isEmpty(manufacturer) ?
+                Optional.empty() : Optional.ofNullable(manufacturer);
+        this.hwVersion = StringUtils.isEmpty(hwVersion) ?
+                Optional.empty() : Optional.ofNullable(hwVersion);
+        this.swVersion = StringUtils.isEmpty(swVersion) ?
+                Optional.empty() : Optional.ofNullable(swVersion);
+        this.testUrl = StringUtils.isEmpty(testUrl) ?
+                Optional.empty() : Optional.ofNullable(testUrl);
+        this.isProxy = isProxy;
     }
 
     @Override
diff --git a/providers/rest/device/src/main/java/org/onosproject/provider/rest/device/impl/RestDeviceConfig.java b/providers/rest/device/src/main/java/org/onosproject/provider/rest/device/impl/RestDeviceConfig.java
index cd290e5..c61884a 100644
--- a/providers/rest/device/src/main/java/org/onosproject/provider/rest/device/impl/RestDeviceConfig.java
+++ b/providers/rest/device/src/main/java/org/onosproject/provider/rest/device/impl/RestDeviceConfig.java
@@ -39,14 +39,15 @@
     private static final String MANUFACTURER = "manufacturer";
     private static final String HWVERSION = "hwVersion";
     private static final String SWVERSION = "swVersion";
+    private static final String PROXY = "isProxy";
     private static final String AUTHENTICATION_SCHEME = "authenticationScheme";
     private static final String TOKEN = "token";
 
     @Override
     public boolean isValid() {
         return hasOnlyFields(IP, PORT, USERNAME, PASSWORD, PROTOCOL, URL,
-                TESTURL, MANUFACTURER, HWVERSION, SWVERSION, AUTHENTICATION_SCHEME,
-                TOKEN) &&
+                TESTURL, MANUFACTURER, HWVERSION, SWVERSION, PROXY,
+                AUTHENTICATION_SCHEME, TOKEN) &&
                 ip() != null;
     }
 
@@ -141,6 +142,18 @@
     }
 
     /**
+     * Gets whether the REST device is a proxy or not.
+     *
+     * @return proxy
+     */
+    public boolean isProxy() {
+        if (!hasField(PROXY)) {
+            return false;
+        }
+        return get(PROXY, false);
+    }
+
+    /**
      * Gets the authentication type of the REST device.
      * Default is 'basic' if username is defined, else default is no_authentication.
      *
diff --git a/providers/rest/device/src/main/java/org/onosproject/provider/rest/device/impl/RestDeviceProvider.java b/providers/rest/device/src/main/java/org/onosproject/provider/rest/device/impl/RestDeviceProvider.java
index 9a279b9..e6d8b7b 100644
--- a/providers/rest/device/src/main/java/org/onosproject/provider/rest/device/impl/RestDeviceProvider.java
+++ b/providers/rest/device/src/main/java/org/onosproject/provider/rest/device/impl/RestDeviceProvider.java
@@ -241,35 +241,26 @@
     private void deviceAdded(RestSBDevice restSBDev) {
         checkNotNull(restSBDev, ISNOTNULL);
 
-        //check if the server is controlling a single or multiple devices
+        Driver driver = driverService.getDriver(restSBDev.manufacturer().get(),
+                                             restSBDev.hwVersion().get(),
+                                             restSBDev.swVersion().get());
+
+        // Check if the server is controlling a single or multiple devices
         if (restSBDev.isProxy()) {
-
-            Driver driver = driverService.getDriver(restSBDev.manufacturer().get(),
-                                                    restSBDev.hwVersion().get(),
-                                                    restSBDev.swVersion().get());
-
             if (driver != null && driver.hasBehaviour(DevicesDiscovery.class)) {
-
-                //Creates the driver to communicate with the server
-                DevicesDiscovery devicesDiscovery =
-                        devicesDiscovery(restSBDev, driver);
+                DevicesDiscovery devicesDiscovery = devicesDiscovery(restSBDev, driver);
                 Set<DeviceId> deviceIds = devicesDiscovery.deviceIds();
                 restSBDev.setActive(true);
                 deviceIds.forEach(deviceId -> {
                     controller.addProxiedDevice(deviceId, restSBDev);
-                    DeviceDescription devDesc =
-                            devicesDiscovery.deviceDetails(deviceId);
-                    checkNotNull(devDesc,
-                                 "deviceDescription cannot be null");
-                    providerService.deviceConnected(
-                            deviceId, mergeAnn(restSBDev.deviceId(), devDesc));
+                    DeviceDescription devDesc = devicesDiscovery.deviceDetails(deviceId);
+                    checkNotNull(devDesc, "DeviceDescription cannot be null");
+                    providerService.deviceConnected(deviceId, mergeAnn(restSBDev.deviceId(), devDesc));
 
                     if (driver.hasBehaviour(DeviceDescriptionDiscovery.class)) {
                         DriverHandler h = driverService.createHandler(deviceId);
-                        DeviceDescriptionDiscovery devDisc =
-                                h.behaviour(DeviceDescriptionDiscovery.class);
-                        providerService.updatePorts(deviceId,
-                                                    devDisc.discoverPortDetails());
+                        DeviceDescriptionDiscovery devDisc = h.behaviour(DeviceDescriptionDiscovery.class);
+                        providerService.updatePorts(deviceId, devDisc.discoverPortDetails());
                     }
 
                     checkAndUpdateDevice(deviceId);
@@ -279,21 +270,36 @@
             }
         } else {
             DeviceId deviceId = restSBDev.deviceId();
-            ChassisId cid = new ChassisId();
-            String ipAddress = restSBDev.ip().toString();
-            SparseAnnotations annotations = DefaultAnnotations.builder()
-                    .set(IPADDRESS, ipAddress)
-                    .set(AnnotationKeys.PROTOCOL, REST.toUpperCase())
-                    .build();
-            DeviceDescription deviceDescription = new DefaultDeviceDescription(
-                    deviceId.uri(),
-                    Device.Type.SWITCH,
-                    UNKNOWN, UNKNOWN,
-                    UNKNOWN, UNKNOWN,
-                    cid,
-                    annotations);
-            restSBDev.setActive(true);
-            providerService.deviceConnected(deviceId, deviceDescription);
+            if (driver != null && driver.hasBehaviour(DevicesDiscovery.class)) {
+                restSBDev.setActive(true);
+                DevicesDiscovery devicesDiscovery = devicesDiscovery(restSBDev, driver);
+                DeviceDescription deviceDescription = devicesDiscovery.deviceDetails(deviceId);
+                checkNotNull(deviceDescription, "DeviceDescription cannot be null");
+                providerService.deviceConnected(deviceId, deviceDescription);
+
+                if (driver.hasBehaviour(DeviceDescriptionDiscovery.class)) {
+                    DriverHandler h = driverService.createHandler(deviceId);
+                    DeviceDescriptionDiscovery deviceDiscovery = h.behaviour(DeviceDescriptionDiscovery.class);
+                    providerService.updatePorts(deviceId, deviceDiscovery.discoverPortDetails());
+                }
+            } else {
+                ChassisId cid = new ChassisId();
+                String ipAddress = restSBDev.ip().toString();
+                SparseAnnotations annotations = DefaultAnnotations.builder()
+                        .set(IPADDRESS, ipAddress)
+                        .set(AnnotationKeys.PROTOCOL, REST.toUpperCase())
+                        .build();
+                DeviceDescription deviceDescription = new DefaultDeviceDescription(
+                        deviceId.uri(),
+                        Device.Type.SWITCH,
+                        UNKNOWN, UNKNOWN,
+                        UNKNOWN, UNKNOWN,
+                        cid,
+                        annotations);
+                restSBDev.setActive(true);
+                providerService.deviceConnected(deviceId, deviceDescription);
+            }
+
             checkAndUpdateDevice(deviceId);
         }
     }
@@ -312,16 +318,14 @@
 
     private DevicesDiscovery devicesDiscovery(RestSBDevice restSBDevice, Driver driver) {
         DriverData driverData = new DefaultDriverData(driver, restSBDevice.deviceId());
-        DevicesDiscovery devicesDiscovery = driver.createBehaviour(driverData,
-                                                                   DevicesDiscovery.class);
+        DevicesDiscovery devicesDiscovery = driver.createBehaviour(driverData, DevicesDiscovery.class);
         devicesDiscovery.setHandler(new DefaultDriverHandler(driverData));
         return devicesDiscovery;
     }
 
     private void checkAndUpdateDevice(DeviceId deviceId) {
         if (deviceService.getDevice(deviceId) == null) {
-            log.warn("Device {} has not been added to store, " +
-                             "maybe due to a problem in connectivity", deviceId);
+            log.warn("Device {} has not been added to store, maybe due to a problem in connectivity", deviceId);
         } else {
             boolean isReachable = isReachable(deviceId);
             if (isReachable && deviceService.isAvailable(deviceId)) {
@@ -398,6 +402,7 @@
                 config.manufacturer(),
                 config.hwVersion(),
                 config.swVersion(),
+                config.isProxy(),
                 config.authenticationScheme(),
                 config.token()
         );
@@ -456,8 +461,7 @@
 
     private void discoverPorts(DeviceId deviceId) {
         Device device = deviceService.getDevice(deviceId);
-        DeviceDescriptionDiscovery deviceDescriptionDiscovery =
-                device.as(DeviceDescriptionDiscovery.class);
+        DeviceDescriptionDiscovery deviceDescriptionDiscovery = device.as(DeviceDescriptionDiscovery.class);
         providerService.updatePorts(deviceId, deviceDescriptionDiscovery.discoverPortDetails());
     }
 
@@ -497,6 +501,11 @@
     private class InternalNetworkConfigListener implements NetworkConfigListener {
         @Override
         public void event(NetworkConfigEvent event) {
+            if (!isRelevant(event)) {
+                log.warn("Irrelevant network configuration event: {}", event);
+                return;
+            }
+
             ExecutorService bg = SharedExecutors.getSingleThreadExecutor();
             if (event.type() == CONFIG_REMOVED) {
                 DeviceId did = (DeviceId) event.subject();