rattail-project/rattail Changeset

Changeset - a580c0fca178

Parent rev.

Child rev.

[Not reviewed]

0 3 0

Lance Edgar (lance) - 11 years ago 2014-01-24 13:54:29
lance@edbob.org

Add "deletable orphan" awareness when recording changes.

Turns out there was a long-standing bug where orphans which were deleted
from the host would be marked as "changed" (instead of deleted), causing
the store databases to keep the orphan.

3 files changed with 158 insertions and 33 deletions:

rattail/db/changes.py

tests/db/test_changes.py

127

tests/db/test_model.py

0 comments (0 inline, 0 general)

rattail/db/changes.py

➞

Show inline comments

@@ @@ -81,8 +81,36 @@ class ChangeRecorder(object): @@
             self.record_change(session, instance)
         for instance in session.dirty:
             if session.is_modified(instance, passive=True):
                 log.debug("ChangeRecorder: found dirty instance: {0}".format(repr(instance)))
                 self.record_change(session, instance)
                 # Orphaned objects which really are pending deletion show up in
                 # session.dirty instead of session.deleted, hence this check.
                 # See also https://groups.google.com/d/msg/sqlalchemy/H4nQTHphc0M/Xr8-Cgra0Z4J
                 if self.is_deletable_orphan(instance):
                     log.debug("ChangeRecorder: found orphan pending deletion: {0}".format(repr(instance)))
                     self.record_change(session, instance, deleted=True)
                 else:
                     log.debug("ChangeRecorder: found dirty instance: {0}".format(repr(instance)))
                     self.record_change(session, instance)
     def is_deletable_orphan(self, instance):
         """
         Determine if an object is an orphan and pending deletion.
         """
         mapper = object_mapper(instance)
         for relationship in mapper.relationships.values():
             # Does this relationship refer back to the instance class?
             backref = relationship.backref or relationship.back_populates
             if backref:
                 # Does the other class mapper's relationship wish to delete orphans?
                 other_relationship = relationship.mapper.relationships[backref]
                 if other_relationship.cascade.delete_orphan:
                     # Is this instance an orphan?
                     if getattr(instance, relationship.key) is None:
                         return True
         return False
     def record_change(self, session, instance, deleted=False):
         """

tests/db/test_changes.py

➞

Show inline comments

@@ @@ -2,8 +2,10 @@ @@
 from unittest import TestCase
 from mock import patch, DEFAULT, Mock, MagicMock, call
 from . import DataTestCase
 from rattail.db import changes
-from rattail.db.model import Change, Batch, BatchColumn, BatchRow, Role, UserRole, Product
+from rattail.db import model
 from sqlalchemy.orm import RelationshipProperty
@@ @@ -33,52 +35,34 @@ class TestChangeRecorder(TestCase): @@
         recorder = changes.ChangeRecorder(False)
         self.assertFalse(recorder.ignore_role_changes)
     def test_call(self):
         recorder = changes.ChangeRecorder()
         recorder.record_change = Mock()
         session = MagicMock()
         session.deleted.__iter__.return_value = ['deleted_instance']
         session.new.__iter__.return_value = ['new_instance']
         session.dirty.__iter__.return_value = ['dirty_instance']
         session.is_modified.return_value = True
         recorder(session, Mock(), Mock())
         self.assertEqual(recorder.record_change.call_count, 3)
         recorder.record_change.assert_has_calls([
                 call(session, 'deleted_instance', deleted=True),
                 call(session, 'new_instance'),
                 call(session, 'dirty_instance'),
                 ])
     def test_record_change(self):
         session = Mock()
         recorder = changes.ChangeRecorder()
         recorder.ensure_uuid = Mock()
         # don't record changes for changes
         self.assertFalse(recorder.record_change(session, Change()))
+        self.assertFalse(recorder.record_change(session, model.Change()))
         # don't record changes for batch data
         self.assertFalse(recorder.record_change(session, Batch()))
         self.assertFalse(recorder.record_change(session, BatchColumn()))
         self.assertFalse(recorder.record_change(session, BatchRow()))
         self.assertFalse(recorder.record_change(session, model.Batch()))
         self.assertFalse(recorder.record_change(session, model.BatchColumn()))
         self.assertFalse(recorder.record_change(session, model.BatchRow()))
         # don't record changes for objects with no uuid attribute
         self.assertFalse(recorder.record_change(session, object()))
         # don't record changes for role data if so configured
         recorder.ignore_role_changes = True
         self.assertFalse(recorder.record_change(session, Role()))
         self.assertFalse(recorder.record_change(session, UserRole()))
         self.assertFalse(recorder.record_change(session, model.Role()))
         self.assertFalse(recorder.record_change(session, model.UserRole()))
         # none of the above should have involved a call to `ensure_uuid()`
         self.assertFalse(recorder.ensure_uuid.called)
         # make sure role data is *not* ignored if so configured
         recorder.ignore_role_changes = False
         self.assertTrue(recorder.record_change(session, Role()))
         self.assertTrue(recorder.record_change(session, UserRole()))
         self.assertTrue(recorder.record_change(session, model.Role()))
         self.assertTrue(recorder.record_change(session, model.UserRole()))
         # so far no *new* changes have been created
         self.assertFalse(session.add.called)
@@ @@ -86,7 +70,7 @@ class TestChangeRecorder(TestCase): @@
         # mock up session to force new change creation
         session.query.return_value = session
         session.get.return_value = None
         self.assertTrue(recorder.record_change(session, Product()))
+        self.assertTrue(recorder.record_change(session, model.Product()))
     @patch.multiple('rattail.db.changes', get_uuid=DEFAULT, object_mapper=DEFAULT)
     def test_ensure_uuid(self, get_uuid, object_mapper):
@@ @@ -95,7 +79,7 @@ class TestChangeRecorder(TestCase): @@
         object_mapper.return_value.columns.__getitem__.return_value = uuid_column
         # uuid already present
         product = Product(uuid='some_uuid')
+        product = model.Product(uuid='some_uuid')
         recorder.ensure_uuid(product)
         self.assertEqual(product.uuid, 'some_uuid')
         self.assertFalse(get_uuid.called)
@@ @@ -103,7 +87,7 @@ class TestChangeRecorder(TestCase): @@
         # no uuid yet, auto-generate
         uuid_column.foreign_keys = False
         get_uuid.return_value = 'another_uuid'
         product = Product()
+        product = model.Product()
         self.assertIsNone(product.uuid)
         recorder.ensure_uuid(product)
         get_uuid.assert_called_once_with()
@@ @@ -130,3 +114,116 @@ class TestChangeRecorder(TestCase): @@
         recorder.ensure_uuid(instance)
         get_uuid.assert_called_once_with()
         self.assertEqual(instance.uuid, 'fallback_uuid')
 class TestFunctionalChanges(DataTestCase):
     def setUp(self):
         super(TestFunctionalChanges, self).setUp()
         changes.record_changes(self.session)
     def test_add(self):
         product = model.Product()
         self.session.add(product)
         self.session.commit()
         self.assertEqual(self.session.query(model.Change).count(), 1)
         change = self.session.query(model.Change).one()
         self.assertEqual(change.class_name, 'Product')
         self.assertEqual(change.uuid, product.uuid)
         self.assertFalse(change.deleted)
     def test_change(self):
         product = model.Product()
         self.session.add(product)
         self.session.commit()
         self.assertEqual(self.session.query(model.Change).count(), 1)
         self.session.query(model.Change).delete()
         self.assertEqual(self.session.query(model.Change).count(), 0)
         product.description = 'Acme Bricks'
         self.session.commit()
         self.assertEqual(self.session.query(model.Change).count(), 1)
         change = self.session.query(model.Change).one()
         self.assertEqual(change.class_name, 'Product')
         self.assertEqual(change.uuid, product.uuid)
         self.assertFalse(change.deleted)
     def test_delete(self):
         product = model.Product()
         self.session.add(product)
         self.session.commit()
         self.assertEqual(self.session.query(model.Change).count(), 1)
         self.session.query(model.Change).delete()
         self.assertEqual(self.session.query(model.Change).count(), 0)
         self.session.delete(product)
         self.assertEqual(self.session.query(model.Change).count(), 1)
         change = self.session.query(model.Change).one()
         self.assertEqual(change.class_name, 'Product')
         self.assertEqual(change.uuid, product.uuid)
         self.assertTrue(change.deleted)
     def test_orphan_change(self):
         department = model.Department()
         subdepartment = model.Subdepartment()
         department.subdepartments.append(subdepartment)
         self.session.add(department)
         self.session.commit()
         self.assertEqual(self.session.query(model.Change).count(), 2)
         change = self.session.query(model.Change).filter_by(class_name='Department').one()
         self.assertFalse(change.deleted)
         change = self.session.query(model.Change).filter_by(class_name='Subdepartment').one()
         self.assertFalse(change.deleted)
         self.session.query(model.Change).delete()
         self.assertEqual(self.session.query(model.Change).count(), 0)
         # Creating an orphaned Subdepartment, which should be recorded as a
         # *change* due to the cascade rules in effect.
         department.subdepartments.remove(subdepartment)
         self.session.commit()
         self.assertEqual(self.session.query(model.Change).count(), 2)
         change = self.session.query(model.Change).filter_by(class_name='Department').one()
         self.assertFalse(change.deleted)
         change = self.session.query(model.Change).filter_by(class_name='Subdepartment').one()
         self.assertFalse(change.deleted)
         self.assertEqual(self.session.query(model.Subdepartment).count(), 1)
     def test_orphan_delete(self):
         customer = model.Customer()
         group = model.CustomerGroup()
         customer.groups.append(group)
         self.session.add(customer)
         self.session.commit()
         self.assertEqual(self.session.query(model.Change).count(), 3)
         change = self.session.query(model.Change).filter_by(class_name='Customer').one()
         self.assertFalse(change.deleted)
         change = self.session.query(model.Change).filter_by(class_name='CustomerGroup').one()
         self.assertFalse(change.deleted)
         change = self.session.query(model.Change).filter_by(class_name='CustomerGroupAssignment').one()
         self.assertFalse(change.deleted)
         self.session.query(model.Change).delete()
         self.assertEqual(self.session.query(model.Change).count(), 0)
         # Creating an orphaned CustomerGroupAssociation, which should be
         # recorded as a *deletion* due to the cascade rules in effect.  Note
         # that the CustomerGroup is not technically an orphan and in fact is
         # not even changed.
         customer.groups.remove(group)
         self.session.commit()
         self.assertEqual(self.session.query(model.Change).count(), 2)
         change = self.session.query(model.Change).filter_by(class_name='Customer').one()
         self.assertFalse(change.deleted)
         change = self.session.query(model.Change).filter_by(class_name='CustomerGroupAssignment').one()
         self.assertTrue(change.deleted)
         self.assertEqual(self.session.query(model.CustomerGroupAssignment).count(), 0)

tests/db/test_model.py

➞

Show inline comments

@@ @@ -224,7 +224,7 @@ class TestCustomerEmailAddress(DataTestCase): @@
         email_changes = changes.filter_by(class_name='CustomerEmailAddress')
         self.assertEqual(email_changes.count(), 2)
-        self.assertEqual([x.deleted for x in email_changes], [False, False])
+        self.assertEqual([x.deleted for x in email_changes], [True, True])
 class TestLabelProfile(DataTestCase):

0 comments (0 inline, 0 general)