Make sure we query the lib before createing plts + some migration issues
This commit is contained in:
Petitminion
parent
97737f0b98
commit
a2e2187c4c
|
|
@ -346,6 +346,7 @@ def activity_pass_object_privacy_level(context, routing):
|
|||
if object_type in MUSIC_OBJECT_TYPE:
|
||||
return True
|
||||
|
||||
# to do : support sending activity to followers only
|
||||
if object and obj_privacy_level and obj_privacy_level in ["me", "instance"]:
|
||||
return False
|
||||
|
||||
|
|
|
|||
|
|
@ -814,11 +814,12 @@ def inbox_update_playlist(payload, context):
|
|||
serializer = serializers.PlaylistSerializer(data=payload["object"])
|
||||
if serializer.is_valid(raise_exception=True):
|
||||
playlist = serializer.save()
|
||||
# we trigger a scan since we use this activity to avoid sending many PlaylistTracks activities
|
||||
playlist.schedule_scan(actors.get_service_actor(), force=True)
|
||||
# we update the playlist.library
|
||||
# we update the playlist.library to get the plt.track.uploads locally
|
||||
if follows := playlist.library.received_follows.filter(approved=True):
|
||||
playlist.library.schedule_scan(follows[0].actor, force=True)
|
||||
# we trigger a scan since we use this activity to avoid sending many PlaylistTracks activities
|
||||
playlist.schedule_scan(actors.get_service_actor(), force=True)
|
||||
|
||||
return
|
||||
else:
|
||||
logger.debug(
|
||||
|
|
|
|||
|
|
@ -1133,7 +1133,12 @@ class CollectionPageSerializer(jsonld.JsonLdSerializer):
|
|||
"last": last,
|
||||
"items": [
|
||||
conf["item_serializer"](
|
||||
i, context={"actor": conf["actor"], "include_ap_context": False}
|
||||
i,
|
||||
context={
|
||||
"actor": conf["actor"],
|
||||
"library": conf.get("library", None),
|
||||
"include_ap_context": False,
|
||||
},
|
||||
).data
|
||||
for i in page.object_list
|
||||
],
|
||||
|
|
@ -1674,7 +1679,8 @@ class UploadSerializer(jsonld.JsonLdSerializer):
|
|||
def validate_library(self, v):
|
||||
lb = self.context.get("library")
|
||||
if lb:
|
||||
if lb.fid != v and not lb.playlist:
|
||||
# the upload can come from a playlist lib
|
||||
if lb.fid != v and not lb.playlist.library and lb.playlist.library.fid != v:
|
||||
raise serializers.ValidationError("Invalid library fid")
|
||||
return lb
|
||||
|
||||
|
|
@ -1741,11 +1747,12 @@ class UploadSerializer(jsonld.JsonLdSerializer):
|
|||
return music_models.Upload.objects.create(**data)
|
||||
|
||||
def to_representation(self, instance):
|
||||
lib = instance.library if instance.library else self.context.get("library")
|
||||
track = instance.track
|
||||
d = {
|
||||
"type": "Audio",
|
||||
"id": instance.get_federation_id(),
|
||||
"library": instance.library.fid,
|
||||
"library": lib.fid,
|
||||
"name": track.full_name,
|
||||
"published": instance.creation_date.isoformat(),
|
||||
"bitrate": instance.bitrate,
|
||||
|
|
@ -1764,12 +1771,8 @@ class UploadSerializer(jsonld.JsonLdSerializer):
|
|||
},
|
||||
],
|
||||
"track": TrackSerializer(track, context={"include_ap_context": False}).data,
|
||||
"to": (
|
||||
contexts.AS.Public
|
||||
if instance.library.privacy_level == "everyone"
|
||||
else ""
|
||||
),
|
||||
"attributedTo": instance.library.actor.fid,
|
||||
"to": (contexts.AS.Public if lib.privacy_level == "everyone" else ""),
|
||||
"attributedTo": lib.actor.fid,
|
||||
}
|
||||
if instance.modification_date:
|
||||
d["updated"] = instance.modification_date.isoformat()
|
||||
|
|
@ -2350,7 +2353,6 @@ class PlaylistTrackSerializer(jsonld.JsonLdSerializer):
|
|||
"fid": validated_data["id"],
|
||||
},
|
||||
)
|
||||
|
||||
return plt
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -416,8 +416,8 @@ class MusicLibraryViewSet(
|
|||
)
|
||||
),
|
||||
"item_serializer": serializers.UploadSerializer,
|
||||
"library": lb,
|
||||
}
|
||||
|
||||
return get_collection_response(
|
||||
conf=conf,
|
||||
querystring=request.GET,
|
||||
|
|
|
|||
|
|
@ -1,5 +1,4 @@
|
|||
from funkwhale_api.common import admin
|
||||
from funkwhale_api.playlists import models as playlist_models
|
||||
|
||||
from . import models
|
||||
|
||||
|
|
@ -85,9 +84,9 @@ class UploadAdmin(admin.ModelAdmin):
|
|||
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
|
||||
)
|
||||
kwargs["queryset"] = models.Library.objects.filter(
|
||||
playlist_uploads=object_id
|
||||
).distinct()
|
||||
return super().formfield_for_foreignkey(db_field, request, **kwargs)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -42,6 +42,7 @@ def migrate_libraries_to_playlist(apps, schema_editor):
|
|||
or library.name.startswith("playlist_")
|
||||
or Playlist.objects.filter(library=library).exists()
|
||||
or not federation_utils.is_local(library.fid)
|
||||
or library.name in ["me", "instance", "everyone"]
|
||||
):
|
||||
continue
|
||||
|
||||
|
|
@ -74,7 +75,7 @@ def migrate_libraries_to_playlist(apps, schema_editor):
|
|||
|
||||
# migrate uploads to new built-in libraries
|
||||
for actor in Actor.objects.all():
|
||||
if not federation_utils.is_local(actor.fid):
|
||||
if not federation_utils.is_local(actor.fid) or actor.name == "service":
|
||||
continue
|
||||
|
||||
privacy_levels = ["me", "instance", "everyone"]
|
||||
|
|
|
|||
|
|
@ -1,6 +1,8 @@
|
|||
import django.db.models.deletion
|
||||
from django.db import migrations, models, transaction
|
||||
from funkwhale_api.federation import utils as federation_utils
|
||||
from django.urls import reverse
|
||||
import uuid
|
||||
|
||||
|
||||
# to do : test migration
|
||||
|
|
@ -26,6 +28,13 @@ def create_playlist_libraries(apps, schema_editor):
|
|||
name="playlist_" + playlist.name,
|
||||
privacy_level="me",
|
||||
actor=playlist.actor,
|
||||
uuid=(new_uuid := uuid.uuid4()),
|
||||
fid=federation_utils.full_url(
|
||||
reverse(
|
||||
"federation:music:playlist-tracks-detail",
|
||||
kwargs={"uuid": new_uuid},
|
||||
)
|
||||
),
|
||||
)
|
||||
library.save()
|
||||
playlist.library = library
|
||||
|
|
|
|||
|
|
@ -53,9 +53,12 @@ class PlaylistSerializer(serializers.ModelSerializer):
|
|||
)
|
||||
read_only_fields = ["id", "modification_date", "creation_date"]
|
||||
|
||||
@extend_schema_field(OpenApiTypes.UUID)
|
||||
@extend_schema_field(OpenApiTypes.URI)
|
||||
def get_library(self, obj):
|
||||
if obj.library:
|
||||
return obj.library.fid
|
||||
else:
|
||||
return None
|
||||
|
||||
@extend_schema_field(OpenApiTypes.BOOL)
|
||||
def get_is_playable(self, obj):
|
||||
|
|
|
|||
|
|
@ -55,6 +55,7 @@ def get_playlist_page(playlist, page_url, actor):
|
|||
context={
|
||||
"playlist": playlist,
|
||||
"item_serializer": serializers.PlaylistTrackSerializer,
|
||||
"conf": {"library": playlist.library},
|
||||
},
|
||||
)
|
||||
serializer.is_valid(raise_exception=True)
|
||||
|
|
@ -67,6 +68,8 @@ def get_playlist_page(playlist, page_url, actor):
|
|||
"playlist_scan",
|
||||
)
|
||||
def start_playlist_scan(playlist_scan):
|
||||
playlist_scan.playlist.playlist_tracks.all().delete()
|
||||
|
||||
try:
|
||||
data = get_playlist_data(playlist_scan.playlist.fid, actor=playlist_scan.actor)
|
||||
except Exception:
|
||||
|
|
@ -97,18 +100,30 @@ def start_playlist_scan(playlist_scan):
|
|||
)
|
||||
def scan_playlist_page(playlist_scan, page_url):
|
||||
data = get_playlist_page(playlist_scan.playlist, page_url, playlist_scan.actor)
|
||||
tracks = []
|
||||
plts = []
|
||||
for item_serializer in data["items"]:
|
||||
try:
|
||||
track = item_serializer.save(playlist=playlist_scan.playlist.fid)
|
||||
tracks.append(track)
|
||||
plt = item_serializer.save(playlist=playlist_scan.playlist.fid)
|
||||
# we get any upload owned by the playlist.actor and add a m2m with playlist_libraries
|
||||
upload_qs = plt.track.uploads.filter(
|
||||
library__actor=playlist_scan.playlist.actor
|
||||
)
|
||||
if not upload_qs:
|
||||
logger.debug(
|
||||
f"Could not find a upload for the playlist track {plt.track.title}. Probably the \
|
||||
playlist.library library_scan failed or was not launched by inbox_update_playlist ?"
|
||||
)
|
||||
else:
|
||||
upload_qs[0].playlist_libraries.add(playlist_scan.playlist.library)
|
||||
logger.debug(f"Added {plt.track.title} to playlist library")
|
||||
plts.append(plt)
|
||||
except Exception as e:
|
||||
logger.info(
|
||||
f"Error while saving track to playlist {playlist_scan.playlist}: {e}"
|
||||
)
|
||||
continue
|
||||
|
||||
playlist_scan.processed_files = F("processed_files") + len(tracks)
|
||||
playlist_scan.processed_files = F("processed_files") + len(plts)
|
||||
playlist_scan.modification_date = timezone.now()
|
||||
update_fields = ["modification_date", "processed_files"]
|
||||
|
||||
|
|
|
|||
|
|
@ -170,7 +170,7 @@ class PlaylistViewSet(
|
|||
playlist.playlist_tracks.all().delete()
|
||||
playlist.save(update_fields=["modification_date"])
|
||||
playlist.library.uploads.filter().delete()
|
||||
playlist.schedule_scan(playlist.actor)
|
||||
playlist.schedule_scan(playlist.actor, force=True)
|
||||
return Response(status=204)
|
||||
|
||||
def get_queryset(self):
|
||||
|
|
|
|||
|
|
@ -17,6 +17,8 @@ def test_scan_playlist_page_fetches_page_and_creates_tracks(
|
|||
for i in range(5)
|
||||
]
|
||||
|
||||
for plt in tracks:
|
||||
factories["music.Upload"](track=plt.track, library__actor=scan.playlist.actor)
|
||||
page_conf = {
|
||||
"actor": scan.playlist.actor,
|
||||
"id": scan.playlist.fid,
|
||||
|
|
@ -35,7 +37,8 @@ def test_scan_playlist_page_fetches_page_and_creates_tracks(
|
|||
|
||||
assert len(plts) == 3
|
||||
for track in tracks[:3]:
|
||||
scan.playlist.playlist_tracks.get(fid=track.fid)
|
||||
plt = scan.playlist.playlist_tracks.get(fid=track.fid)
|
||||
scan.playlist.library in plt.track.uploads.all()[0].playlist_libraries.all()
|
||||
|
||||
assert scan.status == "scanning"
|
||||
assert scan.processed_files == 3
|
||||
|
|
|
|||
|
|
@ -197,8 +197,15 @@ def test_add_multiple_tracks_at_once_update_pl_library(
|
|||
for track in tracks:
|
||||
factories["music.Upload"](track=track, library__actor=actor)
|
||||
|
||||
track_already_in_playlist = factories["music.Track"]()
|
||||
upload_already_in_playlist = factories["music.Upload"](
|
||||
track=track_already_in_playlist, library__actor=actor
|
||||
)
|
||||
upload_already_in_playlist.playlist_libraries.add(playlist.library)
|
||||
|
||||
track_ids = [t.id for t in tracks]
|
||||
track_ids.append(not_user_track.id)
|
||||
track_ids.append(track_already_in_playlist.id)
|
||||
mocker.spy(playlist, "insert_many")
|
||||
url = reverse("api:v1:playlists-add", kwargs={"pk": playlist.pk})
|
||||
response = logged_in_api_client.post(url, {"tracks": track_ids})
|
||||
|
|
@ -209,7 +216,8 @@ def test_add_multiple_tracks_at_once_update_pl_library(
|
|||
assert not_user_upload not in playlist.library.uploads.all()
|
||||
for plt in playlist.playlist_tracks.all():
|
||||
for upload in playlist.library.uploads.all():
|
||||
upload.tracks.filter(id=plt.track.id).exists()
|
||||
assert upload.tracks.filter(id=plt.track.id).exists()
|
||||
assert len(upload.playlist_libraries.all()) == 1
|
||||
|
||||
|
||||
def test_honor_max_playlist_size(factories, mocker, logged_in_api_client, preferences):
|
||||
|
|
|
|||
|
|
@ -19,7 +19,13 @@ Users will be able to click on a "Request access to playlist audios files" butto
|
|||
|
||||
`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`
|
||||
`Upload` has also a many_to_many with `Library` through `playlist_libraries` (the same upload can be share various time through various playlists). Reverse relation is `library.playlist_uploads`
|
||||
|
||||
We could migrate from O2M to M2M, but this is super complicated since : - it adds a lot of extra logic (you can't query the m2m if the instance is not save -> this generated problem to validate incoming AP objects) - having a built-in lib and playlist libs make verifications easier (only three built-in lib, playlist_lib are always private)
|
||||
|
||||
##### Workflow
|
||||
|
||||
Playlist activity -> library_scan(get the uploads) -> playlist_scan (set the upload.playlist_relation and create plts)
|
||||
|
||||
##### Federation
|
||||
|
||||
|
|
@ -41,7 +47,12 @@ There is no other reason to share the playlit.library to remote.
|
|||
- [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
|
||||
- [x] update the federation api : when we receive a fetch for a library the upload serializer need to know which lib (playlist lib or user lib)
|
||||
- [x] Support library.playlist_uploads in library scan -> add playlist_uploads in items in library federation viewset
|
||||
- [x] investigate library scan bug : don't delete old content of the lib (local cache?): we need to empty the playlist before the scan(not ideal but less work)
|
||||
- [ ] if the user change the upload to another built-in lib, make sure the upload is not delete (we would loose the playlist_library relation) but only updated.
|
||||
- [ ] enforce actor.libraries to only have three entries.
|
||||
- [ ] enforce upload.playlist_libraries to always be private
|
||||
|
||||
### Follow up
|
||||
|
||||
|
|
@ -49,3 +60,5 @@ There is no other reason to share the playlit.library to remote.
|
|||
- [ ] Finish library drop (delete libraries endpoints)
|
||||
- [ ] Playlist discovery : fetch federation endpoint for playlists
|
||||
- [ ] Playlist discovery : add the playlist to my playlist collection = follow request to playlist
|
||||
- [ ] PLaylist Track activity (to avoid having to refetch the whole playlist)
|
||||
- [ ] Document : The user that want to federate need to activate remote activities in it's user settings. Even if the library is public the playlist activities will not be sended to remote -> We need to implement a followers activity setting
|
||||
|
|
|
|||
|
|
@ -4241,7 +4241,7 @@
|
|||
"showStatus": "Show information about the upload status for this track"
|
||||
},
|
||||
"empty": {
|
||||
"noTracks": "No tracks have been added to this libray yet"
|
||||
"noTracks": "No tracks have been added to this library yet"
|
||||
},
|
||||
"label": {
|
||||
"importStatus": "Import status",
|
||||
|
|
|
|||
Loading…
Reference in New Issue