From dfc23e8efe23d4e9b4092c0cdad0c6eba639bf0f Mon Sep 17 00:00:00 2001 From: FichteFoll Date: Fri, 14 Sep 2018 02:43:38 +0200 Subject: [PATCH] Implement database and model revision checking Prevents reloading a model from the database when it hasn't changed. Now we're back to almost the same speed as before the addition of album field fallbacks. --- beets/dbcore/db.py | 35 ++++++++++++++++++++++++++++++-- test/test_dbcore.py | 49 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 2 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 5aa418632..3786bbbd2 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -251,6 +251,11 @@ class Model(object): value is the same as the old value (e.g., `o.f = o.f`). """ + _revision = -1 + """A revision number from when the model was loaded from or written + to the database. + """ + @classmethod def _getters(cls): """Return a mapping from field names to getter functions. @@ -303,9 +308,11 @@ class Model(object): def clear_dirty(self): """Mark all fields as *clean* (i.e., not needing to be stored to - the database). + the database). Also update the revision. """ self._dirty = set() + if self._db: + self._revision = self._db.revision def _check_db(self, need_id=True): """Ensure that this object is associated with a database row: it @@ -533,8 +540,14 @@ class Model(object): def load(self): """Refresh the object's metadata from the library database. + + If check_revision is true, the database is only queried loaded when a + transaction has been committed since the item was last loaded. """ self._check_db() + if not self._dirty and self._db.revision == self._revision: + # Exit early + return stored_obj = self._db._get(type(self), self.id) assert stored_obj is not None, u"object {0} not in DB".format(self.id) self._values_fixed = LazyConvertDict(self) @@ -789,6 +802,12 @@ class Transaction(object): """A context manager for safe, concurrent access to the database. All SQL commands should be executed through a transaction. """ + + _mutated = False + """A flag storing whether a mutation has been executed in the + current transaction. + """ + def __init__(self, db): self.db = db @@ -810,12 +829,15 @@ class Transaction(object): entered but not yet exited transaction. If it is the last active transaction, the database updates are committed. """ + # Beware of races; currently secured by db._db_lock + self.db.revision += self._mutated with self.db._tx_stack() as stack: assert stack.pop() is self empty = not stack if empty: # Ending a "root" transaction. End the SQLite transaction. self.db._connection().commit() + self._mutated = False self.db._db_lock.release() def query(self, statement, subvals=()): @@ -831,7 +853,6 @@ class Transaction(object): """ try: cursor = self.db._connection().execute(statement, subvals) - return cursor.lastrowid except sqlite3.OperationalError as e: # In two specific cases, SQLite reports an error while accessing # the underlying database file. We surface these exceptions as @@ -841,9 +862,14 @@ class Transaction(object): raise DBAccessError(e.args[0]) else: raise + else: + self._mutated = True + return cursor.lastrowid def script(self, statements): """Execute a string containing multiple SQL statements.""" + # We don't know whether this mutates, but quite likely it does. + self._mutated = True self.db._connection().executescript(statements) @@ -859,6 +885,11 @@ class Database(object): supports_extensions = hasattr(sqlite3.Connection, 'enable_load_extension') """Whether or not the current version of SQLite supports extensions""" + revision = 0 + """The current revision of the database. To be increased whenever + data is written in a transaction. + """ + def __init__(self, path, timeout=5.0): self.path = path self.timeout = timeout diff --git a/test/test_dbcore.py b/test/test_dbcore.py index 9bf78de67..c3a8a60ee 100644 --- a/test/test_dbcore.py +++ b/test/test_dbcore.py @@ -224,6 +224,31 @@ class MigrationTest(unittest.TestCase): self.fail("select failed") +class TransactionTest(unittest.TestCase): + def setUp(self): + self.db = DatabaseFixture1(':memory:') + + def tearDown(self): + self.db._connection().close() + + def test_mutate_increase_revision(self): + old_rev = self.db.revision + with self.db.transaction() as tx: + tx.mutate( + 'INSERT INTO {0} ' + '(field_one) ' + 'VALUES (?);'.format(ModelFixture1._table), + (111,), + ) + self.assertGreater(self.db.revision, old_rev) + + def test_query_no_increase_revision(self): + old_rev = self.db.revision + with self.db.transaction() as tx: + tx.query('PRAGMA table_info(%s)' % ModelFixture1._table) + self.assertEqual(self.db.revision, old_rev) + + class ModelTest(unittest.TestCase): def setUp(self): self.db = DatabaseFixture1(':memory:') @@ -245,6 +270,30 @@ class ModelTest(unittest.TestCase): row = self.db._connection().execute('select * from test').fetchone() self.assertEqual(row['field_one'], 123) + def test_revision(self): + old_rev = self.db.revision + model = ModelFixture1() + model.add(self.db) + model.store() + self.assertEqual(model._revision, self.db.revision) + self.assertGreater(self.db.revision, old_rev) + + mid_rev = self.db.revision + model2 = ModelFixture1() + model2.add(self.db) + model2.store() + self.assertGreater(model2._revision, mid_rev) + self.assertGreater(self.db.revision, model._revision) + + # revision changed, so the model should be re-loaded + model.load() + self.assertEqual(model._revision, self.db.revision) + + # revision did not change, so no reload + mod2_old_rev = model2._revision + model2.load() + self.assertEqual(model2._revision, mod2_old_rev) + def test_retrieve_by_id(self): model = ModelFixture1() model.add(self.db)