Merge pull request #959 from geigerzaehler/dbcore-refactor

Refactor DBcore without changing behaviour

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']:

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):
@@ -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.
@@ -160,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].normalize(value)
-        if flex_values:
-            for key, value in flex_values.items():
-                if key in cls._types:
-                    value = cls._types[key].normalize(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):
@@ -208,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
@@ -332,19 +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 + '=?,'
-                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)
+                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.

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,26 +79,45 @@ 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):
+        """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
+
 
 # Reusable types.
 
+class Default(Type):
+    null = None
+
+
 class Integer(Type):
     """A basic 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):
@@ -128,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`.
@@ -151,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):
@@ -165,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))
@@ -175,7 +193,7 @@ class Boolean(Type):
 
 
 # Shared instances of common types.
-BASE_TYPE = Type()
+DEFAULT = Default()
 INTEGER = Integer()
 PRIMARY_ID = Id(True)
 FOREIGN_ID = Id(False)

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),
@@ -89,6 +87,7 @@ class DateType(types.Type):
 class PathType(types.Type):
     sql = u'BLOB'
     query = PathQuery
+    model_type = str
 
     def format(self, value):
         return util.displayable_path(value)
@@ -109,6 +108,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 +192,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()

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