Merge "Store topic of a change in Notedb"
This commit is contained in:
		| @@ -250,6 +250,7 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp { | |||||||
|     } |     } | ||||||
|     db.patchSets().insert(Collections.singleton(patchSet)); |     db.patchSets().insert(Collections.singleton(patchSet)); | ||||||
|     db.changes().insert(Collections.singleton(change)); |     db.changes().insert(Collections.singleton(change)); | ||||||
|  |     update.setTopic(change.getTopic()); | ||||||
|     LabelTypes labelTypes = ctl.getProjectControl().getLabelTypes(); |     LabelTypes labelTypes = ctl.getProjectControl().getLabelTypes(); | ||||||
|     approvalsUtil.addReviewers(db, update, labelTypes, change, |     approvalsUtil.addReviewers(db, update, labelTypes, change, | ||||||
|         patchSet, patchSetInfo, reviewers, Collections.<Account.Id> emptySet()); |         patchSet, patchSetInfo, reviewers, Collections.<Account.Id> emptySet()); | ||||||
|   | |||||||
| @@ -116,6 +116,7 @@ public class PutTopic implements RestModifyView<ChangeResource, Input>, | |||||||
|             oldTopicName, newTopicName); |             oldTopicName, newTopicName); | ||||||
|       } |       } | ||||||
|       change.setTopic(Strings.emptyToNull(newTopicName)); |       change.setTopic(Strings.emptyToNull(newTopicName)); | ||||||
|  |       ctx.getChangeUpdate().setTopic(change.getTopic()); | ||||||
|       ChangeUtil.updated(change); |       ChangeUtil.updated(change); | ||||||
|       ctx.getDb().changes().update(Collections.singleton(change)); |       ctx.getDb().changes().update(Collections.singleton(change)); | ||||||
|  |  | ||||||
|   | |||||||
| @@ -101,6 +101,7 @@ import com.google.gerrit.server.edit.ChangeEdit; | |||||||
| import com.google.gerrit.server.edit.ChangeEditUtil; | import com.google.gerrit.server.edit.ChangeEditUtil; | ||||||
| import com.google.gerrit.server.events.CommitReceivedEvent; | import com.google.gerrit.server.events.CommitReceivedEvent; | ||||||
| import com.google.gerrit.server.extensions.events.GitReferenceUpdated; | import com.google.gerrit.server.extensions.events.GitReferenceUpdated; | ||||||
|  | import com.google.gerrit.server.git.BatchUpdate.ChangeContext; | ||||||
| import com.google.gerrit.server.git.MultiProgressMonitor.Task; | import com.google.gerrit.server.git.MultiProgressMonitor.Task; | ||||||
| import com.google.gerrit.server.git.validators.CommitValidationException; | import com.google.gerrit.server.git.validators.CommitValidationException; | ||||||
| import com.google.gerrit.server.git.validators.CommitValidationMessage; | import com.google.gerrit.server.git.validators.CommitValidationMessage; | ||||||
| @@ -1795,6 +1796,16 @@ public class ReceiveCommits { | |||||||
|               ins.getChange().getId(), |               ins.getChange().getId(), | ||||||
|               hashtagsFactory.create(new HashtagsInput(magicBranch.hashtags)) |               hashtagsFactory.create(new HashtagsInput(magicBranch.hashtags)) | ||||||
|                 .setRunHooks(false)); |                 .setRunHooks(false)); | ||||||
|  |           if (!Strings.isNullOrEmpty(magicBranch.topic)) { | ||||||
|  |             bu.addOp( | ||||||
|  |                 ins.getChange().getId(), | ||||||
|  |                 new BatchUpdate.Op() { | ||||||
|  |                   @Override | ||||||
|  |                   public void updateChange(ChangeContext ctx) throws Exception { | ||||||
|  |                     ctx.getChangeUpdate().setTopic(magicBranch.topic); | ||||||
|  |                   } | ||||||
|  |                 }); | ||||||
|  |           } | ||||||
|         } |         } | ||||||
|         bu.execute(); |         bu.execute(); | ||||||
|       } |       } | ||||||
| @@ -2241,11 +2252,15 @@ public class ReceiveCommits { | |||||||
|         recipients.add(magicBranch.getMailRecipients()); |         recipients.add(magicBranch.getMailRecipients()); | ||||||
|         approvals = magicBranch.labels; |         approvals = magicBranch.labels; | ||||||
|         Set<String> hashtags = magicBranch.hashtags; |         Set<String> hashtags = magicBranch.hashtags; | ||||||
|  |         ChangeNotes notes = changeCtl.getNotes().load(); | ||||||
|         if (!hashtags.isEmpty()) { |         if (!hashtags.isEmpty()) { | ||||||
|           ChangeNotes notes = changeCtl.getNotes().load(); |  | ||||||
|           hashtags.addAll(notes.getHashtags()); |           hashtags.addAll(notes.getHashtags()); | ||||||
|           update.setHashtags(hashtags); |           update.setHashtags(hashtags); | ||||||
|         } |         } | ||||||
|  |         if (magicBranch.topic != null | ||||||
|  |             && !magicBranch.topic.equals(notes.getChange().getTopic())) { | ||||||
|  |           update.setTopic(magicBranch.topic); | ||||||
|  |         } | ||||||
|       } |       } | ||||||
|       recipients.add(getRecipientsFromFooters(accountResolver, newPatchSet, footerLines)); |       recipients.add(getRecipientsFromFooters(accountResolver, newPatchSet, footerLines)); | ||||||
|       recipients.remove(me); |       recipients.remove(me); | ||||||
|   | |||||||
| @@ -33,6 +33,7 @@ public class ChangeNoteUtil { | |||||||
|   static final FooterKey FOOTER_STATUS = new FooterKey("Status"); |   static final FooterKey FOOTER_STATUS = new FooterKey("Status"); | ||||||
|   static final FooterKey FOOTER_SUBMITTED_WITH = |   static final FooterKey FOOTER_SUBMITTED_WITH = | ||||||
|       new FooterKey("Submitted-with"); |       new FooterKey("Submitted-with"); | ||||||
|  |   static final FooterKey FOOTER_TOPIC = new FooterKey("Topic"); | ||||||
|  |  | ||||||
|   public static String changeRefName(Change.Id id) { |   public static String changeRefName(Change.Id id) { | ||||||
|     StringBuilder r = new StringBuilder(); |     StringBuilder r = new StringBuilder(); | ||||||
|   | |||||||
| @@ -18,6 +18,7 @@ import static com.google.gerrit.server.notedb.ChangeNoteUtil.GERRIT_PLACEHOLDER_ | |||||||
|  |  | ||||||
| import com.google.common.annotations.VisibleForTesting; | import com.google.common.annotations.VisibleForTesting; | ||||||
| import com.google.common.base.Function; | import com.google.common.base.Function; | ||||||
|  | import com.google.common.base.Strings; | ||||||
| import com.google.common.collect.ImmutableList; | import com.google.common.collect.ImmutableList; | ||||||
| import com.google.common.collect.ImmutableListMultimap; | import com.google.common.collect.ImmutableListMultimap; | ||||||
| import com.google.common.collect.ImmutableSet; | import com.google.common.collect.ImmutableSet; | ||||||
| @@ -264,6 +265,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> { | |||||||
|       allChangeMessages = parser.buildAllMessages(); |       allChangeMessages = parser.buildAllMessages(); | ||||||
|       comments = ImmutableListMultimap.copyOf(parser.comments); |       comments = ImmutableListMultimap.copyOf(parser.comments); | ||||||
|       noteMap = parser.commentNoteMap; |       noteMap = parser.commentNoteMap; | ||||||
|  |       change.setTopic(Strings.emptyToNull(parser.topic)); | ||||||
|  |  | ||||||
|       if (parser.hashtags != null) { |       if (parser.hashtags != null) { | ||||||
|         hashtags = ImmutableSet.copyOf(parser.hashtags); |         hashtags = ImmutableSet.copyOf(parser.hashtags); | ||||||
|   | |||||||
| @@ -18,6 +18,7 @@ import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_HASHTAGS; | |||||||
| import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_LABEL; | import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_LABEL; | ||||||
| import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PATCH_SET; | import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PATCH_SET; | ||||||
| import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_STATUS; | import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_STATUS; | ||||||
|  | import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_TOPIC; | ||||||
| import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_SUBMITTED_WITH; | import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_SUBMITTED_WITH; | ||||||
| import static com.google.gerrit.server.notedb.ChangeNoteUtil.GERRIT_PLACEHOLDER_HOST; | import static com.google.gerrit.server.notedb.ChangeNoteUtil.GERRIT_PLACEHOLDER_HOST; | ||||||
|  |  | ||||||
| @@ -77,6 +78,7 @@ class ChangeNotesParser implements AutoCloseable { | |||||||
|   final Multimap<RevId, PatchLineComment> comments; |   final Multimap<RevId, PatchLineComment> comments; | ||||||
|   NoteMap commentNoteMap; |   NoteMap commentNoteMap; | ||||||
|   Change.Status status; |   Change.Status status; | ||||||
|  |   String topic; | ||||||
|   Set<String> hashtags; |   Set<String> hashtags; | ||||||
|  |  | ||||||
|   private final Change.Id changeId; |   private final Change.Id changeId; | ||||||
| @@ -155,6 +157,9 @@ class ChangeNotesParser implements AutoCloseable { | |||||||
|     PatchSet.Id psId = parsePatchSetId(commit); |     PatchSet.Id psId = parsePatchSetId(commit); | ||||||
|     Account.Id accountId = parseIdent(commit); |     Account.Id accountId = parseIdent(commit); | ||||||
|     parseChangeMessage(psId, accountId, commit); |     parseChangeMessage(psId, accountId, commit); | ||||||
|  |     if (topic == null) { | ||||||
|  |       topic = parseTopic(commit); | ||||||
|  |     } | ||||||
|     parseHashtags(commit); |     parseHashtags(commit); | ||||||
|  |  | ||||||
|  |  | ||||||
| @@ -175,6 +180,18 @@ class ChangeNotesParser implements AutoCloseable { | |||||||
|     } |     } | ||||||
|   } |   } | ||||||
|  |  | ||||||
|  |   private String parseTopic(RevCommit commit) | ||||||
|  |       throws ConfigInvalidException { | ||||||
|  |     List<String> topicLines = commit.getFooterLines(FOOTER_TOPIC); | ||||||
|  |     if (topicLines.isEmpty()) { | ||||||
|  |       return null; | ||||||
|  |     } else if (topicLines.size() > 1) { | ||||||
|  |       throw expectedOneFooter(FOOTER_TOPIC, topicLines); | ||||||
|  |     } | ||||||
|  |     return topicLines.get(0); | ||||||
|  |   } | ||||||
|  |  | ||||||
|  |  | ||||||
|   private void parseHashtags(RevCommit commit) throws ConfigInvalidException { |   private void parseHashtags(RevCommit commit) throws ConfigInvalidException { | ||||||
|     // Commits are parsed in reverse order and only the last set of hashtags should be used. |     // Commits are parsed in reverse order and only the last set of hashtags should be used. | ||||||
|     if (hashtags != null) { |     if (hashtags != null) { | ||||||
|   | |||||||
| @@ -20,11 +20,13 @@ import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_LABEL; | |||||||
| import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PATCH_SET; | import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PATCH_SET; | ||||||
| import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_STATUS; | import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_STATUS; | ||||||
| import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_SUBMITTED_WITH; | import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_SUBMITTED_WITH; | ||||||
|  | import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_TOPIC; | ||||||
| import static com.google.gerrit.server.notedb.CommentsInNotesUtil.addCommentToMap; | import static com.google.gerrit.server.notedb.CommentsInNotesUtil.addCommentToMap; | ||||||
|  |  | ||||||
| import com.google.common.annotations.VisibleForTesting; | import com.google.common.annotations.VisibleForTesting; | ||||||
| import com.google.common.base.Joiner; | import com.google.common.base.Joiner; | ||||||
| import com.google.common.base.Optional; | import com.google.common.base.Optional; | ||||||
|  | import com.google.common.base.Strings; | ||||||
| import com.google.common.collect.ImmutableList; | import com.google.common.collect.ImmutableList; | ||||||
| import com.google.common.collect.Lists; | import com.google.common.collect.Lists; | ||||||
| import com.google.common.collect.Maps; | import com.google.common.collect.Maps; | ||||||
| @@ -92,6 +94,7 @@ public class ChangeUpdate extends AbstractChangeUpdate { | |||||||
|   private List<SubmitRecord> submitRecords; |   private List<SubmitRecord> submitRecords; | ||||||
|   private final CommentsInNotesUtil commentsUtil; |   private final CommentsInNotesUtil commentsUtil; | ||||||
|   private List<PatchLineComment> comments; |   private List<PatchLineComment> comments; | ||||||
|  |   private String topic; | ||||||
|   private Set<String> hashtags; |   private Set<String> hashtags; | ||||||
|   private String changeMessage; |   private String changeMessage; | ||||||
|   private ChangeNotes notes; |   private ChangeNotes notes; | ||||||
| @@ -316,6 +319,10 @@ public class ChangeUpdate extends AbstractChangeUpdate { | |||||||
|  |  | ||||||
|   } |   } | ||||||
|  |  | ||||||
|  |   public void setTopic(String topic) { | ||||||
|  |     this.topic = Strings.nullToEmpty(topic); | ||||||
|  |   } | ||||||
|  |  | ||||||
|   public void setHashtags(Set<String> hashtags) { |   public void setHashtags(Set<String> hashtags) { | ||||||
|     this.hashtags = hashtags; |     this.hashtags = hashtags; | ||||||
|   } |   } | ||||||
| @@ -418,6 +425,10 @@ public class ChangeUpdate extends AbstractChangeUpdate { | |||||||
|       addFooter(msg, FOOTER_STATUS, status.name().toLowerCase()); |       addFooter(msg, FOOTER_STATUS, status.name().toLowerCase()); | ||||||
|     } |     } | ||||||
|  |  | ||||||
|  |     if (topic != null) { | ||||||
|  |       addFooter(msg, FOOTER_TOPIC, topic); | ||||||
|  |     } | ||||||
|  |  | ||||||
|     if (hashtags != null) { |     if (hashtags != null) { | ||||||
|       addFooter(msg, FOOTER_HASHTAGS, Joiner.on(",").join(hashtags)); |       addFooter(msg, FOOTER_HASHTAGS, Joiner.on(",").join(hashtags)); | ||||||
|     } |     } | ||||||
| @@ -481,7 +492,8 @@ public class ChangeUpdate extends AbstractChangeUpdate { | |||||||
|         && status == null |         && status == null | ||||||
|         && subject == null |         && subject == null | ||||||
|         && submitRecords == null |         && submitRecords == null | ||||||
|         && hashtags == null; |         && hashtags == null | ||||||
|  |         && topic == null; | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   private static StringBuilder addFooter(StringBuilder sb, FooterKey footer) { |   private static StringBuilder addFooter(StringBuilder sb, FooterKey footer) { | ||||||
|   | |||||||
| @@ -168,6 +168,23 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest { | |||||||
|         + "Reviewer: 1@gerrit\n"); |         + "Reviewer: 1@gerrit\n"); | ||||||
|   } |   } | ||||||
|  |  | ||||||
|  |   @Test | ||||||
|  |   public void parseTopic() throws Exception { | ||||||
|  |     assertParseSucceeds("Update change\n" | ||||||
|  |         + "\n" | ||||||
|  |         + "Patch-Set: 1\n" | ||||||
|  |         + "Topic: Some Topic"); | ||||||
|  |     assertParseSucceeds("Update change\n" | ||||||
|  |         + "\n" | ||||||
|  |         + "Patch-Set: 1\n" | ||||||
|  |         + "Topic:"); | ||||||
|  |     assertParseFails("Update change\n" | ||||||
|  |         + "\n" | ||||||
|  |         + "Patch-Set: 1\n" | ||||||
|  |         + "Topic: Some Topic\n" | ||||||
|  |         + "Topic: Other Topic"); | ||||||
|  |   } | ||||||
|  |  | ||||||
|   private RevCommit writeCommit(String body) throws Exception { |   private RevCommit writeCommit(String body) throws Exception { | ||||||
|     return writeCommit(body, ChangeNoteUtil.newIdent( |     return writeCommit(body, ChangeNoteUtil.newIdent( | ||||||
|         changeOwner.getAccount(), TimeUtil.nowTs(), serverIdent, |         changeOwner.getAccount(), TimeUtil.nowTs(), serverIdent, | ||||||
|   | |||||||
| @@ -370,6 +370,43 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { | |||||||
|     assertThat(notes.getHashtags()).isEqualTo(hashtags); |     assertThat(notes.getHashtags()).isEqualTo(hashtags); | ||||||
|   } |   } | ||||||
|  |  | ||||||
|  |   @Test | ||||||
|  |   public void topicChangeNotes() throws Exception { | ||||||
|  |     Change c = newChange(); | ||||||
|  |     ChangeUpdate update = newUpdate(c, changeOwner); | ||||||
|  |  | ||||||
|  |     // initially topic is not set | ||||||
|  |     ChangeNotes notes = newNotes(c); | ||||||
|  |     notes = newNotes(c); | ||||||
|  |     assertThat(notes.getChange().getTopic()).isNull(); | ||||||
|  |  | ||||||
|  |     // set topic | ||||||
|  |     String topic = "myTopic"; | ||||||
|  |     update.setTopic(topic); | ||||||
|  |     update.commit(); | ||||||
|  |     notes = newNotes(c); | ||||||
|  |     assertThat(notes.getChange().getTopic()).isEqualTo(topic); | ||||||
|  |  | ||||||
|  |     // clear topic by setting empty string | ||||||
|  |     update.setTopic(""); | ||||||
|  |     update.commit(); | ||||||
|  |     notes = newNotes(c); | ||||||
|  |     assertThat(notes.getChange().getTopic()).isNull(); | ||||||
|  |  | ||||||
|  |     // set other topic | ||||||
|  |     topic = "otherTopic"; | ||||||
|  |     update.setTopic(topic); | ||||||
|  |     update.commit(); | ||||||
|  |     notes = newNotes(c); | ||||||
|  |     assertThat(notes.getChange().getTopic()).isEqualTo(topic); | ||||||
|  |  | ||||||
|  |     // clear topic by setting null | ||||||
|  |     update.setTopic(null); | ||||||
|  |     update.commit(); | ||||||
|  |     notes = newNotes(c); | ||||||
|  |     assertThat(notes.getChange().getTopic()).isNull(); | ||||||
|  |   } | ||||||
|  |  | ||||||
|   @Test |   @Test | ||||||
|   public void emptyExceptSubject() throws Exception { |   public void emptyExceptSubject() throws Exception { | ||||||
|     ChangeUpdate update = newUpdate(newChange(), changeOwner); |     ChangeUpdate update = newUpdate(newChange(), changeOwner); | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 Edwin Kempin
					Edwin Kempin