SONAR-6493 java.lang.NullPointerException in the PopulateProjectsUuidColumns migration

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 c1ce39f0b36740f5253ec8995f90e4e59009148e..09bc1d46205cf44593858b97def5c4f7344c1a49 100644 (file)
--- 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<Long, String> uuidByComponentId = newHashMap();
-          migrateEnabledComponents(readSession, writeSession, project, uuidByComponentId);
-          migrateDisabledComponents(readSession, writeSession, project, uuidByComponentId);
+          List<Component> 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<Long, String> uuidByComponentId) {
-    Map<Long, Component> componentsBySnapshotId = newHashMap();
-
-    List<Component> components = readSession.getMapper(Migration50Mapper.class).selectComponentChildrenForProjects(project.getId());
-    components.add(project);
-    List<Component> 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<Long, String> 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<Long, Component> componentsBySnapshotId, Map<Long, String> 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<Long, Component> componentsBySnapshotId = newHashMap();
+    private final Map<Long, String> uuidByComponentId = newHashMap();
+    private final List<Component> componentsToMigrate = newArrayList();
+
+    private MigrationContext(DbSession readSession, DbSession writeSession, Component project, List<Component> 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<Long, String> 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 20aec985042b530ab306b2ae4079b316c0ea3dde..a836a6ec4567978585986f11c0d7d56666f86a7b 100644 (file)
--- 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 (file)
index 0000000..809d310
--- /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 @@
+<dataset>
+
+  <!-- root project -->
+  <projects id="1" root_id="[null]" scope="PRJ" qualifier="TRK" kee="org.struts:struts" name="Struts"
+            uuid="[null]" project_uuid="[null]" module_uuid="[null]" module_uuid_path="[null]"
+            description="the description" long_name="Apache Struts"
+            enabled="[true]" language="[null]" copy_resource_id="[null]" person_id="[null]" path="[null]" deprecated_kee="[null]"
+            created_at="2014-06-18" />
+  <snapshots id="1" project_id="1" parent_snapshot_id="[null]" root_project_id="1" root_snapshot_id="[null]"
+             status="P" islast="[true]" purge_status="[null]"
+             period1_mode="[null]" period1_param="[null]" period1_date="[null]"
+             period2_mode="[null]" period2_param="[null]" period2_date="[null]"
+             period3_mode="[null]" period3_param="[null]" period3_date="[null]"
+             period4_mode="[null]" period4_param="[null]" period4_date="[null]"
+             period5_mode="[null]" period5_param="[null]" period5_date="[null]"
+             depth="[null]" scope="PRJ" qualifier="TRK" created_at="2008-12-02 13:58:00.00" build_date="2008-12-02 13:58:00.00"
+             version="[null]" path=""/>
+
+  <!-- module with null root id (probably a project that became a module) -->
+  <projects id="2" root_id="[null]" kee="org.struts:struts-core" name="Struts Core"
+            uuid="[null]" project_uuid="[null]" module_uuid="[null]" module_uuid_path="[null]"
+            scope="PRJ" qualifier="BRC" long_name="Struts Core" deprecated_kee="[null]"
+            description="[null]" enabled="[true]" language="[null]" copy_resource_id="[null]" person_id="[null]" created_at="2014-06-18" />
+  <snapshots id="2" project_id="2" parent_snapshot_id="1" root_project_id="1" root_snapshot_id="1"
+             status="P" islast="[true]" purge_status="[null]"
+             period1_mode="[null]" period1_param="[null]" period1_date="[null]"
+             period2_mode="[null]" period2_param="[null]" period2_date="[null]"
+             period3_mode="[null]" period3_param="[null]" period3_date="[null]"
+             period4_mode="[null]" period4_param="[null]" period4_date="[null]"
+             period5_mode="[null]" period5_param="[null]" period5_date="[null]"
+             depth="[null]" scope="PRJ" qualifier="BRC" created_at="2008-12-02 13:58:00.00" build_date="2008-12-02 13:58:00.00"
+             version="[null]" path="1."/>
+
+</dataset>

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 (file)
index 0000000..63db224
--- /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 @@
+<dataset>
+
+  <!-- root project with 2 snapshots having islast to true-->
+  <projects id="1" root_id="[null]" scope="PRJ" qualifier="TRK" kee="org.struts:struts" name="Struts"
+            uuid="[null]" project_uuid="[null]" module_uuid="[null]" module_uuid_path="[null]"
+            description="the description" long_name="Apache Struts"
+            enabled="[true]" language="[null]" copy_resource_id="[null]" person_id="[null]" path="[null]" deprecated_kee="[null]"
+            created_at="2014-06-18" />
+  <snapshots id="1" project_id="1" parent_snapshot_id="[null]" root_project_id="1" root_snapshot_id="[null]"
+             status="P" islast="[true]" purge_status="[null]"
+             period1_mode="[null]" period1_param="[null]" period1_date="[null]"
+             period2_mode="[null]" period2_param="[null]" period2_date="[null]"
+             period3_mode="[null]" period3_param="[null]" period3_date="[null]"
+             period4_mode="[null]" period4_param="[null]" period4_date="[null]"
+             period5_mode="[null]" period5_param="[null]" period5_date="[null]"
+             depth="[null]" scope="PRJ" qualifier="TRK" created_at="2008-12-02 13:58:00.00" build_date="2008-12-02 13:58:00.00"
+             version="[null]" path=""/>
+  <snapshots id="10" project_id="1" parent_snapshot_id="[null]" root_project_id="1" root_snapshot_id="[null]"
+             status="P" islast="[true]" purge_status="[null]"
+             period1_mode="[null]" period1_param="[null]" period1_date="[null]"
+             period2_mode="[null]" period2_param="[null]" period2_date="[null]"
+             period3_mode="[null]" period3_param="[null]" period3_date="[null]"
+             period4_mode="[null]" period4_param="[null]" period4_date="[null]"
+             period5_mode="[null]" period5_param="[null]" period5_date="[null]"
+             depth="[null]" scope="PRJ" qualifier="TRK" created_at="2008-12-02 13:58:00.00" build_date="2008-12-02 13:58:00.00"
+             version="[null]" path=""/>
+
+  <!-- module linked on second active snapshot of the project -->
+  <projects id="2" root_id="1" kee="org.struts:struts-core" name="Struts Core"
+            uuid="[null]" project_uuid="[null]" module_uuid="[null]" module_uuid_path="[null]"
+            scope="PRJ" qualifier="BRC" long_name="Struts Core" deprecated_kee="[null]"
+            description="[null]" enabled="[true]" language="[null]" copy_resource_id="[null]" person_id="[null]" created_at="2014-06-18" />
+  <snapshots id="2" project_id="2" parent_snapshot_id="10" root_project_id="1" root_snapshot_id="10"
+             status="P" islast="[true]" purge_status="[null]"
+             period1_mode="[null]" period1_param="[null]" period1_date="[null]"
+             period2_mode="[null]" period2_param="[null]" period2_date="[null]"
+             period3_mode="[null]" period3_param="[null]" period3_date="[null]"
+             period4_mode="[null]" period4_param="[null]" period4_date="[null]"
+             period5_mode="[null]" period5_param="[null]" period5_date="[null]"
+             depth="[null]" scope="PRJ" qualifier="BRC" created_at="2008-12-02 13:58:00.00" build_date="2008-12-02 13:58:00.00"
+             version="[null]" path="10."/>
+
+</dataset>

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 b16a9d1e65114d0d42af25060a28950e0af823fb..bf64a3cfc5763f746d34d132d25bf6aa8a8eefef 100644 (file)
--- 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;