From 3a1b309e342a4b61cbc7ff06c6900150beee3573 Mon Sep 17 00:00:00 2001 From: simonbrandhof Date: Thu, 26 Jan 2012 23:13:09 +0100 Subject: [PATCH] SONAR-2757 improve reentrance of project deletion --- .../sonar/core/persistence/BatchSession.java | 38 +++++------ .../java/org/sonar/core/purge/PurgeDao.java | 38 ++++++----- .../org/sonar/core/purge/PurgeMapper.java | 4 +- .../org/sonar/core/purge/PurgeMapper.xml | 24 +++++-- .../sonar/core/persistence/DaoTestCase.java | 2 +- .../org/sonar/core/purge/PurgeDaoTest.java | 25 ++++++- .../PurgeDaoTest/shouldDeleteProject.xml | 67 +++++++++++++++++++ .../PurgeDaoTest/shouldDeleteResource.xml | 21 ++++++ .../java/org/sonar/server/ui/JRubyFacade.java | 5 ++ .../main/webapp/WEB-INF/app/models/project.rb | 36 +++++----- 10 files changed, 192 insertions(+), 68 deletions(-) create mode 100644 sonar-core/src/test/resources/org/sonar/core/purge/PurgeDaoTest/shouldDeleteProject.xml create mode 100644 sonar-core/src/test/resources/org/sonar/core/purge/PurgeDaoTest/shouldDeleteResource.xml diff --git a/sonar-core/src/main/java/org/sonar/core/persistence/BatchSession.java b/sonar-core/src/main/java/org/sonar/core/persistence/BatchSession.java index 2a416fd11b3..4413fff3aee 100644 --- a/sonar-core/src/main/java/org/sonar/core/persistence/BatchSession.java +++ b/sonar-core/src/main/java/org/sonar/core/persistence/BatchSession.java @@ -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; + } } diff --git a/sonar-core/src/main/java/org/sonar/core/purge/PurgeDao.java b/sonar-core/src/main/java/org/sonar/core/purge/PurgeDao.java index 726f59dcb52..e751cc32f20 100644 --- a/sonar-core/src/main/java/org/sonar/core/purge/PurgeDao.java +++ b/sonar-core/src/main/java/org/sonar/core/purge/PurgeDao.java @@ -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 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 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() { diff --git a/sonar-core/src/main/java/org/sonar/core/purge/PurgeMapper.java b/sonar-core/src/main/java/org/sonar/core/purge/PurgeMapper.java index 31993b1896f..d663cd44c53 100644 --- a/sonar-core/src/main/java/org/sonar/core/purge/PurgeMapper.java +++ b/sonar-core/src/main/java/org/sonar/core/purge/PurgeMapper.java @@ -25,6 +25,8 @@ public interface PurgeMapper { List selectSnapshotIds(PurgeSnapshotQuery query); + List 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 selectPurgeableSnapshotsWithVersionEvent(long resourceId); List selectPurgeableSnapshotsWithoutVersionEvent(long resourceId); diff --git a/sonar-core/src/main/resources/org/sonar/core/purge/PurgeMapper.xml b/sonar-core/src/main/resources/org/sonar/core/purge/PurgeMapper.xml index e0a7f656c3d..34c85d4f7e2 100644 --- a/sonar-core/src/main/resources/org/sonar/core/purge/PurgeMapper.xml +++ b/sonar-core/src/main/resources/org/sonar/core/purge/PurgeMapper.xml @@ -34,7 +34,8 @@ and s.qualifier in - #{qualifier} + #{qualifier} + @@ -48,13 +49,17 @@ - + select id from projects where root_id=#{id} and scope='PRJ' + + + @@ -105,7 +114,8 @@ 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})) diff --git a/sonar-core/src/test/java/org/sonar/core/persistence/DaoTestCase.java b/sonar-core/src/test/java/org/sonar/core/persistence/DaoTestCase.java index d7b54126fd4..02f6be17c39 100644 --- a/sonar-core/src/test/java/org/sonar/core/persistence/DaoTestCase.java +++ b/sonar-core/src/test/java/org/sonar/core/persistence/DaoTestCase.java @@ -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); } diff --git a/sonar-core/src/test/java/org/sonar/core/purge/PurgeDaoTest.java b/sonar-core/src/test/java/org/sonar/core/purge/PurgeDaoTest.java index a165063ee30..c1a3541aeb0 100644 --- a/sonar-core/src/test/java/org/sonar/core/purge/PurgeDaoTest.java +++ b/sonar-core/src/test/java/org/sonar/core/purge/PurgeDaoTest.java @@ -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 { 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 index 00000000000..2a1e333079f --- /dev/null +++ b/sonar-core/src/test/resources/org/sonar/core/purge/PurgeDaoTest/shouldDeleteProject.xml @@ -0,0 +1,67 @@ + + + + + + + + + + + + + + + + + + + + + + + \ 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 index 00000000000..3ed5a6f4d36 --- /dev/null +++ b/sonar-core/src/test/resources/org/sonar/core/purge/PurgeDaoTest/shouldDeleteResource.xml @@ -0,0 +1,21 @@ + + + + + + + + + \ No newline at end of file diff --git a/sonar-server/src/main/java/org/sonar/server/ui/JRubyFacade.java b/sonar-server/src/main/java/org/sonar/server/ui/JRubyFacade.java index 29029b02d6b..d20f65a6cb5 100644 --- a/sonar-server/src/main/java/org/sonar/server/ui/JRubyFacade.java +++ b/sonar-server/src/main/java/org/sonar/server/ui/JRubyFacade.java @@ -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); } diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/project.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/project.rb index 0e4d1e27ec9..b0144926048 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/models/project.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/models/project.rb @@ -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 -- 2.39.5