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 fef7f3708e..10daabccb5 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 @@ -22,6 +22,8 @@ import org.apache.commons.lang3.StringUtils; import org.onlab.packet.IpAddress; import org.onosproject.net.DeviceId; +import static com.google.common.base.Strings.nullToEmpty; + import java.net.URI; import java.net.URISyntaxException; import java.util.Objects; @@ -191,6 +193,7 @@ public class DefaultRestSBDevice implements RestSBDevice { } + // FIXME revisit equality condition. Why urls are not included? @Override public boolean equals(Object obj) { if (obj == this) { @@ -199,15 +202,16 @@ public class DefaultRestSBDevice implements RestSBDevice { if (!(obj instanceof RestSBDevice)) { return false; } - RestSBDevice device = (RestSBDevice) obj; - return this.username.equals(device.username()) && this.ip.equals(device.ip()) && - this.port == device.port(); + RestSBDevice that = (RestSBDevice) obj; + return Objects.equals(this.ip, that.ip()) && + this.port == that.port() && + nullToEmpty(this.username).equals(nullToEmpty(that.username())); } @Override public int hashCode() { - return Objects.hash(ip, port); + return Objects.hash(ip, port, nullToEmpty(username)); } } 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 51f82ef7ae..3e97a3d1b4 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 @@ -24,6 +24,7 @@ import org.apache.felix.scr.annotations.Deactivate; import org.apache.felix.scr.annotations.Reference; import org.apache.felix.scr.annotations.ReferenceCardinality; import org.onlab.packet.ChassisId; +import org.onlab.util.SharedExecutors; import org.onlab.util.SharedScheduledExecutorService; import org.onlab.util.SharedScheduledExecutors; import org.onosproject.core.ApplicationId; @@ -85,6 +86,7 @@ import java.util.stream.Collectors; import static com.google.common.base.Preconditions.checkNotNull; import static org.onlab.util.Tools.groupedThreads; import static org.onosproject.net.config.NetworkConfigEvent.Type.CONFIG_ADDED; +import static org.onosproject.net.config.NetworkConfigEvent.Type.CONFIG_REMOVED; import static org.onosproject.net.config.NetworkConfigEvent.Type.CONFIG_UPDATED; import static org.slf4j.LoggerFactory.getLogger; @@ -346,23 +348,27 @@ public class RestDeviceProvider extends AbstractProvider .map(deviceId -> { RestDeviceConfig config = cfgService.getConfig(deviceId, RestDeviceConfig.class); - return new DefaultRestSBDevice(config.ip(), - config.port(), - config.username(), - config.password(), - config.protocol(), - config.url(), - false, - config.testUrl(), - config.manufacturer(), - config.hwVersion(), - config.swVersion(), - config.authenticationScheme(), - config.token() - ); + return toInactiveRestSBDevice(config); }).collect(Collectors.toSet())); } + private RestSBDevice toInactiveRestSBDevice(RestDeviceConfig config) { + return new DefaultRestSBDevice(config.ip(), + config.port(), + config.username(), + config.password(), + config.protocol(), + config.url(), + false, + config.testUrl(), + config.manufacturer(), + config.hwVersion(), + config.swVersion(), + config.authenticationScheme(), + config.token() + ); + } + private void connectDevices(Set devices) { //Precomputing the devices to be removed Set toBeRemoved = new HashSet<>(controller.getDevices().values()); @@ -379,6 +385,16 @@ public class RestDeviceProvider extends AbstractProvider toBeRemoved.forEach(device -> deviceRemoved(device.deviceId())); } + private void connectDevice(RestSBDevice device) { + // TODO borrowed from above, + // not sure why setting it to inactive + device.setActive(false); + controller.addDevice(device); + if (testDeviceConnection(device)) { + deviceAdded(device); + } + } + private ScheduledFuture schedulePolling() { return portStatisticsExecutor.scheduleAtFixedRate(this::executePortStatisticsUpdate, DEFAULT_POLL_FREQUENCY_SECONDS / 2, @@ -453,14 +469,24 @@ public class RestDeviceProvider extends AbstractProvider private class InternalNetworkConfigListener implements NetworkConfigListener { @Override public void event(NetworkConfigEvent event) { - executor.execute(RestDeviceProvider.this::createAndConnectDevices); + ExecutorService bg = SharedExecutors.getSingleThreadExecutor(); + if (event.type() == CONFIG_REMOVED) { + DeviceId did = (DeviceId) event.subject(); + bg.execute(() -> deviceRemoved(did)); + } else { + // CONFIG_ADDED or CONFIG_UPDATED + RestDeviceConfig cfg = (RestDeviceConfig) event.config().get(); + RestSBDevice restSBDevice = toInactiveRestSBDevice(cfg); + bg.execute(() -> connectDevice(restSBDevice)); + } } @Override public boolean isRelevant(NetworkConfigEvent event) { return event.configClass().equals(RestDeviceConfig.class) && (event.type() == CONFIG_ADDED || - event.type() == CONFIG_UPDATED); + event.type() == CONFIG_UPDATED || + event.type() == CONFIG_REMOVED); } }