Skip to content
Merged
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
60 changes: 60 additions & 0 deletions src/__tests__/deduplicate-inflight.decorator.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,4 +168,64 @@ describe('DeduplicateInflight decorator', () => {
expect(keyBuilderSpy).toHaveBeenCalledWith('user-1', 'v2')
})
})

describe('shared wrap context across instances', () => {
// base-decorators creates the wrap closure once and reassigns its shared
// `context.target` to the current `this` before every call. Two instances of
// the same decorated class (e.g. two AuthProcessors) therefore share one
// context. If the post-`await` cleanup re-reads `context.target`, it can
// delete the WRONG instance's entry when calls interleave — orphaning a
// settled promise so the owning instance short-circuits forever. Each call
// must operate on its own instance's map regardless of later `context.target`
// reassignment.
it('cleans up the calling instance, not whichever instance called last', async () => {
const methodSpy = jest.fn()

class AuthProcessorLike {
readonly inflightMap = new Map<string, Promise<unknown>>()
private resolveCurrent?: () => void

@DeduplicateInflight(() => 'authenticate')
async authenticate(): Promise<void> {
methodSpy()
return new Promise<void>(resolve => {
this.resolveCurrent = resolve
})
}

settle(): void {
this.resolveCurrent?.()
}
}

const a = new AuthProcessorLike()
const b = new AuthProcessorLike()

const callA = a.authenticate() // shared context.target = a
const callB = b.authenticate() // shared context.target flips to b

expect(a.inflightMap.has('authenticate')).toBe(true)
expect(b.inflightMap.has('authenticate')).toBe(true)

// Resolve A while the shared context.target points at B.
a.settle()
await callA

// A must clean ITS OWN entry; B's in-flight entry must be untouched.
expect(a.inflightMap.size).toBe(0)
expect(b.inflightMap.has('authenticate')).toBe(true)

// A must not be wedged: a fresh call actually re-invokes the method
// instead of short-circuiting on a stale, orphaned promise.
expect(methodSpy).toHaveBeenCalledTimes(2)
const callA2 = a.authenticate()
expect(methodSpy).toHaveBeenCalledTimes(3)

// settle the still-in-flight promises so the test leaves no dangling work
a.settle()
await callA2
b.settle()
await callB
})
})
})
14 changes: 11 additions & 3 deletions src/deduplicate-inflight.decorator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,20 +38,28 @@ function DeduplicateInflight<TArgs extends unknown[]>(keyBuilder: KeyBuilder<TAr
return Wrap<{ inflightMap: Map<string, Promise<unknown>> }, TArgs, Promise<unknown>>((method, context) => async (...args: TArgs): Promise<unknown> => {
const key = keyBuilder(...args)

const existing = context.target.inflightMap.get(key)
// Capture this call's map synchronously, before any `await`. base-decorators
// shares a single `context` across all instances of the decorated class and
// reassigns `context.target` to the current `this` before every call, so
// re-reading `context.target` after the `await` below can resolve to a
// DIFFERENT instance (e.g. a second AuthProcessor authenticating concurrently)
// — the cleanup would then delete from the wrong map and orphan this entry.
const { inflightMap } = context.target

const existing = inflightMap.get(key)
if (existing) {
return existing
}

const promise = method(...args)

context.target.inflightMap.set(key, promise)
inflightMap.set(key, promise)

try {
return await promise
}
finally {
context.target.inflightMap.delete(key)
inflightMap.delete(key)
}
}, INFLIGHT_EXCLUSION_KEY)
}
Expand Down
Loading