Merge "Migrate part of ChangeIndex FieldDefs to the new format SchemaFields"
diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index 47b0a53..654b3a4 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -977,6 +977,7 @@
Map<String, String> lowerNames = new HashMap<>();
submitRequirementSections = new LinkedHashMap<>();
for (String name : rc.getSubsections(SUBMIT_REQUIREMENT)) {
+ checkDuplicateSrDefinition(rc, name);
String lower = name.toLowerCase();
if (lowerNames.containsKey(lower)) {
error(
@@ -1034,6 +1035,40 @@
}
}
+ private void checkDuplicateSrDefinition(Config rc, String srName) {
+ if (rc.getStringList(SUBMIT_REQUIREMENT, srName, KEY_SR_DESCRIPTION).length > 1) {
+ error(
+ String.format(
+ "Multiple definitions of %s for submit requirement '%s'",
+ KEY_SR_DESCRIPTION, srName));
+ }
+ if (rc.getStringList(SUBMIT_REQUIREMENT, srName, KEY_SR_APPLICABILITY_EXPRESSION).length > 1) {
+ error(
+ String.format(
+ "Multiple definitions of %s for submit requirement '%s'",
+ KEY_SR_APPLICABILITY_EXPRESSION, srName));
+ }
+ if (rc.getStringList(SUBMIT_REQUIREMENT, srName, KEY_SR_SUBMITTABILITY_EXPRESSION).length > 1) {
+ error(
+ String.format(
+ "Multiple definitions of %s for submit requirement '%s'",
+ KEY_SR_SUBMITTABILITY_EXPRESSION, srName));
+ }
+ if (rc.getStringList(SUBMIT_REQUIREMENT, srName, KEY_SR_OVERRIDE_EXPRESSION).length > 1) {
+ error(
+ String.format(
+ "Multiple definitions of %s for submit requirement '%s'",
+ KEY_SR_OVERRIDE_EXPRESSION, srName));
+ }
+ if (rc.getStringList(SUBMIT_REQUIREMENT, srName, KEY_SR_OVERRIDE_IN_CHILD_PROJECTS).length
+ > 1) {
+ error(
+ String.format(
+ "Multiple definitions of %s for submit requirement '%s'",
+ KEY_SR_OVERRIDE_IN_CHILD_PROJECTS, srName));
+ }
+ }
+
/**
* Report unsupported submit requirement parameters as errors.
*
diff --git a/javatests/com/google/gerrit/acceptance/api/project/ProjectConfigIT.java b/javatests/com/google/gerrit/acceptance/api/project/ProjectConfigIT.java
index 168819c..52207db 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/ProjectConfigIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/ProjectConfigIT.java
@@ -25,6 +25,8 @@
import com.google.gerrit.common.RawInputUtil;
import com.google.gerrit.entities.LabelFunction;
import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.entities.SubmitRequirement;
+import com.google.gerrit.entities.SubmitRequirementExpression;
import com.google.gerrit.extensions.api.changes.PublishChangeEditInput;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.common.ChangeInfo;
@@ -36,6 +38,7 @@
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.AnyObjectId;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
import org.junit.Test;
public class ProjectConfigIT extends AbstractDaemonTest {
@@ -389,6 +392,168 @@
}
@Test
+ public void rejectSubmitRequirement_duplicateDescriptionKeys() throws Exception {
+ fetchRefsMetaConfig();
+ PushOneCommit push =
+ pushFactory.create(
+ admin.newIdent(),
+ testRepo,
+ "Test Change",
+ ProjectConfig.PROJECT_CONFIG,
+ "[submit-requirement \"Foo\"]\n"
+ + " description = description 1\n "
+ + " submittableIf = label:Code-Review=MAX\n"
+ + "[submit-requirement \"Foo\"]\n"
+ + " description = description 2\n");
+ PushOneCommit.Result r = push.to(RefNames.REFS_CONFIG);
+ r.assertErrorStatus(
+ String.format("commit %s: invalid project configuration", abbreviateName(r.getCommit())));
+ r.assertMessage(
+ String.format(
+ "ERROR: commit %s: project.config: multiple definitions of description"
+ + " for submit requirement 'foo'",
+ abbreviateName(r.getCommit())));
+ }
+
+ @Test
+ public void rejectSubmitRequirement_duplicateApplicableIfKeys() throws Exception {
+ fetchRefsMetaConfig();
+ PushOneCommit push =
+ pushFactory.create(
+ admin.newIdent(),
+ testRepo,
+ "Test Change",
+ ProjectConfig.PROJECT_CONFIG,
+ "[submit-requirement \"Foo\"]\n "
+ + " applicableIf = is:true\n "
+ + " submittableIf = label:Code-Review=MAX\n"
+ + "[submit-requirement \"Foo\"]\n"
+ + " applicableIf = is:false\n");
+ PushOneCommit.Result r = push.to(RefNames.REFS_CONFIG);
+ r.assertErrorStatus(
+ String.format("commit %s: invalid project configuration", abbreviateName(r.getCommit())));
+ r.assertMessage(
+ String.format(
+ "ERROR: commit %s: project.config: multiple definitions of applicableif"
+ + " for submit requirement 'foo'",
+ abbreviateName(r.getCommit())));
+ }
+
+ @Test
+ public void rejectSubmitRequirement_duplicateSubmittableIfKeys() throws Exception {
+ fetchRefsMetaConfig();
+ PushOneCommit push =
+ pushFactory.create(
+ admin.newIdent(),
+ testRepo,
+ "Test Change",
+ ProjectConfig.PROJECT_CONFIG,
+ "[submit-requirement \"Foo\"]\n"
+ + " submittableIf = label:Code-Review=MAX\n"
+ + "[submit-requirement \"Foo\"]\n"
+ + " submittableIf = label:Code-Review=MIN\n");
+ PushOneCommit.Result r = push.to(RefNames.REFS_CONFIG);
+ r.assertErrorStatus(
+ String.format("commit %s: invalid project configuration", abbreviateName(r.getCommit())));
+ r.assertMessage(
+ String.format(
+ "ERROR: commit %s: project.config: multiple definitions of submittableif"
+ + " for submit requirement 'foo'",
+ abbreviateName(r.getCommit())));
+ }
+
+ @Test
+ public void rejectSubmitRequirement_duplicateOverrideIfKeys() throws Exception {
+ fetchRefsMetaConfig();
+ PushOneCommit push =
+ pushFactory.create(
+ admin.newIdent(),
+ testRepo,
+ "Test Change",
+ ProjectConfig.PROJECT_CONFIG,
+ "[submit-requirement \"Foo\"]\n"
+ + " overrideIf = is:true\n "
+ + " submittableIf = label:Code-Review=MAX\n"
+ + "[submit-requirement \"Foo\"]\n"
+ + " overrideIf = is:false\n");
+ PushOneCommit.Result r = push.to(RefNames.REFS_CONFIG);
+ r.assertErrorStatus(
+ String.format("commit %s: invalid project configuration", abbreviateName(r.getCommit())));
+ r.assertMessage(
+ String.format(
+ "ERROR: commit %s: project.config: multiple definitions of overrideif"
+ + " for submit requirement 'foo'",
+ abbreviateName(r.getCommit())));
+ }
+
+ @Test
+ public void rejectSubmitRequirement_duplicateCanOverrideInChildProjectsKey() throws Exception {
+ fetchRefsMetaConfig();
+ PushOneCommit push =
+ pushFactory.create(
+ admin.newIdent(),
+ testRepo,
+ "Test Change",
+ ProjectConfig.PROJECT_CONFIG,
+ "[submit-requirement \"Foo\"]\n"
+ + " canOverrideInChildProjects = true\n"
+ + " submittableIf = label:Code-Review=MAX\n"
+ + "[submit-requirement \"Foo\"]\n "
+ + " canOverrideInChildProjects = false\n");
+ PushOneCommit.Result r = push.to(RefNames.REFS_CONFIG);
+ r.assertErrorStatus(
+ String.format("commit %s: invalid project configuration", abbreviateName(r.getCommit())));
+ r.assertMessage(
+ String.format(
+ "ERROR: commit %s: project.config: multiple definitions of canoverrideinchildprojects"
+ + " for submit requirement 'foo'",
+ abbreviateName(r.getCommit())));
+ }
+
+ @Test
+ public void submitRequirementsAreParsed_forExistingDuplicateDefinitions() throws Exception {
+ // Duplicate submit requirement definitions are rejected on config change uploads. For setups
+ // already containing duplicate SR definitions, the server is able to parse the "submit
+ // requirements correctly"
+
+ RevCommit revision;
+ // Commit a change to the project config, bypassing server validation.
+ try (TestRepository<Repository> testRepo =
+ new TestRepository<>(repoManager.openRepository(project))) {
+ revision =
+ testRepo
+ .branch(RefNames.REFS_CONFIG)
+ .commit()
+ .add(
+ ProjectConfig.PROJECT_CONFIG,
+ "[submit-requirement \"Foo\"]\n"
+ + " canOverrideInChildProjects = true\n"
+ + " submittableIf = label:Code-Review=MAX\n"
+ + "[submit-requirement \"Foo\"]\n "
+ + " canOverrideInChildProjects = false\n")
+ .parent(projectOperations.project(project).getHead(RefNames.REFS_CONFIG))
+ .create();
+ }
+
+ try (Repository git = repoManager.openRepository(project)) {
+ // Server is able to parse the config.
+ ProjectConfig cfg = projectConfigFactory.create(project);
+ cfg.load(git, revision);
+
+ // One of the two definitions takes precedence and overrides the other.
+ assertThat(cfg.getSubmitRequirementSections())
+ .containsExactly(
+ "Foo",
+ SubmitRequirement.builder()
+ .setName("Foo")
+ .setAllowOverrideInChildProjects(false)
+ .setSubmittabilityExpression(
+ SubmitRequirementExpression.create("label:Code-Review=MAX"))
+ .build());
+ }
+ }
+
+ @Test
public void testRejectChangingLabelFunction_toMaxWithBlock() throws Exception {
testChangingLabelFunction(
/* initialLabelFunction= */ LabelFunction.NO_BLOCK,
