History: support classes modifying instances in __setstate__ (#1086)

* testHistory: test coverage for manage_historicalComparison

* History: support classes modifying instances in __setstate__

Fixes https://bugs.launchpad.net/zope2/+bug/735999

* History: assign _p_jar after setting state

This way, classes with a migration in __setstate__ have a chance to
modify the object, but these changes are not saved to database

* testHistory: commit transactions when checking ZMI views

if committing transaction raised TemporalParadox we can not view the
ZMI page

* testHistory: test for HistoricalRevisions attribute

* History: fix raising TemporalParadox when editing history

Co-authored-by: Jens Vagelpohl <jens@plyp.com>

CHANGES.rst

@@ -15,6 +15,11 @@ https://github.com/zopefoundation/Zope/blob/4.x/CHANGES.rst
 
 - Update to newest compatible versions of dependencies.
 
+- Fix history page for classes modifying instances in ``__setstate__``,
+  such as ``Products.PythonScripts.PythonScript`` instances.
+  See `launchpad issue 735999
+  <https://bugs.launchpad.net/zope2/+bug/735999>`_.
+
 
 5.7.3 (2022-12-19)
 ------------------

src/OFS/History.py

@@ -45,28 +45,35 @@ class HystoryJar:
 
     def __init__(self, base):
         self.__base__ = base
+        self._needs_to_join = True
+        self._registered_objects = []
 
     def __getattr__(self, name):
         return getattr(self.__base__, name)
 
-    def commit(self, object, transaction):
-        if object._p_changed:
-            raise TemporalParadox("You can't change history!")
+    def commit(self, transaction):
+        raise TemporalParadox("You can't change history!")
 
     def abort(*args, **kw):
         pass
 
     tpc_begin = tpc_finish = abort
 
+    def register(self, obj):
+        if self._needs_to_join:
+            self.transaction_manager.get().join(self)
+        if obj is not None:
+            self._registered_objects.append(obj)
+
 
 def historicalRevision(self, serial):
     state = self._p_jar.oldstate(self, serial)
     rev = self.__class__.__basicnew__()
-    rev._p_jar = HystoryJar(self._p_jar)
     rev._p_oid = self._p_oid
     rev._p_serial = serial
     rev.__setstate__(state)
     rev._p_changed = 0
+    rev._p_jar = HystoryJar(self._p_jar)
     return rev
 
 

src/OFS/tests/testHistory.py

@@ -9,7 +9,9 @@
 import Zope2
 from OFS.Application import Application
 from OFS.History import Historical
+from OFS.History import TemporalParadox
 from OFS.SimpleItem import SimpleItem
+from Testing.makerequest import makerequest
 from ZODB.FileStorage import FileStorage
 
 
@@ -21,6 +23,8 @@ class HistoryItem(SimpleItem, Historical):
 
 
 class HistoryTests(unittest.TestCase):
+    def _getTargetClass(self):
+        return HistoryItem
 
     def setUp(self):
         # set up a zodb
@@ -32,10 +36,10 @@ def setUp(self):
         r = self.connection.root()
         a = Application()
         r['Application'] = a
-        self.root = a
-        # create a python script
-        a['test'] = HistoryItem()
-        self.hi = hi = a.test
+        self.root = makerequest(a)
+        # create a history item object
+        self.root['test'] = self._getTargetClass()()
+        self.hi = hi = self.root.test
         # commit some changes
         hi.title = 'First title'
         t = transaction.get()
@@ -95,3 +99,78 @@ def test_manage_historyCopy(self):
         # that all other attributes will behave the same
         self.assertEqual(self.hi.title,
                          'First title')
+
+    def test_HistoricalRevisions(self):
+        r = self.hi.manage_change_history()
+        historical_revision = self.hi.HistoricalRevisions[r[2]['key']]
+        self.assertIsInstance(historical_revision, self._getTargetClass())
+        self.assertEqual(historical_revision.title, 'First title')
+        # do a commit, just like ZPublisher would
+        transaction.commit()
+
+    def test_HistoricalRevisions_edit_causes_TemporalParadox(self):
+        r = self.hi.manage_change_history()
+        historical_revision = self.hi.HistoricalRevisions[r[2]['key']]
+        historical_revision.title = 'Changed'
+        self.assertRaises(TemporalParadox, transaction.commit)
+
+    def test_manage_historicalComparison(self):
+        r = self.hi.manage_change_history()
+        # compare two revisions
+        self.assertIn(
+            'This object does not provide comparison support.',
+            self.hi.manage_historicalComparison(
+                REQUEST=self.hi.REQUEST,
+                keys=[r[1]['key'], r[2]['key']]))
+
+        # compare a revision with latest
+        self.assertIn(
+            'This object does not provide comparison support.',
+            self.hi.manage_historicalComparison(
+                REQUEST=self.hi.REQUEST,
+                keys=[r[2]['key']]))
+
+        # do a commit, just like ZPublisher would
+        transaction.commit()
+
+
+class HistoryItemWithComparisonSupport(SimpleItem, Historical):
+    def manage_historyCompare(self, rev1, rev2, REQUEST,
+                              historyComparisonResults=''):
+        return super().manage_historyCompare(
+            rev1, rev2, REQUEST,
+            historyComparisonResults=f'old: {rev1.title} new: {rev2.title}')
+
+
+class HistoryWithComparisonSupportTests(HistoryTests):
+    def _getTargetClass(self):
+        return HistoryItemWithComparisonSupport
+
+    def test_manage_historicalComparison(self):
+        r = self.hi.manage_change_history()
+        # compare two revisions
+        self.assertIn(
+            'old: First title new: Second title',
+            self.hi.manage_historicalComparison(
+                REQUEST=self.hi.REQUEST,
+                keys=[r[1]['key'], r[2]['key']]))
+
+        # compare a revision with latest
+        self.assertIn(
+            'old: First title new: Third title',
+            self.hi.manage_historicalComparison(
+                REQUEST=self.hi.REQUEST,
+                keys=[r[2]['key']]))
+
+
+class HistoryItemWithSetState(HistoryItem):
+    """A class with a data migration on __setstate__
+    """
+    def __setstate__(self, state):
+        super().__setstate__(state)
+        self.changed_something = True
+
+
+class HistoryWithSetStateTest(HistoryTests):
+    def _getTargetClass(self):
+        return HistoryItemWithSetState