Merge pull request #1160 from adamcik/feature/model-corner-case-handling

Move new models features to new class
This commit is contained in:
Stein Magnus Jodal 2015-05-05 08:08:05 +02:00
commit 746c3059ba
8 changed files with 378 additions and 108 deletions

View File

@ -77,8 +77,29 @@ Data model helpers
.. autoclass:: mopidy.models.ImmutableObject
:members:
.. autoclass:: mopidy.models.Field
.. autoclass:: mopidy.models.ValidatedImmutableObject
:members: replace
Data model (de)serialization
----------------------------
.. autofunction:: mopidy.models.model_json_decoder
.. autoclass:: mopidy.models.ModelJSONEncoder
.. autofunction:: mopidy.models.model_json_decoder
Data model field types
----------------------
.. autoclass:: mopidy.models.fields.Field
.. autoclass:: mopidy.models.fields.String
.. autoclass:: mopidy.models.fields.Identifier
.. autoclass:: mopidy.models.fields.URI
.. autoclass:: mopidy.models.fields.Date
.. autoclass:: mopidy.models.fields.Integer
.. autoclass:: mopidy.models.fields.Collection

View File

@ -1,15 +1,16 @@
from __future__ import absolute_import, unicode_literals
from mopidy.models import fields
from mopidy.models.immutable import ImmutableObject
from mopidy.models.immutable import ImmutableObject, ValidatedImmutableObject
from mopidy.models.serialize import ModelJSONEncoder, model_json_decoder
__all__ = [
'ImmutableObject', 'Ref', 'Image', 'Artist', 'Album', 'track', 'TlTrack',
'Playlist', 'SearchResult', 'model_json_decoder', 'ModelJSONEncoder']
'Playlist', 'SearchResult', 'model_json_decoder', 'ModelJSONEncoder',
'ValidatedImmutableObject']
class Ref(ImmutableObject):
class Ref(ValidatedImmutableObject):
"""
Model to represent URI references with a human friendly name and type
@ -81,7 +82,7 @@ class Ref(ImmutableObject):
return cls(**kwargs)
class Image(ImmutableObject):
class Image(ValidatedImmutableObject):
"""
:param string uri: URI of the image
@ -99,7 +100,7 @@ class Image(ImmutableObject):
height = fields.Integer(min=0)
class Artist(ImmutableObject):
class Artist(ValidatedImmutableObject):
"""
:param uri: artist URI
@ -120,7 +121,7 @@ class Artist(ImmutableObject):
musicbrainz_id = fields.Identifier()
class Album(ImmutableObject):
class Album(ValidatedImmutableObject):
"""
:param uri: album URI
@ -169,7 +170,7 @@ class Album(ImmutableObject):
# actual usage of this field with more than one image.
class Track(ImmutableObject):
class Track(ValidatedImmutableObject):
"""
:param uri: track URI
@ -253,7 +254,7 @@ class Track(ImmutableObject):
last_modified = fields.Integer(min=0)
class TlTrack(ImmutableObject):
class TlTrack(ValidatedImmutableObject):
"""
A tracklist track. Wraps a regular track and it's tracklist ID.
@ -292,7 +293,7 @@ class TlTrack(ImmutableObject):
return iter([self.tlid, self.track])
class Playlist(ImmutableObject):
class Playlist(ValidatedImmutableObject):
"""
:param uri: playlist URI
@ -329,7 +330,7 @@ class Playlist(ImmutableObject):
return len(self.tracks)
class SearchResult(ImmutableObject):
class SearchResult(ValidatedImmutableObject):
"""
:param uri: search result URI

View File

@ -4,8 +4,9 @@ from __future__ import absolute_import, unicode_literals
class Field(object):
"""
Base field for use in :class:`ImmutableObject`. These fields are
responsible for type checking and other data sanitation in our models.
Base field for use in
:class:`~mopidy.models.immutable.ValidatedImmutableObject`. These fields
are responsible for type checking and other data sanitation in our models.
For simplicity fields use the Python descriptor protocol to store the
values in the instance dictionary. Also note that fields are mutable if
@ -19,7 +20,7 @@ class Field(object):
"""
def __init__(self, default=None, type=None, choices=None):
self._name = None # Set by ImmutableObjectMeta
self._name = None # Set by ValidatedImmutableObjectMeta
self._choices = choices
self._default = default
self._type = type
@ -72,20 +73,41 @@ class String(Field):
class Date(String):
"""
:class:`Field` for storing ISO 8601 dates as a string.
Supported formats are ``YYYY-MM-DD``, ``YYYY-MM`` and ``YYYY``, currently
not validated.
:param default: default value for field
"""
pass # TODO: make this check for YYYY-MM-DD, YYYY-MM, YYYY using strptime.
class Identifier(String):
"""
:class:`Field` for storing ASCII values such as GUIDs or other identifiers.
Values will be interned.
:param default: default value for field
"""
def validate(self, value):
return intern(str(super(Identifier, self).validate(value)))
class URI(Identifier):
"""
:class`Field` for storing URIs
Values will be interned, currently not validated.
:param default: default value for field
"""
pass # TODO: validate URIs?
class Integer(Field):
"""
:class:`Field` for storing integer numbers.
@ -111,7 +133,6 @@ class Integer(Field):
class Collection(Field):
"""
:class:`Field` for storing collections of a given type.

View File

@ -1,7 +1,6 @@
from __future__ import absolute_import, unicode_literals
import copy
import inspect
import itertools
import weakref
@ -9,74 +8,54 @@ from mopidy.models.fields import Field
from mopidy.utils import deprecation
class ImmutableObjectMeta(type):
"""Helper to automatically assign field names to descriptors."""
def __new__(cls, name, bases, attrs):
fields = {}
for key, value in attrs.items():
if isinstance(value, Field):
fields[key] = '_' + key
value._name = key
attrs['_fields'] = fields
attrs['_instances'] = weakref.WeakValueDictionary()
attrs['__slots__'] = list(attrs.get('__slots__', []))
attrs['__slots__'].extend(['_hash'] + fields.values())
for ancestor in [b for base in bases for b in inspect.getmro(base)]:
if '__weakref__' in getattr(ancestor, '__slots__', []):
break
else:
attrs['__slots__'].append('__weakref__')
return super(ImmutableObjectMeta, cls).__new__(cls, name, bases, attrs)
def __call__(cls, *args, **kwargs): # noqa: N805
instance = super(ImmutableObjectMeta, cls).__call__(*args, **kwargs)
return cls._instances.setdefault(weakref.ref(instance), instance)
class ImmutableObject(object):
"""
Superclass for immutable objects whose fields can only be modified via the
constructor. Fields should be :class:`Field` instances to ensure type
safety in our models.
constructor.
Note that since these models can not be changed, we heavily memoize them
to save memory. So constructing a class with the same arguments twice will
give you the same instance twice.
This version of this class has been retained to avoid breaking any clients
relying on it's behavior. Internally in Mopidy we now use
:class:`ValidatedImmutableObject` for type safety and it's much smaller
memory footprint.
:param kwargs: kwargs to set as fields on the object
:type kwargs: any
"""
__metaclass__ = ImmutableObjectMeta
# Any sub-classes that don't set slots won't be effected by the base using
# slots as they will still get an instance dict.
__slots__ = ['__weakref__']
def __init__(self, *args, **kwargs):
for key, value in kwargs.items():
if key not in self._fields:
if not self._is_valid_field(key):
raise TypeError(
'__init__() got an unexpected keyword argument "%s"' %
key)
super(ImmutableObject, self).__setattr__(key, value)
'__init__() got an unexpected keyword argument "%s"' % key)
self._set_field(key, value)
def __setattr__(self, name, value):
if name in self.__slots__:
return super(ImmutableObject, self).__setattr__(name, value)
raise AttributeError('Object is immutable.')
if name.startswith('_'):
object.__setattr__(self, name, value)
else:
raise AttributeError('Object is immutable.')
def __delattr__(self, name):
if name in self.__slots__:
return super(ImmutableObject, self).__delattr__(name)
raise AttributeError('Object is immutable.')
if name.startswith('_'):
object.__delattr__(self, name)
else:
raise AttributeError('Object is immutable.')
def _is_valid_field(self, name):
return hasattr(self, name) and not callable(getattr(self, name))
def _set_field(self, name, value):
if value == getattr(self.__class__, name):
self.__dict__.pop(name, None)
else:
self.__dict__[name] = value
def _items(self):
for field, key in self._fields.items():
if hasattr(self, key):
yield field, getattr(self, key)
return self.__dict__.iteritems()
def __repr__(self):
kwarg_pairs = []
@ -92,12 +71,10 @@ class ImmutableObject(object):
}
def __hash__(self):
if not hasattr(self, '_hash'):
hash_sum = 0
for key, value in self._items():
hash_sum += hash(key) + hash(value)
super(ImmutableObject, self).__setattr__('_hash', hash_sum)
return self._hash
hash_sum = 0
for key, value in self._items():
hash_sum += hash(key) + hash(value)
return hash_sum
def __eq__(self, other):
if not isinstance(other, self.__class__):
@ -111,11 +88,108 @@ class ImmutableObject(object):
def copy(self, **values):
"""
.. deprecated:: 1.1
Use :meth:`replace` instead. Note that we no longer return copies.
Use :meth:`replace` instead.
"""
deprecation.warn('model.immutable.copy')
return self.replace(**values)
def replace(self, **kwargs):
"""
Replace the fields in the model and return a new instance
Examples::
# Returns a track with a new name
Track(name='foo').replace(name='bar')
# Return an album with a new number of tracks
Album(num_tracks=2).replace(num_tracks=5)
:param kwargs: kwargs to set as fields on the object
:type kwargs: any
:rtype: instance of the model with replaced fields
"""
other = copy.copy(self)
for key, value in kwargs.items():
if not self._is_valid_field(key):
raise TypeError(
'copy() got an unexpected keyword argument "%s"' % key)
other._set_field(key, value)
return other
def serialize(self):
data = {}
data['__model__'] = self.__class__.__name__
for key, value in self._items():
if isinstance(value, (set, frozenset, list, tuple)):
value = [
v.serialize() if isinstance(v, ImmutableObject) else v
for v in value]
elif isinstance(value, ImmutableObject):
value = value.serialize()
if not (isinstance(value, list) and len(value) == 0):
data[key] = value
return data
class _ValidatedImmutableObjectMeta(type):
"""Helper that initializes fields, slots and memoizes instance creation."""
def __new__(cls, name, bases, attrs):
fields = {}
for base in bases: # Copy parent fields over to our state
fields.update(getattr(base, '_fields', {}))
for key, value in attrs.items(): # Add our own fields
if isinstance(value, Field):
fields[key] = '_' + key
value._name = key
attrs['_fields'] = fields
attrs['_instances'] = weakref.WeakValueDictionary()
attrs['__slots__'] = list(attrs.get('__slots__', [])) + fields.values()
return super(_ValidatedImmutableObjectMeta, cls).__new__(
cls, name, bases, attrs)
def __call__(cls, *args, **kwargs): # noqa: N805
instance = super(_ValidatedImmutableObjectMeta, cls).__call__(
*args, **kwargs)
return cls._instances.setdefault(weakref.ref(instance), instance)
class ValidatedImmutableObject(ImmutableObject):
"""
Superclass for immutable objects whose fields can only be modified via the
constructor. Fields should be :class:`Field` instances to ensure type
safety in our models.
Note that since these models can not be changed, we heavily memoize them
to save memory. So constructing a class with the same arguments twice will
give you the same instance twice.
"""
__metaclass__ = _ValidatedImmutableObjectMeta
__slots__ = ['_hash']
def __hash__(self):
if not hasattr(self, '_hash'):
hash_sum = super(ValidatedImmutableObject, self).__hash__()
object.__setattr__(self, '_hash', hash_sum)
return self._hash
def _is_valid_field(self, name):
return name in self._fields
def _set_field(self, name, value):
object.__setattr__(self, name, value)
def _items(self):
for field, key in self._fields.items():
if hasattr(self, key):
yield field, getattr(self, key)
def replace(self, **kwargs):
"""
Replace the fields in the model and return a new instance
@ -137,25 +211,7 @@ class ImmutableObject(object):
"""
if not kwargs:
return self
other = copy.copy(self)
for key, value in kwargs.items():
if key not in self._fields:
raise TypeError(
'copy() got an unexpected keyword argument "%s"' % key)
super(ImmutableObject, other).__setattr__(key, value)
super(ImmutableObject, other).__delattr__('_hash')
other = super(ValidatedImmutableObject, self).replace(**kwargs)
if hasattr(self, '_hash'):
object.__delattr__(other, '_hash')
return self._instances.setdefault(weakref.ref(other), other)
def serialize(self):
data = {}
data['__model__'] = self.__class__.__name__
for key, value in self._items():
if isinstance(value, (set, frozenset, list, tuple)):
value = [
v.serialize() if isinstance(v, ImmutableObject) else v
for v in value]
elif isinstance(value, ImmutableObject):
value = value.serialize()
if not (isinstance(value, list) and len(value) == 0):
data[key] = value
return data

View File

@ -2,7 +2,9 @@ from __future__ import absolute_import, unicode_literals
import json
from mopidy.models.immutable import ImmutableObject
from mopidy.models import immutable
_MODELS = ['Ref', 'Artist', 'Album', 'Track', 'TlTrack', 'Playlist']
class ModelJSONEncoder(json.JSONEncoder):
@ -19,7 +21,7 @@ class ModelJSONEncoder(json.JSONEncoder):
"""
def default(self, obj):
if isinstance(obj, ImmutableObject):
if isinstance(obj, immutable.ImmutableObject):
return obj.serialize()
return json.JSONEncoder.default(self, obj)
@ -38,8 +40,8 @@ def model_json_decoder(dct):
"""
if '__model__' in dct:
models = {c.__name__: c for c in ImmutableObject.__subclasses__()}
from mopidy import models
model_name = dct.pop('__model__')
if model_name in models:
return models[model_name](**dct)
if model_name in _MODELS:
return getattr(models, model_name)(**dct)
return dct

View File

@ -3,15 +3,14 @@ from __future__ import absolute_import, unicode_literals
import unittest
from mopidy.models.fields import * # noqa: F403
from mopidy.models.immutable import ImmutableObjectMeta
def create_instance(field):
"""Create an instance of a dummy class for testing fields."""
class Dummy(object):
__metaclass__ = ImmutableObjectMeta
attr = field
attr._name = 'attr'
return Dummy()

164
tests/models/test_legacy.py Normal file
View File

@ -0,0 +1,164 @@
from __future__ import absolute_import, unicode_literals
import unittest
from mopidy.models import ImmutableObject
class Model(ImmutableObject):
uri = None
name = None
models = frozenset()
def __init__(self, *args, **kwargs):
self.__dict__['models'] = frozenset(kwargs.pop('models', None) or [])
super(Model, self).__init__(self, *args, **kwargs)
class SubModel(ImmutableObject):
uri = None
name = None
class GenericCopyTest(unittest.TestCase):
def compare(self, orig, other):
self.assertEqual(orig, other)
self.assertNotEqual(id(orig), id(other))
def test_copying_model(self):
model = Model()
self.compare(model, model.replace())
def test_copying_model_with_basic_values(self):
model = Model(name='foo', uri='bar')
other = model.replace(name='baz')
self.assertEqual('baz', other.name)
self.assertEqual('bar', other.uri)
def test_copying_model_with_missing_values(self):
model = Model(uri='bar')
other = model.replace(name='baz')
self.assertEqual('baz', other.name)
self.assertEqual('bar', other.uri)
def test_copying_model_with_private_internal_value(self):
model = Model(models=[SubModel(name=123)])
other = model.replace(models=[SubModel(name=345)])
self.assertIn(SubModel(name=345), other.models)
def test_copying_model_with_invalid_key(self):
with self.assertRaises(TypeError):
Model().replace(invalid_key=True)
def test_copying_model_to_remove(self):
model = Model(name='foo').replace(name=None)
self.assertEqual(model, Model())
class ModelTest(unittest.TestCase):
def test_uri(self):
uri = 'an_uri'
model = Model(uri=uri)
self.assertEqual(model.uri, uri)
with self.assertRaises(AttributeError):
model.uri = None
def test_name(self):
name = 'a name'
model = Model(name=name)
self.assertEqual(model.name, name)
with self.assertRaises(AttributeError):
model.name = None
def test_submodels(self):
models = [SubModel(name=123), SubModel(name=456)]
model = Model(models=models)
self.assertEqual(set(model.models), set(models))
with self.assertRaises(AttributeError):
model.models = None
def test_models_none(self):
self.assertEqual(set(), Model(models=None).models)
def test_invalid_kwarg(self):
with self.assertRaises(TypeError):
Model(foo='baz')
def test_repr_without_models(self):
self.assertEqual(
"Model(name=u'name', uri=u'uri')",
repr(Model(uri='uri', name='name')))
def test_repr_with_models(self):
self.assertEqual(
"Model(models=[SubModel(name=123)], name=u'name', uri=u'uri')",
repr(Model(uri='uri', name='name', models=[SubModel(name=123)])))
def test_serialize_without_models(self):
self.assertDictEqual(
{'__model__': 'Model', 'uri': 'uri', 'name': 'name'},
Model(uri='uri', name='name').serialize())
def test_serialize_with_models(self):
submodel = SubModel(name=123)
self.assertDictEqual(
{'__model__': 'Model', 'uri': 'uri', 'name': 'name',
'models': [submodel.serialize()]},
Model(uri='uri', name='name', models=[submodel]).serialize())
def test_eq_uri(self):
model1 = Model(uri='uri1')
model2 = Model(uri='uri1')
self.assertEqual(model1, model2)
self.assertEqual(hash(model1), hash(model2))
def test_eq_name(self):
model1 = Model(name='name1')
model2 = Model(name='name1')
self.assertEqual(model1, model2)
self.assertEqual(hash(model1), hash(model2))
def test_eq_models(self):
models = [SubModel()]
model1 = Model(models=models)
model2 = Model(models=models)
self.assertEqual(model1, model2)
self.assertEqual(hash(model1), hash(model2))
def test_eq_models_order(self):
submodel1 = SubModel(name='name1')
submodel2 = SubModel(name='name2')
model1 = Model(models=[submodel1, submodel2])
model2 = Model(models=[submodel2, submodel1])
self.assertEqual(model1, model2)
self.assertEqual(hash(model1), hash(model2))
def test_eq_none(self):
self.assertNotEqual(Model(), None)
def test_eq_other(self):
self.assertNotEqual(Model(), 'other')
def test_ne_uri(self):
model1 = Model(uri='uri1')
model2 = Model(uri='uri2')
self.assertNotEqual(model1, model2)
self.assertNotEqual(hash(model1), hash(model2))
def test_ne_name(self):
model1 = Model(name='name1')
model2 = Model(name='name2')
self.assertNotEqual(model1, model2)
self.assertNotEqual(hash(model1), hash(model2))
def test_ne_models(self):
model1 = Model(models=[SubModel(name='name1')])
model2 = Model(models=[SubModel(name='name2')])
self.assertNotEqual(model1, model2)
self.assertNotEqual(hash(model1), hash(model2))
def test_ignores_values_with_default_value_none(self):
model1 = Model(name='name1')
model2 = Model(name='name1', uri=None)
self.assertEqual(model1, model2)
self.assertEqual(hash(model1), hash(model2))

View File

@ -19,6 +19,7 @@ class InheritanceTest(unittest.TestCase):
pass
def test_sub_class_can_have_its_own_slots(self):
# Needed for things like SpotifyTrack in mopidy-spotify 1.x
class Foo(Track):
__slots__ = ('_foo',)
@ -26,6 +27,17 @@ class InheritanceTest(unittest.TestCase):
f = Foo()
f._foo = 123
def test_sub_class_can_be_initialized(self):
# Fails with following error if fields are not handled across classes.
# TypeError: __init__() got an unexpected keyword argument "type"
# Essentially this is testing that sub-classes take parent _fields into
# account.
class Foo(Ref):
pass
Foo.directory()
class CachingTest(unittest.TestCase):
@ -1156,9 +1168,3 @@ class SearchResultTest(unittest.TestCase):
self.assertDictEqual(
{'__model__': 'SearchResult', 'uri': 'uri'},
SearchResult(uri='uri').serialize())
def test_to_json_and_back(self):
result1 = SearchResult(uri='uri')
serialized = json.dumps(result1, cls=ModelJSONEncoder)
result2 = json.loads(serialized, object_hook=model_json_decoder)
self.assertEqual(result1, result2)