Skip to content

Commit

Permalink
fix(property-provider): do not make extra provider calls when concurr…
Browse files Browse the repository at this point in the history
…ent (#2716)

* fix(property-provider): do not make extra provider invocation when concurrent

Co-authored-by: Trivikram Kamat <16024985+trivikr@users.noreply.github.com>
  • Loading branch information
AllanZhengYP and trivikr authored Aug 27, 2021
1 parent a42033e commit f9aa15e
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 6 deletions.
29 changes: 29 additions & 0 deletions packages/property-provider/src/memoize.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,15 @@ describe("memoize", () => {
}
});

it("should not make extra request for concurrent calls", async () => {
const memoized = memoize(provider);
const results = await Promise.all([...Array(repeatTimes).keys()].map(() => memoized()));
expect(provider).toHaveBeenCalledTimes(1);
for (const res of results) {
expect(res).toStrictEqual(mockReturn);
}
});

it("should retry provider if previous provider is failed", async () => {
provider
.mockReset()
Expand Down Expand Up @@ -131,6 +140,26 @@ describe("memoize", () => {
});
});

describe("should not make extra request for concurrent calls", () => {
const requiresRefreshFalseTest = async () => {
const memoized = memoize(provider, isExpired, requiresRefresh);
const results = await Promise.all([...Array(repeatTimes).keys()].map(() => memoized()));
expect(provider).toHaveBeenCalledTimes(1);
for (const res of results) {
expect(res).toStrictEqual(mockReturn);
}
};

it("when isExpired returns true", () => {
return requiresRefreshFalseTest();
});

it("when isExpired returns false", () => {
isExpired.mockReturnValue(false);
return requiresRefreshFalseTest();
});
});

it("should retry provider if previous provider is failed", async () => {
provider
.mockReset()
Expand Down
26 changes: 20 additions & 6 deletions packages/property-provider/src/memoize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,28 @@ export const memoize: MemoizeOverload = <T>(
isExpired?: (resolved: T) => boolean,
requiresRefresh?: (resolved: T) => boolean
): Provider<T> => {
let resolved: any;
let resolved: T;
let pending: Promise<T> | undefined;
let hasResult: boolean;
// Wrapper over supplied provider with side effect to handle concurrent invocation.
const coalesceProvider: Provider<T> = async () => {
if (!pending) {
pending = provider();
}
try {
resolved = await pending;
hasResult = true;
} finally {
pending = undefined;
}
return resolved;
};

if (isExpired === undefined) {
// This is a static memoization; no need to incorporate refreshing
return async () => {
if (!hasResult) {
resolved = await provider();
hasResult = true;
resolved = await coalesceProvider();
}
return resolved;
};
Expand All @@ -62,8 +76,7 @@ export const memoize: MemoizeOverload = <T>(

return async () => {
if (!hasResult) {
resolved = await provider();
hasResult = true;
resolved = await coalesceProvider();
}
if (isConstant) {
return resolved;
Expand All @@ -74,7 +87,8 @@ export const memoize: MemoizeOverload = <T>(
return resolved;
}
if (isExpired(resolved)) {
return (resolved = await provider());
await coalesceProvider();
return resolved;
}
return resolved;
};
Expand Down

0 comments on commit f9aa15e

Please sign in to comment.