Committed by
Gerrit Code Review
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
Showing
8 changed files
with
30 additions
and
38 deletions
| ... | @@ -28,6 +28,7 @@ import org.onosproject.net.intent.PointToPointIntent; | ... | @@ -28,6 +28,7 @@ import org.onosproject.net.intent.PointToPointIntent; |
| 28 | import com.fasterxml.jackson.databind.JsonNode; | 28 | import com.fasterxml.jackson.databind.JsonNode; |
| 29 | import com.fasterxml.jackson.databind.node.ArrayNode; | 29 | import com.fasterxml.jackson.databind.node.ArrayNode; |
| 30 | import com.fasterxml.jackson.databind.node.ObjectNode; | 30 | import com.fasterxml.jackson.databind.node.ObjectNode; |
| 31 | +import com.google.common.net.UrlEscapers; | ||
| 31 | 32 | ||
| 32 | import static com.google.common.base.Preconditions.checkNotNull; | 33 | import static com.google.common.base.Preconditions.checkNotNull; |
| 33 | import static org.onlab.util.Tools.nullIsIllegal; | 34 | import static org.onlab.util.Tools.nullIsIllegal; |
| ... | @@ -40,7 +41,6 @@ public final class IntentCodec extends JsonCodec<Intent> { | ... | @@ -40,7 +41,6 @@ public final class IntentCodec extends JsonCodec<Intent> { |
| 40 | protected static final String TYPE = "type"; | 41 | protected static final String TYPE = "type"; |
| 41 | protected static final String ID = "id"; | 42 | protected static final String ID = "id"; |
| 42 | protected static final String APP_ID = "appId"; | 43 | protected static final String APP_ID = "appId"; |
| 43 | - protected static final String DETAILS = "details"; | ||
| 44 | protected static final String STATE = "state"; | 44 | protected static final String STATE = "state"; |
| 45 | protected static final String PRIORITY = "priority"; | 45 | protected static final String PRIORITY = "priority"; |
| 46 | protected static final String RESOURCES = "resources"; | 46 | protected static final String RESOURCES = "resources"; |
| ... | @@ -50,11 +50,12 @@ public final class IntentCodec extends JsonCodec<Intent> { | ... | @@ -50,11 +50,12 @@ public final class IntentCodec extends JsonCodec<Intent> { |
| 50 | @Override | 50 | @Override |
| 51 | public ObjectNode encode(Intent intent, CodecContext context) { | 51 | public ObjectNode encode(Intent intent, CodecContext context) { |
| 52 | checkNotNull(intent, "Intent cannot be null"); | 52 | checkNotNull(intent, "Intent cannot be null"); |
| 53 | + | ||
| 53 | final ObjectNode result = context.mapper().createObjectNode() | 54 | final ObjectNode result = context.mapper().createObjectNode() |
| 54 | .put(TYPE, intent.getClass().getSimpleName()) | 55 | .put(TYPE, intent.getClass().getSimpleName()) |
| 55 | .put(ID, intent.id().toString()) | 56 | .put(ID, intent.id().toString()) |
| 56 | - .put(APP_ID, intent.appId().toString()) | 57 | + .put(APP_ID, UrlEscapers.urlPathSegmentEscaper() |
| 57 | - .put(DETAILS, intent.toString()); | 58 | + .escape(intent.appId().name())); |
| 58 | 59 | ||
| 59 | final ArrayNode jsonResources = result.putArray(RESOURCES); | 60 | final ArrayNode jsonResources = result.putArray(RESOURCES); |
| 60 | 61 | ||
| ... | @@ -98,8 +99,8 @@ public final class IntentCodec extends JsonCodec<Intent> { | ... | @@ -98,8 +99,8 @@ public final class IntentCodec extends JsonCodec<Intent> { |
| 98 | */ | 99 | */ |
| 99 | public static void intentAttributes(ObjectNode json, CodecContext context, | 100 | public static void intentAttributes(ObjectNode json, CodecContext context, |
| 100 | Intent.Builder builder) { | 101 | Intent.Builder builder) { |
| 101 | - short appId = (short) nullIsIllegal(json.get(IntentCodec.APP_ID), | 102 | + String appId = nullIsIllegal(json.get(IntentCodec.APP_ID), |
| 102 | - IntentCodec.TYPE + IntentCodec.MISSING_MEMBER_MESSAGE).asInt(); | 103 | + IntentCodec.APP_ID + IntentCodec.MISSING_MEMBER_MESSAGE).asText(); |
| 103 | CoreService service = context.getService(CoreService.class); | 104 | CoreService service = context.getService(CoreService.class); |
| 104 | builder.appId(service.getAppId(appId)); | 105 | builder.appId(service.getAppId(appId)); |
| 105 | 106 | ... | ... |
| ... | @@ -103,8 +103,8 @@ public class IntentCodecTest extends AbstractIntentTest { | ... | @@ -103,8 +103,8 @@ public class IntentCodecTest extends AbstractIntentTest { |
| 103 | final IntentService mockIntentService = new IntentServiceAdapter(); | 103 | final IntentService mockIntentService = new IntentServiceAdapter(); |
| 104 | context.registerService(IntentService.class, mockIntentService); | 104 | context.registerService(IntentService.class, mockIntentService); |
| 105 | context.registerService(CoreService.class, mockCoreService); | 105 | context.registerService(CoreService.class, mockCoreService); |
| 106 | - expect(mockCoreService.getAppId((short) 2)) | 106 | + expect(mockCoreService.getAppId(appId.name())) |
| 107 | - .andReturn(new DefaultApplicationId(2, "app")); | 107 | + .andReturn(appId); |
| 108 | replay(mockCoreService); | 108 | replay(mockCoreService); |
| 109 | } | 109 | } |
| 110 | 110 | ... | ... |
| ... | @@ -443,8 +443,10 @@ public final class IntentJsonMatcher extends TypeSafeDiagnosingMatcher<JsonNode> | ... | @@ -443,8 +443,10 @@ public final class IntentJsonMatcher extends TypeSafeDiagnosingMatcher<JsonNode> |
| 443 | } | 443 | } |
| 444 | 444 | ||
| 445 | // check application id | 445 | // check application id |
| 446 | - final String jsonAppId = jsonIntent.get("appId").asText(); | 446 | + final JsonNode jsonAppIdNode = jsonIntent.get("appId"); |
| 447 | - final String appId = intent.appId().toString(); | 447 | + |
| 448 | + final String jsonAppId = jsonAppIdNode.asText(); | ||
| 449 | + final String appId = intent.appId().name(); | ||
| 448 | if (!jsonAppId.equals(appId)) { | 450 | if (!jsonAppId.equals(appId)) { |
| 449 | description.appendText("appId was " + jsonAppId); | 451 | description.appendText("appId was " + jsonAppId); |
| 450 | return false; | 452 | return false; |
| ... | @@ -458,14 +460,6 @@ public final class IntentJsonMatcher extends TypeSafeDiagnosingMatcher<JsonNode> | ... | @@ -458,14 +460,6 @@ public final class IntentJsonMatcher extends TypeSafeDiagnosingMatcher<JsonNode> |
| 458 | return false; | 460 | return false; |
| 459 | } | 461 | } |
| 460 | 462 | ||
| 461 | - // check details field | ||
| 462 | - final String jsonDetails = jsonIntent.get("details").asText(); | ||
| 463 | - final String details = intent.toString(); | ||
| 464 | - if (!jsonDetails.equals(details)) { | ||
| 465 | - description.appendText("details were " + jsonDetails); | ||
| 466 | - return false; | ||
| 467 | - } | ||
| 468 | - | ||
| 469 | // check resources array | 463 | // check resources array |
| 470 | final JsonNode jsonResources = jsonIntent.get("resources"); | 464 | final JsonNode jsonResources = jsonIntent.get("resources"); |
| 471 | if (intent.resources() != null) { | 465 | if (intent.resources() != null) { | ... | ... |
| ... | @@ -91,14 +91,14 @@ public class IntentsWebResource extends AbstractWebResource { | ... | @@ -91,14 +91,14 @@ public class IntentsWebResource extends AbstractWebResource { |
| 91 | @GET | 91 | @GET |
| 92 | @Produces(MediaType.APPLICATION_JSON) | 92 | @Produces(MediaType.APPLICATION_JSON) |
| 93 | @Path("{appId}/{key}") | 93 | @Path("{appId}/{key}") |
| 94 | - public Response getIntentById(@PathParam("appId") Short appId, | 94 | + public Response getIntentById(@PathParam("appId") String appId, |
| 95 | @PathParam("key") String key) { | 95 | @PathParam("key") String key) { |
| 96 | final ApplicationId app = get(CoreService.class).getAppId(appId); | 96 | final ApplicationId app = get(CoreService.class).getAppId(appId); |
| 97 | 97 | ||
| 98 | Intent intent = get(IntentService.class).getIntent(Key.of(key, app)); | 98 | Intent intent = get(IntentService.class).getIntent(Key.of(key, app)); |
| 99 | if (intent == null) { | 99 | if (intent == null) { |
| 100 | - intent = get(IntentService.class) | 100 | + long numericalKey = Long.decode(key); |
| 101 | - .getIntent(Key.of(Long.parseLong(key), app)); | 101 | + intent = get(IntentService.class).getIntent(Key.of(numericalKey, app)); |
| 102 | } | 102 | } |
| 103 | nullIsNotFound(intent, INTENT_NOT_FOUND); | 103 | nullIsNotFound(intent, INTENT_NOT_FOUND); |
| 104 | 104 | ||
| ... | @@ -140,7 +140,7 @@ public class IntentsWebResource extends AbstractWebResource { | ... | @@ -140,7 +140,7 @@ public class IntentsWebResource extends AbstractWebResource { |
| 140 | */ | 140 | */ |
| 141 | @DELETE | 141 | @DELETE |
| 142 | @Path("{appId}/{key}") | 142 | @Path("{appId}/{key}") |
| 143 | - public void deleteIntentById(@PathParam("appId") Short appId, | 143 | + public void deleteIntentById(@PathParam("appId") String appId, |
| 144 | @PathParam("key") String keyString) { | 144 | @PathParam("key") String keyString) { |
| 145 | final ApplicationId app = get(CoreService.class).getAppId(appId); | 145 | final ApplicationId app = get(CoreService.class).getAppId(appId); |
| 146 | 146 | ... | ... |
| ... | @@ -110,9 +110,11 @@ public class IntentsResourceTest extends ResourceTest { | ... | @@ -110,9 +110,11 @@ public class IntentsResourceTest extends ResourceTest { |
| 110 | } | 110 | } |
| 111 | 111 | ||
| 112 | // check application id | 112 | // check application id |
| 113 | + | ||
| 113 | final String jsonAppId = jsonIntent.get("appId").asString(); | 114 | final String jsonAppId = jsonIntent.get("appId").asString(); |
| 114 | - if (!jsonAppId.equals(intent.appId().toString())) { | 115 | + final String appId = intent.appId().name(); |
| 115 | - reason = "appId " + intent.appId().toString(); | 116 | + if (!jsonAppId.equals(appId)) { |
| 117 | + reason = "appId was " + jsonAppId; | ||
| 116 | return false; | 118 | return false; |
| 117 | } | 119 | } |
| 118 | 120 | ||
| ... | @@ -123,13 +125,6 @@ public class IntentsResourceTest extends ResourceTest { | ... | @@ -123,13 +125,6 @@ public class IntentsResourceTest extends ResourceTest { |
| 123 | return false; | 125 | return false; |
| 124 | } | 126 | } |
| 125 | 127 | ||
| 126 | - // check details field | ||
| 127 | - final String jsonDetails = jsonIntent.get("details").asString(); | ||
| 128 | - if (!jsonDetails.equals(intent.toString())) { | ||
| 129 | - reason = "details " + intent.toString(); | ||
| 130 | - return false; | ||
| 131 | - } | ||
| 132 | - | ||
| 133 | // check state field | 128 | // check state field |
| 134 | final String jsonState = jsonIntent.get("state").asString(); | 129 | final String jsonState = jsonIntent.get("state").asString(); |
| 135 | if (!jsonState.equals("INSTALLED")) { | 130 | if (!jsonState.equals("INSTALLED")) { |
| ... | @@ -196,7 +191,7 @@ public class IntentsResourceTest extends ResourceTest { | ... | @@ -196,7 +191,7 @@ public class IntentsResourceTest extends ResourceTest { |
| 196 | @Override | 191 | @Override |
| 197 | public boolean matchesSafely(JsonArray json) { | 192 | public boolean matchesSafely(JsonArray json) { |
| 198 | boolean intentFound = false; | 193 | boolean intentFound = false; |
| 199 | - final int expectedAttributes = 6; | 194 | + final int expectedAttributes = 5; |
| 200 | for (int jsonIntentIndex = 0; jsonIntentIndex < json.size(); | 195 | for (int jsonIntentIndex = 0; jsonIntentIndex < json.size(); |
| 201 | jsonIntentIndex++) { | 196 | jsonIntentIndex++) { |
| 202 | 197 | ||
| ... | @@ -336,11 +331,12 @@ public class IntentsResourceTest extends ResourceTest { | ... | @@ -336,11 +331,12 @@ public class IntentsResourceTest extends ResourceTest { |
| 336 | .andReturn(intent) | 331 | .andReturn(intent) |
| 337 | .anyTimes(); | 332 | .anyTimes(); |
| 338 | replay(mockIntentService); | 333 | replay(mockIntentService); |
| 339 | - expect(mockCoreService.getAppId(APP_ID.id())) | 334 | + expect(mockCoreService.getAppId(APP_ID.name())) |
| 340 | .andReturn(APP_ID).anyTimes(); | 335 | .andReturn(APP_ID).anyTimes(); |
| 341 | replay(mockCoreService); | 336 | replay(mockCoreService); |
| 342 | final WebResource rs = resource(); | 337 | final WebResource rs = resource(); |
| 343 | - final String response = rs.path("intents/1/0").get(String.class); | 338 | + final String response = rs.path("intents/" + APP_ID.name() |
| 339 | + + "/0").get(String.class); | ||
| 344 | final JsonObject result = JsonObject.readFrom(response); | 340 | final JsonObject result = JsonObject.readFrom(response); |
| 345 | assertThat(result, matchesIntent(intent)); | 341 | assertThat(result, matchesIntent(intent)); |
| 346 | } | 342 | } |
| ... | @@ -371,8 +367,9 @@ public class IntentsResourceTest extends ResourceTest { | ... | @@ -371,8 +367,9 @@ public class IntentsResourceTest extends ResourceTest { |
| 371 | */ | 367 | */ |
| 372 | @Test | 368 | @Test |
| 373 | public void testPost() { | 369 | public void testPost() { |
| 374 | - expect(mockCoreService.getAppId((short) 2)) | 370 | + ApplicationId testId = new DefaultApplicationId(2, "myApp"); |
| 375 | - .andReturn(new DefaultApplicationId(2, "app")); | 371 | + expect(mockCoreService.getAppId("myApp")) |
| 372 | + .andReturn(testId); | ||
| 376 | replay(mockCoreService); | 373 | replay(mockCoreService); |
| 377 | 374 | ||
| 378 | mockIntentService.submit(anyObject()); | 375 | mockIntentService.submit(anyObject()); | ... | ... |
-
Please register or login to post a comment