- Fixed bug where attribute "set" events or columns with

``@validates`` would have events triggered within the flush process,
when those columns were the targets of a "fetch and populate"
operation, such as an autoincremented primary key, a Python side
default, or a server-side default "eagerly" fetched via RETURNING.
fixes #3167

diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst
index fb639ddf73e13894e799b8d035eeff2389feb228..1cbbec3b3b194b90ce7f1569a55a916612bb603c 100644 (file)
--- a/doc/build/changelog/changelog_10.rst
+++ b/doc/build/changelog/changelog_10.rst
@@ -16,6 +16,16 @@
 .. changelog::
        :version: 1.0.0
 
+    .. change::
+        :tags: bug, orm
+        :tickets: 3167
+
+        Fixed bug where attribute "set" events or columns with
+        ``@validates`` would have events triggered within the flush process,
+        when those columns were the targets of a "fetch and populate"
+        operation, such as an autoincremented primary key, a Python side
+        default, or a server-side default "eagerly" fetched via RETURNING.
+
     .. change::
         :tags: bug, orm, py3k
 

diff --git a/doc/build/orm/mapper_config.rst b/doc/build/orm/mapper_config.rst
index 9139b53f0ff827259d65cc559fbb40eb05262035..d0679c7212dfde76d3c2717ae2b50dc6597cb807 100644 (file)
--- a/doc/build/orm/mapper_config.rst
+++ b/doc/build/orm/mapper_config.rst
@@ -667,6 +667,12 @@ issued when the ORM is populating the object::
             assert '@' in address
             return address
 
+.. versionchanged:: 1.0.0 - validators are no longer triggered within
+   the flush process when the newly fetched values for primary key
+   columns as well as some python- or server-side defaults are fetched.
+   Prior to 1.0, validators may be triggered in those cases as well.
+
+
 Validators also receive collection append events, when items are added to a
 collection::
 

diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index fc15769cd48d8c8fa0159ea798bf67f09cb0dd28..1e129185705a5fd5d9c1950b61e8e2463391bec5 100644 (file)
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -1189,14 +1189,6 @@ class Mapper(InspectionAttr):
                 util.ordered_column_set(t.c).\
                 intersection(all_cols)
 
-        # determine cols that aren't expressed within our tables; mark these
-        # as "read only" properties which are refreshed upon INSERT/UPDATE
-        self._readonly_props = set(
-            self._columntoproperty[col]
-            for col in self._columntoproperty
-            if not hasattr(col, 'table') or
-            col.table not in self._cols_by_table)
-
         # if explicit PK argument sent, add those columns to the
         # primary key mappings
         if self._primary_key_argument:
@@ -1247,6 +1239,15 @@ class Mapper(InspectionAttr):
             self.primary_key = tuple(primary_key)
             self._log("Identified primary key columns: %s", primary_key)
 
+        # determine cols that aren't expressed within our tables; mark these
+        # as "read only" properties which are refreshed upon INSERT/UPDATE
+        self._readonly_props = set(
+            self._columntoproperty[col]
+            for col in self._columntoproperty
+            if self._columntoproperty[col] not in self._primary_key_props and
+            (not hasattr(col, 'table') or
+                col.table not in self._cols_by_table))
+
     def _configure_properties(self):
 
         # Column and other ClauseElement objects which are mapped
@@ -2342,18 +2343,26 @@ class Mapper(InspectionAttr):
         dict_ = state.dict
         manager = state.manager
         return [
-            manager[self._columntoproperty[col].key].
+            manager[prop.key].
             impl.get(state, dict_,
                      attributes.PASSIVE_RETURN_NEVER_SET)
-            for col in self.primary_key
+            for prop in self._primary_key_props
         ]
 
+    @_memoized_configured_property
+    def _primary_key_props(self):
+        return [self._columntoproperty[col] for col in self.primary_key]
+
     def _get_state_attr_by_column(
             self, state, dict_, column,
             passive=attributes.PASSIVE_RETURN_NEVER_SET):
         prop = self._columntoproperty[column]
         return state.manager[prop.key].impl.get(state, dict_, passive=passive)
 
+    def _set_committed_state_attr_by_column(self, state, dict_, column, value):
+        prop = self._columntoproperty[column]
+        state.manager[prop.key].impl.set_committed_value(state, dict_, value)
+
     def _set_state_attr_by_column(self, state, dict_, column, value):
         prop = self._columntoproperty[column]
         state.manager[prop.key].impl.set(state, dict_, value, None)

diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py
index 8c9b677feaf522bae7fbf02bbfb7279bc129bb55..d511c0816c95b06129e31bc796c3e107b4b80149 100644 (file)
--- a/lib/sqlalchemy/orm/persistence.py
+++ b/lib/sqlalchemy/orm/persistence.py
@@ -645,13 +645,7 @@ def _emit_insert_statements(base_mapper, uowtransaction,
                                        mapper._pks_by_table[table]):
                         prop = mapper_rec._columntoproperty[col]
                         if state_dict.get(prop.key) is None:
-                            # TODO: would rather say:
-                            # state_dict[prop.key] = pk
-                            mapper_rec._set_state_attr_by_column(
-                                state,
-                                state_dict,
-                                col, pk)
-
+                            state_dict[prop.key] = pk
                 _postfetch(
                     mapper_rec,
                     uowtransaction,
@@ -836,11 +830,11 @@ def _postfetch(mapper, uowtransaction, table,
             for col in returning_cols:
                 if col.primary_key:
                     continue
-                mapper._set_state_attr_by_column(state, dict_, col, row[col])
+                dict_[mapper._columntoproperty[col].key] = row[col]
 
     for c in prefetch_cols:
         if c.key in params and c in mapper._columntoproperty:
-            mapper._set_state_attr_by_column(state, dict_, c, params[c.key])
+            dict_[mapper._columntoproperty[c].key] = params[c.key]
 
     if postfetch_cols:
         state._expire_attributes(state.dict,

diff --git a/test/orm/test_unitofworkv2.py b/test/orm/test_unitofworkv2.py
index c643e6a870b4a1d4ffe9c1ecb16ef0cb2cea2b02..122fe2514bf20f5975eda2d9fc13ffe3c475b973 100644 (file)
--- a/test/orm/test_unitofworkv2.py
+++ b/test/orm/test_unitofworkv2.py
@@ -9,8 +9,9 @@ from sqlalchemy import Integer, String, ForeignKey, func
 from sqlalchemy.orm import mapper, relationship, backref, \
     create_session, unitofwork, attributes,\
     Session, exc as orm_exc
-
+from sqlalchemy.testing.mock import Mock
 from sqlalchemy.testing.assertsql import AllOf, CompiledSQL
+from sqlalchemy import event
 
 
 class AssertsUOW(object):
@@ -1703,3 +1704,46 @@ class LoadersUsingCommittedTest(UOWTest):
             sess.flush()
         except AvoidReferencialError:
             pass
+
+
+class NoAttrEventInFlushTest(fixtures.MappedTest):
+    """test [ticket:3167]"""
+
+    __backend__ = True
+
+    @classmethod
+    def define_tables(cls, metadata):
+        Table(
+            'test', metadata,
+            Column('id', Integer, primary_key=True,
+                   test_needs_autoincrement=True),
+            Column('prefetch_val', Integer, default=5),
+            Column('returning_val', Integer, server_default="5")
+        )
+
+    @classmethod
+    def setup_classes(cls):
+        class Thing(cls.Basic):
+            pass
+
+    @classmethod
+    def setup_mappers(cls):
+        Thing = cls.classes.Thing
+
+        mapper(Thing, cls.tables.test, eager_defaults=True)
+
+    def test_no_attr_events_flush(self):
+        Thing = self.classes.Thing
+        mock = Mock()
+        event.listen(Thing.id, "set", mock.id)
+        event.listen(Thing.prefetch_val, "set", mock.prefetch_val)
+        event.listen(Thing.returning_val, "set", mock.prefetch_val)
+        t1 = Thing()
+        s = Session()
+        s.add(t1)
+        s.flush()
+
+        eq_(len(mock.mock_calls), 0)
+        eq_(t1.id, 1)
+        eq_(t1.prefetch_val, 5)
+        eq_(t1.returning_val, 5)