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.

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

Uppy refresh token logic #1152

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
63 changes: 62 additions & 1 deletion src/api/api.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import { readSciGatewayToken } from '../parseTokens';
import { InventoryManagementSystemSettings, settings } from '../settings';
import { InvalidateTokenType } from '../state/actions/actions.types';
import { tokenRefreshed } from '../state/scigateway.actions';
import { APIError } from './api.types';

// These are for ensuring refresh request is only sent once when multiple requests
Expand Down Expand Up @@ -79,7 +80,7 @@
return new Promise((resolve, reject) => {
failedAuthRequestQueue.push((shouldReject?: boolean) => {
if (shouldReject) reject(error);
else resolve(imsApi(originalRequest));
else resolve(apiClient(originalRequest));

Check warning on line 83 in src/api/api.tsx

View check run for this annotation

Codecov / codecov/patch

src/api/api.tsx#L83

Added line #L83 was not covered by tests
});
});
}
Expand All @@ -91,6 +92,66 @@
return apiClient;
};

export function uppyOnAfterResponse(xhr: XMLHttpRequest) {
if (xhr.status >= 400 && xhr.status < 600) {
const errorMessage: string = (
JSON.parse(xhr.responseText) as APIError
).detail.toLocaleLowerCase();

Check warning on line 99 in src/api/api.tsx

View check run for this annotation

Codecov / codecov/patch

src/api/api.tsx#L97-L99

Added lines #L97 - L99 were not covered by tests

// Check if the token is invalid and needs refreshing
if (
xhr.status === 403 &&
errorMessage.includes('expired token') &&
localStorage.getItem('scigateway:token')
) {

Check warning on line 106 in src/api/api.tsx

View check run for this annotation

Codecov / codecov/patch

src/api/api.tsx#L102-L106

Added lines #L102 - L106 were not covered by tests
// Prevent other requests from also attempting to refresh while waiting for
// SciGateway to refresh the token
if (!isFetchingAccessToken) {
isFetchingAccessToken = true;

Check warning on line 110 in src/api/api.tsx

View check run for this annotation

Codecov / codecov/patch

src/api/api.tsx#L109-L110

Added lines #L109 - L110 were not covered by tests

// Request SciGateway to refresh the token
document.dispatchEvent(
new CustomEvent(MicroFrontendId, {
detail: {
type: InvalidateTokenType,
},
})
);

Check warning on line 119 in src/api/api.tsx

View check run for this annotation

Codecov / codecov/patch

src/api/api.tsx#L113-L119

Added lines #L113 - L119 were not covered by tests

// Create a new promise to wait for the token to be refreshed
const tokenRefreshedPromise = new Promise<void>((resolve, reject) => {
const handler = (e: Event) => {
const action = (e as CustomEvent).detail;
if (tokenRefreshed.match(action)) {
document.removeEventListener(MicroFrontendId, handler);
isFetchingAccessToken = false;
resolve(); // Resolve the promise when the token is refreshed
}
};

Check warning on line 130 in src/api/api.tsx

View check run for this annotation

Codecov / codecov/patch

src/api/api.tsx#L122-L130

Added lines #L122 - L130 were not covered by tests

const timeoutId = setTimeout(() => {

Check warning on line 132 in src/api/api.tsx

View check run for this annotation

Codecov / codecov/patch

src/api/api.tsx#L132

Added line #L132 was not covered by tests
// If the token isn't refreshed within a reasonable timeframe, reject the promise
document.removeEventListener(MicroFrontendId, handler);
isFetchingAccessToken = false;
reject();
}, 20 * 1000); // 20 seconds timeout

Check warning on line 137 in src/api/api.tsx

View check run for this annotation

Codecov / codecov/patch

src/api/api.tsx#L134-L137

Added lines #L134 - L137 were not covered by tests
Comment on lines +132 to +137
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added a reject case to handle scenarios where the expected CustomEvent does not occur or takes too long.
Ensures the Promise does not hang indefinitely


document.addEventListener(MicroFrontendId, handler);

Check warning on line 139 in src/api/api.tsx

View check run for this annotation

Codecov / codecov/patch

src/api/api.tsx#L139

Added line #L139 was not covered by tests

// Cleanup timeout when resolved
handler.resolve = () => clearTimeout(timeoutId);
});

Check warning on line 143 in src/api/api.tsx

View check run for this annotation

Codecov / codecov/patch

src/api/api.tsx#L142-L143

Added lines #L142 - L143 were not covered by tests

return tokenRefreshedPromise;
}
}
}

Check warning on line 148 in src/api/api.tsx

View check run for this annotation

Codecov / codecov/patch

src/api/api.tsx#L145-L148

Added lines #L145 - L148 were not covered by tests
}

export function uppyOnBeforeRequest(xhr: XMLHttpRequest) {
xhr.setRequestHeader('Authorization', `Bearer ${readSciGatewayToken()}`);
}

export const imsApi = createAuthenticatedClient({
getURL: (settings) => settings.imsApiUrl,
});
Expand Down
16 changes: 5 additions & 11 deletions src/common/images/imageGallery.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -243,10 +243,8 @@ const ImageGallery = (props: ImageGalleryProps) => {
.getSortedRowModel()
.rows.map((row) => row.getVisibleCells().map((cell) => cell)[0]);
const displayedImages = table
.getPaginationRowModel()
.rows.map(
(row) => row.getVisibleCells().map((cell) => cell.row.original)[0]
);
.getRowModel()
.rows.map((row) => row.getVisibleCells().map((cell) => cell)[0]);

return (
<>
Expand Down Expand Up @@ -320,18 +318,14 @@ const ImageGallery = (props: ImageGalleryProps) => {
gridTemplateColumns: 'repeat(auto-fit, minmax(350px, 1fr))',
}}
>
{data.map((card, index) => {
const isUndisplayed = !displayedImages?.some(
(img) => img.id === card.row.original.id
);

{displayedImages.map((card, index) => {
const lastPageIndex = Math.floor(
data.length / preservedState.pagination.pageSize
displayedImages.length / preservedState.pagination.pageSize
);
const isLastPage =
preservedState.pagination.pageIndex === lastPageIndex;

return isUndisplayed ? null : (
return (
<Card
component={Grid}
item
Expand Down
19 changes: 11 additions & 8 deletions src/common/images/uploadImagesDialog.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import ProgressBar from '@uppy/progress-bar'; // Import the ProgressBar plugin
import { DashboardModal } from '@uppy/react';
import XHR from '@uppy/xhr-upload';
import React from 'react';
import { uppyOnAfterResponse, uppyOnBeforeRequest } from '../../api/api';
import { settings } from '../../settings';
import { getNonEmptyTrimmedString } from '../../utils';

Expand Down Expand Up @@ -47,6 +48,16 @@ const UploadImagesDialog = (props: UploadImagesDialogProps) => {
endpoint: `${url}/images`,
method: 'POST',
fieldName: 'upload_file',
limit: 1, // Limit uploads to one file at a time
// Reason 1: To avoid overloading the memory of the object-store API.
// Reason 2: To prevent multiple simultaneous uploads from triggering
// the token refresh process multiple times, which could lead to race conditions.
async onBeforeRequest(xhr) {
uppyOnBeforeRequest(xhr);
},
async onAfterResponse(xhr) {
await uppyOnAfterResponse(xhr);
},
});
});

Expand Down Expand Up @@ -86,14 +97,6 @@ const UploadImagesDialog = (props: UploadImagesDialogProps) => {
}
});

uppy.on('upload-error', (_file, _error, response) => {
if (response?.body?.id) {
// TODO: Implement logic to delete metadata using id
// If metadata exists for the given id, remove it from the api
// If not, do nothing and exit the function
}
});

return (
<DashboardModal
open={open}
Expand Down
Loading