chore(sqllab): Remove validation result from state

superset-frontend/src/SqlLab/actions/sqlLab.js

@@ -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';
@@ -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 };
 }
@@ -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({

superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx

@@ -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;
@@ -96,8 +97,8 @@ const AceEditorWrapper = ({
     'id',
     'dbId',
     'sql',
-    'validationResult',
     'schema',
+    'templateParams',
   ]);
   const { data: schemaOptions } = useSchemas({
     ...(autocomplete && { dbId: queryEditor.dbId }),
@@ -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
@@ -313,7 +305,7 @@ const AceEditorWrapper = ({
       editorProps={{ $blockScrolling: true }}
       enableLiveAutocompletion={autocomplete}
       value={sql}
-      annotations={getAceAnnotations()}
+      annotations={annotations}
     />
   );
 };

superset-frontend/src/SqlLab/components/AceEditorWrapper/useAnnotations.test.ts

@@ -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 },
+  );
+});