From d0f4f5380a73f9ab218fcec24b53ea46172ec91f Mon Sep 17 00:00:00 2001 From: Calum Mackervoy Date: Tue, 27 Apr 2021 15:46:26 +0000 Subject: [PATCH] update: updated to new DjangoLDP cache removed workaround, updated tests --- djangoldp_notification/models.py | 21 ++++++++++++- djangoldp_notification/tests/test_cache.py | 35 +++++++++++++++++++-- djangoldp_notification/tests/test_models.py | 3 -- djangoldp_notification/views.py | 7 ----- 4 files changed, 52 insertions(+), 14 deletions(-) diff --git a/djangoldp_notification/models.py b/djangoldp_notification/models.py index cb27de1..580fe29 100644 --- a/djangoldp_notification/models.py +++ b/djangoldp_notification/models.py @@ -7,10 +7,11 @@ from django.db import models from django.db.models.signals import post_save, post_delete from django.dispatch import receiver from django.template import loader -from django.urls import NoReverseMatch, get_resolver +from django.urls import NoReverseMatch, get_resolver, reverse from django.utils.translation import ugettext_lazy as _ from djangoldp.fields import LDPUrlField from djangoldp.models import Model +from djangoldp.views import LDPViewSet from djangoldp.activities.services import ActivityQueueService, ActivityPubService, activity_sending_finished from djangoldp_notification.middlewares import MODEL_MODIFICATION_USER_FIELD from djangoldp_notification.permissions import InboxPermissions, SubscriptionsPermissions @@ -39,6 +40,24 @@ class Notification(Model): owner_perms = ['view', 'change', 'control'] view_set = LDPNotificationsViewSet + # NOTE: this would be our ideal cache behaviour + # the functionality for optimising it was removed because of an issue with extensibility + # https://git.startinblox.com/djangoldp-packages/djangoldp-notification/merge_requests/42#note_58559 + '''def clear_djangoldp_cache(self, cache, cache_entry): + # should only clear the users/x/inbox + + lookup_arg = LDPViewSet.get_lookup_arg(model=get_user_model()) + + url = reverse('{}-{}-list'.format(self.user.__class__.__name__.lower(), self.__class__.__name__.lower()), + args=[getattr(self.user, lookup_arg)]) + url = '{}{}'.format(settings.SITE_URL, url) + + cache.invalidate(cache_entry, url) + + # invalidate the global /notifications/ container also + url = '{}{}'.format(settings.SITE_URL, reverse('{}-list'.format(self.__class__.__name__.lower()))) + cache.invalidate(cache_entry, url)''' + def __str__(self): return '{}'.format(self.type) diff --git a/djangoldp_notification/tests/test_cache.py b/djangoldp_notification/tests/test_cache.py index dafc1bc..ffe1f9d 100644 --- a/djangoldp_notification/tests/test_cache.py +++ b/djangoldp_notification/tests/test_cache.py @@ -2,7 +2,9 @@ import uuid import json from rest_framework.test import APITestCase, APIClient -from djangoldp.serializers import LDListMixin, LDPSerializer +from django.conf import settings +from djangoldp.models import Model +from djangoldp.serializers import GLOBAL_SERIALIZER_CACHE from djangoldp_account.models import LDPUser from djangoldp_notification.models import Notification @@ -22,8 +24,6 @@ class TestSubscription(APITestCase): def setUp(self): self.client = APIClient() - LDListMixin.to_representation_cache.reset() - LDPSerializer.to_representation_cache.reset() def test_indirect_cache(self): self.setUpLoggedInUser() @@ -55,3 +55,32 @@ class TestSubscription(APITestCase): self.assertEqual(response.status_code, 200) notif_serialized = response.data["ldp:contains"][0] self.assertEqual(notif_serialized["unread"], False) + + # NOTE: this would be our ideal cache behaviour + # the functionality for optimising it was removed because of an issue with extensibility + # https://git.startinblox.com/djangoldp-packages/djangoldp-notification/merge_requests/42#note_58559 + '''def test_custom_cache_clear(self): + # going to create two notifications in two different inboxes + self.setUpLoggedInUser() + other_user = self._get_random_user() + notification = self._get_random_notification(recipient=self.user, author=other_user) + notification2 = self._get_random_notification(recipient=other_user, author=self.user) + + # GET the inboxes and asser that the cache is set for both + self.client.get("/users/{}/inbox/".format(self.user.username)) + self.client.get("/users/{}/inbox/".format(other_user.username)) + + # assert cache is set + my_container_urlid = '{}/users/{}/inbox/'.format(settings.SITE_URL, self.user.username) + their_container_urlid = '{}/users/{}/inbox/'.format(settings.SITE_URL, other_user.username) + + self.assertTrue(GLOBAL_SERIALIZER_CACHE.has(Model.get_meta(Notification, 'label'), my_container_urlid)) + self.assertTrue(GLOBAL_SERIALIZER_CACHE.has(Model.get_meta(Notification, 'label'), their_container_urlid)) + + # save my notification - should wipe the cache for my inbox... + notification.unread = False + notification.save() + self.assertFalse(GLOBAL_SERIALIZER_CACHE.has(Model.get_meta(Notification, 'label'), my_container_urlid)) + + # ...but not for theirs + self.assertTrue(GLOBAL_SERIALIZER_CACHE.has(Model.get_meta(Notification, 'label'), their_container_urlid))''' diff --git a/djangoldp_notification/tests/test_models.py b/djangoldp_notification/tests/test_models.py index 6bfaf7b..c8c6487 100644 --- a/djangoldp_notification/tests/test_models.py +++ b/djangoldp_notification/tests/test_models.py @@ -1,7 +1,6 @@ import uuid from rest_framework.test import APITestCase, APIClient -from djangoldp.serializers import LDListMixin, LDPSerializer from djangoldp_account.models import LDPUser from djangoldp_notification.models import Subscription @@ -23,8 +22,6 @@ class TestSubscription(APITestCase): def setUp(self): self.client = APIClient() - LDListMixin.to_representation_cache.reset() - LDPSerializer.to_representation_cache.reset() self.user1 = self._get_random_user() Subscription.objects.create(object=self.circle_user1_url, inbox="http://testserver/users/karl_marx/inbox/") diff --git a/djangoldp_notification/views.py b/djangoldp_notification/views.py index 5abd188..5df49fc 100644 --- a/djangoldp_notification/views.py +++ b/djangoldp_notification/views.py @@ -11,10 +11,3 @@ class LDPNotificationsViewSet(LDPViewSet): '''overridden LDPViewSet to force pagination''' pagination_class = LDPNotificationsPagination depth = 0 - - def update(self, request, *args, **kwargs): - instance = self.get_object() - - LDPSerializer.to_representation_cache.invalidate(instance.user.urlid) - - return super().update(request, *args, **kwargs)