EnxDev commented on code in PR #44017:
URL: https://github.com/apache/superset/pull/44017#discussion_r4066188646
##########
superset/subjects/utils.py:
##########
@@ -206,7 +206,23 @@ def get_user_subject_ids(user_id: int) -> list[int]:
1. The user's own USER-type subject
2. ROLE-type subjects for all direct and group-derived roles the user has
3. GROUP-type subjects for all groups the user belongs to
+
+ Memoised for the duration of the request, keyed by user id. Authorization
+ calls this once per object checked -- ``is_editor``/``is_viewer`` run it
for
+ every chart on a dashboard. Nothing reads a user's subjects after changing
+ them within a single request, so the cached set cannot go stale in place.
"""
+ if not has_request_context():
+ return _query_user_subject_ids(user_id)
+ cache: dict[int, list[int]] = g.setdefault("_user_subject_ids", {})
Review Comment:
Could we tie this cache to the request itself, and add a test with two
requests inside the same app context? `has_request_context()` controls when the
cache is used, but `g` still survives until the enclosing app context ends.
I reproduced this with the changed function: inside one `app.app_context()`,
a first `app.test_request_context('/')` caches `[7, 8]` for user 1. After
changing the mocked query result to `[9]`, a second request still returns `[7,
8]` without querying. The autouse `app_context` fixture makes this relevant to
tests that issue multiple requests and change memberships between them.
The new test explicitly pushes a fresh app context for its second lookup, so
it doesn't catch this case. Storing the cache on the request object would give
it the documented lifetime.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
