From 5ed28fbe9629daf3c9d132d3016982e661e7c10d Mon Sep 17 00:00:00 2001 From: Joey Leingang Date: Thu, 31 Aug 2017 16:48:52 -0700 Subject: [PATCH 1/9] first pass on simple model state to keep track of clock ticks and activities --- temporal_sqlalchemy/bases.py | 120 ++++++++++++++++++++--------- temporal_sqlalchemy/clock.py | 5 +- tests/test_temporal_model_mixin.py | 10 +-- 3 files changed, 93 insertions(+), 42 deletions(-) diff --git a/temporal_sqlalchemy/bases.py b/temporal_sqlalchemy/bases.py index 19a5171..990a71a 100644 --- a/temporal_sqlalchemy/bases.py +++ b/temporal_sqlalchemy/bases.py @@ -8,19 +8,74 @@ import sqlalchemy as sa import sqlalchemy.dialects.postgresql as sap +import sqlalchemy.event as event import sqlalchemy.orm as orm import sqlalchemy.orm.attributes as attributes import psycopg2.extras as psql_extras from temporal_sqlalchemy import nine -from temporal_sqlalchemy.metadata import get_session_metadata -_ClockSet = collections.namedtuple('_ClockSet', ('effective', 'vclock')) +_PersistentClockPair = collections.namedtuple('_PersistentClockPairssssss', + ('effective', 'vclock')) T_PROPS = typing.TypeVar( 'T_PROP', orm.RelationshipProperty, orm.ColumnProperty) +class ActivityState: + def __set__(self, instance, value): + assert instance.temporal_options.activity_cls, "Make this better Joey" + setattr(instance, '_current_activity', value) + + if value: + current_tick = instance.current_tick + current_tick.activity = value + + def __get__(self, instance, owner): + if not instance: + return None + + return getattr(instance, '_current_activity') + + @staticmethod + def reset_activity(target, attr): + target.activity = None + + @staticmethod + def activity_required(target, key, value): + # TODO this doesn't work yet! + if not target.activity: + raise ValueError("activity required") + + +class ClockState: + + def __set__(self, instance, value): + setattr(instance, '_current_tick', value) + + def __get__(self, instance, owner): + if not instance: + return None + vclock = getattr(instance, 'vclock') or 0 + + if not getattr(instance, '_current_tick', None): + new_version = vclock + 1 + instance.vclock = new_version + clock_tick = instance.temporal_options.clock_model(tick=new_version) + instance.current_tick = clock_tick + instance.clock.append(clock_tick) + + return getattr(instance, '_current_tick') + + @staticmethod + def reset_tick(target, attr): + target.current_tick = None + + @staticmethod + def start_clock(target, args, kwargs): + kwargs.setdefault('vclock', target.current_tick.tick) + + class EntityClock(object): id = sa.Column(sap.UUID(as_uuid=True), default=uuid.uuid4, primary_key=True) tick = sa.Column(sa.Integer, nullable=False) @@ -50,11 +105,27 @@ def __init__( temporal_props: typing.Iterable[T_PROPS], clock_model: nine.Type[EntityClock], activity_cls: nine.Type[TemporalActivityMixin] = None): + self.history_models = history_models self.temporal_props = temporal_props self.clock_model = clock_model self.activity_cls = activity_cls + self.model = None + + def bind(self, model: 'Clocked'): + # TODO this method smells + self.model = model + + event.listen(model, 'expire', ClockState.reset_tick) + event.listen(model, 'init', ClockState.start_clock) + + if self.activity_cls: + # TODO fix this + orm.validates(*{prop.key for prop in self.temporal_props}, + include_removes=True)(ActivityState.activity_required) + + event.listen(model, 'expire', ActivityState.reset_activity) @property def clock_table(self): @@ -73,7 +144,7 @@ def history_tables(self): @staticmethod def make_clock(effective_lower: dt.datetime, vclock_lower: int, - **kwargs) -> _ClockSet: + **kwargs) -> _PersistentClockPair: """construct a clock set tuple""" effective_upper = kwargs.get('effective_upper', None) vclock_upper = kwargs.get('vclock_upper', None) @@ -82,7 +153,7 @@ def make_clock(effective_lower: dt.datetime, effective_lower, effective_upper) vclock = psql_extras.NumericRange(vclock_lower, vclock_upper) - return _ClockSet(effective, vclock) + return _PersistentClockPair(effective, vclock) def record_history(self, clocked: 'Clocked', @@ -90,17 +161,8 @@ def record_history(self, timestamp: dt.datetime): """record all history for a given clocked object""" state = attributes.instance_state(clocked) - vclock_history = attributes.get_history(clocked, 'vclock') - try: - new_tick = state.dict['vclock'] - except KeyError: - # TODO understand why this is necessary - new_tick = getattr(clocked, 'vclock') - is_strict_mode = get_session_metadata(session).get('strict_mode', False) - is_vclock_unchanged = vclock_history.unchanged and new_tick == vclock_history.unchanged[0] - - new_clock = self.make_clock(timestamp, new_tick) + new_clock = self.make_clock(timestamp, clocked.current_clock.tick) attr = {'entity': clocked} for prop, cls in self.history_models.items(): @@ -111,16 +173,13 @@ def record_history(self, if isinstance(prop, orm.RelationshipProperty): changes = attributes.get_history( - clocked, prop.key, + clocked, + prop.key, passive=attributes.PASSIVE_NO_INITIALIZE) else: changes = attributes.get_history(clocked, prop.key) if changes.added: - if is_strict_mode: - assert not is_vclock_unchanged, \ - 'flush() has triggered for a changed temporalized property outside of a clock tick' - # Cap previous history row if exists if sa.inspect(clocked).identity is not None: # but only if it already exists!! @@ -194,22 +253,13 @@ def date_modified(self): @contextlib.contextmanager def clock_tick(self, activity: TemporalActivityMixin = None): - warnings.warn("clock_tick is going away in 0.5.0", - PendingDeprecationWarning) - """Increments vclock by 1 with changes scoped to the session""" - if self.temporal_options.activity_cls is not None and activity is None: - raise ValueError("activity is missing on edit") from None - - session = orm.object_session(self) - with session.no_autoflush: - yield self + warnings.warn("clock_tick is deprecated, assign an activity directly", + DeprecationWarning) + self.activity = activity - if session.is_modified(self): - self.vclock += 1 + yield self - new_clock_tick = self.temporal_options.clock_model( - entity=self, tick=self.vclock) - if activity is not None: - new_clock_tick.activity = activity + return - session.add(new_clock_tick) + activity = ActivityState() + current_clock = ClockState() diff --git a/temporal_sqlalchemy/clock.py b/temporal_sqlalchemy/clock.py index 800b60b..a87d13d 100644 --- a/temporal_sqlalchemy/clock.py +++ b/temporal_sqlalchemy/clock.py @@ -12,6 +12,8 @@ from temporal_sqlalchemy import nine, util from temporal_sqlalchemy.bases import ( T_PROPS, + ClockState, + ActivityState, Clocked, TemporalOption, TemporalActivityMixin, @@ -94,8 +96,7 @@ def temporal_map(*track, mapper: orm.Mapper, activity_class=None, schema=None): clock_model=clock_model, activity_cls=activity_class ) - - event.listen(cls, 'init', init_clock) + cls.temporal_options.bind(cls) def init_clock(obj: Clocked, args, kwargs): diff --git a/tests/test_temporal_model_mixin.py b/tests/test_temporal_model_mixin.py index 6389523..018b0be 100644 --- a/tests/test_temporal_model_mixin.py +++ b/tests/test_temporal_model_mixin.py @@ -154,11 +154,11 @@ def test_clock_tick_editing(self, session, newstylemodel): session.commit() activity = models.Activity(description="Activity Description #2") - with newstylemodel.clock_tick(activity=activity): - newstylemodel.description = "this is new" - newstylemodel.int_prop = 2 - newstylemodel.bool_prop = False - newstylemodel.datetime_prop = datetime.datetime(2017, 2, 10) + newstylemodel.activity = activity + newstylemodel.description = "this is new" + newstylemodel.int_prop = 2 + newstylemodel.bool_prop = False + newstylemodel.datetime_prop = datetime.datetime(2017, 2, 10) session.commit() From f2c2229c4ea7f3bc81325694bd2847ccca483f91 Mon Sep 17 00:00:00 2001 From: Joey Leingang Date: Wed, 20 Sep 2017 18:10:23 -0700 Subject: [PATCH 2/9] cleanup to __temporal_ state management --- temporal_sqlalchemy/bases.py | 26 +++++++++++++----------- tests/test_temporal_models.py | 37 ----------------------------------- 2 files changed, 15 insertions(+), 48 deletions(-) diff --git a/temporal_sqlalchemy/bases.py b/temporal_sqlalchemy/bases.py index 990a71a..fbd0140 100644 --- a/temporal_sqlalchemy/bases.py +++ b/temporal_sqlalchemy/bases.py @@ -25,17 +25,18 @@ class ActivityState: def __set__(self, instance, value): assert instance.temporal_options.activity_cls, "Make this better Joey" - setattr(instance, '_current_activity', value) + # TODO should not be able to change activity once changes have been made to temporal properties + setattr(instance, '__temporal_current_activity', value) if value: - current_tick = instance.current_tick - current_tick.activity = value + current_clock = instance.current_clock + current_clock.activity = value def __get__(self, instance, owner): if not instance: return None - return getattr(instance, '_current_activity') + return getattr(instance, '__temporal_current_activity') @staticmethod def reset_activity(target, attr): @@ -51,29 +52,29 @@ def activity_required(target, key, value): class ClockState: def __set__(self, instance, value): - setattr(instance, '_current_tick', value) + setattr(instance, '__temporal_current_tick', value) def __get__(self, instance, owner): if not instance: return None vclock = getattr(instance, 'vclock') or 0 - if not getattr(instance, '_current_tick', None): + if not getattr(instance, '__temporal_current_tick', None): new_version = vclock + 1 instance.vclock = new_version clock_tick = instance.temporal_options.clock_model(tick=new_version) - instance.current_tick = clock_tick + setattr(instance, '__temporal_current_tick', clock_tick) instance.clock.append(clock_tick) - return getattr(instance, '_current_tick') + return getattr(instance, '__temporal_current_tick') @staticmethod def reset_tick(target, attr): - target.current_tick = None + target.current_clock = None @staticmethod def start_clock(target, args, kwargs): - kwargs.setdefault('vclock', target.current_tick.tick) + kwargs.setdefault('vclock', target.current_clock.tick) class EntityClock(object): @@ -255,7 +256,10 @@ def date_modified(self): def clock_tick(self, activity: TemporalActivityMixin = None): warnings.warn("clock_tick is deprecated, assign an activity directly", DeprecationWarning) - self.activity = activity + if self.temporal_options.activity_cls: + if not activity: + raise ValueError + self.activity = activity yield self diff --git a/tests/test_temporal_models.py b/tests/test_temporal_models.py index 97fecc0..22f9875 100644 --- a/tests/test_temporal_models.py +++ b/tests/test_temporal_models.py @@ -224,43 +224,6 @@ def test_multiple_edits(self, session): assert 3 in recorded_history.vclock assert getattr(t, attr) == getattr(recorded_history, attr) - def test_edit_on_double_wrapped(self, session): - double_wrapped_session = temporal.temporal_session(session) - - t = models.SimpleTableTemporal( - prop_a=1, - prop_b='foo', - prop_c=datetime.datetime(2016, 5, 11, 1, 2, 3, - tzinfo=datetime.timezone.utc), - prop_d={'foo': 'old value'}, - prop_e=psql_extras.DateRange(datetime.date(2016, 1, 1), - datetime.date(2016, 1, 10)), - prop_f=['old', 'stuff'] - ) - double_wrapped_session.add(t) - double_wrapped_session.commit() - - t = double_wrapped_session.query(models.SimpleTableTemporal).first() - with t.clock_tick(): - t.prop_a = 2 - t.prop_b = 'bar' - double_wrapped_session.commit() - - history_tables = { - 'prop_a': temporal.get_history_model( - models.SimpleTableTemporal.prop_a), - 'prop_b': temporal.get_history_model( - models.SimpleTableTemporal.prop_b), - } - for attr, history in history_tables.items(): - clock_query = session.query(history) - assert clock_query.count() == 2, \ - "%r missing a history entry for initial value" % history - - recorded_history = clock_query[-1] - assert 2 in recorded_history.vclock - assert getattr(t, attr) == getattr(recorded_history, attr) - def test_doesnt_duplicate_unnecessary_history(self, session): history_tables = { 'prop_a': temporal.get_history_model( From 6512d60fe5496fc37944c17f76396cf1dd43ab98 Mon Sep 17 00:00:00 2001 From: Joey Leingang Date: Wed, 4 Oct 2017 12:16:53 -0700 Subject: [PATCH 3/9] cleanup strict mode session stuff --- temporal_sqlalchemy/bases.py | 43 +++++---------- temporal_sqlalchemy/clock.py | 16 +++--- temporal_sqlalchemy/metadata.py | 24 -------- temporal_sqlalchemy/session.py | 43 ++++++++------- tests/test_temporal_models.py | 98 --------------------------------- 5 files changed, 45 insertions(+), 179 deletions(-) delete mode 100644 temporal_sqlalchemy/metadata.py diff --git a/temporal_sqlalchemy/bases.py b/temporal_sqlalchemy/bases.py index fbd0140..0841bef 100644 --- a/temporal_sqlalchemy/bases.py +++ b/temporal_sqlalchemy/bases.py @@ -8,14 +8,14 @@ import sqlalchemy as sa import sqlalchemy.dialects.postgresql as sap -import sqlalchemy.event as event import sqlalchemy.orm as orm +import sqlalchemy.orm.base as base import sqlalchemy.orm.attributes as attributes import psycopg2.extras as psql_extras from temporal_sqlalchemy import nine -_PersistentClockPair = collections.namedtuple('_PersistentClockPairssssss', +_PersistentClockPair = collections.namedtuple('_PersistentClockPairs', ('effective', 'vclock')) T_PROPS = typing.TypeVar( @@ -36,29 +36,29 @@ def __get__(self, instance, owner): if not instance: return None - return getattr(instance, '__temporal_current_activity') + return getattr(instance, '__temporal_current_activity', None) @staticmethod def reset_activity(target, attr): target.activity = None @staticmethod - def activity_required(target, key, value): - # TODO this doesn't work yet! - if not target.activity: + def activity_required(target, value, oldvalue, initiator): + if not target.activity and oldvalue is not base.NEVER_SET: raise ValueError("activity required") class ClockState: - - def __set__(self, instance, value): + def __set__(self, instance, value: 'EntityClock'): setattr(instance, '__temporal_current_tick', value) + if value: + instance.clock.append(value) def __get__(self, instance, owner): if not instance: return None + vclock = getattr(instance, 'vclock') or 0 - if not getattr(instance, '__temporal_current_tick', None): new_version = vclock + 1 instance.vclock = new_version @@ -72,10 +72,6 @@ def __get__(self, instance, owner): def reset_tick(target, attr): target.current_clock = None - @staticmethod - def start_clock(target, args, kwargs): - kwargs.setdefault('vclock', target.current_clock.tick) - class EntityClock(object): id = sa.Column(sap.UUID(as_uuid=True), default=uuid.uuid4, primary_key=True) @@ -114,20 +110,6 @@ def __init__( self.activity_cls = activity_cls self.model = None - def bind(self, model: 'Clocked'): - # TODO this method smells - self.model = model - - event.listen(model, 'expire', ClockState.reset_tick) - event.listen(model, 'init', ClockState.start_clock) - - if self.activity_cls: - # TODO fix this - orm.validates(*{prop.key for prop in self.temporal_props}, - include_removes=True)(ActivityState.activity_required) - - event.listen(model, 'expire', ActivityState.reset_activity) - @property def clock_table(self): warnings.warn( @@ -244,6 +226,10 @@ class Clocked(object): first_tick = None # type: EntityClock latest_tick = None # type: EntityClock + # temporal descriptors + current_clock = None # type: ClockState + activity = None # type: typing.Optional[ActivityState] + @property def date_created(self): return self.first_tick.timestamp @@ -264,6 +250,3 @@ def clock_tick(self, activity: TemporalActivityMixin = None): yield self return - - activity = ActivityState() - current_clock = ClockState() diff --git a/temporal_sqlalchemy/clock.py b/temporal_sqlalchemy/clock.py index a87d13d..8b95fdc 100644 --- a/temporal_sqlalchemy/clock.py +++ b/temporal_sqlalchemy/clock.py @@ -80,6 +80,10 @@ def temporal_map(*track, mapper: orm.Mapper, activity_class=None, schema=None): backref_name = '%s_clock' % entity_table.name clock_properties['activity'] = \ orm.relationship(lambda: activity_class, backref=backref_name) + cls.activity = ActivityState() + event.listen(cls, 'expire', ActivityState.reset_activity) + for prop in tracked_props: + event.listen(prop, 'set', ActivityState.activity_required) clock_model = build_clock_class(cls.__name__, entity_table.metadata, @@ -96,23 +100,19 @@ def temporal_map(*track, mapper: orm.Mapper, activity_class=None, schema=None): clock_model=clock_model, activity_cls=activity_class ) - cls.temporal_options.bind(cls) + cls.current_clock = ClockState() + event.listen(cls, 'expire', ClockState.reset_tick) + event.listen(cls, 'init', init_clock) def init_clock(obj: Clocked, args, kwargs): kwargs.setdefault('vclock', 1) - initial_tick = obj.temporal_options.clock_model( - tick=kwargs['vclock'], - entity=obj, - ) + obj.current_clock = obj.temporal_options.clock_model(tick=kwargs['vclock']) if obj.temporal_options.activity_cls and 'activity' not in kwargs: raise ValueError( "%r missing keyword argument: activity" % obj.__class__) - if 'activity' in kwargs: - initial_tick.activity = kwargs.pop('activity') - materialize_defaults(obj, kwargs) diff --git a/temporal_sqlalchemy/metadata.py b/temporal_sqlalchemy/metadata.py deleted file mode 100644 index eb01faa..0000000 --- a/temporal_sqlalchemy/metadata.py +++ /dev/null @@ -1,24 +0,0 @@ -import sqlalchemy.orm as orm - -TEMPORAL_METADATA_KEY = '__temporal' - -__all__ = [ - 'get_session_metadata', - 'set_session_metadata', -] - - -def set_session_metadata(session: orm.Session, metadata: dict): - if isinstance(session, orm.Session): - session.info[TEMPORAL_METADATA_KEY] = metadata - elif isinstance(session, orm.sessionmaker): - session.configure(info={TEMPORAL_METADATA_KEY: metadata}) - else: - raise ValueError('Invalid session') - - -def get_session_metadata(session: orm.Session) -> dict: - """ - :return: metadata dictionary, or None if it was never installed - """ - return session.info.get(TEMPORAL_METADATA_KEY) diff --git a/temporal_sqlalchemy/session.py b/temporal_sqlalchemy/session.py index 9e78b41..44adb8c 100644 --- a/temporal_sqlalchemy/session.py +++ b/temporal_sqlalchemy/session.py @@ -4,16 +4,25 @@ import sqlalchemy.event as event import sqlalchemy.orm as orm +import sqlalchemy.util as util from temporal_sqlalchemy.bases import TemporalOption, Clocked -from temporal_sqlalchemy.metadata import ( - get_session_metadata, - set_session_metadata -) -def _temporal_models(session: orm.Session) -> typing.Iterable[Clocked]: - for obj in session: +TEMPORAL_METADATA_KEY = '__temporal' + + +def set_session_metadata(session: orm.Session, metadata: dict): + if isinstance(session, orm.Session): + session.info[TEMPORAL_METADATA_KEY] = metadata + elif isinstance(session, orm.sessionmaker): + session.configure(info={TEMPORAL_METADATA_KEY: metadata}) + else: + raise ValueError('Invalid session') + + +def _temporal_models(iset: util.IdentitySet) -> typing.Iterable[Clocked]: + for obj in iset: if isinstance(getattr(obj, 'temporal_options', None), TemporalOption): yield obj @@ -27,29 +36,25 @@ def persist_history(session: orm.Session, flush_context, instances): obj.temporal_options.record_history(obj, session, correlate_timestamp) -def temporal_session(session: typing.Union[orm.Session, orm.sessionmaker], strict_mode=False) -> orm.Session: +def temporal_session(session: typing.Union[orm.Session, orm.sessionmaker], + **opt) -> orm.Session: """ Setup the session to track changes via temporal :param session: SQLAlchemy ORM session to temporalize - :param strict_mode: if True, will raise exceptions when improper flush() calls are made (default is False) :return: temporalized SQLALchemy ORM session """ - temporal_metadata = { - 'strict_mode': strict_mode - } - - # defer listening to the flush hook until after we update the metadata - install_flush_hook = not is_temporal_session(session) + if is_temporal_session(session): + return session + opt.setdefault('ENABLED', True) # TODO make this significant # update to the latest metadata - set_session_metadata(session, temporal_metadata) - - if install_flush_hook: - event.listen(session, 'before_flush', persist_history) + set_session_metadata(session, opt) + event.listen(session, 'before_flush', persist_history) return session def is_temporal_session(session: orm.Session) -> bool: - return isinstance(session, orm.Session) and get_session_metadata(session) is not None + return isinstance(session, orm.Session) and \ + TEMPORAL_METADATA_KEY in session.info diff --git a/tests/test_temporal_models.py b/tests/test_temporal_models.py index 22f9875..4067ffa 100644 --- a/tests/test_temporal_models.py +++ b/tests/test_temporal_models.py @@ -259,101 +259,3 @@ def test_doesnt_duplicate_unnecessary_history(self, session): recorded_history = clock_query.first() assert 1 in recorded_history.vclock assert getattr(t, attr) == getattr(recorded_history, attr) - - @pytest.mark.parametrize('session_func_name', ( - 'flush', - 'commit' - )) - def test_disallow_flushes_within_clock_ticks_when_strict(self, session, session_func_name): - session = temporal.temporal_session(session, strict_mode=True) - - t = models.SimpleTableTemporal( - prop_a=1, - prop_b='foo', - prop_c=datetime.datetime(2016, 5, 11, - tzinfo=datetime.timezone.utc)) - session.add(t) - session.commit() - - with t.clock_tick(): - t.prop_a = 2 - - with pytest.raises(AssertionError) as excinfo: - eval('session.{func_name}()'.format(func_name=session_func_name)) - - assert re.match( - r'.*flush\(\) has triggered for a changed temporalized property outside of a clock tick.*', - str(excinfo) - ) - - - @pytest.mark.parametrize('session_func_name', ( - 'flush', - 'commit' - )) - def test_allow_flushes_within_clock_ticks_when_strict_but_no_change(self, session, session_func_name): - session = temporal.temporal_session(session, strict_mode=True) - - t = models.SimpleTableTemporal( - prop_a=1, - prop_b='foo', - prop_c=datetime.datetime(2016, 5, 11, - tzinfo=datetime.timezone.utc)) - session.add(t) - session.commit() - - with t.clock_tick(): - t.prop_a = 1 - - eval('session.{func_name}()'.format(func_name=session_func_name)) - - - @pytest.mark.parametrize('session_func_name', ( - 'flush', - 'commit' - )) - def test_disallow_flushes_on_changes_without_clock_ticks_when_strict(self, session, session_func_name): - session = temporal.temporal_session(session, strict_mode=True) - - t = models.SimpleTableTemporal( - prop_a=1, - prop_b='foo', - prop_c=datetime.datetime(2016, 5, 11, - tzinfo=datetime.timezone.utc)) - session.add(t) - session.commit() - - # this change should have been done within a clock tick - t.prop_a = 2 - - with pytest.raises(AssertionError) as excinfo: - eval('session.{func_name}()'.format(func_name=session_func_name)) - - assert re.match( - r'.*flush\(\) has triggered for a changed temporalized property outside of a clock tick.*', - str(excinfo) - ) - - # TODO this test should be removed once strict flush() checking becomes the default behavior - @pytest.mark.parametrize('session_func_name', ( - 'flush', - 'commit' - )) - def test_allow_loose_flushes_when_not_strict(self, session, session_func_name): - t = models.SimpleTableTemporal( - prop_a=1, - prop_b='foo', - prop_c=datetime.datetime(2016, 5, 11, - tzinfo=datetime.timezone.utc)) - session.add(t) - session.commit() - - with t.clock_tick(): - t.prop_a = 2 - - # this should succeed in non-strict mode - eval('session.{func_name}()'.format(func_name=session_func_name)) - - # this should also succeed in non-strict mode - t.prop_a = 3 - eval('session.{func_name}()'.format(func_name=session_func_name)) From 3df510dbd854febe3660fc819735f931a6682dbd Mon Sep 17 00:00:00 2001 From: Joey Leingang Date: Wed, 4 Oct 2017 16:00:43 -0700 Subject: [PATCH 4/9] make them tests pass --- temporal_sqlalchemy/bases.py | 8 ++++---- tests/test_concrete_base.py | 1 - 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/temporal_sqlalchemy/bases.py b/temporal_sqlalchemy/bases.py index 0841bef..875ff0d 100644 --- a/temporal_sqlalchemy/bases.py +++ b/temporal_sqlalchemy/bases.py @@ -57,7 +57,7 @@ def __set__(self, instance, value: 'EntityClock'): def __get__(self, instance, owner): if not instance: return None - + vclock = getattr(instance, 'vclock') or 0 if not getattr(instance, '__temporal_current_tick', None): new_version = vclock + 1 @@ -70,7 +70,8 @@ def __get__(self, instance, owner): @staticmethod def reset_tick(target, attr): - target.current_clock = None + if target: + target.current_clock = None class EntityClock(object): @@ -144,8 +145,6 @@ def record_history(self, timestamp: dt.datetime): """record all history for a given clocked object""" state = attributes.instance_state(clocked) - - new_clock = self.make_clock(timestamp, clocked.current_clock.tick) attr = {'entity': clocked} for prop, cls in self.history_models.items(): @@ -163,6 +162,7 @@ def record_history(self, changes = attributes.get_history(clocked, prop.key) if changes.added: + new_clock = self.make_clock(timestamp, clocked.current_clock.tick) # Cap previous history row if exists if sa.inspect(clocked).identity is not None: # but only if it already exists!! diff --git a/tests/test_concrete_base.py b/tests/test_concrete_base.py index b484833..cadbef1 100644 --- a/tests/test_concrete_base.py +++ b/tests/test_concrete_base.py @@ -260,7 +260,6 @@ def test_doesnt_duplicate_unnecessary_history(self, session): t.prop_a = 1 t.prop_c = datetime.datetime(2016, 5, 11, tzinfo=datetime.timezone.utc) - session.commit() assert t.vclock == 1 From 9afe85b9e320f76f908d6d534ce097bdf133091d Mon Sep 17 00:00:00 2001 From: Joey Leingang Date: Wed, 4 Oct 2017 16:50:04 -0700 Subject: [PATCH 5/9] descriptors return something meaningful --- temporal_sqlalchemy/bases.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/temporal_sqlalchemy/bases.py b/temporal_sqlalchemy/bases.py index 875ff0d..41cb011 100644 --- a/temporal_sqlalchemy/bases.py +++ b/temporal_sqlalchemy/bases.py @@ -34,7 +34,7 @@ def __set__(self, instance, value): def __get__(self, instance, owner): if not instance: - return None + return self return getattr(instance, '__temporal_current_activity', None) @@ -56,7 +56,7 @@ def __set__(self, instance, value: 'EntityClock'): def __get__(self, instance, owner): if not instance: - return None + return self vclock = getattr(instance, 'vclock') or 0 if not getattr(instance, '__temporal_current_tick', None): From d7e52ad0e39a3ff1bad7a0411f47ff1963feb7c3 Mon Sep 17 00:00:00 2001 From: Nicole Zuckerman Date: Wed, 25 Oct 2017 11:59:29 -0700 Subject: [PATCH 6/9] Rename atrribute that's reset for clarity --- temporal_sqlalchemy/bases.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/temporal_sqlalchemy/bases.py b/temporal_sqlalchemy/bases.py index 41cb011..f5e02cb 100644 --- a/temporal_sqlalchemy/bases.py +++ b/temporal_sqlalchemy/bases.py @@ -71,7 +71,7 @@ def __get__(self, instance, owner): @staticmethod def reset_tick(target, attr): if target: - target.current_clock = None + setattr(target, '__temporal_current_tick', None) class EntityClock(object): From c97443ea171ffbda906cbc400b07d1f91c52e1ba Mon Sep 17 00:00:00 2001 From: Nicole Zuckerman Date: Wed, 25 Oct 2017 12:00:12 -0700 Subject: [PATCH 7/9] Add tests for session/clock behavior & expiring tick and activity --- tests/test_session.py | 21 +++++++++++++++++++++ tests/test_temporal_with_activity.py | 13 +++++++++++++ 2 files changed, 34 insertions(+) diff --git a/tests/test_session.py b/tests/test_session.py index a255cc4..848b0b0 100644 --- a/tests/test_session.py +++ b/tests/test_session.py @@ -42,3 +42,24 @@ def test_is_temporal_session_on_raw_session(self, session, connection): assert not is_temporal_session(raw_session) finally: raw_session.close() + + def test_different_sessions_update_vclock(self, session, connection, sessionmaker, newstylemodel): + session.add(newstylemodel) + assert newstylemodel.vclock == 1 + session.commit() + + # create different session + transaction = connection.begin() + second_session = sessionmaker(bind=connection) + refreshed_model = second_session.query(models.NewStyleModel).first() + + # update row within new session + refreshed_model.activity = models.Activity(description="Activity Description") + refreshed_model.description = "a new str" + second_session.add(refreshed_model) + assert refreshed_model.vclock == 2 + second_session.commit() + + # clear out db + transaction.rollback() + second_session.close() diff --git a/tests/test_temporal_with_activity.py b/tests/test_temporal_with_activity.py index af33176..f5981cc 100644 --- a/tests/test_temporal_with_activity.py +++ b/tests/test_temporal_with_activity.py @@ -124,3 +124,16 @@ def test_activity_on_entity_edit_duplicate_activity(self, session): with t.clock_tick(create_activity): t.column = 4567 session.commit() + + def test_expire_clears_current_tick_and_activity(self, session): + create_activity = models.Activity(description='Create temp') + session.add(create_activity) + t = models.FirstTemporalWithActivity(column=1234, + activity=create_activity) + session.add(t) + + assert getattr(t, '__temporal_current_tick', None) + assert t.activity + session.commit() + assert not getattr(t, '__temporal_current_tick', None) + assert not t.activity From de145e4e226044c8049698ab83f22bba8920f907 Mon Sep 17 00:00:00 2001 From: Nicole Zuckerman Date: Mon, 30 Oct 2017 13:37:15 -0700 Subject: [PATCH 8/9] Add some more asserts to session & activity tests --- tests/test_session.py | 4 ++++ tests/test_temporal_with_activity.py | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/test_session.py b/tests/test_session.py index 848b0b0..d5eb62e 100644 --- a/tests/test_session.py +++ b/tests/test_session.py @@ -60,6 +60,10 @@ def test_different_sessions_update_vclock(self, session, connection, sessionmake assert refreshed_model.vclock == 2 second_session.commit() + # see vclock is still 2 after second session commits + refreshed_model = second_session.query(models.NewStyleModel).filter_by( + id=newstylemodel.id).first() + assert refreshed_model.vclock == 2 # clear out db transaction.rollback() second_session.close() diff --git a/tests/test_temporal_with_activity.py b/tests/test_temporal_with_activity.py index f5981cc..413bdf9 100644 --- a/tests/test_temporal_with_activity.py +++ b/tests/test_temporal_with_activity.py @@ -132,7 +132,9 @@ def test_expire_clears_current_tick_and_activity(self, session): activity=create_activity) session.add(t) - assert getattr(t, '__temporal_current_tick', None) + clock_model_instance = getattr(t, '__temporal_current_tick', None) + assert clock_model_instance + assert isinstance(clock_model_instance, models.FirstTemporalWithActivity.temporal_options.clock_model) assert t.activity session.commit() assert not getattr(t, '__temporal_current_tick', None) From e06df104f9e5724217f7d1751737e8417f4ec570 Mon Sep 17 00:00:00 2001 From: Joey Leingang Date: Fri, 3 Nov 2017 14:25:21 -0400 Subject: [PATCH 9/9] some polish on bases.py -- better error messages, and more concise context manager --- temporal_sqlalchemy/bases.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/temporal_sqlalchemy/bases.py b/temporal_sqlalchemy/bases.py index f5e02cb..a249107 100644 --- a/temporal_sqlalchemy/bases.py +++ b/temporal_sqlalchemy/bases.py @@ -24,8 +24,14 @@ class ActivityState: def __set__(self, instance, value): - assert instance.temporal_options.activity_cls, "Make this better Joey" - # TODO should not be able to change activity once changes have been made to temporal properties + if not instance.temporal_options.activity_cls: + raise ValueError( + "Can't set activity state on instance of %r " + "because the activity class is None." + % type(instance).__name__) + + # TODO should not be able to change activity once changes have + # TODO been made to temporal properties setattr(instance, '__temporal_current_activity', value) if value: @@ -58,7 +64,7 @@ def __get__(self, instance, owner): if not instance: return self - vclock = getattr(instance, 'vclock') or 0 + vclock = getattr(instance, 'vclock', 0) or 0 # start at 0 if None if not getattr(instance, '__temporal_current_tick', None): new_version = vclock + 1 instance.vclock = new_version @@ -244,9 +250,7 @@ def clock_tick(self, activity: TemporalActivityMixin = None): DeprecationWarning) if self.temporal_options.activity_cls: if not activity: - raise ValueError + raise ValueError("activity is missing on edit") from None self.activity = activity yield self - - return