Fix AttributeError in orderObjects after deleting an object and restarting Zope

news/50.bugfix

@@ -0,0 +1 @@
+Fix `AttributeError` in `orderObjects` after deleting an object and restarting Zope. @wesleybl

src/plone/folder/default.py

@@ -7,6 +7,8 @@
 from zope.container.contained import notifyContainerModified
 from zope.interface import implementer
 
+import warnings
+
 
 @implementer(IExplicitOrdering)
 @adapter(IOrderableFolder)
@@ -132,9 +134,7 @@ def keyfn(obj_id):
                     return attr()
                 return attr
 
-            # order.sort(cmd=None, key=keyfn, reverse=bool(reverse))
-            order = sorted(order, key=keyfn, reverse=bool(reverse))
-            self._set_order(order)
+            order.sort(key=keyfn, reverse=bool(reverse))
         for n, obj_id in enumerate(order):
             pos[obj_id] = n
         return -1
@@ -166,10 +166,21 @@ def _order(self, create=False):
         return annotations.get(self.ORDER_KEY, [])
 
     def _set_order(self, value):
-        # We added a setter because in py2 _order is modified inplace
-        # with .sort() while in py3 we sort with sorted and thus need to set it
-        # explicitly
+        warnings.warn(
+            "_set_order is deprecated and no longer used internally. "
+            "Order is now modified in-place. This method will be removed in a future version.",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+
         annotations = IAnnotations(self.context)
+
+        if value is not None and not isinstance(value, PersistentList):
+            try:
+                value = PersistentList(value)
+            except TypeError:
+                pass
+
         annotations[self.ORDER_KEY] = value
 
     def _pos(self, create=False):

src/plone/folder/tests/test_ordersupport.py

@@ -1,9 +1,11 @@
 from OFS.Traversable import Traversable
 from plone.folder.interfaces import IOrdering
 from plone.folder.ordered import OrderedBTreeFolderBase
+from plone.folder.testing import PLONEFOLDER_FUNCTIONAL_TESTING
 from plone.folder.testing import PLONEFOLDER_INTEGRATION_TESTING
 from plone.folder.tests.utils import DummyObject
 
+import transaction
 import unittest
 
 
@@ -13,6 +15,14 @@ class TestFolder(OrderedBTreeFolderBase, Traversable):
     def getId(self):
         return self.id
 
+    def __of__(self, parent):
+        # Allows you to do:
+        # portal._setObject("f1", TestFolder("f1"))
+        return self
+
+    def manage_fixupOwnershipAfterAdd(self):
+        return None
+
 
 class OFSOrderSupportTests(unittest.TestCase):
     """tests borrowed from OFS.tests.testOrderSupport"""
@@ -353,3 +363,34 @@ def testNotifyRemoved(self):
         # normal
         ordering.notifyRemoved("foo")
         self.assertEqual(ordering.idsInOrder(), ["bar", "baz"])
+
+
+class OrderSupportFunctionalTests(unittest.TestCase):
+    """Functional tests for order support"""
+
+    layer = PLONEFOLDER_FUNCTIONAL_TESTING
+
+    def test_order_objects_stale_after_delete_and_restart(self):
+        portal = self.layer["portal"]
+
+        portal._setObject("f1", TestFolder("f1"))
+        folder = portal["f1"]
+        folder["foo"] = DummyObject("foo", "mt1")
+        folder["bar"] = DummyObject("bar", "mt1")
+        folder["baz"] = DummyObject("baz", "mt1")
+        conn = portal._p_jar
+
+        folder.orderObjects("id")
+        transaction.commit()
+
+        del folder["bar"]
+        transaction.commit()
+
+        # Simulate a Zope server restart by ghosting all cached persistent
+        # objects so they are reloaded from the database on next access.
+        conn.cacheMinimize()
+
+        folder = portal["f1"]
+        folder.orderObjects("id")
+        ordering = folder.getOrdering()
+        self.assertEqual(ordering.idsInOrder(), ["baz", "foo"])