SONAR-6589 Fix bad key computation

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 b7655ee32b4608c70d00374465eba1d091bd05ca..12972249c5eef3f95556c9f397fd95df67132e24 100644 (file)
--- 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<String, String> 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<String, String> componentUuidByKey = new HashMap<>();
+      List<ComponentDto> components = dbClient.componentDao().selectComponentsFromProjectKey(session, projectKey);
+      for (ComponentDto componentDto : components) {
+        componentUuidByKey.put(componentDto.getKey(), componentDto.uuid());
+      }
 
-    private Component nearestModule;
+      ComponentContext componentContext = new ComponentContext(reportReader, componentUuidByKey, 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.componentUuidByKey);
+  }
 
-    @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.componentUuidByKey);
+  }
 
-    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<ComponentDto> 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.componentUuidByKey);
+  }
 
-    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<String, String> 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<String, String> componentUuidByKey;
+    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<String, String> componentUuidByKey, @Nullable String branch) {
+      this.reportReader = reportReader;
+      this.componentUuidByKey = new HashMap<>();
+      this.branch = branch;
     }
   }
+
+  @Override
+  public String getDescription() {
+    return "Feed components uuid";
+  }
+
 }

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 66140f7aa7fcf7c6fb3c6e57ac9249409e405e32..f9c04fd40fb99f9047bed1ae8842ba22d983a50e 100644 (file)
--- 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
@@ -88,7 +88,7 @@ public class PopulateComponentsUuidAndKeyStepTest extends BaseStepTest {
   }
 
   @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()
@@ -240,6 +240,76 @@ 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<Integer, Component> 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();
+  }
+
   private static Map<Integer, Component> getComponentsByRef(Component root) {
     Map<Integer, Component> componentsByRef = new HashMap<>();
     feedComponentByRef(root, componentsByRef);