diff options
| -rw-r--r-- | doc/build/changelog/unreleased_13/4990.rst | 11 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/collections.py | 2 | ||||
| -rw-r--r-- | test/orm/test_collection.py | 36 |
3 files changed, 45 insertions, 4 deletions
diff --git a/doc/build/changelog/unreleased_13/4990.rst b/doc/build/changelog/unreleased_13/4990.rst new file mode 100644 index 000000000..702403dd5 --- /dev/null +++ b/doc/build/changelog/unreleased_13/4990.rst @@ -0,0 +1,11 @@ +.. change:: + :tags: bug, orm, py3k + :tickets: 4990 + + Fixed issue where when assigning a collection to itself as a slice, the + mutation operation would fail as it would first erase the assigned + collection inadvertently. As an assignment that does not change the + contents should not generate events, the operation is now a no-op. Note + that the fix only applies to Python 3; in Python 2, the ``__setitem__`` + hook isn't called in this case; ``__setslice__`` is used instead which + recreates the list item-by-item in all cases.
\ No newline at end of file diff --git a/lib/sqlalchemy/orm/collections.py b/lib/sqlalchemy/orm/collections.py index 4b096c152..5008f5727 100644 --- a/lib/sqlalchemy/orm/collections.py +++ b/lib/sqlalchemy/orm/collections.py @@ -1201,6 +1201,8 @@ def _list_decorators(): stop += len(self) if step == 1: + if value is self: + return for i in range(start, stop, step): if len(self) > start: del self[start] diff --git a/test/orm/test_collection.py b/test/orm/test_collection.py index 34b7fc5fe..03b6c8758 100644 --- a/test/orm/test_collection.py +++ b/test/orm/test_collection.py @@ -1,3 +1,4 @@ +import contextlib from operator import and_ from sqlalchemy import event @@ -30,6 +31,15 @@ class Canary(object): self.data = set() self.added = set() self.removed = set() + self.dupe_check = True + + @contextlib.contextmanager + def defer_dupe_check(self): + self.dupe_check = False + try: + yield + finally: + self.dupe_check = True def listen(self, attr): event.listen(attr, "append", self.append) @@ -37,15 +47,17 @@ class Canary(object): event.listen(attr, "set", self.set) def append(self, obj, value, initiator): - assert value not in self.added + if self.dupe_check: + assert value not in self.added + self.added.add(value) self.data.add(value) - self.added.add(value) return value def remove(self, obj, value, initiator): - assert value not in self.removed + if self.dupe_check: + assert value not in self.removed + self.removed.add(value) self.data.remove(value) - self.removed.add(value) def set(self, obj, value, oldvalue, initiator): if isinstance(value, str): @@ -299,6 +311,22 @@ class CollectionsTest(fixtures.ORMTest): control[:] = values assert_eq() + # test slice assignment where we slice assign to self, + # currently a no-op, issue #4990 + # note that in py2k, the bug does not exist but it recreates + # the collection which breaks our fixtures here + with canary.defer_dupe_check(): + direct[:] = direct + control[:] = control + assert_eq() + + # we dont handle assignment of self to slices, as this + # implies duplicate entries. behavior here is not well defined + # and perhaps should emit a warning + # direct[0:1] = list(direct) + # control[0:1] = list(control) + # assert_eq() + # test slice assignment where # slice size goes over the number of items values = [creator(), creator()] |