Merge branch '850-empty-copyright' into 'master'

Fix #850: Ensure empty but optional fields in file metadata don't error during import

See merge request funkwhale/funkwhale!775

api/funkwhale_api/music/metadata.py

@@ -481,14 +481,26 @@ class PermissiveDateField(serializers.CharField):
         return None
 
 
+class MBIDField(serializers.UUIDField):
+    def __init__(self, *args, **kwargs):
+        kwargs.setdefault("allow_null", True)
+        kwargs.setdefault("required", False)
+        super().__init__(*args, **kwargs)
+
+    def to_internal_value(self, v):
+        if v in ["", None]:
+            return None
+        return super().to_internal_value(v)
+
+
 class ArtistSerializer(serializers.Serializer):
     name = serializers.CharField()
-    mbid = serializers.UUIDField(required=False, allow_null=True)
+    mbid = MBIDField()
 
 
 class AlbumSerializer(serializers.Serializer):
     title = serializers.CharField()
-    mbid = serializers.UUIDField(required=False, allow_null=True)
+    mbid = MBIDField()
     release_date = PermissiveDateField(required=False, allow_null=True)
 
 
@@ -512,16 +524,35 @@ class PositionField(serializers.CharField):
 
 class TrackMetadataSerializer(serializers.Serializer):
     title = serializers.CharField()
-    position = PositionField(allow_null=True, required=False)
+    position = PositionField(allow_blank=True, allow_null=True, required=False)
-    disc_number = PositionField(allow_null=True, required=False)
+    disc_number = PositionField(allow_blank=True, allow_null=True, required=False)
-    copyright = serializers.CharField(allow_null=True, required=False)
+    copyright = serializers.CharField(allow_blank=True, allow_null=True, required=False)
-    license = serializers.CharField(allow_null=True, required=False)
+    license = serializers.CharField(allow_blank=True, allow_null=True, required=False)
-    mbid = serializers.UUIDField(allow_null=True, required=False)
+    mbid = MBIDField()
 
     album = AlbumField()
     artists = ArtistField()
     cover_data = CoverDataField()
 
+    remove_blank_null_fields = [
+        "copyright",
+        "license",
+        "position",
+        "disc_number",
+        "mbid",
+    ]
+
+    def validate(self, validated_data):
+        validated_data = super().validate(validated_data)
+        for field in self.remove_blank_null_fields:
+            try:
+                v = validated_data[field]
+            except KeyError:
+                continue
+            if v in ["", None]:
+                validated_data.pop(field)
+        return validated_data
+
 
 class FakeMetadata(Mapping):
     def __init__(self, data, picture=None):

api/tests/music/test_metadata.py

@@ -539,6 +539,33 @@ def test_serializer_album_artist_missing():
     assert serializer.validated_data == expected
 
 
+@pytest.mark.parametrize(
+    "field_name", ["copyright", "license", "mbid", "position", "disc_number"]
+)
+def test_serializer_empty_fields(field_name):
+    data = {
+        "title": "Track Title",
+        "artist": "Track Artist",
+        "album": "Track Album",
+        # empty copyright/license field shouldn't fail, cf #850
+        field_name: "",
+    }
+    expected = {
+        "title": "Track Title",
+        "artists": [{"name": "Track Artist", "mbid": None}],
+        "album": {
+            "title": "Track Album",
+            "mbid": None,
+            "release_date": None,
+            "artists": [],
+        },
+        "cover_data": None,
+    }
+    serializer = metadata.TrackMetadataSerializer(data=metadata.FakeMetadata(data))
+    assert serializer.is_valid(raise_exception=True) is True
+    assert serializer.validated_data == expected
+
+
 def test_artist_field_featuring():
     data = {
         "artist": "Santana feat. Chris Cornell",

changes/changelog.d/850.bugfix

@@ -0,0 +1 @@
+Ensure empty but optional fields in file metadata don't error during import (#850)