From b29ca4479729153201dbbaff755c84523330db74 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Sat, 7 Apr 2018 11:29:40 +0200 Subject: [PATCH] Now store remote library tracks in a dedicated model, this is much simpler --- api/funkwhale_api/federation/actors.py | 35 +++-- api/funkwhale_api/federation/factories.py | 64 +++++++- ...406_1621.py => 0003_auto_20180407_0852.py} | 26 +++- api/funkwhale_api/federation/models.py | 30 +++- api/funkwhale_api/federation/serializers.py | 147 ++++++++++++++++++ api/funkwhale_api/federation/urls.py | 4 +- api/funkwhale_api/federation/views.py | 24 +-- api/funkwhale_api/music/factories.py | 10 +- ...406_1621.py => 0023_auto_20180407_0852.py} | 34 +--- api/funkwhale_api/music/models.py | 29 +--- api/funkwhale_api/music/serializers.py | 124 --------------- api/funkwhale_api/music/tasks.py | 68 ++++---- api/tests/federation/test_actors.py | 35 +++-- api/tests/federation/test_serializers.py | 146 ++++++++++++++++- api/tests/federation/test_views.py | 5 +- api/tests/music/test_import.py | 74 +++++---- api/tests/music/test_serializers.py | 94 ----------- 17 files changed, 555 insertions(+), 394 deletions(-) rename api/funkwhale_api/federation/migrations/{0003_auto_20180406_1621.py => 0003_auto_20180407_0852.py} (65%) rename api/funkwhale_api/music/migrations/{0023_auto_20180406_1621.py => 0023_auto_20180407_0852.py} (68%) delete mode 100644 api/tests/music/test_serializers.py diff --git a/api/funkwhale_api/federation/actors.py b/api/funkwhale_api/federation/actors.py index 205c3486d..ffbafd8b7 100644 --- a/api/funkwhale_api/federation/actors.py +++ b/api/funkwhale_api/federation/actors.py @@ -4,6 +4,7 @@ import uuid import xml from django.conf import settings +from django.db import transaction from django.urls import reverse from django.utils import timezone @@ -192,10 +193,8 @@ class LibraryActor(SystemActor): def manually_approves_followers(self): return settings.FEDERATION_MUSIC_NEEDS_APPROVAL + @transaction.atomic def handle_create(self, ac, sender): - from funkwhale_api.music.serializers import ( - AudioCollectionImportSerializer) - try: remote_library = models.Library.objects.get( actor=sender, @@ -212,18 +211,28 @@ class LibraryActor(SystemActor): if ac['object']['totalItems'] <= 0: return - items = ac['object']['items'] - - serializer = AudioCollectionImportSerializer( - data=ac['object'], - context={'library': remote_library}) - - if not serializer.is_valid(): - logger.error( - 'Cannot import audio collection: %s', serializer.errors) + try: + items = ac['object']['items'] + except KeyError: + logger.warning('No items in collection!') return - serializer.save() + item_serializers = [ + serializers.AudioSerializer( + data=i, context={'library': remote_library}) + for i in items + ] + + valid_serializers = [] + for s in item_serializers: + if s.is_valid(): + valid_serializers.append(s) + else: + logger.debug( + 'Skipping invalid item %s, %s', s.initial_data, s.errors) + + for s in valid_serializers: + s.save() class TestActor(SystemActor): diff --git a/api/funkwhale_api/federation/factories.py b/api/funkwhale_api/federation/factories.py index fc8932396..b3ac72039 100644 --- a/api/funkwhale_api/federation/factories.py +++ b/api/funkwhale_api/federation/factories.py @@ -128,11 +128,73 @@ class LibraryFactory(factory.DjangoModelFactory): url = factory.Faker('url') federation_enabled = True download_files = False + autoimport = False class Meta: model = models.Library +class ArtistMetadataFactory(factory.Factory): + name = factory.Faker('name') + + class Meta: + model = dict + + class Params: + musicbrainz = factory.Trait( + musicbrainz_id=factory.Faker('uuid4') + ) + + +class ReleaseMetadataFactory(factory.Factory): + title = factory.Faker('sentence') + + class Meta: + model = dict + + class Params: + musicbrainz = factory.Trait( + musicbrainz_id=factory.Faker('uuid4') + ) + + +class RecordingMetadataFactory(factory.Factory): + title = factory.Faker('sentence') + + class Meta: + model = dict + + class Params: + musicbrainz = factory.Trait( + musicbrainz_id=factory.Faker('uuid4') + ) + + +@registry.register(name='federation.LibraryTrackMetadata') +class LibraryTrackMetadataFactory(factory.Factory): + artist = factory.SubFactory(ArtistMetadataFactory) + recording = factory.SubFactory(RecordingMetadataFactory) + release = factory.SubFactory(ReleaseMetadataFactory) + + class Meta: + model = dict + + +@registry.register +class LibraryTrackFactory(factory.DjangoModelFactory): + library = factory.SubFactory(LibraryFactory) + url = factory.Faker('url') + title = factory.Faker('sentence') + artist_name = factory.Faker('sentence') + album_title = factory.Faker('sentence') + audio_url = factory.Faker('url') + audio_mimetype = 'audio/ogg' + metadata = factory.SubFactory(LibraryTrackMetadataFactory) + + class Meta: + model = models.LibraryTrack + + @registry.register(name='federation.Note') class NoteFactory(factory.Factory): type = 'Note' @@ -189,7 +251,7 @@ class AudioFactory(factory.Factory): ) actor = factory.Faker('url') url = factory.SubFactory(LinkFactory, audio=True) - metadata = factory.SubFactory(AudioMetadataFactory) + metadata = factory.SubFactory(LibraryTrackMetadataFactory) class Meta: model = dict diff --git a/api/funkwhale_api/federation/migrations/0003_auto_20180406_1621.py b/api/funkwhale_api/federation/migrations/0003_auto_20180407_0852.py similarity index 65% rename from api/funkwhale_api/federation/migrations/0003_auto_20180406_1621.py rename to api/funkwhale_api/federation/migrations/0003_auto_20180407_0852.py index f8771752e..afc7ea083 100644 --- a/api/funkwhale_api/federation/migrations/0003_auto_20180406_1621.py +++ b/api/funkwhale_api/federation/migrations/0003_auto_20180407_0852.py @@ -1,5 +1,6 @@ -# Generated by Django 2.0.3 on 2018-04-06 16:21 +# Generated by Django 2.0.3 on 2018-04-07 08:52 +import django.contrib.postgres.fields.jsonb from django.db import migrations, models import django.db.models.deletion import django.utils.timezone @@ -9,6 +10,7 @@ import uuid class Migration(migrations.Migration): dependencies = [ + ('music', '0022_importbatch_import_request'), ('federation', '0002_auto_20180403_1620'), ] @@ -47,10 +49,30 @@ class Migration(migrations.Migration): ('url', models.URLField()), ('federation_enabled', models.BooleanField()), ('download_files', models.BooleanField()), - ('files_count', models.PositiveIntegerField(blank=True, null=True)), + ('autoimport', models.BooleanField()), + ('tracks_count', models.PositiveIntegerField(blank=True, null=True)), ('actor', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='library', to='federation.Actor')), ], ), + migrations.CreateModel( + name='LibraryTrack', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('url', models.URLField(unique=True)), + ('audio_url', models.URLField()), + ('audio_mimetype', models.CharField(max_length=200)), + ('creation_date', models.DateTimeField(default=django.utils.timezone.now)), + ('modification_date', models.DateTimeField(auto_now=True)), + ('fetched_date', models.DateTimeField(blank=True, null=True)), + ('published_date', models.DateTimeField(blank=True, null=True)), + ('artist_name', models.CharField(max_length=500)), + ('album_title', models.CharField(max_length=500)), + ('title', models.CharField(max_length=500)), + ('metadata', django.contrib.postgres.fields.jsonb.JSONField(default={}, max_length=10000)), + ('library', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='tracks', to='federation.Library')), + ('local_track_file', models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='library_track', to='music.TrackFile')), + ], + ), migrations.AddField( model_name='actor', name='followers', diff --git a/api/funkwhale_api/federation/models.py b/api/funkwhale_api/federation/models.py index 8a90bdb76..91f2ea973 100644 --- a/api/funkwhale_api/federation/models.py +++ b/api/funkwhale_api/federation/models.py @@ -1,6 +1,7 @@ import uuid from django.conf import settings +from django.contrib.postgres.fields import JSONField from django.db import models from django.utils import timezone @@ -170,8 +171,35 @@ class Library(models.Model): related_name='library') uuid = models.UUIDField(default=uuid.uuid4) url = models.URLField() + # use this flag to disable federation with a library federation_enabled = models.BooleanField() # should we mirror files locally or hotlink them? download_files = models.BooleanField() - files_count = models.PositiveIntegerField(null=True, blank=True) + # should we automatically import new files from this library? + autoimport = models.BooleanField() + tracks_count = models.PositiveIntegerField(null=True, blank=True) + + +class LibraryTrack(models.Model): + url = models.URLField(unique=True) + audio_url = models.URLField() + audio_mimetype = models.CharField(max_length=200) + creation_date = models.DateTimeField(default=timezone.now) + modification_date = models.DateTimeField( + auto_now=True) + fetched_date = models.DateTimeField(null=True, blank=True) + published_date = models.DateTimeField(null=True, blank=True) + library = models.ForeignKey( + Library, related_name='tracks', on_delete=models.CASCADE) + local_track_file = models.OneToOneField( + 'music.TrackFile', + related_name='library_track', + on_delete=models.CASCADE, + null=True, + blank=True, + ) + artist_name = models.CharField(max_length=500) + album_title = models.CharField(max_length=500) + title = models.CharField(max_length=500) + metadata = JSONField(default={}, max_length=10000) diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py index 05e9c7c8e..17541c50f 100644 --- a/api/funkwhale_api/federation/serializers.py +++ b/api/funkwhale_api/federation/serializers.py @@ -3,6 +3,7 @@ import urllib.parse from django.urls import reverse from django.conf import settings from django.core.paginator import Paginator +from django.db import transaction from rest_framework import serializers from dynamic_preferences.registries import global_preferences_registry @@ -265,3 +266,149 @@ class CollectionPageSerializer(serializers.Serializer): if self.context.get('include_ap_context', True): d['@context'] = AP_CONTEXT return d + + +class ArtistMetadataSerializer(serializers.Serializer): + musicbrainz_id = serializers.UUIDField(required=False) + name = serializers.CharField() + + +class ReleaseMetadataSerializer(serializers.Serializer): + musicbrainz_id = serializers.UUIDField(required=False) + title = serializers.CharField() + + +class RecordingMetadataSerializer(serializers.Serializer): + musicbrainz_id = serializers.UUIDField(required=False) + title = serializers.CharField() + + +class AudioMetadataSerializer(serializers.Serializer): + artist = ArtistMetadataSerializer() + release = ReleaseMetadataSerializer() + recording = RecordingMetadataSerializer() + + +class AudioSerializer(serializers.Serializer): + type = serializers.CharField() + id = serializers.URLField() + url = serializers.JSONField() + published = serializers.DateTimeField() + updated = serializers.DateTimeField(required=False) + metadata = AudioMetadataSerializer() + + def validate_type(self, v): + if v != 'Audio': + raise serializers.ValidationError('Invalid type for audio') + return v + + def validate_url(self, v): + try: + url = v['href'] + except (KeyError, TypeError): + raise serializers.ValidationError('Missing href') + + try: + media_type = v['mediaType'] + except (KeyError, TypeError): + raise serializers.ValidationError('Missing mediaType') + + if not media_type.startswith('audio/'): + raise serializers.ValidationError('Invalid mediaType') + + return url + + def validate_url(self, v): + try: + url = v['href'] + except (KeyError, TypeError): + raise serializers.ValidationError('Missing href') + + try: + media_type = v['mediaType'] + except (KeyError, TypeError): + raise serializers.ValidationError('Missing mediaType') + + if not media_type.startswith('audio/'): + raise serializers.ValidationError('Invalid mediaType') + + return v + + def create(self, validated_data): + defaults = { + 'audio_mimetype': validated_data['url']['mediaType'], + 'audio_url': validated_data['url']['href'], + 'metadata': validated_data['metadata'], + 'artist_name': validated_data['metadata']['artist']['name'], + 'album_title': validated_data['metadata']['release']['title'], + 'title': validated_data['metadata']['recording']['title'], + 'published_date': validated_data['published'], + 'modification_date': validated_data.get('updated'), + } + return models.LibraryTrack.objects.get_or_create( + library=self.context['library'], + url=validated_data['id'], + defaults=defaults + )[0] + + def to_representation(self, instance): + track = instance.track + album = instance.track.album + artist = instance.track.artist + + d = { + 'type': 'Audio', + 'id': instance.get_federation_url(), + 'name': instance.track.full_name, + 'published': instance.creation_date.isoformat(), + 'updated': instance.modification_date.isoformat(), + 'metadata': { + 'artist': { + 'musicbrainz_id': str(artist.mbid) if artist.mbid else None, + 'name': artist.name, + }, + 'release': { + 'musicbrainz_id': str(album.mbid) if album.mbid else None, + 'title': album.title, + }, + 'recording': { + 'musicbrainz_id': str(track.mbid) if track.mbid else None, + 'title': track.title, + }, + }, + 'url': { + 'href': utils.full_url(instance.path), + 'type': 'Link', + 'mediaType': instance.mimetype + }, + 'attributedTo': [ + self.context['actor'].url + ] + } + if self.context.get('include_ap_context', True): + d['@context'] = AP_CONTEXT + return d + + +class CollectionSerializer(serializers.Serializer): + + def to_representation(self, conf): + d = { + 'id': conf['id'], + 'actor': conf['actor'].url, + 'totalItems': len(conf['items']), + 'type': 'Collection', + 'items': [ + conf['item_serializer']( + i, + context={ + 'actor': conf['actor'], + 'include_ap_context': False} + ).data + for i in conf['items'] + ] + } + + if self.context.get('include_ap_context', True): + d['@context'] = AP_CONTEXT + return d diff --git a/api/funkwhale_api/federation/urls.py b/api/funkwhale_api/federation/urls.py index e899869a4..2c24b5257 100644 --- a/api/funkwhale_api/federation/urls.py +++ b/api/funkwhale_api/federation/urls.py @@ -15,10 +15,10 @@ router.register( 'well-known') music_router.register( - r'federation/files', + r'files', views.MusicFilesViewSet, 'files', ) urlpatterns = router.urls + [ - url('music/', include((music_router.urls, 'music'), namespace='music')) + url('federation/music/', include((music_router.urls, 'music'), namespace='music')) ] diff --git a/api/funkwhale_api/federation/views.py b/api/funkwhale_api/federation/views.py index 390a371bc..35d8a75a5 100644 --- a/api/funkwhale_api/federation/views.py +++ b/api/funkwhale_api/federation/views.py @@ -10,7 +10,6 @@ from rest_framework import response from rest_framework.decorators import list_route, detail_route from funkwhale_api.music.models import TrackFile -from funkwhale_api.music.serializers import AudioSerializer from . import actors from . import authentication @@ -119,13 +118,16 @@ class MusicFilesViewSet(FederationMixin, viewsets.GenericViewSet): def list(self, request, *args, **kwargs): page = request.GET.get('page') library = actors.SYSTEM_ACTORS['library'].get_actor_instance() - qs = TrackFile.objects.order_by('-creation_date') + qs = TrackFile.objects.order_by('-creation_date').select_related( + 'track__artist', + 'track__album__artist' + ) if page is None: conf = { 'id': utils.full_url(reverse('federation:music:files-list')), 'page_size': settings.FEDERATION_COLLECTION_PAGE_SIZE, 'items': qs, - 'item_serializer': AudioSerializer, + 'item_serializer': serializers.AudioSerializer, 'actor': library, } serializer = serializers.PaginatedCollectionSerializer(conf) @@ -140,15 +142,15 @@ class MusicFilesViewSet(FederationMixin, viewsets.GenericViewSet): qs, settings.FEDERATION_COLLECTION_PAGE_SIZE) try: page = p.page(page_number) + conf = { + 'id': utils.full_url(reverse('federation:music:files-list')), + 'page': page, + 'item_serializer': serializers.AudioSerializer, + 'actor': library, + } + serializer = serializers.CollectionPageSerializer(conf) + data = serializer.data except paginator.EmptyPage: return response.Response(status=404) - conf = { - 'id': utils.full_url(reverse('federation:music:files-list')), - 'page': page, - 'item_serializer': AudioSerializer, - 'actor': library, - } - serializer = serializers.CollectionPageSerializer(conf) - data = serializer.data return response.Response(data) diff --git a/api/funkwhale_api/music/factories.py b/api/funkwhale_api/music/factories.py index 8da56b2a9..83aad432a 100644 --- a/api/funkwhale_api/music/factories.py +++ b/api/funkwhale_api/music/factories.py @@ -3,9 +3,7 @@ import os from funkwhale_api.factories import registry, ManyToManyFromList from funkwhale_api.federation.factories import ( - AudioMetadataFactory, - ActorFactory, - LibraryFactory, + LibraryTrackFactory, ) from funkwhale_api.users.factories import UserFactory @@ -69,8 +67,6 @@ class ImportBatchFactory(factory.django.DjangoModelFactory): class Params: federation = factory.Trait( submitted_by=None, - source_library=factory.SubFactory(LibraryFactory), - source_library_url=factory.Faker('url'), source='federation', ) @@ -86,9 +82,9 @@ class ImportJobFactory(factory.django.DjangoModelFactory): class Params: federation = factory.Trait( + mbid=None, + library_track=factory.SubFactory(LibraryTrackFactory), batch=factory.SubFactory(ImportBatchFactory, federation=True), - source_library_url=factory.Faker('url'), - metadata=factory.SubFactory(AudioMetadataFactory), ) diff --git a/api/funkwhale_api/music/migrations/0023_auto_20180406_1621.py b/api/funkwhale_api/music/migrations/0023_auto_20180407_0852.py similarity index 68% rename from api/funkwhale_api/music/migrations/0023_auto_20180406_1621.py rename to api/funkwhale_api/music/migrations/0023_auto_20180407_0852.py index 2bc8085a0..b1bbeacef 100644 --- a/api/funkwhale_api/music/migrations/0023_auto_20180406_1621.py +++ b/api/funkwhale_api/music/migrations/0023_auto_20180407_0852.py @@ -1,7 +1,6 @@ -# Generated by Django 2.0.3 on 2018-04-06 16:21 +# Generated by Django 2.0.3 on 2018-04-07 08:52 from django.conf import settings -import django.contrib.postgres.fields.jsonb from django.db import migrations, models import django.db.models.deletion import django.utils.timezone @@ -11,7 +10,7 @@ import uuid class Migration(migrations.Migration): dependencies = [ - ('federation', '0003_auto_20180406_1621'), + ('federation', '0003_auto_20180407_0852'), ('music', '0022_importbatch_import_request'), ] @@ -26,16 +25,6 @@ class Migration(migrations.Migration): name='uuid', field=models.UUIDField(db_index=True, default=uuid.uuid4, unique=True), ), - migrations.AddField( - model_name='importbatch', - name='source_library', - field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='import_batches', to='federation.Library'), - ), - migrations.AddField( - model_name='importbatch', - name='source_library_url', - field=models.URLField(blank=True, null=True), - ), migrations.AddField( model_name='importbatch', name='uuid', @@ -43,13 +32,8 @@ class Migration(migrations.Migration): ), migrations.AddField( model_name='importjob', - name='metadata', - field=django.contrib.postgres.fields.jsonb.JSONField(default={}), - ), - migrations.AddField( - model_name='importjob', - name='source_library_url', - field=models.URLField(blank=True, null=True), + name='library_track', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='import_jobs', to='federation.LibraryTrack'), ), migrations.AddField( model_name='importjob', @@ -76,16 +60,6 @@ class Migration(migrations.Migration): name='modification_date', field=models.DateTimeField(auto_now=True), ), - migrations.AddField( - model_name='trackfile', - name='source_library', - field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='track_files', to='federation.Library'), - ), - migrations.AddField( - model_name='trackfile', - name='source_library_url', - field=models.URLField(blank=True, null=True), - ), migrations.AddField( model_name='trackfile', name='uuid', diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index 4c0b6a098..bcf691bcd 100644 --- a/api/funkwhale_api/music/models.py +++ b/api/funkwhale_api/music/models.py @@ -9,7 +9,6 @@ import uuid from django.conf import settings from django.db import models -from django.contrib.postgres.fields import JSONField from django.core.files.base import ContentFile from django.core.files import File from django.db.models.signals import post_save @@ -416,16 +415,6 @@ class TrackFile(models.Model): source = models.URLField(null=True, blank=True) creation_date = models.DateTimeField(default=timezone.now) modification_date = models.DateTimeField(auto_now=True) - - source_library = models.ForeignKey( - 'federation.Library', - null=True, - blank=True, - on_delete=models.SET_NULL, - related_name='track_files') - # points to the URL of the original trackfile ActivityPub Object - source_library_url = models.URLField(null=True, blank=True) - duration = models.IntegerField(null=True, blank=True) acoustid_track_id = models.UUIDField(null=True, blank=True) mimetype = models.CharField(null=True, blank=True, max_length=200) @@ -503,14 +492,6 @@ class ImportBatch(models.Model): blank=True, on_delete=models.CASCADE) - source_library = models.ForeignKey( - 'federation.Library', - null=True, - blank=True, - on_delete=models.SET_NULL, - related_name='import_batches') - source_library_url = models.URLField(null=True, blank=True) - class Meta: ordering = ['-creation_date'] @@ -540,8 +521,14 @@ class ImportJob(models.Model): choices=IMPORT_STATUS_CHOICES, default='pending', max_length=30) audio_file = models.FileField( upload_to='imports/%Y/%m/%d', max_length=255, null=True, blank=True) - source_library_url = models.URLField(null=True, blank=True) - metadata = JSONField(default={}) + + library_track = models.ForeignKey( + 'federation.LibraryTrack', + related_name='import_jobs', + on_delete=models.SET_NULL, + null=True, + blank=True + ) class Meta: ordering = ('id', ) diff --git a/api/funkwhale_api/music/serializers.py b/api/funkwhale_api/music/serializers.py index 78b2f7c04..42795dbea 100644 --- a/api/funkwhale_api/music/serializers.py +++ b/api/funkwhale_api/music/serializers.py @@ -153,127 +153,3 @@ class TrackActivitySerializer(activity_serializers.ModelSerializer): def get_type(self, obj): return 'Audio' - - -class AudioMetadataSerializer(serializers.Serializer): - artist = serializers.CharField(required=False) - release = serializers.CharField(required=False) - recording = serializers.CharField(required=False) - - -class AudioSerializer(serializers.Serializer): - type = serializers.CharField() - id = serializers.URLField() - url = serializers.JSONField() - metadata = AudioMetadataSerializer() - - def validate_type(self, v): - if v != 'Audio': - raise serializers.ValidationError('Invalid type for audio') - return v - - def validate_url(self, v): - try: - url = v['href'] - except (KeyError, TypeError): - raise serializers.ValidationError('Missing href') - - try: - media_type = v['mediaType'] - except (KeyError, TypeError): - raise serializers.ValidationError('Missing mediaType') - - if not media_type.startswith('audio/'): - raise serializers.ValidationError('Invalid mediaType') - - return url - - def validate_url(self, v): - try: - url = v['href'] - except (KeyError, TypeError): - raise serializers.ValidationError('Missing href') - - try: - media_type = v['mediaType'] - except (KeyError, TypeError): - raise serializers.ValidationError('Missing mediaType') - - if not media_type.startswith('audio/'): - raise serializers.ValidationError('Invalid mediaType') - - return v - - def create(self, validated_data, batch): - metadata = validated_data['metadata'].copy() - metadata['mediaType'] = validated_data['url']['mediaType'] - return models.ImportJob.objects.create( - batch=batch, - source=validated_data['url']['href'], - source_library_url=validated_data['id'], - metadata=metadata, - ) - - def to_representation(self, instance): - d = { - 'type': 'Audio', - 'id': instance.get_federation_url(), - 'name': instance.track.full_name, - 'metadata': { - 'artist': instance.track.artist.musicbrainz_url, - 'release': instance.track.album.musicbrainz_url, - 'track': instance.track.musicbrainz_url, - }, - 'url': { - 'href': federation_utils.full_url(instance.path), - 'type': 'Link', - 'mediaType': instance.mimetype - }, - 'attributedTo': [ - self.context['actor'].url - ] - } - if self.context.get('include_ap_context', True): - d['@context'] = AP_CONTEXT - return d - - -class AudioCollectionImportSerializer(serializers.Serializer): - id = serializers.URLField() - items = serializers.ListField( - child=AudioSerializer(), - min_length=1, - ) - - @transaction.atomic - def create(self, validated_data): - batch = models.ImportBatch.objects.create( - source_library=self.context['library'], - source_library_url=validated_data['id'], - source='federation', - ) - for i in validated_data['items']: - s = AudioSerializer(data=i) - job = s.create(i, batch) - return batch - - def to_representation(self, instance): - d = { - 'id': instance['id'], - 'actor': instance['actor'].url, - 'totalItems': len(instance['items']), - 'type': 'Collection', - 'items': [ - AudioSerializer( - i, - context={ - 'actor': instance['actor'], - 'include_ap_context': False - } - ).data - for i in instance['items'] - ] - } - if self.context.get('include_ap_context', True): - d['@context'] = AP_CONTEXT - return d diff --git a/api/funkwhale_api/music/tasks.py b/api/funkwhale_api/music/tasks.py index 67b538c1e..c58eb7136 100644 --- a/api/funkwhale_api/music/tasks.py +++ b/api/funkwhale_api/music/tasks.py @@ -25,42 +25,46 @@ def set_acoustid_on_track_file(track_file): return update(result['id']) -def get_mbid(url, type): - prefix = 'https://musicbrainz.org/{}/'.format(type) - if url.startswith(prefix): - return url.replace(prefix, '') - - -def import_track_from_metadata(metadata): - raw_track = metadata['recording'] - if isinstance(raw_track, str): - track_mbid = get_mbid(raw_track, 'recording') +def import_track_from_remote(library_track): + metadata = library_track.metadata + try: + track_mbid = metadata['recording']['musicbrainz_id'] + assert track_mbid # for null/empty values + except (KeyError, AssertionError): + pass + else: return models.Track.get_or_create_from_api(mbid=track_mbid) - raw_album = metadata['release'] - if isinstance(raw_album, str): - album_mbid = get_mbid(raw_album, 'release') + try: + album_mbid = metadata['release']['musicbrainz_id'] + assert album_mbid # for null/empty values + except (KeyError, AssertionError): + pass + else: album = models.Album.get_or_create_from_api(mbid=album_mbid) return models.Track.get_or_create_from_title( - raw_track['title'], artist=album.artist, album=album) + library_track.title, artist=album.artist, album=album) - raw_artist = metadata['artist'] - if isinstance(raw_artist, str): - artist_mbid = get_mbid(raw_artist, 'artist') + try: + artist_mbid = metadata['artist']['musicbrainz_id'] + assert artist_mbid # for null/empty values + except (KeyError, AssertionError): + pass + else: artist = models.Artist.get_or_create_from_api(mbid=artist_mbid) album = models.Album.get_or_create_from_title( - raw_album['title'], artist=artist) + library_track.album_title, artist=artist) return models.Track.get_or_create_from_title( - raw_track['title'], artist=artist, album=album) + library_track.title, artist=artist, album=album) # worst case scenario, we have absolutely no way to link to a # musicbrainz resource, we rely on the name/titles artist = models.Artist.get_or_create_from_name( - raw_artist['name']) + library_track.artist_name) album = models.Album.get_or_create_from_title( - raw_album['title'], artist=artist) + library_track.album_title, artist=artist) return models.Track.get_or_create_from_title( - raw_track['title'], artist=artist, album=album) + library_track.title, artist=artist, album=album) def _do_import(import_job, replace, use_acoustid=True): @@ -83,12 +87,13 @@ def _do_import(import_job, replace, use_acoustid=True): track, _ = models.Track.get_or_create_from_api(mbid=mbid) elif import_job.audio_file: track = import_track_data_from_path(import_job.audio_file.path) + elif import_job.library_track: + track = import_track_from_remote(import_job.library_track) else: - # probably federation, we use metadata stored on the job itself - if not import_job.metadata: - raise ValueError('We cannot import without at least metadatas') + raise ValueError( + 'Not enough data to process import, ' + 'add a mbid, an audio file or a library track') - track = import_track_from_metadata(import_job.metadata) track_file = None if replace: track_file = track.files.first() @@ -102,14 +107,13 @@ def _do_import(import_job, replace, use_acoustid=True): track_file = track_file or models.TrackFile( track=track, source=import_job.source) track_file.acoustid_track_id = acoustid_track_id - track_file.source_library = import_job.batch.source_library - track_file.source_library_url = import_job.source_library_url if from_file: track_file.audio_file = ContentFile(import_job.audio_file.read()) track_file.audio_file.name = import_job.audio_file.name track_file.duration = duration - elif import_job.source_library_url: - if track_file.source_library.download_files: + elif import_job.library_track: + track_file.mimetype = import_job.library_track.audio_mimetype + if import_job.library_track.library.download_files: raise NotImplementedError() else: # no downloading, we hotlink @@ -117,6 +121,10 @@ def _do_import(import_job, replace, use_acoustid=True): else: track_file.download_file() track_file.save() + if import_job.library_track: + import_job.library_track.local_track_file = track_file + import_job.library_track.save( + update_fields=['modification_date', 'local_track_file']) import_job.status = 'finished' import_job.track_file = track_file if import_job.audio_file: diff --git a/api/tests/federation/test_actors.py b/api/tests/federation/test_actors.py index 6f5e88663..107047b56 100644 --- a/api/tests/federation/test_actors.py +++ b/api/tests/federation/test_actors.py @@ -1,3 +1,4 @@ +import arrow import pytest import uuid @@ -10,7 +11,6 @@ from funkwhale_api.federation import actors from funkwhale_api.federation import models from funkwhale_api.federation import serializers from funkwhale_api.federation import utils -from funkwhale_api.music import models as music_models def test_actor_fetching(r_mock): @@ -375,7 +375,7 @@ def test_library_actor_handle_create_audio_no_library(mocker, factories): # when we receive inbox create audio, we should not do anything # if we don't have a configured library matching the sender mocked_create = mocker.patch( - 'funkwhale_api.music.serializers.AudioCollectionImportSerializer.create' + 'funkwhale_api.federation.serializers.AudioSerializer.create' ) actor = factories['federation.Actor']() library_actor = actors.SYSTEM_ACTORS['library'].get_actor_instance() @@ -393,7 +393,7 @@ def test_library_actor_handle_create_audio_no_library(mocker, factories): library_actor.system_conf.post_inbox(data, actor=actor) mocked_create.assert_not_called() - music_models.ImportBatch.objects.count() == 0 + models.LibraryTrack.objects.count() == 0 def test_library_actor_handle_create_audio_no_library_enabled( @@ -401,7 +401,7 @@ def test_library_actor_handle_create_audio_no_library_enabled( # when we receive inbox create audio, we should not do anything # if we don't have an enabled library mocked_create = mocker.patch( - 'funkwhale_api.music.serializers.AudioCollectionImportSerializer.create' + 'funkwhale_api.federation.serializers.AudioSerializer.create' ) disabled_library = factories['federation.Library']( federation_enabled=False) @@ -421,12 +421,14 @@ def test_library_actor_handle_create_audio_no_library_enabled( library_actor.system_conf.post_inbox(data, actor=actor) mocked_create.assert_not_called() - music_models.ImportBatch.objects.count() == 0 + models.LibraryTrack.objects.count() == 0 def test_library_actor_handle_create_audio(mocker, factories): library_actor = actors.SYSTEM_ACTORS['library'].get_actor_instance() - remote_library = factories['federation.Library']() + remote_library = factories['federation.Library']( + federation_enabled=True + ) data = { 'actor': remote_library.actor.url, @@ -442,14 +444,19 @@ def test_library_actor_handle_create_audio(mocker, factories): library_actor.system_conf.post_inbox(data, actor=remote_library.actor) - batch = remote_library.import_batches.latest('id') + lts = list(remote_library.tracks.order_by('id')) - assert batch.source_library_url == data['object']['id'] - assert batch.source_library == remote_library - assert batch.jobs.count() == 2 + assert len(lts) == 2 - jobs = list(batch.jobs.order_by('id')) for i, a in enumerate(data['object']['items']): - job = jobs[i] - assert job.source_library_url == a['id'] - assert job.source == a['url']['href'] + lt = lts[i] + assert lt.pk is not None + assert lt.url == a['id'] + assert lt.library == remote_library + assert lt.audio_url == a['url']['href'] + assert lt.audio_mimetype == a['url']['mediaType'] + assert lt.metadata == a['metadata'] + assert lt.title == a['metadata']['recording']['title'] + assert lt.artist_name == a['metadata']['artist']['name'] + assert lt.album_title == a['metadata']['release']['title'] + assert lt.published_date == arrow.get(a['published']) diff --git a/api/tests/federation/test_serializers.py b/api/tests/federation/test_serializers.py index 1e580040e..45778ed48 100644 --- a/api/tests/federation/test_serializers.py +++ b/api/tests/federation/test_serializers.py @@ -1,10 +1,13 @@ +import arrow + from django.urls import reverse from django.core.paginator import Paginator +from funkwhale_api.federation import actors from funkwhale_api.federation import keys from funkwhale_api.federation import models from funkwhale_api.federation import serializers -from funkwhale_api.music.serializers import AudioSerializer +from funkwhale_api.federation import utils def test_actor_serializer_from_ap(db): @@ -174,7 +177,7 @@ def test_paginated_collection_serializer(factories): conf = { 'id': 'https://test.federation/test', 'items': tfs, - 'item_serializer': AudioSerializer, + 'item_serializer': serializers.AudioSerializer, 'actor': actor, 'page_size': 2, } @@ -204,7 +207,7 @@ def test_collection_page_serializer(factories): conf = { 'id': 'https://test.federation/test', - 'item_serializer': AudioSerializer, + 'item_serializer': serializers.AudioSerializer, 'actor': actor, 'page': Paginator(tfs, 2).page(2), } @@ -235,3 +238,140 @@ def test_collection_page_serializer(factories): serializer = serializers.CollectionPageSerializer(conf) assert serializer.data == expected + + +def test_activity_pub_audio_serializer_to_library_track(factories): + remote_library = factories['federation.Library']() + audio = factories['federation.Audio']() + serializer = serializers.AudioSerializer( + data=audio, context={'library': remote_library}) + + assert serializer.is_valid(raise_exception=True) + + lt = serializer.save() + + assert lt.pk is not None + assert lt.url == audio['id'] + assert lt.library == remote_library + assert lt.audio_url == audio['url']['href'] + assert lt.audio_mimetype == audio['url']['mediaType'] + assert lt.metadata == audio['metadata'] + assert lt.title == audio['metadata']['recording']['title'] + assert lt.artist_name == audio['metadata']['artist']['name'] + assert lt.album_title == audio['metadata']['release']['title'] + assert lt.published_date == arrow.get(audio['published']) + + +def test_activity_pub_audio_serializer_to_ap(factories): + tf = factories['music.TrackFile'](mimetype='audio/mp3') + library = actors.SYSTEM_ACTORS['library'].get_actor_instance() + expected = { + '@context': serializers.AP_CONTEXT, + 'type': 'Audio', + 'id': tf.get_federation_url(), + 'name': tf.track.full_name, + 'published': tf.creation_date.isoformat(), + 'updated': tf.modification_date.isoformat(), + 'metadata': { + 'artist': { + 'musicbrainz_id': tf.track.artist.mbid, + 'name': tf.track.artist.name, + }, + 'release': { + 'musicbrainz_id': tf.track.album.mbid, + 'title': tf.track.album.title, + }, + 'recording': { + 'musicbrainz_id': tf.track.mbid, + 'title': tf.track.title, + }, + }, + 'url': { + 'href': utils.full_url(tf.path), + 'type': 'Link', + 'mediaType': 'audio/mp3' + }, + 'attributedTo': [ + library.url + ] + } + + serializer = serializers.AudioSerializer(tf, context={'actor': library}) + + assert serializer.data == expected + + +def test_activity_pub_audio_serializer_to_ap_no_mbid(factories): + tf = factories['music.TrackFile']( + mimetype='audio/mp3', + track__mbid=None, + track__album__mbid=None, + track__album__artist__mbid=None, + ) + library = actors.SYSTEM_ACTORS['library'].get_actor_instance() + expected = { + '@context': serializers.AP_CONTEXT, + 'type': 'Audio', + 'id': tf.get_federation_url(), + 'name': tf.track.full_name, + 'published': tf.creation_date.isoformat(), + 'updated': tf.modification_date.isoformat(), + 'metadata': { + 'artist': { + 'name': tf.track.artist.name, + 'musicbrainz_id': None, + }, + 'release': { + 'title': tf.track.album.title, + 'musicbrainz_id': None, + }, + 'recording': { + 'title': tf.track.title, + 'musicbrainz_id': None, + }, + }, + 'url': { + 'href': utils.full_url(tf.path), + 'type': 'Link', + 'mediaType': 'audio/mp3' + }, + 'attributedTo': [ + library.url + ] + } + + serializer = serializers.AudioSerializer(tf, context={'actor': library}) + + assert serializer.data == expected + + +def test_collection_serializer_to_ap(factories): + tf1 = factories['music.TrackFile'](mimetype='audio/mp3') + tf2 = factories['music.TrackFile'](mimetype='audio/ogg') + library = actors.SYSTEM_ACTORS['library'].get_actor_instance() + expected = { + '@context': serializers.AP_CONTEXT, + 'id': 'https://test.id', + 'actor': library.url, + 'totalItems': 2, + 'type': 'Collection', + 'items': [ + serializers.AudioSerializer( + tf1, context={'actor': library, 'include_ap_context': False} + ).data, + serializers.AudioSerializer( + tf2, context={'actor': library, 'include_ap_context': False} + ).data, + ] + } + + collection = { + 'id': expected['id'], + 'actor': library, + 'items': [tf1, tf2], + 'item_serializer': serializers.AudioSerializer + } + serializer = serializers.CollectionSerializer( + collection, context={'actor': library, 'id': 'https://test.id'}) + + assert serializer.data == expected diff --git a/api/tests/federation/test_views.py b/api/tests/federation/test_views.py index c7e1fc012..c26810dad 100644 --- a/api/tests/federation/test_views.py +++ b/api/tests/federation/test_views.py @@ -7,7 +7,6 @@ from funkwhale_api.federation import actors from funkwhale_api.federation import serializers from funkwhale_api.federation import utils from funkwhale_api.federation import webfinger -from funkwhale_api.music.serializers import AudioSerializer @pytest.mark.parametrize('system_actor', actors.SYSTEM_ACTORS.keys()) @@ -87,7 +86,7 @@ def test_audio_file_list_actor_no_page( 'id': utils.full_url(reverse('federation:music:files-list')), 'page_size': 2, 'items': list(reversed(tfs)), # we order by -creation_date - 'item_serializer': AudioSerializer, + 'item_serializer': serializers.AudioSerializer, 'actor': library } expected = serializers.PaginatedCollectionSerializer(conf).data @@ -107,7 +106,7 @@ def test_audio_file_list_actor_page( conf = { 'id': utils.full_url(reverse('federation:music:files-list')), 'page': Paginator(list(reversed(tfs)), 2).page(2), - 'item_serializer': AudioSerializer, + 'item_serializer': serializers.AudioSerializer, 'actor': library } expected = serializers.CollectionPageSerializer(conf).data diff --git a/api/tests/music/test_import.py b/api/tests/music/test_import.py index 7e3ebcc19..a15f027ba 100644 --- a/api/tests/music/test_import.py +++ b/api/tests/music/test_import.py @@ -38,32 +38,23 @@ def test_create_import_can_bind_to_request( assert batch.import_request == request -@pytest.mark.parametrize('url,type,expected', [ - ('https://musicbrainz.org/artist/test', 'artist', 'test'), - ('https://musicbrainz.org/release/test', 'release', 'test'), - ('https://musicbrainz.org/recording/test', 'recording', 'test'), - ('https://musicbrainz.org/recording/test', 'artist', None), -]) -def test_get_mbid(url, type, expected): - assert tasks.get_mbid(url, type) == expected - - def test_import_job_from_federation_no_musicbrainz(factories): + lt = factories['federation.LibraryTrack']( + artist_name='Hello', + album_title='World', + title='Ping', + ) job = factories['music.ImportJob']( federation=True, - metadata__artist={'name': 'Hello'}, - metadata__release={'title': 'World'}, - metadata__recording={'title': 'Ping'}, - mbid=None, + library_track=lt, ) tasks.import_job_run(import_job_id=job.pk) job.refresh_from_db() tf = job.track_file - assert tf.source == job.source - assert tf.source_library == job.batch.source_library - assert tf.source_library_url == job.source_library_url + assert tf.mimetype == lt.audio_mimetype + assert tf.library_track == job.library_track assert tf.track.title == 'Ping' assert tf.track.artist.name == 'Hello' assert tf.track.album.title == 'World' @@ -74,23 +65,25 @@ def test_import_job_from_federation_musicbrainz_recording(factories, mocker): track_from_api = mocker.patch( 'funkwhale_api.music.models.Track.get_or_create_from_api', return_value=t) + lt = factories['federation.LibraryTrack']( + metadata__recording__musicbrainz=True, + artist_name='Hello', + album_title='World', + ) job = factories['music.ImportJob']( federation=True, - metadata__artist={'name': 'Hello'}, - metadata__release={'title': 'World'}, - mbid=None, + library_track=lt, ) tasks.import_job_run(import_job_id=job.pk) job.refresh_from_db() tf = job.track_file - assert tf.source == job.source - assert tf.source_library == job.batch.source_library - assert tf.source_library_url == job.source_library_url + assert tf.mimetype == lt.audio_mimetype + assert tf.library_track == job.library_track assert tf.track == t track_from_api.assert_called_once_with( - mbid=tasks.get_mbid(job.metadata['recording'], 'recording')) + mbid=lt.metadata['recording']['musicbrainz_id']) def test_import_job_from_federation_musicbrainz_release(factories, mocker): @@ -98,26 +91,28 @@ def test_import_job_from_federation_musicbrainz_release(factories, mocker): album_from_api = mocker.patch( 'funkwhale_api.music.models.Album.get_or_create_from_api', return_value=a) + lt = factories['federation.LibraryTrack']( + metadata__release__musicbrainz=True, + artist_name='Hello', + title='Ping', + ) job = factories['music.ImportJob']( federation=True, - metadata__artist={'name': 'Hello'}, - metadata__recording={'title': 'Ping'}, - mbid=None, + library_track=lt, ) tasks.import_job_run(import_job_id=job.pk) job.refresh_from_db() tf = job.track_file - assert tf.source_library == job.batch.source_library - assert tf.source_library_url == job.source_library_url - assert tf.source == job.source + assert tf.mimetype == lt.audio_mimetype + assert tf.library_track == job.library_track assert tf.track.title == 'Ping' assert tf.track.artist == a.artist assert tf.track.album == a album_from_api.assert_called_once_with( - mbid=tasks.get_mbid(job.metadata['release'], 'release')) + mbid=lt.metadata['release']['musicbrainz_id']) def test_import_job_from_federation_musicbrainz_artist(factories, mocker): @@ -125,24 +120,27 @@ def test_import_job_from_federation_musicbrainz_artist(factories, mocker): artist_from_api = mocker.patch( 'funkwhale_api.music.models.Artist.get_or_create_from_api', return_value=a) + lt = factories['federation.LibraryTrack']( + metadata__artist__musicbrainz=True, + album_title='World', + title='Ping', + ) job = factories['music.ImportJob']( federation=True, - metadata__release={'title': 'World'}, - metadata__recording={'title': 'Ping'}, - mbid=None, + library_track=lt, ) tasks.import_job_run(import_job_id=job.pk) job.refresh_from_db() tf = job.track_file - assert tf.source == job.source - assert tf.source_library == job.batch.source_library - assert tf.source_library_url == job.source_library_url + assert tf.mimetype == lt.audio_mimetype + assert tf.library_track == job.library_track + assert tf.track.title == 'Ping' assert tf.track.artist == a assert tf.track.album.artist == a assert tf.track.album.title == 'World' artist_from_api.assert_called_once_with( - mbid=tasks.get_mbid(job.metadata['artist'], 'artist')) + mbid=lt.metadata['artist']['musicbrainz_id']) diff --git a/api/tests/music/test_serializers.py b/api/tests/music/test_serializers.py deleted file mode 100644 index a1f5999e7..000000000 --- a/api/tests/music/test_serializers.py +++ /dev/null @@ -1,94 +0,0 @@ -from funkwhale_api.federation import actors -from funkwhale_api.federation import utils as federation_utils -from funkwhale_api.federation.serializers import AP_CONTEXT -from funkwhale_api.music import serializers - - -def test_activity_pub_audio_collection_serializer_to_import(factories): - remote_library = factories['federation.Library']() - - collection = { - 'id': 'https://batch.import', - 'type': 'Collection', - 'totalItems': 2, - 'items': factories['federation.Audio'].create_batch(size=2) - } - - serializer = serializers.AudioCollectionImportSerializer( - data=collection, context={'library': remote_library}) - - assert serializer.is_valid(raise_exception=True) - - batch = serializer.save() - jobs = list(batch.jobs.all()) - - assert batch.source == 'federation' - assert batch.source_library_url == collection['id'] - assert batch.source_library == remote_library - assert len(jobs) == 2 - - for i, a in enumerate(collection['items']): - job = jobs[i] - assert job.source_library_url == a['id'] - assert job.source == a['url']['href'] - a['metadata']['mediaType'] = a['url']['mediaType'] - assert job.metadata == a['metadata'] - - -def test_activity_pub_audio_serializer_to_ap(factories): - tf = factories['music.TrackFile'](mimetype='audio/mp3') - library = actors.SYSTEM_ACTORS['library'].get_actor_instance() - expected = { - '@context': AP_CONTEXT, - 'type': 'Audio', - 'id': tf.get_federation_url(), - 'name': tf.track.full_name, - 'metadata': { - 'artist': tf.track.artist.musicbrainz_url, - 'release': tf.track.album.musicbrainz_url, - 'track': tf.track.musicbrainz_url, - }, - 'url': { - 'href': federation_utils.full_url(tf.path), - 'type': 'Link', - 'mediaType': 'audio/mp3' - }, - 'attributedTo': [ - library.url - ] - } - - serializer = serializers.AudioSerializer(tf, context={'actor': library}) - - assert serializer.data == expected - - -def test_activity_pub_audio_collection_serializer_to_ap(factories): - tf1 = factories['music.TrackFile'](mimetype='audio/mp3') - tf2 = factories['music.TrackFile'](mimetype='audio/ogg') - library = actors.SYSTEM_ACTORS['library'].get_actor_instance() - expected = { - '@context': AP_CONTEXT, - 'id': 'https://test.id', - 'actor': library.url, - 'totalItems': 2, - 'type': 'Collection', - 'items': [ - serializers.AudioSerializer( - tf1, context={'actor': library, 'include_ap_context': False} - ).data, - serializers.AudioSerializer( - tf2, context={'actor': library, 'include_ap_context': False} - ).data, - ] - } - - collection = { - 'id': expected['id'], - 'actor': library, - 'items': [tf1, tf2], - } - serializer = serializers.AudioCollectionImportSerializer( - collection, context={'actor': library, 'id': 'https://test.id'}) - - assert serializer.data == expected