From 2e24cfd57b9ffa406a95018f23bc5034567a761a Mon Sep 17 00:00:00 2001 From: Julien HENRY Date: Fri, 25 Sep 2015 11:02:12 +0200 Subject: [PATCH] SONAR-6665 Fix StackOverflow error when analyzing project with several modules having same key --- .../modules/module1/module1/sources/Fake.java | 1 - .../modules/module1/sources/Fake.java | 1 - .../sonar-project.properties | 19 ------ .../java/batch/suite/ProjectBuilderTest.java | 21 +------ .../batch/scan/ProjectReactorBuilder.java | 29 +++++---- .../batch/scan/ProjectReactorBuilderTest.java | 62 ++++++++++++------- .../sonar-project.properties | 12 ++++ .../sonar-project.properties | 2 - 8 files changed, 70 insertions(+), 77 deletions(-) delete mode 100644 it/it-projects/batch/multi-module-repeated-names/modules/module1/module1/sources/Fake.java delete mode 100644 it/it-projects/batch/multi-module-repeated-names/modules/module1/sources/Fake.java delete mode 100644 it/it-projects/batch/multi-module-repeated-names/sonar-project.properties create mode 100644 sonar-batch/src/test/resources/org/sonar/batch/scan/ProjectReactorBuilderTest/multi-module-duplicate-id/sonar-project.properties diff --git a/it/it-projects/batch/multi-module-repeated-names/modules/module1/module1/sources/Fake.java b/it/it-projects/batch/multi-module-repeated-names/modules/module1/module1/sources/Fake.java deleted file mode 100644 index e67004defc5c..000000000000 --- a/it/it-projects/batch/multi-module-repeated-names/modules/module1/module1/sources/Fake.java +++ /dev/null @@ -1 +0,0 @@ -class Fake {} diff --git a/it/it-projects/batch/multi-module-repeated-names/modules/module1/sources/Fake.java b/it/it-projects/batch/multi-module-repeated-names/modules/module1/sources/Fake.java deleted file mode 100644 index e67004defc5c..000000000000 --- a/it/it-projects/batch/multi-module-repeated-names/modules/module1/sources/Fake.java +++ /dev/null @@ -1 +0,0 @@ -class Fake {} diff --git a/it/it-projects/batch/multi-module-repeated-names/sonar-project.properties b/it/it-projects/batch/multi-module-repeated-names/sonar-project.properties deleted file mode 100644 index b1816be030b9..000000000000 --- a/it/it-projects/batch/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 diff --git a/it/it-tests/src/test/java/batch/suite/ProjectBuilderTest.java b/it/it-tests/src/test/java/batch/suite/ProjectBuilderTest.java index 8dff64ce5fd0..9728fc0a3d27 100644 --- a/it/it-tests/src/test/java/batch/suite/ProjectBuilderTest.java +++ b/it/it-tests/src/test/java/batch/suite/ProjectBuilderTest.java @@ -5,18 +5,15 @@ */ package batch.suite; -import static org.assertj.core.api.Assertions.assertThat; - -import com.sonar.orchestrator.build.BuildFailureException; - -import com.sonar.orchestrator.build.SonarRunner; -import util.ItUtils; import com.sonar.orchestrator.Orchestrator; import com.sonar.orchestrator.build.MavenBuild; import org.junit.ClassRule; import org.junit.Test; import org.sonar.wsclient.services.Resource; import org.sonar.wsclient.services.ResourceQuery; +import util.ItUtils; + +import static org.assertj.core.api.Assertions.assertThat; /** * Test the extension point org.sonar.api.batch.bootstrap.ProjectBuilder @@ -47,18 +44,6 @@ public void shouldDefineProjectFromPlugin() { assertThat(getResource("com.sonarsource.it.projects.batch:project-builder-module-b:src/IgnoredFile.java")).isNull(); } - @Test - // SONAR-6665 - public void errorSubModuleSameName() { - SonarRunner build = SonarRunner.create(ItUtils.projectDir("batch/multi-module-repeated-names")); - - try { - orchestrator.executeBuild(build); - } catch (BuildFailureException e) { - assertThat(e.getResult().getLogs()).contains("Two modules have the same id: module1"); - } - } - private void checkProject() { Resource project = getResource("com.sonarsource.it.projects.batch:project-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 d25a5d0e33ea..e3fe21f32f99 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectReactorBuilder.java +++ b/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectReactorBuilder.java @@ -117,21 +117,22 @@ public ProjectReactorBuilder(AnalysisProperties props, AnalysisMode analysisMode public ProjectReactor execute() { Profiler profiler = Profiler.create(LOG).startInfo("Process project properties"); - Map> propertiesByModuleId = new HashMap<>(); - extractPropertiesByModule(propertiesByModuleId, "", taskProps.properties()); - ProjectDefinition rootProject = defineRootProject(propertiesByModuleId.get(""), null); + Map> propertiesByModuleIdPath = new HashMap<>(); + extractPropertiesByModule(propertiesByModuleIdPath, "", "", taskProps.properties()); + ProjectDefinition rootProject = defineRootProject(propertiesByModuleIdPath.get(""), null); rootProjectWorkDir = rootProject.getWorkDir(); - defineChildren(rootProject, propertiesByModuleId); + defineChildren(rootProject, propertiesByModuleIdPath, ""); cleanAndCheckProjectDefinitions(rootProject); // Since task properties are now empty we should add root module properties - taskProps.properties().putAll(propertiesByModuleId.get("")); + taskProps.properties().putAll(propertiesByModuleIdPath.get("")); profiler.stopDebug(); return new ProjectReactor(rootProject); } - private static void extractPropertiesByModule(Map> propertiesByModuleId, String currentModuleId, Map 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)); + private static void extractPropertiesByModule(Map> propertiesByModuleIdPath, String currentModuleId, String currentModuleIdPath, + Map parentProperties) { + if (propertiesByModuleIdPath.containsKey(currentModuleIdPath)) { + throw new IllegalStateException(String.format("Two modules have the same id: '%s'. Each module must have a unique id.", currentModuleId)); } Map currentModuleProperties = new HashMap<>(); @@ -153,10 +154,11 @@ private static void extractPropertiesByModule(Map> p Arrays.sort(moduleIds); ArrayUtils.reverse(moduleIds); - propertiesByModuleId.put(currentModuleId, currentModuleProperties); + propertiesByModuleIdPath.put(currentModuleIdPath, currentModuleProperties); for (String moduleId : moduleIds) { - extractPropertiesByModule(propertiesByModuleId, moduleId, currentModuleProperties); + String subModuleIdPath = currentModuleIdPath.isEmpty() ? moduleId : (currentModuleIdPath + "." + moduleId); + extractPropertiesByModule(propertiesByModuleIdPath, moduleId, subModuleIdPath, currentModuleProperties); } } @@ -232,16 +234,17 @@ private static File initModuleBuildDir(File moduleBaseDir, Map m return new File(moduleBaseDir, customBuildDir.getPath()); } - private void defineChildren(ProjectDefinition parentProject, Map> propertiesByModuleId) { + private void defineChildren(ProjectDefinition parentProject, Map> propertiesByModuleIdPath, String parentModuleIdPath) { Map parentProps = parentProject.properties(); if (parentProps.containsKey(PROPERTY_MODULES)) { for (String moduleId : getListFromProperty(parentProps, PROPERTY_MODULES)) { - Map moduleProps = propertiesByModuleId.get(moduleId); + String moduleIdPath = parentModuleIdPath.isEmpty() ? moduleId : (parentModuleIdPath + "." + moduleId); + Map moduleProps = propertiesByModuleIdPath.get(moduleIdPath); ProjectDefinition childProject = loadChildProject(parentProject, moduleProps, moduleId); // check the uniqueness of the child key checkUniquenessOfChildKey(childProject, parentProject); // the child project may have children as well - defineChildren(childProject, propertiesByModuleId); + defineChildren(childProject, propertiesByModuleIdPath, moduleIdPath); // and finally add this child project to its parent parentProject.addSubProject(childProject); } 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 75900b811ded..b55f0c64e31b 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/scan/ProjectReactorBuilderTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/scan/ProjectReactorBuilderTest.java @@ -19,18 +19,7 @@ */ package org.sonar.batch.scan; -import org.apache.commons.lang.StringUtils; -import org.junit.Before; -import org.sonar.api.batch.AnalysisMode; -import org.sonar.batch.analysis.AnalysisProperties; import com.google.common.collect.Maps; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ExpectedException; -import org.sonar.api.batch.bootstrap.ProjectDefinition; -import org.sonar.api.batch.bootstrap.ProjectReactor; -import org.sonar.test.TestUtils; - import java.io.File; import java.io.FileInputStream; import java.io.IOException; @@ -38,11 +27,20 @@ import java.util.List; import java.util.Map; import java.util.Properties; +import org.apache.commons.lang.StringUtils; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.sonar.api.batch.AnalysisMode; +import org.sonar.api.batch.bootstrap.ProjectDefinition; +import org.sonar.api.batch.bootstrap.ProjectReactor; +import org.sonar.batch.analysis.AnalysisProperties; +import org.sonar.test.TestUtils; -import static org.mockito.Mockito.when; - -import static org.mockito.Mockito.mock; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class ProjectReactorBuilderTest { @@ -50,12 +48,12 @@ public class ProjectReactorBuilderTest { public ExpectedException thrown = ExpectedException.none(); private AnalysisMode mode; - + @Before public void setUp() { mode = mock(AnalysisMode.class); } - + @Test public void shouldDefineSimpleProject() { ProjectDefinition projectDefinition = loadProjectDefinition("simple-project"); @@ -91,10 +89,28 @@ public void shouldNotFailIfBlankSourceDirectory() { } @Test - public void modulesRepeatedIds() { + public void modulesDuplicateIds() { thrown.expect(IllegalStateException.class); - thrown.expectMessage("Two modules have the same id: module1"); - loadProjectDefinition("multi-module-repeated-id"); + thrown.expectMessage("Two modules have the same id: 'module1'. Each module must have a unique id."); + + loadProjectDefinition("multi-module-duplicate-id"); + } + + @Test + public void modulesRepeatedIds() { + ProjectDefinition rootProject = loadProjectDefinition("multi-module-repeated-id"); + + List modules = rootProject.getSubProjects(); + assertThat(modules.size()).isEqualTo(1); + // Module 1 + ProjectDefinition module1 = modules.get(0); + assertThat(module1.getKey()).isEqualTo("com.foo.project:module1"); + assertThat(module1.getName()).isEqualTo("Foo Module 1"); + + // Module 1 -> Module 1 + ProjectDefinition module1_module1 = module1.getSubProjects().get(0); + assertThat(module1_module1.getKey()).isEqualTo("com.foo.project:module1:module1"); + assertThat(module1_module1.getName()).isEqualTo("Foo Sub Module 1"); } @Test @@ -460,12 +476,12 @@ public void shouldInitRootWorkDir() { assertThat(workDir).isEqualTo(new File(baseDir, ".sonar")); } - + @Test public void nonAssociatedMode() { when(mode.isIssues()).thenReturn(true); ProjectDefinition project = loadProjectDefinition("multi-module-with-basedir-not-associated"); - + assertThat(project.getKey()).isEqualTo("project"); } @@ -537,10 +553,10 @@ public void shouldSetModuleKeyIfNotPresent() { private ProjectDefinition loadProjectDefinition(String projectFolder) { Map props = loadProps(projectFolder); AnalysisProperties bootstrapProps = new AnalysisProperties(props, null); - ProjectReactor projectReactor = new ProjectReactorBuilder(bootstrapProps,mode).execute(); + ProjectReactor projectReactor = new ProjectReactorBuilder(bootstrapProps, mode).execute(); return projectReactor.getRoot(); } - + protected static Properties toProperties(File propertyFile) { Properties propsFromFile = new Properties(); try (FileInputStream fileInputStream = new FileInputStream(propertyFile)) { diff --git a/sonar-batch/src/test/resources/org/sonar/batch/scan/ProjectReactorBuilderTest/multi-module-duplicate-id/sonar-project.properties b/sonar-batch/src/test/resources/org/sonar/batch/scan/ProjectReactorBuilderTest/multi-module-duplicate-id/sonar-project.properties new file mode 100644 index 000000000000..0a971805d13d --- /dev/null +++ b/sonar-batch/src/test/resources/org/sonar/batch/scan/ProjectReactorBuilderTest/multi-module-duplicate-id/sonar-project.properties @@ -0,0 +1,12 @@ +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 + +module1.sonar.sources=src 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 index b1816be030b9..e63d3da926ce 100644 --- 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 @@ -10,10 +10,8 @@ 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