Merge changes from topic "revid-to-objectid"
* changes:
Adapt to new PatchSet constructor
Adapt to RevId removal
Use ObjectId instead of RevId in NoteDb storage layer
Adapt to BranchNameKey refactoring
diff --git a/java/com/google/gerrit/plugins/checks/CheckerMergeValidator.java b/java/com/google/gerrit/plugins/checks/CheckerMergeValidator.java
index dfd16df..a7cad91 100644
--- a/java/com/google/gerrit/plugins/checks/CheckerMergeValidator.java
+++ b/java/com/google/gerrit/plugins/checks/CheckerMergeValidator.java
@@ -14,7 +14,7 @@
package com.google.gerrit.plugins.checks;
-import com.google.gerrit.reviewdb.client.Branch;
+import com.google.gerrit.reviewdb.client.BranchNameKey;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.config.AllProjectsName;
@@ -39,7 +39,7 @@
Repository repo,
CodeReviewCommit commit,
ProjectState destProject,
- Branch.NameKey destBranch,
+ BranchNameKey destBranch,
PatchSet.Id patchSetId,
IdentifiedUser caller)
throws MergeValidationException {
diff --git a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperations.java b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperations.java
index 055baa0..ecf975b 100644
--- a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperations.java
+++ b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperations.java
@@ -19,7 +19,7 @@
import com.google.gerrit.plugins.checks.CheckKey;
import com.google.gerrit.plugins.checks.ListChecksOption;
import com.google.gerrit.plugins.checks.api.CheckInfo;
-import com.google.gerrit.reviewdb.client.RevId;
+import org.eclipse.jgit.lib.ObjectId;
/**
* An aggregation of operations on checks for test purposes.
@@ -96,7 +96,7 @@
*
* @return the map with the checks of the change as text in a map keyed by the revision IDs
*/
- ImmutableMap<RevId, String> notesAsText() throws Exception;
+ ImmutableMap<ObjectId, String> notesAsText() throws Exception;
/**
* Returns this check as {@link CheckInfo}.
diff --git a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImpl.java b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImpl.java
index 8982df5..c732548 100644
--- a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImpl.java
+++ b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImpl.java
@@ -28,11 +28,11 @@
import com.google.gerrit.plugins.checks.ChecksUpdate;
import com.google.gerrit.plugins.checks.ListChecksOption;
import com.google.gerrit.plugins.checks.api.CheckInfo;
-import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.server.ServerInitiated;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.inject.Inject;
import com.google.inject.Provider;
+import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.notes.Note;
@@ -88,7 +88,7 @@
}
@Override
- public ImmutableMap<RevId, String> notesAsText() throws Exception {
+ public ImmutableMap<ObjectId, String> notesAsText() throws Exception {
try (Repository repo = repoManager.openRepository(key.repository());
RevWalk rw = new RevWalk(repo)) {
Ref checkRef =
@@ -96,12 +96,11 @@
checkNotNull(checkRef);
NoteMap notes = NoteMap.read(rw.getObjectReader(), rw.parseCommit(checkRef.getObjectId()));
- ImmutableMap.Builder<RevId, String> raw = ImmutableMap.builder();
+ ImmutableMap.Builder<ObjectId, String> raw = ImmutableMap.builder();
for (Note note : notes) {
raw.put(
- new RevId(note.name()),
- new String(notes.getCachedBytes(note.toObjectId(), Integer.MAX_VALUE)));
+ note.copy(), new String(notes.getCachedBytes(note.toObjectId(), Integer.MAX_VALUE)));
}
return raw.build();
}
diff --git a/java/com/google/gerrit/plugins/checks/db/CheckNotes.java b/java/com/google/gerrit/plugins/checks/db/CheckNotes.java
index fc64346..148fc82 100644
--- a/java/com/google/gerrit/plugins/checks/db/CheckNotes.java
+++ b/java/com/google/gerrit/plugins/checks/db/CheckNotes.java
@@ -19,7 +19,6 @@
import com.google.gerrit.plugins.checks.CheckerRef;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.server.logging.TraceContext;
import com.google.gerrit.server.logging.TraceContext.TraceTimer;
import com.google.gerrit.server.notedb.AbstractChangeNotes;
@@ -40,7 +39,7 @@
private final Change change;
- private ImmutableMap<RevId, NoteDbCheckMap> entities;
+ private ImmutableMap<ObjectId, NoteDbCheckMap> entities;
private CheckRevisionNoteMap revisionNoteMap;
private ObjectId metaId;
@@ -50,7 +49,7 @@
this.change = change;
}
- public ImmutableMap<RevId, NoteDbCheckMap> getChecks() {
+ public ImmutableMap<ObjectId, NoteDbCheckMap> getChecks() {
return entities;
}
@@ -83,8 +82,8 @@
args.changeNoteJson, reader, NoteMap.read(reader, tipCommit));
}
- ImmutableMap.Builder<RevId, NoteDbCheckMap> cs = ImmutableMap.builder();
- for (Map.Entry<RevId, CheckRevisionNote> rn : revisionNoteMap.revisionNotes.entrySet()) {
+ ImmutableMap.Builder<ObjectId, NoteDbCheckMap> cs = ImmutableMap.builder();
+ for (Map.Entry<ObjectId, CheckRevisionNote> rn : revisionNoteMap.revisionNotes.entrySet()) {
cs.put(rn.getKey(), rn.getValue().getOnlyEntity());
}
entities = cs.build();
diff --git a/java/com/google/gerrit/plugins/checks/db/CheckRevisionNoteMap.java b/java/com/google/gerrit/plugins/checks/db/CheckRevisionNoteMap.java
index b18320e..14fba14 100644
--- a/java/com/google/gerrit/plugins/checks/db/CheckRevisionNoteMap.java
+++ b/java/com/google/gerrit/plugins/checks/db/CheckRevisionNoteMap.java
@@ -15,28 +15,28 @@
package com.google.gerrit.plugins.checks.db;
import com.google.common.collect.ImmutableMap;
-import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.server.notedb.ChangeNoteJson;
import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.notes.Note;
import org.eclipse.jgit.notes.NoteMap;
public class CheckRevisionNoteMap {
final NoteMap noteMap;
- final ImmutableMap<RevId, CheckRevisionNote> revisionNotes;
+ final ImmutableMap<ObjectId, CheckRevisionNote> revisionNotes;
static CheckRevisionNoteMap parseChecks(
ChangeNoteJson changeNoteJson, ObjectReader reader, NoteMap noteMap)
throws ConfigInvalidException, IOException {
- Map<RevId, CheckRevisionNote> result = new HashMap<>();
+ Map<ObjectId, CheckRevisionNote> result = new HashMap<>();
for (Note note : noteMap) {
CheckRevisionNote rn = new CheckRevisionNote(changeNoteJson, reader, note.getData());
rn.parse();
- result.put(new RevId(note.name()), rn);
+ result.put(note.copy(), rn);
}
return new CheckRevisionNoteMap(noteMap, ImmutableMap.copyOf(result));
}
@@ -46,7 +46,7 @@
}
private CheckRevisionNoteMap(
- NoteMap noteMap, ImmutableMap<RevId, CheckRevisionNote> revisionNotes) {
+ NoteMap noteMap, ImmutableMap<ObjectId, CheckRevisionNote> revisionNotes) {
this.noteMap = noteMap;
this.revisionNotes = revisionNotes;
}
diff --git a/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java b/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java
index 5a38bfd..f8203c1 100644
--- a/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java
+++ b/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java
@@ -106,7 +106,7 @@
checkNotes.load();
ImmutableList<Check> existingChecks =
- checkNotes.getChecks().getOrDefault(patchSet.getRevision(), NoteDbCheckMap.empty()).checks
+ checkNotes.getChecks().getOrDefault(patchSet.getCommitId(), NoteDbCheckMap.empty()).checks
.entrySet().stream()
.map(e -> e.getValue().toCheck(repositoryName, psId, CheckerUuid.parse(e.getKey())))
.collect(toImmutableList());
diff --git a/java/com/google/gerrit/plugins/checks/db/NoteDbChecksUpdate.java b/java/com/google/gerrit/plugins/checks/db/NoteDbChecksUpdate.java
index b473a6a..85d36e7 100644
--- a/java/com/google/gerrit/plugins/checks/db/NoteDbChecksUpdate.java
+++ b/java/com/google/gerrit/plugins/checks/db/NoteDbChecksUpdate.java
@@ -30,7 +30,6 @@
import com.google.gerrit.plugins.checks.CheckerUuid;
import com.google.gerrit.plugins.checks.Checkers;
import com.google.gerrit.plugins.checks.ChecksUpdate;
-import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
@@ -221,19 +220,19 @@
if (patchSetRef == null) {
throw new IOException(String.format("patchset %s not found", checkKey.patchSet()));
}
- RevId revId = new RevId(patchSetRef.getObjectId().name());
+ ObjectId commitId = patchSetRef.getObjectId();
// Read a fresh copy of the notes map
- Map<RevId, NoteDbCheckMap> newNotes = getRevisionNoteByRevId(rw, curr);
- if (!newNotes.containsKey(revId)) {
+ Map<ObjectId, NoteDbCheckMap> newNotes = getRevisionNoteByCommitId(rw, curr);
+ if (!newNotes.containsKey(commitId)) {
if (operation == Operation.UPDATE) {
throw new IOException(String.format("checker %s not found", checkKey.checkerUuid()));
}
- newNotes.put(revId, NoteDbCheckMap.empty());
+ newNotes.put(commitId, NoteDbCheckMap.empty());
}
- NoteDbCheckMap checksForRevision = newNotes.get(revId);
+ NoteDbCheckMap checksForRevision = newNotes.get(commitId);
if (!checksForRevision.checks.containsKey(checkKey.checkerUuid().get())) {
if (operation == Operation.UPDATE) {
throw new IOException(String.format("checker %s not found", checkKey.checkerUuid()));
@@ -264,11 +263,11 @@
}
private void writeNotesMap(
- Map<RevId, NoteDbCheckMap> notesMap, CommitBuilder cb, ObjectInserter ins)
+ Map<ObjectId, NoteDbCheckMap> notesMap, CommitBuilder cb, ObjectInserter ins)
throws IOException {
CheckRevisionNoteMap output = CheckRevisionNoteMap.emptyMap();
- for (Map.Entry<RevId, NoteDbCheckMap> e : notesMap.entrySet()) {
- ObjectId id = ObjectId.fromString(e.getKey().get());
+ for (Map.Entry<ObjectId, NoteDbCheckMap> e : notesMap.entrySet()) {
+ ObjectId id = e.getKey();
byte[] data = toData(e.getValue());
if (data.length != 0) {
ObjectId dataBlob = ins.insert(OBJ_BLOB, data);
@@ -278,14 +277,14 @@
cb.setTreeId(output.noteMap.writeTree(ins));
}
- private Map<RevId, NoteDbCheckMap> getRevisionNoteByRevId(RevWalk rw, ObjectId curr)
+ private Map<ObjectId, NoteDbCheckMap> getRevisionNoteByCommitId(RevWalk rw, ObjectId curr)
throws ConfigInvalidException, IOException {
CheckRevisionNoteMap existingNotes = getRevisionNoteMap(rw, curr);
// Generate a list with all current checks keyed by patch set
- Map<RevId, NoteDbCheckMap> newNotes =
+ Map<ObjectId, NoteDbCheckMap> newNotes =
Maps.newHashMapWithExpectedSize(existingNotes.revisionNotes.size());
- for (Map.Entry<RevId, CheckRevisionNote> e : existingNotes.revisionNotes.entrySet()) {
+ for (Map.Entry<ObjectId, CheckRevisionNote> e : existingNotes.revisionNotes.entrySet()) {
newNotes.put(e.getKey(), e.getValue().getOnlyEntity());
}
return newNotes;
@@ -335,12 +334,12 @@
if (patchSetRef == null) {
throw new IllegalStateException("patchset " + checkKey.patchSet() + " not found");
}
- RevId revId = new RevId(patchSetRef.getObjectId().name());
- Map<RevId, NoteDbCheckMap> newNotes = getRevisionNoteByRevId(rw, tip);
- if (!newNotes.containsKey(revId)) {
- throw new IllegalStateException("revision " + revId + " not found");
+ ObjectId commitId = patchSetRef.getObjectId();
+ Map<ObjectId, NoteDbCheckMap> newNotes = getRevisionNoteByCommitId(rw, tip);
+ if (!newNotes.containsKey(commitId)) {
+ throw new IllegalStateException("revision " + commitId.name() + " not found");
}
- Map<String, NoteDbCheck> checks = newNotes.get(revId).checks;
+ Map<String, NoteDbCheck> checks = newNotes.get(commitId).checks;
String checkerUuidString = checkKey.checkerUuid().get();
if (!checks.containsKey(checkerUuidString)) {
throw new IllegalStateException("checker " + checkerUuidString + " not found");
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/CheckerRefsIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/CheckerRefsIT.java
index c3a9e35..34e92d7 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/CheckerRefsIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/CheckerRefsIT.java
@@ -28,7 +28,7 @@
import com.google.gerrit.plugins.checks.CheckerRef;
import com.google.gerrit.plugins.checks.CheckerUuid;
import com.google.gerrit.plugins.checks.api.CheckerStatus;
-import com.google.gerrit.reviewdb.client.Branch;
+import com.google.gerrit.reviewdb.client.BranchNameKey;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.change.ChangeInserter;
@@ -112,7 +112,7 @@
String checkerRef = CheckerUuid.parse("foo:bar").toRefName();
allow(checkerRef, Permission.CREATE, adminGroupUuid());
- createBranch(Branch.nameKey(project, checkerRef));
+ createBranch(BranchNameKey.create(project, checkerRef));
// checker ref can be deleted in any project except All-Projects
TestRepository<InMemoryRepository> testRepo = cloneProject(project);
@@ -145,7 +145,7 @@
String checkerRef = CheckerUuid.parse("foo:bar").toRefName();
allow(checkerRef, Permission.CREATE, adminGroupUuid());
- createBranch(Branch.nameKey(project, checkerRef));
+ createBranch(BranchNameKey.create(project, checkerRef));
TestRepository<InMemoryRepository> repo = cloneProject(project, admin);
fetch(repo, checkerRef + ":checkerRef");
@@ -186,7 +186,7 @@
String checkerRef = CheckerUuid.parse("foo:bar").toRefName();
allow(checkerRef, Permission.CREATE, adminGroupUuid());
- createBranch(Branch.nameKey(project, checkerRef));
+ createBranch(BranchNameKey.create(project, checkerRef));
String changeId = createChangeWithoutCommitValidation(project, checkerRef);
@@ -230,7 +230,7 @@
String checkerRef = CheckerUuid.parse("foo:bar").toRefName();
allow(checkerRef, Permission.CREATE, adminGroupUuid());
- createBranch(Branch.nameKey(project, checkerRef));
+ createBranch(BranchNameKey.create(project, checkerRef));
TestRepository<InMemoryRepository> repo = cloneProject(project, admin);
fetch(repo, checkerRef + ":checkerRef");
@@ -269,7 +269,7 @@
String checkerRef = CheckerUuid.parse("foo:bar").toRefName();
allow(checkerRef, Permission.CREATE, adminGroupUuid());
- createBranch(Branch.nameKey(project, checkerRef));
+ createBranch(BranchNameKey.create(project, checkerRef));
TestRepository<InMemoryRepository> repo = cloneProject(project, admin);
fetch(repo, checkerRef + ":checkerRef");
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java
index b725cd1..bcbee51 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java
@@ -35,7 +35,6 @@
import com.google.gerrit.plugins.checks.api.CheckerStatus;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.gerrit.testing.TestTimeUtil;
import com.google.inject.Inject;
@@ -43,6 +42,7 @@
import java.time.Instant;
import java.util.Map;
import java.util.concurrent.TimeUnit;
+import org.eclipse.jgit.lib.ObjectId;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@@ -51,7 +51,7 @@
@Inject private RequestScopeOperations requestScopeOperations;
private PatchSet.Id patchSetId;
- private RevId revId;
+ private ObjectId commitId;
@Before
public void setUp() throws Exception {
@@ -59,8 +59,9 @@
TestTimeUtil.setClock(Timestamp.from(Instant.EPOCH));
patchSetId = createChange().getPatchSetId();
- revId =
- new RevId(gApi.changes().id(patchSetId.changeId().get()).current().commit(false).commit);
+ commitId =
+ ObjectId.fromString(
+ gApi.changes().id(patchSetId.changeId().get()).current().commit(false).commit);
}
@After
@@ -91,10 +92,10 @@
PerCheckOperations perCheckOps = checkOperations.check(key);
// TODO(gerrit-team) Add a Truth subject for the notes map
- Map<RevId, String> notes = perCheckOps.notesAsText();
+ Map<ObjectId, String> notes = perCheckOps.notesAsText();
assertThat(notes)
.containsExactly(
- revId,
+ commitId,
noteDbContent(checkerUuid.get(), expectedCreationTimestamp, expectedCreationTimestamp));
}
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImplTest.java b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImplTest.java
index 51910ad..d831d3d 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImplTest.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImplTest.java
@@ -35,7 +35,6 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.server.util.time.TimeUtil;
import java.io.IOException;
import java.sql.Timestamp;
@@ -492,9 +491,9 @@
CheckKey checkKey =
createCheckInServer(project, patchSetId, createArbitraryCheckInput(checkerUuid));
- ImmutableMap<RevId, String> notesAsText = checkOperations.check(checkKey).notesAsText();
- RevId expectedRevId = getRevId(patchSetId);
- assertThat(notesAsText).containsExactly(expectedRevId, readNoteText(checkKey));
+ ImmutableMap<ObjectId, String> notesAsText = checkOperations.check(checkKey).notesAsText();
+ ObjectId expectedCommitId = getCommitId(patchSetId);
+ assertThat(notesAsText).containsExactly(expectedCommitId, readNoteText(checkKey));
}
private String readNoteText(CheckKey checkKey) throws Exception {
@@ -506,7 +505,7 @@
checkNotNull(checkRef);
NoteMap notes = NoteMap.read(reader, rw.parseCommit(checkRef.getObjectId()));
- ObjectId noteId = notes.get(ObjectId.fromString(getRevId(checkKey.patchSet()).get()));
+ ObjectId noteId = notes.get(getCommitId(checkKey.patchSet()));
return new String(reader.open(noteId, OBJ_BLOB).getCachedBytes(Integer.MAX_VALUE), UTF_8);
}
}
@@ -560,8 +559,8 @@
return CheckKey.create(repositoryName, patchSetId, CheckerUuid.parse(checkInput.checkerUuid));
}
- private RevId getRevId(PatchSet.Id patchSetId) throws Exception {
- return new RevId(
+ private ObjectId getCommitId(PatchSet.Id patchSetId) throws Exception {
+ return ObjectId.fromString(
gApi.changes()
.id(patchSetId.changeId().get())
.revision(patchSetId.get())
diff --git a/javatests/com/google/gerrit/plugins/checks/rules/BUILD b/javatests/com/google/gerrit/plugins/checks/rules/BUILD
index 9ac3548..fa82ed4 100644
--- a/javatests/com/google/gerrit/plugins/checks/rules/BUILD
+++ b/javatests/com/google/gerrit/plugins/checks/rules/BUILD
@@ -11,6 +11,7 @@
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/testing:gerrit-test-util",
"//lib:guava",
+ "//lib/jgit/org.eclipse.jgit:jgit",
"//lib/truth",
"//plugins/checks:checks__plugin",
],
diff --git a/javatests/com/google/gerrit/plugins/checks/rules/ChecksSubmitRuleTest.java b/javatests/com/google/gerrit/plugins/checks/rules/ChecksSubmitRuleTest.java
index 8724299..f892b37 100644
--- a/javatests/com/google/gerrit/plugins/checks/rules/ChecksSubmitRuleTest.java
+++ b/javatests/com/google/gerrit/plugins/checks/rules/ChecksSubmitRuleTest.java
@@ -31,6 +31,7 @@
import com.google.gerrit.testing.GerritBaseTests;
import java.util.Collection;
import org.easymock.EasyMock;
+import org.eclipse.jgit.lib.ObjectId;
import org.junit.Test;
public class ChecksSubmitRuleTest extends GerritBaseTests {
@@ -63,7 +64,8 @@
ChangeData cd = EasyMock.createStrictMock(ChangeData.class);
expect(cd.project()).andReturn(Project.nameKey("My-Project"));
expect(cd.getId()).andReturn(Change.id(1));
- expect(cd.currentPatchSet()).andReturn(new PatchSet(PatchSet.id(changeId, 1)));
+ expect(cd.currentPatchSet())
+ .andReturn(new PatchSet(PatchSet.id(changeId, 1), ObjectId.zeroId()));
replay(cd);
Collection<SubmitRecord> submitRecords =
