opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-2.13' into stable-2.14

Browse Source 
* stable-2.13:
  ReceiveCommits: Fix NEW_PATCHSET regex pattern
  CreateChangeIT#createNewChangeSignedOffByFooter: Remove unnecessary assume
  CommitValidators: Always check for Change-Id in subject line
  AbstractPushForReview: Factor repeated code to setRequireChangeId method

Change-Id: I01c93d3d104558ec74f757843620e8f55d2233a2
This commit is contained in:
David Pursehouse 2018-02-05 16:57:11 +09:00
commit fe39bf86f4
5 changed files with 40 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
View File

@ -823,6 +823,15 @@ public abstract class AbstractDaemonTest {
}
}
protected void setRequireChangeId(InheritableBoolean value) throws Exception {
try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) {
ProjectConfig config = ProjectConfig.read(md);
config.getProject().setRequireChangeID(value);
config.commit(md);
projectCache.evict(config.getProject());
}
}
protected void deny(String permission, AccountGroup.UUID id, String ref) throws Exception {
deny(project, permission, id, ref);
}

25
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
View File

@ -988,9 +988,7 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
assertThat(GitUtil.getChangeId(testRepo, c).isPresent()).isFalse();
pushForReviewRejected(testRepo, "missing Change-Id in commit message footer");
ProjectConfig config = projectCache.checkedGet(project).getConfig();
config.getProject().setRequireChangeID(InheritableBoolean.FALSE);
saveProjectConfig(project, config);
setRequireChangeId(InheritableBoolean.FALSE);
pushForReviewOk(testRepo);
}
@ -1014,9 +1012,7 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
+ "Change-Id: Ie9a132e107def33bdd513b7854b50de911edba0a\n");
pushForReviewRejected(testRepo, "multiple Change-Id lines in commit message footer");
ProjectConfig config = projectCache.checkedGet(project).getConfig();
config.getProject().setRequireChangeID(InheritableBoolean.FALSE);
saveProjectConfig(project, config);
setRequireChangeId(InheritableBoolean.FALSE);
pushForReviewRejected(testRepo, "multiple Change-Id lines in commit message footer");
}
@ -1035,9 +1031,7 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
createCommit(testRepo, "Message with invalid Change-Id\n\nChange-Id: X\n");
pushForReviewRejected(testRepo, "invalid Change-Id line format in commit message footer");
ProjectConfig config = projectCache.checkedGet(project).getConfig();
config.getProject().setRequireChangeID(InheritableBoolean.FALSE);
saveProjectConfig(project, config);
setRequireChangeId(InheritableBoolean.FALSE);
pushForReviewRejected(testRepo, "invalid Change-Id line format in commit message footer");
}
@ -1061,12 +1055,19 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
+ "Change-Id: I0000000000000000000000000000000000000000\n");
pushForReviewRejected(testRepo, "invalid Change-Id line format in commit message footer");
ProjectConfig config = projectCache.checkedGet(project).getConfig();
config.getProject().setRequireChangeID(InheritableBoolean.FALSE);
saveProjectConfig(project, config);
setRequireChangeId(InheritableBoolean.FALSE);
pushForReviewRejected(testRepo, "invalid Change-Id line format in commit message footer");
}
@Test
public void pushWithChangeIdInSubjectLine() throws Exception {
createCommit(testRepo, "Change-Id: I1234000000000000000000000000000000000000");
pushForReviewRejected(testRepo, "missing subject; Change-Id must be in commit message footer");
setRequireChangeId(InheritableBoolean.FALSE);
pushForReviewRejected(testRepo, "missing subject; Change-Id must be in commit message footer");
}
@Test
public void pushCommitWithSameChangeIdAsPredecessorChange() throws Exception {
PushOneCommit push =

12
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
View File

@ -37,6 +37,7 @@ import com.google.gerrit.extensions.common.ChangeInput;
import com.google.gerrit.extensions.common.MergeInput;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
@ -94,6 +95,16 @@ public class CreateChangeIT extends AbstractDaemonTest {
assertCreateFails(ci, BadRequestException.class, "unsupported change status");
}
@Test
public void createEmptyChange_InvalidChangeId() throws Exception {
ChangeInput ci = newChangeInput(ChangeStatus.NEW);
ci.subject = "Subject\n\nChange-Id: I0000000000000000000000000000000000000000";
assertCreateFails(
ci,
ResourceConflictException.class,
"invalid Change-Id line format in commit message footer");
}
@Test
public void createNewChange() throws Exception {
assertCreateSucceeds(newChangeInput(ChangeStatus.NEW));
@ -124,7 +135,6 @@ public class CreateChangeIT extends AbstractDaemonTest {
@Test
public void createNewChangeSignedOffByFooter() throws Exception {
assume().that(isAllowDrafts()).isTrue();
setSignedOffByFooter();
ChangeInfo info = assertCreateSucceeds(newChangeInput(ChangeStatus.NEW));
String message = info.revisions.get(info.currentRevision).commit.message;

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

@ -181,7 +181,7 @@ public class ReceiveCommits {
private static final Logger log = LoggerFactory.getLogger(ReceiveCommits.class);
public static final Pattern NEW_PATCHSET =
Pattern.compile("^" + REFS_CHANGES + "(?:[0-9][0-9]/)?([1-9][0-9]*)(?:/new)?$");
Pattern.compile("^" + REFS_CHANGES + "(?:[0-9][0-9]/)?([1-9][0-9]*)(?:/[1-9][0-9]*)?$");
private static final String COMMAND_REJECTION_MESSAGE_FOOTER =
"Please read the documentation and contact an administrator\n"

14
gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java
View File

@ -248,15 +248,13 @@ public class CommitValidators {
String sha1 = commit.abbreviate(RevId.ABBREV_LEN).name();
if (idList.isEmpty()) {
String shortMsg = commit.getShortMessage();
if (shortMsg.startsWith(CHANGE_ID_PREFIX)
&& CHANGE_ID.matcher(shortMsg.substring(CHANGE_ID_PREFIX.length()).trim()).matches()) {
String errMsg = String.format(MISSING_SUBJECT_MSG, sha1);
throw new CommitValidationException(errMsg);
}
if (projectControl.getProjectState().isRequireChangeID()) {
String shortMsg = commit.getShortMessage();
if (shortMsg.startsWith(CHANGE_ID_PREFIX)
&& CHANGE_ID
.matcher(shortMsg.substring(CHANGE_ID_PREFIX.length()).trim())
.matches()) {
String errMsg = String.format(MISSING_SUBJECT_MSG, sha1);
throw new CommitValidationException(errMsg);
}
String errMsg = String.format(MISSING_CHANGE_ID_MSG, sha1);
messages.add(getMissingChangeIdErrorMsg(errMsg, commit));
throw new CommitValidationException(errMsg, messages);