Minimal yjs based collaboration by max-nextcloud · Pull Request #2971 · nextcloud/text

max-nextcloud · 2022-09-20T11:35:22Z

Target version: master
Fixes Y.js backend drop in replacement #2847

cypress · 2022-12-07T07:11:22Z

Passing run #8385 ↗︎

0	132	0	0	0

Details:

Minimal yjs based collaboration
Project: Text	Commit: `b49ade4770`
Status: Passed	Duration: 06:41 💡
Started: Jan 31, 2023 9:43 AM	Ended: Jan 31, 2023 9:50 AM

_{This comment has been generated by cypress-bot as a result of this
project's GitHub integration settings.}

juliusknorr · 2022-12-09T13:44:27Z

 			// This way the user can still resolve conflicts in the editor view
-			if ($document->getLastSavedVersion() !== $document->getCurrentVersion()) {
+			$stepsVersion = $this->stepMapper->getLatestVersion($document->getId());
+			if ($stepsVersion && ($document->getLastSavedVersion() !== $stepsVersion)) {


If we drop the step version that check against the last saved version will no longer work, so maybe we need to find a different approach to determine the collision case.

juliusknorr

First half of the review for backend bits

juliusknorr · 2023-01-19T09:55:13Z

-			$oldVersion = $document->getCurrentVersion();
-			$newVersion = $oldVersion + count($steps);
+			$stepsVersion = $this->stepMapper->getLatestVersion($document->getId());
+			$newVersion = $stepsVersion + count($steps);


If we get rid of the lock we may want to wrap this in a transaction so that reading the current version, increasing and writing steps is an atomic operation.

Should be easy with https://docs.nextcloud.com/server/latest/developer_manual/basics/storage/database.html#transactions

I created a follow up issue in #3726

max-nextcloud · 2023-01-25T21:48:32Z

Locally only sync, attachments and the flaky Links tests are failing (with npx cypress run).

juliusknorr · 2023-01-26T23:11:50Z

Pushed a bit of polishing for the backend code to make CI happy and a fix for direct editing to pass the initial session data to the web socket polyfill. Let's see what CI says.

juliusknorr · 2023-01-27T17:18:20Z

@max-nextcloud So far for my week I pushed couple of fixes and polishing. I ended up also refactoring the PollingBackend a bit to clean this up. Cypress tests pass now though there are two draft commits that we should discuss on Monday about on how to best handle that.

f232328 It seemed that there was still an issue where longer running sessions ended up playing ping pong with messages which lead to heavy load. I figured out that we may not need to send back steps during the push operations as this was originally more a shortcut to avoid another polling round. I feel this is more stable and also makes more sense to just have one channel where steps get sent from the server to the clients. This was they can also continue to fetch them in order of the database
00879a6 As we no longer store the version of the document the left TODO statement // TODO: figure out when to autosave caused that the files were not saved. In order to have a working branch I pushed this draft commit to always send the documentState and autosaveContent with each sync request, but we should probably be smarter about that.

max-nextcloud · 2023-01-27T21:12:41Z

@juliushaertl Wow... thanks for looking into this and pushing the pr forward like that!
❤️

max-nextcloud · 2023-01-29T09:47:55Z

@juliushaertl Given your last commit i wonder what happens if...

two users edit the doc
it's update on the server through a different mechanis -> conflict
one user resolves the conflict

Ideally the conflict would get resolved for the other user as well, right? In particular so they do not pick different strategies.
Is that the case already, or what happens?
Did we account for that in the past?

juliusknorr · 2023-01-30T08:21:29Z

Ideally the conflict would get resolved for the other user as well, right? In particular so they do not pick different strategies.
Is that the case already, or what happens?
Did we account for that in the past?

We didn't have any handling for different strategies in the past. My additional commit would already solve this by just moving any resulution to happen trough y.js, so individual users can also undo. One thing that might need further work is to timely propagate the resolved conflict, but I'd not consider that a blocker for the merge as it has been worse before ;)

Signed-off-by: Julius Härtl <jus@bitgrid.net>

Signed-off-by: Julius Härtl <jus@bitgrid.net> Signed-off-by: Max <max@nextcloud.com>

Signed-off-by: Julius Härtl <jus@bitgrid.net>

…flict through force save Signed-off-by: Julius Härtl <jus@bitgrid.net> Signed-off-by: Max <max@nextcloud.com>

Signed-off-by: Julius Härtl <jus@bitgrid.net>

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

Replaying old awareness state will trigger cursor moves which in turn trigger new awareness messages. Signed-off-by: Max <max@nextcloud.com>

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

Sometimes loading the doc will trigger one sync, sometimes two. Make sure we always inspect the actual save request by waiting for two syncs before we trigger and inspect it. Signed-off-by: Max <max@nextcloud.com>

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

szaimen · 2023-01-31T11:40:08Z

🎉🎉🎉🎉🎉

max-nextcloud force-pushed the experiment/minimal-yjs branch from 5df761c to 11f5abd Compare September 20, 2022 11:38

vinicius73 reviewed Sep 21, 2022

View reviewed changes

Comment thread src/components/Editor.vue Outdated

juliusknorr added this to the Nextcloud 26 milestone Sep 22, 2022

juliusknorr added enhancement New feature or request 2. developing labels Oct 11, 2022

max-nextcloud force-pushed the experiment/minimal-yjs branch from 9cd3d29 to 0b8e260 Compare October 17, 2022 13:12

max-nextcloud force-pushed the experiment/minimal-yjs branch from 0b8e260 to 3057169 Compare December 7, 2022 06:58

juliusknorr reviewed Dec 9, 2022

View reviewed changes

max-nextcloud force-pushed the experiment/minimal-yjs branch 2 times, most recently from 5ab0d4c to 9cae5ea Compare December 12, 2022 11:51

juliusknorr reviewed Dec 21, 2022

View reviewed changes

Comment thread lib/Service/DocumentService.php Outdated

max-nextcloud force-pushed the experiment/minimal-yjs branch from 2dc7029 to 1c3564a Compare January 10, 2023 06:51

max-nextcloud force-pushed the experiment/minimal-yjs branch from 1c3564a to 2885e2a Compare January 18, 2023 10:56

juliusknorr reviewed Jan 19, 2023

View reviewed changes

max-nextcloud force-pushed the experiment/minimal-yjs branch 4 times, most recently from e9f4dd1 to 1e7224f Compare January 25, 2023 17:11

juliusknorr force-pushed the experiment/minimal-yjs branch from 1e7224f to 4326eb3 Compare January 26, 2023 23:10

This comment was marked as resolved.

Sign in to view

juliusknorr force-pushed the experiment/minimal-yjs branch 2 times, most recently from 98bffa2 to 0ae0ea0 Compare January 27, 2023 16:10

juliusknorr and others added 15 commits January 31, 2023 09:28

fix: Also pass initial session to websocket polyfill

c2a2e5a

Signed-off-by: Julius Härtl <jus@bitgrid.net>

style(eslint): Fix eslint errors

003ac50

Signed-off-by: Julius Härtl <jus@bitgrid.net>

tests: Fix cypress tests

de6d718

Signed-off-by: Julius Härtl <jus@bitgrid.net>

fix: Use real api name for inserting files

483fa32

Signed-off-by: Julius Härtl <jus@bitgrid.net>

perf(sync): Increase step count to speed up initial document loading

2cfe3d2

Signed-off-by: Julius Härtl <jus@bitgrid.net>

draft(sync): Try to avoid sending back all steps

b1bec3f

Signed-off-by: Julius Härtl <jus@bitgrid.net> Signed-off-by: Max <max@nextcloud.com>

refactor(PollingBackend): Try to simplify polling backend a bit

07779ca

Signed-off-by: Julius Härtl <jus@bitgrid.net>

fix(ConflictDialog): Reconnect the polling backend on resolving a con…

6bc730f

…flict through force save Signed-off-by: Julius Härtl <jus@bitgrid.net> Signed-off-by: Max <max@nextcloud.com>

fix(Cursors): Show user cursor position

e644116

Signed-off-by: Julius Härtl <jus@bitgrid.net>

draft: We still have no proper autosave behaviour especially when closng

f20fe1f

Signed-off-by: Julius Härtl <jus@bitgrid.net>

feat(conflict): Allow conflict resultion without restarting the session

7512ef2

Signed-off-by: Julius Härtl <jus@bitgrid.net>

fix: lint errors

c586ea6

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

fix: filter out awareness updates in query responses

ecb26c5

Replaying old awareness state will trigger cursor moves which in turn trigger new awareness messages. Signed-off-by: Max <max@nextcloud.com>

enh(PollingBackend): only autosave every 30 sec.

b31a18e

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

fix(cypress): use same preparation to get save requests

23c9e2b

Sometimes loading the doc will trigger one sync, sometimes two. Make sure we always inspect the actual save request by waiting for two syncs before we trigger and inspect it. Signed-off-by: Max <max@nextcloud.com>

max-nextcloud force-pushed the experiment/minimal-yjs branch from 3f4f352 to 23c9e2b Compare January 31, 2023 08:28

doc(DocumentService): comment on magic value "AAE"

b49ade4

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

juliusknorr approved these changes Jan 31, 2023

View reviewed changes

juliusknorr requested review from julien-nc and mejo- January 31, 2023 10:37

juliusknorr added 3. to review and removed 2. developing labels Jan 31, 2023

max-nextcloud mentioned this pull request Jan 31, 2023

Wrap storing steps in a transaction #3726

Open

max-nextcloud merged commit a337849 into main Jan 31, 2023

delete-merged-branch Bot deleted the experiment/minimal-yjs branch January 31, 2023 11:22

This was referenced Feb 1, 2023

Path ignore filter may not work as expected nextcloud/.github#124

Closed

Editing cursor #90

Closed

blizzz mentioned this pull request Feb 9, 2023

25.0.4 RC1 nextcloud/server#36560

Merged

mejo- mentioned this pull request Feb 15, 2023

Improve collaboration experience (in Text app) nextcloud/collectives#274

Closed

Conversation

max-nextcloud commented Sep 20, 2022 • edited by juliusknorr Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

cypress Bot commented Dec 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Passing run #8385 ↗︎

Uh oh!

juliusknorr Dec 9, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

juliusknorr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

juliusknorr Jan 19, 2023

Choose a reason for hiding this comment

Uh oh!

max-nextcloud Jan 31, 2023

Choose a reason for hiding this comment

Uh oh!

max-nextcloud commented Jan 25, 2023

Uh oh!

juliusknorr commented Jan 26, 2023

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

juliusknorr commented Jan 27, 2023

Uh oh!

max-nextcloud commented Jan 27, 2023

Uh oh!

max-nextcloud commented Jan 29, 2023

Uh oh!

juliusknorr commented Jan 30, 2023

Uh oh!

szaimen commented Jan 31, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

max-nextcloud commented Sep 20, 2022 •

edited by juliusknorr

Loading

cypress Bot commented Dec 7, 2022 •

edited

Loading