SONAR-11639 Some directories are not collapsed in pull requests

2 files changed, 125 insertions, 56 deletions

diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/ComponentTreeBuilder.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/ComponentTreeBuilder.java
index 01844871aaa..7a7bd3b5b51 100644
--- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/ComponentTreeBuilder.java
+++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/ComponentTreeBuilder.java
@@ -19,7 +19,6 @@
  */
 package org.sonar.ce.task.projectanalysis.component;
 
-import com.google.common.base.Preconditions;
 import java.util.ArrayList;
 import java.util.LinkedHashMap;
 import java.util.LinkedList;
@@ -42,6 +41,7 @@ import static java.lang.String.format;
 import static java.util.Objects.requireNonNull;
 import static org.apache.commons.lang.StringUtils.removeStart;
 import static org.apache.commons.lang.StringUtils.trimToNull;
+import static org.sonar.scanner.protocol.output.ScannerReport.Component.ComponentType.FILE;
 
 public class ComponentTreeBuilder {
 
@@ -64,7 +64,6 @@ public class ComponentTreeBuilder {
   private final Function<Integer, ScannerReport.Component> scannerComponentSupplier;
   private final Project project;
   private final Branch branch;
-  @Nullable
   private final ProjectAttributes projectAttributes;
 
   private ScannerReport.Component rootComponent;
@@ -97,11 +96,10 @@ public class ComponentTreeBuilder {
   }
 
   private Node createProjectHierarchy(ScannerReport.Component rootComponent) {
-    Preconditions.checkArgument(rootComponent.getType() == ScannerReport.Component.ComponentType.PROJECT, "Expected root component of type 'PROJECT'");
+    checkArgument(rootComponent.getType() == ScannerReport.Component.ComponentType.PROJECT, "Expected root component of type 'PROJECT'");
 
     LinkedList<ScannerReport.Component> queue = new LinkedList<>();
-    rootComponent.getChildRefList()
-      .stream()
+    rootComponent.getChildRefList().stream()
       .map(scannerComponentSupplier)
       .forEach(queue::addLast);
 
@@ -110,19 +108,14 @@ public class ComponentTreeBuilder {
 
     while (!queue.isEmpty()) {
       ScannerReport.Component component = queue.removeFirst();
-      switch (component.getType()) {
-        case FILE:
-          addFile(root, component);
-          break;
-        default:
-          throw new IllegalArgumentException(format("Unsupported component type '%s'", component.getType()));
-      }
+      checkArgument(component.getType() == FILE, "Unsupported component type '%s'", component.getType());
+      addFile(root, component);
     }
     return root;
   }
 
   private static void addFile(Node root, ScannerReport.Component file) {
-    Preconditions.checkArgument(!StringUtils.isEmpty(file.getProjectRelativePath()), "Files should have a project relative path: " + file);
+    checkArgument(!StringUtils.isEmpty(file.getProjectRelativePath()), "Files should have a project relative path: " + file);
     String[] split = StringUtils.split(file.getProjectRelativePath(), '/');
     Node currentNode = root;
 
@@ -137,7 +130,7 @@ public class ComponentTreeBuilder {
     ScannerReport.Component component = node.reportComponent();
 
     if (component != null) {
-      if (component.getType() == ScannerReport.Component.ComponentType.FILE) {
+      if (component.getType() == FILE) {
         return buildFile(component);
       } else if (component.getType() == ScannerReport.Component.ComponentType.PROJECT) {
         return buildProject(childComponents);
@@ -220,72 +213,76 @@ public class ComponentTreeBuilder {
   }
 
   public Component buildChangedComponentTreeRoot(Component project) {
-    return buildChangedComponentTree(project);
-  }
-
-  private static ComponentImpl.Builder changedComponentBuilder(Component component) {
-    return ComponentImpl.builder(component.getType())
-      .setUuid(component.getUuid())
-      .setDbKey(component.getDbKey())
-      .setKey(component.getKey())
-      .setStatus(component.getStatus())
-      .setReportAttributes(component.getReportAttributes())
-      .setName(component.getName())
-      .setShortName(component.getShortName())
-      .setDescription(component.getDescription());
+    return buildChangedComponentTree(project, "");
   }
 
   @Nullable
-  private static Component buildChangedComponentTree(Component component) {
+  private static Component buildChangedComponentTree(Component component, String parentPath) {
     switch (component.getType()) {
       case PROJECT:
         return buildChangedProject(component);
       case DIRECTORY:
-        return buildChangedIntermediate(component);
+        return buildChangedDirectory(component, parentPath);
       case FILE:
         return buildChangedFile(component);
-
       default:
         throw new IllegalArgumentException(format("Unsupported component type '%s'", component.getType()));
     }
   }
 
   private static Component buildChangedProject(Component component) {
-    return changedComponentBuilder(component)
+    return changedComponentBuilder(component, "", "")
       .setProjectAttributes(new ProjectAttributes(component.getProjectAttributes()))
       .addChildren(buildChangedComponentChildren(component))
       .build();
   }
 
   @Nullable
-  private static Component buildChangedIntermediate(Component component) {
+  private static Component buildChangedDirectory(Component component, String parentPath) {
     List<Component> children = buildChangedComponentChildren(component);
     if (children.isEmpty()) {
       return null;
     }
-    return changedComponentBuilder(component)
-      .addChildren(children)
-      .build();
-  }
 
-  @Nullable
-  private static Component buildChangedFile(Component component) {
-    if (component.getStatus() == Component.Status.SAME) {
-      return null;
+    if (children.size() == 1 && children.get(0).getType() == Component.Type.DIRECTORY) {
+      Component child = children.get(0);
+      return changedComponentBuilder(child, parentPath, child.getName())
+        .addChildren(child.getChildren())
+        .build();
+    } else {
+      return changedComponentBuilder(component, parentPath, component.getName())
+        .addChildren(children)
+        .build();
     }
-    return changedComponentBuilder(component)
-      .setFileAttributes(component.getFileAttributes())
-      .build();
   }
 
   private static List<Component> buildChangedComponentChildren(Component component) {
-    return component.getChildren()
-      .stream()
-      .map(ComponentTreeBuilder::buildChangedComponentTree)
+    return component.getChildren().stream()
+      .map(c -> ComponentTreeBuilder.buildChangedComponentTree(c, component.getName()))
       .filter(Objects::nonNull)
       .collect(MoreCollectors.toList());
   }
 
+  private static ComponentImpl.Builder changedComponentBuilder(Component component, String parentPath, String path) {
+    return ComponentImpl.builder(component.getType())
+      .setUuid(component.getUuid())
+      .setDbKey(component.getDbKey())
+      .setKey(component.getKey())
+      .setStatus(component.getStatus())
+      .setReportAttributes(component.getReportAttributes())
+      .setName(component.getName())
+      .setShortName(removeStart(removeStart(path, parentPath), "/"))
+      .setDescription(component.getDescription());
+  }
+
+  @Nullable
+  private static Component buildChangedFile(Component component) {
+    if (component.getStatus() == Component.Status.SAME) {
+      return null;
+    }
+    return component;
+  }
+
   private void setNameAndDescription(ScannerReport.Component component, ComponentImpl.Builder builder) {
     if (branch.isMain()) {
       builder
@@ -340,7 +337,7 @@ public class ComponentTreeBuilder {
   }
 
   private static FileAttributes createFileAttributes(ScannerReport.Component component) {
-    checkArgument(component.getType() == ScannerReport.Component.ComponentType.FILE);
+    checkArgument(component.getType() == FILE);
     checkArgument(component.getLines() > 0, "File '%s' has no line", component.getProjectRelativePath());
     String lang = trimToNull(component.getLanguage());
     return new FileAttributes(
diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/BuildComponentTreeStepTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/BuildComponentTreeStepTest.java
index 3c5307a120a..153cbf4d0d6 100644
--- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/BuildComponentTreeStepTest.java
+++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/BuildComponentTreeStepTest.java
@@ -40,6 +40,7 @@ import org.sonar.ce.task.projectanalysis.component.ReportModulesPath;
 import org.sonar.ce.task.step.TestComputationStepContext;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbTester;
+import org.sonar.db.component.BranchType;
 import org.sonar.db.component.ComponentDto;
 import org.sonar.db.component.SnapshotDto;
 import org.sonar.db.organization.OrganizationDto;
@@ -74,16 +75,13 @@ public class BuildComponentTreeStepTest {
   private static final int UNCHANGED_FILE_REF = 10;
 
   private static final String REPORT_PROJECT_KEY = "REPORT_PROJECT_KEY";
-  private static final String REPORT_MODULE_KEY = "MODULE_KEY";
   private static final String REPORT_DIR_PATH_1 = "src/main/java/dir1";
   private static final String REPORT_FILE_PATH_1 = "src/main/java/dir1/File1.java";
   private static final String REPORT_FILE_NAME_1 = "File1.java";
   private static final String REPORT_DIR_PATH_2 = "src/main/java/dir2";
   private static final String REPORT_FILE_PATH_2 = "src/main/java/dir2/File2.java";
   private static final String REPORT_FILE_PATH_3 = "src/main/java/dir2/File3.java";
-  private static final String REPORT_LEAFLESS_MODULE_KEY = "LEAFLESS_MODULE_KEY";
-  private static final String REPORT_LEAFLESS_DIR_PATH = "src/main/java/leafless";
-  private static final String REPORT_UNCHANGED_FILE_PATH = "src/main/java/leafless/File3.java";
+  private static final String REPORT_UNCHANGED_FILE_PATH = "src/main/File3.java";
 
   private static final long ANALYSIS_DATE = 123456789L;
 
@@ -138,6 +136,65 @@ public class BuildComponentTreeStepTest {
   }
 
   @Test
+  public void verify_tree_is_correctly_built_in_prs() {
+    setAnalysisMetadataHolder(true);
+    reportReader.putComponent(component(ROOT_REF, PROJECT, REPORT_PROJECT_KEY, FILE_1_REF, FILE_2_REF, FILE_3_REF, UNCHANGED_FILE_REF));
+    reportReader.putComponent(componentWithPath(FILE_1_REF, FILE, REPORT_FILE_PATH_1));
+    reportReader.putComponent(componentWithPath(FILE_2_REF, FILE, REPORT_FILE_PATH_2));
+    reportReader.putComponent(componentWithPath(FILE_3_REF, FILE, REPORT_FILE_PATH_3));
+    reportReader.putComponent(unchangedComponentWithPath(UNCHANGED_FILE_REF, FILE, REPORT_UNCHANGED_FILE_PATH));
+
+    TestComputationStepContext context = new TestComputationStepContext();
+    underTest.execute(context);
+
+    // modified root
+    Component mRoot = treeRootHolder.getRoot();
+    verifyComponent(mRoot, Component.Type.PROJECT, ROOT_REF, 1);
+
+    Component mDir = mRoot.getChildren().get(0);
+    assertThat(mDir.getName()).isEqualTo("src/main/java");
+    verifyComponent(mDir, Component.Type.DIRECTORY, null, 2);
+
+    Component mDir1 = mDir.getChildren().get(0);
+    assertThat(mDir1.getName()).isEqualTo("src/main/java/dir1");
+    verifyComponent(mDir1, Component.Type.DIRECTORY, null, 1);
+    verifyComponent(mDir1.getChildren().get(0), Component.Type.FILE, FILE_1_REF, 0);
+
+    Component mDir2 = mDir.getChildren().get(1);
+    assertThat(mDir2.getName()).isEqualTo("src/main/java/dir2");
+    verifyComponent(mDir2, Component.Type.DIRECTORY, null, 2);
+    verifyComponent(mDir2.getChildren().get(0), Component.Type.FILE, FILE_2_REF, 0);
+    verifyComponent(mDir2.getChildren().get(1), Component.Type.FILE, FILE_3_REF, 0);
+
+    // root
+    Component root = treeRootHolder.getReportTreeRoot();
+    verifyComponent(root, Component.Type.PROJECT, ROOT_REF, 1);
+
+    Component dir = root.getChildren().get(0);
+    assertThat(dir.getName()).isEqualTo("src/main");
+    verifyComponent(dir, Component.Type.DIRECTORY, null, 2);
+
+    Component dir1 = dir.getChildren().get(0);
+    assertThat(dir1.getName()).isEqualTo("src/main/java");
+    verifyComponent(dir1, Component.Type.DIRECTORY, null, 2);
+    verifyComponent(dir1.getChildren().get(0), Component.Type.DIRECTORY, null, 1);
+    verifyComponent(dir1.getChildren().get(1), Component.Type.DIRECTORY, null, 2);
+
+    Component dir2 = dir1.getChildren().get(0);
+    assertThat(dir2.getName()).isEqualTo("src/main/java/dir1");
+    verifyComponent(dir2, Component.Type.DIRECTORY, null, 1);
+    verifyComponent(dir2.getChildren().get(0), Component.Type.FILE, FILE_1_REF, 0);
+
+    Component dir3 = dir1.getChildren().get(1);
+    assertThat(dir3.getName()).isEqualTo("src/main/java/dir2");
+    verifyComponent(dir3, Component.Type.DIRECTORY, null, 2);
+    verifyComponent(dir3.getChildren().get(0), Component.Type.FILE, FILE_2_REF, 0);
+    verifyComponent(dir3.getChildren().get(1), Component.Type.FILE, FILE_3_REF, 0);
+
+    context.getStatistics().assertValue("components", 7);
+  }
+
+  @Test
   public void compute_keys_and_uuids() {
     setAnalysisMetadataHolder();
     reportReader.putComponent(component(ROOT_REF, PROJECT, REPORT_PROJECT_KEY, FILE_1_REF));
@@ -426,18 +483,22 @@ public class BuildComponentTreeStepTest {
   }
 
   private static ScannerReport.Component component(int componentRef, ComponentType componentType, String key, int... children) {
-    return component(componentRef, componentType, key, null, children);
+    return component(componentRef, componentType, key, FileStatus.CHANGED, null, children);
+  }
+
+  private static ScannerReport.Component unchangedComponentWithPath(int componentRef, ComponentType componentType, String path, int... children) {
+    return component(componentRef, componentType, REPORT_PROJECT_KEY + ":" + path, FileStatus.SAME, path, children);
   }
 
   private static ScannerReport.Component componentWithPath(int componentRef, ComponentType componentType, String path, int... children) {
-    return component(componentRef, componentType, REPORT_PROJECT_KEY + ":" + path, path, children);
+    return component(componentRef, componentType, REPORT_PROJECT_KEY + ":" + path, FileStatus.CHANGED, path, children);
   }
 
-  private static ScannerReport.Component component(int componentRef, ComponentType componentType, String key, @Nullable String path, int... children) {
+  private static ScannerReport.Component component(int componentRef, ComponentType componentType, String key, FileStatus status, @Nullable String path, int... children) {
     ScannerReport.Component.Builder builder = ScannerReport.Component.newBuilder()
       .setType(componentType)
       .setRef(componentRef)
-      .setStatus(FileStatus.UNAVAILABLE)
+      .setStatus(status)
       .setLines(1)
       .setKey(key);
     if (path != null) {
@@ -490,9 +551,14 @@ public class BuildComponentTreeStepTest {
   }
 
   private void setAnalysisMetadataHolder() {
+    setAnalysisMetadataHolder(false);
+  }
+
+  private void setAnalysisMetadataHolder(boolean isPr) {
+    Branch branch = isPr ? new PrBranch() : new DefaultBranchImpl();
     analysisMetadataHolder.setRootComponentRef(ROOT_REF)
       .setAnalysisDate(ANALYSIS_DATE)
-      .setBranch(new DefaultBranchImpl())
+      .setBranch(branch)
       .setProject(Project.from(newPrivateProjectDto(newOrganizationDto()).setDbKey(REPORT_PROJECT_KEY)));
   }
 
@@ -505,4 +571,10 @@ public class BuildComponentTreeStepTest {
     return builder.build();
   }
 
+  private static class PrBranch extends DefaultBranchImpl {
+    @Override
+    public BranchType getType() {
+      return BranchType.PULL_REQUEST;
+    }
+  }
 }