Clear related pending requests on session disconnect #1354
Conversation
|
| } | ||
|
|
||
| func disconnect(topic: String) async throws { | ||
| invalidRequestsSanitiser.removeSessionRequestsWith(topic: topic) |
There was a problem hiding this comment.
A couple of notes:
- I'm not sure what the performance impact of this call is, if it blocks the thread and for how long
- Not sure if this is thread safe
We should check before merging.
| @@ -17,4 +24,27 @@ final class InvalidRequestsSanitiser { | |||
| history.deleteAll(forTopics: Array(invalidTopics)) | |||
| } | |||
| } | |||
|
|
|||
| func removeSessionRequestsWith(topic: String) { | |||
There was a problem hiding this comment.
just a style comment as it would be more consistent with what we have, can we declare this func like: removeSessionRequests(with topic: String)?
| let pendingRequestTopics = historyService | ||
| .getPendingRequests(topic: topic) | ||
| .map(\.request) | ||
| .map(\.topic) |
There was a problem hiding this comment.
do we need to perform this operation? don't we get a single topic that was provided during a function call as a result?
| } | ||
|
|
||
| func disconnect(topic: String) async throws { | ||
| invalidRequestsSanitiser.removeSessionRequestsWith(topic: topic) | ||
|
|
||
| if sessionStorage.hasSession(forTopic: topic) { | ||
| try await deleteSessionService.delete(topic: topic) |
There was a problem hiding this comment.
should we maybe call invalidRequestsSanitiser.removeSessionRequestsWith(topic: topic) here?
There was a problem hiding this comment.
but I think we could also just call rpchistory.deleteAll(forTopic: topic)
Description
This is a follow up to #1353.
In addition to clearing stale requests on initialisation, this PR also makes sure requests related to the session that is being
disconnected are also cleared.Resolves #1343
How Has This Been Tested?
Note
This has not yet been tested against the bug as I am still unable to reproduce, this will be tested before merging.
Due Dilligence