Merge pull request #9 from ambitioninc/develop

fixed error related to handling deletions of multiple objects

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
@@ -148,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.
@@ -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):

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']

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')

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
@@ -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.
@@ -160,99 +175,101 @@ 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'))
-
-            # 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'))
+        self.assertEqual(MockInitialData, InitialDataUpdater().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, 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
+        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, 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
+        """
+        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', 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.
+        """
         # 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):
+            InitialDataUpdater().update_app('MockThree')
 
-            with self.assertRaises(InitialDataCircularDependency):
-                initial_data_manager.update_app('MockThree')
-
-        initial_data_manager = InitialDataUpdater()
+    def test_get_dependency_call_list_initial_data_missing(self):
+        """
+        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')

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):
         """

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):