From 8aa717f464bdd595da5ae434941978caea281aaf Mon Sep 17 00:00:00 2001 From: Vivek Agrawal Date: Thu, 17 Mar 2022 02:04:15 +0530 Subject: [PATCH 1/5] feat: cache public channel filter fields --- contentcuration/contentcuration/models.py | 4 + contentcuration/contentcuration/views/base.py | 102 ++++++++++-------- 2 files changed, 63 insertions(+), 43 deletions(-) diff --git a/contentcuration/contentcuration/models.py b/contentcuration/contentcuration/models.py index 5aaf1ea364..48dedbda28 100644 --- a/contentcuration/contentcuration/models.py +++ b/contentcuration/contentcuration/models.py @@ -838,6 +838,8 @@ def on_create(self): # if this change affects the public channel list, clear the channel cache if self.public: delete_public_channel_cache_keys() + cache.delete_many(["public_channel_list", "public_channel_languages", "public_channel_licenses", + "public_channel_kinds", "public_channel_collections"]) def on_update(self): from contentcuration.utils.user import calculate_user_storage @@ -879,6 +881,8 @@ def on_update(self): # if this change affects the public channel list, clear the channel cache if "public" in original_values: delete_public_channel_cache_keys() + cache.delete_many(["public_channel_list", "public_channel_languages", "public_channel_licenses", + "public_channel_kinds", "public_channel_collections"]) def save(self, *args, **kwargs): if self._state.adding: diff --git a/contentcuration/contentcuration/views/base.py b/contentcuration/contentcuration/views/base.py index 78f774c1c7..74ac05a671 100644 --- a/contentcuration/contentcuration/views/base.py +++ b/contentcuration/contentcuration/views/base.py @@ -5,6 +5,7 @@ from django.conf import settings from django.contrib.auth.decorators import login_required +from django.core.cache import cache from django.core.exceptions import PermissionDenied from django.db.models import Count from django.db.models import IntegerField @@ -125,53 +126,70 @@ def channel_list(request): current_user = current_user_for_context(None if anon else request.user) preferences = DEFAULT_USER_PREFERENCES if anon else request.user.content_defaults - public_channel_list = Channel.objects.filter( - public=True, main_tree__published=True, deleted=False, - ).values_list("main_tree__tree_id", flat=True) + public_channel_list = cache.get("public_channel_list") + if public_channel_list is None: + public_channel_list = Channel.objects.filter( + public=True, main_tree__published=True, deleted=False, + ).values_list("main_tree__tree_id", flat=True) + cache.set("public_channel_list", public_channel_list, None) # Get public channel languages - public_lang_query = ( - Language.objects.filter( - channel_language__public=True, - channel_language__main_tree__published=True, - channel_language__deleted=False, + languages = cache.get("public_channel_languages") + if languages is None: + public_lang_query = ( + Language.objects.filter( + channel_language__public=True, + channel_language__main_tree__published=True, + channel_language__deleted=False, + ) + .values("lang_code") + .annotate(count=Count("lang_code")) + .order_by("lang_code") ) - .values("lang_code") - .annotate(count=Count("lang_code")) - .order_by("lang_code") - ) - languages = {lang["lang_code"]: lang["count"] for lang in public_lang_query} + languages = {lang["lang_code"]: lang["count"] for lang in public_lang_query} + cache.set("public_channel_languages", json_for_parse_from_data(languages), None) # Get public channel licenses - public_license_query = ( - License.objects.filter(contentnode__tree_id__in=public_channel_list) - .values_list("id", flat=True) - .order_by("id") - .distinct() - ) - licenses = list(public_license_query) + licenses = cache.get("public_channel_licenses") + if licenses is None: + public_license_query = ( + License.objects.filter(contentnode__tree_id__in=public_channel_list) + .values_list("id", flat=True) + .order_by("id") + .distinct() + ) + licenses = list(public_license_query) + cache.set("public_channel_licenses", json_for_parse_from_data(licenses), None) # Get public channel kinds - public_kind_query = ( - ContentKind.objects.filter(contentnodes__tree_id__in=public_channel_list) - .values_list("kind", flat=True) - .order_by("kind") - .distinct() - ) - kinds = list(public_kind_query) + kinds = cache.get("public_channel_kinds") + if kinds is None: + public_kind_query = ( + ContentKind.objects.filter(contentnodes__tree_id__in=public_channel_list) + .values_list("kind", flat=True) + .order_by("kind") + .distinct() + ) + kinds = list(public_kind_query) + cache.set("public_channel_kinds", json_for_parse_from_data(kinds), None) # Get public channel sets - public_channelset_query = ChannelSet.objects.filter(public=True).annotate( - count=SQCountDistinct( - Channel.objects.filter( - secret_tokens=OuterRef("secret_token"), - public=True, - main_tree__published=True, - deleted=False, - ).values_list("id", flat=True), - field="id", + collections = cache.get("public_channel_collections") + if collections is None: + public_channelset_query = ChannelSet.objects.filter(public=True).annotate( + count=SQCountDistinct( + Channel.objects.filter( + secret_tokens=OuterRef("secret_token"), + public=True, + main_tree__published=True, + deleted=False, + ).values_list("id", flat=True), + field="id", + ) ) - ) + cache.set("public_channel_collections", json_for_parse_from_serializer( + PublicChannelSetSerializer(public_channelset_query, many=True)), None) + return render( request, "channel_list.html", @@ -180,12 +198,10 @@ def channel_list(request): PREFERENCES: json_for_parse_from_data(preferences), MESSAGES: json_for_parse_from_data(get_messages()), "LIBRARY_MODE": settings.LIBRARY_MODE, - "public_languages": json_for_parse_from_data(languages), - "public_kinds": json_for_parse_from_data(kinds), - "public_licenses": json_for_parse_from_data(licenses), - "public_collections": json_for_parse_from_serializer( - PublicChannelSetSerializer(public_channelset_query, many=True) - ), + "public_languages": languages, + "public_kinds": kinds, + "public_licenses": licenses, + "public_collections": collections, }, ) From d60075f9210a199fe543c03a4d91f264d7287550 Mon Sep 17 00:00:00 2001 From: Vivek Agrawal Date: Tue, 5 Apr 2022 21:48:13 +0530 Subject: [PATCH 2/5] fix: use constant for cache keys and clear only when published channels are hit --- contentcuration/contentcuration/models.py | 12 +++------ .../contentcuration/utils/cache.py | 3 +++ contentcuration/contentcuration/views/base.py | 27 ++++++++++++------- 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/contentcuration/contentcuration/models.py b/contentcuration/contentcuration/models.py index 48dedbda28..1e08a86d9c 100644 --- a/contentcuration/contentcuration/models.py +++ b/contentcuration/contentcuration/models.py @@ -835,11 +835,9 @@ def on_create(self): node_id=self.id, ) - # if this change affects the public channel list, clear the channel cache - if self.public: + # if this change affects the published channel list, clear the channel cache + if self.public and (self.main_tree and self.main_tree.published): delete_public_channel_cache_keys() - cache.delete_many(["public_channel_list", "public_channel_languages", "public_channel_licenses", - "public_channel_kinds", "public_channel_collections"]) def on_update(self): from contentcuration.utils.user import calculate_user_storage @@ -878,11 +876,9 @@ def on_update(self): if self.main_tree and self.main_tree._field_updates.changed(): self.main_tree.save() - # if this change affects the public channel list, clear the channel cache - if "public" in original_values: + # if this change affects the published channel list, clear the channel cache + if "public" in original_values and (self.main_tree and self.main_tree.published): delete_public_channel_cache_keys() - cache.delete_many(["public_channel_list", "public_channel_languages", "public_channel_licenses", - "public_channel_kinds", "public_channel_collections"]) def save(self, *args, **kwargs): if self._state.adding: diff --git a/contentcuration/contentcuration/utils/cache.py b/contentcuration/contentcuration/utils/cache.py index 412ad3d731..2399843232 100644 --- a/contentcuration/contentcuration/utils/cache.py +++ b/contentcuration/contentcuration/utils/cache.py @@ -9,6 +9,7 @@ from django_redis.client import DefaultClient from django_redis.client.default import _main_exceptions +from contentcuration.views.base import PUBLIC_CHANNELS_CACHE_KEYS DEFERRED_FLAG = "__DEFERRED" @@ -105,6 +106,7 @@ def delete_public_channel_cache_keys(): """ delete_cache_keys("*get_public_channel_list*") delete_cache_keys("*get_user_public_channels*") + django_cache.delete_many(list(PUBLIC_CHANNELS_CACHE_KEYS.values())) def redis_retry(func): @@ -134,6 +136,7 @@ class ResourceSizeCache: If the django_cache is Redis, then we use the lower level Redis client to use its hash commands, HSET and HGET, to ensure we can store lots of data in performant way """ + def __init__(self, node, cache=None): self.node = node self.cache = cache or django_cache diff --git a/contentcuration/contentcuration/views/base.py b/contentcuration/contentcuration/views/base.py index 74ac05a671..e8f9e4196d 100644 --- a/contentcuration/contentcuration/views/base.py +++ b/contentcuration/contentcuration/views/base.py @@ -50,6 +50,13 @@ from contentcuration.viewsets.channelset import PublicChannelSetSerializer PUBLIC_CHANNELS_CACHE_DURATION = 30 # seconds +PUBLIC_CHANNELS_CACHE_KEYS = { + "list": "public_channel_list", + "languages": "public_channel_languages", + "licenses": "public_channel_licenses", + "kinds": "public_channel_kinds", + "collections": "public_channel_collections", +} MESSAGES = "i18n_messages" PREFERENCES = "user_preferences" @@ -126,15 +133,15 @@ def channel_list(request): current_user = current_user_for_context(None if anon else request.user) preferences = DEFAULT_USER_PREFERENCES if anon else request.user.content_defaults - public_channel_list = cache.get("public_channel_list") + public_channel_list = cache.get(PUBLIC_CHANNELS_CACHE_KEYS["list"]) if public_channel_list is None: public_channel_list = Channel.objects.filter( public=True, main_tree__published=True, deleted=False, ).values_list("main_tree__tree_id", flat=True) - cache.set("public_channel_list", public_channel_list, None) + cache.set(PUBLIC_CHANNELS_CACHE_KEYS["list"], public_channel_list, None) # Get public channel languages - languages = cache.get("public_channel_languages") + languages = cache.get(PUBLIC_CHANNELS_CACHE_KEYS["languages"]) if languages is None: public_lang_query = ( Language.objects.filter( @@ -147,10 +154,10 @@ def channel_list(request): .order_by("lang_code") ) languages = {lang["lang_code"]: lang["count"] for lang in public_lang_query} - cache.set("public_channel_languages", json_for_parse_from_data(languages), None) + cache.set(PUBLIC_CHANNELS_CACHE_KEYS["languages"], json_for_parse_from_data(languages), None) # Get public channel licenses - licenses = cache.get("public_channel_licenses") + licenses = cache.get(PUBLIC_CHANNELS_CACHE_KEYS["licenses"]) if licenses is None: public_license_query = ( License.objects.filter(contentnode__tree_id__in=public_channel_list) @@ -159,10 +166,10 @@ def channel_list(request): .distinct() ) licenses = list(public_license_query) - cache.set("public_channel_licenses", json_for_parse_from_data(licenses), None) + cache.set(PUBLIC_CHANNELS_CACHE_KEYS["licenses"], json_for_parse_from_data(licenses), None) # Get public channel kinds - kinds = cache.get("public_channel_kinds") + kinds = cache.get(PUBLIC_CHANNELS_CACHE_KEYS["kinds"]) if kinds is None: public_kind_query = ( ContentKind.objects.filter(contentnodes__tree_id__in=public_channel_list) @@ -171,10 +178,10 @@ def channel_list(request): .distinct() ) kinds = list(public_kind_query) - cache.set("public_channel_kinds", json_for_parse_from_data(kinds), None) + cache.set(PUBLIC_CHANNELS_CACHE_KEYS["kinds"], json_for_parse_from_data(kinds), None) # Get public channel sets - collections = cache.get("public_channel_collections") + collections = cache.get(PUBLIC_CHANNELS_CACHE_KEYS["collections"]) if collections is None: public_channelset_query = ChannelSet.objects.filter(public=True).annotate( count=SQCountDistinct( @@ -187,7 +194,7 @@ def channel_list(request): field="id", ) ) - cache.set("public_channel_collections", json_for_parse_from_serializer( + cache.set(PUBLIC_CHANNELS_CACHE_KEYS["collections"], json_for_parse_from_serializer( PublicChannelSetSerializer(public_channelset_query, many=True)), None) return render( From 5111e0c7b6df129e85f60ef58a7f3650fb36f666 Mon Sep 17 00:00:00 2001 From: Vivek Agrawal Date: Tue, 5 Apr 2022 22:50:54 +0530 Subject: [PATCH 3/5] fix: cache is now getting cleared on content node updates! --- contentcuration/contentcuration/models.py | 4 ++++ contentcuration/contentcuration/utils/cache.py | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/contentcuration/contentcuration/models.py b/contentcuration/contentcuration/models.py index 1e08a86d9c..69bbb39acd 100644 --- a/contentcuration/contentcuration/models.py +++ b/contentcuration/contentcuration/models.py @@ -1658,9 +1658,13 @@ def recalculate_editors_storage(self): def on_create(self): self.changed = True self.recalculate_editors_storage() + if self.published: + delete_public_channel_cache_keys() def on_update(self): self.changed = self.changed or self.has_changes() + if self.published: + delete_public_channel_cache_keys() def move_to(self, target, *args, **kwargs): parent_was_trashtree = self.parent.channel_trash.exists() diff --git a/contentcuration/contentcuration/utils/cache.py b/contentcuration/contentcuration/utils/cache.py index 2399843232..436d02c17e 100644 --- a/contentcuration/contentcuration/utils/cache.py +++ b/contentcuration/contentcuration/utils/cache.py @@ -9,7 +9,6 @@ from django_redis.client import DefaultClient from django_redis.client.default import _main_exceptions -from contentcuration.views.base import PUBLIC_CHANNELS_CACHE_KEYS DEFERRED_FLAG = "__DEFERRED" @@ -104,6 +103,8 @@ def delete_public_channel_cache_keys(): """ Delete all caches related to the public channel caching. """ + from contentcuration.views.base import PUBLIC_CHANNELS_CACHE_KEYS + delete_cache_keys("*get_public_channel_list*") delete_cache_keys("*get_user_public_channels*") django_cache.delete_many(list(PUBLIC_CHANNELS_CACHE_KEYS.values())) From a14d09b7a065ce17ab284afcba05c7ae4e9784c5 Mon Sep 17 00:00:00 2001 From: Vivek Agrawal Date: Mon, 11 Apr 2022 23:19:59 +0530 Subject: [PATCH 4/5] fix: clear cache on publish --- contentcuration/contentcuration/models.py | 4 ---- contentcuration/contentcuration/utils/publish.py | 4 ++++ 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/contentcuration/contentcuration/models.py b/contentcuration/contentcuration/models.py index 69bbb39acd..1e08a86d9c 100644 --- a/contentcuration/contentcuration/models.py +++ b/contentcuration/contentcuration/models.py @@ -1658,13 +1658,9 @@ def recalculate_editors_storage(self): def on_create(self): self.changed = True self.recalculate_editors_storage() - if self.published: - delete_public_channel_cache_keys() def on_update(self): self.changed = self.changed or self.has_changes() - if self.published: - delete_public_channel_cache_keys() def move_to(self, target, *args, **kwargs): parent_was_trashtree = self.parent.channel_trash.exists() diff --git a/contentcuration/contentcuration/utils/publish.py b/contentcuration/contentcuration/utils/publish.py index 4c8a54ad73..f211813882 100644 --- a/contentcuration/contentcuration/utils/publish.py +++ b/contentcuration/contentcuration/utils/publish.py @@ -40,6 +40,7 @@ from contentcuration import models as ccmodels from contentcuration.statistics import record_publish_stats +from contentcuration.utils.cache import delete_public_channel_cache_keys from contentcuration.utils.files import create_thumbnail_from_base64 from contentcuration.utils.files import get_thumbnail_encoding from contentcuration.utils.parser import extract_value @@ -782,6 +783,9 @@ def publish_channel( channel.main_tree.published = True channel.main_tree.save() + # Delete public channel cache. + delete_public_channel_cache_keys() + if send_email: send_emails(channel, user_id, version_notes=version_notes) From fd8a2055c009fc35f775a20b2b4e2e8e5cee0862 Mon Sep 17 00:00:00 2001 From: Vivek Agrawal Date: Tue, 12 Apr 2022 20:46:57 +0530 Subject: [PATCH 5/5] fix: clear cache only when channel is public --- contentcuration/contentcuration/utils/publish.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/contentcuration/contentcuration/utils/publish.py b/contentcuration/contentcuration/utils/publish.py index 0bdbbef9ac..63fc79c1b0 100644 --- a/contentcuration/contentcuration/utils/publish.py +++ b/contentcuration/contentcuration/utils/publish.py @@ -810,7 +810,8 @@ def publish_channel( channel.main_tree.save() # Delete public channel cache. - delete_public_channel_cache_keys() + if channel.public: + delete_public_channel_cache_keys() if send_email: send_emails(channel, user_id, version_notes=version_notes)