villebro commented on code in PR #30549:
URL: https://github.com/apache/superset/pull/30549#discussion_r1792147785
##########
tests/integration_tests/sqla_models_tests.py:
##########
@@ -164,6 +164,24 @@ def test_extra_cache_keys(self, mock_user_email,
mock_username, mock_user_id):
self.assertTrue(table1.has_extra_cache_key_calls(query_obj))
assert set(extra_cache_keys) == {1, "abc", "[email protected]"}
+ # Table with Jinja callable with set.
+ table1 = SqlaTable(
+ table_name="test_has_extra_cache_keys_table_with_set",
+ sql="""
+ {% set user_email = current_user_email() %}
+ SELECT
+ '{{ current_user_id() }}' as id,
+ '{{ current_username() }}' as username,
+ '{{ user_email }}' as email
+ """,
+ database=get_example_database(),
+ )
+
+ query_obj = dict(**base_query_obj, extras={})
+ extra_cache_keys = table1.get_extra_cache_keys(query_obj)
+ assert table1.has_extra_cache_key_calls(query_obj) is True
+ assert set(extra_cache_keys) == {1, "abc", "[email protected]"}
+
Review Comment:
bycatch - this test is huge, and could do with some heavy parametrization.
Not necessarily something that has to be done here, but it would leave the
codebase in a much tidier state.
##########
superset/jinja_context.py:
##########
@@ -104,13 +104,11 @@ class ExtraCache:
# Regular expression for detecting the presence of templated methods which
could
# be added to the cache key.
regex = re.compile(
- r"\{\{.*("
- r"current_user_id\(.*\)|"
+ r"(current_user_id\(.*\)|"
r"current_username\(.*\)|"
r"current_user_email\(.*\)|"
r"cache_key_wrapper\(.*\)|"
- r"url_param\(.*\)"
- r").*\}\}"
+ r"url_param\(.*\))"
Review Comment:
I agree with Daniel. We should check for both valid cases `{{` and `{%` to
avoid false positives (although I admit they're going to be rare).
--
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]
