See #170: now record downloads count on tracks/uploads

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
+)

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:

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,

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),
+        ),
+    ]

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()
 

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)

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,
         )
 
 

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")

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(

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

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,
     )
 
 

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