]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-2757 improve reentrance of project deletion
authorsimonbrandhof <simon.brandhof@gmail.com>
Thu, 26 Jan 2012 22:13:09 +0000 (23:13 +0100)
committersimonbrandhof <simon.brandhof@gmail.com>
Thu, 26 Jan 2012 22:13:43 +0000 (23:13 +0100)
sonar-core/src/main/java/org/sonar/core/persistence/BatchSession.java
sonar-core/src/main/java/org/sonar/core/purge/PurgeDao.java
sonar-core/src/main/java/org/sonar/core/purge/PurgeMapper.java
sonar-core/src/main/resources/org/sonar/core/purge/PurgeMapper.xml
sonar-core/src/test/java/org/sonar/core/persistence/DaoTestCase.java
sonar-core/src/test/java/org/sonar/core/purge/PurgeDaoTest.java
sonar-core/src/test/resources/org/sonar/core/purge/PurgeDaoTest/shouldDeleteProject.xml [new file with mode: 0644]
sonar-core/src/test/resources/org/sonar/core/purge/PurgeDaoTest/shouldDeleteResource.xml [new file with mode: 0644]
sonar-server/src/main/java/org/sonar/server/ui/JRubyFacade.java
sonar-server/src/main/webapp/WEB-INF/app/models/project.rb

index 2a416fd11b3432afc8cb304f8720296030435ac6..4413fff3aee21e1c46d439b04895aa17160cf06e 100644 (file)
@@ -46,21 +46,6 @@ public final class BatchSession implements SqlSession {
     this.batchSize = batchSize;
   }
 
-  /**
-   * This method must be called when executing SQL requests.
-   */
-  public BatchSession increment(int nbSqlRequests) {
-    count += nbSqlRequests;
-    if (count >= batchSize) {
-      commit();
-    }
-    return this;
-  }
-
-  private void reset() {
-    count=0;
-  }
-
   public void select(String statement, Object parameter, ResultHandler handler) {
     reset();
     session.select(statement, parameter, handler);
@@ -117,32 +102,32 @@ public final class BatchSession implements SqlSession {
   }
 
   public int insert(String statement) {
-    increment(1);
+    increment();
     return session.insert(statement);
   }
 
   public int insert(String statement, Object parameter) {
-    increment(1);
+    increment();
     return session.insert(statement, parameter);
   }
 
   public int update(String statement) {
-    increment(1);
+    increment();
     return session.update(statement);
   }
 
   public int update(String statement, Object parameter) {
-    increment(1);
+    increment();
     return session.update(statement, parameter);
   }
 
   public int delete(String statement) {
-    increment(1);
+    increment();
     return session.delete(statement);
   }
 
   public int delete(String statement, Object parameter) {
-    increment(1);
+    increment();
     return session.delete(statement, parameter);
   }
 
@@ -192,4 +177,15 @@ public final class BatchSession implements SqlSession {
     return session.getConnection();
   }
 
+  private BatchSession increment() {
+    count += 1;
+    if (count >= batchSize) {
+      commit();
+    }
+    return this;
+  }
+
+  private void reset() {
+    count = 0;
+  }
 }
index 726f59dcb525d743d69d510c63c230aaa51b8e0d..e751cc32f2018bb8ed03166c6731df2c23afa4c1 100644 (file)
@@ -24,7 +24,6 @@ import com.google.common.collect.Lists;
 import org.apache.ibatis.session.ResultContext;
 import org.apache.ibatis.session.ResultHandler;
 import org.apache.ibatis.session.SqlSession;
-import org.sonar.core.persistence.BatchSession;
 import org.sonar.core.persistence.MyBatis;
 import org.sonar.core.resource.ResourceDao;
 
@@ -100,28 +99,33 @@ public class PurgeDao {
   }
 
   public PurgeDao deleteProject(long rootProjectId) {
-    final BatchSession session = mybatis.openBatchSession();
+    final SqlSession session = mybatis.openBatchSession();
+    final PurgeMapper mapper = session.getMapper(PurgeMapper.class);
     try {
-      final PurgeMapper mapper = session.getMapper(PurgeMapper.class);
-      List<Long> projectIds = resourceDao.getDescendantProjectIdsAndSelf(rootProjectId, session);
-      for (Long projectId : projectIds) {
-        session.select("org.sonar.core.purge.PurgeMapper.selectResourceIdsByRootId", projectId, new ResultHandler() {
-          public void handleResult(ResultContext context) {
-            Long resourceId = (Long) context.getResultObject();
-            deleteResource(resourceId, session, mapper);
-          }
-        });
-      }
-      session.commit();
+      deleteProject(rootProjectId, session, mapper);
       return this;
-
     } finally {
       MyBatis.closeQuietly(session);
     }
   }
 
-  void deleteResource(final long resourceId, final BatchSession session, final PurgeMapper mapper) {
-    session.select("org.sonar.core.purge.PurgeMapper.selectSnapshotIdsByResource", new ResultHandler() {
+  private void deleteProject(final long rootProjectId, final SqlSession session, final PurgeMapper mapper) {
+    List<Long> childrenIds = mapper.selectProjectIdsByRootId(rootProjectId);
+    for (Long childId : childrenIds) {
+      deleteProject(childId, session, mapper);
+    }
+
+    session.select("org.sonar.core.purge.PurgeMapper.selectResourceTreeIdsByRootId", rootProjectId, new ResultHandler() {
+      public void handleResult(ResultContext context) {
+        Long resourceId = (Long) context.getResultObject();
+        deleteResource(resourceId, session, mapper);
+      }
+    });
+    session.commit();
+  }
+
+  void deleteResource(final long resourceId, final SqlSession session, final PurgeMapper mapper) {
+    session.select("org.sonar.core.purge.PurgeMapper.selectSnapshotIdsByResource", resourceId, new ResultHandler() {
       public void handleResult(ResultContext context) {
         Long snapshotId = (Long) context.getResultObject();
         deleteSnapshot(snapshotId, mapper);
@@ -149,7 +153,7 @@ public class PurgeDao {
 
 
   public PurgeDao deleteSnapshots(PurgeSnapshotQuery query) {
-    final BatchSession session = mybatis.openBatchSession();
+    final SqlSession session = mybatis.openBatchSession();
     try {
       final PurgeMapper mapper = session.getMapper(PurgeMapper.class);
       session.select("org.sonar.core.purge.PurgeMapper.selectSnapshotIds", query, new ResultHandler() {
index 31993b1896f23032c9a2a1d3c3657de6dbe05e5d..d663cd44c536c824c9c2e760379f1bdab5b90d06 100644 (file)
@@ -25,6 +25,8 @@ public interface PurgeMapper {
 
   List<Long> selectSnapshotIds(PurgeSnapshotQuery query);
 
+  List<Long> selectProjectIdsByRootId(long rootResourceId);
+
   void deleteSnapshot(long snapshotId);
 
   void deleteSnapshotDependencies(long snapshotId);
@@ -70,7 +72,7 @@ public interface PurgeMapper {
   void deleteResourceEvents(long resourceId);
 
   void closeResourceReviews(long resourceId);
-  
+
   List<PurgeableSnapshotDto> selectPurgeableSnapshotsWithVersionEvent(long resourceId);
 
   List<PurgeableSnapshotDto> selectPurgeableSnapshotsWithoutVersionEvent(long resourceId);
index e0a7f656c3da99bcf0f5bf5c1bdd42ee649b9059..34c85d4f7e21b758e2c54cf4ea523feda1c53b49 100644 (file)
@@ -34,7 +34,8 @@
       </if>
       <if test="qualifiers != null">
         and s.qualifier in
-        <foreach item="qualifier" index="index" collection="qualifiers" open="(" separator="," close=")">#{qualifier}</foreach>
+        <foreach item="qualifier" index="index" collection="qualifiers" open="(" separator="," close=")">#{qualifier}
+        </foreach>
       </if>
       <if test="withVersionEvent != null">
         <if test="withVersionEvent">
   </select>
 
   <select id="selectPurgeableSnapshotsWithVersionEvent" parameterType="long" resultType="PurgeableSnapshot">
-    select s.id as "snapshotId", s.created_at as "date", ${_true} as "hasVersionEvent", islast as "isLast" from snapshots s
-    where s.project_id=#{id} and s.status='P' and s.qualifier &lt;&gt; 'LIB' and exists(select e.id from events e where e.snapshot_id=s.id and e.category='Version')
+    select s.id as "snapshotId", s.created_at as "date", ${_true} as "hasVersionEvent", islast as "isLast" from
+    snapshots s
+    where s.project_id=#{id} and s.status='P' and s.qualifier &lt;&gt; 'LIB' and exists(select e.id from events e where
+    e.snapshot_id=s.id and e.category='Version')
   </select>
 
   <select id="selectPurgeableSnapshotsWithoutVersionEvent" parameterType="long" resultType="PurgeableSnapshot">
-    select s.id as "snapshotId", s.created_at as "date", ${_false} as "hasVersionEvent", islast as "isLast" from snapshots s
-    where s.project_id=#{id} and s.status='P' and s.qualifier &lt;&gt; 'LIB' and not exists(select e.id from events e where e.snapshot_id=s.id and e.category='Version')
+    select s.id as "snapshotId", s.created_at as "date", ${_false} as "hasVersionEvent", islast as "isLast" from
+    snapshots s
+    where s.project_id=#{id} and s.status='P' and s.qualifier &lt;&gt; 'LIB' and not exists(select e.id from events e
+    where e.snapshot_id=s.id and e.category='Version')
   </select>
 
   <select id="selectResourceIdsToDisable" resultType="long" parameterType="long">
     and not exists(select s.project_id from snapshots s where s.islast=${_true} and s.project_id=p.id)
   </select>
 
-  <select id="selectResourceIdsByRootId" resultType="long" parameterType="long">
+  <select id="selectProjectIdsByRootId" resultType="long" parameterType="long">
+    select id from projects where root_id=#{id} and scope='PRJ'
+  </select>
+
+  <select id="selectResourceTreeIdsByRootId" resultType="long" parameterType="long">
     select id from projects where root_id=#{id} or id=#{id}
   </select>
 
 
   <delete id="deleteSnapshotWastedMeasures" parameterType="long">
     delete from project_measures where snapshot_id=#{id} and
-    (characteristic_id is not null or rule_id is not null or metric_id in (select id from metrics where delete_historical_data=${_true}))
+    (characteristic_id is not null or rule_id is not null or metric_id in (select id from metrics where
+    delete_historical_data=${_true}))
   </delete>
 
   <update id="updatePurgeStatusToOne" parameterType="long">
index d7b54126fd4687e3e94a24e6d2fabf3446773009..02f6be17c390de9913f8d52102103a91fee48d4f 100644 (file)
@@ -185,7 +185,7 @@ public abstract class DaoTestCase {
   protected final void assertEmptyTables(String... emptyTables) {
     for (String table : emptyTables) {
       try {
-        Assert.assertEquals(0, getCurrentDataSet().getTable(table).getRowCount());
+        Assert.assertEquals("Table " + table + " not empty.", 0, getCurrentDataSet().getTable(table).getRowCount());
       } catch (DataSetException e) {
         throw translateException("Error while checking results", e);
       }
index a165063ee3076a4ab4af396304f1a1ecba5d755e..c1a3541aeb0f2875c523c678b064e653e3f5806b 100644 (file)
@@ -29,7 +29,6 @@ import org.sonar.core.persistence.MyBatis;
 import org.sonar.core.resource.ResourceDao;
 
 import java.util.List;
-import java.util.SortedSet;
 
 import static org.hamcrest.Matchers.is;
 import static org.junit.Assert.assertThat;
@@ -156,6 +155,28 @@ public class PurgeDaoTest extends DaoTestCase {
     assertThat(snapshots.size(), is(3));
   }
 
+  @Test
+  public void shouldDeleteResource() {
+    setupData("shouldDeleteResource");
+    SqlSession session = getMyBatis().openSession();
+    try {
+      // this method does not commit and close the session
+      dao.deleteResource(1L, session, session.getMapper(PurgeMapper.class));
+      session.commit();
+
+    } finally {
+      MyBatis.closeQuietly(session);
+    }
+    assertEmptyTables("projects", "snapshots", "events");
+  }
+
+  @Test
+  public void shouldDeleteProject() {
+    setupData("shouldDeleteProject");
+    dao.deleteProject(1L);
+    assertEmptyTables("projects", "snapshots");
+  }
+
   static final class SnapshotMatcher extends BaseMatcher<PurgeableSnapshotDto> {
     long snapshotId;
     boolean isLast;
@@ -169,7 +190,7 @@ public class PurgeDaoTest extends DaoTestCase {
 
     public boolean matches(Object o) {
       PurgeableSnapshotDto obj = (PurgeableSnapshotDto) o;
-      return obj.getSnapshotId() == snapshotId && obj.isLast()==isLast && obj.hasVersionEvent()==hasVersionEvent;
+      return obj.getSnapshotId() == snapshotId && obj.isLast() == isLast && obj.hasVersionEvent() == hasVersionEvent;
     }
 
     public void describeTo(Description description) {
diff --git a/sonar-core/src/test/resources/org/sonar/core/purge/PurgeDaoTest/shouldDeleteProject.xml b/sonar-core/src/test/resources/org/sonar/core/purge/PurgeDaoTest/shouldDeleteProject.xml
new file mode 100644 (file)
index 0000000..2a1e333
--- /dev/null
@@ -0,0 +1,67 @@
+<dataset>
+
+  <!-- root -->
+  <projects id="1" enabled="true" root_id="[null]"
+            long_name="[null]" scope="PRJ" qualifier="TRK" kee="project" name="project"
+            description="[null]" language="java" copy_resource_id="[null]" profile_id="[null]"/>
+
+  <snapshots id="1" project_id="1" parent_snapshot_id="[null]" root_project_id="[null]" root_snapshot_id="[null]"
+             status="P" islast="false" 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="[null]"/>
+
+
+  <!-- modules -->
+  <projects id="2" enabled="true" root_id="1"
+            long_name="[null]" scope="PRJ" qualifier="BRC" kee="module1" name="module1"
+            description="[null]" language="java" copy_resource_id="[null]" profile_id="[null]"/>
+
+  <snapshots id="2" project_id="2" parent_snapshot_id="1" root_project_id="1" root_snapshot_id="1"
+             status="P" islast="false" 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="[null]"/>
+
+
+  <projects id="3" enabled="false" root_id="1"
+            long_name="[null]" scope="PRJ" qualifier="BRC" kee="module2" name="module2"
+            description="[null]" language="java" copy_resource_id="[null]" profile_id="[null]"/>
+
+  <snapshots id="3" project_id="3" 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="[null]"/>
+
+  <!-- file of module 2-->
+  <projects id="4" enabled="false" root_id="3"
+            long_name="[null]" scope="FIL" qualifier="FIL" kee="module2:File.java" name="File"
+            description="[null]" language="java" copy_resource_id="[null]" profile_id="[null]"/>
+
+  <snapshots id="4" project_id="4" parent_snapshot_id="3" 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="FIL" qualifier="FIL" created_at="2008-12-02 13:58:00.00"
+             build_date="2008-12-02 13:58:00.00"
+             version="[null]" path="[null]"/>
+</dataset>
\ No newline at end of file
diff --git a/sonar-core/src/test/resources/org/sonar/core/purge/PurgeDaoTest/shouldDeleteResource.xml b/sonar-core/src/test/resources/org/sonar/core/purge/PurgeDaoTest/shouldDeleteResource.xml
new file mode 100644 (file)
index 0000000..3ed5a6f
--- /dev/null
@@ -0,0 +1,21 @@
+<dataset>
+
+  <projects id="1" enabled="true" root_id="[null]"
+            long_name="[null]" scope="PRJ" qualifier="TRK" kee="project" name="project"
+            description="[null]" language="java" copy_resource_id="[null]" profile_id="[null]"/>
+
+  <snapshots id="1" project_id="1" parent_snapshot_id="[null]" root_project_id="[null]" root_snapshot_id="[null]"
+             status="P" islast="false" 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="[null]"/>
+
+  <events id="1" name="Version 1.0" resource_id="1" snapshot_id="1" category="VERSION" description="[null]"
+          event_date="2008-12-02 13:58:00.00" created_at="[null]"/>
+
+</dataset>
\ No newline at end of file
index 29029b02d6bfa6fc1e27667d7bda1bb95eac9ee0..d20f65a6cb50e1691a9c80bb71eaf7d142dd92bf 100644 (file)
@@ -37,6 +37,7 @@ import org.sonar.api.web.*;
 import org.sonar.core.i18n.RuleI18nManager;
 import org.sonar.core.persistence.Database;
 import org.sonar.core.persistence.DatabaseMigrator;
+import org.sonar.core.purge.PurgeDao;
 import org.sonar.core.resource.ResourceIndexerDao;
 import org.sonar.markdown.Markdown;
 import org.sonar.server.configuration.Backup;
@@ -381,6 +382,10 @@ public final class JRubyFacade {
     getContainer().getComponentByType(ResourceIndexerDao.class).indexProjects();
   }
 
+  public void deleteProject(long rootProjectId) {
+    getContainer().getComponentByType(PurgeDao.class).deleteProject(rootProjectId);
+  }
+
   public void logError(String message) {
     LoggerFactory.getLogger(getClass()).error(message);
   }
index 0e4d1e27ec9976d379665d6964c30d720af0c09b..b0144926048476e69f004ceaaebe4785400c41d9 100644 (file)
@@ -41,10 +41,8 @@ class Project < ActiveRecord::Base
   end
 
   def self.delete_project(project)
-    if project
-      Snapshot.update_all(['islast=?', false], ['(root_project_id=? OR project_id=?) AND islast=?', project.id, project.id, true])
-      Project.delete_all(['id=? OR root_id=? or copy_resource_id=?', project.id, project.id, project.id])
-      ResourceIndex.delete_all(['root_project_id=?', project.id])
+    if project && project.project?
+      Java::OrgSonarServerUi::JRubyFacade.getInstance().deleteProject(project.id)
     end
   end
 
@@ -54,20 +52,20 @@ class Project < ActiveRecord::Base
 
   def root_project
     @root_project ||=
-        begin
-          parent_module(self)
-        end
+      begin
+        parent_module(self)
+      end
   end
 
   def last_snapshot
     @last_snapshot ||=
-        begin
-          snapshot=Snapshot.find(:first, :conditions => {:islast => true, :project_id => id})
-          if snapshot
-            snapshot.project=self
-          end
-          snapshot
+      begin
+        snapshot=Snapshot.find(:first, :conditions => {:islast => true, :project_id => id})
+        if snapshot
+          snapshot.project=self
         end
+        snapshot
+      end
   end
 
   def events_with_snapshot
@@ -97,12 +95,12 @@ class Project < ActiveRecord::Base
 
   def chart_measures(metric_id)
     sql = Project.send(:sanitize_sql, ['select s.created_at as created_at, m.value as value ' +
-                                           ' from project_measures m, snapshots s ' +
-                                           ' where s.id=m.snapshot_id and ' +
-                                           " s.status='%s' and " +
-                                           ' s.project_id=%s and m.metric_id=%s ', Snapshot::STATUS_PROCESSED, self.id, metric_id]) +
-        ' and m.rule_id IS NULL and m.rule_priority IS NULL' +
-        ' order by s.created_at'
+                                         ' from project_measures m, snapshots s ' +
+                                         ' where s.id=m.snapshot_id and ' +
+                                         " s.status='%s' and " +
+                                         ' s.project_id=%s and m.metric_id=%s ', Snapshot::STATUS_PROCESSED, self.id, metric_id]) +
+      ' and m.rule_id IS NULL and m.rule_priority IS NULL' +
+      ' order by s.created_at'
     create_chart_measures(Project.connection.select_all(sql), 'created_at', 'value')
   end