fix(sync): send first update without initial document state #7787
Conversation
Process initial document state as incoming message. This way it will be tracked as received from the remote server by y-websocket and the update send to the server will exclude it. Also use the sync services `opened` event inside WebsocketPolyfill rather than mixing async `syncService.open().then` and event handling. This allows us to emit `opened` and then `sync` from `syncService.open()`. Signed-off-by: Max <max@nextcloud.com>
max-nextcloud
added
bug
|
/backport to stable32 |
|
/backport to stable31 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7787 +/- ##
==========================================
- Coverage 60.19% 60.17% -0.02%
==========================================
Files 502 502
Lines 38561 38572 +11
Branches 1135 1134 -1
==========================================
+ Hits 23210 23212 +2
- Misses 15245 15254 +9
Partials 106 106 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Cypress failures are due to nextcloud/server#55726 |
| return [ | ||
| 'steps' => $stepsToReturn, | ||
| 'version' => $newVersion, | ||
| 'version' => isset($documentState) ? $document->getLastSavedVersion() : 0, |
There was a problem hiding this comment.
Can you tell if this has any other side effects? We now no longer return the latest version (number of steps in the db) but just the last saved. We used this in the past to compare and see if the document had unsaved changes, but I think that part was changed already.
There was a problem hiding this comment.
Otherwise $newVersion is no longer used as a variable
There was a problem hiding this comment.
Good catch wrt $newVersion. We were not using the version in the response on the client side anymore.
The unsaved changes check is on the server when cleaning up docs, right? That should still work. this is not changing the data structures there.
There was a problem hiding this comment.
Right there we still have it, i just remembered that we also used it in the frontend in the early days
max-nextcloud
force-pushed
the
fix/receive-doc-state-as-message
branch
2 times, most recently
from
6bc7587 to
8763eb9
Compare
* Emit the documents last saved version in the push response if document state is send as well. * Otherwise send the version as 0 so it cannot increase the sync service version. * Use the versions returned by create and push requests alongside the document state to construct a consistent step with the last saved version that matches the document state. This will result in the sync service version getting bumped to the documents last saved version once the coresponding document state has been processed. Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
max-nextcloud
force-pushed
the
fix/receive-doc-state-as-message
branch
from
8763eb9 to
205c7cb
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Process initial document state as incoming message.
This way it will be tracked as received from the remote server by y-websocket
and the update send to the server will exclude it.
Also use the sync services
openedevent inside WebsocketPolyfillrather than mixing async
syncService.open().thenand event handling.This allows us to emit
openedand thensyncfromsyncService.open().