diff --git a/api/funkwhale_api/music/metadata.py b/api/funkwhale_api/music/metadata.py index 387b8ffe7..55044dbc7 100644 --- a/api/funkwhale_api/music/metadata.py +++ b/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) - disc_number = PositionField(allow_null=True, required=False) - copyright = serializers.CharField(allow_null=True, required=False) - license = serializers.CharField(allow_null=True, required=False) - mbid = serializers.UUIDField(allow_null=True, required=False) + position = PositionField(allow_blank=True, allow_null=True, required=False) + disc_number = PositionField(allow_blank=True, allow_null=True, required=False) + copyright = serializers.CharField(allow_blank=True, allow_null=True, required=False) + license = serializers.CharField(allow_blank=True, 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): diff --git a/api/tests/music/test_metadata.py b/api/tests/music/test_metadata.py index 1656ece49..52c4e3026 100644 --- a/api/tests/music/test_metadata.py +++ b/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", diff --git a/changes/changelog.d/850.bugfix b/changes/changelog.d/850.bugfix new file mode 100644 index 000000000..0e26ce773 --- /dev/null +++ b/changes/changelog.d/850.bugfix @@ -0,0 +1 @@ +Ensure empty but optional fields in file metadata don't error during import (#850)