From 96651c2341184362a22b89fe7a8e24b2a23b4695 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Mon, 27 Apr 2015 17:45:47 +0200 Subject: [PATCH] SONAR-6493 java.lang.NullPointerException in the PopulateProjectsUuidColumns migration --- .../PopulateProjectsUuidColumnsMigration.java | 166 ++++++++++-------- ...ulateProjectsUuidColumnsMigrationTest.java | 44 +++++ .../not_fail_when_module_has_no_root_id.xml | 34 ++++ ..._when_project_has_two_active_snapshots.xml | 43 +++++ .../persistence/migration/v50/Component.java | 2 +- 5 files changed, 212 insertions(+), 77 deletions(-) create mode 100644 server/sonar-server/src/test/resources/org/sonar/server/db/migrations/v50/PopulateProjectsUuidColumnsMigrationTest/not_fail_when_module_has_no_root_id.xml create mode 100644 server/sonar-server/src/test/resources/org/sonar/server/db/migrations/v50/PopulateProjectsUuidColumnsMigrationTest/not_fail_when_project_has_two_active_snapshots.xml diff --git a/server/sonar-server/src/main/java/org/sonar/server/db/migrations/v50/PopulateProjectsUuidColumnsMigration.java b/server/sonar-server/src/main/java/org/sonar/server/db/migrations/v50/PopulateProjectsUuidColumnsMigration.java index c1ce39f0b36..09bc1d46205 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/db/migrations/v50/PopulateProjectsUuidColumnsMigration.java +++ b/server/sonar-server/src/main/java/org/sonar/server/db/migrations/v50/PopulateProjectsUuidColumnsMigration.java @@ -26,6 +26,8 @@ import org.apache.ibatis.session.ResultContext; import org.apache.ibatis.session.ResultHandler; import org.sonar.api.resources.Scopes; import org.sonar.api.utils.internal.Uuids; +import org.sonar.api.utils.log.Logger; +import org.sonar.api.utils.log.Loggers; import org.sonar.core.persistence.DbSession; import org.sonar.core.persistence.migration.v50.Component; import org.sonar.core.persistence.migration.v50.Migration50Mapper; @@ -47,6 +49,8 @@ import static com.google.common.collect.Maps.newHashMap; */ public class PopulateProjectsUuidColumnsMigration implements DatabaseMigration { + private static final Logger LOG = Loggers.get(PopulateProjectsUuidColumnsMigration.class); + private final DbClient db; private final AtomicLong counter = new AtomicLong(0L); @@ -66,9 +70,10 @@ public class PopulateProjectsUuidColumnsMigration implements DatabaseMigration { @Override public void handleResult(ResultContext context) { Component project = (Component) context.getResultObject(); - Map uuidByComponentId = newHashMap(); - migrateEnabledComponents(readSession, writeSession, project, uuidByComponentId); - migrateDisabledComponents(readSession, writeSession, project, uuidByComponentId); + List components = readSession.getMapper(Migration50Mapper.class).selectComponentChildrenForProjects(project.getId()); + MigrationContext migrationContext = new MigrationContext(readSession, writeSession, project, components); + migrateEnabledComponents(migrationContext); + migrateDisabledComponents(migrationContext); } }); writeSession.commit(true); @@ -86,98 +91,107 @@ public class PopulateProjectsUuidColumnsMigration implements DatabaseMigration { } } - private void migrateEnabledComponents(DbSession readSession, DbSession writeSession, Component project, Map uuidByComponentId) { - Map componentsBySnapshotId = newHashMap(); - - List components = readSession.getMapper(Migration50Mapper.class).selectComponentChildrenForProjects(project.getId()); - components.add(project); - List componentsToMigrate = newArrayList(); - for (Component component : components) { - componentsBySnapshotId.put(component.getSnapshotId(), component); - - // Not migrate components already having an UUID - if (component.getUuid() == null) { - component.setUuid(getOrCreateUuid(component, uuidByComponentId)); - component.setProjectUuid(getOrCreateUuid(project, uuidByComponentId)); - component.setModuleUuidPath(""); - componentsToMigrate.add(component); + private void migrateEnabledComponents(MigrationContext migrationContext) { + saveComponent(migrationContext.writeSession, migrationContext.project); + for (Component component : migrationContext.componentsToMigrate) { + migrationContext.updateComponent(component); + if (Strings.isNullOrEmpty(component.getModuleUuidPath())) { + LOG.warn(String.format("Ignoring component id '%s' because the module uuid path could not be created", component.getId())); + } else { + migrationContext.updateComponent(component); + saveComponent(migrationContext.writeSession, component); } } + } - for (Component component : componentsToMigrate) { - updateComponent(component, project, componentsBySnapshotId, uuidByComponentId); - writeSession.getMapper(Migration50Mapper.class).updateComponentUuids(component); - counter.getAndIncrement(); + private void migrateDisabledComponents(MigrationContext migrationContext) { + for (Component component : migrationContext.readSession.getMapper(Migration50Mapper.class).selectDisabledDirectComponentChildrenForProjects(migrationContext.project.getId())) { + migrationContext.updateComponent(component); + saveComponent(migrationContext.writeSession, component); + } + for (Component component : migrationContext.readSession.getMapper(Migration50Mapper.class).selectDisabledNoneDirectComponentChildrenForProjects( + migrationContext.project.getId())) { + migrationContext.updateComponent(component); + saveComponent(migrationContext.writeSession, component); } } - private void migrateDisabledComponents(DbSession readSession, DbSession writeSession, Component project, Map uuidByComponentId) { - String projectUuid = getOrCreateUuid(project, uuidByComponentId); - for (Component component : readSession.getMapper(Migration50Mapper.class).selectDisabledDirectComponentChildrenForProjects(project.getId())) { - component.setUuid(getOrCreateUuid(component, uuidByComponentId)); - component.setProjectUuid(projectUuid); - component.setModuleUuidPath(""); - - writeSession.getMapper(Migration50Mapper.class).updateComponentUuids(component); - counter.getAndIncrement(); + private void migrateComponentsWithoutUuid(DbSession readSession, DbSession writeSession) { + for (Component component : readSession.getMapper(Migration50Mapper.class).selectComponentsWithoutUuid()) { + String uuid = Uuids.create(); + component.setUuid(uuid); + component.setProjectUuid(uuid); + saveComponent(writeSession, component); } - for (Component component : readSession.getMapper(Migration50Mapper.class).selectDisabledNoneDirectComponentChildrenForProjects(project.getId())) { - component.setUuid(getOrCreateUuid(component, uuidByComponentId)); - component.setProjectUuid(projectUuid); - component.setModuleUuidPath(""); + } - writeSession.getMapper(Migration50Mapper.class).updateComponentUuids(component); - counter.getAndIncrement(); - } + private void saveComponent(DbSession writeSession, Component component) { + writeSession.getMapper(Migration50Mapper.class).updateComponentUuids(component); + counter.getAndIncrement(); } - private void updateComponent(Component component, Component project, Map componentsBySnapshotId, Map uuidByComponentId) { - String snapshotPath = component.getSnapshotPath(); - StringBuilder moduleUuidPath = new StringBuilder(); - Component lastModule = null; - if (!Strings.isNullOrEmpty(snapshotPath)) { - for (String s : Splitter.on(".").omitEmptyStrings().split(snapshotPath)) { - Long snapshotId = Long.valueOf(s); - Component currentComponent = componentsBySnapshotId.get(snapshotId); - if (currentComponent.getScope().equals(Scopes.PROJECT)) { - lastModule = currentComponent; - moduleUuidPath.append(currentComponent.getUuid()).append("."); + private static class MigrationContext { + private final DbSession readSession; + private final DbSession writeSession; + private final Component project; + private final Map componentsBySnapshotId = newHashMap(); + private final Map uuidByComponentId = newHashMap(); + private final List componentsToMigrate = newArrayList(); + + private MigrationContext(DbSession readSession, DbSession writeSession, Component project, List components) { + this.readSession = readSession; + this.writeSession = writeSession; + this.project = project; + + project.setUuid(getOrCreateUuid(project)); + project.setProjectUuid(project.getUuid()); + + componentsBySnapshotId.put(project.getSnapshotId(), project); + for (Component component : components) { + componentsBySnapshotId.put(component.getSnapshotId(), component); + if (component.getUuid() == null) { + componentsToMigrate.add(component); } } } - if (moduleUuidPath.length() > 0) { - // Remove last '.' - moduleUuidPath.deleteCharAt(moduleUuidPath.length() - 1); - component.setModuleUuidPath(moduleUuidPath.toString()); - } - // Module UUID contains direct module of a component - if (lastModule != null) { - component.setModuleUuid(getOrCreateUuid(lastModule, uuidByComponentId)); - } - } + public void updateComponent(Component component) { + component.setUuid(getOrCreateUuid(component)); + component.setProjectUuid(getOrCreateUuid(project)); + + String snapshotPath = component.getSnapshotPath(); + StringBuilder moduleUuidPath = new StringBuilder(); + String lastModuleUuid = null; + if (!Strings.isNullOrEmpty(snapshotPath)) { + for (String s : Splitter.on(".").omitEmptyStrings().split(snapshotPath)) { + Long snapshotId = Long.valueOf(s); + Component currentComponent = componentsBySnapshotId.get(snapshotId); + if (currentComponent != null && currentComponent.getScope().equals(Scopes.PROJECT)) { + lastModuleUuid = getOrCreateUuid(currentComponent); + moduleUuidPath.append(lastModuleUuid).append("."); + } + } + } - private void migrateComponentsWithoutUuid(DbSession readSession, DbSession writeSession) { - for (Component component : readSession.getMapper(Migration50Mapper.class).selectComponentsWithoutUuid()) { - String uuid = Uuids.create(); - component.setUuid(uuid); - component.setProjectUuid(uuid); - component.setModuleUuidPath(""); + if (moduleUuidPath.length() > 0 && lastModuleUuid != null) { + // Remove last '.' + moduleUuidPath.deleteCharAt(moduleUuidPath.length() - 1); - writeSession.getMapper(Migration50Mapper.class).updateComponentUuids(component); - counter.getAndIncrement(); + component.setModuleUuidPath(moduleUuidPath.toString()); + component.setModuleUuid(lastModuleUuid); + } } - } - private static String getOrCreateUuid(Component component, Map uuidByComponentId) { - String existingUuid = component.getUuid(); - String uuid = existingUuid == null ? uuidByComponentId.get(component.getId()) : existingUuid; - if (uuid == null) { - String newUuid = Uuids.create(); - uuidByComponentId.put(component.getId(), newUuid); - return newUuid; + private String getOrCreateUuid(Component component) { + String existingUuid = component.getUuid(); + String uuid = existingUuid == null ? uuidByComponentId.get(component.getId()) : existingUuid; + if (uuid == null) { + String newUuid = Uuids.create(); + uuidByComponentId.put(component.getId(), newUuid); + return newUuid; + } + return uuid; } - return uuid; } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/db/migrations/v50/PopulateProjectsUuidColumnsMigrationTest.java b/server/sonar-server/src/test/java/org/sonar/server/db/migrations/v50/PopulateProjectsUuidColumnsMigrationTest.java index 20aec985042..a836a6ec456 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/db/migrations/v50/PopulateProjectsUuidColumnsMigrationTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/db/migrations/v50/PopulateProjectsUuidColumnsMigrationTest.java @@ -289,4 +289,48 @@ public class PopulateProjectsUuidColumnsMigrationTest { assertThat(file.getModuleUuidPath()).isEmpty(); } + @Test + public void not_fail_when_module_has_no_root_id() throws Exception { + db.prepareDbUnit(getClass(), "not_fail_when_module_has_no_root_id.xml"); + + migration.execute(); + session.commit(); + + // Root project migrated + Component root = mapper.selectComponentByKey("org.struts:struts"); + assertThat(root.getUuid()).isNotNull(); + assertThat(root.getProjectUuid()).isEqualTo(root.getUuid()); + assertThat(root.getModuleUuid()).isNull(); + assertThat(root.getModuleUuidPath()).isEmpty(); + + // The module without uuid will be migrated as a standalone component + Component module = mapper.selectComponentByKey("org.struts:struts-core"); + assertThat(module.getUuid()).isNotNull(); + assertThat(module.getProjectUuid()).isEqualTo(module.getUuid()); + assertThat(module.getModuleUuid()).isNull(); + assertThat(module.getModuleUuidPath()).isEmpty(); + } + + @Test + public void not_fail_when_project_has_two_active_snapshots() throws Exception { + db.prepareDbUnit(getClass(), "not_fail_when_project_has_two_active_snapshots.xml"); + + migration.execute(); + session.commit(); + + // Root project migrated + Component root = mapper.selectComponentByKey("org.struts:struts"); + assertThat(root.getUuid()).isNotNull(); + assertThat(root.getProjectUuid()).isEqualTo(root.getUuid()); + assertThat(root.getModuleUuid()).isNull(); + assertThat(root.getModuleUuidPath()).isEmpty(); + + // The module linked on second active snapshot should be migrated a standalone component + Component module = mapper.selectComponentByKey("org.struts:struts-core"); + assertThat(module.getUuid()).isNotNull(); + assertThat(module.getProjectUuid()).isEqualTo(module.getUuid()); + assertThat(module.getModuleUuid()).isNull(); + assertThat(module.getModuleUuidPath()).isEmpty(); + } + } diff --git a/server/sonar-server/src/test/resources/org/sonar/server/db/migrations/v50/PopulateProjectsUuidColumnsMigrationTest/not_fail_when_module_has_no_root_id.xml b/server/sonar-server/src/test/resources/org/sonar/server/db/migrations/v50/PopulateProjectsUuidColumnsMigrationTest/not_fail_when_module_has_no_root_id.xml new file mode 100644 index 00000000000..809d310d05f --- /dev/null +++ b/server/sonar-server/src/test/resources/org/sonar/server/db/migrations/v50/PopulateProjectsUuidColumnsMigrationTest/not_fail_when_module_has_no_root_id.xml @@ -0,0 +1,34 @@ + + + + + + + + + + + diff --git a/server/sonar-server/src/test/resources/org/sonar/server/db/migrations/v50/PopulateProjectsUuidColumnsMigrationTest/not_fail_when_project_has_two_active_snapshots.xml b/server/sonar-server/src/test/resources/org/sonar/server/db/migrations/v50/PopulateProjectsUuidColumnsMigrationTest/not_fail_when_project_has_two_active_snapshots.xml new file mode 100644 index 00000000000..63db22438fe --- /dev/null +++ b/server/sonar-server/src/test/resources/org/sonar/server/db/migrations/v50/PopulateProjectsUuidColumnsMigrationTest/not_fail_when_project_has_two_active_snapshots.xml @@ -0,0 +1,43 @@ + + + + + + + + + + + + diff --git a/sonar-core/src/main/java/org/sonar/core/persistence/migration/v50/Component.java b/sonar-core/src/main/java/org/sonar/core/persistence/migration/v50/Component.java index b16a9d1e651..bf64a3cfc57 100644 --- a/sonar-core/src/main/java/org/sonar/core/persistence/migration/v50/Component.java +++ b/sonar-core/src/main/java/org/sonar/core/persistence/migration/v50/Component.java @@ -34,7 +34,7 @@ public class Component { private String uuid; private String projectUuid; private String moduleUuid; - private String moduleUuidPath; + private String moduleUuidPath = ""; public Long getId() { return id; -- 2.39.5