FileStorage: Fix restore to correctly check restored data content (#409)

In commit 8160568 (FileStorage: fix rare data corruption when using
restore after multiple undos (#395)) we fixed FileStorage.restore to
work correctly in the presence of multiple undos for the same oid in the
same transaction by reworking oid data records of one transaction to be
all scanned in the loop. But unfortunately that commit introduced a
regression:

When restore is instructed to do an undo, in other words, to make a copy
of oid data by referring to that data by backpointer, it checks content
of the pointed-to record, and if data content there does not match data
content passed to restore call, the copy-via-backpointer optimization is
silently disabled. However 8160568 contained a thinko when doing the
check: after discovering pointed-to oid data record, the intent there it
was to load actual data from there, but the loading was performed not
from the offset where the data lives, but from then beginning of some
next data-record header instead. As the result the
"copy-via-backpointer" was always disabled.

-> Fix that by seeking to correct position before loading the data.

I originally discovered this issue by running Zodbtools tests wrt ZODB6
and getting failures for `zodb restore` related tests with difference in
between restored and expected states as e.g.

    @@ -296,15 +296,21 @@
     txn 0285cbad77777800 " "
     user "root1.0\nYour\nMagesty "
     description "undo 1.0\nmore detailed description\n\nzzz ...\t"
     extension ""
    -obj 0000000000000005 from 0285cbad6962fd19
    +obj 0000000000000005 30 sha1:5fe26f63145e7ad5ac7c2a6bc813e706c5548278
    +c__main__
    +Object
    +q^A.U^Ea1.22q^B.

/cc @d-maurer
/reviewed-by @perrinjerome
/reviewed-on #409

Author: navytux

CHANGES.rst

@@ -9,6 +9,12 @@
 
  - repozo: prevent an incorrect "option ignored" warning when running backup or verify.
 
+ - FileStorage: fix `restore` regression introduced in ZODB 6.0 in `#395
+   <https://github.com/zopefoundation/ZODB/pull/395>`_: when restoring data
+   records with undo the `restore` was no longer emitting backpointers and was
+   emitting duplicate data copies instead. `#409 <https://github.com/zopefoundation/ZODB/pull/409>`_
+   fixes `restore` back to emit data records with backpointers for undo again.
+
 
 6.0 (2024-03-20)
 ================

src/ZODB/FileStorage/FileStorage.py

@@ -704,6 +704,7 @@ def _data_find(self, tpos, oid, data):
                 logger.error("Mismatch between data and"
                              " backpointer at %d", pos)
                 return 0
+            self._file.seek(data_pos + DATA_HDR_LEN)
             _data = self._file.read(data_hdr.plen)
             if data != _data:
                 return 0

src/ZODB/tests/testFileStorage.py

@@ -35,6 +35,7 @@
 from ZODB._compat import dumps
 from ZODB.Connection import TransactionMetaData
 from ZODB.fsIndex import fsIndex
+from ZODB.interfaces import IStorageWrapper
 from ZODB.tests import BasicStorage
 from ZODB.tests import ConflictResolution
 from ZODB.tests import Corruption
@@ -242,6 +243,46 @@ def testRestoreBumpsOid(self):
         # Before ZODB 3.2.6, this failed, with ._oid == z64.
         self.assertEqual(self._storage._oid, giant_oid)
 
+    def testRestoreCopy(self):
+        # Verify that restore with prev_txn restores data record with
+        # backpointer to prev_txn.
+        oid = p64(1)
+
+        t = TransactionMetaData()
+        self._storage.tpc_begin(t)
+        self._storage.store(oid, z64, b'data rev 1', b'', t)
+        self._storage.tpc_vote(t)
+        self._storage.tpc_finish(t)
+        rev1 = self._storage.lastTransaction()
+
+        rev2 = p64(U64(rev1)+1)
+        t = TransactionMetaData()
+        self._storage.tpc_begin(t, rev2)
+        # restore should write data record with backpointer to rev1
+        self._storage.restore(oid, rev2, b'data rev 1', b'', rev1, t)
+        self._storage.tpc_vote(t)
+        self._storage.tpc_finish(t)
+        self.assertEqual(self._storage.lastTransaction(), rev2)
+
+        # locate rev2 data record and verify it is backpointer to rev1
+        pos2 = self._storage._lookup_pos(oid)
+        h2 = self._storage._read_data_header(pos2, oid)
+        self.assertEqual(h2.oid, oid)
+        self.assertEqual(h2.tid, rev2)
+        self.assertEqual(h2.plen, 0)
+        self.assertNotEqual(h2.back, 0)
+
+        h1 = self._storage._read_data_header(h2.back, oid)
+        self.assertEqual(h1.oid, oid)
+        self.assertEqual(h1.tid, rev1)
+        self.assertNotEqual(h1.plen, 0)
+        self.assertEqual(h1.back, 0)
+
+        data1 = self._storage._file.read(h1.plen)
+        if IStorageWrapper.providedBy(self._storage):
+            data1 = self._storage.untransform_record_data(data1)
+        self.assertEqual(data1, b'data rev 1')
+
     def testCorruptionInPack(self):
         # This sets up a corrupt .fs file, with a redundant transaction
         # length mismatch.  The implementation of pack in many releases of