SONAR-8622 Fix preview mode/JSON report on branches

7 files changed, 100 insertions, 51 deletions

diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/tracking/LocalIssueTracking.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/tracking/LocalIssueTracking.java
index f94464d6176..80cfbe0caff 100644
--- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/tracking/LocalIssueTracking.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/tracking/LocalIssueTracking.java
@@ -21,7 +21,6 @@ package org.sonar.scanner.issue.tracking;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
-
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -37,6 +36,7 @@ import org.sonar.api.batch.fs.InputFile;
 import org.sonar.api.batch.fs.InputFile.Status;
 import org.sonar.api.batch.fs.InputModule;
 import org.sonar.api.batch.fs.internal.DefaultInputFile;
+import org.sonar.api.batch.fs.internal.DefaultInputModule;
 import org.sonar.api.batch.fs.internal.InputComponentTree;
 import org.sonar.api.batch.rule.ActiveRule;
 import org.sonar.api.batch.rule.ActiveRules;
@@ -164,8 +164,9 @@ public class LocalIssueTracking {
   private SourceHashHolder loadSourceHashes(InputComponent component) {
     SourceHashHolder sourceHashHolder = null;
     if (component.isFile()) {
+      DefaultInputModule module = (DefaultInputModule) componentTree.getParent(componentTree.getParent(component));
       DefaultInputFile file = (DefaultInputFile) component;
-      sourceHashHolder = new SourceHashHolder(file, lastLineHashes);
+      sourceHashHolder = new SourceHashHolder(module, file, lastLineHashes);
     }
     return sourceHashHolder;
   }
diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/tracking/ServerIssueRepository.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/tracking/ServerIssueRepository.java
index fbd952bca2f..7d7eb16271a 100644
--- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/tracking/ServerIssueRepository.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/tracking/ServerIssueRepository.java
@@ -20,12 +20,12 @@
 package org.sonar.scanner.issue.tracking;
 
 import java.util.function.Function;
-
 import javax.annotation.Nullable;
+import org.sonar.api.batch.InstantiationStrategy;
 import org.sonar.api.batch.ScannerSide;
+import org.sonar.api.batch.bootstrap.ProjectDefinition;
 import org.sonar.api.batch.fs.InputComponent;
 import org.sonar.api.batch.fs.internal.DefaultInputComponent;
-import org.sonar.api.batch.InstantiationStrategy;
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
 import org.sonar.api.utils.log.Profiler;
@@ -76,14 +76,18 @@ public class ServerIssueRepository {
       if (issue == null) {
         return null;
       }
-      String componentKey = ComponentKeys.createEffectiveKey(issue.getModuleKey(), issue.hasPath() ? issue.getPath() : null);
-      DefaultInputComponent r = (DefaultInputComponent) resourceCache.getByKey(componentKey);
-      if (r == null) {
-        // Deleted resource
-        issuesCache.put(0, issue.getKey(), issue);
-      } else {
-        issuesCache.put(r.batchId(), issue.getKey(), issue);
+      String moduleKeyWithBranch = issue.getModuleKey();
+      ProjectDefinition projectDefinition = reactor.getProjectDefinition(moduleKeyWithBranch);
+      if (projectDefinition != null) {
+        String componentKeyWithoutBranch = ComponentKeys.createEffectiveKey(projectDefinition.getKey(), issue.hasPath() ? issue.getPath() : null);
+        DefaultInputComponent r = (DefaultInputComponent) resourceCache.getByKey(componentKeyWithoutBranch);
+        if (r != null) {
+          issuesCache.put(r.batchId(), issue.getKey(), issue);
+          return null;
+        }
       }
+      // Deleted resource
+      issuesCache.put(0, issue.getKey(), issue);
       return null;
     }
   }
diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/tracking/SourceHashHolder.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/tracking/SourceHashHolder.java
index bf939ebc8f5..236e034b4a2 100644
--- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/tracking/SourceHashHolder.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/tracking/SourceHashHolder.java
@@ -19,23 +19,25 @@
  */
 package org.sonar.scanner.issue.tracking;
 
-import org.sonar.api.batch.fs.InputFile.Status;
-import org.sonar.api.batch.fs.internal.DefaultInputFile;
-
-import javax.annotation.CheckForNull;
-
 import java.util.Collection;
 import java.util.Collections;
+import javax.annotation.CheckForNull;
+import org.sonar.api.batch.fs.InputFile.Status;
+import org.sonar.api.batch.fs.internal.DefaultInputFile;
+import org.sonar.api.batch.fs.internal.DefaultInputModule;
+import org.sonar.core.component.ComponentKeys;
 
 public class SourceHashHolder {
 
+  private final DefaultInputModule module;
+  private final DefaultInputFile inputFile;
   private final ServerLineHashesLoader lastSnapshots;
 
   private FileHashes hashedReference;
   private FileHashes hashedSource;
-  private DefaultInputFile inputFile;
 
-  public SourceHashHolder(DefaultInputFile inputFile, ServerLineHashesLoader lastSnapshots) {
+  public SourceHashHolder(DefaultInputModule module, DefaultInputFile inputFile, ServerLineHashesLoader lastSnapshots) {
+    this.module = module;
     this.inputFile = inputFile;
     this.lastSnapshots = lastSnapshots;
   }
@@ -49,7 +51,9 @@ public class SourceHashHolder {
       } else if (status == Status.SAME) {
         hashedReference = hashedSource;
       } else {
-        String[] lineHashes = lastSnapshots.getLineHashes(inputFile.key());
+        // Need key with branch
+        String serverSideKey = ComponentKeys.createEffectiveKey(module.definition().getKeyWithBranch(), inputFile);
+        String[] lineHashes = lastSnapshots.getLineHashes(serverSideKey);
         hashedReference = lineHashes != null ? FileHashes.create(lineHashes) : null;
       }
     }
diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/DefaultComponentTree.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/DefaultComponentTree.java
index 29dcc8155b8..59098eabb8d 100644
--- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/DefaultComponentTree.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/DefaultComponentTree.java
@@ -19,20 +19,17 @@
  */
 package org.sonar.scanner.scan;
 
+import com.google.common.base.Preconditions;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedHashSet;
 import java.util.Map;
 import java.util.Set;
-
 import javax.annotation.CheckForNull;
-
 import org.sonar.api.batch.fs.InputComponent;
 import org.sonar.api.batch.fs.internal.InputComponentTree;
 
-import com.google.common.base.Preconditions;
-
 public class DefaultComponentTree implements InputComponentTree {
   private Map<InputComponent, InputComponent> parents = new HashMap<>();
   private Map<InputComponent, Set<InputComponent>> children = new HashMap<>();
@@ -41,13 +38,7 @@ public class DefaultComponentTree implements InputComponentTree {
     Preconditions.checkNotNull(component);
     Preconditions.checkNotNull(parent);
     parents.put(component, parent);
-    Set<InputComponent> list = children.get(parent);
-    if (list == null) {
-      list = new LinkedHashSet<>();
-      children.put(parent, list);
-    }
-
-    list.add(component);
+    children.computeIfAbsent(parent, k -> new LinkedHashSet<>()).add(component);
   }
 
   @Override
diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/report/JSONReport.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/report/JSONReport.java
index 3827fff1676..05ab73ac247 100644
--- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/report/JSONReport.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/report/JSONReport.java
@@ -37,10 +37,12 @@ import org.sonar.api.Properties;
 import org.sonar.api.Property;
 import org.sonar.api.PropertyType;
 import org.sonar.api.batch.fs.FileSystem;
+import org.sonar.api.batch.fs.InputComponent;
 import org.sonar.api.batch.fs.InputDir;
-import org.sonar.api.batch.fs.internal.DefaultInputDir;
+import org.sonar.api.batch.fs.InputPath;
 import org.sonar.api.batch.fs.internal.DefaultInputFile;
 import org.sonar.api.batch.fs.internal.DefaultInputModule;
+import org.sonar.api.batch.fs.internal.InputComponentTree;
 import org.sonar.api.batch.fs.internal.InputModuleHierarchy;
 import org.sonar.api.batch.rule.Rule;
 import org.sonar.api.batch.rule.Rules;
@@ -48,6 +50,7 @@ import org.sonar.api.config.Settings;
 import org.sonar.api.platform.Server;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.utils.text.JsonWriter;
+import org.sonar.core.component.ComponentKeys;
 import org.sonar.scanner.issue.IssueCache;
 import org.sonar.scanner.issue.tracking.TrackedIssue;
 import org.sonar.scanner.protocol.input.ScannerInput;
@@ -74,9 +77,10 @@ public class JSONReport implements Reporter {
   private final DefaultInputModule rootModule;
   private final UserRepositoryLoader userRepository;
   private final InputModuleHierarchy moduleHierarchy;
+  private final InputComponentTree inputComponentTree;
 
   public JSONReport(InputModuleHierarchy moduleHierarchy, Settings settings, FileSystem fileSystem, Server server, Rules rules, IssueCache issueCache,
-    DefaultInputModule rootModule, InputComponentStore componentStore, UserRepositoryLoader userRepository) {
+    DefaultInputModule rootModule, InputComponentStore componentStore, UserRepositoryLoader userRepository, InputComponentTree inputComponentTree) {
     this.moduleHierarchy = moduleHierarchy;
     this.settings = settings;
     this.fileSystem = fileSystem;
@@ -86,6 +90,7 @@ public class JSONReport implements Reporter {
     this.rootModule = rootModule;
     this.componentStore = componentStore;
     this.userRepository = userRepository;
+    this.inputComponentTree = inputComponentTree;
   }
 
   @Override
@@ -130,10 +135,15 @@ public class JSONReport implements Reporter {
     json.name("issues").beginArray();
     for (TrackedIssue issue : getIssues()) {
       if (issue.resolution() == null) {
+        InputComponent component = componentStore.getByKey(issue.componentKey());
+        String componentKey = getModule(component).definition().getKeyWithBranch();
+        if (component instanceof InputPath) {
+          componentKey = ComponentKeys.createEffectiveKey(componentKey, (InputPath) component);
+        }
         json
           .beginObject()
           .prop("key", issue.key())
-          .prop("component", issue.componentKey())
+          .prop("component", componentKey)
           .prop("line", issue.startLine())
           .prop("startLine", issue.startLine())
           .prop("startOffset", issue.startLineOffset())
@@ -158,27 +168,39 @@ public class JSONReport implements Reporter {
     json.endArray();
   }
 
+  private DefaultInputModule getModule(InputComponent component) {
+    if (component.isFile()) {
+      return (DefaultInputModule) inputComponentTree.getParent(inputComponentTree.getParent(component));
+    } else if (component instanceof InputDir) {
+      return (DefaultInputModule) inputComponentTree.getParent(component);
+    } else {
+      return (DefaultInputModule) component;
+    }
+  }
+
   private void writeJsonComponents(JsonWriter json) throws IOException {
     json.name("components").beginArray();
     // Dump modules
     writeJsonModuleComponents(json, rootModule);
     for (DefaultInputFile inputFile : componentStore.allFilesToPublish()) {
-      String key = inputFile.key();
+      String moduleKey = getModule(inputFile).definition().getKeyWithBranch();
+      String key = ComponentKeys.createEffectiveKey(moduleKey, inputFile);
       json
         .beginObject()
         .prop("key", key)
         .prop("path", inputFile.relativePath())
-        .prop("moduleKey", inputFile.moduleKey())
+        .prop("moduleKey", moduleKey)
         .prop("status", inputFile.status().name())
         .endObject();
     }
     for (InputDir inputDir : componentStore.allDirs()) {
-      String key = ((DefaultInputDir) inputDir).key();
+      String moduleKey = getModule(inputDir).definition().getKeyWithBranch();
+      String key = ComponentKeys.createEffectiveKey(moduleKey, inputDir);
       json
         .beginObject()
         .prop("key", key)
         .prop("path", inputDir.relativePath())
-        .prop("moduleKey", StringUtils.substringBeforeLast(key, ":"))
+        .prop("moduleKey", moduleKey)
         .endObject();
 
     }
@@ -188,7 +210,7 @@ public class JSONReport implements Reporter {
   private void writeJsonModuleComponents(JsonWriter json, DefaultInputModule module) {
     json
       .beginObject()
-      .prop("key", module.key())
+      .prop("key", module.definition().getKeyWithBranch())
       .prop("path", moduleHierarchy.relativePath(module))
       .endObject();
     for (DefaultInputModule subModule : moduleHierarchy.children(module)) {
diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/issue/tracking/SourceHashHolderTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/issue/tracking/SourceHashHolderTest.java
index b7605bff482..f2f783a6bb3 100644
--- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/issue/tracking/SourceHashHolderTest.java
+++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/issue/tracking/SourceHashHolderTest.java
@@ -19,18 +19,19 @@
  */
 package org.sonar.scanner.issue.tracking;
 
+import java.io.File;
+import java.nio.charset.StandardCharsets;
 import org.apache.commons.io.FileUtils;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 import org.mockito.Mockito;
+import org.sonar.api.CoreProperties;
+import org.sonar.api.batch.bootstrap.ProjectDefinition;
 import org.sonar.api.batch.fs.InputFile;
 import org.sonar.api.batch.fs.internal.DefaultInputFile;
-import org.sonar.scanner.issue.tracking.ServerLineHashesLoader;
-import org.sonar.scanner.issue.tracking.SourceHashHolder;
-import java.io.File;
-import java.nio.charset.StandardCharsets;
+import org.sonar.api.batch.fs.internal.DefaultInputModule;
 
 import static org.apache.commons.codec.digest.DigestUtils.md5Hex;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -49,6 +50,7 @@ public class SourceHashHolderTest {
   DefaultInputFile file;
 
   private File ioFile;
+  private ProjectDefinition def = ProjectDefinition.create();
 
   @Before
   public void setUp() throws Exception {
@@ -60,7 +62,7 @@ public class SourceHashHolderTest {
     when(file.lines()).thenReturn(1);
     when(file.charset()).thenReturn(StandardCharsets.UTF_8);
 
-    sourceHashHolder = new SourceHashHolder(file, lastSnapshots);
+    sourceHashHolder = new SourceHashHolder(new DefaultInputModule(def, 1), file, lastSnapshots);
   }
 
   @Test
@@ -71,8 +73,6 @@ public class SourceHashHolderTest {
 
     assertThat(sourceHashHolder.getHashedSource().getHash(1)).isEqualTo(md5Hex(source));
     assertThat(sourceHashHolder.getHashedSource().getHash(2)).isEqualTo("");
-    verify(file).key();
-    verify(file).status();
 
     assertThat(sourceHashHolder.getHashedSource().getHash(1)).isEqualTo(md5Hex(source));
   }
@@ -80,9 +80,28 @@ public class SourceHashHolderTest {
   @Test
   public void should_lazy_load_reference_hashes_when_status_changed() throws Exception {
     final String source = "source";
+    FileUtils.write(ioFile, source, StandardCharsets.UTF_8);
+    def.setKey("foo");
+    when(file.relativePath()).thenReturn("src/Foo.java");
     String key = "foo:src/Foo.java";
+    when(file.status()).thenReturn(InputFile.Status.CHANGED);
+    when(lastSnapshots.getLineHashes(key)).thenReturn(new String[] {md5Hex(source)});
+
+    assertThat(sourceHashHolder.getHashedReference().getHash(1)).isEqualTo(md5Hex(source));
+    verify(lastSnapshots).getLineHashes(key);
+
+    assertThat(sourceHashHolder.getHashedReference().getHash(1)).isEqualTo(md5Hex(source));
+    Mockito.verifyNoMoreInteractions(lastSnapshots);
+  }
+
+  @Test
+  public void should_lazy_load_reference_hashes_when_status_changed_on_branch() throws Exception {
+    final String source = "source";
     FileUtils.write(ioFile, source, StandardCharsets.UTF_8);
-    when(file.key()).thenReturn(key);
+    def.setKey("foo");
+    def.properties().put(CoreProperties.PROJECT_BRANCH_PROPERTY, "myBranch");
+    when(file.relativePath()).thenReturn("src/Foo.java");
+    String key = "foo:myBranch:src/Foo.java";
     when(file.status()).thenReturn(InputFile.Status.CHANGED);
     when(lastSnapshots.getLineHashes(key)).thenReturn(new String[] {md5Hex(source)});
 
diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/report/JSONReportTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/report/JSONReportTest.java
index 25fd1ca01fd..59ce9e71958 100644
--- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/report/JSONReportTest.java
+++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/report/JSONReportTest.java
@@ -39,8 +39,8 @@ import org.sonar.api.batch.fs.internal.InputModuleHierarchy;
 import org.sonar.api.batch.fs.internal.TestInputFileBuilder;
 import org.sonar.api.batch.rule.Rules;
 import org.sonar.api.batch.rule.internal.RulesBuilder;
-import org.sonar.api.config.Settings;
 import org.sonar.api.config.MapSettings;
+import org.sonar.api.config.Settings;
 import org.sonar.api.issue.Issue;
 import org.sonar.api.platform.Server;
 import org.sonar.api.rule.RuleKey;
@@ -48,6 +48,7 @@ import org.sonar.scanner.issue.IssueCache;
 import org.sonar.scanner.issue.tracking.TrackedIssue;
 import org.sonar.scanner.protocol.input.ScannerInput;
 import org.sonar.scanner.repository.user.UserRepositoryLoader;
+import org.sonar.scanner.scan.DefaultComponentTree;
 import org.sonar.scanner.scan.filesystem.InputComponentStore;
 
 import static net.javacrumbs.jsonunit.assertj.JsonAssert.assertThatJson;
@@ -83,13 +84,20 @@ public class JSONReportTest {
     DefaultInputDir inputDir = new DefaultInputDir("struts", "src/main/java/org/apache/struts", TestInputFileBuilder.nextBatchId());
     DefaultInputFile inputFile = new TestInputFileBuilder("struts", "src/main/java/org/apache/struts/Action.java").build();
     inputFile.setStatus(InputFile.Status.CHANGED);
-    InputComponentStore fileCache = mock(InputComponentStore.class);
-    when(fileCache.allFilesToPublish()).thenReturn(Collections.singleton(inputFile));
-    when(fileCache.allDirs()).thenReturn(Collections.singleton(inputDir));
+    inputFile.setPublish(true);
+    InputComponentStore fileCache = new InputComponentStore();
+    fileCache.put(inputFile);
+    fileCache.put(inputDir);
 
+    DefaultComponentTree inputComponentTree = new DefaultComponentTree();
     DefaultInputModule rootModule = new DefaultInputModule("struts");
     DefaultInputModule moduleA = new DefaultInputModule("struts-core");
+    inputComponentTree.index(moduleA, rootModule);
     DefaultInputModule moduleB = new DefaultInputModule("struts-ui");
+    inputComponentTree.index(moduleB, rootModule);
+
+    inputComponentTree.index(inputDir, rootModule);
+    inputComponentTree.index(inputFile, inputDir);
 
     when(moduleHierarchy.children(rootModule)).thenReturn(Arrays.asList(moduleA, moduleB));
     when(moduleHierarchy.parent(moduleA)).thenReturn(rootModule);
@@ -100,7 +108,7 @@ public class JSONReportTest {
     RulesBuilder builder = new RulesBuilder();
     builder.add(RuleKey.of("squid", "AvoidCycles")).setName("Avoid Cycles");
     rules = builder.build();
-    jsonReport = new JSONReport(moduleHierarchy, settings, fs, server, rules, issueCache, rootModule, fileCache, userRepository);
+    jsonReport = new JSONReport(moduleHierarchy, settings, fs, server, rules, issueCache, rootModule, fileCache, userRepository, inputComponentTree);
   }
 
   @Test