Adding DeviceAdminService facade and tests for SimpleDeviceManager.

+package org.onlab.onos.net.device;
+
+import org.onlab.onos.net.DeviceId;
+import org.onlab.onos.net.MastershipRole;
+
+/**
+ * Service for administering the inventory of infrastructure devices.
+ */
+public interface DeviceAdminService {
+
+    /**
+     * Applies the current mastership role for the specified device.
+     *
+     * @param deviceId device identifier
+     * @param role     requested role
+     */
+    void setRole(DeviceId deviceId, MastershipRole role);
+
+    /**
+     * Removes the device with the specified identifier.
+     *
+     * @param deviceId device identifier
+     */
+    void removeDevice(DeviceId deviceId);
+
+    // TODO: add ability to administratively suspend/resume device
+
+}

@@ -18,6 +18,12 @@ public class DeviceEvent extends AbstractEvent<DeviceEvent.Type, Device> {
         DEVICE_ADDED,
 
         /**
+         * Signifies that some device attributes have changed; excludes
+         * availability changes.
+         */
+        DEVICE_UPDATED,
+
+        /**
          * Signifies that a device has been removed.
          */
         DEVICE_REMOVED,

@@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkState;
 public abstract class AbstractProviderRegistry<P extends Provider, S extends ProviderService<P>>
         implements ProviderRegistry<P, S> {
 
+    private final Map<ProviderId, P> providers = new HashMap<>();
     private final Map<ProviderId, S> services = new HashMap<>();
 
     /**
@@ -34,6 +35,7 @@ public abstract class AbstractProviderRegistry<P extends Provider, S extends Pro
         checkState(!services.containsKey(provider.id()), "Provider %s already registered", provider.id());
         S service = createProviderService(provider);
         services.put(provider.id(), service);
+        providers.put(provider.id(), provider);
         return service;
     }
 
@@ -44,6 +46,7 @@ public abstract class AbstractProviderRegistry<P extends Provider, S extends Pro
         if (service != null && service instanceof AbstractProviderService) {
             ((AbstractProviderService) service).invalidate();
             services.remove(provider.id());
+            providers.remove(provider.id());
         }
     }
 
@@ -52,4 +55,14 @@ public abstract class AbstractProviderRegistry<P extends Provider, S extends Pro
         return ImmutableSet.copyOf(services.keySet());
     }
 
+    /**
+     * Returns the provider registered with the specified provider ID.
+     *
+     * @param providerId provider identifier
+     * @return provider
+     */
+    protected synchronized P getProvider(ProviderId providerId) {
+        return providers.get(providerId);
+    }
+
 }

@@ -13,6 +13,7 @@ import org.onlab.onos.net.DeviceId;
 import org.onlab.onos.net.MastershipRole;
 import org.onlab.onos.net.Port;
 import org.onlab.onos.net.PortNumber;
+import org.onlab.onos.net.device.DeviceAdminService;
 import org.onlab.onos.net.device.DeviceDescription;
 import org.onlab.onos.net.device.DeviceEvent;
 import org.onlab.onos.net.device.DeviceListener;
@@ -37,22 +38,23 @@ import static org.slf4j.LoggerFactory.getLogger;
 @Service
 public class SimpleDeviceManager
         extends AbstractProviderRegistry<DeviceProvider, DeviceProviderService>
-        implements DeviceService, DeviceProviderRegistry {
+        implements DeviceService, DeviceAdminService, DeviceProviderRegistry {
 
-    public static final String DEVICE_ID_NULL = "Device ID cannot be null";
-    public static final String PORT_NUMBER_NULL = "Port number cannot be null";
-    public static final String DEVICE_DESCRIPTION_NULL = "Device description cannot be null";
-    public static final String PORT_DESCRIPTION_NULL = "Port description cannot be null";
+    private static final String DEVICE_ID_NULL = "Device ID cannot be null";
+    private static final String PORT_NUMBER_NULL = "Port number cannot be null";
+    private static final String DEVICE_DESCRIPTION_NULL = "Device description cannot be null";
+    private static final String PORT_DESCRIPTION_NULL = "Port description cannot be null";
+    private static final String ROLE_NULL = "Role cannot be null";
 
     private final Logger log = getLogger(getClass());
 
     private final AbstractListenerRegistry<DeviceEvent, DeviceListener>
             listenerRegistry = new AbstractListenerRegistry<>();
 
-    private final DeviceStore store = new DeviceStore();
+    private final SimpleDeviceStore store = new SimpleDeviceStore();
 
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
-    private EventDeliveryService eventDispatcher;
+    protected EventDeliveryService eventDispatcher;
 
     @Activate
     public void activate() {
@@ -69,12 +71,12 @@ public class SimpleDeviceManager
     @Override
     public MastershipRole getRole(DeviceId deviceId) {
         checkNotNull(deviceId, DEVICE_ID_NULL);
-        return null;
+        return store.getRole(deviceId);
     }
 
     @Override
     public Iterable<Device> getDevices() {
-        return null;
+        return store.getDevices();
     }
 
     @Override
@@ -111,6 +113,28 @@ public class SimpleDeviceManager
         return new InternalDeviceProviderService(provider);
     }
 
+    @Override
+    public void setRole(DeviceId deviceId, MastershipRole newRole) {
+        checkNotNull(deviceId, DEVICE_ID_NULL);
+        checkNotNull(newRole, ROLE_NULL);
+        DeviceEvent event = store.setRole(deviceId, newRole);
+        if (event != null) {
+            Device device = event.subject();
+            DeviceProvider provider = getProvider(device.providerId());
+            if (provider != null) {
+                provider.roleChanged(device, newRole);
+            }
+            post(event);
+        }
+    }
+
+    @Override
+    public void removeDevice(DeviceId deviceId) {
+        checkNotNull(deviceId, DEVICE_ID_NULL);
+        DeviceEvent event = store.removeDevice(deviceId);
+        post(event);
+    }
+
     // Personalized device provider service issued to the supplied provider.
     private class InternalDeviceProviderService extends AbstractProviderService<DeviceProvider>
             implements DeviceProviderService {
@@ -124,7 +148,8 @@ public class SimpleDeviceManager
             checkNotNull(deviceId, DEVICE_ID_NULL);
             checkNotNull(deviceDescription, DEVICE_DESCRIPTION_NULL);
             log.info("Device {} connected: {}", deviceId, deviceDescription);
-            DeviceEvent event = store.createOrUpdateDevice(deviceId, deviceDescription);
+            DeviceEvent event = store.createOrUpdateDevice(provider().id(),
+                                                           deviceId, deviceDescription);
             post(event);
         }
 
@@ -132,7 +157,7 @@ public class SimpleDeviceManager
         public void deviceDisconnected(DeviceId deviceId) {
             checkNotNull(deviceId, DEVICE_ID_NULL);
             log.info("Device {} disconnected", deviceId);
-            DeviceEvent event = store.removeDevice(deviceId);
+            DeviceEvent event = store.markOffline(deviceId);
             post(event);
         }
 

 package org.onlab.onos.net.trivial.impl;
 
+import org.onlab.onos.net.DefaultDevice;
 import org.onlab.onos.net.Device;
 import org.onlab.onos.net.DeviceId;
+import org.onlab.onos.net.MastershipRole;
 import org.onlab.onos.net.Port;
 import org.onlab.onos.net.PortNumber;
 import org.onlab.onos.net.device.DeviceDescription;
 import org.onlab.onos.net.device.DeviceEvent;
 import org.onlab.onos.net.device.PortDescription;
+import org.onlab.onos.net.provider.ProviderId;
 
 import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 
+import static com.google.common.base.Preconditions.checkArgument;
+import static org.onlab.onos.net.device.DeviceEvent.Type.DEVICE_AVAILABILITY_CHANGED;
+import static org.onlab.onos.net.device.DeviceEvent.Type.DEVICE_MASTERSHIP_CHANGED;
+import static org.onlab.onos.net.device.DeviceEvent.Type.DEVICE_REMOVED;
+
 /**
  * Manages inventory of infrastructure devices.
  */
-public class DeviceStore {
+class SimpleDeviceStore {
+
+    private final Map<DeviceId, DefaultDevice> devices = new ConcurrentHashMap<>();
+    private final Set<DeviceId> available = new HashSet<>();
+    private final Map<DeviceId, MastershipRole> roles = new HashMap<>();
+
+    /**
+     * Returns an iterable collection of all devices known to the system.
+     *
+     * @return device collection
+     */
+    Iterable<Device> getDevices() {
+        return Collections.unmodifiableSet(new HashSet<Device>(devices.values()));
+    }
 
-    private final Map<DeviceId, Device> devices = new ConcurrentHashMap<>();
+    /**
+     * Returns the device with the specified identifier.
+     *
+     * @param deviceId device identifier
+     * @return device
+     */
+    Device getDevice(DeviceId deviceId) {
+        return devices.get(deviceId);
+    }
 
     /**
      * Creates a new infrastructure device, or updates an existing one using
      * the supplied device description.
      *
+     * @param providerId        provider identifier
      * @param deviceId          device identifier
      * @param deviceDescription device description
      * @return ready to send event describing what occurred; null if no change
      */
-    public DeviceEvent createOrUpdateDevice(DeviceId deviceId,
-                                            DeviceDescription deviceDescription) {
-        return null;
+    DeviceEvent createOrUpdateDevice(ProviderId providerId, DeviceId deviceId,
+                                     DeviceDescription deviceDescription) {
+        DefaultDevice device = devices.get(deviceId);
+        if (device == null) {
+            return createDevice(providerId, deviceId, deviceDescription);
+        }
+        return updateDevice(providerId, device, deviceDescription);
+    }
+
+    // Creates the device and returns the appropriate event if necessary.
+    private DeviceEvent createDevice(ProviderId providerId, DeviceId deviceId,
+                                     DeviceDescription desc) {
+        DefaultDevice device = new DefaultDevice(providerId, deviceId, desc.type(),
+                                                 desc.manufacturer(),
+                                                 desc.hwVersion(), desc.swVersion(),
+                                                 desc.serialNumber());
+        synchronized (this) {
+            devices.put(deviceId, device);
+            available.add(deviceId);
+        }
+        return new DeviceEvent(DeviceEvent.Type.DEVICE_ADDED, device);
+    }
+
+    // Updates the device and returns the appropriate event if necessary.
+    private DeviceEvent updateDevice(ProviderId providerId, DefaultDevice device,
+                                     DeviceDescription desc) {
+        // We allow only certain attributes to trigger update
+        if (!Objects.equals(device.hwVersion(), desc.hwVersion()) ||
+                !Objects.equals(device.swVersion(), desc.swVersion())) {
+            DefaultDevice updated = new DefaultDevice(providerId, device.id(),
+                                                      desc.type(),
+                                                      desc.manufacturer(),
+                                                      desc.hwVersion(),
+                                                      desc.swVersion(),
+                                                      desc.serialNumber());
+            synchronized (this) {
+                devices.put(device.id(), updated);
+                available.add(device.id());
+            }
+            return new DeviceEvent(DeviceEvent.Type.DEVICE_UPDATED, device);
+        }
+
+        // Otherwise merely attempt to change availability
+        synchronized (this) {
+            boolean added = available.add(device.id());
+            return added ? new DeviceEvent(DEVICE_AVAILABILITY_CHANGED, device) : null;
+        }
     }
 
     /**
@@ -39,8 +118,13 @@ public class DeviceStore {
      * @param deviceId device identifier
      * @return ready to send event describing what occurred; null if no change
      */
-    public DeviceEvent removeDevice(DeviceId deviceId) {
-        return null;
+    DeviceEvent markOffline(DeviceId deviceId) {
+        synchronized (this) {
+            Device device = devices.get(deviceId);
+            checkArgument(device != null, "Device with ID %s is not found", deviceId);
+            boolean removed = available.remove(deviceId);
+            return removed ? new DeviceEvent(DEVICE_AVAILABILITY_CHANGED, device) : null;
+        }
     }
 
     /**
@@ -51,8 +135,8 @@ public class DeviceStore {
      * @param portDescriptions list of port descriptions
      * @return ready to send events describing what occurred; empty list if no change
      */
-    public List<DeviceEvent> updatePorts(DeviceId deviceId,
-                                         List<PortDescription> portDescriptions) {
+    List<DeviceEvent> updatePorts(DeviceId deviceId,
+                                  List<PortDescription> portDescriptions) {
         return new ArrayList<>();
     }
 
@@ -64,18 +148,8 @@ public class DeviceStore {
      * @param portDescription port description
      * @return ready to send event describing what occurred; null if no change
      */
-    public DeviceEvent updatePortStatus(DeviceId deviceId,
-                                        PortDescription portDescription) {
-        return null;
-    }
-
-    /**
-     * Returns the device with the specified identifier.
-     *
-     * @param deviceId device identifier
-     * @return device
-     */
-    public Device getDevice(DeviceId deviceId) {
+    DeviceEvent updatePortStatus(DeviceId deviceId,
+                                 PortDescription portDescription) {
         return null;
     }
 
@@ -85,7 +159,7 @@ public class DeviceStore {
      * @param deviceId device identifier
      * @return list of device ports
      */
-    public List<Port> getPorts(DeviceId deviceId) {
+    List<Port> getPorts(DeviceId deviceId) {
         return null;
     }
 
@@ -96,7 +170,45 @@ public class DeviceStore {
      * @param portNumber port number
      * @return device port
      */
-    public Port getPort(DeviceId deviceId, PortNumber portNumber) {
+    Port getPort(DeviceId deviceId, PortNumber portNumber) {
         return null;
     }
+
+    /**
+     * Returns the mastership role determined for this device.
+     *
+     * @param deviceId device identifier
+     * @return mastership role
+     */
+    MastershipRole getRole(DeviceId deviceId) {
+        MastershipRole role = roles.get(deviceId);
+        return role != null ? role : MastershipRole.NONE;
+    }
+
+    /**
+     * Administratively sets the role of the specified device.
+     *
+     * @param deviceId device identifier
+     * @param role     mastership role to apply
+     * @return mastership role change event or null if no change
+     */
+    DeviceEvent setRole(DeviceId deviceId, MastershipRole role) {
+        Device device = getDevice(deviceId);
+        checkArgument(device != null, "Device with ID %s not found");
+        MastershipRole oldRole = roles.put(deviceId, role);
+        return oldRole == role ? null : new DeviceEvent(DEVICE_MASTERSHIP_CHANGED, device);
+    }
+
+    /**
+     * Administratively removes the specified device from the store.
+     *
+     * @param deviceId device to be removed
+     */
+    DeviceEvent removeDevice(DeviceId deviceId) {
+        synchronized (this) {
+            roles.remove(deviceId);
+            Device device = devices.remove(deviceId);
+            return device != null ? new DeviceEvent(DEVICE_REMOVED, device) : null;
+        }
+    }
 }

+package org.onlab.onos.net.trivial.impl;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.onlab.onos.event.Event;
+import org.onlab.onos.net.Device;
+import org.onlab.onos.net.DeviceId;
+import org.onlab.onos.net.MastershipRole;
+import org.onlab.onos.net.device.DefaultDeviceDescription;
+import org.onlab.onos.net.device.DeviceAdminService;
+import org.onlab.onos.net.device.DeviceDescription;
+import org.onlab.onos.net.device.DeviceEvent;
+import org.onlab.onos.net.device.DeviceListener;
+import org.onlab.onos.net.device.DeviceProvider;
+import org.onlab.onos.net.device.DeviceProviderRegistry;
+import org.onlab.onos.net.device.DeviceProviderService;
+import org.onlab.onos.net.device.DeviceService;
+import org.onlab.onos.net.provider.AbstractProvider;
+import org.onlab.onos.net.provider.ProviderId;
+
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+
+import static org.junit.Assert.*;
+import static org.onlab.onos.net.Device.Type.SWITCH;
+import static org.onlab.onos.net.DeviceId.deviceId;
+import static org.onlab.onos.net.device.DeviceEvent.Type.*;
+
+/**
+ * Test codifying the device service & device provider service contracts.
+ */
+public class SimpleDeviceManagerTest {
+
+    private static final ProviderId PID = new ProviderId("foo");
+    private static final DeviceId DID1 = deviceId("of:foo");
+    private static final DeviceId DID2 = deviceId("of:bar");
+    private static final String MFR = "whitebox";
+    private static final String HW = "1.1.x";
+    private static final String SW1 = "3.8.1";
+    private static final String SW2 = "3.9.5";
+    private static final String SN = "43311-12345";
+
+
+    private SimpleDeviceManager mgr;
+
+    protected DeviceService service;
+    protected DeviceAdminService admin;
+    protected DeviceProviderRegistry registry;
+    protected DeviceProviderService providerService;
+    protected TestProvider provider;
+    protected TestListener listener = new TestListener();
+
+    @Before
+    public void setUp() {
+        mgr = new SimpleDeviceManager();
+        service = mgr;
+        admin = mgr;
+        registry = mgr;
+        mgr.eventDispatcher = new TestEventDispatcher();
+        mgr.activate();
+
+        service.addListener(listener);
+
+        provider = new TestProvider();
+        providerService = registry.register(provider);
+        assertTrue("provider should be registered",
+                   registry.getProviders().contains(provider.id()));
+    }
+
+    @After
+    public void tearDown() {
+        registry.unregister(provider);
+        assertFalse("provider should not be registered",
+                    registry.getProviders().contains(provider.id()));
+        service.removeListener(listener);
+        mgr.deactivate();
+    }
+
+    private void connectDevice(DeviceId deviceId, String swVersion) {
+        DeviceDescription description =
+                new DefaultDeviceDescription(deviceId.uri(), SWITCH, MFR,
+                                             HW, swVersion, SN);
+        providerService.deviceConnected(deviceId, description);
+        assertNotNull("device should be found", service.getDevice(DID1));
+    }
+
+    @Test
+    public void deviceConnected() {
+        assertNull("device should not be found", service.getDevice(DID1));
+        connectDevice(DID1, SW1);
+        validateEvents(DEVICE_ADDED);
+
+        Iterator<Device> it = service.getDevices().iterator();
+        assertNotNull("one device expected", it.next());
+        assertFalse("only one device expected", it.hasNext());
+    }
+
+    @Test
+    public void deviceDisconnected() {
+        connectDevice(DID1, SW1);
+        connectDevice(DID2, SW1);
+        validateEvents(DEVICE_ADDED, DEVICE_ADDED);
+
+        // Disconnect
+        providerService.deviceDisconnected(DID1);
+        assertNotNull("device should not be found", service.getDevice(DID1));
+        validateEvents(DEVICE_AVAILABILITY_CHANGED);
+
+        // Reconnect
+        connectDevice(DID1, SW1);
+        validateEvents(DEVICE_AVAILABILITY_CHANGED);
+    }
+
+    @Test
+    public void deviceUpdated() {
+        connectDevice(DID1, SW1);
+        validateEvents(DEVICE_ADDED);
+
+        connectDevice(DID1, SW2);
+        validateEvents(DEVICE_UPDATED);
+    }
+
+    @Test
+    public void getRole() {
+        connectDevice(DID1, SW1);
+        assertEquals("incorrect role", MastershipRole.NONE, service.getRole(DID1));
+    }
+
+    @Test
+    public void setRole() {
+        connectDevice(DID1, SW1);
+        admin.setRole(DID1, MastershipRole.MASTER);
+        validateEvents(DEVICE_ADDED, DEVICE_MASTERSHIP_CHANGED);
+        assertEquals("incorrect role", MastershipRole.MASTER, service.getRole(DID1));
+        assertEquals("incorrect device", DID1, provider.deviceReceived.id());
+        assertEquals("incorrect role", MastershipRole.MASTER, provider.roleReceived);
+    }
+
+
+
+    protected void validateEvents(Enum... types) {
+        int i = 0;
+        assertEquals("wrong events received", types.length, listener.events.size());
+        for (Event event : listener.events) {
+            assertEquals("incorrect event type", types[i], event.type());
+            i++;
+        }
+        listener.events.clear();
+    }
+
+
+    private class TestProvider extends AbstractProvider implements DeviceProvider {
+        private Device deviceReceived;
+        private MastershipRole roleReceived;
+
+        public TestProvider() {
+            super(PID);
+        }
+
+        @Override
+        public void triggerProbe(Device device) {
+        }
+
+        @Override
+        public void roleChanged(Device device, MastershipRole newRole) {
+            deviceReceived = device;
+            roleReceived = newRole;
+        }
+    }
+
+    private static class TestListener implements DeviceListener {
+        final List<DeviceEvent> events = new ArrayList<>();
+
+        @Override
+        public void event(DeviceEvent event) {
+            events.add(event);
+        }
+    }
+
+}

+package org.onlab.onos.net.trivial.impl;
+
+import org.onlab.onos.event.DefaultEventSinkRegistry;
+import org.onlab.onos.event.Event;
+import org.onlab.onos.event.EventDeliveryService;
+import org.onlab.onos.event.EventSink;
+
+import static com.google.common.base.Preconditions.checkState;
+
+/**
+ * Implements event delivery system that delivers events synchronously, or
+ * in-line with the post method invocation.
+ */
+public class TestEventDispatcher extends DefaultEventSinkRegistry
+        implements EventDeliveryService {
+
+    @Override
+    public void post(Event event) {
+        EventSink sink = getSink(event.getClass());
+        checkState(sink != null, "No sink for event %s", event);
+        sink.process(event);
+    }
+
+}