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
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 66abebf..0ec5cd8 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -336,6 +336,11 @@
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
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetChange.java
index 4db5459..7213a94 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetChange.java
+++ b/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 @@
@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());
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDetail.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDetail.java
index 2375fb0..936edd6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDetail.java
+++ b/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) {
- this.json = json
- .addOption(ListChangesOption.LABELS)
- .addOption(ListChangesOption.DETAILED_LABELS)
- .addOption(ListChangesOption.DETAILED_ACCOUNTS)
- .addOption(ListChangesOption.MESSAGES);
+ GetDetail(GetChange delegate) {
+ this.delegate = delegate;
+ delegate.addOption(ListChangesOption.LABELS);
+ delegate.addOption(ListChangesOption.DETAILED_LABELS);
+ delegate.addOption(ListChangesOption.DETAILED_ACCOUNTS);
+ delegate.addOption(ListChangesOption.MESSAGES);
}
@Override
public Object apply(ChangeResource rsrc) throws OrmException {
- return Response.ok(json.format(rsrc))
- .caching(CacheControl.PRIVATE(0, TimeUnit.SECONDS).setMustRevalidate());
+ return delegate.apply(rsrc);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetReview.java
index 08186fe..3676a1e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetReview.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetReview.java
@@ -20,17 +20,17 @@
import com.google.inject.Inject;
public class GetReview implements RestReadView<RevisionResource> {
- private final ChangeJson json;
+ private final GetChange delegate;
@Inject
- GetReview(ChangeJson json) {
- this.json = json
- .addOption(ListChangesOption.DETAILED_LABELS)
- .addOption(ListChangesOption.DETAILED_ACCOUNTS);
+ GetReview(GetChange delegate) {
+ this.delegate = delegate;
+ delegate.addOption(ListChangesOption.DETAILED_LABELS);
+ delegate.addOption(ListChangesOption.DETAILED_ACCOUNTS);
}
@Override
- public Object apply(RevisionResource resource) throws OrmException {
- return json.format(resource);
+ public Object apply(RevisionResource rsrc) throws OrmException {
+ return delegate.apply(rsrc);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/RevisionResource.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/RevisionResource.java
index f891729..b15719f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/RevisionResource.java
+++ b/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.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 @@
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();
}
