From 01c2d139f25607e06a5dd09a489879ea7f1a422a Mon Sep 17 00:00:00 2001 From: David Read Date: Fri, 15 May 2015 16:05:30 +0100 Subject: [PATCH] [#2426] Acknowledge that before_delete only fires on PURGE not DELETE of an object. Adjusted docs and added test. --- ckan/plugins/interfaces.py | 8 +++-- ckan/tests/legacy/ckantestplugins.py | 15 ++++++--- ckan/tests/legacy/test_plugins.py | 50 ++++++++++++++++++++-------- 3 files changed, 54 insertions(+), 19 deletions(-) diff --git a/ckan/plugins/interfaces.py b/ckan/plugins/interfaces.py index bd151dbb279..f81b76f9ba2 100644 --- a/ckan/plugins/interfaces.py +++ b/ckan/plugins/interfaces.py @@ -129,7 +129,9 @@ def before_update(self, mapper, connection, instance): def before_delete(self, mapper, connection, instance): """ - Receive an object instance before that instance is DELETEed. + Receive an object instance before that instance is PURGEd. + (whereas usually in ckan 'delete' means to change the state property to + deleted, so use before_update for that case.) """ def after_insert(self, mapper, connection, instance): @@ -144,7 +146,9 @@ def after_update(self, mapper, connection, instance): def after_delete(self, mapper, connection, instance): """ - Receive an object instance after that instance is DELETEed. + Receive an object instance after that instance is PURGEd. + (whereas usually in ckan 'delete' means to change the state property to + deleted, so use before_update for that case.) """ diff --git a/ckan/tests/legacy/ckantestplugins.py b/ckan/tests/legacy/ckantestplugins.py index 81b51091718..49f81b17bd9 100644 --- a/ckan/tests/legacy/ckantestplugins.py +++ b/ckan/tests/legacy/ckantestplugins.py @@ -8,18 +8,25 @@ class MapperPlugin(p.SingletonPlugin): p.implements(p.IMapper, inherit=True) def __init__(self, *args, **kw): - self.added = [] - self.deleted = [] + self.calls = [] def before_insert(self, mapper, conn, instance): - self.added.append(instance) + self.calls.append(('before_insert', instance.name)) + + def after_insert(self, mapper, conn, instance): + self.calls.append(('after_insert', instance.name)) def before_delete(self, mapper, conn, instance): - self.deleted.append(instance) + self.calls.append(('before_delete', instance.name)) + + def after_delete(self, mapper, conn, instance): + self.calls.append(('after_delete', instance.name)) + class MapperPlugin2(MapperPlugin): p.implements(p.IMapper) + class SessionPlugin(p.SingletonPlugin): p.implements(p.ISession, inherit=True) diff --git a/ckan/tests/legacy/test_plugins.py b/ckan/tests/legacy/test_plugins.py index 401e6eb5f13..954ba03bd02 100644 --- a/ckan/tests/legacy/test_plugins.py +++ b/ckan/tests/legacy/test_plugins.py @@ -1,7 +1,7 @@ """ Tests for plugin loading via PCA """ -from nose.tools import raises +from nose.tools import raises, assert_equal from unittest import TestCase from pyutilib.component.core import PluginGlobals from pylons import config @@ -11,7 +11,7 @@ import ckan.plugins as plugins from ckan.plugins.core import find_system_plugins from ckan.lib.create_test_data import CreateTestData - +from ckan.tests import factories def _make_calls(*args): out = [] @@ -19,6 +19,11 @@ def _make_calls(*args): out.append(((arg,), {})) return out +def get_calls(mock_observer_func): + '''Given a mock IPluginObserver method, returns the plugins that caused its + methods to be called, so basically a list of plugins that + loaded/unloaded''' + return [call_tuple[0][0].name for call_tuple in mock_observer_func.calls] class IFoo(plugins.Interface): pass @@ -52,8 +57,8 @@ def test_provided_by(self): assert IFoo.provided_by(FooBarImpl()) assert not IFoo.provided_by(BarImpl()) -class TestIPluginObserverPlugin(object): +class TestIPluginObserverPlugin(object): @classmethod def setup(cls): @@ -67,11 +72,11 @@ def test_notified_on_load(self): observer = self.observer observer.reset_calls() - with plugins.use_plugin('action_plugin') as action: - assert observer.before_load.calls == _make_calls(action), observer.before_load.calls - assert observer.after_load.calls == _make_calls(action), observer.after_load.calls - assert observer.before_unload.calls == [] - assert observer.after_unload.calls == [] + with plugins.use_plugin('action_plugin'): + assert_equal(get_calls(observer.before_load), ['action_plugin']) + assert_equal(get_calls(observer.after_load), ['action_plugin']) + assert_equal(get_calls(observer.before_unload), []) + assert_equal(get_calls(observer.after_unload), []) def test_notified_on_unload(self): @@ -139,13 +144,32 @@ def test_plugin_loading_order(self): config['ckan.plugins'] = config_plugins plugins.load_all(config) - def test_mapper_plugin_fired(self): + def test_mapper_plugin_fired_on_insert(self): + with plugins.use_plugin('mapper_plugin') as mapper_plugin: + CreateTestData.create_arbitrary([{'name': u'testpkg'}]) + assert mapper_plugin.calls == [ + ('before_insert', 'testpkg'), + ('after_insert', 'testpkg'), + ] + + def test_mapper_plugin_fired_on_delete(self): with plugins.use_plugin('mapper_plugin') as mapper_plugin: - CreateTestData.create_arbitrary([{'name':u'testpkg'}]) + CreateTestData.create_arbitrary([{'name': u'testpkg'}]) + mapper_plugin.calls = [] # remove this data - CreateTestData.delete() - assert len(mapper_plugin.added) == 1 - assert mapper_plugin.added[0].name == 'testpkg' + user = factories.User() + context = {'user': user['name']} + logic.get_action('package_delete')(context, {'id': 'testpkg'}) + # state=deleted doesn't trigger before_delete() + assert_equal(mapper_plugin.calls, []) + from ckan import model + # purging the package does trigger before_delete() + model.Package.get('testpkg').purge() + model.Session.commit() + model.Session.remove() + assert_equal(mapper_plugin.calls, + [('before_delete', 'testpkg'), + ('after_delete', 'testpkg')]) def test_routes_plugin_fired(self): with plugins.use_plugin('routes_plugin'):