Optimize project reactor builder

diff --git a/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectReactorBuilder.java b/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectReactorBuilder.java
index abbb9602d1a7df2e6e704cd23b3768d8573f4391..a20e8abd3e44f07c13d86fb0483d60f8c46f790c 100644 (file)
--- a/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectReactorBuilder.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectReactorBuilder.java
@@ -19,6 +19,8 @@
  */
 package org.sonar.batch.scan;
 
+import org.apache.commons.lang.ArrayUtils;
+
 import org.sonar.batch.analysis.AnalysisProperties;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.Lists;
@@ -30,14 +32,12 @@ import java.io.IOException;
 import java.text.MessageFormat;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Properties;
-import java.util.Set;
 
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
@@ -118,59 +118,46 @@ public class ProjectReactorBuilder {
 
   public ProjectReactor execute() {
     Profiler profiler = Profiler.create(LOG).startInfo("Process project properties");
-    Map<String, Map<String, String>> propertiesByModuleId = extractPropertiesByModule("", taskProps.properties());
+    Map<String, Map<String, String>> propertiesByModuleId = new HashMap<>();
+    extractPropertiesByModule(propertiesByModuleId, "", taskProps.properties());
     ProjectDefinition rootProject = defineRootProject(propertiesByModuleId.get(""), null);
     rootProjectWorkDir = rootProject.getWorkDir();
     defineChildren(rootProject, propertiesByModuleId);
     cleanAndCheckProjectDefinitions(rootProject);
     // Since task properties are now empty we should add root module properties
-    for (Map.Entry<String, String> entry : propertiesByModuleId.get("").entrySet()) {
-      taskProps.properties().put((String) entry.getKey(), (String) entry.getValue());
-    }
+    taskProps.properties().putAll(propertiesByModuleId.get(""));
     profiler.stopDebug();
     return new ProjectReactor(rootProject);
   }
 
-  private static Map<String, Map<String, String>> extractPropertiesByModule(String currentModuleId, Map<String, String> parentProperties) {
-    Map<String, String> allProperties = new HashMap<>();
-    allProperties.putAll(parentProperties);
+  private static void extractPropertiesByModule(Map<String, Map<String, String>> propertiesByModuleId, String currentModuleId, Map<String, String> parentProperties) {
+    if (propertiesByModuleId.containsKey(currentModuleId)) {
+      throw new IllegalStateException(String.format("Two modules have the same id: %s. Each module must have a unique id.", currentModuleId));
+    }
+
     Map<String, String> currentModuleProperties = new HashMap<>();
-    String prefix = !currentModuleId.isEmpty() ? currentModuleId + "." : "";
+    String prefix = !currentModuleId.isEmpty() ? (currentModuleId + ".") : "";
+    int prefixLength = prefix.length();
+
     // By default all properties starting with module prefix belong to current module
-    for (Map.Entry<String, String> entry : allProperties.entrySet()) {
-      String key = entry.getKey();
-      int prefixLength = prefix.length();
+    Iterator<Entry<String, String>> it = parentProperties.entrySet().iterator();
+    while (it.hasNext()) {
+      Map.Entry<String, String> e = it.next();
+      String key = e.getKey();
       if (key.startsWith(prefix)) {
-        currentModuleProperties.put(key.substring(prefixLength), entry.getValue());
-        parentProperties.remove(key);
+        currentModuleProperties.put(key.substring(prefixLength), e.getValue());
+        it.remove();
       }
     }
-    List<String> moduleIds = new ArrayList<>(Arrays.asList(getListFromProperty(currentModuleProperties, PROPERTY_MODULES)));
+    String[] moduleIds = 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);
+    Arrays.sort(moduleIds);
+    ArrayUtils.reverse(moduleIds);
 
-    Map<String, Map<String, String>> result = new HashMap<>();
-    result.put(currentModuleId, currentModuleProperties);
+    propertiesByModuleId.put(currentModuleId, currentModuleProperties);
 
     for (String moduleId : moduleIds) {
-      Map<String, Map<String, String>> subModuleProps = extractPropertiesByModule(moduleId, currentModuleProperties);
-      checkRepeatedModuleNames(result.keySet(), subModuleProps.keySet());
-      result.putAll(subModuleProps);
-    }
-    return result;
-  }
-
-  private static void checkRepeatedModuleNames(Collection<String> currentModules, Collection<String> modulesToMerge) {
-    Set<String> union = new HashSet<>();
-    union.addAll(currentModules);
-    union.retainAll(modulesToMerge);
-    if (!union.isEmpty()) {
-      if (union.size() > 1) {
-        throw new IllegalStateException(String.format("Modules have the following repeated names: %s. Each module must have a unique name.", union));
-      } else {
-        throw new IllegalStateException(String.format("Two modules have the same name: %s. Each module must have a unique name.", union.iterator().next()));
-      }
+      extractPropertiesByModule(propertiesByModuleId, moduleId, currentModuleProperties);
     }
   }
 

diff --git a/sonar-batch/src/test/java/org/sonar/batch/scan/ProjectReactorBuilderTest.java b/sonar-batch/src/test/java/org/sonar/batch/scan/ProjectReactorBuilderTest.java
index b4bdeb07877b7388e636e0c95877423ef207104e..82db9d87976df9c739937559bd516e8597f3f332 100644 (file)
--- a/sonar-batch/src/test/java/org/sonar/batch/scan/ProjectReactorBuilderTest.java
+++ b/sonar-batch/src/test/java/org/sonar/batch/scan/ProjectReactorBuilderTest.java
@@ -79,11 +79,11 @@ public class ProjectReactorBuilderTest {
   }
   
   @Test
-  public void modulesRepeatedNames() {
+  public void modulesRepeatedIds() {
     thrown.expect(IllegalStateException.class);
-    thrown.expectMessage("Two modules have the same name: module1");
+    thrown.expectMessage("Two modules have the same id: module1");
     
-    loadProjectDefinition("multi-module-repeated-names");
+    loadProjectDefinition("multi-module-repeated-id");
   }
 
   @Test

diff --git a/sonar-batch/src/test/resources/org/sonar/batch/scan/ProjectReactorBuilderTest/multi-module-repeated-id/modules/module1/module1/sources/Fake.java b/sonar-batch/src/test/resources/org/sonar/batch/scan/ProjectReactorBuilderTest/multi-module-repeated-id/modules/module1/module1/sources/Fake.java
new file mode 100644 (file)
index 0000000..e67004d
--- /dev/null
+++ b/sonar-batch/src/test/resources/org/sonar/batch/scan/ProjectReactorBuilderTest/multi-module-repeated-id/modules/module1/module1/sources/Fake.java
@@ -0,0 +1 @@
+class Fake {}

diff --git a/sonar-batch/src/test/resources/org/sonar/batch/scan/ProjectReactorBuilderTest/multi-module-repeated-id/modules/module1/sources/Fake.java b/sonar-batch/src/test/resources/org/sonar/batch/scan/ProjectReactorBuilderTest/multi-module-repeated-id/modules/module1/sources/Fake.java
new file mode 100644 (file)
index 0000000..e67004d
--- /dev/null
+++ b/sonar-batch/src/test/resources/org/sonar/batch/scan/ProjectReactorBuilderTest/multi-module-repeated-id/modules/module1/sources/Fake.java
@@ -0,0 +1 @@
+class Fake {}

diff --git a/sonar-batch/src/test/resources/org/sonar/batch/scan/ProjectReactorBuilderTest/multi-module-repeated-id/sonar-project.properties b/sonar-batch/src/test/resources/org/sonar/batch/scan/ProjectReactorBuilderTest/multi-module-repeated-id/sonar-project.properties
new file mode 100644 (file)
index 0000000..b1816be
--- /dev/null
+++ b/sonar-batch/src/test/resources/org/sonar/batch/scan/ProjectReactorBuilderTest/multi-module-repeated-id/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.sonar.projectBaseDir=modules/module1
+module1.sonar.projectKey=com.foo.project.module1
+module1.sonar.projectName=Foo Module 1
+module1.sonar.modules=module1
+
+module1.module1.sonar.projectBaseDir=module1
+module1.module1.sonar.projectKey=com.foo.project.module1.module1
+module1.module1.sonar.projectName=Foo Sub Module 1

diff --git a/sonar-batch/src/test/resources/org/sonar/batch/scan/ProjectReactorBuilderTest/multi-module-repeated-names/modules/module1/module1/sources/Fake.java b/sonar-batch/src/test/resources/org/sonar/batch/scan/ProjectReactorBuilderTest/multi-module-repeated-names/modules/module1/module1/sources/Fake.java
deleted file mode 100644 (file)
index e67004d..0000000
--- a/sonar-batch/src/test/resources/org/sonar/batch/scan/ProjectReactorBuilderTest/multi-module-repeated-names/modules/module1/module1/sources/Fake.java
+++ /dev/null
@@ -1 +0,0 @@
-class Fake {}

diff --git a/sonar-batch/src/test/resources/org/sonar/batch/scan/ProjectReactorBuilderTest/multi-module-repeated-names/modules/module1/sources/Fake.java b/sonar-batch/src/test/resources/org/sonar/batch/scan/ProjectReactorBuilderTest/multi-module-repeated-names/modules/module1/sources/Fake.java
deleted file mode 100644 (file)
index e67004d..0000000
--- a/sonar-batch/src/test/resources/org/sonar/batch/scan/ProjectReactorBuilderTest/multi-module-repeated-names/modules/module1/sources/Fake.java
+++ /dev/null
@@ -1 +0,0 @@
-class Fake {}

diff --git a/sonar-batch/src/test/resources/org/sonar/batch/scan/ProjectReactorBuilderTest/multi-module-repeated-names/sonar-project.properties b/sonar-batch/src/test/resources/org/sonar/batch/scan/ProjectReactorBuilderTest/multi-module-repeated-names/sonar-project.properties
deleted file mode 100644 (file)
index b1816be..0000000
--- a/sonar-batch/src/test/resources/org/sonar/batch/scan/ProjectReactorBuilderTest/multi-module-repeated-names/sonar-project.properties
+++ /dev/null
@@ -1,19 +0,0 @@
-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.sonar.projectBaseDir=modules/module1
-module1.sonar.projectKey=com.foo.project.module1
-module1.sonar.projectName=Foo Module 1
-module1.sonar.modules=module1
-
-module1.module1.sonar.projectBaseDir=module1
-module1.module1.sonar.projectKey=com.foo.project.module1.module1
-module1.module1.sonar.projectName=Foo Sub Module 1