From 4134a5173a6ad641106dd4a09ae662f9a49172a9 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Sun, 14 Sep 2014 15:02:50 +0200 Subject: [PATCH 1/7] Split Type.normalize() responsibilities into normalize() and from_sql() Previously `normalize()` was used to convert values set by the model API consumer and values received from the database. These are two orthogonal uses. The commit does not change behaviour since the `from_sql()` method uses the `normalize()` implementation. --- beets/dbcore/db.py | 4 ++-- beets/dbcore/types.py | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 0ec24dfd6..fc5820173 100644 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -169,11 +169,11 @@ class Model(object): obj = cls(db) if fixed_values: for key, value in fixed_values.items(): - obj._values_fixed[key] = cls._fields[key].normalize(value) + obj._values_fixed[key] = cls._fields[key].from_sql(value) if flex_values: for key, value in flex_values.items(): if key in cls._types: - value = cls._types[key].normalize(value) + value = cls._types[key].from_sql(value) obj._values_flex[key] = value return obj diff --git a/beets/dbcore/types.py b/beets/dbcore/types.py index 3c56121d0..28e28f596 100644 --- a/beets/dbcore/types.py +++ b/beets/dbcore/types.py @@ -69,6 +69,12 @@ class Type(object): else: return value + def from_sql(self, sql_value): + """Convert a value received from the database adapter to a value + stored in the model object. + """ + return self.normalize(sql_value) + # Reusable types. From 4cfb59bfba27ff2f1ac7aff44a963167dbe51ebf Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Sun, 14 Sep 2014 15:11:59 +0200 Subject: [PATCH 2/7] Add Type.to_sql() and simplify model code Instead of encoding the conversion behaviour in the model class (via the `_bytes_keys` attribute) we define it on the type. This gives us a more extensible interface and separates logic. This should not change any behaviour (as one can see by closely staring at the code). --- beets/dbcore/db.py | 11 +---------- beets/dbcore/types.py | 6 ++++++ beets/library.py | 6 +++++- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index fc5820173..070f7104d 100644 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -115,11 +115,6 @@ class Model(object): keys are field names and the values are `Type` objects. """ - _bytes_keys = () - """Keys whose values should be stored as raw bytes blobs rather than - strings. - """ - _search_fields = () """The fields that should be queried by default by unqualified query terms. @@ -338,11 +333,7 @@ class Model(object): if key != 'id' and key in self._dirty: self._dirty.remove(key) assignments += key + '=?,' - value = self[key] - # Wrap path strings in buffers so they get stored - # "in the raw". - if key in self._bytes_keys and isinstance(value, str): - value = buffer(value) + value = self._type(key).to_sql(self[key]) subvars.append(value) assignments = assignments[:-1] # Knock off last , diff --git a/beets/dbcore/types.py b/beets/dbcore/types.py index 28e28f596..2738f3425 100644 --- a/beets/dbcore/types.py +++ b/beets/dbcore/types.py @@ -75,6 +75,12 @@ class Type(object): """ return self.normalize(sql_value) + def to_sql(self, model_value): + """Convert a value as stored in the model object to a value used + by the database adapter. + """ + return model_value + # Reusable types. diff --git a/beets/library.py b/beets/library.py index 3a6829176..595eaa833 100644 --- a/beets/library.py +++ b/beets/library.py @@ -109,6 +109,11 @@ class PathType(types.Type): else: return value + def to_sql(self, value): + if isinstance(value, str): + value = buffer(value) + return value + class MusicalKey(types.String): """String representing the musical key of a song. @@ -188,7 +193,6 @@ class WriteError(FileOperationError): class LibModel(dbcore.Model): """Shared concrete functionality for Items and Albums. """ - _bytes_keys = ('path', 'artpath') def _template_funcs(self): funcs = DefaultTemplateFunctions(self, self._db).functions() From 19d3ca822ef539ac99ec9143d97e625c34c94516 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Sun, 14 Sep 2014 15:36:25 +0200 Subject: [PATCH 3/7] Minor simplifications in Model class --- beets/dbcore/db.py | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 070f7104d..307f044f9 100644 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -155,21 +155,17 @@ class Model(object): self.clear_dirty() @classmethod - def _awaken(cls, db=None, fixed_values=None, flex_values=None): + def _awaken(cls, db=None, fixed_values={}, flex_values={}): """Create an object with values drawn from the database. This is a performance optimization: the checks involved with ordinary construction are bypassed. """ obj = cls(db) - if fixed_values: - for key, value in fixed_values.items(): - obj._values_fixed[key] = cls._fields[key].from_sql(value) - if flex_values: - for key, value in flex_values.items(): - if key in cls._types: - value = cls._types[key].from_sql(value) - obj._values_flex[key] = value + for key, value in fixed_values.iteritems(): + obj._values_fixed[key] = cls._type(key).from_sql(value) + for key, value in flex_values.iteritems(): + obj._values_flex[key] = cls._type(key).from_sql(value) return obj def __repr__(self): @@ -327,15 +323,15 @@ class Model(object): self._check_db() # Build assignments for query. - assignments = '' + assignments = [] subvars = [] for key in self._fields: if key != 'id' and key in self._dirty: self._dirty.remove(key) - assignments += key + '=?,' + assignments.append(key + '=?') value = self._type(key).to_sql(self[key]) subvars.append(value) - assignments = assignments[:-1] # Knock off last , + assignments = ','.join(assignments) with self._db.transaction() as tx: # Main table update. From 492cf389278784db0e030b00efef0bbd61ba1cc6 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Sun, 14 Sep 2014 15:47:20 +0200 Subject: [PATCH 4/7] Use item[field] instead of getattr(item, field) --- beets/autotag/match.py | 8 ++++---- beetsplug/zero.py | 7 +++---- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/beets/autotag/match.py b/beets/autotag/match.py index aa0c21dba..4232c719d 100644 --- a/beets/autotag/match.py +++ b/beets/autotag/match.py @@ -65,10 +65,10 @@ def current_metadata(items): fields = ['artist', 'album', 'albumartist', 'year', 'disctotal', 'mb_albumid', 'label', 'catalognum', 'country', 'media', 'albumdisambig'] - for key in fields: - values = [getattr(item, key) for item in items if item] - likelies[key], freq = plurality(values) - consensus[key] = (freq == len(values)) + for field in fields: + values = [item[field] for item in items if item] + likelies[field], freq = plurality(values) + consensus[field] = (freq == len(values)) # If there's an album artist consensus, use this for the artist. if consensus['albumartist'] and likelies['albumartist']: diff --git a/beetsplug/zero.py b/beetsplug/zero.py index 30f287aea..b9768431c 100644 --- a/beetsplug/zero.py +++ b/beetsplug/zero.py @@ -85,12 +85,11 @@ class ZeroPlugin(BeetsPlugin): return for field, patterns in self.patterns.items(): - try: - value = getattr(item, field) - except AttributeError: + if field not in item.keys(): log.error(u'[zero] no such field: {0}'.format(field)) continue + value = item[field] if self.match_patterns(value, patterns): log.debug(u'[zero] {0}: {1} -> None'.format(field, value)) - setattr(item, field, None) + item[field] = None From 629fc0087d4f23185d75bc3fcc23937840a3df15 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Sun, 14 Sep 2014 16:17:12 +0200 Subject: [PATCH 5/7] DateType inherits from Float type --- beets/library.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/beets/library.py b/beets/library.py index 595eaa833..c6c48d7cc 100644 --- a/beets/library.py +++ b/beets/library.py @@ -61,12 +61,10 @@ class PathQuery(dbcore.FieldQuery): # Library-specific field types. -class DateType(types.Type): +class DateType(types.Float): # TODO representation should be `datetime` object # TODO distinguish beetween date and time types - sql = u'REAL' query = dbcore.query.DateQuery - null = 0.0 def format(self, value): return time.strftime(beets.config['time_format'].get(unicode), From 30addc12b4e5d8195da4692364094dfd40928e9a Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Sun, 14 Sep 2014 16:47:21 +0200 Subject: [PATCH 6/7] Do not use abstract base class instance We want to move common code into the base class with out changing the behaviour of the default type. --- beets/dbcore/db.py | 4 ++-- beets/dbcore/types.py | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 307f044f9..b2d2df360 100644 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -24,8 +24,8 @@ import collections import beets from beets.util.functemplate import Template +from beets.dbcore import types from .query import MatchQuery, NullSort -from .types import BASE_TYPE class FormattedMapping(collections.Mapping): @@ -199,7 +199,7 @@ class Model(object): If the field has no explicit type, it is given the base `Type`, which does no conversion. """ - return self._fields.get(key) or self._types.get(key) or BASE_TYPE + return self._fields.get(key) or self._types.get(key) or types.DEFAULT def __getitem__(self, key): """Get the value for a field. Raise a KeyError if the field is diff --git a/beets/dbcore/types.py b/beets/dbcore/types.py index 2738f3425..f96863f9c 100644 --- a/beets/dbcore/types.py +++ b/beets/dbcore/types.py @@ -84,6 +84,10 @@ class Type(object): # Reusable types. +class Default(Type): + pass + + class Integer(Type): """A basic integer type. """ @@ -187,7 +191,7 @@ class Boolean(Type): # Shared instances of common types. -BASE_TYPE = Type() +DEFAULT = Default() INTEGER = Integer() PRIMARY_ID = Id(True) FOREIGN_ID = Id(False) From fafc56c95b6b2d8cdcb6e8511acf4189944de9f9 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Sun, 14 Sep 2014 17:02:58 +0200 Subject: [PATCH 7/7] Unify similar code from Type subclasses in base class Instead of implementing the `parse()` and `format()` methods and setting the `null` attribute in subclasses, we set the `model_type` field and the generic implementations of the two methods take care of the rest. Again: No change in behaviour! --- beets/dbcore/types.py | 80 ++++++++++++++++++++++--------------------- beets/library.py | 1 + 2 files changed, 42 insertions(+), 39 deletions(-) diff --git a/beets/dbcore/types.py b/beets/dbcore/types.py index f96863f9c..2cf51bc81 100644 --- a/beets/dbcore/types.py +++ b/beets/dbcore/types.py @@ -34,30 +34,42 @@ class Type(object): """The `Query` subclass to be used when querying the field. """ - null = None - """The value to be exposed when the underlying value is None. + model_type = unicode + """The python type that is used to represent the value in the model. + + The model is guaranteed to return a value of this type if the field + is accessed. To this end, the constructor is used by the `normalize` + and `from_sql` methods and the `default` property. """ + @property + def null(self): + """The value to be exposed when the underlying value is None. + """ + return self.model_type() + def format(self, value): """Given a value of this type, produce a Unicode string representing the value. This is used in template evaluation. """ # Fallback formatter. Convert to Unicode at all cost. if value is None: - return u'' - elif isinstance(value, basestring): - if isinstance(value, bytes): - return value.decode('utf8', 'ignore') - else: - return value - else: - return unicode(value) + value = self.null + if value is None: + value = u'' + if isinstance(value, bytes): + value = value.decode('utf8', 'ignore') + + return unicode(value) def parse(self, string): """Parse a (possibly human-written) string and return the indicated value of this type. """ - return string + try: + return self.model_type(string) + except ValueError: + return self.null def normalize(self, value): """Given a value that will be assigned into a field of this @@ -67,17 +79,29 @@ class Type(object): if value is None: return self.null else: + # TODO This should eventually be replaced by + # `self.model_type(value)` return value def from_sql(self, sql_value): - """Convert a value received from the database adapter to a value - stored in the model object. + """Receives the value stored in the SQL backend and return the + value to be stored in the model. + + For fixed fields the type of `value` is determined by the column + type given in the `sql` property and the SQL to Python mapping + given here: + https://docs.python.org/2/library/sqlite3.html#sqlite-and-python-types + + For flexible field the value is a unicode object. The method + must therefore be able to parse them. """ return self.normalize(sql_value) def to_sql(self, model_value): """Convert a value as stored in the model object to a value used by the database adapter. + For flexible field the value is a unicode object. The method + must therefore be able to parse them. """ return model_value @@ -85,7 +109,7 @@ class Type(object): # Reusable types. class Default(Type): - pass + null = None class Integer(Type): @@ -93,16 +117,7 @@ class Integer(Type): """ sql = u'INTEGER' query = query.NumericQuery - null = 0 - - def format(self, value): - return unicode(value or 0) - - def parse(self, string): - try: - return int(string) - except ValueError: - return 0 + model_type = int class PaddedInt(Integer): @@ -144,17 +159,11 @@ class Float(Type): """ sql = u'REAL' query = query.NumericQuery - null = 0.0 + model_type = float def format(self, value): return u'{0:.1f}'.format(value or 0.0) - def parse(self, string): - try: - return float(string) - except ValueError: - return 0.0 - class NullFloat(Float): """Same as `Float`, but does not normalize `None` to `0.0`. @@ -167,13 +176,6 @@ class String(Type): """ sql = u'TEXT' query = query.SubstringQuery - null = u'' - - def format(self, value): - return unicode(value) if value else u'' - - def parse(self, string): - return string class Boolean(Type): @@ -181,7 +183,7 @@ class Boolean(Type): """ sql = u'INTEGER' query = query.BooleanQuery - null = False + model_type = bool def format(self, value): return unicode(bool(value)) diff --git a/beets/library.py b/beets/library.py index c6c48d7cc..105da3631 100644 --- a/beets/library.py +++ b/beets/library.py @@ -87,6 +87,7 @@ class DateType(types.Float): class PathType(types.Type): sql = u'BLOB' query = PathQuery + model_type = str def format(self, value): return util.displayable_path(value)