From b10897e64ba976b206734c7edadfa371534877f7 Mon Sep 17 00:00:00 2001 From: Herculino Trotta Date: Mon, 2 Nov 2026 17:24:28 -0300 Subject: [PATCH] fix(updates): keep update banner across page loads and restarts The update check shared the file-based default cache with cachalot, whose culling randomly deleted the key as pages loaded. Store only the latest release in a dedicated cache, compare it to APP_VERSION at render time, and drop the startup delete. Hide the banner when CHECK_FOR_UPDATES is off or APP_VERSION isn't a valid version. --- app/WYGIWYH/settings.py | 7 +- app/apps/common/apps.py | 5 -- app/apps/common/tasks.py | 27 ++------ app/apps/common/templatetags/cache_access.py | 25 +++++--- app/apps/common/tests/test_update_check.py | 67 ++++++++++++++++++++ 5 files changed, 97 insertions(+), 34 deletions(-) create mode 100644 app/apps/common/tests/test_update_check.py diff --git a/app/WYGIWYH/settings.py b/app/WYGIWYH/settings.py index e479980d..f7756b73 100644 --- a/app/WYGIWYH/settings.py +++ b/app/WYGIWYH/settings.py @@ -328,7 +328,12 @@ CACHES = { "default": { "BACKEND": "django.core.cache.backends.filebased.FileBasedCache", "LOCATION": "/var/tmp/django_cache", - } + }, + # Kept apart from "default": cachalot fills it and FileBasedCache culls random entries past MAX_ENTRIES + "update_check": { + "BACKEND": "django.core.cache.backends.filebased.FileBasedCache", + "LOCATION": "/var/tmp/django_cache_update_check", + }, } DJANGO_VITE_ASSETS_PATH = STATIC_ROOT diff --git a/app/apps/common/apps.py b/app/apps/common/apps.py index e934f443..02d422e7 100644 --- a/app/apps/common/apps.py +++ b/app/apps/common/apps.py @@ -1,5 +1,4 @@ from django.apps import AppConfig -from django.core.cache import cache class CommonConfig(AppConfig): @@ -20,9 +19,5 @@ class CommonConfig(AppConfig): admin.site.unregister(SocialApp) admin.site.unregister(SocialToken) - # Delete the cache for update checks to prevent false-positives when the app is restarted - # this will be recreated by the check_for_updates task - cache.delete("update_check") - # Register system checks for required environment variables from apps.common import checks # noqa: F401 diff --git a/app/apps/common/tasks.py b/app/apps/common/tasks.py index 370e2594..042398bb 100644 --- a/app/apps/common/tasks.py +++ b/app/apps/common/tasks.py @@ -5,7 +5,7 @@ from asgiref.sync import sync_to_async from django.conf import settings from django.core import management from django.db import DEFAULT_DB_ALIAS -from django.core.cache import cache +from django.core.cache import caches from procrastinate import builtin_tasks from procrastinate.contrib.django import app @@ -107,26 +107,13 @@ def check_for_updates(timestamp=None): if latest_version: try: - current_v = parse_version(settings.APP_VERSION) + latest_version = str(parse_version(latest_version)) except InvalidVersion: - current_v = parse_version("0.0.0") - try: - latest_v = parse_version(latest_version) - except InvalidVersion: - latest_v = parse_version("0.0.0") - - update_info = { - "update_available": False, - "current_version": str(current_v), - "latest_version": str(latest_v), - } - - if latest_v > current_v: - update_info["update_available"] = True - - # Cache the entire dictionary - cache.set("update_check", update_info, 60 * 60 * 25) - logger.info(f"Update check complete. Result: {update_info}") + logger.warning(f"Invalid version tag from GitHub: {latest_version}") + return f"Invalid version tag: {latest_version}" + # Only the latest tag is stored; it's compared to APP_VERSION at render time so upgrades never leave it stale + caches["update_check"].set("latest_version", latest_version, None) + logger.info(f"Update check complete. Latest version: {latest_version}") else: logger.warning("Could not find 'tag_name' in GitHub API response.") diff --git a/app/apps/common/templatetags/cache_access.py b/app/apps/common/templatetags/cache_access.py index 154986f6..48cb6478 100644 --- a/app/apps/common/templatetags/cache_access.py +++ b/app/apps/common/templatetags/cache_access.py @@ -1,6 +1,7 @@ -# core/templatetags/update_tags.py from django import template -from django.core.cache import cache +from django.conf import settings +from django.core.cache import caches +from packaging.version import InvalidVersion, parse as parse_version register = template.Library() @@ -8,10 +9,18 @@ register = template.Library() @register.simple_tag def get_update_check(): """ - Retrieves the update status dictionary from the cache. - Returns a default dictionary if nothing is found. + Compares the latest GitHub release (cached by the check_for_updates task) + against the running APP_VERSION. """ - return cache.get("update_check") or { - "update_available": False, - "latest_version": "N/A", - } + latest_version = caches["update_check"].get("latest_version") + # The cached tag never expires, so opting out must also hide what was found before + if not settings.CHECK_FOR_UPDATES or not latest_version: + return {"update_available": False, "latest_version": "N/A"} + + try: + update_available = parse_version(latest_version) > parse_version(settings.APP_VERSION) + except InvalidVersion: + # Builds without a real version (dev, "unknown") can't be compared, so don't claim an update + update_available = False + + return {"update_available": update_available, "latest_version": latest_version} diff --git a/app/apps/common/tests/test_update_check.py b/app/apps/common/tests/test_update_check.py new file mode 100644 index 00000000..2f053dae --- /dev/null +++ b/app/apps/common/tests/test_update_check.py @@ -0,0 +1,67 @@ +from unittest import mock + +from django.core.cache import caches +from django.test import SimpleTestCase, override_settings + +from apps.common.tasks import check_for_updates +from apps.common.templatetags.cache_access import get_update_check + +# Distinct LOCATIONs: LocMemCache aliases with the same LOCATION share one store and would hide wrong-cache bugs +CACHES = { + "default": {"BACKEND": "django.core.cache.backends.locmem.LocMemCache", "LOCATION": "default"}, + "update_check": {"BACKEND": "django.core.cache.backends.locmem.LocMemCache", "LOCATION": "update_check"}, +} + + +@override_settings(CACHES=CACHES) +class GetUpdateCheckTests(SimpleTestCase): + def setUp(self): + caches["update_check"].clear() + + def render_with(self, app_version, latest): + if latest: + caches["update_check"].set("latest_version", latest) + with self.settings(APP_VERSION=app_version): + return get_update_check() + + def test_newer_release_is_available(self): + self.assertEqual( + self.render_with("1.0.0", "1.1.0"), + {"update_available": True, "latest_version": "1.1.0"}, + ) + + def test_after_upgrade_no_update(self): + self.assertFalse(self.render_with("1.1.0", "1.1.0")["update_available"]) + self.assertFalse(self.render_with("1.2.0", "1.1.0")["update_available"]) + + def test_nothing_cached(self): + self.assertEqual( + self.render_with("1.0.0", None), + {"update_available": False, "latest_version": "N/A"}, + ) + + def test_unknown_app_version_hides_update(self): + self.assertFalse(self.render_with("unknown", "1.1.0")["update_available"]) + + @override_settings(CHECK_FOR_UPDATES=False) + def test_disabled_hides_cached_update(self): + self.assertFalse(self.render_with("1.0.0", "1.1.0")["update_available"]) + + +@override_settings(CACHES=CACHES, CHECK_FOR_UPDATES=True) +class CheckForUpdatesTaskTests(SimpleTestCase): + def setUp(self): + caches["update_check"].clear() + + def run_task(self, tag): + with mock.patch("apps.common.tasks.requests.get") as get: + get.return_value.json.return_value = {"tag_name": tag} + check_for_updates.func() + return caches["update_check"].get("latest_version") + + def test_stores_normalized_version(self): + self.assertEqual(self.run_task("v1.8.0"), "1.8.0") + + def test_invalid_tag_keeps_previous_value(self): + caches["update_check"].set("latest_version", "1.7.0") + self.assertEqual(self.run_task("not-a-version"), "1.7.0")