From 81e7b900fea7874adda47846c5740ab9b01f64aa Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Thu, 10 Jan 2019 11:55:48 +0100 Subject: [PATCH 1/2] Fixed https url-reversing issue in development --- .env.dev | 3 +++ api/config/settings/local.py | 3 +++ api/funkwhale_api/common/middleware.py | 22 ++++++++++++++++++++++ 3 files changed, 28 insertions(+) diff --git a/.env.dev b/.env.dev index f13026e26..84d8ca19b 100644 --- a/.env.dev +++ b/.env.dev @@ -12,3 +12,6 @@ MUSIC_DIRECTORY_PATH=/music BROWSABLE_API_ENABLED=True FORWARDED_PROTO=http LDAP_ENABLED=False + +# Uncomment this if you're using traefik/https +# FORCE_HTTPS_URLS=True diff --git a/api/config/settings/local.py b/api/config/settings/local.py index 91a202b64..ce0a72cfc 100644 --- a/api/config/settings/local.py +++ b/api/config/settings/local.py @@ -14,6 +14,7 @@ from .common import * # noqa # DEBUG # ------------------------------------------------------------------------------ DEBUG = env.bool("DJANGO_DEBUG", default=True) +FORCE_HTTPS_URLS = env.bool("FORCE_HTTPS_URLS", default=False) TEMPLATES[0]["OPTIONS"]["debug"] = DEBUG # SECRET CONFIGURATION @@ -80,3 +81,5 @@ CSRF_TRUSTED_ORIGINS = [o for o in ALLOWED_HOSTS] if env.bool("WEAK_PASSWORDS", default=False): # Faster during tests PASSWORD_HASHERS = ("django.contrib.auth.hashers.MD5PasswordHasher",) + +MIDDLEWARE = ("funkwhale_api.common.middleware.DevHttpsMiddleware",) + MIDDLEWARE diff --git a/api/funkwhale_api/common/middleware.py b/api/funkwhale_api/common/middleware.py index 090d67c9e..96e9c45a6 100644 --- a/api/funkwhale_api/common/middleware.py +++ b/api/funkwhale_api/common/middleware.py @@ -135,3 +135,25 @@ class SPAFallbackMiddleware: return serve_spa(request) return response + + +class DevHttpsMiddleware: + """ + In development, it's sometimes difficult to have django use HTTPS + when we have django behind nginx behind traefix. + + We thus use a simple setting (in dev ONLY) to control that. + """ + + def __init__(self, get_response): + self.get_response = get_response + + def __call__(self, request): + if settings.FORCE_HTTPS_URLS: + setattr(request.__class__, "scheme", "https") + setattr( + request, + "get_host", + lambda: request.__class__.get_host(request).replace(":80", ":443"), + ) + return self.get_response(request) From 3916986fb878221325715807aac7d2044f900548 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Thu, 10 Jan 2019 11:58:18 +0100 Subject: [PATCH 2/2] Performance improvement when fetching favorites, down to a single, small http request --- api/funkwhale_api/favorites/views.py | 16 ++++++++++++++++ api/tests/favorites/test_favorites.py | 12 ++++++++++++ .../favorites-performance.enhancement | 1 + front/src/store/favorites.js | 12 ++---------- 4 files changed, 31 insertions(+), 10 deletions(-) create mode 100644 changes/changelog.d/favorites-performance.enhancement diff --git a/api/funkwhale_api/favorites/views.py b/api/funkwhale_api/favorites/views.py index 5f4e1cd42..e02d1a3e4 100644 --- a/api/funkwhale_api/favorites/views.py +++ b/api/funkwhale_api/favorites/views.py @@ -71,3 +71,19 @@ class TrackFavoriteViewSet( return Response({}, status=400) favorite.delete() return Response([], status=status.HTTP_204_NO_CONTENT) + + @list_route(methods=["get"]) + def all(self, request, *args, **kwargs): + """ + Return all the favorites of the current user, with only limited data + to have a performant endpoint and avoid lots of queries just to display + favorites status in the UI + """ + if not request.user.is_authenticated: + return Response({"results": [], "count": 0}, status=200) + + favorites = list( + request.user.track_favorites.values("id", "track").order_by("id") + ) + payload = {"results": favorites, "count": len(favorites)} + return Response(payload, status=200) diff --git a/api/tests/favorites/test_favorites.py b/api/tests/favorites/test_favorites.py index 0b99c9340..7e8d1d3fd 100644 --- a/api/tests/favorites/test_favorites.py +++ b/api/tests/favorites/test_favorites.py @@ -39,6 +39,18 @@ def test_user_can_get_his_favorites(api_request, factories, logged_in_client, cl assert response.data["results"] == expected +def test_user_can_retrieve_all_favorites_at_once( + api_request, factories, logged_in_client, client +): + favorite = factories["favorites.TrackFavorite"](user=logged_in_client.user) + factories["favorites.TrackFavorite"]() + url = reverse("api:v1:favorites:tracks-all") + response = logged_in_client.get(url, {"user": logged_in_client.user.pk}) + expected = [{"track": favorite.track.id, "id": favorite.id}] + assert response.status_code == 200 + assert response.data["results"] == expected + + def test_user_can_add_favorite_via_api(factories, logged_in_client, activity_muted): track = factories["music.Track"]() url = reverse("api:v1:favorites:tracks-list") diff --git a/changes/changelog.d/favorites-performance.enhancement b/changes/changelog.d/favorites-performance.enhancement new file mode 100644 index 000000000..c35828fc2 --- /dev/null +++ b/changes/changelog.d/favorites-performance.enhancement @@ -0,0 +1 @@ +Performance improvement when fetching favorites, down to a single, small http request diff --git a/front/src/store/favorites.js b/front/src/store/favorites.js index 131db24bd..1d1302eb6 100644 --- a/front/src/store/favorites.js +++ b/front/src/store/favorites.js @@ -60,20 +60,12 @@ export default { page_size: 50, ordering: '-creation_date' } - let promise - if (url) { - promise = axios.get(url) - } else { - promise = axios.get('favorites/tracks/', {params: params}) - } + let promise = axios.get('favorites/tracks/all/', {params: params}) return promise.then((response) => { logger.default.info('Fetched a batch of ' + response.data.results.length + ' favorites') response.data.results.forEach(result => { - commit('track', {id: result.track.id, value: true}) + commit('track', {id: result.track, value: true}) }) - if (response.data.next) { - dispatch('fetch', response.data.next) - } }) } }