Merge changes I8b4cf63f,Id49ccf39
* changes:
GetChange: populate ChangeInfo.metaRevId with the NoteDb meta ref SHA1
ChangeData: use a Java type for RefState rather than byte[]
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 096b068..3b9320c 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -6467,6 +6467,8 @@
Only set if link:#current-revision[the current revision] is requested
(in which case it will only contain a key for the current revision) or
if link:#all-revisions[all revisions] are requested.
+|`meta_rev_id` |optional|
+The SHA1 of the NoteDb meta ref.
|`tracking_ids` |optional|
A list of link:#tracking-id-info[TrackingIdInfo] entities describing
references to external tracking systems. Only set if
diff --git a/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java b/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
index 969ffa5..162654d 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
@@ -37,6 +37,7 @@
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.QueryOptions;
+import com.google.gerrit.index.RefState;
import com.google.gerrit.index.Schema;
import com.google.gerrit.index.query.DataSource;
import com.google.gerrit.index.query.Predicate;
@@ -354,7 +355,7 @@
// Ref-state.
if (fields.contains(ChangeField.REF_STATE.getName())) {
- cd.setRefStates(getByteArray(source, ChangeField.REF_STATE.getName()));
+ cd.setRefStates(RefState.parseStates(getByteArray(source, ChangeField.REF_STATE.getName())));
}
// Ref-state-pattern.
diff --git a/java/com/google/gerrit/extensions/common/ChangeInfo.java b/java/com/google/gerrit/extensions/common/ChangeInfo.java
index 7ed2f95..528efe3 100644
--- a/java/com/google/gerrit/extensions/common/ChangeInfo.java
+++ b/java/com/google/gerrit/extensions/common/ChangeInfo.java
@@ -70,6 +70,7 @@
public String submissionId;
public Integer cherryPickOfChange;
public Integer cherryPickOfPatchSet;
+ public String metaRevId;
/**
* Whether the change contains conflicts.
diff --git a/java/com/google/gerrit/index/RefState.java b/java/com/google/gerrit/index/RefState.java
index 956dcab..ed38de9 100644
--- a/java/com/google/gerrit/index/RefState.java
+++ b/java/com/google/gerrit/index/RefState.java
@@ -20,8 +20,7 @@
import com.google.auto.value.AutoValue;
import com.google.common.base.Splitter;
-import com.google.common.collect.MultimapBuilder;
-import com.google.common.collect.SetMultimap;
+import com.google.common.collect.ImmutableSetMultimap;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Project;
import com.google.gerrit.git.ObjectIds;
@@ -33,10 +32,10 @@
@AutoValue
public abstract class RefState {
- public static SetMultimap<Project.NameKey, RefState> parseStates(Iterable<byte[]> states) {
+ public static ImmutableSetMultimap<Project.NameKey, RefState> parseStates(
+ Iterable<byte[]> states) {
RefState.check(states != null, null);
- SetMultimap<Project.NameKey, RefState> result =
- MultimapBuilder.hashKeys().hashSetValues().build();
+ ImmutableSetMultimap.Builder<Project.NameKey, RefState> result = ImmutableSetMultimap.builder();
for (byte[] b : states) {
RefState.check(b != null, null);
String s = new String(b, UTF_8);
@@ -44,7 +43,7 @@
RefState.check(parts.size() == 3 && !parts.get(0).isEmpty() && !parts.get(1).isEmpty(), s);
result.put(Project.nameKey(parts.get(0)), RefState.create(parts.get(1), parts.get(2)));
}
- return result;
+ return result.build();
}
public static RefState create(String ref, String sha) {
diff --git a/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index f4b9e69..c3d4440 100644
--- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -49,6 +49,7 @@
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.QueryOptions;
+import com.google.gerrit.index.RefState;
import com.google.gerrit.index.Schema;
import com.google.gerrit.index.query.FieldBundle;
import com.google.gerrit.index.query.Predicate;
@@ -702,7 +703,7 @@
}
private void decodeRefStates(ListMultimap<String, IndexableField> doc, ChangeData cd) {
- cd.setRefStates(copyAsBytes(doc.get(REF_STATE_FIELD)));
+ cd.setRefStates(RefState.parseStates(copyAsBytes(doc.get(REF_STATE_FIELD))));
}
private void decodeRefStatePatterns(ListMultimap<String, IndexableField> doc, ChangeData cd) {
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index f292245..a461b59 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -54,6 +54,7 @@
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.RefNames;
import com.google.gerrit.entities.SubmitRecord;
import com.google.gerrit.entities.SubmitRecord.Status;
import com.google.gerrit.entities.SubmitRequirement;
@@ -75,6 +76,7 @@
import com.google.gerrit.extensions.common.SubmitRequirementInfo;
import com.google.gerrit.extensions.common.TrackingIdInfo;
import com.google.gerrit.extensions.restapi.Url;
+import com.google.gerrit.index.RefState;
import com.google.gerrit.index.query.QueryResult;
import com.google.gerrit.metrics.Description;
import com.google.gerrit.metrics.Description.Units;
@@ -570,6 +572,15 @@
out.totalCommentCount = cd.totalCommentCount();
out.unresolvedCommentCount = cd.unresolvedCommentCount();
+ if (cd.getRefStates() != null) {
+ String metaName = RefNames.changeMetaRef(cd.getId());
+ Optional<RefState> metaState =
+ cd.getRefStates().values().stream().filter(r -> r.ref().equals(metaName)).findAny();
+
+ // metaState should always be there, but it doesn't hurt to be extra careful.
+ metaState.ifPresent(rs -> out.metaRevId = rs.id().getName());
+ }
+
if (user.isIdentifiedUser()) {
Collection<String> stars = cd.stars(user.getAccountId());
out.starred = stars.contains(StarredChangesUtil.DEFAULT_LABEL) ? true : null;
diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index 5f2525e..59600e0 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -57,7 +57,6 @@
import com.google.gerrit.entities.converter.PatchSetProtoConverter;
import com.google.gerrit.entities.converter.ProtoConverter;
import com.google.gerrit.index.FieldDef;
-import com.google.gerrit.index.RefState;
import com.google.gerrit.index.SchemaUtil;
import com.google.gerrit.json.OutputFormat;
import com.google.gerrit.proto.Protos;
@@ -66,9 +65,7 @@
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.index.change.StalenessChecker.RefStatePattern;
-import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
-import com.google.gerrit.server.notedb.RobotCommentNotes;
import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
@@ -982,27 +979,9 @@
.buildRepeatable(
cd -> {
List<byte[]> result = new ArrayList<>();
- Project.NameKey project = cd.change().getProject();
-
- cd.editRefs()
- .values()
- .forEach(r -> result.add(RefState.of(r).toByteArray(project)));
- cd.starRefs()
- .values()
- .forEach(r -> result.add(RefState.of(r.ref()).toByteArray(allUsers(cd))));
-
- ChangeNotes notes = cd.notes();
- result.add(
- RefState.create(notes.getRefName(), notes.getMetaId()).toByteArray(project));
- notes.getRobotComments(); // Force loading robot comments.
- RobotCommentNotes robotNotes = notes.getRobotCommentNotes();
- result.add(
- RefState.create(robotNotes.getRefName(), robotNotes.getMetaId())
- .toByteArray(project));
- cd.draftRefs()
- .values()
- .forEach(r -> result.add(RefState.of(r).toByteArray(allUsers(cd))));
-
+ cd.getRefStates()
+ .entries()
+ .forEach(e -> result.add(e.getValue().toByteArray(e.getKey())));
return result;
});
diff --git a/java/com/google/gerrit/server/index/change/StalenessChecker.java b/java/com/google/gerrit/server/index/change/StalenessChecker.java
index 7e50104..ad5cc2b 100644
--- a/java/com/google/gerrit/server/index/change/StalenessChecker.java
+++ b/java/com/google/gerrit/server/index/change/StalenessChecker.java
@@ -93,7 +93,7 @@
return StalenessCheckResult.stale("Document %s missing from index", id);
}
ChangeData cd = result.get();
- return check(repoManager, id, parseStates(cd), parsePatterns(cd));
+ return check(repoManager, id, cd.getRefStates(), parsePatterns(cd));
}
/**
@@ -127,10 +127,6 @@
return StalenessCheckResult.notStale();
}
- private SetMultimap<Project.NameKey, RefState> parseStates(ChangeData cd) {
- return RefState.parseStates(cd.getRefStates());
- }
-
private ListMultimap<Project.NameKey, RefStatePattern> parsePatterns(ChangeData cd) {
return parsePatterns(cd.getRefStatePatterns());
}
diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java
index 6de263e..05ecc61 100644
--- a/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -26,13 +26,16 @@
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSetMultimap;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
+import com.google.common.collect.SetMultimap;
import com.google.common.primitives.Ints;
import com.google.gerrit.common.Nullable;
+import com.google.gerrit.common.UsedAt;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.entities.Change;
@@ -43,6 +46,7 @@
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.Project.NameKey;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.entities.RobotComment;
import com.google.gerrit.entities.SubmitRecord;
@@ -50,6 +54,7 @@
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.index.RefState;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.CommentsUtil;
@@ -69,6 +74,7 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.notedb.RobotCommentNotes;
import com.google.gerrit.server.patch.DiffSummary;
import com.google.gerrit.server.patch.DiffSummaryKey;
import com.google.gerrit.server.patch.PatchListCache;
@@ -308,7 +314,7 @@
private Integer totalCommentCount;
private LabelTypes labelTypes;
private Optional<Timestamp> mergedOn;
- private ImmutableList<byte[]> refStates;
+ private ImmutableSetMultimap<NameKey, RefState> refStates;
private ImmutableList<byte[]> refStatePatterns;
@Inject
@@ -1169,12 +1175,38 @@
}
}
- public ImmutableList<byte[]> getRefStates() {
+ public SetMultimap<NameKey, RefState> getRefStates() {
+ if (refStates == null) {
+ if (!lazyLoad) {
+ return ImmutableSetMultimap.of();
+ }
+
+ ImmutableSetMultimap.Builder<NameKey, RefState> result = ImmutableSetMultimap.builder();
+ editRefs().values().forEach(r -> result.put(project, RefState.of(r)));
+ starRefs().values().forEach(r -> result.put(allUsersName, RefState.of(r.ref())));
+
+ // TODO: instantiating the notes is too much. We don't want to parse NoteDb, we just want the
+ // refs.
+ result.put(project, RefState.create(notes().getRefName(), notes().getMetaId()));
+ notes().getRobotComments(); // Force loading robot comments.
+ RobotCommentNotes robotNotes = notes().getRobotCommentNotes();
+ result.put(project, RefState.create(robotNotes.getRefName(), robotNotes.getMetaId()));
+ draftRefs().values().forEach(r -> result.put(allUsersName, RefState.of(r)));
+
+ refStates = result.build();
+ }
+
return refStates;
}
+ @UsedAt(UsedAt.Project.GOOGLE)
public void setRefStates(Iterable<byte[]> refStates) {
- this.refStates = ImmutableList.copyOf(refStates);
+ // TODO(hanwen): remove Google use, and drop this method.
+ setRefStates(RefState.parseStates(refStates));
+ }
+
+ public void setRefStates(ImmutableSetMultimap<Project.NameKey, RefState> refStates) {
+ this.refStates = refStates;
}
public ImmutableList<byte[]> getRefStatePatterns() {
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeMetaIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeMetaIT.java
new file mode 100644
index 0000000..3787a35
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeMetaIT.java
@@ -0,0 +1,58 @@
+// Copyright (C) 2017 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.acceptance.rest.change;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.entities.RefNames.changeMetaRef;
+
+import com.google.common.collect.Iterables;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import org.eclipse.jgit.lib.Repository;
+import org.junit.Test;
+
+/** Test handling of the NoteDb commit hash in the GetChange endpoint */
+public class ChangeMetaIT extends AbstractDaemonTest {
+ @Test
+ public void metaSha1_fromIndex() throws Exception {
+ PushOneCommit.Result result = createChange();
+ String changeId = result.getChangeId();
+
+ try (AutoCloseable ignored = disableNoteDb()) {
+ ChangeInfo change =
+ Iterables.getOnlyElement(gApi.changes().query().withQuery("change:" + changeId).get());
+
+ try (Repository repo = repoManager.openRepository(project)) {
+ assertThat(change.metaRevId)
+ .isEqualTo(
+ repo.exactRef(changeMetaRef(Change.id(change._number))).getObjectId().getName());
+ }
+ }
+ }
+
+ @Test
+ public void metaSha1_fromNoteDb() throws Exception {
+ PushOneCommit.Result result = createChange();
+ String changeId = result.getChangeId();
+ ChangeInfo before = gApi.changes().id(changeId).get();
+ try (Repository repo = repoManager.openRepository(project)) {
+ assertThat(before.metaRevId)
+ .isEqualTo(
+ repo.exactRef(changeMetaRef(Change.id(before._number))).getObjectId().getName());
+ }
+ }
+}
