From 2874096e2724938e46cd86f3ae386f385379cddf Mon Sep 17 00:00:00 2001 From: "JUST.in DO IT" Date: Fri, 17 Jan 2025 16:16:02 -0800 Subject: [PATCH] refactor(sqllab): migrate share queries via kv by permalink (#29163) --- RESOURCES/FEATURE_FLAGS.md | 1 + UPDATING.md | 4 +- .../src/utils/featureFlags.ts | 1 - .../src/SqlLab/actions/sqlLab.js | 24 ++- .../ShareSqlLabQuery.test.tsx | 76 +++------ .../components/ShareSqlLabQuery/index.tsx | 82 +++------- .../TabbedSqlEditors.test.tsx | 48 +++++- .../components/TabbedSqlEditors/index.tsx | 10 +- .../commands/sql_lab/permalink/__init__.py | 16 ++ superset/commands/sql_lab/permalink/base.py | 35 +++++ superset/commands/sql_lab/permalink/create.py | 49 ++++++ superset/commands/sql_lab/permalink/get.py | 71 +++++++++ superset/config.py | 2 - superset/initialization/__init__.py | 4 +- superset/key_value/types.py | 2 + superset/sqllab/permalink/__init__.py | 16 ++ superset/sqllab/permalink/api.py | 144 ++++++++++++++++++ superset/sqllab/permalink/exceptions.py | 31 ++++ superset/sqllab/permalink/schemas.py | 46 ++++++ superset/sqllab/permalink/types.py | 26 ++++ superset/views/key_value.py | 67 -------- superset/views/sqllab.py | 4 +- tests/integration_tests/core_tests.py | 30 ---- .../sql_lab/permalink/__init__.py | 16 ++ .../sql_lab/permalink/api_tests.py | 102 +++++++++++++ .../integration_tests/superset_test_config.py | 2 - .../superset_test_config_thumbnails.py | 2 - 27 files changed, 682 insertions(+), 229 deletions(-) create mode 100644 superset/commands/sql_lab/permalink/__init__.py create mode 100644 superset/commands/sql_lab/permalink/base.py create mode 100644 superset/commands/sql_lab/permalink/create.py create mode 100644 superset/commands/sql_lab/permalink/get.py create mode 100644 superset/sqllab/permalink/__init__.py create mode 100644 superset/sqllab/permalink/api.py create mode 100644 superset/sqllab/permalink/exceptions.py create mode 100644 superset/sqllab/permalink/schemas.py create mode 100644 superset/sqllab/permalink/types.py delete mode 100644 superset/views/key_value.py create mode 100644 tests/integration_tests/sql_lab/permalink/__init__.py create mode 100644 tests/integration_tests/sql_lab/permalink/api_tests.py diff --git a/RESOURCES/FEATURE_FLAGS.md b/RESOURCES/FEATURE_FLAGS.md index a09fb00b556..b5ab3f6d6fd 100644 --- a/RESOURCES/FEATURE_FLAGS.md +++ b/RESOURCES/FEATURE_FLAGS.md @@ -62,6 +62,7 @@ These features flags are **safe for production**. They have been tested and will [//]: # "PLEASE KEEP THESE LISTS SORTED ALPHABETICALLY" ### Flags on the path to feature launch and flag deprecation/removal + - DASHBOARD_VIRTUALIZATION - DRILL_BY - DISABLE_LEGACY_DATASOURCE_EDITOR diff --git a/UPDATING.md b/UPDATING.md index eca1acea29a..eb2b6714af0 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -23,6 +23,7 @@ This file documents any backwards-incompatible changes in Superset and assists people when migrating to a new version. ## Next + - [31894](https://github.com/apache/superset/pull/31894) Domain sharding is deprecated in favor of HTTP2. The `SUPERSET_WEBSERVER_DOMAINS` configuration will be removed in the next major version (6.0) - [31774](https://github.com/apache/superset/pull/31774): Fixes the spelling of the `USE-ANALAGOUS-COLORS` feature flag. Please update any scripts/configuration item to use the new/corrected `USE-ANALOGOUS-COLORS` flag spelling. - [31582](https://github.com/apache/superset/pull/31582) Removed the legacy Area, Bar, Event Flow, Heatmap, Histogram, Line, Sankey, and Sankey Loop charts. They were all automatically migrated to their ECharts counterparts with the exception of the Event Flow and Sankey Loop charts which were removed as they were not actively maintained and not widely used. If you were using the Event Flow or Sankey Loop charts, you will need to find an alternative solution. @@ -33,8 +34,9 @@ assists people when migrating to a new version. - [31262](https://github.com/apache/superset/pull/31262) NOTE: deprecated `pylint` in favor of `ruff` as our only python linter. Only affect development workflows positively (not the release itself). It should cover most important rules, be much faster, but some things linting rules that were enforced before may not be enforce in the exact same way as before. - [31173](https://github.com/apache/superset/pull/31173) Modified `fetch_csrf_token` to align with HTTP standards, particularly regarding how cookies are handled. If you encounter any issues related to CSRF functionality, please report them as a new issue and reference this PR for context. - [31385](https://github.com/apache/superset/pull/31385) Significant docker refactor, reducing access levels for the `superset` user, streamlining layer building, ... -- [31503](https://github.com/apache/superset/pull/31503) Deprecating python 3.9.x support, 3.11 is now the recommended version and 3.10 is still supported over the Superset 5.0 lifecycle. +- [31503](https://github.com/apache/superset/pull/31503) Deprecating python 3.9.x support, 3.11 is now the recommended version and 3.10 is still supported over the Superset 5.0 lifecycle. - [29121](https://github.com/apache/superset/pull/29121) Removed the `css`, `position_json`, and `json_metadata` from the payload of the dashboard list endpoint (`GET api/v1/dashboard`) for performance reasons. +- [29163](https://github.com/apache/superset/pull/29163) Removed the `SHARE_QUERIES_VIA_KV_STORE` and `KV_STORE` feature flags and changed the way Superset shares SQL Lab queries to use permalinks. The legacy `/kv` API was removed but we still support legacy links in 5.0. In 6.0, only permalinks will be supported. ### Potential Downtime diff --git a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts b/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts index 3bf3c0c6e39..34e8d51f27f 100644 --- a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts +++ b/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts @@ -52,7 +52,6 @@ export enum FeatureFlag { HorizontalFilterBar = 'HORIZONTAL_FILTER_BAR', ListviewsDefaultCardView = 'LISTVIEWS_DEFAULT_CARD_VIEW', ScheduledQueries = 'SCHEDULED_QUERIES', - ShareQueriesViaKvStore = 'SHARE_QUERIES_VIA_KV_STORE', SqllabBackendPersistence = 'SQLLAB_BACKEND_PERSISTENCE', SqlValidatorsByEngine = 'SQL_VALIDATORS_BY_ENGINE', SshTunneling = 'SSH_TUNNELING', diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index 107ff660ac9..d8ec7a19a56 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -1167,9 +1167,31 @@ export function persistEditorHeight(queryEditor, northPercent, southPercent) { }; } +export function popPermalink(key) { + return function (dispatch) { + return SupersetClient.get({ endpoint: `/api/v1/sqllab/permalink/${key}` }) + .then(({ json }) => + dispatch( + addQueryEditor({ + name: json.name ? json.name : t('Shared query'), + dbId: json.dbId ? parseInt(json.dbId, 10) : null, + catalog: json.catalog ? json.catalog : null, + schema: json.schema ? json.schema : null, + autorun: json.autorun ? json.autorun : false, + sql: json.sql ? json.sql : 'SELECT ...', + templateParams: json.templateParams, + }), + ), + ) + .catch(() => dispatch(addDangerToast(ERR_MSG_CANT_LOAD_QUERY))); + }; +} + export function popStoredQuery(urlId) { return function (dispatch) { - return SupersetClient.get({ endpoint: `/kv/${urlId}` }) + return SupersetClient.get({ + endpoint: `/api/v1/sqllab/permalink/kv:${urlId}`, + }) .then(({ json }) => dispatch( addQueryEditor({ diff --git a/superset-frontend/src/SqlLab/components/ShareSqlLabQuery/ShareSqlLabQuery.test.tsx b/superset-frontend/src/SqlLab/components/ShareSqlLabQuery/ShareSqlLabQuery.test.tsx index d522d73d902..ae3c7d56130 100644 --- a/superset-frontend/src/SqlLab/components/ShareSqlLabQuery/ShareSqlLabQuery.test.tsx +++ b/superset-frontend/src/SqlLab/components/ShareSqlLabQuery/ShareSqlLabQuery.test.tsx @@ -23,10 +23,9 @@ import fetchMock from 'fetch-mock'; import * as uiCore from '@superset-ui/core'; import { Provider } from 'react-redux'; import { supersetTheme, ThemeProvider } from '@superset-ui/core'; -import { render, screen, act } from '@testing-library/react'; +import { render, screen, act, waitFor } from '@testing-library/react'; import '@testing-library/jest-dom'; import userEvent from '@testing-library/user-event'; -import * as utils from 'src/utils/common'; import ShareSqlLabQuery from 'src/SqlLab/components/ShareSqlLabQuery'; import { initialState } from 'src/SqlLab/fixtures'; @@ -92,20 +91,24 @@ const standardProviderWithUnsaved: FC = ({ children }) => ( ); describe('ShareSqlLabQuery', () => { - const storeQueryUrl = 'glob:*/kv/store/'; - const storeQueryMockId = '123'; + const storeQueryUrl = 'glob:*/api/v1/sqllab/permalink'; + const storeQueryMockId = 'ci39c3'; beforeEach(async () => { - fetchMock.post(storeQueryUrl, () => ({ id: storeQueryMockId }), { - overwriteRoutes: true, - }); + fetchMock.post( + storeQueryUrl, + () => ({ key: storeQueryMockId, url: `/p/${storeQueryMockId}` }), + { + overwriteRoutes: true, + }, + ); fetchMock.resetHistory(); jest.clearAllMocks(); }); afterAll(fetchMock.reset); - describe('via /kv/store', () => { + describe('via permalink api', () => { beforeAll(() => { isFeatureEnabledMock = jest .spyOn(uiCore, 'isFeatureEnabled') @@ -124,11 +127,13 @@ describe('ShareSqlLabQuery', () => { }); const button = screen.getByRole('button'); const { id, remoteId, ...expected } = mockQueryEditor; - const storeQuerySpy = jest.spyOn(utils, 'storeQuery'); userEvent.click(button); - expect(storeQuerySpy.mock.calls).toHaveLength(1); - expect(storeQuerySpy).toHaveBeenCalledWith(expected); - storeQuerySpy.mockRestore(); + await waitFor(() => + expect(fetchMock.calls(storeQueryUrl)).toHaveLength(1), + ); + expect( + JSON.parse(fetchMock.calls(storeQueryUrl)[0][1]?.body as string), + ).toEqual(expected); }); it('calls storeQuery() with unsaved changes', async () => { @@ -139,48 +144,13 @@ describe('ShareSqlLabQuery', () => { }); const button = screen.getByRole('button'); const { id, ...expected } = unsavedQueryEditor; - const storeQuerySpy = jest.spyOn(utils, 'storeQuery'); userEvent.click(button); - expect(storeQuerySpy.mock.calls).toHaveLength(1); - expect(storeQuerySpy).toHaveBeenCalledWith(expected); - storeQuerySpy.mockRestore(); - }); - }); - - describe('via saved query', () => { - beforeAll(() => { - isFeatureEnabledMock = jest - .spyOn(uiCore, 'isFeatureEnabled') - .mockImplementation(() => false); - }); - - afterAll(() => { - isFeatureEnabledMock.mockReset(); - }); - - it('does not call storeQuery() with the query when getCopyUrl() is called and feature is not enabled', async () => { - await act(async () => { - render(, { - wrapper: standardProvider, - }); - }); - const storeQuerySpy = jest.spyOn(utils, 'storeQuery'); - const button = screen.getByRole('button'); - userEvent.click(button); - expect(storeQuerySpy.mock.calls).toHaveLength(0); - storeQuerySpy.mockRestore(); - }); - - it('button is disabled and there is a request to save the query', async () => { - const updatedProps = { - queryEditorId: disabled.id, - }; - - render(, { - wrapper: standardProvider, - }); - const button = await screen.findByRole('button', { name: /copy link/i }); - expect(button).toBeDisabled(); + await waitFor(() => + expect(fetchMock.calls(storeQueryUrl)).toHaveLength(1), + ); + expect( + JSON.parse(fetchMock.calls(storeQueryUrl)[0][1]?.body as string), + ).toEqual(expected); }); }); }); diff --git a/superset-frontend/src/SqlLab/components/ShareSqlLabQuery/index.tsx b/superset-frontend/src/SqlLab/components/ShareSqlLabQuery/index.tsx index c1a45eccadf..988b96b51bd 100644 --- a/superset-frontend/src/SqlLab/components/ShareSqlLabQuery/index.tsx +++ b/superset-frontend/src/SqlLab/components/ShareSqlLabQuery/index.tsx @@ -17,18 +17,16 @@ * under the License. */ import { - FeatureFlag, styled, t, useTheme, - isFeatureEnabled, getClientErrorObject, + SupersetClient, } from '@superset-ui/core'; import Button from 'src/components/Button'; import Icons from 'src/components/Icons'; import withToasts from 'src/components/MessageToasts/withToasts'; import CopyToClipboard from 'src/components/CopyToClipboard'; -import { storeQuery } from 'src/utils/common'; import useQueryEditor from 'src/SqlLab/hooks/useQueryEditor'; import { LOG_ACTIONS_SQLLAB_COPY_LINK } from 'src/logger/LogUtils'; import useLogAction from 'src/logger/useLogAction'; @@ -54,23 +52,21 @@ const ShareSqlLabQuery = ({ }: ShareSqlLabQueryProps) => { const theme = useTheme(); const logAction = useLogAction({ queryEditorId }); - const { dbId, name, schema, autorun, sql, remoteId, templateParams } = - useQueryEditor(queryEditorId, [ - 'dbId', - 'name', - 'schema', - 'autorun', - 'sql', - 'remoteId', - 'templateParams', - ]); + const { dbId, name, schema, autorun, sql, templateParams } = useQueryEditor( + queryEditorId, + ['dbId', 'name', 'schema', 'autorun', 'sql', 'templateParams'], + ); - const getCopyUrlForKvStore = (callback: Function) => { + const getCopyUrlForPermalink = (callback: Function) => { const sharedQuery = { dbId, name, schema, autorun, sql, templateParams }; - return storeQuery(sharedQuery) - .then(shortUrl => { - callback(shortUrl); + return SupersetClient.post({ + endpoint: '/api/v1/sqllab/permalink', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(sharedQuery), + }) + .then(({ json }) => { + callback(json.url); }) .catch(response => { getClientErrorObject(response).then(() => { @@ -79,61 +75,29 @@ const ShareSqlLabQuery = ({ }); }; - const getCopyUrlForSavedQuery = (callback: Function) => { - let savedQueryToastContent; - - if (remoteId) { - savedQueryToastContent = `${ - window.location.origin + window.location.pathname - }?savedQueryId=${remoteId}`; - callback(savedQueryToastContent); - } else { - savedQueryToastContent = t('Please save the query to enable sharing'); - callback(savedQueryToastContent); - } - }; const getCopyUrl = (callback: Function) => { logAction(LOG_ACTIONS_SQLLAB_COPY_LINK, { shortcut: false, }); - if (isFeatureEnabled(FeatureFlag.ShareQueriesViaKvStore)) { - return getCopyUrlForKvStore(callback); - } - return getCopyUrlForSavedQuery(callback); + return getCopyUrlForPermalink(callback); }; - const buildButton = (canShare: boolean) => { - const tooltip = canShare - ? t('Copy query link to your clipboard') - : t('Save the query to enable this feature'); + const buildButton = () => { + const tooltip = t('Copy query link to your clipboard'); return ( - ); }; - const canShare = - !!remoteId || isFeatureEnabled(FeatureFlag.ShareQueriesViaKvStore); - return ( - <> - {canShare ? ( - - ) : ( - buildButton(canShare) - )} - + ); }; diff --git a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/TabbedSqlEditors.test.tsx b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/TabbedSqlEditors.test.tsx index f1468fe05a7..09619abd594 100644 --- a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/TabbedSqlEditors.test.tsx +++ b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/TabbedSqlEditors.test.tsx @@ -28,10 +28,6 @@ import { Store } from 'redux'; import { RootState } from 'src/views/store'; import { SET_ACTIVE_QUERY_EDITOR } from 'src/SqlLab/actions/sqlLab'; -fetchMock.get('glob:*/api/v1/database/*', {}); -fetchMock.get('glob:*/api/v1/saved_query/*', {}); -fetchMock.get('glob:*/kv/*', {}); - jest.mock('src/SqlLab/components/SqlEditor', () => () => (
)); @@ -46,11 +42,20 @@ const setup = (overridesStore?: Store, initialState?: RootState) => initialState, ...(overridesStore && { store: overridesStore }), }); +let pathStub = jest.spyOn(URI.prototype, 'path'); beforeEach(() => { + fetchMock.get('glob:*/api/v1/database/*', {}); + fetchMock.get('glob:*/api/v1/saved_query/*', {}); + pathStub = jest.spyOn(URI.prototype, 'path').mockReturnValue(`/sqllab/`); store.clearActions(); }); +afterEach(() => { + fetchMock.reset(); + pathStub.mockReset(); +}); + describe('componentDidMount', () => { let uriStub = jest.spyOn(URI.prototype, 'search'); let replaceState = jest.spyOn(window.history, 'replaceState'); @@ -62,7 +67,13 @@ describe('componentDidMount', () => { replaceState.mockReset(); uriStub.mockReset(); }); - test('should handle id', () => { + test('should handle id', async () => { + const id = 1; + fetchMock.get(`glob:*/api/v1/sqllab/permalink/kv:${id}`, { + label: 'test permalink', + sql: 'SELECT * FROM test_table', + dbId: 1, + }); uriStub.mockReturnValue({ id: 1 }); setup(store); expect(replaceState).toHaveBeenCalledWith( @@ -70,6 +81,33 @@ describe('componentDidMount', () => { expect.anything(), '/sqllab', ); + await waitFor(() => + expect( + fetchMock.calls(`glob:*/api/v1/sqllab/permalink/kv:${id}`), + ).toHaveLength(1), + ); + fetchMock.reset(); + }); + test('should handle permalink', async () => { + const key = '9sadkfl'; + fetchMock.get(`glob:*/api/v1/sqllab/permalink/${key}`, { + label: 'test permalink', + sql: 'SELECT * FROM test_table', + dbId: 1, + }); + pathStub.mockReturnValue(`/sqllab/p/${key}`); + setup(store); + await waitFor(() => + expect( + fetchMock.calls(`glob:*/api/v1/sqllab/permalink/${key}`), + ).toHaveLength(1), + ); + expect(replaceState).toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + '/sqllab', + ); + fetchMock.reset(); }); test('should handle savedQueryId', () => { uriStub.mockReturnValue({ savedQueryId: 1 }); diff --git a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx index df3099994ec..3105aa94bda 100644 --- a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx +++ b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx @@ -77,6 +77,7 @@ class TabbedSqlEditors extends PureComponent { // the reducer. const bootstrapData = getBootstrapData(); const queryParameters = URI(window.location).search(true); + const path = URI(window.location).path(); const { id, name, @@ -96,10 +97,13 @@ class TabbedSqlEditors extends PureComponent { ...bootstrapData.requested_query, ...queryParameters, } as Record; + const permalink = path.match(/\/p\/\w+/)?.[0].slice(3); // Popping a new tab based on the querystring - if (id || sql || savedQueryId || datasourceKey || queryId) { - if (id) { + if (permalink || id || sql || savedQueryId || datasourceKey || queryId) { + if (permalink) { + this.props.actions.popPermalink(permalink); + } else if (id) { this.props.actions.popStoredQuery(id); } else if (savedQueryId) { this.props.actions.popSavedQuery(savedQueryId); @@ -132,7 +136,7 @@ class TabbedSqlEditors extends PureComponent { }; this.props.actions.addQueryEditor(newQueryEditor); } - this.popNewTab(pick(urlParams, Object.keys(queryParameters))); + this.popNewTab(pick(urlParams, Object.keys(queryParameters ?? {}))); } else if (isNewQuery || this.props.queryEditors.length === 0) { this.newQueryEditor(); diff --git a/superset/commands/sql_lab/permalink/__init__.py b/superset/commands/sql_lab/permalink/__init__.py new file mode 100644 index 00000000000..13a83393a91 --- /dev/null +++ b/superset/commands/sql_lab/permalink/__init__.py @@ -0,0 +1,16 @@ +# 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. diff --git a/superset/commands/sql_lab/permalink/base.py b/superset/commands/sql_lab/permalink/base.py new file mode 100644 index 00000000000..1619ddb91de --- /dev/null +++ b/superset/commands/sql_lab/permalink/base.py @@ -0,0 +1,35 @@ +# 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 abc import ABC + +from superset.commands.base import BaseCommand +from superset.key_value.shared_entries import get_permalink_salt +from superset.key_value.types import ( + KeyValueResource, + MarshmallowKeyValueCodec, + SharedKey, +) +from superset.sqllab.permalink.schemas import SqlLabPermalinkSchema + + +class BaseSqlLabPermalinkCommand(BaseCommand, ABC): + resource: KeyValueResource = KeyValueResource.SQLLAB_PERMALINK + codec = MarshmallowKeyValueCodec(SqlLabPermalinkSchema()) + + @property + def salt(self) -> str: + return get_permalink_salt(SharedKey.SQLLAB_PERMALINK_SALT) diff --git a/superset/commands/sql_lab/permalink/create.py b/superset/commands/sql_lab/permalink/create.py new file mode 100644 index 00000000000..150b069d6f7 --- /dev/null +++ b/superset/commands/sql_lab/permalink/create.py @@ -0,0 +1,49 @@ +# 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. +import logging +from typing import Any + +from superset import db +from superset.commands.sql_lab.permalink.base import BaseSqlLabPermalinkCommand +from superset.daos.key_value import KeyValueDAO +from superset.key_value.exceptions import KeyValueCodecEncodeException +from superset.key_value.utils import encode_permalink_key +from superset.sqllab.permalink.exceptions import SqlLabPermalinkCreateFailedError + +logger = logging.getLogger(__name__) + + +class CreateSqlLabPermalinkCommand(BaseSqlLabPermalinkCommand): + def __init__(self, state: dict[str, Any]): + self._properties = state.copy() + + def run(self) -> str: + self.validate() + try: + entry = KeyValueDAO.create_entry( + self.resource, self._properties, self.codec + ) + db.session.flush() + key = entry.id + if key is None: + raise SqlLabPermalinkCreateFailedError("Unexpected missing key id") + return encode_permalink_key(key=key, salt=self.salt) + except KeyValueCodecEncodeException as ex: + raise SqlLabPermalinkCreateFailedError(str(ex)) from ex + + def validate(self) -> None: + pass diff --git a/superset/commands/sql_lab/permalink/get.py b/superset/commands/sql_lab/permalink/get.py new file mode 100644 index 00000000000..b6786e0e4d5 --- /dev/null +++ b/superset/commands/sql_lab/permalink/get.py @@ -0,0 +1,71 @@ +# 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. +import logging +from typing import Optional + +from superset import db +from superset.commands.dataset.exceptions import DatasetNotFoundError +from superset.commands.sql_lab.permalink.base import BaseSqlLabPermalinkCommand +from superset.daos.key_value import KeyValueDAO +from superset.key_value.exceptions import ( + KeyValueCodecDecodeException, + KeyValueGetFailedError, + KeyValueParseKeyError, +) +from superset.key_value.utils import decode_permalink_id +from superset.models import core as models +from superset.sqllab.permalink.exceptions import SqlLabPermalinkGetFailedError +from superset.sqllab.permalink.types import SqlLabPermalinkValue +from superset.utils import core as utils, json + +logger = logging.getLogger(__name__) + + +class GetSqlLabPermalinkCommand(BaseSqlLabPermalinkCommand): + def __init__(self, key: str): + self.key = key + + def run(self) -> Optional[SqlLabPermalinkValue]: + self.validate() + if self.key.startswith("kv:"): + id = int(self.key[3]) + try: + kv = db.session.query(models.KeyValue).filter_by(id=id).scalar() + if not kv: + return None + return json.loads(kv.value) + except Exception as ex: + raise SqlLabPermalinkGetFailedError( + message=utils.error_msg_from_exception(ex) + ) from ex + + try: + key = decode_permalink_id(self.key, salt=self.salt) + value = KeyValueDAO.get_value(self.resource, key, self.codec) + if value: + return value + return None + except ( + DatasetNotFoundError, + KeyValueCodecDecodeException, + KeyValueGetFailedError, + KeyValueParseKeyError, + ) as ex: + raise SqlLabPermalinkGetFailedError(message=ex.message) from ex + + def validate(self) -> None: + pass diff --git a/superset/config.py b/superset/config.py index d7b54449fc6..3c6d58d839d 100644 --- a/superset/config.py +++ b/superset/config.py @@ -472,7 +472,6 @@ DEFAULT_FEATURE_FLAGS: dict[str, bool] = { # geospatial ones) by inputting javascript in controls. This exposes # an XSS security vulnerability "ENABLE_JAVASCRIPT_CONTROLS": False, # deprecated - "KV_STORE": False, # deprecated # When this feature is enabled, nested types in Presto will be # expanded into extra columns and/or arrays. This is experimental, # and doesn't work with all nested types. @@ -487,7 +486,6 @@ DEFAULT_FEATURE_FLAGS: dict[str, bool] = { # This feature flag is used by the download feature in the dashboard view. # It is dependent on ENABLE_DASHBOARD_SCREENSHOT_ENDPOINT being enabled. "ENABLE_DASHBOARD_DOWNLOAD_WEBDRIVER_SCREENSHOT": False, - "SHARE_QUERIES_VIA_KV_STORE": False, "TAGGING_SYSTEM": False, "SQLLAB_BACKEND_PERSISTENCE": True, "LISTVIEWS_DEFAULT_CARD_VIEW": False, diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index 10a5e50898a..a57325c3590 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -157,6 +157,7 @@ class SupersetAppInitializer: # pylint: disable=too-many-public-methods from superset.row_level_security.api import RLSRestApi from superset.security.api import SecurityRestApi from superset.sqllab.api import SqlLabRestApi + from superset.sqllab.permalink.api import SqlLabPermalinkRestApi from superset.tags.api import TagRestApi from superset.views.alerts import AlertView, ReportView from superset.views.all_entities import TaggedObjectsModelView @@ -178,7 +179,6 @@ class SupersetAppInitializer: # pylint: disable=too-many-public-methods from superset.views.dynamic_plugins import DynamicPluginsView from superset.views.error_handling import set_app_error_handlers from superset.views.explore import ExplorePermalinkView, ExploreView - from superset.views.key_value import KV from superset.views.log.api import LogRestApi from superset.views.log.views import LogModelView from superset.views.sql_lab.views import ( @@ -227,6 +227,7 @@ class SupersetAppInitializer: # pylint: disable=too-many-public-methods appbuilder.add_api(SavedQueryRestApi) appbuilder.add_api(TagRestApi) appbuilder.add_api(SqlLabRestApi) + appbuilder.add_api(SqlLabPermalinkRestApi) # # Setup regular views # @@ -304,7 +305,6 @@ class SupersetAppInitializer: # pylint: disable=too-many-public-methods appbuilder.add_view_no_menu(EmbeddedView) appbuilder.add_view_no_menu(ExploreView) appbuilder.add_view_no_menu(ExplorePermalinkView) - appbuilder.add_view_no_menu(KV) appbuilder.add_view_no_menu(SavedQueryView) appbuilder.add_view_no_menu(SavedQueryViewApi) appbuilder.add_view_no_menu(SliceAsync) diff --git a/superset/key_value/types.py b/superset/key_value/types.py index 348697234f8..3b2da06493c 100644 --- a/superset/key_value/types.py +++ b/superset/key_value/types.py @@ -45,11 +45,13 @@ class KeyValueResource(StrEnum): EXPLORE_PERMALINK = "explore_permalink" METASTORE_CACHE = "superset_metastore_cache" LOCK = "lock" + SQLLAB_PERMALINK = "sqllab_permalink" class SharedKey(StrEnum): DASHBOARD_PERMALINK_SALT = "dashboard_permalink_salt" EXPLORE_PERMALINK_SALT = "explore_permalink_salt" + SQLLAB_PERMALINK_SALT = "sqllab_permalink_salt" class KeyValueCodec(ABC): diff --git a/superset/sqllab/permalink/__init__.py b/superset/sqllab/permalink/__init__.py new file mode 100644 index 00000000000..13a83393a91 --- /dev/null +++ b/superset/sqllab/permalink/__init__.py @@ -0,0 +1,16 @@ +# 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. diff --git a/superset/sqllab/permalink/api.py b/superset/sqllab/permalink/api.py new file mode 100644 index 00000000000..c86fb99a5ed --- /dev/null +++ b/superset/sqllab/permalink/api.py @@ -0,0 +1,144 @@ +# 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. +import logging + +from flask import request, Response +from flask_appbuilder.api import expose, protect, safe +from marshmallow import ValidationError + +from superset.commands.sql_lab.permalink.create import CreateSqlLabPermalinkCommand +from superset.commands.sql_lab.permalink.get import GetSqlLabPermalinkCommand +from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP +from superset.extensions import event_logger +from superset.key_value.exceptions import KeyValueAccessDeniedError +from superset.sqllab.permalink.exceptions import SqlLabPermalinkInvalidStateError +from superset.sqllab.permalink.schemas import SqlLabPermalinkSchema +from superset.views.base_api import BaseSupersetApi, requires_json, statsd_metrics + +logger = logging.getLogger(__name__) + + +class SqlLabPermalinkRestApi(BaseSupersetApi): + add_model_schema = SqlLabPermalinkSchema() + method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP + allow_browser_login = True + class_permission_name = "SqlLabPermalinkRestApi" + resource_name = "sqllab" + openapi_spec_tag = "SQL Lab Permanent Link" + openapi_spec_component_schemas = (SqlLabPermalinkSchema,) + + @expose("/permalink", methods=("POST",)) + @protect() + @safe + @statsd_metrics + @event_logger.log_this_with_context( + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.post", + log_to_statsd=False, + ) + @requires_json + def post(self) -> Response: + """Create a new permanent link for SQL Lab editor + --- + post: + summary: Create a new permanent link + requestBody: + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/ExplorePermalinkStateSchema' + responses: + 201: + description: The permanent link was stored successfully. + content: + application/json: + schema: + type: object + properties: + key: + type: string + description: The key to retrieve the permanent link data. + url: + type: string + description: permanent link. + 400: + $ref: '#/components/responses/400' + 401: + $ref: '#/components/responses/401' + 422: + $ref: '#/components/responses/422' + 500: + $ref: '#/components/responses/500' + """ + try: + state = self.add_model_schema.load(request.json) + key = CreateSqlLabPermalinkCommand(state=state).run() + http_origin = request.headers.environ.get("HTTP_ORIGIN") + url = f"{http_origin}/sqllab/p/{key}" + return self.response(201, key=key, url=url) + except ValidationError as ex: + return self.response(400, message=ex.messages) + except KeyValueAccessDeniedError as ex: + return self.response(403, message=str(ex)) + + @expose("/permalink/", methods=("GET",)) + @protect() + @safe + @statsd_metrics + @event_logger.log_this_with_context( + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.get", + log_to_statsd=False, + ) + def get(self, key: str) -> Response: + """Get chart's permanent link state. + --- + get: + summary: Get chart's permanent link state + parameters: + - in: path + schema: + type: string + name: key + responses: + 200: + description: Returns the stored form_data. + content: + application/json: + schema: + type: object + properties: + state: + type: object + description: The stored state + 400: + $ref: '#/components/responses/400' + 401: + $ref: '#/components/responses/401' + 404: + $ref: '#/components/responses/404' + 422: + $ref: '#/components/responses/422' + 500: + $ref: '#/components/responses/500' + """ + try: + value = GetSqlLabPermalinkCommand(key=key).run() + if not value: + return self.response_404() + return self.response(200, **value) + except SqlLabPermalinkInvalidStateError as ex: + return self.response(400, message=str(ex)) diff --git a/superset/sqllab/permalink/exceptions.py b/superset/sqllab/permalink/exceptions.py new file mode 100644 index 00000000000..5d8dd7a9f43 --- /dev/null +++ b/superset/sqllab/permalink/exceptions.py @@ -0,0 +1,31 @@ +# 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 flask_babel import lazy_gettext as _ + +from superset.commands.exceptions import CommandException, CreateFailedError + + +class SqlLabPermalinkInvalidStateError(CreateFailedError): + message = _("Invalid state.") + + +class SqlLabPermalinkCreateFailedError(CreateFailedError): + message = _("An error occurred while creating the copy link.") + + +class SqlLabPermalinkGetFailedError(CommandException): + message = _("An error occurred while accessing the copy link.") diff --git a/superset/sqllab/permalink/schemas.py b/superset/sqllab/permalink/schemas.py new file mode 100644 index 00000000000..d89c2b5796c --- /dev/null +++ b/superset/sqllab/permalink/schemas.py @@ -0,0 +1,46 @@ +# 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 marshmallow import fields, Schema + + +class SqlLabPermalinkSchema(Schema): + autorun = fields.Boolean() + dbId = fields.Integer( # noqa: N815 + required=True, + allow_none=False, + metadata={"description": "The id of the database"}, + ) + name = fields.String( + required=True, + allow_none=False, + metadata={"description": "The label of the editor tab"}, + ) + schema = fields.String( + required=False, + allow_none=True, + metadata={"description": "The schema name of the query"}, + ) + sql = fields.String( + required=True, + allow_none=False, + metadata={"description": "SQL query text"}, + ) + templateParams = fields.String( # noqa: N815 + required=False, + allow_none=True, + metadata={"description": "stringfied JSON string for template parameters"}, + ) diff --git a/superset/sqllab/permalink/types.py b/superset/sqllab/permalink/types.py new file mode 100644 index 00000000000..adc127da111 --- /dev/null +++ b/superset/sqllab/permalink/types.py @@ -0,0 +1,26 @@ +# 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 typing import Optional, TypedDict + + +class SqlLabPermalinkValue(TypedDict): + dbId: int + name: str + schema: Optional[str] + sql: str + autorun: bool + templateParams: Optional[str] diff --git a/superset/views/key_value.py b/superset/views/key_value.py deleted file mode 100644 index 69a5314c5fb..00000000000 --- a/superset/views/key_value.py +++ /dev/null @@ -1,67 +0,0 @@ -# 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 flask import request, Response -from flask_appbuilder import expose -from flask_appbuilder.hooks import before_request -from flask_appbuilder.security.decorators import has_access_api -from werkzeug.exceptions import NotFound - -from superset import db, event_logger, is_feature_enabled -from superset.models import core as models -from superset.superset_typing import FlaskResponse -from superset.utils import core as utils, json -from superset.views.base import BaseSupersetView, deprecated, json_error_response - - -class KV(BaseSupersetView): - """Used for storing and retrieving key value pairs""" - - @staticmethod - def is_enabled() -> bool: - return is_feature_enabled("KV_STORE") - - @before_request - def ensure_enabled(self) -> None: - if not self.is_enabled(): - raise NotFound() - - @event_logger.log_this - @has_access_api - @expose("/store/", methods=("POST",)) - @deprecated(eol_version="5.0.0") - def store(self) -> FlaskResponse: - try: - value = request.form.get("data") - obj = models.KeyValue(value=value) - db.session.add(obj) - db.session.commit() # pylint: disable=consider-using-transaction - except Exception as ex: # pylint: disable=broad-except - return json_error_response(utils.error_msg_from_exception(ex)) - return Response(json.dumps({"id": obj.id}), status=200) - - @event_logger.log_this - @has_access_api - @expose("//", methods=("GET",)) - @deprecated(eol_version="5.0.0") - def get_value(self, key_id: int) -> FlaskResponse: - try: - kv = db.session.query(models.KeyValue).filter_by(id=key_id).scalar() - if not kv: - return Response(status=404, content_type="text/plain") - except Exception as ex: # pylint: disable=broad-except - return json_error_response(utils.error_msg_from_exception(ex)) - return Response(kv.value, status=200, content_type="text/plain") diff --git a/superset/views/sqllab.py b/superset/views/sqllab.py index 5677237c482..b34a3af7d4e 100644 --- a/superset/views/sqllab.py +++ b/superset/views/sqllab.py @@ -15,6 +15,7 @@ # specific language governing permissions and limitations # under the License. import contextlib +from typing import Any from flask import request from flask_appbuilder import permission_name @@ -36,10 +37,11 @@ class SqllabView(BaseSupersetView): method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP @expose("/", methods=["GET", "POST"]) + @expose("/p//", methods=["GET"]) @has_access @permission_name("read") @event_logger.log_this - def root(self) -> FlaskResponse: + def root(self, **kwargs: Any) -> FlaskResponse: payload = {} if form_data := request.form.get("form_data"): with contextlib.suppress(json.JSONDecodeError): diff --git a/tests/integration_tests/core_tests.py b/tests/integration_tests/core_tests.py index d518cc47994..4a47da95c46 100644 --- a/tests/integration_tests/core_tests.py +++ b/tests/integration_tests/core_tests.py @@ -53,7 +53,6 @@ from superset.utils import core as utils, json from superset.utils.core import backend from superset.utils.database import get_example_database from superset.views.database.views import DatabaseView -from tests.integration_tests.conftest import with_feature_flags from tests.integration_tests.constants import ADMIN_USERNAME, GAMMA_USERNAME from tests.integration_tests.fixtures.birth_names_dashboard import ( load_birth_names_dashboard_with_slices, # noqa: F401 @@ -371,35 +370,6 @@ class TestCore(SupersetTestCase): } ] - @with_feature_flags(KV_STORE=False) - def test_kv_disabled(self): - self.login(ADMIN_USERNAME) - - resp = self.client.get("/kv/10001/") - assert 404 == resp.status_code - - value = json.dumps({"data": "this is a test"}) - resp = self.client.post("/kv/store/", data=dict(data=value)) # noqa: C408 - assert resp.status_code == 404 - - @with_feature_flags(KV_STORE=True) - def test_kv_enabled(self): - self.login(ADMIN_USERNAME) - - resp = self.client.get("/kv/10001/") - assert 404 == resp.status_code - - value = json.dumps({"data": "this is a test"}) - resp = self.client.post("/kv/store/", data=dict(data=value)) # noqa: C408 - assert resp.status_code == 200 - kv = db.session.query(models.KeyValue).first() - kv_value = kv.value - assert json.loads(value) == json.loads(kv_value) - - resp = self.client.get(f"/kv/{kv.id}/") - assert resp.status_code == 200 - assert json.loads(value) == json.loads(resp.data.decode("utf-8")) - def test_gamma(self): self.login(GAMMA_USERNAME) assert "Charts" in self.get_resp("/chart/list/") diff --git a/tests/integration_tests/sql_lab/permalink/__init__.py b/tests/integration_tests/sql_lab/permalink/__init__.py new file mode 100644 index 00000000000..13a83393a91 --- /dev/null +++ b/tests/integration_tests/sql_lab/permalink/__init__.py @@ -0,0 +1,16 @@ +# 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. diff --git a/tests/integration_tests/sql_lab/permalink/api_tests.py b/tests/integration_tests/sql_lab/permalink/api_tests.py new file mode 100644 index 00000000000..7d515b796f8 --- /dev/null +++ b/tests/integration_tests/sql_lab/permalink/api_tests.py @@ -0,0 +1,102 @@ +# 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. +import json +from collections.abc import Iterator +from typing import Any +from uuid import uuid3 + +import pytest + +from superset import db +from superset.key_value.models import KeyValueEntry +from superset.key_value.types import KeyValueResource +from superset.key_value.utils import decode_permalink_id +from tests.integration_tests.constants import ( + GAMMA_SQLLAB_USERNAME, +) + + +@pytest.fixture +def tab_state_data() -> dict[str, Any]: + return { + "dbId": 1, + "name": "Untitled Query 1", + "schema": "main", + "sql": "SELECT 'foo' as bar", + "autorun": False, + "templateParams": '{"param1": "value1"}', + } + + +@pytest.fixture +def permalink_salt(app_context) -> Iterator[str]: + from superset.key_value.shared_entries import get_permalink_salt, get_uuid_namespace + from superset.key_value.types import SharedKey + + key = SharedKey.SQLLAB_PERMALINK_SALT + salt = get_permalink_salt(key) + yield salt + namespace = get_uuid_namespace(salt) + db.session.query(KeyValueEntry).filter_by( + resource=KeyValueResource.APP, + uuid=uuid3(namespace, key), + ) + db.session.commit() + + +def test_post( + tab_state_data: dict[str, Any], permalink_salt: str, test_client, login_as +): + login_as(GAMMA_SQLLAB_USERNAME) + resp = test_client.post("api/v1/sqllab/permalink", json=tab_state_data) + assert resp.status_code == 201 + data = resp.json + key = data["key"] + url = data["url"] + assert key in url + id_ = decode_permalink_id(key, permalink_salt) + db.session.query(KeyValueEntry).filter_by(id=id_).delete() + db.session.commit() + + +def test_post_access_denied(tab_state_data: dict[str, Any], test_client, login_as): + resp = test_client.post("api/v1/sqllab/permalink", json=tab_state_data) + assert resp.status_code == 401 + + +def test_post_invalid_schema(test_client, login_as): + login_as(GAMMA_SQLLAB_USERNAME) + resp = test_client.post( + "api/v1/sqllab/permalink", json={"name": "Untitled Query 1", "sql": "Test"} + ) + assert resp.status_code == 400 + + +def test_get( + tab_state_data: dict[str, Any], permalink_salt: str, test_client, login_as +): + login_as(GAMMA_SQLLAB_USERNAME) + resp = test_client.post("api/v1/sqllab/permalink", json=tab_state_data) + data = resp.json + key = data["key"] + resp = test_client.get(f"api/v1/sqllab/permalink/{key}") + assert resp.status_code == 200 + result = json.loads(resp.data.decode("utf-8")) + assert result == tab_state_data + id_ = decode_permalink_id(key, permalink_salt) + db.session.query(KeyValueEntry).filter_by(id=id_).delete() + db.session.commit() diff --git a/tests/integration_tests/superset_test_config.py b/tests/integration_tests/superset_test_config.py index 4358ba875bc..599df86742b 100644 --- a/tests/integration_tests/superset_test_config.py +++ b/tests/integration_tests/superset_test_config.py @@ -68,8 +68,6 @@ SQLLAB_CTAS_NO_LIMIT = True # SQL_MAX_ROW will not take effect for the CTA quer FEATURE_FLAGS = { **FEATURE_FLAGS, # noqa: F405 "foo": "bar", - "KV_STORE": True, - "SHARE_QUERIES_VIA_KV_STORE": True, "ENABLE_TEMPLATE_PROCESSING": True, "ALERT_REPORTS": True, "AVOID_COLORS_COLLISION": True, diff --git a/tests/integration_tests/superset_test_config_thumbnails.py b/tests/integration_tests/superset_test_config_thumbnails.py index 010e587718e..594b0b509b1 100644 --- a/tests/integration_tests/superset_test_config_thumbnails.py +++ b/tests/integration_tests/superset_test_config_thumbnails.py @@ -74,8 +74,6 @@ CELERY_CONFIG = CeleryConfig FEATURE_FLAGS = { "foo": "bar", - "KV_STORE": False, - "SHARE_QUERIES_VIA_KV_STORE": False, "THUMBNAILS": True, "THUMBNAILS_SQLA_LISTENERS": False, }