sadpandajoe commented on code in PR #43303:
URL: https://github.com/apache/superset/pull/43303#discussion_r4207907520
##########
tests/integration_tests/charts/data/api_tests.py:
##########
@@ -1269,6 +1270,161 @@ def test_chart_data_get(self):
assert data["result"][0]["status"] == "success"
assert data["result"][0]["rowcount"] == 2
+ # ------------------------------------------------------------------ #
+ # #33615 — synthesized query_context read-path verification.
+ #
+ # These exercise the REAL read/authz/RLS path (get_data →
+ # ChartDataCommand.validate() → raise_for_access) on a chart whose
+ # query_context was produced by `build_query_context_config` — the same
+ # helper the importer runs (ADR-013/014). The verifier is NOT mocked
+ # (SECURITY-CRITICAL discipline). They are the api-side counterpart of the
+ # importer round-trip (F3-T1), baseline-equivalence (F3-T2), and security
+ # (F3-T3) tasks. The existing 68 tests in this module stand unmodified as
+ # the NFR-COMPAT-01 regression guard.
+ # ------------------------------------------------------------------ #
+
+ #: params equivalent to the "Genders" chart's saved viz form-data.
+ _GENDERS_PARAMS = {
+ "metrics": ["sum__num"],
+ "groupby": ["gender"],
+ "time_range": "1900-01-01T00:00:00 : 2000-01-01T00:00:00",
+ "granularity_sqla": "ds",
+ "row_limit": 50000,
+ "order_desc": True,
+ }
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_synthesized_query_context_returns_data(self):
Review Comment:
These four new read-path tests sit inside `TestGetChartDataApi`, which is
still decorated with the class-level `@pytest.mark.skip` (line 1212). None of
them run, even with a configured database, so a synthesized `query_context`
that the GET `/data/` endpoint rejects with a 400 would still pass the importer
persistence assertions. Could they move to a class or module that actually
executes, so the 200-with-rows and denies-unauthorized-datasource assertions
are exercised?
##########
superset-frontend/src/backend-querycontext/entry.ts:
##########
@@ -0,0 +1,78 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Backend query-context generation entry (Apache Superset #33615).
+ *
+ * The mapping `form_data -> query_context` lives in each viz plugin's JS
+ * `buildQuery` (there is no server-side equivalent), so a faithful backend
+ * synthesis must run the SAME code the frontend runs. This entry imports the
+ * plugins' pure `buildQuery` functions DIRECTLY (never the plugin index, which
+ * pulls React/DOM), builds a `viz_type -> buildQuery` registry, and exposes a
+ * single JSON-in/JSON-out `generateQueryContext` callable. It is bundled by
+ * `scripts/build-backend-querycontext.mjs` into a self-contained IIFE that a
+ * bare V8 (py_mini_racer, on the Python backend) evaluates and calls.
+ *
+ * COVERAGE: the `viz_type -> buildQuery` map is code-generated from the plugin
+ * registrations (MainPreset `.configure({ key })` joined to each package's
default
+ * `buildQuery`) by `scripts/gen-qc-registry.mjs` -> `registry.generated.ts`.
Re-run
+ * that script when plugins change. Viz types with no `buildQuery` (markup,
handlebars
+ * static, deck.gl static, cartodiagram, ...) are intentionally absent and
correctly
+ * non-derivable: the Python caller then falls back to the pure generic
derivation or
+ * leaves query_context NULL.
+ */
+
+// Generated viz_type -> buildQuery map (imports each plugin's DOM-free default
+// buildQuery by source path). Regenerate via `node
scripts/gen-qc-registry.mjs`.
+import { REGISTRY, VIZ_TYPES } from './registry.generated';
+
+export { VIZ_TYPES };
+
+/**
+ * @param vizType the chart's viz_type
+ * @param formDataJson JSON string of the chart's form_data (params)
+ * @returns JSON string: the query_context, OR `{__unsupported__:true,...}`
when
+ * the viz_type is not registered, OR `{__error__:"..."}` on failure.
+ * Never throws — the Python caller falls back on any sentinel.
+ */
+export function generateQueryContext(
+ vizType: string,
+ formDataJson: string,
+): string {
+ try {
+ const fn = REGISTRY[vizType];
+ if (!fn) {
+ return JSON.stringify({ __unsupported__: true, viz_type: vizType });
+ }
+ const formData = JSON.parse(formDataJson);
+ const queryContext = fn(formData);
Review Comment:
The stored params go straight into the plugin `buildQuery`, without the
defaults Explore applies to form data first. For a `box_plot` chart whose
params have metrics and dimensions but no `whiskerOptions`, `boxplotOperator`
returns `undefined`, so the persisted `query_context` has `post_processing: []`
(the committed `expected/box_plot.json` golden shows the same). The chart would
then return raw rows instead of box statistics on its first `/data/` read.
Should defaults be merged into the form data here before calling the builder?
--
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]
