From 335247481f0bfeafe14b25035db5a9b299fb21d9 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Tue, 6 May 2014 11:13:39 +0200 Subject: [PATCH] Make _formatted_mapping lazy and an instance The motivation for this is to increase the performance of template evalutation. Previously, the `model._formatted_mapping()` method returned a dictionary that served as the environment (mapping of variable names to values) when evaluating a template. To populate the dictionary, we iterated over all keys in the model, formatted the values, and assigned it to the dictionary. This meant we formatted every field, even if the template did not require it. With this commit `_formatted_mapping()` does not return a populated dictionary but a proxy (or view) instance. The object only knows about the model and the keys it provides the formatted view for. If a variable is requested by the template it computes the formatted value on the fly. The class-based approach has one additional advantage: In the future, we can separate the formatting logic from the database logic. --- beets/dbcore/db.py | 31 +++++++++++++++++++----- beets/library.py | 60 +++++++++++++++++++++++++++++++++------------- 2 files changed, 68 insertions(+), 23 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 3901107f3..63a668601 100644 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -390,12 +390,7 @@ class Model(object): """Get a mapping containing all values on this object formatted as human-readable strings. """ - # In the future, this could be made "lazy" to avoid computing - # fields unnecessarily. - out = {} - for key in self.keys(True): - out[key] = self._get_formatted(key, for_path) - return out + return FormattedMapping(self, for_path) def evaluate_template(self, template, for_path=False): """Evaluate a template (a string or a `Template` object) using @@ -430,6 +425,30 @@ class Model(object): return string +class FormattedMapping(object): + """A `dict`-like formatted view of a model. + + The accessor ``mapping[key]`` returns the formated version of + ``model[key]``. The formatting is handled by `model._format()`. + """ + # TODO Move all formatting logic here + # TODO Add caching + + def __init__(self, model, for_path=False): + self.for_path = for_path + self.model = model + self.model_keys = model.keys(True) + + def __getitem__(self, key): + if key in self.model_keys: + return self.model._get_formatted(key, self.for_path) + else: + raise KeyError(key) + + def __contains__(self, key): + return key in self.model_keys + + # Database controller and supporting interfaces. class Results(object): diff --git a/beets/library.py b/beets/library.py index 1df2b337e..3a2ccd62c 100644 --- a/beets/library.py +++ b/beets/library.py @@ -485,23 +485,7 @@ class Item(LibModel): """Get a mapping containing string-formatted values from either this item or the associated album, if any. """ - mapping = super(Item, self)._formatted_mapping(for_path) - - # Merge in album-level fields. - album = self.get_album() - if album: - for key in album.keys(True): - if key in Album.item_keys or key not in self._fields.keys(): - mapping[key] = album._get_formatted(key, for_path) - - # Use the album artist if the track artist is not set and - # vice-versa. - if not mapping['artist']: - mapping['artist'] = mapping['albumartist'] - if not mapping['albumartist']: - mapping['albumartist'] = mapping['artist'] - - return mapping + return FormattedItemMapping(self, for_path) def destination(self, fragment=False, basedir=None, platform=None, path_formats=None): @@ -573,6 +557,48 @@ class Item(LibModel): return normpath(os.path.join(basedir, subpath)) +class FormattedItemMapping(dbcore.db.FormattedMapping): + """A `dict`-like formatted view of an item that inherits album fields. + + The accessor ``mapping[key]`` returns the formated version of either + ``item[key]`` or ``album[key]``. Here `album` is the album + associated to `item` if it exists. + """ + # FIXME This class should be in the same module as `FormattedMapping` + + def __init__(self, item, for_path=False): + self.for_path = for_path + self.model = item + self.model_keys = item.keys(True) + self.album = item.get_album() + self.album_keys = [] + if self.album: + for key in self.album.keys(True): + if key in Album.item_keys or key not in item._fields.keys(): + self.album_keys.append(key) + + def get(self, key): + if key in self.album_keys: + return self.album._get_formatted(key, self.for_path) + elif key in self.model_keys: + return self.model._get_formatted(key, self.for_path) + else: + raise KeyError(key) + + def __getitem__(self, key): + value = self.get(key) + + if key == 'artist' and not value: + return self.get('albumartist') + if key == 'albumartist' and not value: + return self.get('artist') + + return value + + def __contains__(self, key): + return key in self.model_keys or key in self.album_keys + + class Album(LibModel): """Provides access to information about albums stored in a library. Reflects the library's "albums" table, including album