Standardize REST endpoints for getting changes

The /detail and /review endpoints are practically identical to the
standard change endpoint except for the default set of options. Make
this equivalence explicit in the code by delegating to a GetChange
instance from the other handlers.

This change naturally gets caching for the other endpoints as well.

Allow specifying options on /change/X; prior to this, users could add
options to the default set from /detail but could not remove options.

Change-Id: I03bb96141e79ae00359f8d5ec51413d1b27204dc

Documentation/rest-api-changes.txt

@@ -336,6 +336,11 @@ Get Change
 
 Retrieves a change.
 
+Additional fields can be obtained by adding `o` parameters, each
+option requires more database lookups and slows down the query
+response time to the client so they are generally disabled by
+default. Fields are described in link:#list-changes[Query Changes].
+
 .Request
 ----
   GET /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940 HTTP/1.0

gerrit-server/src/main/java/com/google/gerrit/server/change/GetChange.java

@@ -14,13 +14,30 @@
 
 package com.google.gerrit.server.change;
 
+import com.google.gerrit.common.changes.ListChangesOption;
+import com.google.gerrit.extensions.restapi.CacheControl;
+import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 
+import org.kohsuke.args4j.Option;
+
+import java.util.concurrent.TimeUnit;
+
 public class GetChange implements RestReadView<ChangeResource> {
   private final ChangeJson json;
 
+  @Option(name = "-o", multiValued = true, usage = "Output options")
+  void addOption(ListChangesOption o) {
+    json.addOption(o);
+  }
+
+  @Option(name = "-O", usage = "Output option flags, in hex")
+  void setOptionFlagsHex(String hex) {
+    json.addOptions(ListChangesOption.fromBits(Integer.parseInt(hex, 16)));
+  }
+
   @Inject
   GetChange(ChangeJson json) {
     this.json = json;
@@ -28,6 +45,15 @@ public class GetChange implements RestReadView<ChangeResource> {
 
   @Override
   public Object apply(ChangeResource rsrc) throws OrmException {
-    return json.format(rsrc);
+    return cache(json.format(rsrc));
+  }
+
+  Object apply(RevisionResource rsrc) throws OrmException {
+    return cache(json.format(rsrc));
+  }
+
+  private Object cache(Object res) {
+    return Response.ok(res)
+        .caching(CacheControl.PRIVATE(0, TimeUnit.SECONDS).setMustRevalidate());
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/change/GetDetail.java

@@ -15,41 +15,36 @@
 package com.google.gerrit.server.change;
 
 import com.google.gerrit.common.changes.ListChangesOption;
-import com.google.gerrit.extensions.restapi.CacheControl;
-import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 
 import org.kohsuke.args4j.Option;
 
-import java.util.concurrent.TimeUnit;
-
 public class GetDetail implements RestReadView<ChangeResource> {
-  private final ChangeJson json;
+  private final GetChange delegate;
 
   @Option(name = "-o", multiValued = true, usage = "Output options")
   void addOption(ListChangesOption o) {
-    json.addOption(o);
+    delegate.addOption(o);
   }
 
   @Option(name = "-O", usage = "Output option flags, in hex")
   void setOptionFlagsHex(String hex) {
-    json.addOptions(ListChangesOption.fromBits(Integer.parseInt(hex, 16)));
+    delegate.setOptionFlagsHex(hex);
   }
 
   @Inject
-  GetDetail(ChangeJson json) {
+  GetDetail(GetChange delegate) {
-    this.json = json
+    this.delegate = delegate;
-        .addOption(ListChangesOption.LABELS)
+    delegate.addOption(ListChangesOption.LABELS);
-        .addOption(ListChangesOption.DETAILED_LABELS)
+    delegate.addOption(ListChangesOption.DETAILED_LABELS);
-        .addOption(ListChangesOption.DETAILED_ACCOUNTS)
+    delegate.addOption(ListChangesOption.DETAILED_ACCOUNTS);
-        .addOption(ListChangesOption.MESSAGES);
+    delegate.addOption(ListChangesOption.MESSAGES);
   }
 
   @Override
   public Object apply(ChangeResource rsrc) throws OrmException {
-    return Response.ok(json.format(rsrc))
+    return delegate.apply(rsrc);
-        .caching(CacheControl.PRIVATE(0, TimeUnit.SECONDS).setMustRevalidate());
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/change/GetReview.java

@@ -20,17 +20,17 @@ import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 
 public class GetReview implements RestReadView<RevisionResource> {
-  private final ChangeJson json;
+  private final GetChange delegate;
 
   @Inject
-  GetReview(ChangeJson json) {
+  GetReview(GetChange delegate) {
-    this.json = json
+    this.delegate = delegate;
-        .addOption(ListChangesOption.DETAILED_LABELS)
+    delegate.addOption(ListChangesOption.DETAILED_LABELS);
-        .addOption(ListChangesOption.DETAILED_ACCOUNTS);
+    delegate.addOption(ListChangesOption.DETAILED_ACCOUNTS);
   }
 
   @Override
-  public Object apply(RevisionResource resource) throws OrmException {
+  public Object apply(RevisionResource rsrc) throws OrmException {
-    return json.format(resource);
+    return delegate.apply(rsrc);
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/change/RevisionResource.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.server.change;
 
 import com.google.gerrit.extensions.restapi.RestResource;
+import com.google.gerrit.extensions.restapi.RestResource.HasETag;
 import com.google.gerrit.extensions.restapi.RestView;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
@@ -23,7 +24,7 @@ import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.inject.TypeLiteral;
 
-public class RevisionResource implements RestResource {
+public class RevisionResource implements RestResource, HasETag {
   public static final TypeLiteral<RestView<RevisionResource>> REVISION_KIND =
       new TypeLiteral<RestView<RevisionResource>>() {};
 
@@ -56,6 +57,14 @@ public class RevisionResource implements RestResource {
     return ps;
   }
 
+  @Override
+  public String getETag() {
+    // Conservative estimate: refresh the revision if its parent change has
+    // changed, so we don't have to check whether a given modification affected
+    // this revision specifically.
+    return change.getETag();
+  }
+
   Account.Id getAccountId() {
     return getUser().getAccountId();
   }