SONAR-6589 Fix project and module key validation

diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/step/ValidateProjectStep.java b/server/sonar-server/src/main/java/org/sonar/server/computation/step/ValidateProjectStep.java
index 120b507a6de7e75448d52e5c8c36aecf9bdecb6b..92409fc2acf75eb52212b7d2e12c430b1a533411 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/step/ValidateProjectStep.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/step/ValidateProjectStep.java
@@ -31,6 +31,7 @@ import javax.annotation.Nonnull;
 import org.sonar.api.CoreProperties;
 import org.sonar.api.config.Settings;
 import org.sonar.batch.protocol.output.BatchReport;
+import org.sonar.batch.protocol.output.BatchReportReader;
 import org.sonar.core.component.ComponentDto;
 import org.sonar.core.component.ComponentKeys;
 import org.sonar.core.persistence.DbSession;
@@ -74,7 +75,7 @@ public class ValidateProjectStep implements ComputationStep {
         }
       });
       ValidateProjectsVisitor visitor = new ValidateProjectsVisitor(session, dbClient.componentDao(), context.getReportMetadata(),
-        settings.getBoolean(CoreProperties.CORE_PREVENT_AUTOMATIC_PROJECT_CREATION), modulesByKey);
+        context.getReportReader(), settings.getBoolean(CoreProperties.CORE_PREVENT_AUTOMATIC_PROJECT_CREATION), modulesByKey);
       visitor.visit(context.getRoot());
 
       if (!visitor.validationMessages.isEmpty()) {
@@ -94,18 +95,21 @@ public class ValidateProjectStep implements ComputationStep {
     private final DbSession session;
     private final ComponentDao componentDao;
     private final BatchReport.Metadata reportMetadata;
+    private final BatchReportReader batchReportReader;
     private final boolean preventAutomaticProjectCreation;
     private final Map<String, ComponentDto> modulesByKey;
     private final List<String> validationMessages = new ArrayList<>();
 
     private Component root;
 
-    public ValidateProjectsVisitor(DbSession session, ComponentDao componentDao, BatchReport.Metadata reportMetadata, boolean preventAutomaticProjectCreation,
-      Map<String, ComponentDto> modulesByKey) {
+    public ValidateProjectsVisitor(DbSession session, ComponentDao componentDao, BatchReport.Metadata reportMetadata, BatchReportReader batchReportReader,
+      boolean preventAutomaticProjectCreation, Map<String, ComponentDto> modulesByKey) {
       super(Component.Type.MODULE, Order.PRE_ORDER);
       this.session = session;
       this.componentDao = componentDao;
       this.reportMetadata = reportMetadata;
+      this.batchReportReader = batchReportReader;
+
       this.preventAutomaticProjectCreation = preventAutomaticProjectCreation;
       this.modulesByKey = modulesByKey;
     }
@@ -114,6 +118,7 @@ public class ValidateProjectStep implements ComputationStep {
     public void visitProject(Component project) {
       this.root = project;
       validateBranch();
+      validateBatchKey(project);
 
       String projectKey = project.getKey();
       ComponentDto projectDto = loadComponent(projectKey);
@@ -128,14 +133,13 @@ public class ValidateProjectStep implements ComputationStep {
           + "If you really want to stop directly analysing project \"%s\", please first delete it from SonarQube and then relaunch the analysis of project \"%s\".",
           projectKey, anotherProject.key(), anotherProject.key(), projectKey));
       }
-      validateKey(projectKey);
     }
 
     @Override
     public void visitModule(Component module) {
-      String moduleKey = module.getKey();
       String projectKey = root.getKey();
-      validateKey(moduleKey);
+      String moduleKey = module.getKey();
+      validateBatchKey(module);
 
       ComponentDto moduleDto = loadComponent(moduleKey);
       if (moduleDto == null) {
@@ -152,10 +156,11 @@ public class ValidateProjectStep implements ComputationStep {
       }
     }
 
-    private void validateKey(String moduleKey) {
-      if (!ComponentKeys.isValidModuleKey(moduleKey)) {
+    private void validateBatchKey(Component component) {
+      String batchKey = batchReportReader.readComponent(component.getRef()).getKey();
+      if (!ComponentKeys.isValidModuleKey(batchKey)) {
         validationMessages.add(String.format("\"%s\" is not a valid project or module key. "
-          + "Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit.", moduleKey));
+          + "Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit.", batchKey));
       }
     }
 

diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/step/ValidateProjectStepTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/step/ValidateProjectStepTest.java
index 434c3dfca867b694d22f866ac79892589643af14..8bfcf5a067aab150b242ee0397a70de778650fc2 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/computation/step/ValidateProjectStepTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/computation/step/ValidateProjectStepTest.java
@@ -21,7 +21,6 @@
 package org.sonar.server.computation.step;
 
 import java.io.File;
-import java.io.IOException;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.ClassRule;
@@ -31,6 +30,7 @@ import org.junit.rules.ExpectedException;
 import org.junit.rules.TemporaryFolder;
 import org.sonar.api.CoreProperties;
 import org.sonar.api.config.Settings;
+import org.sonar.batch.protocol.Constants;
 import org.sonar.batch.protocol.output.BatchReport;
 import org.sonar.batch.protocol.output.BatchReportReader;
 import org.sonar.batch.protocol.output.BatchReportWriter;
@@ -65,6 +65,9 @@ public class ValidateProjectStepTest {
 
   Settings settings;
 
+  File reportDir;
+  BatchReportWriter writer;
+
   ValidateProjectStep sut;
 
   @Before
@@ -73,6 +76,8 @@ public class ValidateProjectStepTest {
     dbClient = new DbClient(dbTester.database(), dbTester.myBatis(), new ComponentDao());
     dbSession = dbClient.openSession(false);
     settings = new Settings();
+    reportDir = temp.newFolder();
+    writer = new BatchReportWriter(reportDir);
 
     sut = new ValidateProjectStep(dbClient, settings);
   }
@@ -84,11 +89,18 @@ public class ValidateProjectStepTest {
 
   @Test
   public void not_fail_if_provisioning_enforced_and_project_exists() throws Exception {
+    writer.writeMetadata(BatchReport.Metadata.newBuilder().build());
+    writer.writeComponent(BatchReport.Component.newBuilder()
+      .setRef(1)
+      .setType(Constants.ComponentType.PROJECT)
+      .setKey(PROJECT_KEY)
+      .build());
+
     settings.appendProperty(CoreProperties.CORE_PREVENT_AUTOMATIC_PROJECT_CREATION, "true");
     dbClient.componentDao().insert(dbSession, ComponentTesting.newProjectDto("ABCD").setKey(PROJECT_KEY));
     dbSession.commit();
 
-    sut.execute(new ComputationContext(createBasicBatchReportReader(), null, null, null,
+    sut.execute(new ComputationContext(new BatchReportReader(reportDir), null, null, null,
       ComponentTreeBuilders.from(new DumbComponent(Component.Type.PROJECT, 1, "ABCD", PROJECT_KEY)), null));
   }
 
@@ -97,46 +109,66 @@ public class ValidateProjectStepTest {
     thrown.expect(IllegalArgumentException.class);
     thrown.expectMessage("Unable to scan non-existing project '" + PROJECT_KEY + "'");
 
+    writer.writeMetadata(BatchReport.Metadata.newBuilder().build());
+    writer.writeComponent(BatchReport.Component.newBuilder()
+      .setRef(1)
+      .setType(Constants.ComponentType.PROJECT)
+      .setKey(PROJECT_KEY)
+      .build());
+
     settings.appendProperty(CoreProperties.CORE_PREVENT_AUTOMATIC_PROJECT_CREATION, "true");
 
-    sut.execute(new ComputationContext(createBasicBatchReportReader(), null, null, null,
+    sut.execute(new ComputationContext(new BatchReportReader(reportDir), null, null, null,
       ComponentTreeBuilders.from(new DumbComponent(Component.Type.PROJECT, 1, "ABCD", PROJECT_KEY)), null));
   }
 
   @Test
   public void fail_if_provisioning_not_enforced_and_project_does_not_exists() throws Exception {
+    writer.writeMetadata(BatchReport.Metadata.newBuilder().build());
+    writer.writeComponent(BatchReport.Component.newBuilder()
+      .setRef(1)
+      .setType(Constants.ComponentType.PROJECT)
+      .setKey(PROJECT_KEY)
+      .build());
+
     settings.appendProperty(CoreProperties.CORE_PREVENT_AUTOMATIC_PROJECT_CREATION, "false");
 
-    sut.execute(new ComputationContext(createBasicBatchReportReader(), null, null, null,
+    sut.execute(new ComputationContext(new BatchReportReader(reportDir), null, null, null,
       ComponentTreeBuilders.from(new DumbComponent(Component.Type.PROJECT, 1, "ABCD", PROJECT_KEY)), null));
   }
 
   @Test
   public void not_fail_on_valid_branch() throws Exception {
-    File reportDir = temp.newFolder();
-    BatchReportWriter writer = new BatchReportWriter(reportDir);
     writer.writeMetadata(BatchReport.Metadata.newBuilder()
       .setBranch("origin/master")
       .build());
+    writer.writeComponent(BatchReport.Component.newBuilder()
+      .setRef(1)
+      .setType(Constants.ComponentType.PROJECT)
+      .setKey(PROJECT_KEY)
+      .build());
 
     sut.execute(new ComputationContext(new BatchReportReader(reportDir), null, null, null,
-      ComponentTreeBuilders.from(new DumbComponent(Component.Type.PROJECT, 1, "ABCD", PROJECT_KEY)), null));
+      ComponentTreeBuilders.from(new DumbComponent(Component.Type.PROJECT, 1, "ABCD", PROJECT_KEY + ":origin/master")), null));
   }
 
   @Test
   public void fail_on_invalid_branch() throws Exception {
-    File reportDir = temp.newFolder();
     thrown.expect(IllegalArgumentException.class);
     thrown.expectMessage("Validation of project failed:\n" +
       "  o \"bran#ch\" is not a valid branch name. Allowed characters are alphanumeric, '-', '_', '.' and '/'.");
 
-    BatchReportWriter writer = new BatchReportWriter(reportDir);
     writer.writeMetadata(BatchReport.Metadata.newBuilder()
       .setBranch("bran#ch")
       .build());
+    writer.writeComponent(BatchReport.Component.newBuilder()
+      .setRef(1)
+      .setType(Constants.ComponentType.PROJECT)
+      .setKey(PROJECT_KEY)
+      .build());
 
     sut.execute(new ComputationContext(new BatchReportReader(reportDir), null, null, null,
-      ComponentTreeBuilders.from(new DumbComponent(Component.Type.PROJECT, 1, "ABCD", PROJECT_KEY)), null));
+      ComponentTreeBuilders.from(new DumbComponent(Component.Type.PROJECT, 1, "ABCD", PROJECT_KEY + ":bran#ch")), null));
   }
 
   @Test
@@ -148,9 +180,22 @@ public class ValidateProjectStepTest {
       "  o \"Project\\Key\" is not a valid project or module key. Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit.\n" +
       "  o \"Module$Key\" is not a valid project or module key. Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit");
 
+    writer.writeMetadata(BatchReport.Metadata.newBuilder().build());
+    writer.writeComponent(BatchReport.Component.newBuilder()
+      .setRef(1)
+      .setType(Constants.ComponentType.PROJECT)
+      .setKey(invalidProjectKey)
+      .addChildRef(2)
+      .build());
+    writer.writeComponent(BatchReport.Component.newBuilder()
+      .setRef(2)
+      .setType(Constants.ComponentType.MODULE)
+      .setKey("Module$Key")
+      .build());
+
     DumbComponent root = new DumbComponent(Component.Type.PROJECT, 1, "ABCD", invalidProjectKey,
       new DumbComponent(Component.Type.MODULE, 2, "BCDE", "Module$Key"));
-    sut.execute(new ComputationContext(createBasicBatchReportReader(), null, null, null, ComponentTreeBuilders.from(root), null));
+    sut.execute(new ComputationContext(new BatchReportReader(reportDir), null, null, null, ComponentTreeBuilders.from(root), null));
   }
 
   @Test
@@ -161,13 +206,26 @@ public class ValidateProjectStepTest {
       "If you really want to stop directly analysing project \"" + MODULE_KEY + "\", please first delete it from SonarQube and then relaunch the analysis of project \""
       + PROJECT_KEY + "\".");
 
+    writer.writeMetadata(BatchReport.Metadata.newBuilder().build());
+    writer.writeComponent(BatchReport.Component.newBuilder()
+      .setRef(1)
+      .setType(Constants.ComponentType.PROJECT)
+      .setKey(PROJECT_KEY)
+      .addChildRef(2)
+      .build());
+    writer.writeComponent(BatchReport.Component.newBuilder()
+      .setRef(2)
+      .setType(Constants.ComponentType.MODULE)
+      .setKey(MODULE_KEY)
+      .build());
+
     ComponentDto project = ComponentTesting.newProjectDto("ABCD").setKey(MODULE_KEY);
     dbClient.componentDao().insert(dbSession, project);
     dbSession.commit();
 
     DumbComponent root = new DumbComponent(Component.Type.PROJECT, 1, "ABCD", PROJECT_KEY,
       new DumbComponent(Component.Type.MODULE, 2, "BCDE", MODULE_KEY));
-    sut.execute(new ComputationContext(createBasicBatchReportReader(), null, null, null,
+    sut.execute(new ComputationContext(new BatchReportReader(reportDir), null, null, null,
       ComponentTreeBuilders.from(root), null));
   }
 
@@ -178,6 +236,19 @@ public class ValidateProjectStepTest {
     thrown.expectMessage("Validation of project failed:\n" +
       "  o Module \"" + MODULE_KEY + "\" is already part of project \"" + anotherProjectKey + "\"");
 
+    writer.writeMetadata(BatchReport.Metadata.newBuilder().build());
+    writer.writeComponent(BatchReport.Component.newBuilder()
+      .setRef(1)
+      .setType(Constants.ComponentType.PROJECT)
+      .setKey(PROJECT_KEY)
+      .addChildRef(2)
+      .build());
+    writer.writeComponent(BatchReport.Component.newBuilder()
+      .setRef(2)
+      .setType(Constants.ComponentType.MODULE)
+      .setKey(MODULE_KEY)
+      .build());
+
     ComponentDto project = ComponentTesting.newProjectDto("ABCD").setKey(PROJECT_KEY);
     ComponentDto anotherProject = ComponentTesting.newProjectDto().setKey(anotherProjectKey);
     dbClient.componentDao().insert(dbSession, project, anotherProject);
@@ -187,7 +258,7 @@ public class ValidateProjectStepTest {
 
     DumbComponent root = new DumbComponent(Component.Type.PROJECT, 1, "ABCD", PROJECT_KEY,
       new DumbComponent(Component.Type.MODULE, 2, "BCDE", MODULE_KEY));
-    sut.execute(new ComputationContext(createBasicBatchReportReader(), null, null, null,
+    sut.execute(new ComputationContext(new BatchReportReader(reportDir), null, null, null,
       ComponentTreeBuilders.from(root), null));
   }
 
@@ -200,6 +271,19 @@ public class ValidateProjectStepTest {
       "If you really want to stop directly analysing project \"" + anotherProjectKey + "\", please first delete it from SonarQube and then relaunch the analysis of project \""
       + PROJECT_KEY + "\".");
 
+    writer.writeMetadata(BatchReport.Metadata.newBuilder().build());
+    writer.writeComponent(BatchReport.Component.newBuilder()
+      .setRef(1)
+      .setType(Constants.ComponentType.PROJECT)
+      .setKey(PROJECT_KEY)
+      .addChildRef(2)
+      .build());
+    writer.writeComponent(BatchReport.Component.newBuilder()
+      .setRef(2)
+      .setType(Constants.ComponentType.MODULE)
+      .setKey(MODULE_KEY)
+      .build());
+
     ComponentDto anotherProject = ComponentTesting.newProjectDto().setKey(anotherProjectKey);
     dbClient.componentDao().insert(dbSession, anotherProject);
     ComponentDto module = ComponentTesting.newModuleDto("ABCD", anotherProject).setKey(PROJECT_KEY);
@@ -208,15 +292,8 @@ public class ValidateProjectStepTest {
 
     DumbComponent root = new DumbComponent(Component.Type.PROJECT, 1, "ABCD", PROJECT_KEY,
       new DumbComponent(Component.Type.MODULE, 2, "BCDE", MODULE_KEY));
-    sut.execute(new ComputationContext(createBasicBatchReportReader(), null, null, null,
+    sut.execute(new ComputationContext(new BatchReportReader(reportDir), null, null, null,
       ComponentTreeBuilders.from(root), null));
   }
 
-  private BatchReportReader createBasicBatchReportReader() throws IOException {
-    File reportDir = temp.newFolder();
-    BatchReportWriter writer = new BatchReportWriter(reportDir);
-    writer.writeMetadata(BatchReport.Metadata.newBuilder()
-      .build());
-    return new BatchReportReader(reportDir);
-  }
 }