Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optim, add user identities cache #582

Merged
merged 6 commits into from
Jan 3, 2025
Merged

Optim, add user identities cache #582

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
782193c
feat(): cache user identities locally and avoid multiple fetches if i…
sBouzols Dec 16, 2024
e9efc3a
Merge branch 'main' into feat_optim_users-identity_cache
sBouzols Dec 19, 2024
cea9317
Merge branch 'main' into feat_optim_users-identity_cache
sBouzols Jan 2, 2025
6faf1f1
Merge branch 'main' into feat_optim_users-identity_cache
EstherDarkish Jan 3, 2025
9685f17
remove debug console log
sBouzols Jan 3, 2025
367d201
fix filter by knownUsersIdentitiesRef owner OR lastModifiedBy
sBouzols Jan 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 19 additions & 9 deletions src/hooks/useDirectoryContent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const getName = (userId: string, data: UsersIdentitiesMap): string => {

export const useDirectoryContent = () => {
const currentChildren = useSelector((state: AppState) => state.currentChildren);
const knownUsersIdentitiesRef = useRef<UsersIdentitiesMap>({});
const [childrenMetadata, setChildrenMetadata] = useState<Record<UUID, ElementAttributes>>({});
const { snackError } = useSnackMessage();
const previousData = useRef<ElementAttributes[]>();
Expand Down Expand Up @@ -58,8 +59,14 @@ export const useDirectoryContent = () => {

const metadata: Record<UUID, ElementAttributes> = {};
const childrenToFetchElementsInfos = Object.values(currentChildren).map((e) => e.elementUuid);

const fetchUsersIdentitiesPromise = fetchUsersIdentities(childrenToFetchElementsInfos).catch(() => {
const childrenToFetchUsersIdentitiesInfos = Object.values(currentChildren)
.filter(
(e) =>
knownUsersIdentitiesRef.current?.[e.owner] === undefined &&
knownUsersIdentitiesRef.current?.[e.lastModifiedBy] === undefined
)
.map((e) => e.elementUuid);
const fetchUsersIdentitiesPromise = fetchUsersIdentities(childrenToFetchUsersIdentitiesInfos).catch(() => {
// Last resort, server down, error 500, fallback to subs as users Identities
// We write this code to have the same behavior as when there are partial results,
// (missing users identities), see getName()
Expand All @@ -77,16 +84,19 @@ export const useDirectoryContent = () => {
});

if (childrenToFetchElementsInfos.length > 0) {
Promise.all([
fetchUsersIdentitiesPromise, // TODO cache user identities across elements
fetchElementsInfos(childrenToFetchElementsInfos),
])
Promise.all([fetchUsersIdentitiesPromise, fetchElementsInfos(childrenToFetchElementsInfos)])
.then((res) => {
if (res[0] && res[0].data) {
Object.entries(res[0].data).forEach(([k, v]) => {
knownUsersIdentitiesRef.current[k] = v;
});
}

// discarding request for older directory
if (previousData.current === currentChildren) {
res[1].forEach((e) => {
e.ownerLabel = getName(e.owner, res[0].data);
e.lastModifiedByLabel = getName(e.lastModifiedBy, res[0].data);
e.ownerLabel = getName(e.owner, knownUsersIdentitiesRef.current);
e.lastModifiedByLabel = getName(e.lastModifiedBy, knownUsersIdentitiesRef.current);
metadata[e.elementUuid] = e;
});
setChildrenMetadata(metadata);
Expand All @@ -100,7 +110,7 @@ export const useDirectoryContent = () => {
}
}, [handleError, currentChildren]);

// TODO remove this when global user identity caching is implemented
// TODO remove this when currentChildren and metadata are fetched at once
const currentChildrenWithOwnerNames = useMemo(() => {
if (!currentChildren) {
return currentChildren;
Expand Down
6 changes: 5 additions & 1 deletion src/utils/rest-api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,11 @@ export function fetchVersion() {
}

export function fetchUsersIdentities(elementUuids: string[]) {
console.info('fetching users identities for elements : %s', elementUuids);
if (elementUuids.length === 0) {
return Promise.resolve();
}
console.info('fetching users identities for %s elements.', elementUuids.length);
console.debug('fetching users identities for elements: %s', elementUuids);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this debug line necessary ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

const idsParams = getRequestParamFromList('ids', elementUuids).toString();
const fetchParams = `${PREFIX_EXPLORE_SERVER_QUERIES}/v1/explore/elements/users-identities?${idsParams}`;
return backendFetchJson(fetchParams, {
Expand Down
Loading