From d9a0a445e74aa8a2cecb1185ccf3273bc3e766e5 Mon Sep 17 00:00:00 2001 From: Diego Medina Date: Fri, 20 Jan 2023 16:34:44 -0300 Subject: [PATCH 1/7] wip: chore: migrate /sql_json and /results to apiv1 --- .../src/SqlLab/actions/sqlLab.js | 18 +- superset/constants.py | 2 + superset/initialization/__init__.py | 2 + superset/sqllab/api.py | 253 ++++++++++++++++++ .../{command.py => commands/execute.py} | 0 superset/sqllab/commands/results.py | 134 ++++++++++ .../sqllab/execution_context_convertor.py | 34 ++- superset/sqllab/query_render.py | 2 +- superset/sqllab/schemas.py | 83 ++++++ superset/sqllab/validators.py | 2 +- superset/views/core.py | 5 +- tests/integration_tests/sql_lab/api_tests.py | 167 ++++++++++++ 12 files changed, 683 insertions(+), 19 deletions(-) create mode 100644 superset/sqllab/api.py rename superset/sqllab/{command.py => commands/execute.py} (100%) create mode 100644 superset/sqllab/commands/results.py create mode 100644 superset/sqllab/schemas.py create mode 100644 tests/integration_tests/sql_lab/api_tests.py diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index 12487d1a9437..0881af87a881 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -17,6 +17,7 @@ * under the License. */ import shortid from 'shortid'; +import rison from 'rison'; import { SupersetClient, t } from '@superset-ui/core'; import invert from 'lodash/invert'; import mapKeys from 'lodash/mapKeys'; @@ -305,8 +306,13 @@ export function fetchQueryResults(query, displayLimit) { return function (dispatch) { dispatch(requestQueryResults(query)); - return SupersetClient.get({ - endpoint: `/superset/results/${query.resultsKey}/?rows=${displayLimit}`, + const queryParams = rison.encode({ + key: query.resultsKey, + rows: displayLimit, + }); + + SupersetClient.get({ + endpoint: `/api/v1/sqllab/results/?q=${queryParams}`, parseMethod: 'json-bigint', }) .then(({ json }) => dispatch(querySuccess(query, json))) @@ -347,7 +353,7 @@ export function runQuery(query) { const search = window.location.search || ''; return SupersetClient.post({ - endpoint: `/superset/sql_json/${search}`, + endpoint: `/api/v1/sqllab/execute/${search}`, body: JSON.stringify(postPayload), headers: { 'Content-Type': 'application/json' }, parseMethod: 'json-bigint', @@ -359,7 +365,11 @@ export function runQuery(query) { }) .catch(response => getClientErrorObject(response).then(error => { - let message = error.error || error.statusText || t('Unknown error'); + let message = + error.error || + error.message || + error.statusText || + t('Unknown error'); if (message.includes('CSRF token')) { message = t(COMMON_ERR_MESSAGES.SESSION_TIMED_OUT); } diff --git a/superset/constants.py b/superset/constants.py index ea7920ff2fd7..17745bd4fb3b 100644 --- a/superset/constants.py +++ b/superset/constants.py @@ -140,6 +140,8 @@ class RouteMethod: # pylint: disable=too-few-public-methods "get_data": "read", "samples": "read", "delete_ssh_tunnel": "write", + "get_results": "read", + "execute_sql_query": "read", } EXTRA_FORM_DATA_APPEND_KEYS = { diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index 7e3778150231..92d8c4aeb9e9 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -149,6 +149,7 @@ def init_views(self) -> None: from superset.reports.logs.api import ReportExecutionLogRestApi from superset.row_level_security.api import RLSRestApi from superset.security.api import SecurityRestApi + from superset.sqllab.api import SqlLabRestApi from superset.views.access_requests import AccessRequestsModelView from superset.views.alerts import AlertView, ReportView from superset.views.annotations import AnnotationLayerView @@ -218,6 +219,7 @@ def init_views(self) -> None: appbuilder.add_api(ReportExecutionLogRestApi) appbuilder.add_api(RLSRestApi) appbuilder.add_api(SavedQueryRestApi) + appbuilder.add_api(SqlLabRestApi) # # Setup regular views # diff --git a/superset/sqllab/api.py b/superset/sqllab/api.py new file mode 100644 index 000000000000..746e3d5badfe --- /dev/null +++ b/superset/sqllab/api.py @@ -0,0 +1,253 @@ +# 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, cast, Dict, Optional + +from flask import request, Response +from flask_appbuilder.api import expose, protect, rison, safe +from flask_appbuilder.models.sqla.interface import SQLAInterface +from marshmallow import ValidationError +from superset.views.base import handle_api_exception +from superset.sqllab.sqllab_execution_context import SqlJsonExecutionContext +from superset.queries.dao import QueryDAO +from superset.sqllab.sql_json_executer import ( + ASynchronousSqlJsonExecutor, + SqlJsonExecutor, + SynchronousSqlJsonExecutor, +) +from superset.sqllab.exceptions import ( + QueryIsForbiddenToAccessException, + SqlLabException, +) +from superset.sqllab.command_status import SqlJsonExecutionStatus +from superset.sql_lab import get_sql_results +from superset.sqllab.commands.execute import CommandResult, ExecuteSqlCommand +from superset.sqllab.execution_context_convertor import ExecutionContextConvertor +from superset.databases.dao import DatabaseDAO +from superset.sqllab.validators import CanAccessQueryValidatorImpl +from superset.sqllab.query_render import SqlQueryRenderImpl +from superset.jinja_context import get_template_processor + +from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP +from superset.extensions import event_logger +from superset.models.sql_lab import Query +from superset.sqllab.commands.results import SqlExecutionResultsCommand +from superset.sqllab.schemas import ( + ExecutePayloadSchema, + QueryExecutionResponseSchema, + sql_lab_get_results_schema, +) +from superset.views.base_api import ( + BaseSupersetModelRestApi, + requires_json, + statsd_metrics, +) +from superset import ( + app, + is_feature_enabled, +) + +config = app.config +logger = logging.getLogger(__name__) + + +class SqlLabRestApi(BaseSupersetModelRestApi): + datamodel = SQLAInterface(Query) + + include_route_methods = { + "get_results", + "execute_sql_query", + } + resource_name = "sqllab" + allow_browser_login = True + + class_permission_name = "Query" + method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP + + execute_model_schema = ExecutePayloadSchema() + + apispec_parameter_schemas = { + "sql_lab_get_results_schema": sql_lab_get_results_schema, + } + openapi_spec_tag = "SQL Lab" + openapi_spec_component_schemas = ( + ExecutePayloadSchema, + QueryExecutionResponseSchema, + ) + + @expose("/results/") + @protect() + @safe + @statsd_metrics + @rison(sql_lab_get_results_schema) + @handle_api_exception + @event_logger.log_this_with_context( + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}" + f".get_results", + log_to_statsd=False, + ) + def get_results(self, **kwargs: Any) -> Response: + """Gets the result of a SQL query execution + --- + get: + summary: >- + Gets the result of a SQL query execution + parameters: + - in: query + name: q + content: + application/json: + schema: + $ref: '#/components/schemas/sql_lab_get_results_schema' + responses: + 200: + description: SQL query execution result + content: + application/json: + schema: + $ref: '#/components/schemas/QueryExecutionResponseSchema' + 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' + """ + params = kwargs["rison"] + key = params.get("key") + rows = params.get("rows") + result = SqlExecutionResultsCommand(key=key, rows=rows).run() + return self.response(200, **result) + + @expose("/execute/", methods=["POST"]) + @protect() + @statsd_metrics + @requires_json + @handle_api_exception + @event_logger.log_this_with_context( + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}" + f".get_results", + log_to_statsd=False, + ) + def execute_sql_query(self) -> Response: + """Executes a SQL query + --- + post: + description: >- + Starts the execution of a SQL query + requestBody: + description: SQL query and params + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/ExecutePayloadSchema' + responses: + 200: + description: Query execution result + content: + application/json: + schema: + $ref: '#/components/schemas/QueryExecutionResponseSchema' + 202: + description: Query execution result, query still running + content: + application/json: + schema: + $ref: '#/components/schemas/QueryExecutionResponseSchema' + 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: + self.execute_model_schema.load(request.json) + except ValidationError as error: + return self.response_400(message=error.messages) + + try: + log_params = { + "user_agent": cast(Optional[str], request.headers.get("USER_AGENT")) + } + execution_context = SqlJsonExecutionContext(request.json) + command = self._create_sql_json_command(execution_context, log_params) + command_result: CommandResult = command.run() + + response_status = ( + 202 + if command_result["status"] == SqlJsonExecutionStatus.QUERY_IS_RUNNING + else 200 + ) + return self.response(response_status, **command_result["payload"]) + except SqlLabException as ex: + payload = {"errors": [ex.to_dict()]} + + response_status = ( + 403 if isinstance(ex, QueryIsForbiddenToAccessException) else ex.status + ) + return self.response(response_status, **payload) + + @staticmethod + def _create_sql_json_command( + execution_context: SqlJsonExecutionContext, log_params: Optional[Dict[str, Any]] + ) -> ExecuteSqlCommand: + query_dao = QueryDAO() + sql_json_executor = SqlLabRestApi._create_sql_json_executor( + execution_context, query_dao + ) + execution_context_convertor = ExecutionContextConvertor() + execution_context_convertor.set_serialize_to_json(False) + execution_context_convertor.set_max_row_in_display( + int(config.get("DISPLAY_MAX_ROW")) # type: ignore + ) + return ExecuteSqlCommand( + execution_context, + query_dao, + DatabaseDAO(), + CanAccessQueryValidatorImpl(), + SqlQueryRenderImpl(get_template_processor), + sql_json_executor, + execution_context_convertor, + config.get("SQLLAB_CTAS_NO_LIMIT"), + log_params, + ) + + @staticmethod + def _create_sql_json_executor( + execution_context: SqlJsonExecutionContext, query_dao: QueryDAO + ) -> SqlJsonExecutor: + sql_json_executor: SqlJsonExecutor + if execution_context.is_run_asynchronous(): + sql_json_executor = ASynchronousSqlJsonExecutor(query_dao, get_sql_results) + else: + sql_json_executor = SynchronousSqlJsonExecutor( + query_dao, + get_sql_results, + config.get("SQLLAB_TIMEOUT"), # type: ignore + is_feature_enabled("SQLLAB_BACKEND_PERSISTENCE"), + ) + return sql_json_executor diff --git a/superset/sqllab/command.py b/superset/sqllab/commands/execute.py similarity index 100% rename from superset/sqllab/command.py rename to superset/sqllab/commands/execute.py diff --git a/superset/sqllab/commands/results.py b/superset/sqllab/commands/results.py new file mode 100644 index 000000000000..190b0e092525 --- /dev/null +++ b/superset/sqllab/commands/results.py @@ -0,0 +1,134 @@ +# 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. +# pylint: disable=too-few-public-methods, too-many-arguments +from __future__ import annotations + +import logging +from typing import Any, cast, Dict, Optional +from flask_babel import gettext as __, lazy_gettext as _ + +from superset import ( + app, + db, + results_backend, + results_backend_use_msgpack, +) +from superset.commands.base import BaseCommand +from superset.errors import ErrorLevel, SupersetError, SupersetErrorType +from superset.exceptions import ( + SerializationError, + SupersetErrorException, +) +from superset.models.sql_lab import Query +from superset.sqllab.utils import apply_display_max_row_configuration_if_require +from superset.utils import core as utils +from superset.utils.dates import now_as_float +from superset.views.utils import _deserialize_results_payload + +config = app.config +SQLLAB_QUERY_COST_ESTIMATE_TIMEOUT = config["SQLLAB_QUERY_COST_ESTIMATE_TIMEOUT"] +stats_logger = config["STATS_LOGGER"] + +logger = logging.getLogger(__name__) + + +class SqlExecutionResultsCommand(BaseCommand): + _key: str + _rows: Optional[int] + _blob: Any + _query: Query + + def __init__( + self, + key: str, + rows: Optional[int] = None, + ) -> None: + self._key = key + self._rows = rows + + def validate(self) -> None: + if not results_backend: + raise SupersetErrorException( + SupersetError( + message=__("Results backend is not configured."), + error_type=SupersetErrorType.RESULTS_BACKEND_NOT_CONFIGURED_ERROR, + level=ErrorLevel.ERROR, + ) + ) + + read_from_results_backend_start = now_as_float() + self._blob = results_backend.get(self._key) + stats_logger.timing( + "sqllab.query.results_backend_read", + now_as_float() - read_from_results_backend_start, + ) + + if not self._blob: + raise SupersetErrorException( + SupersetError( + message=__( + "Data could not be retrieved from the results backend. You " + "need to re-run the original query." + ), + error_type=SupersetErrorType.RESULTS_BACKEND_ERROR, + level=ErrorLevel.ERROR, + ), + status=410, + ) + + self._query = db.session.query(Query).filter_by(results_key=self._key).one_or_none() + if self._query is None: + raise SupersetErrorException( + SupersetError( + message=__( + "The query associated with these results could not be find. " + "You need to re-run the original query." + ), + error_type=SupersetErrorType.RESULTS_BACKEND_ERROR, + level=ErrorLevel.ERROR, + ), + status=404, + ) + + def run( + self, + ) -> Dict[str, Any]: + """Runs arbitrary sql and returns data as json""" + self.validate() + payload = utils.zlib_decompress(self._blob, decode=not results_backend_use_msgpack) + try: + obj = _deserialize_results_payload( + payload, self._query, cast(bool, results_backend_use_msgpack) + ) + except SerializationError as ex: + raise SupersetErrorException( + SupersetError( + message=__( + "Data could not be deserialized from the results backend. The " + "storage format might have changed, rendering the old data " + "stake. You need to re-run the original query." + ), + error_type=SupersetErrorType.RESULTS_BACKEND_ERROR, + level=ErrorLevel.ERROR, + ), + status=404, + ) from ex + + if self._rows: + obj = apply_display_max_row_configuration_if_require(obj, self._rows) + + return obj diff --git a/superset/sqllab/execution_context_convertor.py b/superset/sqllab/execution_context_convertor.py index f49fbd9a31db..d788d567a508 100644 --- a/superset/sqllab/execution_context_convertor.py +++ b/superset/sqllab/execution_context_convertor.py @@ -17,7 +17,7 @@ from __future__ import annotations import logging -from typing import Any, Dict, TYPE_CHECKING +from typing import Any, Dict, TYPE_CHECKING, Union import simplejson as json @@ -34,6 +34,7 @@ class ExecutionContextConvertor: + _serialize_to_json = True _max_row_in_display_configuration: int # pylint: disable=invalid-name _exc_status: SqlJsonExecutionStatus payload: Dict[str, Any] @@ -41,6 +42,9 @@ class ExecutionContextConvertor: def set_max_row_in_display(self, value: int) -> None: self._max_row_in_display_configuration = value # pylint: disable=invalid-name + def set_serialize_to_json(self, value: bool) -> None: + self._serialize_to_json = value + def set_payload( self, execution_context: SqlJsonExecutionContext, @@ -52,17 +56,25 @@ def set_payload( else: self.payload = execution_context.query.to_dict() - def serialize_payload(self) -> str: + def serialize_payload(self) -> Union[Dict[str, Any], str]: if self._exc_status == SqlJsonExecutionStatus.HAS_RESULTS: - return json.dumps( - apply_display_max_row_configuration_if_require( - self.payload, self._max_row_in_display_configuration - ), - default=utils.pessimistic_json_iso_dttm_ser, - ignore_nan=True, - encoding=None, + payload = apply_display_max_row_configuration_if_require( + self.payload, self._max_row_in_display_configuration + ) + return ( + json.dumps( + payload, + default=utils.pessimistic_json_iso_dttm_ser, + ignore_nan=True, + encoding=None, + ) + if self._serialize_to_json + else payload ) - return json.dumps( - {"query": self.payload}, default=utils.json_int_dttm_ser, ignore_nan=True + payload = {"query": self.payload} + return ( + json.dumps(payload, default=utils.json_int_dttm_ser, ignore_nan=True) + if self._serialize_to_json + else payload ) diff --git a/superset/sqllab/query_render.py b/superset/sqllab/query_render.py index df631784e35b..2854a7e39077 100644 --- a/superset/sqllab/query_render.py +++ b/superset/sqllab/query_render.py @@ -25,7 +25,7 @@ from superset import is_feature_enabled from superset.errors import SupersetErrorType -from superset.sqllab.command import SqlQueryRender +from superset.sqllab.commands.execute import SqlQueryRender from superset.sqllab.exceptions import SqlLabException from superset.utils import core as utils diff --git a/superset/sqllab/schemas.py b/superset/sqllab/schemas.py new file mode 100644 index 000000000000..5e750b3ee0e3 --- /dev/null +++ b/superset/sqllab/schemas.py @@ -0,0 +1,83 @@ +# 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 + +sql_lab_get_results_schema = { + "type": "object", + "properties": { + "key": {"type": "string"}, + }, + "required": ["key"], +} + + +class ExecutePayloadSchema(Schema): + database_id = fields.Integer(required=True) + sql = fields.String(required=True) + client_id = fields.String(allow_none=True) + queryLimit = fields.Integer(allow_none=True) + sql_editor_id = fields.String(allow_none=True) + schema = fields.String(allow_none=True) + tab = fields.String(allow_none=True) + ctas_method = fields.String(allow_none=True) + templateParams = fields.String(allow_none=True) + tmp_table_name = fields.String(allow_none=True) + select_as_cta = fields.Boolean(allow_none=True) + json = fields.Boolean(allow_none=True) + runAsync = fields.Boolean(allow_none=True) + expand_data = fields.Boolean(allow_none=True) + + +class QueryResultSchema(Schema): + changedOn = fields.DateTime() + changed_on = fields.String() + dbId = fields.Integer() + db = fields.String() + endDttm = fields.Float() + errorMessage = fields.String(allow_none=True) + executedSql = fields.String() + id = fields.String() + queryId = fields.Integer() + limit = fields.Integer() + limitingFactor = fields.String() + progress = fields.Integer() + rows = fields.Integer() + schema = fields.String() + ctas = fields.Boolean() + serverId = fields.Integer() + sql = fields.String() + sqlEditorId = fields.String() + startDttm = fields.Float() + state = fields.String() + tab = fields.String() + tempSchema = fields.String(allow_none=True) + tempTable = fields.String(allow_none=True) + userId = fields.Integer() + user = fields.String() + resultsKey = fields.String() + trackingUrl = fields.String(allow_none=True) + extra = fields.Dict(keys=fields.String()) + + +class QueryExecutionResponseSchema(Schema): + status = fields.String() + data = fields.List(fields.Dict()) + columns = fields.List(fields.Dict()) + selected_columns = fields.List(fields.Dict()) + expanded_columns = fields.List(fields.Dict()) + query = fields.Nested(QueryResultSchema) + query_id = fields.Integer() diff --git a/superset/sqllab/validators.py b/superset/sqllab/validators.py index 726a2760e291..5bc8a622531c 100644 --- a/superset/sqllab/validators.py +++ b/superset/sqllab/validators.py @@ -20,7 +20,7 @@ from typing import TYPE_CHECKING from superset import security_manager -from superset.sqllab.command import CanAccessQueryValidator +from superset.sqllab.commands.execute import CanAccessQueryValidator if TYPE_CHECKING: from superset.models.sql_lab import Query diff --git a/superset/views/core.py b/superset/views/core.py index 62a7c5b96352..be8947af9e8c 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -109,8 +109,8 @@ from superset.sql_lab import get_sql_results from superset.sql_parse import ParsedQuery from superset.sql_validators import get_validator_by_name -from superset.sqllab.command import CommandResult, ExecuteSqlCommand from superset.sqllab.command_status import SqlJsonExecutionStatus +from superset.sqllab.commands.execute import CommandResult, ExecuteSqlCommand from superset.sqllab.exceptions import ( QueryIsForbiddenToAccessException, SqlLabException, @@ -2399,6 +2399,7 @@ def validate_sql_json( @handle_api_exception @event_logger.log_this @expose("/sql_json/", methods=["POST"]) + @deprecated() def sql_json(self) -> FlaskResponse: errors = SqlJsonPayloadSchema().validate(request.json) if errors: @@ -2438,7 +2439,7 @@ def _create_sql_json_command( SqlQueryRenderImpl(get_template_processor), sql_json_executor, execution_context_convertor, - config.get("SQLLAB_CTAS_NO_LIMIT"), # type: ignore + config.get("SQLLAB_CTAS_NO_LIMIT"), log_params, ) diff --git a/tests/integration_tests/sql_lab/api_tests.py b/tests/integration_tests/sql_lab/api_tests.py new file mode 100644 index 000000000000..00c7233a5155 --- /dev/null +++ b/tests/integration_tests/sql_lab/api_tests.py @@ -0,0 +1,167 @@ +# 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. +# isort:skip_file +"""Unit tests for Superset""" +import datetime +import json +import random + +import pytest +import prison +from sqlalchemy.sql import func +from unittest import mock + +from tests.integration_tests.test_app import app +from superset import sql_lab +from superset.common.db_query_status import QueryStatus +from superset.models.core import Database +from superset.utils.database import get_example_database, get_main_database +from superset.utils import core as utils +from superset.models.sql_lab import Query + +from tests.integration_tests.base_tests import SupersetTestCase + +QUERIES_FIXTURE_COUNT = 10 + + +class TestSqlLabApi(SupersetTestCase): + def test_execute_required_params(self): + self.login() + client_id = "{}".format(random.getrandbits(64))[:10] + + data = {"client_id": client_id} + rv = self.client.post( + "/api/v1/sqllab/execute/", + json=data, + ) + failed_resp = { + "message": { + "sql": ["Missing data for required field."], + "database_id": ["Missing data for required field."], + } + } + resp_data = json.loads(rv.data.decode("utf-8")) + self.assertDictEqual(resp_data, failed_resp) + self.assertEqual(rv.status_code, 400) + + data = {"sql": "SELECT 1", "client_id": client_id} + rv = self.client.post( + "/api/v1/sqllab/execute/", + json=data, + ) + failed_resp = {"message": {"database_id": ["Missing data for required field."]}} + resp_data = json.loads(rv.data.decode("utf-8")) + self.assertDictEqual(resp_data, failed_resp) + self.assertEqual(rv.status_code, 400) + + data = {"database_id": 1, "client_id": client_id} + rv = self.client.post( + "/api/v1/sqllab/execute/", + json=data, + ) + failed_resp = {"message": {"sql": ["Missing data for required field."]}} + resp_data = json.loads(rv.data.decode("utf-8")) + self.assertDictEqual(resp_data, failed_resp) + self.assertEqual(rv.status_code, 400) + + data = {"sql": "SELECT 1", "database_id": 1, "client_id": client_id} + rv = self.client.post( + "/api/v1/sqllab/execute/", + json=data, + ) + resp_data = json.loads(rv.data.decode("utf-8")) + self.assertEqual(resp_data.get("status"), "success") + self.assertEqual(rv.status_code, 200) + + @mock.patch( + "tests.integration_tests.superset_test_custom_template_processors.datetime" + ) + @mock.patch("superset.sqllab.api.get_sql_results") + def test_execute_custom_templated(self, sql_lab_mock, mock_dt) -> None: + mock_dt.utcnow = mock.Mock(return_value=datetime.datetime(1970, 1, 1)) + self.login() + sql = "SELECT '$DATE()' as test" + resp = { + "status": QueryStatus.SUCCESS, + "query": {"rows": 1}, + "data": [{"test": "'1970-01-01'"}], + } + sql_lab_mock.return_value = resp + + dbobj = self.create_fake_db_for_macros() + json_payload = dict(database_id=dbobj.id, sql=sql) + self.get_json_resp( + "/api/v1/sqllab/execute/", raise_on_error=False, json_=json_payload + ) + assert sql_lab_mock.called + self.assertEqual(sql_lab_mock.call_args[0][1], "SELECT '1970-01-01' as test") + + self.delete_fake_db_for_macros() + + @mock.patch("superset.sqllab.commands.results.results_backend_use_msgpack", False) + def test_display_limit(self): + from superset.sqllab.commands import results as command + + command.results_backend = mock.Mock() + self.login() + + data = [{"col_0": i} for i in range(100)] + payload = { + "status": QueryStatus.SUCCESS, + "query": {"rows": 100}, + "data": data, + } + # limit results to 1 + expected_key = {"status": "success", "query": {"rows": 100}, "data": data} + limited_data = data[:1] + expected_limited = { + "status": "success", + "query": {"rows": 100}, + "data": limited_data, + "displayLimitReached": True, + } + + query_mock = mock.Mock() + query_mock.sql = "SELECT *" + query_mock.database = 1 + query_mock.schema = "superset" + + # do not apply msgpack serialization + use_msgpack = app.config["RESULTS_BACKEND_USE_MSGPACK"] + app.config["RESULTS_BACKEND_USE_MSGPACK"] = False + serialized_payload = sql_lab._serialize_payload(payload, False) + compressed = utils.zlib_compress(serialized_payload) + command.results_backend.get.return_value = compressed + + with mock.patch("superset.sqllab.commands.results.db") as mock_superset_db: + mock_superset_db.session.query().filter_by().one_or_none.return_value = ( + query_mock + ) + # get all results + arguments = {"key": "key"} + result_key = json.loads( + self.get_resp(f"/api/v1/sqllab/results/?q={prison.dumps(arguments)}") + ) + arguments = {"key": "key", "rows": 1} + result_limited = json.loads( + self.get_resp(f"/api/v1/sqllab/results/?q={prison.dumps(arguments)}") + ) + + self.assertEqual(result_key, expected_key) + self.assertEqual(result_limited, expected_limited) + + app.config["RESULTS_BACKEND_USE_MSGPACK"] = use_msgpack From 7d13a305f2394c3b6ef9d1e01eca2d7f629a0fc9 Mon Sep 17 00:00:00 2001 From: Diego Medina Date: Fri, 20 Jan 2023 17:16:06 -0300 Subject: [PATCH 2/7] cleanup --- docs/static/resources/openapi.json | 1616 ++++++++++++++--- .../cypress/integration/sqllab/query.test.ts | 4 +- .../src/SqlLab/actions/sqlLab.test.js | 7 +- .../components/SqlEditor/SqlEditor.test.jsx | 2 +- superset/sqllab/api.py | 41 +- superset/sqllab/commands/results.py | 23 +- superset/sqllab/schemas.py | 2 +- superset/views/core.py | 1 + tests/integration_tests/base_tests.py | 2 +- tests/integration_tests/celery_tests.py | 2 +- tests/integration_tests/sql_lab/api_tests.py | 10 +- 11 files changed, 1452 insertions(+), 258 deletions(-) diff --git a/docs/static/resources/openapi.json b/docs/static/resources/openapi.json index 8279811b53dc..7675ad52d22b 100644 --- a/docs/static/resources/openapi.json +++ b/docs/static/resources/openapi.json @@ -1680,7 +1680,7 @@ "type": "string" }, "changed_by": { - "$ref": "#/components/schemas/ChartDataRestApi.get_list.User1" + "$ref": "#/components/schemas/ChartDataRestApi.get_list.User" }, "changed_by_name": { "readOnly": true @@ -1742,7 +1742,7 @@ "type": "string" }, "last_saved_by": { - "$ref": "#/components/schemas/ChartDataRestApi.get_list.User" + "$ref": "#/components/schemas/ChartDataRestApi.get_list.User1" }, "owners": { "$ref": "#/components/schemas/ChartDataRestApi.get_list.User2" @@ -1809,10 +1809,6 @@ "maxLength": 64, "type": "string" }, - "id": { - "format": "int32", - "type": "integer" - }, "last_name": { "maxLength": 64, "type": "string" @@ -1830,6 +1826,10 @@ "maxLength": 64, "type": "string" }, + "id": { + "format": "int32", + "type": "integer" + }, "last_name": { "maxLength": 64, "type": "string" @@ -2472,7 +2472,7 @@ "type": "string" }, "changed_by": { - "$ref": "#/components/schemas/ChartRestApi.get_list.User1" + "$ref": "#/components/schemas/ChartRestApi.get_list.User" }, "changed_by_name": { "readOnly": true @@ -2534,7 +2534,7 @@ "type": "string" }, "last_saved_by": { - "$ref": "#/components/schemas/ChartRestApi.get_list.User" + "$ref": "#/components/schemas/ChartRestApi.get_list.User1" }, "owners": { "$ref": "#/components/schemas/ChartRestApi.get_list.User2" @@ -2601,10 +2601,6 @@ "maxLength": 64, "type": "string" }, - "id": { - "format": "int32", - "type": "integer" - }, "last_name": { "maxLength": 64, "type": "string" @@ -2622,6 +2618,10 @@ "maxLength": 64, "type": "string" }, + "id": { + "format": "int32", + "type": "integer" + }, "last_name": { "maxLength": 64, "type": "string" @@ -4172,6 +4172,12 @@ }, "DatabaseSSHTunnel": { "properties": { + "id": { + "description": "SSH Tunnel ID (for updates)", + "format": "int32", + "nullable": true, + "type": "integer" + }, "password": { "type": "string" }, @@ -4801,7 +4807,7 @@ "$ref": "#/components/schemas/DatasetRestApi.get.TableColumn" }, "created_by": { - "$ref": "#/components/schemas/DatasetRestApi.get.User2" + "$ref": "#/components/schemas/DatasetRestApi.get.User1" }, "created_on": { "format": "date-time", @@ -4865,7 +4871,7 @@ "type": "integer" }, "owners": { - "$ref": "#/components/schemas/DatasetRestApi.get.User1" + "$ref": "#/components/schemas/DatasetRestApi.get.User2" }, "schema": { "maxLength": 255, @@ -5079,23 +5085,14 @@ "maxLength": 64, "type": "string" }, - "id": { - "format": "int32", - "type": "integer" - }, "last_name": { "maxLength": 64, "type": "string" - }, - "username": { - "maxLength": 64, - "type": "string" } }, "required": [ "first_name", - "last_name", - "username" + "last_name" ], "type": "object" }, @@ -5105,14 +5102,23 @@ "maxLength": 64, "type": "string" }, + "id": { + "format": "int32", + "type": "integer" + }, "last_name": { "maxLength": 64, "type": "string" + }, + "username": { + "maxLength": 64, + "type": "string" } }, "required": [ "first_name", - "last_name" + "last_name", + "username" ], "type": "object" }, @@ -5512,6 +5518,71 @@ }, "type": "object" }, + "ExecutePayloadSchema": { + "properties": { + "client_id": { + "nullable": true, + "type": "string" + }, + "ctas_method": { + "nullable": true, + "type": "string" + }, + "database_id": { + "format": "int32", + "type": "integer" + }, + "expand_data": { + "nullable": true, + "type": "boolean" + }, + "json": { + "nullable": true, + "type": "boolean" + }, + "queryLimit": { + "format": "int32", + "nullable": true, + "type": "integer" + }, + "runAsync": { + "nullable": true, + "type": "boolean" + }, + "schema": { + "nullable": true, + "type": "string" + }, + "select_as_cta": { + "nullable": true, + "type": "boolean" + }, + "sql": { + "type": "string" + }, + "sql_editor_id": { + "nullable": true, + "type": "string" + }, + "tab": { + "nullable": true, + "type": "string" + }, + "templateParams": { + "nullable": true, + "type": "string" + }, + "tmp_table_name": { + "nullable": true, + "type": "string" + } + }, + "required": [ + "database_id", + "sql" + ], + "type": "object" + }, "ExploreContextSchema": { "properties": { "dataset": { @@ -5964,6 +6035,45 @@ }, "type": "object" }, + "QueryExecutionResponseSchema": { + "properties": { + "columns": { + "items": { + "type": "object" + }, + "type": "array" + }, + "data": { + "items": { + "type": "object" + }, + "type": "array" + }, + "expanded_columns": { + "items": { + "type": "object" + }, + "type": "array" + }, + "query": { + "$ref": "#/components/schemas/QueryResult" + }, + "query_id": { + "format": "int32", + "type": "integer" + }, + "selected_columns": { + "items": { + "type": "object" + }, + "type": "array" + }, + "status": { + "type": "string" + } + }, + "type": "object" + }, "QueryRestApi.get": { "properties": { "changed_on": { @@ -6164,105 +6274,409 @@ }, "type": "object" }, - "RelatedResponseSchema": { + "QueryResult": { "properties": { - "count": { - "description": "The total number of related values", + "changedOn": { + "format": "date-time", + "type": "string" + }, + "changed_on": { + "type": "string" + }, + "ctas": { + "type": "boolean" + }, + "db": { + "type": "string" + }, + "dbId": { "format": "int32", "type": "integer" }, - "result": { - "items": { - "$ref": "#/components/schemas/RelatedResultResponse" - }, - "type": "array" - } - }, - "type": "object" - }, - "RelatedResultResponse": { - "properties": { + "endDttm": { + "format": "float", + "type": "number" + }, + "errorMessage": { + "nullable": true, + "type": "string" + }, + "executedSql": { + "type": "string" + }, "extra": { - "description": "The extra metadata for related item", "type": "object" }, - "text": { - "description": "The related item string representation", + "id": { "type": "string" }, - "value": { - "description": "The related item identifier", + "limit": { "format": "int32", "type": "integer" - } - }, - "type": "object" - }, - "ReportExecutionLogRestApi.get": { - "properties": { - "end_dttm": { - "format": "date-time", - "nullable": true, + }, + "limitingFactor": { "type": "string" }, - "error_message": { - "nullable": true, + "progress": { + "format": "int32", + "type": "integer" + }, + "queryId": { + "format": "int32", + "type": "integer" + }, + "resultsKey": { "type": "string" }, - "id": { + "rows": { "format": "int32", "type": "integer" }, - "scheduled_dttm": { - "format": "date-time", + "schema": { "type": "string" }, - "start_dttm": { - "format": "date-time", - "nullable": true, + "serverId": { + "format": "int32", + "type": "integer" + }, + "sql": { "type": "string" }, + "sqlEditorId": { + "type": "string" + }, + "startDttm": { + "format": "float", + "type": "number" + }, "state": { - "maxLength": 50, "type": "string" }, - "uuid": { - "format": "uuid", + "tab": { + "type": "string" + }, + "tempSchema": { "nullable": true, "type": "string" }, - "value": { - "format": "float", + "tempTable": { "nullable": true, - "type": "number" + "type": "string" }, - "value_row_json": { + "trackingUrl": { "nullable": true, "type": "string" + }, + "user": { + "type": "string" + }, + "userId": { + "format": "int32", + "type": "integer" } }, - "required": [ - "scheduled_dttm", - "state" - ], "type": "object" }, - "ReportExecutionLogRestApi.get_list": { + "RLSRestApi.get": { "properties": { - "end_dttm": { - "format": "date-time", - "nullable": true, + "clause": { + "description": "This is the condition that will be added to the WHERE clause. For example, to only return rows for a particular client, you might define a regular filter with the clause `client_id = 9`. To display no rows unless a user belongs to a RLS filter role, a base filter can be created with the clause `1 = 0` (always false).", "type": "string" }, - "error_message": { - "nullable": true, + "description": { + "description": "Detailed description", + "type": "string" + }, + "filter_type": { + "description": "Regular filters add where clauses to queries if a user belongs to a role referenced in the filter, base filters apply filters to all queries except the roles defined in the filter, and can be used to define what users can see if no RLS filters within a filter group apply to them.", + "enum": [ + "Regular", + "Base" + ], + "type": "string" + }, + "group_key": { + "description": "Filters with the same group key will be ORed together within the group, while different filter groups will be ANDed together. Undefined group keys are treated as unique groups, i.e. are not grouped together. For example, if a table has three filters, of which two are for departments Finance and Marketing (group key = 'department'), and one refers to the region Europe (group key = 'region'), the filter clause would apply the filter (department = 'Finance' OR department = 'Marketing') AND (region = 'Europe').", "type": "string" }, "id": { + "description": "Unique if of rls filter", "format": "int32", "type": "integer" }, - "scheduled_dttm": { + "name": { + "description": "Name of rls filter", + "type": "string" + }, + "roles": { + "items": { + "$ref": "#/components/schemas/Roles1" + }, + "type": "array" + }, + "tables": { + "items": { + "$ref": "#/components/schemas/Tables" + }, + "type": "array" + } + }, + "type": "object" + }, + "RLSRestApi.get_list": { + "properties": { + "changed_on_delta_humanized": { + "readOnly": true + }, + "clause": { + "description": "This is the condition that will be added to the WHERE clause. For example, to only return rows for a particular client, you might define a regular filter with the clause `client_id = 9`. To display no rows unless a user belongs to a RLS filter role, a base filter can be created with the clause `1 = 0` (always false).", + "type": "string" + }, + "description": { + "description": "Detailed description", + "type": "string" + }, + "filter_type": { + "description": "Regular filters add where clauses to queries if a user belongs to a role referenced in the filter, base filters apply filters to all queries except the roles defined in the filter, and can be used to define what users can see if no RLS filters within a filter group apply to them.", + "enum": [ + "Regular", + "Base" + ], + "type": "string" + }, + "group_key": { + "description": "Filters with the same group key will be ORed together within the group, while different filter groups will be ANDed together. Undefined group keys are treated as unique groups, i.e. are not grouped together. For example, if a table has three filters, of which two are for departments Finance and Marketing (group key = 'department'), and one refers to the region Europe (group key = 'region'), the filter clause would apply the filter (department = 'Finance' OR department = 'Marketing') AND (region = 'Europe').", + "type": "string" + }, + "id": { + "description": "Unique if of rls filter", + "format": "int32", + "type": "integer" + }, + "name": { + "description": "Name of rls filter", + "type": "string" + }, + "roles": { + "items": { + "$ref": "#/components/schemas/Roles1" + }, + "type": "array" + }, + "tables": { + "items": { + "$ref": "#/components/schemas/Tables" + }, + "type": "array" + } + }, + "type": "object" + }, + "RLSRestApi.post": { + "properties": { + "clause": { + "description": "This is the condition that will be added to the WHERE clause. For example, to only return rows for a particular client, you might define a regular filter with the clause `client_id = 9`. To display no rows unless a user belongs to a RLS filter role, a base filter can be created with the clause `1 = 0` (always false).", + "type": "string" + }, + "description": { + "description": "Detailed description", + "nullable": true, + "type": "string" + }, + "filter_type": { + "description": "Regular filters add where clauses to queries if a user belongs to a role referenced in the filter, base filters apply filters to all queries except the roles defined in the filter, and can be used to define what users can see if no RLS filters within a filter group apply to them.", + "enum": [ + "Regular", + "Base" + ], + "type": "string" + }, + "group_key": { + "description": "Filters with the same group key will be ORed together within the group, while different filter groups will be ANDed together. Undefined group keys are treated as unique groups, i.e. are not grouped together. For example, if a table has three filters, of which two are for departments Finance and Marketing (group key = 'department'), and one refers to the region Europe (group key = 'region'), the filter clause would apply the filter (department = 'Finance' OR department = 'Marketing') AND (region = 'Europe').", + "nullable": true, + "type": "string" + }, + "name": { + "description": "Name of rls filter", + "maxLength": 255, + "minLength": 1, + "type": "string" + }, + "roles": { + "description": "For regular filters, these are the roles this filter will be applied to. For base filters, these are the roles that the filter DOES NOT apply to, e.g. Admin if admin should see all data.", + "items": { + "format": "int32", + "type": "integer" + }, + "type": "array" + }, + "tables": { + "description": "These are the tables this filter will be applied to.", + "items": { + "format": "int32", + "type": "integer" + }, + "minItems": 1, + "type": "array" + } + }, + "required": [ + "clause", + "filter_type", + "name", + "roles", + "tables" + ], + "type": "object" + }, + "RLSRestApi.put": { + "properties": { + "clause": { + "description": "This is the condition that will be added to the WHERE clause. For example, to only return rows for a particular client, you might define a regular filter with the clause `client_id = 9`. To display no rows unless a user belongs to a RLS filter role, a base filter can be created with the clause `1 = 0` (always false).", + "type": "string" + }, + "description": { + "description": "Detailed description", + "nullable": true, + "type": "string" + }, + "filter_type": { + "description": "Regular filters add where clauses to queries if a user belongs to a role referenced in the filter, base filters apply filters to all queries except the roles defined in the filter, and can be used to define what users can see if no RLS filters within a filter group apply to them.", + "enum": [ + "Regular", + "Base" + ], + "type": "string" + }, + "group_key": { + "description": "Filters with the same group key will be ORed together within the group, while different filter groups will be ANDed together. Undefined group keys are treated as unique groups, i.e. are not grouped together. For example, if a table has three filters, of which two are for departments Finance and Marketing (group key = 'department'), and one refers to the region Europe (group key = 'region'), the filter clause would apply the filter (department = 'Finance' OR department = 'Marketing') AND (region = 'Europe').", + "nullable": true, + "type": "string" + }, + "name": { + "description": "Name of rls filter", + "maxLength": 255, + "minLength": 1, + "type": "string" + }, + "roles": { + "description": "For regular filters, these are the roles this filter will be applied to. For base filters, these are the roles that the filter DOES NOT apply to, e.g. Admin if admin should see all data.", + "items": { + "format": "int32", + "type": "integer" + }, + "type": "array" + }, + "tables": { + "description": "These are the tables this filter will be applied to.", + "items": { + "format": "int32", + "type": "integer" + }, + "type": "array" + } + }, + "type": "object" + }, + "RelatedResponseSchema": { + "properties": { + "count": { + "description": "The total number of related values", + "format": "int32", + "type": "integer" + }, + "result": { + "items": { + "$ref": "#/components/schemas/RelatedResultResponse" + }, + "type": "array" + } + }, + "type": "object" + }, + "RelatedResultResponse": { + "properties": { + "extra": { + "description": "The extra metadata for related item", + "type": "object" + }, + "text": { + "description": "The related item string representation", + "type": "string" + }, + "value": { + "description": "The related item identifier", + "format": "int32", + "type": "integer" + } + }, + "type": "object" + }, + "ReportExecutionLogRestApi.get": { + "properties": { + "end_dttm": { + "format": "date-time", + "nullable": true, + "type": "string" + }, + "error_message": { + "nullable": true, + "type": "string" + }, + "id": { + "format": "int32", + "type": "integer" + }, + "scheduled_dttm": { + "format": "date-time", + "type": "string" + }, + "start_dttm": { + "format": "date-time", + "nullable": true, + "type": "string" + }, + "state": { + "maxLength": 50, + "type": "string" + }, + "uuid": { + "format": "uuid", + "nullable": true, + "type": "string" + }, + "value": { + "format": "float", + "nullable": true, + "type": "number" + }, + "value_row_json": { + "nullable": true, + "type": "string" + } + }, + "required": [ + "scheduled_dttm", + "state" + ], + "type": "object" + }, + "ReportExecutionLogRestApi.get_list": { + "properties": { + "end_dttm": { + "format": "date-time", + "nullable": true, + "type": "string" + }, + "error_message": { + "nullable": true, + "type": "string" + }, + "id": { + "format": "int32", + "type": "integer" + }, + "scheduled_dttm": { "format": "date-time", "type": "string" }, @@ -8220,6 +8634,18 @@ }, "type": "object" }, + "Roles1": { + "properties": { + "id": { + "format": "int32", + "type": "integer" + }, + "name": { + "type": "string" + } + }, + "type": "object" + }, "SavedQueryRestApi.get": { "properties": { "changed_on_delta_humanized": { @@ -8561,31 +8987,67 @@ }, "type": "object" }, - "StopQuerySchema": { + "SqlLabRestApi.get": { "properties": { - "client_id": { - "type": "string" + "id": { + "format": "int32", + "type": "integer" } }, "type": "object" }, - "TableExtraMetadataResponseSchema": { + "SqlLabRestApi.get_list": { "properties": { - "clustering": { - "type": "object" - }, - "metadata": { - "type": "object" - }, - "partitions": { - "type": "object" + "id": { + "format": "int32", + "type": "integer" } }, "type": "object" }, - "TableMetadataColumnsResponse": { + "SqlLabRestApi.post": { "properties": { - "duplicates_constraint": { + "id": { + "format": "int32", + "type": "integer" + } + }, + "type": "object" + }, + "SqlLabRestApi.put": { + "properties": { + "id": { + "format": "int32", + "type": "integer" + } + }, + "type": "object" + }, + "StopQuerySchema": { + "properties": { + "client_id": { + "type": "string" + } + }, + "type": "object" + }, + "TableExtraMetadataResponseSchema": { + "properties": { + "clustering": { + "type": "object" + }, + "metadata": { + "type": "object" + }, + "partitions": { + "type": "object" + } + }, + "type": "object" + }, + "TableMetadataColumnsResponse": { + "properties": { + "duplicates_constraint": { "type": "string" }, "keys": { @@ -8725,6 +9187,21 @@ }, "type": "object" }, + "Tables": { + "properties": { + "id": { + "format": "int32", + "type": "integer" + }, + "schema": { + "type": "string" + }, + "table_name": { + "type": "string" + } + }, + "type": "object" + }, "TemporaryCachePostSchema": { "properties": { "value": { @@ -9103,6 +9580,17 @@ }, "type": "object" }, + "sql_lab_get_results_schema": { + "properties": { + "key": { + "type": "string" + } + }, + "required": [ + "key" + ], + "type": "object" + }, "thumbnail_query_schema": { "properties": { "force": { @@ -14217,6 +14705,10 @@ "engine_information": { "description": "Dict with public properties form the DB Engine", "properties": { + "disable_ssh_tunneling": { + "description": "Whether the engine supports SSH Tunnels", + "type": "boolean" + }, "supports_file_upload": { "description": "Whether the engine supports file uploads", "type": "boolean" @@ -18259,9 +18751,9 @@ ] } }, - "/api/v1/saved_query/": { + "/api/v1/rowlevelsecurity/": { "delete": { - "description": "Deletes multiple saved queries in a bulk operation.", + "description": "Deletes multiple RLS rules in a bulk operation.", "parameters": [ { "content": { @@ -18289,11 +18781,14 @@ } } }, - "description": "Saved queries bulk delete" + "description": "RLS Rule bulk delete" }, "401": { "$ref": "#/components/responses/401" }, + "403": { + "$ref": "#/components/responses/403" + }, "404": { "$ref": "#/components/responses/404" }, @@ -18310,11 +18805,11 @@ } ], "tags": [ - "Queries" + "Row Level Security" ] }, "get": { - "description": "Get a list of saved queries, use Rison or JSON query parameters for filtering, sorting, pagination and for selecting specific columns and metadata.", + "description": "Get a list of models", "parameters": [ { "content": { @@ -18387,7 +18882,7 @@ "result": { "description": "The result from the get list query", "items": { - "$ref": "#/components/schemas/SavedQueryRestApi.get_list" + "$ref": "#/components/schemas/RLSRestApi.get_list" }, "type": "array" } @@ -18417,20 +18912,20 @@ } ], "tags": [ - "Queries" + "Row Level Security" ] }, "post": { - "description": "Create a saved query", + "description": "Create a new RLS Rule", "requestBody": { "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/SavedQueryRestApi.post" + "$ref": "#/components/schemas/RLSRestApi.post" } } }, - "description": "Model schema", + "description": "RLS schema", "required": true }, "responses": { @@ -18440,17 +18935,17 @@ "schema": { "properties": { "id": { - "type": "string" + "type": "number" }, "result": { - "$ref": "#/components/schemas/SavedQueryRestApi.post" + "$ref": "#/components/schemas/RLSRestApi.post" } }, "type": "object" } } }, - "description": "Item inserted" + "description": "RLS Rule added" }, "400": { "$ref": "#/components/responses/400" @@ -18458,6 +18953,9 @@ "401": { "$ref": "#/components/responses/401" }, + "404": { + "$ref": "#/components/responses/404" + }, "422": { "$ref": "#/components/responses/422" }, @@ -18471,11 +18969,11 @@ } ], "tags": [ - "Queries" + "Row Level Security" ] } }, - "/api/v1/saved_query/_info": { + "/api/v1/rowlevelsecurity/_info": { "get": { "description": "Get metadata information about this API resource", "parameters": [ @@ -18557,11 +19055,11 @@ } ], "tags": [ - "Queries" + "Row Level Security" ] } }, - "/api/v1/saved_query/distinct/{column_name}": { + "/api/v1/rowlevelsecurity/related/{column_name}": { "get": { "parameters": [ { @@ -18589,11 +19087,11 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/DistincResponseSchema" + "$ref": "#/components/schemas/RelatedResponseSchema" } } }, - "description": "Distinct field data" + "description": "Related column data" }, "400": { "$ref": "#/components/responses/400" @@ -18614,88 +19112,22 @@ } ], "tags": [ - "Queries" + "Row Level Security" ] } }, - "/api/v1/saved_query/export/": { - "get": { - "description": "Exports multiple saved queries and downloads them as YAML files", + "/api/v1/rowlevelsecurity/{pk}": { + "delete": { "parameters": [ { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/get_export_ids_schema" - } - } - }, - "in": "query", - "name": "q" - } - ], - "responses": { - "200": { - "content": { - "application/zip": { - "schema": { - "format": "binary", - "type": "string" - } - } - }, - "description": "A zip file with saved query(ies) and database(s) as YAML" - }, - "400": { - "$ref": "#/components/responses/400" - }, - "401": { - "$ref": "#/components/responses/401" - }, - "404": { - "$ref": "#/components/responses/404" - }, - "500": { - "$ref": "#/components/responses/500" - } - }, - "security": [ - { - "jwt": [] + "in": "path", + "name": "pk", + "required": true, + "schema": { + "type": "integer" + } } ], - "tags": [ - "Queries" - ] - } - }, - "/api/v1/saved_query/import/": { - "post": { - "requestBody": { - "content": { - "multipart/form-data": { - "schema": { - "properties": { - "formData": { - "description": "upload file (ZIP)", - "format": "binary", - "type": "string" - }, - "overwrite": { - "description": "overwrite existing saved queries?", - "type": "boolean" - }, - "passwords": { - "description": "JSON map of passwords for each featured database in the ZIP file. If the ZIP includes a database config in the path `databases/MyDatabase.yaml`, the password should be provided in the following format: `{\"databases/MyDatabase.yaml\": \"my_password\"}`.", - "type": "string" - } - }, - "type": "object" - } - } - }, - "required": true - }, "responses": { "200": { "content": { @@ -18710,13 +19142,10 @@ } } }, - "description": "Saved Query import result" - }, - "400": { - "$ref": "#/components/responses/400" + "description": "Item deleted" }, - "401": { - "$ref": "#/components/responses/401" + "404": { + "$ref": "#/components/responses/404" }, "422": { "$ref": "#/components/responses/422" @@ -18731,26 +19160,25 @@ } ], "tags": [ - "Queries" + "Row Level Security" ] - } - }, - "/api/v1/saved_query/related/{column_name}": { + }, "get": { + "description": "Get an item model", "parameters": [ { "in": "path", - "name": "column_name", + "name": "pk", "required": true, "schema": { - "type": "string" + "type": "integer" } }, { "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/get_related_schema" + "$ref": "#/components/schemas/get_item_schema" } } }, @@ -18763,11 +19191,52 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/RelatedResponseSchema" + "properties": { + "description_columns": { + "properties": { + "column_name": { + "description": "The description for the column name. Will be translated by babel", + "example": "A Nice description for the column", + "type": "string" + } + }, + "type": "object" + }, + "id": { + "description": "The item id", + "type": "string" + }, + "label_columns": { + "properties": { + "column_name": { + "description": "The label for the column name. Will be translated by babel", + "example": "A Nice label for the column", + "type": "string" + } + }, + "type": "object" + }, + "result": { + "$ref": "#/components/schemas/RLSRestApi.get" + }, + "show_columns": { + "description": "A list of columns", + "items": { + "type": "string" + }, + "type": "array" + }, + "show_title": { + "description": "A title to render. Will be translated by babel", + "example": "Show Item Details", + "type": "string" + } + }, + "type": "object" } } }, - "description": "Related column data" + "description": "Item from Model" }, "400": { "$ref": "#/components/responses/400" @@ -18778,6 +19247,9 @@ "404": { "$ref": "#/components/responses/404" }, + "422": { + "$ref": "#/components/responses/422" + }, "500": { "$ref": "#/components/responses/500" } @@ -18788,15 +19260,14 @@ } ], "tags": [ - "Queries" + "Row Level Security" ] - } - }, - "/api/v1/saved_query/{pk}": { - "delete": { - "description": "Delete saved query", + }, + "put": { + "description": "Updates an RLS Rule", "parameters": [ { + "description": "The Rule pk", "in": "path", "name": "pk", "required": true, @@ -18805,11 +19276,616 @@ } } ], - "responses": { - "200": { - "content": { - "application/json": { - "schema": { + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RLSRestApi.put" + } + } + }, + "description": "RLS schema", + "required": true + }, + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "properties": { + "id": { + "type": "number" + }, + "result": { + "$ref": "#/components/schemas/RLSRestApi.put" + } + }, + "type": "object" + } + } + }, + "description": "Rule changed" + }, + "400": { + "$ref": "#/components/responses/400" + }, + "401": { + "$ref": "#/components/responses/401" + }, + "403": { + "$ref": "#/components/responses/403" + }, + "404": { + "$ref": "#/components/responses/404" + }, + "422": { + "$ref": "#/components/responses/422" + }, + "500": { + "$ref": "#/components/responses/500" + } + }, + "security": [ + { + "jwt": [] + } + ], + "tags": [ + "Row Level Security" + ] + } + }, + "/api/v1/saved_query/": { + "delete": { + "description": "Deletes multiple saved queries in a bulk operation.", + "parameters": [ + { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/get_delete_ids_schema" + } + } + }, + "in": "query", + "name": "q" + } + ], + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "properties": { + "message": { + "type": "string" + } + }, + "type": "object" + } + } + }, + "description": "Saved queries bulk delete" + }, + "401": { + "$ref": "#/components/responses/401" + }, + "404": { + "$ref": "#/components/responses/404" + }, + "422": { + "$ref": "#/components/responses/422" + }, + "500": { + "$ref": "#/components/responses/500" + } + }, + "security": [ + { + "jwt": [] + } + ], + "tags": [ + "Queries" + ] + }, + "get": { + "description": "Get a list of saved queries, use Rison or JSON query parameters for filtering, sorting, pagination and for selecting specific columns and metadata.", + "parameters": [ + { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/get_list_schema" + } + } + }, + "in": "query", + "name": "q" + } + ], + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "properties": { + "count": { + "description": "The total record count on the backend", + "type": "number" + }, + "description_columns": { + "properties": { + "column_name": { + "description": "The description for the column name. Will be translated by babel", + "example": "A Nice description for the column", + "type": "string" + } + }, + "type": "object" + }, + "ids": { + "description": "A list of item ids, useful when you don't know the column id", + "items": { + "type": "string" + }, + "type": "array" + }, + "label_columns": { + "properties": { + "column_name": { + "description": "The label for the column name. Will be translated by babel", + "example": "A Nice label for the column", + "type": "string" + } + }, + "type": "object" + }, + "list_columns": { + "description": "A list of columns", + "items": { + "type": "string" + }, + "type": "array" + }, + "list_title": { + "description": "A title to render. Will be translated by babel", + "example": "List Items", + "type": "string" + }, + "order_columns": { + "description": "A list of allowed columns to sort", + "items": { + "type": "string" + }, + "type": "array" + }, + "result": { + "description": "The result from the get list query", + "items": { + "$ref": "#/components/schemas/SavedQueryRestApi.get_list" + }, + "type": "array" + } + }, + "type": "object" + } + } + }, + "description": "Items from Model" + }, + "400": { + "$ref": "#/components/responses/400" + }, + "401": { + "$ref": "#/components/responses/401" + }, + "422": { + "$ref": "#/components/responses/422" + }, + "500": { + "$ref": "#/components/responses/500" + } + }, + "security": [ + { + "jwt": [] + } + ], + "tags": [ + "Queries" + ] + }, + "post": { + "description": "Create a saved query", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/SavedQueryRestApi.post" + } + } + }, + "description": "Model schema", + "required": true + }, + "responses": { + "201": { + "content": { + "application/json": { + "schema": { + "properties": { + "id": { + "type": "string" + }, + "result": { + "$ref": "#/components/schemas/SavedQueryRestApi.post" + } + }, + "type": "object" + } + } + }, + "description": "Item inserted" + }, + "400": { + "$ref": "#/components/responses/400" + }, + "401": { + "$ref": "#/components/responses/401" + }, + "422": { + "$ref": "#/components/responses/422" + }, + "500": { + "$ref": "#/components/responses/500" + } + }, + "security": [ + { + "jwt": [] + } + ], + "tags": [ + "Queries" + ] + } + }, + "/api/v1/saved_query/_info": { + "get": { + "description": "Get metadata information about this API resource", + "parameters": [ + { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/get_info_schema" + } + } + }, + "in": "query", + "name": "q" + } + ], + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "properties": { + "add_columns": { + "type": "object" + }, + "edit_columns": { + "type": "object" + }, + "filters": { + "properties": { + "column_name": { + "items": { + "properties": { + "name": { + "description": "The filter name. Will be translated by babel", + "type": "string" + }, + "operator": { + "description": "The filter operation key to use on list filters", + "type": "string" + } + }, + "type": "object" + }, + "type": "array" + } + }, + "type": "object" + }, + "permissions": { + "description": "The user permissions for this API resource", + "items": { + "type": "string" + }, + "type": "array" + } + }, + "type": "object" + } + } + }, + "description": "Item from Model" + }, + "400": { + "$ref": "#/components/responses/400" + }, + "401": { + "$ref": "#/components/responses/401" + }, + "422": { + "$ref": "#/components/responses/422" + }, + "500": { + "$ref": "#/components/responses/500" + } + }, + "security": [ + { + "jwt": [] + } + ], + "tags": [ + "Queries" + ] + } + }, + "/api/v1/saved_query/distinct/{column_name}": { + "get": { + "parameters": [ + { + "in": "path", + "name": "column_name", + "required": true, + "schema": { + "type": "string" + } + }, + { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/get_related_schema" + } + } + }, + "in": "query", + "name": "q" + } + ], + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/DistincResponseSchema" + } + } + }, + "description": "Distinct field data" + }, + "400": { + "$ref": "#/components/responses/400" + }, + "401": { + "$ref": "#/components/responses/401" + }, + "404": { + "$ref": "#/components/responses/404" + }, + "500": { + "$ref": "#/components/responses/500" + } + }, + "security": [ + { + "jwt": [] + } + ], + "tags": [ + "Queries" + ] + } + }, + "/api/v1/saved_query/export/": { + "get": { + "description": "Exports multiple saved queries and downloads them as YAML files", + "parameters": [ + { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/get_export_ids_schema" + } + } + }, + "in": "query", + "name": "q" + } + ], + "responses": { + "200": { + "content": { + "application/zip": { + "schema": { + "format": "binary", + "type": "string" + } + } + }, + "description": "A zip file with saved query(ies) and database(s) as YAML" + }, + "400": { + "$ref": "#/components/responses/400" + }, + "401": { + "$ref": "#/components/responses/401" + }, + "404": { + "$ref": "#/components/responses/404" + }, + "500": { + "$ref": "#/components/responses/500" + } + }, + "security": [ + { + "jwt": [] + } + ], + "tags": [ + "Queries" + ] + } + }, + "/api/v1/saved_query/import/": { + "post": { + "requestBody": { + "content": { + "multipart/form-data": { + "schema": { + "properties": { + "formData": { + "description": "upload file (ZIP)", + "format": "binary", + "type": "string" + }, + "overwrite": { + "description": "overwrite existing saved queries?", + "type": "boolean" + }, + "passwords": { + "description": "JSON map of passwords for each featured database in the ZIP file. If the ZIP includes a database config in the path `databases/MyDatabase.yaml`, the password should be provided in the following format: `{\"databases/MyDatabase.yaml\": \"my_password\"}`.", + "type": "string" + } + }, + "type": "object" + } + } + }, + "required": true + }, + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "properties": { + "message": { + "type": "string" + } + }, + "type": "object" + } + } + }, + "description": "Saved Query import result" + }, + "400": { + "$ref": "#/components/responses/400" + }, + "401": { + "$ref": "#/components/responses/401" + }, + "422": { + "$ref": "#/components/responses/422" + }, + "500": { + "$ref": "#/components/responses/500" + } + }, + "security": [ + { + "jwt": [] + } + ], + "tags": [ + "Queries" + ] + } + }, + "/api/v1/saved_query/related/{column_name}": { + "get": { + "parameters": [ + { + "in": "path", + "name": "column_name", + "required": true, + "schema": { + "type": "string" + } + }, + { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/get_related_schema" + } + } + }, + "in": "query", + "name": "q" + } + ], + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RelatedResponseSchema" + } + } + }, + "description": "Related column data" + }, + "400": { + "$ref": "#/components/responses/400" + }, + "401": { + "$ref": "#/components/responses/401" + }, + "404": { + "$ref": "#/components/responses/404" + }, + "500": { + "$ref": "#/components/responses/500" + } + }, + "security": [ + { + "jwt": [] + } + ], + "tags": [ + "Queries" + ] + } + }, + "/api/v1/saved_query/{pk}": { + "delete": { + "description": "Delete saved query", + "parameters": [ + { + "in": "path", + "name": "pk", + "required": true, + "schema": { + "type": "integer" + } + } + ], + "responses": { + "200": { + "content": { + "application/json": { + "schema": { "properties": { "message": { "type": "string" @@ -19201,6 +20277,120 @@ ] } }, + "/api/v1/sqllab/execute/": { + "post": { + "description": "Starts the execution of a SQL query", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ExecutePayloadSchema" + } + } + }, + "description": "SQL query and params", + "required": true + }, + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/QueryExecutionResponseSchema" + } + } + }, + "description": "Query execution result" + }, + "202": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/QueryExecutionResponseSchema" + } + } + }, + "description": "Query execution result, query still running" + }, + "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" + } + }, + "security": [ + { + "jwt": [] + } + ], + "tags": [ + "SQL Lab" + ] + } + }, + "/api/v1/sqllab/results/": { + "get": { + "parameters": [ + { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/sql_lab_get_results_schema" + } + } + }, + "in": "query", + "name": "q" + } + ], + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/QueryExecutionResponseSchema" + } + } + }, + "description": "SQL query execution result" + }, + "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" + } + }, + "security": [ + { + "jwt": [] + } + ], + "summary": "Gets the result of a SQL query execution", + "tags": [ + "SQL Lab" + ] + } + }, "/api/{version}/_openapi": { "get": { "description": "Get the OpenAPI spec for a specific API version", diff --git a/superset-frontend/cypress-base/cypress/integration/sqllab/query.test.ts b/superset-frontend/cypress-base/cypress/integration/sqllab/query.test.ts index f75a29bc886d..d8cafee98e45 100644 --- a/superset-frontend/cypress-base/cypress/integration/sqllab/query.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/sqllab/query.test.ts @@ -36,7 +36,7 @@ describe('SqlLab query panel', () => { cy.intercept({ method: 'POST', - url: '/superset/sql_json/', + url: '/api/v1/sqllab/execute/', }).as('mockSQLResponse'); cy.get('.TableSelector .Select:eq(0)').click(); @@ -149,7 +149,7 @@ describe('SqlLab query panel', () => { }); it('Create a chart from a query', () => { - cy.intercept('/superset/sql_json/').as('queryFinished'); + cy.intercept('/api/v1/sqllab/execute/').as('queryFinished'); cy.intercept('**/api/v1/explore/**').as('explore'); cy.intercept('**/api/v1/chart/**').as('chart'); diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.test.js b/superset-frontend/src/SqlLab/actions/sqlLab.test.js index ca1e68b5b287..80126ec9fa0d 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.test.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.test.js @@ -55,13 +55,13 @@ describe('async actions', () => { afterEach(fetchMock.resetHistory); - const fetchQueryEndpoint = 'glob:*/superset/results/*'; + const fetchQueryEndpoint = 'glob:*/api/v1/sqllab/results/*'; fetchMock.get( fetchQueryEndpoint, JSON.stringify({ data: mockBigNumber, query: { sqlEditorId: 'dfsadfs' } }), ); - const runQueryEndpoint = 'glob:*/superset/sql_json/'; + const runQueryEndpoint = 'glob:*/api/v1/sqllab/execute/'; fetchMock.post(runQueryEndpoint, `{ "data": ${mockBigNumber} }`); describe('saveQuery', () => { @@ -280,7 +280,8 @@ describe('async actions', () => { }; it('makes the fetch request', async () => { - const runQueryEndpointWithParams = 'glob:*/superset/sql_json/?foo=bar'; + const runQueryEndpointWithParams = + 'glob:*/api/v1/sqllab/execute/?foo=bar'; fetchMock.post( runQueryEndpointWithParams, `{ "data": ${mockBigNumber} }`, diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.jsx b/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.jsx index 614878e49d9f..f82a46072990 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.jsx +++ b/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.jsx @@ -55,7 +55,7 @@ const MOCKED_SQL_EDITOR_HEIGHT = 500; fetchMock.get('glob:*/api/v1/database/*', { result: [] }); fetchMock.get('glob:*/superset/tables/*', { options: [] }); -fetchMock.post('glob:*/sql_json/*', { result: [] }); +fetchMock.post('glob:*/sqllab/execute/*', { result: [] }); const middlewares = [thunk]; const mockStore = configureStore(middlewares); diff --git a/superset/sqllab/api.py b/superset/sqllab/api.py index 746e3d5badfe..13be31f83293 100644 --- a/superset/sqllab/api.py +++ b/superset/sqllab/api.py @@ -21,45 +21,42 @@ from flask_appbuilder.api import expose, protect, rison, safe from flask_appbuilder.models.sqla.interface import SQLAInterface from marshmallow import ValidationError -from superset.views.base import handle_api_exception -from superset.sqllab.sqllab_execution_context import SqlJsonExecutionContext + +from superset import app, is_feature_enabled +from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP +from superset.databases.dao import DatabaseDAO +from superset.extensions import event_logger +from superset.jinja_context import get_template_processor +from superset.models.sql_lab import Query from superset.queries.dao import QueryDAO -from superset.sqllab.sql_json_executer import ( - ASynchronousSqlJsonExecutor, - SqlJsonExecutor, - SynchronousSqlJsonExecutor, -) +from superset.sql_lab import get_sql_results +from superset.sqllab.command_status import SqlJsonExecutionStatus +from superset.sqllab.commands.execute import CommandResult, ExecuteSqlCommand +from superset.sqllab.commands.results import SqlExecutionResultsCommand from superset.sqllab.exceptions import ( QueryIsForbiddenToAccessException, SqlLabException, ) -from superset.sqllab.command_status import SqlJsonExecutionStatus -from superset.sql_lab import get_sql_results -from superset.sqllab.commands.execute import CommandResult, ExecuteSqlCommand from superset.sqllab.execution_context_convertor import ExecutionContextConvertor -from superset.databases.dao import DatabaseDAO -from superset.sqllab.validators import CanAccessQueryValidatorImpl from superset.sqllab.query_render import SqlQueryRenderImpl -from superset.jinja_context import get_template_processor - -from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP -from superset.extensions import event_logger -from superset.models.sql_lab import Query -from superset.sqllab.commands.results import SqlExecutionResultsCommand from superset.sqllab.schemas import ( ExecutePayloadSchema, QueryExecutionResponseSchema, sql_lab_get_results_schema, ) +from superset.sqllab.sql_json_executer import ( + ASynchronousSqlJsonExecutor, + SqlJsonExecutor, + SynchronousSqlJsonExecutor, +) +from superset.sqllab.sqllab_execution_context import SqlJsonExecutionContext +from superset.sqllab.validators import CanAccessQueryValidatorImpl +from superset.views.base import handle_api_exception from superset.views.base_api import ( BaseSupersetModelRestApi, requires_json, statsd_metrics, ) -from superset import ( - app, - is_feature_enabled, -) config = app.config logger = logging.getLogger(__name__) diff --git a/superset/sqllab/commands/results.py b/superset/sqllab/commands/results.py index 190b0e092525..551d6e1a0307 100644 --- a/superset/sqllab/commands/results.py +++ b/superset/sqllab/commands/results.py @@ -19,20 +19,13 @@ import logging from typing import Any, cast, Dict, Optional + from flask_babel import gettext as __, lazy_gettext as _ -from superset import ( - app, - db, - results_backend, - results_backend_use_msgpack, -) +from superset import app, db, results_backend, results_backend_use_msgpack from superset.commands.base import BaseCommand from superset.errors import ErrorLevel, SupersetError, SupersetErrorType -from superset.exceptions import ( - SerializationError, - SupersetErrorException, -) +from superset.exceptions import SerializationError, SupersetErrorException from superset.models.sql_lab import Query from superset.sqllab.utils import apply_display_max_row_configuration_if_require from superset.utils import core as utils @@ -90,7 +83,9 @@ def validate(self) -> None: status=410, ) - self._query = db.session.query(Query).filter_by(results_key=self._key).one_or_none() + self._query = ( + db.session.query(Query).filter_by(results_key=self._key).one_or_none() + ) if self._query is None: raise SupersetErrorException( SupersetError( @@ -109,7 +104,9 @@ def run( ) -> Dict[str, Any]: """Runs arbitrary sql and returns data as json""" self.validate() - payload = utils.zlib_decompress(self._blob, decode=not results_backend_use_msgpack) + payload = utils.zlib_decompress( + self._blob, decode=not results_backend_use_msgpack + ) try: obj = _deserialize_results_payload( payload, self._query, cast(bool, results_backend_use_msgpack) @@ -130,5 +127,5 @@ def run( if self._rows: obj = apply_display_max_row_configuration_if_require(obj, self._rows) - + return obj diff --git a/superset/sqllab/schemas.py b/superset/sqllab/schemas.py index 5e750b3ee0e3..f238fda5c918 100644 --- a/superset/sqllab/schemas.py +++ b/superset/sqllab/schemas.py @@ -46,7 +46,7 @@ class QueryResultSchema(Schema): changedOn = fields.DateTime() changed_on = fields.String() dbId = fields.Integer() - db = fields.String() + db = fields.String() # pylint: disable=invalid-name endDttm = fields.Float() errorMessage = fields.String(allow_none=True) executedSql = fields.String() diff --git a/superset/views/core.py b/superset/views/core.py index 928e0cbc1cc0..d409b6207c3a 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -2177,6 +2177,7 @@ def theme(self) -> FlaskResponse: @has_access_api @expose("/results//") @event_logger.log_this + @deprecated() def results(self, key: str) -> FlaskResponse: return self.results_exec(key) diff --git a/tests/integration_tests/base_tests.py b/tests/integration_tests/base_tests.py index 20e324559363..999f22dd2b89 100644 --- a/tests/integration_tests/base_tests.py +++ b/tests/integration_tests/base_tests.py @@ -347,7 +347,7 @@ def run_sql( json_payload["schema"] = schema resp = self.get_json_resp( - "/superset/sql_json/", raise_on_error=False, json_=json_payload + "/api/v1/sqllab/execute/", raise_on_error=False, json_=json_payload ) if raise_on_error and "error" in resp: raise Exception("run_sql failed") diff --git a/tests/integration_tests/celery_tests.py b/tests/integration_tests/celery_tests.py index da6db727e711..5aafed36e9ce 100644 --- a/tests/integration_tests/celery_tests.py +++ b/tests/integration_tests/celery_tests.py @@ -96,7 +96,7 @@ def run_sql( ): db_id = get_example_database().id return test_client.post( - "/superset/sql_json/", + "/api/v1/sqllab/execute/", json=dict( database_id=db_id, sql=sql, diff --git a/tests/integration_tests/sql_lab/api_tests.py b/tests/integration_tests/sql_lab/api_tests.py index 00c7233a5155..53060c7dbddd 100644 --- a/tests/integration_tests/sql_lab/api_tests.py +++ b/tests/integration_tests/sql_lab/api_tests.py @@ -39,6 +39,7 @@ class TestSqlLabApi(SupersetTestCase): + @mock.patch("superset.sqllab.commands.results.results_backend_use_msgpack", False) def test_execute_required_params(self): self.login() client_id = "{}".format(random.getrandbits(64))[:10] @@ -78,12 +79,19 @@ def test_execute_required_params(self): self.assertDictEqual(resp_data, failed_resp) self.assertEqual(rv.status_code, 400) + from superset import sql_lab as core + + core.results_backend = mock.Mock() + core.results_backend.get.return_value = {} + data = {"sql": "SELECT 1", "database_id": 1, "client_id": client_id} rv = self.client.post( "/api/v1/sqllab/execute/", json=data, ) resp_data = json.loads(rv.data.decode("utf-8")) + print("R:\n") + print(resp_data) self.assertEqual(resp_data.get("status"), "success") self.assertEqual(rv.status_code, 200) @@ -113,7 +121,7 @@ def test_execute_custom_templated(self, sql_lab_mock, mock_dt) -> None: self.delete_fake_db_for_macros() @mock.patch("superset.sqllab.commands.results.results_backend_use_msgpack", False) - def test_display_limit(self): + def test_get_results_with_display_limit(self): from superset.sqllab.commands import results as command command.results_backend = mock.Mock() From 508e3bbf04d2979410ad837fd9d3f874c4f59ff2 Mon Sep 17 00:00:00 2001 From: Diego Medina Date: Mon, 23 Jan 2023 10:14:27 -0300 Subject: [PATCH 3/7] more tests --- tests/integration_tests/sql_lab/api_tests.py | 2 - .../sql_lab/commands_tests.py | 161 ++++++++++++++++++ 2 files changed, 161 insertions(+), 2 deletions(-) create mode 100644 tests/integration_tests/sql_lab/commands_tests.py diff --git a/tests/integration_tests/sql_lab/api_tests.py b/tests/integration_tests/sql_lab/api_tests.py index 53060c7dbddd..f2a68205a95c 100644 --- a/tests/integration_tests/sql_lab/api_tests.py +++ b/tests/integration_tests/sql_lab/api_tests.py @@ -90,8 +90,6 @@ def test_execute_required_params(self): json=data, ) resp_data = json.loads(rv.data.decode("utf-8")) - print("R:\n") - print(resp_data) self.assertEqual(resp_data.get("status"), "success") self.assertEqual(rv.status_code, 200) diff --git a/tests/integration_tests/sql_lab/commands_tests.py b/tests/integration_tests/sql_lab/commands_tests.py new file mode 100644 index 000000000000..74c1fe708210 --- /dev/null +++ b/tests/integration_tests/sql_lab/commands_tests.py @@ -0,0 +1,161 @@ +# 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 import mock, skip +from unittest.mock import patch + +import pytest + +from superset import db, sql_lab +from superset.common.db_query_status import QueryStatus +from superset.errors import SupersetErrorType +from superset.exceptions import SerializationError, SupersetErrorException +from superset.models.core import Database +from superset.models.sql_lab import Query +from superset.sqllab.commands import results +from superset.utils import core as utils +from tests.integration_tests.base_tests import SupersetTestCase + + +class TestSqlExecutionResultsCommand(SupersetTestCase): + @mock.patch("superset.sqllab.commands.results.results_backend_use_msgpack", False) + def test_validation_no_results_backend(self) -> None: + results.results_backend = None + + command = results.SqlExecutionResultsCommand("test", 1000) + + with pytest.raises(SupersetErrorException) as ex_info: + command.run() + assert ( + ex_info.value.error.error_type + == SupersetErrorType.RESULTS_BACKEND_NOT_CONFIGURED_ERROR + ) + + @mock.patch("superset.sqllab.commands.results.results_backend_use_msgpack", False) + def test_validation_data_cannot_be_retrieved(self) -> None: + results.results_backend = mock.Mock() + results.results_backend.get.return_value = None + + command = results.SqlExecutionResultsCommand("test", 1000) + + with pytest.raises(SupersetErrorException) as ex_info: + command.run() + assert ex_info.value.error.error_type == SupersetErrorType.RESULTS_BACKEND_ERROR + + @mock.patch("superset.sqllab.commands.results.results_backend_use_msgpack", False) + def test_validation_query_not_found(self) -> None: + data = [{"col_0": i} for i in range(100)] + payload = { + "status": QueryStatus.SUCCESS, + "query": {"rows": 100}, + "data": data, + } + serialized_payload = sql_lab._serialize_payload(payload, False) + compressed = utils.zlib_compress(serialized_payload) + + results.results_backend = mock.Mock() + results.results_backend.get.return_value = compressed + + command = results.SqlExecutionResultsCommand("test", 1000) + + with pytest.raises(SupersetErrorException) as ex_info: + command.run() + assert ex_info.value.error.error_type == SupersetErrorType.RESULTS_BACKEND_ERROR + + @mock.patch("superset.sqllab.commands.results.results_backend_use_msgpack", False) + def test_validation_query_not_found2(self) -> None: + data = [{"col_0": i} for i in range(104)] + payload = { + "status": QueryStatus.SUCCESS, + "query": {"rows": 104}, + "data": data, + } + serialized_payload = sql_lab._serialize_payload(payload, False) + compressed = utils.zlib_compress(serialized_payload) + + results.results_backend = mock.Mock() + results.results_backend.get.return_value = compressed + + database = Database(database_name="my_database", sqlalchemy_uri="sqlite://") + query_obj = Query( + client_id="foo", + database=database, + tab_name="test_tab", + sql_editor_id="test_editor_id", + sql="select * from bar", + select_sql="select * from bar", + executed_sql="select * from bar", + limit=100, + select_as_cta=False, + rows=104, + error_message="none", + results_key="test_abc", + ) + + db.session.add(database) + db.session.add(query_obj) + + with mock.patch( + "superset.views.utils._deserialize_results_payload", + side_effect=SerializationError(), + ): + with pytest.raises(SupersetErrorException) as ex_info: + command = results.SqlExecutionResultsCommand("test", 1000) + command.run() + assert ( + ex_info.value.error.error_type + == SupersetErrorType.RESULTS_BACKEND_ERROR + ) + + @mock.patch("superset.sqllab.commands.results.results_backend_use_msgpack", False) + def test_run_succeeds(self) -> None: + data = [{"col_0": i} for i in range(104)] + payload = { + "status": QueryStatus.SUCCESS, + "query": {"rows": 104}, + "data": data, + } + serialized_payload = sql_lab._serialize_payload(payload, False) + compressed = utils.zlib_compress(serialized_payload) + + results.results_backend = mock.Mock() + results.results_backend.get.return_value = compressed + + database = Database(database_name="my_database", sqlalchemy_uri="sqlite://") + query_obj = Query( + client_id="foo", + database=database, + tab_name="test_tab", + sql_editor_id="test_editor_id", + sql="select * from bar", + select_sql="select * from bar", + executed_sql="select * from bar", + limit=100, + select_as_cta=False, + rows=104, + error_message="none", + results_key="test_abc", + ) + + db.session.add(database) + db.session.add(query_obj) + + command = results.SqlExecutionResultsCommand("test_abc", 1000) + result = command.run() + + assert result.get("status") == "success" + assert result.get("query").get("rows") == 104 + assert result.get("data") == data From 3119930fcb8fb92d276c601f4f64b94785ab2c16 Mon Sep 17 00:00:00 2001 From: Diego Medina Date: Mon, 23 Jan 2023 13:32:45 -0300 Subject: [PATCH 4/7] test --- superset-frontend/src/SqlLab/actions/sqlLab.js | 4 ++-- superset-frontend/src/SqlLab/fixtures.ts | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index f94109184895..71f210e5791e 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -320,10 +320,10 @@ export function fetchQueryResults(query, displayLimit) { const queryParams = rison.encode({ key: query.resultsKey, - rows: displayLimit, + rows: displayLimit || null, }); - SupersetClient.get({ + return SupersetClient.get({ endpoint: `/api/v1/sqllab/results/?q=${queryParams}`, parseMethod: 'json-bigint', }) diff --git a/superset-frontend/src/SqlLab/fixtures.ts b/superset-frontend/src/SqlLab/fixtures.ts index 5c3b06a1074d..456a83a3faf1 100644 --- a/superset-frontend/src/SqlLab/fixtures.ts +++ b/superset-frontend/src/SqlLab/fixtures.ts @@ -688,6 +688,7 @@ export const query = { sql: 'SELECT * FROM something', description: 'test description', schema: 'test schema', + resultsKey: 'test', }; export const queryId = 'clientId2353'; From 99da4713014aa88b352a840eebaf1eb61a0a1f14 Mon Sep 17 00:00:00 2001 From: Diego Medina Date: Tue, 24 Jan 2023 14:46:55 -0300 Subject: [PATCH 5/7] pr comments --- docs/static/resources/openapi.json | 215 +++++++++++++------ superset/sqllab/api.py | 10 +- superset/views/base_api.py | 1 + tests/integration_tests/sql_lab/api_tests.py | 5 + 4 files changed, 165 insertions(+), 66 deletions(-) diff --git a/docs/static/resources/openapi.json b/docs/static/resources/openapi.json index 7675ad52d22b..8984d70746e8 100644 --- a/docs/static/resources/openapi.json +++ b/docs/static/resources/openapi.json @@ -61,6 +61,94 @@ }, "description": "Not found" }, + "410": { + "content": { + "application/json": { + "schema": { + "properties": { + "errors": { + "items": { + "properties": { + "error_type": { + "enum": [ + "FRONTEND_CSRF_ERROR", + "FRONTEND_NETWORK_ERROR", + "FRONTEND_TIMEOUT_ERROR", + "GENERIC_DB_ENGINE_ERROR", + "COLUMN_DOES_NOT_EXIST_ERROR", + "TABLE_DOES_NOT_EXIST_ERROR", + "SCHEMA_DOES_NOT_EXIST_ERROR", + "CONNECTION_INVALID_USERNAME_ERROR", + "CONNECTION_INVALID_PASSWORD_ERROR", + "CONNECTION_INVALID_HOSTNAME_ERROR", + "CONNECTION_PORT_CLOSED_ERROR", + "CONNECTION_INVALID_PORT_ERROR", + "CONNECTION_HOST_DOWN_ERROR", + "CONNECTION_ACCESS_DENIED_ERROR", + "CONNECTION_UNKNOWN_DATABASE_ERROR", + "CONNECTION_DATABASE_PERMISSIONS_ERROR", + "CONNECTION_MISSING_PARAMETERS_ERROR", + "OBJECT_DOES_NOT_EXIST_ERROR", + "SYNTAX_ERROR", + "CONNECTION_DATABASE_TIMEOUT", + "VIZ_GET_DF_ERROR", + "UNKNOWN_DATASOURCE_TYPE_ERROR", + "FAILED_FETCHING_DATASOURCE_INFO_ERROR", + "TABLE_SECURITY_ACCESS_ERROR", + "DATASOURCE_SECURITY_ACCESS_ERROR", + "DATABASE_SECURITY_ACCESS_ERROR", + "QUERY_SECURITY_ACCESS_ERROR", + "MISSING_OWNERSHIP_ERROR", + "USER_ACTIVITY_SECURITY_ACCESS_ERROR", + "BACKEND_TIMEOUT_ERROR", + "DATABASE_NOT_FOUND_ERROR", + "MISSING_TEMPLATE_PARAMS_ERROR", + "INVALID_TEMPLATE_PARAMS_ERROR", + "RESULTS_BACKEND_NOT_CONFIGURED_ERROR", + "DML_NOT_ALLOWED_ERROR", + "INVALID_CTAS_QUERY_ERROR", + "INVALID_CVAS_QUERY_ERROR", + "SQLLAB_TIMEOUT_ERROR", + "RESULTS_BACKEND_ERROR", + "ASYNC_WORKERS_ERROR", + "ADHOC_SUBQUERY_NOT_ALLOWED_ERROR", + "GENERIC_COMMAND_ERROR", + "GENERIC_BACKEND_ERROR", + "INVALID_PAYLOAD_FORMAT_ERROR", + "INVALID_PAYLOAD_SCHEMA_ERROR", + "REPORT_NOTIFICATION_ERROR" + ], + "type": "string" + }, + "extra": { + "type": "object" + }, + "level": { + "enum": [ + "info", + "warning", + "error" + ], + "type": "string" + }, + "message": { + "type": "string" + } + }, + "type": "object" + }, + "type": "array" + }, + "message": { + "type": "string" + } + }, + "type": "object" + } + } + }, + "description": "Gone" + }, "422": { "content": { "application/json": { @@ -1680,7 +1768,7 @@ "type": "string" }, "changed_by": { - "$ref": "#/components/schemas/ChartDataRestApi.get_list.User" + "$ref": "#/components/schemas/ChartDataRestApi.get_list.User1" }, "changed_by_name": { "readOnly": true @@ -1695,7 +1783,7 @@ "readOnly": true }, "created_by": { - "$ref": "#/components/schemas/ChartDataRestApi.get_list.User3" + "$ref": "#/components/schemas/ChartDataRestApi.get_list.User2" }, "created_on_delta_humanized": { "readOnly": true @@ -1742,10 +1830,10 @@ "type": "string" }, "last_saved_by": { - "$ref": "#/components/schemas/ChartDataRestApi.get_list.User1" + "$ref": "#/components/schemas/ChartDataRestApi.get_list.User3" }, "owners": { - "$ref": "#/components/schemas/ChartDataRestApi.get_list.User2" + "$ref": "#/components/schemas/ChartDataRestApi.get_list.User" }, "params": { "nullable": true, @@ -1809,14 +1897,23 @@ "maxLength": 64, "type": "string" }, + "id": { + "format": "int32", + "type": "integer" + }, "last_name": { "maxLength": 64, "type": "string" + }, + "username": { + "maxLength": 64, + "type": "string" } }, "required": [ "first_name", - "last_name" + "last_name", + "username" ], "type": "object" }, @@ -1826,10 +1923,6 @@ "maxLength": 64, "type": "string" }, - "id": { - "format": "int32", - "type": "integer" - }, "last_name": { "maxLength": 64, "type": "string" @@ -1854,16 +1947,11 @@ "last_name": { "maxLength": 64, "type": "string" - }, - "username": { - "maxLength": 64, - "type": "string" } }, "required": [ "first_name", - "last_name", - "username" + "last_name" ], "type": "object" }, @@ -2472,7 +2560,7 @@ "type": "string" }, "changed_by": { - "$ref": "#/components/schemas/ChartRestApi.get_list.User" + "$ref": "#/components/schemas/ChartRestApi.get_list.User1" }, "changed_by_name": { "readOnly": true @@ -2487,7 +2575,7 @@ "readOnly": true }, "created_by": { - "$ref": "#/components/schemas/ChartRestApi.get_list.User3" + "$ref": "#/components/schemas/ChartRestApi.get_list.User2" }, "created_on_delta_humanized": { "readOnly": true @@ -2534,10 +2622,10 @@ "type": "string" }, "last_saved_by": { - "$ref": "#/components/schemas/ChartRestApi.get_list.User1" + "$ref": "#/components/schemas/ChartRestApi.get_list.User3" }, "owners": { - "$ref": "#/components/schemas/ChartRestApi.get_list.User2" + "$ref": "#/components/schemas/ChartRestApi.get_list.User" }, "params": { "nullable": true, @@ -2601,14 +2689,23 @@ "maxLength": 64, "type": "string" }, + "id": { + "format": "int32", + "type": "integer" + }, "last_name": { "maxLength": 64, "type": "string" + }, + "username": { + "maxLength": 64, + "type": "string" } }, "required": [ "first_name", - "last_name" + "last_name", + "username" ], "type": "object" }, @@ -2618,10 +2715,6 @@ "maxLength": 64, "type": "string" }, - "id": { - "format": "int32", - "type": "integer" - }, "last_name": { "maxLength": 64, "type": "string" @@ -2646,16 +2739,11 @@ "last_name": { "maxLength": 64, "type": "string" - }, - "username": { - "maxLength": 64, - "type": "string" } }, "required": [ "first_name", - "last_name", - "username" + "last_name" ], "type": "object" }, @@ -3312,7 +3400,7 @@ "type": "string" }, "changed_by": { - "$ref": "#/components/schemas/DashboardRestApi.get_list.User" + "$ref": "#/components/schemas/DashboardRestApi.get_list.User1" }, "changed_by_name": { "readOnly": true @@ -3353,7 +3441,7 @@ "type": "string" }, "owners": { - "$ref": "#/components/schemas/DashboardRestApi.get_list.User1" + "$ref": "#/components/schemas/DashboardRestApi.get_list.User" }, "position_json": { "nullable": true, @@ -3401,6 +3489,10 @@ }, "DashboardRestApi.get_list.User": { "properties": { + "email": { + "maxLength": 64, + "type": "string" + }, "first_name": { "maxLength": 64, "type": "string" @@ -3419,6 +3511,7 @@ } }, "required": [ + "email", "first_name", "last_name", "username" @@ -3427,10 +3520,6 @@ }, "DashboardRestApi.get_list.User1": { "properties": { - "email": { - "maxLength": 64, - "type": "string" - }, "first_name": { "maxLength": 64, "type": "string" @@ -3449,7 +3538,6 @@ } }, "required": [ - "email", "first_name", "last_name", "username" @@ -4793,7 +4881,7 @@ "type": "integer" }, "changed_by": { - "$ref": "#/components/schemas/DatasetRestApi.get.User" + "$ref": "#/components/schemas/DatasetRestApi.get.User1" }, "changed_on": { "format": "date-time", @@ -4807,7 +4895,7 @@ "$ref": "#/components/schemas/DatasetRestApi.get.TableColumn" }, "created_by": { - "$ref": "#/components/schemas/DatasetRestApi.get.User1" + "$ref": "#/components/schemas/DatasetRestApi.get.User2" }, "created_on": { "format": "date-time", @@ -4871,7 +4959,7 @@ "type": "integer" }, "owners": { - "$ref": "#/components/schemas/DatasetRestApi.get.User2" + "$ref": "#/components/schemas/DatasetRestApi.get.User" }, "schema": { "maxLength": 255, @@ -5068,14 +5156,23 @@ "maxLength": 64, "type": "string" }, + "id": { + "format": "int32", + "type": "integer" + }, "last_name": { "maxLength": 64, "type": "string" + }, + "username": { + "maxLength": 64, + "type": "string" } }, "required": [ "first_name", - "last_name" + "last_name", + "username" ], "type": "object" }, @@ -5102,23 +5199,14 @@ "maxLength": 64, "type": "string" }, - "id": { - "format": "int32", - "type": "integer" - }, "last_name": { "maxLength": 64, "type": "string" - }, - "username": { - "maxLength": 64, - "type": "string" } }, "required": [ "first_name", - "last_name", - "username" + "last_name" ], "type": "object" }, @@ -6974,7 +7062,7 @@ "type": "boolean" }, "changed_by": { - "$ref": "#/components/schemas/ReportScheduleRestApi.get_list.User" + "$ref": "#/components/schemas/ReportScheduleRestApi.get_list.User1" }, "changed_on": { "format": "date-time", @@ -7040,7 +7128,7 @@ "type": "string" }, "owners": { - "$ref": "#/components/schemas/ReportScheduleRestApi.get_list.User1" + "$ref": "#/components/schemas/ReportScheduleRestApi.get_list.User" }, "recipients": { "$ref": "#/components/schemas/ReportScheduleRestApi.get_list.ReportRecipients" @@ -7084,6 +7172,10 @@ "maxLength": 64, "type": "string" }, + "id": { + "format": "int32", + "type": "integer" + }, "last_name": { "maxLength": 64, "type": "string" @@ -7101,10 +7193,6 @@ "maxLength": 64, "type": "string" }, - "id": { - "format": "int32", - "type": "integer" - }, "last_name": { "maxLength": 64, "type": "string" @@ -20318,12 +20406,12 @@ "401": { "$ref": "#/components/responses/401" }, + "403": { + "$ref": "#/components/responses/403" + }, "404": { "$ref": "#/components/responses/404" }, - "422": { - "$ref": "#/components/responses/422" - }, "500": { "$ref": "#/components/responses/500" } @@ -20370,11 +20458,14 @@ "401": { "$ref": "#/components/responses/401" }, + "403": { + "$ref": "#/components/responses/403" + }, "404": { "$ref": "#/components/responses/404" }, - "422": { - "$ref": "#/components/responses/422" + "410": { + "$ref": "#/components/responses/410" }, "500": { "$ref": "#/components/responses/500" diff --git a/superset/sqllab/api.py b/superset/sqllab/api.py index 13be31f83293..d8b75997dbd0 100644 --- a/superset/sqllab/api.py +++ b/superset/sqllab/api.py @@ -121,10 +121,12 @@ def get_results(self, **kwargs: Any) -> Response: $ref: '#/components/responses/400' 401: $ref: '#/components/responses/401' + 403: + $ref: '#/components/responses/403' 404: $ref: '#/components/responses/404' - 422: - $ref: '#/components/responses/422' + 410: + $ref: '#/components/responses/410' 500: $ref: '#/components/responses/500' """ @@ -174,10 +176,10 @@ def execute_sql_query(self) -> Response: $ref: '#/components/responses/400' 401: $ref: '#/components/responses/401' + 403: + $ref: '#/components/responses/403' 404: $ref: '#/components/responses/404' - 422: - $ref: '#/components/responses/422' 500: $ref: '#/components/responses/500' """ diff --git a/superset/views/base_api.py b/superset/views/base_api.py index 47fc611ba21a..074be9acb1ca 100644 --- a/superset/views/base_api.py +++ b/superset/views/base_api.py @@ -251,6 +251,7 @@ class BaseSupersetModelRestApi(ModelRestApi): "401": {"description": "Unauthorized", "content": error_payload_content}, "403": {"description": "Forbidden", "content": error_payload_content}, "404": {"description": "Not found", "content": error_payload_content}, + "410": {"description": "Gone", "content": error_payload_content}, "422": { "description": "Could not process entity", "content": error_payload_content, diff --git a/tests/integration_tests/sql_lab/api_tests.py b/tests/integration_tests/sql_lab/api_tests.py index f2a68205a95c..4c2080ad4cc2 100644 --- a/tests/integration_tests/sql_lab/api_tests.py +++ b/tests/integration_tests/sql_lab/api_tests.py @@ -79,11 +79,16 @@ def test_execute_required_params(self): self.assertDictEqual(resp_data, failed_resp) self.assertEqual(rv.status_code, 400) + @mock.patch("superset.sqllab.commands.results.results_backend_use_msgpack", False) + def test_execute_valid_request(self) -> None: from superset import sql_lab as core core.results_backend = mock.Mock() core.results_backend.get.return_value = {} + self.login() + client_id = "{}".format(random.getrandbits(64))[:10] + data = {"sql": "SELECT 1", "database_id": 1, "client_id": client_id} rv = self.client.post( "/api/v1/sqllab/execute/", From 7437aaac6a894232303c6f32643e82b9f8982792 Mon Sep 17 00:00:00 2001 From: Diego Medina Date: Wed, 25 Jan 2023 23:05:30 -0300 Subject: [PATCH 6/7] pr comments --- superset/sqllab/api.py | 24 ++++++++----- .../sqllab/execution_context_convertor.py | 34 ++++++------------- 2 files changed, 26 insertions(+), 32 deletions(-) diff --git a/superset/sqllab/api.py b/superset/sqllab/api.py index d8b75997dbd0..ae32740d6d02 100644 --- a/superset/sqllab/api.py +++ b/superset/sqllab/api.py @@ -17,6 +17,7 @@ import logging from typing import Any, cast, Dict, Optional +import simplejson as json from flask import request, Response from flask_appbuilder.api import expose, protect, rison, safe from flask_appbuilder.models.sqla.interface import SQLAInterface @@ -51,7 +52,9 @@ ) from superset.sqllab.sqllab_execution_context import SqlJsonExecutionContext from superset.sqllab.validators import CanAccessQueryValidatorImpl -from superset.views.base import handle_api_exception +from superset.superset_typing import FlaskResponse +from superset.utils import core as utils +from superset.views.base import handle_api_exception, json_success from superset.views.base_api import ( BaseSupersetModelRestApi, requires_json, @@ -88,16 +91,14 @@ class SqlLabRestApi(BaseSupersetModelRestApi): @expose("/results/") @protect() - @safe @statsd_metrics @rison(sql_lab_get_results_schema) - @handle_api_exception @event_logger.log_this_with_context( action=lambda self, *args, **kwargs: f"{self.__class__.__name__}" f".get_results", log_to_statsd=False, ) - def get_results(self, **kwargs: Any) -> Response: + def get_results(self, **kwargs: Any) -> FlaskResponse: """Gets the result of a SQL query execution --- get: @@ -134,19 +135,24 @@ def get_results(self, **kwargs: Any) -> Response: key = params.get("key") rows = params.get("rows") result = SqlExecutionResultsCommand(key=key, rows=rows).run() - return self.response(200, **result) + # return the result without special encoding + return json_success( + json.dumps( + result, default=utils.json_iso_dttm_ser, ignore_nan=True, encoding=None + ), + 200, + ) @expose("/execute/", methods=["POST"]) @protect() @statsd_metrics @requires_json - @handle_api_exception @event_logger.log_this_with_context( action=lambda self, *args, **kwargs: f"{self.__class__.__name__}" f".get_results", log_to_statsd=False, ) - def execute_sql_query(self) -> Response: + def execute_sql_query(self) -> FlaskResponse: """Executes a SQL query --- post: @@ -201,7 +207,8 @@ def execute_sql_query(self) -> Response: if command_result["status"] == SqlJsonExecutionStatus.QUERY_IS_RUNNING else 200 ) - return self.response(response_status, **command_result["payload"]) + # return the execution result without special encoding + return json_success(command_result["payload"], response_status) except SqlLabException as ex: payload = {"errors": [ex.to_dict()]} @@ -219,7 +226,6 @@ def _create_sql_json_command( execution_context, query_dao ) execution_context_convertor = ExecutionContextConvertor() - execution_context_convertor.set_serialize_to_json(False) execution_context_convertor.set_max_row_in_display( int(config.get("DISPLAY_MAX_ROW")) # type: ignore ) diff --git a/superset/sqllab/execution_context_convertor.py b/superset/sqllab/execution_context_convertor.py index d788d567a508..f49fbd9a31db 100644 --- a/superset/sqllab/execution_context_convertor.py +++ b/superset/sqllab/execution_context_convertor.py @@ -17,7 +17,7 @@ from __future__ import annotations import logging -from typing import Any, Dict, TYPE_CHECKING, Union +from typing import Any, Dict, TYPE_CHECKING import simplejson as json @@ -34,7 +34,6 @@ class ExecutionContextConvertor: - _serialize_to_json = True _max_row_in_display_configuration: int # pylint: disable=invalid-name _exc_status: SqlJsonExecutionStatus payload: Dict[str, Any] @@ -42,9 +41,6 @@ class ExecutionContextConvertor: def set_max_row_in_display(self, value: int) -> None: self._max_row_in_display_configuration = value # pylint: disable=invalid-name - def set_serialize_to_json(self, value: bool) -> None: - self._serialize_to_json = value - def set_payload( self, execution_context: SqlJsonExecutionContext, @@ -56,25 +52,17 @@ def set_payload( else: self.payload = execution_context.query.to_dict() - def serialize_payload(self) -> Union[Dict[str, Any], str]: + def serialize_payload(self) -> str: if self._exc_status == SqlJsonExecutionStatus.HAS_RESULTS: - payload = apply_display_max_row_configuration_if_require( - self.payload, self._max_row_in_display_configuration - ) - return ( - json.dumps( - payload, - default=utils.pessimistic_json_iso_dttm_ser, - ignore_nan=True, - encoding=None, - ) - if self._serialize_to_json - else payload + return json.dumps( + apply_display_max_row_configuration_if_require( + self.payload, self._max_row_in_display_configuration + ), + default=utils.pessimistic_json_iso_dttm_ser, + ignore_nan=True, + encoding=None, ) - payload = {"query": self.payload} - return ( - json.dumps(payload, default=utils.json_int_dttm_ser, ignore_nan=True) - if self._serialize_to_json - else payload + return json.dumps( + {"query": self.payload}, default=utils.json_int_dttm_ser, ignore_nan=True ) From 386979f6b1ce1aecd987671a4673d04e61ae919a Mon Sep 17 00:00:00 2001 From: Diego Medina Date: Wed, 25 Jan 2023 23:12:10 -0300 Subject: [PATCH 7/7] pr comments --- superset/sqllab/api.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/superset/sqllab/api.py b/superset/sqllab/api.py index ae32740d6d02..3827cc32e16b 100644 --- a/superset/sqllab/api.py +++ b/superset/sqllab/api.py @@ -18,8 +18,8 @@ from typing import Any, cast, Dict, Optional import simplejson as json -from flask import request, Response -from flask_appbuilder.api import expose, protect, rison, safe +from flask import request +from flask_appbuilder.api import expose, protect, rison from flask_appbuilder.models.sqla.interface import SQLAInterface from marshmallow import ValidationError @@ -54,7 +54,7 @@ from superset.sqllab.validators import CanAccessQueryValidatorImpl from superset.superset_typing import FlaskResponse from superset.utils import core as utils -from superset.views.base import handle_api_exception, json_success +from superset.views.base import json_success from superset.views.base_api import ( BaseSupersetModelRestApi, requires_json,