Fixed equals methods for better efficiency.

Cleaned up ClusterService API.

@@ -8,7 +8,7 @@ import org.onlab.onos.event.AbstractEvent;
 public class ClusterEvent extends AbstractEvent<ClusterEvent.Type, ControllerInstance> {
 
     /**
-     * Type of device events.
+     * Type of cluster-related events.
      */
     public enum Type {
         /**
@@ -24,14 +24,13 @@ public class ClusterEvent extends AbstractEvent<ClusterEvent.Type, ControllerIns
         /**
          * Signifies that a cluster instance became active.
          */
-        INSTANCE_ACTIVE,
+        INSTANCE_ACTIVATED,
 
         /**
          * Signifies that a cluster instance became inactive.
          */
-        INSTANCE_INACTIVE
+        INSTANCE_DEACTIVATED
     }
-    // TODO: do we need to fix the verv/adjective mix? discuss
 
     /**
      * Creates an event of a given type and for the specified instance and the

+package org.onlab.onos.cluster;
+
+import org.onlab.onos.event.EventListener;
+
+/**
+ * Entity capable of receiving device cluster-related events.
+ */
+public interface ClusterEventListener  extends EventListener<ClusterEvent> {
+}

@@ -21,9 +21,19 @@ public interface ClusterService {
      * @return availability state
      */
     ControllerInstance.State getState(ControllerInstance instance);
-    // TODO: determine if this would be better attached to ControllerInstance directly
 
+    /**
+     * Adds the specified cluster event listener.
+     *
+     * @param listener the cluster listener
+     */
+    void addListener(ClusterEventListener listener);
 
-    // addListener, removeListener
+    /**
+     * Removes the specified cluster event listener.
+     *
+     * @param listener the cluster listener
+     */
+    void removeListener(ClusterEventListener listener);
 
 }

 package org.onlab.onos.cluster;
 
+import java.util.Objects;
+
+import static com.google.common.base.MoreObjects.toStringHelper;
+
 /**
  * Controller cluster identity.
  */
-public interface InstanceId {
+public class InstanceId {
+
+    private final String id;
+
+    // Default constructor for serialization
+    protected InstanceId() {
+        id = null;
+    }
+
+    /**
+     * Creates a new cluster instance identifier from the specified string.
+     *
+     * @param id string identifier
+     */
+    public InstanceId(String id) {
+        this.id = id;
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(id);
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
+        if (obj instanceof InstanceId) {
+            final InstanceId other = (InstanceId) obj;
+            return Objects.equals(this.id, other.id);
+        }
+        return false;
+    }
+
+    @Override
+    public String toString() {
+        return toStringHelper(this).add("id", id).toString();
+    }
+
 }

@@ -6,5 +6,4 @@ import org.onlab.onos.event.EventListener;
  * Entity capable of receiving device mastership-related events.
  */
 public interface MastershipListener extends EventListener<MastershipEvent> {
-
 }

@@ -38,14 +38,14 @@ public interface MastershipService {
     MastershipRole requestRoleFor(DeviceId deviceId);
 
     /**
-     * Adds the specified mastership listener.
+     * Adds the specified mastership change listener.
      *
      * @param listener the mastership listener
      */
     void addListener(MastershipListener listener);
 
     /**
-     * Removes the specified device listener.
+     * Removes the specified mastership change listener.
      *
      * @param listener the mastership listener
      */

@@ -20,8 +20,8 @@ public interface MastershipStore {
      * @param role     new role
      * @return a mastership event
      */
-    MastershipEvent setRole(
-            InstanceId instance, DeviceId deviceId, MastershipRole role);
+    MastershipEvent setRole(InstanceId instance, DeviceId deviceId,
+                            MastershipRole role);
 
     /**
      * Adds or updates the mastership information for a device.
@@ -31,8 +31,8 @@ public interface MastershipStore {
      * @param role     new role
      * @return a mastership event
      */
-    MastershipEvent addOrUpdateDevice(
-            InstanceId instance, DeviceId deviceId, MastershipRole role);
+    MastershipEvent addOrUpdateDevice(InstanceId instance, DeviceId deviceId,
+                                      MastershipRole role);
 
     /**
      * Returns the master for a device.
@@ -45,7 +45,7 @@ public interface MastershipStore {
     /**
      * Returns the devices that a controller instance is master of.
      *
-     * @param  instanceId the instance identifier
+     * @param instanceId the instance identifier
      * @return a set of device identifiers
      */
     Set<DeviceId> getDevices(InstanceId instanceId);
@@ -54,7 +54,7 @@ public interface MastershipStore {
      * Returns the role of a device for a specific controller instance.
      *
      * @param instanceId the instance identifier
-     * @param deviceId the device identifiers
+     * @param deviceId   the device identifiers
      * @return the role
      */
     MastershipRole getRole(InstanceId instanceId, DeviceId deviceId);

@@ -66,6 +66,9 @@ public class ConnectPoint {
 
     @Override
     public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
         if (obj instanceof ConnectPoint) {
             final ConnectPoint other = (ConnectPoint) obj;
             return Objects.equals(this.elementId, other.elementId) &&

@@ -85,6 +85,9 @@ public class DefaultDevice extends AbstractElement implements Device {
 
     @Override
     public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
         if (obj instanceof DefaultDevice) {
             final DefaultDevice other = (DefaultDevice) obj;
             return Objects.equals(this.id, other.id) &&

@@ -63,6 +63,9 @@ public class DefaultHost extends AbstractElement implements Host {
 
     @Override
     public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
         if (obj instanceof DefaultHost) {
             final DefaultHost other = (DefaultHost) obj;
             return Objects.equals(this.id, other.id) &&

@@ -53,6 +53,9 @@ public class DefaultLink extends AbstractModel implements Link {
 
     @Override
     public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
         if (obj instanceof DefaultLink) {
             final DefaultLink other = (DefaultLink) obj;
             return Objects.equals(this.src, other.src) &&

@@ -57,11 +57,14 @@ public class DefaultPath extends DefaultLink implements Path {
 
     @Override
     public int hashCode() {
-        return 31 * super.hashCode() + Objects.hash(links);
+        return Objects.hash(links);
     }
 
     @Override
     public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
         if (obj instanceof DefaultPath) {
             final DefaultPath other = (DefaultPath) obj;
             return Objects.equals(this.links, other.links);

@@ -58,6 +58,9 @@ public class DefaultPort implements Port {
 
     @Override
     public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
         if (obj instanceof DefaultPort) {
             final DefaultPort other = (DefaultPort) obj;
             return Objects.equals(this.element.id(), other.element.id()) &&

@@ -40,6 +40,9 @@ public abstract class ElementId {
 
     @Override
     public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
         if (obj instanceof ElementId) {
             final ElementId that = (ElementId) obj;
             return this.getClass() == that.getClass() &&

@@ -79,6 +79,9 @@ public final class PortNumber {
 
     @Override
     public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
         if (obj instanceof PortNumber) {
             final PortNumber other = (PortNumber) obj;
             return this.number == other.number;

@@ -147,7 +147,6 @@ public class DefaultFlowRule implements FlowRule {
      * @see java.lang.Object#equals(java.lang.Object)
      */
     public boolean equals(Object obj) {
-
         if (this == obj) {
             return true;
         }

@@ -54,6 +54,9 @@ public class DefaultInboundPacket implements InboundPacket {
 
     @Override
     public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
         if (obj instanceof InboundPacket) {
             final DefaultInboundPacket other = (DefaultInboundPacket) obj;
             return Objects.equals(this.receivedFrom, other.receivedFrom) &&

@@ -50,12 +50,12 @@ public class ProviderId {
         if (this == obj) {
             return true;
         }
-        if (obj == null || getClass() != obj.getClass()) {
-            return false;
+        if (obj instanceof ProviderId) {
+            final ProviderId other = (ProviderId) obj;
+            return Objects.equals(this.scheme, other.scheme) &&
+                    Objects.equals(this.id, other.id);
         }
-        final ProviderId other = (ProviderId) obj;
-        return Objects.equals(this.scheme, other.scheme) &&
-                Objects.equals(this.id, other.id);
+        return false;
     }
 
     @Override

@@ -43,6 +43,9 @@ public final class ClusterId {
 
     @Override
     public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
         if (obj instanceof ClusterId) {
             final ClusterId other = (ClusterId) obj;
             return Objects.equals(this.id, other.id);

@@ -59,6 +59,9 @@ public class DefaultTopologyCluster implements TopologyCluster {
 
     @Override
     public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
         if (obj instanceof DefaultTopologyCluster) {
             final DefaultTopologyCluster other = (DefaultTopologyCluster) obj;
             return Objects.equals(this.id, other.id) &&

@@ -50,6 +50,9 @@ public class DefaultTopologyEdge implements TopologyEdge {
 
     @Override
     public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
         if (obj instanceof DefaultTopologyEdge) {
             final DefaultTopologyEdge other = (DefaultTopologyEdge) obj;
             return Objects.equals(this.link, other.link);

@@ -32,6 +32,9 @@ public class DefaultTopologyVertex implements TopologyVertex {
 
     @Override
     public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
         if (obj instanceof DefaultTopologyVertex) {
             final DefaultTopologyVertex other = (DefaultTopologyVertex) obj;
             return Objects.equals(this.deviceId, other.deviceId);

@@ -28,6 +28,9 @@ class PathKey {
 
     @Override
     public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
         if (obj instanceof PathKey) {
             final PathKey other = (PathKey) obj;
             return Objects.equals(this.src, other.src) && Objects.equals(this.dst, other.dst);

@@ -179,6 +179,9 @@ public class SimpleLinkStore implements LinkStore {
 
         @Override
         public boolean equals(Object obj) {
+            if (this == obj) {
+                return true;
+            }
             if (obj instanceof LinkKey) {
                 final LinkKey other = (LinkKey) obj;
                 return Objects.equals(this.src, other.src) &&

@@ -41,6 +41,9 @@ public abstract class AbstractEdge<V extends Vertex> implements Edge<V> {
 
     @Override
     public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
         if (obj instanceof AbstractEdge) {
             final AbstractEdge other = (AbstractEdge) obj;
             return Objects.equals(this.src, other.src) && Objects.equals(this.dst, other.dst);

@@ -80,6 +80,9 @@ public class AdjacencyListsGraph<V extends Vertex, E extends Edge<V>>
 
     @Override
     public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
         if (obj instanceof AdjacencyListsGraph) {
             AdjacencyListsGraph that = (AdjacencyListsGraph) obj;
             return this.getClass() == that.getClass() &&

@@ -107,6 +107,9 @@ public class DefaultMutablePath<V extends Vertex, E extends Edge<V>> implements
 
     @Override
     public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
         if (obj instanceof DefaultMutablePath) {
             final DefaultMutablePath other = (DefaultMutablePath) obj;
             return Objects.equals(this.cost, other.cost) &&

@@ -72,6 +72,9 @@ public class DefaultPath<V extends Vertex, E extends Edge<V>> implements Path<V,
 
     @Override
     public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
         if (obj instanceof DefaultPath) {
             final DefaultPath other = (DefaultPath) obj;
             return Objects.equals(this.src, other.src) &&

@@ -166,6 +166,9 @@ public class Heap<T> {
 
     @Override
     public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
         if (obj instanceof Heap) {
             Heap that = (Heap) obj;
             return this.getClass() == that.getClass() &&

@@ -39,6 +39,9 @@ public class TestEdge extends AbstractEdge<TestVertex> {
 
     @Override
     public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
         if (obj instanceof TestEdge) {
             final TestEdge other = (TestEdge) obj;
             return super.equals(obj) && Objects.equals(this.weight, other.weight);

@@ -20,6 +20,9 @@ public class TestVertex implements Vertex {
 
     @Override
     public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
         if (obj instanceof TestVertex) {
             final TestVertex other = (TestVertex) obj;
             return Objects.equals(this.name, other.name);