diff --git a/api/config/settings/common.py b/api/config/settings/common.py index 9ce7a092d05ee1ac10f487c709fcbbb394869258..5925118b8f5ce2db92267234f29ed407f12de689 100644 --- a/api/config/settings/common.py +++ b/api/config/settings/common.py @@ -955,3 +955,7 @@ INSTANCE_SUPPORT_MESSAGE_DELAY = env.int("INSTANCE_SUPPORT_MESSAGE_DELAY", defau FUNKWHALE_SUPPORT_MESSAGE_DELAY = env.int("FUNKWHALE_SUPPORT_MESSAGE_DELAY", default=15) # XXX Stable release: remove USE_FULL_TEXT_SEARCH = env.bool("USE_FULL_TEXT_SEARCH", default=True) + +MIN_DELAY_BETWEEN_DOWNLOADS_COUNT = env.int( + "MIN_DELAY_BETWEEN_DOWNLOADS_COUNT", default=60 * 60 * 6 +) diff --git a/api/funkwhale_api/audio/serializers.py b/api/funkwhale_api/audio/serializers.py index a946df9a90e09ae80722dc7d80357cf76593b65b..205ec383ca2536097c577bf77f7f141192bf6afa 100644 --- a/api/funkwhale_api/audio/serializers.py +++ b/api/funkwhale_api/audio/serializers.py @@ -97,6 +97,15 @@ class ChannelSerializer(serializers.ModelSerializer): def get_artist(self, obj): return music_serializers.serialize_artist_simple(obj.artist) + def to_representation(self, obj): + data = super().to_representation(obj) + if self.context.get("subscriptions_count"): + data["subscriptions_count"] = self.get_subscriptions_count(obj) + return data + + def get_subscriptions_count(self, obj): + return obj.actor.received_follows.exclude(approved=False).count() + class SubscriptionSerializer(serializers.Serializer): approved = serializers.BooleanField(read_only=True) diff --git a/api/funkwhale_api/audio/views.py b/api/funkwhale_api/audio/views.py index ba9983672a6ff826d97fcc4bedd42539191bdf57..5162730a6310b06c2ccaaf13d9f0f6eafacaa094 100644 --- a/api/funkwhale_api/audio/views.py +++ b/api/funkwhale_api/audio/views.py @@ -92,6 +92,11 @@ class ChannelViewSet( request.user.actor.emitted_follows.filter(target=object.actor).delete() return response.Response(status=204) + def get_serializer_context(self): + context = super().get_serializer_context() + context["subscriptions_count"] = self.action in ["retrieve", "create", "update"] + return context + class SubscriptionsViewSet( ChannelsMixin, diff --git a/api/funkwhale_api/common/throttling.py b/api/funkwhale_api/common/throttling.py index fe4a5d93464f5c1560352e490ef9f79c6262acc9..8d37efe245180c51bc725c3f7a459ae1f5336133 100644 --- a/api/funkwhale_api/common/throttling.py +++ b/api/funkwhale_api/common/throttling.py @@ -6,9 +6,9 @@ from rest_framework import throttling as rest_throttling from django.conf import settings -def get_ident(request): - if hasattr(request, "user") and request.user.is_authenticated: - return {"type": "authenticated", "id": request.user.pk} +def get_ident(user, request): + if user and user.is_authenticated: + return {"type": "authenticated", "id": user.pk} ident = rest_throttling.BaseThrottle().get_ident(request) return {"type": "anonymous", "id": ident} @@ -89,7 +89,7 @@ class FunkwhaleThrottle(rest_throttling.SimpleRateThrottle): def allow_request(self, request, view): self.request = request - self.ident = get_ident(request) + self.ident = get_ident(getattr(request, "user", None), request) action = getattr(view, "action", "*") view_scopes = getattr(view, "throttling_scopes", {}) if view_scopes is None: diff --git a/api/funkwhale_api/common/views.py b/api/funkwhale_api/common/views.py index b6f8d5a0af4ff36af0f51d5877bbcbf360bf5117..58758773de02be354442f3e686b1f766cf8b687c 100644 --- a/api/funkwhale_api/common/views.py +++ b/api/funkwhale_api/common/views.py @@ -135,7 +135,7 @@ class RateLimitView(views.APIView): throttle_classes = [] def get(self, request, *args, **kwargs): - ident = throttling.get_ident(request) + ident = throttling.get_ident(getattr(request, "user", None), request) data = { "enabled": settings.THROTTLING_ENABLED, "ident": ident, diff --git a/api/funkwhale_api/music/migrations/0048_auto_20200120_0900.py b/api/funkwhale_api/music/migrations/0048_auto_20200120_0900.py new file mode 100644 index 0000000000000000000000000000000000000000..5f9e1b46635959a7d32e48a85a87b7dd61d23b8b --- /dev/null +++ b/api/funkwhale_api/music/migrations/0048_auto_20200120_0900.py @@ -0,0 +1,23 @@ +# Generated by Django 2.2.9 on 2020-01-20 09:00 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('music', '0047_auto_20200116_1246'), + ] + + operations = [ + migrations.AddField( + model_name='track', + name='downloads_count', + field=models.PositiveIntegerField(default=0), + ), + migrations.AddField( + model_name='upload', + name='downloads_count', + field=models.PositiveIntegerField(default=0), + ), + ] diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index 0d02236dd94f2822e7be1003108dcdc5b4c52047..57379453aa5d479d745bb658c837abb04ad93755 100644 --- a/api/funkwhale_api/music/models.py +++ b/api/funkwhale_api/music/models.py @@ -498,7 +498,7 @@ class Track(APIModelMixin): on_delete=models.SET_NULL, related_name="covered_track", ) - + downloads_count = models.PositiveIntegerField(default=0) federation_namespace = "tracks" musicbrainz_model = "recording" api = musicbrainz.api.recordings @@ -731,6 +731,7 @@ class Upload(models.Model): from_activity = models.ForeignKey( "federation.Activity", null=True, on_delete=models.SET_NULL, blank=True ) + downloads_count = models.PositiveIntegerField(default=0) objects = UploadQuerySet.as_manager() diff --git a/api/funkwhale_api/music/utils.py b/api/funkwhale_api/music/utils.py index d576ad0672c10706186e0cd727c5091d15c72ddb..5097db9ab9f2d6a4bd2f866d9ab7cd4e4c81fe73 100644 --- a/api/funkwhale_api/music/utils.py +++ b/api/funkwhale_api/music/utils.py @@ -4,6 +4,11 @@ import magic import mutagen import pydub +from django.conf import settings +from django.core.cache import cache +from django.db.models import F + +from funkwhale_api.common import throttling from funkwhale_api.common.search import get_fts_query # noqa from funkwhale_api.common.search import get_query # noqa from funkwhale_api.common.search import normalize_query # noqa @@ -91,3 +96,25 @@ def transcode_file(input, output, input_format, output_format, **kwargs): def transcode_audio(audio, output, output_format, **kwargs): with output.open("wb"): return audio.export(output, format=output_format, **kwargs) + + +def increment_downloads_count(upload, user, wsgi_request): + ident = throttling.get_ident(user=user, request=wsgi_request) + cache_key = "downloads_count:upload-{}:{}-{}".format( + upload.pk, ident["type"], ident["id"] + ) + + value = cache.get(cache_key) + if value: + # download already tracked + return + + upload.downloads_count = F("downloads_count") + 1 + upload.track.downloads_count = F("downloads_count") + 1 + + upload.save(update_fields=["downloads_count"]) + upload.track.save(update_fields=["downloads_count"]) + + duration = max(upload.duration or 0, settings.MIN_DELAY_BETWEEN_DOWNLOADS_COUNT) + + cache.set(cache_key, 1, duration) diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index 28babcc9629bfc6201fda254a8e6e71d55235e44..2ec0c78be00a8ad1589bdb1ca87c44e4ae022954 100644 --- a/api/funkwhale_api/music/views.py +++ b/api/funkwhale_api/music/views.py @@ -432,6 +432,23 @@ def get_content_disposition(filename): return "attachment; {}".format(filename) +def record_downloads(f): + def inner(*args, **kwargs): + user = kwargs.get("user") + wsgi_request = kwargs.pop("wsgi_request") + upload = kwargs.get("upload") + response = f(*args, **kwargs) + if response.status_code >= 200 and response.status_code < 400: + utils.increment_downloads_count( + upload=upload, user=user, wsgi_request=wsgi_request + ) + + return response + + return inner + + +@record_downloads def handle_serve( upload, user, format=None, max_bitrate=None, proxy_media=True, download=True ): @@ -537,12 +554,13 @@ class ListenViewSet(mixins.RetrieveModelMixin, viewsets.GenericViewSet): if max_bitrate: max_bitrate = max_bitrate * 1000 return handle_serve( - upload, + upload=upload, user=request.user, format=format, max_bitrate=max_bitrate, proxy_media=settings.PROXY_MEDIA, download=download, + wsgi_request=request._request, ) diff --git a/api/funkwhale_api/subsonic/views.py b/api/funkwhale_api/subsonic/views.py index 4e70f55838c8e81fee817e326fb5b270c132e408..e656b5bdc6281861c4db111bc3973b82a9ad36c7 100644 --- a/api/funkwhale_api/subsonic/views.py +++ b/api/funkwhale_api/subsonic/views.py @@ -285,6 +285,7 @@ class SubsonicViewSet(viewsets.GenericViewSet): # Subsonic clients don't expect 302 redirection unfortunately, # So we have to proxy media files proxy_media=True, + wsgi_request=request._request, ) @action(detail=False, methods=["get", "post"], url_name="star", url_path="star") diff --git a/api/tests/audio/test_serializers.py b/api/tests/audio/test_serializers.py index b431e8e96fdf19551596442f67933d24dd27546d..7f39bb338f149e7f3d956aa92f8b6ffc544cdbe7 100644 --- a/api/tests/audio/test_serializers.py +++ b/api/tests/audio/test_serializers.py @@ -90,6 +90,16 @@ def test_channel_serializer_representation(factories, to_api_date): assert serializers.ChannelSerializer(channel).data == expected +def test_channel_serializer_representation_subscriptions_count(factories, to_api_date): + channel = factories["audio.Channel"]() + factories["federation.Follow"](target=channel.actor) + factories["federation.Follow"](target=channel.actor, approved=False) + serializer = serializers.ChannelSerializer( + channel, context={"subscriptions_count": True} + ) + assert serializer.data["subscriptions_count"] == 1 + + def test_subscription_serializer(factories, to_api_date): subscription = factories["audio.Subscription"]() expected = { diff --git a/api/tests/audio/test_views.py b/api/tests/audio/test_views.py index 0724bfa98b07e69008c0d4abaa5a04625fede72b..935ee434276fcb686ea2589c3af61d1b9546038b 100644 --- a/api/tests/audio/test_views.py +++ b/api/tests/audio/test_views.py @@ -41,7 +41,9 @@ def test_channel_create(logged_in_api_client): def test_channel_detail(factories, logged_in_api_client): channel = factories["audio.Channel"](artist__description=None) url = reverse("api:v1:channels-detail", kwargs={"uuid": channel.uuid}) - expected = serializers.ChannelSerializer(channel).data + expected = serializers.ChannelSerializer( + channel, context={"subscriptions_count": True} + ).data response = logged_in_api_client.get(url) assert response.status_code == 200 diff --git a/api/tests/common/test_throttling.py b/api/tests/common/test_throttling.py index 6b000fcaf6ad3bc286eab799d0f442602792bbd9..76ff2578b676ea3d2d30cc5970aed499ff8c156e 100644 --- a/api/tests/common/test_throttling.py +++ b/api/tests/common/test_throttling.py @@ -10,15 +10,14 @@ def test_get_ident_anonymous(api_request): expected = {"id": ip, "type": "anonymous"} - assert throttling.get_ident(request) == expected + assert throttling.get_ident(None, request) == expected def test_get_ident_authenticated(api_request, factories): user = factories["users.User"]() request = api_request.get("/") - setattr(request, "user", user) expected = {"id": user.pk, "type": "authenticated"} - assert throttling.get_ident(request) == expected + assert throttling.get_ident(user, request) == expected @pytest.mark.parametrize( diff --git a/api/tests/music/test_utils.py b/api/tests/music/test_utils.py index 982422c34a07978d9438d44044cd59f550682f30..c05b530aa2949c439b8a3a34599eee60148e977b 100644 --- a/api/tests/music/test_utils.py +++ b/api/tests/music/test_utils.py @@ -45,3 +45,49 @@ def test_guess_mimetype_dont_crash_with_s3(factories, mocker, settings): f = factories["music.Upload"].build(audio_file__filename="test.mp3") assert utils.guess_mimetype(f.audio_file) == "audio/mpeg" + + +def test_increment_downloads_count(factories, mocker, cache, anonymous_user, settings): + ident = {"type": "anonymous", "id": "noop"} + get_ident = mocker.patch( + "funkwhale_api.common.throttling.get_ident", return_value=ident + ) + cache_set = mocker.spy(utils.cache, "set") + wsgi_request = mocker.Mock(META={}) + upload = factories["music.Upload"]() + utils.increment_downloads_count( + upload=upload, user=anonymous_user, wsgi_request=wsgi_request + ) + + upload.refresh_from_db() + get_ident.assert_called_once_with(user=anonymous_user, request=wsgi_request) + + assert upload.downloads_count == 1 + assert upload.track.downloads_count == 1 + cache_set.assert_called_once_with( + "downloads_count:upload-{}:{}-{}".format(upload.pk, ident["type"], ident["id"]), + 1, + settings.MIN_DELAY_BETWEEN_DOWNLOADS_COUNT, + ) + + +def test_increment_downloads_count_already_tracked( + factories, mocker, cache, anonymous_user +): + ident = {"type": "anonymous", "id": "noop"} + mocker.patch("funkwhale_api.common.throttling.get_ident", return_value=ident) + wsgi_request = mocker.Mock(META={}) + upload = factories["music.Upload"]() + cache.set( + "downloads_count:upload-{}:{}-{}".format(upload.pk, ident["type"], ident["id"]), + 1, + ) + + utils.increment_downloads_count( + upload=upload, user=anonymous_user, wsgi_request=wsgi_request + ) + + upload.refresh_from_db() + + assert upload.downloads_count == 0 + assert upload.track.downloads_count == 0 diff --git a/api/tests/music/test_views.py b/api/tests/music/test_views.py index f78e6988c9aee7a0bd72d669e0b4ea3f8db39d5a..68ee32b36fd595240609efef66bc93597c9826ea 100644 --- a/api/tests/music/test_views.py +++ b/api/tests/music/test_views.py @@ -352,11 +352,16 @@ def test_serve_updates_access_date(factories, settings, api_client, preferences) assert upload.accessed_date > now -def test_listen_no_track(factories, logged_in_api_client): +def test_listen_no_track(factories, logged_in_api_client, mocker): + increment_downloads_count = mocker.patch( + "funkwhale_api.music.utils.increment_downloads_count" + ) + url = reverse("api:v1:listen-detail", kwargs={"uuid": "noop"}) response = logged_in_api_client.get(url) assert response.status_code == 404 + increment_downloads_count.call_count == 0 def test_listen_no_file(factories, logged_in_api_client): @@ -375,7 +380,10 @@ def test_listen_no_available_file(factories, logged_in_api_client): assert response.status_code == 404 -def test_listen_correct_access(factories, logged_in_api_client): +def test_listen_correct_access(factories, logged_in_api_client, mocker): + increment_downloads_count = mocker.patch( + "funkwhale_api.music.utils.increment_downloads_count" + ) logged_in_api_client.user.create_actor() upload = factories["music.Upload"]( library__actor=logged_in_api_client.user.actor, @@ -391,6 +399,12 @@ def test_listen_correct_access(factories, logged_in_api_client): urllib.parse.quote(expected_filename) ) + increment_downloads_count.assert_called_once_with( + upload=upload, + user=logged_in_api_client.user, + wsgi_request=response.wsgi_request, + ) + def test_listen_correct_access_download_false(factories, logged_in_api_client): logged_in_api_client.user.create_actor() @@ -419,12 +433,13 @@ def test_listen_explicit_file(factories, logged_in_api_client, mocker, settings) assert response.status_code == 200 mocked_serve.assert_called_once_with( - upload2, + upload=upload2, user=logged_in_api_client.user, format=None, max_bitrate=None, proxy_media=settings.PROXY_MEDIA, download=True, + wsgi_request=response.wsgi_request, ) @@ -484,10 +499,13 @@ def test_should_transcode_according_to_preference(value, preferences, factories) assert views.should_transcode(upload, "mp3") is expected -def test_handle_serve_create_mp3_version(factories, now): +def test_handle_serve_create_mp3_version(factories, now, mocker): + mocker.patch("funkwhale_api.music.utils.increment_downloads_count") user = factories["users.User"]() upload = factories["music.Upload"](bitrate=42) - response = views.handle_serve(upload, user, format="mp3") + response = views.handle_serve( + upload=upload, user=user, format="mp3", wsgi_request=None + ) expected_filename = upload.track.full_name + ".mp3" version = upload.versions.latest("id") @@ -514,12 +532,13 @@ def test_listen_transcode(factories, now, logged_in_api_client, mocker, settings assert response.status_code == 200 handle_serve.assert_called_once_with( - upload, + upload=upload, user=logged_in_api_client.user, format="mp3", max_bitrate=None, proxy_media=settings.PROXY_MEDIA, download=True, + wsgi_request=response.wsgi_request, ) @@ -547,12 +566,13 @@ def test_listen_transcode_bitrate( assert response.status_code == 200 handle_serve.assert_called_once_with( - upload, + upload=upload, user=logged_in_api_client.user, format=None, max_bitrate=expected, proxy_media=settings.PROXY_MEDIA, download=True, + wsgi_request=response.wsgi_request, ) @@ -578,12 +598,13 @@ def test_listen_transcode_in_place( assert response.status_code == 200 handle_serve.assert_called_once_with( - upload, + upload=upload, user=logged_in_api_client.user, format="mp3", max_bitrate=None, proxy_media=settings.PROXY_MEDIA, download=True, + wsgi_request=response.wsgi_request, ) diff --git a/api/tests/subsonic/test_views.py b/api/tests/subsonic/test_views.py index 0e7acfd66d0d910bc7970108cb79b5756612c4ca..24c66273b082418fa6ab85194258dbd2da2ce9d1 100644 --- a/api/tests/subsonic/test_views.py +++ b/api/tests/subsonic/test_views.py @@ -236,6 +236,7 @@ def test_stream( format=None, max_bitrate=None, proxy_media=True, + wsgi_request=response.wsgi_request, ) assert response.status_code == 200 playable_by.assert_called_once_with(music_models.Track.objects.all(), None) @@ -256,6 +257,7 @@ def test_stream_format(format, expected, logged_in_api_client, factories, mocker format=expected, max_bitrate=None, proxy_media=True, + wsgi_request=response.wsgi_request, ) assert response.status_code == 200 @@ -305,6 +307,7 @@ def test_stream_transcode( format=expected_format, max_bitrate=expected_bitrate, proxy_media=True, + wsgi_request=response.wsgi_request, ) assert response.status_code == 200