Addressed comments. This includes:

- Moved Status enum to ChangeRequestInfo.
- Switching from ChangeRequest.builer() to ChangeRequestInfo.builder()
- Fixing README accordingly
- Used times() for mocking instead of repeated expect()
- Snippets and documentation work with ChangeRequestInfo. Fixes googleapis#788.
- Equals uses getClass instead if instanceof.
- Removed unnecessary imports.

README.md

@@ -252,7 +252,7 @@ Zone zone = dns.create(zoneInfo);
 The second snippet shows how to create records inside a zone. The complete code can be found on [CreateOrUpdateRecordSets.java](./gcloud-java-examples/src/main/java/com/google/gcloud/examples/dns/snippets/CreateOrUpdateRecordSets.java).
 
 ```java
-import com.google.gcloud.dns.ChangeRequest;
+import com.google.gcloud.dns.ChangeRequestInfo;
 import com.google.gcloud.dns.Dns;
 import com.google.gcloud.dns.DnsOptions;
 import com.google.gcloud.dns.RecordSet;
@@ -269,7 +269,7 @@ RecordSet toCreate = RecordSet.builder("www.someexampledomain.com.", RecordSet.T
     .ttl(24, TimeUnit.HOURS)
     .addRecord(ip)
     .build();
-ChangeRequest.Builder changeBuilder = ChangeRequest.builder().add(toCreate);
+ChangeRequestInfo.Builder changeBuilder = ChangeRequestInfo.builder().add(toCreate);
 
 // Verify that the record does not exist yet.
 // If it does exist, we will overwrite it with our prepared record.
@@ -282,7 +282,7 @@ while (recordSetIterator.hasNext()) {
   }
 }
 
-ChangeRequest changeRequest = changeBuilder.build();
+ChangeRequestInfo changeRequest = changeBuilder.build();
 zone.applyChangeRequest(changeRequest);
 ```
 

gcloud-java-dns/README.md

@@ -159,7 +159,7 @@ our zone that creates a record set of type A and points URL www.someexampledomai
 IP address 12.13.14.15. Start by adding
 
 ```java
-import com.google.gcloud.dns.ChangeRequest;
+import com.google.gcloud.dns.ChangeRequestInfo;
 import com.google.gcloud.dns.RecordSet;
 
 import java.util.concurrent.TimeUnit;
@@ -176,7 +176,7 @@ RecordSet toCreate = RecordSet.builder("www." + zone.dnsName(), RecordSet.Type.A
     .build();
 
 // Make a change
-ChangeRequest changeRequest = ChangeRequest.builder().add(toCreate).build();
+ChangeRequestInfo changeRequest = ChangeRequestInfo.builder().add(toCreate).build();
 
 // Build and apply the change request to our zone
 changeRequest = zone.applyChangeRequest(changeRequest);
@@ -198,7 +198,7 @@ and in the code
 
 ```java
 // Make a change
-ChangeRequest.Builder changeBuilder = ChangeRequest.builder().add(toCreate);
+ChangeRequestInfo.Builder changeBuilder = ChangeRequestInfo.builder().add(toCreate);
 
 // Verify the type A record does not exist yet.
 // If it does exist, we will overwrite it with our prepared record.
@@ -211,7 +211,7 @@ while (recordSetIterator.hasNext()) {
 }
 
 // Build and apply the change request to our zone
-ChangeRequest changeRequest = changeBuilder.build();
+ChangeRequestInfo changeRequest = changeBuilder.build();
 zone.applyChangeRequest(changeRequest);
 ```
 You can find more information about changes in the [Cloud DNS documentation] (https://cloud.google.com/dns/what-is-cloud-dns#cloud_dns_api_concepts).
@@ -220,7 +220,7 @@ When the change request is applied, it is registered with the Cloud DNS service
 can wait for its completion as follows:
 
 ```java
-while (ChangeRequest.Status.PENDING.equals(changeRequest.status())) {
+while (ChangeRequestInfo.Status.PENDING.equals(changeRequest.status())) {
   try {
     Thread.sleep(500L);
   } catch (InterruptedException e) {
@@ -262,9 +262,17 @@ while (recordSetIterator.hasNext()) {
 }
 ```
 
-You can also list the history of change requests that were applied to a zone:
+You can also list the history of change requests that were applied to a zone.
+First add:
 
 ```java
+import java.util.ChangeRequest;
+```
+
+and then:
+
+```java
+
 // List the change requests applied to a particular zone
 Iterator<ChangeRequest> changeIterator = zone.listChangeRequests().iterateAll();
 System.out.println(String.format("The history of changes in %s:", zone.name()));
@@ -280,7 +288,7 @@ First, you need to empty the zone by deleting all its records except for the def
 
 ```java
 // Make a change for deleting the record sets
-changeBuilder = ChangeRequest.builder();
+changeBuilder = ChangeRequestInfo.builder();
 while (recordIterator.hasNext()) {
   RecordSet current = recordIterator.next();
   // SOA and NS records cannot be deleted
@@ -290,14 +298,14 @@ while (recordIterator.hasNext()) {
 }
 
 // Build and apply the change request to our zone if it contains records to delete
-ChangeRequest changeRequest = changeBuilder.build();
+ChangeRequestInfo changeRequest = changeBuilder.build();
 if (!changeRequest.deletions().isEmpty()) {
   changeRequest = dns.applyChangeRequest(zoneName, changeRequest);
 
   // Wait for change to finish, but save data traffic by transferring only ID and status
   Dns.ChangeRequestOption option =
       Dns.ChangeRequestOption.fields(Dns.ChangeRequestField.STATUS);
-  while (ChangeRequest.Status.PENDING.equals(changeRequest.status())) {
+  while (ChangeRequestInfo.Status.PENDING.equals(changeRequest.status())) {
     System.out.println("Waiting for change to complete. Going to sleep for 500ms...");
     try {
       Thread.sleep(500);

gcloud-java-dns/src/main/java/com/google/gcloud/dns/ChangeRequest.java

@@ -16,6 +16,8 @@
 
 package com.google.gcloud.dns;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
 import com.google.api.services.dns.model.Change;
 import com.google.common.base.Function;
 
@@ -25,41 +27,30 @@
 import java.util.Objects;
 
 /**
- * A class representing an atomic update to a collection of {@link RecordSet}s within a {@code
- * Zone}.
+ * An immutable class representing an atomic update to a collection of {@link RecordSet}s within a
+ * {@code Zone}.
  *
  * @see <a href="https://cloud.google.com/dns/api/v1/changes">Google Cloud DNS documentation</a>
  */
 public class ChangeRequest extends ChangeRequestInfo {
 
-  private static final long serialVersionUID = -9027378042756366333L;
+  private static final long serialVersionUID = 5335667200595081449L;
   private final DnsOptions options;
-  private final String zoneName;
+  private final String zone;
   private transient Dns dns;
 
-  /**
-   * This enumerates the possible states of a {@code ChangeRequest}.
-   *
-   * @see <a href="https://cloud.google.com/dns/api/v1/changes#resource">Google Cloud DNS
-   * documentation</a>
-   */
-  public enum Status {
-    PENDING,
-    DONE
-  }
-
   /**
    * A builder for {@code ChangeRequest}s.
    */
   public static class Builder extends ChangeRequestInfo.Builder {
 
     private final Dns dns;
-    private final String zoneName;
+    private final String zone;
     private final ChangeRequestInfo.BuilderImpl infoBuilder;
 
     private Builder(ChangeRequest cr) {
       this.dns = cr.dns;
-      this.zoneName = cr.zoneName;
+      this.zone = cr.zone;
       this.infoBuilder = new ChangeRequestInfo.BuilderImpl(cr);
     }
 
@@ -131,45 +122,36 @@ Builder status(Status status) {
 
     @Override
     public ChangeRequest build() {
-      return new ChangeRequest(dns, zoneName, infoBuilder);
+      return new ChangeRequest(dns, zone, infoBuilder);
     }
   }
 
-  ChangeRequest(Dns dns, String zoneName, ChangeRequest.BuilderImpl infoBuilder) {
+  ChangeRequest(Dns dns, String zone, ChangeRequest.BuilderImpl infoBuilder) {
     super(infoBuilder);
-    this.zoneName = zoneName;
-    this.dns = dns;
+    this.zone = checkNotNull(zone);
+    this.dns = checkNotNull(dns);
     this.options = dns.options();
   }
 
-  static Function<Change, ChangeRequest> fromPbFunction(final Dns dns, final String zoneName) {
-    return new Function<Change, ChangeRequest>() {
-      @Override
-      public ChangeRequest apply(com.google.api.services.dns.model.Change pb) {
-        return ChangeRequest.fromPb(dns, zoneName, pb);
-      }
-    };
-  }
-
   /**
    * Returns the name of the {@link Zone} associated with this change request.
    */
-  public String zoneName() {
-    return this.zoneName;
+  public String zone() {
+    return this.zone;
   }
 
   /**
-   * Returns the {@link Dns} service object associated with this change request.
+   * Returns the change request's {@code Dns} object used to issue requests.
    */
   public Dns dns() {
     return dns;
   }
 
   /**
-   * Applies this change request to a zone that it is associated with.
+   * Applies this change request to the associated zone.
    */
   public ChangeRequest applyTo(Dns.ChangeRequestOption... options) {
-    return dns.applyChangeRequest(zoneName, this, options);
+    return dns.applyChangeRequest(zone, this, options);
   }
 
   @Override
@@ -179,24 +161,37 @@ public Builder toBuilder() {
 
   @Override
   public boolean equals(Object obj) {
-    return obj instanceof ChangeRequest && Objects.equals(toPb(), ((ChangeRequest) obj).toPb())
-        && Objects.equals(options, ((ChangeRequest) obj).options)
-        && Objects.equals(zoneName, ((ChangeRequest) obj).zoneName);
+    if (obj == null || !obj.getClass().equals(ChangeRequest.class)) {
+      return false;
+    } else {
+      ChangeRequest other = (ChangeRequest) obj;
+      return Objects.equals(options, other.options)
+          && Objects.equals(zone, other.zone)
+          && Objects.equals(toPb(), other.toPb());
+    }
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(super.hashCode(), options, zoneName);
+    return Objects.hash(super.hashCode(), options, zone);
   }
 
-  private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException {
-    in.defaultReadObject();
+  private void readObject(ObjectInputStream input) throws IOException, ClassNotFoundException {
+    input.defaultReadObject();
     this.dns = options.service();
   }
 
-  static ChangeRequest fromPb(Dns dns, String zoneName,
-      com.google.api.services.dns.model.Change pb) {
+  static ChangeRequest fromPb(Dns dns, String zoneName, Change pb) {
     ChangeRequestInfo info = ChangeRequestInfo.fromPb(pb);
     return new ChangeRequest(dns, zoneName, new ChangeRequestInfo.BuilderImpl(info));
   }
+
+  static Function<Change, ChangeRequest> fromPbFunction(final Dns dns, final String zoneName) {
+    return new Function<Change, ChangeRequest>() {
+      @Override
+      public ChangeRequest apply(com.google.api.services.dns.model.Change pb) {
+        return ChangeRequest.fromPb(dns, zoneName, pb);
+      }
+    };
+  }
 }

gcloud-java-dns/src/main/java/com/google/gcloud/dns/ChangeRequestInfo.java

@@ -52,7 +52,18 @@ public ChangeRequestInfo apply(Change pb) {
   private final List<RecordSet> deletions;
   private final String id;
   private final Long startTimeMillis;
-  private final ChangeRequest.Status status;
+  private final ChangeRequestInfo.Status status;
+
+  /**
+   * This enumerates the possible states of a change request.
+   *
+   * @see <a href="https://cloud.google.com/dns/api/v1/changes#resource">Google Cloud DNS
+   * documentation</a>
+   */
+  public enum Status {
+    PENDING,
+    DONE
+  }
 
   /**
    * A builder for {@code ChangeRequestInfo}.
@@ -110,7 +121,7 @@ public abstract static class Builder {
     /**
      * Associates a server-assigned id to this {@code ChangeRequestInfo}.
      */
-    abstract  Builder id(String id);
+    abstract Builder id(String id);
 
     /**
      * Sets the time when this change request was started by a server.
@@ -134,7 +145,7 @@ static class BuilderImpl extends Builder {
     private List<RecordSet> deletions;
     private String id;
     private Long startTimeMillis;
-    private ChangeRequest.Status status;
+    private ChangeRequestInfo.Status status;
 
     BuilderImpl() {
       this.additions = new LinkedList<>();
@@ -215,7 +226,7 @@ Builder startTimeMillis(long startTimeMillis) {
     }
 
     @Override
-    Builder status(ChangeRequest.Status status) {
+    Builder status(ChangeRequestInfo.Status status) {
       this.status = checkNotNull(status);
       return this;
     }
@@ -274,15 +285,15 @@ public Long startTimeMillis() {
   }
 
   /**
-   * Returns the status of this {@code ChangeRequest}.
+   * Returns the status of this {@code ChangeRequest}. If the change request has not been applied
+   * yet, the status is {@code PENDING}.
    */
-  public ChangeRequest.Status status() {
+  public ChangeRequestInfo.Status status() {
     return status;
   }
 
-  com.google.api.services.dns.model.Change toPb() {
-    com.google.api.services.dns.model.Change pb =
-        new com.google.api.services.dns.model.Change();
+  Change toPb() {
+    Change pb = new Change();
     // set id
     if (id() != null) {
       pb.setId(id());
@@ -302,7 +313,7 @@ com.google.api.services.dns.model.Change toPb() {
     return pb;
   }
 
-  static ChangeRequestInfo fromPb(com.google.api.services.dns.model.Change pb) {
+  static ChangeRequestInfo fromPb(Change pb) {
     Builder builder = builder();
     if (pb.getId() != null) {
       builder.id(pb.getId());
@@ -325,8 +336,8 @@ static ChangeRequestInfo fromPb(com.google.api.services.dns.model.Change pb) {
 
   @Override
   public boolean equals(Object other) {
-    return (other instanceof ChangeRequestInfo)
-        && toPb().equals(((ChangeRequestInfo) other).toPb());
+    return other != null && other.getClass().equals(ChangeRequestInfo.class)
+        && other instanceof ChangeRequestInfo && toPb().equals(((ChangeRequestInfo) other).toPb());
   }
 
   @Override

gcloud-java-dns/src/main/java/com/google/gcloud/dns/DnsImpl.java

@@ -19,7 +19,6 @@
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.gcloud.RetryHelper.RetryHelperException;
 import static com.google.gcloud.RetryHelper.runWithRetries;
-import static com.google.gcloud.dns.ChangeRequest.fromPb;
 
 import com.google.api.services.dns.model.Change;
 import com.google.api.services.dns.model.ManagedZone;
@@ -170,7 +169,7 @@ public DnsRpc.ListResult<Change> call() {
       Iterable<ChangeRequest> changes = result.results() == null
           ? ImmutableList.<ChangeRequest>of()
           : Iterables.transform(result.results(),
-          ChangeRequest.fromPbFunction(serviceOptions.service(), zoneName));
+              ChangeRequest.fromPbFunction(serviceOptions.service(), zoneName));
       return new PageImpl<>(new ChangeRequestPageFetcher(zoneName, serviceOptions, cursor,
           optionsMap), cursor, changes);
     } catch (RetryHelperException e) {
@@ -285,7 +284,7 @@ public com.google.api.services.dns.model.Change call() {
                   return dnsRpc.applyChangeRequest(zoneName, changeRequest.toPb(), optionsMap);
                 }
               }, options().retryParams(), EXCEPTION_HANDLER);
-      return answer == null ? null : fromPb(this, zoneName, answer); // should never be null
+      return answer == null ? null : ChangeRequest.fromPb(this, zoneName, answer); // not null
     } catch (RetryHelper.RetryHelperException ex) {
       throw DnsException.translateAndThrow(ex);
     }

gcloud-java-dns/src/main/java/com/google/gcloud/dns/Zone.java

@@ -146,7 +146,7 @@ public Page<RecordSet> listRecordSets(Dns.RecordSetListOption... options) {
   }
 
   /**
-   * Submits {@link ChangeRequest} to the service for it to applied to this zone. The method
+   * Submits {@link ChangeRequestInfo} to the service for it to applied to this zone. The method
    * searches for zone by name.
    *
    * @param options optional restriction on what fields of {@link ChangeRequest} should be returned