From ca21bbca2a25041a764b0604d5d91ee4a7ed0930 Mon Sep 17 00:00:00 2001 From: Petitminion Date: Fri, 14 Mar 2025 22:56:41 +0100 Subject: [PATCH] resolving real migration issues --- api/funkwhale_api/music/admin.py | 9 +++ .../0061_migrate_libraries_to_playlist.py | 61 +++++++++++-------- api/funkwhale_api/playlists/factories.py | 6 +- .../migrations/0009_playlist_library.py | 34 ++++++----- api/funkwhale_api/playlists/models.py | 2 + api/tests/music/test_migrations.py | 19 ++++++ .../playlist-library-federation/index.md | 18 +++++- 7 files changed, 104 insertions(+), 45 deletions(-) diff --git a/api/funkwhale_api/music/admin.py b/api/funkwhale_api/music/admin.py index 338520624..8fc334754 100644 --- a/api/funkwhale_api/music/admin.py +++ b/api/funkwhale_api/music/admin.py @@ -1,4 +1,5 @@ from funkwhale_api.common import admin +from funkwhale_api.playlists import models as playlist_models from . import models @@ -81,6 +82,14 @@ class UploadAdmin(admin.ModelAdmin): ] list_filter = ["mimetype", "import_status", "library__privacy_level"] + def formfield_for_manytomany(self, db_field, request, **kwargs): + if db_field.name == "playlist_libraries": + object_id = request.resolver_match.kwargs.get("object_id") + kwargs["queryset"] = playlist_models.Playlist.objects.filter( + playlist_tracks__track__uploads=object_id + ) + return super().formfield_for_foreignkey(db_field, request, **kwargs) + @admin.register(models.UploadVersion) class UploadVersionAdmin(admin.ModelAdmin): diff --git a/api/funkwhale_api/music/migrations/0061_migrate_libraries_to_playlist.py b/api/funkwhale_api/music/migrations/0061_migrate_libraries_to_playlist.py index 97db650ff..756938c18 100644 --- a/api/funkwhale_api/music/migrations/0061_migrate_libraries_to_playlist.py +++ b/api/funkwhale_api/music/migrations/0061_migrate_libraries_to_playlist.py @@ -1,6 +1,7 @@ # Generated by Django 4.2.9 on 2025-01-03 16:12 -from django.db import migrations, models +from django.db import migrations, models, transaction +from django.conf import settings from funkwhale_api.federation import utils as federation_utils from django.urls import reverse @@ -9,35 +10,43 @@ import uuid def insert_tracks_to_playlist(apps, playlist, uploads): PlaylistTrack = apps.get_model("playlists", "PlaylistTrack") - plts = [ - PlaylistTrack( - creation_date=playlist.creation_date, - playlist=playlist, - track=upload.track, - index=0 + i, - uuid=(new_uuid := uuid.uuid4()), - fid=federation_utils.full_url( - reverse( - "federation:music:playlist-tracks-detail", - kwargs={"uuid": new_uuid}, - ) - ), - ) - for i, upload in enumerate(uploads) - if upload.track - ] - - return PlaylistTrack.objects.bulk_create(plts) + for i, upload in enumerate(uploads): + if upload.track: + PlaylistTrack.objects.create( + creation_date=playlist.creation_date, + playlist=playlist, + track=upload.track, + index=0 + i, + uuid=(new_uuid := uuid.uuid4()), + fid=federation_utils.full_url( + reverse( + "federation:music:playlist-tracks-detail", + kwargs={"uuid": new_uuid}, + ) + ), + ) + upload.library = None + upload.save() +@transaction.atomic def migrate_libraries_to_playlist(apps, schema_editor): Playlist = apps.get_model("playlists", "Playlist") Library = apps.get_model("music", "Library") Actor = apps.get_model("federation", "Actor") + Channel = apps.get_model("audio", "Channel") for library in Library.objects.all(): + if ( + Channel.objects.filter(library=library).exists() + or library.name.startswith("playlist_") + or Playlist.objects.filter(library=library).exists() + or not federation_utils.is_local(library.fid) + ): + continue + try: - playlist = Playlist.objects.create( + playlist, created = Playlist.objects.get_or_create( name=library.name, library=library, actor=library.actor, @@ -56,17 +65,21 @@ def migrate_libraries_to_playlist(apps, schema_editor): if library.uploads.all().exists(): uploads = library.uploads.all() - insert_tracks_to_playlist(apps, playlist, uploads) + with transaction.atomic(): + insert_tracks_to_playlist(apps, playlist, uploads) playlist.library.playlist_uploads.set(uploads) except Exception as e: - print(f"An error occurred during library.playlist creation {e}") + print(f"An error occurred during library.playlist creation : {e}") continue # migrate uploads to new built-in libraries for actor in Actor.objects.all(): + if not federation_utils.is_local(actor.fid): + continue + privacy_levels = ["me", "instance", "everyone"] for privacy_level in privacy_levels: - build_in_lib = Library.objects.create( + build_in_lib, created = Library.objects.get_or_create( actor=actor, privacy_level=privacy_level, name=privacy_level, diff --git a/api/funkwhale_api/playlists/factories.py b/api/funkwhale_api/playlists/factories.py index 641f3c643..6ba1087d6 100644 --- a/api/funkwhale_api/playlists/factories.py +++ b/api/funkwhale_api/playlists/factories.py @@ -3,7 +3,7 @@ from django.conf import settings from funkwhale_api.factories import NoUpdateOnCreate, registry from funkwhale_api.federation import models -from funkwhale_api.federation.factories import ActorFactory +from funkwhale_api.federation.factories import ActorFactory, MusicLibraryFactory from funkwhale_api.music.factories import TrackFactory @@ -13,9 +13,7 @@ class PlaylistFactory(NoUpdateOnCreate, factory.django.DjangoModelFactory): actor = factory.SubFactory(ActorFactory) fid = factory.Faker("federation_url") uuid = factory.Faker("uuid4") - library = factory.SubFactory( - "funkwhale_api.federation.factories.MusicLibraryFactory" - ) + library = factory.SubFactory(MusicLibraryFactory) class Meta: model = "playlists.Playlist" diff --git a/api/funkwhale_api/playlists/migrations/0009_playlist_library.py b/api/funkwhale_api/playlists/migrations/0009_playlist_library.py index b04b69965..dc724e36b 100644 --- a/api/funkwhale_api/playlists/migrations/0009_playlist_library.py +++ b/api/funkwhale_api/playlists/migrations/0009_playlist_library.py @@ -1,32 +1,43 @@ import django.db.models.deletion -from django.db import migrations, models +from django.db import migrations, models, transaction +from funkwhale_api.federation import utils as federation_utils # to do : test migration def add_uploads_to_pl_library(playlist, library): for plt in playlist.playlist_tracks.all(): for upload in plt.track.uploads.filter(library__actor=playlist.actor): - library.uploads.add(upload) + library.playlist_uploads.add(upload) +@transaction.atomic def create_playlist_libraries(apps, schema_editor): Playlist = apps.get_model("playlists", "Playlist") Library = apps.get_model("music", "Library") for playlist in Playlist.objects.all(): + if not federation_utils.is_local(playlist.fid): + continue library = playlist.library - if library is None: + if not library: try: + # we don't want to get_or_create in case it's a channel lib library = Library.objects.create( - name=playlist.name, privacy_level="me", actor=playlist.actor + name="playlist_" + playlist.name, + privacy_level="me", + actor=playlist.actor, ) library.save() playlist.library = library playlist.save() - add_uploads_to_pl_library(playlist, library) + with transaction.atomic(): + add_uploads_to_pl_library(playlist, library) except Exception as e: - print(f"An error occurred during playlist.library creation {e}") - continue + print( + f"An error occurred during playlist.library creation, raising since we want\ + to enforce one lib per playlist" + ) + raise e class Migration(migrations.Migration): @@ -49,13 +60,4 @@ class Migration(migrations.Migration): migrations.RunPython( create_playlist_libraries, reverse_code=migrations.RunPython.noop ), - migrations.AlterField( - model_name="playlist", - name="library", - field=models.OneToOneField( - on_delete=django.db.models.deletion.CASCADE, - related_name="playlist", - to="music.library", - ), - ), ] diff --git a/api/funkwhale_api/playlists/models.py b/api/funkwhale_api/playlists/models.py index f7aa49cad..1b306883a 100644 --- a/api/funkwhale_api/playlists/models.py +++ b/api/funkwhale_api/playlists/models.py @@ -90,6 +90,8 @@ class Playlist(federation_models.FederationMixin): federation_namespace = "playlists" library = models.OneToOneField( "music.Library", + null=True, + blank=True, on_delete=models.CASCADE, related_name="playlist", ) diff --git a/api/tests/music/test_migrations.py b/api/tests/music/test_migrations.py index b7aef68df..8738b048a 100644 --- a/api/tests/music/test_migrations.py +++ b/api/tests/music/test_migrations.py @@ -122,6 +122,16 @@ def test_migrate_libraries_to_playlist(migrator): description="This is a description", ) + library_not_local = Library.objects.create( + name="This should becane playlist name", + fid="https://asupernotlocal.acab/federation/music/libraries/8505207e-45da-449a-9ec8-ed12a848fcea", + actor=target_actor, + creation_date=now(), + privacy_level="everyone", + uuid=uuid4(), + description="This is a description recalling to eat the rich", + ) + Track.objects.create() Track.objects.create() track = Track.objects.create() @@ -134,6 +144,10 @@ def test_migrate_libraries_to_playlist(migrator): Upload.objects.create(library=library, track=track3), ] + Upload.objects.create(library=library_not_local, track=track), + Upload.objects.create(library=library_not_local, track=track2), + Upload.objects.create(library=library_not_local, track=track3), + library_follow = LibraryFollow.objects.create( uuid=uuid4(), target=library, @@ -151,6 +165,7 @@ def test_migrate_libraries_to_playlist(migrator): Playlist = new_apps.get_model("playlists", "Playlist") PlaylistTrack = new_apps.get_model("playlists", "PlaylistTrack") LibraryFollow = new_apps.get_model("federation", "LibraryFollow") + Library = new_apps.get_model("music", "Library") # Assertions @@ -180,6 +195,10 @@ def test_migrate_libraries_to_playlist(migrator): assert upload.pk not in [u.pk for u in playlist.library.uploads.all()] assert not playlist.library.uploads.all() + # Not local + library_not_local = Library.objects.get(fid=library_not_local.fid) + assert not library_not_local.playlist_uploads.all() + # Test fail but works on real db I don't get why # no library are found in the new app # NewAppLibrary = new_apps.get_model("music", "Library") diff --git a/docs/specs/playlist-library-federation/index.md b/docs/specs/playlist-library-federation/index.md index 53d3837f2..bf91e3887 100644 --- a/docs/specs/playlist-library-federation/index.md +++ b/docs/specs/playlist-library-federation/index.md @@ -1,4 +1,4 @@ -## Playlist Import export +## Playlist libraries to share audio files ### The Issue @@ -15,12 +15,28 @@ Users will be able to click on a "Request access to playlist audios files" butto #### Backend +##### Data model + +`Playlist` one_to_one with `Library` through `library` field +`Upload` many_to_one with `Library` through `library` (reverse is `library.uploads`) +`Upload` has also a many_to_many with `Library` through `playlist_libraries` (the same upload can be share various time in through various playlists). Reverse relation is `library.playlist_uploads` + +##### Migrations + +1. Remote library are not changed +2. Local lib are not deleted but are assigned to a playlist +3. Libraries Follows are not touched +4. Remote want fetch local libs as always but they will need to update the data or fail (migrating uploads from `library` to `playlist_library`) + +##### Done + - [x] `PlaylistViewSet` `add` `clear` `remove` update the uploads.playlist_libraries relationships - [x] `PlaylistViewSet` `add` `clear` `remove` -> `schedule_scan` -> Update activity to remote -> playlist.library scan on remote - [x] library and playlist scan delay are long (24h), force on ap update - [x] make sure only owned upload are added to the playlist.library - [x] update the "drop library" migrations to use the playlist.library instead of user follow - [ ] make sure user get the new libraries created after library drop +- [ ] update the federation api to send the playlist_library info ### Follow up