From 2df15e648ef11b6e3ec40dae2a93ad250cb62170 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Fri, 29 May 2015 14:54:03 +0200 Subject: [PATCH] SONAR-6589 Fix bad uuid computation --- .../PopulateComponentsUuidAndKeyStep.java | 144 +++++++------ .../server/component/ComponentTesting.java | 7 +- .../PopulateComponentsUuidAndKeyStepTest.java | 202 +++++++++++++++++- 3 files changed, 279 insertions(+), 74 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/step/PopulateComponentsUuidAndKeyStep.java b/server/sonar-server/src/main/java/org/sonar/server/computation/step/PopulateComponentsUuidAndKeyStep.java index b7655ee32b4..9f7203905b8 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/step/PopulateComponentsUuidAndKeyStep.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/step/PopulateComponentsUuidAndKeyStep.java @@ -33,7 +33,6 @@ import org.sonar.core.persistence.DbSession; import org.sonar.server.computation.ComputationContext; import org.sonar.server.computation.component.Component; import org.sonar.server.computation.component.ComponentImpl; -import org.sonar.server.computation.component.DepthTraversalTypeAwareVisitor; import org.sonar.server.db.DbClient; /** @@ -49,92 +48,95 @@ public class PopulateComponentsUuidAndKeyStep implements ComputationStep { @Override public void execute(ComputationContext context) { - new ComponentDepthTraversalTypeAwareVisitor(context).visit(context.getRoot()); - } - - @Override - public String getDescription() { - return "Feed components uuid"; - } - - private class ComponentDepthTraversalTypeAwareVisitor extends DepthTraversalTypeAwareVisitor { - - private final BatchReportReader reportReader; - private final Map componentUuidByKey; + DbSession session = dbClient.openSession(false); + try { + BatchReportReader reportReader = context.getReportReader(); + String branch = context.getReportMetadata().hasBranch() ? context.getReportMetadata().getBranch() : null; + BatchReport.Component project = reportReader.readComponent(context.getReportMetadata().getRootComponentRef()); + String projectKey = ComponentKeys.createKey(project.getKey(), branch); - @Nullable - private final String branch; + Map componentUuidsByKey = new HashMap<>(); + List components = dbClient.componentDao().selectComponentsFromProjectKey(session, projectKey); + for (ComponentDto componentDto : components) { + componentUuidsByKey.put(componentDto.getKey(), componentDto.uuid()); + } - private Component nearestModule; + ComponentContext componentContext = new ComponentContext(reportReader, componentUuidsByKey, branch); - public ComponentDepthTraversalTypeAwareVisitor(ComputationContext context) { - super(Component.Type.FILE, Order.PRE_ORDER); - this.componentUuidByKey = new HashMap<>(); - this.branch = context.getReportMetadata().hasBranch() ? context.getReportMetadata().getBranch() : null; - this.reportReader = context.getReportReader(); - this.nearestModule = null; + Component root = context.getRoot(); + processProject(componentContext, root, projectKey); + processChildren(componentContext, root, root); + session.commit(); + } finally { + session.close(); } + } - @Override - public void visitProject(Component project) { - executeForProject(project); - nearestModule = project; + private void recursivelyProcessComponent(ComponentContext componentContext, Component component, Component module) { + switch (component.getType()) { + case MODULE: + processModule(componentContext, component); + processChildren(componentContext, component, component); + break; + case DIRECTORY: + case FILE: + processDirectoryAndFile(componentContext, component, module); + processChildren(componentContext, component, module); + break; + default: + throw new IllegalStateException(String.format("Unsupported component type '%s'", component.getType())); } + } - @Override - public void visitModule(Component module) { - executeForModule(module); - nearestModule = module; + private void processChildren(ComponentContext componentContext, Component component, Component nearestModule) { + for (Component child : component.getChildren()) { + recursivelyProcessComponent(componentContext, child, nearestModule); } + } - @Override - public void visitDirectory(Component directory) { - executeForDirectoryAndFile(directory); - } + private void processProject(ComponentContext componentContext, Component component, String projectKey) { + feedComponent((ComponentImpl) component, projectKey, componentContext.componentUuidsByKey); + } - @Override - public void visitFile(Component file) { - executeForDirectoryAndFile(file); - } + private void processModule(ComponentContext componentContext, Component component) { + BatchReport.Component batchComponent = componentContext.reportReader.readComponent(component.getRef()); + String componentKey = ComponentKeys.createKey(batchComponent.getKey(), componentContext.branch); + feedComponent((ComponentImpl) component, componentKey, componentContext.componentUuidsByKey); + } - private void executeForProject(Component component) { - BatchReport.Component project = reportReader.readComponent(component.getRef()); - String projectKey = ComponentKeys.createKey(project.getKey(), branch); - DbSession session = dbClient.openSession(false); - try { - List components = dbClient.componentDao().selectComponentsFromProjectKey(session, projectKey); - for (ComponentDto componentDto : components) { - componentUuidByKey.put(componentDto.getKey(), componentDto.uuid()); - } - - feedComponent((ComponentImpl) component, projectKey); - } finally { - session.close(); - } - } + private void processDirectoryAndFile(ComponentContext componentContext, Component component, Component module) { + BatchReport.Component batchComponent = componentContext.reportReader.readComponent(component.getRef()); + // TODO fail if path is null + String componentKey = ComponentKeys.createEffectiveKey(module.getKey(), batchComponent.getPath()); + feedComponent((ComponentImpl) component, componentKey, componentContext.componentUuidsByKey); + } - private void executeForModule(Component component) { - BatchReport.Component batchComponent = reportReader.readComponent(component.getRef()); - String componentKey = ComponentKeys.createKey(batchComponent.getKey(), branch); - feedComponent((ComponentImpl) component, componentKey); - } + private void feedComponent(ComponentImpl component, String componentKey, Map componentUuidByKey) { + component.setKey(componentKey); - private void executeForDirectoryAndFile(Component component) { - BatchReport.Component batchComponent = reportReader.readComponent(component.getRef()); - // TODO fail if path is null - String componentKey = ComponentKeys.createEffectiveKey(nearestModule.getKey(), batchComponent.getPath()); - feedComponent((ComponentImpl) component, componentKey); + String componentUuid = componentUuidByKey.get(componentKey); + if (componentUuid == null) { + component.setUuid(Uuids.create()); + } else { + component.setUuid(componentUuid); } + } - private void feedComponent(ComponentImpl component, String componentKey) { - component.setKey(componentKey); + private static class ComponentContext { + private final BatchReportReader reportReader; + private final Map componentUuidsByKey; + private final String branch; - String componentUuid = componentUuidByKey.get(componentKey); - if (componentUuid == null) { - component.setUuid(Uuids.create()); - } else { - component.setUuid(componentUuid); - } + public ComponentContext(BatchReportReader reportReader, Map componentUuidsByKey, @Nullable String branch) { + this.reportReader = reportReader; + this.componentUuidsByKey = componentUuidsByKey; + this.branch = branch; } } + + @Override + public String getDescription() { + return "Feed components uuid"; + } + } diff --git a/server/sonar-server/src/test/java/org/sonar/server/component/ComponentTesting.java b/server/sonar-server/src/test/java/org/sonar/server/component/ComponentTesting.java index 8f59656ba35..8578e2fedc9 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/component/ComponentTesting.java +++ b/server/sonar-server/src/test/java/org/sonar/server/component/ComponentTesting.java @@ -46,8 +46,7 @@ public class ComponentTesting { .setLanguage("xoo"); } - public static ComponentDto newDirectory(ComponentDto module, String path) { - String uuid = Uuids.create(); + public static ComponentDto newDirectory(ComponentDto module, String uuid, String path) { return newChildComponent(uuid, module) .setKey(!path.equals("/") ? module.getKey() + ":" + path : module.getKey() + ":/") .setName(path) @@ -57,6 +56,10 @@ public class ComponentTesting { .setQualifier(Qualifiers.DIRECTORY); } + public static ComponentDto newDirectory(ComponentDto module, String path) { + return newDirectory(module, Uuids.create(), path); + } + public static ComponentDto newModuleDto(String uuid, ComponentDto subProjectOrProject) { return newChildComponent(uuid, subProjectOrProject) .setModuleUuidPath(subProjectOrProject.moduleUuidPath() + uuid + MODULE_UUID_PATH_SEP) diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/step/PopulateComponentsUuidAndKeyStepTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/step/PopulateComponentsUuidAndKeyStepTest.java index 66140f7aa7f..6625ca01266 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/step/PopulateComponentsUuidAndKeyStepTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/step/PopulateComponentsUuidAndKeyStepTest.java @@ -23,6 +23,7 @@ package org.sonar.server.computation.step; import java.io.File; import java.util.HashMap; import java.util.Map; +import org.junit.After; import org.junit.Before; import org.junit.ClassRule; import org.junit.Rule; @@ -34,8 +35,10 @@ import org.sonar.batch.protocol.Constants; import org.sonar.batch.protocol.output.BatchReport; import org.sonar.batch.protocol.output.BatchReportReader; import org.sonar.batch.protocol.output.BatchReportWriter; +import org.sonar.core.component.ComponentDto; import org.sonar.core.persistence.DbSession; import org.sonar.core.persistence.DbTester; +import org.sonar.server.component.ComponentTesting; import org.sonar.server.component.db.ComponentDao; import org.sonar.server.computation.ComputationContext; import org.sonar.server.computation.component.Component; @@ -82,13 +85,18 @@ public class PopulateComponentsUuidAndKeyStepTest extends BaseStepTest { sut = new PopulateComponentsUuidAndKeyStep(dbClient); } + @After + public void tearDown() throws Exception { + session.close(); + } + @Override protected ComputationStep step() { return sut; } @Test - public void add_components() throws Exception { + public void compute_keys_and_uuids() throws Exception { File reportDir = temp.newFolder(); BatchReportWriter writer = new BatchReportWriter(reportDir); writer.writeMetadata(BatchReport.Metadata.newBuilder() @@ -139,6 +147,67 @@ public class PopulateComponentsUuidAndKeyStepTest extends BaseStepTest { assertThat(componentsByRef.get(4).getUuid()).isNotNull(); } + @Test + public void return_existing_uuids() throws Exception { + ComponentDto project = ComponentTesting.newProjectDto("ABCD").setKey(PROJECT_KEY); + dbClient.componentDao().insert(session, project); + ComponentDto module = ComponentTesting.newModuleDto("BCDE", project).setKey("MODULE_KEY"); + dbClient.componentDao().insert(session, module); + ComponentDto directory = ComponentTesting.newDirectory(module, "CDEF", "src/main/java/dir").setKey("MODULE_KEY:src/main/java/dir"); + ComponentDto file = ComponentTesting.newFileDto(module, "DEFG").setKey("MODULE_KEY:src/main/java/dir/Foo.java"); + dbClient.componentDao().insert(session, directory, file); + session.commit(); + + File reportDir = temp.newFolder(); + BatchReportWriter writer = new BatchReportWriter(reportDir); + writer.writeMetadata(BatchReport.Metadata.newBuilder() + .setRootComponentRef(1) + .build()); + + writer.writeComponent(BatchReport.Component.newBuilder() + .setRef(1) + .setType(Constants.ComponentType.PROJECT) + .setKey(PROJECT_KEY) + .addChildRef(2) + .build()); + writer.writeComponent(BatchReport.Component.newBuilder() + .setRef(2) + .setType(Constants.ComponentType.MODULE) + .setKey("MODULE_KEY") + .addChildRef(3) + .build()); + writer.writeComponent(BatchReport.Component.newBuilder() + .setRef(3) + .setType(Constants.ComponentType.DIRECTORY) + .setPath("src/main/java/dir") + .addChildRef(4) + .build()); + writer.writeComponent(BatchReport.Component.newBuilder() + .setRef(4) + .setType(Constants.ComponentType.FILE) + .setPath("src/main/java/dir/Foo.java") + .build()); + + BatchReportReader batchReportReader = new BatchReportReader(reportDir); + ComputationContext context = new ComputationContext(batchReportReader, PROJECT_KEY, projectSettings, + dbClient, ComponentTreeBuilders.from(batchReportReader), languageRepository); + sut.execute(context); + + Map componentsByRef = getComponentsByRef(context.getRoot()); + + assertThat(componentsByRef.get(1).getKey()).isEqualTo(PROJECT_KEY); + assertThat(componentsByRef.get(1).getUuid()).isEqualTo("ABCD"); + + assertThat(componentsByRef.get(2).getKey()).isEqualTo("MODULE_KEY"); + assertThat(componentsByRef.get(2).getUuid()).isEqualTo("BCDE"); + + assertThat(componentsByRef.get(3).getKey()).isEqualTo("MODULE_KEY:src/main/java/dir"); + assertThat(componentsByRef.get(3).getUuid()).isEqualTo("CDEF"); + + assertThat(componentsByRef.get(4).getKey()).isEqualTo("MODULE_KEY:src/main/java/dir/Foo.java"); + assertThat(componentsByRef.get(4).getUuid()).isEqualTo("DEFG"); + } + @Test public void use_latest_module_for_files_key() throws Exception { File reportDir = temp.newFolder(); @@ -240,6 +309,137 @@ public class PopulateComponentsUuidAndKeyStepTest extends BaseStepTest { assertThat(componentsByRef.get(4).getKey()).isEqualTo("MODULE_KEY:origin/master:src/main/java/dir/Foo.java"); } + @Test + public void compute_keys_and_uuids_on_project_having_module_and_directory() throws Exception { + File reportDir = temp.newFolder(); + BatchReportWriter writer = new BatchReportWriter(reportDir); + writer.writeMetadata(BatchReport.Metadata.newBuilder() + .setRootComponentRef(1) + .build()); + + writer.writeComponent(BatchReport.Component.newBuilder() + .setRef(1) + .setType(Constants.ComponentType.PROJECT) + .setKey(PROJECT_KEY) + .addChildRef(2) + .addChildRef(5) + .build()); + writer.writeComponent(BatchReport.Component.newBuilder() + .setRef(2) + .setType(Constants.ComponentType.MODULE) + .setKey("MODULE_KEY") + .addChildRef(3) + .build()); + writer.writeComponent(BatchReport.Component.newBuilder() + .setRef(3) + .setType(Constants.ComponentType.DIRECTORY) + .setPath("src/main/java/dir") + .addChildRef(4) + .build()); + writer.writeComponent(BatchReport.Component.newBuilder() + .setRef(4) + .setType(Constants.ComponentType.FILE) + .setPath("src/main/java/dir/Foo.java") + .build()); + writer.writeComponent(BatchReport.Component.newBuilder() + .setRef(5) + .setType(Constants.ComponentType.DIRECTORY) + .setPath("/") + .addChildRef(6) + .build()); + writer.writeComponent(BatchReport.Component.newBuilder() + .setRef(6) + .setType(Constants.ComponentType.FILE) + .setPath("pom.xml") + .build()); + + BatchReportReader batchReportReader = new BatchReportReader(reportDir); + ComputationContext context = new ComputationContext(batchReportReader, PROJECT_KEY, projectSettings, + dbClient, ComponentTreeBuilders.from(batchReportReader), languageRepository); + sut.execute(context); + + Map componentsByRef = getComponentsByRef(context.getRoot()); + + assertThat(componentsByRef.get(1).getKey()).isEqualTo(PROJECT_KEY); + assertThat(componentsByRef.get(1).getUuid()).isNotNull(); + + assertThat(componentsByRef.get(2).getKey()).isEqualTo("MODULE_KEY"); + assertThat(componentsByRef.get(2).getUuid()).isNotNull(); + + assertThat(componentsByRef.get(3).getKey()).isEqualTo("MODULE_KEY:src/main/java/dir"); + assertThat(componentsByRef.get(3).getUuid()).isNotNull(); + + assertThat(componentsByRef.get(4).getKey()).isEqualTo("MODULE_KEY:src/main/java/dir/Foo.java"); + assertThat(componentsByRef.get(4).getUuid()).isNotNull(); + + assertThat(componentsByRef.get(5).getKey()).isEqualTo(PROJECT_KEY + ":/"); + assertThat(componentsByRef.get(5).getUuid()).isNotNull(); + + assertThat(componentsByRef.get(6).getKey()).isEqualTo(PROJECT_KEY + ":pom.xml"); + assertThat(componentsByRef.get(6).getUuid()).isNotNull(); + } + + @Test + public void compute_keys_and_uuids_on_multi_modules() throws Exception { + File reportDir = temp.newFolder(); + BatchReportWriter writer = new BatchReportWriter(reportDir); + writer.writeMetadata(BatchReport.Metadata.newBuilder() + .setRootComponentRef(1) + .build()); + + writer.writeComponent(BatchReport.Component.newBuilder() + .setRef(1) + .setType(Constants.ComponentType.PROJECT) + .setKey(PROJECT_KEY) + .addChildRef(2) + .build()); + writer.writeComponent(BatchReport.Component.newBuilder() + .setRef(2) + .setType(Constants.ComponentType.MODULE) + .setKey("MODULE_KEY") + .addChildRef(3) + .build()); + writer.writeComponent(BatchReport.Component.newBuilder() + .setRef(3) + .setType(Constants.ComponentType.MODULE) + .setKey("SUB_MODULE_KEY") + .addChildRef(4) + .build()); + writer.writeComponent(BatchReport.Component.newBuilder() + .setRef(4) + .setType(Constants.ComponentType.DIRECTORY) + .setPath("src/main/java/dir") + .addChildRef(5) + .build()); + writer.writeComponent(BatchReport.Component.newBuilder() + .setRef(5) + .setType(Constants.ComponentType.FILE) + .setPath("src/main/java/dir/Foo.java") + .build()); + + BatchReportReader batchReportReader = new BatchReportReader(reportDir); + ComputationContext context = new ComputationContext(batchReportReader, PROJECT_KEY, projectSettings, + dbClient, ComponentTreeBuilders.from(batchReportReader), languageRepository); + sut.execute(context); + + Map componentsByRef = getComponentsByRef(context.getRoot()); + + assertThat(componentsByRef.get(1).getKey()).isEqualTo(PROJECT_KEY); + assertThat(componentsByRef.get(1).getUuid()).isNotNull(); + + assertThat(componentsByRef.get(2).getKey()).isEqualTo("MODULE_KEY"); + assertThat(componentsByRef.get(2).getUuid()).isNotNull(); + + assertThat(componentsByRef.get(3).getKey()).isEqualTo("SUB_MODULE_KEY"); + assertThat(componentsByRef.get(3).getUuid()).isNotNull(); + + assertThat(componentsByRef.get(4).getKey()).isEqualTo("SUB_MODULE_KEY:src/main/java/dir"); + assertThat(componentsByRef.get(4).getUuid()).isNotNull(); + + assertThat(componentsByRef.get(5).getKey()).isEqualTo("SUB_MODULE_KEY:src/main/java/dir/Foo.java"); + assertThat(componentsByRef.get(5).getUuid()).isNotNull(); + } + private static Map getComponentsByRef(Component root) { Map componentsByRef = new HashMap<>(); feedComponentByRef(root, componentsByRef); -- 2.39.5