[ONOS-7573][Test successful] Mark online does not notify peers device subsystem

Change-Id: Ie347241325047cb65c836c849bec6aebad758820
diff --git a/core/api/src/main/java/org/onosproject/net/device/DeviceStore.java b/core/api/src/main/java/org/onosproject/net/device/DeviceStore.java
index 54bdf17..bd59838 100644
--- a/core/api/src/main/java/org/onosproject/net/device/DeviceStore.java
+++ b/core/api/src/main/java/org/onosproject/net/device/DeviceStore.java
@@ -93,9 +93,9 @@
      * Marks the device as available.
      *
      * @param deviceId device identifier
-     * @return true if availability change request was accepted and changed the state
+     * @return ready to send event describing what occurred; null if no change
      */
-    boolean markOnline(DeviceId deviceId);
+    DeviceEvent markOnline(DeviceId deviceId);
 
     /**
      * Updates the ports of the specified infrastructure device using the given
diff --git a/core/api/src/test/java/org/onosproject/net/device/DeviceStoreAdapter.java b/core/api/src/test/java/org/onosproject/net/device/DeviceStoreAdapter.java
index a5a2a90..63971b9 100644
--- a/core/api/src/test/java/org/onosproject/net/device/DeviceStoreAdapter.java
+++ b/core/api/src/test/java/org/onosproject/net/device/DeviceStoreAdapter.java
@@ -62,8 +62,8 @@
     }
 
     @Override
-    public boolean markOnline(DeviceId deviceId) {
-        return false;
+    public DeviceEvent markOnline(DeviceId deviceId) {
+        return null;
     }
 
     @Override
diff --git a/core/common/src/test/java/org/onosproject/store/trivial/SimpleDeviceStore.java b/core/common/src/test/java/org/onosproject/store/trivial/SimpleDeviceStore.java
index 5a41057..fc8e9e4 100644
--- a/core/common/src/test/java/org/onosproject/store/trivial/SimpleDeviceStore.java
+++ b/core/common/src/test/java/org/onosproject/store/trivial/SimpleDeviceStore.java
@@ -250,9 +250,9 @@
 
     // implement differently if desired
     @Override
-    public boolean markOnline(DeviceId deviceId) {
+    public DeviceEvent markOnline(DeviceId deviceId) {
         log.warn("Mark online not supported");
-        return false;
+        return null;
     }
 
     @Override
diff --git a/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java b/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java
index 8359bdf..d7869bb 100644
--- a/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java
+++ b/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java
@@ -857,7 +857,7 @@
             case MASTER:
                 final Device device = getDevice(did);
                 if ((device != null) && !isAvailable(did)) {
-                    store.markOnline(did);
+                    post(store.markOnline(did));
                 }
                 // TODO: should apply role only if there is mismatch
                 log.debug("Applying role {} to {}", myNextRole, did);
diff --git a/core/store/dist/src/main/java/org/onosproject/store/device/impl/ECDeviceStore.java b/core/store/dist/src/main/java/org/onosproject/store/device/impl/ECDeviceStore.java
index 680f0a0..177c41ed 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/device/impl/ECDeviceStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/device/impl/ECDeviceStore.java
@@ -363,12 +363,14 @@
     }
 
     // FIXME publicization of markOnline -- trigger some action independently?
-    public boolean markOnline(DeviceId deviceId) {
+    public DeviceEvent markOnline(DeviceId deviceId) {
         if (devices.containsKey(deviceId)) {
-            return availableDevices.add(deviceId);
+            if (availableDevices.add(deviceId)) {
+                return new DeviceEvent(DEVICE_AVAILABILITY_CHANGED, devices.get(deviceId), null);
+            }
         }
         log.warn("Device {} does not exist in store", deviceId);
-        return false;
+        return null;
     }
 
     @Override
diff --git a/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java b/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java
index faaed09..f5c055a 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java
@@ -106,7 +106,7 @@
 import static org.onosproject.net.device.DeviceEvent.Type.PORT_STATS_UPDATED;
 import static org.onosproject.net.device.DeviceEvent.Type.PORT_UPDATED;
 import static org.onosproject.store.device.impl.GossipDeviceStoreMessageSubjects.DEVICE_ADVERTISE;
-import static org.onosproject.store.device.impl.GossipDeviceStoreMessageSubjects.DEVICE_OFFLINE;
+import static org.onosproject.store.device.impl.GossipDeviceStoreMessageSubjects.DEVICE_STATUS_CHANGE;
 import static org.onosproject.store.device.impl.GossipDeviceStoreMessageSubjects.DEVICE_REMOVED;
 import static org.onosproject.store.device.impl.GossipDeviceStoreMessageSubjects.DEVICE_REMOVE_REQ;
 import static org.onosproject.store.device.impl.GossipDeviceStoreMessageSubjects.DEVICE_UPDATE;
@@ -176,7 +176,7 @@
                     .register(DistributedStoreSerializers.STORE_COMMON)
                     .nextId(DistributedStoreSerializers.STORE_CUSTOM_BEGIN)
                     .register(new InternalDeviceEventSerializer(), InternalDeviceEvent.class)
-                    .register(new InternalDeviceOfflineEventSerializer(), InternalDeviceOfflineEvent.class)
+                    .register(new InternalDeviceStatusChangeEventSerializer(), InternalDeviceStatusChangeEvent.class)
                     .register(InternalDeviceRemovedEvent.class)
                     .register(new InternalPortEventSerializer(), InternalPortEvent.class)
                     .register(new InternalPortStatusEventSerializer(), InternalPortStatusEvent.class)
@@ -201,7 +201,7 @@
                 newSingleThreadScheduledExecutor(minPriority(groupedThreads("onos/device", "bg-%d", log)));
 
         addSubscriber(DEVICE_UPDATE, this::handleDeviceEvent);
-        addSubscriber(DEVICE_OFFLINE, this::handleDeviceOfflineEvent);
+        addSubscriber(DEVICE_STATUS_CHANGE, this::handleDeviceStatusChangeEvent);
         addSubscriber(DEVICE_REMOVE_REQ, this::handleRemoveRequest);
         addSubscriber(DEVICE_REMOVED, this::handleDeviceRemovedEvent);
         addSubscriber(PORT_UPDATE, this::handlePortEvent);
@@ -262,7 +262,7 @@
         devicePorts.clear();
         availableDevices.clear();
         clusterCommunicator.removeSubscriber(DEVICE_UPDATE);
-        clusterCommunicator.removeSubscriber(DEVICE_OFFLINE);
+        clusterCommunicator.removeSubscriber(DEVICE_STATUS_CHANGE);
         clusterCommunicator.removeSubscriber(DEVICE_REMOVE_REQ);
         clusterCommunicator.removeSubscriber(DEVICE_REMOVED);
         clusterCommunicator.removeSubscriber(PORT_UPDATE);
@@ -395,7 +395,7 @@
                providerId, oldDevice, newDevice);
 
         if (!providerId.isAncillary()) {
-            markOnline(newDevice.id(), timestamp);
+            markOnline(newDevice.id(), timestamp, false);
         }
 
         log.debug("Device {} added", newDevice.id());
@@ -434,11 +434,7 @@
         }
 
         if (!providerId.isAncillary() && forceAvailable) {
-            boolean wasOnline = availableDevices.contains(newDevice.id());
-            markOnline(newDevice.id(), newTimestamp);
-            if (!wasOnline) {
-                notifyDelegateIfNotNull(new DeviceEvent(DEVICE_AVAILABILITY_CHANGED, newDevice, null));
-            }
+            notifyDelegateIfNotNull(markOnline(newDevice.id(), newTimestamp, false));
         }
         return event;
     }
@@ -467,7 +463,7 @@
         if (event != null) {
             log.debug("Notifying peers of a device offline topology event for deviceId: {} {}",
                      deviceId, timestamp);
-            notifyPeers(new InternalDeviceOfflineEvent(deviceId, timestamp));
+            notifyPeers(new InternalDeviceStatusChangeEvent(deviceId, timestamp, false));
         }
         return event;
     }
@@ -507,22 +503,8 @@
     }
 
     @Override
-    public boolean markOnline(DeviceId deviceId) {
-        if (devices.containsKey(deviceId)) {
-            final Timestamp timestamp = deviceClockService.getTimestamp(deviceId);
-            Map<?, ?> deviceLock = getOrCreateDeviceDescriptionsMap(deviceId);
-            synchronized (deviceLock) {
-                if (markOnline(deviceId, timestamp)) {
-                    notifyDelegate(new DeviceEvent(DEVICE_AVAILABILITY_CHANGED, getDevice(deviceId), null));
-                    return true;
-                } else {
-                    return false;
-                }
-            }
-        }
-        log.warn("Device {} does not exist in store", deviceId);
-        return false;
-
+    public DeviceEvent markOnline(DeviceId deviceId) {
+        return markOnline(deviceId, deviceClockService.getTimestamp(deviceId), true);
     }
 
     /**
@@ -531,20 +513,40 @@
      *
      * @param deviceId  identifier of the device
      * @param timestamp of the event triggering this change.
-     * @return true if availability change request was accepted and changed the state
+     * @param notifyPeers if the event needs to be notified to peers.
+     * @return ready to send event describing what occurred; null if no change
      */
-    // Guarded by deviceDescs value (=Device lock)
-    private boolean markOnline(DeviceId deviceId, Timestamp timestamp) {
-        // accept on-line if given timestamp is newer than
-        // the latest offline request Timestamp
-        Timestamp offlineTimestamp = offline.get(deviceId);
-        if (offlineTimestamp == null ||
-                offlineTimestamp.compareTo(timestamp) < 0) {
-
-            offline.remove(deviceId);
-            return availableDevices.add(deviceId);
+    private DeviceEvent markOnline(DeviceId deviceId, Timestamp timestamp, boolean notifyPeers) {
+        final DeviceEvent event = markOnlineInternal(deviceId, timestamp);
+        if (event != null && notifyPeers) {
+            log.debug("Notifying peers of a device online topology event for deviceId: {} {}",
+                      deviceId, timestamp);
+            notifyPeers(new InternalDeviceStatusChangeEvent(deviceId, timestamp, true));
         }
-        return false;
+        return event;
+    }
+
+    // Guarded by deviceDescs value (=Device lock)
+    private DeviceEvent markOnlineInternal(DeviceId deviceId, Timestamp timestamp) {
+        if (devices.containsKey(deviceId)) {
+            Map<?, ?> deviceLock = getOrCreateDeviceDescriptionsMap(deviceId);
+            synchronized (deviceLock) {
+                // accept on-line if given timestamp is newer than
+                // the latest offline request Timestamp
+                Timestamp offlineTimestamp = offline.get(deviceId);
+                if (offlineTimestamp == null ||
+                        offlineTimestamp.compareTo(timestamp) < 0) {
+                    offline.remove(deviceId);
+                    Device device = devices.get(deviceId);
+                    boolean add = availableDevices.add(deviceId);
+                    if (add) {
+                        return new DeviceEvent(DEVICE_AVAILABILITY_CHANGED, device, null);
+                    }
+                }
+            }
+        }
+        log.warn("Device {} does not exist in store", deviceId);
+        return null;
     }
 
     @Override
@@ -1261,8 +1263,8 @@
         broadcastMessage(DEVICE_UPDATE, event);
     }
 
-    private void notifyPeers(InternalDeviceOfflineEvent event) {
-        broadcastMessage(GossipDeviceStoreMessageSubjects.DEVICE_OFFLINE, event);
+    private void notifyPeers(InternalDeviceStatusChangeEvent event) {
+        broadcastMessage(GossipDeviceStoreMessageSubjects.DEVICE_STATUS_CHANGE, event);
     }
 
     private void notifyPeers(InternalDeviceRemovedEvent event) {
@@ -1285,9 +1287,9 @@
         }
     }
 
-    private void notifyPeer(NodeId recipient, InternalDeviceOfflineEvent event) {
+    private void notifyPeer(NodeId recipient, InternalDeviceStatusChangeEvent event) {
         try {
-            unicastMessage(recipient, GossipDeviceStoreMessageSubjects.DEVICE_OFFLINE, event);
+            unicastMessage(recipient, GossipDeviceStoreMessageSubjects.DEVICE_STATUS_CHANGE, event);
         } catch (IOException e) {
             log.error("Failed to send" + event + " to " + recipient, e);
         }
@@ -1456,7 +1458,7 @@
                 Timestamp lOffline = offline.get(deviceId);
                 if (lOffline != null && rOffline == null) {
                     // locally offline, but remote is online, suggest offline
-                    notifyPeer(sender, new InternalDeviceOfflineEvent(deviceId, lOffline));
+                    notifyPeer(sender, new InternalDeviceStatusChangeEvent(deviceId, lOffline, false));
                 }
 
                 // remove device offline Ad already processed
@@ -1565,14 +1567,19 @@
         }
     }
 
-    private void handleDeviceOfflineEvent(InternalDeviceOfflineEvent event) {
+    private void handleDeviceStatusChangeEvent(InternalDeviceStatusChangeEvent event) {
         DeviceId deviceId = event.deviceId();
         Timestamp timestamp = event.timestamp();
+        Boolean available = event.available();
 
         try {
-            notifyDelegateIfNotNull(markOfflineInternal(deviceId, timestamp));
+            if (available) {
+                notifyDelegateIfNotNull(markOnlineInternal(deviceId, timestamp));
+            } else {
+                notifyDelegateIfNotNull(markOfflineInternal(deviceId, timestamp));
+            }
         } catch (Exception e) {
-            log.warn("Exception thrown handling device offline", e);
+            log.warn("Exception thrown handling device status change event", e);
         }
     }
 
diff --git a/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStoreMessageSubjects.java b/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStoreMessageSubjects.java
index 38ab9be..d970061 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStoreMessageSubjects.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStoreMessageSubjects.java
@@ -25,7 +25,7 @@
     private GossipDeviceStoreMessageSubjects() {}
 
     public static final MessageSubject DEVICE_UPDATE = new MessageSubject("peer-device-update");
-    public static final MessageSubject DEVICE_OFFLINE = new MessageSubject("peer-device-offline");
+    public static final MessageSubject DEVICE_STATUS_CHANGE = new MessageSubject("peer-device-status-change");
     public static final MessageSubject DEVICE_REMOVE_REQ = new MessageSubject("peer-device-remove-request");
     public static final MessageSubject DEVICE_REMOVED = new MessageSubject("peer-device-removed");
     public static final MessageSubject PORT_UPDATE = new MessageSubject("peer-port-update");
diff --git a/core/store/dist/src/main/java/org/onosproject/store/device/impl/InternalDeviceOfflineEvent.java b/core/store/dist/src/main/java/org/onosproject/store/device/impl/InternalDeviceStatusChangeEvent.java
similarity index 70%
rename from core/store/dist/src/main/java/org/onosproject/store/device/impl/InternalDeviceOfflineEvent.java
rename to core/store/dist/src/main/java/org/onosproject/store/device/impl/InternalDeviceStatusChangeEvent.java
index 028e43d..6763615 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/device/impl/InternalDeviceOfflineEvent.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/device/impl/InternalDeviceStatusChangeEvent.java
@@ -24,19 +24,22 @@
  * Information published by GossipDeviceStore to notify peers of a device
  * going offline.
  */
-public class InternalDeviceOfflineEvent {
+public class InternalDeviceStatusChangeEvent {
 
     private final DeviceId deviceId;
     private final Timestamp timestamp;
+    private final Boolean available;
 
     /**
-     * Creates a InternalDeviceOfflineEvent.
-     * @param deviceId identifier of device going offline.
-     * @param timestamp timestamp of when the device went offline.
+     * Creates a InternalDeviceStatusChangeEvent.
+     * @param deviceId identifier of device.
+     * @param timestamp timestamp of when the device status is changed.
+     * @param available device status is available or not.
      */
-    public InternalDeviceOfflineEvent(DeviceId deviceId, Timestamp timestamp) {
+    public InternalDeviceStatusChangeEvent(DeviceId deviceId, Timestamp timestamp, Boolean available) {
         this.deviceId = deviceId;
         this.timestamp = timestamp;
+        this.available = available;
     }
 
     public DeviceId deviceId() {
@@ -47,18 +50,24 @@
         return timestamp;
     }
 
+    public Boolean available() {
+        return available;
+    }
+
     @Override
     public String toString() {
         return MoreObjects.toStringHelper(getClass())
                 .add("deviceId", deviceId)
                 .add("timestamp", timestamp)
+                .add("available", available)
                 .toString();
     }
 
     // for serializer
     @SuppressWarnings("unused")
-    private InternalDeviceOfflineEvent() {
+    private InternalDeviceStatusChangeEvent() {
         deviceId = null;
         timestamp = null;
+        available = null;
     }
 }
diff --git a/core/store/dist/src/main/java/org/onosproject/store/device/impl/InternalDeviceOfflineEventSerializer.java b/core/store/dist/src/main/java/org/onosproject/store/device/impl/InternalDeviceStatusChangeEventSerializer.java
similarity index 64%
rename from core/store/dist/src/main/java/org/onosproject/store/device/impl/InternalDeviceOfflineEventSerializer.java
rename to core/store/dist/src/main/java/org/onosproject/store/device/impl/InternalDeviceStatusChangeEventSerializer.java
index 7c35587..b97d573 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/device/impl/InternalDeviceOfflineEventSerializer.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/device/impl/InternalDeviceStatusChangeEventSerializer.java
@@ -25,30 +25,32 @@
 import com.esotericsoftware.kryo.io.Output;
 
 /**
- * Kryo Serializer for {@link InternalDeviceOfflineEvent}.
+ * Kryo Serializer for {@link InternalDeviceStatusChangeEvent}.
  */
-public class InternalDeviceOfflineEventSerializer extends Serializer<InternalDeviceOfflineEvent> {
+public class InternalDeviceStatusChangeEventSerializer extends Serializer<InternalDeviceStatusChangeEvent> {
 
     /**
-     * Creates a serializer for {@link InternalDeviceOfflineEvent}.
+     * Creates a serializer for {@link InternalDeviceStatusChangeEvent}.
      */
-    public InternalDeviceOfflineEventSerializer() {
+    public InternalDeviceStatusChangeEventSerializer() {
         // does not accept null
         super(false);
     }
 
     @Override
-    public void write(Kryo kryo, Output output, InternalDeviceOfflineEvent event) {
+    public void write(Kryo kryo, Output output, InternalDeviceStatusChangeEvent event) {
         kryo.writeObject(output, event.deviceId(), deviceIdSerializer());
         kryo.writeClassAndObject(output, event.timestamp());
+        kryo.writeClassAndObject(output, event.available());
     }
 
     @Override
-    public InternalDeviceOfflineEvent read(Kryo kryo, Input input,
-                               Class<InternalDeviceOfflineEvent> type) {
+    public InternalDeviceStatusChangeEvent read(Kryo kryo, Input input,
+                                                Class<InternalDeviceStatusChangeEvent> type) {
         DeviceId deviceId = kryo.readObject(input, DeviceId.class, deviceIdSerializer());
         Timestamp timestamp = (Timestamp) kryo.readClassAndObject(input);
+        Boolean available = (Boolean) kryo.readClassAndObject(input);
 
-        return new InternalDeviceOfflineEvent(deviceId, timestamp);
+        return new InternalDeviceStatusChangeEvent(deviceId, timestamp, available);
     }
 }