From a4b17a9aa81afcb7041c92544ad55a6cfed60ef2 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 22 Sep 2014 21:48:26 +0200 Subject: [PATCH] models: Fix equality for fields set to the default Fixes #837 (cherry picked from commit bdd1fb983b3abbbb604686e63647a8aaaa5d97a0) --- docs/changelog.rst | 6 ++++++ mopidy/models.py | 8 ++++---- tests/test_models.py | 22 ++++++++++++++++++++-- 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index c0a1a9b8..bde9759b 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -10,6 +10,12 @@ v0.19.5 (UNRELEASED) - Models: Hide empty collections from :func:`repr()` representations. +- Field values are no longer stored on the model instance when the value + matches the default value for the field. This makes two models equal when + they have a field which in one case is implicitly set to the default value + and in the other case explicitly set to the default value, but with otherwise + equal fields. (Fixes: :issue:`837`) + v0.19.4 (2014-09-01) ==================== diff --git a/mopidy/models.py b/mopidy/models.py index 83888ae5..510cb56a 100644 --- a/mopidy/models.py +++ b/mopidy/models.py @@ -18,6 +18,8 @@ class ImmutableObject(object): raise TypeError( '__init__() got an unexpected keyword argument "%s"' % key) + if value == getattr(self, key): + continue # Don't explicitly set default values self.__dict__[key] = value def __setattr__(self, name, value): @@ -72,13 +74,11 @@ class ImmutableObject(object): for key in self.__dict__.keys(): public_key = key.lstrip('_') value = values.pop(public_key, self.__dict__[key]) - if value is not None: - data[public_key] = value + data[public_key] = value for key in values.keys(): if hasattr(self, key): value = values.pop(key) - if value is not None: - data[key] = value + data[key] = value if values: raise TypeError( 'copy() got an unexpected keyword argument "%s"' % key) diff --git a/tests/test_models.py b/tests/test_models.py index 448d6208..09610b99 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -171,8 +171,8 @@ class ArtistTest(unittest.TestCase): def test_serialize_falsy_values(self): self.assertDictEqual( - {'__model__': 'Artist', 'uri': '', 'name': None}, - Artist(uri='', name=None).serialize()) + {'__model__': 'Artist', 'uri': '', 'name': ''}, + Artist(uri='', name='').serialize()) def test_to_json_and_back(self): artist1 = Artist(uri='uri', name='name') @@ -735,6 +735,24 @@ class TrackTest(unittest.TestCase): self.assertNotEqual(track1, track2) self.assertNotEqual(hash(track1), hash(track2)) + def test_ignores_values_with_default_value_none(self): + track1 = Track(name='name1') + track2 = Track(name='name1', album=None) + self.assertEqual(track1, track2) + self.assertEqual(hash(track1), hash(track2)) + + def test_ignores_values_with_default_value_zero(self): + track1 = Track(name='name1') + track2 = Track(name='name1', track_no=0) + self.assertEqual(track1, track2) + self.assertEqual(hash(track1), hash(track2)) + + def test_copy_can_reset_to_default_value(self): + track1 = Track(name='name1') + track2 = Track(name='name1', album=Album()).copy(album=None) + self.assertEqual(track1, track2) + self.assertEqual(hash(track1), hash(track2)) + class TlTrackTest(unittest.TestCase): def test_tlid(self):