Found a few codec inconsistencies between JSON encoders and decoders
- Fix EtherType inconsistency in coding VLAN_PUSH and MPLS_POP instructions
- Fix group id inconsistency in coding a Group instruction
- Fix Group identifiers to be treated as unsigned integers in GroupCodec
- Add a JSON decoder for FlowEntry
Change-Id: Id80b9e009550e3b71b9274adb116e5179391fa66
(cherry picked from commit 494f586853c1a87ba62f20b477e211f996baded5)
diff --git a/core/common/src/main/java/org/onosproject/codec/impl/DecodeInstructionCodecHelper.java b/core/common/src/main/java/org/onosproject/codec/impl/DecodeInstructionCodecHelper.java
index 2730c38..2933ca9 100644
--- a/core/common/src/main/java/org/onosproject/codec/impl/DecodeInstructionCodecHelper.java
+++ b/core/common/src/main/java/org/onosproject/codec/impl/DecodeInstructionCodecHelper.java
@@ -476,8 +476,10 @@
return Instructions.transition(nullIsIllegal(json.get(InstructionCodec.TABLE_ID),
InstructionCodec.TABLE_ID + InstructionCodec.MISSING_MEMBER_MESSAGE).asInt());
} else if (type.equals(Instruction.Type.GROUP.name())) {
- GroupId groupId = new GroupId(nullIsIllegal(json.get(InstructionCodec.GROUP_ID),
- InstructionCodec.GROUP_ID + InstructionCodec.MISSING_MEMBER_MESSAGE).asInt());
+ // a group id should be an unsigned integer
+ Long id = nullIsIllegal(json.get(InstructionCodec.GROUP_ID),
+ InstructionCodec.GROUP_ID + InstructionCodec.MISSING_MEMBER_MESSAGE).asLong();
+ GroupId groupId = new GroupId(id.intValue());
return Instructions.createGroup(groupId);
} else if (type.equals(Instruction.Type.METER.name())) {
MeterId meterId = MeterId.meterId(nullIsIllegal(json.get(InstructionCodec.METER_ID),
diff --git a/core/common/src/main/java/org/onosproject/codec/impl/EncodeInstructionCodecHelper.java b/core/common/src/main/java/org/onosproject/codec/impl/EncodeInstructionCodecHelper.java
index 0f3c125..a30068e 100644
--- a/core/common/src/main/java/org/onosproject/codec/impl/EncodeInstructionCodecHelper.java
+++ b/core/common/src/main/java/org/onosproject/codec/impl/EncodeInstructionCodecHelper.java
@@ -18,6 +18,7 @@
import com.fasterxml.jackson.databind.node.ObjectNode;
import org.onlab.osgi.DefaultServiceDirectory;
import org.onlab.osgi.ServiceDirectory;
+import org.onlab.packet.EthType;
import org.onlab.util.HexString;
import org.onosproject.codec.CodecContext;
import org.onosproject.net.Device;
@@ -40,7 +41,6 @@
import org.onosproject.net.pi.runtime.PiActionProfileMemberId;
import org.slf4j.Logger;
-import static org.onlab.util.Tools.toHexWithPrefix;
import static org.slf4j.LoggerFactory.getLogger;
/**
@@ -145,7 +145,8 @@
case VLAN_PUSH:
final L2ModificationInstruction.ModVlanHeaderInstruction pushVlanInstruction =
(L2ModificationInstruction.ModVlanHeaderInstruction) l2Instruction;
- result.put(InstructionCodec.ETHERNET_TYPE, pushVlanInstruction.ethernetType().toString());
+ result.put(InstructionCodec.ETHERNET_TYPE,
+ toHexEthernetType(pushVlanInstruction.ethernetType().toString()));
break;
case VLAN_POP:
break;
@@ -174,7 +175,7 @@
final L2ModificationInstruction.ModMplsHeaderInstruction popHeaderInstruction =
(L2ModificationInstruction.ModMplsHeaderInstruction) l2Instruction;
result.put(InstructionCodec.ETHERNET_TYPE,
- toHexWithPrefix(popHeaderInstruction.ethernetType().toShort()));
+ toHexEthernetType(popHeaderInstruction.ethernetType().toString()));
break;
case DEC_MPLS_TTL:
break;
@@ -185,6 +186,16 @@
}
/**
+ * Encode ethernet types in the conventional way of 4-digit hex string.
+ * @param ethTypeStr
+ * @return EtherType representation
+ */
+ private String toHexEthernetType(String ethTypeStr) {
+ EthType ethType = EthType.EtherType.valueOf(ethTypeStr.toUpperCase()).ethType();
+ return String.format("0x%04x", ethType.toShort());
+ }
+
+ /**
* Encode an L3 modification instruction.
*
* @param result json node that the instruction attributes are added to
@@ -329,7 +340,9 @@
case GROUP:
final Instructions.GroupInstruction groupInstruction =
(Instructions.GroupInstruction) instruction;
- result.put(InstructionCodec.GROUP_ID, groupInstruction.groupId().toString());
+ // a group id should be an unsigned integer
+ result.put(InstructionCodec.GROUP_ID,
+ Integer.toUnsignedLong(groupInstruction.groupId().id()));
break;
case METER:
diff --git a/core/common/src/main/java/org/onosproject/codec/impl/FlowEntryCodec.java b/core/common/src/main/java/org/onosproject/codec/impl/FlowEntryCodec.java
index f0a53e1..0ffeeb2 100644
--- a/core/common/src/main/java/org/onosproject/codec/impl/FlowEntryCodec.java
+++ b/core/common/src/main/java/org/onosproject/codec/impl/FlowEntryCodec.java
@@ -15,12 +15,13 @@
*/
package org.onosproject.codec.impl;
+import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import org.onosproject.codec.CodecContext;
import org.onosproject.codec.JsonCodec;
-import org.onosproject.core.ApplicationId;
-import org.onosproject.core.CoreService;
+import org.onosproject.net.flow.DefaultFlowEntry;
import org.onosproject.net.flow.FlowEntry;
+import org.onosproject.net.flow.FlowRule;
import org.onosproject.net.flow.TrafficSelector;
import org.onosproject.net.flow.TrafficTreatment;
@@ -31,46 +32,74 @@
*/
public final class FlowEntryCodec extends JsonCodec<FlowEntry> {
+ public static final String GROUP_ID = "groupId";
+ public static final String STATE = "state";
+ public static final String LIFE = "life";
+ public static final String LIVE_TYPE = "liveType";
+ public static final String LAST_SEEN = "lastSeen";
+ public static final String PACKETS = "packets";
+ public static final String BYTES = "bytes";
+ public static final String TREATMENT = "treatment";
+ public static final String SELECTOR = "selector";
+
@Override
public ObjectNode encode(FlowEntry flowEntry, CodecContext context) {
checkNotNull(flowEntry, "Flow entry cannot be null");
- CoreService service = context.getService(CoreService.class);
-
- ApplicationId appId = service.getAppId(flowEntry.appId());
-
- String strAppId = (appId == null) ? "<none>" : appId.name();
-
- final ObjectNode result = context.mapper().createObjectNode()
- .put("id", Long.toString(flowEntry.id().value()))
- .put("tableId", flowEntry.table().toString())
- .put("appId", strAppId)
- .put("groupId", flowEntry.groupId().id())
- .put("priority", flowEntry.priority())
- .put("timeout", flowEntry.timeout())
- .put("isPermanent", flowEntry.isPermanent())
- .put("deviceId", flowEntry.deviceId().toString())
- .put("state", flowEntry.state().toString())
- .put("life", flowEntry.life()) //FIXME life is destroying precision (seconds granularity is default)
- .put("packets", flowEntry.packets())
- .put("bytes", flowEntry.bytes())
- .put("liveType", flowEntry.liveType().toString())
- .put("lastSeen", flowEntry.lastSeen());
+ ObjectNode result = context.mapper().createObjectNode()
+ .put(GROUP_ID, flowEntry.groupId().id())
+ .put(STATE, flowEntry.state().toString())
+ //FIXME life is destroying precision (seconds granularity is default)
+ .put(LIFE, flowEntry.life())
+ .put(LIVE_TYPE, flowEntry.liveType().toString())
+ .put(LAST_SEEN, flowEntry.lastSeen())
+ .put(PACKETS, flowEntry.packets())
+ .put(BYTES, flowEntry.bytes())
+ // encode FlowRule-specific fields using the FlowRule codec
+ .setAll(context.codec(FlowRule.class).encode((FlowRule) flowEntry, context));
if (flowEntry.treatment() != null) {
final JsonCodec<TrafficTreatment> treatmentCodec =
context.codec(TrafficTreatment.class);
- result.set("treatment", treatmentCodec.encode(flowEntry.treatment(), context));
+ result.set(TREATMENT, treatmentCodec.encode(flowEntry.treatment(), context));
}
if (flowEntry.selector() != null) {
final JsonCodec<TrafficSelector> selectorCodec =
context.codec(TrafficSelector.class);
- result.set("selector", selectorCodec.encode(flowEntry.selector(), context));
+ result.set(SELECTOR, selectorCodec.encode(flowEntry.selector(), context));
}
return result;
}
+ @Override
+ public FlowEntry decode(ObjectNode json, CodecContext context) {
+ checkNotNull(json, "JSON object cannot be null");
+
+ // decode FlowRule-specific fields using the FlowRule codec
+ FlowRule flowRule = context.codec(FlowRule.class).decode(json, context);
+
+ JsonNode stateNode = json.get(STATE);
+ FlowEntry.FlowEntryState state = (null == stateNode) ?
+ FlowEntry.FlowEntryState.ADDED : FlowEntry.FlowEntryState.valueOf(stateNode.asText());
+
+ JsonNode lifeNode = json.get(LIFE);
+ long life = (null == lifeNode) ? 0 : lifeNode.asLong();
+
+ JsonNode liveTypeNode = json.get(LIVE_TYPE);
+ FlowEntry.FlowLiveType liveType = (null == liveTypeNode) ?
+ FlowEntry.FlowLiveType.UNKNOWN : FlowEntry.FlowLiveType.valueOf(liveTypeNode.asText());
+
+ JsonNode packetsNode = json.get(PACKETS);
+ long packets = (null == packetsNode) ? 0 : packetsNode.asLong();
+
+ JsonNode bytesNode = json.get(BYTES);
+ long bytes = (null == bytesNode) ? 0 : bytesNode.asLong();
+
+ return new DefaultFlowEntry(flowRule, state, life,
+ liveType, packets, bytes);
+ }
+
}
diff --git a/core/common/src/main/java/org/onosproject/codec/impl/GroupCodec.java b/core/common/src/main/java/org/onosproject/codec/impl/GroupCodec.java
index 37c0319..8955226 100644
--- a/core/common/src/main/java/org/onosproject/codec/impl/GroupCodec.java
+++ b/core/common/src/main/java/org/onosproject/codec/impl/GroupCodec.java
@@ -71,7 +71,8 @@
public ObjectNode encode(Group group, CodecContext context) {
checkNotNull(group, "Group cannot be null");
ObjectNode result = context.mapper().createObjectNode()
- .put(ID, group.id().id().toString())
+ // a Group id should be an unsigned integer
+ .put(ID, Integer.toUnsignedLong(group.id().id()))
.put(STATE, group.state().toString())
.put(LIFE, group.life())
.put(PACKETS, group.packets())
@@ -89,7 +90,8 @@
}
if (group.givenGroupId() != null) {
- result.put(GIVEN_GROUP_ID, group.givenGroupId().toString());
+ // a given Group id should be an unsigned integer
+ result.put(GIVEN_GROUP_ID, Integer.toUnsignedLong(group.givenGroupId()));
}
ArrayNode buckets = context.mapper().createArrayNode();
@@ -110,10 +112,24 @@
final JsonCodec<GroupBucket> groupBucketCodec = context.codec(GroupBucket.class);
CoreService coreService = context.getService(CoreService.class);
- // parse group id
- int groupIdInt = nullIsIllegal(json.get(GROUP_ID),
- GROUP_ID + MISSING_MEMBER_MESSAGE).asInt();
- GroupId groupId = new GroupId(groupIdInt);
+ // parse uInt Group id from the ID field
+ JsonNode idNode = json.get(ID);
+ Long id = (null == idNode) ?
+ // use GROUP_ID for the corresponding Group id if ID is not supplied
+ nullIsIllegal(json.get(GROUP_ID), ID + MISSING_MEMBER_MESSAGE).asLong() :
+ idNode.asLong();
+ GroupId groupId = GroupId.valueOf(id.intValue());
+
+ // parse uInt Group id given by caller. see the GroupDescription javadoc
+ JsonNode givenGroupIdNode = json.get(GIVEN_GROUP_ID);
+ // if GIVEN_GROUP_ID is not supplied, set null so the group subsystem
+ // to choose the Group id (which is the value of ID indeed).
+ // else, must be same with ID to show both originate from the same source
+ Integer givenGroupIdInt = (null == givenGroupIdNode) ?
+ null : new Long(json.get(GIVEN_GROUP_ID).asLong()).intValue();
+ if (givenGroupIdInt != null && !givenGroupIdInt.equals(groupId.id())) {
+ throw new IllegalArgumentException(GIVEN_GROUP_ID + " must be same with " + ID);
+ }
// parse group key (appCookie)
String groupKeyStr = nullIsIllegal(json.get(APP_COOKIE),
@@ -157,7 +173,6 @@
}
// parse group buckets
-
GroupBuckets buckets = null;
List<GroupBucket> groupBucketList = new ArrayList<>();
JsonNode bucketsJson = json.get(BUCKETS);
@@ -173,7 +188,7 @@
}
GroupDescription groupDescription = new DefaultGroupDescription(deviceId,
- groupType, buckets, groupKey, groupIdInt, appId);
+ groupType, buckets, groupKey, givenGroupIdInt, appId);
return new DefaultGroup(groupId, groupDescription);
}