From 6eef54bfd11a60ed965735c8bac6f4b0cea42c97 Mon Sep 17 00:00:00 2001 From: Wes Kendall Date: Mon, 12 May 2014 22:10:01 -0400 Subject: [PATCH 1/4] uses djangos import_by_path method --- dynamic_initial_data/base.py | 17 ++- dynamic_initial_data/tests/base_tests.py | 141 ++++++++++---------- dynamic_initial_data/tests/mocks.py | 3 +- dynamic_initial_data/tests/util_tests.py | 23 ---- dynamic_initial_data/utils/__init__.py | 0 dynamic_initial_data/utils/import_string.py | 25 ---- dynamic_initial_data/version.py | 2 +- 7 files changed, 87 insertions(+), 124 deletions(-) delete mode 100644 dynamic_initial_data/tests/util_tests.py delete mode 100644 dynamic_initial_data/utils/__init__.py delete mode 100644 dynamic_initial_data/utils/import_string.py diff --git a/dynamic_initial_data/base.py b/dynamic_initial_data/base.py index bce1494..80376aa 100644 --- a/dynamic_initial_data/base.py +++ b/dynamic_initial_data/base.py @@ -2,11 +2,12 @@ from django.conf import settings from django.contrib.contenttypes.models import ContentType +from django.core.exceptions import ImproperlyConfigured from django.db.transaction import atomic +from django.utils.module_loading import import_by_path from dynamic_initial_data.exceptions import InitialDataCircularDependency, InitialDataMissingApp from dynamic_initial_data.models import RegisteredForDeletionReceipt -from dynamic_initial_data.utils.import_string import import_string class BaseInitialData(object): @@ -88,10 +89,14 @@ def load_app(self, app): return self.loaded_apps.get(app) self.loaded_apps[app] = None - initial_data_class = import_string(self.get_class_path(app)) - if initial_data_class and issubclass(initial_data_class, BaseInitialData): - self.log('Loaded app {0}'.format(app)) - self.loaded_apps[app] = initial_data_class + try: + initial_data_class = import_by_path(self.get_class_path(app)) + if issubclass(initial_data_class, BaseInitialData): + self.log('Loaded app {0}'.format(app)) + self.loaded_apps[app] = initial_data_class + except ImproperlyConfigured: + pass + return self.loaded_apps[app] @atomic @@ -188,6 +193,7 @@ def get_dependency_call_list(self, app, call_list=None): """ # start the call_list with the current app if one wasn't passed in recursively call_list = call_list or [app] + # load the initial data class for the app initial_data_class = self.load_app(app) if initial_data_class: @@ -201,6 +207,7 @@ def get_dependency_call_list(self, app, call_list=None): call_list.extend(dependencies) else: raise InitialDataMissingApp(dep=app) + return call_list[1:] def log(self, str): diff --git a/dynamic_initial_data/tests/base_tests.py b/dynamic_initial_data/tests/base_tests.py index ffd2da1..e3e5927 100644 --- a/dynamic_initial_data/tests/base_tests.py +++ b/dynamic_initial_data/tests/base_tests.py @@ -1,8 +1,8 @@ from datetime import datetime -from django.conf import settings from django.contrib.contenttypes.models import ContentType from django.test import TestCase +from django.test.utils import override_settings from django_dynamic_fixture import G from freezegun import freeze_time from mock import patch @@ -160,99 +160,102 @@ def test_verbose_option(self): # cover the branch that prints if verbose is true initial_data_manager.log('test') - def test_load_app(self): + @patch('dynamic_initial_data.base.import_by_path', return_value=MockInitialData) + def test_load_app_exists(self, import_patch): """ - Tests the load_app method + Tests the load_app method on an app that exists """ - with patch('dynamic_initial_data.base.import_string') as import_patch: - import_patch.return_value = MockInitialData - initial_data_manager = InitialDataUpdater() - self.assertEqual(MockInitialData, initial_data_manager.load_app('fake')) + self.assertEqual(MockInitialData, InitialDataUpdater().load_app('fake')) - # try to load an app that doesn't exist - initial_data_manager = InitialDataUpdater() - import_patch.return_value = MockClass - self.assertIsNone(initial_data_manager.load_app('fake')) - - def test_update_app(self): + @patch('dynamic_initial_data.base.import_by_path', return_value=MockInitialData) + def test_load_app_cached(self, import_patch): """ - Tests the update_app method + Tests that the cache is hit since import is only called once. """ - # an error should only be raised for missing dependencies and not for directly - # calling update on an app that doesn't have an initial data file - initial_data_manager = InitialDataUpdater() - initial_data_manager.update_app('fake') + initial_data_updater = InitialDataUpdater() + initial_data_updater.load_app('fake') + initial_data_updater.load_app('fake') + initial_data_updater.load_app('fake') + self.assertEquals(import_patch.call_count, 1) + + @patch('dynamic_initial_data.base.import_by_path', return_value=MockClass) + def test_load_app_doesnt_exist(self, import_patch): + """ + Tests the load_app method on an app that doesnt exist + """ + self.assertIsNone(InitialDataUpdater().load_app('fake')) - # make sure app gets added to updated apps - initial_data_manager = InitialDataUpdater() - with patch('dynamic_initial_data.base.InitialDataUpdater.get_class_path', spec_set=True) as get_path_patch: - get_path_patch.return_value = 'dynamic_initial_data.tests.mocks.MockInitialData' + @patch.object(InitialDataUpdater, 'load_app', return_value=MockInitialData, set_spec=True) + def test_update_app_no_errors_raised(self, mock_load_app): + """ + Tests the update_app method. No errors should be raised since it has all of the required + components + """ + InitialDataUpdater().update_app('fake') - # patch the update_initial_data method so we make sure it is called - update_initial_data_patcher = patch('dynamic_initial_data.tests.mocks.MockInitialData.update_initial_data') - update_initial_data_patch = update_initial_data_patcher.start() - initial_data_manager.update_app('dynamic_initial_data') - self.assertEqual(1, update_initial_data_patch.call_count) + def test_update_app_cant_load_initial_data(self): + """ + Tests when the initial data class can't be loaded. It should still execute + """ + InitialDataUpdater().update_app('bad_app_path') - # make sure it doesn't call update static again - initial_data_manager.update_app('dynamic_initial_data') - self.assertEqual(1, update_initial_data_patch.call_count) + @patch.object(InitialDataUpdater, 'load_app', return_value=MockInitialData, set_spec=True) + @patch('dynamic_initial_data.tests.mocks.MockInitialData.update_initial_data', set_spec=True) + def test_update_app_cached_updated_apps(self, update_initial_data_patch, mock_load_app): + """ + Tests that the app gets added to updated apps and it is cached + """ + initial_data_manager = InitialDataUpdater() + initial_data_manager.update_app('dynamic_initial_data') + self.assertEqual(1, update_initial_data_patch.call_count) - # stop the patcher - update_initial_data_patcher.stop() + # make sure it doesn't call update static again + initial_data_manager.update_app('dynamic_initial_data') + self.assertEqual(1, update_initial_data_patch.call_count) - # make sure the app is in the updated_apps list - self.assertIn('dynamic_initial_data', initial_data_manager.updated_apps) + # make sure the app is in the updated_apps list + self.assertIn('dynamic_initial_data', initial_data_manager.updated_apps) + @patch('dynamic_initial_data.tests.mocks.MockOne.update_initial_data', set_spec=True) + @patch('dynamic_initial_data.tests.mocks.MockTwo.update_initial_data', set_spec=True) + def test_update_app_dependencies(self, update_initial_data_patch2, update_initial_data_patch1): + """ + Tests the update_app method when there are dependencies. + """ # test dependencies def app_loader(app): if app == 'MockOne': return MockOne - elif app == 'MockTwo': + else: return MockTwo - return None - - # coverage - app_loader(None) - - initial_data_manager = InitialDataUpdater() - with patch('dynamic_initial_data.base.InitialDataUpdater.load_app', spec_set=True) as load_app_patch: - load_app_patch.side_effect = app_loader - - # patch update_initial_data methods - update_initial_data_patcher1 = patch('dynamic_initial_data.tests.mocks.MockOne.update_initial_data') - update_initial_data_patcher2 = patch('dynamic_initial_data.tests.mocks.MockTwo.update_initial_data') - update_initial_data_patch1 = update_initial_data_patcher1.start() - update_initial_data_patch2 = update_initial_data_patcher2.start() - initial_data_manager.update_app('MockTwo') + with patch.object(InitialDataUpdater, 'load_app', side_effect=app_loader, spec_set=True): + InitialDataUpdater().update_app('MockTwo') self.assertEqual(1, update_initial_data_patch1.call_count) self.assertEqual(1, update_initial_data_patch2.call_count) - update_initial_data_patcher1.stop() - update_initial_data_patcher2.stop() - - def test_update_all_apps(self): + @override_settings(INSTALLED_APPS=('hello', 'world',)) + @patch('dynamic_initial_data.base.InitialDataUpdater.update_app', spec_set=True) + def test_update_all_apps(self, update_app_patch): """ Verifies that update_app is called with all installed apps """ - num_apps = len(settings.INSTALLED_APPS) - with patch('dynamic_initial_data.base.InitialDataUpdater.update_app', spec_set=True) as update_app_patch: - initial_data_manager = InitialDataUpdater() - initial_data_manager.update_all_apps() - self.assertEqual(num_apps, update_app_patch.call_count) + initial_data_manager = InitialDataUpdater() + initial_data_manager.update_all_apps() + self.assertEqual(2, update_app_patch.call_count) - def test_get_dependency_call_list(self): + @patch('dynamic_initial_data.base.InitialDataUpdater.load_app', return_value=MockThree, spec_set=True) + def test_get_dependency_call_list_circular_dependency(self, load_app_patch): """ - Makes sure that dependency cycles are found and raises an exception + Tests when a circular dependency is found """ - initial_data_manager = InitialDataUpdater() - with patch('dynamic_initial_data.base.InitialDataUpdater.load_app', spec_set=True) as load_app_patch: - load_app_patch.return_value = MockThree - - with self.assertRaises(InitialDataCircularDependency): - initial_data_manager.update_app('MockThree') + with self.assertRaises(InitialDataCircularDependency): + InitialDataUpdater().update_app('MockThree') - initial_data_manager = InitialDataUpdater() + @patch('dynamic_initial_data.base.InitialDataUpdater.load_app', return_value=None, spec_set=True) + def test_get_dependency_call_list_initial_data_missing(self, load_app_patch): + """ + Tests when the initial data is missing. + """ with self.assertRaises(InitialDataMissingApp): - initial_data_manager.get_dependency_call_list('fake') + InitialDataUpdater().get_dependency_call_list('fake') diff --git a/dynamic_initial_data/tests/mocks.py b/dynamic_initial_data/tests/mocks.py index 4c14e0b..5aa4171 100644 --- a/dynamic_initial_data/tests/mocks.py +++ b/dynamic_initial_data/tests/mocks.py @@ -8,7 +8,8 @@ class MockClass(object): class MockInitialData(BaseInitialData): - pass + def update_initial_data(self): + pass class MockOne(BaseInitialData): diff --git a/dynamic_initial_data/tests/util_tests.py b/dynamic_initial_data/tests/util_tests.py deleted file mode 100644 index 29cbc19..0000000 --- a/dynamic_initial_data/tests/util_tests.py +++ /dev/null @@ -1,23 +0,0 @@ -from django.test import TestCase -from dynamic_initial_data.utils.import_string import import_string - - -class UtilTest(TestCase): - """ - Tests the functions in the utils file - """ - def test_import_string(self): - """ - Tests all outcomes out import_string - """ - # Make sure an invalid module path returns None - self.assertIsNone(import_string('nope.nope')) - - # Make sure an invalid module name returns None - self.assertIsNone(import_string('dynamic_initial_data.nope')) - - # For test coverage, import a null value - self.assertIsNone(import_string('dynamic_initial_data.tests.mocks.mock_null_value')) - - # For test coverage, import a real class - self.assertIsNotNone(import_string('dynamic_initial_data.tests.mocks.MockClass')) diff --git a/dynamic_initial_data/utils/__init__.py b/dynamic_initial_data/utils/__init__.py deleted file mode 100644 index e69de29..0000000 diff --git a/dynamic_initial_data/utils/import_string.py b/dynamic_initial_data/utils/import_string.py deleted file mode 100644 index da73493..0000000 --- a/dynamic_initial_data/utils/import_string.py +++ /dev/null @@ -1,25 +0,0 @@ -def import_string(module_string): - """ - Loads the class specified by module string. This should be the full class to the path - Example: app.name.module.ClassName - """ - parts = module_string.split('.') - assert len(parts) > 1 - - path = '.'.join(parts[:-1]) - module_name = parts[-1] - file_name = parts[-2] - - try: - module_path = __import__(path, globals(), locals(), [file_name]) - except ImportError: - return None - - if not module_path or not hasattr(module_path, module_name): - return None - - module = getattr(module_path, module_name) - if not module: - return None - - return module diff --git a/dynamic_initial_data/version.py b/dynamic_initial_data/version.py index 7fd229a..fc79d63 100644 --- a/dynamic_initial_data/version.py +++ b/dynamic_initial_data/version.py @@ -1 +1 @@ -__version__ = '0.2.0' +__version__ = '0.2.1' From b5c37eef26c054960692df54e14f360f3573cd9b Mon Sep 17 00:00:00 2001 From: Wes Kendall Date: Mon, 12 May 2014 22:15:31 -0400 Subject: [PATCH 2/4] removed unnecessary patch --- dynamic_initial_data/tests/base_tests.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dynamic_initial_data/tests/base_tests.py b/dynamic_initial_data/tests/base_tests.py index e3e5927..65a3661 100644 --- a/dynamic_initial_data/tests/base_tests.py +++ b/dynamic_initial_data/tests/base_tests.py @@ -252,8 +252,7 @@ def test_get_dependency_call_list_circular_dependency(self, load_app_patch): with self.assertRaises(InitialDataCircularDependency): InitialDataUpdater().update_app('MockThree') - @patch('dynamic_initial_data.base.InitialDataUpdater.load_app', return_value=None, spec_set=True) - def test_get_dependency_call_list_initial_data_missing(self, load_app_patch): + def test_get_dependency_call_list_initial_data_missing(self): """ Tests when the initial data is missing. """ From cd753558ab9a2ea53dd52a5673adaed6fc4045f4 Mon Sep 17 00:00:00 2001 From: Wes Kendall Date: Mon, 19 May 2014 12:42:17 -0400 Subject: [PATCH 3/4] changed set_spec to spec_set --- dynamic_initial_data/tests/base_tests.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/dynamic_initial_data/tests/base_tests.py b/dynamic_initial_data/tests/base_tests.py index 65a3661..26bba30 100644 --- a/dynamic_initial_data/tests/base_tests.py +++ b/dynamic_initial_data/tests/base_tests.py @@ -185,7 +185,7 @@ def test_load_app_doesnt_exist(self, import_patch): """ self.assertIsNone(InitialDataUpdater().load_app('fake')) - @patch.object(InitialDataUpdater, 'load_app', return_value=MockInitialData, set_spec=True) + @patch.object(InitialDataUpdater, 'load_app', return_value=MockInitialData, spec_set=True) def test_update_app_no_errors_raised(self, mock_load_app): """ Tests the update_app method. No errors should be raised since it has all of the required @@ -199,8 +199,8 @@ def test_update_app_cant_load_initial_data(self): """ InitialDataUpdater().update_app('bad_app_path') - @patch.object(InitialDataUpdater, 'load_app', return_value=MockInitialData, set_spec=True) - @patch('dynamic_initial_data.tests.mocks.MockInitialData.update_initial_data', set_spec=True) + @patch.object(InitialDataUpdater, 'load_app', return_value=MockInitialData, spec_set=True) + @patch('dynamic_initial_data.tests.mocks.MockInitialData.update_initial_data', spec_set=True) def test_update_app_cached_updated_apps(self, update_initial_data_patch, mock_load_app): """ Tests that the app gets added to updated apps and it is cached @@ -216,8 +216,8 @@ def test_update_app_cached_updated_apps(self, update_initial_data_patch, mock_lo # make sure the app is in the updated_apps list self.assertIn('dynamic_initial_data', initial_data_manager.updated_apps) - @patch('dynamic_initial_data.tests.mocks.MockOne.update_initial_data', set_spec=True) - @patch('dynamic_initial_data.tests.mocks.MockTwo.update_initial_data', set_spec=True) + @patch('dynamic_initial_data.tests.mocks.MockOne.update_initial_data', spec_set=True) + @patch('dynamic_initial_data.tests.mocks.MockTwo.update_initial_data', spec_set=True) def test_update_app_dependencies(self, update_initial_data_patch2, update_initial_data_patch1): """ Tests the update_app method when there are dependencies. From 76bdae1c3672974fcb060711d9b2f3b262ef0a52 Mon Sep 17 00:00:00 2001 From: Wes Kendall Date: Fri, 23 May 2014 20:53:45 -0400 Subject: [PATCH 4/4] fixed error related to handling deletions of multiple objects --- dynamic_initial_data/base.py | 2 +- ...eletionreceipt_model_obj_type_model_obj.py | 37 +++++++++++++++++++ dynamic_initial_data/models.py | 3 ++ dynamic_initial_data/tests/base_tests.py | 15 ++++++++ .../tests/integration_tests.py | 25 +++++++++++++ dynamic_initial_data/version.py | 2 +- 6 files changed, 82 insertions(+), 2 deletions(-) create mode 100644 dynamic_initial_data/migrations/0002_auto__add_unique_registeredfordeletionreceipt_model_obj_type_model_obj.py diff --git a/dynamic_initial_data/base.py b/dynamic_initial_data/base.py index 80376aa..79fb34e 100644 --- a/dynamic_initial_data/base.py +++ b/dynamic_initial_data/base.py @@ -153,7 +153,7 @@ def handle_deletions(self): RegisteredForDeletionReceipt( model_obj_type=ContentType.objects.get_for_model(model_obj), model_obj_id=model_obj.id, register_time=now) - for model_obj in self.model_objs_registered_for_deletion + for model_obj in set(self.model_objs_registered_for_deletion) ] # Do a bulk upsert on all of the receipts, updating their registration time. diff --git a/dynamic_initial_data/migrations/0002_auto__add_unique_registeredfordeletionreceipt_model_obj_type_model_obj.py b/dynamic_initial_data/migrations/0002_auto__add_unique_registeredfordeletionreceipt_model_obj_type_model_obj.py new file mode 100644 index 0000000..fa557be --- /dev/null +++ b/dynamic_initial_data/migrations/0002_auto__add_unique_registeredfordeletionreceipt_model_obj_type_model_obj.py @@ -0,0 +1,37 @@ +# -*- coding: utf-8 -*- +import datetime +from south.db import db +from south.v2 import SchemaMigration +from django.db import models + + +class Migration(SchemaMigration): + + def forwards(self, orm): + # Adding unique constraint on 'RegisteredForDeletionReceipt', fields ['model_obj_type', 'model_obj_id'] + db.create_unique(u'dynamic_initial_data_registeredfordeletionreceipt', ['model_obj_type_id', 'model_obj_id']) + + + def backwards(self, orm): + # Removing unique constraint on 'RegisteredForDeletionReceipt', fields ['model_obj_type', 'model_obj_id'] + db.delete_unique(u'dynamic_initial_data_registeredfordeletionreceipt', ['model_obj_type_id', 'model_obj_id']) + + + models = { + u'contenttypes.contenttype': { + 'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"}, + 'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '100'}) + }, + u'dynamic_initial_data.registeredfordeletionreceipt': { + 'Meta': {'unique_together': "(('model_obj_type', 'model_obj_id'),)", 'object_name': 'RegisteredForDeletionReceipt'}, + u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'model_obj_id': ('django.db.models.fields.PositiveIntegerField', [], {}), + 'model_obj_type': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['contenttypes.ContentType']"}), + 'register_time': ('django.db.models.fields.DateTimeField', [], {}) + } + } + + complete_apps = ['dynamic_initial_data'] \ No newline at end of file diff --git a/dynamic_initial_data/models.py b/dynamic_initial_data/models.py index 13f9056..a3339e5 100644 --- a/dynamic_initial_data/models.py +++ b/dynamic_initial_data/models.py @@ -19,3 +19,6 @@ class RegisteredForDeletionReceipt(models.Model): # Use manager utils for bulk updating capabilities objects = ManagerUtilsManager() + + class Meta: + unique_together = ('model_obj_type', 'model_obj_id') diff --git a/dynamic_initial_data/tests/base_tests.py b/dynamic_initial_data/tests/base_tests.py index 26bba30..77d8418 100644 --- a/dynamic_initial_data/tests/base_tests.py +++ b/dynamic_initial_data/tests/base_tests.py @@ -72,6 +72,21 @@ def test_create_one_obj(self): self.assertEquals(receipt.model_obj_id, account.id) self.assertEquals(receipt.register_time, datetime(2013, 4, 12)) + def test_create_dup_objs(self): + """ + Tests creating duplicate objects for deletion. + """ + account = G(Account) + self.initial_data_updater.model_objs_registered_for_deletion = [account, account] + + self.assertEquals(RegisteredForDeletionReceipt.objects.count(), 0) + with freeze_time('2013-04-12'): + self.initial_data_updater.handle_deletions() + receipt = RegisteredForDeletionReceipt.objects.get() + self.assertEquals(receipt.model_obj_type, ContentType.objects.get_for_model(Account)) + self.assertEquals(receipt.model_obj_id, account.id) + self.assertEquals(receipt.register_time, datetime(2013, 4, 12)) + def test_create_delete_one_obj(self): """ Tests creating one object to handle for deletion and then deleting it. diff --git a/dynamic_initial_data/tests/integration_tests.py b/dynamic_initial_data/tests/integration_tests.py index 645f911..8c13f62 100644 --- a/dynamic_initial_data/tests/integration_tests.py +++ b/dynamic_initial_data/tests/integration_tests.py @@ -31,6 +31,31 @@ def update_initial_data(self): # Verify an account object was created self.assertEquals(Account.objects.count(), 1) + @override_settings(INSTALLED_APPS=('one_installed_test_app',)) + def test_multiple_same_objects(self): + """ + Tests initial data when registering the same object for deletion twice. + """ + class AccountInitialData1(BaseInitialData): + """ + Initial data code that registers the same object many times for deletion + """ + def update_initial_data(self): + # Return the object from update_initial_data, thus registering it for deletion + account = Account.objects.get_or_create()[0] + return [account, account, account] + + # Verify no account objects exist + self.assertEquals(Account.objects.count(), 0) + + with patch.object(InitialDataUpdater, 'load_app', return_value=AccountInitialData1): + InitialDataUpdater().update_all_apps() + InitialDataUpdater().update_all_apps() + + # Verify an account object was created and is managed by a deletion receipt + self.assertEquals(Account.objects.count(), 1) + self.assertEquals(RegisteredForDeletionReceipt.objects.count(), 1) + @override_settings(INSTALLED_APPS=('one_installed_test_app',)) def test_handle_deletions_returned_from_update_initial_data(self): """ diff --git a/dynamic_initial_data/version.py b/dynamic_initial_data/version.py index fc79d63..0404d81 100644 --- a/dynamic_initial_data/version.py +++ b/dynamic_initial_data/version.py @@ -1 +1 @@ -__version__ = '0.2.1' +__version__ = '0.3.0'