opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-2.15'

Browse Source 
* stable-2.15:
  Reset state flags on loadData calls
  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
  Bazel: Use rules_closure from HEAD

Change-Id: Ifb35377d72c3f4bd085944f16a6a1264ecad2141
This commit is contained in:
David Pursehouse 2018-02-06 08:45:03 +09:00
commit 6e78391950
4 changed files with 40 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
java/com/google/gerrit/acceptance/AbstractDaemonTest.java
View File

@ -952,6 +952,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().setBooleanConfig(BooleanProjectConfig.REQUIRE_CHANGE_ID, value);
config.commit(md);
projectCache.evict(config.getProject());
}
}
protected void deny(String ref, String permission, AccountGroup.UUID id) throws Exception {
deny(project, ref, permission, id);
}

16
java/com/google/gerrit/server/git/validators/CommitValidators.java
View File

@ -82,7 +82,7 @@ public class CommitValidators {
private static final Logger log = LoggerFactory.getLogger(CommitValidators.class);
public static final Pattern NEW_PATCHSET_PATTERN =
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]*)?$");
@Singleton
public static class Factory {
@ -291,15 +291,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 (projectState.is(BooleanProjectConfig.REQUIRE_CHANGE_ID)) {
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);

34
javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
View File

@ -1332,11 +1332,7 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
assertThat(GitUtil.getChangeId(testRepo, c)).isEmpty();
pushForReviewRejected(testRepo, "missing Change-Id in commit message footer");
ProjectConfig config = projectCache.checkedGet(project).getConfig();
config
.getProject()
.setBooleanConfig(BooleanProjectConfig.REQUIRE_CHANGE_ID, InheritableBoolean.FALSE);
saveProjectConfig(project, config);
setRequireChangeId(InheritableBoolean.FALSE);
pushForReviewOk(testRepo);
}
@ -1383,11 +1379,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()
.setBooleanConfig(BooleanProjectConfig.REQUIRE_CHANGE_ID, InheritableBoolean.FALSE);
saveProjectConfig(project, config);
setRequireChangeId(InheritableBoolean.FALSE);
pushForReviewRejected(testRepo, "multiple Change-Id lines in commit message footer");
}
@ -1406,11 +1398,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()
.setBooleanConfig(BooleanProjectConfig.REQUIRE_CHANGE_ID, InheritableBoolean.FALSE);
saveProjectConfig(project, config);
setRequireChangeId(InheritableBoolean.FALSE);
pushForReviewRejected(testRepo, "invalid Change-Id line format in commit message footer");
}
@ -1434,15 +1422,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()
.setBooleanConfig(BooleanProjectConfig.REQUIRE_CHANGE_ID, 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 =

11
javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
View File

@ -42,6 +42,7 @@ import com.google.gerrit.extensions.common.CommitInfo;
import com.google.gerrit.extensions.common.MergeInput;
import com.google.gerrit.extensions.common.RevisionInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
@ -95,6 +96,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));