Skip to content

Commit

Permalink
chore: Migrate /superset/tables/* to API v1 (apache#22501)
Browse files Browse the repository at this point in the history
  • Loading branch information
diegomedina248 authored and PawankumarES committed Feb 13, 2023
1 parent 6197cef commit 2fd8f4c
Show file tree
Hide file tree
Showing 17 changed files with 623 additions and 65 deletions.
274 changes: 236 additions & 38 deletions docs/static/resources/openapi.json

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ describe('SqlLab query panel', () => {
});

it.skip('successfully saves a query', () => {
cy.intercept('superset/tables/**').as('getTables');
cy.intercept('api/v1/database/**/tables/**').as('getTables');
cy.intercept('savedqueryviewapi/**').as('getSavedQuery');

const query =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ jest.mock('src/SqlLab/components/SqlEditorLeftBar', () => () => (
const MOCKED_SQL_EDITOR_HEIGHT = 500;

fetchMock.get('glob:*/api/v1/database/*', { result: [] });
fetchMock.get('glob:*/superset/tables/*', { options: [] });
fetchMock.get('glob:*/api/v1/database/*/tables/*', { options: [] });
fetchMock.post('glob:*/sqllab/execute/*', { result: [] });

const middlewares = [thunk];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,14 @@ const mockStore = configureStore(middlewares);
const store = mockStore(initialState);

fetchMock.get('glob:*/api/v1/database/*/schemas/?*', { result: [] });
fetchMock.get('glob:*/superset/tables/**', {
options: [
fetchMock.get('glob:*/api/v1/database/*/tables/*', {
count: 1,
result: [
{
label: 'ab_user',
value: 'ab_user',
},
],
tableLength: 1,
});

const renderAndWait = (props, store) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ const getSchemaMockFunction = async () =>
const getTableMockFunction = async () =>
({
json: {
options: [
count: 4,
result: [
{ label: 'table_a', value: 'table_a' },
{ label: 'table_b', value: 'table_b' },
{ label: 'table_c', value: 'table_c' },
Expand Down
18 changes: 10 additions & 8 deletions superset-frontend/src/hooks/apiResources/tables.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ import { useTables } from './tables';

const fakeApiResult = {
json: {
options: [
count: 2,
result: [
{
id: 1,
name: 'fake api result1',
Expand All @@ -35,13 +36,13 @@ const fakeApiResult = {
label: 'fake api label2',
},
],
tableLength: 2,
},
};

const fakeHasMoreApiResult = {
json: {
options: [
count: 4,
result: [
{
id: 1,
name: 'fake api result1',
Expand All @@ -53,17 +54,16 @@ const fakeHasMoreApiResult = {
label: 'fake api label2',
},
],
tableLength: 4,
},
};

const expectedData = {
...fakeApiResult.json,
options: [...fakeApiResult.json.result],
hasMore: false,
};

const expectedHasMoreData = {
...fakeHasMoreApiResult.json,
options: [...fakeHasMoreApiResult.json.result],
hasMore: true,
};

Expand Down Expand Up @@ -103,15 +103,17 @@ describe('useTables hook', () => {
});
expect(SupersetClient.get).toHaveBeenCalledTimes(1);
expect(SupersetClient.get).toHaveBeenCalledWith({
endpoint: `/superset/tables/${expectDbId}/${expectedSchema}/${forceRefresh}/`,
endpoint: `/api/v1/database/${expectDbId}/tables/?q=(force:!${
forceRefresh ? 't' : 'f'
},schema_name:${expectedSchema})`,
});
expect(result.current.data).toEqual(expectedData);
await act(async () => {
result.current.refetch();
});
expect(SupersetClient.get).toHaveBeenCalledTimes(2);
expect(SupersetClient.get).toHaveBeenCalledWith({
endpoint: `/superset/tables/${expectDbId}/${expectedSchema}/true/`,
endpoint: `/api/v1/database/${expectDbId}/tables/?q=(force:!t,schema_name:${expectedSchema})`,
});
expect(result.current.data).toEqual(expectedData);
});
Expand Down
22 changes: 16 additions & 6 deletions superset-frontend/src/hooks/apiResources/tables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
import { useRef } from 'react';
import { useQuery, UseQueryOptions } from 'react-query';
import rison from 'rison';
import { SupersetClient } from '@superset-ui/core';

export type FetchTablesQueryParams = {
Expand All @@ -39,11 +40,15 @@ export interface Table {
}

type QueryData = {
json: { options: Table[]; tableLength: number };
json: {
count: number;
result: Table[];
};
response: Response;
};

export type Data = QueryData['json'] & {
export type Data = {
options: Table[];
hasMore: boolean;
};

Expand All @@ -53,10 +58,15 @@ export function fetchTables({
forceRefresh,
}: FetchTablesQueryParams) {
const encodedSchema = schema ? encodeURIComponent(schema) : '';
const params = rison.encode({
force: forceRefresh,
schema_name: encodedSchema,
});

// TODO: Would be nice to add pagination in a follow-up. Needs endpoint changes.
const endpoint = `/superset/tables/${
const endpoint = `/api/v1/database/${
dbId ?? 'undefined'
}/${encodedSchema}/${forceRefresh}/`;
}/tables/?q=${params}`;
return SupersetClient.get({ endpoint }) as Promise<QueryData>;
}

Expand All @@ -72,8 +82,8 @@ export function useTables(options: Params) {
() => fetchTables({ ...params, forceRefresh: forceRefreshRef.current }),
{
select: ({ json }) => ({
...json,
hasMore: json.tableLength > json.options.length,
options: json.result,
hasMore: json.count > json.result.length,
}),
enabled: Boolean(dbId && schema),
onSuccess,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import LeftPanel from 'src/views/CRUD/data/dataset/AddDataset/LeftPanel';

const databasesEndpoint = 'glob:*/api/v1/database/?q*';
const schemasEndpoint = 'glob:*/api/v1/database/*/schemas*';
const tablesEndpoint = 'glob:*/superset/tables*';
const tablesEndpoint = 'glob:*/api/v1/database/*/tables/?q*';

fetchMock.get(databasesEndpoint, {
count: 2,
Expand Down Expand Up @@ -136,8 +136,8 @@ fetchMock.get(schemasEndpoint, {
});

fetchMock.get(tablesEndpoint, {
tableLength: 3,
options: [
count: 3,
result: [
{ value: 'Sheet1', type: 'table', extra: null },
{ value: 'Sheet2', type: 'table', extra: null },
{ value: 'Sheet3', type: 'table', extra: null },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
* under the License.
*/
import React, { useEffect, useState, SetStateAction, Dispatch } from 'react';
import rison from 'rison';
import {
SupersetClient,
t,
Expand Down Expand Up @@ -177,7 +178,7 @@ export default function LeftPanel({
const getTablesList = (url: string) => {
SupersetClient.get({ url })
.then(({ json }) => {
const options: TableOption[] = json.options.map((table: Table) => {
const options: TableOption[] = json.result.map((table: Table) => {
const option: TableOption = {
value: table.value,
label: <TableOption table={table} />,
Expand Down Expand Up @@ -213,9 +214,12 @@ export default function LeftPanel({

useEffect(() => {
if (loadTables) {
const endpoint = encodeURI(
`/superset/tables/${dbId}/${encodedSchema}/${refresh}/`,
);
const params = rison.encode({
force: refresh,
schema_name: encodedSchema,
});

const endpoint = `/api/v1/database/${dbId}/tables/?q=${params}`;
getTablesList(endpoint);
}
}, [loadTables]);
Expand Down
1 change: 1 addition & 0 deletions superset/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ class RouteMethod: # pylint: disable=too-few-public-methods
"put": "write",
"related": "read",
"related_objects": "read",
"tables": "read",
"schemas": "read",
"select_star": "read",
"table_metadata": "read",
Expand Down
74 changes: 74 additions & 0 deletions superset/databases/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,13 @@
DatabaseDeleteFailedError,
DatabaseInvalidError,
DatabaseNotFoundError,
DatabaseTablesUnexpectedError,
DatabaseUpdateFailedError,
InvalidParametersError,
)
from superset.databases.commands.export import ExportDatabasesCommand
from superset.databases.commands.importers.dispatcher import ImportDatabasesCommand
from superset.databases.commands.tables import TablesDatabaseCommand
from superset.databases.commands.test_connection import TestConnectionDatabaseCommand
from superset.databases.commands.update import UpdateDatabaseCommand
from superset.databases.commands.validate import ValidateDatabaseParametersCommand
Expand All @@ -58,10 +60,12 @@
from superset.databases.filters import DatabaseFilter, DatabaseUploadEnabledFilter
from superset.databases.schemas import (
database_schemas_query_schema,
database_tables_query_schema,
DatabaseFunctionNamesResponse,
DatabasePostSchema,
DatabasePutSchema,
DatabaseRelatedObjectsResponse,
DatabaseTablesResponse,
DatabaseTestConnectionSchema,
DatabaseValidateParametersSchema,
get_export_ids_schema,
Expand Down Expand Up @@ -104,6 +108,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
include_route_methods = RouteMethod.REST_MODEL_VIEW_CRUD_SET | {
RouteMethod.EXPORT,
RouteMethod.IMPORT,
"tables",
"table_metadata",
"table_extra_metadata",
"select_star",
Expand Down Expand Up @@ -210,13 +215,15 @@ class DatabaseRestApi(BaseSupersetModelRestApi):

apispec_parameter_schemas = {
"database_schemas_query_schema": database_schemas_query_schema,
"database_tables_query_schema": database_tables_query_schema,
"get_export_ids_schema": get_export_ids_schema,
}

openapi_spec_tag = "Database"
openapi_spec_component_schemas = (
DatabaseFunctionNamesResponse,
DatabaseRelatedObjectsResponse,
DatabaseTablesResponse,
DatabaseTestConnectionSchema,
DatabaseValidateParametersSchema,
TableExtraMetadataResponseSchema,
Expand Down Expand Up @@ -555,6 +562,73 @@ def schemas(self, pk: int, **kwargs: Any) -> FlaskResponse:
except SupersetException as ex:
return self.response(ex.status, message=ex.message)

@expose("/<int:pk>/tables/")
@protect()
@safe
@rison(database_tables_query_schema)
@statsd_metrics
@event_logger.log_this_with_context(
action=lambda self, *args, **kwargs: f"{self.__class__.__name__}" f".tables",
log_to_statsd=False,
)
def tables(self, pk: int, **kwargs: Any) -> FlaskResponse:
"""Get a list of tables for given database
---
get:
summary: Get a list of tables for given database
parameters:
- in: path
schema:
type: integer
name: pk
description: The database id
- in: query
name: q
content:
application/json:
schema:
$ref: '#/components/schemas/database_tables_query_schema'
responses:
200:
description: Tables list
content:
application/json:
schema:
type: object
properties:
count:
type: integer
result:
description: >-
A List of tables for given database
type: array
items:
$ref: '#/components/schemas/DatabaseTablesResponse'
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'
"""
force = kwargs["rison"].get("force", False)
schema_name = kwargs["rison"].get("schema_name", "")

try:
command = TablesDatabaseCommand(pk, schema_name, force)
payload = command.run()
return self.response(200, **payload)
except DatabaseNotFoundError:
return self.response_404()
except SupersetException as ex:
return self.response(ex.status, message=ex.message)
except DatabaseTablesUnexpectedError as ex:
return self.response_422(ex.message)

@expose("/<int:pk>/table/<table_name>/<schema_name>/", methods=["GET"])
@protect()
@check_datasource_access
Expand Down
5 changes: 5 additions & 0 deletions superset/databases/commands/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,11 @@ class DatabaseTestConnectionUnexpectedError(SupersetErrorsException):
message = _("Unexpected error occurred, please check your logs for details")


class DatabaseTablesUnexpectedError(Exception):
status = 422
message = _("Unexpected error occurred, please check your logs for details")


class NoValidatorConfigFoundError(SupersetErrorException):
status = 422
message = _("no SQL validator is configured")
Expand Down
Loading

0 comments on commit 2fd8f4c

Please sign in to comment.