| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
patch_apply never throws: it drops a hunk whose context is gone, and fuzzy matching can place one at the wrong offset while still reporting success, so neither shape is visible to the receiver. A diverged client then commits its whole paragraph and the server stores it unconditionally, making that text authoritative for everyone. Senders now attach checksums of their text before and after the patch. A receiver that started from the same text but did not reach the same result knows it diverged and asks for that single paragraph again via GET_PARAGRAPH, instead of refetching the whole note. Commits carry the checksum of the text the client believed the server held, and the server keeps its own copy when they disagree. Both fields are optional, so clients that omit them behave as before. PatchParagraphSend and PatchParagraphReceived were mapped to the opposite directions, which left the receiving side without noteId or the new fields; they are swapped to match what is actually sent and received.
| Back | FazBrowse Home | New Git URL |
What is this PR for?
In collaborative mode paragraph text is synchronised with diff-match-patch, and both the receiving client (paragraph-base.ts) and the server (NotebookService.java) use only element [0] of patch_apply, discarding the per-hunk success array. A patch that cannot be applied does not throw, and it fails in two different shapes:
The second shape is why inspecting the success array is not enough, and the ticket already ruled out both that approach and Match_Threshold = 0.
A diverged paragraph does not stay local. Focusing and blurring calls commitParagraph, which sends the client's whole paragraph, and the server stores it through p.setText(text) without comparing it to what it holds, then broadcasts it to every connection. Whichever client commits first makes its own text authoritative for everyone.
This PR keeps the existing patch mechanism and makes divergence detectable instead:
Both fields are optional: clients that do not send them behave exactly as before.
Two things I deliberately left out of scope, and one I had to include:
Finally, this does not make the sync conflict-free. diff-match-patch locates a hunk by matching surrounding context, so a concurrent edit that removes that context can still make a patch land in the wrong place. Editors like Google Docs avoid this class of bug with Operational Transformation or CRDTs, where each edit carries an explicit position the server transforms against concurrent operations. Replacing Zeppelin's sync engine would be far larger than this ticket, so this PR takes the narrower path the ticket points at: make divergence detectable, and stop one client's diverged text from becoming everyone's.
What type of PR is it?
Bug Fix
Todos
What is the Jira issue?
How should this be tested?
New test NotebookServiceTest#testUpdateParagraphChecksIsBasedOnCurrentServerText covers the commit check: a commit based on the current server text is stored, one based on text the server no longer holds is rejected and leaves the server copy untouched, and a commit without a checksum keeps the previous behaviour.
Result: Tests run: 7, Failures: 0, Errors: 0 — this run is on top of the merged ZEPPELIN-6556, so the personalized-mode tests and the new check pass together. If a local Zeppelin server is running it holds the Lucene index lock and every test errors out in setUp, so stop it first or point ZEPPELIN_SEARCH_INDEX_PATH at a scratch directory.
The checksum uses the same algorithm as String.hashCode() so both sides compute the same value; I verified Java and TypeScript agree on empty strings, newlines, non-ASCII text, emoji (surrogate pairs) and long input.
Manual: open the same note in two browser sessions and edit the same paragraph concurrently, with one session deleting a line while the other edits it. I confirmed the built bundle ships the new fields and that ordinary collaborative sync still works, but I could not reproduce the divergence by hand — on a local server the patch round trip is fast enough that the window never opens. This matches the ticket's note that existing e2e coverage does not exercise failed or misapplied patches, which is why the commit check is covered by unit tests instead. Rejecting stale commit of paragraph in the server log marks a refused commit.
Screenshots (if appropriate)
N/A
Questions: