Merge branch stable-2.12 * stable-2.12: Adapt to the JGit v4.5.x Change-Id: I45ca07a7bc98d3d8acb2d8af0516a42eef90e845
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewnotes/CreateReviewNotes.java b/src/main/java/com/googlesource/gerrit/plugins/reviewnotes/CreateReviewNotes.java index 72c9ec9..249700a 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/reviewnotes/CreateReviewNotes.java +++ b/src/main/java/com/googlesource/gerrit/plugins/reviewnotes/CreateReviewNotes.java
@@ -17,12 +17,10 @@ import com.google.gerrit.common.Nullable; import com.google.gerrit.common.data.LabelType; import com.google.gerrit.common.data.LabelTypes; -import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.Project; -import com.google.gerrit.reviewdb.client.RevId; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.GerritPersonIdent; @@ -31,12 +29,16 @@ import com.google.gerrit.server.config.AnonymousCowardName; import com.google.gerrit.server.config.CanonicalWebUrl; import com.google.gerrit.server.git.NotesBranchUtil; +import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectState; +import com.google.gerrit.server.query.change.ChangeData; +import com.google.gerrit.server.query.change.InternalChangeQuery; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; +import com.google.inject.Provider; import com.google.inject.assistedinject.Assisted; import org.eclipse.jgit.api.errors.ConcurrentRefUpdateException; @@ -78,8 +80,10 @@ private final LabelTypes labelTypes; private final ApprovalsUtil approvalsUtil; private final ChangeControl.GenericFactory changeControlFactory; + private final ChangeNotes.Factory notesFactory; private final IdentifiedUser.GenericFactory userFactory; private final NotesBranchUtil.Factory notesBranchUtilFactory; + private final Provider<InternalChangeQuery> queryProvider; private final String canonicalWebUrl; private final ReviewDb reviewDb; private final Project.NameKey project; @@ -90,18 +94,20 @@ private StringBuilder message; @Inject - CreateReviewNotes(@GerritPersonIdent final PersonIdent gerritIdent, - final AccountCache accountCache, - @AnonymousCowardName final String anonymousCowardName, - final ProjectCache projectCache, - final ApprovalsUtil approvalsUtil, - final ChangeControl.GenericFactory changeControlFactory, - final IdentifiedUser.GenericFactory userFactory, - final NotesBranchUtil.Factory notesBranchUtilFactory, - @Nullable @CanonicalWebUrl final String canonicalWebUrl, - @Assisted final ReviewDb reviewDb, - @Assisted final Project.NameKey project, - @Assisted final Repository git) { + CreateReviewNotes(@GerritPersonIdent PersonIdent gerritIdent, + AccountCache accountCache, + @AnonymousCowardName String anonymousCowardName, + ProjectCache projectCache, + ApprovalsUtil approvalsUtil, + ChangeControl.GenericFactory changeControlFactory, + ChangeNotes.Factory notesFactory, + IdentifiedUser.GenericFactory userFactory, + NotesBranchUtil.Factory notesBranchUtilFactory, + Provider<InternalChangeQuery> queryProvider, + @Nullable @CanonicalWebUrl String canonicalWebUrl, + @Assisted ReviewDb reviewDb, + @Assisted Project.NameKey project, + @Assisted Repository git) { this.gerritServerIdent = gerritIdent; this.accountCache = accountCache; this.anonymousCowardName = anonymousCowardName; @@ -115,8 +121,10 @@ } this.approvalsUtil = approvalsUtil; this.changeControlFactory = changeControlFactory; + this.notesFactory = notesFactory; this.userFactory = userFactory; this.notesBranchUtilFactory = notesBranchUtilFactory; + this.queryProvider = queryProvider; this.canonicalWebUrl = canonicalWebUrl; this.reviewDb = reviewDb; this.project = project; @@ -148,29 +156,38 @@ } for (RevCommit c : rw) { - ObjectId content = createNoteContent(loadPatchSet(c, branch)); - if (content != null) { - monitor.update(1); - getNotes().set(c, content); - getMessage().append("* ").append(c.getShortMessage()).append("\n"); + PatchSet ps = loadPatchSet(c, branch); + if (ps != null) { + ChangeNotes notes = notesFactory.create(reviewDb, project, + ps.getId().getParentKey()); + ObjectId content = createNoteContent(notes, ps); + if (content != null) { + monitor.update(1); + getNotes().set(c, content); + getMessage().append("* ").append(c.getShortMessage()).append("\n"); + } + } else { + log.debug("no note for this commit since it is a direct push: " + + c.getName().substring(0, 7)); } } } } - void createNotes(List<Change> changes, ProgressMonitor monitor) + void createNotes(List<ChangeNotes> notes, ProgressMonitor monitor) throws OrmException, IOException { try (RevWalk rw = new RevWalk(git)) { if (monitor == null) { monitor = NullProgressMonitor.INSTANCE; } - for (Change c : changes) { + for (ChangeNotes cn : notes) { monitor.update(1); - PatchSet ps = reviewDb.patchSets().get(c.currentPatchSetId()); + PatchSet ps = + reviewDb.patchSets().get(cn.getChange().currentPatchSetId()); ObjectId commitId = ObjectId.fromString(ps.getRevision().get()); RevCommit commit = rw.parseCommit(commitId); - getNotes().set(commitId, createNoteContent(ps)); + getNotes().set(commitId, createNoteContent(cn, ps)); getMessage().append("* ").append(commit.getShortMessage()).append("\n"); } } @@ -217,13 +234,13 @@ } } - private ObjectId createNoteContent(PatchSet ps) + private ObjectId createNoteContent(ChangeNotes notes, PatchSet ps) throws OrmException, IOException { HeaderFormatter fmt = new HeaderFormatter(gerritServerIdent.getTimeZone(), anonymousCowardName); if (ps != null) { try { - createCodeReviewNote(ps, fmt); + createCodeReviewNote(notes, ps, fmt); return getInserter().insert(Constants.OBJ_BLOB, fmt.toString().getBytes("UTF-8")); } catch (NoSuchChangeException e) { @@ -235,46 +252,34 @@ private PatchSet loadPatchSet(RevCommit c, String destBranch) throws OrmException { - List<PatchSet> patches = reviewDb.patchSets().byRevision(new RevId(c.name())) - .toList(); - if (patches.isEmpty()) { - return null; // TODO: createNoCodeReviewNote(branch, c, fmt); - } else if (patches.size() == 1) { - return patches.get(0); - } else { - Branch.NameKey dest = new Branch.NameKey(project, destBranch); - for (PatchSet ps : patches) { - Change.Id cid = ps.getId().getParentKey(); - Change change = reviewDb.changes().get(cid); - if (change.getDest().equals(dest)) { + String hash = c.name(); + for (ChangeData cd : queryProvider.get() + .byBranchCommit(project.get(), destBranch, hash)) { + for (PatchSet ps : cd.patchSets()) { + if (ps.getRevision().matches(hash)) { return ps; } } } - return null; + return null; // TODO: createNoCodeReviewNote(branch, c, fmt); } - private void createCodeReviewNote(PatchSet ps, HeaderFormatter fmt) - throws OrmException, NoSuchChangeException { - createCodeReviewNote( - reviewDb.changes().get(ps.getId().getParentKey()), ps, fmt); - } - - private void createCodeReviewNote(Change change, PatchSet ps, + private void createCodeReviewNote(ChangeNotes notes, PatchSet ps, HeaderFormatter fmt) throws OrmException, NoSuchChangeException { // This races with the label normalization/writeback done by MergeOp. It may // repeat some work, but results should be identical except in the case of // an additional race with a permissions change. // TODO(dborowitz): These will eventually be stamped in the ChangeNotes at // commit time so we will be able to skip this normalization step. + Change change = notes.getChange(); ChangeControl ctl = changeControlFactory.controlFor( - change, userFactory.create(change.getOwner())); + notes, userFactory.create(change.getOwner())); PatchSetApproval submit = null; for (PatchSetApproval a : approvalsUtil.byPatchSet(reviewDb, ctl, ps.getId())) { if (a.getValue() == 0) { // Ignore 0 values. - } else if (a.isSubmit()) { + } else if (a.isLegacySubmit()) { submit = a; } else { LabelType type = labelTypes.byLabel(a.getLabelId());
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewnotes/ExportReviewNotes.java b/src/main/java/com/googlesource/gerrit/plugins/reviewnotes/ExportReviewNotes.java index 2b1968c..e62e1fc 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/reviewnotes/ExportReviewNotes.java +++ b/src/main/java/com/googlesource/gerrit/plugins/reviewnotes/ExportReviewNotes.java
@@ -14,10 +14,15 @@ package com.googlesource.gerrit.plugins.reviewnotes; +import com.google.common.base.Predicate; +import com.google.common.collect.ImmutableListMultimap; +import com.google.common.collect.ListMultimap; +import com.google.common.collect.MultimapBuilder; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.git.GitRepositoryManager; +import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.sshd.SshCommand; import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.SchemaFactory; @@ -28,15 +33,11 @@ import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.TextProgressMonitor; import org.eclipse.jgit.lib.ThreadSafeProgressMonitor; -import org.eclipse.jgit.util.BlockList; import org.kohsuke.args4j.Option; import java.io.IOException; -import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Map.Entry; /** Export review notes for all submitted changes in all projects. */ public class ExportReviewNotes extends SshCommand { @@ -52,7 +53,10 @@ @Inject private CreateReviewNotes.Factory reviewNotesFactory; - private Map<Project.NameKey, List<Change>> changes; + @Inject + private ChangeNotes.Factory notesFactory; + + private ListMultimap<Project.NameKey, ChangeNotes> changes; private ThreadSafeProgressMonitor monitor; @Override @@ -61,12 +65,10 @@ threads = 1; } - List<Change> allChangeList = allChanges(); - monitor = new ThreadSafeProgressMonitor(new TextProgressMonitor(stdout)); - monitor.beginTask("Scanning changes", allChangeList.size()); - changes = cluster(allChangeList); - allChangeList = null; + changes = mergedChanges(); + monitor = new ThreadSafeProgressMonitor(new TextProgressMonitor(stdout)); + monitor.beginTask("Scanning merged changes", changes.size()); monitor.startWorkers(threads); for (int tid = 0; tid < threads; tid++) { new Worker().start(); @@ -75,37 +77,26 @@ monitor.endTask(); } - private List<Change> allChanges() { - try (ReviewDb db = database.open()){ - return db.changes().all().toList(); - } catch (OrmException e) { + private ListMultimap<Project.NameKey, ChangeNotes> mergedChanges() { + try (ReviewDb db = database.open()) { + return MultimapBuilder.hashKeys().arrayListValues() + .build(notesFactory.create(db, new Predicate<ChangeNotes>() { + @Override + public boolean apply(ChangeNotes notes) { + return notes.getChange().getStatus() == Change.Status.MERGED; + } + })); + } catch (OrmException | IOException e) { stderr.println("Cannot read changes from database " + e.getMessage()); - return Collections.emptyList(); + return ImmutableListMultimap.of(); } } - private Map<Project.NameKey, List<Change>> cluster(List<Change> changes) { - HashMap<Project.NameKey, List<Change>> m = new HashMap<>(); - for (Change change : changes) { - if (change.getStatus() == Change.Status.MERGED) { - List<Change> l = m.get(change.getProject()); - if (l == null) { - l = new BlockList<>(); - m.put(change.getProject(), l); - } - l.add(change); - } else { - monitor.update(1); - } - } - return m; - } - - private void export(ReviewDb db, Project.NameKey project, List<Change> changes) - throws IOException, OrmException { + private void export(ReviewDb db, Project.NameKey project, + List<ChangeNotes> notes) throws IOException, OrmException { try (Repository git = gitManager.openRepository(project)) { CreateReviewNotes crn = reviewNotesFactory.create(db, project, git); - crn.createNotes(changes, monitor); + crn.createNotes(notes, monitor); crn.commitNotes(); } catch (RepositoryNotFoundException e) { stderr.println("Unable to open project: " + project.get()); @@ -114,27 +105,27 @@ } } - private Map.Entry<Project.NameKey, List<Change>> next() { + private Map.Entry<Project.NameKey, List<ChangeNotes>> next() { synchronized (changes) { if (changes.isEmpty()) { return null; } final Project.NameKey name = changes.keySet().iterator().next(); - final List<Change> list = changes.remove(name); - return new Map.Entry<Project.NameKey, List<Change>>() { + final List<ChangeNotes> list = changes.removeAll(name); + return new Map.Entry<Project.NameKey, List<ChangeNotes>>() { @Override public Project.NameKey getKey() { return name; } @Override - public List<Change> getValue() { + public List<ChangeNotes> getValue() { return list; } @Override - public List<Change> setValue(List<Change> value) { + public List<ChangeNotes> setValue(List<ChangeNotes> value) { throw new UnsupportedOperationException(); } }; @@ -144,9 +135,9 @@ private class Worker extends Thread { @Override public void run() { - try (ReviewDb db = database.open()){ + try (ReviewDb db = database.open()) { for (;;) { - Entry<Project.NameKey, List<Change>> next = next(); + Map.Entry<Project.NameKey, List<ChangeNotes>> next = next(); if (next != null) { try { export(db, next.getKey(), next.getValue());