SONAR-5878 Fix regression when parsing project properties

sonar-batch/src/main/java/org/sonar/batch/scan/ProjectReactorBuilder.java

@@ -44,6 +44,8 @@ import java.io.FileInputStream;
 import java.io.IOException;
 import java.text.MessageFormat;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -136,7 +138,10 @@ public class ProjectReactorBuilder {
         parentProperties.remove(key);
       }
     }
-    String[] moduleIds = getListFromProperty(currentModuleProperties, PROPERTY_MODULES);
+    List<String> moduleIds = new ArrayList<String>(Arrays.asList(getListFromProperty(currentModuleProperties, PROPERTY_MODULES)));
+    // Sort module by reverse lexicographic order to avoid issue when one module id is a prefix of another one
+    Collections.sort(moduleIds);
+    Collections.reverse(moduleIds);
     Map<String, Map<String, String>> result = new HashMap<String, Map<String, String>>();
     for (String moduleId : moduleIds) {
       result.putAll(extractPropertiesByModule(moduleId, currentModuleProperties));

sonar-batch/src/test/java/org/sonar/batch/scan/ProjectReactorBuilderTest.java

@@ -565,6 +565,72 @@ public class ProjectReactorBuilderTest {
     assertThat(buildDir.getName()).isEqualTo("build");
   }
 
+  @Test
+  public void doNotMixPropertiesWhenModuleKeyIsPrefixOfAnother() throws IOException {
+    ProjectDefinition rootProject = loadProjectDefinition("multi-module-definitions-same-prefix");
+
+    // CHECK ROOT
+    assertThat(rootProject.getKey()).isEqualTo("com.foo.project");
+    assertThat(rootProject.getName()).isEqualTo("Foo Project");
+    assertThat(rootProject.getVersion()).isEqualTo("1.0-SNAPSHOT");
+    assertThat(rootProject.getDescription()).isEqualTo("Description of Foo Project");
+    // root project must not contain some properties - even if they are defined in the root properties file
+    assertThat(rootProject.getSourceDirs().contains("sources")).isFalse();
+    assertThat(rootProject.getTestDirs().contains("tests")).isFalse();
+    assertThat(rootProject.getBinaries().contains("target/classes")).isFalse();
+    // and module properties must have been cleaned
+    assertThat(rootProject.getProperties().getProperty("module1.sonar.projectKey")).isNull();
+    assertThat(rootProject.getProperties().getProperty("module2.sonar.projectKey")).isNull();
+    // Check baseDir and workDir
+    assertThat(rootProject.getBaseDir().getCanonicalFile())
+      .isEqualTo(TestUtils.getResource(this.getClass(), "multi-module-definitions-same-prefix"));
+    assertThat(rootProject.getWorkDir().getCanonicalFile())
+      .isEqualTo(new File(TestUtils.getResource(this.getClass(), "multi-module-definitions-same-prefix"), ".sonar"));
+
+    // CHECK MODULES
+    List<ProjectDefinition> modules = rootProject.getSubProjects();
+    assertThat(modules.size()).isEqualTo(2);
+
+    // Module 1
+    ProjectDefinition module1 = modules.get(0);
+    assertThat(module1.getBaseDir().getCanonicalFile()).isEqualTo(TestUtils.getResource(this.getClass(), "multi-module-definitions-same-prefix/module1"));
+    assertThat(module1.getKey()).isEqualTo("com.foo.project:module1");
+    assertThat(module1.getName()).isEqualTo("module1");
+    assertThat(module1.getVersion()).isEqualTo("1.0-SNAPSHOT");
+    // Description should not be inherited from parent if not set
+    assertThat(module1.getDescription()).isNull();
+    assertThat(module1.getSourceDirs()).contains("sources");
+    assertThat(module1.getTestDirs()).contains("tests");
+    assertThat(module1.getBinaries()).contains("target/classes");
+    // and module properties must have been cleaned
+    assertThat(module1.getProperties().getProperty("module1.sonar.projectKey")).isNull();
+    assertThat(module1.getProperties().getProperty("module2.sonar.projectKey")).isNull();
+    // Check baseDir and workDir
+    assertThat(module1.getBaseDir().getCanonicalFile())
+      .isEqualTo(TestUtils.getResource(this.getClass(), "multi-module-definitions-same-prefix/module1"));
+    assertThat(module1.getWorkDir().getCanonicalFile())
+      .isEqualTo(new File(TestUtils.getResource(this.getClass(), "multi-module-definitions-same-prefix"), ".sonar/com.foo.project_module1"));
+
+    // Module 1 Feature
+    ProjectDefinition module1Feature = modules.get(1);
+    assertThat(module1Feature.getBaseDir().getCanonicalFile()).isEqualTo(TestUtils.getResource(this.getClass(), "multi-module-definitions-same-prefix/module1.feature"));
+    assertThat(module1Feature.getKey()).isEqualTo("com.foo.project:com.foo.project.module1.feature");
+    assertThat(module1Feature.getName()).isEqualTo("Foo Module 1 Feature");
+    assertThat(module1Feature.getVersion()).isEqualTo("1.0-SNAPSHOT");
+    assertThat(module1Feature.getDescription()).isEqualTo("Description of Module 1 Feature");
+    assertThat(module1Feature.getSourceDirs()).contains("src");
+    assertThat(module1Feature.getTestDirs()).contains("tests");
+    assertThat(module1Feature.getBinaries()).contains("target/classes");
+    // and module properties must have been cleaned
+    assertThat(module1Feature.getProperties().getProperty("module1.sonar.projectKey")).isNull();
+    assertThat(module1Feature.getProperties().getProperty("module2.sonar.projectKey")).isNull();
+    // Check baseDir and workDir
+    assertThat(module1Feature.getBaseDir().getCanonicalFile())
+      .isEqualTo(TestUtils.getResource(this.getClass(), "multi-module-definitions-same-prefix/module1.feature"));
+    assertThat(module1Feature.getWorkDir().getCanonicalFile())
+      .isEqualTo(new File(TestUtils.getResource(this.getClass(), "multi-module-definitions-same-prefix"), ".sonar/com.foo.project_com.foo.project.module1.feature"));
+  }
+
   private Map<String, String> loadPropsFromFile(String filePath) throws IOException {
     Properties props = new Properties();
     FileInputStream fileInputStream = null;

sonar-batch/src/test/resources/org/sonar/batch/scan/ProjectReactorBuilderTest/multi-module-definitions-same-prefix/module1.feature/src/Fake.java

@@ -0,0 +1 @@
+Fake

sonar-batch/src/test/resources/org/sonar/batch/scan/ProjectReactorBuilderTest/multi-module-definitions-same-prefix/module1/sources/Fake.java

@@ -0,0 +1 @@
+Fake

sonar-batch/src/test/resources/org/sonar/batch/scan/ProjectReactorBuilderTest/multi-module-definitions-same-prefix/sonar-project.properties

@@ -0,0 +1,19 @@
+sonar.projectKey=com.foo.project
+sonar.projectName=Foo Project
+sonar.projectVersion=1.0-SNAPSHOT
+sonar.projectDescription=Description of Foo Project
+
+sonar.sources=sources
+sonar.tests=tests
+sonar.binaries=target/classes
+
+sonar.modules=module1,\
+              module1.feature
+
+# Mandatory properties for module1 are all inferred from the module ID
+
+module1.feature.sonar.projectKey=com.foo.project.module1.feature
+module1.feature.sonar.projectName=Foo Module 1 Feature
+# redefine some properties
+module1.feature.sonar.projectDescription=Description of Module 1 Feature
+module1.feature.sonar.sources=src