Send a SSS response immediately if the config has changed and there are new results to sync#19714
Conversation
…re new results to sync
bnjbvr
changed the title
bnjbvr
force-pushed
the
bnjbvr/fix-sss-subscription-not-returning-immediately
branch
from
074ea41 to
30117dc
Compare
bnjbvr
force-pushed
the
bnjbvr/fix-sss-subscription-not-returning-immediately
branch
from
30117dc to
4a9158e
Compare
This just makes it easier to follow.
erikjohnston
left a comment
erikjohnston
left a comment
There was a problem hiding this comment.
Thanks for finding this!
|
|
| @@ -0,0 +1 @@ | |||
| Have SSS return a new response immediately if a room subscription have changed and produced a new response. | |||
There was a problem hiding this comment.
The PR description says this fixes #18844
But I don't see an associated test update as described in #18844 (comment)
| sync_config.user.to_string(), | ||
| timeout_ms, | ||
| current_sync_callback, | ||
| from_token=now_token, |
There was a problem hiding this comment.
Just to make sure I'm following this logic, we use now_token here because we already checked for data between from_token -> now_token with the first current_sync_for_user(...) call above and found nothing. So we can save trying to lookup anything in that range and jump from now_token -> token after waiting
Feels like this deserves a comment as an optimization. Something along the lines of from_token.stream_token still being the right answer but we can save ... because ...
4 participants
This fixes the bug described in #19713 (and double-checked against the SDK integration test, which now passes with this change). A sync response must be returned immediately if a room subscription configuration change caused a new non-empty response (checked with
if responsein the code) to be produced.Fixes #19713.
Fixes #18844.