-
Notifications
You must be signed in to change notification settings - Fork 66
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
Add demonstrative DeduplicationInterceptor
to reduce duplicate requests
#2190
Conversation
expect(testService.fetchData).toHaveBeenCalledTimes(2); | ||
}); | ||
|
||
it.todo('test all variations of registry key'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we approve of the implementation, these tests will be expanded.
}); | ||
|
||
it('should handle different requests separately', async () => { | ||
(testService.fetchData as jest.Mock).mockClear(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll align this more with our mocking approach should be move foward with it.
@@ -160,6 +161,10 @@ export class AppModule implements NestModule { | |||
provide: APP_INTERCEPTOR, | |||
useClass: CacheControlInterceptor, | |||
}, | |||
{ | |||
provide: APP_INTERCEPTOR, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might need be localised to datasources, otherwise different calls to datasources within a mapping might still make separate requests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've just experimented some more and this is indeed the case.
I've since learnt that inteceptors are controller-focused. As such, we'd need take another approach. As such, I'm going to close this. |
Summary
Ideates on #2181 and our previous (reverted) promise registry implementation.
Due to the async nature of the project, services can send several requests of the same requests in parallel. Only after one of these succeeds is the result cached.
This implements an interceptor which registers each
Request
promise in a registry. Should a subsequentRequest
happen and the aforementioned registration exist, it is returned instead of making a parallel one.Changes
DeduplicationInterceptor
in the app