nacx requested changes on this pull request.
Thanks @andreaturli!
> + VPC() {}
+
+ public static Builder builder() {
+ return new AutoValue_VPC.Builder();
+ }
+
+ @AutoValue.Builder
+ public abstract static class Builder {
+
+ public abstract Builder id(String id);
+ public abstract Builder state(State state);
+ public abstract Builder cidrBlock(String cidrBlock);
+ public abstract Builder dhcpOptionsId(String dhcpOptionsId);
+ public abstract Builder instanceTenancy(String instanceTenancy);
+ public abstract Builder isDefault(Boolean isDefault);
+ public abstract Builder tags(Map<String, String> tags);
Enforce an immutable map by providing a hidden method for auto-value. [This is
the pattern](d31a85af1fa1b7de3676946ce9eaf29ad23e3b1f) we use.
> + PENDING, UNRECOGNIZED;
+ public String value() {
+ return name().toLowerCase();
+ }
+
+ public static State fromValue(String v) {
+ try {
+ return valueOf(v.toUpperCase());
+ } catch (IllegalArgumentException e) {
+ return UNRECOGNIZED;
+ }
+ }
+ }
+
+ @Nullable
+ public abstract String id();
Is this really nullable?
> + }
+ }
+ }
+
+ @Nullable
+ public abstract String id();
+ @Nullable
+ public abstract State state();
+ @Nullable
+ public abstract String cidrBlock();
+ @Nullable
+ public abstract String dhcpOptionsId();
+ @Nullable
+ public abstract String instanceTenancy();
+ @Nullable
+ public abstract Boolean isDefault();
Same here. Is this really nullable? Use a primitive type if not?
> + */
+@RequestFilters(FormSigner.class)
+@VirtualHost
+public interface VPCApi {
+
+ /**
+ * Describes all of your VPCs
+ *
+ * @return VPCs or empty if there are none
+ * @see <a href=
+ *
"http://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_DescribeVpcs.html";
+ * >docs</href>
+ */
+ @Named("DescribeVpcs")
+ @POST
+ @Path("/")
Move this annotation to the class and remove it from all methods.
> +import static org.testng.Assert.assertTrue;
+
+import org.jclouds.aws.ec2.AWSEC2Api;
+import org.jclouds.aws.ec2.domain.VPC;
+import org.jclouds.aws.ec2.options.CreateVpcOptions;
+import org.jclouds.compute.internal.BaseComputeServiceContextLiveTest;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import com.google.common.collect.FluentIterable;
+
+/**
+ * Tests behavior of {@code VPCApi}
+ */
+@Test(groups = "live", singleThreaded = true)
+public class VPCApiLiveTest extends BaseComputeServiceContextLiveTest {
SHouldn't this better extend the BaseHttpApiLiveTest?
> + client = view.unwrapApi(AWSEC2Api.class).getVPCApi().get();
+ }
+
+ @Test
+ public void testCreate() {
+ vpc = client.createVpc(null, "10.0.0.0/16", CreateVpcOptions.NONE);
+ assertNotNull(vpc);
+ }
+
+ @Test(dependsOnMethods = "testCreate")
+ public void testGet() {
+ FluentIterable<VPC> vpcs = client.describeVpcsInRegion(null, vpc.id());
+ assertTrue(vpcs.toList().size() == 1);
+ }
+
+ @Test(dependsOnMethods = "testGet")
Depend on the `testCreate` to avoid skipping this if the get test fails.
> + assertNotNull(vpc);
+ }
+
+ @Test(dependsOnMethods = "testCreate")
+ public void testGet() {
+ FluentIterable<VPC> vpcs = client.describeVpcsInRegion(null, vpc.id());
+ assertTrue(vpcs.toList().size() == 1);
+ }
+
+ @Test(dependsOnMethods = "testGet")
+ public void testList() {
+ FluentIterable<VPC> vpcs = client.describeVpcsInRegion(null);
+ assertTrue(!vpcs.toList().isEmpty());
+ }
+
+ @Test(dependsOnMethods = "testList")
`@Test(dependsOnMethods = {"testList", "testGet"}, alwaysRun = true)` to make
sure this is never skipped and we don't leak resources.
> + @EndpointParam(parser = RegionToEndpointOrProviderIfNull.class)
> @Nullable String region,
+ @FormParam("CidrBlock") String cidrBlock, CreateVpcOptions...
options);
+ /**
+ * Deletes {@code VPC}.
+ *
+ * @param region
+ * VPCs are tied to the Region where its files are located within
Amazon S3.
+ * @param vpcId
+ * The VPC ID.
+ *
+ */
+ @Named("DeleteVpc")
+ @POST
+ @Path("/")
+ @FormParams(keys = ACTION, values = "DeleteVpc")
+ @XMLResponseParser(ReturnValueHandler.class)
Add a fallback to return false if the vpc does not exist? It is a common
pattern in jclouds.
> + @Test
+ public void testCreate() {
+ vpc = client.createVpc(null, "10.0.0.0/16", CreateVpcOptions.NONE);
+ assertNotNull(vpc);
+ }
+
+ @Test(dependsOnMethods = "testCreate")
+ public void testGet() {
+ FluentIterable<VPC> vpcs = client.describeVpcsInRegion(null, vpc.id());
+ assertTrue(vpcs.toList().size() == 1);
+ }
+
+ @Test(dependsOnMethods = "testGet")
+ public void testList() {
+ FluentIterable<VPC> vpcs = client.describeVpcsInRegion(null);
+ assertTrue(!vpcs.toList().isEmpty());
Better use just assertFalse. Also, check that the created VPC is in the list,
and I'd check the method when passing the region id too.
> +
+ assertNotNull(result);
+
+ assertPosted(DEFAULT_REGION, "Action=DescribeRegions");
+ assertPosted(DEFAULT_REGION, "Action=CreateVpc&CidrBlock=10.0.0.0/16");
+ }
+
+ public void describeVpcsInRegion() throws Exception {
+ enqueueRegions(DEFAULT_REGION);
+ enqueueXml(DEFAULT_REGION, "/describe_vpcs.xml");
+ FluentIterable<VPC> result =
vpcApi().describeVpcsInRegion(DEFAULT_REGION);
+
+ assertFalse(result.isEmpty());
+ assertPosted(DEFAULT_REGION, "Action=DescribeRegions");
+ assertPosted(DEFAULT_REGION, "Action=DescribeVpcs");
+ }
Add tests to exercise all defined fallbacks.
> +import org.jclouds.aws.ec2.domain.VPC;
+import org.jclouds.aws.ec2.internal.BaseAWSEC2ApiMockTest;
+import org.jclouds.aws.ec2.options.CreateVpcOptions;
+import org.testng.annotations.Test;
+
+import com.google.common.collect.FluentIterable;
+
+@Test(groups = "unit", testName = "VPCApiMockTest", singleThreaded = true)
+public class VPCApiMockTest extends BaseAWSEC2ApiMockTest {
+
+ public void create() throws Exception {
+ enqueueRegions(DEFAULT_REGION);
+ enqueueXml(DEFAULT_REGION, "/create_vpc.xml");
+ VPC result = vpcApi().createVpc(DEFAULT_REGION, "10.0.0.0/16",
CreateVpcOptions.NONE);
+
+ assertNotNull(result);
Verify (at least) the id field to make sure the response is properly
deserialized.
--
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/pull/1032#pullrequestreview-6370065
