MSONAR-205 Remove sonar.projectKey for submodules

Fix a bug where the scanner-engine would reject a multi-module project
where the `sonar.projectKey` is specified in the root or in a non-leaf
module.

When the user specifies `sonar.projectKey` as a maven property in the root (or
in any non-leaf project), the property is inherited by its sub modules.
All (sub)modules would then share the same `sonar.projectKey` clashing
with a legacy test in the scanner-engine that would ensure that each
submodule has a different key (where the key is defined as whatever
value is set for the property `sonar.projectKey` of that module).

MSONAR-205

Author: dorian-burihabwa-sonarsource

src/it/java-multi-module-project-key-in-pom/module1/pom.xml

@@ -0,0 +1,22 @@
+<?xml version="1.0"?>
+<project xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd" xmlns="http://maven.apache.org/POM/4.0.0"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
+  <modelVersion>4.0.0</modelVersion>
+  <parent>
+    <groupId>org.example</groupId>
+    <artifactId>java-multi-module-project-key-in-pom</artifactId>
+    <version>1.0-SNAPSHOT</version>
+  </parent>
+
+  <artifactId>module1</artifactId>
+  <version>1.0-SNAPSHOT</version>
+
+  <dependencies>
+    <dependency>
+      <groupId>junit</groupId>
+      <artifactId>junit</artifactId>
+      <version>RELEASE</version>
+      <scope>test</scope>
+    </dependency>
+  </dependencies>
+</project>

src/it/java-multi-module-project-key-in-pom/module1/src/main/java/org/example/App.java

@@ -0,0 +1,13 @@
+package org.example;
+
+/**
+ * Hello world!
+ *
+ */
+public class App 
+{
+    public static void main( String[] args )
+    {
+        System.out.println( "Hello World!" );
+    }
+}

src/it/java-multi-module-project-key-in-pom/module1/src/test/java/org/example/AppTest.java

@@ -0,0 +1,38 @@
+package org.example;
+
+import junit.framework.Test;
+import junit.framework.TestCase;
+import junit.framework.TestSuite;
+
+/**
+ * Unit test for simple App.
+ */
+public class AppTest 
+    extends TestCase
+{
+    /**
+     * Create the test case
+     *
+     * @param testName name of the test case
+     */
+    public AppTest( String testName )
+    {
+        super( testName );
+    }
+
+    /**
+     * @return the suite of tests being tested
+     */
+    public static Test suite()
+    {
+        return new TestSuite( AppTest.class );
+    }
+
+    /**
+     * Rigourous Test :-)
+     */
+    public void testApp()
+    {
+        assertTrue( true );
+    }
+}

src/it/java-multi-module-project-key-in-pom/pom.xml

@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+
+  <groupId>org.example</groupId>
+  <artifactId>java-multi-module-project-key-in-pom</artifactId>
+  <packaging>pom</packaging>
+  <version>1.0-SNAPSHOT</version>
+
+  <modules>
+    <module>module1</module>
+  </modules>
+
+  <properties>
+    <sonar.projectKey>this-property-overrides-the-project-key</sonar.projectKey>
+  </properties>
+</project>

src/it/java-multi-module-project-key-in-pom/verify.groovy

@@ -0,0 +1,15 @@
+Properties properties = new Properties()
+File propertiesFile = new File(basedir, 'out.properties')
+propertiesFile.withInputStream {
+    properties.load(it)
+}
+
+def projectKey = 'sonar.projectKey'
+def modules = 'sonar.modules'
+
+// We test that the project key is the one in the pom properties of the project
+assert properties.'sonar.projectKey' == 'this-property-overrides-the-project-key'
+// We test that we have one submodule detected by the scanner
+assert properties.'sonar.modules' == 'org.example:module1'
+// We test that the submodule does not have a sonar.projectKey property
+assert !properties.hasProperty('org.example:module1.sonar.projectKey')

src/main/java/org/sonarsource/scanner/maven/bootstrap/MavenProjectConverter.java

@@ -127,6 +127,12 @@ public class MavenProjectConverter {
 
   private boolean sourceDirsIsOverridden = false;
 
+  /**
+   * This field is introduced to keep track of the root project in multi-module projects and can be used to decide
+   * whether to apply specific treatments to submodules as we recursively configure them.
+   */
+  private MavenProject root;
+
   public MavenProjectConverter(Log log, MavenCompilerResolver mavenCompilerResolver, Properties envProperties) {
     this.log = log;
     this.mavenCompilerResolver = mavenCompilerResolver;
@@ -151,6 +157,7 @@ Map<String, String> configure(List<MavenProject> mavenProjects, MavenProject roo
     Map<MavenProject, Map<String, String>> propsByModule = new LinkedHashMap<>();
 
     try {
+      this.root = root;
       configureModules(mavenProjects, propsByModule);
       Map<String, String> props = new HashMap<>();
       props.put(ScanProperties.PROJECT_KEY, getArtifactKey(root));
@@ -163,6 +170,8 @@ Map<String, String> configure(List<MavenProject> mavenProjects, MavenProject roo
       return props;
     } catch (IOException e) {
       throw new IllegalStateException("Cannot configure project", e);
+    } finally {
+      this.root = null;
     }
   }
 
@@ -252,7 +261,7 @@ private static MavenProject findMavenProject(final File modulePath, Collection<M
 
   private Map<String, String> computeSonarQubeProperties(MavenProject pom) throws MojoExecutionException {
     Map<String, String> props = new HashMap<>();
-    defineModuleKey(pom, props, specifiedProjectKey);
+    defineModuleKey(pom, props);
     props.put(ScanProperties.PROJECT_VERSION, pom.getVersion());
     props.put(ScanProperties.PROJECT_NAME, pom.getName());
     String description = pom.getDescription();
@@ -280,16 +289,24 @@ private static String specifiedProjectKey(Properties userProperties, MavenProjec
     return projectKey;
   }
 
-  private static void defineModuleKey(MavenProject pom, Map<String, String> props, @Nullable String specifiedProjectKey) {
+  /**
+   * Generates a unique module key for a (sub)module and adds it to the existing properties.
+   * If the project is the root, we try to use the specified project key ({@link MavenProjectConverter#specifiedProjectKey}) if available.
+   * Otherwise, we use the artifact key ({@link MavenProjectConverter#getArtifactKey(MavenProject)}.
+   *
+   * @param project The maven submodule for which a key must be generated
+   * @param props The existing properties where the module key will be added
+   * @return The generated module key
+   */
+  private String defineModuleKey(MavenProject project, Map<String, String> props) {
     String key;
-    if (pom.getModel().getProperties().containsKey(ScanProperties.PROJECT_KEY)) {
-      key = pom.getModel().getProperties().getProperty(ScanProperties.PROJECT_KEY);
-    } else if (specifiedProjectKey != null) {
-      key = specifiedProjectKey + ":" + getArtifactKey(pom);
+    if (project.equals(root) && this.specifiedProjectKey != null) {
+      key = this.specifiedProjectKey;
     } else {
-      key = getArtifactKey(pom);
+      key = getArtifactKey(project);
     }
     props.put(MODULE_KEY, key);
+    return key;
   }
 
   private static String getArtifactKey(MavenProject pom) {
@@ -393,7 +410,9 @@ private void synchronizeFileSystemAndOtherProps(MavenProject pom, Map<String, St
     // IMPORTANT NOTE : reference on properties from POM model must not be saved,
     // instead they should be copied explicitly - see SONAR-2896
     for (String k : pom.getModel().getProperties().stringPropertyNames()) {
-      props.put(k, pom.getModel().getProperties().getProperty(k));
+      if (!ScanProperties.PROJECT_KEY.equals(k) || pom.equals(this.root)) {
+        props.put(k, pom.getModel().getProperties().getProperty(k));
+      }
     }
 
     MavenUtils.putAll(envProperties, props);

src/test/java/org/sonarsource/scanner/maven/bootstrap/MavenProjectConverterTest.java

@@ -29,11 +29,9 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.Properties;
-
 import org.apache.maven.plugin.MojoExecutionException;
 import org.apache.maven.plugin.logging.Log;
 import org.apache.maven.project.MavenProject;
-
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
@@ -638,6 +636,53 @@ void two_modules_in_same_folder() throws Exception {
       .containsEntry(module2Key + ".sonar.projectBaseDir", modulesBaseDir.getAbsolutePath());
   }
 
+  @Test
+  void submodules_are_not_assigned_user_provided_project_key_from_parent() throws MojoExecutionException, IOException {
+    Properties rootPomProperties = new Properties();
+    rootPomProperties.put(ScanProperties.PROJECT_KEY, "the_greatest_project_key_there_ever_was");
+    File baseDir = temp.toFile();
+    baseDir.mkdirs();
+    MavenProject root = createProject(rootPomProperties, "pom");
+    root.setGroupId("org.example");
+    root.setArtifactId("root");
+
+    File module1BaseDir = temp.resolve("module1").toFile();
+    module1BaseDir.mkdirs();
+    File module1Pom = new File(module1BaseDir, "pom.xml");
+    MavenProject module1 = new MavenProject();
+    File target = new File(module1Pom.getParentFile(), "target");
+    File classes = new File(target, "classes");
+    File testClasses = new File(target, "test-classes");
+    classes.mkdirs();
+    testClasses.mkdirs();
+
+    module1.getModel().setGroupId("org.example");
+    module1.getModel().setArtifactId("module1");
+    module1.getModel().setName("My Project");
+    module1.getModel().setDescription("My sample module1");
+    module1.getModel().setVersion("2.1");
+    module1.getModel().setPackaging("jar");
+    module1.getBuild().setOutputDirectory(classes.getAbsolutePath());
+    module1.getBuild().setTestOutputDirectory(testClasses.getAbsolutePath());
+    module1.getModel().setProperties(rootPomProperties);
+    module1.getBuild().setDirectory(new File(module1Pom.getParentFile(), "target").getAbsolutePath());
+    module1.setFile(module1Pom);
+    module1.setParent(root);
+    root.getModules().add("module1");
+
+    Map<String, String> properties = projectConverter.configure(
+      Arrays.asList(module1, root),
+      root,
+      new Properties()
+    );
+
+    assertThat(properties.get(ScanProperties.PROJECT_KEY))
+      .isNotNull()
+      .isEqualTo("the_greatest_project_key_there_ever_was");
+    String keyPrefixForModule1 = module1.getGroupId() + ":" + module1.getArtifactId() + ".";
+    assertThat(properties).doesNotContainKey(keyPrefixForModule1 + ScanProperties.PROJECT_KEY);
+  }
+
   private MavenProject createProject(Properties pomProps, String packaging) throws IOException {
     File pom = temp.resolve("pom.xml").toFile();
     pom.createNewFile();