mirror of
https://github.com/apache/superset.git
synced 2026-06-06 16:19:18 +00:00
perf(dashboard): skip thumbnail_url computing on single dashboard endpoint (#38015)
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
committed by
GitHub
parent
0b77ace110
commit
f5a5a804e2
@@ -41,12 +41,14 @@ function tryParsePayload(payload: Payload) {
|
||||
*/
|
||||
function getFullUrl(partialUrl: string, params: CallApi['searchParams']) {
|
||||
if (params) {
|
||||
const url = new URL(partialUrl, window.location.href);
|
||||
const search =
|
||||
params instanceof URLSearchParams ? params : new URLSearchParams(params);
|
||||
// will completely override any existing search params
|
||||
url.search = search.toString();
|
||||
return url.href;
|
||||
const searchString = search.toString();
|
||||
if (searchString) {
|
||||
const url = new URL(partialUrl, window.location.href);
|
||||
url.search = searchString;
|
||||
return url.href;
|
||||
}
|
||||
}
|
||||
return partialUrl;
|
||||
}
|
||||
|
||||
@@ -591,6 +591,21 @@ describe('callApi()', () => {
|
||||
);
|
||||
});
|
||||
|
||||
test('should preserve existing query params when searchParams is empty', async () => {
|
||||
window.location.href = 'http://localhost';
|
||||
fetchMock.get(`glob:*/get-search*`, { yes: 'ok' });
|
||||
const response = await callApi({
|
||||
url: '/get-search?q=existing',
|
||||
searchParams: {},
|
||||
method: 'GET',
|
||||
});
|
||||
const result = await response.json();
|
||||
expect(result).toEqual({ yes: 'ok' });
|
||||
expect(fetchMock.callHistory.lastCall()?.url).toEqual(
|
||||
`http://localhost/get-search?q=existing`,
|
||||
);
|
||||
});
|
||||
|
||||
test('should accept URLSearchParams', async () => {
|
||||
expect.assertions(2);
|
||||
window.location.href = 'http://localhost';
|
||||
|
||||
@@ -18,7 +18,28 @@
|
||||
*/
|
||||
import { renderHook } from '@testing-library/react-hooks';
|
||||
import fetchMock from 'fetch-mock';
|
||||
import { useDashboardDatasets } from './dashboards';
|
||||
import { useDashboard, useDashboardDatasets } from './dashboards';
|
||||
|
||||
test('useDashboard excludes thumbnail_url from request', async () => {
|
||||
fetchMock.get('glob:*/api/v1/dashboard/5?q=*', {
|
||||
result: {
|
||||
id: 5,
|
||||
dashboard_title: 'Test',
|
||||
json_metadata: '{}',
|
||||
position_json: '{}',
|
||||
owners: [],
|
||||
},
|
||||
});
|
||||
|
||||
const { waitForNextUpdate } = renderHook(() => useDashboard(5));
|
||||
await waitForNextUpdate();
|
||||
|
||||
const calledUrl = fetchMock.callHistory.lastCall()?.url ?? '';
|
||||
expect(calledUrl).toContain('?q=');
|
||||
expect(calledUrl).not.toContain('thumbnail_url');
|
||||
|
||||
fetchMock.clearHistory().removeRoutes();
|
||||
});
|
||||
|
||||
// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks
|
||||
describe('useDashboardDatasets', () => {
|
||||
|
||||
@@ -17,14 +17,42 @@
|
||||
* under the License.
|
||||
*/
|
||||
|
||||
import rison from 'rison';
|
||||
import { Dashboard, Datasource, EmbeddedDashboard } from 'src/dashboard/types';
|
||||
import { Chart } from 'src/types/Chart';
|
||||
import { Currency } from '@superset-ui/core';
|
||||
import { useApiV1Resource, useTransformedResource } from './apiResources';
|
||||
|
||||
export const useDashboard = (idOrSlug: string | number) =>
|
||||
useTransformedResource(
|
||||
useApiV1Resource<Dashboard>(`/api/v1/dashboard/${idOrSlug}`),
|
||||
const DASHBOARD_GET_COLUMNS = [
|
||||
'id',
|
||||
'slug',
|
||||
'url',
|
||||
'dashboard_title',
|
||||
'published',
|
||||
'css',
|
||||
'theme',
|
||||
'json_metadata',
|
||||
'position_json',
|
||||
'certified_by',
|
||||
'certification_details',
|
||||
'changed_by_name',
|
||||
'changed_by',
|
||||
'changed_on',
|
||||
'created_by',
|
||||
'charts',
|
||||
'owners',
|
||||
'roles',
|
||||
'tags',
|
||||
'changed_on_delta_humanized',
|
||||
'created_on_delta_humanized',
|
||||
'is_managed_externally',
|
||||
'uuid',
|
||||
];
|
||||
|
||||
export const useDashboard = (idOrSlug: string | number) => {
|
||||
const q = rison.encode({ columns: DASHBOARD_GET_COLUMNS });
|
||||
return useTransformedResource(
|
||||
useApiV1Resource<Dashboard>(`/api/v1/dashboard/${idOrSlug}?q=${q}`),
|
||||
dashboard => ({
|
||||
...dashboard,
|
||||
// TODO: load these at the API level
|
||||
@@ -35,6 +63,7 @@ export const useDashboard = (idOrSlug: string | number) =>
|
||||
owners: dashboard.owners || [],
|
||||
}),
|
||||
);
|
||||
};
|
||||
|
||||
// gets the chart definitions for a dashboard
|
||||
export const useDashboardCharts = (idOrSlug: string | number) =>
|
||||
|
||||
@@ -22,6 +22,7 @@ from io import BytesIO
|
||||
from typing import Any, Callable, cast
|
||||
from zipfile import is_zipfile, ZipFile
|
||||
|
||||
import prison
|
||||
from flask import current_app, g, redirect, request, Response, send_file, url_for
|
||||
from flask_appbuilder import permission_name
|
||||
from flask_appbuilder.api import expose, merge_response_func, protect, rison, safe
|
||||
@@ -467,6 +468,12 @@ class DashboardRestApi(CustomTagsOptimizationMixin, BaseSupersetModelRestApi):
|
||||
type: string
|
||||
name: id_or_slug
|
||||
description: Either the id of the dashboard, or its slug
|
||||
- in: query
|
||||
name: q
|
||||
content:
|
||||
application/json:
|
||||
schema:
|
||||
$ref: '#/components/schemas/get_item_schema'
|
||||
responses:
|
||||
200:
|
||||
description: Dashboard
|
||||
@@ -486,7 +493,26 @@ class DashboardRestApi(CustomTagsOptimizationMixin, BaseSupersetModelRestApi):
|
||||
404:
|
||||
$ref: '#/components/responses/404'
|
||||
"""
|
||||
result = self.dashboard_get_response_schema.dump(dash)
|
||||
columns: list[str] | None = None
|
||||
if q := request.args.get("q"):
|
||||
try:
|
||||
args = prison.loads(q)
|
||||
except Exception:
|
||||
return self.response_400(message="Invalid rison query parameter")
|
||||
if isinstance(args, dict):
|
||||
columns = args.get("columns")
|
||||
|
||||
if columns:
|
||||
schema_fields = self.dashboard_get_response_schema.fields
|
||||
key_to_name = {
|
||||
field.data_key or name: name for name, field in schema_fields.items()
|
||||
}
|
||||
only = [key_to_name[c] for c in columns if c in key_to_name]
|
||||
schema = DashboardGetResponseSchema(only=only)
|
||||
else:
|
||||
schema = self.dashboard_get_response_schema
|
||||
|
||||
result = schema.dump(dash)
|
||||
add_extra_log_payload(
|
||||
dashboard_id=dash.id, action=f"{self.__class__.__name__}.get"
|
||||
)
|
||||
|
||||
@@ -569,6 +569,87 @@ class TestDashboardApi(ApiOwnersTestCaseMixin, InsertChartMixin, SupersetTestCas
|
||||
db.session.delete(dashboard)
|
||||
db.session.commit()
|
||||
|
||||
def test_get_dashboard_with_columns(self):
|
||||
"""
|
||||
Dashboard API: Test get dashboard with column selection via q param
|
||||
"""
|
||||
admin = self.get_user("admin")
|
||||
dashboard = self.insert_dashboard(
|
||||
"title", "slug1", [admin.id], created_by=admin
|
||||
)
|
||||
self.login(ADMIN_USERNAME)
|
||||
params = prison.dumps({"columns": ["id", "dashboard_title"]})
|
||||
uri = f"api/v1/dashboard/{dashboard.id}?q={params}"
|
||||
rv = self.get_assert_metric(uri, "get")
|
||||
assert rv.status_code == 200
|
||||
data = json.loads(rv.data.decode("utf-8"))
|
||||
result = data["result"]
|
||||
assert result["id"] == dashboard.id
|
||||
assert result["dashboard_title"] == "title"
|
||||
assert "thumbnail_url" not in result
|
||||
assert "slug" not in result
|
||||
assert "owners" not in result
|
||||
# rollback changes
|
||||
db.session.delete(dashboard)
|
||||
db.session.commit()
|
||||
|
||||
def test_get_dashboard_with_invalid_rison_q(self):
|
||||
"""
|
||||
Dashboard API: Test get dashboard with malformed rison returns 400
|
||||
"""
|
||||
admin = self.get_user("admin")
|
||||
dashboard = self.insert_dashboard(
|
||||
"title", "slug1", [admin.id], created_by=admin
|
||||
)
|
||||
self.login(ADMIN_USERNAME)
|
||||
uri = f"api/v1/dashboard/{dashboard.id}?q=(("
|
||||
rv = self.get_assert_metric(uri, "get")
|
||||
assert rv.status_code == 400
|
||||
# rollback changes
|
||||
db.session.delete(dashboard)
|
||||
db.session.commit()
|
||||
|
||||
def test_get_dashboard_with_non_dict_q(self):
|
||||
"""
|
||||
Dashboard API: Test get dashboard with non-dict rison returns full response
|
||||
"""
|
||||
admin = self.get_user("admin")
|
||||
dashboard = self.insert_dashboard(
|
||||
"title", "slug1", [admin.id], created_by=admin
|
||||
)
|
||||
self.login(ADMIN_USERNAME)
|
||||
uri = f"api/v1/dashboard/{dashboard.id}?q=a_string"
|
||||
rv = self.get_assert_metric(uri, "get")
|
||||
assert rv.status_code == 200
|
||||
data = json.loads(rv.data.decode("utf-8"))
|
||||
# non-dict q is ignored, full response returned
|
||||
assert "thumbnail_url" in data["result"]
|
||||
# rollback changes
|
||||
db.session.delete(dashboard)
|
||||
db.session.commit()
|
||||
|
||||
def test_get_dashboard_with_columns_data_key_mapping(self):
|
||||
"""
|
||||
Dashboard API: Test that data_key columns like changed_on_delta_humanized work
|
||||
"""
|
||||
admin = self.get_user("admin")
|
||||
dashboard = self.insert_dashboard(
|
||||
"title", "slug1", [admin.id], created_by=admin
|
||||
)
|
||||
self.login(ADMIN_USERNAME)
|
||||
params = prison.dumps({"columns": ["id", "changed_on_delta_humanized"]})
|
||||
uri = f"api/v1/dashboard/{dashboard.id}?q={params}"
|
||||
rv = self.get_assert_metric(uri, "get")
|
||||
assert rv.status_code == 200
|
||||
data = json.loads(rv.data.decode("utf-8"))
|
||||
result = data["result"]
|
||||
assert "id" in result
|
||||
assert "changed_on_delta_humanized" in result
|
||||
assert "dashboard_title" not in result
|
||||
# rollback changes
|
||||
db.session.delete(dashboard)
|
||||
db.session.commit()
|
||||
|
||||
@patch("superset.dashboards.schemas.security_manager.has_guest_access")
|
||||
@patch("superset.dashboards.schemas.security_manager.is_guest_user")
|
||||
def test_get_dashboard_as_guest(self, is_guest_user, has_guest_access):
|
||||
|
||||
99
tests/unit_tests/dashboards/api_test.py
Normal file
99
tests/unit_tests/dashboards/api_test.py
Normal file
@@ -0,0 +1,99 @@
|
||||
# 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.
|
||||
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from superset.dashboards.schemas import DashboardGetResponseSchema
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def mock_dashboard() -> MagicMock:
|
||||
dash = MagicMock()
|
||||
dash.id = 1
|
||||
dash.slug = "test-slug"
|
||||
dash.url = "/superset/dashboard/test-slug/"
|
||||
dash.dashboard_title = "Test Dashboard"
|
||||
dash.thumbnail_url = "http://example.com/thumb.png"
|
||||
dash.published = True
|
||||
dash.css = ""
|
||||
dash.theme = None
|
||||
dash.json_metadata = "{}"
|
||||
dash.position_json = "{}"
|
||||
dash.certified_by = None
|
||||
dash.certification_details = None
|
||||
dash.changed_by_name = "admin"
|
||||
dash.changed_by = MagicMock(id=1, first_name="admin", last_name="user")
|
||||
dash.changed_on = None
|
||||
dash.changed_on_humanized = "2 days ago"
|
||||
dash.created_by = MagicMock(id=1, first_name="admin", last_name="user")
|
||||
dash.created_on_humanized = "5 days ago"
|
||||
dash.charts = []
|
||||
dash.owners = []
|
||||
dash.roles = []
|
||||
dash.tags = []
|
||||
dash.custom_tags = []
|
||||
dash.is_managed_externally = False
|
||||
dash.uuid = None
|
||||
return dash
|
||||
|
||||
|
||||
def test_schema_column_selection_excludes_thumbnail(
|
||||
mock_dashboard: MagicMock,
|
||||
) -> None:
|
||||
schema = DashboardGetResponseSchema(only=["id", "dashboard_title"])
|
||||
result = schema.dump(mock_dashboard)
|
||||
assert "id" in result
|
||||
assert "dashboard_title" in result
|
||||
assert "thumbnail_url" not in result
|
||||
assert "slug" not in result
|
||||
|
||||
|
||||
def test_schema_column_selection_with_data_key(
|
||||
mock_dashboard: MagicMock,
|
||||
) -> None:
|
||||
"""Fields with data_key should work when using the internal field name."""
|
||||
schema = DashboardGetResponseSchema(only=["id", "changed_on_humanized"])
|
||||
result = schema.dump(mock_dashboard)
|
||||
assert "id" in result
|
||||
assert "changed_on_delta_humanized" in result
|
||||
assert "dashboard_title" not in result
|
||||
|
||||
|
||||
def test_schema_full_response_includes_thumbnail(
|
||||
mock_dashboard: MagicMock,
|
||||
) -> None:
|
||||
schema = DashboardGetResponseSchema()
|
||||
result = schema.dump(mock_dashboard)
|
||||
assert "thumbnail_url" in result
|
||||
assert "id" in result
|
||||
assert "dashboard_title" in result
|
||||
|
||||
|
||||
def test_data_key_mapping_logic() -> None:
|
||||
"""The key_to_name mapping used in the API correctly maps data_key to field name."""
|
||||
schema = DashboardGetResponseSchema()
|
||||
key_to_name = {
|
||||
field.data_key or name: name for name, field in schema.fields.items()
|
||||
}
|
||||
# changed_on_delta_humanized is the data_key for changed_on_humanized
|
||||
assert key_to_name["changed_on_delta_humanized"] == "changed_on_humanized"
|
||||
assert key_to_name["created_on_delta_humanized"] == "created_on_humanized"
|
||||
# fields without data_key map to themselves
|
||||
assert key_to_name["id"] == "id"
|
||||
assert key_to_name["thumbnail_url"] == "thumbnail_url"
|
||||
Reference in New Issue
Block a user