- fixed issue where slice assignment on relation properties truncates the relation (#529)

- fix for #530, don't require collection classes to respond to len requests

2 files changed, 171 insertions, 5 deletions

diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py
index 699a2f887..f706ca7e7 100644
--- a/lib/sqlalchemy/orm/attributes.py
+++ b/lib/sqlalchemy/orm/attributes.py
@@ -566,16 +566,18 @@ class InstrumentedList(object):
 
     def __contains__(self, item): return item in self.data
 
-    def __len__(self): return len(self.data)
+    def __len__(self):
+        try:
+            return len(self.data)
+        except TypeError:
+            return len(list(self.data))
 
     def __setslice__(self, i, j, other):
-        i = max(i, 0); j = max(j, 0)
-        [self.__delrecord(x) for x in self.data[i:]]
+        [self.__delrecord(x) for x in self.data[i:j]]
         g = [a for a in list(other) if self.__setrecord(a)]
-        self.data[i:] = g
+        self.data[i:j] = g
 
     def __delslice__(self, i, j):
-        i = max(i, 0); j = max(j, 0)
         for a in self.data[i:j]:
             self.__delrecord(a)
         del self.data[i:j]
diff --git a/test/orm/relationships.py b/test/orm/relationships.py
index fac484975..cc7eec915 100644
--- a/test/orm/relationships.py
+++ b/test/orm/relationships.py
@@ -745,6 +745,170 @@ class CustomCollectionsTest(testbase.ORMTest):
         assert len(list(f.bars)) == 2
         f.bars.clear()
 
+    def testlist(self):
+        class Parent(object):
+            pass
+        class Child(object):
+            pass
+
+        mapper(Parent, sometable, properties={
+            'children':relation(Child, collection_class=list)
+        })
+        mapper(Child, someothertable)
+
+        control = list()
+        p = Parent()
+
+        o = Child()
+        control.append(o)
+        p.children.append(o)
+        assert control == p.children.data
+        assert control == list(p.children)
+
+        o = [Child(), Child(), Child(), Child()]
+        control.extend(o)
+        p.children.extend(o)
+        assert control == p.children.data
+        assert control == list(p.children)
+
+        assert control[0] == p.children[0]
+        assert control[-1] == p.children[-1]
+        assert control[1:3] == p.children[1:3]
+
+        del control[1]
+        del p.children[1]
+        assert control == p.children.data
+        assert control == list(p.children)
+
+        o = [Child()]
+        control[1:3] = o
+        p.children[1:3] = o
+        assert control == p.children.data
+        assert control == list(p.children)
+
+        o = [Child(), Child(), Child(), Child()]
+        control[1:3] = o
+        p.children[1:3] = o
+        assert control == p.children.data
+        assert control == list(p.children)
+
+        o = [Child(), Child(), Child(), Child()]
+        control[-1:-2] = o
+        p.children[-1:-2] = o
+        assert control == p.children.data
+        assert control == list(p.children)
+
+        o = [Child(), Child(), Child(), Child()]
+        control[4:] = o
+        p.children[4:] = o
+        assert control == p.children.data
+        assert control == list(p.children)
+        
+        o = Child()
+        control.insert(0, o)
+        p.children.insert(0, o)
+        assert control == p.children.data
+        assert control == list(p.children)
+
+        o = Child()
+        control.insert(3, o)
+        p.children.insert(3, o)
+        assert control == p.children.data
+        assert control == list(p.children)
+
+        o = Child()
+        control.insert(999, o)
+        p.children.insert(999, o)
+        assert control == p.children.data
+        assert control == list(p.children)
+
+        del control[0:1]
+        del p.children[0:1]
+        assert control == p.children.data
+        assert control == list(p.children)
+
+        del control[1:1]
+        del p.children[1:1]
+        assert control == p.children.data
+        assert control == list(p.children)
+
+        del control[1:3]
+        del p.children[1:3]
+        assert control == p.children.data
+        assert control == list(p.children)
+
+        del control[7:]
+        del p.children[7:]
+        assert control == p.children.data
+        assert control == list(p.children)
+
+        assert control.pop() == p.children.pop()
+        assert control == p.children.data
+        assert control == list(p.children)
+
+        assert control.pop(0) == p.children.pop(0)
+        assert control == p.children.data
+        assert control == list(p.children)
+
+        assert control.pop(2) == p.children.pop(2)
+        assert control == p.children.data
+        assert control == list(p.children)
+
+        o = Child()
+        control.insert(2, o)
+        p.children.insert(2, o)
+        assert control == p.children.data
+        assert control == list(p.children)
+
+        control.remove(o)
+        p.children.remove(o)
+        assert control == p.children.data
+        assert control == list(p.children)
+
+    def testobj(self):
+        class Parent(object):
+            pass
+        class Child(object):
+            pass
+
+        class MyCollection(object):
+            def __init__(self): self.data = []
+            def append(self, value): self.data.append(value)
+            def __iter__(self): return iter(self.data)
+            def clear(self): self.data.clear()
+
+        mapper(Parent, sometable, properties={
+            'children':relation(Child, collection_class=MyCollection)
+        })
+        mapper(Child, someothertable)
+
+        control = list()
+        p1 = Parent()
+
+        o = Child()
+        control.append(o)
+        p1.children.append(o)
+        assert control == list(p1.children)
+
+        o = Child()
+        control.append(o)
+        p1.children.append(o)
+        assert control == list(p1.children)
+
+        o = Child()
+        control.append(o)
+        p1.children.append(o)
+        assert control == list(p1.children)
+
+        sess = create_session()
+        sess.save(p1)
+        sess.flush()
+        sess.clear()
+        
+        p2 = sess.query(Parent).get(p1.col1)
+        o = list(p2.children)
+        assert len(o) == 3
+
 class ViewOnlyTest(testbase.ORMTest):
     """test a view_only mapping where a third table is pulled into the primary join condition,
     using overlapping PK column names (should not produce "conflicting column" error)"""