fix memory leak in ruby database migration

9 files changed, 241 insertions, 70 deletions

diff --git a/sonar-server/src/main/java/org/sonar/server/platform/DefaultServerUpgradeStatus.java b/sonar-server/src/main/java/org/sonar/server/platform/DefaultServerUpgradeStatus.java
index 545f2c336f7..0230b53708e 100644
--- a/sonar-server/src/main/java/org/sonar/server/platform/DefaultServerUpgradeStatus.java
+++ b/sonar-server/src/main/java/org/sonar/server/platform/DefaultServerUpgradeStatus.java
@@ -42,10 +42,10 @@ public final class DefaultServerUpgradeStatus implements ServerUpgradeStatus {
   }
 
   public boolean isUpgraded() {
-    return !isInstalledFromScratch() &&(initialDbVersion < SchemaMigration.LAST_VERSION);
+    return !isFreshInstall() &&(initialDbVersion < SchemaMigration.LAST_VERSION);
   }
 
-  public boolean isInstalledFromScratch() {
+  public boolean isFreshInstall() {
     return initialDbVersion <= 0;
   }
 
diff --git a/sonar-server/src/main/java/org/sonar/server/platform/Platform.java b/sonar-server/src/main/java/org/sonar/server/platform/Platform.java
index 302ff2a2fc7..3850ae6d799 100644
--- a/sonar-server/src/main/java/org/sonar/server/platform/Platform.java
+++ b/sonar-server/src/main/java/org/sonar/server/platform/Platform.java
@@ -28,7 +28,6 @@ import org.sonar.api.Plugins;
 import org.sonar.api.database.configuration.DatabaseConfiguration;
 import org.sonar.api.platform.Environment;
 import org.sonar.api.platform.Server;
-import org.sonar.api.platform.ServerUpgradeStatus;
 import org.sonar.api.profiles.AnnotationProfileParser;
 import org.sonar.api.profiles.XMLProfileParser;
 import org.sonar.api.profiles.XMLProfileSerializer;
@@ -205,6 +204,7 @@ public final class Platform {
       startupContainer.as(Characteristics.CACHE).addComponent(JdbcDriverDeployer.class);
       startupContainer.as(Characteristics.CACHE).addComponent(ServerMetadataPersister.class);
       startupContainer.as(Characteristics.CACHE).addComponent(RegisterQualityModels.class);
+      startupContainer.as(Characteristics.CACHE).addComponent(DeleteDeprecatedMeasures.class);
       startupContainer.start();
 
       startupContainer.getComponent(ServerLifecycleNotifier.class).notifyStart();
diff --git a/sonar-server/src/main/java/org/sonar/server/startup/DeleteDeprecatedMeasures.java b/sonar-server/src/main/java/org/sonar/server/startup/DeleteDeprecatedMeasures.java
index 97c6a3f4912..42f23331663 100644
--- a/sonar-server/src/main/java/org/sonar/server/startup/DeleteDeprecatedMeasures.java
+++ b/sonar-server/src/main/java/org/sonar/server/startup/DeleteDeprecatedMeasures.java
@@ -19,10 +19,61 @@
  */
 package org.sonar.server.startup;
 
+import org.slf4j.LoggerFactory;
+import org.sonar.api.database.DatabaseSession;
+import org.sonar.api.database.model.MeasureModel;
+import org.sonar.api.platform.ServerUpgradeStatus;
+import org.sonar.api.utils.Logs;
 import org.sonar.jpa.session.DatabaseSessionFactory;
 
-public class DeleteDeprecatedMeasures {
-  public DeleteDeprecatedMeasures(DatabaseSessionFactory sessionFactory) {
+import javax.persistence.Query;
+import java.util.List;
 
+/**
+ * This purge script can not be executed with ActiveRecord. It fails with an out of memory error.
+ *
+ * @since 2.5 this component could be removed after 4 or 5 releases.
+ */
+public final class DeleteDeprecatedMeasures {
+
+  private ServerUpgradeStatus status;
+  private DatabaseSessionFactory sessionFactory;
+  private static final int MAX_IN_ELEMENTS = 500;
+
+  public DeleteDeprecatedMeasures(DatabaseSessionFactory sessionFactory, ServerUpgradeStatus status) {
+    this.sessionFactory = sessionFactory;
+    this.status = status;
+  }
+
+  public void start() {
+    if (mustDoPurge()) {
+      doPurge();
+    }
+  }
+
+  boolean mustDoPurge() {
+    return status.isUpgraded() && status.getInitialDbVersion()<=162;
+  }
+
+  void doPurge() {
+    Logs.INFO.info("Deleting measures with deprecated ISO category");
+    deleteRows("SELECT m.id FROM " + MeasureModel.class.getSimpleName() + " m WHERE m.ruleId IS NULL AND m.rulesCategoryId IS NOT NULL");
+
+    Logs.INFO.info("Deleting measures with deprecated priority");
+    deleteRows("SELECT m.id FROM " + MeasureModel.class.getSimpleName() + " m WHERE m.ruleId IS NULL AND m.rulePriority IS NOT NULL");
+  }
+
+  private void deleteRows(String hql) {
+    DatabaseSession session = sessionFactory.getSession();
+    List ids = session.getEntityManager().createQuery(hql).getResultList();
+    int index = 0;
+    while (index < ids.size()) {
+      List paginedSids = ids.subList(index, Math.min(ids.size(), index + MAX_IN_ELEMENTS));
+      Query query = session.createQuery("DELETE FROM " + MeasureModel.class.getSimpleName() + " WHERE id IN (:ids)");
+      query.setParameter("ids", paginedSids);
+      query.executeUpdate();
+      index += MAX_IN_ELEMENTS;
+      session.commit();
+    }
   }
 }
diff --git a/sonar-server/src/main/webapp/WEB-INF/db/migrate/161_add_snapshots_period_columns.rb b/sonar-server/src/main/webapp/WEB-INF/db/migrate/161_add_snapshots_period_columns.rb
index a0d0945d22b..34783863e6c 100644
--- a/sonar-server/src/main/webapp/WEB-INF/db/migrate/161_add_snapshots_period_columns.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/db/migrate/161_add_snapshots_period_columns.rb
@@ -24,25 +24,33 @@
 class AddSnapshotsPeriodColumns < ActiveRecord::Migration
 
   def self.up
-     add_column :snapshots, :period1_mode, :string, :null => true, :limit => 100
-     add_column :snapshots, :period1_param, :string, :null => true, :limit => 100
-     add_column :snapshots, :period1_date, :datetime, :null => true
+     Snapshot.reset_column_information()
+     
+     add_period_column('period1_mode', :string, :null => true, :limit => 100)
+     add_period_column('period1_param', :string, :null => true, :limit => 100)
+     add_period_column('period1_date', :datetime, :null => true)
 
-     add_column :snapshots, :period2_mode, :string, :null => true, :limit => 100
-     add_column :snapshots, :period2_param, :string, :null => true, :limit => 100
-     add_column :snapshots, :period2_date, :datetime, :null => true
+     add_period_column('period2_mode', :string, :null => true, :limit => 100)
+     add_period_column('period2_param', :string, :null => true, :limit => 100)
+     add_period_column('period2_date', :datetime, :null => true)
 
-     add_column :snapshots, :period3_mode, :string, :null => true, :limit => 100
-     add_column :snapshots, :period3_param, :string, :null => true, :limit => 100
-     add_column :snapshots, :period3_date, :datetime, :null => true
+     add_period_column('period3_mode', :string, :null => true, :limit => 100)
+     add_period_column('period3_param', :string, :null => true, :limit => 100)
+     add_period_column('period3_date', :datetime, :null => true)
 
-     add_column :snapshots, :period4_mode, :string, :null => true, :limit => 100
-     add_column :snapshots, :period4_param, :string, :null => true, :limit => 100
-     add_column :snapshots, :period4_date, :datetime, :null => true
+     add_period_column('period4_mode', :string, :null => true, :limit => 100)
+     add_period_column('period4_param', :string, :null => true, :limit => 100)
+     add_period_column('period4_date', :datetime, :null => true)
 
-     add_column :snapshots, :period5_mode, :string, :null => true, :limit => 100
-     add_column :snapshots, :period5_param, :string, :null => true, :limit => 100
-     add_column :snapshots, :period5_date, :datetime, :null => true
+     add_period_column('period5_mode', :string, :null => true, :limit => 100)
+     add_period_column('period5_param', :string, :null => true, :limit => 100)
+     add_period_column('period5_date', :datetime, :null => true)
   end
 
+  private
+  def self.add_period_column(name, type, options={})
+    unless Snapshot.column_names.include?(name)
+      add_column(:snapshots, name, type, options)
+    end
+  end
 end
diff --git a/sonar-server/src/main/webapp/WEB-INF/db/migrate/162_delete_iso_rule_categories.rb b/sonar-server/src/main/webapp/WEB-INF/db/migrate/162_delete_iso_rule_categories.rb
index 2b426cb6520..f9cebf73c1e 100644
--- a/sonar-server/src/main/webapp/WEB-INF/db/migrate/162_delete_iso_rule_categories.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/db/migrate/162_delete_iso_rule_categories.rb
@@ -24,13 +24,6 @@
 class DeleteIsoRuleCategories < ActiveRecord::Migration
 
   def self.up
-    remove_rule_categories
-    delete_measures_on_iso_category
-  end
-
-  private
-
-  def self.remove_rule_categories
     begin
       remove_column('rules', 'rules_category_id')
     rescue
@@ -38,10 +31,4 @@ class DeleteIsoRuleCategories < ActiveRecord::Migration
     end
   end
 
-  def self.delete_measures_on_iso_category
-    ids=ProjectMeasure.connection.select_values("SELECT ID FROM PROJECT_MEASURES WHERE RULE_ID IS NULL AND RULES_CATEGORY_ID IS NOT NULL")
-    ids.in_groups_of(900, false) do |group|
-      ProjectMeasure.delete(group.map{|id| id.to_i}) unless group.empty?
-    end
-  end
 end
diff --git a/sonar-server/src/main/webapp/WEB-INF/db/migrate/164_delete_measures_on_violations_and_priority.rb b/sonar-server/src/main/webapp/WEB-INF/db/migrate/164_delete_measures_on_violations_and_priority.rb
deleted file mode 100644
index 56f2febf0e5..00000000000
--- a/sonar-server/src/main/webapp/WEB-INF/db/migrate/164_delete_measures_on_violations_and_priority.rb
+++ /dev/null
@@ -1,33 +0,0 @@
-#
-# Sonar, entreprise quality control tool.
-# Copyright (C) 2009 SonarSource SA
-# mailto:contact AT sonarsource DOT com
-#
-# Sonar is free software; you can redistribute it and/or
-# modify it under the terms of the GNU Lesser General Public
-# License as published by the Free Software Foundation; either
-# version 3 of the License, or (at your option) any later version.
-#
-# Sonar is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-# Lesser General Public License for more details.
-#
-# You should have received a copy of the GNU Lesser General Public
-# License along with Sonar; if not, write to the Free Software
-# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
-#
-
-#
-# Sonar 2.5
-#
-class DeleteMeasuresOnViolationsAndPriority < ActiveRecord::Migration
-
-  def self.up
-    ids=ProjectMeasure.connection.select_values("SELECT ID FROM PROJECT_MEASURES WHERE RULE_ID IS NULL AND RULE_PRIORITY IS NOT NULL")
-    ids.in_groups_of(900, false) do |group|
-      ProjectMeasure.delete(group.map{|id| id.to_i}) unless group.empty?
-    end
-  end
-
-end
diff --git a/sonar-server/src/test/java/org/sonar/server/platform/DefaultServerUpgradeStatusTest.java b/sonar-server/src/test/java/org/sonar/server/platform/DefaultServerUpgradeStatusTest.java
index be801d2ac83..ecfe58793c4 100644
--- a/sonar-server/src/test/java/org/sonar/server/platform/DefaultServerUpgradeStatusTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/platform/DefaultServerUpgradeStatusTest.java
@@ -31,14 +31,14 @@ import static org.mockito.Mockito.when;
 public class DefaultServerUpgradeStatusTest {
 
   @Test
-  public void shouldBeInstalledFromScratch() {
+  public void shouldBeFreshInstallation() {
     DatabaseConnector connector = mock(DatabaseConnector.class);
     when(connector.getDatabaseVersion()).thenReturn(-1);
 
     DefaultServerUpgradeStatus status = new DefaultServerUpgradeStatus(connector);
     status.start();
 
-    assertThat(status.isInstalledFromScratch(), is(true));
+    assertThat(status.isFreshInstall(), is(true));
     assertThat(status.isUpgraded(), is(false));
     assertThat(status.getInitialDbVersion(), is(-1));
   }
@@ -51,7 +51,7 @@ public class DefaultServerUpgradeStatusTest {
     DefaultServerUpgradeStatus status = new DefaultServerUpgradeStatus(connector);
     status.start();
 
-    assertThat(status.isInstalledFromScratch(), is(false));
+    assertThat(status.isFreshInstall(), is(false));
     assertThat(status.isUpgraded(), is(true));
     assertThat(status.getInitialDbVersion(), is(50));
   }
@@ -64,7 +64,7 @@ public class DefaultServerUpgradeStatusTest {
     DefaultServerUpgradeStatus status = new DefaultServerUpgradeStatus(connector);
     status.start();
     
-    assertThat(status.isInstalledFromScratch(), is(false));
+    assertThat(status.isFreshInstall(), is(false));
     assertThat(status.isUpgraded(), is(false));
     assertThat(status.getInitialDbVersion(), is(SchemaMigration.LAST_VERSION));
   }
diff --git a/sonar-server/src/test/java/org/sonar/server/startup/DeleteDeprecatedMeasuresTest.java b/sonar-server/src/test/java/org/sonar/server/startup/DeleteDeprecatedMeasuresTest.java
new file mode 100644
index 00000000000..438058322de
--- /dev/null
+++ b/sonar-server/src/test/java/org/sonar/server/startup/DeleteDeprecatedMeasuresTest.java
@@ -0,0 +1,106 @@
+/*
+ * Sonar, open source software quality management tool.
+ * Copyright (C) 2009 SonarSource SA
+ * mailto:contact AT sonarsource DOT com
+ *
+ * Sonar is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * Sonar is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with Sonar; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
+ */
+package org.sonar.server.startup;
+
+import org.junit.Test;
+import org.sonar.api.database.model.MeasureModel;
+import org.sonar.api.platform.ServerUpgradeStatus;
+import org.sonar.jpa.entity.SchemaMigration;
+import org.sonar.jpa.test.AbstractDbUnitTestCase;
+
+import java.sql.SQLException;
+import java.util.List;
+
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertThat;
+import static org.junit.internal.matchers.IsCollectionContaining.hasItems;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class DeleteDeprecatedMeasuresTest extends AbstractDbUnitTestCase {
+
+  @Test
+  public void shouldDeleteMeasuresWithCategory() throws SQLException {
+    setupData("sharedFixture");
+
+    ServerUpgradeStatus upgradeStatus = mock(ServerUpgradeStatus.class);
+
+    final DeleteDeprecatedMeasures purge = new DeleteDeprecatedMeasures(getSessionFactory(), upgradeStatus);
+    purge.doPurge();
+
+    List rows = getSession().createQuery("from " + MeasureModel.class.getSimpleName() + " where ruleId is null and rulesCategoryId is not null").getResultList();
+    assertThat(rows.size(), is(0));
+  }
+
+  @Test
+  public void shouldDeleteMeasuresWithPriority() throws SQLException {
+    setupData("sharedFixture");
+
+    ServerUpgradeStatus upgradeStatus = mock(ServerUpgradeStatus.class);
+    final DeleteDeprecatedMeasures purge = new DeleteDeprecatedMeasures(getSessionFactory(), upgradeStatus);
+    purge.doPurge();
+
+    List rowsToDelete = getSession().createQuery("from " + MeasureModel.class.getSimpleName() + " where ruleId is null and rulePriority is not null").getResultList();
+    assertThat(rowsToDelete.size(), is(0));
+
+    List<Long> rowIdsToKeep = getSession().createQuery("select id from " + MeasureModel.class.getSimpleName()).getResultList();
+    assertThat(rowIdsToKeep, hasItems(1L, 2L, 4L, 6L));
+  }
+
+  @Test
+  public void shouldDoPurgeOnUpgradeBefore162() {
+    ServerUpgradeStatus upgradeStatus = mock(ServerUpgradeStatus.class);
+    when(upgradeStatus.isUpgraded()).thenReturn(true);
+    when(upgradeStatus.getInitialDbVersion()).thenReturn(50);
+
+    final DeleteDeprecatedMeasures purge = new DeleteDeprecatedMeasures(getSessionFactory(), upgradeStatus);
+    assertThat(purge.mustDoPurge(), is(true));
+  }
+
+  @Test
+  public void shouldNotDoPurgeOnUpgradeAfter162() {
+    ServerUpgradeStatus upgradeStatus = mock(ServerUpgradeStatus.class);
+    when(upgradeStatus.isUpgraded()).thenReturn(true);
+    when(upgradeStatus.getInitialDbVersion()).thenReturn(170);
+
+    final DeleteDeprecatedMeasures purge = new DeleteDeprecatedMeasures(getSessionFactory(), upgradeStatus);
+    assertThat(purge.mustDoPurge(), is(false));
+  }
+
+  @Test
+  public void shouldNotDoPurgeOnFreshInstall() {
+    ServerUpgradeStatus upgradeStatus = mock(ServerUpgradeStatus.class);
+    when(upgradeStatus.isUpgraded()).thenReturn(false);
+    when(upgradeStatus.getInitialDbVersion()).thenReturn(-1);
+
+    final DeleteDeprecatedMeasures purge = new DeleteDeprecatedMeasures(getSessionFactory(), upgradeStatus);
+    assertThat(purge.mustDoPurge(), is(false));
+  }
+
+  @Test
+  public void shouldNotDoPurgeOnStandardStartup() {
+    ServerUpgradeStatus upgradeStatus = mock(ServerUpgradeStatus.class);
+    when(upgradeStatus.isUpgraded()).thenReturn(false);
+    when(upgradeStatus.getInitialDbVersion()).thenReturn(SchemaMigration.LAST_VERSION);
+
+    final DeleteDeprecatedMeasures purge = new DeleteDeprecatedMeasures(getSessionFactory(), upgradeStatus);
+    assertThat(purge.mustDoPurge(), is(false));
+  }
+}
diff --git a/sonar-server/src/test/resources/org/sonar/server/startup/DeleteDeprecatedMeasuresTest/sharedFixture.xml b/sonar-server/src/test/resources/org/sonar/server/startup/DeleteDeprecatedMeasuresTest/sharedFixture.xml
new file mode 100644
index 00000000000..870e3a48412
--- /dev/null
+++ b/sonar-server/src/test/resources/org/sonar/server/startup/DeleteDeprecatedMeasuresTest/sharedFixture.xml
@@ -0,0 +1,52 @@
+<dataset>
+
+
+  <!-- classic measures -->
+  <project_measures characteristic_id="[null]" url="[null]" variation_value_1="[null]" variation_value_2="[null]" variation_value_3="[null]" variation_value_4="[null]" variation_value_5="[null]"
+                    rule_priority="[null]"
+                    alert_text="[null]" ID="1" VALUE="10.0" METRIC_ID="1" SNAPSHOT_ID="1" rules_category_id="[null]"
+                    RULE_ID="1"
+                    text_value="[null]" tendency="[null]" measure_date="[null]" project_id="[null]"
+                    alert_status="[null]" description="[null]"/>
+
+  <project_measures characteristic_id="[null]" url="[null]" variation_value_1="[null]" variation_value_2="[null]" variation_value_3="[null]" variation_value_4="[null]" variation_value_5="[null]"
+                    rule_priority="[null]"
+                    alert_text="[null]" ID="2" VALUE="10.0" METRIC_ID="1" SNAPSHOT_ID="2" rules_category_id="[null]"
+                    RULE_ID="1"
+                    text_value="[null]" tendency="[null]" measure_date="[null]" project_id="[null]"
+                    alert_status="[null]" description="[null]"/>
+
+  <!-- measure with ISO category and no rule -->
+  <project_measures characteristic_id="[null]" url="[null]" variation_value_1="[null]" variation_value_2="[null]" variation_value_3="[null]" variation_value_4="[null]" variation_value_5="[null]"
+                    rule_priority="[null]"
+                    alert_text="[null]" ID="3" VALUE="10.0" METRIC_ID="1" SNAPSHOT_ID="3" rules_category_id="2"
+                    RULE_ID="[null]"
+                    text_value="[null]" tendency="[null]" measure_date="[null]" project_id="[null]"
+                    alert_status="[null]" description="[null]"/>
+
+  <!-- measures with ISO category and rule -->
+  <project_measures characteristic_id="[null]" url="[null]" variation_value_1="[null]" variation_value_2="[null]" variation_value_3="[null]" variation_value_4="[null]" variation_value_5="[null]"
+                    rule_priority="1"
+                    alert_text="[null]" ID="4" VALUE="10.0" METRIC_ID="1" SNAPSHOT_ID="4" rules_category_id="6"
+                    RULE_ID="1"
+                    text_value="[null]" tendency="[null]" measure_date="[null]" project_id="[null]"
+                    alert_status="[null]" description="[null]"/>
+
+
+  <!-- measures with priority and no rule -->
+  <project_measures characteristic_id="[null]" url="[null]" variation_value_1="[null]" variation_value_2="[null]" variation_value_3="[null]" variation_value_4="[null]" variation_value_5="[null]"
+                    rule_priority="2"
+                    alert_text="[null]" ID="5" VALUE="10.0" METRIC_ID="1" SNAPSHOT_ID="3" rules_category_id="[null]"
+                    RULE_ID="[null]"
+                    text_value="[null]" tendency="[null]" measure_date="[null]" project_id="[null]"
+                    alert_status="[null]" description="[null]"/>
+
+  <!-- measures with priority and rule -->
+  <project_measures characteristic_id="[null]" url="[null]" variation_value_1="[null]" variation_value_2="[null]" variation_value_3="[null]" variation_value_4="[null]" variation_value_5="[null]"
+                    rule_priority="1"
+                    alert_text="[null]" ID="6" VALUE="10.0" METRIC_ID="1" SNAPSHOT_ID="4" rules_category_id="[null]"
+                    RULE_ID="1"
+                    text_value="[null]" tendency="[null]" measure_date="[null]" project_id="[null]"
+                    alert_status="[null]" description="[null]"/>
+
+</dataset>
\ No newline at end of file