transaction objects to control DB access

In an attempt to finally address the longstanding SQLite locking issues, I'm
introducing a way to explicitly, lexically scope transactions. The Transaction
class is a context manager that always fully fetches after SELECTs and
automatically commits on exit. No direct access to the library is allowed, so
all changes will eventually be committed and all queries will be completed. This
will also provide a debugging mechanism to show where concurrent transactions
are beginning and ending.

To support composition (transaction reentrancy), an internal, per-Library stack
of transactions is maintained. Commits only happen when the outermost
transaction exits. This means that, while it's possible to introduce atomicity
bugs by invoking Library methods outside of a transaction, you can conveniently
call them *without* a currently-active transaction to get a single atomic
action.

Note that this "transaction stack" concepts assumes a single Library object per
thread. Because we need to duplicate Library objects for concurrent access due
to sqlite3 limitation already, this is fine for now. Later, the interface should
provide one transaction stack per thread for shared Library objects.
This commit is contained in:
Adrian Sampson 2012-05-06 23:24:05 -07:00
parent 5a9cc6a2d9
commit a28f930c52
15 changed files with 343 additions and 312 deletions

View file

@ -718,7 +718,7 @@ def apply_choices(config):
# Add items -- before path changes -- to the library. We add the
# items now (rather than at the end) so that album structures
# are in place before calls to destination().
try:
with lib.transaction():
# Remove old items.
for replaced in replaced_items.itervalues():
for item in replaced:
@ -735,8 +735,6 @@ def apply_choices(config):
# Add tracks.
for item in items:
lib.add(item)
finally:
lib.save()
# Move/copy files.
task.old_paths = [item.path for item in items] # For deletion.
@ -764,11 +762,9 @@ def apply_choices(config):
item.write()
# Save new paths.
try:
with lib.transaction():
for item in items:
lib.store(item)
finally:
lib.save()
def fetch_art(config):
"""A coroutine that fetches and applies album art for albums where
@ -786,11 +782,8 @@ def fetch_art(config):
# Save the art if any was found.
if artpath:
try:
album = lib.get_album(task.album_id)
album.set_art(artpath, not (config.delete or config.move))
finally:
lib.save(False)
album = lib.get_album(task.album_id)
album.set_art(artpath, not (config.delete or config.move))
if (config.delete or config.move) and task.toppath:
task.prune(artpath)

View file

@ -8,10 +8,11 @@
# distribute, sublicense, and/or sell copies of the Software, and to
# permit persons to whom the Software is furnished to do so, subject to
# the following conditions:
#
#
# The above copyright notice and this permission notice shall be
# included in all copies or substantial portions of the Software.
from __future__ import with_statement
import sqlite3
import os
import re
@ -165,7 +166,7 @@ class Item(object):
self.dirty = {}
self._fill_record(values)
self._clear_dirty()
@classmethod
def from_path(cls, path):
"""Creates a new item from the media file at the specified path.
@ -175,7 +176,7 @@ class Item(object):
'album_id': None,
})
i.read(path)
i.mtime = i.current_mtime() # Initial mtime.
i.mtime = i.current_mtime() # Initial mtime.
return i
def _fill_record(self, values):
@ -211,7 +212,7 @@ class Item(object):
sets the record entry for that key to value. Note that to change
the attribute in the database or in the file's tags, one must
call store() or write().
Otherwise, performs an ordinary setattr.
"""
# Encode unicode paths and read buffers.
@ -230,10 +231,10 @@ class Item(object):
self.mtime = 0 # Reset mtime on dirty.
else:
super(Item, self).__setattr__(key, value)
# Interaction with file metadata.
def read(self, read_path=None):
"""Read the metadata from the associated file. If read_path is
specified, read metadata from that file instead.
@ -251,7 +252,7 @@ class Item(object):
# Database's mtime should now reflect the on-disk value.
if read_path == self.path:
self.mtime = self.current_mtime()
def write(self):
"""Writes the item's metadata to the associated file.
"""
@ -278,7 +279,7 @@ class Item(object):
util.copy(self.path, dest)
else:
util.move(self.path, dest)
# Either copying or moving succeeded, so update the stored path.
self.path = dest
@ -366,16 +367,14 @@ class Query(object):
clause, subvals = self.clause()
return ('SELECT ' + columns + ' FROM items WHERE ' + clause, subvals)
def count(self, library):
def count(self, tx):
"""Returns `(num, length)` where `num` is the number of items in
the library matching this query and `length` is their total
length in seconds.
"""
clause, subvals = self.clause()
statement = 'SELECT COUNT(id), SUM(length) FROM items WHERE ' + clause
c = library.conn.execute(statement, subvals)
result = c.fetchone()
c.close()
result = tx.query(statement, subvals)[0]
return (result[0], result[1] or 0.0)
class FieldQuery(Query):
@ -387,7 +386,7 @@ class FieldQuery(Query):
raise InvalidFieldError(field + ' is not an item key')
self.field = field
self.pattern = pattern
class MatchQuery(FieldQuery):
"""A query that looks for exact matches in an item field."""
def clause(self):
@ -457,7 +456,7 @@ class CollectionQuery(Query):
"""
def __init__(self, subqueries=()):
self.subqueries = subqueries
# is there a better way to do this?
def __len__(self): return len(self.subqueries)
def __getitem__(self, key): return self.subqueries[key]
@ -674,7 +673,7 @@ class PathQuery(Query):
# Match the path as a single file.
self.file_path = normpath(path)
# As a directory (prefix).
self.dir_path = os.path.join(self.file_path, '')
self.dir_path = os.path.join(self.file_path, '')
def match(self, item):
return (item.path == self.file_path) or \
@ -686,21 +685,18 @@ class PathQuery(Query):
return '(path = ?) || (path LIKE ?)', (file_blob, dir_pat)
class ResultIterator(object):
"""An iterator into an item query result set. The iterator eagerly
fetches all of the results from the cursor but lazily constructs
Item objects that reflect them.
"""An iterator into an item query result set. The iterator lazily
constructs Item objects that reflect database rows.
"""
def __init__(self, cursor):
# Fetch all of the rows, closing the cursor (and unlocking the
# database).
self.rows = cursor.fetchall()
def __init__(self, rows):
self.rows = rows
self.rowiter = iter(self.rows)
def __iter__(self):
return self
def next(self):
row = self.rowiter.next() # May raise StopIteration.
row = self.rowiter.next() # May raise StopIteration.
return Item(row)
@ -755,12 +751,6 @@ class BaseLibrary(object):
"""
raise NotImplementedError
def save(self):
"""Ensure that the library is consistent on disk. A no-op by
default.
"""
pass
def load(self, item, load_id=None):
"""Refresh the item's metadata from the library database. If
fetch_id is not specified, use the item's current id.
@ -887,6 +877,47 @@ class BaseAlbum(object):
# Concrete DB-backed library.
class Transaction(object):
"""A context manager for safe, concurrent access to the database.
All SQL commands should be executed through a transaction.
"""
def __init__(self, lib):
self.lib = lib
def __enter__(self):
"""Begin a transaction. This transaction may be created while
another is active.
"""
self.lib._tx_stack.append(self)
return self
def __exit__(self, exc_type, exc_value, traceback):
"""Complete a transaction. This must be the most recently
entered but not yet exited transaction. If it is the last active
transaction, the database updates are committed.
"""
assert self.lib._tx_stack.pop() is self
if not self.lib._tx_stack:
self.lib.conn.commit()
def query(self, statement, subvals=()):
"""Execute an SQL statement with substitution values and return
a list of rows from the database.
"""
cursor = self.lib.conn.execute(statement, subvals)
return cursor.fetchall()
def mutate(self, statement, subvals=()):
"""Execute an SQL statement with substitution values and return
the row ID of the last affected row.
"""
cursor = self.lib.conn.execute(statement, subvals)
return cursor.lastrowid
def script(self, statements):
"""Execute a string containing multiple SQL statements."""
self.lib.conn.executescript(statements)
class Library(BaseLibrary):
"""A music library using an SQLite database as a metadata store."""
def __init__(self, path='library.blb',
@ -907,6 +938,9 @@ class Library(BaseLibrary):
self.art_filename = bytestring_path(art_filename)
self.replacements = replacements
self._memotable = {} # Used for template substitution performance.
self._tx_stack = []
self.timeout = timeout
self.conn = sqlite3.connect(self.path, timeout)
# This way we can access our SELECT results like dictionaries.
@ -929,29 +963,27 @@ class Library(BaseLibrary):
self._make_table('items', item_fields)
self._make_table('albums', album_fields)
self._memotable = {} # Used for template substitution performance.
def _make_table(self, table, fields):
"""Set up the schema of the library file. fields is a list of
all the fields that should be present in the indicated table.
Columns are added if necessary.
"""
# Get current schema.
cur = self.conn.cursor()
cur.execute('PRAGMA table_info(%s)' % table)
current_fields = set([row[1] for row in cur])
with self.transaction() as tx:
rows = tx.query('PRAGMA table_info(%s)' % table)
current_fields = set([row[1] for row in rows])
field_names = set([f[0] for f in fields])
if current_fields.issuperset(field_names):
# Table exists and has all the required columns.
return
if not current_fields:
# No table exists.
# No table exists.
setup_sql = 'CREATE TABLE %s (' % table
setup_sql += ', '.join(['%s %s' % f[:2] for f in fields])
setup_sql += ');\n'
else:
# Table exists but is missing fields.
setup_sql = ''
@ -971,8 +1003,14 @@ class Library(BaseLibrary):
'albumartist' not in current_fields:
setup_sql += "UPDATE ALBUMS SET albumartist=artist;\n"
self.conn.executescript(setup_sql)
self.conn.commit()
with self.transaction() as tx:
tx.script(setup_sql)
def transaction(self):
"""Get a transaction object for interacting with the database.
This should *always* be used as a context manager.
"""
return Transaction(self)
def destination(self, item, pathmod=None, fragment=False,
basedir=None, platform=None):
@ -1031,7 +1069,7 @@ class Library(BaseLibrary):
return subpath
else:
basedir = basedir or self.directory
return normpath(os.path.join(basedir, subpath))
return normpath(os.path.join(basedir, subpath))
# Item manipulation.
@ -1041,7 +1079,7 @@ class Library(BaseLibrary):
if copy:
self.move(item, copy=True)
# build essential parts of query
# Build essential parts of query.
columns = ','.join([key for key in ITEM_KEYS if key != 'id'])
values = ','.join(['?'] * (len(ITEM_KEYS) - 1))
subvars = []
@ -1052,41 +1090,30 @@ class Library(BaseLibrary):
value = buffer(value)
subvars.append(value)
# issue query
c = self.conn.cursor()
# Issue query.
query = 'INSERT INTO items (' + columns + ') VALUES (' + values + ')'
c.execute(query, subvars)
new_id = c.lastrowid
c.close()
with self.transaction() as tx:
new_id = tx.mutate(query, subvars)
item._clear_dirty()
item.id = new_id
self._memotable = {}
return new_id
def save(self, event=True):
"""Writes the library to disk (completing an sqlite
transaction).
"""
self.conn.commit()
if event:
plugins.send('save', lib=self)
def load(self, item, load_id=None):
if load_id is None:
load_id = item.id
c = self.conn.execute(
'SELECT * FROM items WHERE id=?', (load_id,) )
item._fill_record(c.fetchone())
with self.transaction() as tx:
rows = tx.query('SELECT * FROM items WHERE id=?', (load_id,))
item._fill_record(rows[0])
item._clear_dirty()
c.close()
def store(self, item, store_id=None, store_all=False):
if store_id is None:
store_id = item.id
# build assignments for query
# Build assignments for query.
assignments = ''
subvars = []
for key in ITEM_KEYS:
@ -1102,14 +1129,15 @@ class Library(BaseLibrary):
if not assignments:
# nothing to store (i.e., nothing was dirty)
return
assignments = assignments[:-1] # knock off last ,
# finish the query
assignments = assignments[:-1] # Knock off last ,
# Finish the query.
query = 'UPDATE items SET ' + assignments + ' WHERE id=?'
subvars.append(store_id)
self.conn.execute(query, subvars)
with self.transaction() as tx:
tx.mutate(query, subvars)
item._clear_dirty()
self._memotable = {}
@ -1121,7 +1149,8 @@ class Library(BaseLibrary):
"""
album = self.get_album(item) if with_album else None
self.conn.execute('DELETE FROM items WHERE id=?', (item.id,))
with self.transaction() as tx:
tx.mutate('DELETE FROM items WHERE id=?', (item.id,))
if album:
item_iter = album.items()
@ -1143,26 +1172,26 @@ class Library(BaseLibrary):
directory (provided by destination()). Subdirectories are
created as needed. If the operation succeeds, the item's path
field is updated to reflect the new location.
If copy is True, moving the file is copied rather than moved.
basedir overrides the library base directory for the
destination.
If the item is in an album, the album is given an opportunity to
move its art. (This can be disabled by passing
with_album=False.)
The item is stored to the database if it is in the database, so
any dirty fields prior to the move() call will be written as a
side effect. You probably want to call save() to commit the DB
transaction.
"""
dest = self.destination(item, basedir=basedir)
# Create necessary ancestry for the move.
util.mkdirall(dest)
# Perform the move and store the change.
old_path = item.path
item.move(dest, copy)
@ -1192,8 +1221,9 @@ class Library(BaseLibrary):
"WHERE " + where + \
" ORDER BY %s, album" % \
_orelse("albumartist_sort", "albumartist")
c = self.conn.execute(sql, subvals)
return [Album(self, dict(res)) for res in c.fetchall()]
with self.transaction() as tx:
rows = tx.query(sql, subvals)
return [Album(self, dict(res)) for res in rows]
def items(self, query=None, artist=None, album=None, title=None):
queries = [self._get_query(query, False)]
@ -1211,8 +1241,9 @@ class Library(BaseLibrary):
" ORDER BY %s, album, disc, track" % \
_orelse("artist_sort", "artist")
log.debug('Getting items with SQL: %s' % sql)
c = self.conn.execute(sql, subvals)
return ResultIterator(c)
with self.transaction() as tx:
rows = tx.query(sql, subvals)
return ResultIterator(rows)
# Convenience accessors.
@ -1220,13 +1251,14 @@ class Library(BaseLibrary):
def get_item(self, id):
"""Fetch an Item by its ID. Returns None if no match is found.
"""
c = self.conn.execute("SELECT * FROM items WHERE id=?", (id,))
it = ResultIterator(c)
with self.transaction() as tx:
rows = tx.query("SELECT * FROM items WHERE id=?", (id,))
it = ResultIterator(rows)
try:
return it.next()
except StopIteration:
return None
def get_album(self, item_or_id):
"""Given an album ID or an item associated with an album,
return an Album object for the album. If no such album exists,
@ -1239,16 +1271,13 @@ class Library(BaseLibrary):
if album_id is None:
return None
c = self.conn.execute(
'SELECT * FROM albums WHERE id=?',
(album_id,)
)
try:
record = c.fetchone()
finally:
c.close()
if record:
return Album(self, dict(record))
with self.transaction() as tx:
rows = tx.query(
'SELECT * FROM albums WHERE id=?',
(album_id,)
)
if rows:
return Album(self, dict(rows[0]))
def add_album(self, items):
"""Create a new album in the database with metadata derived
@ -1260,12 +1289,20 @@ class Library(BaseLibrary):
item_values = dict(
(key, getattr(items[0], key)) for key in ALBUM_KEYS_ITEM)
sql = 'INSERT INTO albums (%s) VALUES (%s)' % \
(', '.join(ALBUM_KEYS_ITEM),
', '.join(['?'] * len(ALBUM_KEYS_ITEM)))
subvals = [item_values[key] for key in ALBUM_KEYS_ITEM]
c = self.conn.execute(sql, subvals)
album_id = c.lastrowid
with self.transaction() as tx:
sql = 'INSERT INTO albums (%s) VALUES (%s)' % \
(', '.join(ALBUM_KEYS_ITEM),
', '.join(['?'] * len(ALBUM_KEYS_ITEM)))
subvals = [item_values[key] for key in ALBUM_KEYS_ITEM]
album_id = tx.mutate(sql, subvals)
# Add the items to the library.
for item in items:
item.album_id = album_id
if item.id is None:
self.add(item)
else:
self.store(item)
# Construct the new Album object.
record = {}
@ -1278,14 +1315,6 @@ class Library(BaseLibrary):
record['id'] = album_id
album = Album(self, record)
# Add the items to the library.
for item in items:
item.album_id = album_id
if item.id is None:
self.add(item)
else:
self.store(item)
return album
class Album(BaseAlbum):
@ -1318,7 +1347,8 @@ class Album(BaseAlbum):
# Change album table.
sql = 'UPDATE albums SET %s=? WHERE id=?' % key
self._library.conn.execute(sql, (value, self.id))
with self._library.transaction() as tx:
tx.mutate(sql, (value, self.id))
# Possibly make modification on items as well.
if key in ALBUM_KEYS_ITEM:
@ -1342,11 +1372,12 @@ class Album(BaseAlbum):
"""Returns an iterable over the items associated with this
album.
"""
c = self._library.conn.execute(
'SELECT * FROM items WHERE album_id=?',
(self.id,)
)
return ResultIterator(c)
with self._library.transaction() as tx:
rows = tx.query(
'SELECT * FROM items WHERE album_id=?',
(self.id,)
)
return ResultIterator(rows)
def remove(self, delete=False, with_items=True):
"""Removes this album and all its associated items from the
@ -1355,22 +1386,23 @@ class Album(BaseAlbum):
containing the album are also removed (recursively) if empty.
Set with_items to False to avoid removing the album's items.
"""
if with_items:
# Remove items.
for item in self.items():
self._library.remove(item, delete, False)
if delete:
# Delete art file.
artpath = self.artpath
if artpath:
util.soft_remove(artpath)
# Remove album from database.
self._library.conn.execute(
'DELETE FROM albums WHERE id=?',
(self.id,)
)
with self._library.transaction() as tx:
if with_items:
# Remove items.
for item in self.items():
self._library.remove(item, delete, False)
# Remove album from database.
tx.mutate(
'DELETE FROM albums WHERE id=?',
(self.id,)
)
def move_art(self, copy=False):
"""Move or copy any existing album art so that it remains in the

View file

@ -849,89 +849,89 @@ def update_items(lib, query, album, move, color, pretend):
"""For all the items matched by the query, update the library to
reflect the item's embedded tags.
"""
items, _ = _do_query(lib, query, album)
with lib.transaction():
items, _ = _do_query(lib, query, album)
# Walk through the items and pick up their changes.
affected_albums = set()
for item in items:
# Item deleted?
if not os.path.exists(syspath(item.path)):
print_(u'X %s - %s' % (item.artist, item.title))
if not pretend:
lib.remove(item, True)
affected_albums.add(item.album_id)
continue
# Did the item change since last checked?
if item.current_mtime() <= item.mtime:
log.debug(u'skipping %s because mtime is up to date (%i)' %
(displayable_path(item.path), item.mtime))
continue
# Read new data.
old_data = dict(item.record)
item.read()
# Special-case album artist when it matches track artist. (Hacky
# but necessary for preserving album-level metadata for non-
# autotagged imports.)
if not item.albumartist and \
old_data['albumartist'] == old_data['artist'] == item.artist:
item.albumartist = old_data['albumartist']
item.dirty['albumartist'] = False
# Get and save metadata changes.
changes = {}
for key in library.ITEM_KEYS_META:
if item.dirty[key]:
changes[key] = old_data[key], getattr(item, key)
if changes:
# Something changed.
print_(u'* %s - %s' % (item.artist, item.title))
for key, (oldval, newval) in changes.iteritems():
_showdiff(key, oldval, newval, color)
# If we're just pretending, then don't move or save.
if pretend:
# Walk through the items and pick up their changes.
affected_albums = set()
for item in items:
# Item deleted?
if not os.path.exists(syspath(item.path)):
print_(u'X %s - %s' % (item.artist, item.title))
if not pretend:
lib.remove(item, True)
affected_albums.add(item.album_id)
continue
# Move the item if it's in the library.
if move and lib.directory in ancestry(item.path):
lib.move(item)
# Did the item change since last checked?
if item.current_mtime() <= item.mtime:
log.debug(u'skipping %s because mtime is up to date (%i)' %
(displayable_path(item.path), item.mtime))
continue
lib.store(item)
affected_albums.add(item.album_id)
elif not pretend:
# The file's mtime was different, but there were no changes
# to the metadata. Store the new mtime, which is set in the
# call to read(), so we don't check this again in the
# future.
lib.store(item)
# Read new data.
old_data = dict(item.record)
item.read()
# Skip album changes while pretending.
if pretend:
return
# Special-case album artist when it matches track artist. (Hacky
# but necessary for preserving album-level metadata for non-
# autotagged imports.)
if not item.albumartist and \
old_data['albumartist'] == old_data['artist'] == \
item.artist:
item.albumartist = old_data['albumartist']
item.dirty['albumartist'] = False
# Modify affected albums to reflect changes in their items.
for album_id in affected_albums:
if album_id is None: # Singletons.
continue
album = lib.get_album(album_id)
if not album: # Empty albums have already been removed.
log.debug('emptied album %i' % album_id)
continue
al_items = list(album.items())
# Get and save metadata changes.
changes = {}
for key in library.ITEM_KEYS_META:
if item.dirty[key]:
changes[key] = old_data[key], getattr(item, key)
if changes:
# Something changed.
print_(u'* %s - %s' % (item.artist, item.title))
for key, (oldval, newval) in changes.iteritems():
_showdiff(key, oldval, newval, color)
# Update album structure to reflect an item in it.
for key in library.ALBUM_KEYS_ITEM:
setattr(album, key, getattr(al_items[0], key))
# If we're just pretending, then don't move or save.
if pretend:
continue
# Move album art (and any inconsistent items).
if move and lib.directory in ancestry(al_items[0].path):
log.debug('moving album %i' % album_id)
album.move()
# Move the item if it's in the library.
if move and lib.directory in ancestry(item.path):
lib.move(item)
lib.save()
lib.store(item)
affected_albums.add(item.album_id)
elif not pretend:
# The file's mtime was different, but there were no changes
# to the metadata. Store the new mtime, which is set in the
# call to read(), so we don't check this again in the
# future.
lib.store(item)
# Skip album changes while pretending.
if pretend:
return
# Modify affected albums to reflect changes in their items.
for album_id in affected_albums:
if album_id is None: # Singletons.
continue
album = lib.get_album(album_id)
if not album: # Empty albums have already been removed.
log.debug('emptied album %i' % album_id)
continue
al_items = list(album.items())
# Update album structure to reflect an item in it.
for key in library.ALBUM_KEYS_ITEM:
setattr(album, key, getattr(al_items[0], key))
# Move album art (and any inconsistent items).
if move and lib.directory in ancestry(al_items[0].path):
log.debug('moving album %i' % album_id)
album.move()
update_cmd = ui.Subcommand('update',
help='update the library', aliases=('upd','up',))
@ -972,14 +972,13 @@ def remove_items(lib, query, album, delete=False):
return
# Remove (and possibly delete) items.
if album:
for al in albums:
al.remove(delete)
else:
for item in items:
lib.remove(item, delete)
lib.save()
with lib.transaction():
if album:
for al in albums:
al.remove(delete)
else:
for item in items:
lib.remove(item, delete)
remove_cmd = ui.Subcommand('remove',
help='remove matching items from the library', aliases=('rm',))
@ -1091,23 +1090,23 @@ def modify_items(lib, mods, query, write, move, album, color, confirm):
return
# Apply changes to database.
for obj in objs:
for field, value in fsets.iteritems():
setattr(obj, field, value)
with lib.transaction():
for obj in objs:
for field, value in fsets.iteritems():
setattr(obj, field, value)
if move:
cur_path = obj.item_dir() if album else obj.path
if lib.directory in ancestry(cur_path): # In library?
log.debug('moving object %s' % cur_path)
if album:
obj.move()
else:
lib.move(obj)
if move:
cur_path = obj.item_dir() if album else obj.path
if lib.directory in ancestry(cur_path): # In library?
log.debug('moving object %s' % cur_path)
if album:
obj.move()
else:
lib.move(obj)
# When modifying items, we have to store them to the database.
if not album:
lib.store(obj)
lib.save()
# When modifying items, we have to store them to the database.
if not album:
lib.store(obj)
# Apply tags if requested.
if write:
@ -1166,7 +1165,6 @@ def move_items(lib, dest, query, copy, album):
else:
lib.move(obj, copy, basedir=dest)
lib.store(obj)
lib.save()
move_cmd = ui.Subcommand('move',
help='move or copy items', aliases=('mv',))

View file

@ -892,14 +892,13 @@ class Server(BaseServer):
def cmd_stats(self, conn):
"""Sends some statistics about the library."""
songs, totaltime = beets.library.TrueQuery().count(self.lib)
with self.lib.transaction() as tx:
songs, totaltime = beets.library.TrueQuery().count(tx)
statement = 'SELECT COUNT(DISTINCT artist), ' \
'COUNT(DISTINCT album) FROM items'
c = self.lib.conn.execute(statement)
result = c.fetchone()
c.close()
artists, albums = result[0], result[1]
statement = 'SELECT COUNT(DISTINCT artist), ' \
'COUNT(DISTINCT album) FROM items'
result = tx.query(statement)[0]
artists, albums = result[0], result[1]
yield (u'artists: ' + unicode(artists),
u'albums: ' + unicode(albums),
@ -996,10 +995,10 @@ class Server(BaseServer):
statement = 'SELECT DISTINCT ' + show_key + \
' FROM items WHERE ' + clause + \
' ORDER BY ' + show_key
c = self.lib.conn.cursor()
c.execute(statement, subvals)
with self.lib.transaction() as tx:
rows = tx.query(statement, subvals)
for row in c:
for row in rows:
yield show_tag_canon + u': ' + unicode(row[0])
def cmd_count(self, conn, tag, value):

View file

@ -166,7 +166,6 @@ def album_imported(lib, album, config):
if genre:
log.debug(u'adding last.fm album genre: %s' % genre)
album.genre = genre
lib.save()
if config.write:
for item in album.items():
@ -180,7 +179,6 @@ def item_imported(lib, item, config):
log.debug(u'adding last.fm item genre: %s' % genre)
item.genre = genre
lib.store(item)
lib.save()
if config.write:
item.write()

View file

@ -177,7 +177,6 @@ def fetch_item_lyrics(lib, loglevel, item, write):
if write:
item.write()
lib.store(item)
lib.save()
AUTOFETCH = True
class LyricsPlugin(BeetsPlugin):

View file

@ -101,6 +101,6 @@ class MPDUpdatePlugin(BeetsPlugin):
options['password'] = \
ui.config_val(config, 'mpdupdate', 'password', '')
@MPDUpdatePlugin.listen('save')
@MPDUpdatePlugin.listen('import')
def update(lib=None):
update_mpd(options['host'], options['port'], options['password'])

View file

@ -59,8 +59,9 @@ def single_item(item_id):
@app.route('/item/')
def all_items():
c = g.lib.conn.execute("SELECT id FROM items")
all_ids = [row[0] for row in c]
with g.lib.transaction() as tx:
rows = tx.query("SELECT id FROM items")
all_ids = [row[0] for row in rows]
return flask.jsonify(item_ids=all_ids)
@app.route('/item/<int:item_id>/file')
@ -84,8 +85,9 @@ def single_album(album_id):
@app.route('/album/')
def all_albums():
c = g.lib.conn.execute("SELECT id FROM albums")
all_ids = [row[0] for row in c]
with g.lib.transaction() as tx:
rows = tx.query("SELECT id FROM albums")
all_ids = [row[0] for row in rows]
return flask.jsonify(album_ids=all_ids)
@app.route('/album/query/<path:query>')

View file

@ -64,6 +64,8 @@ Changelog
Mecucci).
* Filenames are normalized with Unicode Normal Form D (NFD) on Mac OS X and NFC
on all other platforms.
* Significant internal restructuring to avoid SQLite locking errors. As part of
these changes, the not-very-useful "save" plugin event has been removed.
.. _pyacoustid: https://github.com/sampsyo/pyacoustid

View file

@ -116,9 +116,6 @@ currently available are:
* *pluginload*: called after all the plugins have been loaded after the ``beet``
command starts
* *save*: called whenever the library is changed and written to disk (the
``lib`` keyword argument is the Library object that was written)
* *import*: called after a ``beet import`` command fishes (the ``lib`` keyword
argument is a Library object; ``paths`` is a list of paths (strings) that were
imported)

View file

@ -8,7 +8,7 @@
# distribute, sublicense, and/or sell copies of the Software, and to
# permit persons to whom the Software is furnished to do so, subject to
# the following conditions:
#
#
# The above copyright notice and this permission notice shall be
# included in all copies or substantial portions of the Software.
@ -29,10 +29,18 @@ import beets.library
from beets import util
from beets import plugins
TEMP_LIB = os.path.join(_common.RSRC, 'test_copy.blb')
def lib():
return beets.library.Library(os.path.join(_common.RSRC, 'test.blb'))
def boracay(l): return beets.library.Item(l.conn.execute('select * from items '
'where id=3').fetchone())
shutil.copy(os.path.join(_common.RSRC, 'test.blb'), TEMP_LIB)
return beets.library.Library(TEMP_LIB)
def remove_lib():
if os.path.exists(TEMP_LIB):
os.unlink(TEMP_LIB)
def boracay(l):
return beets.library.Item(
l.conn.execute('select * from items where id=3').fetchone()
)
np = util.normpath
class LoadTest(unittest.TestCase):
@ -41,13 +49,14 @@ class LoadTest(unittest.TestCase):
self.i = boracay(self.lib)
def tearDown(self):
self.lib.conn.close()
remove_lib()
def test_load_restores_data_from_db(self):
original_title = self.i.title
self.i.title = 'something'
self.lib.load(self.i)
self.assertEqual(original_title, self.i.title)
def test_load_clears_dirty_flags(self):
self.i.artist = 'something'
self.lib.load(self.i)
@ -59,14 +68,15 @@ class StoreTest(unittest.TestCase):
self.i = boracay(self.lib)
def tearDown(self):
self.lib.conn.close()
remove_lib()
def test_store_changes_database_value(self):
self.i.year = 1987
self.lib.store(self.i)
new_year = self.lib.conn.execute('select year from items where '
'title="Boracay"').fetchone()['year']
self.assertEqual(new_year, 1987)
def test_store_only_writes_dirty_fields(self):
original_genre = self.i.genre
self.i.record['genre'] = 'beatboxing' # change value w/o dirtying
@ -74,7 +84,7 @@ class StoreTest(unittest.TestCase):
new_genre = self.lib.conn.execute('select genre from items where '
'title="Boracay"').fetchone()['genre']
self.assertEqual(new_genre, original_genre)
def test_store_clears_dirty_flags(self):
self.i.composer = 'tvp'
self.lib.store(self.i)
@ -86,27 +96,28 @@ class AddTest(unittest.TestCase):
self.i = item()
def tearDown(self):
self.lib.conn.close()
def test_item_add_inserts_row(self):
self.lib.add(self.i)
new_grouping = self.lib.conn.execute('select grouping from items '
'where composer="the composer"').fetchone()['grouping']
self.assertEqual(new_grouping, self.i.grouping)
def test_library_add_path_inserts_row(self):
i = beets.library.Item.from_path(os.path.join(_common.RSRC, 'full.mp3'))
self.lib.add(i)
new_grouping = self.lib.conn.execute('select grouping from items '
'where composer="the composer"').fetchone()['grouping']
self.assertEqual(new_grouping, self.i.grouping)
class RemoveTest(unittest.TestCase):
def setUp(self):
self.lib = lib()
self.i = boracay(self.lib)
def tearDown(self):
self.lib.conn.close()
remove_lib()
def test_remove_deletes_from_db(self):
self.lib.remove(self.i)
c = self.lib.conn.execute('select * from items where id=3')
@ -115,19 +126,19 @@ class RemoveTest(unittest.TestCase):
class GetSetTest(unittest.TestCase):
def setUp(self):
self.i = item()
def test_set_changes_value(self):
self.i.bpm = 4915
self.assertEqual(self.i.bpm, 4915)
def test_set_sets_dirty_flag(self):
self.i.comp = not self.i.comp
self.assertTrue(self.i.dirty['comp'])
def test_set_does_not_dirty_if_value_unchanged(self):
self.i.title = self.i.title
self.assertTrue(not self.i.dirty['title'])
def test_invalid_field_raises_attributeerror(self):
self.assertRaises(AttributeError, getattr, self.i, 'xyzzy')
@ -137,17 +148,17 @@ class DestinationTest(unittest.TestCase):
self.i = item()
def tearDown(self):
self.lib.conn.close()
def test_directory_works_with_trailing_slash(self):
self.lib.directory = 'one/'
self.lib.path_formats = [('default', 'two')]
self.assertEqual(self.lib.destination(self.i), np('one/two'))
def test_directory_works_without_trailing_slash(self):
self.lib.directory = 'one'
self.lib.path_formats = [('default', 'two')]
self.assertEqual(self.lib.destination(self.i), np('one/two'))
def test_destination_substitues_metadata_values(self):
self.lib.directory = 'base'
self.lib.path_formats = [('default', '$album/$artist $title')]
@ -156,21 +167,21 @@ class DestinationTest(unittest.TestCase):
self.i.album = 'one'
self.assertEqual(self.lib.destination(self.i),
np('base/one/two three'))
def test_destination_preserves_extension(self):
self.lib.directory = 'base'
self.lib.path_formats = [('default', '$title')]
self.i.path = 'hey.audioformat'
self.assertEqual(self.lib.destination(self.i),
np('base/the title.audioformat'))
def test_lower_case_extension(self):
self.lib.directory = 'base'
self.lib.path_formats = [('default', '$title')]
self.i.path = 'hey.MP3'
self.assertEqual(self.lib.destination(self.i),
np('base/the title.mp3'))
def test_destination_pads_some_indices(self):
self.lib.directory = 'base'
self.lib.path_formats = [('default', '$track $tracktotal ' \
@ -191,38 +202,38 @@ class DestinationTest(unittest.TestCase):
self.i.day = 3
self.assertEqual(self.lib.destination(self.i),
np('base/0001-02-03'))
def test_destination_escapes_slashes(self):
self.i.album = 'one/two'
dest = self.lib.destination(self.i)
self.assertTrue('one' in dest)
self.assertTrue('two' in dest)
self.assertFalse('one/two' in dest)
def test_destination_escapes_leading_dot(self):
self.i.album = '.something'
dest = self.lib.destination(self.i)
self.assertTrue('something' in dest)
self.assertFalse('/.' in dest)
def test_destination_preserves_legitimate_slashes(self):
self.i.artist = 'one'
self.i.album = 'two'
dest = self.lib.destination(self.i)
self.assertTrue(os.path.join('one', 'two') in dest)
def test_destination_long_names_truncated(self):
self.i.title = 'X'*300
self.i.artist = 'Y'*300
for c in self.lib.destination(self.i).split(os.path.sep):
self.assertTrue(len(c) <= 255)
def test_destination_long_names_keep_extension(self):
self.i.title = 'X'*300
self.i.path = 'something.extn'
dest = self.lib.destination(self.i)
self.assertEqual(dest[-5:], '.extn')
def test_distination_windows_removes_both_separators(self):
self.i.title = 'one \\ two / three.mp3'
p = self.lib.destination(self.i, ntpath)
@ -230,15 +241,15 @@ class DestinationTest(unittest.TestCase):
self.assertFalse('one / two' in p)
self.assertFalse('two \\ three' in p)
self.assertFalse('two / three' in p)
def test_sanitize_unix_replaces_leading_dot(self):
p = util.sanitize_path(u'one/.two/three', posixpath)
self.assertFalse('.' in p)
def test_sanitize_windows_replaces_trailing_dot(self):
p = util.sanitize_path(u'one/two./three', ntpath)
self.assertFalse('.' in p)
def test_sanitize_windows_replaces_illegal_chars(self):
p = util.sanitize_path(u':*?"<>|', ntpath)
self.assertFalse(':' in p)
@ -261,7 +272,7 @@ class DestinationTest(unittest.TestCase):
self.lib.path_formats = [('default', '$album ($year)/$track $title')]
dest1, dest2 = self.lib.destination(i1), self.lib.destination(i2)
self.assertEqual(os.path.dirname(dest1), os.path.dirname(dest2))
def test_default_path_for_non_compilations(self):
self.i.comp = False
self.lib.add_album([self.i])
@ -435,7 +446,7 @@ class DestinationFunctionTest(unittest.TestCase, PathFormattingMixin):
def test_upper_case_literal(self):
self._setf(u'%upper{foo}')
self._assert_dest('/base/FOO')
def test_upper_case_variable(self):
self._setf(u'%upper{$title}')
self._assert_dest('/base/THE TITLE')
@ -447,7 +458,7 @@ class DestinationFunctionTest(unittest.TestCase, PathFormattingMixin):
def test_left_variable(self):
self._setf(u'%left{$title, 3}')
self._assert_dest('/base/the')
def test_right_variable(self):
self._setf(u'%right{$title,3}')
self._assert_dest('/base/tle')
@ -455,11 +466,11 @@ class DestinationFunctionTest(unittest.TestCase, PathFormattingMixin):
def test_if_false(self):
self._setf(u'x%if{,foo}')
self._assert_dest('/base/x')
def test_if_true(self):
self._setf(u'%if{bar,foo}')
self._assert_dest('/base/foo')
def test_if_else_false(self):
self._setf(u'%if{,foo,baz}')
self._assert_dest('/base/baz')
@ -484,7 +495,7 @@ class DisambiguationTest(unittest.TestCase, PathFormattingMixin):
self.i2 = item()
self.i2.year = 2002
self.lib.add_album([self.i2])
self.lib.save()
self.lib.conn.commit()
self._setf(u'foo%aunique{albumartist album,year}/$title')
@ -497,21 +508,21 @@ class DisambiguationTest(unittest.TestCase, PathFormattingMixin):
def test_unique_with_default_arguments_uses_albumtype(self):
album2 = self.lib.get_album(self.i1)
album2.albumtype = 'bar'
self.lib.save()
self.lib.conn.commit()
self._setf(u'foo%aunique{}/$title')
self._assert_dest('/base/foo [bar]/the title', self.i1)
def test_unique_expands_to_nothing_for_distinct_albums(self):
album2 = self.lib.get_album(self.i2)
album2.album = 'different album'
self.lib.save()
self.lib.conn.commit()
self._assert_dest('/base/foo/the title', self.i1)
def test_use_fallback_numbers_when_identical(self):
album2 = self.lib.get_album(self.i2)
album2.year = 2001
self.lib.save()
self.lib.conn.commit()
self._assert_dest('/base/foo 1/the title', self.i1)
self._assert_dest('/base/foo 2/the title', self.i2)
@ -550,13 +561,13 @@ class PluginDestinationTest(unittest.TestCase):
def test_undefined_value_not_substituted(self):
self._assert_dest('the artist $foo')
def test_plugin_value_not_substituted(self):
self._tv_map = {
'foo': 'bar',
}
self._assert_dest('the artist bar')
def test_plugin_value_overrides_attribute(self):
self._tv_map = {
'artist': 'bar',
@ -568,7 +579,7 @@ class PluginDestinationTest(unittest.TestCase):
'foo': 'bar/baz',
}
self._assert_dest('the artist bar_baz')
class MigrationTest(unittest.TestCase):
"""Tests the ability to change the database schema between
versions.
@ -579,7 +590,7 @@ class MigrationTest(unittest.TestCase):
self.old_fields = self.older_fields + [('field_two', 'int')]
self.new_fields = self.old_fields + [('field_three', 'int')]
self.newer_fields = self.new_fields + [('field_four', 'int')]
# Set up a library with old_fields.
self.libfile = os.path.join(_common.RSRC, 'templib.blb')
old_lib = beets.library.Library(self.libfile,
@ -588,12 +599,12 @@ class MigrationTest(unittest.TestCase):
old_lib.conn.execute(
'insert into items (field_one, field_two) values (4, 2)'
)
old_lib.save()
old_lib.conn.commit()
del old_lib
def tearDown(self):
os.unlink(self.libfile)
def test_open_with_same_fields_leaves_untouched(self):
new_lib = beets.library.Library(self.libfile,
item_fields=self.old_fields)
@ -601,7 +612,7 @@ class MigrationTest(unittest.TestCase):
c.execute("select * from items")
row = c.fetchone()
self.assertEqual(len(row), len(self.old_fields))
def test_open_with_new_field_adds_column(self):
new_lib = beets.library.Library(self.libfile,
item_fields=self.new_fields)
@ -609,7 +620,7 @@ class MigrationTest(unittest.TestCase):
c.execute("select * from items")
row = c.fetchone()
self.assertEqual(len(row), len(self.new_fields))
def test_open_with_fewer_fields_leaves_untouched(self):
new_lib = beets.library.Library(self.libfile,
item_fields=self.older_fields)
@ -617,7 +628,7 @@ class MigrationTest(unittest.TestCase):
c.execute("select * from items")
row = c.fetchone()
self.assertEqual(len(row), len(self.old_fields))
def test_open_with_multiple_new_fields(self):
new_lib = beets.library.Library(self.libfile,
item_fields=self.newer_fields)
@ -689,13 +700,13 @@ class AlbumInfoTest(unittest.TestCase):
ai.artpath = '/my/great/art'
new_ai = self.lib.get_album(self.i)
self.assertEqual(new_ai.artpath, '/my/great/art')
def test_albuminfo_for_two_items_doesnt_duplicate_row(self):
i2 = item()
self.lib.add(i2)
self.lib.get_album(self.i)
self.lib.get_album(i2)
c = self.lib.conn.cursor()
c.execute('select * from albums where album=?', (self.i.album,))
# Cursor should only return one row.
@ -768,11 +779,11 @@ class ArtDestinationTest(unittest.TestCase):
self.i.path = self.lib.destination(self.i)
self.lib.art_filename = 'artimage'
self.ai = self.lib.add_album((self.i,))
def test_art_filename_respects_setting(self):
art = self.ai.art_destination('something.jpg')
self.assert_('%sartimage.jpg' % os.path.sep in art)
def test_art_path_in_item_dir(self):
art = self.ai.art_destination('something.jpg')
track = self.lib.destination(self.i)

View file

@ -292,7 +292,7 @@ class ImportApplyTest(unittest.TestCase, _common.ExtraAsserts):
# First, add the item to the library.
temp_item = library.Item.from_path(self.srcpath)
self.lib.add(temp_item)
self.lib.save()
self.lib.conn.commit()
# Then, re-import the same file.
coro = importer.apply_choices(_common.iconfig(self.lib))
@ -748,7 +748,7 @@ class ArtFetchTest(unittest.TestCase, _common.ExtraAsserts):
self.i = _common.item()
self.i.path = itempath
self.album = self.lib.add_album([self.i])
self.lib.save()
self.lib.conn.commit()
# Set up an art-fetching coroutine.
self.config = _common.iconfig(self.lib)

View file

@ -296,7 +296,6 @@ class MemoryGetTest(unittest.TestCase, AssertsMixin):
def test_unicode_query(self):
self.single_item.title = u'caf\xe9'
self.lib.store(self.single_item)
self.lib.save()
q = u'title:caf\xe9'
results = self.lib.items(q)
@ -410,13 +409,15 @@ class CountTest(unittest.TestCase):
self.lib.add(self.item)
def test_count_gets_single_item(self):
songs, totaltime = beets.library.TrueQuery().count(self.lib)
with self.lib.transaction() as tx:
songs, totaltime = beets.library.TrueQuery().count(tx)
self.assertEqual(songs, 1)
self.assertEqual(totaltime, self.item.length)
def test_count_works_for_empty_library(self):
self.lib.remove(self.item)
songs, totaltime = beets.library.TrueQuery().count(self.lib)
with self.lib.transaction() as tx:
songs, totaltime = beets.library.TrueQuery().count(tx)
self.assertEqual(songs, 0)
self.assertEqual(totaltime, 0.0)

View file

@ -54,7 +54,7 @@ class ListTest(unittest.TestCase):
def test_list_unicode_query(self):
self.item.title = u'na\xefve'
self.lib.store(self.item)
self.lib.save()
self.lib.conn.commit()
commands.list_items(self.lib, [u'na\xefve'], False, False, None)
out = self.io.getoutput()

View file

@ -26,7 +26,6 @@ class VFSTest(unittest.TestCase):
])
self.lib.add(_common.item())
self.lib.add_album([_common.item()])
self.lib.save()
self.tree = vfs.libtree(self.lib)
def test_singleton_item(self):