fix: key enum_name_overrides cache on settings object identity, not just language - #1522
Open
juneja-varun wants to merge 1 commit into
Open
Conversation
…ust language load_enum_name_overrides() was cached only on language via functools.lru_cache, but it internally reads the mutable global spectacular_settings.ENUM_NAME_OVERRIDES. SpectacularAPIView's custom_settings context manager patches this global temporarily to serve multiple schemas with different enum overrides from the same process. Since the cache didn't account for that, loading overrides for one ENUM_NAME_OVERRIDES dict would return the stale cached result for a different one with the same language, once both had been requested. Add id(spectacular_settings.ENUM_NAME_OVERRIDES) to the cache key. The dict itself (or its values, which can be raw choice lists) isn't hashable, so it can't be used as a cache key/argument directly - but each distinct settings profile is a genuinely distinct dict object for the lifetime of the process, so identity is a safe and simple proxy. Fixes tfranzel#1244
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
load_enum_name_overrides()is cached viafunctools.lru_cachekeyed only onlanguage, but it internally reads the mutable globalspectacular_settings.ENUM_NAME_OVERRIDES.SpectacularAPIView'scustom_settingscontext manager patches this global temporarily, which is how a project serves multiple schemas with different enum overrides from the same process. Since the cache key doesn't account for that, whicheverENUM_NAME_OVERRIDESwas in effect on the first call for a given language gets baked into the cached result — a second schema with different overrides but the same language silently gets the first schema's overrides back.Fix
Add
id(spectacular_settings.ENUM_NAME_OVERRIDES)to the cache key alongsidelanguage. I considered passing the settings dict itself as an argument (as sketched in the issue), but that doesn't work: the dict itself isn't hashable, and neither are some of the values it can legitimately hold (raw choice lists, not just strings/classes/callables), sofunctools.lru_cachewould raiseTypeErrorimmediately rather than silently misbehave. Usingid()sidesteps that entirely — each distinct settings profile (default vs. per-schemacustom_settings) is a genuinely distinct dict object for the lifetime of the process, so identity alone is enough to key the cache correctly.Also updated the second read site inside the function (the duplicate-check at the bottom) to use the same local variable, so it's consistent with what was actually cached.
Testing
Added
test_global_enum_naming_override_not_stale_across_settings_objects, which loads overrides for two differentENUM_NAME_OVERRIDESdicts (same language, no cache clear in between) and asserts the second load reflects its own settings rather than the first's. Confirmed it fails with the exact stale-result symptom on unpatched code, and passes with the fix.Full suite: 544 passed (3 pre-existing failures unrelated to this change — missing GDAL system library and an oauth2_provider deploy-check warning, confirmed present on
mastertoo).flake8,isort --check, andmypyall clean on the changed files.Fixes #1244