Skip to content

Commit

Permalink
fix-3815 cache presigned s3 binary urls (medplum#3834)
Browse files Browse the repository at this point in the history
* fix-3815 cache presigned s3 binary urls

* feedback

* prettier
  • Loading branch information
dillonstreator committed Jan 30, 2024
1 parent 638e42b commit 37931f2
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 3 deletions.
1 change: 1 addition & 0 deletions packages/react-hooks/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ export * from './MedplumProvider/MedplumProvider';
export * from './MedplumProvider/MedplumProvider.context';
export * from './useResource/useResource';
export * from './useSearch/useSearch';
export * from './useCachedBinaryUrl/useCachedBinaryUrl';
57 changes: 57 additions & 0 deletions packages/react-hooks/src/useCachedBinaryUrl/useCachedBinaryUrl.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { useMemo } from 'react';

// Maintain a cache of urls to avoid unnecessary re-download of attachments
// The following is a workaround for the fact that each request to a resource containing a Binary data reference
// returns a NEW signed S3 URL for each bypassing the native browser caching mechanism
// resulting in unnecessary bandwidth consumption.
// https://www.medplum.com/docs/fhir-datastore/binary-data#consuming-a-fhir-binary-in-an-application
// https://github.com/medplum/medplum/issues/3815

// TODO: possibly retain this cache in and restore from local storage to persist across page refreshes?
// The S3 presigned URLs expire after 1 hour with the default configuration and hard refreshes are not uncommon even in SPAs so this
// could be a good way to get additional cache hits
// This would require additional logic for initialization, saving, and purging of expired keys
const urls = new Map<string, string>();

export const useCachedBinaryUrl = (binaryUrl: string | undefined): string | undefined => {
return useMemo(() => {
if (!binaryUrl) {
return undefined;
}

const binaryResourceUrl = binaryUrl.split('?')[0];
if (!binaryResourceUrl) {
return binaryUrl;
}

// Check if the binaryUrl is a presigned S3 URL
const binaryUrlSearchParams = new URLSearchParams(new URL(binaryUrl).search);
if (!binaryUrlSearchParams.has('Key-Pair-Id') || !binaryUrlSearchParams.has('Signature')) {
return binaryUrl;
}

// https://stackoverflow.com/questions/23929145/how-to-test-if-a-given-time-stamp-is-in-seconds-or-milliseconds
const binaryUrlExpires = binaryUrlSearchParams.get('Expires');
if (!binaryUrlExpires || binaryUrlExpires.length > 13) {
// Expires is expected to be in seconds, not milliseconds
return binaryUrl;
}

const cachedUrl = urls.get(binaryResourceUrl);
if (cachedUrl) {
const searchParams = new URLSearchParams(new URL(cachedUrl).search);

// This is fairly brittle as it relies on the current structure of the Medplum returned URL
const expires = searchParams.get('Expires');

// `expires` is in seconds, Date.now() is in ms
// Add padding to mitigate expiration between time of check and time of use
if (expires && parseInt(expires, 10) * 1000 - 5_000 > Date.now()) {
return cachedUrl;
}
}

urls.set(binaryResourceUrl, binaryUrl);
return binaryUrl;
}, [binaryUrl]);
};
4 changes: 3 additions & 1 deletion packages/react/src/AttachmentDisplay/AttachmentDisplay.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import { Anchor } from '@mantine/core';
import { Attachment } from '@medplum/fhirtypes';
import { useCachedBinaryUrl } from '@medplum/react-hooks';

export interface AttachmentDisplayProps {
readonly value?: Attachment;
readonly maxWidth?: number;
}

export function AttachmentDisplay(props: AttachmentDisplayProps): JSX.Element | null {
const { contentType, url, title } = props.value ?? {};
const { contentType, url: uncachedUrl, title } = props.value ?? {};
const url = useCachedBinaryUrl(uncachedUrl);

if (!url) {
return null;
Expand Down
5 changes: 3 additions & 2 deletions packages/react/src/ResourceAvatar/ResourceAvatar.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Avatar, AvatarProps } from '@mantine/core';
import { getDisplayString, getImageSrc } from '@medplum/core';
import { Reference, Resource } from '@medplum/fhirtypes';
import { useResource } from '@medplum/react-hooks';
import { useCachedBinaryUrl, useResource } from '@medplum/react-hooks';
import { MedplumLink } from '../MedplumLink/MedplumLink';

export interface ResourceAvatarProps extends AvatarProps {
Expand All @@ -12,7 +12,8 @@ export interface ResourceAvatarProps extends AvatarProps {
export function ResourceAvatar(props: ResourceAvatarProps): JSX.Element {
const resource = useResource(props.value);
const text = resource ? getDisplayString(resource) : props.alt ?? '';
const imageUrl = (resource && getImageSrc(resource)) ?? props.src;
const uncachedImageUrl = (resource && getImageSrc(resource)) ?? props.src;
const imageUrl = useCachedBinaryUrl(uncachedImageUrl ?? undefined);
const radius = props.radius ?? 'xl';

const avatarProps = { ...props };
Expand Down

0 comments on commit 37931f2

Please sign in to comment.