diff --git a/Documentation/cmd-review.txt b/Documentation/cmd-review.txt index 0590337240..c3d86512e0 100644 --- a/Documentation/cmd-review.txt +++ b/Documentation/cmd-review.txt @@ -19,6 +19,7 @@ gerrit review - Apply reviews to one or more patch sets [--delete] [--verified ] [--code-review ] [--label Label-Name=] + [--tag TAG] {COMMIT | CHANGEID,PATCHSET}... -- @@ -134,6 +135,15 @@ branch. permitted for the user, or the vote is on an outdated or closed patch set, return an error instead of silently discarding the vote. +--tag:: +-t:: + Apply a 'TAG' to the change message, votes, and inline comments. The 'TAG' + can represent an external system like CI that does automated verification + of the change. Comments with specific 'TAG' values can be filtered out in + the web UI. + NOTE: To apply different tags on on different votes/comments multiple + invocations of the SSH command are required. + == ACCESS Any user who has configured an SSH key. diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt index 51ba60fc5d..330eed8866 100644 --- a/Documentation/rest-api-changes.txt +++ b/Documentation/rest-api-changes.txt @@ -2721,6 +2721,7 @@ link:#review-input[ReviewInput] entity. Content-Type: application/json; charset=UTF-8 { + "tag": "jenkins", "message": "Some nits need to be fixed.", "labels": { "Code-Review": -1 @@ -3970,6 +3971,11 @@ user is permitted to vote on the label. If absent, the user is not permitted to vote on that label. |`date` |optional| The time and date describing when the approval was made. +|`tag` |optional| +Value of the `tag` field from link:#review-input[ReviewInput] set +while posting the review. +NOTE: To apply different tags on on different votes/comments multiple +invocations of the REST call are required. |=========================== [[change-edit-input]] @@ -4132,6 +4138,11 @@ Unset if written by the Gerrit system. |`date` || The link:rest-api.html#timestamp[timestamp] this message was posted. |`message` ||The text left by the user. +|`tag` |optional| +Value of the `tag` field from link:#review-input[ReviewInput] set +while posting the review. +NOTE: To apply different tags on on different votes/comments multiple +invocations of the REST call are required. |`_revision_number` |optional| Which patchset (if any) generated this message. |================================== @@ -4182,6 +4193,11 @@ written. The author of the message as an link:rest-api-accounts.html#account-info[AccountInfo] entity. + Unset for draft comments, assumed to be the calling user. +|`tag` |optional| +Value of the `tag` field from link:#review-input[ReviewInput] set +while posting the review. +NOTE: To apply different tags on on different votes/comments multiple +invocations of the REST call are required. |=========================== [[comment-input]] @@ -4715,6 +4731,11 @@ revision. |Field Name ||Description |`message` |optional| The message to be added as review comment. +|`tag` |optional| +Apply this tag to the review comment message, votes, and inline +comments. Tags may be used by CI or other automated systems to +distinguish them from human reviews. Comments with specific tag +values can be filtered out in the web UI. |`labels` |optional| The votes that should be added to the revision as a map that maps the label names to the voting values. diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeMessagesIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeMessagesIT.java index 91f2962360..40b0391444 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeMessagesIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeMessagesIT.java @@ -81,13 +81,34 @@ public class ChangeMessagesIT extends AbstractDaemonTest { assertMessage(secondMessage, it.next().message); } + @Test + public void postMessageWithTag() throws Exception { + String changeId = createChange().getChangeId(); + String tag = "jenkins"; + String msg = "Message with tag."; + postMessage(changeId, msg, tag); + ChangeInfo c = get(changeId); + assertThat(c.messages).isNotNull(); + assertThat(c.messages).hasSize(2); + Iterator it = c.messages.iterator(); + assertThat(it.next().message).isEqualTo("Uploaded patch set 1."); + ChangeMessageInfo actual = it.next(); + assertMessage(msg, actual.message); + assertThat(actual.tag).isEqualTo(tag); + } + private void assertMessage(String expected, String actual) { assertThat(actual).isEqualTo("Patch Set 1:\n\n" + expected); } private void postMessage(String changeId, String msg) throws Exception { + postMessage(changeId, msg, null); + } + + private void postMessage(String changeId, String msg, String tag) throws Exception { ReviewInput in = new ReviewInput(); in.message = msg; + in.tag = tag; gApi.changes().id(changeId).current().review(in); } } diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ReviewInput.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ReviewInput.java index e6ce6b2ee4..0ae0aa53ed 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ReviewInput.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ReviewInput.java @@ -26,6 +26,8 @@ public class ReviewInput { @DefaultInput public String message; + public String tag; + public Map labels; public Map> comments; diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ApprovalInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ApprovalInfo.java index ce120cd78b..6d28dbc0f6 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ApprovalInfo.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ApprovalInfo.java @@ -17,6 +17,7 @@ package com.google.gerrit.extensions.common; import java.sql.Timestamp; public class ApprovalInfo extends AccountInfo { + public String tag; public Integer value; public Timestamp date; diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeMessageInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeMessageInfo.java index 7ad40b5d8e..e79918ffcb 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeMessageInfo.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeMessageInfo.java @@ -18,6 +18,7 @@ import java.sql.Timestamp; public class ChangeMessageInfo { public String id; + public String tag; public AccountInfo author; public Timestamp date; public String message; diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/CommentInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/CommentInfo.java index b1f8183f57..b7535e11e9 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/CommentInfo.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/CommentInfo.java @@ -18,4 +18,5 @@ import com.google.gerrit.extensions.client.Comment; public class CommentInfo extends Comment { public AccountInfo author; + public String tag; } diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/ChangeMessage.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/ChangeMessage.java index 70169a0d86..898dc94ebd 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/ChangeMessage.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/ChangeMessage.java @@ -74,6 +74,10 @@ public final class ChangeMessage { @Column(id = 5, notNull = false) protected PatchSet.Id patchset; + /** Tag associated with change message */ + @Column(id = 6, notNull = false) + protected String tag; + protected ChangeMessage() { } @@ -117,6 +121,14 @@ public final class ChangeMessage { message = s; } + public String getTag() { + return tag; + } + + public void setTag(String tag) { + this.tag = tag; + } + public PatchSet.Id getPatchSetId() { return patchset; } @@ -132,6 +144,7 @@ public final class ChangeMessage { + ", author=" + author + ", writtenOn=" + writtenOn + ", patchset=" + patchset + + ", tag=" + tag + ", message=[" + message + "]}"; } diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchLineComment.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchLineComment.java index a4c5da5e91..31bebd74ee 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchLineComment.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchLineComment.java @@ -122,6 +122,9 @@ public final class PatchLineComment { @Column(id = 9, notNull = false) protected CommentRange range; + @Column(id = 10, notNull = false) + protected String tag; + /** * The RevId for the commit to which this comment is referring. * @@ -249,6 +252,14 @@ public final class PatchLineComment { return revId; } + public void setTag(String tag) { + this.tag = tag; + } + + public String getTag() { + return tag; + } + @Override public boolean equals(Object o) { if (o instanceof PatchLineComment) { @@ -262,7 +273,8 @@ public final class PatchLineComment { && Objects.equals(message, c.getMessage()) && Objects.equals(parentUuid, c.getParentUuid()) && Objects.equals(range, c.getRange()) - && Objects.equals(revId, c.getRevId()); + && Objects.equals(revId, c.getRevId()) + && Objects.equals(tag, c.getTag()); } return false; } @@ -289,6 +301,7 @@ public final class PatchLineComment { builder.append("range=").append(Objects.toString(range, "")) .append(','); builder.append("revId=").append(revId != null ? revId.get() : ""); + builder.append("tag=").append(Objects.toString(tag, "")); builder.append('}'); return builder.toString(); } diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java index c89be30761..1d0d29bc66 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java @@ -90,6 +90,9 @@ public final class PatchSetApproval { @Column(id = 3) protected Timestamp granted; + @Column(id = 6, notNull = false) + protected String tag; + // DELETED: id = 4 (changeOpen) // DELETED: id = 5 (changeSortKey) @@ -145,6 +148,10 @@ public final class PatchSetApproval { } } + public void setTag(String t) { + tag = t; + } + public String getLabel() { return getLabelId().get(); } @@ -153,10 +160,14 @@ public final class PatchSetApproval { return LabelId.LEGACY_SUBMIT_NAME.equals(getLabel()); } + public String getTag() { + return tag; + } + @Override public String toString() { return new StringBuilder().append('[').append(key).append(": ") - .append(value).append(']').toString(); + .append(value).append(",tag:").append(tag).append(']').toString(); } @Override @@ -165,13 +176,14 @@ public final class PatchSetApproval { PatchSetApproval p = (PatchSetApproval) o; return Objects.equals(key, p.key) && Objects.equals(value, p.value) - && Objects.equals(granted, p.granted); + && Objects.equals(granted, p.granted) + && Objects.equals(tag, p.tag); } return false; } @Override public int hashCode() { - return Objects.hash(key, value, granted); + return Objects.hash(key, value, granted, tag); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeMessagesUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeMessagesUtil.java index af953e0c05..91647ff973 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeMessagesUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeMessagesUtil.java @@ -76,6 +76,7 @@ public class ChangeMessagesUtil { "cannot store change message by %s in update by %s", changeMessage.getAuthor(), update.getAccountId()); update.setChangeMessage(changeMessage.getMessage()); + update.setTag(changeMessage.getTag()); db.changeMessages().insert(Collections.singleton(changeMessage)); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java index 0f68ee27d4..a0d3fac3ba 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java @@ -633,6 +633,7 @@ public class ChangeJson { continue; } Integer value; + String tag = null; Timestamp date = null; PatchSetApproval psa = current.get(accountId, lt.getName()); if (psa != null) { @@ -643,6 +644,7 @@ public class ChangeJson { // label. value = labelNormalizer.canVote(ctl, lt, accountId) ? 0 : null; } + tag = psa.getTag(); date = psa.getGranted(); } else { // Either the user cannot vote on this label, or they were added as a @@ -650,7 +652,8 @@ public class ChangeJson { // user can vote on this label. value = labelNormalizer.canVote(ctl, lt, accountId) ? 0 : null; } - addApproval(e.getValue().label(), approvalInfo(accountId, value, date)); + addApproval(e.getValue().label(), + approvalInfo(accountId, value, tag, date)); } } } @@ -708,7 +711,7 @@ public class ChangeJson { if (detailed) { for (Map.Entry entry : labels.entrySet()) { - ApprovalInfo ai = approvalInfo(accountId, 0, null); + ApprovalInfo ai = approvalInfo(accountId, 0, null, null); byLabel.put(entry.getKey(), ai); addApproval(entry.getValue().label(), ai); } @@ -724,6 +727,7 @@ public class ChangeJson { if (info != null) { info.value = Integer.valueOf(val); info.date = psa.getGranted(); + info.tag = psa.getTag(); } if (!standard) { continue; @@ -735,10 +739,12 @@ public class ChangeJson { return labels; } - private ApprovalInfo approvalInfo(Account.Id id, Integer value, Timestamp date) { + private ApprovalInfo approvalInfo(Account.Id id, Integer value, String tag, + Timestamp date) { ApprovalInfo ai = new ApprovalInfo(id.get()); ai.value = value; ai.date = date; + ai.tag = tag; accountLoader.put(ai); return ai; } @@ -816,6 +822,7 @@ public class ChangeJson { cmi.author = accountLoader.get(message.getAuthor()); cmi.date = message.getWrittenOn(); cmi.message = message.getMessage(); + cmi.tag = message.getTag(); cmi._revisionNumber = patchNum != null ? patchNum.get() : null; result.add(cmi); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CommentJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CommentJson.java index b155b84035..0af5656980 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CommentJson.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CommentJson.java @@ -136,6 +136,7 @@ class CommentJson { r.message = Strings.emptyToNull(c.getMessage()); r.updated = c.getWrittenOn(); r.range = toRange(c.getRange()); + r.tag = c.getTag(); if (loader != null) { r.author = loader.get(c.getAuthor()); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java index bae54f2786..0487c3ad69 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java @@ -444,6 +444,7 @@ public class PostReview implements RestModifyView e.setSide(c.side == Side.PARENT ? (short) 0 : (short) 1); setCommentRevId(e, patchListCache, ctx.getChange(), ps); e.setMessage(c.message); + e.setTag(in.tag); if (c.range != null) { e.setRange(new CommentRange( c.range.startLine, @@ -500,6 +501,7 @@ public class PostReview implements RestModifyView Map drafts = Maps.newHashMap(); for (PatchLineComment c : plcUtil.draftByChangeAuthor( ctx.getDb(), ctx.getNotes(), user.getAccountId())) { + c.setTag(in.tag); drafts.put(c.getKey().get(), c); } return drafts; @@ -528,6 +530,7 @@ public class PostReview implements RestModifyView PatchLineComment c, PatchSet ps) throws OrmException { c.setStatus(PatchLineComment.Status.PUBLISHED); c.setWrittenOn(ctx.getWhen()); + c.setTag(in.tag); setCommentRevId(c, patchListCache, ctx.getChange(), checkNotNull(ps)); return c; } @@ -607,6 +610,7 @@ public class PostReview implements RestModifyView } else if (c != null && c.getValue() != ent.getValue()) { c.setValue(ent.getValue()); c.setGranted(ctx.getWhen()); + c.setTag(in.tag); ups.add(c); addLabelDelta(normName, c.getValue()); oldApprovals.put(normName, previous.get(normName)); @@ -622,6 +626,7 @@ public class PostReview implements RestModifyView user.getAccountId(), lt.getLabelId()), ent.getValue(), ctx.getWhen()); + c.setTag(in.tag); c.setGranted(ctx.getWhen()); ups.add(c); addLabelDelta(normName, c.getValue()); @@ -656,6 +661,7 @@ public class PostReview implements RestModifyView ctx.getControl().getLabelTypes().getLabelTypes().get(0) .getLabelId()), (short) 0, ctx.getWhen()); + c.setTag(in.tag); c.setGranted(ctx.getWhen()); ups.add(c); } else { @@ -719,6 +725,7 @@ public class PostReview implements RestModifyView user.getAccountId(), ctx.getWhen(), psId); + message.setTag(in.tag); message.setMessage(String.format( "Patch Set %d:%s", psId.get(), diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java index 51b80cd797..9e43e218f9 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java @@ -222,13 +222,13 @@ public class ChangeBundle { // the Change with the state implied by a ChangeNotes. 101); checkColumns(ChangeMessage.Key.class, 1, 2); - checkColumns(ChangeMessage.class, 1, 2, 3, 4, 5); + checkColumns(ChangeMessage.class, 1, 2, 3, 4, 5, 6); checkColumns(PatchSet.Id.class, 1, 2); checkColumns(PatchSet.class, 1, 2, 3, 4, 5, 6, 8); checkColumns(PatchSetApproval.Key.class, 1, 2, 3); - checkColumns(PatchSetApproval.class, 1, 2, 3); + checkColumns(PatchSetApproval.class, 1, 2, 3, 6); checkColumns(PatchLineComment.Key.class, 1, 2); - checkColumns(PatchLineComment.class, 1, 2, 3, 4, 5, 6, 7, 8, 9); + checkColumns(PatchLineComment.class, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10); } private final Change change; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java index ce7d1fbbc6..23faf9319b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java @@ -74,6 +74,7 @@ public class ChangeNoteUtil { static final FooterKey FOOTER_SUBMITTED_WITH = new FooterKey("Submitted-with"); static final FooterKey FOOTER_TOPIC = new FooterKey("Topic"); + static final FooterKey FOOTER_TAG = new FooterKey("Tag"); private static final String AUTHOR = "Author"; private static final String BASE_PATCH_SET = "Base-for-patch-set"; @@ -84,6 +85,7 @@ public class ChangeNoteUtil { private static final String PATCH_SET = "Patch-set"; private static final String REVISION = "Revision"; private static final String UUID = "UUID"; + private static final String TAG = FOOTER_TAG.getName(); public static String changeRefName(Change.Id id) { StringBuilder r = new StringBuilder(); @@ -212,6 +214,14 @@ public class ChangeNoteUtil { } String uuid = parseStringField(note, curr, changeId, UUID); + + boolean hasTag = + (RawParseUtils.match(note, curr.value, TAG.getBytes(UTF_8))) != -1; + String tag = null; + if (hasTag) { + tag = parseStringField(note, curr, changeId, TAG); + } + int commentLength = parseCommentLength(note, curr, changeId); String message = RawParseUtils.decode( @@ -222,6 +232,7 @@ public class ChangeNoteUtil { new PatchLineComment.Key(new Patch.Key(psId, currentFileName), uuid), range.getEndLine(), aId, parentUUID, commentTime); plc.setMessage(message); + plc.setTag(tag); plc.setSide((short) (isForBase ? 0 : 1)); if (range.getStartCharacter() != -1) { plc.setRange(range); @@ -501,6 +512,10 @@ public class ChangeNoteUtil { appendHeaderField(writer, UUID, c.getKey().get()); + if (c.getTag() != null) { + appendHeaderField(writer, TAG, c.getTag()); + } + byte[] messageBytes = c.getMessage().getBytes(UTF_8); appendHeaderField(writer, LENGTH, Integer.toString(messageBytes.length)); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java index 7bf79601cb..8ec59422d2 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java @@ -25,6 +25,7 @@ import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_STATUS; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_SUBJECT; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_SUBMISSION_ID; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_SUBMITTED_WITH; +import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_TAG; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_TOPIC; import static com.google.gerrit.server.notedb.NoteDbTable.CHANGES; @@ -85,6 +86,7 @@ import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.NavigableSet; import java.util.Set; import java.util.TreeMap; @@ -113,6 +115,7 @@ class ChangeNotesParser implements AutoCloseable { String subject; String originalSubject; String submissionId; + String tag; PatchSet.Id currentPatchSetId; RevisionNoteMap revisionNoteMap; @@ -123,7 +126,7 @@ class ChangeNotesParser implements AutoCloseable { private final RevWalk walk; private final Repository repo; private final Map>> approvals; + Table, Optional>> approvals; private final List allChangeMessages; private final Multimap changeMessagesByPatchSet; @@ -206,6 +209,9 @@ class ChangeNotesParser implements AutoCloseable { boolean updateTs = commit.getParentCount() == 0; createdOn = ts; + parseTag(commit); + updateTs |= tag != null; + if (branch == null) { branch = parseBranch(commit); updateTs |= branch != null; @@ -405,6 +411,18 @@ class ChangeNotesParser implements AutoCloseable { } } + private void parseTag(RevCommit commit) throws ConfigInvalidException { + tag = null; + List tagLines = commit.getFooterLines(FOOTER_TAG); + if (tagLines.isEmpty()) { + return; + } else if (tagLines.size() == 1) { + tag = tagLines.get(0); + } else { + throw expectedOneFooter(FOOTER_TAG, tagLines); + } + } + private Change.Status parseStatus(RevCommit commit) throws ConfigInvalidException { List statusLines = commit.getFooterLines(FOOTER_STATUS); @@ -505,6 +523,7 @@ class ChangeNotesParser implements AutoCloseable { ts, psId); changeMessage.setMessage(changeMsgString); + changeMessage.setTag(tag); changeMessagesByPatchSet.put(psId, changeMessage); allChangeMessages.add(changeMessage); return changeMessage; @@ -570,36 +589,39 @@ class ChangeNotesParser implements AutoCloseable { throw pe; } - Table> curr = - getApprovalsTableIfNoVotePresent(psId, accountId, l.label()); + Entry label = Maps.immutableEntry(l.label(), tag); + Table, Optional> curr = + getApprovalsTableIfNoVotePresent(psId, accountId, label); if (curr != null) { - curr.put(accountId, l.label(), Optional.of(new PatchSetApproval( + PatchSetApproval psa = new PatchSetApproval( new PatchSetApproval.Key( psId, accountId, new LabelId(l.label())), l.value(), - ts))); + ts); + psa.setTag(tag); + curr.put(accountId, label, Optional.of(psa)); } } private void parseRemoveApproval(PatchSet.Id psId, Account.Id committerId, String line) throws ConfigInvalidException { Account.Id accountId; - String label; + Entry label; int s = line.indexOf(' '); if (s > 0) { - label = line.substring(1, s); + label = Maps.immutableEntry(line.substring(1, s), tag); PersonIdent ident = RawParseUtils.parsePersonIdent(line.substring(s + 1)); checkFooter(ident != null, FOOTER_LABEL, line); accountId = noteUtil.parseIdent(ident, id); } else { - label = line.substring(1); + label = Maps.immutableEntry(line.substring(1), tag); accountId = committerId; } try { - LabelType.checkNameInternal(label); + LabelType.checkNameInternal(label.getKey()); } catch (IllegalArgumentException e) { ConfigInvalidException pe = parseException("invalid %s: %s", FOOTER_LABEL, line); @@ -607,18 +629,18 @@ class ChangeNotesParser implements AutoCloseable { throw pe; } - Table> curr = + Table, Optional> curr = getApprovalsTableIfNoVotePresent(psId, accountId, label); if (curr != null) { curr.put(accountId, label, Optional. absent()); } } - private Table> + private Table, Optional> getApprovalsTableIfNoVotePresent(PatchSet.Id psId, Account.Id accountId, - String label) { + Entry label) { - Table> curr = + Table, Optional> curr = approvals.get(psId); if (curr != null) { if (curr.contains(accountId, label)) { @@ -626,11 +648,11 @@ class ChangeNotesParser implements AutoCloseable { } } else { curr = Tables.newCustomTable( - Maps.>> + Maps., Optional>> newHashMapWithExpectedSize(2), - new Supplier>>() { + new Supplier, Optional>>() { @Override - public Map> get() { + public Map, Optional> get() { return Maps.newLinkedHashMap(); } }); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java index b67fb59654..a947cb498b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java @@ -29,6 +29,7 @@ import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_STATUS; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_SUBJECT; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_SUBMISSION_ID; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_SUBMITTED_WITH; +import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_TAG; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_TOPIC; import static org.eclipse.jgit.lib.Constants.OBJ_BLOB; @@ -121,6 +122,7 @@ public class ChangeUpdate extends AbstractChangeUpdate { private String commit; private Set hashtags; private String changeMessage; + private String tag; private PatchSetState psState; private Iterable groups; private String pushCert; @@ -293,6 +295,10 @@ public class ChangeUpdate extends AbstractChangeUpdate { this.changeMessage = changeMessage; } + public void setTag(String tag) { + this.tag = tag; + } + public void putComment(PatchLineComment c) { verifyComment(c); createDraftUpdateIfNull(); @@ -393,6 +399,7 @@ public class ChangeUpdate extends AbstractChangeUpdate { RevisionNoteBuilder.Cache cache = new RevisionNoteBuilder.Cache(rnm); for (PatchLineComment c : comments) { + c.setTag(tag); cache.get(c.getRevId()).putComment(c); } if (pushCert != null) { @@ -526,6 +533,10 @@ public class ChangeUpdate extends AbstractChangeUpdate { addFooter(msg, FOOTER_HASHTAGS, comma.join(hashtags)); } + if (tag != null) { + addFooter(msg, FOOTER_TAG, tag); + } + if (groups != null) { addFooter(msg, FOOTER_GROUPS, comma.join(groups)); } @@ -621,7 +632,8 @@ public class ChangeUpdate extends AbstractChangeUpdate { && topic == null && commit == null && psState == null - && groups == null; + && groups == null + && tag == null; } ChangeDraftUpdate getDraftUpdate() { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java index 8ddc86d2bc..7602bf489b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java @@ -32,7 +32,7 @@ import java.util.List; /** A version of the database schema. */ public abstract class SchemaVersion { /** The current schema version. */ - public static final Class C = Schema_121.class; + public static final Class C = Schema_122.class; public static int getBinaryVersion() { return guessVersion(C); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_122.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_122.java new file mode 100644 index 0000000000..b5b799d4e6 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_122.java @@ -0,0 +1,27 @@ +// Copyright (C) 2016 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.schema; + +import com.google.inject.Inject; +import com.google.inject.Provider; + +public class Schema_122 extends SchemaVersion { + @Inject + Schema_122(Provider prior) { + super(prior); + } + + // Adds tag column +} diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeBundleTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeBundleTest.java index fd208528d9..df5dad13bb 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeBundleTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeBundleTest.java @@ -280,14 +280,14 @@ public class ChangeBundleTest { assertDiffs(b1, b2, "Differing numbers of ChangeMessages for Change.Id " + id + ":\n" + "ChangeMessage{key=" + id + ",uuid1, author=100," - + " writtenOn=2009-09-30 17:00:06.0, patchset=" + id + ",1," + + " writtenOn=2009-09-30 17:00:06.0, patchset=" + id + ",1, tag=null," + " message=[message 2]}\n" + "ChangeMessage{key=" + id + ",uuid2, author=100," - + " writtenOn=2009-09-30 17:00:12.0, patchset=" + id + ",1," + + " writtenOn=2009-09-30 17:00:12.0, patchset=" + id + ",1, tag=null," + " message=[null]}\n" + "--- vs. ---\n" + "ChangeMessage{key=" + id + ",uuid1, author=100," - + " writtenOn=2009-09-30 17:00:06.0, patchset=" + id + ",1," + + " writtenOn=2009-09-30 17:00:06.0, patchset=" + id + ",1, tag=null," + " message=[message 2]}"); } diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java index 2968a62ae7..dc00eaa547 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java @@ -414,6 +414,32 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest { assertParseFails(writeCommit(msg, serverIdent)); } + @Test + public void parseTag() throws Exception { + assertParseSucceeds("Update change\n" + + "\n" + + "Patch-Set: 1\n" + + "Branch: refs/heads/master\n" + + "Change-id: I577fb248e474018276351785930358ec0450e9f7\n" + + "Subject: Change subject\n" + + "Tag:\n"); + assertParseSucceeds("Update change\n" + + "\n" + + "Patch-Set: 1\n" + + "Branch: refs/heads/master\n" + + "Change-id: I577fb248e474018276351785930358ec0450e9f7\n" + + "Subject: Change subject\n" + + "Tag: jenkins\n"); + assertParseFails("Update change\n" + + "\n" + + "Patch-Set: 1\n" + + "Branch: refs/heads/master\n" + + "Change-id: I577fb248e474018276351785930358ec0450e9f7\n" + + "Subject: Change subject\n" + + "Tag: ci\n" + + "Tag: jenkins\n"); + } + private RevCommit writeCommit(String body) throws Exception { return writeCommit(body, noteUtil.newIdent( changeOwner.getAccount(), TimeUtil.nowTs(), serverIdent, diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java index 3a9e08a58e..ad2f50abde 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java @@ -69,6 +69,117 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { @Inject private DraftCommentNotes.Factory draftNotesFactory; + @Test + public void tagChangeMessage() throws Exception { + String tag = "jenkins"; + Change c = newChange(); + ChangeUpdate update = newUpdate(c, changeOwner); + update.setChangeMessage("verification from jenkins"); + update.setTag(tag); + update.commit(); + + ChangeNotes notes = newNotes(c); + + assertThat(notes.getChangeMessages()).hasSize(1); + assertThat(notes.getChangeMessages().get(0).getTag()).isEqualTo(tag); + } + + @Test + public void tagInlineCommenrts() throws Exception { + String tag = "jenkins"; + Change c = newChange(); + RevCommit commit = tr.commit().message("PS2").create(); + ChangeUpdate update = newUpdate(c, changeOwner); + update.putComment(newPublishedComment(c.currentPatchSetId(), "a.txt", + "uuid1", new CommentRange(1, 2, 3, 4), 1, changeOwner, null, + TimeUtil.nowTs(), "Comment", (short) 1, commit.name())); + update.setTag(tag); + update.commit(); + + ChangeNotes notes = newNotes(c); + + ImmutableListMultimap comments = notes.getComments(); + assertThat(comments).hasSize(1); + assertThat( + comments.entries().asList().get(0).getValue().getTag()) + .isEqualTo(tag); + } + + @Test + public void tagApprovals() throws Exception { + String tag1 = "jenkins"; + String tag2 = "ip"; + Change c = newChange(); + ChangeUpdate update = newUpdate(c, changeOwner); + update.putApproval("Verified", (short) -1); + update.setTag(tag1); + update.commit(); + + update = newUpdate(c, changeOwner); + update.putApproval("Verified", (short) 1); + update.setTag(tag2); + update.commit(); + + ChangeNotes notes = newNotes(c); + + ImmutableListMultimap approvals = + notes.getApprovals(); + assertThat(approvals).hasSize(2); + assertThat(approvals.entries().asList().get(0).getValue().getTag()) + .isEqualTo(tag1); + assertThat(approvals.entries().asList().get(1).getValue().getTag()) + .isEqualTo(tag2); + } + + @Test + public void multipleTags() throws Exception { + String ipTag = "ip"; + String coverageTag = "coverage"; + String integrationTag = "integration"; + Change c = newChange(); + + ChangeUpdate update = newUpdate(c, changeOwner); + update.putApproval("Verified", (short) -1); + update.setChangeMessage("integration verification"); + update.setTag(integrationTag); + update.commit(); + + RevCommit commit = tr.commit().message("PS2").create(); + update = newUpdate(c, changeOwner); + update.putComment(newPublishedComment(c.currentPatchSetId(), "a.txt", + "uuid1", new CommentRange(1, 2, 3, 4), 1, changeOwner, null, + TimeUtil.nowTs(), "Comment", (short) 1, commit.name())); + update.setChangeMessage("coverage verification"); + update.setTag(coverageTag); + update.commit(); + + update = newUpdate(c, changeOwner); + update.setChangeMessage("ip clear"); + update.setTag(ipTag); + update.commit(); + + ChangeNotes notes = newNotes(c); + + ImmutableListMultimap approvals = + notes.getApprovals(); + assertThat(approvals).hasSize(1); + PatchSetApproval approval = approvals.entries().asList().get(0).getValue(); + assertThat(approval.getTag()).isEqualTo(integrationTag); + assertThat(approval.getValue()).isEqualTo(-1); + + ImmutableListMultimap comments = + notes.getComments(); + assertThat(comments).hasSize(1); + assertThat(comments.entries().asList().get(0).getValue().getTag()) + .isEqualTo(coverageTag); + + ImmutableList messages = notes.getChangeMessages(); + assertThat(messages).hasSize(3); + assertThat(messages.get(0).getTag()).isEqualTo(integrationTag); + assertThat(messages.get(1).getTag()).isEqualTo(coverageTag); + assertThat(messages.get(2).getTag()).isEqualTo(ipTag); + } + @Test public void approvalsOnePatchSet() throws Exception { Change c = newChange(); diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/CommitMessageOutputTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/CommitMessageOutputTest.java index 907db6b229..47c8a03549 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/CommitMessageOutputTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/CommitMessageOutputTest.java @@ -272,6 +272,23 @@ public class CommitMessageOutputTest extends AbstractChangeNotesTest { update.getResult()); } + @Test + public void changeMessageWithTag() throws Exception { + Change c = newChange(); + ChangeUpdate update = newUpdate(c, changeOwner); + update.setChangeMessage("Change message with tag"); + update.setTag("jenkins"); + update.commit(); + + assertBodyEquals("Update patch set 1\n" + + "\n" + + "Change message with tag\n" + + "\n" + + "Patch-set: 1\n" + + "Tag: jenkins\n", + update.getResult()); + } + private RevCommit parseCommit(ObjectId id) throws Exception { if (id instanceof RevCommit) { return (RevCommit) id; diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java index 00cf53f36f..998d6e57fb 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java @@ -124,6 +124,9 @@ public class ReviewCommand extends SshCommand { + "specified can be applied to the given patch set(s)") private boolean strictLabels; + @Option(name = "--tag", aliases = "-t", usage = "applies a tag to the given review", metaVar = "TAG") + private String changeTag; + @Option(name = "--label", aliases = "-l", usage = "custom label(s) to assign", metaVar = "LABEL=VALUE") void addLabel(final String token) { LabelVote v = LabelVote.parseWithEquals(token); @@ -198,6 +201,9 @@ public class ReviewCommand extends SshCommand { if (rebaseChange) { throw error("json and rebase actions are mutually exclusive"); } + if (changeTag != null) { + throw error("json and tag actions are mutually exclusive"); + } } if (rebaseChange) { if (deleteDraftPatchSet) { @@ -268,6 +274,7 @@ public class ReviewCommand extends SshCommand { ReviewInput review = new ReviewInput(); review.message = Strings.emptyToNull(changeComment); + review.tag = Strings.emptyToNull(changeTag); review.notify = notify; review.labels = Maps.newTreeMap(); review.drafts = ReviewInput.DraftHandling.PUBLISH;