Add a method to return full edit ref state to ChangeData
Currently, only ObjectID of the edit refs is available from
ChangeData#editRefs method.
Add a method that returns full Ref objects. This is needed for our
internal change index implementation.
As a side effect, the change also removes the ability to restore edits
from index. This seems to have been unused before this change.
Change-Id: I39381b9f73df12f5cf40bb571e6cfae4442f7073
Google-Bug-Id: b/261864182
Release-Notes: skip
diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java
index ec1fcad..78615bf 100644
--- a/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -334,7 +334,7 @@
* Map from {@link com.google.gerrit.entities.Account.Id} to the tip of the edit ref for this
* change and a given user.
*/
- private Table<Account.Id, PatchSet.Id, ObjectId> editsByUser;
+ private Table<Account.Id, PatchSet.Id, Ref> editRefsByUser;
private Set<Account.Id> reviewedBy;
/**
@@ -1097,8 +1097,8 @@
return editRefs().rowKeySet();
}
- public Table<Account.Id, PatchSet.Id, ObjectId> editRefs() {
- if (editsByUser == null) {
+ public Table<Account.Id, PatchSet.Id, Ref> editRefs() {
+ if (editRefsByUser == null) {
if (!lazyload()) {
return HashBasedTable.create();
}
@@ -1106,7 +1106,7 @@
if (c == null) {
return HashBasedTable.create();
}
- editsByUser = HashBasedTable.create();
+ editRefsByUser = HashBasedTable.create();
Change.Id id = requireNonNull(change.getId());
try (Repository repo = repoManager.openRepository(project())) {
for (Ref ref : repo.getRefDatabase().getRefsByPrefix(RefNames.REFS_USERS)) {
@@ -1117,7 +1117,7 @@
if (id.equals(ps.changeId())) {
Account.Id accountId = Account.Id.fromRef(ref.getName());
if (accountId != null) {
- editsByUser.put(accountId, ps, ref.getObjectId());
+ editRefsByUser.put(accountId, ps, ref);
}
}
}
@@ -1125,7 +1125,7 @@
throw new StorageException(e);
}
}
- return editsByUser;
+ return editRefsByUser;
}
public Set<Account.Id> draftsByUser() {
@@ -1273,13 +1273,13 @@
}
ImmutableSetMultimap.Builder<NameKey, RefState> result = ImmutableSetMultimap.builder();
- for (Table.Cell<Account.Id, PatchSet.Id, ObjectId> edit : editRefs().cellSet()) {
+ for (Table.Cell<Account.Id, PatchSet.Id, Ref> edit : editRefs().cellSet()) {
result.put(
project,
RefState.create(
RefNames.refsEdit(
edit.getRowKey(), edit.getColumnKey().changeId(), edit.getColumnKey()),
- edit.getValue()));
+ edit.getValue().getObjectId()));
}
// TODO: instantiating the notes is too much. We don't want to parse NoteDb, we just want the
@@ -1309,21 +1309,6 @@
.forEach(r -> draftsByUser.put(Account.Id.fromRef(r.ref()), r.id()));
}
}
- if (editsByUser == null) {
- // Recover edit refs as well. Edits are represented as refs in the repository.
- // ChangeData exposes #editsByUser which just provides a Set of Account.Ids of users who
- // have edits on this change. Recovering this list from RefStates makes it available even
- // on ChangeData instances retrieved from the index.
- editsByUser = HashBasedTable.create();
- if (refStates.containsKey(project())) {
- refStates.get(project()).stream()
- .filter(r -> RefNames.isRefsEdit(r.ref()))
- .forEach(
- r ->
- editsByUser.put(
- Account.Id.fromRef(r.ref()), PatchSet.Id.fromEditRef(r.ref()), r.id()));
- }
- }
}
public ImmutableList<byte[]> getRefStatePatterns() {
diff --git a/java/com/google/gerrit/testing/TestChanges.java b/java/com/google/gerrit/testing/TestChanges.java
index 4a97bc5..c8f89cf 100644
--- a/java/com/google/gerrit/testing/TestChanges.java
+++ b/java/com/google/gerrit/testing/TestChanges.java
@@ -32,6 +32,7 @@
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.inject.Injector;
import java.time.ZoneId;
+import java.util.Optional;
import java.util.concurrent.atomic.AtomicInteger;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.ObjectId;
@@ -77,15 +78,20 @@
}
public static ChangeUpdate newUpdate(
- Injector injector, Change c, CurrentUser user, boolean shouldExist) throws Exception {
+ Injector injector, Change c, Optional<CurrentUser> user, boolean shouldExist)
+ throws Exception {
injector =
injector.createChildInjector(
new FactoryModule() {
@Override
public void configure() {
- bind(CurrentUser.class).toInstance(user);
+ if (user.isPresent()) {
+ // user may be already bound in injector
+ bind(CurrentUser.class).toInstance(user.get());
+ }
}
});
+ CurrentUser currentUser = injector.getProvider(CurrentUser.class).get();
ChangeUpdate update =
injector
.getInstance(ChangeUpdate.Factory.class)
@@ -93,7 +99,7 @@
new ChangeNotes(
injector.getInstance(AbstractChangeNotes.Args.class), c, shouldExist, null)
.load(),
- user,
+ currentUser,
TimeUtil.now(),
Ordering.natural());
@@ -109,7 +115,9 @@
try (Repository repo = repoManager.openRepository(c.getProject());
TestRepository<Repository> tr = new TestRepository<>(repo)) {
PersonIdent ident =
- user.asIdentifiedUser().newCommitterIdent(update.getWhen(), ZoneId.systemDefault());
+ currentUser
+ .asIdentifiedUser()
+ .newCommitterIdent(update.getWhen(), ZoneId.systemDefault());
TestRepository<Repository>.CommitBuilder cb =
tr.commit()
.author(ident)
diff --git a/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
index 37d8468..be8f1f9 100644
--- a/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
@@ -76,6 +76,7 @@
import com.google.inject.TypeLiteral;
import java.time.Instant;
import java.time.ZoneId;
+import java.util.Optional;
import java.util.concurrent.ExecutorService;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
@@ -276,7 +277,7 @@
protected ChangeUpdate newUpdate(
Injector injector, Change c, CurrentUser user, boolean shouldExist) throws Exception {
- ChangeUpdate update = TestChanges.newUpdate(injector, c, user, shouldExist);
+ ChangeUpdate update = TestChanges.newUpdate(injector, c, Optional.of(user), shouldExist);
update.setPatchSetId(c.currentPatchSetId());
update.setAllowWriteToNewRef(true);
return update;
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index b1a9a49..8919e04 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -122,6 +122,7 @@
import com.google.gerrit.server.index.change.ChangeIndexer;
import com.google.gerrit.server.index.change.IndexedChangeQuery;
import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.Sequences;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectConfig;
@@ -135,6 +136,7 @@
import com.google.gerrit.testing.GerritServerTests;
import com.google.gerrit.testing.InMemoryRepositoryManager;
import com.google.gerrit.testing.InMemoryRepositoryManager.Repo;
+import com.google.gerrit.testing.TestChanges;
import com.google.gerrit.testing.TestTimeUtil;
import com.google.inject.Inject;
import com.google.inject.Injector;
@@ -158,7 +160,6 @@
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
-import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.util.SystemReader;
@@ -3230,28 +3231,27 @@
@Test
public void reindexIfStale() throws Exception {
- Account.Id user = createAccount("user");
Project.NameKey project = Project.nameKey("repo");
TestRepository<Repo> repo = createProject(project.get());
Change change = insert(repo, newChange(repo));
String changeId = change.getKey().get();
- ChangeNotes notes = notesFactory.create(change.getProject(), change.getId());
- PatchSet ps = psUtil.get(notes, change.currentPatchSetId());
- requestContext.setContext(newRequestContext(user));
- gApi.changes().id(changeId).edit().create();
- assertQuery("has:edit", change);
+ Account.Id anotherUser = createAccount("another-user");
+ requestContext.setContext(newRequestContext(anotherUser));
+ gApi.changes().id(changeId).addReviewer(anotherUser.toString());
+
+ assertQuery("reviewer:self", change);
assertThat(indexer.reindexIfStale(project, change.getId()).get()).isFalse();
- // Delete edit ref behind index's back.
- RefUpdate ru = repo.getRepository().updateRef(RefNames.refsEdit(user, change.getId(), ps.id()));
- ru.setForceUpdate(true);
- assertThat(ru.delete()).isEqualTo(RefUpdate.Result.FORCED);
+ // Remove reviewer behind index's back.
+ ChangeUpdate update = newUpdate(change);
+ update.removeReviewer(anotherUser);
+ update.commit();
// Index is stale.
- assertQuery("has:edit", change);
+ assertQuery("reviewer:self", change);
assertThat(indexer.reindexIfStale(project, change.getId()).get()).isTrue();
- assertQuery("has:edit");
+ assertQuery("reviewer:self");
}
@Test
@@ -4413,4 +4413,12 @@
protected Schema<ChangeData> getSchema() {
return indexes.getSearchIndex().getSchema();
}
+
+ protected ChangeUpdate newUpdate(Change c) throws Exception {
+ ChangeUpdate update =
+ TestChanges.newUpdate(injector, c, Optional.empty(), /* shouldExist= */ true);
+ update.setPatchSetId(c.currentPatchSetId());
+ update.setAllowWriteToNewRef(true);
+ return update;
+ }
}
