opendev/gerrit
Code Issues Proposed changes

Store topic of a change in Notedb

Browse Source 
This adds a 'Topic' footer to  the commit messages of the
refs/changes/XX/YYYY/meta notes branch when the topic changes.

Similar as for hashtags we cannot rebuild the history for topic
changes since the database only stores the current topic.

Change-Id: I5ca2c1ed6326ba03dfbee21f1a7d2328b73ad322
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2015-10-21 18:16:14 +02:00
parent 232c18da1b
commit aa0a3f93a0
9 changed files with 105 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
View File

@@ -250,6 +250,7 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
}
db.patchSets().insert(Collections.singleton(patchSet));
db.changes().insert(Collections.singleton(change));
update.setTopic(change.getTopic());
LabelTypes labelTypes = ctl.getProjectControl().getLabelTypes();
approvalsUtil.addReviewers(db, update, labelTypes, change,
patchSet, patchSetInfo, reviewers, Collections.<Account.Id> emptySet());

1
gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java
View File

@@ -116,6 +116,7 @@ public class PutTopic implements RestModifyView<ChangeResource, Input>,
oldTopicName, newTopicName);
}
change.setTopic(Strings.emptyToNull(newTopicName));
ctx.getChangeUpdate().setTopic(change.getTopic());
ChangeUtil.updated(change);
ctx.getDb().changes().update(Collections.singleton(change));

17
gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
View File

@@ -102,6 +102,7 @@ import com.google.gerrit.server.edit.ChangeEdit;
import com.google.gerrit.server.edit.ChangeEditUtil;
import com.google.gerrit.server.events.CommitReceivedEvent;
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.validators.CommitValidationException;
import com.google.gerrit.server.git.validators.CommitValidationMessage;
@@ -1796,6 +1797,16 @@ public class ReceiveCommits {
ins.getChange().getId(),
hashtagsFactory.create(new HashtagsInput(magicBranch.hashtags))
.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();
}
@@ -2242,11 +2253,15 @@ public class ReceiveCommits {
recipients.add(magicBranch.getMailRecipients());
approvals = magicBranch.labels;
Set<String> hashtags = magicBranch.hashtags;
ChangeNotes notes = changeCtl.getNotes().load();
if (!hashtags.isEmpty()) {
ChangeNotes notes = changeCtl.getNotes().load();
hashtags.addAll(notes.getHashtags());
update.setHashtags(hashtags);
}
if (magicBranch.topic != null
&& !magicBranch.topic.equals(notes.getChange().getTopic())) {
update.setTopic(magicBranch.topic);
}
}
recipients.add(getRecipientsFromFooters(accountResolver, newPatchSet, footerLines));
recipients.remove(me);

1
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
View File

@@ -33,6 +33,7 @@ public class ChangeNoteUtil {
static final FooterKey FOOTER_STATUS = new FooterKey("Status");
static final FooterKey FOOTER_SUBMITTED_WITH =
new FooterKey("Submitted-with");
static final FooterKey FOOTER_TOPIC = new FooterKey("Topic");
public static String changeRefName(Change.Id id) {
StringBuilder r = new StringBuilder();

2
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
View File

@@ -18,6 +18,7 @@ import static com.google.gerrit.server.notedb.ChangeNoteUtil.GERRIT_PLACEHOLDER_
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Function;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableSet;
@@ -264,6 +265,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
allChangeMessages = parser.buildAllMessages();
comments = ImmutableListMultimap.copyOf(parser.comments);
noteMap = parser.commentNoteMap;
change.setTopic(Strings.emptyToNull(parser.topic));
if (parser.hashtags != null) {
hashtags = ImmutableSet.copyOf(parser.hashtags);

17
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
View File

@@ -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_PATCH_SET;
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.GERRIT_PLACEHOLDER_HOST;
@@ -77,6 +78,7 @@ class ChangeNotesParser implements AutoCloseable {
final Multimap<RevId, PatchLineComment> comments;
NoteMap commentNoteMap;
Change.Status status;
String topic;
Set<String> hashtags;
private final Change.Id changeId;
@@ -155,6 +157,9 @@ class ChangeNotesParser implements AutoCloseable {
PatchSet.Id psId = parsePatchSetId(commit);
Account.Id accountId = parseIdent(commit);
parseChangeMessage(psId, accountId, commit);
if (topic == null) {
topic = parseTopic(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 {
// Commits are parsed in reverse order and only the last set of hashtags should be used.
if (hashtags != null) {

14
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
View File

@@ -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_STATUS;
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 com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Joiner;
import com.google.common.base.Optional;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
@@ -92,6 +94,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
private List<SubmitRecord> submitRecords;
private final CommentsInNotesUtil commentsUtil;
private List<PatchLineComment> comments;
private String topic;
private Set<String> hashtags;
private String changeMessage;
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) {
this.hashtags = hashtags;
}
@@ -418,6 +425,10 @@ public class ChangeUpdate extends AbstractChangeUpdate {
addFooter(msg, FOOTER_STATUS, status.name().toLowerCase());
}
if (topic != null) {
addFooter(msg, FOOTER_TOPIC, topic);
}
if (hashtags != null) {
addFooter(msg, FOOTER_HASHTAGS, Joiner.on(",").join(hashtags));
}
@@ -481,7 +492,8 @@ public class ChangeUpdate extends AbstractChangeUpdate {
&& status == null
&& subject == null
&& submitRecords == null
&& hashtags == null;
&& hashtags == null
&& topic == null;
}
private static StringBuilder addFooter(StringBuilder sb, FooterKey footer) {

17
gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
View File

@@ -168,6 +168,23 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest {
+ "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 {
return writeCommit(body, ChangeNoteUtil.newIdent(
changeOwner.getAccount(), TimeUtil.nowTs(), serverIdent,

37
gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
View File

@@ -370,6 +370,43 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
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
public void emptyExceptSubject() throws Exception {
ChangeUpdate update = newUpdate(newChange(), changeOwner);