ONOS-4751-Fix:creating GroupType INDIRECT, multiple bucket(REST API)

Change-Id: Ic8d5a04b8b00ca43bfadbb3019aa5d72e799da6f

ONOS-4751-Fix:creating GroupType INDIRECT, multiple bucket(REST API)
Change-Id: Ic8d5a04b8b00ca43bfadbb3019aa5d72e799da6f
Jayasree Ghosh · Gerrit Code Review
Commit 2d45985953e890668070f799847b4af34ee042b3 2d459859 1 parent 451175eb
Showing 5 changed files with 39 additions and 14 deletions
core/api/src/main/java/org/onosproject/net/group/DefaultGroupDescription.java
core/api/src/test/java/org/onosproject/net/group/DefaultGroupTest.java
core/common/src/main/java/org/onosproject/codec/impl/GroupCodec.java
core/common/src/test/java/org/onosproject/codec/impl/GroupCodecTest.java
core/store/dist/src/test/java/org/onosproject/store/group/impl/DistributedGroupStoreTest.java
--- a/core/api/src/main/java/org/onosproject/net/group/DefaultGroupDescription.java
View file @2d45985
+++ b/core/api/src/main/java/org/onosproject/net/group/DefaultGroupDescription.java
View file @2d45985
@@ -17,6 +17,7 @@ package org.onosproject.net.group;
 import static com.google.common.base.MoreObjects.toStringHelper;
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkArgument;
 import java.util.Objects;
@@ -61,6 +62,10 @@ public class DefaultGroupDescription implements GroupDescription {
         this.type = checkNotNull(type);
         this.deviceId = checkNotNull(deviceId);
         this.buckets = checkNotNull(buckets);
+        if (this.type == GroupDescription.Type.INDIRECT) {
+            checkArgument(buckets.buckets().size() == 1, "Indirect group " +
+                    "should have only one action bucket");
+       }
         this.appCookie = appCookie;
         this.givenGroupId = groupId;
         this.appId = appId;
--- a/core/api/src/test/java/org/onosproject/net/group/DefaultGroupTest.java
View file @2d45985
+++ b/core/api/src/test/java/org/onosproject/net/group/DefaultGroupTest.java
View file @2d45985
@@ -34,6 +34,8 @@ import static org.onosproject.net.NetTestTools.did;
 public class DefaultGroupTest {
     private final GroupId id1 = new DefaultGroupId(6);
     private final GroupId id2 = new DefaultGroupId(7);
+    private final GroupId id3 = new DefaultGroupId(1234);
+
     private final GroupBucket bucket =
             DefaultGroupBucket.createSelectGroupBucket(
                     DefaultTrafficTreatment.emptyTreatment());
@@ -48,10 +50,16 @@ public class DefaultGroupTest {
                     GroupDescription.Type.FAILOVER,
                     groupBuckets);
+     private final GroupDescription groupDesc3 =
+            new DefaultGroupDescription(did("3"),
+                    GroupDescription.Type.INDIRECT,
+                    groupBuckets);
+
     DefaultGroup group1 = new DefaultGroup(id1, groupDesc1);
     DefaultGroup sameAsGroup1 = new DefaultGroup(id1, groupDesc1);
     DefaultGroup group2 = new DefaultGroup(id1, groupDesc2);
     DefaultGroup group3 = new DefaultGroup(id2, groupDesc2);
+    DefaultGroup group4 = new DefaultGroup(id3, groupDesc3);
     /**
      * Tests for proper operation of equals(), hashCode() and toString() methods.
@@ -62,6 +70,7 @@ public class DefaultGroupTest {
                 .addEqualityGroup(group1, sameAsGroup1)
                 .addEqualityGroup(group2)
                 .addEqualityGroup(group3)
+                .addEqualityGroup(group4)
                 .testEquals();
     }
@@ -85,7 +94,7 @@ public class DefaultGroupTest {
     @Test
     public void checkConstructionWithDid() {
         DefaultGroup group = new DefaultGroup(id2, NetTestTools.did("1"),
-                GroupDescription.Type.INDIRECT, groupBuckets);
+                GroupDescription.Type.ALL, groupBuckets);
         assertThat(group.id(), is(id2));
         assertThat(group.bytes(), is(0L));
         assertThat(group.life(), is(0L));
--- a/core/common/src/main/java/org/onosproject/codec/impl/GroupCodec.java
View file @2d45985
+++ b/core/common/src/main/java/org/onosproject/codec/impl/GroupCodec.java
View file @2d45985
@@ -156,7 +156,7 @@ public final class GroupCodec extends JsonCodec<Group> {
         }
         // parse group buckets
-        // TODO: make sure that INDIRECT group only has one bucket
+
         GroupBuckets buckets = null;
         List<GroupBucket> groupBucketList = new ArrayList<>();
         JsonNode bucketsJson = json.get(BUCKETS);
--- a/core/common/src/test/java/org/onosproject/codec/impl/GroupCodecTest.java
View file @2d45985
+++ b/core/common/src/test/java/org/onosproject/codec/impl/GroupCodecTest.java
View file @2d45985
@@ -33,7 +33,6 @@ import org.onosproject.net.group.DefaultGroupBucket;
 import org.onosproject.net.group.Group;
 import org.onosproject.net.group.GroupBucket;
 import org.onosproject.net.group.GroupBuckets;
-import org.onosproject.net.group.GroupDescription;
 import java.io.IOException;
 import java.io.InputStream;
@@ -47,6 +46,7 @@ import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.equalTo;
 import static org.onosproject.codec.impl.GroupJsonMatcher.matchesGroup;
 import static org.onosproject.net.NetTestTools.APP_ID;
+import static org.onosproject.net.group.GroupDescription.Type.*;
 /**
  * Group codec unit tests.
@@ -81,19 +81,27 @@ public class GroupCodecTest {
         GroupBucket bucket2 = DefaultGroupBucket
                 .createIndirectGroupBucket(DefaultTrafficTreatment.emptyTreatment());
         GroupBuckets buckets = new GroupBuckets(ImmutableList.of(bucket1, bucket2));
-
+        GroupBuckets bucketsIndirect = new GroupBuckets(ImmutableList.of(bucket2));
         DefaultGroup group = new DefaultGroup(
                 new DefaultGroupId(1),
                 NetTestTools.did("d1"),
-                GroupDescription.Type.INDIRECT,
+                ALL,
                 buckets);
+        DefaultGroup group1 = new DefaultGroup(
+                new DefaultGroupId(2),
+                NetTestTools.did("d2"),
+                INDIRECT,
+                bucketsIndirect);
         MockCodecContext context = new MockCodecContext();
         GroupCodec codec = new GroupCodec();
         ObjectNode groupJson = codec.encode(group, context);
+        ObjectNode groupJsonIndirect = codec.encode(group1, context);
+
         assertThat(groupJson, matchesGroup(group));
+        assertThat(groupJsonIndirect, matchesGroup(group1));
     }
     @Test
@@ -108,6 +116,7 @@ public class GroupCodecTest {
         Instruction instruction1 = groupBucket.treatment().allInstructions().get(0);
         assertThat(instruction1.type(), is(Instruction.Type.OUTPUT));
         assertThat(((Instructions.OutputInstruction) instruction1).port(), is(PortNumber.portNumber(2)));
+
     }
     @Test(expected = IllegalArgumentException.class)
@@ -129,6 +138,7 @@ public class GroupCodecTest {
         assertThat(group.id().id(), is(1));
     }
+
     /**
      * Reads in a group from the given resource and decodes it.
      *
--- a/core/store/dist/src/test/java/org/onosproject/store/group/impl/DistributedGroupStoreTest.java
View file @2d45985
+++ b/core/store/dist/src/test/java/org/onosproject/store/group/impl/DistributedGroupStoreTest.java
View file @2d45985
@@ -62,7 +62,8 @@ import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.nullValue;
 import static org.onosproject.net.NetTestTools.APP_ID;
 import static org.onosproject.net.NetTestTools.did;
-
+import static org.onosproject.net.group.GroupDescription.Type.*;
+import static org.onosproject.net.group.GroupStore.UpdateType.*;
 /**
  * Distributed group store test.
  */
@@ -88,21 +89,21 @@ public class DistributedGroupStoreTest {
     GroupBuckets buckets = new GroupBuckets(ImmutableList.of(selectGroupBucket));
     GroupDescription groupDescription1 = new DefaultGroupDescription(
             deviceId1,
-            GroupDescription.Type.INDIRECT,
+            ALL,
             buckets,
             groupKey1,
             groupId1.id(),
             APP_ID);
     GroupDescription groupDescription2 = new DefaultGroupDescription(
             deviceId2,
-            GroupDescription.Type.INDIRECT,
+            INDIRECT,
             buckets,
             groupKey2,
             groupId2.id(),
             APP_ID);
     GroupDescription groupDescription3 = new DefaultGroupDescription(
             deviceId2,
-            GroupDescription.Type.INDIRECT,
+            INDIRECT,
             buckets,
             groupKey3,
             groupId3.id(),
@@ -264,7 +265,7 @@ public class DistributedGroupStoreTest {
         GroupDescription groupDescription3 = new DefaultGroupDescription(
                 deviceId1,
-                GroupDescription.Type.SELECT,
+                SELECT,
                 buckets,
                 new DefaultGroupKey("aaa".getBytes()),
                 null,
@@ -325,7 +326,7 @@ public class DistributedGroupStoreTest {
         GroupOperation opAdd =
                 GroupOperation.createAddGroupOperation(groupId1,
-                        GroupDescription.Type.INDIRECT,
+                        INDIRECT,
                         buckets);
         groupStore.groupOperationFailed(deviceId1, opAdd);
@@ -339,7 +340,7 @@ public class DistributedGroupStoreTest {
         GroupOperation opModify =
                 GroupOperation.createModifyGroupOperation(groupId2,
-                        GroupDescription.Type.INDIRECT,
+                        INDIRECT,
                         buckets);
         groupStore.groupOperationFailed(deviceId2, opModify);
         List<GroupEvent> eventsAfterModifyFailed = delegate.eventsSeen();
@@ -350,7 +351,7 @@ public class DistributedGroupStoreTest {
         GroupOperation opDelete =
                 GroupOperation.createDeleteGroupOperation(groupId2,
-                        GroupDescription.Type.INDIRECT);
+                        INDIRECT);
         groupStore.groupOperationFailed(deviceId2, opDelete);
         List<GroupEvent> eventsAfterDeleteFailed = delegate.eventsSeen();
         assertThat(eventsAfterDeleteFailed, hasSize(1));
@@ -397,7 +398,7 @@ public class DistributedGroupStoreTest {
         GroupKey newKey = new DefaultGroupKey("123".getBytes());
         groupStore.updateGroupDescription(deviceId1,
                 groupKey1,
-                GroupStore.UpdateType.ADD,
+                ADD,
                 buckets,
                 newKey);
         Group group1 = groupStore.getGroup(deviceId1, groupId1);