Skip to content

Commit

Permalink
Avoid unnecessary refetchs (#133)
Browse files Browse the repository at this point in the history
* Refactor ITwinGrid and related hooks to use serverEnvironmentPrefix for API server URL construction

* Remove unused import from useITwinFavorites hook

* Add change log entry to avoid unnecessary refetches to API in @itwin/imodel-browser-react
  • Loading branch information
arome authored Dec 9, 2024
1 parent deb9125 commit 95d04cf
Show file tree
Hide file tree
Showing 11 changed files with 88 additions and 31 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@itwin/imodel-browser-react",
"comment": "Avoid unnecessary refetchs to API",
"type": "patch"
}
],
"packageName": "@itwin/imodel-browser-react"
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,43 @@ describe("ITwinGrid", () => {
expect(wrapper.getByRole("table")).toBeDefined();
expect(wrapper.getAllByRole("row").length).toEqual(3); // First row is header
});

it("should not refetch iTwins favorites when component rerenders", async () => {
// Arrange
const fetchMore = jest.fn();
jest.spyOn(useITwinData, "useITwinData").mockReturnValue({
iTwins: [],
status: DataStatus.Complete,
fetchMore,
});
// Act
const signal = new AbortController().signal;
const wrapper = render(
<ITwinGrid
accessToken="accessToken"
apiOverrides={{ serverEnvironmentPrefix: "qa" }}
viewMode="cells"
/>
);
wrapper.rerender(
<ITwinGrid
accessToken="accessToken"
apiOverrides={{ serverEnvironmentPrefix: "qa" }}
viewMode="tile"
/>
);

expect(fetch).toHaveBeenCalledTimes(1);
expect(fetch).toHaveBeenCalledWith(
"https://qa-api.bentley.com/itwins/favorites?subClass=Project",
{
headers: {
Accept: "application/vnd.bentley.itwin-platform.v1+json",
"Cache-Control": "",
authorization: "accessToken",
},
signal: signal,
}
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ export const ITwinGrid = ({
removeITwinFromFavorites,
shouldRefetchFavorites,
resetShouldRefetchFavorites,
} = useITwinFavorites(accessToken, apiOverrides);
} = useITwinFavorites(accessToken, apiOverrides?.serverEnvironmentPrefix);

const strings = _mergeStrings(
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export const useITwinData = ({
resetShouldRefetchFavorites,
}: ProjectDataHookOptions) => {
const data = apiOverrides?.data;
const serverEnvironmentPrefix = apiOverrides?.serverEnvironmentPrefix;
const [projects, setProjects] = React.useState<ITwinFull[]>([]);
const [status, setStatus] = React.useState<DataStatus>();
const filteredProjects = useITwinFilter(projects, filterOptions);
Expand Down Expand Up @@ -69,7 +70,7 @@ export const useITwinData = ({
setProjects([]);
setPage(0);
setMorePages(true);
}, [accessToken, requestType, iTwinSubClass, data, apiOverrides]);
}, [accessToken, requestType, iTwinSubClass, data, serverEnvironmentPrefix]);

React.useEffect(() => {
if (!morePages) {
Expand Down Expand Up @@ -100,7 +101,7 @@ export const useITwinData = ({
? ""
: `&$search=${filterOptions}`;
const url = `${_getAPIServer(
apiOverrides
serverEnvironmentPrefix
)}/itwins/${endpoint}${subClass}${paging}${search}`;

const makeFetchRequest = async () => {
Expand Down Expand Up @@ -152,7 +153,7 @@ export const useITwinData = ({
accessToken,
requestType,
data,
apiOverrides,
serverEnvironmentPrefix,
filterOptions,
page,
morePages,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

import { useCallback, useEffect, useState } from "react";

import { ApiOverrides, ITwinFull } from "../../types";
import { _getAPIServer } from "../../utils/_apiOverrides";

const HOOK_ABORT_ERROR =
Expand All @@ -24,7 +23,7 @@ const HOOK_ABORT_ERROR =
*/
export const useITwinFavorites = (
accessToken: string | (() => Promise<string>) | undefined,
apiOverrides?: ApiOverrides<ITwinFull[]>
serverEnvironmentPrefix?: "dev" | "qa" | ""
): {
iTwinFavorites: Set<string>;
addITwinToFavorites: (iTwinId: string) => Promise<void>;
Expand All @@ -45,7 +44,9 @@ export const useITwinFavorites = (
if (!accessToken || !iTwinId || iTwinId === "") {
return;
}
const url = `${_getAPIServer(apiOverrides)}/itwins/favorites/${iTwinId}`;
const url = `${_getAPIServer(
serverEnvironmentPrefix
)}/itwins/favorites/${iTwinId}`;
try {
const result = await fetch(url, {
method: "POST",
Expand All @@ -68,7 +69,7 @@ export const useITwinFavorites = (
console.error(error);
}
},
[accessToken, apiOverrides]
[accessToken, serverEnvironmentPrefix]
);

/**
Expand All @@ -81,7 +82,9 @@ export const useITwinFavorites = (
if (!accessToken || !iTwinId || iTwinId === "") {
return;
}
const url = `${_getAPIServer(apiOverrides)}/itwins/favorites/${iTwinId}`;
const url = `${_getAPIServer(
serverEnvironmentPrefix
)}/itwins/favorites/${iTwinId}`;
try {
const result = await fetch(url, {
method: "DELETE",
Expand All @@ -108,7 +111,7 @@ export const useITwinFavorites = (
console.error(error);
}
},
[accessToken, apiOverrides]
[accessToken, serverEnvironmentPrefix]
);

/**
Expand All @@ -123,7 +126,7 @@ export const useITwinFavorites = (
return [];
}
const url = `${_getAPIServer(
apiOverrides
serverEnvironmentPrefix
)}/itwins/favorites?subClass=Project`;
const result = await fetch(url, {
headers: {
Expand Down Expand Up @@ -152,7 +155,7 @@ export const useITwinFavorites = (
const response: ITwinFavoritesResponse = await result.json();
return response.iTwins;
},
[accessToken, apiOverrides, shouldRefetchFavorites]
[accessToken, serverEnvironmentPrefix, shouldRefetchFavorites]
);

const resetShouldRefetchFavorites = useCallback(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ export const useITwinTableConfig = ({
iTwinActions,
iTwinFavorites,
removeITwinFromFavorites,
strings.addToFavorites,
strings.removeFromFavorites,
strings.tableColumnDescription,
strings.tableColumnFavorites,
strings.tableColumnLastModified,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { useIModelData } from "./useIModelData";
import { useIModelTableConfig } from "./useIModelTableConfig";
export interface IModelGridProps {
/**
* Access token that requires the `imodels:read` scope. Provide a function that returns the token to prevent the token from expiring. */
* Access token that requires the `imodels:read` scope. Must be memoized. Provide a function that returns the token to prevent the token from expiring. */
accessToken?: string | (() => Promise<string>) | undefined;
/** ITwin Id to list the iModels from (mutually exclusive to assetId) */
iTwinId?: string | undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export const useIModelData = ({
accessToken,
iTwinId,
apiOverrides?.data,
apiOverrides,
apiOverrides?.serverEnvironmentPrefix,
searchText,
maxCount,
]);
Expand Down Expand Up @@ -110,10 +110,9 @@ export const useIModelData = ({
const searching = searchText?.trim() ? `&$search=${searchText}` : "";

const abortController = new AbortController();
const url = `${_getAPIServer({
data: apiOverrides?.data,
serverEnvironmentPrefix: apiOverrides?.serverEnvironmentPrefix,
})}/imodels/${selection}${sorting}${paging}${searching}`;
const url = `${_getAPIServer(
apiOverrides?.serverEnvironmentPrefix
)}/imodels/${selection}${sorting}${paging}${searching}`;

const makeFetchRequest = async () => {
const options: RequestInit = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ export const useIModelThumbnail = (
};

const response = await fetch(
`${_getAPIServer(apiOverrides)}/imodels/${iModelId}/thumbnail`,
`${_getAPIServer(
apiOverrides?.serverEnvironmentPrefix
)}/imodels/${iModelId}/thumbnail`,
options
);
const thumbnail: string = response.ok
Expand All @@ -76,6 +78,12 @@ export const useIModelThumbnail = (
return () => {
abortController.abort();
};
}, [accessToken, iModelId, thumbnail, apiOverrides?.data, apiOverrides]);
}, [
accessToken,
iModelId,
thumbnail,
apiOverrides?.data,
apiOverrides?.serverEnvironmentPrefix,
]);
return thumbnail;
};
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@ import { _getAPIServer, _mergeStrings } from "./_apiOverrides";

describe("apiOverrides", () => {
describe("getAPIServer", () => {
it("returns https://api.bentley.com with no serverEnvironmentPrefix", () => {
const result = _getAPIServer({});
it("returns https://api.bentley.com with empty serverEnvironmentPrefix", () => {
const result = _getAPIServer("");

expect(result).toEqual("https://api.bentley.com");
});
it("returns https://[prefix]-api.bentley.com when provided with [prefix]", () => {
const result = _getAPIServer({ serverEnvironmentPrefix: "qa" });
const result = _getAPIServer("qa");

expect(result).toEqual("https://qa-api.bentley.com");
});
it("handles undefined overrdies", () => {
it("handles undefined overrides", () => {
const result = _getAPIServer(undefined);

expect(result).toEqual("https://api.bentley.com");
Expand Down
9 changes: 2 additions & 7 deletions packages/modules/imodel-browser/src/utils/_apiOverrides.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,13 @@
* Copyright (c) Bentley Systems, Incorporated. All rights reserved.
* See LICENSE.md in the project root for license terms and full copyright notice.
*--------------------------------------------------------------------------------------------*/
import { ApiOverrides } from "../types";

/** Build APIM server url out of overrides
* @private
*/
export const _getAPIServer = (
apiOverrides: ApiOverrides<unknown> | undefined
) =>
export const _getAPIServer = (serverEnvironmentPrefix?: "dev" | "qa" | "") =>
`https://${
apiOverrides?.serverEnvironmentPrefix
? `${apiOverrides.serverEnvironmentPrefix}-`
: ""
serverEnvironmentPrefix ? `${serverEnvironmentPrefix}-` : ""
}api.bentley.com`;

/**
Expand Down

0 comments on commit 95d04cf

Please sign in to comment.