- Fixed ORM orphaning bug with _raw_append method

- Promoted _reorder to reorder
- Now horking docstrings of overloaded methods from list
- Added a doctest

3 files changed, 103 insertions, 60 deletions

diff --git a/lib/sqlalchemy/ext/orderinglist.py b/lib/sqlalchemy/ext/orderinglist.py
index 21adc85a8..68d10e715 100644
--- a/lib/sqlalchemy/ext/orderinglist.py
+++ b/lib/sqlalchemy/ext/orderinglist.py
@@ -1,8 +1,8 @@
 """A custom list that manages index/position information for its children.
 
-``orderinglist`` is a custom list collection implementation for mapped relations
-that keeps an arbitrary "position" attribute on contained objects in sync with
-each object's position in the Python list.
+``orderinglist`` is a custom list collection implementation for mapped
+relations that keeps an arbitrary "position" attribute on contained objects in
+sync with each object's position in the Python list.
 
 The collection acts just like a normal Python ``list``, with the added
 behavior that as you manipulate the list (via ``insert``, ``pop``, assignment,
@@ -10,49 +10,61 @@ deletion, what have you), each of the objects it contains is updated as needed
 to reflect its position.  This is very useful for managing ordered relations
 which have a user-defined, serialized order::
 
-  from sqlalchemy.ext.orderinglist import ordering_list
-
-  users = Table('users', metadata,
-                Column('id', Integer, primary_key=True))
-  blurbs = Table('user_top_ten_list', metadata,
-                 Column('id', Integer, primary_key=True),
-                 Column('user_id', Integer, ForeignKey('users.id')),
-                 Column('position', Integer),
-                 Column('blurb', String(80)))
-
-  class User(object): pass
-  class Blurb(object):
-      def __init__(self, blurb):
-          self.blurb = blurb
-
-  mapper(User, users, properties={
-    'topten': relation(Blurb, collection_class=ordering_list('position'),
-                       order_by=[blurbs.c.position])
-  })
-  mapper(Blurb, blurbs)
-
-  u = User()
-  u.topten.append(Blurb('Number one!'))
-  u.topten.append(Blurb('Number two!'))
-
-  # Like magic.
-  assert [blurb.position for blurb in u.topten] == [0, 1]
-
-  # The objects will be renumbered automaticaly after any list-changing
-  # operation, for example an insert:
-  u.topten.insert(1, Blurb('I am the new Number Two.'))
-
-  assert [blurb.position for blurb in u.topten] == [0, 1, 2]
-  assert u.topten[1].blurb == 'I am the new Number Two.'
-  assert u.topten[1].position == 1
+  >>> from sqlalchemy import MetaData, Table, Column, Integer, String, ForeignKey
+  >>> from sqlalchemy.orm import mapper, relation
+  >>> from sqlalchemy.ext.orderinglist import ordering_list
+
+A simple model of users their "top 10" things::
+
+  >>> metadata = MetaData()
+  >>> users = Table('users', metadata,
+  ...               Column('id', Integer, primary_key=True))
+  >>> blurbs = Table('user_top_ten_list', metadata,
+  ...               Column('id', Integer, primary_key=True),
+  ...               Column('user_id', Integer, ForeignKey('users.id')),
+  ...               Column('position', Integer),
+  ...               Column('blurb', String(80)))
+  >>> class User(object):
+  ...   pass
+  ...
+  >>> class Blurb(object):
+  ...    def __init__(self, blurb):
+  ...        self.blurb = blurb
+  ...
+  >>> mapper(User, users, properties={
+  ...  'topten': relation(Blurb, collection_class=ordering_list('position'),
+  ...                     order_by=[blurbs.c.position])})
+  <Mapper ...>
+  >>> mapper(Blurb, blurbs)
+  <Mapper ...>
+
+Acts just like a regular list::
+
+  >>> u = User()
+  >>> u.topten.append(Blurb('Number one!'))
+  >>> u.topten.append(Blurb('Number two!'))
+
+But the ``.position`` attibute is set automatically behind the scenes::
+
+  >>> assert [blurb.position for blurb in u.topten] == [0, 1]
+
+The objects will be renumbered automaticaly after any list-changing operation,
+for example an ``insert()``::
+
+  >>> u.topten.insert(1, Blurb('I am the new Number Two.'))
+  >>> assert [blurb.position for blurb in u.topten] == [0, 1, 2]
+  >>> assert u.topten[1].blurb == 'I am the new Number Two.'
+  >>> assert u.topten[1].position == 1
 
 Numbering and serialization are both highly configurable.  See the docstrings
 in this module and the main SQLAlchemy documentation for more information and
 examples.
 
-The [sqlalchemy.ext.orderinglist#ordering_list] function is the ORM-compatible
-constructor for OrderingList instances.
+The [sqlalchemy.ext.orderinglist#ordering_list] factory function is the
+ORM-compatible constructor for `OrderingList` instances.
+
 """
+from sqlalchemy.orm.collections import collection
 
 
 __all__ = [ 'ordering_list' ]
@@ -123,8 +135,9 @@ class OrderingList(list):
     """A custom list that manages position information for its children.
 
     See the module and __init__ documentation for more details.  The
-    ``ordering_list`` function is used to configure ``OrderingList``
+    ``ordering_list`` factory function is used to configure ``OrderingList``
     collections in ``mapper`` relation definitions.
+
     """
 
     def __init__(self, ordering_attr=None, ordering_func=None,
@@ -139,12 +152,13 @@ class OrderingList(list):
         so be **sure** to put an ``order_by`` on your relation.
 
         ordering_attr
-          Name of the attribute that stores the object's order in the relation.
+          Name of the attribute that stores the object's order in the
+          relation.
 
         ordering_func
           Optional.  A function that maps the position in the Python list to a
-          value to store in the ``ordering_attr``.  Values returned are usually
-          (but need not be!) integers.
+          value to store in the ``ordering_attr``.  Values returned are
+          usually (but need not be!) integers.
 
           An ``ordering_func`` is called with two positional parameters: the
           index of the element in the list, and the list itself.
@@ -172,11 +186,11 @@ class OrderingList(list):
           concurrent modification error.  Spooky action at a distance.
 
           Recommend leaving this with the default of False, and just call
-          ``_reorder()`` if you're doing ``append()`` operations with
+          ``reorder()`` if you're doing ``append()`` operations with
           previously ordered instances or when doing some housekeeping after
           manual sql operations.
-        """
 
+        """
         self.ordering_attr = ordering_attr
         if ordering_func is None:
             ordering_func = count_from_0
@@ -191,13 +205,19 @@ class OrderingList(list):
     def _set_order_value(self, entity, value):
         setattr(entity, self.ordering_attr, value)
 
-    def _reorder(self):
-        """Sweep through the list and ensure that each object has accurate
-        ordering information set."""
+    def reorder(self):
+        """Synchronize ordering for the entire collection.
 
+        Sweeps through the list and ensures that each object has accurate
+        ordering information set.
+
+        """
         for index, entity in enumerate(self):
             self._order_entity(index, entity, True)
 
+    # As of 0.5, _reorder is no longer semi-private
+    _reorder = reorder
+
     def _order_entity(self, index, entity, reorder=True):
         have = self._get_order_value(entity)
 
@@ -213,6 +233,7 @@ class OrderingList(list):
         super(OrderingList, self).append(entity)
         self._order_entity(len(self) - 1, entity, self.reorder_on_append)
 
+    @collection.adds(1)
     def _raw_append(self, entity):
         """Append without any ordering behavior."""
 
@@ -249,3 +270,14 @@ class OrderingList(list):
     def __delslice__(self, start, end):
         super(OrderingList, self).__delslice__(start, end)
         self._reorder()
+
+    for func_name, func in locals().items():
+        if (callable(func) and func.func_name == func_name and
+            not func.__doc__ and hasattr(list, func_name)):
+            func.__doc__ = getattr(list, func_name).__doc__
+    del func_name, func
+
+if __name__ == '__main__':
+    import doctest
+    doctest.testmod(optionflags=doctest.ELLIPSIS)
+
diff --git a/test/ext/alltests.py b/test/ext/alltests.py
index 1b6dc53d2..ff645b335 100644
--- a/test/ext/alltests.py
+++ b/test/ext/alltests.py
@@ -1,16 +1,21 @@
 import testenv; testenv.configure_for_tests()
 import doctest, sys, unittest
 
+
 def suite():
-    unittest_modules = [
-                        'ext.declarative',
-                        'ext.orderinglist',
-                        'ext.associationproxy']
+    unittest_modules = (
+        'ext.declarative',
+        'ext.orderinglist',
+        'ext.associationproxy',
+        )
 
-    if sys.version_info >= (2, 4):
-        doctest_modules = ['sqlalchemy.ext.sqlsoup']
+    if sys.version_info < (2, 4):
+        doctest_modules = ()
     else:
-        doctest_modules = []
+        doctest_modules = (
+            ('sqlalchemy.ext.orderinglist', {'optionflags': doctest.ELLIPSIS}),
+            ('sqlalchemy.ext.sqlsoup', {})
+            )
 
     alltests = unittest.TestSuite()
     for name in unittest_modules:
@@ -18,8 +23,8 @@ def suite():
         for token in name.split('.')[1:]:
             mod = getattr(mod, token)
         alltests.addTest(unittest.findTestCases(mod, suiteClass=None))
-    for name in doctest_modules:
-        alltests.addTest(doctest.DocTestSuite(name))
+    for name, opts in doctest_modules:
+        alltests.addTest(doctest.DocTestSuite(name, **opts))
     return alltests
 
 
diff --git a/test/ext/orderinglist.py b/test/ext/orderinglist.py
index 2d8d6193f..460599ae5 100644
--- a/test/ext/orderinglist.py
+++ b/test/ext/orderinglist.py
@@ -189,6 +189,12 @@ class OrderingListTest(TestBase):
         self.assert_(s1.bullets[2].position == 3)
         self.assert_(s1.bullets[3].position == 4)
 
+        s1.bullets._raw_append(Bullet('raw'))
+        self.assert_(s1.bullets[4].position is None)
+
+        s1.bullets._reorder()
+        self.assert_(s1.bullets[4].position == 5)
+
         session = create_session()
         session.save(s1)
         session.flush()
@@ -200,9 +206,9 @@ class OrderingListTest(TestBase):
         srt = session.query(Slide).get(id)
 
         self.assert_(srt.bullets)
-        self.assert_(len(srt.bullets) == 4)
+        self.assert_(len(srt.bullets) == 5)
 
-        titles = ['s1/b1','s1/b2','s1/b100','s1/b4']
+        titles = ['s1/b1','s1/b2','s1/b100','s1/b4', 'raw']
         found = [b.text for b in srt.bullets]
 
         self.assert_(titles == found)