From 0358d5828fad6357cccaef0309fa91cad1b2b30c Mon Sep 17 00:00:00 2001 From: Marco Ziccardi Date: Tue, 15 Mar 2016 13:00:43 +0100 Subject: [PATCH] Refactor compute operations - Add Type enum to OperationId and type() getter - Replace instanceof with switch on type() - Add better javadoc to Operation - Remove final from Operation, make hashCode and equals final --- .../com/google/gcloud/compute/Compute.java | 72 ++++++++----------- .../google/gcloud/compute/ComputeImpl.java | 21 +++--- .../gcloud/compute/GlobalOperationId.java | 5 ++ .../com/google/gcloud/compute/Operation.java | 36 ++++++---- .../google/gcloud/compute/OperationId.java | 26 +++++++ .../gcloud/compute/RegionOperationId.java | 5 ++ .../gcloud/compute/ZoneOperationId.java | 5 ++ .../google/gcloud/spi/DefaultComputeRpc.java | 2 +- .../gcloud/compute/OperationIdTest.java | 9 +++ .../gcloud/compute/it/ITComputeTest.java | 2 +- 10 files changed, 117 insertions(+), 66 deletions(-) diff --git a/gcloud-java-compute/src/main/java/com/google/gcloud/compute/Compute.java b/gcloud-java-compute/src/main/java/com/google/gcloud/compute/Compute.java index faa890a1e982..935cf1fa9d7f 100644 --- a/gcloud-java-compute/src/main/java/com/google/gcloud/compute/Compute.java +++ b/gcloud-java-compute/src/main/java/com/google/gcloud/compute/Compute.java @@ -288,7 +288,7 @@ abstract class ListFilter implements Serializable { enum ComparisonOperator { /** - * Defines an equality filter. + * Defines an equals filter. */ EQ, @@ -340,11 +340,11 @@ class DiskTypeFilter extends ListFilter { } /** - * Returns an equality filter for the given field and string value. For string fields, + * Returns an equals filter for the given field and string value. For string fields, * {@code value} is interpreted as a regular expression using RE2 syntax. {@code value} must * match the entire field. * - * @see RE2 + * @see RE2 */ public static DiskTypeFilter equals(DiskTypeField field, String value) { return new DiskTypeFilter(checkNotNull(field), ComparisonOperator.EQ, checkNotNull(value)); @@ -355,14 +355,14 @@ public static DiskTypeFilter equals(DiskTypeField field, String value) { * {@code value} is interpreted as a regular expression using RE2 syntax. {@code value} must * match the entire field. * - * @see RE2 + * @see RE2 */ public static DiskTypeFilter notEquals(DiskTypeField field, String value) { return new DiskTypeFilter(checkNotNull(field), ComparisonOperator.NE, checkNotNull(value)); } /** - * Returns an equality filter for the given field and long value. + * Returns an equals filter for the given field and long value. */ public static DiskTypeFilter equals(DiskTypeField field, long value) { return new DiskTypeFilter(checkNotNull(field), ComparisonOperator.EQ, value); @@ -388,11 +388,11 @@ class MachineTypeFilter extends ListFilter { } /** - * Returns an equality filter for the given field and string value. For string fields, + * Returns an equals filter for the given field and string value. For string fields, * {@code value} is interpreted as a regular expression using RE2 syntax. {@code value} must * match the entire field. * - * @see RE2 + * @see RE2 */ public static MachineTypeFilter equals(MachineTypeField field, String value) { return new MachineTypeFilter(checkNotNull(field), ComparisonOperator.EQ, checkNotNull(value)); @@ -403,14 +403,14 @@ public static MachineTypeFilter equals(MachineTypeField field, String value) { * {@code value} is interpreted as a regular expression using RE2 syntax. {@code value} must * match the entire field. * - * @see RE2 + * @see RE2 */ public static MachineTypeFilter notEquals(MachineTypeField field, String value) { return new MachineTypeFilter(checkNotNull(field), ComparisonOperator.NE, checkNotNull(value)); } /** - * Returns an equality filter for the given field and long value. + * Returns an equals filter for the given field and long value. */ public static MachineTypeFilter equals(MachineTypeField field, long value) { return new MachineTypeFilter(checkNotNull(field), ComparisonOperator.EQ, value); @@ -436,11 +436,11 @@ class RegionFilter extends ListFilter { } /** - * Returns an equality filter for the given field and string value. For string fields, + * Returns an equals filter for the given field and string value. For string fields, * {@code value} is interpreted as a regular expression using RE2 syntax. {@code value} must * match the entire field. * - * @see RE2 + * @see RE2 */ public static RegionFilter equals(RegionField field, String value) { return new RegionFilter(checkNotNull(field), ComparisonOperator.EQ, value); @@ -451,7 +451,7 @@ public static RegionFilter equals(RegionField field, String value) { * {@code value} is interpreted as a regular expression using RE2 syntax. {@code value} must * match the entire field. * - * @see RE2 + * @see RE2 */ public static RegionFilter notEquals(RegionField field, String value) { return new RegionFilter(checkNotNull(field), ComparisonOperator.NE, checkNotNull(value)); @@ -470,11 +470,11 @@ class ZoneFilter extends ListFilter { } /** - * Returns an equality filter for the given field and string value. For string fields, + * Returns an equals filter for the given field and string value. For string fields, * {@code value} is interpreted as a regular expression using RE2 syntax. {@code value} must * match the entire field. * - * @see RE2 + * @see RE2 */ public static ZoneFilter equals(ZoneField field, String value) { return new ZoneFilter(checkNotNull(field), ComparisonOperator.EQ, checkNotNull(value)); @@ -485,7 +485,7 @@ public static ZoneFilter equals(ZoneField field, String value) { * {@code value} is interpreted as a regular expression using RE2 syntax. {@code value} must * match the entire field. * - * @see RE2 + * @see RE2 */ public static ZoneFilter notEquals(ZoneField field, String value) { return new ZoneFilter(checkNotNull(field), ComparisonOperator.NE, checkNotNull(value)); @@ -504,11 +504,11 @@ class OperationFilter extends ListFilter { } /** - * Returns an equality filter for the given field and string value. For string fields, + * Returns an equals filter for the given field and string value. For string fields, * {@code value} is interpreted as a regular expression using RE2 syntax. {@code value} must * match the entire field. * - * @see RE2 + * @see RE2 */ public static OperationFilter equals(OperationField field, String value) { return new OperationFilter(checkNotNull(field), ComparisonOperator.EQ, checkNotNull(value)); @@ -519,14 +519,14 @@ public static OperationFilter equals(OperationField field, String value) { * {@code value} is interpreted as a regular expression using RE2 syntax. {@code value} must * match the entire field. * - * @see RE2 + * @see RE2 */ public static OperationFilter notEquals(OperationField field, String value) { return new OperationFilter(checkNotNull(field), ComparisonOperator.NE, checkNotNull(value)); } /** - * Returns an equality filter for the given field and long value. + * Returns an equals filter for the given field and long value. */ public static OperationFilter equals(OperationField field, long value) { return new OperationFilter(checkNotNull(field), ComparisonOperator.EQ, value); @@ -538,20 +538,6 @@ public static OperationFilter equals(OperationField field, long value) { public static OperationFilter notEquals(OperationField field, long value) { return new OperationFilter(checkNotNull(field), ComparisonOperator.NE, value); } - - /** - * Returns an equality filter for the given field and integer value. - */ - public static OperationFilter equals(OperationField field, int value) { - return new OperationFilter(checkNotNull(field), ComparisonOperator.EQ, value); - } - - /** - * Returns a not-equals filter for the given field and integer value. - */ - public static OperationFilter notEquals(OperationField field, int value) { - return new OperationFilter(checkNotNull(field), ComparisonOperator.NE, value); - } } /** @@ -595,7 +581,7 @@ public static DiskTypeListOption filter(DiskTypeFilter filter) { } /** - * Returns an option to specify the maximum number of disk types to be returned. + * Returns an option to specify the maximum number of disk types returned per page. */ public static DiskTypeListOption pageSize(long pageSize) { return new DiskTypeListOption(ComputeRpc.Option.MAX_RESULTS, pageSize); @@ -640,7 +626,7 @@ public static DiskTypeAggregatedListOption filter(DiskTypeFilter filter) { } /** - * Returns an option to specify the maximum number of disk types to be returned. + * Returns an option to specify the maximum number of disk types returned per page. */ public static DiskTypeAggregatedListOption pageSize(long pageSize) { return new DiskTypeAggregatedListOption(ComputeRpc.Option.MAX_RESULTS, pageSize); @@ -695,7 +681,7 @@ public static MachineTypeListOption filter(MachineTypeFilter filter) { } /** - * Returns an option to specify the maximum number of machine types to be returned. + * Returns an option to specify the maximum number of machine types returned per page. */ public static MachineTypeListOption pageSize(long pageSize) { return new MachineTypeListOption(ComputeRpc.Option.MAX_RESULTS, pageSize); @@ -740,7 +726,7 @@ public static MachineTypeAggregatedListOption filter(MachineTypeFilter filter) { } /** - * Returns an option to specify the maximum number of machine types to be returned. + * Returns an option to specify the maximum number of machine types returned per page. */ public static MachineTypeAggregatedListOption pageSize(long pageSize) { return new MachineTypeAggregatedListOption(ComputeRpc.Option.MAX_RESULTS, pageSize); @@ -795,7 +781,7 @@ public static RegionListOption filter(RegionFilter filter) { } /** - * Returns an option to specify the maximum number of regions to be returned. + * Returns an option to specify the maximum number of regions returned per page. */ public static RegionListOption pageSize(long pageSize) { return new RegionListOption(ComputeRpc.Option.MAX_RESULTS, pageSize); @@ -862,7 +848,7 @@ public static ZoneListOption filter(ZoneFilter filter) { } /** - * Returns an option to specify the maximum number of zones to be returned. + * Returns an option to specify the maximum number of zones returned per page. */ public static ZoneListOption pageSize(long pageSize) { return new ZoneListOption(ComputeRpc.Option.MAX_RESULTS, pageSize); @@ -951,7 +937,7 @@ public static OperationListOption filter(OperationFilter filter) { } /** - * Returns an option to specify the maximum number of operations to be returned. + * Returns an option to specify the maximum number of operations returned per page. */ public static OperationListOption pageSize(long pageSize) { return new OperationListOption(ComputeRpc.Option.MAX_RESULTS, pageSize); @@ -1090,14 +1076,16 @@ public static OperationListOption fields(OperationField... fields) { Page listGlobalOperations(OperationListOption... options); /** - * Lists the operations in the provided region. + * Lists the operations for the provided region. These are operations that create/modify/delete + * resources that live in a region (e.g. subnetworks). * * @throws ComputeException upon failure */ Page listRegionOperations(String region, OperationListOption... options); /** - * Lists the operations in the provided zone. + * Lists the operations for the provided zone. These are operations that create/modify/delete + * resources that live in a zone (e.g. instances). * * @throws ComputeException upon failure */ diff --git a/gcloud-java-compute/src/main/java/com/google/gcloud/compute/ComputeImpl.java b/gcloud-java-compute/src/main/java/com/google/gcloud/compute/ComputeImpl.java index 453a0f5a8d53..cc537c38c2fc 100644 --- a/gcloud-java-compute/src/main/java/com/google/gcloud/compute/ComputeImpl.java +++ b/gcloud-java-compute/src/main/java/com/google/gcloud/compute/ComputeImpl.java @@ -533,16 +533,17 @@ public Operation get(final OperationId operationId, OperationOption... options) runWithRetries(new Callable() { @Override public com.google.api.services.compute.model.Operation call() { - if (operationId instanceof RegionOperationId) { - RegionOperationId regionOperationId = (RegionOperationId) operationId; - return computeRpc.getRegionOperation(regionOperationId.region(), - regionOperationId.operation(), optionsMap); - } else if (operationId instanceof ZoneOperationId) { - ZoneOperationId zoneOperationId = (ZoneOperationId) operationId; - return computeRpc.getZoneOperation(zoneOperationId.zone(), - zoneOperationId.operation(), optionsMap); - } else { - return computeRpc.getGlobalOperation(operationId.operation(), optionsMap); + switch (operationId.type()) { + case REGION: + RegionOperationId regionOperationId = (RegionOperationId) operationId; + return computeRpc.getRegionOperation(regionOperationId.region(), + regionOperationId.operation(), optionsMap); + case ZONE: + ZoneOperationId zoneOperationId = (ZoneOperationId) operationId; + return computeRpc.getZoneOperation(zoneOperationId.zone(), + zoneOperationId.operation(), optionsMap); + default: + return computeRpc.getGlobalOperation(operationId.operation(), optionsMap); } } }, options().retryParams(), EXCEPTION_HANDLER); diff --git a/gcloud-java-compute/src/main/java/com/google/gcloud/compute/GlobalOperationId.java b/gcloud-java-compute/src/main/java/com/google/gcloud/compute/GlobalOperationId.java index 3c1601e8c4d0..c12e24c903cc 100644 --- a/gcloud-java-compute/src/main/java/com/google/gcloud/compute/GlobalOperationId.java +++ b/gcloud-java-compute/src/main/java/com/google/gcloud/compute/GlobalOperationId.java @@ -40,6 +40,11 @@ private GlobalOperationId(String project, String operation) { this.operation = checkNotNull(operation); } + @Override + public Type type() { + return Type.GLOBAL; + } + @Override public String operation() { return operation; diff --git a/gcloud-java-compute/src/main/java/com/google/gcloud/compute/Operation.java b/gcloud-java-compute/src/main/java/com/google/gcloud/compute/Operation.java index 6734de60804b..d857f90bc84f 100644 --- a/gcloud-java-compute/src/main/java/com/google/gcloud/compute/Operation.java +++ b/gcloud-java-compute/src/main/java/com/google/gcloud/compute/Operation.java @@ -17,6 +17,8 @@ package com.google.gcloud.compute; import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.gcloud.compute.OperationId.Type.REGION; +import static com.google.gcloud.compute.OperationId.Type.ZONE; import com.google.common.base.Function; import com.google.common.base.MoreObjects; @@ -43,7 +45,7 @@ * To get an {@code Operation} object with the most recent information, use * {@link #reload(Compute.OperationOption...)}. */ -public final class Operation implements Serializable { +public class Operation implements Serializable { private static final long serialVersionUID = -8979001444590023899L; private static final DateTimeFormatter TIMESTAMP_FORMATTER = ISODateTimeFormat.dateTime(); @@ -124,7 +126,7 @@ public String code() { } /** - * Returns the field in the request which caused the error. Might be {@code null}. + * Returns the field in the request which caused the error. This value is optional. */ public String location() { return location; @@ -213,7 +215,8 @@ public com.google.api.services.compute.model.Operation.Warnings apply( } /** - * Returns a warning identifier for this warning. + * Returns a warning identifier for this warning. For example, {@code NO_RESULTS_ON_PAGE} if + * there are no results in the response. */ public String code() { return code; @@ -227,7 +230,12 @@ public String message() { } /** - * Returns metadata about this warning. + * Returns metadata about this warning. Each key provides more detail on the warning being + * returned. For example, for warnings where there are no results in a list request for a + * particular zone, this key might be {@code scope} and the key's value might be the zone name. + * Other examples might be a key indicating a deprecated resource, and a suggested replacement, + * or a warning about invalid network settings (for example, if an instance attempts to perform + * IP forwarding but is not enabled for IP forwarding). */ public Map metadata() { return metadata; @@ -587,13 +595,15 @@ public Long insertTime() { /** * Returns the time that this operation was started by the service. In milliseconds since epoch. + * This value will be {@code null} if the operation has not started yet. */ public Long startTime() { return startTime; } /** - * Returns the time that this operation was completed. In milliseconds since epoch. + * Returns the time that this operation was completed. In milliseconds since epoch. This value + * will be {@code null} if the operation has not finished yet. */ public Long endTime() { return endTime; @@ -717,12 +727,12 @@ public String toString() { } @Override - public int hashCode() { + public final int hashCode() { return Objects.hash(id); } @Override - public boolean equals(Object obj) { + public final boolean equals(Object obj) { return obj instanceof Operation && Objects.equals(toPb(), ((Operation) obj).toPb()) && Objects.equals(options, ((Operation) obj).options); @@ -739,11 +749,13 @@ com.google.api.services.compute.model.Operation toPb() { } operationPb.setName(operationId.operation()); operationPb.setClientOperationId(clientOperationId); - if (operationId instanceof RegionOperationId) { - operationPb.setRegion(this.operationId().regionId().selfLink()); - } - if (operationId instanceof ZoneOperationId) { - operationPb.setZone(this.operationId().zoneId().selfLink()); + switch (operationId.type()) { + case REGION: + operationPb.setRegion(this.operationId().regionId().selfLink()); + break; + case ZONE: + operationPb.setZone(this.operationId().zoneId().selfLink()); + break; } if (operationType != null) { operationPb.setOperationType(operationType); diff --git a/gcloud-java-compute/src/main/java/com/google/gcloud/compute/OperationId.java b/gcloud-java-compute/src/main/java/com/google/gcloud/compute/OperationId.java index eaaf7dee0ca4..7f9100aa61a2 100644 --- a/gcloud-java-compute/src/main/java/com/google/gcloud/compute/OperationId.java +++ b/gcloud-java-compute/src/main/java/com/google/gcloud/compute/OperationId.java @@ -21,6 +21,32 @@ */ public interface OperationId { + /** + * Possible types for a Google Compute Engine operation identity. + */ + enum Type { + /** + * Global operations are those operations that deal with global resources, such as global + * addresses or snapshots. + */ + GLOBAL, + /** + * Region operations are those operations that deal with resources that live in a region, such + * as subnetworks. + */ + REGION, + /** + * Zone operations are those operations that deal with resources that live in a zone, such as + * disks and instances. + */ + ZONE + } + + /** + * Returns the type of this operation identity. + */ + Type type(); + /** * Returns the name of the project. */ diff --git a/gcloud-java-compute/src/main/java/com/google/gcloud/compute/RegionOperationId.java b/gcloud-java-compute/src/main/java/com/google/gcloud/compute/RegionOperationId.java index e5c70bf23876..96a772b5b9ea 100644 --- a/gcloud-java-compute/src/main/java/com/google/gcloud/compute/RegionOperationId.java +++ b/gcloud-java-compute/src/main/java/com/google/gcloud/compute/RegionOperationId.java @@ -40,6 +40,11 @@ private RegionOperationId(String project, String region, String operation) { this.operation = checkNotNull(operation); } + @Override + public Type type() { + return Type.REGION; + } + @Override public String operation() { return operation; diff --git a/gcloud-java-compute/src/main/java/com/google/gcloud/compute/ZoneOperationId.java b/gcloud-java-compute/src/main/java/com/google/gcloud/compute/ZoneOperationId.java index c0364b0ead3f..837ca6888c83 100644 --- a/gcloud-java-compute/src/main/java/com/google/gcloud/compute/ZoneOperationId.java +++ b/gcloud-java-compute/src/main/java/com/google/gcloud/compute/ZoneOperationId.java @@ -40,6 +40,11 @@ private ZoneOperationId(String project, String zone, String operation) { this.operation = checkNotNull(operation); } + @Override + public Type type() { + return Type.ZONE; + } + @Override public String operation() { return operation; diff --git a/gcloud-java-compute/src/main/java/com/google/gcloud/spi/DefaultComputeRpc.java b/gcloud-java-compute/src/main/java/com/google/gcloud/spi/DefaultComputeRpc.java index d54840e3fff0..3209084a5983 100644 --- a/gcloud-java-compute/src/main/java/com/google/gcloud/spi/DefaultComputeRpc.java +++ b/gcloud-java-compute/src/main/java/com/google/gcloud/spi/DefaultComputeRpc.java @@ -378,7 +378,7 @@ public boolean deleteZoneOperation(String zone, String operation) { private static T nullForNotFound(IOException exception) { ComputeException serviceException = translate(exception); if (serviceException.code() == HTTP_NOT_FOUND) { - return (T) null; + return null; } throw serviceException; } diff --git a/gcloud-java-compute/src/test/java/com/google/gcloud/compute/OperationIdTest.java b/gcloud-java-compute/src/test/java/com/google/gcloud/compute/OperationIdTest.java index 36ca5ef19090..76a53cdd1cd2 100644 --- a/gcloud-java-compute/src/test/java/com/google/gcloud/compute/OperationIdTest.java +++ b/gcloud-java-compute/src/test/java/com/google/gcloud/compute/OperationIdTest.java @@ -45,35 +45,43 @@ public class OperationIdTest { @Test public void testOf() { GlobalOperationId operationId = GlobalOperationId.of(PROJECT, NAME); + assertEquals(OperationId.Type.GLOBAL, operationId.type()); assertEquals(PROJECT, operationId.project()); assertEquals(NAME, operationId.operation()); assertEquals(GLOBAL_URL, operationId.selfLink()); operationId = GlobalOperationId.of(NAME); + assertEquals(OperationId.Type.GLOBAL, operationId.type()); assertNull(operationId.project()); assertEquals(NAME, operationId.operation()); ZoneOperationId zoneOperationId = ZoneOperationId.of(PROJECT, ZONE, NAME); + assertEquals(OperationId.Type.ZONE, zoneOperationId.type()); assertEquals(PROJECT, zoneOperationId.project()); assertEquals(ZONE, zoneOperationId.zone()); assertEquals(NAME, zoneOperationId.operation()); assertEquals(ZONE_URL, zoneOperationId.selfLink()); zoneOperationId = ZoneOperationId.of(ZONE, NAME); + assertEquals(OperationId.Type.ZONE, zoneOperationId.type()); assertNull(zoneOperationId.project()); assertEquals(ZONE, zoneOperationId.zone()); assertEquals(NAME, zoneOperationId.operation()); zoneOperationId = ZoneOperationId.of(ZoneId.of(PROJECT, ZONE), NAME); + assertEquals(OperationId.Type.ZONE, zoneOperationId.type()); assertEquals(PROJECT, zoneOperationId.project()); assertEquals(ZONE, zoneOperationId.zone()); assertEquals(NAME, zoneOperationId.operation()); RegionOperationId regionOperationId = RegionOperationId.of(PROJECT, REGION, NAME); + assertEquals(OperationId.Type.REGION, regionOperationId.type()); assertEquals(PROJECT, regionOperationId.project()); assertEquals(REGION, regionOperationId.region()); assertEquals(NAME, regionOperationId.operation()); assertEquals(REGION_URL, regionOperationId.selfLink()); regionOperationId = RegionOperationId.of(REGION, NAME); + assertEquals(OperationId.Type.REGION, regionOperationId.type()); assertNull(regionOperationId.project()); assertEquals(REGION, regionOperationId.region()); assertEquals(NAME, regionOperationId.operation()); regionOperationId = RegionOperationId.of(RegionId.of(PROJECT, REGION), NAME); + assertEquals(OperationId.Type.REGION, regionOperationId.type()); assertEquals(PROJECT, regionOperationId.project()); assertEquals(REGION, regionOperationId.region()); assertEquals(NAME, regionOperationId.operation()); @@ -151,6 +159,7 @@ private void compareZoneOperationId(ZoneOperationId expected, ZoneOperationId va private void compareRegionOperationId(RegionOperationId expected, RegionOperationId value) { assertEquals(expected, value); + assertEquals(expected.type(), value.type()); assertEquals(expected.project(), expected.project()); assertEquals(expected.region(), expected.region()); assertEquals(expected.operation(), expected.operation()); diff --git a/gcloud-java-compute/src/test/java/com/google/gcloud/compute/it/ITComputeTest.java b/gcloud-java-compute/src/test/java/com/google/gcloud/compute/it/ITComputeTest.java index da446821a088..56960903d6c7 100644 --- a/gcloud-java-compute/src/test/java/com/google/gcloud/compute/it/ITComputeTest.java +++ b/gcloud-java-compute/src/test/java/com/google/gcloud/compute/it/ITComputeTest.java @@ -57,7 +57,7 @@ public class ITComputeTest { public Timeout globalTimeout = Timeout.seconds(300); @BeforeClass - public static void beforeClass() throws InterruptedException { + public static void beforeClass() { RemoteComputeHelper computeHelper = RemoteComputeHelper.create(); compute = computeHelper.options().service(); }