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

fix(client): correct loading behavior, add MedplumClient lifecycle events #4701

Merged
merged 4 commits into from
Jun 23, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
fix(client): make sure isLoading() and loading are actually in sync
  • Loading branch information
ThatOneBro committed Jun 19, 2024
commit a437f2fa29818167622d1da9affa986f561e3c8c
14 changes: 12 additions & 2 deletions packages/core/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,14 @@ import {
UserConfiguration,
ValueSet,
} from '@medplum/fhirtypes';
import { TypedEventTarget } from './eventtarget';
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
/** @ts-ignore */
import type { CustomTableLayout, TDocumentDefinitions, TFontDictionary } from 'pdfmake/interfaces';
import { encodeBase64 } from './base64';
import { LRUCache } from './cache';
import { ContentType } from './contenttype';
import { encryptSHA256, getRandomString } from './crypto';
import { EventTarget } from './eventtarget';
import {
CurrentContext,
FhircastConnection,
Expand Down Expand Up @@ -682,6 +682,13 @@ export interface RequestProfileSchemaOptions {
expandProfile?: boolean;
}

export type MedplumClientEventMap = {
change: { type: 'change' };
offline: { type: 'offline' };
profileRefreshed: { type: 'profileRefreshed' };
storageInitialized: { type: 'storageInitialized' };
};

/**
* The MedplumClient class provides a client for the Medplum FHIR server.
*
Expand Down Expand Up @@ -739,7 +746,7 @@ export interface RequestProfileSchemaOptions {
* <meta name="algolia:pageRank" content="100" />
* </head>
*/
export class MedplumClient extends EventTarget {
export class MedplumClient extends TypedEventTarget<MedplumClientEventMap> {
private readonly options: MedplumClientOptions;
private readonly fetch: FetchLike;
private readonly createPdfImpl?: CreatePdfFunction;
Expand Down Expand Up @@ -817,6 +824,7 @@ export class MedplumClient extends EventTarget {
this.attemptResumeActiveLogin().catch(console.error);
}
this.initPromise = Promise.resolve();
this.dispatchEvent({ type: 'storageInitialized' });
} else {
this.initComplete = false;
this.initPromise = this.storage.getInitPromise();
Expand All @@ -826,6 +834,7 @@ export class MedplumClient extends EventTarget {
this.attemptResumeActiveLogin().catch(console.error);
}
this.initComplete = true;
this.dispatchEvent({ type: 'storageInitialized' });
})
.catch(console.error);
}
Expand Down Expand Up @@ -2721,6 +2730,7 @@ export class MedplumClient extends EventTarget {
if (profileChanged) {
this.dispatchEvent({ type: 'change' });
}
this.dispatchEvent({ type: 'profileRefreshed' });
resolve(result.profile);
})
.catch(reject);
Expand Down
42 changes: 36 additions & 6 deletions packages/mock/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import {
BinarySource,
ContentType,
CreateBinaryOptions,
IClientStorage,
LoginState,
MedplumClient,
MedplumClientOptions,
Expand Down Expand Up @@ -93,9 +92,21 @@ export interface MockClientOptions extends MedplumClientOptions {
* MedplumContext.profile returning undefined as if no one were logged in.
*/
readonly profile?: ReturnType<MedplumClient['getProfile']> | null;
readonly storage?: IClientStorage;
/**
* Override the `MockFetchClient` used by this `MockClient`.
*/
readonly mockFetchOverride?: MockFetchOverrideOptions;
}

/**
* Override must contain all of `router`, `repo`, and `client`.
*/
export type MockFetchOverrideOptions = {
client: MockFetchClient;
router: FhirRouter;
repo: MemoryRepository;
};

export class MockClient extends MedplumClient {
readonly router: FhirRouter;
readonly repo: MemoryRepository;
Expand All @@ -107,11 +118,30 @@ export class MockClient extends MedplumClient {
subManager: MockSubscriptionManager | undefined;

constructor(clientOptions?: MockClientOptions) {
const router = new FhirRouter();
const repo = new MemoryRepository();

const baseUrl = clientOptions?.baseUrl ?? 'https://example.com/';
const client = new MockFetchClient(router, repo, baseUrl, clientOptions?.debug);

let router: FhirRouter;
let repo: MemoryRepository;
let client: MockFetchClient;

if (clientOptions?.mockFetchOverride) {
if (
!(
clientOptions.mockFetchOverride?.router &&
clientOptions.mockFetchOverride?.repo &&
clientOptions.mockFetchOverride?.client
)
) {
throw new Error('mockFetchOverride must specify all fields: client, repo, router');
}
router = clientOptions.mockFetchOverride.router;
repo = clientOptions.mockFetchOverride.repo;
client = clientOptions.mockFetchOverride.client;
} else {
router = new FhirRouter();
repo = new MemoryRepository();
client = new MockFetchClient(router, repo, baseUrl, clientOptions?.debug);
}

super({
baseUrl,
Expand Down
212 changes: 182 additions & 30 deletions packages/react-hooks/src/MedplumProvider/MedplumProvider.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
import { MockAsyncClientStorage, ProfileResource, getDisplayString, sleep } from '@medplum/core';
import { MockClient } from '@medplum/mock';
import {
ClientStorage,
MemoryStorage,
MockAsyncClientStorage,
ProfileResource,
getDisplayString,
sleep,
} from '@medplum/core';
import { FhirRouter, MemoryRepository } from '@medplum/fhir-router';
import { MockClient, MockFetchClient } from '@medplum/mock';
import { act, render, screen } from '@testing-library/react';
import { MedplumProvider } from './MedplumProvider';
import { useMedplum, useMedplumContext, useMedplumNavigate, useMedplumProfile } from './MedplumProvider.context';
Expand Down Expand Up @@ -36,50 +44,194 @@ describe('MedplumProvider', () => {
expect(screen.getByText('Profile: true')).toBeInTheDocument();
});

test('Loading is always in sync with isLoading()', async () => {
describe('Loading is always in sync with MedplumClient#isLoading()', () => {
function MyComponent(): JSX.Element {
const { loading } = useMedplumContext();
return loading ? <div>Loading...</div> : <div>Loaded!</div>;
}

let storage: MockAsyncClientStorage;
let medplum!: MockClient;
test('No active login & AsyncClientStorage', async () => {
let storage: MockAsyncClientStorage;
let medplum!: MockClient;

act(() => {
storage = new MockAsyncClientStorage();
medplum = new MockClient({ storage });
});
act(() => {
storage = new MockAsyncClientStorage();
medplum = new MockClient({ storage });
});

expect(medplum.isLoading()).toEqual(true);
expect(medplum.isLoading()).toEqual(true);

act(() => {
render(
<MedplumProvider medplum={medplum}>
<MyComponent />
</MedplumProvider>
);
act(() => {
render(
<MedplumProvider medplum={medplum}>
<MyComponent />
</MedplumProvider>
);
});

expect(screen.getByText('Loading...')).toBeInTheDocument();
expect(medplum.isLoading()).toEqual(true);

// Sleep to make sure that loading doesn't go to false before we set storage to initialized
await act(async () => {
await sleep(250);
});

expect(screen.getByText('Loading...')).toBeInTheDocument();
expect(medplum.isLoading()).toEqual(true);

// Finally set storage to initialized
act(() => {
storage.setInitialized();
});

expect(screen.getByText('Loading...')).toBeInTheDocument();
expect(medplum.isLoading()).toEqual(true);

expect(await screen.findByText('Loaded!')).toBeInTheDocument();
expect(medplum.isLoading()).toEqual(false);
});

expect(screen.getByText('Loading...')).toBeInTheDocument();
expect(medplum.isLoading()).toEqual(true);
test('Active login & AsyncClientStorage', async () => {
let storage: MockAsyncClientStorage;
let medplum!: MockClient;

act(() => {
storage = new MockAsyncClientStorage();
storage.setObject('activeLogin', {
accessToken:
'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwiaWF0IjoxNTE2MjM5MDIyLCJsb2dpbl9pZCI6InRlc3RpbmcxMjMifQ.lJGCbp2taTarRbamxaKFsTR_VRVgzvttKMmI5uFQSM0',
refreshToken: '456',
profile: {
reference: 'Practitioner/123',
},
project: {
reference: 'Project/123',
},
});
medplum = new MockClient({
storage,
});
});

const getSpy = jest.spyOn(medplum, 'get');

expect(medplum.isLoading()).toEqual(true);

act(() => {
render(
<MedplumProvider medplum={medplum}>
<MyComponent />
</MedplumProvider>
);
});

expect(screen.getByText('Loading...')).toBeInTheDocument();
expect(medplum.isLoading()).toEqual(true);

// Sleep to make sure that loading doesn't go to false before we set storage to initialized
await act(async () => {
await sleep(250);
});

expect(screen.getByText('Loading...')).toBeInTheDocument();
expect(medplum.isLoading()).toEqual(true);

// Sleep to make sure that loading doesn't go to false before we set storage to initialized
await act(async () => {
await sleep(500);
// Finally set storage to initialized
act(() => {
storage.setInitialized();
});

expect(screen.getByText('Loading...')).toBeInTheDocument();
expect(medplum.isLoading()).toEqual(true);

// Sleep to make sure that we can resolve stuff
await act(async () => {
await sleep(0);
});

expect(await screen.findByText('Loaded!')).toBeInTheDocument();
expect(medplum.isLoading()).toEqual(false);
expect(getSpy).toHaveBeenCalledWith('auth/me');

getSpy.mockRestore();
});

expect(screen.getByText('Loading...')).toBeInTheDocument();
expect(medplum.isLoading()).toEqual(true);
test('No active login & NO AsyncClientStorage', async () => {
const baseUrl = 'https://example.com/';
let medplum!: MockClient;
const router = new FhirRouter();
const repo = new MemoryRepository();
const client = new MockFetchClient(router, repo, baseUrl);
const mockFetchSpy = jest.spyOn(client, 'mockFetch');

act(() => {
medplum = new MockClient();
});

expect(medplum.isLoading()).toEqual(true);

act(() => {
render(
<MedplumProvider medplum={medplum}>
<MyComponent />
</MedplumProvider>
);
});

// Finally set storage to initialized
act(() => {
storage.setInitialized();
expect(screen.getByText('Loading...')).toBeInTheDocument();
expect(medplum.isLoading()).toEqual(true);

expect(await screen.findByText('Loaded!')).toBeInTheDocument();
expect(medplum.isLoading()).toEqual(false);
expect(mockFetchSpy).not.toHaveBeenCalledWith(`${baseUrl}auth/me`, expect.objectContaining({ method: 'GET' }));
});

expect(await screen.findByText('Loaded!')).toBeInTheDocument();
expect(medplum.isLoading()).toEqual(false);
test('Active login & NO AsyncClientStorage', async () => {
const baseUrl = 'https://example.com/';
const storage = new ClientStorage(new MemoryStorage());
storage.setObject('activeLogin', {
accessToken:
'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwiaWF0IjoxNTE2MjM5MDIyLCJsb2dpbl9pZCI6InRlc3RpbmcxMjMifQ.lJGCbp2taTarRbamxaKFsTR_VRVgzvttKMmI5uFQSM0',
refreshToken: '456',
profile: {
reference: 'Practitioner/123',
},
project: {
reference: 'Project/123',
},
});
let medplum!: MockClient;
const router = new FhirRouter();
const repo = new MemoryRepository();
const client = new MockFetchClient(router, repo, baseUrl);
const mockFetchSpy = jest.spyOn(client, 'mockFetch');

act(() => {
medplum = new MockClient({
storage,
mockFetchOverride: { router, repo, client },
});
});

expect(medplum.isLoading()).toEqual(true);

act(() => {
render(
<MedplumProvider medplum={medplum}>
<MyComponent />
</MedplumProvider>
);
});

// Make sure we are actually authed after loading === false
expect(medplum.getProfile()).toBeDefined();
expect(screen.getByText('Loading...')).toBeInTheDocument();
expect(medplum.isLoading()).toEqual(true);

expect(await screen.findByText('Loaded!')).toBeInTheDocument();
expect(medplum.isLoading()).toEqual(false);
expect(mockFetchSpy).toHaveBeenCalledWith(`${baseUrl}auth/me`, expect.objectContaining({ method: 'GET' }));

mockFetchSpy.mockRestore();
});
});
});
Loading