Store subject and original subject in notedb

This adds a 'Subject' footer to the commit message of the
refs/changes/XX/YYYY/meta notes branch when the change is created and
whenever a new patch set with a different subject is added.

The current subject is taken from most recent commit on the notes
branch that contains a 'Subject' footer. The original subject is taken
from the first commit on the notes branch.

Tests which manually insert new changes must be adapted so that the
initial commit to the notes branch contains the subject.

In order to set the 'Subject' footer only when the subject was
changed, we must compare the new subject to the old subject when a new
patch set is inserted. To be able to access the old subject when a
a new patch set is created, PatchSetInserter must ensure that the body
of the commit was parsed. Otherwise the subject would not be available
and trying to get it would fail with a NullPointerException.

Change-Id: I71bfe88e16c3e0b41530f399441b16e2cd44f59e
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2016-01-21 14:51:10 +01:00
parent e8618b8e55
commit 12fbf41e95
16 changed files with 144 additions and 52 deletions

View File

@@ -585,6 +585,7 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest {
ChangeUpdate u = changeUpdateFactory.create( ChangeUpdate u = changeUpdateFactory.create(
changeControlFactory.controlFor(c, userFactory.create(adminId))); changeControlFactory.controlFor(c, userFactory.create(adminId)));
u.setSubject(c.getSubject());
u.setBranch(c.getDest().get()); u.setBranch(c.getDest().get());
u.commit(); u.commit();

View File

@@ -563,10 +563,18 @@ public final class Change {
return subject; return subject;
} }
public void setSubject(String subject) {
this.subject = subject;
}
public String getOriginalSubject() { public String getOriginalSubject() {
return originalSubject != null ? originalSubject : subject; return originalSubject != null ? originalSubject : subject;
} }
public void setOriginalSubject(String originalSubject) {
this.originalSubject = originalSubject;
}
/** Get the id of the most current {@link PatchSet} in this change. */ /** Get the id of the most current {@link PatchSet} in this change. */
public PatchSet.Id currentPatchSetId() { public PatchSet.Id currentPatchSetId() {
if (currentPatchSetId > 0) { if (currentPatchSetId > 0) {

View File

@@ -28,7 +28,7 @@ import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Singleton; import com.google.inject.Singleton;
import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.revwalk.RevCommit;
import java.sql.Timestamp; import java.sql.Timestamp;
import java.util.Collections; import java.util.Collections;
@@ -61,7 +61,7 @@ public class PatchSetUtil {
} }
public PatchSet insert(ReviewDb db, ChangeUpdate update, PatchSet.Id psId, public PatchSet insert(ReviewDb db, ChangeUpdate update, PatchSet.Id psId,
ObjectId id, boolean draft, Iterable<String> groups, RevCommit commit, boolean draft, Iterable<String> groups,
String pushCertificate) throws OrmException { String pushCertificate) throws OrmException {
Change.Id changeId = update.getChange().getId(); Change.Id changeId = update.getChange().getId();
checkArgument(psId.getParentKey().equals(changeId), checkArgument(psId.getParentKey().equals(changeId),
@@ -75,7 +75,7 @@ public class PatchSetUtil {
} }
PatchSet ps = new PatchSet(psId); PatchSet ps = new PatchSet(psId);
ps.setRevision(new RevId(id.name())); ps.setRevision(new RevId(commit.name()));
ps.setUploader(update.getUser().getAccountId()); ps.setUploader(update.getUser().getAccountId());
ps.setCreatedOn(new Timestamp(update.getWhen().getTime())); ps.setCreatedOn(new Timestamp(update.getWhen().getTime()));
ps.setDraft(draft); ps.setDraft(draft);
@@ -83,6 +83,10 @@ public class PatchSetUtil {
ps.setPushCertificate(pushCertificate); ps.setPushCertificate(pushCertificate);
db.patchSets().insert(Collections.singleton(ps)); db.patchSets().insert(Collections.singleton(ps));
if (!update.getChange().getSubject().equals(commit.getShortMessage())) {
update.setSubject(commit.getShortMessage());
}
if (migration.writeChanges()) { if (migration.writeChanges()) {
// TODO(dborowitz): Write to notedb. // TODO(dborowitz): Write to notedb.
} }

View File

@@ -296,7 +296,8 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
ctx.getChange().setCurrentPatchSet(patchSetInfo); ctx.getChange().setCurrentPatchSet(patchSetInfo);
ChangeUpdate update = ctx.getUpdate(psId); ChangeUpdate update = ctx.getUpdate(psId);
update.setSubject("Create change"); update.setSubjectForCommit("Create change");
update.setSubject(change.getSubject());
update.setBranch(change.getDest().get()); update.setBranch(change.getDest().get());
update.setTopic(change.getTopic()); update.setTopic(change.getTopic());

View File

@@ -210,7 +210,7 @@ public class PatchSetInserter extends BatchUpdate.Op {
change = ctx.getChange(); change = ctx.getChange();
ChangeUpdate update = ctx.getUpdate(psId); ChangeUpdate update = ctx.getUpdate(psId);
update.setSubject("Create patch set " + psId.get()); update.setSubjectForCommit("Create patch set " + psId.get());
if (!change.getStatus().isOpen() && !allowClosed) { if (!change.getStatus().isOpen() && !allowClosed) {
throw new InvalidChangeOperationException(String.format( throw new InvalidChangeOperationException(String.format(
@@ -222,6 +222,7 @@ public class PatchSetInserter extends BatchUpdate.Op {
PatchSet prevPs = psUtil.current(ctx.getDb(), ctx.getNotes()); PatchSet prevPs = psUtil.current(ctx.getDb(), ctx.getNotes());
newGroups = prevPs != null ? prevPs.getGroups() : null; newGroups = prevPs != null ? prevPs.getGroups() : null;
} }
ctx.getRevWalk().parseBody(commit);
patchSet = psUtil.insert(ctx.getDb(), ctx.getUpdate(psId), psId, commit, patchSet = psUtil.insert(ctx.getDb(), ctx.getUpdate(psId), psId, commit,
draft, newGroups, null); draft, newGroups, null);

View File

@@ -2241,7 +2241,7 @@ public class ReceiveCommits {
Map<String, Short> approvals = new HashMap<>(); Map<String, Short> approvals = new HashMap<>();
ChangeUpdate update = updateFactory.create(changeCtl, createdOn); ChangeUpdate update = updateFactory.create(changeCtl, createdOn);
update.setSubject("Create patch set " + psId.get()); update.setSubjectForCommit("Create patch set " + psId.get());
update.setPatchSetId(psId); update.setPatchSetId(psId);
if (magicBranch != null) { if (magicBranch != null) {

View File

@@ -33,6 +33,7 @@ public class ChangeNoteUtil {
static final FooterKey FOOTER_LABEL = new FooterKey("Label"); static final FooterKey FOOTER_LABEL = new FooterKey("Label");
static final FooterKey FOOTER_PATCH_SET = new FooterKey("Patch-set"); static final FooterKey FOOTER_PATCH_SET = new FooterKey("Patch-set");
static final FooterKey FOOTER_STATUS = new FooterKey("Status"); static final FooterKey FOOTER_STATUS = new FooterKey("Status");
static final FooterKey FOOTER_SUBJECT = new FooterKey("Subject");
static final FooterKey FOOTER_SUBMISSION_ID = new FooterKey("Submission-id"); static final FooterKey FOOTER_SUBMISSION_ID = new FooterKey("Submission-id");
static final FooterKey FOOTER_SUBMITTED_WITH = static final FooterKey FOOTER_SUBMITTED_WITH =
new FooterKey("Submitted-with"); new FooterKey("Submitted-with");

View File

@@ -266,6 +266,8 @@ 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.setOriginalSubject(parser.originalSubject);
change.setSubject(parser.subject);
change.setDest(new Branch.NameKey(getProjectName(), parser.branch)); change.setDest(new Branch.NameKey(getProjectName(), parser.branch));
change.setTopic(Strings.emptyToNull(parser.topic)); change.setTopic(Strings.emptyToNull(parser.topic));
change.setCreatedOn(parser.createdOn); change.setCreatedOn(parser.createdOn);

View File

@@ -19,6 +19,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_SUBJECT;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_SUBMISSION_ID; 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_SUBMITTED_WITH;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_TOPIC; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_TOPIC;
@@ -91,6 +92,8 @@ class ChangeNotesParser implements AutoCloseable {
Timestamp createdOn; Timestamp createdOn;
Timestamp lastUpdatedOn; Timestamp lastUpdatedOn;
Account.Id ownerId; Account.Id ownerId;
String subject;
String originalSubject;
String submissionId; String submissionId;
private final Change.Id changeId; private final Change.Id changeId;
@@ -177,6 +180,10 @@ class ChangeNotesParser implements AutoCloseable {
PatchSet.Id psId = parsePatchSetId(commit); PatchSet.Id psId = parsePatchSetId(commit);
Account.Id accountId = parseIdent(commit); Account.Id accountId = parseIdent(commit);
ownerId = accountId; ownerId = accountId;
if (subject == null) {
subject = parseSubject(commit);
}
originalSubject = parseSubject(commit);
parseChangeMessage(psId, accountId, commit); parseChangeMessage(psId, accountId, commit);
if (topic == null) { if (topic == null) {
topic = parseTopic(commit); topic = parseTopic(commit);
@@ -205,35 +212,31 @@ class ChangeNotesParser implements AutoCloseable {
private String parseSubmissionId(RevCommit commit) private String parseSubmissionId(RevCommit commit)
throws ConfigInvalidException { throws ConfigInvalidException {
List<String> submissionIdLines = commit.getFooterLines(FOOTER_SUBMISSION_ID); return parseOneFooter(commit, FOOTER_SUBMISSION_ID);
if (submissionIdLines.isEmpty()) {
return null;
} else if (submissionIdLines.size() > 1) {
throw expectedOneFooter(FOOTER_SUBMISSION_ID, submissionIdLines);
}
return submissionIdLines.get(0);
} }
private String parseBranch(RevCommit commit) private String parseBranch(RevCommit commit) throws ConfigInvalidException {
throws ConfigInvalidException { String branch = parseOneFooter(commit, FOOTER_BRANCH);
List<String> branchLines = commit.getFooterLines(FOOTER_BRANCH); return branch != null ? RefNames.fullName(branch) : null;
if (branchLines.isEmpty()) {
return null;
} else if (branchLines.size() > 1) {
throw expectedOneFooter(FOOTER_BRANCH, branchLines);
}
return RefNames.fullName(branchLines.get(0));
} }
private String parseTopic(RevCommit commit) private String parseSubject(RevCommit commit) throws ConfigInvalidException {
throws ConfigInvalidException { return parseOneFooter(commit, FOOTER_SUBJECT);
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 String parseTopic(RevCommit commit) throws ConfigInvalidException {
return parseOneFooter(commit, FOOTER_TOPIC);
}
private String parseOneFooter(RevCommit commit, FooterKey footerKey)
throws ConfigInvalidException {
List<String> footerLines = commit.getFooterLines(footerKey);
if (footerLines.isEmpty()) {
return null;
} else if (footerLines.size() > 1) {
throw expectedOneFooter(footerKey, footerLines);
}
return footerLines.get(0);
} }
private void parseHashtags(RevCommit commit) throws ConfigInvalidException { private void parseHashtags(RevCommit commit) throws ConfigInvalidException {
@@ -545,6 +548,9 @@ class ChangeNotesParser implements AutoCloseable {
if (branch == null) { if (branch == null) {
missing.add(FOOTER_BRANCH); missing.add(FOOTER_BRANCH);
} }
if (originalSubject == null || subject == null) {
missing.add(FOOTER_SUBJECT);
}
if (!missing.isEmpty()) { if (!missing.isEmpty()) {
throw parseException("Missing footers: " + Joiner.on(", ") throw parseException("Missing footers: " + Joiner.on(", ")
.join(Lists.transform(missing, new Function<FooterKey, String>() { .join(Lists.transform(missing, new Function<FooterKey, String>() {

View File

@@ -318,11 +318,12 @@ public class ChangeRebuilder {
@Override @Override
void apply(ChangeUpdate update) { void apply(ChangeUpdate update) {
checkUpdate(update); checkUpdate(update);
update.setSubject(change.getSubject());
if (ps.getPatchSetId() == 1) { if (ps.getPatchSetId() == 1) {
update.setSubject("Create change"); update.setSubjectForCommit("Create change");
update.setBranch(change.getDest().get()); update.setBranch(change.getDest().get());
} else { } else {
update.setSubject("Create patch set " + ps.getPatchSetId()); update.setSubjectForCommit("Create patch set " + ps.getPatchSetId());
} }
} }
} }

View File

@@ -20,6 +20,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_SUBJECT;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_SUBMISSION_ID; 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_SUBMITTED_WITH;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_TOPIC; import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_TOPIC;
@@ -93,11 +94,12 @@ public class ChangeUpdate extends AbstractChangeUpdate {
} }
private final AccountCache accountCache; private final AccountCache accountCache;
private String commitSubject;
private String subject;
private final Table<String, Account.Id, Optional<Short>> approvals; private final Table<String, Account.Id, Optional<Short>> approvals;
private final Map<Account.Id, ReviewerStateInternal> reviewers; private final Map<Account.Id, ReviewerStateInternal> reviewers;
private String branch; private String branch;
private Change.Status status; private Change.Status status;
private String subject;
private List<SubmitRecord> submitRecords; private List<SubmitRecord> submitRecords;
private String submissionId; private String submissionId;
private final CommentsInNotesUtil commentsUtil; private final CommentsInNotesUtil commentsUtil;
@@ -219,6 +221,10 @@ public class ChangeUpdate extends AbstractChangeUpdate {
"no submit records specified at submit time"); "no submit records specified at submit time");
} }
public void setSubjectForCommit(String commitSubject) {
this.commitSubject = commitSubject;
}
public void setSubject(String subject) { public void setSubject(String subject) {
this.subject = subject; this.subject = subject;
} }
@@ -435,8 +441,8 @@ public class ChangeUpdate extends AbstractChangeUpdate {
int ps = psId != null ? psId.get() : getChange().currentPatchSetId().get(); int ps = psId != null ? psId.get() : getChange().currentPatchSetId().get();
StringBuilder msg = new StringBuilder(); StringBuilder msg = new StringBuilder();
if (subject != null) { if (commitSubject != null) {
msg.append(subject); msg.append(commitSubject);
} else { } else {
msg.append("Update patch set ").append(ps); msg.append("Update patch set ").append(ps);
} }
@@ -450,6 +456,10 @@ public class ChangeUpdate extends AbstractChangeUpdate {
addFooter(msg, FOOTER_PATCH_SET, ps); addFooter(msg, FOOTER_PATCH_SET, ps);
if (subject != null) {
addFooter(msg, FOOTER_SUBJECT, subject);
}
if (branch != null) { if (branch != null) {
addFooter(msg, FOOTER_BRANCH, branch); addFooter(msg, FOOTER_BRANCH, branch);
} }
@@ -526,7 +536,8 @@ public class ChangeUpdate extends AbstractChangeUpdate {
} }
private boolean isEmpty() { private boolean isEmpty() {
return approvals.isEmpty() return commitSubject == null
&& approvals.isEmpty()
&& changeMessage == null && changeMessage == null
&& comments.isEmpty() && comments.isEmpty()
&& reviewers.isEmpty() && reviewers.isEmpty()

View File

@@ -322,6 +322,7 @@ public class CommentsTest extends GerritServerTests {
private Change newChange() throws Exception { private Change newChange() throws Exception {
Change c = TestChanges.newChange(project, changeOwner.getAccountId()); Change c = TestChanges.newChange(project, changeOwner.getAccountId());
ChangeUpdate u = newUpdate(c, changeOwner); ChangeUpdate u = newUpdate(c, changeOwner);
u.setSubject(c.getSubject());
u.setBranch(c.getDest().get()); u.setBranch(c.getDest().get());
u.commit(); u.commit();
return c; return c;

View File

@@ -166,6 +166,7 @@ public class AbstractChangeNotesTest extends GerritBaseTests {
throws IOException, OrmException, ConfigInvalidException { throws IOException, OrmException, ConfigInvalidException {
Change c = TestChanges.newChange(project, changeOwner.getAccountId()); Change c = TestChanges.newChange(project, changeOwner.getAccountId());
ChangeUpdate u = newUpdate(c, changeOwner); ChangeUpdate u = newUpdate(c, changeOwner);
u.setSubject(c.getSubject());
u.setBranch(c.getDest().get()); u.setBranch(c.getDest().get());
u.commit(); u.commit();
return c; return c;

View File

@@ -51,7 +51,8 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest {
assertParseSucceeds("Update change\n" assertParseSucceeds("Update change\n"
+ "\n" + "\n"
+ "Branch: refs/heads/master\n" + "Branch: refs/heads/master\n"
+ "Patch-Set: 1\n"); + "Patch-Set: 1\n"
+ "Subject: This is a test change\n");
assertParseFails(writeCommit("Update change\n" assertParseFails(writeCommit("Update change\n"
+ "\n" + "\n"
+ "Patch-Set: 1\n", + "Patch-Set: 1\n",
@@ -70,12 +71,14 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest {
+ "\n" + "\n"
+ "Branch: refs/heads/master\n" + "Branch: refs/heads/master\n"
+ "Patch-Set: 1\n" + "Patch-Set: 1\n"
+ "Status: NEW\n"); + "Status: NEW\n"
+ "Subject: This is a test change\n");
assertParseSucceeds("Update change\n" assertParseSucceeds("Update change\n"
+ "\n" + "\n"
+ "Branch: refs/heads/master\n" + "Branch: refs/heads/master\n"
+ "Patch-Set: 1\n" + "Patch-Set: 1\n"
+ "Status: new\n"); + "Status: new\n"
+ "Subject: This is a test change\n");
assertParseFails("Update change\n" assertParseFails("Update change\n"
+ "\n" + "\n"
+ "Patch-Set: 1\n" + "Patch-Set: 1\n"
@@ -92,7 +95,8 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest {
assertParseSucceeds("Update change\n" assertParseSucceeds("Update change\n"
+ "\n" + "\n"
+ "Branch: refs/heads/master\n" + "Branch: refs/heads/master\n"
+ "Patch-Set: 1\n"); + "Patch-Set: 1\n"
+ "Subject: This is a test change\n");
assertParseFails("Update change\n" assertParseFails("Update change\n"
+ "\n"); + "\n");
assertParseFails("Update change\n" assertParseFails("Update change\n"
@@ -102,7 +106,8 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest {
assertParseSucceeds("Update change\n" assertParseSucceeds("Update change\n"
+ "\n" + "\n"
+ "Branch: refs/heads/master\n" + "Branch: refs/heads/master\n"
+ "Patch-Set: 1\n"); + "Patch-Set: 1\n"
+ "Subject: This is a test change\n");
assertParseFails("Update change\n" assertParseFails("Update change\n"
+ "\n" + "\n"
+ "Patch-Set: x\n"); + "Patch-Set: x\n");
@@ -117,13 +122,15 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest {
+ "Label: Label1=+1\n" + "Label: Label1=+1\n"
+ "Label: Label2=1\n" + "Label: Label2=1\n"
+ "Label: Label3=0\n" + "Label: Label3=0\n"
+ "Label: Label4=-1\n"); + "Label: Label4=-1\n"
+ "Subject: This is a test change\n");
assertParseSucceeds("Update change\n" assertParseSucceeds("Update change\n"
+ "\n" + "\n"
+ "Branch: refs/heads/master\n" + "Branch: refs/heads/master\n"
+ "Patch-Set: 1\n" + "Patch-Set: 1\n"
+ "Label: -Label1\n" + "Label: -Label1\n"
+ "Label: -Label1 Other Account <2@gerrit>\n"); + "Label: -Label1 Other Account <2@gerrit>\n"
+ "Subject: This is a test change\n");
assertParseFails("Update change\n" assertParseFails("Update change\n"
+ "\n" + "\n"
+ "Patch-Set: 1\n" + "Patch-Set: 1\n"
@@ -156,6 +163,7 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest {
+ "\n" + "\n"
+ "Branch: refs/heads/master\n" + "Branch: refs/heads/master\n"
+ "Patch-Set: 1\n" + "Patch-Set: 1\n"
+ "Subject: This is a test change\n"
+ "Submitted-with: NOT_READY\n" + "Submitted-with: NOT_READY\n"
+ "Submitted-with: OK: Verified: Change Owner <1@gerrit>\n" + "Submitted-with: OK: Verified: Change Owner <1@gerrit>\n"
+ "Submitted-with: NEED: Code-Review\n" + "Submitted-with: NEED: Code-Review\n"
@@ -186,6 +194,7 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest {
+ "\n" + "\n"
+ "Branch: refs/heads/master\n" + "Branch: refs/heads/master\n"
+ "Patch-Set: 1\n" + "Patch-Set: 1\n"
+ "Subject: This is a test change\n"
+ "Submission-id: 1-1453387607626-96fabc25"); + "Submission-id: 1-1453387607626-96fabc25");
assertParseFails("Update change\n" assertParseFails("Update change\n"
+ "\n" + "\n"
@@ -201,7 +210,8 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest {
+ "Branch: refs/heads/master\n" + "Branch: refs/heads/master\n"
+ "Patch-Set: 1\n" + "Patch-Set: 1\n"
+ "Reviewer: Change Owner <1@gerrit>\n" + "Reviewer: Change Owner <1@gerrit>\n"
+ "CC: Other Account <2@gerrit>\n"); + "CC: Other Account <2@gerrit>\n"
+ "Subject: This is a test change\n");
assertParseFails("Update change\n" assertParseFails("Update change\n"
+ "\n" + "\n"
+ "Patch-Set: 1\n" + "Patch-Set: 1\n"
@@ -214,12 +224,14 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest {
+ "\n" + "\n"
+ "Branch: refs/heads/master\n" + "Branch: refs/heads/master\n"
+ "Patch-Set: 1\n" + "Patch-Set: 1\n"
+ "Topic: Some Topic"); + "Topic: Some Topic\n"
+ "Subject: This is a test change\n");
assertParseSucceeds("Update change\n" assertParseSucceeds("Update change\n"
+ "\n" + "\n"
+ "Branch: refs/heads/master\n" + "Branch: refs/heads/master\n"
+ "Patch-Set: 1\n" + "Patch-Set: 1\n"
+ "Topic:"); + "Topic:\n"
+ "Subject: This is a test change\n");
assertParseFails("Update change\n" assertParseFails("Update change\n"
+ "\n" + "\n"
+ "Patch-Set: 1\n" + "Patch-Set: 1\n"
@@ -232,11 +244,13 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest {
assertParseSucceeds("Update change\n" assertParseSucceeds("Update change\n"
+ "\n" + "\n"
+ "Branch: refs/heads/master\n" + "Branch: refs/heads/master\n"
+ "Patch-Set: 1"); + "Patch-Set: 1\n"
+ "Subject: This is a test change\n");
assertParseSucceeds("Update change\n" assertParseSucceeds("Update change\n"
+ "\n" + "\n"
+ "Branch: master\n" + "Branch: master\n"
+ "Patch-Set: 1"); + "Patch-Set: 1\n"
+ "Subject: This is a test change\n");
assertParseFails("Update change\n" assertParseFails("Update change\n"
+ "\n" + "\n"
+ "Patch-Set: 1\n" + "Patch-Set: 1\n"
@@ -244,6 +258,20 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest {
+ "Branch: refs/heads/stable"); + "Branch: refs/heads/stable");
} }
@Test
public void parseSubject() throws Exception {
assertParseSucceeds("Update change\n"
+ "\n"
+ "Patch-Set: 1\n"
+ "Branch: refs/heads/master\n"
+ "Subject: Some subject of a change\n");
assertParseFails("Update change\n"
+ "\n"
+ "Patch-Set: 1\n"
+ "Subject: Some subject of a change\n"
+ "Subject: Some other subject\n");
}
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,

View File

@@ -350,7 +350,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
public void submitRecords() throws Exception { public void submitRecords() throws Exception {
Change c = newChange(); Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner); ChangeUpdate update = newUpdate(c, changeOwner);
update.setSubject("Submit patch set 1"); update.setSubjectForCommit("Submit patch set 1");
update.merge("1-1453387607626-96fabc25", ImmutableList.of( update.merge("1-1453387607626-96fabc25", ImmutableList.of(
submitRecord("NOT_READY", null, submitRecord("NOT_READY", null,
@@ -380,7 +380,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
public void latestSubmitRecordsOnly() throws Exception { public void latestSubmitRecordsOnly() throws Exception {
Change c = newChange(); Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner); ChangeUpdate update = newUpdate(c, changeOwner);
update.setSubject("Submit patch set 1"); update.setSubjectForCommit("Submit patch set 1");
update.merge("1-1453387607626-96fabc25", ImmutableList.of( update.merge("1-1453387607626-96fabc25", ImmutableList.of(
submitRecord("OK", null, submitRecord("OK", null,
submitLabel("Code-Review", "OK", otherUser.getAccountId())))); submitLabel("Code-Review", "OK", otherUser.getAccountId()))));
@@ -388,7 +388,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
incrementPatchSet(c); incrementPatchSet(c);
update = newUpdate(c, changeOwner); update = newUpdate(c, changeOwner);
update.setSubject("Submit patch set 2"); update.setSubjectForCommit("Submit patch set 2");
update.merge("1-1453387901516-5d1e2450", ImmutableList.of( update.merge("1-1453387901516-5d1e2450", ImmutableList.of(
submitRecord("OK", null, submitRecord("OK", null,
submitLabel("Code-Review", "OK", changeOwner.getAccountId())))); submitLabel("Code-Review", "OK", changeOwner.getAccountId()))));
@@ -511,6 +511,32 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
new Branch.NameKey(project, otherBranch)); new Branch.NameKey(project, otherBranch));
} }
@Test
public void subjectChangeNotes() throws Exception {
Change c = newChange();
ChangeNotes notes = newNotes(c);
assertThat(notes.getChange().getSubject()).isEqualTo(c.getSubject());
assertThat(notes.getChange().getOriginalSubject()).isEqualTo(c.getSubject());
// An unrelated update doesn't affect the subject
ChangeUpdate update = newUpdate(c, changeOwner);
update.setTopic("topic"); // Change something to get a new commit.
update.commit();
notes = newNotes(c);
assertThat(notes.getChange().getSubject()).isEqualTo(c.getSubject());
assertThat(notes.getChange().getOriginalSubject()).isEqualTo(c.getSubject());
// An update of the subject doesn't affect the original subject
update = newUpdate(c, changeOwner);
String newSubject = "other subject";
update.setSubject(newSubject);
update.commit();
notes = newNotes(c);
assertThat(notes.getChange().getSubject()).isEqualTo(newSubject);
assertThat(notes.getChange().getOriginalSubject()).isEqualTo(c.getSubject());
}
@Test @Test
public void ownerChangeNotes() throws Exception { public void ownerChangeNotes() throws Exception {
Change c = newChange(); Change c = newChange();
@@ -560,7 +586,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
@Test @Test
public void emptyExceptSubject() throws Exception { public void emptyExceptSubject() throws Exception {
ChangeUpdate update = newUpdate(newChange(), changeOwner); ChangeUpdate update = newUpdate(newChange(), changeOwner);
update.setSubject("Create change"); update.setSubjectForCommit("Create change");
update.commit(); update.commit();
assertThat(update.getRevision()).isNotNull(); assertThat(update.getRevision()).isNotNull();
} }

View File

@@ -106,7 +106,7 @@ public class CommitMessageOutputTest extends AbstractChangeNotesTest {
public void submitCommitFormat() throws Exception { public void submitCommitFormat() throws Exception {
Change c = newChange(); Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner); ChangeUpdate update = newUpdate(c, changeOwner);
update.setSubject("Submit patch set 1"); update.setSubjectForCommit("Submit patch set 1");
update.merge("1-1453387607626-96fabc25", ImmutableList.of( update.merge("1-1453387607626-96fabc25", ImmutableList.of(
submitRecord("NOT_READY", null, submitRecord("NOT_READY", null,
@@ -172,7 +172,7 @@ public class CommitMessageOutputTest extends AbstractChangeNotesTest {
public void submitWithErrorMessage() throws Exception { public void submitWithErrorMessage() throws Exception {
Change c = newChange(); Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner); ChangeUpdate update = newUpdate(c, changeOwner);
update.setSubject("Submit patch set 1"); update.setSubjectForCommit("Submit patch set 1");
update.merge("1-1453387607626-96fabc25", ImmutableList.of( update.merge("1-1453387607626-96fabc25", ImmutableList.of(
submitRecord("RULE_ERROR", "Problem with patch set:\n1"))); submitRecord("RULE_ERROR", "Problem with patch set:\n1")));