Step to rationalize rest SBI
- Only update device, which configuration changed,
not walk everything everytime
- workaround to handle cases where username can be null or "".
Change-Id: Iaeda3dbe3cae20c3248735a1c318f4ace40e0c46
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 fef7f37..10daabc 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.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 @@
}
+ // FIXME revisit equality condition. Why urls are not included?
@Override
public boolean equals(Object obj) {
if (obj == this) {
@@ -199,15 +202,16 @@
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 51f82ef..3e97a3d 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.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 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 @@
.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<RestSBDevice> devices) {
//Precomputing the devices to be removed
Set<RestSBDevice> toBeRemoved = new HashSet<>(controller.getDevices().values());
@@ -379,6 +385,16 @@
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 @@
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);
}
}