From e33497569a292ae2cd916a7f2425383f750ff8b9 Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Sun, 13 Oct 2024 10:16:57 +0200 Subject: [PATCH] Apply query optimizations for posts --- .../js/pages/homepage/actions/posts.js | 4 +-- .../js/tests/homepage/actions/post.test.js | 14 ++++----- .../js/tests/homepage/reducers/post.test.js | 2 +- .../tests/homepage/reducers/selected.test.js | 4 +-- src/newsreader/news/collection/endpoints.py | 7 +++-- .../tests/endpoints/rule/list/tests.py | 23 +------------- src/newsreader/news/core/endpoints.py | 23 +++++++------- src/newsreader/news/core/filters.py | 27 ----------------- .../tests/endpoints/category/list/tests.py | 30 ++----------------- .../core/tests/endpoints/post/list/tests.py | 10 +++---- 10 files changed, 37 insertions(+), 107 deletions(-) diff --git a/src/newsreader/js/pages/homepage/actions/posts.js b/src/newsreader/js/pages/homepage/actions/posts.js index 6a0cd7a..68ebe32 100644 --- a/src/newsreader/js/pages/homepage/actions/posts.js +++ b/src/newsreader/js/pages/homepage/actions/posts.js @@ -124,10 +124,10 @@ export const fetchPostsBySection = (section, next = false) => { switch (section.type) { case RULE_TYPE: - url = next ? next : `/api/rules/${section.id}/posts/?read=false`; + url = next ? next : `/api/rules/${section.id}/posts/`; break; case CATEGORY_TYPE: - url = next ? next : `/api/categories/${section.id}/posts/?read=false`; + url = next ? next : `/api/categories/${section.id}/posts/`; break; } diff --git a/src/newsreader/js/tests/homepage/actions/post.test.js b/src/newsreader/js/tests/homepage/actions/post.test.js index d30e549..6fd5d3d 100644 --- a/src/newsreader/js/tests/homepage/actions/post.test.js +++ b/src/newsreader/js/tests/homepage/actions/post.test.js @@ -304,10 +304,10 @@ describe('post actions', () => { type: constants.RULE_TYPE, }; - fetchMock.getOnce('/api/rules/4/posts/?read=false', { + fetchMock.getOnce('/api/rules/4/posts/', { body: { count: 2, - next: 'https://durp.com/api/rules/4/posts/?cursor=jabadabar&read=false', + next: 'https://durp.com/api/rules/4/posts/?cursor=jabadabar', previous: null, results: posts, }, @@ -325,7 +325,7 @@ describe('post actions', () => { { type: actions.REQUEST_POSTS }, { type: actions.RECEIVE_POSTS, - next: 'https://durp.com/api/rules/4/posts/?cursor=jabadabar&read=false', + next: 'https://durp.com/api/rules/4/posts/?cursor=jabadabar', posts, }, ]; @@ -373,10 +373,10 @@ describe('post actions', () => { type: constants.CATEGORY_TYPE, }; - fetchMock.getOnce('/api/categories/1/posts/?read=false', { + fetchMock.getOnce('/api/categories/1/posts/', { body: { count: 2, - next: 'https://durp.com/api/categories/4/posts/?cursor=jabadabar&read=false', + next: 'https://durp.com/api/categories/4/posts/?cursor=jabadabar', previous: null, results: posts, }, @@ -394,7 +394,7 @@ describe('post actions', () => { { type: actions.REQUEST_POSTS }, { type: actions.RECEIVE_POSTS, - next: 'https://durp.com/api/categories/4/posts/?cursor=jabadabar&read=false', + next: 'https://durp.com/api/categories/4/posts/?cursor=jabadabar', posts, }, ]; @@ -600,7 +600,7 @@ describe('post actions', () => { const errorMessage = 'Page not found'; - fetchMock.getOnce(`/api/rules/${rule.id}/posts/?read=false`, () => { + fetchMock.getOnce(`/api/rules/${rule.id}/posts/`, () => { throw new Error(errorMessage); }); diff --git a/src/newsreader/js/tests/homepage/reducers/post.test.js b/src/newsreader/js/tests/homepage/reducers/post.test.js index 249a9f1..59f0e36 100644 --- a/src/newsreader/js/tests/homepage/reducers/post.test.js +++ b/src/newsreader/js/tests/homepage/reducers/post.test.js @@ -81,7 +81,7 @@ describe('post actions', () => { const action = { type: actions.RECEIVE_POSTS, - next: 'https://durp.com/api/rules/4/posts/?page=2&read=false', + next: 'https://durp.com/api/rules/4/posts/?page=2', posts, }; diff --git a/src/newsreader/js/tests/homepage/reducers/selected.test.js b/src/newsreader/js/tests/homepage/reducers/selected.test.js index 40561a3..0d1a7ae 100644 --- a/src/newsreader/js/tests/homepage/reducers/selected.test.js +++ b/src/newsreader/js/tests/homepage/reducers/selected.test.js @@ -254,13 +254,13 @@ describe('selected reducer', () => { const action = { type: postActions.RECEIVE_POSTS, - next: 'https://durp.com/api/rules/4/posts/?page=2&read=false', + next: 'https://durp.com/api/rules/4/posts/?page=2', posts, }; const expectedState = { ...defaultState, - next: 'https://durp.com/api/rules/4/posts/?page=2&read=false', + next: 'https://durp.com/api/rules/4/posts/?page=2', lastReached: false, }; diff --git a/src/newsreader/news/collection/endpoints.py b/src/newsreader/news/collection/endpoints.py index 1b11d31..24918c9 100644 --- a/src/newsreader/news/collection/endpoints.py +++ b/src/newsreader/news/collection/endpoints.py @@ -1,3 +1,4 @@ +from django.db.models import Prefetch from rest_framework import status from rest_framework.generics import ( GenericAPIView, @@ -10,7 +11,6 @@ from rest_framework.response import Response from newsreader.core.pagination import CursorPagination from newsreader.news.collection.models import CollectionRule from newsreader.news.collection.serializers import RuleSerializer -from newsreader.news.core.filters import ReadFilter from newsreader.news.core.models import Post from newsreader.news.core.serializers import PostSerializer @@ -24,7 +24,6 @@ class NestedRuleView(ListAPIView): queryset = CollectionRule.objects.prefetch_related("posts").all() serializer_class = PostSerializer pagination_class = CursorPagination - filter_backends = [ReadFilter] def get_queryset(self): lookup_url_kwarg = self.lookup_url_kwarg or self.lookup_field @@ -33,7 +32,9 @@ class NestedRuleView(ListAPIView): # filtered on the user. filter_kwargs = {self.lookup_field: self.kwargs[lookup_url_kwarg]} - rule = get_object_or_404(self.queryset, **filter_kwargs) + prefetch = Prefetch("posts", queryset=Post.objects.filter(read=False)) + queryset = CollectionRule.objects.prefetch_related(prefetch) + rule = get_object_or_404(queryset, **filter_kwargs) self.check_object_permissions(self.request, rule) return rule.posts.order_by("-publication_date") diff --git a/src/newsreader/news/collection/tests/endpoints/rule/list/tests.py b/src/newsreader/news/collection/tests/endpoints/rule/list/tests.py index 51334b5..24d4b9a 100644 --- a/src/newsreader/news/collection/tests/endpoints/rule/list/tests.py +++ b/src/newsreader/news/collection/tests/endpoints/rule/list/tests.py @@ -202,7 +202,7 @@ class NestedRuleListViewTestCase(TestCase): with self.subTest(post=post): self.assertEqual(post["rule"]["id"], rule.pk) - def test_unread_posts(self): + def test_posts(self): rule = FeedFactory.create(user=self.user) FeedPostFactory.create_batch(size=10, rule=rule, read=False) @@ -210,7 +210,6 @@ class NestedRuleListViewTestCase(TestCase): response = self.client.get( reverse("api:news:collection:rules-nested-posts", kwargs={"pk": rule.pk}), - {"read": "false"}, ) data = response.json() @@ -221,23 +220,3 @@ class NestedRuleListViewTestCase(TestCase): for post in data["results"]: with self.subTest(post=post): self.assertEqual(post["read"], False) - - def test_read_posts(self): - rule = FeedFactory.create(user=self.user) - - FeedPostFactory.create_batch(size=20, rule=rule, read=False) - FeedPostFactory.create_batch(size=10, rule=rule, read=True) - - response = self.client.get( - reverse("api:news:collection:rules-nested-posts", kwargs={"pk": rule.pk}), - {"read": "true"}, - ) - - data = response.json() - - self.assertEqual(response.status_code, 200) - self.assertEqual(len(data["results"]), 10) - - for post in data["results"]: - with self.subTest(post=post): - self.assertEqual(post["read"], True) diff --git a/src/newsreader/news/core/endpoints.py b/src/newsreader/news/core/endpoints.py index 184515b..6ed9f39 100644 --- a/src/newsreader/news/core/endpoints.py +++ b/src/newsreader/news/core/endpoints.py @@ -1,3 +1,4 @@ +from django.db.models import Prefetch from rest_framework import status from rest_framework.generics import ( GenericAPIView, @@ -11,18 +12,19 @@ from rest_framework.response import Response from newsreader.accounts.permissions import IsPostOwner from newsreader.core.pagination import CursorPagination +from newsreader.news.collection.models import CollectionRule from newsreader.news.collection.serializers import RuleSerializer -from newsreader.news.core.filters import ReadFilter, SavedFilter +from newsreader.news.core.filters import SavedFilter from newsreader.news.core.models import Category, Post from newsreader.news.core.serializers import CategorySerializer, PostSerializer class ListPostView(ListAPIView): - queryset = Post.objects.all() + queryset = Post.objects.filter(read=False) serializer_class = PostSerializer permission_classes = (IsAuthenticated, IsPostOwner) pagination_class = CursorPagination - filter_backends = [ReadFilter, SavedFilter] + filter_backends = [SavedFilter] class DetailPostView(RetrieveUpdateAPIView): @@ -63,10 +65,8 @@ class NestedRuleCategoryView(ListAPIView): class NestedPostCategoryView(ListAPIView): - queryset = Category.objects.prefetch_related("rules", "rules__posts").all() serializer_class = PostSerializer pagination_class = CursorPagination - filter_backends = [ReadFilter] def get_queryset(self): lookup_url_kwarg = self.lookup_url_kwarg or self.lookup_field @@ -75,13 +75,16 @@ class NestedPostCategoryView(ListAPIView): # filtered on the user. filter_kwargs = {self.lookup_field: self.kwargs[lookup_url_kwarg]} - category = get_object_or_404(self.queryset, **filter_kwargs) + rules_queryset = CollectionRule.objects.filter(user=self.request.user) + prefetch = Prefetch("rules", queryset=rules_queryset, to_attr="user_rules") + category_queryset = Category.objects.prefetch_related(prefetch).filter( + user=self.request.user + ) + category = get_object_or_404(category_queryset, **filter_kwargs) + self.check_object_permissions(self.request, category) - rules = category.rules.values_list("id", flat=True) - queryset = Post.objects.filter(rule__in=rules) - - return queryset + return Post.objects.filter(rule__in=category.user_rules, read=False) class CategoryReadView(GenericAPIView): diff --git a/src/newsreader/news/core/filters.py b/src/newsreader/news/core/filters.py index 05f9157..9883304 100644 --- a/src/newsreader/news/core/filters.py +++ b/src/newsreader/news/core/filters.py @@ -4,33 +4,6 @@ from rest_framework import filters from rest_framework.compat import coreapi, coreschema -class ReadFilter(filters.BaseFilterBackend): - query_param = "read" - - def filter_queryset(self, request, queryset, view): - key = request.query_params.get(self.query_param, None) - available_values = {"True": True, "true": True, "False": False, "false": False} - - if not key or key not in available_values.keys(): - return queryset - - value = available_values[key] - return queryset.filter(read=value) - - def get_schema_fields(self, view): - return [ - coreapi.Field( - name=self.query_param, - required=False, - location="query", - schema=coreschema.String( - title=str(self.query_param), - description=str(_("Wether posts should be read or not")), - ), - ) - ] - - class SavedFilter(filters.BaseFilterBackend): query_param = "saved" diff --git a/src/newsreader/news/core/tests/endpoints/category/list/tests.py b/src/newsreader/news/core/tests/endpoints/category/list/tests.py index 34060ea..610dc58 100644 --- a/src/newsreader/news/core/tests/endpoints/category/list/tests.py +++ b/src/newsreader/news/core/tests/endpoints/category/list/tests.py @@ -409,7 +409,7 @@ class NestedCategoryPostView(TestCase): reverse("api:news:core:categories-nested-posts", kwargs={"pk": category.pk}) ) - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 404) def test_ordering(self): category = CategoryFactory.create(user=self.user) @@ -503,9 +503,9 @@ class NestedCategoryPostView(TestCase): self.assertEqual(posts[0]["rule"]["id"], guardian_rule.pk) self.assertEqual(posts[1]["rule"]["id"], guardian_rule.pk) - def test_unread_posts(self): + def test_posts(self): category = CategoryFactory.create(user=self.user) - rule = FeedFactory(category=category) + rule = FeedFactory(category=category, user=self.user) FeedPostFactory.create_batch(size=10, rule=rule, read=False) FeedPostFactory.create_batch(size=10, rule=rule, read=True) @@ -514,7 +514,6 @@ class NestedCategoryPostView(TestCase): reverse( "api:news:core:categories-nested-posts", kwargs={"pk": category.pk} ), - {"read": "false"}, ) data = response.json() @@ -525,26 +524,3 @@ class NestedCategoryPostView(TestCase): for post in posts: self.assertEqual(post["read"], False) - - def test_read_posts(self): - category = CategoryFactory.create(user=self.user) - rule = FeedFactory(category=category) - - FeedPostFactory.create_batch(size=20, rule=rule, read=False) - FeedPostFactory.create_batch(size=10, rule=rule, read=True) - - response = self.client.get( - reverse( - "api:news:core:categories-nested-posts", kwargs={"pk": category.pk} - ), - {"read": "true"}, - ) - - data = response.json() - posts = data["results"] - - self.assertEqual(response.status_code, 200) - self.assertEqual(len(data["results"]), 10) - - for post in posts: - self.assertEqual(post["read"], True) diff --git a/src/newsreader/news/core/tests/endpoints/post/list/tests.py b/src/newsreader/news/core/tests/endpoints/post/list/tests.py index f88e575..467fd05 100644 --- a/src/newsreader/news/core/tests/endpoints/post/list/tests.py +++ b/src/newsreader/news/core/tests/endpoints/post/list/tests.py @@ -53,25 +53,23 @@ class PostListViewTestCase(TestCase): with self.subTest(post=post): self.assertEqual(data["results"][index]["id"], post.pk) - def test_read_posts(self): + def test_posts(self): rule = FeedFactory(user=self.user, category=CategoryFactory(user=self.user)) FeedPostFactory.create_batch(size=20, rule=rule, read=False) FeedPostFactory.create_batch(size=10, rule=rule, read=True) - response = self.client.get( - reverse("api:news:core:posts-list"), {"read": "true"} - ) + response = self.client.get(reverse("api:news:core:posts-list")) data = response.json() posts = data["results"] self.assertEqual(response.status_code, 200) - self.assertEqual(len(data["results"]), 10) + self.assertEqual(len(data["results"]), 20) for post in posts: with self.subTest(post=post): - self.assertEqual(post["read"], True) + self.assertEqual(post["read"], False) def test_saved_posts(self): rule = FeedFactory(user=self.user, category=CategoryFactory(user=self.user))