Repository: porscheinformatik/sonarqube-licensecheck (java)

Issue:
Maven Dependencies do not take precedence over "auto determined" licence
**prologue**
we have a project that is using several components from the Jersey framework
* com.sun.jersey:jersey-core
* com.sun.jersey:jersey-client
* com.sun.jersey.contribs:jersey-apache-client4

in maven these libs are labled with several licences (CDDL 1.1, GPL 1.1....)
see e.g.: 
* https://mvnrepository.com/artifact/com.sun.jersey.contribs/jersey-apache-client4 
* https://mvnrepository.com/artifact/com.sun.jersey.contribs/jersey-apache-client4
* https://mvnrepository.com/artifact/org.glassfish.jersey.core/jersey-client

.... and actually all these lables are incorrect as the jersey project itself states that all components are licenced under EPL 2 / GPL 2)
see: https://jersey.github.io/license.html 

In order to make the report correct again I now added a maven licence mapping via the administration ui of the plugin
![image](https://user-images.githubusercontent.com/4927859/60875154-951c3480-a239-11e9-9c3f-0d7cbb07012d.png)


**issue**
* run a scan on such a project
* **expectation** : these 3 components of jersey are reported as EPL 2 (as hopefully my explicit maven mapping takes precedence over the automatically determined license)
* **actual** : these 3 components of jersey are reported as GPL 2
![image](https://user-images.githubusercontent.com/4927859/60875280-ce54a480-a239-11e9-9112-766602a6c6b5.png)


**context**
* sonarqube Community EditionVersion 7.7 (build 23042)
* plugin version v3 beta 3



Pull request:
Precedence for Maven dependency regex to license mapping, close #77
Mapping of Maven dependencies to licenses should have precedence to the
license found in the pom.xml file (and mapped via Maven license mapping)

Patch:
diff --git a/src/main/java/at/porscheinformatik/sonarqube/licensecheck/maven/MavenDependencyScanner.java b/src/main/java/at/porscheinformatik/sonarqube/licensecheck/maven/MavenDependencyScanner.java
index 3de00cd..31b6af8 100644
--- a/src/main/java/at/porscheinformatik/sonarqube/licensecheck/maven/MavenDependencyScanner.java
+++ b/src/main/java/at/porscheinformatik/sonarqube/licensecheck/maven/MavenDependencyScanner.java
@@ -41,11 +41,10 @@ import at.porscheinformatik.sonarqube.licensecheck.mavenlicense.MavenLicenseServ
 public class MavenDependencyScanner implements Scanner
 {
     private static final Logger LOGGER = LoggerFactory.getLogger(MavenDependencyScanner.class);
-
     private static final String MAVEN_REPO_LOCAL = "maven.repo.local";
+    private static final Pattern DEPENDENCY_PATTERN = Pattern.compile("\\s*([^:]*):([^:]*):[^:]*:([^:]*):[^:]*:(.*)");
 
     private final MavenLicenseService mavenLicenseService;
-
     private final MavenDependencyService mavenDependencyService;
 
     public MavenDependencyScanner(MavenLicenseService mavenLicenseService,
@@ -81,13 +80,13 @@ public class MavenDependencyScanner implements Scanner
             }
         }
 
-        return this.readDependencyList(moduleDir, userSettings, globalSettings)
-            .map(this.loadLicenseFromPom(mavenLicenseService.getLicenseMap(), userSettings, globalSettings))
+        return readDependencyList(moduleDir, userSettings, globalSettings)
             .map(this::mapMavenDependencyToLicense)
+            .map(this.loadLicenseFromPom(mavenLicenseService.getLicenseMap(), userSettings, globalSettings))
             .collect(Collectors.toSet());
     }
 
-    private Stream<Dependency> readDependencyList(File moduleDir, String userSettings, String globalSettings)
+    private static Stream<Dependency> readDependencyList(File moduleDir, String userSettings, String globalSettings)
     {
         Path tempFile = createTempFile();
         if (tempFile == null)
@@ -117,6 +116,11 @@ public class MavenDependencyScanner implements Scanner
         }
         request.setProperties(properties);
 
+        return invokeMaven(request, tempFile);
+    }
+
+    private static Stream<Dependency> invokeMaven(InvocationRequest request, Path mavenOutputFile)
+    {
         try
         {
             StringBuilder mavenExecutionErrors = new StringBuilder();
@@ -135,7 +139,7 @@ public class MavenDependencyScanner implements Scanner
                 LOGGER.warn("Could not get dependency list via maven", result.getExecutionException());
                 LOGGER.warn(mavenExecutionErrors.toString());
             }
-            return Files.lines(tempFile)
+            return Files.lines(mavenOutputFile)
                 .filter(StringUtils::isNotBlank)
                 .map(MavenDependencyScanner::findDependency)
                 .filter(Objects::nonNull);
@@ -151,7 +155,7 @@ public class MavenDependencyScanner implements Scanner
         return Stream.empty();
     }
 
-    private Path createTempFile()
+    private static Path createTempFile()
     {
         try
         {
@@ -166,7 +170,6 @@ public class MavenDependencyScanner implements Scanner
         }
     }
 
-    private static final Pattern DEPENDENCY_PATTERN = Pattern.compile("\\s*([^:]*):([^:]*):[^:]*:([^:]*):[^:]*:(.*)");
     private static Dependency findDependency(String line)
     {
         // Remove module info introduced with Maven Dependency Plugin 3.0 (and JDK > 9)
@@ -191,8 +194,8 @@ public class MavenDependencyScanner implements Scanner
     {
         return (Dependency dependency) ->
         {
-            String path = dependency.getLocalPath();
-            if (path == null)
+            if (StringUtils.isNotBlank(dependency.getLicense())
+                || dependency.getLocalPath() == null)
             {
                 return dependency;
             }
@@ -201,7 +204,7 @@ public class MavenDependencyScanner implements Scanner
         };
     }
 
-    private Dependency loadLicense(Map<Pattern, String> licenseMap, String userSettings, String globalSettings,
+    private static Dependency loadLicense(Map<Pattern, String> licenseMap, String userSettings, String globalSettings,
         Dependency dependency)
     {
         String path = dependency.getLocalPath();
@@ -224,13 +227,13 @@ public class MavenDependencyScanner implements Scanner
         return dependency;
     }
 
-    private Dependency licenseMatcher(Map<Pattern, String> licenseMap, Dependency dependency, License license)
+    private static void licenseMatcher(Map<Pattern, String> licenseMap, Dependency dependency, License license)
     {
         String licenseName = license.getName();
         if (StringUtils.isBlank(licenseName))
         {
             LOGGER.info("Dependency '{}' has no license set.", dependency.getName());
-            return dependency;
+            return;
         }
 
         for (Entry<Pattern, String> entry : licenseMap.entrySet())
@@ -238,25 +241,26 @@ public class MavenDependencyScanner implements Scanner
             if (entry.getKey().matcher(licenseName).matches())
             {
                 dependency.setLicense(entry.getValue());
-                return dependency;
+                return;
             }
         }
 
         LOGGER.info("No licenses found for '{}'", licenseName);
-        return dependency;
     }
 
     private Dependency mapMavenDependencyToLicense(Dependency dependency)
     {
-        if (StringUtils.isBlank(dependency.getLicense()))
+        if (StringUtils.isNotBlank(dependency.getLicense()))
         {
-            for (MavenDependency allowedDependency : mavenDependencyService.getMavenDependencies())
+            return dependency;
+        }
+
+        for (MavenDependency allowedDependency : mavenDependencyService.getMavenDependencies())
+        {
+            String matchString = allowedDependency.getKey();
+            if (dependency.getName().matches(matchString))
             {
-                String matchString = allowedDependency.getKey();
-                if (dependency.getName().matches(matchString))
-                {
-                    dependency.setLicense(allowedDependency.getLicense());
-                }
+                dependency.setLicense(allowedDependency.getLicense());
             }
         }
         return dependency;


Test patch:
diff --git a/src/test/java/at/porscheinformatik/sonarqube/licensecheck/maven/MavenDependencyScannerTest.java b/src/test/java/at/porscheinformatik/sonarqube/licensecheck/maven/MavenDependencyScannerTest.java
index 6894084..265b93d 100644
--- a/src/test/java/at/porscheinformatik/sonarqube/licensecheck/maven/MavenDependencyScannerTest.java
+++ b/src/test/java/at/porscheinformatik/sonarqube/licensecheck/maven/MavenDependencyScannerTest.java
@@ -1,5 +1,6 @@
 package at.porscheinformatik.sonarqube.licensecheck.maven;
 
+import static java.util.Collections.singletonList;
 import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertThat;
 import static org.mockito.Mockito.when;
@@ -7,8 +8,8 @@ import static org.mockito.Mockito.when;
 import java.io.File;
 import java.io.IOException;
 import java.nio.file.Files;
-import java.util.Arrays;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.regex.Pattern;
@@ -30,13 +31,10 @@ public class MavenDependencyScannerTest
     {
         File moduleDir = new File(".");
 
-        Map<Pattern, String> licenseMap = new HashMap<>();
-        licenseMap.put(Pattern.compile(".*Apache.*2.*"), "Apache-2.0");
-        MavenLicenseService licenseService = Mockito.mock(MavenLicenseService.class);
-        when(licenseService.getLicenseMap()).thenReturn(licenseMap);
         final MavenDependencyService dependencyService = Mockito.mock(MavenDependencyService.class);
-        when(dependencyService.getMavenDependencies()).thenReturn(Arrays.asList(new MavenDependency("org.apache.*", "Apache-2.0")));
-        Scanner scanner = new MavenDependencyScanner(licenseService, dependencyService);
+        when(dependencyService.getMavenDependencies()).thenReturn(
+            singletonList(new MavenDependency("org.apache.*", "Apache-2.0")));
+        Scanner scanner = new MavenDependencyScanner(mockLicenseService(), dependencyService);
 
         // -
         Set<Dependency> dependencies = scanner.scan(moduleDir);
@@ -57,6 +55,15 @@ public class MavenDependencyScannerTest
         }
     }
 
+    private MavenLicenseService mockLicenseService()
+    {
+        Map<Pattern, String> licenseMap = new HashMap<>();
+        licenseMap.put(Pattern.compile(".*Apache.*2.*"), "Apache-2.0");
+        MavenLicenseService licenseService = Mockito.mock(MavenLicenseService.class);
+        when(licenseService.getLicenseMap()).thenReturn(licenseMap);
+        return licenseService;
+    }
+
     @Test
     public void testNullMavenProjectDependencies() throws IOException
     {
@@ -70,4 +77,23 @@ public class MavenDependencyScannerTest
 
         assertThat(dependencies.size(), is(0));
     }
+
+    @Test
+    public void mavenDependencyMappingHandledBeforePomLicense()
+    {
+        File moduleDir = new File(".");
+        MavenDependencyService dependencyService = Mockito.mock(MavenDependencyService.class);
+        List<MavenDependency> mavenDependencies =
+            singletonList(new MavenDependency("org.apache.commons:commons.*", "TEST"));
+        when(dependencyService.getMavenDependencies()).thenReturn(mavenDependencies);
+        Scanner scanner = new MavenDependencyScanner(mockLicenseService(), dependencyService);
+
+        Set<Dependency> dependencies = scanner.scan(moduleDir);
+
+        Dependency commonsLang = dependencies.stream()
+            .filter(d -> "org.apache.commons:commons-lang3".equals(d.getName()))
+            .findFirst().orElse(null);
+
+        assertThat(commonsLang.getLicense(), is("TEST"));
+    }
 }