From 7096d6e979acb20b0a0ec025c6d8c7ac5f329246 Mon Sep 17 00:00:00 2001 From: David Read Date: Fri, 11 Sep 2015 17:25:32 +0100 Subject: [PATCH] [#1572] Improvements * no revision is needed when purging the package. The only thing being created is a Activity, which is not revisioned. * no need to explicitly delete an object's revision objects. Despite the very old comment, it is clear that this does happen naturally through cascades. This is verified in the test and by reading the SQL produced. * Added a resource to the test for fullness. * Added helpful comments to the test-core.ini to show people how to use it to see generated SQL commands. --- ckan/logic/action/delete.py | 3 ++- .../migration/versions/080_continuity_id_indexes.py | 13 +++++++++++++ ckan/model/domain_object.py | 7 ++----- ckan/tests/logic/action/test_delete.py | 7 +++++-- test-core.ini | 5 ++++- 5 files changed, 26 insertions(+), 9 deletions(-) create mode 100644 ckan/migration/versions/080_continuity_id_indexes.py diff --git a/ckan/logic/action/delete.py b/ckan/logic/action/delete.py index 1d741f6875e..e8b70d40968 100644 --- a/ckan/logic/action/delete.py +++ b/ckan/logic/action/delete.py @@ -110,7 +110,8 @@ def dataset_purge(context, data_dict): m.purge() pkg = model.Package.get(id) - model.repo.new_revision() + # no new_revision() needed since there are no object_revisions created + # during purge pkg.purge() model.repo.commit_and_remove() diff --git a/ckan/migration/versions/080_continuity_id_indexes.py b/ckan/migration/versions/080_continuity_id_indexes.py new file mode 100644 index 00000000000..1b223ecf47d --- /dev/null +++ b/ckan/migration/versions/080_continuity_id_indexes.py @@ -0,0 +1,13 @@ +def upgrade(migrate_engine): + migrate_engine.execute( + ''' + CREATE INDEX idx_member_continuity_id + ON member_revision (continuity_id); + CREATE INDEX idx_package_tag_continuity_id + ON package_tag_revision (continuity_id); + CREATE INDEX idx_package_continuity_id + ON package_revision (continuity_id); + CREATE INDEX idx_package_extra_continuity_id + ON package_extra_revision (continuity_id); + ''' + ) diff --git a/ckan/model/domain_object.py b/ckan/model/domain_object.py index 4237d77aea7..9e3a2a287f9 100644 --- a/ckan/model/domain_object.py +++ b/ckan/model/domain_object.py @@ -76,15 +76,12 @@ def remove(self): self.Session.remove() def delete(self): + # stateful objects have this method overridden - see + # vmd.base.StatefulObjectMixin self.Session.delete(self) def purge(self): self.Session().autoflush = False - if hasattr(self, '__revisioned__'): # only for versioned objects ... - # this actually should auto occur due to cascade relationships but - # ... - for rev in self.all_revisions: - self.Session.delete(rev) self.Session.delete(self) def as_dict(self): diff --git a/ckan/tests/logic/action/test_delete.py b/ckan/tests/logic/action/test_delete.py index 37875c94a90..8aeacca68cf 100644 --- a/ckan/tests/logic/action/test_delete.py +++ b/ckan/tests/logic/action/test_delete.py @@ -207,6 +207,7 @@ def test_purged_dataset_leaves_no_trace_in_the_model(self): groups=[{'name': 'group1'}], owner_org=org['id'], extras=[{'key': 'testkey', 'value': 'testvalue'}]) + factories.Resource(package_id=dataset['id']) num_revisions_before = model.Session.query(model.Revision).count() helpers.call_action('dataset_purge', @@ -216,6 +217,7 @@ def test_purged_dataset_leaves_no_trace_in_the_model(self): # the Package and related objects are gone assert_equals(model.Session.query(model.Package).all(), []) + assert_equals(model.Session.query(model.Resource).all(), []) assert_equals(model.Session.query(model.PackageTag).all(), []) # there is no clean-up of the tag object itself, just the PackageTag. assert_equals([t.name for t in model.Session.query(model.Tag).all()], @@ -230,13 +232,14 @@ def test_purged_dataset_leaves_no_trace_in_the_model(self): # all the object revisions were purged too assert_equals(model.Session.query(model.PackageRevision).all(), []) + assert_equals(model.Session.query(model.ResourceRevision).all(), []) assert_equals(model.Session.query(model.PackageTagRevision).all(), []) assert_equals(model.Session.query(model.PackageExtraRevision).all(), []) # Member is not revisioned - # No Revision objects were purged, in fact 1 is created for the purge - assert_equals(num_revisions_after - num_revisions_before, 1) + # No Revision objects were purged or created + assert_equals(num_revisions_after - num_revisions_before, 0) def test_missing_id_returns_error(self): assert_raises(logic.ValidationError, diff --git a/test-core.ini b/test-core.ini index 4a0aa72580f..0602a2e40e4 100644 --- a/test-core.ini +++ b/test-core.ini @@ -127,7 +127,10 @@ level = INFO [logger_sqlalchemy] handlers = qualname = sqlalchemy.engine -level = WARN +level = WARNING +# "level = INFO" logs SQL queries. +# "level = DEBUG" logs SQL queries and results. +# "level = WARNING" logs neither. [handler_console] class = StreamHandler