Merge "GetChange: provide meta=SHA1 option"
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 3b9320c..3a5fb96 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -564,6 +564,12 @@
}
----
+Historical state of the change can be retrieved by specifying the
+`meta=SHA1` parameter. This will use a historical NoteDb snapshot to
+populate ChangeInfo. If the SHA1 is not reachable as a NoteDb state,
+status code 412 is returned.
+
+
[[get-change-detail]]
=== Get Change Detail
--
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index a461b59..c7aedb6 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -47,6 +47,7 @@
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Address;
import com.google.gerrit.entities.Change;
@@ -121,6 +122,7 @@
import java.util.Set;
import java.util.stream.Collectors;
import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.ObjectId;
/**
* Produces {@link ChangeInfo} (which is serialized to JSON afterwards) from {@link ChangeData}.
@@ -283,6 +285,11 @@
return format(changeDataFactory.create(change));
}
+ public ChangeInfo format(Change change, @Nullable ObjectId metaRevId) {
+ ChangeNotes notes = notesFactory.createChecked(change.getProject(), change.getId(), metaRevId);
+ return format(changeDataFactory.create(notes));
+ }
+
public ChangeInfo format(ChangeData cd) {
return format(cd, Optional.empty(), true, getPluginInfos(cd));
}
@@ -325,9 +332,13 @@
}
public ChangeInfo format(Project.NameKey project, Change.Id id) {
+ return format(project, id, null);
+ }
+
+ public ChangeInfo format(Project.NameKey project, Change.Id id, @Nullable ObjectId metaRevId) {
ChangeNotes notes;
try {
- notes = notesFactory.createChecked(project, id);
+ notes = notesFactory.createChecked(project, id, metaRevId);
} catch (StorageException e) {
if (!has(CHECK)) {
throw e;
diff --git a/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java b/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
index 9b5b4d4..a7c7757 100644
--- a/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
@@ -37,6 +37,8 @@
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
/** View of contents at a single ref related to some change. * */
public abstract class AbstractChangeNotes<T> {
@@ -118,9 +120,14 @@
private ObjectId revision;
private boolean loaded;
- protected AbstractChangeNotes(Args args, Change.Id changeId) {
+ protected AbstractChangeNotes(Args args, Change.Id changeId, @Nullable ObjectId metaSha1) {
this.args = requireNonNull(args);
this.changeId = requireNonNull(changeId);
+ this.revision = metaSha1;
+ }
+
+ protected AbstractChangeNotes(Args args, Change.Id changeId) {
+ this(args, changeId, null);
}
public Change.Id getChangeId() {
@@ -144,7 +151,7 @@
Repository repo = args.repoManager.openRepository(getProjectName());
// Call openHandle even if reading is disabled, to trigger
// auto-rebuilding before this object may get passed to a ChangeUpdate.
- LoadHandle handle = openHandle(repo)) {
+ LoadHandle handle = openHandle(repo, revision)) {
revision = handle.id();
onLoad(handle);
loaded = true;
@@ -168,13 +175,17 @@
* @param repo open repository.
* @return handle for reading the entity.
* @throws NoSuchChangeException change does not exist.
+ * @throws MissingMetaObjectException specified SHA1 isn't reachable from meta branch.
* @throws IOException a repo-level error occurred.
*/
- protected LoadHandle openHandle(Repository repo) throws NoSuchChangeException, IOException {
- return openHandle(repo, readRef(repo));
- }
+ protected LoadHandle openHandle(Repository repo, @Nullable ObjectId id)
+ throws NoSuchChangeException, IOException, MissingMetaObjectException {
+ if (id == null) {
+ id = readRef(repo);
+ } else {
+ verifyMetaId(repo, id);
+ }
- protected LoadHandle openHandle(Repository repo, ObjectId id) {
return new LoadHandle(repo, id);
}
@@ -215,4 +226,20 @@
protected final T self() {
return (T) this;
}
+
+ private void verifyMetaId(Repository repo, ObjectId id)
+ throws IOException, MissingMetaObjectException {
+ try (RevWalk rw = new RevWalk(repo)) {
+ Ref ref = repo.getRefDatabase().exactRef(getRefName());
+ RevCommit tip = rw.parseCommit(ref.getObjectId());
+ rw.markStart(tip);
+ for (RevCommit rev : rw) {
+ if (id.equals(rev)) {
+ return;
+ }
+ }
+ }
+
+ throw new MissingMetaObjectException(id.getName() + " not reachable from " + getRefName());
+ }
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java
index cf09ff3..77d7bc0 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -114,9 +114,14 @@
return createChecked(c.getProject(), c.getId());
}
- public ChangeNotes createChecked(Project.NameKey project, Change.Id changeId) {
+ public ChangeNotes createChecked(
+ Project.NameKey project, Change.Id changeId, @Nullable ObjectId metaRevId) {
Change change = newChange(project, changeId);
- return new ChangeNotes(args, change, true, null).load();
+ return new ChangeNotes(args, change, true, null, metaRevId).load();
+ }
+
+ public ChangeNotes createChecked(Project.NameKey project, Change.Id changeId) {
+ return createChecked(project, changeId, null);
}
public static Change newChange(Project.NameKey project, Change.Id changeId) {
@@ -333,14 +338,23 @@
private ImmutableListMultimap<PatchSet.Id, PatchSetApproval> approvals;
private ImmutableSet<Comment.Key> commentKeys;
- @VisibleForTesting
- public ChangeNotes(Args args, Change change, boolean shouldExist, @Nullable RefCache refs) {
- super(args, change.getId());
+ public ChangeNotes(
+ Args args,
+ Change change,
+ boolean shouldExist,
+ @Nullable RefCache refs,
+ @Nullable ObjectId metaSha1) {
+ super(args, change.getId(), metaSha1);
this.change = new Change(change);
this.shouldExist = shouldExist;
this.refs = refs;
}
+ @VisibleForTesting
+ public ChangeNotes(Args args, Change change, boolean shouldExist, @Nullable RefCache refs) {
+ this(args, change, shouldExist, refs, null);
+ }
+
public Change getChange() {
return change;
}
diff --git a/java/com/google/gerrit/server/notedb/MissingMetaObjectException.java b/java/com/google/gerrit/server/notedb/MissingMetaObjectException.java
new file mode 100644
index 0000000..ffe9fa8
--- /dev/null
+++ b/java/com/google/gerrit/server/notedb/MissingMetaObjectException.java
@@ -0,0 +1,22 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.notedb;
+
+/** Separate exception type to throw if requested meta SHA1 is not available. */
+public class MissingMetaObjectException extends RuntimeException {
+ MissingMetaObjectException(String msg) {
+ super(msg);
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/change/GetChange.java b/java/com/google/gerrit/server/restapi/change/GetChange.java
index f28d2b3..2f1c61e 100644
--- a/java/com/google/gerrit/server/restapi/change/GetChange.java
+++ b/java/com/google/gerrit/server/restapi/change/GetChange.java
@@ -16,12 +16,15 @@
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.Streams;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Change;
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.client.ListOption;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.PluginDefinedInfo;
import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.PreconditionFailedException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.server.DynamicOptions;
@@ -31,12 +34,15 @@
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.PluginDefinedAttributesFactories;
import com.google.gerrit.server.change.RevisionResource;
+import com.google.gerrit.server.notedb.MissingMetaObjectException;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
import java.util.Collection;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.Map;
+import org.eclipse.jgit.errors.InvalidObjectIdException;
+import org.eclipse.jgit.lib.ObjectId;
import org.kohsuke.args4j.Option;
public class GetChange
@@ -53,6 +59,9 @@
options.add(o);
}
+ @Option(name = "--meta", usage = "NoteDb meta SHA1")
+ String metaRevId = "";
+
@Option(name = "-O", usage = "Output option flags, in hex")
void setOptionFlagsHex(String hex) {
options.addAll(ListOption.fromBits(ListChangesOption.class, Integer.parseInt(hex, 16)));
@@ -75,14 +84,35 @@
}
@Override
- public Response<ChangeInfo> apply(ChangeResource rsrc) {
- return Response.withMustRevalidate(newChangeJson().format(rsrc));
+ public Response<ChangeInfo> apply(ChangeResource rsrc)
+ throws BadRequestException, PreconditionFailedException {
+ try {
+ return Response.withMustRevalidate(newChangeJson().format(rsrc.getChange(), getMetaRevId()));
+ } catch (MissingMetaObjectException e) {
+ throw new PreconditionFailedException(e.getMessage());
+ }
}
Response<ChangeInfo> apply(RevisionResource rsrc) {
return Response.withMustRevalidate(newChangeJson().format(rsrc));
}
+ @Nullable
+ private ObjectId getMetaRevId() throws BadRequestException {
+ if (metaRevId.isEmpty()) {
+ return null;
+ }
+
+ // It might be interesting to also allow {SHA1}^^, so callers can walk back into history
+ // without having to fetch the entire /meta ref. If we do so, we have to be careful that
+ // the error messages can't be abused to fetch hidden data.
+ try {
+ return ObjectId.fromString(metaRevId);
+ } catch (InvalidObjectIdException e) {
+ throw new BadRequestException("invalid meta SHA1: " + metaRevId, e);
+ }
+ }
+
private ChangeJson newChangeJson() {
return json.create(options, this::createPluginDefinedInfos);
}
diff --git a/java/com/google/gerrit/server/restapi/change/GetDetail.java b/java/com/google/gerrit/server/restapi/change/GetDetail.java
index e31d84b..15362d5 100644
--- a/java/com/google/gerrit/server/restapi/change/GetDetail.java
+++ b/java/com/google/gerrit/server/restapi/change/GetDetail.java
@@ -16,6 +16,8 @@
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.PreconditionFailedException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.server.DynamicOptions;
@@ -58,7 +60,8 @@
}
@Override
- public Response<ChangeInfo> apply(ChangeResource rsrc) {
+ public Response<ChangeInfo> apply(ChangeResource rsrc)
+ throws BadRequestException, PreconditionFailedException {
return delegate.apply(rsrc);
}
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeMetaIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeMetaIT.java
index 3787a35..e025c52 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeMetaIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeMetaIT.java
@@ -20,8 +20,19 @@
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.entities.Change;
import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.PluginDefinedInfo;
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.server.change.ChangePluginDefinedInfoFactory;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gson.reflect.TypeToken;
+import com.google.gson.stream.JsonReader;
+import com.google.inject.AbstractModule;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
import org.eclipse.jgit.lib.Repository;
import org.junit.Test;
@@ -55,4 +66,85 @@
repo.exactRef(changeMetaRef(Change.id(before._number))).getObjectId().getName());
}
}
+
+ @Test
+ public void ChangeInfo_metaSha1_parameter() throws Exception {
+ PushOneCommit.Result result = createChange();
+ String changeId = result.getChangeId();
+ gApi.changes().id(changeId).setMessage("before\n\n" + "Change-Id: " + result.getChangeId());
+ ChangeInfo before = gApi.changes().id(changeId).get();
+ gApi.changes().id(changeId).setMessage("after\n\n" + "Change-Id: " + result.getChangeId());
+ ChangeInfo after = gApi.changes().id(changeId).get();
+ assertThat(after.metaRevId).isNotEqualTo(before.metaRevId);
+
+ RestResponse resp = adminRestSession.get("/changes/" + changeId + "/?meta=" + before.metaRevId);
+ resp.assertOK();
+
+ ChangeInfo got;
+ try (JsonReader jsonReader = new JsonReader(resp.getReader())) {
+ jsonReader.setLenient(true);
+ got = newGson().fromJson(jsonReader, ChangeInfo.class);
+ }
+ assertThat(got.subject).isEqualTo(before.subject);
+ }
+
+ @Test
+ public void metaUnreachableSha1() throws Exception {
+ PushOneCommit.Result ch1 = createChange();
+ PushOneCommit.Result ch2 = createChange();
+
+ ChangeInfo info2 = gApi.changes().id(ch2.getChangeId()).get();
+
+ RestResponse resp =
+ adminRestSession.get("/changes/" + ch1.getChangeId() + "/?meta=" + info2.metaRevId);
+
+ resp.assertStatus(412);
+ }
+
+ protected static class PluginDefinedSimpleAttributeModule extends AbstractModule {
+ static class MyMetaHash extends PluginDefinedInfo {
+ String myMetaRef;
+ };
+
+ static PluginDefinedInfo newMyMetaHash(ChangeData cd) {
+ MyMetaHash mmh = new MyMetaHash();
+ mmh.myMetaRef = cd.notes().getMetaId().name();
+ return mmh;
+ }
+
+ @Override
+ public void configure() {
+ DynamicSet.bind(binder(), ChangePluginDefinedInfoFactory.class)
+ .toInstance(
+ (cds, bp, p) -> {
+ Map<Change.Id, PluginDefinedInfo> out = new HashMap<>();
+ cds.forEach(cd -> out.put(cd.getId(), newMyMetaHash(cd)));
+ return out;
+ });
+ }
+ }
+
+ @Test
+ public void pluginDefinedAttribute() throws Exception {
+ try (AutoCloseable ignored =
+ installPlugin("my-plugin", PluginDefinedSimpleAttributeModule.class)) {
+ PushOneCommit.Result result = createChange();
+ String changeId = result.getChangeId();
+ gApi.changes().id(changeId).setMessage("before\n\n" + "Change-Id: " + result.getChangeId());
+ ChangeInfo before = gApi.changes().id(changeId).get();
+ gApi.changes().id(changeId).setMessage("after\n\n" + "Change-Id: " + result.getChangeId());
+ ChangeInfo after = gApi.changes().id(changeId).get();
+
+ RestResponse resp =
+ adminRestSession.get("/changes/" + changeId + "/?meta=" + before.metaRevId);
+ resp.assertOK();
+
+ Map<String, Object> changeInfo =
+ newGson().fromJson(resp.getReader(), new TypeToken<Map<String, Object>>() {}.getType());
+ List<Object> plugins = (List<Object>) changeInfo.get("plugins");
+ Map<String, Object> myplugin = (Map<String, Object>) plugins.get(0);
+
+ assertThat(myplugin.get("my_meta_ref")).isEqualTo(before.metaRevId);
+ }
+ }
}
