rusackas commented on code in PR #44406:
URL: https://github.com/apache/superset/pull/44406#discussion_r4139021305
##########
tests/unit_tests/common/test_query_context_processor.py:
##########
@@ -2226,6 +2396,180 @@ def
test_get_df_payload_no_warning_when_not_memory_limited() -> None:
assert result["warning"] is None
+def
test_get_df_payload_result_decouples_annotation_cache_from_dataframe_cache():
Review Comment:
Good catch, added the return type hint.
##########
tests/unit_tests/common/test_query_context_processor.py:
##########
@@ -2226,6 +2396,180 @@ def
test_get_df_payload_no_warning_when_not_memory_limited() -> None:
assert result["warning"] is None
+def
test_get_df_payload_result_decouples_annotation_cache_from_dataframe_cache():
+ """
+ The dataframe cache entry must stay shareable across viewers, and
+ annotation-layer data must be resolved through its own (per-user) cache
+ path -- not stored on the dataframe's cache entry -- so that two viewers
+ of the same annotated chart share one dataframe cache hit while each
+ still gets their own annotation-security-scoped payload.
+ """
+ from superset.common.query_object import QueryObject
+
+ mock_query_context = MagicMock()
+ mock_query_context.force = False
+ mock_datasource = MagicMock()
+ mock_datasource.column_names = ["col1"]
+
+ processor = QueryContextProcessor(mock_query_context)
+ processor._qc_datasource = mock_datasource
+
+ query_obj = QueryObject(
+ datasource=mock_datasource,
+ columns=["col1"],
+ annotation_layers=[
+ {
+ "annotationType": "EVENT",
+ "sourceType": "NATIVE",
+ "name": "a",
+ "value": 1,
+ }
+ ],
+ )
+
+ class MockCache:
+ def __init__(self):
+ self.is_loaded = True
+ self.applied_filter_columns = ["col1"]
+ self.df = pd.DataFrame({"col1": [1, 2, 3]})
+ self.query = ""
+ self.status = "success"
+ self.cache_dttm = "2024-01-01T00:00:00"
+ self.queried_dttm = "2024-01-01T00:00:00"
+ self.stacktrace = None
+ self.error_message = None
+ self.is_cached = True
+ self.sql_rowcount = 0
+ self.cache_value = None
+ self.applied_template_filters = []
+ self.rejected_filter_columns = []
+ self.annotation_data = {"stale": "should not be used"}
+ self.bq_memory_limited = False
+ self.bq_memory_limited_row_count = 0
+ self.result_persisted = False
+ self.set_query_result = MagicMock()
+
+ mock_cache = MockCache()
+
+ with (
+ patch(
+ "superset.common.query_context_processor.QueryCacheManager"
+ ) as mock_cache_manager,
+ patch.object(query_obj, "validate", return_value=None),
+ patch.object(processor, "query_cache_key", return_value="df-key"),
+ patch.object(processor, "annotation_cache_key",
return_value="ann-key"),
+ patch.object(
+ processor, "_get_annotation_data_cached", return_value={"a": [1,
2]}
+ ) as mock_get_annotation,
+ patch.object(processor, "get_cache_timeout", return_value=3600),
+ ):
+ mock_cache_manager.get.return_value = mock_cache
+ result = processor.get_df_payload(query_obj, force_cached=False)
+
+ # The dataframe cache is a hit, so the (expensive) query is never re-run,
+ # and its cache entry is never rewritten.
+ mock_cache.set_query_result.assert_not_called()
+
+ # Annotation data is resolved through the separate, per-user cache path,
+ # keyed by the dedicated annotation cache key -- not the dataframe's key.
+ mock_get_annotation.assert_called_once()
+ _, kwargs = mock_get_annotation.call_args
+ assert kwargs["cache_key"] == "ann-key"
+
+ # The payload serves the freshly-resolved annotation data, not whatever
+ # (stale) value happened to sit on the dataframe's cache object.
+ assert result["annotation_data"] == {"a": [1, 2]}
+
+
+def test_get_df_payload_result_annotation_refresh_independent_of_df_marker():
Review Comment:
Good catch, added type hints on the mock and the nested helper.
--
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]
