You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Trevor Flanagan <no...@github.com> on 2017/12/06 17:11:57 UTC
[jclouds/jclouds-labs] Adding the Dimension Data Tag API interface.
(#425)
This will be useful for supporting Groups and also for the functions that create the subclasses of `org.jclouds.compute.domain.internal.ComputeMetadataImpl#tags`
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds-labs/pull/425
-- Commit Summary --
* Adding the Dimension Data Tag API interface.
-- File Changes --
M dimensiondata/pom.xml (2)
M dimensiondata/src/main/java/org/jclouds/dimensiondata/cloudcontrol/DimensionDataCloudControlApi.java (4)
M dimensiondata/src/main/java/org/jclouds/dimensiondata/cloudcontrol/domain/Tag.java (1)
A dimensiondata/src/main/java/org/jclouds/dimensiondata/cloudcontrol/features/TagApi.java (192)
A dimensiondata/src/test/java/org/jclouds/dimensiondata/cloudcontrol/features/TagApiLiveTest.java (139)
A dimensiondata/src/test/java/org/jclouds/dimensiondata/cloudcontrol/features/TagApiMockTest.java (159)
M dimensiondata/src/test/java/org/jclouds/dimensiondata/cloudcontrol/internal/BaseDimensionDataCloudControlApiLiveTest.java (1)
A dimensiondata/src/test/resources/createTagKeyResponse.json (14)
A dimensiondata/src/test/resources/tagkey.json (7)
A dimensiondata/src/test/resources/tagkeys.json (70)
A dimensiondata/src/test/resources/tags.json (48)
-- Patch Links --
https://github.com/jclouds/jclouds-labs/pull/425.patch
https://github.com/jclouds/jclouds-labs/pull/425.diff
--
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
Re: [jclouds/jclouds-labs] Adding the Dimension Data Tag API
interface. (#425)
Posted by Ignasi Barrera <no...@github.com>.
Merged to [master](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/eaeea5c3) and [2.0.x](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/dab07d44). Thanks!
--
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#issuecomment-350268851
Re: [jclouds/jclouds-labs] Adding the Dimension Data Tag API
interface. (#425)
Posted by Trevor Flanagan <no...@github.com>.
trevorflanagan commented on this pull request.
> @@ -143,7 +143,7 @@ public void testDeleteTagKey() throws Exception {
@Test
public void testTagKeyById() throws Exception {
- server.enqueue(jsonResponse("/tagKey.json"));
+ server.enqueue(jsonResponse("/tagkey.json"));
caught out by the case. developing on windows here.
--
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-81602260
Re: [jclouds/jclouds-labs] Adding the Dimension Data Tag API
interface. (#425)
Posted by Ignasi Barrera <no...@github.com>.
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
Re: [jclouds/jclouds-labs] Adding the Dimension Data Tag API
interface. (#425)
Posted by Trevor Flanagan <no...@github.com>.
@trevorflanagan pushed 1 commit.
aab0692 Adding the Dimension Data Tag API interface.
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/425/files/b5021dae177ec8f76f1ec76bab53498cacc1e466..aab069263f3475b4f19d74a75b83507235ee1260
Re: [jclouds/jclouds-labs] Adding the Dimension Data Tag API
interface. (#425)
Posted by Trevor Flanagan <no...@github.com>.
@trevorflanagan pushed 1 commit.
565dfa5 Adding the Dimension Data Tag API interface. Updates for PR.
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/425/files/aab069263f3475b4f19d74a75b83507235ee1260..565dfa54bd59c0ca738d982186f52a7593370a49
Re: [jclouds/jclouds-labs] Adding the Dimension Data Tag API
interface. (#425)
Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.
> +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)
This one still needs to be removed
--
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#discussion_r155653441
Re: [jclouds/jclouds-labs] Adding the Dimension Data Tag API
interface. (#425)
Posted by Ignasi Barrera <no...@github.com>.
Thanks, @trevorflanagan! Just a couple minor final comments. Mind squashing the commits directly when fixing those?
--
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#issuecomment-350108361
Re: [jclouds/jclouds-labs] Adding the Dimension Data Tag API
interface. (#425)
Posted by Trevor Flanagan <no...@github.com>.
@nacx thanks for reviewing. Commits are squashed and pushed.
--
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#issuecomment-350240771
Re: [jclouds/jclouds-labs] Adding the Dimension Data Tag API
interface. (#425)
Posted by Ignasi Barrera <no...@github.com>.
Closed #425.
--
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#event-1378864451
Re: [jclouds/jclouds-labs] Adding the Dimension Data Tag API
interface. (#425)
Posted by Trevor Flanagan <no...@github.com>.
trevorflanagan commented on this pull request.
> + @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)
You are right this is consistent with the other feature api's we have all pushed. I made some additional changes to add more tests for pagination for the list interface methods.
--
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#discussion_r155542991
Re: [jclouds/jclouds-labs] Adding the Dimension Data Tag API
interface. (#425)
Posted by Trevor Flanagan <no...@github.com>.
trevorflanagan commented on this pull request.
> +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);
Thanks @nacx I verified with our API and these values are mandatory and therefore primitive is correct approach here.
--
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#discussion_r155542328
Re: [jclouds/jclouds-labs] Adding the Dimension Data Tag API
interface. (#425)
Posted by Trevor Flanagan <no...@github.com>.
@trevorflanagan pushed 1 commit.
b5021da Adding the Dimension Data Tag API interface.
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/425/files/e67b0018f713086d9e901b7c8b8bf6d11578a5a5..b5021dae177ec8f76f1ec76bab53498cacc1e466
Re: [jclouds/jclouds-labs] Adding the Dimension Data Tag API
interface. (#425)
Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.
> + public void testApplyTags() {
+ api().applyTags(SERVER_ID, assetType, Collections.singletonList(TagInfo.create(tagKeyId, "jcloudsValue")));
+ }
+
+ @Test
+ public void testListTags() {
+ PagedIterable<Tag> response = api().listTags();
+ assertTrue(FluentIterable.from(response.concat().toList()).anyMatch(new Predicate<Tag>() {
+ @Override
+ public boolean apply(Tag input) {
+ return input.tagKeyId().equals(tagKeyId);
+ }
+ }), String.format("Couldn't find tagKeyId %s in listTags response", tagKeyId));
+ }
+
+ @Test
Configure this test to be run after the `listTags`and `listTagKeys` tests?
--
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-82009108