From 4882ea80f8b10706fef9d6b9a325f8820dde75e3 Mon Sep 17 00:00:00 2001 From: Duarte Meneses Date: Thu, 30 Jan 2020 08:01:11 -0600 Subject: [PATCH] SONAR-11639 Some directories are not collapsed in pull requests --- .../component/ComponentTreeBuilder.java | 91 +++++++++---------- .../step/BuildComponentTreeStepTest.java | 90 ++++++++++++++++-- 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 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 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 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 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; @@ -137,6 +135,65 @@ public class BuildComponentTreeStepTest { context.getStatistics().assertValue("components", 7); } + @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(); @@ -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; + } + } } -- 2.39.5