ONOS-7898 Action profile group/member refactoring

Also includes:
- New abstract P4Runtime codec implementation. Currently used for action
profile members/groups encoding/deconding, the plan is to handle all
other codecs via this.
- Improved read requests in P4RuntimeClientImpl
- Removed handling of max group size in P4Runtime driver. Instead, added
modified group translator to specify a max group size by using
information from the pipeline model.

Change-Id: I684bae0184d683bb448ba19863c561f9848479d2
diff --git a/core/api/src/main/java/org/onosproject/net/pi/model/PiActionProfileModel.java b/core/api/src/main/java/org/onosproject/net/pi/model/PiActionProfileModel.java
index 66b8c53..0a66ee9 100644
--- a/core/api/src/main/java/org/onosproject/net/pi/model/PiActionProfileModel.java
+++ b/core/api/src/main/java/org/onosproject/net/pi/model/PiActionProfileModel.java
@@ -41,16 +41,28 @@
     Collection<PiTableId> tables();
 
     /**
-     * Returns true if this action profile implements dynamic selection, false otherwise.
+     * Returns true if this action profile implements dynamic selection, false
+     * otherwise.
      *
-     * @return true if action profile implements dynamic selection, false otherwise
+     * @return true if action profile implements dynamic selection, false
+     * otherwise
      */
     boolean hasSelector();
 
     /**
-     * Returns the maximum number of member entries of this action profile.
+     * Returns the maximum number of member entries that this action profile can
+     * hold.
      *
      * @return maximum number of member entries
      */
-    long maxSize();
+    long size();
+
+    /**
+     * Returns the maximum number of members a group of this action profile can
+     * hold. This method is meaningful only if the action profile implements
+     * dynamic selection. 0 signifies that an explicit limit is not set.
+     *
+     * @return maximum number of members a group can hold
+     */
+    int maxGroupSize();
 }
diff --git a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiAction.java b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiAction.java
index 24655b6..78a62fd 100644
--- a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiAction.java
+++ b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiAction.java
@@ -30,21 +30,24 @@
 import static com.google.common.base.Preconditions.checkNotNull;
 
 /**
- * Instance of an action, and its runtime parameters, of a table entry in a protocol-independent pipeline.
+ * Instance of an action, and its runtime parameters, of a table entry in a
+ * protocol-independent pipeline.
  */
 @Beta
 public final class PiAction implements PiTableAction {
 
     private final PiActionId actionId;
-    private final Map<PiActionParamId, PiActionParam> runtimeParams;
+    private final ImmutableMap<PiActionParamId, PiActionParam> runtimeParams;
 
     /**
-     * Creates a new action instance for the given action identifier and runtime parameters.
+     * Creates a new action instance for the given action identifier and runtime
+     * parameters.
      *
      * @param actionId      action identifier
      * @param runtimeParams list of runtime parameters
      */
-    private PiAction(PiActionId actionId, Map<PiActionParamId, PiActionParam> runtimeParams) {
+    private PiAction(PiActionId actionId,
+                     Map<PiActionParamId, PiActionParam> runtimeParams) {
         this.actionId = actionId;
         this.runtimeParams = ImmutableMap.copyOf(runtimeParams);
     }
@@ -64,8 +67,8 @@
     }
 
     /**
-     * Returns all runtime parameters of this action. Return an empty collection if the action doesn't take any runtime
-     * parameters.
+     * Returns all runtime parameters of this action. Return an empty collection
+     * if the action doesn't take any runtime parameters.
      *
      * @return list of byte sequences
      */
diff --git a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileGroup.java b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileGroup.java
index 96168b9..f86e70c 100644
--- a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileGroup.java
+++ b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileGroup.java
@@ -19,13 +19,15 @@
 import com.google.common.annotations.Beta;
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Objects;
-import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
 import org.onosproject.net.pi.model.PiActionProfileId;
 
 import java.util.Collection;
 import java.util.Map;
+import java.util.Optional;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 
 /**
@@ -34,70 +36,96 @@
 @Beta
 public final class PiActionProfileGroup implements PiEntity {
 
-    private final PiActionProfileGroupId id;
-    private final ImmutableSet<PiActionProfileMember> members;
     private final PiActionProfileId actionProfileId;
+    private final PiActionProfileGroupId groupId;
+    private final ImmutableMap<PiActionProfileMemberId, WeightedMember> members;
+    private final int maxSize;
 
-    private PiActionProfileGroup(PiActionProfileGroupId id,
-                                 ImmutableSet<PiActionProfileMember> members,
-                                 PiActionProfileId actionProfileId) {
-        this.id = id;
+    private PiActionProfileGroup(PiActionProfileGroupId groupId,
+                                 ImmutableMap<PiActionProfileMemberId, WeightedMember> members,
+                                 PiActionProfileId actionProfileId,
+                                 int maxSize) {
+        this.groupId = groupId;
         this.members = members;
         this.actionProfileId = actionProfileId;
+        this.maxSize = maxSize;
     }
 
     /**
-     * Returns the identifier of this action profile group.
+     * Returns the ID of this action profile group.
      *
-     * @return action profile group identifier
+     * @return action profile group ID
      */
     public PiActionProfileGroupId id() {
-        return id;
+        return groupId;
     }
 
     /**
-     * Returns the members of this action profile group.
+     * Returns the list of member references of this action profile group.
      *
      * @return collection of action profile members.
      */
-    public Collection<PiActionProfileMember> members() {
-        return members;
+    public Collection<WeightedMember> members() {
+        return members.values();
     }
 
     /**
-     * Gets identifier of the action profile.
+     * Returns the group member identified by the given action profile member
+     * ID, if present.
      *
-     * @return action profile id
+     * @param memberId action profile member ID
+     * @return optional group member
      */
-    public PiActionProfileId actionProfileId() {
+    public Optional<WeightedMember> member(PiActionProfileMemberId memberId) {
+        return Optional.of(members.get(memberId));
+    }
+
+    /**
+     * Returns the maximum number of members that this group can hold. 0
+     * signifies that a limit is not set.
+     *
+     * @return maximum number of members that this group can hold
+     */
+    public int maxSize() {
+        return maxSize;
+    }
+
+    /**
+     * Returns the ID of the action profile where this group belong.
+     *
+     * @return action profile ID
+     */
+    public PiActionProfileId actionProfile() {
         return actionProfileId;
     }
 
     @Override
-    public boolean equals(Object o) {
-        if (this == o) {
+    public boolean equals(Object obj) {
+        if (this == obj) {
             return true;
         }
-        if (o == null || !(o instanceof PiActionProfileGroup)) {
+        if (obj == null || getClass() != obj.getClass()) {
             return false;
         }
-        PiActionProfileGroup that = (PiActionProfileGroup) o;
-        return Objects.equal(id, that.id) &&
-                Objects.equal(members, that.members) &&
-                Objects.equal(actionProfileId, that.actionProfileId);
+        final PiActionProfileGroup other = (PiActionProfileGroup) obj;
+        return Objects.equal(this.groupId, other.groupId)
+                && Objects.equal(this.members, other.members)
+                && Objects.equal(this.maxSize, other.maxSize)
+                && Objects.equal(this.actionProfileId, other.actionProfileId);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hashCode(id, members);
+        return Objects.hashCode(groupId, members, maxSize, actionProfileId);
     }
 
     @Override
     public String toString() {
         return MoreObjects.toStringHelper(this)
-                .add("groupId", id)
+                .add("actionProfile", actionProfileId)
+                .add("id", groupId)
                 .add("members", members)
-                .add("piActionProfileId", actionProfileId)
+                .add("maxSize", maxSize)
                 .toString();
     }
 
@@ -120,55 +148,115 @@
      */
     public static final class Builder {
 
-        private PiActionProfileGroupId id;
-        private Map<PiActionProfileMemberId, PiActionProfileMember> members = Maps.newHashMap();
-        private PiActionProfileId piActionProfileId;
+        private PiActionProfileGroupId groupId;
+        private Map<PiActionProfileMemberId, WeightedMember> members = Maps.newHashMap();
+        private PiActionProfileId actionProfileId;
+        private int maxSize;
 
         private Builder() {
             // hides constructor.
         }
 
         /**
-         * Sets the identifier of this action profile group.
+         * Sets the ID of this action profile group.
          *
-         * @param id action profile group identifier
+         * @param id action profile group ID
          * @return this
          */
         public Builder withId(PiActionProfileGroupId id) {
-            this.id = id;
+            this.groupId = id;
             return this;
         }
 
         /**
-         * Adds one member to this action profile group.
+         * Adds one member to this action profile.
          *
-         * @param member action profile member
+         * @param member member to add
          * @return this
          */
-        public Builder addMember(PiActionProfileMember member) {
+        public Builder addMember(WeightedMember member) {
+            checkNotNull(member);
             members.put(member.id(), member);
             return this;
         }
 
         /**
-         * Adds many members to this action profile group.
+         * Adds one member to this action profile group with default weight.
          *
-         * @param members action profile members
+         * @param memberId ID of the action profile member to add
          * @return this
          */
-        public Builder addMembers(Collection<PiActionProfileMember> members) {
-            members.forEach(this::addMember);
+        public Builder addMember(PiActionProfileMemberId memberId) {
+            addMember(new WeightedMember(memberId, WeightedMember.DEFAULT_WEIGHT));
             return this;
         }
 
         /**
-         * Sets the identifier of the action profile.
+         * Adds one member to this action profile group with default weight.
          *
-         * @param piActionProfileId the identifier of the action profile
+         * @param memberInstance the action profile member instance to add
+         * @return this
+         */
+        public Builder addMember(PiActionProfileMember memberInstance) {
+            addMember(new WeightedMember(memberInstance, WeightedMember.DEFAULT_WEIGHT));
+            return this;
+        }
+
+        /**
+         * Adds all members to this action profile group with default weight.
+         *
+         * @param memberInstances the action profile member instance to add
+         * @return this
+         */
+        public Builder addMembers(Iterable<PiActionProfileMember> memberInstances) {
+            memberInstances.forEach(this::addMember);
+            return this;
+        }
+
+        /**
+         * Adds one member to this action profile group with the given weight.
+         *
+         * @param memberId ID of the action profile member to add
+         * @param weight   weight
+         * @return this
+         */
+        public Builder addMember(PiActionProfileMemberId memberId, int weight) {
+            addMember(new WeightedMember(memberId, weight));
+            return this;
+        }
+
+        /**
+         * Adds one member to this action profile group with the given weight.
+         *
+         * @param memberInstance the action profile member instance to add
+         * @param weight         weight
+         * @return this
+         */
+        public Builder addMember(PiActionProfileMember memberInstance, int weight) {
+            addMember(new WeightedMember(memberInstance, weight));
+            return this;
+        }
+
+        /**
+         * Sets the ID of the action profile.
+         *
+         * @param piActionProfileId the ID of the action profile
          * @return this
          */
         public Builder withActionProfileId(PiActionProfileId piActionProfileId) {
-            this.piActionProfileId = piActionProfileId;
+            this.actionProfileId = piActionProfileId;
+            return this;
+        }
+
+        /**
+         * Sets the maximum number of members that this group can hold.
+         *
+         * @param maxSize maximum number of members that this group can hold
+         * @return this
+         */
+        public Builder withMaxSize(int maxSize) {
+            checkArgument(maxSize >= 0, "maxSize cannot be negative");
+            this.maxSize = maxSize;
             return this;
         }
 
@@ -178,10 +266,119 @@
          * @return action profile group
          */
         public PiActionProfileGroup build() {
-            checkNotNull(id);
-            checkNotNull(piActionProfileId);
+            checkNotNull(groupId);
+            checkNotNull(actionProfileId);
+            checkArgument(maxSize == 0 || members.size() <= maxSize,
+                          "The number of members cannot exceed maxSize");
+            final boolean validActionProfileId = members.isEmpty() || members.values()
+                    .stream().allMatch(m -> m.instance() == null || m.instance()
+                            .actionProfile().equals(actionProfileId));
+            checkArgument(
+                    validActionProfileId,
+                    "The members' action profile ID must match the group one");
             return new PiActionProfileGroup(
-                    id, ImmutableSet.copyOf(members.values()), piActionProfileId);
+                    groupId, ImmutableMap.copyOf(members), actionProfileId, maxSize);
+        }
+    }
+
+    /**
+     * Weighted reference to an action profile member as used in an action
+     * profile group.
+     */
+    public static final class WeightedMember {
+
+        public static final int DEFAULT_WEIGHT = 1;
+
+        private final PiActionProfileMemberId memberId;
+        private final int weight;
+        private final PiActionProfileMember memberInstance;
+
+        /**
+         * Creates a new reference for the given action profile member ID and
+         * weight.
+         *
+         * @param memberId action profile member ID
+         * @param weight   weight
+         */
+        public WeightedMember(PiActionProfileMemberId memberId, int weight) {
+            checkNotNull(memberId);
+            this.memberId = memberId;
+            this.weight = weight;
+            this.memberInstance = null;
+        }
+
+        /**
+         * Creates a new reference from the given action profile member instance
+         * and weight. This constructor should be used when performing one-shot
+         * group programming (see {@link #instance()}).
+         *
+         * @param memberInstance action profile member instance
+         * @param weight         weight
+         */
+        public WeightedMember(PiActionProfileMember memberInstance, int weight) {
+            checkNotNull(memberInstance);
+            this.memberId = memberInstance.id();
+            this.weight = weight;
+            this.memberInstance = memberInstance;
+        }
+
+        /**
+         * Returns the ID of the action profile member.
+         *
+         * @return action profile member ID
+         */
+        public PiActionProfileMemberId id() {
+            return memberId;
+        }
+
+        /**
+         * Returns the weight of this group member.
+         *
+         * @return weight
+         */
+        public int weight() {
+            return weight;
+        }
+
+        /**
+         * If present, returns the instance of the action profile member pointed
+         * by this reference, otherwise returns null. This method is provided as
+         * a convenient way to perform one-shot group programming, and as such
+         * is meaningful only when performing write operations to a device. In
+         * other words, when reading groups from a device only the member
+         * reference should be returned and not the actual instance, hence this
+         * method should return null.
+         *
+         * @return action profile member instance, or null
+         */
+        public PiActionProfileMember instance() {
+            return memberInstance;
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hashCode(memberId, weight);
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (this == obj) {
+                return true;
+            }
+            if (obj == null || getClass() != obj.getClass()) {
+                return false;
+            }
+            final WeightedMember other = (WeightedMember) obj;
+            return Objects.equal(this.memberId, other.memberId)
+                    && Objects.equal(this.weight, other.weight);
+        }
+
+        @Override
+        public String toString() {
+            return MoreObjects.toStringHelper(this)
+                    .add("memberId", memberId)
+                    .add("weight", weight)
+                    .toString();
         }
     }
 }
diff --git a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileGroupHandle.java b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileGroupHandle.java
index 42ed272..519a1c4 100644
--- a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileGroupHandle.java
+++ b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileGroupHandle.java
@@ -34,7 +34,7 @@
 
     private PiActionProfileGroupHandle(DeviceId deviceId, PiActionProfileGroup group) {
         super(deviceId);
-        actionProfileId = group.actionProfileId();
+        actionProfileId = group.actionProfile();
         groupId = group.id();
     }
 
@@ -81,7 +81,7 @@
     public String toString() {
         return MoreObjects.toStringHelper(this)
                 .add("deviceId", deviceId())
-                .add("actionProfileId", actionProfileId)
+                .add("actionProfile", actionProfileId)
                 .add("groupId", groupId)
                 .toString();
     }
diff --git a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileMember.java b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileMember.java
index 92c3562..b5df9d0 100644
--- a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileMember.java
+++ b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileMember.java
@@ -32,19 +32,13 @@
     private final PiActionProfileId actionProfileId;
     private final PiActionProfileMemberId memberId;
     private final PiAction action;
-    // FIXME: in P4Runtime weight is an attribute of the member reference in a
-    // group. Either remove it from this class or define the containing group
-    // ID.
-    private final int weight;
 
     private PiActionProfileMember(PiActionProfileId actionProfileId,
                                   PiActionProfileMemberId memberId,
-                                  PiAction action,
-                                  int weight) {
+                                  PiAction action) {
         this.actionProfileId = actionProfileId;
         this.memberId = memberId;
         this.action = action;
-        this.weight = weight;
     }
 
     /**
@@ -74,15 +68,6 @@
         return action;
     }
 
-    /**
-     * Returns the weight associated to this member.
-     *
-     * @return weight
-     */
-    public int weight() {
-        return weight;
-    }
-
     @Override
     public PiEntityType piEntityType() {
         return PiEntityType.ACTION_PROFILE_MEMBER;
@@ -97,15 +82,14 @@
             return false;
         }
         PiActionProfileMember that = (PiActionProfileMember) o;
-        return weight == that.weight &&
-                Objects.equal(actionProfileId, that.actionProfileId) &&
+        return Objects.equal(actionProfileId, that.actionProfileId) &&
                 Objects.equal(memberId, that.memberId) &&
                 Objects.equal(action, that.action);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hashCode(actionProfileId, memberId, action, weight);
+        return Objects.hashCode(actionProfileId, memberId, action);
     }
 
     @Override
@@ -114,7 +98,6 @@
                 .add("actionProfile", actionProfileId)
                 .add("id", memberId)
                 .add("action", action)
-                .add("weight", weight)
                 .toString();
     }
 
@@ -133,9 +116,8 @@
     public static final class Builder {
 
         private PiActionProfileId actionProfileId;
-        private PiActionProfileMemberId id;
+        private PiActionProfileMemberId memberId;
         private PiAction action;
-        private int weight;
 
         private Builder() {
             // Hides constructor.
@@ -159,7 +141,7 @@
          * @return this
          */
         public Builder withId(PiActionProfileMemberId id) {
-            this.id = id;
+            this.memberId = id;
             return this;
         }
 
@@ -175,28 +157,15 @@
         }
 
         /**
-         * Sets the weight of this member.
-         * <p>
-         * Default value is 0.
-         *
-         * @param weight weight
-         * @return this
-         */
-        public Builder withWeight(int weight) {
-            this.weight = weight;
-            return this;
-        }
-
-        /**
          * Creates a new action profile member.
          *
          * @return action profile member
          */
         public PiActionProfileMember build() {
             checkNotNull(actionProfileId);
-            checkNotNull(id);
+            checkNotNull(memberId);
             checkNotNull(action);
-            return new PiActionProfileMember(actionProfileId, id, action, weight);
+            return new PiActionProfileMember(actionProfileId, memberId, action);
         }
     }
 }
diff --git a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileMemberHandle.java b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileMemberHandle.java
index 8771650..40cb960 100644
--- a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileMemberHandle.java
+++ b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileMemberHandle.java
@@ -24,17 +24,17 @@
 import static com.google.common.base.Preconditions.checkNotNull;
 
 /**
- * Global identifier of a PI action profile group member, uniquely defined by a
+ * Global identifier of a PI action profile member, uniquely defined by a
  * device ID, action profile ID, and member ID.
  */
 public final class PiActionProfileMemberHandle extends PiHandle<PiActionProfileMember> {
 
-    private final PiActionProfileMemberId memberId;
     private final PiActionProfileId actionProfileId;
+    private final PiActionProfileMemberId memberId;
 
     private PiActionProfileMemberHandle(DeviceId deviceId,
-                                      PiActionProfileId actionProfileId,
-                                      PiActionProfileMemberId memberId) {
+                                        PiActionProfileId actionProfileId,
+                                        PiActionProfileMemberId memberId) {
         super(deviceId);
         this.actionProfileId = actionProfileId;
         this.memberId = memberId;
@@ -119,7 +119,7 @@
     public String toString() {
         return MoreObjects.toStringHelper(this)
                 .add("deviceId", deviceId())
-                .add("actionProfileId", actionProfileId)
+                .add("actionProfile", actionProfileId)
                 .add("memberId", memberId)
                 .toString();
     }
diff --git a/core/api/src/test/java/org/onosproject/net/pi/runtime/PiActionProfileGroupTest.java b/core/api/src/test/java/org/onosproject/net/pi/runtime/PiActionProfileGroupTest.java
index 0fecc72..4e6a30d 100644
--- a/core/api/src/test/java/org/onosproject/net/pi/runtime/PiActionProfileGroupTest.java
+++ b/core/api/src/test/java/org/onosproject/net/pi/runtime/PiActionProfileGroupTest.java
@@ -16,58 +16,118 @@
 
 package org.onosproject.net.pi.runtime;
 
-import com.google.common.collect.Lists;
 import com.google.common.testing.EqualsTester;
-import org.apache.commons.collections.CollectionUtils;
 import org.junit.Test;
 import org.onosproject.net.pi.model.PiActionId;
 import org.onosproject.net.pi.model.PiActionParamId;
-
-import java.util.Collection;
+import org.onosproject.net.pi.model.PiActionProfileId;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.nullValue;
 import static org.onlab.junit.ImmutableClassChecker.assertThatClassIsImmutable;
 import static org.onlab.util.ImmutableByteSequence.copyFrom;
-import static org.onosproject.net.pi.runtime.PiConstantsTest.ACTION_PROF_ID;
+import static org.onosproject.net.pi.runtime.PiActionProfileGroup.WeightedMember.DEFAULT_WEIGHT;
 import static org.onosproject.net.pi.runtime.PiConstantsTest.DST_ADDR;
 import static org.onosproject.net.pi.runtime.PiConstantsTest.MOD_NW_DST;
+import static org.onosproject.net.pi.runtime.PiConstantsTest.MOD_VLAN_VID;
+import static org.onosproject.net.pi.runtime.PiConstantsTest.VID;
 
 /**
  * Unit tests for PiActionProfileGroup class.
  */
 public class PiActionProfileGroupTest {
 
-    private final PiActionProfileMemberId piActionProfileMemberId = PiActionProfileMemberId.of(10);
-    private final PiAction piAction = PiAction.builder().withId(PiActionId.of(MOD_NW_DST))
+    private final PiActionProfileId actionProfileId1 = PiActionProfileId.of("foo");
+    private final PiActionProfileId actionProfileId2 = PiActionProfileId.of("bar");
+
+    private final PiActionProfileGroupId groupId1 = PiActionProfileGroupId.of(100);
+    private final PiActionProfileGroupId groupId2 = PiActionProfileGroupId.of(200);
+
+    private final PiActionProfileMemberId actProfMemberId1 = PiActionProfileMemberId.of(10);
+    private final PiActionProfileMemberId actProfMemberId2 = PiActionProfileMemberId.of(20);
+
+    private final PiAction piAction1 = PiAction.builder().withId(PiActionId.of(MOD_NW_DST))
             .withParameter(new PiActionParam(PiActionParamId.of(DST_ADDR), copyFrom(0x0a010101)))
             .build();
-
-    private final PiActionProfileMember piActionProfileMember = PiActionProfileMember.builder()
-            .forActionProfile(ACTION_PROF_ID)
-            .withId(piActionProfileMemberId)
-            .withAction(piAction)
-            .withWeight(10)
-            .build();
-    private PiActionProfileGroupId piActionGroupId = PiActionProfileGroupId.of(10);
-    private PiActionProfileGroup piActionGroup1 = PiActionProfileGroup.builder()
-            .addMember(piActionProfileMember)
-            .withId(piActionGroupId)
-            .withActionProfileId(ACTION_PROF_ID)
+    private final PiAction piAction2 = PiAction.builder().withId(PiActionId.of(MOD_VLAN_VID))
+            .withParameter(new PiActionParam(PiActionParamId.of(VID), copyFrom(0x0b)))
             .build();
 
-    private PiActionProfileGroup sameAsPiActionProfileGroup1 = PiActionProfileGroup.builder()
-            .addMember(piActionProfileMember)
-            .withId(piActionGroupId)
-            .withActionProfileId(ACTION_PROF_ID)
+    private final PiActionProfileMember actProfMember11 = PiActionProfileMember.builder()
+            .forActionProfile(actionProfileId1)
+            .withId(actProfMemberId1)
+            .withAction(piAction1)
+            .build();
+    private final PiActionProfileMember actProfMember12 = PiActionProfileMember.builder()
+            .forActionProfile(actionProfileId1)
+            .withId(actProfMemberId2)
+            .withAction(piAction2)
+            .build();
+    private final PiActionProfileMember actProfMember21 = PiActionProfileMember.builder()
+            .forActionProfile(actionProfileId2)
+            .withId(actProfMemberId1)
+            .withAction(piAction1)
+            .build();
+    private final PiActionProfileMember actProfMember22 = PiActionProfileMember.builder()
+            .forActionProfile(actionProfileId2)
+            .withId(actProfMemberId2)
+            .withAction(piAction2)
             .build();
 
-    private PiActionProfileGroupId piActionGroupId2 = PiActionProfileGroupId.of(20);
-    private PiActionProfileGroup piActionGroup2 = PiActionProfileGroup.builder()
-            .addMember(piActionProfileMember)
-            .withId(piActionGroupId2)
-            .withActionProfileId(ACTION_PROF_ID)
+    private final PiActionProfileGroup.WeightedMember weightedMember1 = new PiActionProfileGroup.WeightedMember(
+            actProfMemberId1, DEFAULT_WEIGHT);
+    private final PiActionProfileGroup.WeightedMember weightedMember2 = new PiActionProfileGroup.WeightedMember(
+            actProfMemberId2, DEFAULT_WEIGHT);
+
+    private PiActionProfileGroup group1 = PiActionProfileGroup.builder()
+            .withActionProfileId(actionProfileId1)
+            // Group members defined with PiActionProfileMember instance.
+            .addMember(actProfMember11)
+            .addMember(actProfMember12)
+            .withId(groupId1)
+            .build();
+
+    private PiActionProfileGroup sameAsGroup1 = PiActionProfileGroup.builder()
+            .withActionProfileId(actionProfileId1)
+            // Group members defined with PiActionProfileMember instance, in
+            // different order.
+            .addMember(actProfMember12)
+            .addMember(actProfMember11)
+            .withId(groupId1)
+            .build();
+
+    private PiActionProfileGroup sameAsGroup1NoInstance = PiActionProfileGroup.builder()
+            .withActionProfileId(actionProfileId1)
+            // Group members defined with WeightedMember instances.
+            .addMember(weightedMember1)
+            .addMember(weightedMember2)
+            .withId(groupId1)
+            .build();
+
+    private PiActionProfileGroup group2 = PiActionProfileGroup.builder()
+            .withActionProfileId(actionProfileId2)
+            // Group members defined with PiActionProfileMember instance.
+            .addMember(actProfMember21)
+            .addMember(actProfMember22)
+            .withId(groupId2)
+            .build();
+
+    private PiActionProfileGroup sameAsGroup2NoInstance = PiActionProfileGroup.builder()
+            .withActionProfileId(actionProfileId2)
+            // Members defined by their ID only.
+            .addMember(actProfMemberId1)
+            .addMember(actProfMemberId2)
+            .withId(groupId2)
+            .build();
+
+    private PiActionProfileGroup asGroup2WithDifferentWeights = PiActionProfileGroup.builder()
+            .withActionProfileId(actionProfileId2)
+            // Members defined by their ID only and different weight.
+            .addMember(actProfMemberId1, 100)
+            .addMember(actProfMemberId2, 100)
+            .withId(groupId2)
             .build();
 
     /**
@@ -86,8 +146,9 @@
     public void testEquals() {
 
         new EqualsTester()
-                .addEqualityGroup(piActionGroup1, sameAsPiActionProfileGroup1)
-                .addEqualityGroup(piActionGroup2)
+                .addEqualityGroup(group1, sameAsGroup1, sameAsGroup1NoInstance)
+                .addEqualityGroup(group2, sameAsGroup2NoInstance)
+                .addEqualityGroup(asGroup2WithDifferentWeights)
                 .testEquals();
     }
 
@@ -96,13 +157,21 @@
      */
     @Test
     public void testMethods() {
-
-        Collection<PiActionProfileMember> piActionProfileMembers = Lists.newArrayList();
-
-        piActionProfileMembers.add(piActionProfileMember);
-        assertThat(piActionGroup1, is(notNullValue()));
-        assertThat(piActionGroup1.id(), is(piActionGroupId));
-        assertThat("Incorrect members value",
-                   CollectionUtils.isEqualCollection(piActionGroup1.members(), piActionProfileMembers));
+        assertThat(group1, is(notNullValue()));
+        assertThat(group1.id(), is(groupId1));
+        assertThat(group1.actionProfile(), is(actionProfileId1));
+        assertThat(group1.members().size(), is(2));
+        // Check members (with instance)
+        assertThat(group1.members().contains(weightedMember1), is(true));
+        assertThat(group1.members().contains(weightedMember2), is(true));
+        assertThat(group1.member(actProfMemberId1).isPresent(), is(notNullValue()));
+        assertThat(group1.member(actProfMemberId2).isPresent(), is(notNullValue()));
+        assertThat(group1.member(actProfMemberId1).get().instance(), is(actProfMember11));
+        assertThat(group1.member(actProfMemberId2).get().instance(), is(actProfMember12));
+        // Check members (no instance)
+        assertThat(sameAsGroup2NoInstance.member(actProfMemberId1).isPresent(), is(true));
+        assertThat(sameAsGroup2NoInstance.member(actProfMemberId2).isPresent(), is(true));
+        assertThat(sameAsGroup2NoInstance.member(actProfMemberId1).get().instance(), is(nullValue()));
+        assertThat(sameAsGroup2NoInstance.member(actProfMemberId2).get().instance(), is(nullValue()));
     }
 }
diff --git a/core/api/src/test/java/org/onosproject/net/pi/runtime/PiActionProfileMemberTest.java b/core/api/src/test/java/org/onosproject/net/pi/runtime/PiActionProfileMemberTest.java
index 317f322..5d8f99e 100644
--- a/core/api/src/test/java/org/onosproject/net/pi/runtime/PiActionProfileMemberTest.java
+++ b/core/api/src/test/java/org/onosproject/net/pi/runtime/PiActionProfileMemberTest.java
@@ -29,6 +29,8 @@
 import static org.onlab.util.ImmutableByteSequence.copyFrom;
 import static org.onosproject.net.pi.runtime.PiConstantsTest.DST_ADDR;
 import static org.onosproject.net.pi.runtime.PiConstantsTest.MOD_NW_DST;
+import static org.onosproject.net.pi.runtime.PiConstantsTest.MOD_VLAN_VID;
+import static org.onosproject.net.pi.runtime.PiConstantsTest.VID;
 
 /**
  * Unit tests for PiActionProfileMember class.
@@ -37,34 +39,34 @@
 
     private final PiActionProfileId actionProfileId1 = PiActionProfileId.of("foo");
     private final PiActionProfileId actionProfileId2 = PiActionProfileId.of("bar");
-    private final PiActionProfileMemberId piActionProfileMemberId = PiActionProfileMemberId.of(10);
-    private final PiAction piAction = PiAction.builder().withId(PiActionId.of(MOD_NW_DST))
+    private final PiActionProfileMemberId piActionProfileMemberId1 = PiActionProfileMemberId.of(10);
+    private final PiActionProfileMemberId piActionProfileMemberId2 = PiActionProfileMemberId.of(20);
+    private final PiAction piAction1 = PiAction.builder().withId(PiActionId.of(MOD_NW_DST))
             .withParameter(new PiActionParam(PiActionParamId.of(DST_ADDR), copyFrom(0x0a010101)))
             .build();
+    private final PiAction piAction2 = PiAction.builder().withId(PiActionId.of(MOD_VLAN_VID))
+            .withParameter(new PiActionParam(PiActionParamId.of(VID), copyFrom(0x0b)))
+            .build();
 
     private final PiActionProfileMember piActionProfileMember1 = PiActionProfileMember.builder()
             .forActionProfile(actionProfileId1)
-            .withId(piActionProfileMemberId)
-            .withAction(piAction)
-            .withWeight(10)
+            .withId(piActionProfileMemberId1)
+            .withAction(piAction1)
             .build();
     private final PiActionProfileMember sameAsPiActionProfileMember1 = PiActionProfileMember.builder()
             .forActionProfile(actionProfileId1)
-            .withId(piActionProfileMemberId)
-            .withAction(piAction)
-            .withWeight(10)
+            .withId(piActionProfileMemberId1)
+            .withAction(piAction1)
             .build();
     private final PiActionProfileMember piActionProfileMember2 = PiActionProfileMember.builder()
             .forActionProfile(actionProfileId1)
-            .withId(piActionProfileMemberId)
-            .withAction(piAction)
-            .withWeight(20)
+            .withId(piActionProfileMemberId2)
+            .withAction(piAction2)
             .build();
-    private final PiActionProfileMember piActionGroupMember1ForOtherProfile = PiActionProfileMember.builder()
+    private final PiActionProfileMember piActionProfileMember3 = PiActionProfileMember.builder()
             .forActionProfile(actionProfileId2)
-            .withId(piActionProfileMemberId)
-            .withAction(piAction)
-            .withWeight(10)
+            .withId(piActionProfileMemberId1)
+            .withAction(piAction1)
             .build();
 
     /**
@@ -85,7 +87,7 @@
         new EqualsTester()
                 .addEqualityGroup(piActionProfileMember1, sameAsPiActionProfileMember1)
                 .addEqualityGroup(piActionProfileMember2)
-                .addEqualityGroup(piActionGroupMember1ForOtherProfile)
+                .addEqualityGroup(piActionProfileMember3)
                 .testEquals();
     }
 
@@ -96,8 +98,7 @@
     public void testMethods() {
 
         assertThat(piActionProfileMember1, is(notNullValue()));
-        assertThat(piActionProfileMember1.weight(), is(10));
-        assertThat(piActionProfileMember1.id(), is(piActionProfileMemberId));
-        assertThat(piActionProfileMember1.action(), is(piAction));
+        assertThat(piActionProfileMember1.id(), is(piActionProfileMemberId1));
+        assertThat(piActionProfileMember1.action(), is(piAction1));
     }
 }