kopia lustrzana https://dev.funkwhale.audio/funkwhale/funkwhale
Added "refresh=true" API param to artist, track and album detail to retrieve up-to-date data if needed
rodzic
2c697ae2cc
commit
4a277c17bb
|
@ -722,3 +722,6 @@ SUBSONIC_DEFAULT_TRANSCODING_FORMAT = (
|
||||||
|
|
||||||
# extra tags will be ignored
|
# extra tags will be ignored
|
||||||
TAGS_MAX_BY_OBJ = env.int("TAGS_MAX_BY_OBJ", default=30)
|
TAGS_MAX_BY_OBJ = env.int("TAGS_MAX_BY_OBJ", default=30)
|
||||||
|
FEDERATION_OBJECT_FETCH_DELAY = env.int(
|
||||||
|
"FEDERATION_OBJECT_FETCH_DELAY", default=60 * 24 * 3
|
||||||
|
)
|
||||||
|
|
|
@ -202,6 +202,11 @@ class Artist(APIModelMixin):
|
||||||
related_name="attributed_artists",
|
related_name="attributed_artists",
|
||||||
)
|
)
|
||||||
tagged_items = GenericRelation(tags_models.TaggedItem)
|
tagged_items = GenericRelation(tags_models.TaggedItem)
|
||||||
|
fetches = GenericRelation(
|
||||||
|
"federation.Fetch",
|
||||||
|
content_type_field="object_content_type",
|
||||||
|
object_id_field="object_id",
|
||||||
|
)
|
||||||
|
|
||||||
api = musicbrainz.api.artists
|
api = musicbrainz.api.artists
|
||||||
objects = ArtistQuerySet.as_manager()
|
objects = ArtistQuerySet.as_manager()
|
||||||
|
@ -282,6 +287,11 @@ class Album(APIModelMixin):
|
||||||
related_name="attributed_albums",
|
related_name="attributed_albums",
|
||||||
)
|
)
|
||||||
tagged_items = GenericRelation(tags_models.TaggedItem)
|
tagged_items = GenericRelation(tags_models.TaggedItem)
|
||||||
|
fetches = GenericRelation(
|
||||||
|
"federation.Fetch",
|
||||||
|
content_type_field="object_content_type",
|
||||||
|
object_id_field="object_id",
|
||||||
|
)
|
||||||
|
|
||||||
api_includes = ["artist-credits", "recordings", "media", "release-groups"]
|
api_includes = ["artist-credits", "recordings", "media", "release-groups"]
|
||||||
api = musicbrainz.api.releases
|
api = musicbrainz.api.releases
|
||||||
|
@ -463,6 +473,11 @@ class Track(APIModelMixin):
|
||||||
import_hooks = [import_tags]
|
import_hooks = [import_tags]
|
||||||
objects = TrackQuerySet.as_manager()
|
objects = TrackQuerySet.as_manager()
|
||||||
tagged_items = GenericRelation(tags_models.TaggedItem)
|
tagged_items = GenericRelation(tags_models.TaggedItem)
|
||||||
|
fetches = GenericRelation(
|
||||||
|
"federation.Fetch",
|
||||||
|
content_type_field="object_content_type",
|
||||||
|
object_id_field="object_id",
|
||||||
|
)
|
||||||
|
|
||||||
class Meta:
|
class Meta:
|
||||||
ordering = ["album", "disc_number", "position"]
|
ordering = ["album", "disc_number", "position"]
|
||||||
|
|
|
@ -1,3 +1,4 @@
|
||||||
|
import datetime
|
||||||
import logging
|
import logging
|
||||||
import urllib
|
import urllib
|
||||||
|
|
||||||
|
@ -21,7 +22,9 @@ from funkwhale_api.federation.authentication import SignatureAuthentication
|
||||||
from funkwhale_api.federation import actors
|
from funkwhale_api.federation import actors
|
||||||
from funkwhale_api.federation import api_serializers as federation_api_serializers
|
from funkwhale_api.federation import api_serializers as federation_api_serializers
|
||||||
from funkwhale_api.federation import decorators as federation_decorators
|
from funkwhale_api.federation import decorators as federation_decorators
|
||||||
|
from funkwhale_api.federation import models as federation_models
|
||||||
from funkwhale_api.federation import routes
|
from funkwhale_api.federation import routes
|
||||||
|
from funkwhale_api.federation import tasks as federation_tasks
|
||||||
from funkwhale_api.tags.models import Tag, TaggedItem
|
from funkwhale_api.tags.models import Tag, TaggedItem
|
||||||
from funkwhale_api.tags.serializers import TagSerializer
|
from funkwhale_api.tags.serializers import TagSerializer
|
||||||
from funkwhale_api.users.oauth import permissions as oauth_permissions
|
from funkwhale_api.users.oauth import permissions as oauth_permissions
|
||||||
|
@ -59,6 +62,37 @@ def get_libraries(filter_uploads):
|
||||||
return libraries
|
return libraries
|
||||||
|
|
||||||
|
|
||||||
|
def refetch_obj(obj, queryset):
|
||||||
|
"""
|
||||||
|
Given an Artist/Album/Track instance, if the instance is from a remote pod,
|
||||||
|
will attempt to update local data with the latest ActivityPub representation.
|
||||||
|
"""
|
||||||
|
if obj.is_local:
|
||||||
|
return obj
|
||||||
|
|
||||||
|
now = timezone.now()
|
||||||
|
limit = now - datetime.timedelta(minutes=settings.FEDERATION_OBJECT_FETCH_DELAY)
|
||||||
|
last_fetch = obj.fetches.order_by("-creation_date").first()
|
||||||
|
if last_fetch is not None and last_fetch.creation_date > limit:
|
||||||
|
# we fetched recently, no need to do it again
|
||||||
|
return obj
|
||||||
|
|
||||||
|
logger.info("Refetching %s:%s at %s…", obj._meta.label, obj.pk, obj.fid)
|
||||||
|
actor = actors.get_service_actor()
|
||||||
|
fetch = federation_models.Fetch.objects.create(actor=actor, url=obj.fid, object=obj)
|
||||||
|
try:
|
||||||
|
federation_tasks.fetch(fetch_id=fetch.pk)
|
||||||
|
except Exception:
|
||||||
|
logger.exception(
|
||||||
|
"Error while refetching %s:%s at %s…", obj._meta.label, obj.pk, obj.fid
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
fetch.refresh_from_db()
|
||||||
|
if fetch.status == "finished":
|
||||||
|
obj = queryset.get(pk=obj.pk)
|
||||||
|
return obj
|
||||||
|
|
||||||
|
|
||||||
class ArtistViewSet(common_views.SkipFilterForGetObject, viewsets.ReadOnlyModelViewSet):
|
class ArtistViewSet(common_views.SkipFilterForGetObject, viewsets.ReadOnlyModelViewSet):
|
||||||
queryset = models.Artist.objects.all().select_related("attributed_to")
|
queryset = models.Artist.objects.all().select_related("attributed_to")
|
||||||
serializer_class = serializers.ArtistWithAlbumsSerializer
|
serializer_class = serializers.ArtistWithAlbumsSerializer
|
||||||
|
@ -71,6 +105,16 @@ class ArtistViewSet(common_views.SkipFilterForGetObject, viewsets.ReadOnlyModelV
|
||||||
fetches = federation_decorators.fetches_route()
|
fetches = federation_decorators.fetches_route()
|
||||||
mutations = common_decorators.mutations_route(types=["update"])
|
mutations = common_decorators.mutations_route(types=["update"])
|
||||||
|
|
||||||
|
def get_object(self):
|
||||||
|
obj = super().get_object()
|
||||||
|
|
||||||
|
if (
|
||||||
|
self.action == "retrieve"
|
||||||
|
and self.request.GET.get("refresh", "").lower() == "true"
|
||||||
|
):
|
||||||
|
obj = refetch_obj(obj, self.get_queryset())
|
||||||
|
return obj
|
||||||
|
|
||||||
def get_queryset(self):
|
def get_queryset(self):
|
||||||
queryset = super().get_queryset()
|
queryset = super().get_queryset()
|
||||||
albums = models.Album.objects.with_tracks_count()
|
albums = models.Album.objects.with_tracks_count()
|
||||||
|
@ -106,6 +150,16 @@ class AlbumViewSet(common_views.SkipFilterForGetObject, viewsets.ReadOnlyModelVi
|
||||||
fetches = federation_decorators.fetches_route()
|
fetches = federation_decorators.fetches_route()
|
||||||
mutations = common_decorators.mutations_route(types=["update"])
|
mutations = common_decorators.mutations_route(types=["update"])
|
||||||
|
|
||||||
|
def get_object(self):
|
||||||
|
obj = super().get_object()
|
||||||
|
|
||||||
|
if (
|
||||||
|
self.action == "retrieve"
|
||||||
|
and self.request.GET.get("refresh", "").lower() == "true"
|
||||||
|
):
|
||||||
|
obj = refetch_obj(obj, self.get_queryset())
|
||||||
|
return obj
|
||||||
|
|
||||||
def get_queryset(self):
|
def get_queryset(self):
|
||||||
queryset = super().get_queryset()
|
queryset = super().get_queryset()
|
||||||
tracks = (
|
tracks = (
|
||||||
|
@ -212,6 +266,16 @@ class TrackViewSet(common_views.SkipFilterForGetObject, viewsets.ReadOnlyModelVi
|
||||||
fetches = federation_decorators.fetches_route()
|
fetches = federation_decorators.fetches_route()
|
||||||
mutations = common_decorators.mutations_route(types=["update"])
|
mutations = common_decorators.mutations_route(types=["update"])
|
||||||
|
|
||||||
|
def get_object(self):
|
||||||
|
obj = super().get_object()
|
||||||
|
|
||||||
|
if (
|
||||||
|
self.action == "retrieve"
|
||||||
|
and self.request.GET.get("refresh", "").lower() == "true"
|
||||||
|
):
|
||||||
|
obj = refetch_obj(obj, self.get_queryset())
|
||||||
|
return obj
|
||||||
|
|
||||||
def get_queryset(self):
|
def get_queryset(self):
|
||||||
queryset = super().get_queryset()
|
queryset = super().get_queryset()
|
||||||
filter_favorites = self.request.GET.get("favorites", None)
|
filter_favorites = self.request.GET.get("favorites", None)
|
||||||
|
|
|
@ -1,7 +1,9 @@
|
||||||
|
import datetime
|
||||||
import io
|
import io
|
||||||
import magic
|
import magic
|
||||||
import os
|
import os
|
||||||
import urllib.parse
|
import urllib.parse
|
||||||
|
import uuid
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
from django.urls import reverse
|
from django.urls import reverse
|
||||||
|
@ -10,6 +12,7 @@ from django.utils import timezone
|
||||||
from funkwhale_api.common import utils
|
from funkwhale_api.common import utils
|
||||||
from funkwhale_api.federation import api_serializers as federation_api_serializers
|
from funkwhale_api.federation import api_serializers as federation_api_serializers
|
||||||
from funkwhale_api.federation import utils as federation_utils
|
from funkwhale_api.federation import utils as federation_utils
|
||||||
|
from funkwhale_api.federation import tasks as federation_tasks
|
||||||
from funkwhale_api.music import licenses, models, serializers, tasks, views
|
from funkwhale_api.music import licenses, models, serializers, tasks, views
|
||||||
|
|
||||||
DATA_DIR = os.path.dirname(os.path.abspath(__file__))
|
DATA_DIR = os.path.dirname(os.path.abspath(__file__))
|
||||||
|
@ -826,3 +829,94 @@ def test_oembed_artist(factories, no_api_auth, api_client, settings):
|
||||||
response = api_client.get(url, {"url": artist_url, "format": "json"})
|
response = api_client.get(url, {"url": artist_url, "format": "json"})
|
||||||
|
|
||||||
assert response.data == expected
|
assert response.data == expected
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"factory_name, url_name",
|
||||||
|
[
|
||||||
|
("music.Artist", "api:v1:artists-detail"),
|
||||||
|
("music.Album", "api:v1:albums-detail"),
|
||||||
|
("music.Track", "api:v1:tracks-detail"),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_refresh_remote_entity_when_param_is_true(
|
||||||
|
factories,
|
||||||
|
factory_name,
|
||||||
|
url_name,
|
||||||
|
mocker,
|
||||||
|
logged_in_api_client,
|
||||||
|
queryset_equal_queries,
|
||||||
|
):
|
||||||
|
obj = factories[factory_name](mbid=None)
|
||||||
|
|
||||||
|
assert obj.is_local is False
|
||||||
|
|
||||||
|
new_mbid = uuid.uuid4()
|
||||||
|
|
||||||
|
def fake_refetch(obj, queryset):
|
||||||
|
obj.mbid = new_mbid
|
||||||
|
return obj
|
||||||
|
|
||||||
|
refetch_obj = mocker.patch.object(views, "refetch_obj", side_effect=fake_refetch)
|
||||||
|
url = reverse(url_name, kwargs={"pk": obj.pk})
|
||||||
|
response = logged_in_api_client.get(url, {"refresh": "true"})
|
||||||
|
|
||||||
|
assert response.status_code == 200
|
||||||
|
assert response.data["mbid"] == str(new_mbid)
|
||||||
|
assert refetch_obj.call_count == 1
|
||||||
|
assert refetch_obj.call_args[0][0] == obj
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("param", ["false", "0", ""])
|
||||||
|
def test_refresh_remote_entity_no_param(
|
||||||
|
factories, param, mocker, logged_in_api_client, service_actor
|
||||||
|
):
|
||||||
|
obj = factories["music.Artist"](mbid=None)
|
||||||
|
|
||||||
|
assert obj.is_local is False
|
||||||
|
|
||||||
|
fetch_task = mocker.patch.object(federation_tasks, "fetch")
|
||||||
|
url = reverse("api:v1:artists-detail", kwargs={"pk": obj.pk})
|
||||||
|
response = logged_in_api_client.get(url, {"refresh": param})
|
||||||
|
|
||||||
|
assert response.status_code == 200
|
||||||
|
fetch_task.assert_not_called()
|
||||||
|
assert service_actor.fetches.count() == 0
|
||||||
|
|
||||||
|
|
||||||
|
def test_refetch_obj_not_local(mocker, factories, service_actor):
|
||||||
|
obj = factories["music.Artist"](local=True)
|
||||||
|
fetch_task = mocker.patch.object(federation_tasks, "fetch")
|
||||||
|
assert views.refetch_obj(obj, obj.__class__.objects.all()) == obj
|
||||||
|
fetch_task.assert_not_called()
|
||||||
|
assert service_actor.fetches.count() == 0
|
||||||
|
|
||||||
|
|
||||||
|
def test_refetch_obj_last_fetch_date_too_close(
|
||||||
|
mocker, factories, settings, service_actor
|
||||||
|
):
|
||||||
|
settings.FEDERATION_OBJECT_FETCH_DELAY = 300
|
||||||
|
obj = factories["music.Artist"]()
|
||||||
|
factories["federation.Fetch"](
|
||||||
|
object=obj,
|
||||||
|
creation_date=timezone.now()
|
||||||
|
- datetime.timedelta(minutes=settings.FEDERATION_OBJECT_FETCH_DELAY - 1),
|
||||||
|
)
|
||||||
|
fetch_task = mocker.patch.object(federation_tasks, "fetch")
|
||||||
|
assert views.refetch_obj(obj, obj.__class__.objects.all()) == obj
|
||||||
|
fetch_task.assert_not_called()
|
||||||
|
assert service_actor.fetches.count() == 0
|
||||||
|
|
||||||
|
|
||||||
|
def test_refetch_obj(mocker, factories, settings, service_actor):
|
||||||
|
settings.FEDERATION_OBJECT_FETCH_DELAY = 300
|
||||||
|
obj = factories["music.Artist"]()
|
||||||
|
factories["federation.Fetch"](
|
||||||
|
object=obj,
|
||||||
|
creation_date=timezone.now()
|
||||||
|
- datetime.timedelta(minutes=settings.FEDERATION_OBJECT_FETCH_DELAY + 1),
|
||||||
|
)
|
||||||
|
fetch_task = mocker.patch.object(federation_tasks, "fetch")
|
||||||
|
views.refetch_obj(obj, obj.__class__.objects.all())
|
||||||
|
fetch = obj.fetches.filter(actor=service_actor).order_by("-creation_date").first()
|
||||||
|
fetch_task.assert_called_once_with(fetch_id=fetch.pk)
|
||||||
|
|
|
@ -0,0 +1 @@
|
||||||
|
Now refetch remote ActivityPub artists, albums and tracks to avoid local stale data
|
|
@ -296,6 +296,7 @@ paths:
|
||||||
summary: Retrieve a single artist
|
summary: Retrieve a single artist
|
||||||
parameters:
|
parameters:
|
||||||
- $ref: "#/parameters/ObjectId"
|
- $ref: "#/parameters/ObjectId"
|
||||||
|
- $ref: "#/parameters/Refresh"
|
||||||
security:
|
security:
|
||||||
- oauth2:
|
- oauth2:
|
||||||
- "read:libraries"
|
- "read:libraries"
|
||||||
|
@ -395,6 +396,7 @@ paths:
|
||||||
summary: Retrieve a single album
|
summary: Retrieve a single album
|
||||||
parameters:
|
parameters:
|
||||||
- $ref: "#/parameters/ObjectId"
|
- $ref: "#/parameters/ObjectId"
|
||||||
|
- $ref: "#/parameters/Refresh"
|
||||||
|
|
||||||
security:
|
security:
|
||||||
- oauth2:
|
- oauth2:
|
||||||
|
@ -518,6 +520,7 @@ paths:
|
||||||
get:
|
get:
|
||||||
parameters:
|
parameters:
|
||||||
- $ref: "#/parameters/ObjectId"
|
- $ref: "#/parameters/ObjectId"
|
||||||
|
- $ref: "#/parameters/Refresh"
|
||||||
summary: Retrieve a single track
|
summary: Retrieve a single track
|
||||||
|
|
||||||
security:
|
security:
|
||||||
|
@ -974,6 +977,14 @@ parameters:
|
||||||
schema:
|
schema:
|
||||||
required: false
|
required: false
|
||||||
type: "boolean"
|
type: "boolean"
|
||||||
|
Refresh:
|
||||||
|
name: "refresh"
|
||||||
|
in: "query"
|
||||||
|
default: false
|
||||||
|
description: "Trigger an ActivityPub fetch to refresh local data"
|
||||||
|
schema:
|
||||||
|
required: false
|
||||||
|
type: "boolean"
|
||||||
|
|
||||||
responses:
|
responses:
|
||||||
200:
|
200:
|
||||||
|
|
|
@ -146,7 +146,7 @@ export default {
|
||||||
this.isLoading = true
|
this.isLoading = true
|
||||||
let url = FETCH_URL + this.id + "/"
|
let url = FETCH_URL + this.id + "/"
|
||||||
logger.default.debug('Fetching album "' + this.id + '"')
|
logger.default.debug('Fetching album "' + this.id + '"')
|
||||||
axios.get(url).then(response => {
|
axios.get(url, {params: {refresh: 'true'}}).then(response => {
|
||||||
self.object = backend.Album.clean(response.data)
|
self.object = backend.Album.clean(response.data)
|
||||||
self.discs = self.object.tracks.reduce(groupByDisc, [])
|
self.discs = self.object.tracks.reduce(groupByDisc, [])
|
||||||
self.isLoading = false
|
self.isLoading = false
|
||||||
|
|
|
@ -179,7 +179,7 @@ export default {
|
||||||
|
|
||||||
})
|
})
|
||||||
|
|
||||||
let artistPromise = axios.get("artists/" + this.id + "/").then(response => {
|
let artistPromise = axios.get("artists/" + this.id + "/", {params: {refresh: 'true'}}).then(response => {
|
||||||
self.object = response.data
|
self.object = response.data
|
||||||
})
|
})
|
||||||
await trackPromise
|
await trackPromise
|
||||||
|
|
|
@ -155,7 +155,7 @@ export default {
|
||||||
this.isLoadingTrack = true
|
this.isLoadingTrack = true
|
||||||
let url = FETCH_URL + this.id + "/"
|
let url = FETCH_URL + this.id + "/"
|
||||||
logger.default.debug('Fetching track "' + this.id + '"')
|
logger.default.debug('Fetching track "' + this.id + '"')
|
||||||
axios.get(url).then(response => {
|
axios.get(url, {params: {refresh: 'true'}}).then(response => {
|
||||||
self.track = response.data
|
self.track = response.data
|
||||||
self.isLoadingTrack = false
|
self.isLoadingTrack = false
|
||||||
})
|
})
|
||||||
|
|
Ładowanie…
Reference in New Issue