From 609982747f91cefb1f986ea77c84f1fca03cdde6 Mon Sep 17 00:00:00 2001 From: Ignasi Barrera Date: Fri, 22 Aug 2014 09:04:02 +0200 Subject: [PATCH] JCLOUDS-678: Do not silently return null in POST operations --- .../googlecomputeengine/features/DiskApi.java | 4 +- .../features/InstanceApi.java | 12 +--- .../features/DiskApiExpectTest.java | 4 +- .../features/InstanceApiExpectTest.java | 57 +++++++++++++++++-- .../features/InstanceApiLiveTest.java | 15 +++++ .../src/test/resources/instance_set_tags.json | 4 ++ 6 files changed, 76 insertions(+), 20 deletions(-) create mode 100644 providers/google-compute-engine/src/test/resources/instance_set_tags.json diff --git a/providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/DiskApi.java b/providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/DiskApi.java index dcb1942675..58a9562b82 100644 --- a/providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/DiskApi.java +++ b/providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/DiskApi.java @@ -235,7 +235,7 @@ public interface DiskApi { * * @param zone the zone the disk is in. * @param diskName the name of the disk. - * @param snapshotName the name for the snapshot to be craeted. + * @param snapshotName the name for the snapshot to be created. * * @return an Operation resource. To check on the status of an operation, poll the Operations resource returned to * you, and look for the status field. @@ -245,9 +245,7 @@ public interface DiskApi { @Consumes(MediaType.APPLICATION_JSON) @Path("/zones/{zone}/disks/{disk}/createSnapshot") @OAuthScopes(COMPUTE_SCOPE) - @Fallback(NullOnNotFoundOr404.class) @MapBinder(BindToJsonPayload.class) - @Nullable Operation createSnapshotInZone(@PathParam("zone") String zone, @PathParam("disk") String diskName, @PayloadParam("name") String snapshotName); diff --git a/providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/InstanceApi.java b/providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/InstanceApi.java index 2a88a79333..357f53dc1d 100644 --- a/providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/InstanceApi.java +++ b/providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/InstanceApi.java @@ -264,15 +264,13 @@ public interface InstanceApi { * @param zone the zone the instance is in * @param instanceName the instance name * @return an Operation resource. To check on the status of an operation, poll the Operations resource returned to - * you, and look for the status field. If the instance did not exist the result is null. + * you, and look for the status field. */ @Named("Instances:reset") @POST @Consumes(MediaType.APPLICATION_JSON) @Path("/zones/{zone}/instances/{instance}/reset") @OAuthScopes(COMPUTE_SCOPE) - @Fallback(NullOnNotFoundOr404.class) - @Nullable Operation resetInZone(@PathParam("zone") String zone, @PathParam("instance") String instanceName); @@ -292,8 +290,6 @@ public interface InstanceApi { @Produces(MediaType.APPLICATION_JSON) @Path("/zones/{zone}/instances/{instance}/attachDisk") @OAuthScopes(COMPUTE_SCOPE) - @Fallback(NullOnNotFoundOr404.class) - @Nullable Operation attachDiskInZone(@PathParam("zone") String zone, @PathParam("instance") String instanceName, @BinderParam(BindToJsonPayload.class) AttachDiskOptions attachDiskOptions); @@ -313,8 +309,6 @@ public interface InstanceApi { @Consumes(MediaType.APPLICATION_JSON) @Path("/zones/{zone}/instances/{instance}/detachDisk") @OAuthScopes(COMPUTE_SCOPE) - @Fallback(NullOnNotFoundOr404.class) - @Nullable Operation detachDiskInZone(@PathParam("zone") String zone, @PathParam("instance") String instanceName, @QueryParam("deviceName") String deviceName); @@ -345,9 +339,7 @@ public interface InstanceApi { @OAuthScopes(COMPUTE_SCOPE) @Consumes(MediaType.APPLICATION_JSON) @Produces(MediaType.APPLICATION_JSON) - @Fallback(NullOnNotFoundOr404.class) @MapBinder(MetadataBinder.class) - @Nullable Operation setMetadataInZone(@PathParam("zone") String zone, @PathParam("instance") String instanceName, @PayloadParam("items") Map metadata, @@ -369,9 +361,7 @@ public interface InstanceApi { @OAuthScopes(COMPUTE_SCOPE) @Consumes(MediaType.APPLICATION_JSON) @Produces(MediaType.APPLICATION_JSON) - @Fallback(NullOnNotFoundOr404.class) @MapBinder(BindToJsonPayload.class) - @Nullable Operation setTagsInZone(@PathParam("zone") String zone, @PathParam("instance") String instanceName, @PayloadParam("items") Set items, diff --git a/providers/google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/DiskApiExpectTest.java b/providers/google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/DiskApiExpectTest.java index 6aaf8fa457..32009b1732 100644 --- a/providers/google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/DiskApiExpectTest.java +++ b/providers/google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/DiskApiExpectTest.java @@ -30,6 +30,7 @@ import org.jclouds.googlecomputeengine.parse.ParseDiskTest; import org.jclouds.googlecomputeengine.parse.ParseOperationTest; import org.jclouds.http.HttpRequest; import org.jclouds.http.HttpResponse; +import org.jclouds.rest.ResourceNotFoundException; import org.testng.annotations.Test; @Test(groups = "unit") @@ -132,6 +133,7 @@ public class DiskApiExpectTest extends BaseGoogleComputeEngineApiExpectTest { assertEquals(api.createSnapshotInZone("us-central1-a", "testimage1", "test-snap"), new ParseOperationTest().expected()); } + @Test(expectedExceptions = ResourceNotFoundException.class) public void testCreateSnapshotResponseIs4xx() { HttpRequest createSnapshotRequest = HttpRequest .builder() @@ -149,7 +151,7 @@ public class DiskApiExpectTest extends BaseGoogleComputeEngineApiExpectTest { TOKEN_RESPONSE, createSnapshotRequest, createSnapshotResponse).getDiskApiForProject("myproject"); - assertNull(api.createSnapshotInZone("us-central1-a", "testimage1", "test-snap")); + api.createSnapshotInZone("us-central1-a", "testimage1", "test-snap"); } public void testDeleteDiskResponseIs2xx() { diff --git a/providers/google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/InstanceApiExpectTest.java b/providers/google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/InstanceApiExpectTest.java index f460de661e..bb681c709b 100644 --- a/providers/google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/InstanceApiExpectTest.java +++ b/providers/google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/InstanceApiExpectTest.java @@ -41,9 +41,11 @@ import org.jclouds.googlecomputeengine.parse.ParseInstanceTest; import org.jclouds.googlecomputeengine.parse.ParseOperationTest; import org.jclouds.http.HttpRequest; import org.jclouds.http.HttpResponse; +import org.jclouds.rest.ResourceNotFoundException; import org.testng.annotations.Test; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; @Test(groups = "unit") public class InstanceApiExpectTest extends BaseGoogleComputeEngineApiExpectTest { @@ -263,6 +265,7 @@ public class InstanceApiExpectTest extends BaseGoogleComputeEngineApiExpectTest new ParseOperationTest().expected()); } + @Test(expectedExceptions = ResourceNotFoundException.class) public void testSetInstanceMetadataResponseIs4xx() { HttpRequest setMetadata = HttpRequest .builder() @@ -279,7 +282,48 @@ public class InstanceApiExpectTest extends BaseGoogleComputeEngineApiExpectTest InstanceApi api = requestsSendResponses(requestForScopes(COMPUTE_SCOPE), TOKEN_RESPONSE, setMetadata, setMetadataResponse).getInstanceApiForProject("myproject"); - assertNull(api.setMetadataInZone("us-central1-a", "test-1", ImmutableMap.of("foo", "bar"), "efgh")); + api.setMetadataInZone("us-central1-a", "test-1", ImmutableMap.of("foo", "bar"), "efgh"); + } + + public void testSetInstanceTagsResponseIs2xx() { + HttpRequest setTags = HttpRequest + .builder() + .method("POST") + .endpoint("https://www.googleapis" + + ".com/compute/v1/projects/myproject/zones/us-central1-a/instances/test-1/setTags") + .addHeader("Accept", "application/json") + .addHeader("Authorization", "Bearer " + TOKEN) + .payload(payloadFromResourceWithContentType("/instance_set_tags.json", MediaType.APPLICATION_JSON)) + .build(); + + HttpResponse setTagsResponse = HttpResponse.builder().statusCode(200) + .payload(payloadFromResource("/zone_operation.json")).build(); + + InstanceApi api = requestsSendResponses(requestForScopes(COMPUTE_SCOPE), + TOKEN_RESPONSE, setTags, setTagsResponse).getInstanceApiForProject("myproject"); + + assertEquals(api.setTagsInZone("us-central1-a", "test-1", ImmutableSet.of("foo", "bar"), "efgh"), + new ParseOperationTest().expected()); + } + + @Test(expectedExceptions = ResourceNotFoundException.class) + public void testSetInstanceTagsResponseIs4xx() { + HttpRequest setTags = HttpRequest + .builder() + .method("POST") + .endpoint("https://www.googleapis" + + ".com/compute/v1/projects/myproject/zones/us-central1-a/instances/test-1/setTags") + .addHeader("Accept", "application/json") + .addHeader("Authorization", "Bearer " + TOKEN) + .payload(payloadFromResourceWithContentType("/instance_set_tags.json", MediaType.APPLICATION_JSON)) + .build(); + + HttpResponse setTagsResponse = HttpResponse.builder().statusCode(404).build(); + + InstanceApi api = requestsSendResponses(requestForScopes(COMPUTE_SCOPE), + TOKEN_RESPONSE, setTags, setTagsResponse).getInstanceApiForProject("myproject"); + + api.setTagsInZone("us-central1-a", "test-1", ImmutableSet.of("foo", "bar"), "efgh"); } public void testResetInstanceResponseIs2xx() { @@ -301,6 +345,7 @@ public class InstanceApiExpectTest extends BaseGoogleComputeEngineApiExpectTest new ParseOperationTest().expected()); } + @Test(expectedExceptions = ResourceNotFoundException.class) public void testResetInstanceResponseIs4xx() { HttpRequest reset = HttpRequest .builder() @@ -315,7 +360,7 @@ public class InstanceApiExpectTest extends BaseGoogleComputeEngineApiExpectTest InstanceApi api = requestsSendResponses(requestForScopes(COMPUTE_SCOPE), TOKEN_RESPONSE, reset, resetResponse).getInstanceApiForProject("myproject"); - assertNull(api.resetInZone("us-central1-a", "test-1")); + api.resetInZone("us-central1-a", "test-1"); } public void testAttachDiskResponseIs2xx() { @@ -343,6 +388,7 @@ public class InstanceApiExpectTest extends BaseGoogleComputeEngineApiExpectTest new ParseOperationTest().expected()); } + @Test(expectedExceptions = ResourceNotFoundException.class) public void testAttachDiskResponseIs4xx() { HttpRequest attach = HttpRequest .builder() @@ -359,11 +405,11 @@ public class InstanceApiExpectTest extends BaseGoogleComputeEngineApiExpectTest InstanceApi api = requestsSendResponses(requestForScopes(COMPUTE_SCOPE), TOKEN_RESPONSE, attach, attachResponse).getInstanceApiForProject("myproject"); - assertNull(api.attachDiskInZone("us-central1-a", "test-1", + api.attachDiskInZone("us-central1-a", "test-1", new AttachDiskOptions() .mode(DiskMode.READ_ONLY) .source(URI.create("https://www.googleapis.com/compute/v1/projects/myproject/zones/us-central1-a/disks/testimage1")) - .type(DiskType.PERSISTENT))); + .type(DiskType.PERSISTENT)); } @@ -388,6 +434,7 @@ public class InstanceApiExpectTest extends BaseGoogleComputeEngineApiExpectTest new ParseOperationTest().expected()); } + @Test(expectedExceptions = ResourceNotFoundException.class) public void testDetachDiskResponseIs4xx() { HttpRequest detach = HttpRequest .builder() @@ -404,7 +451,7 @@ public class InstanceApiExpectTest extends BaseGoogleComputeEngineApiExpectTest InstanceApi api = requestsSendResponses(requestForScopes(COMPUTE_SCOPE), TOKEN_RESPONSE, detach, detachResponse).getInstanceApiForProject("myproject"); - assertNull(api.detachDiskInZone("us-central1-a", "test-1", "test-disk-1")); + api.detachDiskInZone("us-central1-a", "test-1", "test-disk-1"); } } diff --git a/providers/google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/InstanceApiLiveTest.java b/providers/google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/InstanceApiLiveTest.java index 871f136460..fd80df0c72 100644 --- a/providers/google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/InstanceApiLiveTest.java +++ b/providers/google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/InstanceApiLiveTest.java @@ -23,6 +23,7 @@ import static org.testng.Assert.assertTrue; import java.net.URI; import java.util.List; import java.util.Properties; +import java.util.Set; import org.jclouds.collect.PagedIterable; import org.jclouds.googlecomputeengine.GoogleComputeEngineApi; @@ -41,6 +42,7 @@ import org.testng.annotations.Test; import com.google.common.base.Predicate; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.inject.Module; @@ -55,6 +57,7 @@ public class InstanceApiLiveTest extends BaseGoogleComputeEngineApiLiveTest { private static final String IPV4_RANGE = "10.0.0.0/8"; private static final String METADATA_ITEM_KEY = "instanceLiveTestTestProp"; private static final String METADATA_ITEM_VALUE = "instanceLiveTestTestValue"; + private static final Set TAGS = ImmutableSet.of("instanceLiveTestTag1", "instanceLiveTestTag2"); private static final String ATTACH_DISK_NAME = "instance-api-live-test-attach-disk"; private static final String ATTACH_DISK_DEVICE_NAME = "attach-disk-1"; @@ -144,7 +147,19 @@ public class InstanceApiLiveTest extends BaseGoogleComputeEngineApiLiveTest { assertEquals(modifiedInstance.getMetadata().getItems().get(METADATA_ITEM_KEY), METADATA_ITEM_VALUE); assertNotNull(modifiedInstance.getMetadata().getFingerprint()); + } + @Test(groups = "live", dependsOnMethods = "testListInstance") + public void testSetTagsForInstance() { + Instance originalInstance = api().getInZone(DEFAULT_ZONE_NAME, INSTANCE_NAME); + assertZoneOperationDoneSucessfully(api().setTagsInZone(DEFAULT_ZONE_NAME, INSTANCE_NAME, TAGS, + originalInstance.getMetadata().getFingerprint()), + TIME_WAIT); + + Instance modifiedInstance = api().getInZone(DEFAULT_ZONE_NAME, INSTANCE_NAME); + + assertTrue(modifiedInstance.getTags().getItems().containsAll(TAGS)); + assertNotNull(modifiedInstance.getTags().getFingerprint()); } @Test(groups = "live", dependsOnMethods = "testSetMetadataForInstance") diff --git a/providers/google-compute-engine/src/test/resources/instance_set_tags.json b/providers/google-compute-engine/src/test/resources/instance_set_tags.json new file mode 100644 index 0000000000..e775a22a6b --- /dev/null +++ b/providers/google-compute-engine/src/test/resources/instance_set_tags.json @@ -0,0 +1,4 @@ +{ + "items": [ "foo", "bar" ], + "fingerprint": "efgh" +} \ No newline at end of file