ONOS-2144 - Complete implementation of REST API for flows

- URL for creation of a single flow is now /flows/{deviceId}
- On successful creation of a flow, Location header contains
  a reference to the new object's URI
- POST operation returns status code of CREATED
- implement DELETE operation for /flows/{deviceId}/{flowId}
- removed deprecations and warnings from REST API unit
  test for flows.

Change-Id: Idb43a651a659e60c07a6f36dfd69004c814b146b

@@ -33,7 +33,6 @@ import static org.onlab.util.Tools.nullIsIllegal;
  */
 public final class FlowRuleCodec extends JsonCodec<FlowRule> {
 
-    private static final String APP_ID = "appId";
     private static final String PRIORITY = "priority";
     private static final String TIMEOUT = "timeout";
     private static final String IS_PERMANENT = "isPermanent";
@@ -42,6 +41,7 @@ public final class FlowRuleCodec extends JsonCodec<FlowRule> {
     private static final String SELECTOR = "selector";
     private static final String MISSING_MEMBER_MESSAGE =
             " member is required in FlowRule";
+    public static final String REST_APP_ID = "org.onosproject.rest";
 
 
     @Override
@@ -52,10 +52,9 @@ public final class FlowRuleCodec extends JsonCodec<FlowRule> {
 
         FlowRule.Builder resultBuilder = new DefaultFlowRule.Builder();
 
-        short appId = nullIsIllegal(json.get(APP_ID),
-                APP_ID + MISSING_MEMBER_MESSAGE).shortValue();
         CoreService coreService = context.getService(CoreService.class);
-        resultBuilder.fromApp(coreService.getAppId(appId));
+        resultBuilder.fromApp(coreService
+                .registerApplication(REST_APP_ID));
 
         int priority = nullIsIllegal(json.get(PRIORITY),
                 PRIORITY + MISSING_MEMBER_MESSAGE).asInt();

@@ -97,7 +97,7 @@ public class FlowRuleCodecTest {
         flowRuleCodec = context.codec(FlowRule.class);
         assertThat(flowRuleCodec, notNullValue());
 
-        expect(mockCoreService.getAppId(APP_ID.id()))
+        expect(mockCoreService.registerApplication(FlowRuleCodec.REST_APP_ID))
                 .andReturn(APP_ID).anyTimes();
         replay(mockCoreService);
         context.registerService(CoreService.class, mockCoreService);

 {
-  "appId":-29467,
   "priority":1,
   "isPermanent":"false",
   "timeout":1,

 {
-  "appId":-29467,
   "priority":1,
   "isPermanent":"false",
   "timeout":1,

 {
-  "appId":-29467,
   "priority":1,
   "isPermanent":"false",
   "timeout":1,

 {
-   "appId":-29467,
    "priority":1,
    "isPermanent":"false",
    "timeout":1,

@@ -17,8 +17,12 @@ package org.onosproject.rest.resources;
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.stream.StreamSupport;
 
 import javax.ws.rs.Consumes;
+import javax.ws.rs.DELETE;
 import javax.ws.rs.GET;
 import javax.ws.rs.POST;
 import javax.ws.rs.Path;
@@ -36,6 +40,7 @@ import org.onosproject.net.flow.FlowRule;
 import org.onosproject.net.flow.FlowRuleService;
 import org.onosproject.rest.AbstractWebResource;
 
+import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.node.ArrayNode;
 import com.fasterxml.jackson.databind.node.ObjectNode;
 
@@ -125,22 +130,58 @@ public class FlowsWebResource extends AbstractWebResource {
      * Creates a flow rule from a POST of a JSON string and attempts to apply it.
      *
      * @param stream input JSON
-     * @return status of the request - ACCEPTED if the JSON is correct,
+     * @return status of the request - CREATED if the JSON is correct,
      * BAD_REQUEST if the JSON is invalid
      */
     @POST
+    @Path("{deviceId}")
     @Consumes(MediaType.APPLICATION_JSON)
     @Produces(MediaType.APPLICATION_JSON)
-    public Response createFlow(InputStream stream) {
+    public Response createFlow(@PathParam("deviceId") String deviceId,
+                               InputStream stream) {
+        URI location;
         try {
             FlowRuleService service = get(FlowRuleService.class);
             ObjectNode root = (ObjectNode) mapper().readTree(stream);
+            JsonNode specifiedDeviceId = root.get("deviceId");
+            if (specifiedDeviceId != null &&
+                    !specifiedDeviceId.asText().equals(deviceId)) {
+                throw new IllegalArgumentException(
+                        "Invalid deviceId in flow creation request");
+            }
+            root.put("deviceId", deviceId);
             FlowRule rule = codec(FlowRule.class).decode(root, this);
             service.applyFlowRules(rule);
-        } catch (IOException ex) {
+            location = new URI(Long.toString(rule.id().value()));
+        } catch (IOException | URISyntaxException ex) {
             return Response.status(Response.Status.INTERNAL_SERVER_ERROR).build();
         }
-        return Response.status(Response.Status.ACCEPTED).build();
+        return Response
+                .created(location)
+                .build();
+    }
+
+    /**
+     * Removes the flows for a given device with the given flow id.
+     *
+     * @param deviceId Id of device to look up
+     * @param flowId   Id of flow to look up
+     */
+    @DELETE
+    @Produces(MediaType.APPLICATION_JSON)
+    @Path("{deviceId}/{flowId}")
+    public void deleteFlowByDeviceIdAndFlowId(@PathParam("deviceId") String deviceId,
+                                                  @PathParam("flowId") long flowId) {
+        final Iterable<FlowEntry> deviceEntries =
+                service.getFlowEntries(DeviceId.deviceId(deviceId));
+
+        if (!deviceEntries.iterator().hasNext()) {
+            throw new ItemNotFoundException(DEVICE_NOT_FOUND);
+        }
+
+        StreamSupport.stream(deviceEntries.spliterator(), false)
+                .filter(entry -> entry.id().value() == flowId)
+                .forEach(service::removeFlowRules);
     }
 
 }

@@ -23,6 +23,7 @@ import java.util.Set;
 import javax.ws.rs.core.MediaType;
 
 import org.hamcrest.Description;
+import org.hamcrest.Matchers;
 import org.hamcrest.TypeSafeMatcher;
 import org.junit.After;
 import org.junit.Before;
@@ -33,12 +34,14 @@ import org.onlab.packet.MacAddress;
 import org.onlab.rest.BaseResource;
 import org.onosproject.codec.CodecService;
 import org.onosproject.codec.impl.CodecManager;
+import org.onosproject.codec.impl.FlowRuleCodec;
 import org.onosproject.core.CoreService;
 import org.onosproject.core.DefaultGroupId;
 import org.onosproject.core.GroupId;
 import org.onosproject.net.DefaultDevice;
 import org.onosproject.net.Device;
 import org.onosproject.net.DeviceId;
+import org.onosproject.net.IndexedLambda;
 import org.onosproject.net.NetTestTools;
 import org.onosproject.net.device.DeviceService;
 import org.onosproject.net.flow.DefaultTrafficSelector;
@@ -74,6 +77,7 @@ import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.fail;
+import static org.onosproject.net.NetTestTools.APP_ID;
 
 /**
  * Unit tests for Flows REST APIs.
@@ -103,6 +107,10 @@ public class FlowsResourceTest extends ResourceTest {
     final MockFlowEntry flow5 = new MockFlowEntry(deviceId2, 5);
     final MockFlowEntry flow6 = new MockFlowEntry(deviceId2, 6);
 
+    private static final String FLOW_JSON = "{\"priority\":1,\"isPermanent\":true,"
+            + "\"treatment\":{\"instructions\":[ {\"type\":\"OUTPUT\",\"port\":2}]},"
+            + "\"selector\":{\"criteria\":[ {\"type\":\"ETH_TYPE\",\"ethType\":2054}]}}";
+
     /**
      * Mock class for a flow entry.
      */
@@ -214,8 +222,8 @@ public class FlowsResourceTest extends ResourceTest {
      */
     private void setupMockFlows() {
         flow2.treatment = DefaultTrafficTreatment.builder()
-                .add(Instructions.modL0Lambda((short) 4))
-                .add(Instructions.modL0Lambda((short) 5))
+                .add(Instructions.modL0Lambda(new IndexedLambda((short) 4)))
+                .add(Instructions.modL0Lambda(new IndexedLambda((short) 5)))
                 .setEthDst(MacAddress.BROADCAST)
                 .build();
         flow2.selector = DefaultTrafficSelector.builder()
@@ -223,15 +231,15 @@ public class FlowsResourceTest extends ResourceTest {
                 .matchIPProtocol((byte) 9)
                 .build();
         flow4.treatment = DefaultTrafficTreatment.builder()
-                .add(Instructions.modL0Lambda((short) 6))
+                .add(Instructions.modL0Lambda(new IndexedLambda((short) 6)))
                 .build();
         final Set<FlowEntry> flows1 = new HashSet<>();
         flows1.add(flow1);
         flows1.add(flow2);
 
         final Set<FlowEntry> flows2 = new HashSet<>();
-        flows1.add(flow3);
-        flows1.add(flow4);
+        flows2.add(flow3);
+        flows2.add(flow4);
 
         rules.put(deviceId1, flows1);
         rules.put(deviceId2, flows2);
@@ -258,6 +266,8 @@ public class FlowsResourceTest extends ResourceTest {
         // Mock Core Service
         expect(mockCoreService.getAppId(anyShort()))
                 .andReturn(NetTestTools.APP_ID).anyTimes();
+        expect(mockCoreService.registerApplication(FlowRuleCodec.REST_APP_ID))
+                .andReturn(APP_ID).anyTimes();
         replay(mockCoreService);
 
         // Register the services needed for the test
@@ -565,10 +575,7 @@ public class FlowsResourceTest extends ResourceTest {
      */
     @Test
     public void testPost() {
-        String json = "{\"appId\":2,\"priority\":1,\"isPermanent\":true,"
-        + "\"deviceId\":\"of:0000000000000001\","
-        + "\"treatment\":{\"instructions\":[ {\"type\":\"OUTPUT\",\"port\":2}]},"
-        + "\"selector\":{\"criteria\":[ {\"type\":\"ETH_TYPE\",\"ethType\":2054}]}}";
+
 
         mockFlowService.applyFlowRules(anyObject());
         expectLastCall();
@@ -577,9 +584,32 @@ public class FlowsResourceTest extends ResourceTest {
         WebResource rs = resource();
 
 
-        ClientResponse response = rs.path("flows/")
+        ClientResponse response = rs.path("flows/of:0000000000000001")
+                .type(MediaType.APPLICATION_JSON_TYPE)
+                .post(ClientResponse.class, FLOW_JSON);
+        assertThat(response.getStatus(), is(HttpURLConnection.HTTP_CREATED));
+        String location = response.getLocation().getPath();
+        assertThat(location, Matchers.startsWith("/flows/of:0000000000000001/"));
+    }
+
+    /**
+     * Tests deleting a flow.
+     */
+    @Test
+    public void testDelete() {
+        setupMockFlows();
+        mockFlowService.removeFlowRules(anyObject());
+        expectLastCall();
+        replay(mockFlowService);
+
+        WebResource rs = resource();
+
+        String location = "/flows/1/155";
+
+        ClientResponse deleteResponse = rs.path(location)
                 .type(MediaType.APPLICATION_JSON_TYPE)
-                .post(ClientResponse.class, json);
-        assertThat(response.getStatus(), is(HttpURLConnection.HTTP_ACCEPTED));
+                .delete(ClientResponse.class);
+        assertThat(deleteResponse.getStatus(),
+                is(HttpURLConnection.HTTP_NO_CONTENT));
     }
 }