Compare commits

...

4 Commits

Author SHA1 Message Date
Beto Dealmeida
7ab6953f1a Small improvements 2025-09-10 10:51:59 -04:00
Beto Dealmeida
b55cb206a5 Update schema 2025-09-08 12:39:59 -04:00
Beto Dealmeida
4e52bc8a0b Update frontend 2025-09-03 15:57:01 -04:00
Beto Dealmeida
7f55d3ff30 feat: use default catalog/schema on DB selector 2025-09-03 14:36:31 -04:00
11 changed files with 382 additions and 68 deletions

View File

@@ -73,11 +73,14 @@ beforeEach(() => {
dbId: expectDbId,
forceRefresh: false,
},
fakeSchemaApiResult.map(value => ({
value,
label: value,
title: value,
})),
{
result: fakeSchemaApiResult.map(value => ({
value,
label: value,
title: value,
})),
default: null,
},
),
);
store.dispatch(
@@ -307,11 +310,14 @@ test('returns long keywords with docText', async () => {
dbId: expectLongKeywordDbId,
forceRefresh: false,
},
['short', longKeyword].map(value => ({
value,
label: value,
title: value,
})),
{
result: ['short', longKeyword].map(value => ({
value,
label: value,
title: value,
})),
default: null,
},
),
);
});

View File

@@ -164,7 +164,7 @@ export function useKeywords(
const schemaKeywords = useMemo(
() =>
(schemaOptions ?? []).map(s => ({
(schemaOptions?.result ?? []).map(s => ({
name: s.label,
value: s.value,
score: SCHEMA_AUTOCOMPLETE_SCORE,

View File

@@ -163,11 +163,13 @@ const fakeDatabaseApiResultInReverseOrder = {
const fakeSchemaApiResult = {
count: 2,
result: ['information_schema', 'public'],
default: 'public',
};
const fakeCatalogApiResult = {
count: 0,
result: [],
default: null,
};
const fakeFunctionNamesApiResult = {
@@ -382,3 +384,143 @@ test('Sends the correct schema when changing the schema', async () => {
);
expect(props.onSchemaChange).toHaveBeenCalledTimes(1);
});
test('Should auto-select default schema on load', async () => {
const props = createProps();
// Remove initial schema to test auto-selection
const propsWithoutSchema = { ...props, schema: undefined };
render(<DatabaseSelector {...propsWithoutSchema} />, {
useRedux: true,
store,
});
// Wait for the default schema to be auto-selected
await waitFor(() => {
expect(propsWithoutSchema.onSchemaChange).toHaveBeenCalledWith('public');
});
});
test('Should auto-select default catalog for multi-catalog databases', async () => {
const multiCatalogApiResult = {
count: 2,
result: ['catalog1', 'catalog2'],
default: 'catalog1',
};
fetchMock.get(catalogApiRoute, multiCatalogApiResult, {
overwriteRoutes: true,
});
const props = createProps();
const multiCatalogDb = {
...props.db!,
allow_multi_catalog: true,
};
const multiCatalogProps = {
...props,
db: multiCatalogDb,
catalog: undefined,
onCatalogChange: jest.fn(),
};
render(<DatabaseSelector {...multiCatalogProps} />, {
useRedux: true,
store,
});
// Wait for the default catalog to be auto-selected
await waitFor(() => {
expect(multiCatalogProps.onCatalogChange).toHaveBeenCalledWith('catalog1');
});
});
test('Should disable schema dropdown while catalogs are loading', async () => {
const props = createProps();
const multiCatalogDb = {
...props.db!,
allow_multi_catalog: true,
};
const multiCatalogProps = {
...props,
db: multiCatalogDb,
catalog: undefined,
};
// Mock a delayed catalog response
fetchMock.get(
catalogApiRoute,
new Promise(resolve =>
setTimeout(
() =>
resolve({
count: 1,
result: ['default_catalog'],
default: 'default_catalog',
}),
100,
),
),
{ overwriteRoutes: true },
);
render(<DatabaseSelector {...multiCatalogProps} />, {
useRedux: true,
store,
});
// Initially, schema dropdown should be disabled while catalogs load
const schemaSelect = screen.getByRole('combobox', {
name: /select schema or type to search schemas/i,
});
expect(schemaSelect).toBeDisabled();
// Wait for catalogs to load and schema dropdown to be enabled
await waitFor(
() => {
expect(schemaSelect).toBeEnabled();
},
{ timeout: 200 },
);
});
test('Should not fetch schemas until catalog is selected for multi-catalog databases', async () => {
const props = createProps();
const multiCatalogDb = {
...props.db!,
allow_multi_catalog: true,
};
const multiCatalogProps = {
...props,
db: multiCatalogDb,
catalog: undefined,
};
render(<DatabaseSelector {...multiCatalogProps} />, {
useRedux: true,
store,
});
// Initially, schemas should not be fetched
expect(fetchMock.calls(schemaApiRoute).length).toBe(0);
// Wait a bit to ensure schemas are still not fetched
await new Promise(resolve => setTimeout(resolve, 100));
expect(fetchMock.calls(schemaApiRoute).length).toBe(0);
});
test('Should fetch schemas immediately for non-catalog databases', async () => {
const props = createProps();
// Non-catalog database (allow_multi_catalog is not set)
render(<DatabaseSelector {...props} />, {
useRedux: true,
store,
});
// Schemas should be fetched immediately for non-catalog databases
await waitFor(() => {
expect(fetchMock.calls(schemaApiRoute).length).toBe(1);
});
});

View File

@@ -238,23 +238,20 @@ export function DatabaseSelector({
}
}
const shouldFetchSchemas =
currentDb?.value &&
(!showCatalogSelector || (currentCatalog && currentCatalog.value));
const {
currentData: schemaData,
isFetching: loadingSchemas,
refetch: refetchSchemas,
} = useSchemas({
dbId: currentDb?.value,
dbId: shouldFetchSchemas ? currentDb?.value : undefined,
catalog: currentCatalog?.value,
onSuccess: (schemas, isFetched) => {
onSuccess: (schemas, isFetched, defaultValue) => {
setErrorPayload(null);
if (schemas.length === 1) {
changeSchema(schemas[0]);
} else if (
!schemas.find(schemaOption => schemaRef.current === schemaOption.value)
) {
changeSchema(undefined);
}
autoSelectSchema(schemas, defaultValue);
if (isFetched) {
addSuccessToast('List refreshed');
}
@@ -268,11 +265,13 @@ export function DatabaseSelector({
},
});
const schemaOptions = schemaData || EMPTY_SCHEMA_OPTIONS;
const schemaOptions = schemaData?.result || EMPTY_SCHEMA_OPTIONS;
function changeCatalog(catalog: CatalogOption | null | undefined) {
setCurrentCatalog(catalog);
setCurrentSchema(undefined);
// Clear schema ref so auto-selection works for the new catalog
schemaRef.current = undefined;
if (onCatalogChange && catalog?.value !== catalogRef.current) {
onCatalogChange(catalog?.value);
}
@@ -284,20 +283,9 @@ export function DatabaseSelector({
refetch: refetchCatalogs,
} = useCatalogs({
dbId: showCatalogSelector ? currentDb?.value : undefined,
onSuccess: (catalogs, isFetched) => {
onSuccess: (catalogs, isFetched, defaultValue) => {
setErrorPayload(null);
if (!showCatalogSelector) {
changeCatalog(null);
} else if (catalogs.length === 1) {
changeCatalog(catalogs[0]);
} else if (
!catalogs.find(
catalogOption => catalogRef.current === catalogOption.value,
)
) {
changeCatalog(undefined);
}
autoSelectCatalog(catalogs, defaultValue);
if (showCatalogSelector && isFetched) {
addSuccessToast('List refreshed');
}
@@ -313,7 +301,59 @@ export function DatabaseSelector({
},
});
const catalogOptions = catalogData || EMPTY_CATALOG_OPTIONS;
const catalogOptions = catalogData?.result || EMPTY_CATALOG_OPTIONS;
// Centralized auto-selection logic
const autoSelectCatalog = useCallback(
(catalogs: CatalogOption[], defaultValue?: string | null) => {
if (!showCatalogSelector) {
changeCatalog(null);
} else if (defaultValue && !catalogRef.current) {
const defaultCatalog = catalogs.find(
catalog => catalog.value === defaultValue,
);
if (defaultCatalog) {
changeCatalog(defaultCatalog);
}
} else if (catalogs.length === 1) {
changeCatalog(catalogs[0]);
} else if (
!catalogs.find(
catalogOption => catalogRef.current === catalogOption.value,
)
) {
changeCatalog(undefined);
}
},
[showCatalogSelector, changeCatalog, catalogRef],
);
const autoSelectSchema = useCallback(
(schemas: SchemaOption[], defaultValue?: string | null) => {
if (defaultValue && !schemaRef.current) {
const defaultSchema = schemas.find(
schema => schema.value === defaultValue,
);
if (defaultSchema) {
changeSchema(defaultSchema);
}
} else if (schemas.length === 1) {
changeSchema(schemas[0]);
} else if (
!schemas.find(schemaOption => schemaRef.current === schemaOption.value)
) {
changeSchema(undefined);
}
},
[schemaRef],
);
// For non-catalog databases, set catalog to null immediately
useEffect(() => {
if (currentDb && !showCatalogSelector) {
setCurrentCatalog(null);
}
}, [currentDb?.id, showCatalogSelector]);
function changeDatabase(
value: { label: string; value: number },
@@ -325,6 +365,9 @@ export function DatabaseSelector({
setCurrentDb(databaseWithId);
setCurrentCatalog(undefined);
setCurrentSchema(undefined);
// Clear refs so auto-selection works when switching back to a database
catalogRef.current = undefined;
schemaRef.current = undefined;
if (onDbChange) {
onDbChange(databaseWithId);
}
@@ -402,7 +445,11 @@ export function DatabaseSelector({
return renderSelectRow(
<Select
ariaLabel={t('Select schema or type to search schemas')}
disabled={!currentDb || readOnly}
disabled={
!currentDb ||
readOnly ||
(showCatalogSelector && (loadingCatalogs || !currentCatalog))
}
header={<FormLabel>{t('Schema')}</FormLabel>}
labelInValue
loading={loadingSchemas}

View File

@@ -27,10 +27,19 @@ export type CatalogOption = {
title: string;
};
export type CatalogResponse = {
result: CatalogOption[];
default: string | null;
};
export type FetchCatalogsQueryParams = {
dbId?: string | number;
forceRefresh: boolean;
onSuccess?: (data: CatalogOption[], isRefetched: boolean) => void;
onSuccess?: (
data: CatalogOption[],
isRefetched: boolean,
defaultValue?: string | null,
) => void;
onError?: (error: ClientErrorObject) => void;
};
@@ -38,19 +47,21 @@ type Params = Omit<FetchCatalogsQueryParams, 'forceRefresh'>;
const catalogApi = api.injectEndpoints({
endpoints: builder => ({
catalogs: builder.query<CatalogOption[], FetchCatalogsQueryParams>({
catalogs: builder.query<CatalogResponse, FetchCatalogsQueryParams>({
providesTags: [{ type: 'Catalogs', id: 'LIST' }],
query: ({ dbId, forceRefresh }) => ({
endpoint: `/api/v1/database/${dbId}/catalogs/`,
urlParams: {
force: forceRefresh,
},
transformResponse: ({ json }: JsonResponse) =>
json.result.sort().map((value: string) => ({
transformResponse: ({ json }: JsonResponse) => ({
result: json.result.sort().map((value: string) => ({
value,
label: value,
title: value,
})),
default: json.default,
}),
}),
serializeQueryArgs: ({ queryArgs: { dbId } }) => ({
dbId,
@@ -89,7 +100,11 @@ export function useCatalogs(options: Params) {
if (dbId && (!result.currentData || forceRefresh)) {
trigger({ dbId, forceRefresh }).then(({ isSuccess, isError, data }) => {
if (isSuccess) {
onSuccess?.(data || EMPTY_CATALOGS, forceRefresh);
onSuccess?.(
data?.result || EMPTY_CATALOGS,
forceRefresh,
data?.default,
);
}
if (isError) {
onError?.(result.error as ClientErrorObject);

View File

@@ -28,29 +28,41 @@ import { useSchemas } from './schemas';
const fakeApiResult = {
result: ['test schema 1', 'test schema b'],
default: 'test schema 1',
};
const fakeApiResult2 = {
result: ['test schema 2', 'test schema a'],
default: null,
};
const fakeApiResult3 = {
result: ['test schema 3', 'test schema c'],
default: 'test schema 3',
};
const expectedResult = fakeApiResult.result.map((value: string) => ({
value,
label: value,
title: value,
}));
const expectedResult2 = fakeApiResult2.result.map((value: string) => ({
value,
label: value,
title: value,
}));
const expectedResult3 = fakeApiResult3.result.map((value: string) => ({
value,
label: value,
title: value,
}));
const expectedResult = {
result: fakeApiResult.result.map((value: string) => ({
value,
label: value,
title: value,
})),
default: fakeApiResult.default,
};
const expectedResult2 = {
result: fakeApiResult2.result.map((value: string) => ({
value,
label: value,
title: value,
})),
default: fakeApiResult2.default,
};
const expectedResult3 = {
result: fakeApiResult3.result.map((value: string) => ({
value,
label: value,
title: value,
})),
default: fakeApiResult3.default,
};
describe('useSchemas hook', () => {
beforeEach(() => {

View File

@@ -27,11 +27,20 @@ export type SchemaOption = {
title: string;
};
export type SchemaResponse = {
result: SchemaOption[];
default: string | null;
};
export type FetchSchemasQueryParams = {
dbId?: string | number;
catalog?: string;
forceRefresh: boolean;
onSuccess?: (data: SchemaOption[], isRefetched: boolean) => void;
onSuccess?: (
data: SchemaOption[],
isRefetched: boolean,
defaultValue?: string | null,
) => void;
onError?: (error: ClientErrorObject) => void;
};
@@ -39,7 +48,7 @@ type Params = Omit<FetchSchemasQueryParams, 'forceRefresh'>;
const schemaApi = api.injectEndpoints({
endpoints: builder => ({
schemas: builder.query<SchemaOption[], FetchSchemasQueryParams>({
schemas: builder.query<SchemaResponse, FetchSchemasQueryParams>({
providesTags: [{ type: 'Schemas', id: 'LIST' }],
query: ({ dbId, catalog, forceRefresh }) => ({
endpoint: `/api/v1/database/${dbId}/schemas/`,
@@ -48,12 +57,14 @@ const schemaApi = api.injectEndpoints({
force: forceRefresh,
...(catalog !== undefined && { catalog }),
},
transformResponse: ({ json }: JsonResponse) =>
json.result.sort().map((value: string) => ({
transformResponse: ({ json }: JsonResponse) => ({
result: json.result.sort().map((value: string) => ({
value,
label: value,
title: value,
})),
default: json.default,
}),
}),
serializeQueryArgs: ({ queryArgs: { dbId, catalog } }) => ({
dbId,
@@ -98,7 +109,11 @@ export function useSchemas(options: Params) {
trigger({ dbId, catalog, forceRefresh }).then(
({ isSuccess, isError, data }) => {
if (isSuccess) {
onSuccess?.(data || EMPTY_SCHEMAS, forceRefresh);
onSuccess?.(
data?.result || EMPTY_SCHEMAS,
forceRefresh,
data?.default,
);
}
if (isError) {
onError?.(result.error as ClientErrorObject);

View File

@@ -172,7 +172,7 @@ export function useTables(options: Params) {
catalog: catalog || undefined,
});
const schemaOptionsMap = useMemo(
() => new Set(schemaOptions?.map(({ value }) => value)),
() => new Set(schemaOptions?.result?.map(({ value }) => value)),
[schemaOptions],
);

View File

@@ -727,7 +727,13 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
database,
catalogs,
)
return self.response(200, result=list(catalogs))
default_catalog = database.get_default_catalog()
# TODO: Consider refactoring API response structure during
# next breaking change window to return catalogs as objects
# with a 'default' flag instead of separate fields
# e.g., result=[{"name": "catalog1", "default": true},
# {"name": "catalog2", "default": false}]
return self.response(200, result=list(catalogs), default=default_catalog)
except OperationalError:
return self.response(
500,
@@ -796,9 +802,10 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
catalog,
schemas,
)
default_schema = database.get_default_schema(catalog)
if params.get("upload_allowed"):
if not database.allow_file_upload:
return self.response(200, result=[])
return self.response(200, result=[], default=default_schema)
if allowed_schemas := database.get_schema_access_for_file_upload():
# some databases might return the list of schemas in uppercase,
# while the list of allowed schemas is manually inputted so
@@ -811,8 +818,14 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
for schema in schemas
if schema.lower() in allowed_schemas
],
default=default_schema,
)
return self.response(200, result=list(schemas))
# TODO: Consider refactoring API response structure during
# next breaking change window to return schemas as objects
# with a 'default' flag instead of separate fields
# e.g., result=[{"name": "schema1", "default": true},
# {"name": "schema2", "default": false}]
return self.response(200, result=list(schemas), default=default_schema)
except OperationalError:
return self.response(
500, message="There was an error connecting to the database"

View File

@@ -724,12 +724,20 @@ class SchemasResponseSchema(Schema):
result = fields.List(
fields.String(metadata={"description": "A database schema name"})
)
default = fields.String(
allow_none=True,
metadata={"description": "The default schema for this database"},
)
class CatalogsResponseSchema(Schema):
result = fields.List(
fields.String(metadata={"description": "A database catalog name"})
)
default = fields.String(
allow_none=True,
metadata={"description": "The default catalog for this database"},
)
class DatabaseTablesResponse(Schema):

View File

@@ -2261,6 +2261,7 @@ def test_catalogs(
"""
database = mocker.MagicMock()
database.get_all_catalog_names.return_value = {"db1", "db2"}
database.get_default_catalog.return_value = "db1"
DatabaseDAO = mocker.patch("superset.databases.api.DatabaseDAO") # noqa: N806
DatabaseDAO.find_by_id.return_value = database
@@ -2272,7 +2273,7 @@ def test_catalogs(
response = client.get("/api/v1/database/1/catalogs/")
assert response.status_code == 200
assert response.json == {"result": ["db2"]}
assert response.json == {"result": ["db2"], "default": "db1"}
database.get_all_catalog_names.assert_called_with(
cache=database.catalog_cache_enabled,
cache_timeout=database.catalog_cache_timeout,
@@ -2344,6 +2345,7 @@ def test_schemas(
database = mocker.MagicMock()
database.get_all_schema_names.return_value = {"schema1", "schema2"}
database.get_default_schema.return_value = "public"
datamodel = mocker.patch.object(DatabaseRestApi, "datamodel")
datamodel.get.return_value = database
@@ -2355,13 +2357,14 @@ def test_schemas(
response = client.get("/api/v1/database/1/schemas/")
assert response.status_code == 200
assert response.json == {"result": ["schema2"]}
assert response.json == {"result": ["schema2"], "default": "public"}
database.get_all_schema_names.assert_called_with(
catalog=None,
cache=database.schema_cache_enabled,
cache_timeout=database.schema_cache_timeout,
force=False,
)
database.get_default_schema.assert_called_with(None)
security_manager.get_schemas_accessible_by_user.assert_called_with(
database,
None,
@@ -2383,6 +2386,7 @@ def test_schemas(
cache_timeout=database.schema_cache_timeout,
force=True,
)
database.get_default_schema.assert_called_with("catalog2")
security_manager.get_schemas_accessible_by_user.assert_called_with(
database,
"catalog2",
@@ -2390,6 +2394,58 @@ def test_schemas(
)
def test_catalogs_with_null_default(
mocker: MockerFixture,
client: Any,
full_api_access: None,
) -> None:
"""
Test the `catalogs` endpoint when default catalog is None.
"""
database = mocker.MagicMock()
database.get_all_catalog_names.return_value = {"db1", "db2"}
database.get_default_catalog.return_value = None
DatabaseDAO = mocker.patch("superset.databases.api.DatabaseDAO") # noqa: N806
DatabaseDAO.find_by_id.return_value = database
security_manager = mocker.patch(
"superset.databases.api.security_manager",
new=mocker.MagicMock(),
)
security_manager.get_catalogs_accessible_by_user.return_value = {"db2"}
response = client.get("/api/v1/database/1/catalogs/")
assert response.status_code == 200
assert response.json == {"result": ["db2"], "default": None}
def test_schemas_with_null_default(
mocker: MockerFixture,
client: Any,
full_api_access: None,
) -> None:
"""
Test the `schemas` endpoint when default schema is None.
"""
from superset.databases.api import DatabaseRestApi
database = mocker.MagicMock()
database.get_all_schema_names.return_value = {"schema1", "schema2"}
database.get_default_schema.return_value = None
datamodel = mocker.patch.object(DatabaseRestApi, "datamodel")
datamodel.get.return_value = database
security_manager = mocker.patch(
"superset.databases.api.security_manager",
new=mocker.MagicMock(),
)
security_manager.get_schemas_accessible_by_user.return_value = {"schema2"}
response = client.get("/api/v1/database/1/schemas/")
assert response.status_code == 200
assert response.json == {"result": ["schema2"], "default": None}
def test_schemas_with_oauth2(
mocker: MockerFixture,
client: Any,