diff --git a/news/50.bugfix b/news/50.bugfix new file mode 100644 index 0000000..5500c83 --- /dev/null +++ b/news/50.bugfix @@ -0,0 +1 @@ +Fix `AttributeError` in `orderObjects` after deleting an object and restarting Zope. @wesleybl diff --git a/src/plone/folder/default.py b/src/plone/folder/default.py index de3f086..2ead14f 100644 --- a/src/plone/folder/default.py +++ b/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): diff --git a/src/plone/folder/tests/test_ordersupport.py b/src/plone/folder/tests/test_ordersupport.py index c05001d..254a53f 100644 --- a/src/plone/folder/tests/test_ordersupport.py +++ b/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"])