Fix ONOS-2090 - Improvements to Intent JSON
- Intents are now identified by the name portion of the appId rather than
the number
- removed the now useless "details" field which had a toString()
dump of the intent for when we didn't support all intent types
- Single Intent GET operations now accept a decimal or hexadecimal
value for the Intent key.
Change-Id: I39d635e68cccf2e59d0d11307b93329a2dc0bc96
diff --git a/core/common/src/main/java/org/onosproject/codec/impl/IntentCodec.java b/core/common/src/main/java/org/onosproject/codec/impl/IntentCodec.java
index 36ff8fa..8613a96 100644
--- a/core/common/src/main/java/org/onosproject/codec/impl/IntentCodec.java
+++ b/core/common/src/main/java/org/onosproject/codec/impl/IntentCodec.java
@@ -28,6 +28,7 @@
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
+import com.google.common.net.UrlEscapers;
import static com.google.common.base.Preconditions.checkNotNull;
import static org.onlab.util.Tools.nullIsIllegal;
@@ -40,7 +41,6 @@
protected static final String TYPE = "type";
protected static final String ID = "id";
protected static final String APP_ID = "appId";
- protected static final String DETAILS = "details";
protected static final String STATE = "state";
protected static final String PRIORITY = "priority";
protected static final String RESOURCES = "resources";
@@ -50,11 +50,12 @@
@Override
public ObjectNode encode(Intent intent, CodecContext context) {
checkNotNull(intent, "Intent cannot be null");
+
final ObjectNode result = context.mapper().createObjectNode()
.put(TYPE, intent.getClass().getSimpleName())
.put(ID, intent.id().toString())
- .put(APP_ID, intent.appId().toString())
- .put(DETAILS, intent.toString());
+ .put(APP_ID, UrlEscapers.urlPathSegmentEscaper()
+ .escape(intent.appId().name()));
final ArrayNode jsonResources = result.putArray(RESOURCES);
@@ -98,8 +99,8 @@
*/
public static void intentAttributes(ObjectNode json, CodecContext context,
Intent.Builder builder) {
- short appId = (short) nullIsIllegal(json.get(IntentCodec.APP_ID),
- IntentCodec.TYPE + IntentCodec.MISSING_MEMBER_MESSAGE).asInt();
+ String appId = nullIsIllegal(json.get(IntentCodec.APP_ID),
+ IntentCodec.APP_ID + IntentCodec.MISSING_MEMBER_MESSAGE).asText();
CoreService service = context.getService(CoreService.class);
builder.appId(service.getAppId(appId));
diff --git a/core/common/src/test/java/org/onosproject/codec/impl/IntentCodecTest.java b/core/common/src/test/java/org/onosproject/codec/impl/IntentCodecTest.java
index 0824bd5..7cbce4d 100644
--- a/core/common/src/test/java/org/onosproject/codec/impl/IntentCodecTest.java
+++ b/core/common/src/test/java/org/onosproject/codec/impl/IntentCodecTest.java
@@ -103,8 +103,8 @@
final IntentService mockIntentService = new IntentServiceAdapter();
context.registerService(IntentService.class, mockIntentService);
context.registerService(CoreService.class, mockCoreService);
- expect(mockCoreService.getAppId((short) 2))
- .andReturn(new DefaultApplicationId(2, "app"));
+ expect(mockCoreService.getAppId(appId.name()))
+ .andReturn(appId);
replay(mockCoreService);
}
diff --git a/core/common/src/test/java/org/onosproject/codec/impl/IntentJsonMatcher.java b/core/common/src/test/java/org/onosproject/codec/impl/IntentJsonMatcher.java
index d30999d..e485a5f 100644
--- a/core/common/src/test/java/org/onosproject/codec/impl/IntentJsonMatcher.java
+++ b/core/common/src/test/java/org/onosproject/codec/impl/IntentJsonMatcher.java
@@ -443,8 +443,10 @@
}
// check application id
- final String jsonAppId = jsonIntent.get("appId").asText();
- final String appId = intent.appId().toString();
+ final JsonNode jsonAppIdNode = jsonIntent.get("appId");
+
+ final String jsonAppId = jsonAppIdNode.asText();
+ final String appId = intent.appId().name();
if (!jsonAppId.equals(appId)) {
description.appendText("appId was " + jsonAppId);
return false;
@@ -458,14 +460,6 @@
return false;
}
- // check details field
- final String jsonDetails = jsonIntent.get("details").asText();
- final String details = intent.toString();
- if (!jsonDetails.equals(details)) {
- description.appendText("details were " + jsonDetails);
- return false;
- }
-
// check resources array
final JsonNode jsonResources = jsonIntent.get("resources");
if (intent.resources() != null) {
diff --git a/core/common/src/test/resources/org/onosproject/codec/impl/HostToHostIntent.json b/core/common/src/test/resources/org/onosproject/codec/impl/HostToHostIntent.json
index 5010c48..fedea25 100644
--- a/core/common/src/test/resources/org/onosproject/codec/impl/HostToHostIntent.json
+++ b/core/common/src/test/resources/org/onosproject/codec/impl/HostToHostIntent.json
@@ -1,6 +1,6 @@
{
"type": "HostToHostIntent",
- "appId": 2,
+ "appId": "test",
"selector": {"criteria": []},
"treatment": {
"instructions": [],
diff --git a/core/common/src/test/resources/org/onosproject/codec/impl/PointToPointIntent.json b/core/common/src/test/resources/org/onosproject/codec/impl/PointToPointIntent.json
index 4c6c4b8..b941bef 100644
--- a/core/common/src/test/resources/org/onosproject/codec/impl/PointToPointIntent.json
+++ b/core/common/src/test/resources/org/onosproject/codec/impl/PointToPointIntent.json
@@ -1,6 +1,6 @@
{
"type": "PointToPointIntent",
- "appId": 2,
+ "appId": "test",
"selector": {
"criteria": [
{
diff --git a/web/api/src/main/java/org/onosproject/rest/resources/IntentsWebResource.java b/web/api/src/main/java/org/onosproject/rest/resources/IntentsWebResource.java
index 9c3e76a..8ef6fbd 100644
--- a/web/api/src/main/java/org/onosproject/rest/resources/IntentsWebResource.java
+++ b/web/api/src/main/java/org/onosproject/rest/resources/IntentsWebResource.java
@@ -91,14 +91,14 @@
@GET
@Produces(MediaType.APPLICATION_JSON)
@Path("{appId}/{key}")
- public Response getIntentById(@PathParam("appId") Short appId,
+ public Response getIntentById(@PathParam("appId") String appId,
@PathParam("key") String key) {
final ApplicationId app = get(CoreService.class).getAppId(appId);
Intent intent = get(IntentService.class).getIntent(Key.of(key, app));
if (intent == null) {
- intent = get(IntentService.class)
- .getIntent(Key.of(Long.parseLong(key), app));
+ long numericalKey = Long.decode(key);
+ intent = get(IntentService.class).getIntent(Key.of(numericalKey, app));
}
nullIsNotFound(intent, INTENT_NOT_FOUND);
@@ -140,7 +140,7 @@
*/
@DELETE
@Path("{appId}/{key}")
- public void deleteIntentById(@PathParam("appId") Short appId,
+ public void deleteIntentById(@PathParam("appId") String appId,
@PathParam("key") String keyString) {
final ApplicationId app = get(CoreService.class).getAppId(appId);
diff --git a/web/api/src/test/java/org/onosproject/rest/IntentsResourceTest.java b/web/api/src/test/java/org/onosproject/rest/IntentsResourceTest.java
index 6572b68..ee39094 100644
--- a/web/api/src/test/java/org/onosproject/rest/IntentsResourceTest.java
+++ b/web/api/src/test/java/org/onosproject/rest/IntentsResourceTest.java
@@ -110,9 +110,11 @@
}
// check application id
+
final String jsonAppId = jsonIntent.get("appId").asString();
- if (!jsonAppId.equals(intent.appId().toString())) {
- reason = "appId " + intent.appId().toString();
+ final String appId = intent.appId().name();
+ if (!jsonAppId.equals(appId)) {
+ reason = "appId was " + jsonAppId;
return false;
}
@@ -123,13 +125,6 @@
return false;
}
- // check details field
- final String jsonDetails = jsonIntent.get("details").asString();
- if (!jsonDetails.equals(intent.toString())) {
- reason = "details " + intent.toString();
- return false;
- }
-
// check state field
final String jsonState = jsonIntent.get("state").asString();
if (!jsonState.equals("INSTALLED")) {
@@ -196,7 +191,7 @@
@Override
public boolean matchesSafely(JsonArray json) {
boolean intentFound = false;
- final int expectedAttributes = 6;
+ final int expectedAttributes = 5;
for (int jsonIntentIndex = 0; jsonIntentIndex < json.size();
jsonIntentIndex++) {
@@ -336,11 +331,12 @@
.andReturn(intent)
.anyTimes();
replay(mockIntentService);
- expect(mockCoreService.getAppId(APP_ID.id()))
+ expect(mockCoreService.getAppId(APP_ID.name()))
.andReturn(APP_ID).anyTimes();
replay(mockCoreService);
final WebResource rs = resource();
- final String response = rs.path("intents/1/0").get(String.class);
+ final String response = rs.path("intents/" + APP_ID.name()
+ + "/0").get(String.class);
final JsonObject result = JsonObject.readFrom(response);
assertThat(result, matchesIntent(intent));
}
@@ -371,8 +367,9 @@
*/
@Test
public void testPost() {
- expect(mockCoreService.getAppId((short) 2))
- .andReturn(new DefaultApplicationId(2, "app"));
+ ApplicationId testId = new DefaultApplicationId(2, "myApp");
+ expect(mockCoreService.getAppId("myApp"))
+ .andReturn(testId);
replay(mockCoreService);
mockIntentService.submit(anyObject());
diff --git a/web/api/src/test/resources/org/onosproject/rest/post-intent.json b/web/api/src/test/resources/org/onosproject/rest/post-intent.json
index 4c6c4b8..b01ef87 100644
--- a/web/api/src/test/resources/org/onosproject/rest/post-intent.json
+++ b/web/api/src/test/resources/org/onosproject/rest/post-intent.json
@@ -1,6 +1,6 @@
{
"type": "PointToPointIntent",
- "appId": 2,
+ "appId": "myApp",
"selector": {
"criteria": [
{