From 6908f4bf74077632e438992db49a57f46ca72f5e Mon Sep 17 00:00:00 2001 From: Georg Krause Date: Mon, 21 Nov 2022 18:12:49 +0000 Subject: [PATCH] Resolve "value too long for type character varying(255) during import_files" --- api/funkwhale_api/audio/serializers.py | 22 ++++--------- api/funkwhale_api/federation/models.py | 6 +++- api/funkwhale_api/federation/serializers.py | 5 ++- .../migrations/0057_auto_20221118_2108.py | 33 +++++++++++++++++++ api/funkwhale_api/music/models.py | 18 +++------- api/funkwhale_api/music/tasks.py | 18 +++------- api/tests/music/test_tasks.py | 26 --------------- changes/changelog.d/1787.enhancement | 1 + 8 files changed, 57 insertions(+), 72 deletions(-) create mode 100644 api/funkwhale_api/music/migrations/0057_auto_20221118_2108.py create mode 100644 changes/changelog.d/1787.enhancement diff --git a/api/funkwhale_api/audio/serializers.py b/api/funkwhale_api/audio/serializers.py index d1b249a8d..b9a905c61 100644 --- a/api/funkwhale_api/audio/serializers.py +++ b/api/funkwhale_api/audio/serializers.py @@ -76,9 +76,9 @@ class ChannelMetadataSerializer(serializers.Serializer): class ChannelCreateSerializer(serializers.Serializer): - name = serializers.CharField(max_length=music_models.MAX_LENGTHS["ARTIST_NAME"]) + name = serializers.CharField(max_length=federation_models.MAX_LENGTHS["ACTOR_NAME"]) username = serializers.CharField( - max_length=music_models.MAX_LENGTHS["ARTIST_NAME"], + max_length=federation_models.MAX_LENGTHS["ACTOR_NAME"], validators=[users_serializers.ASCIIUsernameValidator()], ) description = common_serializers.ContentSerializer(allow_null=True) @@ -159,7 +159,7 @@ NOOP = object() class ChannelUpdateSerializer(serializers.Serializer): - name = serializers.CharField(max_length=music_models.MAX_LENGTHS["ARTIST_NAME"]) + name = serializers.CharField(max_length=federation_models.MAX_LENGTHS["ACTOR_NAME"]) description = common_serializers.ContentSerializer(allow_null=True) tags = tags_serializers.TagsListField() content_category = serializers.ChoiceField( @@ -385,9 +385,7 @@ def get_channel_from_rss_url(url, raise_exception=False): ) ) if parsed_feed.feed.get("rights"): - track_defaults["copyright"] = parsed_feed.feed.rights[ - : music_models.MAX_LENGTHS["COPYRIGHT"] - ] + track_defaults["copyright"] = parsed_feed.feed.rights for entry in entries[: settings.PODCASTS_RSS_FEED_MAX_ITEMS]: logger.debug("Importing feed item %s", entry.id) s = RssFeedItemSerializer(data=entry) @@ -547,9 +545,7 @@ class RssFeedSerializer(serializers.Serializer): **artist_kwargs, defaults={ "attributed_to": service_actor, - "name": validated_data["title"][ - : music_models.MAX_LENGTHS["ARTIST_NAME"] - ], + "name": validated_data["title"], "content_category": "podcast", }, ) @@ -752,16 +748,12 @@ class RssFeedItemSerializer(serializers.Serializer): { "disc_number": validated_data.get("itunes_season", 1) or 1, "position": validated_data.get("itunes_episode", 1) or 1, - "title": validated_data["title"][ - : music_models.MAX_LENGTHS["TRACK_TITLE"] - ], + "title": validated_data["title"], "artist": channel.artist, } ) if "rights" in validated_data: - track_defaults["copyright"] = validated_data["rights"][ - : music_models.MAX_LENGTHS["COPYRIGHT"] - ] + track_defaults["copyright"] = validated_data["rights"] if "published_parsed" in validated_data: track_defaults["creation_date"] = datetime.datetime.fromtimestamp( diff --git a/api/funkwhale_api/federation/models.py b/api/funkwhale_api/federation/models.py index 58a6acf54..5a6b12e53 100644 --- a/api/funkwhale_api/federation/models.py +++ b/api/funkwhale_api/federation/models.py @@ -30,6 +30,10 @@ TYPE_CHOICES = [ ("Service", "Service"), ] +MAX_LENGTHS = { + "ACTOR_NAME": 200, +} + def empty_dict(): return {} @@ -188,7 +192,7 @@ class Actor(models.Model): followers_url = models.URLField(max_length=500, null=True, blank=True) shared_inbox_url = models.URLField(max_length=500, null=True, blank=True) type = models.CharField(choices=TYPE_CHOICES, default="Person", max_length=25) - name = models.CharField(max_length=200, null=True, blank=True) + name = models.CharField(max_length=MAX_LENGTHS["ACTOR_NAME"], null=True, blank=True) domain = models.ForeignKey(Domain, on_delete=models.CASCADE, related_name="actors") summary = models.CharField(max_length=500, null=True, blank=True) summary_obj = models.ForeignKey( diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py index d3263507b..5781bf414 100644 --- a/api/funkwhale_api/federation/serializers.py +++ b/api/funkwhale_api/federation/serializers.py @@ -1796,7 +1796,7 @@ class ChannelUploadSerializer(jsonld.JsonLdSerializer): id = serializers.URLField(max_length=500) type = serializers.ChoiceField(choices=[contexts.AS.Audio]) url = LinkListSerializer(keep_mediatype=["audio/*"], min_length=1) - name = TruncatedCharField(truncate_length=music_models.MAX_LENGTHS["TRACK_TITLE"]) + name = serializers.CharField() published = serializers.DateTimeField(required=False) duration = serializers.IntegerField(min_value=0, required=False) position = serializers.IntegerField(min_value=0, allow_null=True, required=False) @@ -1804,8 +1804,7 @@ class ChannelUploadSerializer(jsonld.JsonLdSerializer): album = serializers.URLField(max_length=500, required=False) license = serializers.URLField(allow_null=True, required=False) attributedTo = serializers.URLField(max_length=500, required=False) - copyright = TruncatedCharField( - truncate_length=music_models.MAX_LENGTHS["COPYRIGHT"], + copyright = serializers.CharField( allow_null=True, required=False, ) diff --git a/api/funkwhale_api/music/migrations/0057_auto_20221118_2108.py b/api/funkwhale_api/music/migrations/0057_auto_20221118_2108.py new file mode 100644 index 000000000..820bb765b --- /dev/null +++ b/api/funkwhale_api/music/migrations/0057_auto_20221118_2108.py @@ -0,0 +1,33 @@ +# Generated by Django 3.2.16 on 2022-11-18 21:08 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('music', '0056_alter_artist_content_category'), + ] + + operations = [ + migrations.AlterField( + model_name='album', + name='title', + field=models.TextField(), + ), + migrations.AlterField( + model_name='artist', + name='name', + field=models.TextField(), + ), + migrations.AlterField( + model_name='track', + name='copyright', + field=models.TextField(blank=True, null=True), + ), + migrations.AlterField( + model_name='track', + name='title', + field=models.TextField(), + ), + ] diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index 37a9a5343..069a915a4 100644 --- a/api/funkwhale_api/music/models.py +++ b/api/funkwhale_api/music/models.py @@ -37,14 +37,6 @@ from . import importers, metadata, utils logger = logging.getLogger(__name__) -MAX_LENGTHS = { - "ARTIST_NAME": 255, - "ALBUM_TITLE": 255, - "TRACK_TITLE": 255, - "COPYRIGHT": 500, -} - - ARTIST_CONTENT_CATEGORY_CHOICES = [ ("music", "music"), ("podcast", "podcast"), @@ -213,7 +205,7 @@ class ArtistQuerySet(common_models.LocalFromFidQuerySet, models.QuerySet): class Artist(APIModelMixin): - name = models.CharField(max_length=MAX_LENGTHS["ARTIST_NAME"]) + name = models.TextField() federation_namespace = "artists" musicbrainz_model = "artist" musicbrainz_mapping = { @@ -338,7 +330,7 @@ class AlbumQuerySet(common_models.LocalFromFidQuerySet, models.QuerySet): class Album(APIModelMixin): - title = models.CharField(max_length=MAX_LENGTHS["ALBUM_TITLE"]) + title = models.TextField() artist = models.ForeignKey(Artist, related_name="albums", on_delete=models.CASCADE) release_date = models.DateField(null=True, blank=True, db_index=True) release_group_id = models.UUIDField(null=True, blank=True) @@ -496,7 +488,7 @@ def get_artist(release_list): class Track(APIModelMixin): mbid = models.UUIDField(db_index=True, null=True, blank=True) - title = models.CharField(max_length=MAX_LENGTHS["TRACK_TITLE"]) + title = models.TextField() artist = models.ForeignKey(Artist, related_name="tracks", on_delete=models.CASCADE) disc_number = models.PositiveIntegerField(null=True, blank=True) position = models.PositiveIntegerField(null=True, blank=True) @@ -520,9 +512,7 @@ class Track(APIModelMixin): on_delete=models.SET_NULL, related_name="attributed_tracks", ) - copyright = models.CharField( - max_length=MAX_LENGTHS["COPYRIGHT"], null=True, blank=True - ) + copyright = models.TextField(null=True, blank=True) description = models.ForeignKey( "common.Content", null=True, blank=True, on_delete=models.SET_NULL ) diff --git a/api/funkwhale_api/music/tasks.py b/api/funkwhale_api/music/tasks.py index 42088d9e6..5eeac3248 100644 --- a/api/funkwhale_api/music/tasks.py +++ b/api/funkwhale_api/music/tasks.py @@ -563,9 +563,7 @@ def _get_track(data, attributed_to=None, **forced_values): else: album_artists = getter(data, "album", "artists", default=artists) or artists album_artist_data = album_artists[0] - album_artist_name = truncate( - album_artist_data.get("name"), models.MAX_LENGTHS["ARTIST_NAME"] - ) + album_artist_name = album_artist_data.get("name") if album_artist_name == artist_name: album_artist = artist else: @@ -609,9 +607,7 @@ def _get_track(data, attributed_to=None, **forced_values): # get / create album if "album" in data: album_data = data["album"] - album_title = truncate( - album_data["title"], models.MAX_LENGTHS["ALBUM_TITLE"] - ) + album_title = album_data["title"] album_fid = album_data.get("fid", None) if album_mbid: @@ -647,11 +643,7 @@ def _get_track(data, attributed_to=None, **forced_values): else: album = None # get / create track - track_title = ( - forced_values["title"] - if "title" in forced_values - else truncate(data["title"], models.MAX_LENGTHS["TRACK_TITLE"]) - ) + track_title = forced_values["title"] if "title" in forced_values else data["title"] position = ( forced_values["position"] if "position" in forced_values @@ -670,7 +662,7 @@ def _get_track(data, attributed_to=None, **forced_values): copyright = ( forced_values["copyright"] if "copyright" in forced_values - else truncate(data.get("copyright"), models.MAX_LENGTHS["COPYRIGHT"]) + else data.get("copyright") ) description = ( {"text": forced_values["description"], "content_type": "text/markdown"} @@ -736,7 +728,7 @@ def _get_track(data, attributed_to=None, **forced_values): def get_artist(artist_data, attributed_to, from_activity_id): artist_mbid = artist_data.get("mbid", None) artist_fid = artist_data.get("fid", None) - artist_name = truncate(artist_data["name"], models.MAX_LENGTHS["ARTIST_NAME"]) + artist_name = artist_data["name"] if artist_mbid: query = Q(mbid=artist_mbid) diff --git a/api/tests/music/test_tasks.py b/api/tests/music/test_tasks.py index 15cecae33..78c55ce68 100644 --- a/api/tests/music/test_tasks.py +++ b/api/tests/music/test_tasks.py @@ -80,32 +80,6 @@ def test_can_create_track_from_file_metadata_attributed_to(factories, mocker): assert track.artist.attributed_to == actor -def test_can_create_track_from_file_metadata_truncates_too_long_values( - factories, mocker -): - metadata = { - "title": "a" * 5000, - "artists": [{"name": "b" * 5000}], - "album": {"title": "c" * 5000, "release_date": datetime.date(2012, 8, 15)}, - "position": 4, - "disc_number": 2, - "copyright": "d" * 5000, - } - - track = tasks.get_track_from_import_metadata(metadata) - - assert track.title == metadata["title"][: models.MAX_LENGTHS["TRACK_TITLE"]] - assert track.copyright == metadata["copyright"][: models.MAX_LENGTHS["COPYRIGHT"]] - assert ( - track.album.title - == metadata["album"]["title"][: models.MAX_LENGTHS["ALBUM_TITLE"]] - ) - assert ( - track.artist.name - == metadata["artists"][0]["name"][: models.MAX_LENGTHS["ARTIST_NAME"]] - ) - - def test_can_create_track_from_file_metadata_featuring(factories): metadata = { "title": "Whole Lotta Love", diff --git a/changes/changelog.d/1787.enhancement b/changes/changelog.d/1787.enhancement new file mode 100644 index 000000000..7b66240db --- /dev/null +++ b/changes/changelog.d/1787.enhancement @@ -0,0 +1 @@ +Allow arbritrary length names for artists, albums and tracks