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 8ad11f1..10f9e5e 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
@@ -483,8 +483,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 102b9ca..0d67eff 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
@@ -335,7 +346,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);
}
diff --git a/core/common/src/test/java/org/onosproject/codec/impl/FlowEntryCodecTest.java b/core/common/src/test/java/org/onosproject/codec/impl/FlowEntryCodecTest.java
new file mode 100644
index 0000000..b6204bb
--- /dev/null
+++ b/core/common/src/test/java/org/onosproject/codec/impl/FlowEntryCodecTest.java
@@ -0,0 +1,151 @@
+/*
+ * Copyright 2015-present Open Networking Foundation
+ *
+ * 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.codec.impl;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import org.junit.Before;
+import org.junit.Test;
+import org.onosproject.codec.JsonCodec;
+import org.onosproject.core.CoreService;
+import org.onosproject.net.DeviceId;
+import org.onosproject.net.PortNumber;
+import org.onosproject.net.flow.DefaultFlowEntry;
+import org.onosproject.net.flow.DefaultTrafficSelector;
+import org.onosproject.net.flow.DefaultTrafficTreatment;
+import org.onosproject.net.flow.FlowEntry;
+import org.onosproject.net.flow.FlowRule;
+import org.onosproject.net.flow.TrafficSelector;
+import org.onosproject.net.flow.TrafficTreatment;
+
+import java.io.IOException;
+import java.io.InputStream;
+
+import static org.easymock.EasyMock.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.onosproject.codec.impl.JsonCodecUtils.assertJsonEncodable;
+import static org.onosproject.net.NetTestTools.APP_ID;
+
+/**
+ * Unit test for FlowEntryCodec.
+ */
+public class FlowEntryCodecTest {
+
+ private static final FlowEntry.FlowEntryState STATE;
+ private static final long LIFE;
+ private static final FlowEntry.FlowLiveType LIVE_TYPE;
+ private static final long PACKETS;
+ private static final long BYTES;
+ private static final TrafficSelector SELECTOR;
+ private static final TrafficTreatment TREATMENT;
+ private static final FlowRule FLOW_RULE;
+ private static final FlowEntry FLOW_ENTRY;
+
+ private CoreService mockCoreService = createMock(CoreService.class);
+ private MockCodecContext context = new MockCodecContext();
+ private JsonCodec<FlowEntry> flowEntryCodec = context.codec(FlowEntry.class);
+
+ private static final String JSON_FILE = "simple-flow-entry.json";
+
+ static {
+ // make sure these members have same values with the corresponding JSON fields
+ STATE = FlowEntry.FlowEntryState.valueOf("ADDED");
+ LIFE = 1000;
+ LIVE_TYPE = FlowEntry.FlowLiveType.valueOf("UNKNOWN");
+ PACKETS = 123;
+ BYTES = 456;
+
+ SELECTOR = DefaultTrafficSelector.builder()
+ .matchEthType((short) (Integer.decode("0x800") & 0xFFFF))
+ .build();
+ TREATMENT = DefaultTrafficTreatment.builder()
+ .setOutput(PortNumber.fromString("CONTROLLER"))
+ .build();
+ FLOW_RULE = DefaultFlowEntry.builder()
+ .withCookie(1)
+ // isPermanent & timeout
+ .makeTemporary(1)
+ .forDevice(DeviceId.deviceId("of:0000000000000001"))
+ .forTable(Integer.valueOf("1"))
+ .withPriority(Integer.valueOf("1"))
+ .withSelector(SELECTOR)
+ .withTreatment(TREATMENT)
+ .build();
+ FLOW_ENTRY = new DefaultFlowEntry(FLOW_RULE, STATE, LIFE, LIVE_TYPE, PACKETS, BYTES);
+ }
+
+ @Before
+ public void setUp() {
+ expect(mockCoreService.registerApplication(FlowRuleCodec.REST_APP_ID))
+ .andReturn(APP_ID).anyTimes();
+ expect(mockCoreService.registerApplication(APP_ID.name()))
+ .andReturn(APP_ID).anyTimes();
+ expect(mockCoreService.getAppId(anyShort())).andReturn(APP_ID).anyTimes();
+ replay(mockCoreService);
+
+ context.registerService(CoreService.class, mockCoreService);
+ }
+
+ @Test
+ public void testCodec() {
+ assertNotNull(flowEntryCodec);
+ assertJsonEncodable(context, flowEntryCodec, FLOW_ENTRY);
+ }
+
+ @Test
+ public void testDecode() throws IOException {
+ InputStream jsonStream = FlowEntryCodec.class.getResourceAsStream(JSON_FILE);
+ JsonNode jsonString = context.mapper().readTree(jsonStream);
+
+ FlowEntry expected = flowEntryCodec.decode((ObjectNode) jsonString, context);
+ assertEquals(expected, FLOW_ENTRY);
+ }
+
+ @Test
+ public void testEncode() throws IOException {
+ InputStream jsonStream = FlowEntryCodec.class.getResourceAsStream(JSON_FILE);
+ ObjectNode jsonString = (ObjectNode) context.mapper().readTree(jsonStream);
+
+ ObjectNode expected = flowEntryCodec.encode(FLOW_ENTRY, context);
+ // only set by the internal FlowRule encoder, so should not appear in the JSON string
+ expected.remove("id");
+ expected.remove("appId");
+ expected.remove("tableName");
+
+ // only set by the FlowEntry encoder but not used for the decoder
+ // so should not appear in the JSON, or a decoding error occurs
+ expected.remove(FlowEntryCodec.GROUP_ID);
+ expected.remove(FlowEntryCodec.LAST_SEEN);
+
+ // assert equality of those values separately. see below
+ assertEquals(expected.get(FlowEntryCodec.LIFE).asLong(), jsonString.get(FlowEntryCodec.LIFE).asLong());
+ assertEquals(expected.get(FlowEntryCodec.PACKETS).asLong(), jsonString.get(FlowEntryCodec.PACKETS).asLong());
+ assertEquals(expected.get(FlowEntryCodec.BYTES).asLong(), jsonString.get(FlowEntryCodec.BYTES).asLong());
+
+ // if those numeric values are included in expected as a result of the encoding,
+ // AssertionError occurs even though both expected and jsonString are semantically identical
+ expected.remove(FlowEntryCodec.LIFE);
+ expected.remove(FlowEntryCodec.PACKETS);
+ expected.remove(FlowEntryCodec.BYTES);
+ jsonString.remove(FlowEntryCodec.LIFE);
+ jsonString.remove(FlowEntryCodec.PACKETS);
+ jsonString.remove(FlowEntryCodec.BYTES);
+
+ assertEquals(expected, jsonString);
+ }
+
+}
diff --git a/core/common/src/test/resources/org/onosproject/codec/impl/simple-flow-entry.json b/core/common/src/test/resources/org/onosproject/codec/impl/simple-flow-entry.json
new file mode 100644
index 0000000..b00ba8d
--- /dev/null
+++ b/core/common/src/test/resources/org/onosproject/codec/impl/simple-flow-entry.json
@@ -0,0 +1,29 @@
+{
+ "priority": 1,
+ "isPermanent": false,
+ "timeout": 1,
+ "deviceId": "of:0000000000000001",
+ "tableId": 1,
+ "treatment": {
+ "instructions": [
+ {
+ "type": "OUTPUT",
+ "port": "CONTROLLER"
+ }
+ ],
+ "deferred": []
+ },
+ "selector": {
+ "criteria": [
+ {
+ "type": "ETH_TYPE",
+ "ethType": "0x800"
+ }
+ ]
+ },
+ "state": "ADDED",
+ "life": 1000,
+ "liveType": "UNKNOWN",
+ "packets": 123,
+ "bytes": 456
+}
\ No newline at end of file
diff --git a/core/common/src/test/resources/org/onosproject/codec/impl/simple-group.json b/core/common/src/test/resources/org/onosproject/codec/impl/simple-group.json
index 865ae23..2229372 100644
--- a/core/common/src/test/resources/org/onosproject/codec/impl/simple-group.json
+++ b/core/common/src/test/resources/org/onosproject/codec/impl/simple-group.json
@@ -2,7 +2,7 @@
"type": "ALL",
"deviceId": "of:0000000000000001",
"appCookie": "0x1234abCD",
- "groupId": "1",
+ "id": "1",
"buckets": [
{
"treatment": {
diff --git a/web/api/src/test/java/org/onosproject/rest/resources/GroupsResourceTest.java b/web/api/src/test/java/org/onosproject/rest/resources/GroupsResourceTest.java
index e13784d..3537c4d 100644
--- a/web/api/src/test/java/org/onosproject/rest/resources/GroupsResourceTest.java
+++ b/web/api/src/test/java/org/onosproject/rest/resources/GroupsResourceTest.java
@@ -276,9 +276,9 @@
@Override
public boolean matchesSafely(JsonObject jsonGroup) {
// check id
- final String jsonId = jsonGroup.get("id").asString();
+ final Long jsonId = jsonGroup.get("id").asLong();
final String groupId = group.id().id().toString();
- if (!jsonId.equals(groupId)) {
+ if (!jsonId.equals(Long.valueOf(groupId))) {
reason = "id " + group.id().id().toString();
return false;
}
@@ -362,8 +362,8 @@
final JsonObject jsonGroup = json.get(jsonGroupIndex).asObject();
final String groupId = group.id().id().toString();
- final String jsonGroupId = jsonGroup.get("id").asString();
- if (jsonGroupId.equals(groupId)) {
+ final Long jsonGroupId = jsonGroup.get("id").asLong();
+ if (jsonGroupId.equals(Long.valueOf(groupId))) {
groupFound = true;
// We found the correct group, check attribute values