Commit 0ce17b49 authored by Jeremy Hylton's avatar Jeremy Hylton

Fix subtle bug in restore().

The _txn_find() must not stop at the pack boundary when it is called
by restore().  It was originally written for _txn_undo() which isn't
supposed to undo to a transaction across a pack.  But it should be
legal to restore() a transaction with a reference to a data record in
a transaction that was packed.

Fix by adding stop_at_pack flag to _txn_find() and add tests of this
behavior for FileStorage.
parent ff316ee1
......@@ -115,7 +115,7 @@
# may have a back pointer to a version record or to a non-version
# record.
#
__version__='$Revision: 1.120 $'[11:-2]
__version__='$Revision: 1.121 $'[11:-2]
import base64
from cPickle import Pickler, Unpickler, loads
......@@ -786,7 +786,7 @@ class FileStorage(BaseStorage.BaseStorage,
try:
prev_pos = 0
if prev_txn is not None:
prev_txn_pos = self._txn_find(prev_txn)
prev_txn_pos = self._txn_find(prev_txn, 0)
if prev_txn_pos:
prev_pos = self._data_find(prev_txn_pos, oid, data)
old = self._index_get(oid, 0)
......@@ -1159,12 +1159,12 @@ class FileStorage(BaseStorage.BaseStorage,
# Find the right transaction to undo and call _txn_undo_write().
tid = base64.decodestring(transaction_id + '\n')
assert len(tid) == 8
tpos = self._txn_find(tid)
tpos = self._txn_find(tid, 1)
tindex = self._txn_undo_write(tpos)
self._tindex.update(tindex)
return tindex.keys()
def _txn_find(self, tid):
def _txn_find(self, tid, stop_at_pack):
pos = self._pos
# XXX Why 39? Only because undoLog() uses it as a boundary.
while pos > 39:
......@@ -1175,8 +1175,9 @@ class FileStorage(BaseStorage.BaseStorage,
_tid = h[:8]
if _tid == tid:
return pos
status = h[16] # get the c in 8s8sc
if status == 'p' or _tid < self._packt:
if stop_at_pack:
# check the status field of the transaction header
if h[16] == 'p' or _tid < self._packt:
break
raise UndoError(None, "Invalid transaction id")
......
......@@ -16,8 +16,11 @@
from ZODB.Transaction import Transaction
from ZODB.tests.IteratorStorage import IteratorDeepCompare
from ZODB.tests.StorageTestBase import MinPO, zodb_unpickle, removefs
from ZODB import DB
from ZODB.referencesf import referencesf
from ZODB.FileStorage import FileStorage
import time
class RecoveryStorage(IteratorDeepCompare):
# Requires a setUp() that creates a self._dst destination storage
......@@ -126,3 +129,27 @@ class RecoveryStorage(IteratorDeepCompare):
self._dst = FileStorage('Dest.fs')
self._dst.copyTransactionsFrom(self._storage)
self.compare(self._storage, self._dst)
def checkRestoreAcrossPack(self):
db = DB(self._storage)
c = db.open()
r = c.root()
obj1 = r["obj1"] = MinPO(1)
get_transaction().commit()
obj1 = r["obj2"] = MinPO(1)
get_transaction().commit()
self._dst.copyTransactionsFrom(self._storage)
self._dst.pack(time.time(), referencesf)
self._undo(self._storage.undoInfo()[0]['id'])
# copy the final transaction manually. even though there
# was a pack, the restore() ought to succeed.
final = list(self._storage.iterator())[-1]
self._dst.tpc_begin(final, final.tid, final.status)
for r in final:
self._dst.restore(r.oid, r.serial, r.data, r.version, r.data_txn,
final)
self._dst.tpc_vote(final)
self._dst.tpc_finish(final)
......@@ -196,7 +196,7 @@ class StorageTestBase(unittest.TestCase):
# The following methods depend on optional storage features.
def _undo(self, tid, oid):
def _undo(self, tid, oid=None):
# Undo a tid that affects a single object (oid).
# XXX This is very specialized
t = Transaction()
......@@ -205,6 +205,7 @@ class StorageTestBase(unittest.TestCase):
oids = self._storage.transactionalUndo(tid, t)
self._storage.tpc_vote(t)
self._storage.tpc_finish(t)
if oid is not None:
self.assertEqual(len(oids), 1)
self.assertEqual(oids[0], oid)
return self._storage.lastTransaction()
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment