Skip to content

Commit

Permalink
chore(sqllab): Remove validation result from state (#24082)
Browse files Browse the repository at this point in the history
  • Loading branch information
justinpark authored Jun 8, 2023
1 parent 7e626c0 commit c4242a3
Show file tree
Hide file tree
Showing 11 changed files with 457 additions and 195 deletions.
61 changes: 0 additions & 61 deletions superset-frontend/src/SqlLab/actions/sqlLab.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,6 @@ export const CLEAR_QUERY_RESULTS = 'CLEAR_QUERY_RESULTS';
export const REMOVE_DATA_PREVIEW = 'REMOVE_DATA_PREVIEW';
export const CHANGE_DATA_PREVIEW_ID = 'CHANGE_DATA_PREVIEW_ID';

export const START_QUERY_VALIDATION = 'START_QUERY_VALIDATION';
export const QUERY_VALIDATION_RETURNED = 'QUERY_VALIDATION_RETURNED';
export const QUERY_VALIDATION_FAILED = 'QUERY_VALIDATION_FAILED';
export const COST_ESTIMATE_STARTED = 'COST_ESTIMATE_STARTED';
export const COST_ESTIMATE_RETURNED = 'COST_ESTIMATE_RETURNED';
export const COST_ESTIMATE_FAILED = 'COST_ESTIMATE_FAILED';
Expand Down Expand Up @@ -139,21 +136,6 @@ export function resetState() {
return { type: RESET_STATE };
}

export function startQueryValidation(query) {
Object.assign(query, {
id: query.id ? query.id : shortid.generate(),
});
return { type: START_QUERY_VALIDATION, query };
}

export function queryValidationReturned(query, results) {
return { type: QUERY_VALIDATION_RETURNED, query, results };
}

export function queryValidationFailed(query, message, error) {
return { type: QUERY_VALIDATION_FAILED, query, message, error };
}

export function updateQueryEditor(alterations) {
return { type: UPDATE_QUERY_EDITOR, alterations };
}
Expand Down Expand Up @@ -440,49 +422,6 @@ export function reRunQuery(query) {
};
}

export function validateQuery(queryEditor, sql) {
return function (dispatch, getState) {
const {
sqlLab: { unsavedQueryEditor },
} = getState();
const qe = {
...queryEditor,
...(queryEditor.id === unsavedQueryEditor.id && unsavedQueryEditor),
};

const query = {
dbId: qe.dbId,
sql,
sqlEditorId: qe.id,
schema: qe.schema,
templateParams: qe.templateParams,
};
dispatch(startQueryValidation(query));

const postPayload = {
schema: query.schema,
sql: query.sql,
template_params: query.templateParams,
};

return SupersetClient.post({
endpoint: `/api/v1/database/${query.dbId}/validate_sql/`,
body: JSON.stringify(postPayload),
headers: { 'Content-Type': 'application/json' },
})
.then(({ json }) => dispatch(queryValidationReturned(query, json.result)))
.catch(response =>
getClientErrorObject(response.result).then(error => {
let message = error.error || error.statusText || t('Unknown error');
if (message.includes('CSRF token')) {
message = t(COMMON_ERR_MESSAGES.SESSION_TIMED_OUT);
}
dispatch(queryValidationFailed(query, message, error));
}),
);
};
}

export function postStopQuery(query) {
return function (dispatch) {
return SupersetClient.post({
Expand Down
26 changes: 9 additions & 17 deletions superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import {
import useQueryEditor from 'src/SqlLab/hooks/useQueryEditor';
import { useSchemas, useTables } from 'src/hooks/apiResources';
import { useDatabaseFunctionsQuery } from 'src/hooks/apiResources/databaseFunctions';
import { useAnnotations } from './useAnnotations';

type HotKey = {
key: string;
Expand Down Expand Up @@ -96,8 +97,8 @@ const AceEditorWrapper = ({
'id',
'dbId',
'sql',
'validationResult',
'schema',
'templateParams',
]);
const { data: schemaOptions } = useSchemas({
...(autocomplete && { dbId: queryEditor.dbId }),
Expand Down Expand Up @@ -286,21 +287,12 @@ const AceEditorWrapper = ({

setWords(words);
}

const getAceAnnotations = () => {
const { validationResult } = queryEditor;
const resultIsReady = validationResult?.completed;
if (resultIsReady && validationResult?.errors?.length) {
const errors = validationResult.errors.map((err: any) => ({
type: 'error',
row: err.line_number - 1,
column: err.start_column - 1,
text: err.message,
}));
return errors;
}
return [];
};
const { data: annotations } = useAnnotations({
dbId: queryEditor.dbId,
schema: queryEditor.schema,
sql: currentSql,
templateParams: queryEditor.templateParams,
});

return (
<StyledAceEditor
Expand All @@ -313,7 +305,7 @@ const AceEditorWrapper = ({
editorProps={{ $blockScrolling: true }}
enableLiveAutocompletion={autocomplete}
value={sql}
annotations={getAceAnnotations()}
annotations={annotations}
/>
);
};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
/**
* 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 fetchMock from 'fetch-mock';
import { act, renderHook } from '@testing-library/react-hooks';
import {
createWrapper,
defaultStore as store,
} from 'spec/helpers/testing-library';
import { api } from 'src/hooks/apiResources/queryApi';
import { initialState } from 'src/SqlLab/fixtures';
import COMMON_ERR_MESSAGES from 'src/utils/errorMessages';
import { useAnnotations } from './useAnnotations';

const fakeApiResult = {
result: [
{
end_column: null,
line_number: 3,
message: 'ERROR: syntax error at or near ";"',
start_column: null,
},
],
};
const expectDbId = 'db1';
const expectSchema = 'my_schema';
const expectSql = 'SELECT * from example_table';
const expectTemplateParams = '{"a": 1, "v": "str"}';
const expectValidatorEngine = 'defined_validator';
const queryValidationApiRoute = `glob:*/api/v1/database/${expectDbId}/validate_sql/`;

jest.mock('@superset-ui/core', () => ({
...jest.requireActual('@superset-ui/core'),
t: (str: string) => str,
}));

afterEach(() => {
fetchMock.reset();
act(() => {
store.dispatch(api.util.resetApiState());
});
});

beforeEach(() => {
fetchMock.post(queryValidationApiRoute, fakeApiResult);
});

const initialize = (withValidator = false) => {
if (withValidator) {
return renderHook(
() =>
useAnnotations({
sql: expectSql,
dbId: expectDbId,
schema: expectSchema,
templateParams: expectTemplateParams,
}),
{
wrapper: createWrapper({
useRedux: true,
initialState: {
...initialState,
sqlLab: {
...initialState.sqlLab,
databases: {
[expectDbId]: {
backend: expectValidatorEngine,
},
},
},
common: {
conf: {
SQL_VALIDATORS_BY_ENGINE: {
[expectValidatorEngine]: true,
},
},
},
},
}),
},
);
}
return renderHook(
() =>
useAnnotations({
sql: expectSql,
dbId: expectDbId,
schema: expectSchema,
templateParams: expectTemplateParams,
}),
{
wrapper: createWrapper({
useRedux: true,
store,
}),
},
);
};

test('skips fetching validation if validator is undefined', () => {
const { result } = initialize();
expect(result.current.data).toEqual([]);
expect(fetchMock.calls(queryValidationApiRoute)).toHaveLength(0);
});

test('returns validation if validator is configured', async () => {
const { result, waitFor } = initialize(true);
await waitFor(() =>
expect(fetchMock.calls(queryValidationApiRoute)).toHaveLength(1),
);
expect(result.current.data).toEqual(
fakeApiResult.result.map(err => ({
type: 'error',
row: (err.line_number || 0) - 1,
column: (err.start_column || 0) - 1,
text: err.message,
})),
);
});

test('returns server error description', async () => {
const errorMessage = 'Unexpected validation api error';
fetchMock.post(
queryValidationApiRoute,
{
throws: new Error(errorMessage),
},
{ overwriteRoutes: true },
);
const { result, waitFor } = initialize(true);
await waitFor(
() =>
expect(result.current.data).toEqual([
{
type: 'error',
row: 0,
column: 0,
text: `The server failed to validate your query.\n${errorMessage}`,
},
]),
{ timeout: 5000 },
);
});

test('returns sesion expire description when CSRF token expired', async () => {
const errorMessage = 'CSRF token expired';
fetchMock.post(
queryValidationApiRoute,
{
throws: new Error(errorMessage),
},
{ overwriteRoutes: true },
);
const { result, waitFor } = initialize(true);
await waitFor(
() =>
expect(result.current.data).toEqual([
{
type: 'error',
row: 0,
column: 0,
text: `The server failed to validate your query.\n${COMMON_ERR_MESSAGES.SESSION_TIMED_OUT}`,
},
]),
{ timeout: 5000 },
);
});
Loading

0 comments on commit c4242a3

Please sign in to comment.