Fix ref state for starred changes to be in All-Users
This was a bug in I622dbbb3, which accidentally referenced the starred
change ref in the project repo. Add tests that hopefully should make
this kind of thing easier to spot.
Change-Id: If908c6ea7bbe99be113b6b0c87b5be311ed7547c
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java
index 1672eea..1d376d5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -971,7 +971,7 @@
input.editRefs().values().forEach(
r -> result.add(RefState.of(r).toByteArray(project)));
input.starRefs().values().forEach(
- r -> result.add(RefState.of(r.ref()).toByteArray(project)));
+ r -> result.add(RefState.of(r.ref()).toByteArray(args.allUsers)));
if (PrimaryStorage.of(input.change()) == PrimaryStorage.NOTE_DB) {
ChangeNotes notes = input.notes();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/StalenessChecker.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/StalenessChecker.java
index 0c3d89c..7c841c5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/StalenessChecker.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/StalenessChecker.java
@@ -62,7 +62,7 @@
private static final Logger log =
LoggerFactory.getLogger(StalenessChecker.class);
- private final ImmutableSet<String> FIELDS = ImmutableSet.of(
+ public static final ImmutableSet<String> FIELDS = ImmutableSet.of(
ChangeField.CHANGE.getName(),
ChangeField.REF_STATE.getName(),
ChangeField.REF_STATE_PATTERN.getName());
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index d22e7a8..5e5a047 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -21,6 +21,7 @@
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static java.util.concurrent.TimeUnit.MINUTES;
import static java.util.concurrent.TimeUnit.SECONDS;
+import static java.util.stream.Collectors.toList;
import com.google.common.base.MoreObjects;
import com.google.common.collect.FluentIterable;
@@ -38,6 +39,7 @@
import com.google.gerrit.extensions.api.changes.HashtagsInput;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.api.changes.ReviewInput.RobotCommentInput;
import com.google.gerrit.extensions.api.changes.StarsInput;
import com.google.gerrit.extensions.api.groups.GroupInput;
import com.google.gerrit.extensions.common.ChangeInfo;
@@ -54,6 +56,7 @@
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.reviewdb.server.ReviewDbUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.Sequences;
@@ -66,10 +69,15 @@
import com.google.gerrit.server.edit.ChangeEditModifier;
import com.google.gerrit.server.git.BatchUpdate;
import com.google.gerrit.server.git.validators.CommitValidators;
+import com.google.gerrit.server.index.IndexConfig;
+import com.google.gerrit.server.index.QueryOptions;
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.index.change.ChangeIndexCollection;
import com.google.gerrit.server.index.change.ChangeIndexer;
+import com.google.gerrit.server.index.change.IndexedChangeQuery;
+import com.google.gerrit.server.index.change.StalenessChecker;
import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.notedb.NoteDbChangeState;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.schema.SchemaCreator;
import com.google.gerrit.server.util.RequestContext;
@@ -100,6 +108,7 @@
import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashMap;
@@ -125,6 +134,7 @@
@Inject protected ChangeEditModifier changeEditModifier;
@Inject protected ChangeIndexCollection indexes;
@Inject protected ChangeIndexer indexer;
+ @Inject protected IndexConfig indexConfig;
@Inject protected InMemoryDatabase schemaFactory;
@Inject protected InMemoryRepositoryManager repoManager;
@Inject protected InternalChangeQuery internalChangeQuery;
@@ -1638,6 +1648,97 @@
assertQuery("has:edit");
}
+ @Test
+ public void refStateFields() throws Exception {
+ Account.Id user = createAccount("user");
+ Project.NameKey project = new Project.NameKey("repo");
+ TestRepository<Repo> repo = createProject(project.get());
+ String path = "file";
+ RevCommit commit = repo.parseBody(
+ repo.commit().message("one").add(path, "contents").create());
+ Change change = insert(repo, newChangeForCommit(repo, commit));
+ Change.Id id = change.getId();
+ int c = id.get();
+ PatchSet ps = db.patchSets().get(change.currentPatchSetId());
+ requestContext.setContext(newRequestContext(user));
+
+ // Ensure one of each type of supported ref is present for the change. If
+ // any more refs are added, update this test to reflect them.
+
+ // Edit
+ assertThat(changeEditModifier.createEdit(change, ps))
+ .isEqualTo(RefUpdate.Result.NEW);
+
+ // Star
+ gApi.accounts()
+ .self()
+ .starChange(change.getId().toString());
+
+ if (notesMigration.readChanges()) {
+ // Robot comment.
+ ReviewInput rin = new ReviewInput();
+ RobotCommentInput rcin = new RobotCommentInput();
+ rcin.robotId = "happyRobot";
+ rcin.robotRunId = "1";
+ rcin.line = 1;
+ rcin.message = "nit: trailing whitespace";
+ rcin.path = path;
+ rin.robotComments = ImmutableMap.of(path, ImmutableList.of(rcin));
+ gApi.changes().id(c).current().review(rin);
+ }
+
+ // Draft.
+ DraftInput din = new DraftInput();
+ din.path = path;
+ din.line = 1;
+ din.message = "draft";
+ gApi.changes().id(c).current().createDraft(din);
+
+ if (notesMigration.readChanges()) {
+ // Force NoteDb primary.
+ change = ReviewDbUtil.unwrapDb(db).changes().get(id);
+ change.setNoteDbState(NoteDbChangeState.NOTE_DB_PRIMARY_STATE);
+ ReviewDbUtil.unwrapDb(db).changes().update(Collections.singleton(change));
+ indexer.index(db, change);
+ }
+
+ QueryOptions opts = IndexedChangeQuery.createOptions(
+ indexConfig, 0, 1, StalenessChecker.FIELDS);
+ ChangeData cd = indexes.getSearchIndex().get(id, opts).get();
+
+ String cs = RefNames.shard(c);
+ int u = user.get();
+ String us = RefNames.shard(u);
+
+ List<String> expectedStates = Lists.newArrayList(
+ "repo:refs/users/" + us + "/edit-" + c + "/1",
+ "All-Users:refs/starred-changes/" + cs + "/" + u);
+ if (notesMigration.readChanges()) {
+ expectedStates.add("repo:refs/changes/" + cs + "/meta");
+ expectedStates.add("repo:refs/changes/" + cs + "/robot-comments");
+ expectedStates.add("All-Users:refs/draft-comments/" + cs + "/" + u);
+ }
+ assertThat(
+ cd.getRefStates().stream()
+ .map(String::new)
+ // Omit SHA-1, we're just concerned with the project/ref names.
+ .map(s -> s.substring(0, s.lastIndexOf(':')))
+ .collect(toList()))
+ .containsExactlyElementsIn(expectedStates);
+
+ List<String> expectedPatterns = Lists.newArrayList(
+ "repo:refs/users/*/edit-" + c + "/*");
+ if (notesMigration.readChanges()) {
+ expectedPatterns.add("All-Users:refs/starred-changes/" + cs + "/*");
+ expectedPatterns.add("All-Users:refs/draft-comments/" + cs + "/*");
+ }
+ assertThat(
+ cd.getRefStatePatterns().stream()
+ .map(String::new)
+ .collect(toList()))
+ .containsExactlyElementsIn(expectedPatterns);
+ }
+
protected ChangeInserter newChange(TestRepository<Repo> repo)
throws Exception {
return newChange(repo, null, null, null, null);
