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

Author: mziccard

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 <a href="https://github.com/google/re2">RE2</a>
+     * @see <a href="https://github.com/google/re2/wiki/Syntax">RE2</a>
      */
     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 <a href="https://github.com/google/re2">RE2</a>
+     * @see <a href="https://github.com/google/re2/wiki/Syntax">RE2</a>
      */
     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 <a href="https://github.com/google/re2">RE2</a>
+     * @see <a href="https://github.com/google/re2/wiki/Syntax">RE2</a>
      */
     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 <a href="https://github.com/google/re2">RE2</a>
+     * @see <a href="https://github.com/google/re2/wiki/Syntax">RE2</a>
      */
     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 <a href="https://github.com/google/re2">RE2</a>
+     * @see <a href="https://github.com/google/re2/wiki/Syntax">RE2</a>
      */
     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 <a href="https://github.com/google/re2">RE2</a>
+     * @see <a href="https://github.com/google/re2/wiki/Syntax">RE2</a>
      */
     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 <a href="https://github.com/google/re2">RE2</a>
+     * @see <a href="https://github.com/google/re2/wiki/Syntax">RE2</a>
      */
     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 <a href="https://github.com/google/re2">RE2</a>
+     * @see <a href="https://github.com/google/re2/wiki/Syntax">RE2</a>
      */
     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 <a href="https://github.com/google/re2">RE2</a>
+     * @see <a href="https://github.com/google/re2/wiki/Syntax">RE2</a>
      */
     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 <a href="https://github.com/google/re2">RE2</a>
+     * @see <a href="https://github.com/google/re2/wiki/Syntax">RE2</a>
      */
     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<Operation> 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<Operation> 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
    */

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<com.google.api.services.compute.model.Operation>() {
             @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);

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;

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<String, String> 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.<RegionOperationId>operationId().regionId().selfLink());
-    }
-    if (operationId instanceof ZoneOperationId) {
-      operationPb.setZone(this.<ZoneOperationId>operationId().zoneId().selfLink());
+    switch (operationId.type()) {
+      case REGION:
+        operationPb.setRegion(this.<RegionOperationId>operationId().regionId().selfLink());
+        break;
+      case ZONE:
+        operationPb.setZone(this.<ZoneOperationId>operationId().zoneId().selfLink());
+        break;
     }
     if (operationType != null) {
       operationPb.setOperationType(operationType);