rusackas opened a new pull request, #42122:
URL: https://github.com/apache/superset/pull/42122
### SUMMARY
Review of #33924 raised a concern (flagged by a bot, echoing a broader point
about the whole macro family) that the user-metadata Jinja macros only add to
the query cache key when their value is *present* — skipping the key when the
value is absent — and that this could cause cache collisions across users:
```python
if user_id := get_user_id():
if add_to_cache_keys:
self.cache_key_wrapper(user_id)
return user_id
return None
```
This affects the whole family: `current_user_id`, `current_username`,
`current_user_email`, `current_user_roles`.
It turns out the skipped path is **not** a collision risk, and this PR adds
tests proving it rather than changing behavior. The reason it's safe: an absent
value renders identically for every user (the macro returns `None`), so absent
users correctly *share* one cache entry instead of colliding, while present
values are always distinct in the key. Where the attribute is absent and a
default is used (as in #33924's `get_guest_user_attribute`), the default is a
literal in the query template, and the template is itself part of the cache key
— so absent renders are still fully determined.
### Tests added (all pass)
- distinct users contribute **disjoint** cache-key values, so neither is
served the other's cached result (the multi-tenant safety property)
- identical users contribute identical values (correct sharing, no needless
fragmentation)
- an anonymous render contributes **nothing**, so it can never collide with
a logged-in user's cache entry
- a change in any single metadata field yields a distinct cache key
### TESTING INSTRUCTIONS
```bash
pytest tests/unit_tests/jinja_context_test.py -k "cache_keys or
anonymous_user_never"
```
No behavior change — these lock in and document the existing correct
behavior, and clear the way for #33924's `get_guest_user_attribute` (which
follows the same pattern).
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Changes UI
--
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]
