SONAR-11463 Prefers existing DB key to migrated ones

diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/ComponentUuidFactoryWithMigration.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/ComponentUuidFactoryWithMigration.java
index 05a906f498a273e9feaf6a671aed9642ae5859a7..f0e64a37ce4536afa3c6b2c83af2e9dd57dd4f8d 100644 (file)
--- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/ComponentUuidFactoryWithMigration.java
+++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/component/ComponentUuidFactoryWithMigration.java
@@ -38,29 +38,23 @@ import org.sonar.db.component.ComponentWithModuleUuidDto;
 import org.sonar.db.component.KeyWithUuidDto;
 
 public class ComponentUuidFactoryWithMigration implements ComponentUuidFactory {
-  private final Map<String, String> uuidsByKey = new HashMap<>();
+  private final Map<String, String> uuidsByDbKey = new HashMap<>();
+  private final Map<String, String> uuidsByMigratedKey = new HashMap<>();
 
   public ComponentUuidFactoryWithMigration(DbClient dbClient, DbSession dbSession, String rootKey, Function<String, String> pathToKey, Map<String, String> reportModulesPath) {
     Map<String, String> modulePathsByUuid;
+    List<KeyWithUuidDto> keys = dbClient.componentDao().selectUuidsByKeyFromProjectKey(dbSession, rootKey);
+    keys.forEach(dto -> uuidsByDbKey.put(dto.key(), dto.uuid()));
 
-    if (reportModulesPath.isEmpty()) {
-      noMigration(dbClient, dbSession, rootKey);
-    } else {
+    if (!reportModulesPath.isEmpty()) {
       modulePathsByUuid = loadModulePathsByUuid(dbClient, dbSession, rootKey, reportModulesPath);
 
-      if (modulePathsByUuid.isEmpty()) {
-        noMigration(dbClient, dbSession, rootKey);
-      } else {
+      if (!modulePathsByUuid.isEmpty()) {
         doMigration(dbClient, dbSession, rootKey, pathToKey, modulePathsByUuid);
       }
     }
   }
 
-  private void noMigration(DbClient dbClient, DbSession dbSession, String rootKey) {
-    List<KeyWithUuidDto> keys = dbClient.componentDao().selectUuidsByKeyFromProjectKey(dbSession, rootKey);
-    keys.forEach(dto -> uuidsByKey.put(dto.key(), dto.uuid()));
-  }
-
   private void doMigration(DbClient dbClient, DbSession dbSession, String rootKey, Function<String, String> pathToKey, Map<String, String> modulePathsByUuid) {
     List<ComponentWithModuleUuidDto> dtos = loadComponentsWithModuleUuid(dbClient, dbSession, rootKey);
     for (ComponentWithModuleUuidDto dto : dtos) {
@@ -74,12 +68,12 @@ public class ComponentUuidFactoryWithMigration implements ComponentUuidFactory {
         if (modulePathFromRootProject != null || StringUtils.isEmpty(dto.moduleUuid())) {
           // means that it's a root or a module with a valid path (to avoid overwriting key of root)
           pathToKey.apply(modulePathFromRootProject);
-          uuidsByKey.put(pathToKey.apply(modulePathFromRootProject), dto.uuid());
+          uuidsByMigratedKey.put(pathToKey.apply(modulePathFromRootProject), dto.uuid());
         }
       } else {
         String modulePathFromRootProject = modulePathsByUuid.get(dto.moduleUuid());
         String componentPath = createComponentPath(dto, modulePathFromRootProject);
-        uuidsByKey.put(pathToKey.apply(componentPath), dto.uuid());
+        uuidsByMigratedKey.put(pathToKey.apply(componentPath), dto.uuid());
       }
     }
   }
@@ -123,10 +117,10 @@ public class ComponentUuidFactoryWithMigration implements ComponentUuidFactory {
   }
 
   /**
-   * Get UUID from database if it exists, otherwise generate a new one.
+   * Get UUID from component having the same key in database if it exists, otherwise look for migrated keys, and finally generate a new one.
    */
   @Override
   public String getOrCreateForKey(String key) {
-    return uuidsByKey.computeIfAbsent(key, k -> Uuids.create());
+    return uuidsByDbKey.computeIfAbsent(key, k1 -> uuidsByMigratedKey.computeIfAbsent(k1, k2 -> Uuids.create()));
   }
 }

diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/component/ComponentUuidFactoryWithMigrationTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/component/ComponentUuidFactoryWithMigrationTest.java
index b394b9c600ad2decc5d1c3ebc00789130aabcd1d..aa53f0ee73f19ae8f5940341282a960f2dff1fe7 100644 (file)
--- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/component/ComponentUuidFactoryWithMigrationTest.java
+++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/component/ComponentUuidFactoryWithMigrationTest.java
@@ -48,7 +48,7 @@ public class ComponentUuidFactoryWithMigrationTest {
     ComponentUuidFactoryWithMigration underTest = new ComponentUuidFactoryWithMigration(db.getDbClient(), db.getSession(), project.getDbKey(), pathToKey, reportModulesPath);
 
     assertThat(underTest.getOrCreateForKey(project.getDbKey())).isEqualTo(project.uuid());
-    assertThat(underTest.getOrCreateForKey(module.getDbKey())).isNotIn(project.uuid(), module.uuid());
+    assertThat(underTest.getOrCreateForKey(module.getDbKey())).isEqualTo(module.uuid());
   }
 
   @Test
@@ -77,12 +77,6 @@ public class ComponentUuidFactoryWithMigrationTest {
 
     // project remains the same
     assertThat(underTest.getOrCreateForKey(project.getDbKey())).isEqualTo(project.uuid());
-
-    // old keys with modules don't exist
-    assertThat(underTest.getOrCreateForKey(module1.getDbKey())).isNotIn(project.uuid(), module1.uuid(), module2.uuid(), file1.uuid(), file2.uuid());
-    assertThat(underTest.getOrCreateForKey(module2.getDbKey())).isNotIn(project.uuid(), module1.uuid(), module2.uuid(), file1.uuid(), file2.uuid());
-    assertThat(underTest.getOrCreateForKey(file1.getDbKey())).isNotIn(project.uuid(), module1.uuid(), module2.uuid(), file1.uuid(), file2.uuid());
-    assertThat(underTest.getOrCreateForKey(file2.getDbKey())).isNotIn(project.uuid(), module1.uuid(), module2.uuid(), file1.uuid(), file2.uuid());
   }
 
   @Test
@@ -133,6 +127,30 @@ public class ComponentUuidFactoryWithMigrationTest {
     assertThat(underTest.getOrCreateForKey(project.getDbKey())).isEqualTo(project.uuid());
   }
 
+  @Test
+  public void prefers_component_having_same_key() {
+    ComponentDto project = db.components().insertPrivateProject(dto -> dto.setDbKey("project"));
+    ComponentDto module1 = db.components().insertComponent(ComponentTesting.newModuleDto(project)
+      .setDbKey("project:module1"));
+    ComponentDto file1 = db.components().insertComponent(ComponentTesting.newFileDto(module1)
+      .setDbKey("project:module1:file1")
+      .setPath("file1"));
+    ComponentDto disabledFileSameKey = db.components().insertComponent(ComponentTesting.newFileDto(project)
+      .setDbKey("project:module1/file1")
+      .setPath("module1_path/file1")
+      .setEnabled(false));
+
+    Map<String, String> modulesRelativePaths = new HashMap<>();
+    modulesRelativePaths.put("project:module1", "module1_path");
+    ComponentUuidFactoryWithMigration underTest = new ComponentUuidFactoryWithMigration(db.getDbClient(), db.getSession(), project.getDbKey(), pathToKey, modulesRelativePaths);
+
+    // in theory we should migrate file1. But since disabledFileSameKey already have the expected migrated key, let's reuse it.
+    assertThat(underTest.getOrCreateForKey("project:module1/file1")).isEqualTo(disabledFileSameKey.uuid());
+
+    // project remains the same
+    assertThat(underTest.getOrCreateForKey(project.getDbKey())).isEqualTo(project.uuid());
+  }
+
   @Test
   public void migrate_branch_with_modules() {
     pathToKey = path -> path != null ? "project:" + path + ":BRANCH:branch1" : "project:BRANCH:branch1";
@@ -160,13 +178,6 @@ public class ComponentUuidFactoryWithMigrationTest {
 
     // project remains the same
     assertThat(underTest.getOrCreateForKey(project.getDbKey())).isEqualTo(project.uuid());
-
-    // old keys with modules don't exist
-    assertThat(underTest.getOrCreateForKey(module1.getDbKey())).isNotIn(project.uuid(), module1.uuid(), module2.uuid(), file1.uuid(), file2.uuid());
-    assertThat(underTest.getOrCreateForKey(module2.getDbKey())).isNotIn(project.uuid(), module1.uuid(), module2.uuid(), file1.uuid(), file2.uuid());
-    assertThat(underTest.getOrCreateForKey(file1.getDbKey())).isNotIn(project.uuid(), module1.uuid(), module2.uuid(), file1.uuid(), file2.uuid());
-    assertThat(underTest.getOrCreateForKey(file2.getDbKey())).isNotIn(project.uuid(), module1.uuid(), module2.uuid(), file1.uuid(), file2.uuid());
-
   }
 
   @Test
@@ -185,10 +196,6 @@ public class ComponentUuidFactoryWithMigrationTest {
 
     // module migrated to folder
     assertThat(underTest.getOrCreateForKey("project:module1_path")).isEqualTo(module1.uuid());
-
-    // doesnt exist
-    assertThat(underTest.getOrCreateForKey("project:module1_path/")).isNotIn(project.uuid(), module1.uuid(), dir1.uuid());
-    assertThat(underTest.getOrCreateForKey("project:module1")).isNotIn(project.uuid(), module1.uuid(), dir1.uuid());
   }
 
   @Test