Merge "Unify NO_VALUE and NEVER_SET"

This commit is contained in:
mike bayer
2019-05-25 14:28:27 +00:00
committed by Gerrit Code Review
6 changed files with 78 additions and 74 deletions
+9
View File
@@ -0,0 +1,9 @@
.. change::
:tags: bug, orm
:tickets: 4696
The internal attribute symbols NO_VALUE and NEVER_SET have been unified, as
there was no meaningful difference between these two symbols, other than a
few codepaths where they were differentiated in subtle and undocumented
ways, these have been fixed.
+32 -29
View File
@@ -28,7 +28,7 @@ from .base import instance_state
from .base import instance_str
from .base import LOAD_AGAINST_COMMITTED
from .base import manager_of_class
from .base import NEVER_SET
from .base import NEVER_SET # noqa
from .base import NO_AUTOFLUSH
from .base import NO_CHANGE # noqa
from .base import NO_RAISE
@@ -40,7 +40,7 @@ from .base import PASSIVE_NO_INITIALIZE
from .base import PASSIVE_NO_RESULT
from .base import PASSIVE_OFF
from .base import PASSIVE_ONLY_PERSISTENT
from .base import PASSIVE_RETURN_NEVER_SET
from .base import PASSIVE_RETURN_NO_VALUE
from .base import RELATED_OBJECT_OK # noqa
from .base import SQL_OK # noqa
from .base import state_str
@@ -677,7 +677,7 @@ class AttributeImpl(object):
key = self.key
if (
key not in state.committed_state
or state.committed_state[key] is NEVER_SET
or state.committed_state[key] is NO_VALUE
):
if not passive & CALLABLES_OK:
return PASSIVE_NO_RESULT
@@ -692,7 +692,7 @@ class AttributeImpl(object):
else:
value = ATTR_EMPTY
if value is PASSIVE_NO_RESULT or value is NEVER_SET:
if value is PASSIVE_NO_RESULT or value is NO_VALUE:
return value
elif value is ATTR_WAS_SET:
try:
@@ -708,7 +708,7 @@ class AttributeImpl(object):
return self.set_committed_value(state, dict_, value)
if not passive & INIT_OK:
return NEVER_SET
return NO_VALUE
else:
# Return a new, empty value
return self.initialize(state, dict_)
@@ -749,7 +749,7 @@ class AttributeImpl(object):
if self.key in state.committed_state:
value = state.committed_state[self.key]
if value in (NO_VALUE, NEVER_SET):
if value is NO_VALUE:
return None
else:
return value
@@ -782,7 +782,7 @@ class ScalarAttributeImpl(AttributeImpl):
def delete(self, state, dict_):
if self.dispatch._active_history:
old = self.get(state, dict_, PASSIVE_RETURN_NEVER_SET)
old = self.get(state, dict_, PASSIVE_RETURN_NO_VALUE)
else:
old = dict_.get(self.key, NO_VALUE)
@@ -802,6 +802,8 @@ class ScalarAttributeImpl(AttributeImpl):
def get_history(self, state, dict_, passive=PASSIVE_OFF):
if self.key in dict_:
return History.from_scalar_attribute(self, state, dict_[self.key])
elif self.key in state.committed_state:
return History.from_scalar_attribute(self, state, NO_VALUE)
else:
if passive & INIT_OK:
passive ^= INIT_OK
@@ -822,7 +824,7 @@ class ScalarAttributeImpl(AttributeImpl):
pop=False,
):
if self.dispatch._active_history:
old = self.get(state, dict_, PASSIVE_RETURN_NEVER_SET)
old = self.get(state, dict_, PASSIVE_RETURN_NO_VALUE)
else:
old = dict_.get(self.key, NO_VALUE)
@@ -920,7 +922,7 @@ class ScalarObjectAttributeImpl(ScalarAttributeImpl):
if (
current is not None
and current is not PASSIVE_NO_RESULT
and current is not NEVER_SET
and current is not NO_VALUE
):
ret = [(instance_state(current), current)]
else:
@@ -931,7 +933,7 @@ class ScalarObjectAttributeImpl(ScalarAttributeImpl):
if (
original is not None
and original is not PASSIVE_NO_RESULT
and original is not NEVER_SET
and original is not NO_VALUE
and original is not current
):
@@ -998,7 +1000,7 @@ class ScalarObjectAttributeImpl(ScalarAttributeImpl):
if previous is not value and previous not in (
None,
PASSIVE_NO_RESULT,
NEVER_SET,
NO_VALUE,
):
self.sethasparent(instance_state(previous), state, False)
@@ -1109,7 +1111,7 @@ class CollectionAttributeImpl(AttributeImpl):
if self.key in state.committed_state:
original = state.committed_state[self.key]
if original not in (NO_VALUE, NEVER_SET):
if original is not NO_VALUE:
current_states = [
((c is not None) and instance_state(c) or None, c)
for c in current
@@ -1142,7 +1144,7 @@ class CollectionAttributeImpl(AttributeImpl):
for fn in self.dispatch.append:
value = fn(state, value, initiator or self._append_token)
state._modified_event(dict_, self, NEVER_SET, True)
state._modified_event(dict_, self, NO_VALUE, True)
if self.trackparent and value is not None:
self.sethasparent(instance_state(value), state, True)
@@ -1158,7 +1160,7 @@ class CollectionAttributeImpl(AttributeImpl):
operations (even though set.pop is the one where it is really needed).
"""
state._modified_event(dict_, self, NEVER_SET, True)
state._modified_event(dict_, self, NO_VALUE, True)
def fire_remove_event(self, state, dict_, value, initiator):
if self.trackparent and value is not None:
@@ -1167,13 +1169,13 @@ class CollectionAttributeImpl(AttributeImpl):
for fn in self.dispatch.remove:
fn(state, value, initiator or self._remove_token)
state._modified_event(dict_, self, NEVER_SET, True)
state._modified_event(dict_, self, NO_VALUE, True)
def delete(self, state, dict_):
if self.key not in dict_:
return
state._modified_event(dict_, self, NEVER_SET, True)
state._modified_event(dict_, self, NO_VALUE, True)
collection = self.get_collection(state, state.dict)
collection.clear_with_event()
@@ -1386,7 +1388,7 @@ def backref_listeners(attribute, key, uselist):
if (
oldchild is not None
and oldchild is not PASSIVE_NO_RESULT
and oldchild is not NEVER_SET
and oldchild is not NO_VALUE
):
# With lazy=None, there's no guarantee that the full collection is
# present when updating via a backref.
@@ -1481,7 +1483,7 @@ def backref_listeners(attribute, key, uselist):
if (
child is not None
and child is not PASSIVE_NO_RESULT
and child is not NEVER_SET
and child is not NO_VALUE
):
child_state, child_dict = (
instance_state(child),
@@ -1549,9 +1551,7 @@ def backref_listeners(attribute, key, uselist):
_NO_HISTORY = util.symbol("NO_HISTORY")
_NO_STATE_SYMBOLS = frozenset(
[id(PASSIVE_NO_RESULT), id(NO_VALUE), id(NEVER_SET)]
)
_NO_STATE_SYMBOLS = frozenset([id(PASSIVE_NO_RESULT), id(NO_VALUE)])
History = util.namedtuple("History", ["added", "unchanged", "deleted"])
@@ -1637,12 +1637,15 @@ class History(History):
original = state.committed_state.get(attribute.key, _NO_HISTORY)
if original is _NO_HISTORY:
if current is NEVER_SET:
if current is NO_VALUE:
return cls((), (), ())
else:
return cls((), [current], ())
# don't let ClauseElement expressions here trip things up
elif attribute.is_equal(current, original) is True:
elif (
current is not NO_VALUE
and attribute.is_equal(current, original) is True
):
return cls((), [current], ())
else:
# current convention on native scalars is to not
@@ -1658,7 +1661,7 @@ class History(History):
current = None
else:
deleted = [original]
if current is NEVER_SET:
if current is NO_VALUE:
return cls((), (), deleted)
else:
return cls([current], (), deleted)
@@ -1668,11 +1671,11 @@ class History(History):
original = state.committed_state.get(attribute.key, _NO_HISTORY)
if original is _NO_HISTORY:
if current is NO_VALUE or current is NEVER_SET:
if current is NO_VALUE:
return cls((), (), ())
else:
return cls((), [current], ())
elif current is original and current is not NEVER_SET:
elif current is original and current is not NO_VALUE:
return cls((), [current], ())
else:
# current convention on related objects is to not
@@ -1688,7 +1691,7 @@ class History(History):
current = None
else:
deleted = [original]
if current is NO_VALUE or current is NEVER_SET:
if current is NO_VALUE:
return cls((), (), deleted)
else:
return cls([current], (), deleted)
@@ -1697,11 +1700,11 @@ class History(History):
def from_collection(cls, attribute, state, current):
original = state.committed_state.get(attribute.key, _NO_HISTORY)
if current is NO_VALUE or current is NEVER_SET:
if current is NO_VALUE:
return cls((), (), ())
current = getattr(current, "_sa_adapter")
if original in (NO_VALUE, NEVER_SET):
if original is NO_VALUE:
return cls(list(current), (), ())
elif original is _NO_HISTORY:
return cls((), list(current), ())
+9 -10
View File
@@ -45,13 +45,12 @@ NO_VALUE = util.symbol(
and flags indicated we were not to load it.
""",
)
NEVER_SET = NO_VALUE
"""
Synonymous with NO_VALUE
NEVER_SET = util.symbol(
"NEVER_SET",
"""Symbol which may be placed as the 'previous' value of an attribute
indicating that the attribute had not been assigned to previously.
""",
)
.. versionchanged:: 1.4 NEVER_SET was merged with NO_VALUE
"""
NO_CHANGE = util.symbol(
"NO_CHANGE",
@@ -126,15 +125,15 @@ PASSIVE_OFF = util.symbol(
RELATED_OBJECT_OK | NON_PERSISTENT_OK | INIT_OK | CALLABLES_OK | SQL_OK
),
)
PASSIVE_RETURN_NEVER_SET = util.symbol(
"PASSIVE_RETURN_NEVER_SET",
PASSIVE_RETURN_NO_VALUE = util.symbol(
"PASSIVE_RETURN_NO_VALUE",
"""PASSIVE_OFF ^ INIT_OK""",
canonical=PASSIVE_OFF ^ INIT_OK,
)
PASSIVE_NO_INITIALIZE = util.symbol(
"PASSIVE_NO_INITIALIZE",
"PASSIVE_RETURN_NEVER_SET ^ CALLABLES_OK",
canonical=PASSIVE_RETURN_NEVER_SET ^ CALLABLES_OK,
"PASSIVE_RETURN_NO_VALUE ^ CALLABLES_OK",
canonical=PASSIVE_RETURN_NO_VALUE ^ CALLABLES_OK,
)
PASSIVE_NO_FETCH = util.symbol(
"PASSIVE_NO_FETCH", "PASSIVE_OFF ^ SQL_OK", canonical=PASSIVE_OFF ^ SQL_OK
+3 -3
View File
@@ -2715,7 +2715,7 @@ class Mapper(InspectionAttr):
return self._identity_key_from_state(state, attributes.PASSIVE_OFF)
def _identity_key_from_state(
self, state, passive=attributes.PASSIVE_RETURN_NEVER_SET
self, state, passive=attributes.PASSIVE_RETURN_NO_VALUE
):
dict_ = state.dict
manager = state.manager
@@ -2769,7 +2769,7 @@ class Mapper(InspectionAttr):
return {prop.key for prop in self._all_pk_props}
def _get_state_attr_by_column(
self, state, dict_, column, passive=attributes.PASSIVE_RETURN_NEVER_SET
self, state, dict_, column, passive=attributes.PASSIVE_RETURN_NO_VALUE
):
prop = self._columntoproperty[column]
return state.manager[prop.key].impl.get(state, dict_, passive=passive)
@@ -2790,7 +2790,7 @@ class Mapper(InspectionAttr):
)
def _get_committed_state_attr_by_column(
self, state, dict_, column, passive=attributes.PASSIVE_RETURN_NEVER_SET
self, state, dict_, column, passive=attributes.PASSIVE_RETURN_NO_VALUE
):
prop = self._columntoproperty[column]
+20 -27
View File
@@ -1028,31 +1028,27 @@ class GetNoValueTest(fixtures.ORMTest):
attributes.PASSIVE_NO_RESULT,
)
def test_passive_no_result_never_set(self):
attr, state, dict_ = self._fixture(attributes.NEVER_SET)
def test_passive_no_result_no_value(self):
attr, state, dict_ = self._fixture(attributes.NO_VALUE)
eq_(
attr.get(state, dict_, passive=attributes.PASSIVE_NO_INITIALIZE),
attributes.PASSIVE_NO_RESULT,
)
assert "attr" not in dict_
def test_passive_ret_never_set_never_set(self):
attr, state, dict_ = self._fixture(attributes.NEVER_SET)
def test_passive_ret_no_value(self):
attr, state, dict_ = self._fixture(attributes.NO_VALUE)
eq_(
attr.get(
state, dict_, passive=attributes.PASSIVE_RETURN_NEVER_SET
),
attributes.NEVER_SET,
attr.get(state, dict_, passive=attributes.PASSIVE_RETURN_NO_VALUE),
attributes.NO_VALUE,
)
assert "attr" not in dict_
def test_passive_ret_never_set_empty(self):
def test_passive_ret_no_value_empty(self):
attr, state, dict_ = self._fixture(None)
eq_(
attr.get(
state, dict_, passive=attributes.PASSIVE_RETURN_NEVER_SET
),
attributes.NEVER_SET,
attr.get(state, dict_, passive=attributes.PASSIVE_RETURN_NO_VALUE),
attributes.NO_VALUE,
)
assert "attr" not in dict_
@@ -1581,7 +1577,7 @@ class PendingBackrefTest(fixtures.ORMTest):
)
eq_(lazy_posts.call_count, 1)
def test_passive_history_collection_never_set(self):
def test_passive_history_collection_no_value(self):
Post, Blog, lazy_posts = self._fixture()
lazy_posts.return_value = attributes.PASSIVE_NO_RESULT
@@ -1594,10 +1590,10 @@ class PendingBackrefTest(fixtures.ORMTest):
attributes.instance_dict(b),
)
# this sets up NEVER_SET on b.posts
# this sets up NO_VALUE on b.posts
p.blog = b
eq_(state.committed_state, {"posts": attributes.NEVER_SET})
eq_(state.committed_state, {"posts": attributes.NO_VALUE})
assert "posts" not in dict_
# then suppose the object was made transient again,
@@ -1607,7 +1603,7 @@ class PendingBackrefTest(fixtures.ORMTest):
p2 = Post("asdf")
p2.blog = b
eq_(state.committed_state, {"posts": attributes.NEVER_SET})
eq_(state.committed_state, {"posts": attributes.NO_VALUE})
eq_(dict_["posts"], [p2])
# then this would fail.
@@ -2080,17 +2076,17 @@ class HistoryTest(fixtures.TestBase):
assert "someattr" not in f.__dict__
assert "someattr" not in attributes.instance_state(f).committed_state
def test_collection_never_set(self):
def test_collection_no_value(self):
Foo = self._fixture(uselist=True, useobject=True, active_history=True)
f = Foo()
eq_(self._someattr_history(f, passive=True), (None, None, None))
def test_scalar_obj_never_set(self):
def test_scalar_obj_no_value(self):
Foo = self._fixture(uselist=False, useobject=True, active_history=True)
f = Foo()
eq_(self._someattr_history(f, passive=True), (None, None, None))
def test_scalar_never_set(self):
def test_scalar_no_value(self):
Foo = self._fixture(
uselist=False, useobject=False, active_history=True
)
@@ -3483,14 +3479,14 @@ class EventPropagateTest(fixtures.TestBase):
b = B()
b.attrib = "foo"
eq_(b.attrib, "foo")
eq_(canary, [("foo", attributes.NEVER_SET)])
eq_(canary, [("foo", attributes.NO_VALUE)])
c = C()
c.attrib = "bar"
eq_(c.attrib, "bar")
eq_(
canary,
[("foo", attributes.NEVER_SET), ("bar", attributes.NEVER_SET)],
[("foo", attributes.NO_VALUE), ("bar", attributes.NO_VALUE)],
)
def test_propagate(self):
@@ -3531,16 +3527,13 @@ class EventPropagateTest(fixtures.TestBase):
d1 = D()
b.attrib = d1
is_(b.attrib, d1)
eq_(canary, [(d1, attributes.NEVER_SET)])
eq_(canary, [(d1, attributes.NO_VALUE)])
c = C()
d2 = D()
c.attrib = d2
is_(c.attrib, d2)
eq_(
canary,
[(d1, attributes.NEVER_SET), (d2, attributes.NEVER_SET)],
)
eq_(canary, [(d1, attributes.NO_VALUE), (d2, attributes.NO_VALUE)])
def _test_propagate_fixtures(self, active_history, useobject):
classes = [None, None, None, None]
+5 -5
View File
@@ -1096,9 +1096,9 @@ class GetterStateTest(_fixtures.FixtureTest):
Address.user.impl.get(
attributes.instance_state(a1),
attributes.instance_dict(a1),
passive=attributes.PASSIVE_RETURN_NEVER_SET,
passive=attributes.PASSIVE_RETURN_NO_VALUE,
),
attributes.NEVER_SET,
attributes.NO_VALUE,
)
assert "user_id" not in a1.__dict__
assert "user" not in a1.__dict__
@@ -1109,7 +1109,7 @@ class GetterStateTest(_fixtures.FixtureTest):
Address.user.impl.get_history(
attributes.instance_state(a1),
attributes.instance_dict(a1),
passive=attributes.PASSIVE_RETURN_NEVER_SET,
passive=attributes.PASSIVE_RETURN_NO_VALUE,
),
((), (), ()),
)
@@ -1174,7 +1174,7 @@ class GetterStateTest(_fixtures.FixtureTest):
Address.user.impl.get(
attributes.instance_state(a1),
attributes.instance_dict(a1),
passive=attributes.PASSIVE_RETURN_NEVER_SET,
passive=attributes.PASSIVE_RETURN_NO_VALUE,
),
User(name="ed"),
)
@@ -1185,7 +1185,7 @@ class GetterStateTest(_fixtures.FixtureTest):
Address.user.impl.get_history(
attributes.instance_state(a1),
attributes.instance_dict(a1),
passive=attributes.PASSIVE_RETURN_NEVER_SET,
passive=attributes.PASSIVE_RETURN_NO_VALUE,
),
((), [User(name="ed")], ()),
)