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.

Sign up for GitHub

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

Jump to bottom

[stable29] Send one full update from y-websocket #6696

Merged

mejo- merged 17 commits into stable29 from backport/6650/stable29

Nov 27, 2024

backportbot bot commented Nov 26, 2024 •

edited by mejo-

Loading

Backport of #6650

backportbot bot requested review from juliusknorr and max-nextcloud

November 26, 2024 14:47

backportbot bot added bug 3. to review feature: sync labels

backportbot bot added this to the Nextcloud 29.0.10 milestone

backportbot bot assigned max-nextcloud

mejo- force-pushed the backport/6650/stable29 branch from 32744d3 to 887eead Compare

November 26, 2024 15:00

mejo- marked this pull request as ready for review

November 26, 2024 15:00

mejo- approved these changes

View reviewed changes

juliusknorr requested changes

View reviewed changes

Member

juliusknorr left a comment

Let's wait with the merge for our testing round tomorrow just to double check.

mejo- force-pushed the backport/6650/stable29 branch from 3d1ad5a to d419faf Compare

November 26, 2024 15:25

mejo- self-requested a review

November 26, 2024 15:41

mejo- force-pushed the backport/6650/stable29 branch 5 times, most recently from 9f1da43 to 7028dc6 Compare

November 27, 2024 11:22

Collaborator

max-nextcloud commented Nov 27, 2024

I suspect it (also) does not like the typescript file (in src/services/Outbox.ts). We probably should turn this into a js file and remove all type annotations.

Member

mejo- commented Nov 27, 2024

I suspect it (also) does not like the typescript file (in src/services/Outbox.ts). We probably should turn this into a js file and remove all type annotations.

Yes, done already. But the cypress component tests also failed due to a version bump of @nextcloud/webpack-vue-config in a former PR. I pushed a commit now to fix this.

max-nextcloud added 10 commits

November 27, 2024 14:33


          fix(sync): inline y-websocket to track received updates there

d5f1afc

Signed-off-by: Max <max@nextcloud.com>


          fix(lint): y-websocket

25f37c3

Signed-off-by: Max <max@nextcloud.com>


          enh(y-websocket): always send full diff to server state

f7e9e5b

Keep an internal ydoc tracking updates that came from the server.
Send updates, that would sync this doc with the current doc state.

Signed-off-by: Max <max@nextcloud.com>


          fix(license): add license header and props to y-websocket

b2245ea

Signed-off-by: Max <max@nextcloud.com>


          fix(awareness): only send updates about the local client

2c52d80

Signed-off-by: Max <max@nextcloud.com>


          fix(sync): make use of steps in push responses

87038f0

The pushed steps are echoed back with all other steps since version immediately.
Processing them reduces the size of the following pushes and syncs.

Signed-off-by: Max <max@nextcloud.com>


          refactor(sync): rename _receiveSteps to receiveSteps

0b83dbb

It has always been publicly called from the PollingBackend.

Signed-off-by: Max <max@nextcloud.com>


          refactor(sync): drop superfluous update message

acfd535

Updates now include all the local structs
that were not yet received from remote.

No need to compute a separate update message anymore.

Signed-off-by: Max <max@nextcloud.com>


          fix(sync): Do not resend document state in first push.

20fbcac

Apply document state as a step.
Process it like other steps received from the remote.
In particular include it in the tracking of steps already applied
and set the version accordingly.

Signed-off-by: Max <max@nextcloud.com>


          fix(sync): reply to queries with steps since last save

27f4a2d

This was a very inefficient attempt to resync
that we did not even process on the client side.

Only the steps since the last save may not be enough
to get back in sync.
However we can expand this by including the document state
or storing it as the first step after a save.

Signed-off-by: Max <max@nextcloud.com>

max-nextcloud and others added 7 commits

November 27, 2024 14:33


          fix(sync): include document state in response to queries

93235e2

Signed-off-by: Max <max@nextcloud.com>


          fix(sync): process document state from push response

568031d

Do not process document state from create response.
During create the editor has not been initialized fully
and the cursor position is 0 - which is invalid
as it is not inside a node with inline content.
(It is inside the doc before the initial paragraph.)

This also allows processing document state later on
in order to recover from out of sync situations.
But we do not make use of that yet.

Signed-off-by: Max <max@nextcloud.com>


          refactor(sync): cache steps in sync service

6c51c92

Store the steps that need to be send
where we also do the debouncing.

They will be updated whenever there is a new message from y-websocket.

Signed-off-by: Max <max@nextcloud.com>


          refactor(Editor): instantiate api outside sync service

a39864d

Signed-off-by: Max <max@nextcloud.com>


          test(cypress): Pass SessionApi into the sync service

ca62b85

Signed-off-by: Jonas <jonas@freesources.org>


          fix(SyncService): only log errors for now

cc6fcbd

Signed-off-by: Max <max@nextcloud.com>


          chore: Fix linter errors and warnings

9874d6f

Signed-off-by: Jonas <jonas@freesources.org>

mejo- force-pushed the backport/6650/stable29 branch from 7028dc6 to 9874d6f Compare

November 27, 2024 13:35

Member

mejo- commented Nov 27, 2024

Failing conflicts.spec.js is unrelated. They're also flaky on stable30 at least and already failed in other stable29 PRs. It seems like they fail more reliable now though.

mejo- approved these changes

View reviewed changes

mejo- merged commit 03d88e0 into stable29

61 of 63 checks passed

mejo- deleted the backport/6650/stable29 branch

November 27, 2024 15:18

skjnldsv mentioned this pull request

29.0.10 RC1 nextcloud/server#49525

Merged

4 tasks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review bug feature: sync