QT/superset2
1
0
Fork 0
mirror of https://github.com/apache/superset.git synced 2026-04-19 08:04:53 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(sqllab): excessive API calls for schemas (#29279)

Browse Source
This commit is contained in:
JUST.in DO IT
2024-06-18 14:02:40 -07:00
committed by GitHub
parent 725afc3848
commit 4537ab6b1a
3 changed files with 59 additions and 100 deletions
Download Patch File Download Diff File Expand all files Collapse all files

54
superset-frontend/src/hooks/apiResources/catalogs.ts
View File

@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import { useCallback, useEffect, useRef } from 'react';
import { useCallback, useEffect } from 'react';
import useEffectEvent from 'src/hooks/useEffectEvent';
import { api, JsonResponse } from './queryApi';
@@ -68,7 +68,6 @@ export const {
export const EMPTY_CATALOGS = [] as CatalogOption[];
export function useCatalogs(options: Params) {
const isMountedRef = useRef(false);
const { dbId, onSuccess, onError } = options || {};
const [trigger] = useLazyCatalogsQuery();
const result = useCatalogsQuery(
@@ -78,47 +77,28 @@ export function useCatalogs(options: Params) {
},
);
const handleOnSuccess = useEffectEvent(
(data: CatalogOption[], isRefetched: boolean) => {
onSuccess?.(data, isRefetched);
const fetchData = useEffectEvent(
(dbId: FetchCatalogsQueryParams['dbId'], forceRefresh = false) => {
if (dbId && (!result.currentData || forceRefresh)) {
trigger({ dbId, forceRefresh }).then(({ isSuccess, isError, data }) => {
if (isSuccess) {
onSuccess?.(data || EMPTY_CATALOGS, forceRefresh);
}
if (isError) {
onError?.();
}
});
}
},
);
const handleOnError = useEffectEvent(() => {
onError?.();
});
const refetch = useCallback(() => {
if (dbId) {
trigger({ dbId, forceRefresh: true }).then(
({ isSuccess, isError, data }) => {
if (isSuccess) {
handleOnSuccess(data || EMPTY_CATALOGS, true);
}
if (isError) {
handleOnError();
}
},
);
}
}, [dbId, handleOnError, handleOnSuccess, trigger]);
fetchData(dbId, true);
}, [dbId, fetchData]);
useEffect(() => {
if (isMountedRef.current) {
const { requestId, isSuccess, isError, isFetching, data, originalArgs } =
result;
if (!originalArgs?.forceRefresh && requestId && !isFetching) {
if (isSuccess) {
handleOnSuccess(data || EMPTY_CATALOGS, false);
}
if (isError) {
handleOnError();
}
}
} else {
isMountedRef.current = true;
}
}, [result, handleOnSuccess, handleOnError]);
fetchData(dbId, false);
}, [dbId, fetchData]);
return {
...result,

30
superset-frontend/src/hooks/apiResources/schemas.test.ts
View File

@@ -88,7 +88,7 @@ describe('useSchemas hook', () => {
})}`,
).length,
).toBe(1);
expect(onSuccess).toHaveBeenCalledTimes(2);
expect(onSuccess).toHaveBeenCalledTimes(1);
act(() => {
result.current.refetch();
});
@@ -100,7 +100,7 @@ describe('useSchemas hook', () => {
})}`,
).length,
).toBe(1);
expect(onSuccess).toHaveBeenCalledTimes(3);
expect(onSuccess).toHaveBeenCalledTimes(2);
expect(result.current.data).toEqual(expectedResult);
});
@@ -149,28 +149,36 @@ describe('useSchemas hook', () => {
},
);
await waitFor(() => expect(result.current.data).toEqual(expectedResult));
await waitFor(() =>
expect(result.current.currentData).toEqual(expectedResult),
);
expect(fetchMock.calls(schemaApiRoute).length).toBe(1);
expect(onSuccess).toHaveBeenCalledTimes(2);
expect(onSuccess).toHaveBeenCalledTimes(1);
rerender({ dbId: 'db2' });
await waitFor(() => expect(result.current.data).toEqual(expectedResult2));
await waitFor(() =>
expect(result.current.currentData).toEqual(expectedResult2),
);
expect(fetchMock.calls(schemaApiRoute).length).toBe(2);
expect(onSuccess).toHaveBeenCalledTimes(4);
expect(onSuccess).toHaveBeenCalledTimes(2);
rerender({ dbId: expectDbId });
await waitFor(() => expect(result.current.data).toEqual(expectedResult));
await waitFor(() =>
expect(result.current.currentData).toEqual(expectedResult),
);
expect(fetchMock.calls(schemaApiRoute).length).toBe(2);
expect(onSuccess).toHaveBeenCalledTimes(5);
expect(onSuccess).toHaveBeenCalledTimes(2);
// clean up cache
act(() => {
store.dispatch(api.util.invalidateTags(['Schemas']));
});
await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(3));
await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(4));
expect(fetchMock.calls(schemaApiRoute)[2][0]).toContain(expectDbId);
await waitFor(() => expect(result.current.data).toEqual(expectedResult));
await waitFor(() =>
expect(result.current.currentData).toEqual(expectedResult),
);
});
test('returns correct schema list by a catalog', async () => {
@@ -201,7 +209,7 @@ describe('useSchemas hook', () => {
await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(1));
expect(result.current.data).toEqual(expectedResult3);
expect(onSuccess).toHaveBeenCalledTimes(2);
expect(onSuccess).toHaveBeenCalledTimes(1);
rerender({ dbId, catalog: 'catalog2' });
await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(2));

75
superset-frontend/src/hooks/apiResources/schemas.ts
View File

@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import { useCallback, useEffect, useRef } from 'react';
import { useCallback, useEffect } from 'react';
import useEffectEvent from 'src/hooks/useEffectEvent';
import { api, JsonResponse } from './queryApi';
@@ -72,7 +72,6 @@ export const {
export const EMPTY_SCHEMAS = [] as SchemaOption[];
export function useSchemas(options: Params) {
const isMountedRef = useRef(false);
const { dbId, catalog, onSuccess, onError } = options || {};
const [trigger] = useLazySchemasQuery();
const result = useSchemasQuery(
@@ -82,62 +81,34 @@ export function useSchemas(options: Params) {
},
);
const handleOnSuccess = useEffectEvent(
(data: SchemaOption[], isRefetched: boolean) => {
onSuccess?.(data, isRefetched);
const fetchData = useEffectEvent(
(
dbId: FetchSchemasQueryParams['dbId'],
catalog: FetchSchemasQueryParams['catalog'],
forceRefresh = false,
) => {
if (dbId && (!result.currentData || forceRefresh)) {
trigger({ dbId, catalog, forceRefresh }).then(
({ isSuccess, isError, data }) => {
if (isSuccess) {
onSuccess?.(data || EMPTY_SCHEMAS, forceRefresh);
}
if (isError) {
onError?.();
}
},
);
}
},
);
const handleOnError = useEffectEvent(() => {
onError?.();
});
useEffect(() => {
if (dbId) {
trigger({ dbId, catalog, forceRefresh: false }).then(
({ isSuccess, isError, data }) => {
if (isSuccess) {
handleOnSuccess(data || EMPTY_SCHEMAS, true);
}
if (isError) {
handleOnError();
}
},
);
}
}, [dbId, catalog, handleOnError, handleOnSuccess, trigger]);
fetchData(dbId, catalog, false);
}, [dbId, catalog, fetchData]);
const refetch = useCallback(() => {
if (dbId) {
trigger({ dbId, catalog, forceRefresh: true }).then(
({ isSuccess, isError, data }) => {
if (isSuccess) {
handleOnSuccess(data || EMPTY_SCHEMAS, true);
}
if (isError) {
handleOnError();
}
},
);
}
}, [dbId, catalog, handleOnError, handleOnSuccess, trigger]);
useEffect(() => {
if (isMountedRef.current) {
const { requestId, isSuccess, isError, isFetching, data, originalArgs } =
result;
if (!originalArgs?.forceRefresh && requestId && !isFetching) {
if (isSuccess) {
handleOnSuccess(data || EMPTY_SCHEMAS, false);
}
if (isError) {
handleOnError();
}
}
} else {
isMountedRef.current = true;
}
}, [catalog, result, handleOnSuccess, handleOnError]);
fetchData(dbId, catalog, true);
}, [dbId, catalog, fetchData]);
return {
...result,