ONOS-895: Group manager implementation

Change-Id: Ie183f722fa39012f8de056961715c325e2388e63

@@ -37,7 +37,7 @@ public class DefaultGroupId implements GroupId {
 
 
     @Override
     @Override
     public int id() {
     public int id() {
-        return 0;
+        return this.id;
     }
     }
 
 
     @Override
     @Override

@@ -17,7 +17,10 @@ package org.onosproject.net.group;
 
 
 import static org.slf4j.LoggerFactory.getLogger;
 import static org.slf4j.LoggerFactory.getLogger;
 
 
+import java.util.Objects;
+
 import org.onosproject.core.GroupId;
 import org.onosproject.core.GroupId;
+import org.onosproject.net.DeviceId;
 import org.slf4j.Logger;
 import org.slf4j.Logger;
 
 
 /**
 /**
@@ -32,6 +35,7 @@ public class DefaultGroup extends DefaultGroupDescription
     private long life;
     private long life;
     private long packets;
     private long packets;
     private long bytes;
     private long bytes;
+    private long referenceCount;
     private GroupId id;
     private GroupId id;
 
 
     /**
     /**
@@ -47,6 +51,29 @@ public class DefaultGroup extends DefaultGroupDescription
         this.life = 0;
         this.life = 0;
         this.packets = 0;
         this.packets = 0;
         this.bytes = 0;
         this.bytes = 0;
+        this.referenceCount = 0;
+    }
+
+    /**
+     * Default group object constructor with the available information
+     * from data plane.
+     *
+     * @param id group identifier
+     * @param deviceId device identifier
+     * @param type type of the group
+     * @param buckets immutable list of group bucket
+     */
+    public DefaultGroup(GroupId id,
+                        DeviceId deviceId,
+                        GroupDescription.Type type,
+                        GroupBuckets buckets) {
+        super(deviceId, type, buckets);
+        this.id = id;
+        this.state = GroupState.PENDING_ADD;
+        this.life = 0;
+        this.packets = 0;
+        this.bytes = 0;
+        this.referenceCount = 0;
     }
     }
 
 
     /**
     /**
@@ -139,4 +166,43 @@ public class DefaultGroup extends DefaultGroupDescription
         this.bytes = bytes;
         this.bytes = bytes;
     }
     }
 
 
+    @Override
+    public void setReferenceCount(long referenceCount) {
+        this.referenceCount = referenceCount;
+    }
+
+    @Override
+    public long referenceCount() {
+        return referenceCount;
+    }
+
+    /*
+     * The deviceId, type and buckets are used for hash.
+     *
+     * (non-Javadoc)
+     * @see java.lang.Object#equals(java.lang.Object)
+     */
+    @Override
+    public int hashCode() {
+        return super.hashCode() + Objects.hash(id);
+    }
+
+    /*
+     * The deviceId, groupId, type and buckets should be same.
+     *
+     * (non-Javadoc)
+     * @see java.lang.Object#equals(java.lang.Object)
+     */
+    @Override
+    public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
+       if (obj instanceof DefaultGroup) {
+            DefaultGroup that = (DefaultGroup) obj;
+            return super.equals(obj) &&
+                    Objects.equals(id, that.id);
+        }
+        return false;
+    }
 }
 }

@@ -18,6 +18,8 @@ package org.onosproject.net.group;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkNotNull;
 
 
+import java.util.Objects;
+
 import org.onosproject.core.GroupId;
 import org.onosproject.core.GroupId;
 import org.onosproject.net.PortNumber;
 import org.onosproject.net.PortNumber;
 import org.onosproject.net.flow.TrafficTreatment;
 import org.onosproject.net.flow.TrafficTreatment;
@@ -178,4 +180,36 @@ public final class DefaultGroupBucket implements GroupBucket {
     public GroupId watchGroup() {
     public GroupId watchGroup() {
         return watchGroup;
         return watchGroup;
     }
     }
+
+    /*
+     * The type and treatment can change on a given bucket
+     *
+     * (non-Javadoc)
+     * @see java.lang.Object#equals(java.lang.Object)
+     */
+    @Override
+    public int hashCode() {
+        return Objects.hash(type, treatment);
+    }
+
+    /*
+     * The priority and statistics can change on a given treatment and selector
+     *
+     * (non-Javadoc)
+     * @see java.lang.Object#equals(java.lang.Object)
+     */
+    @Override
+    public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
+        if (obj instanceof DefaultGroupBucket) {
+            DefaultGroupBucket that = (DefaultGroupBucket) obj;
+            return Objects.equals(type, that.type) &&
+                   this.treatment.instructions().containsAll(that.treatment.instructions()) &&
+                   that.treatment.instructions().containsAll(this.treatment.instructions());
+        }
+        return false;
+    }
+
 }
 }

@@ -17,6 +17,8 @@ package org.onosproject.net.group;
 
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkNotNull;
 
 
+import java.util.Objects;
+
 import org.onosproject.core.ApplicationId;
 import org.onosproject.core.ApplicationId;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.DeviceId;
 
 
@@ -49,8 +51,8 @@ public class DefaultGroupDescription implements GroupDescription {
         this.type = checkNotNull(type);
         this.type = checkNotNull(type);
         this.deviceId = checkNotNull(deviceId);
         this.deviceId = checkNotNull(deviceId);
         this.buckets = checkNotNull(buckets);
         this.buckets = checkNotNull(buckets);
-        this.appCookie = checkNotNull(appCookie);
-        this.appId = checkNotNull(appId);
+        this.appCookie = appCookie;
+        this.appId = appId;
     }
     }
 
 
     /**
     /**
@@ -61,11 +63,27 @@ public class DefaultGroupDescription implements GroupDescription {
      *
      *
      */
      */
     public DefaultGroupDescription(GroupDescription groupDesc) {
     public DefaultGroupDescription(GroupDescription groupDesc) {
-        this.type = checkNotNull(groupDesc.type());
-        this.deviceId = checkNotNull(groupDesc.deviceId());
-        this.buckets = checkNotNull(groupDesc.buckets());
-        this.appCookie = checkNotNull(groupDesc.appCookie());
-        this.appId = checkNotNull(groupDesc.appId());
+        this.type = groupDesc.type();
+        this.deviceId = groupDesc.deviceId();
+        this.buckets = groupDesc.buckets();
+        this.appCookie = groupDesc.appCookie();
+        this.appId = groupDesc.appId();
+    }
+
+    /**
+     * Constructor to be used by group subsystem internal components.
+     * Creates group description object from the information retrieved
+     * from data plane.
+     *
+     * @param deviceId device identifier
+     * @param type type of the group
+     * @param buckets immutable list of group bucket
+     *
+     */
+    public DefaultGroupDescription(DeviceId deviceId,
+                                   GroupDescription.Type type,
+                                   GroupBuckets buckets) {
+        this(deviceId, type, buckets, null, null);
     }
     }
 
 
     /**
     /**
@@ -118,4 +136,36 @@ public class DefaultGroupDescription implements GroupDescription {
         return this.buckets;
         return this.buckets;
     }
     }
 
 
+    @Override
+    /*
+     * The deviceId, type and buckets are used for hash.
+     *
+     * (non-Javadoc)
+     * @see java.lang.Object#equals(java.lang.Object)
+     */
+    public int hashCode() {
+        return Objects.hash(deviceId, type, buckets);
+    }
+
+    @Override
+    /*
+     * The deviceId, type and buckets should be same.
+     *
+     * (non-Javadoc)
+     * @see java.lang.Object#equals(java.lang.Object)
+     */
+    public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
+       if (obj instanceof DefaultGroupDescription) {
+            DefaultGroupDescription that = (DefaultGroupDescription) obj;
+            return Objects.equals(deviceId, that.deviceId) &&
+                    Objects.equals(type, that.type) &&
+                    Objects.equals(buckets, that.buckets);
+
+        }
+        return false;
+    }
+
 }
 }
\ No newline at end of file

@@ -26,6 +26,10 @@ public interface Group extends GroupDescription {
      */
      */
     public enum GroupState {
     public enum GroupState {
         /**
         /**
+         * Group create request is queued as group AUDIT is in progress.
+         */
+        WAITING_AUDIT_COMPLETE,
+        /**
          * Group create request is processed by ONOS and not yet
          * Group create request is processed by ONOS and not yet
          * received the confirmation from data plane.
          * received the confirmation from data plane.
          */
          */
@@ -81,4 +85,11 @@ public interface Group extends GroupDescription {
      * @return number of bytes
      * @return number of bytes
      */
      */
     long bytes();
     long bytes();
+
+    /**
+     * Returns the number of flow rules or other groups reference this group.
+     *
+     * @return number of flow rules or other groups pointing to this group
+     */
+    long referenceCount();
 }
 }

@@ -45,4 +45,25 @@ public final class GroupBuckets {
         return buckets;
         return buckets;
     }
     }
 
 
+    @Override
+    public int hashCode() {
+        int result = 17;
+        int combinedHash = 0;
+        for (GroupBucket bucket:buckets) {
+            combinedHash = combinedHash + bucket.hashCode();
+        }
+        result = 31 * result + combinedHash;
+
+        return result;
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (obj instanceof GroupBuckets) {
+            return (this.buckets.containsAll(((GroupBuckets) obj).buckets) &&
+                    ((GroupBuckets) obj).buckets.containsAll(this.buckets));
+        }
+        return false;
+    }
+
 }
 }
\ No newline at end of file

@@ -17,6 +17,8 @@ package org.onosproject.net.group;
 
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkNotNull;
 
 
+import java.util.Objects;
+
 import org.onosproject.core.GroupId;
 import org.onosproject.core.GroupId;
 
 
 /**
 /**
@@ -142,4 +144,37 @@ public final class GroupOperation {
     public GroupBuckets buckets() {
     public GroupBuckets buckets() {
         return this.buckets;
         return this.buckets;
     }
     }
+
+    @Override
+    /*
+     * The deviceId, type and buckets are used for hash.
+     *
+     * (non-Javadoc)
+     * @see java.lang.Object#equals(java.lang.Object)
+     */
+    public int hashCode() {
+        return (buckets != null) ? Objects.hash(groupId, opType, buckets) :
+            Objects.hash(groupId, opType);
+    }
+
+    @Override
+    /*
+     * The deviceId, type and buckets should be same.
+     *
+     * (non-Javadoc)
+     * @see java.lang.Object#equals(java.lang.Object)
+     */
+    public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
+       if (obj instanceof GroupOperation) {
+           GroupOperation that = (GroupOperation) obj;
+           return Objects.equals(groupId, that.groupId) &&
+                   Objects.equals(opType, that.opType) &&
+                   Objects.equals(buckets, that.buckets);
+
+        }
+        return false;
+    }
 }
 }
\ No newline at end of file

+/*
+ * Copyright 2015 Open Networking Laboratory
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.onosproject.net.group;
+
+import org.onosproject.net.provider.ProviderRegistry;
+
+/**
+ * Abstraction for a group provider registry.
+ */
+public interface GroupProviderRegistry
+        extends ProviderRegistry<GroupProvider, GroupProviderService> {
+}

@@ -16,7 +16,6 @@
 package org.onosproject.net.group;
 package org.onosproject.net.group;
 
 
 import org.onosproject.core.ApplicationId;
 import org.onosproject.core.ApplicationId;
-import org.onosproject.net.Device;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.DeviceId;
 
 
 /**
 /**
@@ -27,7 +26,7 @@ import org.onosproject.net.DeviceId;
  * specified in a group.
  * specified in a group.
  * "group" can also be used for grouping common actions of different flows,
  * "group" can also be used for grouping common actions of different flows,
  * so that in some scenarios only one group entry required to be modified
  * so that in some scenarios only one group entry required to be modified
- * for all the referencing flow entries instead of modifying all of them
+ * for all the referencing flow entries instead of modifying all of them.
  *
  *
  * This implements semantics of a distributed authoritative group store
  * This implements semantics of a distributed authoritative group store
  * where the master copy of the groups lies with the controller and
  * where the master copy of the groups lies with the controller and
@@ -60,7 +59,7 @@ public interface GroupService {
      * NOTE1: The presence of group object in the system does not
      * NOTE1: The presence of group object in the system does not
      * guarantee that the "group" is actually created in device.
      * guarantee that the "group" is actually created in device.
      * GROUP_ADDED notification would confirm the creation of
      * GROUP_ADDED notification would confirm the creation of
-     * this group in data plane
+     * this group in data plane.
      *
      *
      * @param deviceId device identifier
      * @param deviceId device identifier
      * @param appCookie application cookie to be used for lookup
      * @param appCookie application cookie to be used for lookup
@@ -73,7 +72,7 @@ public interface GroupService {
      * Appends buckets to existing group. The caller can optionally
      * Appends buckets to existing group. The caller can optionally
      * associate a new cookie during this updation. GROUP_UPDATED or
      * associate a new cookie during this updation. GROUP_UPDATED or
      * GROUP_UPDATE_FAILED notifications would be provided along with
      * GROUP_UPDATE_FAILED notifications would be provided along with
-     * cookie depending on the result of the operation on the device
+     * cookie depending on the result of the operation on the device.
      *
      *
      * @param deviceId device identifier
      * @param deviceId device identifier
      * @param oldCookie cookie to be used to retrieve the existing group
      * @param oldCookie cookie to be used to retrieve the existing group
@@ -91,7 +90,7 @@ public interface GroupService {
      * Removes buckets from existing group. The caller can optionally
      * Removes buckets from existing group. The caller can optionally
      * associate a new cookie during this updation. GROUP_UPDATED or
      * associate a new cookie during this updation. GROUP_UPDATED or
      * GROUP_UPDATE_FAILED notifications would be provided along with
      * GROUP_UPDATE_FAILED notifications would be provided along with
-     * cookie depending on the result of the operation on the device
+     * cookie depending on the result of the operation on the device.
      *
      *
      * @param deviceId device identifier
      * @param deviceId device identifier
      * @param oldCookie cookie to be used to retrieve the existing group
      * @param oldCookie cookie to be used to retrieve the existing group
@@ -99,7 +98,7 @@ public interface GroupService {
      * @param newCookie immutable cookie to be used post update operation
      * @param newCookie immutable cookie to be used post update operation
      * @param appId Application Id
      * @param appId Application Id
      */
      */
-    void removeBucketsFromGroup(Device deviceId,
+    void removeBucketsFromGroup(DeviceId deviceId,
                                 GroupKey oldCookie,
                                 GroupKey oldCookie,
                                 GroupBuckets buckets,
                                 GroupBuckets buckets,
                                 GroupKey newCookie,
                                 GroupKey newCookie,
@@ -109,13 +108,13 @@ public interface GroupService {
      * Deletes a group associated to an application cookie.
      * Deletes a group associated to an application cookie.
      * GROUP_DELETED or GROUP_DELETE_FAILED notifications would be
      * GROUP_DELETED or GROUP_DELETE_FAILED notifications would be
      * provided along with cookie depending on the result of the
      * provided along with cookie depending on the result of the
-     * operation on the device
+     * operation on the device.
      *
      *
      * @param deviceId device identifier
      * @param deviceId device identifier
      * @param appCookie application cookie to be used for lookup
      * @param appCookie application cookie to be used for lookup
      * @param appId Application Id
      * @param appId Application Id
      */
      */
-    void removeGroup(Device deviceId, GroupKey appCookie, ApplicationId appId);
+    void removeGroup(DeviceId deviceId, GroupKey appCookie, ApplicationId appId);
 
 
     /**
     /**
      * Retrieves all groups created by an application in the specified device
      * Retrieves all groups created by an application in the specified device
@@ -125,7 +124,7 @@ public interface GroupService {
      * @param appId application id
      * @param appId application id
      * @return collection of immutable group objects created by the application
      * @return collection of immutable group objects created by the application
      */
      */
-    Iterable<Group> getGroups(Device deviceId, ApplicationId appId);
+    Iterable<Group> getGroups(DeviceId deviceId, ApplicationId appId);
 
 
     /**
     /**
      * Adds the specified group listener.
      * Adds the specified group listener.

@@ -73,12 +73,14 @@ public interface GroupStore extends Store<GroupEvent, GroupStoreDelegate> {
      * @param deviceId the device ID
      * @param deviceId the device ID
      * @param oldAppCookie the current group key
      * @param oldAppCookie the current group key
      * @param type update type
      * @param type update type
-     * @param newGroupDesc group description with updates
+     * @param newBuckets group buckets for updates
+     * @param newAppCookie optional new group key
      */
      */
     void updateGroupDescription(DeviceId deviceId,
     void updateGroupDescription(DeviceId deviceId,
                                 GroupKey oldAppCookie,
                                 GroupKey oldAppCookie,
                                 UpdateType type,
                                 UpdateType type,
-                                GroupDescription newGroupDesc);
+                                GroupBuckets newBuckets,
+                                GroupKey newAppCookie);
 
 
     /**
     /**
      * Triggers deleting the existing group entry.
      * Triggers deleting the existing group entry.
@@ -102,4 +104,43 @@ public interface GroupStore extends Store<GroupEvent, GroupStoreDelegate> {
      * @param group group entry
      * @param group group entry
      */
      */
     void removeGroupEntry(Group group);
     void removeGroupEntry(Group group);
+
+    /**
+     * A group entry that is present in switch but not in the store.
+     *
+     * @param group group entry
+     */
+    void addOrUpdateExtraneousGroupEntry(Group group);
+
+    /**
+     * Remove the group entry from extraneous database.
+     *
+     * @param group group entry
+     */
+    void removeExtraneousGroupEntry(Group group);
+
+    /**
+     * Returns the extraneous groups associated with a device.
+     *
+     * @param deviceId the device ID
+     *
+     * @return the extraneous group entries
+     */
+    Iterable<Group> getExtraneousGroups(DeviceId deviceId);
+
+    /**
+     * Indicates the first group audit is completed.
+     *
+     * @param deviceId the device ID
+     */
+    void deviceInitialAuditCompleted(DeviceId deviceId);
+
+    /**
+     * Retrieves the initial group audit status for a device.
+     *
+     * @param deviceId the device ID
+     *
+     * @return initial group audit status
+     */
+    boolean deviceInitialAuditStatus(DeviceId deviceId);
 }
 }

@@ -49,4 +49,10 @@ public interface StoredGroupEntry extends Group {
      */
      */
     void setBytes(long bytes);
     void setBytes(long bytes);
 
 
+    /**
+     * Sets number of flow rules or groups referencing this group entry.
+     *
+     * @param referenceCount reference count
+     */
+    void setReferenceCount(long referenceCount);
 }
 }

+/*
+ * Copyright 2015 Open Networking Laboratory
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * Core subsystem for group state.
+ */
+package org.onosproject.net.group.impl;
\ No newline at end of file

@@ -40,8 +40,10 @@ import org.onosproject.net.group.GroupBuckets;
 import org.onosproject.net.group.GroupDescription;
 import org.onosproject.net.group.GroupDescription;
 import org.onosproject.net.group.GroupEvent;
 import org.onosproject.net.group.GroupEvent;
 import org.onosproject.net.group.GroupKey;
 import org.onosproject.net.group.GroupKey;
-import org.onosproject.net.group.GroupStoreDelegate;
 import org.onosproject.net.group.GroupStore.UpdateType;
 import org.onosproject.net.group.GroupStore.UpdateType;
+import org.onosproject.net.group.GroupStoreDelegate;
+
+import com.google.common.collect.Iterables;
 
 
 /**
 /**
  * Test of the simple DeviceStore implementation.
  * Test of the simple DeviceStore implementation.
@@ -135,8 +137,24 @@ public class SimpleGroupStoreTest {
         }
         }
     }
     }
 
 
+    /**
+     * Tests group store operations. The following operations are tested:
+     * a)Tests device group audit completion status change
+     * b)Tests storeGroup operation
+     * c)Tests getGroupCount operation
+     * d)Tests getGroup operation
+     * e)Tests getGroups operation
+     * f)Tests addOrUpdateGroupEntry operation from southbound
+     * g)Tests updateGroupDescription for ADD operation from northbound
+     * h)Tests updateGroupDescription for REMOVE operation from northbound
+     * i)Tests deleteGroupDescription operation from northbound
+     * j)Tests removeGroupEntry operation from southbound
+     */
     @Test
     @Test
     public void testGroupStoreOperations() {
     public void testGroupStoreOperations() {
+        // Set the Device AUDIT completed in the store
+        simpleGroupStore.deviceInitialAuditCompleted(D1);
+
         ApplicationId appId =
         ApplicationId appId =
                 new DefaultApplicationId(2, "org.groupstore.test");
                 new DefaultApplicationId(2, "org.groupstore.test");
         TestGroupKey key = new TestGroupKey("group1");
         TestGroupKey key = new TestGroupKey("group1");
@@ -169,17 +187,17 @@ public class SimpleGroupStoreTest {
                                                groupBuckets,
                                                groupBuckets,
                                                GroupEvent.Type.GROUP_ADD_REQUESTED);
                                                GroupEvent.Type.GROUP_ADD_REQUESTED);
         simpleGroupStore.setDelegate(checkStoreGroupDelegate);
         simpleGroupStore.setDelegate(checkStoreGroupDelegate);
-        /* Testing storeGroup operation */
+        // Testing storeGroup operation
         simpleGroupStore.storeGroupDescription(groupDesc);
         simpleGroupStore.storeGroupDescription(groupDesc);
 
 
-        /* Testing getGroupCount operation */
+        // Testing getGroupCount operation
         assertEquals(1, simpleGroupStore.getGroupCount(D1));
         assertEquals(1, simpleGroupStore.getGroupCount(D1));
 
 
-        /* Testing getGroup operation */
+        // Testing getGroup operation
         Group createdGroup = simpleGroupStore.getGroup(D1, key);
         Group createdGroup = simpleGroupStore.getGroup(D1, key);
         checkStoreGroupDelegate.verifyGroupId(createdGroup.id());
         checkStoreGroupDelegate.verifyGroupId(createdGroup.id());
 
 
-        /* Testing getGroups operation */
+        // Testing getGroups operation
         Iterable<Group> createdGroups = simpleGroupStore.getGroups(D1);
         Iterable<Group> createdGroups = simpleGroupStore.getGroups(D1);
         int groupCount = 0;
         int groupCount = 0;
         for (Group group:createdGroups) {
         for (Group group:createdGroups) {
@@ -189,7 +207,7 @@ public class SimpleGroupStoreTest {
         assertEquals(1, groupCount);
         assertEquals(1, groupCount);
         simpleGroupStore.unsetDelegate(checkStoreGroupDelegate);
         simpleGroupStore.unsetDelegate(checkStoreGroupDelegate);
 
 
-        /* Testing addOrUpdateGroupEntry operation from southbound */
+        // Testing addOrUpdateGroupEntry operation from southbound
         InternalGroupStoreDelegate addGroupEntryDelegate =
         InternalGroupStoreDelegate addGroupEntryDelegate =
                 new InternalGroupStoreDelegate(key,
                 new InternalGroupStoreDelegate(key,
                                                groupBuckets,
                                                groupBuckets,
@@ -198,7 +216,7 @@ public class SimpleGroupStoreTest {
         simpleGroupStore.addOrUpdateGroupEntry(createdGroup);
         simpleGroupStore.addOrUpdateGroupEntry(createdGroup);
         simpleGroupStore.unsetDelegate(addGroupEntryDelegate);
         simpleGroupStore.unsetDelegate(addGroupEntryDelegate);
 
 
-        /* Testing updateGroupDescription for ADD operation from northbound */
+        // Testing updateGroupDescription for ADD operation from northbound
         TestGroupKey addKey = new TestGroupKey("group1AddBuckets");
         TestGroupKey addKey = new TestGroupKey("group1AddBuckets");
         PortNumber[] newNeighborPorts = {PortNumber.portNumber(41),
         PortNumber[] newNeighborPorts = {PortNumber.portNumber(41),
                                          PortNumber.portNumber(42)};
                                          PortNumber.portNumber(42)};
@@ -225,19 +243,14 @@ public class SimpleGroupStoreTest {
                                                updatedGroupBuckets,
                                                updatedGroupBuckets,
                                                GroupEvent.Type.GROUP_UPDATE_REQUESTED);
                                                GroupEvent.Type.GROUP_UPDATE_REQUESTED);
         simpleGroupStore.setDelegate(updateGroupDescDelegate);
         simpleGroupStore.setDelegate(updateGroupDescDelegate);
-        GroupDescription newGroupDesc = new DefaultGroupDescription(
-                                                 D1,
-                                                 Group.Type.SELECT,
-                                                 toAddGroupBuckets,
-                                                 addKey,
-                                                 appId);
         simpleGroupStore.updateGroupDescription(D1,
         simpleGroupStore.updateGroupDescription(D1,
                                                 key,
                                                 key,
                                                 UpdateType.ADD,
                                                 UpdateType.ADD,
-                                                newGroupDesc);
+                                                toAddGroupBuckets,
+                                                addKey);
         simpleGroupStore.unsetDelegate(updateGroupDescDelegate);
         simpleGroupStore.unsetDelegate(updateGroupDescDelegate);
 
 
-        /* Testing updateGroupDescription for REMOVE operation from northbound */
+        // Testing updateGroupDescription for REMOVE operation from northbound
         TestGroupKey removeKey = new TestGroupKey("group1RemoveBuckets");
         TestGroupKey removeKey = new TestGroupKey("group1RemoveBuckets");
         List<GroupBucket> toRemoveBuckets = new ArrayList<GroupBucket>();
         List<GroupBucket> toRemoveBuckets = new ArrayList<GroupBucket>();
         toRemoveBuckets.add(updatedGroupBuckets.buckets().get(0));
         toRemoveBuckets.add(updatedGroupBuckets.buckets().get(0));
@@ -252,23 +265,18 @@ public class SimpleGroupStoreTest {
                                                remainingGroupBuckets,
                                                remainingGroupBuckets,
                                                GroupEvent.Type.GROUP_UPDATE_REQUESTED);
                                                GroupEvent.Type.GROUP_UPDATE_REQUESTED);
         simpleGroupStore.setDelegate(removeGroupDescDelegate);
         simpleGroupStore.setDelegate(removeGroupDescDelegate);
-        GroupDescription removeGroupDesc = new DefaultGroupDescription(
-                                                        D1,
-                                                        Group.Type.SELECT,
-                                                        toRemoveGroupBuckets,
-                                                        removeKey,
-                                                        appId);
         simpleGroupStore.updateGroupDescription(D1,
         simpleGroupStore.updateGroupDescription(D1,
                                                 addKey,
                                                 addKey,
                                                 UpdateType.REMOVE,
                                                 UpdateType.REMOVE,
-                                                removeGroupDesc);
+                                                toRemoveGroupBuckets,
+                                                removeKey);
         simpleGroupStore.unsetDelegate(removeGroupDescDelegate);
         simpleGroupStore.unsetDelegate(removeGroupDescDelegate);
 
 
-        /* Testing getGroup operation */
+        // Testing getGroup operation
         Group existingGroup = simpleGroupStore.getGroup(D1, removeKey);
         Group existingGroup = simpleGroupStore.getGroup(D1, removeKey);
         checkStoreGroupDelegate.verifyGroupId(existingGroup.id());
         checkStoreGroupDelegate.verifyGroupId(existingGroup.id());
 
 
-        /* Testing addOrUpdateGroupEntry operation from southbound */
+        // Testing addOrUpdateGroupEntry operation from southbound
         InternalGroupStoreDelegate updateGroupEntryDelegate =
         InternalGroupStoreDelegate updateGroupEntryDelegate =
                 new InternalGroupStoreDelegate(removeKey,
                 new InternalGroupStoreDelegate(removeKey,
                                                remainingGroupBuckets,
                                                remainingGroupBuckets,
@@ -277,7 +285,7 @@ public class SimpleGroupStoreTest {
         simpleGroupStore.addOrUpdateGroupEntry(existingGroup);
         simpleGroupStore.addOrUpdateGroupEntry(existingGroup);
         simpleGroupStore.unsetDelegate(updateGroupEntryDelegate);
         simpleGroupStore.unsetDelegate(updateGroupEntryDelegate);
 
 
-        /* Testing deleteGroupDescription operation from northbound */
+        // Testing deleteGroupDescription operation from northbound
         InternalGroupStoreDelegate deleteGroupDescDelegate =
         InternalGroupStoreDelegate deleteGroupDescDelegate =
                 new InternalGroupStoreDelegate(removeKey,
                 new InternalGroupStoreDelegate(removeKey,
                                                remainingGroupBuckets,
                                                remainingGroupBuckets,
@@ -286,7 +294,7 @@ public class SimpleGroupStoreTest {
         simpleGroupStore.deleteGroupDescription(D1, removeKey);
         simpleGroupStore.deleteGroupDescription(D1, removeKey);
         simpleGroupStore.unsetDelegate(deleteGroupDescDelegate);
         simpleGroupStore.unsetDelegate(deleteGroupDescDelegate);
 
 
-        /* Testing removeGroupEntry operation from southbound */
+        // Testing removeGroupEntry operation from southbound
         InternalGroupStoreDelegate removeGroupEntryDelegate =
         InternalGroupStoreDelegate removeGroupEntryDelegate =
                 new InternalGroupStoreDelegate(removeKey,
                 new InternalGroupStoreDelegate(removeKey,
                                                remainingGroupBuckets,
                                                remainingGroupBuckets,
@@ -294,17 +302,10 @@ public class SimpleGroupStoreTest {
         simpleGroupStore.setDelegate(removeGroupEntryDelegate);
         simpleGroupStore.setDelegate(removeGroupEntryDelegate);
         simpleGroupStore.removeGroupEntry(existingGroup);
         simpleGroupStore.removeGroupEntry(existingGroup);
 
 
-        /* Testing getGroup operation */
+        // Testing getGroup operation
         existingGroup = simpleGroupStore.getGroup(D1, removeKey);
         existingGroup = simpleGroupStore.getGroup(D1, removeKey);
         assertEquals(null, existingGroup);
         assertEquals(null, existingGroup);
-        Iterable<Group> existingGroups  = simpleGroupStore.getGroups(D1);
-        groupCount = 0;
-        for (Group tmp:existingGroups) {
-            /* To avoid warning */
-            assertEquals(null, tmp);
-            groupCount++;
-        }
-        assertEquals(0, groupCount);
+        assertEquals(0, Iterables.size(simpleGroupStore.getGroups(D1)));
         assertEquals(0, simpleGroupStore.getGroupCount(D1));
         assertEquals(0, simpleGroupStore.getGroupCount(D1));
 
 
         simpleGroupStore.unsetDelegate(removeGroupEntryDelegate);
         simpleGroupStore.unsetDelegate(removeGroupEntryDelegate);