diff options
| -rw-r--r-- | doc/build/changelog/changelog_10.rst | 13 | ||||
| -rw-r--r-- | doc/build/changelog/migration_10.rst | 12 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/attributes.py | 10 | ||||
| -rw-r--r-- | test/orm/test_attributes.py | 37 | ||||
| -rw-r--r-- | test/orm/test_inspect.py | 4 | ||||
| -rw-r--r-- | test/orm/test_relationships.py | 110 | ||||
| -rw-r--r-- | test/orm/test_unitofworkv2.py | 22 |
7 files changed, 144 insertions, 64 deletions
diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst index 9004f5681..862d4103a 100644 --- a/doc/build/changelog/changelog_10.rst +++ b/doc/build/changelog/changelog_10.rst @@ -35,14 +35,11 @@ Adjustment to attribute mechanics concerning when a value is implicitly initialized to None via first access; this action, which has always resulted in a population of the attribute, - now emits an attribute event just like any other attribute set - operation and generates the same kind of history as one. Additionally, - many mapper internal operations will no longer implicitly generate - these "None" values when various never-set attributes are checked. - These are subtle behavioral fixes to attribute mechanics which provide - a better solution to the problem of :ticket:`3060`, which also - involves recognition of attributes explicitly set to ``None`` - vs. attributes that were never set. + no longer does so; the None value is returned but the underlying + attribute receives no set event. This is consistent with how collections + work and allows attribute mechanics to behave more consistently; + in particular, getting an attribute with no value does not squash + the event that should proceed if the value is actually set to None. .. seealso:: diff --git a/doc/build/changelog/migration_10.rst b/doc/build/changelog/migration_10.rst index 1e5156dce..43830197e 100644 --- a/doc/build/changelog/migration_10.rst +++ b/doc/build/changelog/migration_10.rst @@ -62,13 +62,14 @@ attribute first instead:: The above means that the behavior of our "set" operation can be corrupted by the fact that the value was accessed via "get" earlier. In 1.0, this -inconsistency has been resolved:: +inconsistency has been resolved, by no longer actually setting anything +when the default "getter" is used. >>> obj = Foo() >>> obj.someattr None >>> inspect(obj).attrs.someattr.history - History(added=[None], unchanged=(), deleted=()) # 1.0 + History(added=(), unchanged=(), deleted=()) # 1.0 >>> obj.someattr = None >>> inspect(obj).attrs.someattr.history History(added=[None], unchanged=(), deleted=()) @@ -94,13 +95,6 @@ if the primary key attributes have no setting at all, whereas the value would be ``None`` before, it will now be the :data:`.orm.attributes.NEVER_SET` symbol, and no change to the object's state occurs. -Performance-wise, the change has the tradeoff that an attribute will need -to be considered in a unit of work flush process in more cases than before, if it has -been accessed and populated with a default value. However, performance -is improved in the case where the unit of work inspects a pending object for -an existing primary key value, as the state of the object won't change -in the common case that none was set. - :ticket:`3061` .. _behavioral_changes_core_10: diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py index df1f328b7..fecd74f30 100644 --- a/lib/sqlalchemy/orm/attributes.py +++ b/lib/sqlalchemy/orm/attributes.py @@ -556,15 +556,7 @@ class AttributeImpl(object): def initialize(self, state, dict_): """Initialize the given state's attribute with an empty value.""" - old = NEVER_SET - value = None - if self.dispatch.set: - value = self.fire_replace_event(state, dict_, - None, old, None) - state._modified_event(dict_, self, old) - - dict_[self.key] = value - return value + return None def get(self, state, dict_, passive=PASSIVE_OFF): """Retrieve a value from the given object. diff --git a/test/orm/test_attributes.py b/test/orm/test_attributes.py index ab9b9f5d5..59b0078ea 100644 --- a/test/orm/test_attributes.py +++ b/test/orm/test_attributes.py @@ -949,7 +949,7 @@ class GetNoValueTest(fixtures.ORMTest): attr.get(state, dict_, passive=attributes.PASSIVE_OFF), None ) - assert 'attr' in dict_ + assert 'attr' not in dict_ class UtilTest(fixtures.ORMTest): def test_helpers(self): @@ -2386,7 +2386,7 @@ class LazyloadHistoryTest(fixtures.TestBase): 'bar'), ((), (), ['hi'])) assert f.bar is None eq_(attributes.get_state_history(attributes.instance_state(f), - 'bar'), ([None], (), ['hi'])) + 'bar'), ((), (), ['hi'])) def test_scalar_via_lazyload_with_active(self): # TODO: break into individual tests @@ -2430,7 +2430,7 @@ class LazyloadHistoryTest(fixtures.TestBase): 'bar'), ((), (), ['hi'])) assert f.bar is None eq_(attributes.get_state_history(attributes.instance_state(f), - 'bar'), ([None], (), ['hi'])) + 'bar'), ((), (), ['hi'])) def test_scalar_object_via_lazyload(self): # TODO: break into individual tests @@ -2475,7 +2475,7 @@ class LazyloadHistoryTest(fixtures.TestBase): 'bar'), ((), (), [bar1])) assert f.bar is None eq_(attributes.get_state_history(attributes.instance_state(f), - 'bar'), ([None], (), [bar1])) + 'bar'), ((), (), [bar1])) class ListenerTest(fixtures.ORMTest): def test_receive_changes(self): @@ -2569,11 +2569,8 @@ class ListenerTest(fixtures.ORMTest): f1 = Foo() eq_(f1.bar, None) - eq_(canary.mock_calls, [ - call( - f1, None, attributes.NEVER_SET, - attributes.Event(Foo.bar.impl, attributes.OP_REPLACE)) - ]) + # reversal of approach in #3061 + eq_(canary.mock_calls, []) def test_none_init_object(self): canary = Mock() @@ -2586,11 +2583,23 @@ class ListenerTest(fixtures.ORMTest): f1 = Foo() eq_(f1.bar, None) - eq_(canary.mock_calls, [ - call( - f1, None, attributes.NEVER_SET, - attributes.Event(Foo.bar.impl, attributes.OP_REPLACE)) - ]) + # reversal of approach in #3061 + eq_(canary.mock_calls, []) + + def test_none_init_collection(self): + canary = Mock() + class Foo(object): + pass + instrumentation.register_class(Foo) + attributes.register_attribute(Foo, 'bar', useobject=True, uselist=True) + + event.listen(Foo.bar, "set", canary) + + f1 = Foo() + eq_(f1.bar, []) + # reversal of approach in #3061 + eq_(canary.mock_calls, []) + def test_propagate(self): classes = [None, None, None] diff --git a/test/orm/test_inspect.py b/test/orm/test_inspect.py index ee3fe213e..4b69c2a71 100644 --- a/test/orm/test_inspect.py +++ b/test/orm/test_inspect.py @@ -341,10 +341,10 @@ class TestORMInspection(_fixtures.FixtureTest): insp.attrs.id.value, None ) - # now the None is there + # nope, still not set eq_( insp.attrs.id.loaded_value, - None + NO_VALUE ) def test_instance_state_attr_passive_value_collection(self): diff --git a/test/orm/test_relationships.py b/test/orm/test_relationships.py index f2bcd2036..16abbb37b 100644 --- a/test/orm/test_relationships.py +++ b/test/orm/test_relationships.py @@ -230,6 +230,116 @@ class DependencyTwoParentTest(fixtures.MappedTest): session.delete(c) session.flush() +class M2ODontOverwriteFKTest(fixtures.MappedTest): + @classmethod + def define_tables(cls, metadata): + Table( + 'a', metadata, + Column('id', Integer, primary_key=True), + Column('bid', ForeignKey('b.id')) + ) + Table( + 'b', metadata, + Column('id', Integer, primary_key=True), + ) + + def _fixture(self, uselist=False): + a, b = self.tables.a, self.tables.b + + class A(fixtures.BasicEntity): + pass + class B(fixtures.BasicEntity): + pass + + + mapper(A, a, properties={ + 'b': relationship(B, uselist=uselist) + }) + mapper(B, b) + return A, B + + def test_joinedload_doesnt_produce_bogus_event(self): + A, B = self._fixture() + sess = Session() + + b1 = B() + sess.add(b1) + sess.flush() + + a1 = A() + sess.add(a1) + sess.commit() + + # test that was broken by #3060 + from sqlalchemy.orm import joinedload + a1 = sess.query(A).options(joinedload("b")).first() + a1.bid = b1.id + sess.flush() + + eq_(a1.bid, b1.id) + + def test_init_doesnt_produce_scalar_event(self): + A, B = self._fixture() + sess = Session() + + b1 = B() + sess.add(b1) + sess.flush() + + a1 = A() + assert a1.b is None + a1.bid = b1.id + sess.add(a1) + sess.flush() + assert a1.bid is not None + + def test_init_doesnt_produce_collection_event(self): + A, B = self._fixture(uselist=True) + sess = Session() + + b1 = B() + sess.add(b1) + sess.flush() + + a1 = A() + assert a1.b == [] + a1.bid = b1.id + sess.add(a1) + sess.flush() + assert a1.bid is not None + + def test_scalar_relationship_overrides_fk(self): + A, B = self._fixture() + sess = Session() + + b1 = B() + sess.add(b1) + sess.flush() + + a1 = A() + a1.bid = b1.id + a1.b = None + sess.add(a1) + sess.flush() + assert a1.bid is None + + def test_collection_relationship_overrides_fk(self): + A, B = self._fixture(uselist=True) + sess = Session() + + b1 = B() + sess.add(b1) + sess.flush() + + a1 = A() + a1.bid = b1.id + a1.b = [] + sess.add(a1) + sess.flush() + # this is weird + assert a1.bid is not None + + class DirectSelfRefFKTest(fixtures.MappedTest, AssertsCompiledSQL): """Tests the ultimate join condition, a single column diff --git a/test/orm/test_unitofworkv2.py b/test/orm/test_unitofworkv2.py index 787c104e7..9fedc9590 100644 --- a/test/orm/test_unitofworkv2.py +++ b/test/orm/test_unitofworkv2.py @@ -1314,7 +1314,6 @@ class RowswitchM2OTest(fixtures.MappedTest): mapper(C, c) return A, B, C - @testing.fails() def test_set_none_replaces_m2o(self): # we have to deal here with the fact that a # get of an unset attribute implicitly sets it to None @@ -1338,7 +1337,6 @@ class RowswitchM2OTest(fixtures.MappedTest): sess.commit() assert a1.bs[0].c is None - @testing.fails() def test_set_none_w_get_replaces_m2o(self): A, B, C = self._fixture() sess = Session() @@ -1391,26 +1389,6 @@ class RowswitchM2OTest(fixtures.MappedTest): sess.commit() assert a1.bs[0].data is None - def test_joinedload_doesnt_produce_bogus_event(self): - A, B, C = self._fixture() - sess = Session() - - c1 = C() - sess.add(c1) - sess.flush() - - b1 = B() - sess.add(b1) - sess.commit() - - # test that was broken by #3060 - from sqlalchemy.orm import joinedload - b1 = sess.query(B).options(joinedload("c")).first() - b1.cid = c1.id - sess.flush() - - assert b1.cid == c1.id - class BasicStaleChecksTest(fixtures.MappedTest): @classmethod |