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
This commit is contained in:
@@ -971,7 +971,7 @@ public class ChangeField {
|
|||||||
input.editRefs().values().forEach(
|
input.editRefs().values().forEach(
|
||||||
r -> result.add(RefState.of(r).toByteArray(project)));
|
r -> result.add(RefState.of(r).toByteArray(project)));
|
||||||
input.starRefs().values().forEach(
|
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) {
|
if (PrimaryStorage.of(input.change()) == PrimaryStorage.NOTE_DB) {
|
||||||
ChangeNotes notes = input.notes();
|
ChangeNotes notes = input.notes();
|
||||||
|
|||||||
@@ -62,7 +62,7 @@ public class StalenessChecker {
|
|||||||
private static final Logger log =
|
private static final Logger log =
|
||||||
LoggerFactory.getLogger(StalenessChecker.class);
|
LoggerFactory.getLogger(StalenessChecker.class);
|
||||||
|
|
||||||
private final ImmutableSet<String> FIELDS = ImmutableSet.of(
|
public static final ImmutableSet<String> FIELDS = ImmutableSet.of(
|
||||||
ChangeField.CHANGE.getName(),
|
ChangeField.CHANGE.getName(),
|
||||||
ChangeField.REF_STATE.getName(),
|
ChangeField.REF_STATE.getName(),
|
||||||
ChangeField.REF_STATE_PATTERN.getName());
|
ChangeField.REF_STATE_PATTERN.getName());
|
||||||
|
|||||||
@@ -21,6 +21,7 @@ import static java.util.concurrent.TimeUnit.HOURS;
|
|||||||
import static java.util.concurrent.TimeUnit.MILLISECONDS;
|
import static java.util.concurrent.TimeUnit.MILLISECONDS;
|
||||||
import static java.util.concurrent.TimeUnit.MINUTES;
|
import static java.util.concurrent.TimeUnit.MINUTES;
|
||||||
import static java.util.concurrent.TimeUnit.SECONDS;
|
import static java.util.concurrent.TimeUnit.SECONDS;
|
||||||
|
import static java.util.stream.Collectors.toList;
|
||||||
|
|
||||||
import com.google.common.base.MoreObjects;
|
import com.google.common.base.MoreObjects;
|
||||||
import com.google.common.collect.FluentIterable;
|
import com.google.common.collect.FluentIterable;
|
||||||
@@ -38,6 +39,7 @@ import com.google.gerrit.extensions.api.changes.DraftInput;
|
|||||||
import com.google.gerrit.extensions.api.changes.HashtagsInput;
|
import com.google.gerrit.extensions.api.changes.HashtagsInput;
|
||||||
import com.google.gerrit.extensions.api.changes.NotifyHandling;
|
import com.google.gerrit.extensions.api.changes.NotifyHandling;
|
||||||
import com.google.gerrit.extensions.api.changes.ReviewInput;
|
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.changes.StarsInput;
|
||||||
import com.google.gerrit.extensions.api.groups.GroupInput;
|
import com.google.gerrit.extensions.api.groups.GroupInput;
|
||||||
import com.google.gerrit.extensions.common.ChangeInfo;
|
import com.google.gerrit.extensions.common.ChangeInfo;
|
||||||
@@ -54,6 +56,7 @@ import com.google.gerrit.reviewdb.client.PatchSet;
|
|||||||
import com.google.gerrit.reviewdb.client.Project;
|
import com.google.gerrit.reviewdb.client.Project;
|
||||||
import com.google.gerrit.reviewdb.client.RefNames;
|
import com.google.gerrit.reviewdb.client.RefNames;
|
||||||
import com.google.gerrit.reviewdb.server.ReviewDb;
|
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.CurrentUser;
|
||||||
import com.google.gerrit.server.IdentifiedUser;
|
import com.google.gerrit.server.IdentifiedUser;
|
||||||
import com.google.gerrit.server.Sequences;
|
import com.google.gerrit.server.Sequences;
|
||||||
@@ -66,10 +69,15 @@ import com.google.gerrit.server.change.PatchSetInserter;
|
|||||||
import com.google.gerrit.server.edit.ChangeEditModifier;
|
import com.google.gerrit.server.edit.ChangeEditModifier;
|
||||||
import com.google.gerrit.server.git.BatchUpdate;
|
import com.google.gerrit.server.git.BatchUpdate;
|
||||||
import com.google.gerrit.server.git.validators.CommitValidators;
|
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.ChangeField;
|
||||||
import com.google.gerrit.server.index.change.ChangeIndexCollection;
|
import com.google.gerrit.server.index.change.ChangeIndexCollection;
|
||||||
import com.google.gerrit.server.index.change.ChangeIndexer;
|
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.ChangeNotes;
|
||||||
|
import com.google.gerrit.server.notedb.NoteDbChangeState;
|
||||||
import com.google.gerrit.server.project.ChangeControl;
|
import com.google.gerrit.server.project.ChangeControl;
|
||||||
import com.google.gerrit.server.schema.SchemaCreator;
|
import com.google.gerrit.server.schema.SchemaCreator;
|
||||||
import com.google.gerrit.server.util.RequestContext;
|
import com.google.gerrit.server.util.RequestContext;
|
||||||
@@ -100,6 +108,7 @@ import org.junit.Test;
|
|||||||
import java.sql.Timestamp;
|
import java.sql.Timestamp;
|
||||||
import java.util.ArrayList;
|
import java.util.ArrayList;
|
||||||
import java.util.Arrays;
|
import java.util.Arrays;
|
||||||
|
import java.util.Collections;
|
||||||
import java.util.HashSet;
|
import java.util.HashSet;
|
||||||
import java.util.Iterator;
|
import java.util.Iterator;
|
||||||
import java.util.LinkedHashMap;
|
import java.util.LinkedHashMap;
|
||||||
@@ -125,6 +134,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
|
|||||||
@Inject protected ChangeEditModifier changeEditModifier;
|
@Inject protected ChangeEditModifier changeEditModifier;
|
||||||
@Inject protected ChangeIndexCollection indexes;
|
@Inject protected ChangeIndexCollection indexes;
|
||||||
@Inject protected ChangeIndexer indexer;
|
@Inject protected ChangeIndexer indexer;
|
||||||
|
@Inject protected IndexConfig indexConfig;
|
||||||
@Inject protected InMemoryDatabase schemaFactory;
|
@Inject protected InMemoryDatabase schemaFactory;
|
||||||
@Inject protected InMemoryRepositoryManager repoManager;
|
@Inject protected InMemoryRepositoryManager repoManager;
|
||||||
@Inject protected InternalChangeQuery internalChangeQuery;
|
@Inject protected InternalChangeQuery internalChangeQuery;
|
||||||
@@ -1638,6 +1648,97 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
|
|||||||
assertQuery("has:edit");
|
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)
|
protected ChangeInserter newChange(TestRepository<Repo> repo)
|
||||||
throws Exception {
|
throws Exception {
|
||||||
return newChange(repo, null, null, null, null);
|
return newChange(repo, null, null, null, null);
|
||||||
|
|||||||
Reference in New Issue
Block a user