Merge pull request #1546 from jodal/fix/1508-identifier-field-encoding-error

Fix #1508: identifier field encoding error

docs/changelog.rst

@@ -46,6 +46,10 @@ Bug fix release.
   one. Particularly relevant for Mopidy-Scrobbler users, as before it was
   essentially unusable. (Fixes: :issue:`1456`, PR: :issue:`1534`)
 
+- Models: Fix encoding error if :class:`~mopidy.models.fields.Identifier`
+  fields, like the ``musicbrainz_id`` model fields, contained non-ASCII Unicode
+  data. (Fixes: :issue:`1508`, PR: :issue:`1546`)
+
 - File: Ensure path comparision is done between bytestrings only. Fixes crash
   where a :confval:`file/media_dirs` path contained non-ASCII characters.
   (Fixes: :issue:`1345`, PR: :issue:`1493`)

mopidy/models/fields.py

@@ -88,14 +88,17 @@ class Date(String):
 
 class Identifier(String):
     """
-    :class:`Field` for storing ASCII values such as GUIDs or other identifiers.
+    :class:`Field` for storing values such as GUIDs or other identifiers.
 
     Values will be interned.
 
     :param default: default value for field
     """
     def validate(self, value):
-        return compat.intern(str(super(Identifier, self).validate(value)))
+        value = super(Identifier, self).validate(value)
+        if isinstance(value, compat.text_type):
+            value = value.encode('utf-8')
+        return compat.intern(value)
 
 
 class URI(Identifier):

tests/models/test_fields.py

@@ -1,8 +1,10 @@
+# encoding: utf-8
+
 from __future__ import absolute_import, unicode_literals
 
 import unittest
 
-from mopidy.models.fields import Collection, Field, Integer, String
+from mopidy.models.fields import Collection, Field, Identifier, Integer, String
 
 
 def create_instance(field):
@@ -126,6 +128,42 @@ class StringTest(unittest.TestCase):
         self.assertEqual('', instance.attr)
 
 
+class IdentifierTest(unittest.TestCase):
+    def test_default_handling(self):
+        instance = create_instance(Identifier(default='abc'))
+        self.assertEqual('abc', instance.attr)
+
+    def test_native_str_allowed(self):
+        instance = create_instance(Identifier())
+        instance.attr = str('abc')
+        self.assertEqual('abc', instance.attr)
+
+    def test_bytes_allowed(self):
+        instance = create_instance(Identifier())
+        instance.attr = b'abc'
+        self.assertEqual(b'abc', instance.attr)
+
+    def test_unicode_allowed(self):
+        instance = create_instance(Identifier())
+        instance.attr = u'abc'
+        self.assertEqual(u'abc', instance.attr)
+
+    def test_unicode_with_nonascii_allowed(self):
+        instance = create_instance(Identifier())
+        instance.attr = u'æøå'
+        self.assertEqual(u'æøå'.encode('utf-8'), instance.attr)
+
+    def test_other_disallowed(self):
+        instance = create_instance(Identifier())
+        with self.assertRaises(TypeError):
+            instance.attr = 1234
+
+    def test_empty_string(self):
+        instance = create_instance(Identifier())
+        instance.attr = ''
+        self.assertEqual('', instance.attr)
+
+
 class IntegerTest(unittest.TestCase):
     def test_default_handling(self):
         instance = create_instance(Integer(default=1234))