modificiations to emit BACKUP_CHANGED Mastership events

Change-Id: Id61dcc9dc42c8c246313afbec8d19142e6c855a5

Conflicts:
	core/net/src/main/java/org/onlab/onos/net/device/impl/DeviceManager.java
diff --git a/core/api/src/main/java/org/onlab/onos/mastership/MastershipEvent.java b/core/api/src/main/java/org/onlab/onos/mastership/MastershipEvent.java
index 60f0dad..5447545 100644
--- a/core/api/src/main/java/org/onlab/onos/mastership/MastershipEvent.java
+++ b/core/api/src/main/java/org/onlab/onos/mastership/MastershipEvent.java
@@ -40,7 +40,9 @@
         MASTER_CHANGED,
 
         /**
-         * Signifies that the list of backup nodes has changed.
+         * Signifies that the list of backup nodes has changed. If
+         * the change in the backups list is accompanied by a change in
+         * master, the event is subsumed by MASTER_CHANGED.
          */
         BACKUPS_CHANGED
     }
diff --git a/core/store/dist/src/main/java/org/onlab/onos/store/mastership/impl/DistributedMastershipStore.java b/core/store/dist/src/main/java/org/onlab/onos/store/mastership/impl/DistributedMastershipStore.java
index fc89e3a..c9d4422 100644
--- a/core/store/dist/src/main/java/org/onlab/onos/store/mastership/impl/DistributedMastershipStore.java
+++ b/core/store/dist/src/main/java/org/onlab/onos/store/mastership/impl/DistributedMastershipStore.java
@@ -16,6 +16,7 @@
 package org.onlab.onos.store.mastership.impl;
 
 import static org.onlab.onos.mastership.MastershipEvent.Type.MASTER_CHANGED;
+import static org.onlab.onos.mastership.MastershipEvent.Type.BACKUPS_CHANGED;
 import static org.apache.commons.lang3.concurrent.ConcurrentUtils.putIfAbsent;
 
 import java.util.HashSet;
@@ -43,6 +44,7 @@
 import org.onlab.onos.store.serializers.KryoSerializer;
 import org.onlab.util.KryoNamespace;
 
+import com.google.common.base.Objects;
 import com.hazelcast.core.EntryEvent;
 import com.hazelcast.core.EntryListener;
 import com.hazelcast.core.IAtomicLong;
@@ -297,8 +299,7 @@
                 case NONE:
                     rv.reassign(nodeId, NONE, STANDBY);
                     roleMap.put(deviceId, rv);
-                    // TODO: BACKUPS_CHANGED?
-                    return null;
+                    return new MastershipEvent(BACKUPS_CHANGED, deviceId, rv.roleInfo());
                 default:
                     log.warn("unknown Mastership Role {}", currentRole);
             }
@@ -327,7 +328,8 @@
                         roleMap.put(deviceId, rv);
                         return new MastershipEvent(MASTER_CHANGED, deviceId, rv.roleInfo());
                     } else {
-                        // no master candidate
+                        // No master candidate - no more backups, device is likely
+                        // fully disconnected
                         roleMap.put(deviceId, rv);
                         // Should there be new event type?
                         return null;
@@ -338,8 +340,7 @@
                     boolean modified = rv.reassign(nodeId, STANDBY, NONE);
                     if (modified) {
                         roleMap.put(deviceId, rv);
-                        // TODO: BACKUPS_CHANGED?
-                        return null;
+                        return new MastershipEvent(BACKUPS_CHANGED, deviceId, rv.roleInfo());
                     }
                     return null;
                 default:
@@ -441,8 +442,18 @@
 
         @Override
         public void entryUpdated(EntryEvent<DeviceId, RoleValue> event) {
-            notifyDelegate(new MastershipEvent(
-                    MASTER_CHANGED, event.getKey(), event.getValue().roleInfo()));
+            // compare old and current RoleValues. If master is different,
+            // emit MASTER_CHANGED. else, emit BACKUPS_CHANGED.
+            RoleValue oldValue = event.getOldValue();
+            RoleValue newValue = event.getValue();
+
+            if (Objects.equal(oldValue.get(MASTER), newValue.get(MASTER))) {
+                notifyDelegate(new MastershipEvent(
+                        MASTER_CHANGED, event.getKey(), event.getValue().roleInfo()));
+            } else {
+                notifyDelegate(new MastershipEvent(
+                        BACKUPS_CHANGED, event.getKey(), event.getValue().roleInfo()));
+            }
         }
 
         @Override
diff --git a/core/store/dist/src/main/java/org/onlab/onos/store/mastership/impl/RoleValue.java b/core/store/dist/src/main/java/org/onlab/onos/store/mastership/impl/RoleValue.java
index 8cc05e8..aa5d911 100644
--- a/core/store/dist/src/main/java/org/onlab/onos/store/mastership/impl/RoleValue.java
+++ b/core/store/dist/src/main/java/org/onlab/onos/store/mastership/impl/RoleValue.java
@@ -55,6 +55,13 @@
         return value.get(type);
     }
 
+    /**
+     * Returns the first node to match the MastershipRole, or if there
+     * are none, null.
+     *
+     * @param type the role
+     * @return a node ID or null
+     */
     public NodeId get(MastershipRole type) {
         return value.get(type).isEmpty() ? null : value.get(type).get(0);
     }
diff --git a/core/store/dist/src/test/java/org/onlab/onos/store/mastership/impl/DistributedMastershipStoreTest.java b/core/store/dist/src/test/java/org/onlab/onos/store/mastership/impl/DistributedMastershipStoreTest.java
index f598374..d3b7ad1 100644
--- a/core/store/dist/src/test/java/org/onlab/onos/store/mastership/impl/DistributedMastershipStoreTest.java
+++ b/core/store/dist/src/test/java/org/onlab/onos/store/mastership/impl/DistributedMastershipStoreTest.java
@@ -214,11 +214,11 @@
                 dms.roleMap.get(DID1).nodesOfRole(STANDBY).size());
 
         //If STANDBY, should drop to NONE
-        assertNull("wrong event:", dms.relinquishRole(N1, DID1));
+        assertEquals("wrong event:", Type.BACKUPS_CHANGED, dms.relinquishRole(N1, DID1).type());
         assertEquals("wrong role for node:", NONE, dms.getRole(N1, DID1));
 
         //NONE - nothing happens
-        assertNull("wrong event:", dms.relinquishRole(N1, DID2));
+        assertEquals("wrong event:", Type.BACKUPS_CHANGED, dms.relinquishRole(N1, DID2).type());
         assertEquals("wrong role for node:", NONE, dms.getRole(N1, DID2));
 
     }
diff --git a/openflow/ctl/src/main/java/org/onlab/onos/openflow/controller/impl/RoleManager.java b/openflow/ctl/src/main/java/org/onlab/onos/openflow/controller/impl/RoleManager.java
index 166a66b..6e5b236 100644
--- a/openflow/ctl/src/main/java/org/onlab/onos/openflow/controller/impl/RoleManager.java
+++ b/openflow/ctl/src/main/java/org/onlab/onos/openflow/controller/impl/RoleManager.java
@@ -51,13 +51,6 @@
  * After a role request is submitted the role changer keeps track of the
  * pending request, collects the reply (if any) and times out the request
  * if necessary.
- *
- * To simplify role handling we only keep track of the /last/ pending
- * role reply send to the switch. If multiple requests are pending and
- * we receive replies for earlier requests we ignore them. However, this
- * way of handling pending requests implies that we could wait forever if
- * a new request is submitted before the timeout triggers. If necessary
- * we could work around that though.
  */
 class RoleManager implements RoleHandler {
     protected static final long NICIRA_EXPERIMENTER = 0x2320;