Revise Mastership REST API to use synchronous methods

- Add missing swagger docs
- Add missing unit tests

Change-Id: I21a20410ea322e7893c5c0c48f5d9fd0f2f9dfe1

@@ -37,8 +37,6 @@ import javax.ws.rs.core.Response;
 import java.io.IOException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStream;
 import java.util.Set;
 import java.util.Set;
-import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.ExecutionException;
 
 
 import static org.onlab.util.Tools.nullIsNotFound;
 import static org.onlab.util.Tools.nullIsNotFound;
 
 
@@ -48,8 +46,7 @@ import static org.onlab.util.Tools.nullIsNotFound;
 @Path("mastership")
 @Path("mastership")
 public final class MastershipWebResource extends AbstractWebResource {
 public final class MastershipWebResource extends AbstractWebResource {
 
 
-    private static final String NODE = "node";
-    private static final String DEVICES = "devices";
+    private static final String DEVICE_IDS = "deviceIds";
     private static final String DEVICE_ID = "deviceId";
     private static final String DEVICE_ID = "deviceId";
     private static final String NODE_ID = "nodeId";
     private static final String NODE_ID = "nodeId";
 
 
@@ -59,7 +56,6 @@ public final class MastershipWebResource extends AbstractWebResource {
     private static final String NODE_ID_NOT_FOUND = "Node id is not found";
     private static final String NODE_ID_NOT_FOUND = "Node id is not found";
     private static final String ROLE_INFO_NOT_FOUND = "Role info is not found";
     private static final String ROLE_INFO_NOT_FOUND = "Role info is not found";
     private static final String MASTERSHIP_ROLE_NOT_FOUND = "Mastership role is not found";
     private static final String MASTERSHIP_ROLE_NOT_FOUND = "Mastership role is not found";
-    private static final String RESULT_NOT_FOUND = "Result is not found";
 
 
     private final MastershipService mastershipService = get(MastershipService.class);
     private final MastershipService mastershipService = get(MastershipService.class);
     private final MastershipAdminService mastershipAdminService =
     private final MastershipAdminService mastershipAdminService =
@@ -86,7 +82,7 @@ public final class MastershipWebResource extends AbstractWebResource {
      *
      *
      * @param deviceId device identifier
      * @param deviceId device identifier
      * @return the identifier of the master controller for the device
      * @return the identifier of the master controller for the device
-     * // TODO: add swagger doc
+     * @onos.rsModel NodeId
      */
      */
     @GET
     @GET
     @Produces(MediaType.APPLICATION_JSON)
     @Produces(MediaType.APPLICATION_JSON)
@@ -96,7 +92,7 @@ public final class MastershipWebResource extends AbstractWebResource {
                     DeviceId.deviceId(deviceId)), NODE_ID_NOT_FOUND);
                     DeviceId.deviceId(deviceId)), NODE_ID_NOT_FOUND);
 
 
         ObjectNode root = mapper().createObjectNode();
         ObjectNode root = mapper().createObjectNode();
-        root.put(NODE, id.id());
+        root.put(NODE_ID, id.id());
         return ok(root).build();
         return ok(root).build();
     }
     }
 
 
@@ -123,14 +119,14 @@ public final class MastershipWebResource extends AbstractWebResource {
      *
      *
      * @param nodeId controller identifier
      * @param nodeId controller identifier
      * @return a set of device identifiers
      * @return a set of device identifiers
-     * // TODO: add swagger doc
+     * @onos.rsModel DeviceIds
      */
      */
     @GET
     @GET
     @Produces(MediaType.APPLICATION_JSON)
     @Produces(MediaType.APPLICATION_JSON)
     @Path("{nodeId}/device")
     @Path("{nodeId}/device")
     public Response getDeviceOf(@PathParam("nodeId") String nodeId) {
     public Response getDeviceOf(@PathParam("nodeId") String nodeId) {
         ObjectNode root = mapper().createObjectNode();
         ObjectNode root = mapper().createObjectNode();
-        ArrayNode devicesNode = root.putArray(DEVICES);
+        ArrayNode devicesNode = root.putArray(DEVICE_IDS);
 
 
         Set<DeviceId> devices = mastershipService.getDevicesOf(NodeId.nodeId(nodeId));
         Set<DeviceId> devices = mastershipService.getDevicesOf(NodeId.nodeId(nodeId));
         if (devices != null) {
         if (devices != null) {
@@ -152,20 +148,10 @@ public final class MastershipWebResource extends AbstractWebResource {
     @Produces(MediaType.APPLICATION_JSON)
     @Produces(MediaType.APPLICATION_JSON)
     @Path("{deviceId}/request")
     @Path("{deviceId}/request")
     public Response requestRoleFor(@PathParam("deviceId") String deviceId) {
     public Response requestRoleFor(@PathParam("deviceId") String deviceId) {
-
-        // TODO: will not use CompletableFuture when MastershipService
-        // provides a non CompletableFuture object as an output
-        CompletableFuture<MastershipRole> result =
-                nullIsNotFound(mastershipService.requestRoleFor(
-                        DeviceId.deviceId(deviceId)), MASTERSHIP_ROLE_NOT_FOUND);
-
-        try {
-            MastershipRole role = result.get();
-            ObjectNode root = codec(MastershipRole.class).encode(role, this);
-            return ok(root).build();
-        } catch (InterruptedException | ExecutionException e) {
-            throw new IllegalArgumentException(e);
-        }
+        MastershipRole role = nullIsNotFound(mastershipService.requestRoleForSync(
+                                DeviceId.deviceId(deviceId)), MASTERSHIP_ROLE_NOT_FOUND);
+        ObjectNode root = codec(MastershipRole.class).encode(role, this);
+        return ok(root).build();
     }
     }
 
 
     /**
     /**
@@ -181,18 +167,8 @@ public final class MastershipWebResource extends AbstractWebResource {
     @Path("{deviceId}/relinquish")
     @Path("{deviceId}/relinquish")
     public Response relinquishMastership(@PathParam("deviceId") String deviceId) {
     public Response relinquishMastership(@PathParam("deviceId") String deviceId) {
         DeviceId id = DeviceId.deviceId(deviceId);
         DeviceId id = DeviceId.deviceId(deviceId);
-
-        // TODO: will not use CompletableFuture when MastershipService
-        // provides a non CompletableFuture object as an output
-        CompletableFuture<Void> result =
-                nullIsNotFound(mastershipService.relinquishMastership(id), RESULT_NOT_FOUND);
-
-        try {
-            result.get();
-            return Response.created(id.uri()).build();
-        } catch (InterruptedException | ExecutionException e) {
-            throw new IllegalArgumentException(e);
-        }
+        mastershipService.relinquishMastershipSync(id);
+        return Response.created(id.uri()).build();
     }
     }
 
 
     /**
     /**
@@ -222,15 +198,11 @@ public final class MastershipWebResource extends AbstractWebResource {
                 throw new IllegalArgumentException(NODE_ID_INVALID);
                 throw new IllegalArgumentException(NODE_ID_INVALID);
             }
             }
 
 
-            // TODO: will not use CompletableFuture when MastershipAdminService
-            // provides a non CompletableFuture object as an output
-            CompletableFuture<Void> result =
-                    nullIsNotFound(mastershipAdminService.setRole(NodeId.nodeId(nodeIdJson.asText()),
-                    DeviceId.deviceId(deviceIdJson.asText()), role), RESULT_NOT_FOUND);
-            result.get();
+            mastershipAdminService.setRoleSync(NodeId.nodeId(nodeIdJson.asText()),
+                    DeviceId.deviceId(deviceIdJson.asText()), role);
 
 
             return Response.ok().build();
             return Response.ok().build();
-        } catch (InterruptedException | ExecutionException | IOException e) {
+        } catch (IOException e) {
             throw new IllegalArgumentException(e);
             throw new IllegalArgumentException(e);
         }
         }
     }
     }

+{
+  "type": "object",
+  "title": "deviceIds",
+  "required": [
+    "deviceIds"
+  ],
+  "properties": {
+    "deviceIds": {
+      "type": "array",
+      "xml": {
+        "name": "deviceId",
+        "wrapped": true
+      },
+      "items": {
+        "type": "string",
+        "example": "of:0000000000000001"
+      }
+    }
+  }
+}

+{
+  "type": "object",
+  "title": "nodeId",
+  "required": [
+    "nodeId"
+  ],
+  "properties": {
+    "nodeId": {
+      "type": "string",
+      "example": "1"
+    }
+  }
+}
\ No newline at end of file

@@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSet;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.hamcrest.Description;
 import org.hamcrest.Description;
+import org.hamcrest.Matchers;
 import org.hamcrest.TypeSafeMatcher;
 import org.hamcrest.TypeSafeMatcher;
 import org.junit.Before;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.Test;
@@ -37,8 +38,10 @@ import org.onosproject.mastership.MastershipService;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.MastershipRole;
 import org.onosproject.net.MastershipRole;
 
 
+import javax.ws.rs.client.Entity;
 import javax.ws.rs.client.WebTarget;
 import javax.ws.rs.client.WebTarget;
 import javax.ws.rs.core.Response;
 import javax.ws.rs.core.Response;
+import java.io.InputStream;
 import java.net.HttpURLConnection;
 import java.net.HttpURLConnection;
 import java.util.List;
 import java.util.List;
 import java.util.Set;
 import java.util.Set;
@@ -214,9 +217,9 @@ public final class MastershipResourceTest extends ResourceTest {
         assertThat(result, notNullValue());
         assertThat(result, notNullValue());
 
 
         assertThat(result.names(), hasSize(1));
         assertThat(result.names(), hasSize(1));
-        assertThat(result.names().get(0), is("node"));
+        assertThat(result.names().get(0), is("nodeId"));
 
 
-        final String node = result.get("node").asString();
+        final String node = result.get("nodeId").asString();
         assertThat(node, notNullValue());
         assertThat(node, notNullValue());
         assertThat(node, is("node:1"));
         assertThat(node, is("node:1"));
     }
     }
@@ -269,9 +272,9 @@ public final class MastershipResourceTest extends ResourceTest {
         assertThat(result, notNullValue());
         assertThat(result, notNullValue());
 
 
         assertThat(result.names(), hasSize(1));
         assertThat(result.names(), hasSize(1));
-        assertThat(result.names().get(0), is("devices"));
+        assertThat(result.names().get(0), is("deviceIds"));
 
 
-        final JsonArray jsonDevices = result.get("devices").asArray();
+        final JsonArray jsonDevices = result.get("deviceIds").asArray();
         assertThat(jsonDevices, notNullValue());
         assertThat(jsonDevices, notNullValue());
         assertThat(jsonDevices.size(), is(3));
         assertThat(jsonDevices.size(), is(3));
     }
     }
@@ -281,7 +284,21 @@ public final class MastershipResourceTest extends ResourceTest {
      */
      */
     @Test
     @Test
     public void testRequestRoleFor() {
     public void testRequestRoleFor() {
-        // TODO: will be added when CompletableFuture is removed
+        expect(mockService.requestRoleForSync(anyObject())).andReturn(role1).anyTimes();
+        replay(mockService);
+
+        final WebTarget wt = target();
+        final String response = wt.path("mastership/" + deviceId1.toString() +
+                "/request").request().get(String.class);
+        final JsonObject result = Json.parse(response).asObject();
+        assertThat(result, notNullValue());
+
+        assertThat(result.names(), hasSize(1));
+        assertThat(result.names().get(0), is("role"));
+
+        final String role = result.get("role").asString();
+        assertThat(role, notNullValue());
+        assertThat(role, is("MASTER"));
     }
     }
 
 
     /**
     /**
@@ -289,7 +306,16 @@ public final class MastershipResourceTest extends ResourceTest {
      */
      */
     @Test
     @Test
     public void testRelinquishMastership() {
     public void testRelinquishMastership() {
-        // TODO: will be added when CompletableFuture is removed
+        mockService.relinquishMastershipSync(anyObject());
+        expectLastCall();
+        replay(mockService);
+
+        final WebTarget wt = target();
+        final Response response = wt.path("mastership/" + deviceId1.toString() +
+                "/relinquish").request().get();
+        assertThat(response.getStatus(), is(HttpURLConnection.HTTP_CREATED));
+        String location = response.getLocation().toString();
+        assertThat(location, Matchers.startsWith(deviceId1.toString()));
     }
     }
 
 
     /**
     /**
@@ -297,7 +323,16 @@ public final class MastershipResourceTest extends ResourceTest {
      */
      */
     @Test
     @Test
     public void testSetRole() {
     public void testSetRole() {
-        // TODO: will be added when CompletableFuture is removed
+        mockAdminService.setRoleSync(anyObject(), anyObject(), anyObject());
+        expectLastCall();
+        replay(mockAdminService);
+
+        final WebTarget wt = target();
+        final InputStream jsonStream = MetersResourceTest.class
+                .getResourceAsStream("put-set-roles.json");
+        final Response response = wt.path("mastership")
+                                    .request().put(Entity.json(jsonStream));
+        assertThat(response.getStatus(), is(HttpURLConnection.HTTP_OK));
     }
     }
 
 
     /**
     /**