diff --git a/CHANGES.txt b/CHANGES.txt index 8d7ca8e9..a480e73d 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -16,6 +16,18 @@ .. _`PR 23`: https://github.com/zodb/relstorage/pull/23/ +- zodbconvert: The ``--incremental`` option is supported with a + FileStorage (or any storage that implements + ``IStorage.lastTransaction()``) + as a destination, not just RelStorages. + +- zodbconvert: The ``--incremental`` option is supported with a + RelStorage as a destination. See `PR 22`. With contributions by + Sylvain Viollon, Mauro Amico, and Peter Jacobs. Originally reported + by Jan-Wijbrand Kolman. + +.. _`PR 22`: https://github.com/zodb/relstorage/pull/22 + 1.6.0b3 (2014-12-08) -------------------- diff --git a/relstorage/storage.py b/relstorage/storage.py index d33e641b..fa942f00 100644 --- a/relstorage/storage.py +++ b/relstorage/storage.py @@ -1207,6 +1207,8 @@ def _pack_finished(self): # TODO: Remove all old revisions of blobs in history-free mode. def iterator(self, start=None, stop=None): + # XXX: This is broken for purposes of copyTransactionsFrom() because + # it can only be iterated over once. zodbconvert works around this. return TransactionIterator(self._adapter, start, stop) def sync(self, force=True): diff --git a/relstorage/tests/reltestbase.py b/relstorage/tests/reltestbase.py index a2742d7a..ebccac88 100644 --- a/relstorage/tests/reltestbase.py +++ b/relstorage/tests/reltestbase.py @@ -33,8 +33,26 @@ import time import transaction +class StorageCreatingMixin(object): -class RelStorageTestBase(StorageTestBase.StorageTestBase): + def make_adapter(self, options): + # abstract method + raise NotImplementedError() + + def make_storage(self, zap=True, **kw): + from relstorage.options import Options + from relstorage.storage import RelStorage + options = Options(keep_history=self.keep_history, **kw) + adapter = self.make_adapter(options) + storage = RelStorage(adapter, options=options) + storage._batcher_row_limit = 1 + if zap: + storage.zap_all() + return storage + + +class RelStorageTestBase(StorageCreatingMixin, + StorageTestBase.StorageTestBase): keep_history = None # Override _storage_created = None @@ -64,21 +82,6 @@ def set_storage(self, storage): _storage = property(get_storage, set_storage) - def make_adapter(self, options): - # abstract method - raise NotImplementedError() - - def make_storage(self, zap=True, **kw): - from relstorage.options import Options - from relstorage.storage import RelStorage - options = Options(keep_history=self.keep_history, **kw) - adapter = self.make_adapter(options) - storage = RelStorage(adapter, options=options) - storage._batcher_row_limit = 1 - if zap: - storage.zap_all() - return storage - def open(self, read_only=False): # This is used by a few ZODB tests that close and reopen the storage. storage = self._storage @@ -819,6 +822,54 @@ def updater(): finally: db.close() +from .test_zodbconvert import FSZODBConvertTests + +class AbstractRSZodbConvertTests(StorageCreatingMixin, + FSZODBConvertTests): + keep_history = True + filestorage_name = 'source' + relstorage_name = 'destination' + + def _relstorage_contents(self): + raise NotImplementedError() + + def setUp(self): + super(AbstractRSZodbConvertTests, self).setUp() + cfg = """ + %%import relstorage + + path %s + + + %s + + """ % (self.filestorage_name, self.filestorage_file, + self.relstorage_name, self._relstorage_contents()) + self._write_cfg(cfg) + self.make_storage(zap=True) + + +class AbstractRSDestZodbConvertTests(AbstractRSZodbConvertTests): + + @property + def filestorage_file(self): + return self.srcfile + + def _create_dest_storage(self): + return self.make_storage(zap=False) + +class AbstractRSSrcZodbConvertTests(AbstractRSZodbConvertTests): + + filestorage_name = 'destination' + relstorage_name = 'source' + + @property + def filestorage_file(self): + return self.destfile + + + def _create_src_storage(self): + return self.make_storage(zap=False) class DoubleCommitter(Persistent): """A crazy persistent class that changes self in __getstate__""" diff --git a/relstorage/tests/test_zodbconvert.py b/relstorage/tests/test_zodbconvert.py index 464ff54e..e0c3fc60 100644 --- a/relstorage/tests/test_zodbconvert.py +++ b/relstorage/tests/test_zodbconvert.py @@ -11,14 +11,95 @@ # FOR A PARTICULAR PURPOSE. # ############################################################################## +from __future__ import print_function +from contextlib import contextmanager +import os +import tempfile +import transaction import unittest -class ZODBConvertTests(unittest.TestCase): + +from relstorage.zodbconvert import main + +class AbstractZODBConvertBase(unittest.TestCase): + cfgfile = None + + def _create_src_storage(self): + raise NotImplementedError() + + def _create_dest_storage(self): + raise NotImplementedError() + + def _create_src_db(self): + from ZODB.DB import DB + return DB(self._create_src_storage()) + + def _create_dest_db(self): + from ZODB.DB import DB + return DB(self._create_dest_storage()) + + @contextmanager + def __conn(self, name): + db = getattr(self, '_create_' + name + '_db')() + conn = db.open() + try: + yield conn + finally: + conn.close() + db.close() + + def _src_conn(self): + return self.__conn('src') + + def _dest_conn(self): + return self.__conn('dest') + + def _write_value_for_x_in_src(self, x): + with self._src_conn() as conn: + conn.root()['x'] = x + transaction.commit() + + def _check_value_of_x_in_dest(self, x): + with self._dest_conn() as conn2: + db_x = conn2.root().get('x') + self.assertEqual(db_x, x) + + def test_convert(self): + self._write_value_for_x_in_src(10) + main(['', self.cfgfile]) + self._check_value_of_x_in_dest(10) + + + def test_dry_run(self): + self._write_value_for_x_in_src(10) + main(['', '--dry-run', self.cfgfile]) + self._check_value_of_x_in_dest(None) + + + def test_incremental(self): + x = 10 + self._write_value_for_x_in_src(x) + main(['', self.cfgfile]) + self._check_value_of_x_in_dest(x) + + x = "hi" + self._write_value_for_x_in_src(x) + main(['', '--incremental', self.cfgfile]) + self._check_value_of_x_in_dest(x) + + + def test_no_overwrite(self): + db = self._create_src_db() # create the root object + db.close() + db = self._create_dest_db() # create the root object + db.close() + self.assertRaises(SystemExit, main, ['', self.cfgfile]) + +class FSZODBConvertTests(AbstractZODBConvertBase): def setUp(self): - import os - import tempfile + super(FSZODBConvertTests, self).setUp() fd, self.srcfile = tempfile.mkstemp() os.close(fd) @@ -36,19 +117,29 @@ def setUp(self): path %s """ % (self.srcfile, self.destfile) + self._write_cfg(cfg) + def _write_cfg(self, cfg): fd, self.cfgfile = tempfile.mkstemp() os.write(fd, cfg) os.close(fd) def tearDown(self): - import os if os.path.exists(self.destfile): os.remove(self.destfile) if os.path.exists(self.srcfile): os.remove(self.srcfile) if os.path.exists(self.cfgfile): os.remove(self.cfgfile) + super(FSZODBConvertTests, self).tearDown() + + def _create_src_storage(self): + from ZODB.FileStorage import FileStorage + return FileStorage(self.srcfile) + + def _create_dest_storage(self): + from ZODB.FileStorage import FileStorage + return FileStorage(self.destfile) def test_storage_has_data(self): from ZODB.DB import DB @@ -60,75 +151,11 @@ def test_storage_has_data(self): db.close() self.assertTrue(storage_has_data(src)) - def test_convert(self): - from ZODB.DB import DB - from ZODB.FileStorage import FileStorage - from relstorage.zodbconvert import main - from relstorage.zodbconvert import storage_has_data - import transaction - - src = FileStorage(self.srcfile) - db = DB(src) - conn = db.open() - conn.root()['x'] = 10 - transaction.commit() - conn.close() - db.close() - - main(['', self.cfgfile]) - - dest = FileStorage(self.destfile) - db2 = DB(dest) - conn2 = db2.open() - self.assertEqual(conn2.root().get('x'), 10) - conn2.close() - db2.close() - - def test_dry_run(self): - from ZODB.DB import DB - from ZODB.FileStorage import FileStorage - from relstorage.zodbconvert import main - from relstorage.zodbconvert import storage_has_data - import transaction - - src = FileStorage(self.srcfile) - db = DB(src) - conn = db.open() - conn.root()['x'] = 10 - transaction.commit() - conn.close() - db.close() - - main(['', '--dry-run', self.cfgfile]) - - dest = FileStorage(self.destfile) - db2 = DB(dest) - conn2 = db2.open() - self.assertEqual(conn2.root().get('x'), None) - conn2.close() - db2.close() - - def test_no_overwrite(self): - from ZODB.DB import DB - from ZODB.FileStorage import FileStorage - from relstorage.zodbconvert import main - from relstorage.zodbconvert import storage_has_data - import transaction - - src = FileStorage(self.srcfile) - db = DB(src) # create the root object - db.close() - - dest = FileStorage(self.destfile) - db = DB(dest) # create the root object - db.close() - - self.assertRaises(SystemExit, main, ['', self.cfgfile]) def test_suite(): suite = unittest.TestSuite() - suite.addTest(unittest.makeSuite(ZODBConvertTests)) + suite.addTest(unittest.makeSuite(FSZODBConvertTests)) return suite if __name__ == '__main__': - unittest.main() + unittest.main(defaultTest='test_suite') diff --git a/relstorage/tests/testmysql.py b/relstorage/tests/testmysql.py index 6b0cb255..b0d24d88 100644 --- a/relstorage/tests/testmysql.py +++ b/relstorage/tests/testmysql.py @@ -12,7 +12,7 @@ # ############################################################################## """Tests of relstorage.adapters.mysql""" - +from __future__ import absolute_import from relstorage.options import Options from relstorage.tests.hftestbase import HistoryFreeFromFileStorage from relstorage.tests.hftestbase import HistoryFreeRelStorageTests @@ -21,6 +21,8 @@ from relstorage.tests.hptestbase import HistoryPreservingRelStorageTests from relstorage.tests.hptestbase import HistoryPreservingToFileStorage from .util import skipOnCI +from .reltestbase import AbstractRSDestZodbConvertTests +from .reltestbase import AbstractRSSrcZodbConvertTests import logging import os import unittest @@ -110,6 +112,23 @@ def checkConfigureViaZConfig(self): finally: db.close() +class _MySQLCfgMixin(object): + + def _relstorage_contents(self): + return """ + + db relstoragetest + user relstoragetest + passwd relstoragetest + + """ + +class HPMySQLDestZODBConvertTests(UseMySQLAdapter, _MySQLCfgMixin, AbstractRSDestZodbConvertTests): + pass + +class HPMySQLSrcZODBConvertTests(UseMySQLAdapter, _MySQLCfgMixin, AbstractRSSrcZodbConvertTests): + pass + class HPMySQLTests(UseMySQLAdapter, HistoryPreservingRelStorageTests, ZConfigTests): @@ -165,6 +184,8 @@ def test_suite(): HFMySQLFromFile, ]: suite.addTest(unittest.makeSuite(klass, "check")) + suite.addTest(unittest.makeSuite(HPMySQLDestZODBConvertTests)) + suite.addTest(unittest.makeSuite(HPMySQLSrcZODBConvertTests)) try: import ZODB.blob diff --git a/relstorage/tests/testpostgresql.py b/relstorage/tests/testpostgresql.py index 913cab29..ef5d0298 100644 --- a/relstorage/tests/testpostgresql.py +++ b/relstorage/tests/testpostgresql.py @@ -12,7 +12,7 @@ # ############################################################################## """Tests of relstorage.adapters.postgresql""" - +from __future__ import absolute_import from relstorage.options import Options from relstorage.tests.hftestbase import HistoryFreeFromFileStorage from relstorage.tests.hftestbase import HistoryFreeRelStorageTests @@ -20,6 +20,8 @@ from relstorage.tests.hptestbase import HistoryPreservingFromFileStorage from relstorage.tests.hptestbase import HistoryPreservingRelStorageTests from relstorage.tests.hptestbase import HistoryPreservingToFileStorage +from .reltestbase import AbstractRSDestZodbConvertTests +from .reltestbase import AbstractRSSrcZodbConvertTests import logging import os import unittest @@ -105,6 +107,21 @@ def checkConfigureViaZConfig(self): db.close() +class _PgSQLCfgMixin(object): + + def _relstorage_contents(self): + return """ + + dsn dbname='relstoragetest' user='relstoragetest' password='relstoragetest' + + """ + +class HPPostgreSQLDestZODBConvertTests(UsePostgreSQLAdapter, _PgSQLCfgMixin, AbstractRSDestZodbConvertTests): + pass + +class HPPostgreSQLSrcZODBConvertTests(UsePostgreSQLAdapter, _PgSQLCfgMixin, AbstractRSSrcZodbConvertTests): + pass + class HPPostgreSQLTests(UsePostgreSQLAdapter, HistoryPreservingRelStorageTests, ZConfigTests): pass @@ -156,6 +173,8 @@ def test_suite(): HFPostgreSQLFromFile, ]: suite.addTest(unittest.makeSuite(klass, "check")) + suite.addTest(unittest.makeSuite(HPPostgreSQLDestZODBConvertTests)) + suite.addTest(unittest.makeSuite(HPPostgreSQLSrcZODBConvertTests)) try: import ZODB.blob diff --git a/relstorage/zodbconvert.py b/relstorage/zodbconvert.py index 0431874c..72204981 100644 --- a/relstorage/zodbconvert.py +++ b/relstorage/zodbconvert.py @@ -16,6 +16,7 @@ See README.txt for details. """ +from __future__ import print_function import logging import optparse @@ -23,7 +24,7 @@ from StringIO import StringIO import sys import ZConfig -from ZODB.utils import p64, readable_tid_repr +from ZODB.utils import p64, u64, z64, readable_tid_repr schema_xml = """ @@ -53,6 +54,23 @@ def storage_has_data(storage): return True +class _DefaultStartStorageIteration(object): + # At IStorageIteration instance that keeps a default start value. + # This is needed because RelStorage.iterator() does return an object with an + # iterator() method, but that object returns itself, so it can only be iterated + # once! This breaks some implementations of copyTransactionsFrom, notably + # our own. See #22 + + def __init__(self, source, start): + self._source = source + self._start = start + + def iterator(self, start=None, end=None): + return self._source.iterator(start or self._start, end) + + def __getattr__(self, name): + return getattr(self._source, name) + def main(argv=sys.argv): parser = optparse.OptionParser(description=__doc__, usage="%prog [options] config_file") @@ -87,7 +105,7 @@ def main(argv=sys.argv): log.info("Storages opened successfully.") if options.incremental: - if not hasattr(destination, '_adapter'): + if not hasattr(destination, '_adapter') and not hasattr(destination, 'lastTransaction'): msg = ("Error: no API is known for determining the last committed " "transaction of the destination storage. Aborting " "conversion.") @@ -95,10 +113,25 @@ def main(argv=sys.argv): if not storage_has_data(destination): log.warning("Destination empty, start conversion from the beginning.") else: - last_tid = destination._adapter.txncontrol.get_tid( - destination._load_cursor) + wrap_source = False + if hasattr(destination, '_adapter'): + # RelStorage. Note that we implement lastTransaction(), but + # only in-memory, local to the particular object. (We should probably + # change that?) So order matters. + destination.load(z64) # prime the connection + wrap_source = True # compensate for our bug + last_tid = destination._adapter.txncontrol.get_tid( + destination._load_cursor) + else: + # IStorage, like FileStorage + last_tid_s = destination.lastTransaction() + last_tid = u64(last_tid_s) + next_tid = p64(last_tid+1) - source = source.iterator(start=next_tid) + if wrap_source: + source = _DefaultStartStorageIteration(source, next_tid) + else: + source = source.iterator(start=next_tid) log.info("Resuming ZODB copy from %s", readable_tid_repr(next_tid)) @@ -125,7 +158,7 @@ def main(argv=sys.argv): sys.exit(msg) log.info("Done clearing old data.") - if storage_has_data(destination): + if storage_has_data(destination) and not options.incremental: msg = "Error: the destination storage has data. Try --clear." sys.exit(msg)