From cfeeb80f5dd12fab5ff849523066ff92ccab040c Mon Sep 17 00:00:00 2001 From: Alisson Patricio Date: Sat, 3 Oct 2026 21:04:02 +0000 Subject: [PATCH 1/2] perf(hashids): memoize ENABLE_PUBLIC_ID_LOGIC for a few seconds Every ID resolved or rendered goes through the strategy pickers, and each called config.ENABLE_PUBLIC_ID_LOGIC: one constance read (a cache or database round trip) per node. A GraphQL list of 100 occurrences with their plant, taxon, photos and location made 865 reads per request; with constance's database backend and no cache, ~200 queries. The value is now memoized in-process for BASEAPP_PUBLIC_ID_LOGIC_CACHE_SECONDS (default 5, 0 disables it). Changes made through the same process (admin, override_config) clear the memo immediately via constance's config_updated signal; other processes pick them up when their memo expires. A request-scoped memo was considered, but under ASGI Django sends request_started with asend(), which runs receivers in a copied context, so a memo set there never reaches the view; it would need a middleware in every project instead. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019P1oyApi4BLS8iLfMEGm8b --- baseapp_core/hashids/strategies/__init__.py | 39 ++++++++++++++- .../tests/test_hashids_strategy_pickers.py | 49 ++++++++++++++++++- 2 files changed, 86 insertions(+), 2 deletions(-) diff --git a/baseapp_core/hashids/strategies/__init__.py b/baseapp_core/hashids/strategies/__init__.py index 2c0d4908..0bc48718 100644 --- a/baseapp_core/hashids/strategies/__init__.py +++ b/baseapp_core/hashids/strategies/__init__.py @@ -1,6 +1,9 @@ +import time from typing import TYPE_CHECKING, Any, Optional, Type from constance import config +from constance.signals import config_updated +from django.conf import settings from baseapp_core.hashids.models import LegacyWithPkMixin from baseapp_core.hashids.strategies.bundle import HashidsStrategyBundle @@ -12,8 +15,42 @@ from django.db.models import Model +# ENABLE_PUBLIC_ID_LOGIC is read for every ID resolved or rendered (each node of a GraphQL list, +# each serialized object), and every constance read is a cache or database round trip. The value +# is memoized in-process for BASEAPP_PUBLIC_ID_LOGIC_CACHE_SECONDS (0 disables it). Changes made +# through this process (admin, `override_config`) clear the memo at once via constance's +# `config_updated`; other processes pick them up when their memo expires. +DEFAULT_PUBLIC_ID_LOGIC_CACHE_SECONDS = 5 + +# (expires at, per time.monotonic(); value). Replaced as a whole, so concurrent readers never +# see a half-updated memo. +_public_id_logic_memo: tuple[float, bool] | None = None + + +def _clear_public_id_logic_memo(sender: Any = None, key: str | None = None, **kwargs: Any) -> None: + global _public_id_logic_memo + if key in (None, "ENABLE_PUBLIC_ID_LOGIC"): + _public_id_logic_memo = None + + +config_updated.connect( + _clear_public_id_logic_memo, dispatch_uid="baseapp_core.hashids.clear_public_id_logic_memo" +) + + def _is_public_id_logic_enabled() -> bool: - return bool(config.ENABLE_PUBLIC_ID_LOGIC) + global _public_id_logic_memo + now = time.monotonic() + memo = _public_id_logic_memo + if memo is not None and now < memo[0]: + return memo[1] + value = bool(config.ENABLE_PUBLIC_ID_LOGIC) + ttl = getattr( + settings, "BASEAPP_PUBLIC_ID_LOGIC_CACHE_SECONDS", DEFAULT_PUBLIC_ID_LOGIC_CACHE_SECONDS + ) + if ttl > 0: + _public_id_logic_memo = (now + ttl, value) + return value def _is_model_public_id_compatible(model_cls: type) -> bool: diff --git a/baseapp_core/hashids/tests/test_hashids_strategy_pickers.py b/baseapp_core/hashids/tests/test_hashids_strategy_pickers.py index 01b18ffd..5ed5ca95 100644 --- a/baseapp_core/hashids/tests/test_hashids_strategy_pickers.py +++ b/baseapp_core/hashids/tests/test_hashids_strategy_pickers.py @@ -1,5 +1,5 @@ import uuid -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock, PropertyMock, patch import pytest from constance.test import override_config @@ -7,6 +7,8 @@ from baseapp_core.graphql.relay import Node from baseapp_core.hashids.strategies import ( + DEFAULT_PUBLIC_ID_LOGIC_CACHE_SECONDS, + _clear_public_id_logic_memo, _is_public_id_logic_enabled, get_hashids_strategy_from_instance_or_cls, get_legacy_strategy, @@ -43,6 +45,51 @@ def test_is_public_id_logic_enabled_returns_false_when_config_disabled(self) -> with override_config(ENABLE_PUBLIC_ID_LOGIC=False): assert _is_public_id_logic_enabled() is False + def test_the_config_is_read_once_per_memo_period(self) -> None: + _clear_public_id_logic_memo() + reads = PropertyMock(return_value=True) + with ( + patch("baseapp_core.hashids.strategies.config") as config, + patch("baseapp_core.hashids.strategies.time.monotonic", return_value=100.0) as now, + ): + type(config).ENABLE_PUBLIC_ID_LOGIC = reads + results = [_is_public_id_logic_enabled() for _ in range(50)] + assert reads.call_count == 1 + + now.return_value = 100.0 + DEFAULT_PUBLIC_ID_LOGIC_CACHE_SECONDS + _is_public_id_logic_enabled() + + assert results == [True] * 50 + assert reads.call_count == 2 + _clear_public_id_logic_memo() + + def test_changes_through_constance_apply_immediately(self) -> None: + with override_config(ENABLE_PUBLIC_ID_LOGIC=True): + assert _is_public_id_logic_enabled() is True + with override_config(ENABLE_PUBLIC_ID_LOGIC=False): + assert _is_public_id_logic_enabled() is False + assert _is_public_id_logic_enabled() is True + + def test_other_settings_do_not_clear_the_memo(self) -> None: + with override_config(ENABLE_PUBLIC_ID_LOGIC=True): + assert _is_public_id_logic_enabled() is True + with patch("baseapp_core.hashids.strategies.config") as config: + type(config).ENABLE_PUBLIC_ID_LOGIC = PropertyMock(return_value=False) + _clear_public_id_logic_memo(key="SOME_OTHER_SETTING") + assert _is_public_id_logic_enabled() is True + _clear_public_id_logic_memo() + + def test_a_zero_period_disables_the_memo(self, settings) -> None: + settings.BASEAPP_PUBLIC_ID_LOGIC_CACHE_SECONDS = 0 + _clear_public_id_logic_memo() + reads = PropertyMock(return_value=True) + with patch("baseapp_core.hashids.strategies.config") as config: + type(config).ENABLE_PUBLIC_ID_LOGIC = reads + _is_public_id_logic_enabled() + _is_public_id_logic_enabled() + + assert reads.call_count == 2 + @pytest.mark.django_db class TestStrategyGetters: From e1d26792db9ae9ff9387060ad855515e9142ccdf Mon Sep 17 00:00:00 2001 From: Alisson Patricio Date: Fri, 9 Oct 2026 01:24:15 +0000 Subject: [PATCH 2/2] refactor(hashids): ENABLE_PUBLIC_ID_LOGIC is a Django setting, not constance Replaces the time-based memo: the flag is a deployment-level choice that never changes while the app runs, so it doesn't belong in the database. It is read for every ID resolved or rendered, and a setting costs no cache or database trip. - `_is_public_id_logic_enabled()` reads `settings.ENABLE_PUBLIC_ID_LOGIC`, True by default. - The memo, its constance signal handler and BASEAPP_PUBLIC_ID_LOGIC_CACHE_SECONDS are gone, and so is the ENABLE_PUBLIC_ID_LOGIC entry in the testproject's CONSTANCE_CONFIG. - Tests use override_settings instead of override_config for the flag. - hashids README points to the setting. Projects: drop ENABLE_PUBLIC_ID_LOGIC from CONSTANCE_CONFIG; set ENABLE_PUBLIC_ID_LOGIC = False in settings only if public IDs were disabled through constance. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019P1oyApi4BLS8iLfMEGm8b --- .../test_graphql_queries_object_files.py | 9 ++- .../tests/graphql/test_queries_user.py | 6 +- .../test_graphql_queries_object_blocks.py | 11 +-- .../test_graphql_queries_object_comments.py | 27 +++---- baseapp_core/hashids/README.md | 4 +- baseapp_core/hashids/strategies/__init__.py | 44 ++--------- .../tests/test_hashids_strategy_pickers.py | 79 +++++-------------- baseapp_core/tests/test_ids_resolver.py | 4 +- .../test_graphql_queries_object_follows.py | 13 +-- .../test_graphql_queries_object_ratings.py | 7 +- .../test_graphql_queries_object_reactions.py | 7 +- .../test_graphql_queries_object_reports.py | 7 +- testproject/settings.py | 4 - 13 files changed, 74 insertions(+), 148 deletions(-) diff --git a/baseapp/files/tests/test_graphql_queries_object_files.py b/baseapp/files/tests/test_graphql_queries_object_files.py index 17550b81..30511a37 100644 --- a/baseapp/files/tests/test_graphql_queries_object_files.py +++ b/baseapp/files/tests/test_graphql_queries_object_files.py @@ -1,3 +1,5 @@ +from django.test import override_settings + """ Query-count tests for the `FilesInterface`. @@ -9,7 +11,6 @@ import pytest import swapper -from constance.test import override_config from django.contrib.contenttypes.models import ContentType from baseapp_comments.tests.factories import CommentFactory @@ -69,7 +70,7 @@ def attach_files(target, amount: int) -> None: ) -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_files_count_is_flat_regardless_of_file_volume(graphql_client_with_queries): """`filesCount` should be a flat query path: regardless of how many `File` rows point at the target, the GraphQL query should make the same number of @@ -98,7 +99,7 @@ def test_files_count_is_flat_regardless_of_file_volume(graphql_client_with_queri assert small_count == EXPECTED_FILES_INTERFACE_QUERY_COUNT -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_files_count_zero_when_no_files_does_not_extra_query(graphql_client_with_queries): """A target with no files should resolve in the same query budget as one with files — the FileTarget row simply doesn't exist yet and the annotation @@ -124,7 +125,7 @@ def test_files_count_zero_when_no_files_does_not_extra_query(graphql_client_with assert queries_empty.count == EXPECTED_FILES_INTERFACE_QUERY_COUNT -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_listing_comments_with_files_is_flat(graphql_client_with_queries): """Listing comments that each have files must not fan out one FileTarget query per comment — the files annotations ride along in the comment list diff --git a/baseapp_auth/tests/graphql/test_queries_user.py b/baseapp_auth/tests/graphql/test_queries_user.py index 3ff06b89..bae9b7c3 100644 --- a/baseapp_auth/tests/graphql/test_queries_user.py +++ b/baseapp_auth/tests/graphql/test_queries_user.py @@ -1,7 +1,7 @@ import pytest -from constance.test import override_config from django.contrib.auth.models import Permission from django.contrib.contenttypes.models import ContentType +from django.test import override_settings from baseapp_core.tests.factories import UserFactory @@ -157,7 +157,7 @@ def test_overcomplex_queries_are_not_executed(graphql_client_with_queries) -> No assert queries.count == 0 -@override_config(ENABLE_PUBLIC_ID_LOGIC=False) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=False) def test_anon_can_query_users_list_with_optimized_query(graphql_client_with_queries) -> None: UserFactory.create_batch(10) ContentType.objects.clear_cache() @@ -182,7 +182,7 @@ def test_anon_can_query_users_list_with_optimized_query(graphql_client_with_quer # 5. Same as #3 with LIMIT 10 for the returned page slice. -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_anon_can_query_users_list_with_optimized_query_with_public_id( graphql_client_with_queries, ) -> None: diff --git a/baseapp_blocks/tests/test_graphql_queries_object_blocks.py b/baseapp_blocks/tests/test_graphql_queries_object_blocks.py index 44137901..39c4568c 100644 --- a/baseapp_blocks/tests/test_graphql_queries_object_blocks.py +++ b/baseapp_blocks/tests/test_graphql_queries_object_blocks.py @@ -1,3 +1,5 @@ +from django.test import override_settings + """ Query-count tests for `BlocksInterface`: assert the GraphQL `blockersCount` / `blockingCount` fields resolve in a flat number of DB queries regardless of how @@ -6,7 +8,6 @@ """ import pytest -from constance.test import override_config from django.contrib.contenttypes.models import ContentType from baseapp_core.graphql.utils import capture_database_queries @@ -71,7 +72,7 @@ EXPECTED_BLOCKS_INTERFACE_NESTED_LIST_QUERY_COUNT = 9 -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_block_counts_are_flat_regardless_of_block_volume( django_user_client, graphql_user_client ) -> None: @@ -107,7 +108,7 @@ def test_block_counts_are_flat_regardless_of_block_volume( assert small_count == EXPECTED_BLOCKS_INTERFACE_QUERY_COUNT -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_block_counts_zero_when_no_blocks_does_not_extra_query( django_user_client, graphql_user_client ) -> None: @@ -141,7 +142,7 @@ def test_block_counts_zero_when_no_blocks_does_not_extra_query( assert queries_empty.count == EXPECTED_BLOCKS_INTERFACE_QUERY_COUNT -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_block_counts_perm_denied_path_is_flat(graphql_user_client) -> None: """A non-superuser sees `null` for both counts and the perm-check path stays flat regardless of block volume.""" @@ -172,7 +173,7 @@ def test_block_counts_perm_denied_path_is_flat(graphql_user_client) -> None: assert queries_small.count <= EXPECTED_BLOCKS_INTERFACE_PERM_DENIED_UPPER_BOUND -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_block_list_with_target_blockers_count_is_flat( django_user_client, graphql_user_client ) -> None: diff --git a/baseapp_comments/tests/test_graphql_queries_object_comments.py b/baseapp_comments/tests/test_graphql_queries_object_comments.py index 810ffc69..ab7775e5 100644 --- a/baseapp_comments/tests/test_graphql_queries_object_comments.py +++ b/baseapp_comments/tests/test_graphql_queries_object_comments.py @@ -1,5 +1,4 @@ import pytest -from constance.test import override_config from django.contrib.contenttypes.models import ContentType from django.test import override_settings @@ -141,7 +140,7 @@ """ -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_anon_see_comments_and_replies(django_user_client, graphql_client_with_queries) -> None: target = CommentFactory() user = django_user_client.user @@ -186,7 +185,7 @@ def test_anon_see_comments_and_replies(django_user_client, graphql_client_with_q # 13) 'SELECT "users_user"."id", "users_user"."first_name", (SELECT U0."public_id" AS "public_id" FROM "baseapp_core_documentid" U0 WHERE (U0."content_type_id" = 3272 AND U0."object_id" = ("users_user"."id"))) AS "mapped_public_id" FROM "users_user" WHERE ("users_user"."id") IN ((1129))' -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_anon_see_comments_and_replies_with_pagination( django_user_client, graphql_client_with_queries ) -> None: @@ -214,7 +213,7 @@ def test_anon_see_comments_and_replies_with_pagination( assert queries.count == 13 -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_top_level_comments_pagination_on_page_target(django_user_client, graphql_client) -> None: """Reproduces the FE bug: when using first=5 on the top-level comments of a Page (matching the FE CommentsList query), hasNextPage must be True @@ -235,7 +234,7 @@ def test_top_level_comments_pagination_on_page_target(django_user_client, graphq assert comments_connection["pageInfo"]["hasNextPage"] is True -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_top_level_comments_pagination_on_comment_target( django_user_client, graphql_client ) -> None: @@ -256,7 +255,7 @@ def test_top_level_comments_pagination_on_comment_target( assert comments_connection["pageInfo"]["hasNextPage"] is True -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_logged_user_replies_to_a_page_comment_has_next_page_true_when_more_than_5_replies( django_user_client, graphql_user_client ) -> None: @@ -287,7 +286,7 @@ def test_logged_user_replies_to_a_page_comment_has_next_page_true_when_more_than assert comment_replies["pageInfo"]["hasNextPage"] is True -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_get_queryset_skips_filtering_only_when_hint_set( django_user_client, graphql_user_client ) -> None: @@ -348,7 +347,7 @@ def __init__(self, request) -> None: assert result.count() == 3 # blocked user's comments excluded despite cache -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_blocked_profiles_excluded_with_pagination(django_user_client, graphql_user_client) -> None: """Blocked/blocking profiles are excluded and pagination still works correctly.""" page = PageFactory(user=django_user_client.user) @@ -382,7 +381,7 @@ def test_blocked_profiles_excluded_with_pagination(django_user_client, graphql_u assert comments_connection["pageInfo"]["endCursor"] is not None -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_top_level_comments_second_page_with_cursor(django_user_client, graphql_client) -> None: """Fetching the second page with an `after` cursor returns the remaining comments.""" page = PageFactory(user=django_user_client.user) @@ -533,7 +532,7 @@ def test_anon_cant_see_comments(graphql_client) -> None: assert len(content["data"]["node"]["comments"]["edges"]) == 0 -@override_config(ENABLE_PUBLIC_ID_LOGIC=False) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=False) def test_comments_query_is_partially_optimized( django_user_client, graphql_client_with_queries ) -> None: @@ -572,7 +571,7 @@ def test_comments_query_is_partially_optimized( # filters replies by status; this is the "partially optimized" part the test name refers to -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_comments_query_is_optimized_with_nested_replies( django_user_client, graphql_client_with_queries ) -> None: @@ -616,7 +615,7 @@ def test_comments_query_is_optimized_with_nested_replies( assert queries.count == 12 -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_comments_query_is_partially_optimized_with_public_id( django_user_client, graphql_client_with_queries ) -> None: @@ -656,7 +655,7 @@ def test_comments_query_is_partially_optimized_with_public_id( # 11) 'SELECT "profiles_profile"."id", "profiles_profile"."name", (SELECT U0."public_id" FROM "baseapp_core_documentid" U0 WHERE (U0."content_type_id" = 28 AND U0."object_id" = ("profiles_profile"."id"))) AS "mapped_public_id" FROM "profiles_profile" WHERE "profiles_profile"."id" IN (736)' -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_comments_query_is_partially_optimized_with_public_id_and_pagination( django_user_client, graphql_client_with_queries ) -> None: @@ -686,7 +685,7 @@ def test_comments_query_is_partially_optimized_with_public_id_and_pagination( assert queries.count == 12 -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_comments_query_from_foreigh_target_is_partially_optimized_with_public_id( django_user_client, graphql_client_with_queries, diff --git a/baseapp_core/hashids/README.md b/baseapp_core/hashids/README.md index e5d4866d..0aae9aeb 100644 --- a/baseapp_core/hashids/README.md +++ b/baseapp_core/hashids/README.md @@ -55,7 +55,7 @@ The key idea behind this structure is that each folder represents a different st #### Legacy vs Public Id -The Legacy strategy follows the default behavior of Django, DRF, and Graphene. It relies on model primary keys to handle queries and updates coming from external services. If you decide to keep using primary keys in the frontend, which is not recommended, you can disable the Public Id strategy through the constance configuration `ENABLE_PUBLIC_ID_LOGIC`. When disabled, the system will rely exclusively on the Legacy strategy. +The Legacy strategy follows the default behavior of Django, DRF, and Graphene. It relies on model primary keys to handle queries and updates coming from external services. If you decide to keep using primary keys in the frontend, which is not recommended, you can disable the Public Id strategy with the Django setting `ENABLE_PUBLIC_ID_LOGIC = False`. When disabled, the system will rely exclusively on the Legacy strategy. The Public Id strategy uses the `public_id` field from the `DocumentId` model. This strategy is responsible for resolving and exposing public IDs and is the recommended approach. @@ -73,7 +73,7 @@ Public facing functions here are functions that other parts of the system are ex Currently, the heuristics for selecting the Public Id strategy over the Legacy one are: -1. The Public Id strategy must be enabled through the constance configuration `ENABLE_PUBLIC_ID_LOGIC`. +1. The Public Id strategy must be enabled with the Django setting `ENABLE_PUBLIC_ID_LOGIC` (`True` by default). 2. The model being filtered must extend `DocumentIdMixin`. 3. The model being filtered must use an auto incrementing primary key. diff --git a/baseapp_core/hashids/strategies/__init__.py b/baseapp_core/hashids/strategies/__init__.py index 0bc48718..1afd67a4 100644 --- a/baseapp_core/hashids/strategies/__init__.py +++ b/baseapp_core/hashids/strategies/__init__.py @@ -1,8 +1,5 @@ -import time from typing import TYPE_CHECKING, Any, Optional, Type -from constance import config -from constance.signals import config_updated from django.conf import settings from baseapp_core.hashids.models import LegacyWithPkMixin @@ -15,42 +12,13 @@ from django.db.models import Model -# ENABLE_PUBLIC_ID_LOGIC is read for every ID resolved or rendered (each node of a GraphQL list, -# each serialized object), and every constance read is a cache or database round trip. The value -# is memoized in-process for BASEAPP_PUBLIC_ID_LOGIC_CACHE_SECONDS (0 disables it). Changes made -# through this process (admin, `override_config`) clear the memo at once via constance's -# `config_updated`; other processes pick them up when their memo expires. -DEFAULT_PUBLIC_ID_LOGIC_CACHE_SECONDS = 5 - -# (expires at, per time.monotonic(); value). Replaced as a whole, so concurrent readers never -# see a half-updated memo. -_public_id_logic_memo: tuple[float, bool] | None = None - - -def _clear_public_id_logic_memo(sender: Any = None, key: str | None = None, **kwargs: Any) -> None: - global _public_id_logic_memo - if key in (None, "ENABLE_PUBLIC_ID_LOGIC"): - _public_id_logic_memo = None - - -config_updated.connect( - _clear_public_id_logic_memo, dispatch_uid="baseapp_core.hashids.clear_public_id_logic_memo" -) - - def _is_public_id_logic_enabled() -> bool: - global _public_id_logic_memo - now = time.monotonic() - memo = _public_id_logic_memo - if memo is not None and now < memo[0]: - return memo[1] - value = bool(config.ENABLE_PUBLIC_ID_LOGIC) - ttl = getattr( - settings, "BASEAPP_PUBLIC_ID_LOGIC_CACHE_SECONDS", DEFAULT_PUBLIC_ID_LOGIC_CACHE_SECONDS - ) - if ttl > 0: - _public_id_logic_memo = (now + ttl, value) - return value + """`settings.ENABLE_PUBLIC_ID_LOGIC` (default True). + + A deployment-level choice that never changes while the app runs, so it is a Django setting: + it is read for every ID resolved or rendered, and a setting costs no cache or database trip. + """ + return bool(getattr(settings, "ENABLE_PUBLIC_ID_LOGIC", True)) def _is_model_public_id_compatible(model_cls: type) -> bool: diff --git a/baseapp_core/hashids/tests/test_hashids_strategy_pickers.py b/baseapp_core/hashids/tests/test_hashids_strategy_pickers.py index 5ed5ca95..711883d8 100644 --- a/baseapp_core/hashids/tests/test_hashids_strategy_pickers.py +++ b/baseapp_core/hashids/tests/test_hashids_strategy_pickers.py @@ -1,14 +1,12 @@ import uuid -from unittest.mock import MagicMock, PropertyMock, patch +from unittest.mock import MagicMock, patch import pytest -from constance.test import override_config +from django.test import override_settings from graphql_relay import to_global_id from baseapp_core.graphql.relay import Node from baseapp_core.hashids.strategies import ( - DEFAULT_PUBLIC_ID_LOGIC_CACHE_SECONDS, - _clear_public_id_logic_memo, _is_public_id_logic_enabled, get_hashids_strategy_from_instance_or_cls, get_legacy_strategy, @@ -37,58 +35,17 @@ @pytest.mark.django_db class TestPublicIdLogicEnabled: - def test_is_public_id_logic_enabled_returns_true_when_config_enabled(self) -> None: - with override_config(ENABLE_PUBLIC_ID_LOGIC=True): - assert _is_public_id_logic_enabled() is True - - def test_is_public_id_logic_enabled_returns_false_when_config_disabled(self) -> None: - with override_config(ENABLE_PUBLIC_ID_LOGIC=False): - assert _is_public_id_logic_enabled() is False - - def test_the_config_is_read_once_per_memo_period(self) -> None: - _clear_public_id_logic_memo() - reads = PropertyMock(return_value=True) - with ( - patch("baseapp_core.hashids.strategies.config") as config, - patch("baseapp_core.hashids.strategies.time.monotonic", return_value=100.0) as now, - ): - type(config).ENABLE_PUBLIC_ID_LOGIC = reads - results = [_is_public_id_logic_enabled() for _ in range(50)] - assert reads.call_count == 1 - - now.return_value = 100.0 + DEFAULT_PUBLIC_ID_LOGIC_CACHE_SECONDS - _is_public_id_logic_enabled() - - assert results == [True] * 50 - assert reads.call_count == 2 - _clear_public_id_logic_memo() - - def test_changes_through_constance_apply_immediately(self) -> None: - with override_config(ENABLE_PUBLIC_ID_LOGIC=True): - assert _is_public_id_logic_enabled() is True - with override_config(ENABLE_PUBLIC_ID_LOGIC=False): - assert _is_public_id_logic_enabled() is False - assert _is_public_id_logic_enabled() is True - - def test_other_settings_do_not_clear_the_memo(self) -> None: - with override_config(ENABLE_PUBLIC_ID_LOGIC=True): - assert _is_public_id_logic_enabled() is True - with patch("baseapp_core.hashids.strategies.config") as config: - type(config).ENABLE_PUBLIC_ID_LOGIC = PropertyMock(return_value=False) - _clear_public_id_logic_memo(key="SOME_OTHER_SETTING") - assert _is_public_id_logic_enabled() is True - _clear_public_id_logic_memo() - - def test_a_zero_period_disables_the_memo(self, settings) -> None: - settings.BASEAPP_PUBLIC_ID_LOGIC_CACHE_SECONDS = 0 - _clear_public_id_logic_memo() - reads = PropertyMock(return_value=True) - with patch("baseapp_core.hashids.strategies.config") as config: - type(config).ENABLE_PUBLIC_ID_LOGIC = reads - _is_public_id_logic_enabled() - _is_public_id_logic_enabled() - - assert reads.call_count == 2 + def test_enabled_by_the_setting(self, settings) -> None: + settings.ENABLE_PUBLIC_ID_LOGIC = True + assert _is_public_id_logic_enabled() is True + + def test_disabled_by_the_setting(self, settings) -> None: + settings.ENABLE_PUBLIC_ID_LOGIC = False + assert _is_public_id_logic_enabled() is False + + def test_enabled_by_default(self, settings) -> None: + del settings.ENABLE_PUBLIC_ID_LOGIC + assert _is_public_id_logic_enabled() is True @pytest.mark.django_db @@ -359,7 +316,7 @@ def test_returns_none_when_pk_strategy_get_node_returns_none(self) -> None: @pytest.mark.django_db class TestHashidsStrategyIntegrationScenarios: def test_public_id_model_with_public_id_logic_enabled_uses_public_id_strategy(self) -> None: - with override_config(ENABLE_PUBLIC_ID_LOGIC=True): + with override_settings(ENABLE_PUBLIC_ID_LOGIC=True): dummy_instance = DummyPublicIdModelFactory() strategy = get_hashids_strategy_from_instance_or_cls(dummy_instance) @@ -373,7 +330,7 @@ def test_public_id_model_with_public_id_logic_enabled_uses_public_id_strategy(se assert resolved_instance.pk == dummy_instance.pk def test_public_id_model_with_public_id_logic_disabled_uses_legacy_strategy(self) -> None: - with override_config(ENABLE_PUBLIC_ID_LOGIC=False): + with override_settings(ENABLE_PUBLIC_ID_LOGIC=False): dummy_instance = DummyPublicIdModelFactory() strategy = get_hashids_strategy_from_instance_or_cls(dummy_instance) @@ -390,7 +347,7 @@ def test_public_id_model_with_public_id_logic_disabled_uses_legacy_strategy(self def test_legacy_model_always_uses_legacy_strategy(self) -> None: for config_value in [True, False, None, ""]: - with override_config(ENABLE_PUBLIC_ID_LOGIC=config_value): + with override_settings(ENABLE_PUBLIC_ID_LOGIC=config_value): dummy_instance = DummyLegacyModelFactory() strategy = get_hashids_strategy_from_instance_or_cls(dummy_instance) @@ -401,7 +358,7 @@ def test_legacy_model_always_uses_legacy_strategy(self) -> None: assert resolved_id == dummy_instance.pk def test_uuid4_global_id_flow_with_public_id_logic_enabled(self) -> None: - with override_config(ENABLE_PUBLIC_ID_LOGIC=True): + with override_settings(ENABLE_PUBLIC_ID_LOGIC=True): dummy_instance = DummyPublicIdModelFactory() test_uuid = str(dummy_instance.public_id) graphene_type_mock = MagicMock() @@ -439,7 +396,7 @@ def test_mixed_strategy_usage_in_same_test(self) -> None: public_id_instance = DummyPublicIdModelFactory() legacy_instance = DummyLegacyModelFactory() - with override_config(ENABLE_PUBLIC_ID_LOGIC=True): + with override_settings(ENABLE_PUBLIC_ID_LOGIC=True): # Test public ID strategy public_id_strategy = get_hashids_strategy_from_instance_or_cls(public_id_instance) assert isinstance(public_id_strategy.id_resolver, PublicIdResolverStrategy) diff --git a/baseapp_core/tests/test_ids_resolver.py b/baseapp_core/tests/test_ids_resolver.py index c6e37074..39bf2f10 100644 --- a/baseapp_core/tests/test_ids_resolver.py +++ b/baseapp_core/tests/test_ids_resolver.py @@ -1,7 +1,7 @@ from base64 import b64encode import pytest -from constance.test import override_config +from django.test import override_settings from baseapp_core.tests.factories import UserFactory @@ -17,7 +17,7 @@ """ -@override_config(ENABLE_PUBLIC_ID_LOGIC=False) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=False) def test_get_node_with_old_1586_pk_issue(graphql_client) -> None: # The global_id is created by a Base64-encoding string in the format "TypeName:ID". # For certain IDs (like 1586), the resulting Base64 string will mislead how Graphene resolves the global_id to _type and _id. e.g. "1586" will be resolved as _type="ן" and _id="". diff --git a/baseapp_follows/tests/test_graphql_queries_object_follows.py b/baseapp_follows/tests/test_graphql_queries_object_follows.py index 48f0d6d7..413cfaa0 100644 --- a/baseapp_follows/tests/test_graphql_queries_object_follows.py +++ b/baseapp_follows/tests/test_graphql_queries_object_follows.py @@ -1,3 +1,5 @@ +from django.test import override_settings + """ Query-count tests for the FollowsInterface. @@ -14,7 +16,6 @@ """ import pytest -from constance.test import override_config from django.contrib.contenttypes.models import ContentType from baseapp_core.tests.factories import UserFactory @@ -143,7 +144,7 @@ """ -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_anon_followers_and_following_counts_query_count( django_user_client, graphql_client_with_queries ) -> None: @@ -172,7 +173,7 @@ def test_anon_followers_and_following_counts_query_count( assert queries.count == 2 -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_anon_followers_list_query_count_does_not_grow_with_followers( django_user_client, graphql_client_with_queries ) -> None: @@ -212,7 +213,7 @@ def test_anon_followers_list_query_count_does_not_grow_with_followers( assert queries_big.count == small_count -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_anon_following_list_query_count_is_flat( django_user_client, graphql_client_with_queries ) -> None: @@ -244,7 +245,7 @@ def test_anon_following_list_query_count_is_flat( assert queries.count == 10 -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_anon_followers_pagination_query_count_independent_of_page_size( django_user_client, graphql_client_with_queries ) -> None: @@ -282,7 +283,7 @@ def test_anon_followers_pagination_query_count_independent_of_page_size( assert abs(q_big.count - q_small.count) <= 1 -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_anon_nested_followers_count_does_not_explode( django_user_client, graphql_client_with_queries ) -> None: diff --git a/baseapp_ratings/tests/test_graphql_queries_object_ratings.py b/baseapp_ratings/tests/test_graphql_queries_object_ratings.py index c5f60136..c751693b 100644 --- a/baseapp_ratings/tests/test_graphql_queries_object_ratings.py +++ b/baseapp_ratings/tests/test_graphql_queries_object_ratings.py @@ -1,3 +1,5 @@ +from django.test import override_settings + """ Query-count tests for the `RatingsInterface`. @@ -9,7 +11,6 @@ """ import pytest -from constance.test import override_config from django.contrib.contenttypes.models import ContentType from baseapp_core.tests.factories import UserFactory @@ -39,7 +40,7 @@ """ -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_anon_ratings_count_is_flat_regardless_of_rate_volume( django_user_client, graphql_client_with_queries ) -> None: @@ -73,7 +74,7 @@ def test_anon_ratings_count_is_flat_regardless_of_rate_volume( assert small_count == EXPECTED_RATINGS_INTERFACE_QUERY_COUNT -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_anon_ratings_count_zero_when_no_rates_does_not_extra_query( django_user_client, graphql_client_with_queries ) -> None: diff --git a/baseapp_reactions/tests/test_graphql_queries_object_reactions.py b/baseapp_reactions/tests/test_graphql_queries_object_reactions.py index 11069e6e..adc46421 100644 --- a/baseapp_reactions/tests/test_graphql_queries_object_reactions.py +++ b/baseapp_reactions/tests/test_graphql_queries_object_reactions.py @@ -1,3 +1,5 @@ +from django.test import override_settings + """ Query-count tests for the `ReactionsInterface`. @@ -9,7 +11,6 @@ """ import pytest -from constance.test import override_config from django.contrib.contenttypes.models import ContentType from baseapp_comments.tests.factories import CommentFactory @@ -43,7 +44,7 @@ """ -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_anon_reactions_count_is_flat_regardless_of_reaction_volume( graphql_client_with_queries, ) -> None: @@ -76,7 +77,7 @@ def test_anon_reactions_count_is_flat_regardless_of_reaction_volume( assert small_count == EXPECTED_REACTIONS_INTERFACE_QUERY_COUNT -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_anon_reactions_count_zero_when_no_reactions_does_not_extra_query( graphql_client_with_queries, ) -> None: diff --git a/baseapp_reports/tests/test_graphql_queries_object_reports.py b/baseapp_reports/tests/test_graphql_queries_object_reports.py index 1f94b87e..edf8c574 100644 --- a/baseapp_reports/tests/test_graphql_queries_object_reports.py +++ b/baseapp_reports/tests/test_graphql_queries_object_reports.py @@ -1,3 +1,5 @@ +from django.test import override_settings + """ Query-count tests for the `ReportsInterface`. @@ -11,7 +13,6 @@ """ import pytest -from constance.test import override_config from django.contrib.contenttypes.models import ContentType from baseapp_core.tests.factories import UserFactory @@ -42,7 +43,7 @@ """ -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_anon_reports_count_is_flat_regardless_of_report_volume( django_user_client, graphql_client_with_queries ) -> None: @@ -81,7 +82,7 @@ def test_anon_reports_count_is_flat_regardless_of_report_volume( assert small_count == EXPECTED_REPORTS_INTERFACE_QUERY_COUNT -@override_config(ENABLE_PUBLIC_ID_LOGIC=True) +@override_settings(ENABLE_PUBLIC_ID_LOGIC=True) def test_anon_reports_count_zero_when_no_reports_does_not_extra_query( django_user_client, graphql_client_with_queries ) -> None: diff --git a/testproject/settings.py b/testproject/settings.py index 3de6ec57..365a4f0e 100644 --- a/testproject/settings.py +++ b/testproject/settings.py @@ -178,10 +178,6 @@ "The model used to link the Stripe customer entity. It must have an target field that point to an entity with an email field, or have an email field itself.", ), ), - ( - "ENABLE_PUBLIC_ID_LOGIC", - (True, "Enable public ID logic."), - ), ( "ANONYMIZE_TASK_DELAY_DAYS", (19, "Delay in days before running anonymize user task"),