diff options
author | Shawn Pearce <sop@google.com> | 2011-05-18 19:39:38 -0700 |
---|---|---|
committer | Android Code Review <code-review@android.com> | 2011-05-18 19:39:38 -0700 |
commit | 92f56cd95b9897eae3b6908670a80f5530a4c39d (patch) | |
tree | 1d0a5d8b41606f23bd14099ec8f79bc672da8e71 | |
parent | 21ccdc018e78bff77877352c5d6f520a43f7ff02 (diff) | |
parent | 3a64ecdc2fbe458406f4ec7db0f71ab41e37e0a4 (diff) |
Merge "Create Git notes for submitted changes." into stable
5 files changed, 448 insertions, 14 deletions
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java index 15e266546c..0480931ac2 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java @@ -22,6 +22,7 @@ import com.google.gerrit.server.RequestCleanup; import com.google.gerrit.server.account.AccountResolver; import com.google.gerrit.server.account.GroupControl; import com.google.gerrit.server.account.PerformCreateGroup; +import com.google.gerrit.server.git.CreateCodeReviewNotes; import com.google.gerrit.server.git.MergeOp; import com.google.gerrit.server.git.ReceiveCommits; import com.google.gerrit.server.mail.AbandonedSender; @@ -57,6 +58,7 @@ public class GerritRequestModule extends FactoryModule { factory(ChangeQueryBuilder.Factory.class); factory(ReceiveCommits.Factory.class); factory(MergeOp.Factory.class); + factory(CreateCodeReviewNotes.Factory.class); // Not really per-request, but dammit, I don't know where else to // easily park this stuff. diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/CodeReviewNoteCreationException.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/CodeReviewNoteCreationException.java new file mode 100644 index 0000000000..90de785871 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/CodeReviewNoteCreationException.java @@ -0,0 +1,35 @@ +// Copyright (C) 2011 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.gerrit.server.git; + +/** + * Thrown when creation of a code review note fails. + */ +public class CodeReviewNoteCreationException extends Exception { + private static final long serialVersionUID = 1L; + + public CodeReviewNoteCreationException(final String msg) { + super(msg); + } + + public CodeReviewNoteCreationException(final Throwable why) { + super(why); + } + + public CodeReviewNoteCreationException(final CodeReviewCommit commit, + final Throwable cause) { + super("Couldn't create code review note for the following commit: " + + commit, cause); + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/CreateCodeReviewNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/CreateCodeReviewNotes.java new file mode 100644 index 0000000000..67705e407d --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/CreateCodeReviewNotes.java @@ -0,0 +1,260 @@ +// Copyright (C) 2010 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.git; + +import com.google.gerrit.common.data.ApprovalTypes; +import com.google.gerrit.reviewdb.ApprovalCategory; +import com.google.gerrit.reviewdb.PatchSetApproval; +import com.google.gerrit.reviewdb.ReviewDb; +import com.google.gerrit.server.GerritPersonIdent; +import com.google.gerrit.server.account.AccountCache; +import com.google.gerrit.server.config.CanonicalWebUrl; +import com.google.gwtorm.client.OrmException; +import com.google.gwtorm.client.ResultSet; +import com.google.inject.Inject; +import com.google.inject.assistedinject.Assisted; + +import org.eclipse.jgit.lib.CommitBuilder; +import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.ObjectInserter; +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.lib.RefUpdate.Result; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.notes.DefaultNoteMerger; +import org.eclipse.jgit.notes.Note; +import org.eclipse.jgit.notes.NoteMap; +import org.eclipse.jgit.notes.NoteMapMerger; +import org.eclipse.jgit.revwalk.FooterKey; +import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; + +import java.io.IOException; +import java.util.List; + +/** + * This class create code review notes for given {@link CodeReviewCommit}s. + * <p> + * After the {@link #create(List, PersonIdent)} method is invoked once this + * instance must not be reused. Create a new instance of this class if needed. + */ +public class CreateCodeReviewNotes { + public interface Factory { + CreateCodeReviewNotes create(Repository db); + } + + static final String REFS_NOTES_REVIEW = "refs/notes/review"; + private static final int MAX_LOCK_FAILURE_CALLS = 10; + private static final int SLEEP_ON_LOCK_FAILURE_MS = 25; + private static final FooterKey CHANGE_ID = new FooterKey("Change-Id"); + + private final ReviewDb schema; + private final PersonIdent gerritIdent; + private final AccountCache accountCache; + private final ApprovalTypes approvalTypes; + private final String canonicalWebUrl; + private final Repository db; + private final RevWalk revWalk; + private final ObjectInserter inserter; + private final ObjectReader reader; + + private RevCommit baseCommit; + private NoteMap base; + + private RevCommit oursCommit; + private NoteMap ours; + + private List<CodeReviewCommit> commits; + private PersonIdent author; + + @Inject + CreateCodeReviewNotes(final ReviewDb reviewDb, + @GerritPersonIdent final PersonIdent gerritIdent, + final AccountCache accountCache, + final ApprovalTypes approvalTypes, + @CanonicalWebUrl final String canonicalWebUrl, + @Assisted final Repository db) { + schema = reviewDb; + this.gerritIdent = gerritIdent; + this.accountCache = accountCache; + this.approvalTypes = approvalTypes; + this.canonicalWebUrl = canonicalWebUrl; + this.db = db; + + revWalk = new RevWalk(db); + inserter = db.newObjectInserter(); + reader = db.newObjectReader(); + } + + public void create(List<CodeReviewCommit> commits, PersonIdent author) + throws CodeReviewNoteCreationException { + try { + this.commits = commits; + this.author = author; + setBase(); + setOurs(); + + int remainingLockFailureCalls = MAX_LOCK_FAILURE_CALLS; + RefUpdate refUpdate = createRefUpdate(oursCommit, baseCommit); + + for (;;) { + Result result = refUpdate.update(); + + if (result == Result.LOCK_FAILURE) { + if (--remainingLockFailureCalls > 0) { + Thread.sleep(SLEEP_ON_LOCK_FAILURE_MS); + } else { + throw new CodeReviewNoteCreationException( + "Failed to lock the ref: " + REFS_NOTES_REVIEW); + } + + } else if (result == Result.REJECTED) { + RevCommit theirsCommit = + revWalk.parseCommit(refUpdate.getOldObjectId()); + NoteMap theirs = + NoteMap.read(revWalk.getObjectReader(), theirsCommit); + NoteMapMerger merger = new NoteMapMerger(db); + NoteMap merged = merger.merge(base, ours, theirs); + RevCommit mergeCommit = + createCommit(merged, gerritIdent, "Merged note commits\n", + theirsCommit, oursCommit); + refUpdate = createRefUpdate(mergeCommit, theirsCommit); + remainingLockFailureCalls = MAX_LOCK_FAILURE_CALLS; + + } else if (result == Result.IO_FAILURE) { + throw new CodeReviewNoteCreationException( + "Couldn't create code review notes because of IO_FAILURE"); + } else { + break; + } + } + + } catch (IOException e) { + throw new CodeReviewNoteCreationException(e); + } catch (InterruptedException e) { + throw new CodeReviewNoteCreationException(e); + } finally { + reader.release(); + inserter.release(); + revWalk.release(); + } + } + + private void setBase() throws IOException { + Ref notesBranch = db.getRef(REFS_NOTES_REVIEW); + if (notesBranch != null) { + baseCommit = revWalk.parseCommit(notesBranch.getObjectId()); + base = NoteMap.read(revWalk.getObjectReader(), baseCommit); + } + } + + private void setOurs() throws IOException, CodeReviewNoteCreationException { + if (baseCommit != null) { + ours = NoteMap.read(db.newObjectReader(), baseCommit); + } else { + ours = NoteMap.newEmptyMap(); + } + + StringBuilder message = + new StringBuilder("Update notes for submitted changes\n\n"); + for (CodeReviewCommit c : commits) { + ObjectId noteContent = createNoteContent(c); + if (ours.contains(c)) { + // merge the existing and the new note as if they are both new + // means: base == null + // there is not really a common ancestry for these two note revisions + // use the same NoteMerger that is used from the NoteMapMerger + DefaultNoteMerger noteMerger = new DefaultNoteMerger(); + Note newNote = new Note(c, noteContent); + noteContent = + noteMerger.merge(null, newNote, base.getNote(c), reader, inserter) + .getData(); + } + ours.set(c, noteContent); + + message.append("* ").append(c.getShortMessage()).append("\n"); + } + + if (baseCommit != null) { + oursCommit = createCommit(ours, author, message.toString(), baseCommit); + } else { + oursCommit = createCommit(ours, author, message.toString()); + } + } + + private ObjectId createNoteContent(CodeReviewCommit commit) + throws CodeReviewNoteCreationException, IOException { + try { + ReviewNoteHeaderFormatter formatter = + new ReviewNoteHeaderFormatter(author.getTimeZone()); + final List<String> idList = commit.getFooterLines(CHANGE_ID); + if (idList.isEmpty()) + formatter.appendChangeId(commit.change.getKey()); + ResultSet<PatchSetApproval> approvals = + schema.patchSetApprovals().byPatchSet(commit.patchsetId); + PatchSetApproval submit = null; + for (PatchSetApproval a : approvals) { + if (ApprovalCategory.SUBMIT.equals(a.getCategoryId())) { + submit = a; + } else { + formatter.appendApproval( + approvalTypes.getApprovalType(a.getCategoryId()).getCategory(), + a.getValue(), + accountCache.get(a.getAccountId()).getAccount()); + } + } + + formatter.appendSubmittedBy(accountCache.get(submit.getAccountId()).getAccount()); + formatter.appendSubmittedAt(submit.getGranted()); + + formatter.appendReviewedOn(canonicalWebUrl, commit.change.getId()); + formatter.appendProject(commit.change.getProject().get()); + formatter.appendBranch(commit.change.getDest()); + return inserter.insert(Constants.OBJ_BLOB, formatter.toString().getBytes("UTF-8")); + } catch (OrmException e) { + throw new CodeReviewNoteCreationException(commit, e); + } + } + + private RevCommit createCommit(NoteMap map, PersonIdent author, + String message, RevCommit... parents) throws IOException { + CommitBuilder b = new CommitBuilder(); + b.setTreeId(map.writeTree(inserter)); + b.setAuthor(author); + b.setCommitter(gerritIdent); + if (parents.length > 0) { + b.setParentIds(parents); + } + b.setMessage(message); + ObjectId commitId = inserter.insert(b); + inserter.flush(); + return revWalk.parseCommit(commitId); + } + + + private RefUpdate createRefUpdate(ObjectId newObjectId, ObjectId expectedOldObjectId) throws IOException { + RefUpdate refUpdate = db.updateRef(REFS_NOTES_REVIEW); + refUpdate.setNewObjectId(newObjectId); + if (expectedOldObjectId == null) { + refUpdate.setExpectedOldObjectId(ObjectId.zeroId()); + } else { + refUpdate.setExpectedOldObjectId(expectedOldObjectId); + } + return refUpdate; + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java index f8b287c214..60016e5a03 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java @@ -156,6 +156,7 @@ public class MergeOp { private final ChangeHookRunner hooks; private final AccountCache accountCache; + private final CreateCodeReviewNotes.Factory codeReviewNotesFactory; @Inject MergeOp(final GitRepositoryManager grm, final SchemaFactory<ReviewDb> sf, @@ -167,7 +168,8 @@ public class MergeOp { final IdentifiedUser.GenericFactory iuf, @GerritPersonIdent final PersonIdent myIdent, final MergeQueue mergeQueue, @Assisted final Branch.NameKey branch, - final ChangeHookRunner hooks, final AccountCache accountCache) { + final ChangeHookRunner hooks, final AccountCache accountCache, + final CreateCodeReviewNotes.Factory crnf) { repoManager = grm; schemaFactory = sf; functionState = fs; @@ -182,6 +184,7 @@ public class MergeOp { this.mergeQueue = mergeQueue; this.hooks = hooks; this.accountCache = accountCache; + codeReviewNotesFactory = crnf; this.myIdent = myIdent; destBranch = branch; @@ -527,8 +530,22 @@ public class MergeOp { } } + PersonIdent authorIdent = computeAuthor(merged); + + final CommitBuilder mergeCommit = new CommitBuilder(); + mergeCommit.setTreeId(m.getResultTreeId()); + mergeCommit.setParentIds(mergeTip, n); + mergeCommit.setAuthor(authorIdent); + mergeCommit.setCommitter(myIdent); + mergeCommit.setMessage(msgbuf.toString()); + + mergeTip = (CodeReviewCommit) rw.parseCommit(commit(m, mergeCommit)); + } + + private PersonIdent computeAuthor( + final List<CodeReviewCommit> codeReviewCommits) { PatchSetApproval submitter = null; - for (final CodeReviewCommit c : merged) { + for (final CodeReviewCommit c : codeReviewCommits) { PatchSetApproval s = getSubmitter(c.patchsetId); if (submitter == null || (s != null && s.getGranted().compareTo(submitter.getGranted()) > 0)) { @@ -547,7 +564,7 @@ public class MergeOp { IdentifiedUser who = identifiedUserFactory.create(submitter.getAccountId()); Set<String> emails = new HashSet<String>(); - for (RevCommit c : merged) { + for (RevCommit c : codeReviewCommits) { emails.add(c.getAuthorIdent().getEmailAddress()); } @@ -555,22 +572,15 @@ public class MergeOp { final TimeZone tz = myIdent.getTimeZone(); if (emails.size() == 1 && who.getEmailAddresses().contains(emails.iterator().next())) { - authorIdent = new PersonIdent(merged.get(0).getAuthorIdent(), dt, tz); + authorIdent = + new PersonIdent(codeReviewCommits.get(0).getAuthorIdent(), dt, tz); } else { authorIdent = who.newCommitterIdent(dt, tz); } } else { authorIdent = myIdent; } - - final CommitBuilder mergeCommit = new CommitBuilder(); - mergeCommit.setTreeId(m.getResultTreeId()); - mergeCommit.setParentIds(mergeTip, n); - mergeCommit.setAuthor(authorIdent); - mergeCommit.setCommitter(myIdent); - mergeCommit.setMessage(msgbuf.toString()); - - mergeTip = (CodeReviewCommit) rw.parseCommit(commit(m, mergeCommit)); + return authorIdent; } private void markCleanMerges() throws MergeException { @@ -899,7 +909,9 @@ public class MergeOp { } } - private void updateChangeStatus() { + private void updateChangeStatus() throws MergeException { + List<CodeReviewCommit> merged = new ArrayList<CodeReviewCommit>(); + for (final Change c : submitted) { final CodeReviewCommit commit = commits.get(c.getId()); final CommitMergeStatus s = commit != null ? commit.statusCode : null; @@ -915,6 +927,7 @@ public class MergeOp { final String txt = "Change has been successfully merged into the git repository."; setMerged(c, message(c, txt)); + merged.add(commit); break; } @@ -923,11 +936,13 @@ public class MergeOp { "Change has been successfully cherry-picked as " + commit.name() + "."; setMerged(c, message(c, txt)); + merged.add(commit); break; } case ALREADY_MERGED: setMerged(c, null); + merged.add(commit); break; case PATH_CONFLICT: { @@ -976,6 +991,15 @@ public class MergeOp { break; } } + + CreateCodeReviewNotes codeReviewNotes = codeReviewNotesFactory.create(db); + try { + codeReviewNotes.create(merged, computeAuthor(merged)); + } catch (CodeReviewNoteCreationException e) { + log.error(e.getMessage()); + } + replication.scheduleUpdate(destBranch.getParentKey(), + CreateCodeReviewNotes.REFS_NOTES_REVIEW); } private void dependencyError(final CodeReviewCommit commit) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReviewNoteHeaderFormatter.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReviewNoteHeaderFormatter.java new file mode 100644 index 0000000000..e803f180fb --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReviewNoteHeaderFormatter.java @@ -0,0 +1,113 @@ +// Copyright (C) 2010 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.git; + +import com.google.gerrit.reviewdb.Account; +import com.google.gerrit.reviewdb.ApprovalCategory; +import com.google.gerrit.reviewdb.Branch; +import com.google.gerrit.reviewdb.Change; +import com.google.gerrit.reviewdb.Project; + +import java.text.DateFormat; +import java.text.SimpleDateFormat; +import java.util.Calendar; +import java.util.Date; +import java.util.Locale; +import java.util.TimeZone; + +/** + * Formatters for code review note headers. + * <p> + * This class provides a builder like interface for building the content of a + * code review note. After instantiation, call as many as necessary + * <code>append...(...)</code> methods and, at the end, call the + * {@link #toString()} method to get the built note content. + */ +class ReviewNoteHeaderFormatter { + + private final DateFormat rfc2822DateFormatter; + private final StringBuilder sb = new StringBuilder(); + + ReviewNoteHeaderFormatter(TimeZone tz) { + rfc2822DateFormatter = + new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss Z", Locale.US); + rfc2822DateFormatter.setCalendar(Calendar.getInstance(tz, Locale.US)); + } + + void appendChangeId(Change.Key changeKey) { + sb.append("Change-Id: ").append(changeKey.get()).append("\n"); + } + + void appendApproval(ApprovalCategory category, + short value, Account user) { + // TODO: use category.getLabel() when available + sb.append(category.getName().replace(' ', '-')); + sb.append(value < 0 ? "-" : "+").append(Math.abs(value)).append(": "); + appendUserData(user); + sb.append("\n"); + } + + private void appendUserData(Account user) { + boolean needSpace = false; + boolean wroteData = false; + + if (user.getFullName() != null && ! user.getFullName().isEmpty()) { + sb.append(user.getFullName()); + needSpace = true; + wroteData = true; + } + + if (user.getPreferredEmail() != null && ! user.getPreferredEmail().isEmpty()) { + if (needSpace) { + sb.append(" "); + } + sb.append("<").append(user.getPreferredEmail()).append(">"); + wroteData = true; + } + + if (!wroteData) { + sb.append("Anonymous Coward #").append(user.getId()); + } + } + + void appendProject(String projectName) { + sb.append("Project: ").append(projectName).append("\n"); + } + + void appendBranch(Branch.NameKey branch) { + sb.append("Branch: ").append(branch.get()).append("\n"); + } + + void appendSubmittedBy(Account user) { + sb.append("Submitted-by: "); + appendUserData(user); + sb.append("\n"); + } + + void appendSubmittedAt(Date date) { + sb.append("Submitted-at: ").append(rfc2822DateFormatter.format(date)) + .append("\n"); + } + + void appendReviewedOn(String canonicalWebUrl, Change.Id changeId) { + sb.append("Reviewed-on: ").append(canonicalWebUrl).append(changeId.get()) + .append("\n"); + } + + @Override + public String toString() { + return sb.toString(); + } +} |