nacx commented on this pull request.
Thanks @trevorflanagan!
> +import java.util.List;
+
+@RequestFilters({ BasicAuthentication.class, OrganisationIdFilter.class })
+@Consumes(MediaType.APPLICATION_JSON)
+@Path("/{jclouds.api-version}/tag")
+public interface TagApi {
+
+ @Named("tag:createTagKey")
+ @POST
+ @Path("/createTagKey")
+ @Produces(MediaType.APPLICATION_JSON)
+ @MapBinder(BindToJsonPayload.class)
+ @ResponseParser(TagKeyId.class)
+ String createTagKey(@PayloadParam("name") String name,
@PayloadParam("description") String description,
+ @PayloadParam("valueRequired") Boolean valueRequired,
+ @PayloadParam("displayOnReport") Boolean displayOnReport);
[minor] Better use primitives where possible, if it makes sense. Possible
`null` values just add uncertainty.
> +public interface TagApi {
+
+ @Named("tag:createTagKey")
+ @POST
+ @Path("/createTagKey")
+ @Produces(MediaType.APPLICATION_JSON)
+ @MapBinder(BindToJsonPayload.class)
+ @ResponseParser(TagKeyId.class)
+ String createTagKey(@PayloadParam("name") String name,
@PayloadParam("description") String description,
+ @PayloadParam("valueRequired") Boolean valueRequired,
+ @PayloadParam("displayOnReport") Boolean displayOnReport);
+
+ @Named("tag:applyTags")
+ @POST
+ @Path("/applyTags")
+ @Fallback(Fallbacks.NullOnNotFoundOr404.class)
Remove the 404 fallbacks from `POST` operations.
> + @PayloadParam("tagById") List<TagInfo> tagById);
+
+ @Named("tag:removeTags")
+ @POST
+ @Path("/removeTags")
+ @Fallback(Fallbacks.NullOnNotFoundOr404.class)
+ @MapBinder(BindToJsonPayload.class)
+ void removeTags(@PayloadParam("assetId") String assetId,
@PayloadParam("assetType") String assetType,
+ @PayloadParam("tagKeyId") List<String> tagKeyName);
+
+ @Named("tag:tagKey")
+ @GET
+ @Path("/tagKey")
+ @Fallback(Fallbacks.NullOnNotFoundOr404.class)
+ @ResponseParser(ParseTagKeys.class)
+ PaginatedCollection<TagKey> listTagKeys(PaginationOptions options);
Add the method without parameters that returns the `PagedIterable` with all
keys.
> +
+ @Named("tag:editTagKey")
+ @POST
+ @Path("/editTagKey")
+ @Produces(MediaType.APPLICATION_JSON)
+ @MapBinder(BindToJsonPayload.class)
+ void editTagKey(@PayloadParam("name") String name, @PayloadParam("id")
String id,
+ @PayloadParam("description") String description,
@PayloadParam("valueRequired") Boolean valueRequired,
+ @PayloadParam("displayOnReport") Boolean displayOnReport);
+
+ @Named("tag:deleteTagKey")
+ @POST
+ @Path("/deleteTagKey")
+ @Produces(MediaType.APPLICATION_JSON)
+ @MapBinder(BindToJsonPayload.class)
+ void deleteTagKey(@PayloadParam("id") String id);
We usually are defensive against 404 errors and return `null` on delete
operations. If what you want to delete does not exist, let's not fail. The
system is already in the desired status.
> + @PayloadParam("description") String description,
> @PayloadParam("valueRequired") Boolean valueRequired,
+ @PayloadParam("displayOnReport") Boolean displayOnReport);
+
+ @Named("tag:deleteTagKey")
+ @POST
+ @Path("/deleteTagKey")
+ @Produces(MediaType.APPLICATION_JSON)
+ @MapBinder(BindToJsonPayload.class)
+ void deleteTagKey(@PayloadParam("id") String id);
+
+ @Named("tag:tags")
+ @GET
+ @Path("/tag")
+ @Fallback(Fallbacks.NullOnNotFoundOr404.class)
+ @ResponseParser(ParseTags.class)
+ PaginatedCollection<Tag> listTags(PaginationOptions options);
Provide the method to list all here too.
> +import org.jclouds.dimensiondata.cloudcontrol.domain.TagInfo;
+import org.jclouds.dimensiondata.cloudcontrol.domain.TagKey;
+import
org.jclouds.dimensiondata.cloudcontrol.internal.BaseDimensionDataCloudControlApiLiveTest;
+import org.jclouds.dimensiondata.cloudcontrol.options.PaginationOptions;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import java.util.Collections;
+
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertTrue;
+import static org.testng.AssertJUnit.assertEquals;
+
+@Test(groups = "live", testName = "AccountApiLiveTest", singleThreaded = true)
"TagtApiLiveTest"
> + }));
+ }
+
+ private void assertTagKeyExistsAndIsValid(String tagKeyId, String
tagKeyName, String description,
+ boolean valueRequired, boolean displayOnReport) {
+ TagKey tagKey = api().tagKeyById(tagKeyId);
+ assertNotNull(tagKey);
+ assertEquals(tagKey.name(), tagKeyName);
+ assertEquals(tagKey.description(), description);
+ assertEquals(tagKey.valueRequired(), valueRequired);
+ assertEquals(tagKey.displayOnReport(), displayOnReport);
+ }
+
+ @AfterClass(alwaysRun = true)
+ public void cleanup() {
+ if (!tagKeyId.isEmpty()) {
Unlikely, but this could throw an NPE. Better check for `null` too.
> +
+import static javax.ws.rs.HttpMethod.GET;
+import static javax.ws.rs.HttpMethod.POST;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.AssertJUnit.assertEquals;
+
+@Test(groups = "unit", testName = "TagApiMockTest", singleThreaded = true)
+public class TagApiMockTest extends BaseAccountAwareCloudControlMockTest {
+
+ @Test
+ public void testCreateTagKey() throws Exception {
+ server.enqueue(jsonResponse("/createTagKeyResponse.json"));
+ final String tagKeyId = api.getTagApi()
+ .createTagKey("myTagKey", "myTagKeyDescription", Boolean.TRUE,
Boolean.FALSE);
+ assertEquals("c452ceac-8627-423f-a8d2-5bb4a03c01d3", tagKeyId);
+ //server.takeRequest();
Remove comments like this.
> + @MapBinder(BindToJsonPayload.class)
+ void applyTags(@PayloadParam("assetId") String assetId,
@PayloadParam("assetType") String assetType,
+ @PayloadParam("tagById") List<TagInfo> tagById);
+
+ @Named("tag:removeTags")
+ @POST
+ @Path("/removeTags")
+ @Fallback(Fallbacks.NullOnNotFoundOr404.class)
+ @MapBinder(BindToJsonPayload.class)
+ void removeTags(@PayloadParam("assetId") String assetId,
@PayloadParam("assetType") String assetType,
+ @PayloadParam("tagKeyId") List<String> tagKeyName);
+
+ @Named("tag:tagKey")
+ @GET
+ @Path("/tagKey")
+ @Fallback(Fallbacks.NullOnNotFoundOr404.class)
Do not return `null`; return an empty iterable.
> + @MapBinder(BindToJsonPayload.class)
+ void editTagKey(@PayloadParam("name") String name, @PayloadParam("id")
String id,
+ @PayloadParam("description") String description,
@PayloadParam("valueRequired") Boolean valueRequired,
+ @PayloadParam("displayOnReport") Boolean displayOnReport);
+
+ @Named("tag:deleteTagKey")
+ @POST
+ @Path("/deleteTagKey")
+ @Produces(MediaType.APPLICATION_JSON)
+ @MapBinder(BindToJsonPayload.class)
+ void deleteTagKey(@PayloadParam("id") String id);
+
+ @Named("tag:tags")
+ @GET
+ @Path("/tag")
+ @Fallback(Fallbacks.NullOnNotFoundOr404.class)
Same here; return an empty iterable.
> + @Test
+ public void testListTags() throws Exception {
+ server.enqueue(jsonResponse("/tags.json"));
+ PaginatedCollection<Tag> tagPaginatedCollection = api.getTagApi()
+ .listTags(PaginationOptions.Builder.pageSize(100));
+ assertNotNull(tagPaginatedCollection);
+ assertEquals(tagPaginatedCollection.size(), 4);
+ assertEquals(tagPaginatedCollection.getPageSize(), 100);
+
+ assertSent(GET,
"/caas/2.4/6ac1e746-b1ea-4da5-a24e-caf1a978789d/tag/tag?pageSize=100");
+ }
+
+ @Test
+ public void testListTags_404() throws Exception {
+ server.enqueue(response404());
+ api.getTagApi().listTags(PaginationOptions.Builder.pageSize(100));
Verify that the returned value is an empty iterable.
> + server.enqueue(response404());
+ api.getTagApi().listTags(PaginationOptions.Builder.pageSize(100));
+ assertSent(GET,
"/caas/2.4/6ac1e746-b1ea-4da5-a24e-caf1a978789d/tag/tag?pageSize=100");
+ }
+
+ @Test
+ public void testListTagKeys() throws Exception {
+ server.enqueue(jsonResponse("/tagkeys.json"));
+ PaginatedCollection<TagKey> tagPaginatedCollection = api.getTagApi()
+ .listTagKeys(PaginationOptions.Builder.pageSize(100));
+ assertNotNull(tagPaginatedCollection);
+ assertEquals(tagPaginatedCollection.size(), 9);
+ assertEquals(tagPaginatedCollection.getPageSize(), 100);
+
+ assertSent(GET,
"/caas/2.4/6ac1e746-b1ea-4da5-a24e-caf1a978789d/tag/tagKey?pageSize=100");
+ }
Add the test for the 404 fallback.
> +
> "/caas/2.4/6ac1e746-b1ea-4da5-a24e-caf1a978789d/tag/deleteTagKey");
+ assertBodyContains(recordedRequest, "{\"id\":\"tagKeyId\"}");
+ }
+
+ @Test
+ public void testTagKeyById() throws Exception {
+ server.enqueue(jsonResponse("/tagkey.json"));
+ TagKey tagKey = api.getTagApi().tagKeyById("tagKeyId");
+ assertSent(GET,
"/caas/2.4/6ac1e746-b1ea-4da5-a24e-caf1a978789d/tag/tagKey/tagKeyId");
+ assertNotNull(tagKey);
+ }
+
+ @Test
+ public void testTagKeyById_404() throws Exception {
+ server.enqueue(response404());
+ api.getTagApi().tagKeyById("tagKeyId");
Assert that the returned value is `null`.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/425#pullrequestreview-81766649
