More readable toString for BMv2 extension selectors and treatments

Also, added a test for serialization

Change-Id: I77e80fa7597b552c71e80c9d39d03549e0325778
diff --git a/protocols/bmv2/api/pom.xml b/protocols/bmv2/api/pom.xml
index 16c9260..0beb1f0 100644
--- a/protocols/bmv2/api/pom.xml
+++ b/protocols/bmv2/api/pom.xml
@@ -39,6 +39,11 @@
            <groupId>org.apache.felix</groupId>
            <artifactId>org.apache.felix.scr.annotations</artifactId>
        </dependency>
+       <dependency>
+           <groupId>org.onosproject</groupId>
+           <artifactId>onos-core-serializers</artifactId>
+           <version>${project.version}</version>
+       </dependency>
    </dependencies>
 
 </project>
\ No newline at end of file
diff --git a/protocols/bmv2/api/src/main/java/org/onosproject/bmv2/api/runtime/Bmv2ExtensionSelector.java b/protocols/bmv2/api/src/main/java/org/onosproject/bmv2/api/runtime/Bmv2ExtensionSelector.java
index d9d9d0e..1e357c4 100644
--- a/protocols/bmv2/api/src/main/java/org/onosproject/bmv2/api/runtime/Bmv2ExtensionSelector.java
+++ b/protocols/bmv2/api/src/main/java/org/onosproject/bmv2/api/runtime/Bmv2ExtensionSelector.java
@@ -29,9 +29,10 @@
 import org.onosproject.net.flow.AbstractExtension;
 import org.onosproject.net.flow.criteria.ExtensionSelector;
 import org.onosproject.net.flow.criteria.ExtensionSelectorType;
+import org.onosproject.store.serializers.KryoNamespaces;
 
 import java.nio.ByteBuffer;
-import java.util.HashMap;
+import java.util.Collections;
 import java.util.Map;
 
 import static com.google.common.base.Preconditions.*;
@@ -46,9 +47,8 @@
 @Beta
 public final class Bmv2ExtensionSelector extends AbstractExtension implements ExtensionSelector {
 
-    private final KryoNamespace appKryo = new KryoNamespace.Builder()
-            .register(HashMap.class)
-            .register(Bmv2MatchParam.class)
+    private static final KryoNamespace APP_KRYO = new KryoNamespace.Builder()
+            .register(KryoNamespaces.API)
             .register(Bmv2ExactMatchParam.class)
             .register(Bmv2TernaryMatchParam.class)
             .register(Bmv2LpmMatchParam.class)
@@ -83,12 +83,12 @@
 
     @Override
     public byte[] serialize() {
-        return appKryo.serialize(parameterMap);
+        return APP_KRYO.serialize(parameterMap);
     }
 
     @Override
     public void deserialize(byte[] data) {
-        this.parameterMap = appKryo.deserialize(data);
+        this.parameterMap = APP_KRYO.deserialize(data);
     }
 
     @Override
@@ -110,9 +110,31 @@
 
     @Override
     public String toString() {
-        return MoreObjects.toStringHelper(this)
-                .add("parameterMap", parameterMap)
-                .toString();
+        MoreObjects.ToStringHelper helper = MoreObjects.toStringHelper(this);
+        parameterMap.forEach((name, param) -> {
+            switch (param.type()) {
+                case EXACT:
+                    Bmv2ExactMatchParam e = (Bmv2ExactMatchParam) param;
+                    helper.add(name, e.value());
+                    break;
+                case TERNARY:
+                    Bmv2TernaryMatchParam t = (Bmv2TernaryMatchParam) param;
+                    helper.add(name, t.value() + "&&&" + t.mask());
+                    break;
+                case LPM:
+                    Bmv2LpmMatchParam l = (Bmv2LpmMatchParam) param;
+                    helper.add(name, l.value() + "/" + String.valueOf(l.prefixLength()));
+                    break;
+                case VALID:
+                    Bmv2ValidMatchParam v = (Bmv2ValidMatchParam) param;
+                    helper.add(name, v.flag() ? "VALID" : "NOT_VALID");
+                    break;
+                default:
+                    helper.add(name, param);
+                    break;
+            }
+        });
+        return helper.toString();
     }
 
     /**
@@ -121,7 +143,7 @@
      * @return a BMv2 extension treatment
      */
     public static Bmv2ExtensionSelector empty() {
-        return new Bmv2ExtensionSelector(null);
+        return new Bmv2ExtensionSelector(Collections.emptyMap());
     }
 
     /**
diff --git a/protocols/bmv2/api/src/main/java/org/onosproject/bmv2/api/runtime/Bmv2ExtensionTreatment.java b/protocols/bmv2/api/src/main/java/org/onosproject/bmv2/api/runtime/Bmv2ExtensionTreatment.java
index b2a9ba3..7062f41 100644
--- a/protocols/bmv2/api/src/main/java/org/onosproject/bmv2/api/runtime/Bmv2ExtensionTreatment.java
+++ b/protocols/bmv2/api/src/main/java/org/onosproject/bmv2/api/runtime/Bmv2ExtensionTreatment.java
@@ -29,11 +29,14 @@
 import org.onosproject.net.flow.AbstractExtension;
 import org.onosproject.net.flow.instructions.ExtensionTreatment;
 import org.onosproject.net.flow.instructions.ExtensionTreatmentType;
+import org.onosproject.store.serializers.KryoNamespaces;
 
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.StringJoiner;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
@@ -47,16 +50,25 @@
 @Beta
 public final class Bmv2ExtensionTreatment extends AbstractExtension implements ExtensionTreatment {
 
-    private final KryoNamespace appKryo = new KryoNamespace.Builder().build();
+    private static final KryoNamespace APP_KRYO = new KryoNamespace.Builder()
+            .register(KryoNamespaces.API)
+            .register(Bmv2ExtensionTreatment.class)
+            .register(Bmv2Action.class)
+            .build();
+
+    private List<String> parameterNames;
     private Bmv2Action action;
 
     /**
      * Creates a new extension treatment for the given BMv2 action.
+     * The list of action parameters name is also required for visualization purposes (i.e. nicer toString()).
      *
-     * @param action an action
+     * @param action         an action
+     * @param parameterNames a list of strings
      */
-    private Bmv2ExtensionTreatment(Bmv2Action action) {
+    private Bmv2ExtensionTreatment(Bmv2Action action, List<String> parameterNames) {
         this.action = action;
+        this.parameterNames = parameterNames;
     }
 
     /**
@@ -75,12 +87,14 @@
 
     @Override
     public byte[] serialize() {
-        return appKryo.serialize(action);
+        return APP_KRYO.serialize(this);
     }
 
     @Override
     public void deserialize(byte[] data) {
-        action = appKryo.deserialize(data);
+        Bmv2ExtensionTreatment other = APP_KRYO.deserialize(data);
+        action = other.action;
+        parameterNames = other.parameterNames;
     }
 
     @Override
@@ -102,8 +116,12 @@
 
     @Override
     public String toString() {
+        StringJoiner stringJoiner = new StringJoiner(", ", "(", ")");
+        for (int i = 0; i < parameterNames.size(); i++) {
+            stringJoiner.add(parameterNames.get(i) + "=" + action.parameters().get(i));
+        }
         return MoreObjects.toStringHelper(this)
-                .add("action", action)
+                .addValue(action.name() + stringJoiner.toString())
                 .toString();
     }
 
@@ -113,7 +131,7 @@
      * @return a BMv2 extension treatment
      */
     public static Bmv2ExtensionTreatment empty() {
-        return new Bmv2ExtensionTreatment(null);
+        return new Bmv2ExtensionTreatment(null, Collections.emptyList());
     }
 
     /**
@@ -232,6 +250,7 @@
                           "invalid number of parameters", actionName);
 
             List<ImmutableByteSequence> newParameters = new ArrayList<>(parameters.size());
+            List<String> parameterNames = new ArrayList<>(parameters.size());
 
             for (String parameterName : parameters.keySet()) {
                 Bmv2RuntimeDataModel runtimeData = actionModel.runtimeData(parameterName);
@@ -242,13 +261,14 @@
                     ImmutableByteSequence newSequence = fitByteSequence(parameters.get(parameterName), bitWidth);
                     int idx = actionModel.runtimeDatas().indexOf(runtimeData);
                     newParameters.add(idx, newSequence);
+                    parameterNames.add(idx, parameterName);
                 } catch (Bmv2TranslatorUtils.ByteSequenceFitException e) {
                     throw new IllegalArgumentException(e.getMessage() +
                                                                " [" + actionName + "->" + runtimeData.name() + "]");
                 }
             }
 
-            return new Bmv2ExtensionTreatment(new Bmv2Action(actionName, newParameters));
+            return new Bmv2ExtensionTreatment(new Bmv2Action(actionName, newParameters), parameterNames);
         }
 
 
diff --git a/protocols/bmv2/api/src/test/java/org/onosproject/bmv2/api/runtime/Bmv2ExtensionBuilderTest.java b/protocols/bmv2/api/src/test/java/org/onosproject/bmv2/api/runtime/Bmv2ExtensionsTest.java
similarity index 72%
rename from protocols/bmv2/api/src/test/java/org/onosproject/bmv2/api/runtime/Bmv2ExtensionBuilderTest.java
rename to protocols/bmv2/api/src/test/java/org/onosproject/bmv2/api/runtime/Bmv2ExtensionsTest.java
index 2c9cc95..d3ffbdd 100644
--- a/protocols/bmv2/api/src/test/java/org/onosproject/bmv2/api/runtime/Bmv2ExtensionBuilderTest.java
+++ b/protocols/bmv2/api/src/test/java/org/onosproject/bmv2/api/runtime/Bmv2ExtensionsTest.java
@@ -18,6 +18,7 @@
 
 import com.eclipsesource.json.Json;
 import com.eclipsesource.json.JsonObject;
+import com.google.common.testing.EqualsTester;
 import org.junit.Before;
 import org.junit.Test;
 import org.onlab.packet.MacAddress;
@@ -30,7 +31,7 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
 
-public class Bmv2ExtensionBuilderTest {
+public class Bmv2ExtensionsTest {
 
     private Bmv2Configuration config;
 
@@ -42,7 +43,7 @@
     }
 
     @Test
-    public void testExtensionSelector() throws Exception {
+    public void testExtensionSelectorBuilder() throws Exception {
 
         Bmv2ExtensionSelector extSelectorExact = Bmv2ExtensionSelector.builder()
                 .forConfiguration(config)
@@ -85,7 +86,7 @@
     }
 
     @Test
-    public void testExtensionTreatment() throws Exception {
+    public void testExtensionTreatmentBuilder() throws Exception {
 
         Bmv2ExtensionTreatment treatment = Bmv2ExtensionTreatment.builder()
                 .forConfiguration(config)
@@ -97,4 +98,40 @@
 
         // TODO add more tests, e.g. check for byte sequences content and size.
     }
+
+    @Test
+    public void testExtensionSelectorSerialization() throws Exception {
+
+        Bmv2ExtensionSelector original = Bmv2ExtensionSelector.builder()
+                .forConfiguration(config)
+                .matchExact("standard_metadata", "ingress_port", (short) 255)
+                .matchLpm("ethernet", "etherType", 512, 4)
+                .matchTernary("ethernet", "dstAddr", 1024L, 512L)
+                .matchValid("ethernet", "srcAddr", true)
+                .build();
+
+        Bmv2ExtensionSelector other = Bmv2ExtensionSelector.empty();
+        other.deserialize(original.serialize());
+
+        new EqualsTester()
+                .addEqualityGroup(original, other)
+                .testEquals();
+    }
+
+    @Test
+    public void testExtensionTreatmentSerialization() throws Exception {
+
+        Bmv2ExtensionTreatment original = Bmv2ExtensionTreatment.builder()
+                .forConfiguration(config)
+                .setActionName("set_egress_port")
+                .addParameter("port", 1)
+                .build();
+
+        Bmv2ExtensionTreatment other = Bmv2ExtensionTreatment.empty();
+        other.deserialize(original.serialize());
+
+        new EqualsTester()
+                .addEqualityGroup(original, other)
+                .testEquals();
+    }
 }
\ No newline at end of file