Merge pull request #26 from betagouv/activate-without-commit

remove automatic trip of value at modify time

sqlalchemy_api_handler/__init__.py

@@ -1,4 +1,4 @@
 from sqlalchemy_api_handler.api_errors import *
 from sqlalchemy_api_handler.api_handler import *
 
-__version__ = '0.13.0'
+__version__ = '0.14.1'

sqlalchemy_api_handler/bases/activator.py → sqlalchemy_api_handler/bases/activate.py

@@ -6,31 +6,24 @@
 from sqlalchemy_api_handler.bases.accessor import Accessor
 from sqlalchemy_api_handler.bases.errors import ActivityError
 from sqlalchemy_api_handler.bases.save import Save
-from sqlalchemy_api_handler.utils.datum import relationships_in
+from sqlalchemy_api_handler.utils.datum import merged_datum_from_activities, \
+                                               relationships_in
 
 
-def merged_datum_from_activities(activities,
-                                 model,
-                                 initial=None):
-    return reduce(lambda agg, activity: {**agg, **relationships_in(activity.patch, model)},
-                  activities,
-                  relationships_in(initial, model) if initial else {})
-
-
-class Activator(Save):
+class Activate(Save):
 
     @classmethod
     def get_activity(cls):
-        return Activator.activity_cls
+        return Activate.activity_cls
 
     @classmethod
     def set_activity(cls, activity_cls):
-        Activator.activity_cls = activity_cls
+        Activate.activity_cls = activity_cls
 
     @staticmethod
     def activate(*activities,
                  with_check_not_soft_deleted=True):
-        Activity = Activator.get_activity()
+        Activity = Activate.get_activity()
         for (entity_identifier, grouped_activities) in groupby(activities, key=lambda activity: activity.entityIdentifier):
             grouped_activities = sorted(grouped_activities,
                                         key=lambda activity: activity.dateCreated)
@@ -45,10 +38,9 @@ def activate(*activities,
                 entity = query.one()
                 entity_id = entity.id
                 query.delete()
-                Activator.get_db().session.commit()
                 delete_activity = entity.__deleteActivity__
                 delete_activity.dateCreated = first_activity.dateCreated
-                Save.save(delete_activity)
+                Save.add(delete_activity)
 
                 # want to make as if first_activity was the delete_activity one
                 # for such route like operations
@@ -60,7 +52,7 @@ def activate(*activities,
                 if delete_activity.transaction:
                     first_activity.transaction = Activity.transaction.mapper.class_()
                     first_activity.transaction.actor  = delete_activity.transaction.actor
-                Activator.activate(*grouped_activities[1:],
+                Activate.activate(*grouped_activities[1:],
                                    with_check_not_soft_deleted=with_check_not_soft_deleted)
                 continue
 
@@ -74,10 +66,11 @@ def activate(*activities,
             if not entity_id:
                 entity = model(**relationships_in(first_activity.patch, model))
                 entity.activityIdentifier = entity_identifier
-                Activator.save(entity)
+                Save.add(entity)
+                Activate.get_db().session.flush()
                 insert_activity = entity.__insertActivity__
                 insert_activity.dateCreated = first_activity.dateCreated
-                Save.save(insert_activity)
+                Save.add(insert_activity)
                 # want to make as if first_activity was the insert_activity one
                 # very useful for the routes operation
                 # '''
@@ -89,7 +82,7 @@ def activate(*activities,
                 if insert_activity.transaction:
                     first_activity.transaction = Activity.transaction.mapper.class_()
                     first_activity.transaction.actor  = insert_activity.transaction.actor
-                Activator.activate(*grouped_activities[1:],
+                Activate.activate(*grouped_activities[1:],
                                    with_check_not_soft_deleted=with_check_not_soft_deleted)
                 continue
 
@@ -104,30 +97,18 @@ def activate(*activities,
                                                    key=lambda activity: activity.dateCreated)
 
             entity = model.query.get(entity_id)
-            before_data = all_activities_since_min_date[0].old_data \
-                           or entity.just_before_activity_from(all_activities_since_min_date[0]).data
-            merged_datum = {}
-            for activity in all_activities_since_min_date:
-                activity.old_data = before_data
-                activity.verb = 'update'
-                before_data = { **before_data,
-                                 **activity.changed_data }
-                merged_datum = { **merged_datum,
-                                 **relationships_in(activity.patch, model) }
-
-
-            if model.id.key in merged_datum:
-                del merged_datum[model.id.key]
+            merged_datum = merged_datum_from_activities(entity, all_activities_since_min_date)
 
-            db = Activator.get_db()
+            db = Activate.get_db()
             db.session.add_all(grouped_activities)
             db.session.execute(f'ALTER TABLE {model.__tablename__} DISABLE TRIGGER audit_trigger_update;')
+            if model.id.key in merged_datum:
+                del merged_datum[model.id.key]
             entity.modify(merged_datum,
                           with_add=True,
                           with_check_not_soft_deleted=with_check_not_soft_deleted)
             db.session.flush()
             db.session.execute(f'ALTER TABLE {model.__tablename__} ENABLE TRIGGER audit_trigger_update;')
-            db.session.commit()
 
 
     @classmethod
@@ -154,11 +135,11 @@ def downgrade(op):
 
     def upgrade(op):
         from sqlalchemy_api_handler.mixins.activity_mixin import ActivityMixin
-        db = Activator.get_db()
+        db = Activate.get_db()
         versioning_manager.init(db.Model)
         versioning_manager.transaction_cls.__table__.create(op.get_bind())
         class Activity(ActivityMixin,
-                       Activator,
+                       Activate,
                        versioning_manager.activity_cls):
             __table_args__ = {'extend_existing': True}
 

sqlalchemy_api_handler/bases/errors.py

@@ -118,6 +118,10 @@ class EmptyFilterError(ApiErrors):
     pass
 
 
+class IdNoneError(ApiErrors):
+    pass
+
+
 class ForbiddenError(ApiErrors):
     pass
 

sqlalchemy_api_handler/bases/modify.py

@@ -301,10 +301,10 @@ def _try_to_set_attribute(self, column, key, value):
                 self._try_to_set_attribute_with_decimal_value(column, key, value, 'integer')
             elif isinstance(column.type, (Float, Numeric)):
                 self._try_to_set_attribute_with_decimal_value(column, key, value, 'float')
-            elif isinstance(column.type, String):
-                setattr(self, key, value.strip() if value else value)
             elif isinstance(column.type, DateTime):
                 self._try_to_set_attribute_with_deserialized_datetime(column, key, value)
+            elif isinstance(column.type, String):
+                setattr(self, key, value)
             elif isinstance(column.type, UUID):
                 self._try_to_set_attribute_with_uuid(column, key, value)
         elif not isinstance(value, datetime) and isinstance(column.type, DateTime):

sqlalchemy_api_handler/bases/tasker.py

@@ -5,19 +5,19 @@
 from sqlalchemy.dialects.postgresql import JSON, UUID
 from sqlalchemy.sql import func
 
-from sqlalchemy_api_handler.bases.activator import Activator
+from sqlalchemy_api_handler.bases.activate import Activate
 from sqlalchemy_api_handler.mixins.task_mixin import TaskState
 from sqlalchemy_api_handler.utils.date import strptime
 
 
-class Tasker(Activator):
+class Tasker(Activate):
 
     @classmethod
     def set_celery(cls, celery_app, flask_app):
         module = sys.modules['celery.signals']
-        Task = Activator.model_from_name('Task')
+        Task = Activate.model_from_name('Task')
         BaseTask = celery_app.Task
-        db = Activator.get_db()
+        db = Activate.get_db()
         task_db_session = sys.modules['flask_sqlalchemy'].SQLAlchemy().session
 
         class AppTask(BaseTask):

sqlalchemy_api_handler/mixins/has_activities_mixin.py

@@ -3,7 +3,8 @@
                        Column, \
                        desc
 from sqlalchemy.dialects.postgresql import UUID
-from sqlalchemy_api_handler.bases.activator import Activator
+from sqlalchemy_api_handler.bases.activate import Activate
+from sqlalchemy_api_handler.bases.errors import IdNoneError
 from sqlalchemy.orm.collections import InstrumentedList
 
 
@@ -15,20 +16,25 @@ class HasActivitiesMixin(object):
                                      index=True)
 
     def _get_activity_join_by_entity_id_filter(self):
-        Activity = Activator.get_activity()
+        Activity = Activate.get_activity()
         id_key = self.__class__.id.property.key
         id_value = getattr(self, id_key)
+        if id_value is None:
+            errors = IdNoneError()
+            errors.add_error('_get_activity_join_by_entity_id_filter',
+                             f'tried to filter with a None id value for a {self.__class__.__name__} entity')
+            raise errors
         return ((Activity.old_data[id_key].astext.cast(BigInteger) == id_value) | \
                 (Activity.changed_data[id_key].astext.cast(BigInteger) == id_value))
 
     def _get_activity_join_filter(self):
-        Activity = Activator.get_activity()
+        Activity = Activate.get_activity()
         return ((Activity.table_name == self.__tablename__) & \
                 (self._get_activity_join_by_entity_id_filter()))
 
     @property
     def __activities__(self):
-        Activity = Activator.get_activity()
+        Activity = Activate.get_activity()
         query_filter = self._get_activity_join_filter()
         return InstrumentedList(Activity.query.filter(query_filter) \
                                               .order_by(Activity.dateCreated) \
@@ -37,23 +43,21 @@ def __activities__(self):
 
     @property
     def __deleteActivity__(self):
-        Activity = Activator.get_activity()
-        query_filter = (
-            (self._get_activity_join_filter()) & \
-            (Activity.verb == 'delete')
-        )
+        Activity = Activate.get_activity()
+        query_filter = ((self._get_activity_join_filter()) & \
+                        (Activity.verb == 'delete'))
         return Activity.query.filter(query_filter).one()
 
     @property
     def __insertActivity__(self):
-        Activity = Activator.get_activity()
+        Activity = Activate.get_activity()
         query_filter = (self._get_activity_join_filter()) & \
                        (Activity.verb == 'insert')
         return Activity.query.filter(query_filter).one()
 
     @property
     def __lastActivity__(self):
-        Activity = Activator.get_activity()
+        Activity = Activate.get_activity()
         query_filter = (self._get_activity_join_filter()) & \
                        (Activity.verb == 'update')
         return Activity.query.filter(query_filter) \
@@ -63,7 +67,7 @@ def __lastActivity__(self):
 
 
     def just_before_activity_from(self, activity):
-        Activity = Activator.get_activity()
+        Activity = Activate.get_activity()
         query_filter = (self._get_activity_join_filter()) & \
                        (Activity.dateCreated < activity.dateCreated)
         return Activity.query.filter(query_filter) \

sqlalchemy_api_handler/utils/datum.py

@@ -111,3 +111,27 @@ def relationships_in(datum, model):
                                   .one()
             relationed_datum[key] = instance
     return relationed_datum
+
+
+
+def old_data_from(entity, activity):
+    if activity.verb == 'insert':
+        return activity.changed_data
+    if activity.old_data:
+        return activity.old_data
+    return entity.just_before_activity_from(activity).data
+
+
+def merged_datum_from_activities(entity,
+                                 activities,
+                                 initial=None):
+    merged_datum = {}
+    old_data = old_data_from(entity, activities[0])
+    for activity in activities:
+        activity.old_data = old_data
+        activity.verb = 'update'
+        old_data = { **old_data,
+                     **activity.changed_data }
+        merged_datum = { **merged_datum,
+                         **relationships_in(activity.patch, entity.__class__) }
+    return merged_datum

tests/bases/activator_test.py → tests/bases/activate_test.py

@@ -13,7 +13,7 @@
 from api.models.user import User
 
 
-class ActivatorTest:
+class ActivateTest:
     def test_models(self, app):
         # When
         models = ApiHandler.models()
@@ -45,6 +45,9 @@ def test_instance_an_activity(self, app):
         # Then
         assert activity.patch['offerId'] == offer.humanizedId
 
+
+
+
     @with_delete
     def test_create_offer_saves_an_insert_activity(self, app):
         # Given
@@ -144,6 +147,57 @@ def test_create_activity_on_not_existing_offer_saves_an_insert_activity(self, ap
         assert insert_offer_activity.datum['id'] == humanize(offer.id)
         assert insert_offer_activity.patch['id'] == humanize(offer.id)
 
+
+
+
+    @with_delete
+    def test_create_activity_on_not_existing_offers_saves_two_insert_activities(self, app):
+        # Given
+        offer1_activity_identifier = uuid4()
+        patch1 = { 'name': 'bar', 'type': 'foo' }
+        activity1 = Activity(dateCreated=datetime.utcnow(),
+                             entityIdentifier=offer1_activity_identifier,
+                             patch=patch1,
+                             tableName='offer')
+        offer2_activity_identifier = uuid4()
+        patch2 = { 'name': 'bor', 'type': 'fee' }
+        activity2 = Activity(dateCreated=datetime.utcnow(),
+                             entityIdentifier=offer2_activity_identifier,
+                             patch=patch2,
+                             tableName='offer')
+
+        # When
+        ApiHandler.activate(activity1, activity2)
+
+        # Then
+        all_activities = Activity.query.all()
+        assert len(all_activities) == 2
+
+        offer1 = Offer.query.filter_by(activityIdentifier=offer1_activity_identifier).one()
+        offer1_activities = offer1.__activities__
+        insert_offer1_activity = offer1_activities[0]
+        assert len(offer1_activities) == 1
+        assert insert_offer1_activity.entityIdentifier == offer1.activityIdentifier
+        assert insert_offer1_activity.verb == 'insert'
+        assert patch1.items() <= insert_offer1_activity.datum.items()
+        assert patch1.items() <= insert_offer1_activity.patch.items()
+        assert insert_offer1_activity.datum['id'] == humanize(offer1.id)
+        assert insert_offer1_activity.patch['id'] == humanize(offer1.id)
+
+        offer2 = Offer.query.filter_by(activityIdentifier=offer2_activity_identifier).one()
+        offer2_activities = offer2.__activities__
+        insert_offer2_activity = offer2_activities[0]
+        assert len(offer2_activities) == 1
+        assert insert_offer2_activity.entityIdentifier == offer2.activityIdentifier
+        assert insert_offer2_activity.verb == 'insert'
+        assert patch2.items() <= insert_offer2_activity.datum.items()
+        assert patch2.items() <= insert_offer2_activity.patch.items()
+        assert insert_offer2_activity.datum['id'] == humanize(offer2.id)
+        assert insert_offer2_activity.patch['id'] == humanize(offer2.id)
+
+
+
+
     @with_delete
     def test_create_activities_on_existing_offer_saves_update_activities(self, app):
         # Given

tests/bases/modify_test.py

@@ -32,24 +32,6 @@
 now = datetime.utcnow()
 
 class ModifyTest:
-    def test_user_string_fields_are_stripped_of_whitespace(self):
-        # Given
-        user_data = { 'email': '   test@example.com',
-                      'firstName': 'John   ',
-                      'lastName': None,
-                      'postalCode': '   93100   ',
-                      'publicName': '' }
-
-        # When
-        user = User(**user_data)
-
-        # Then
-        assert user.email == 'test@example.com'
-        assert user.firstName == 'John'
-        assert user.lastName == None
-        assert user.postalCode == '93100'
-        assert user.publicName == ''
-
     def test_for_sql_integer_value_with_string_raises_decimal_cast_error(self):
         # Given
         test_object = Foo()