From c79da6a5279287cb59a5ad7eb6977fb4735932da Mon Sep 17 00:00:00 2001 From: Julien HENRY Date: Thu, 13 Dec 2018 14:43:08 +0100 Subject: [PATCH] SONAR-11463 Prefers existing DB key to migrated ones --- .../ComponentUuidFactoryWithMigration.java | 26 +++++------ ...ComponentUuidFactoryWithMigrationTest.java | 43 +++++++++++-------- 2 files changed, 35 insertions(+), 34 deletions(-) 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 05a906f498a..f0e64a37ce4 100644 --- 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 uuidsByKey = new HashMap<>(); + private final Map uuidsByDbKey = new HashMap<>(); + private final Map uuidsByMigratedKey = new HashMap<>(); public ComponentUuidFactoryWithMigration(DbClient dbClient, DbSession dbSession, String rootKey, Function pathToKey, Map reportModulesPath) { Map modulePathsByUuid; + List 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 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 pathToKey, Map modulePathsByUuid) { List 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 b394b9c600a..aa53f0ee73f 100644 --- 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 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 -- 2.39.5