From b8a66ec09a8fb1cf0dfe2eb1d800c0e1341a82ee Mon Sep 17 00:00:00 2001 From: Eugene Ghanizadeh Date: Mon, 15 Jan 2024 10:57:57 +0100 Subject: [PATCH] suppress self-stuttering (#70) * suppress self-stuttering * add jsdoc --- .../apollon-editor-component.tsx | 10 +- .../webapp/src/main/utils/patch-verifier.ts | 119 ++++++++++++++++++ 2 files changed, 127 insertions(+), 2 deletions(-) create mode 100644 packages/webapp/src/main/utils/patch-verifier.ts diff --git a/packages/webapp/src/main/components/apollon-editor-component/apollon-editor-component.tsx b/packages/webapp/src/main/components/apollon-editor-component/apollon-editor-component.tsx index 5ad24ab9..31f7adac 100644 --- a/packages/webapp/src/main/components/apollon-editor-component/apollon-editor-component.tsx +++ b/packages/webapp/src/main/components/apollon-editor-component/apollon-editor-component.tsx @@ -23,6 +23,7 @@ import { withApollonEditor } from './with-apollon-editor'; import { toast } from 'react-toastify'; import { selectionDiff } from '../../utils/selection-diff'; import { CollaborationMessage } from '../../utils/collaboration-message-type'; +import { PatchVerifier } from '../../utils/patch-verifier'; const ApollonContainer = styled.div` display: flex; @@ -89,11 +90,13 @@ const enhance = compose>( ), ); + class ApollonEditorComponent extends Component { private readonly containerRef: (element: HTMLDivElement) => void; private ref?: HTMLDivElement; private client: any; private selection: Selection = { elements: {}, relationships: {} }; + private patchVerifier = new PatchVerifier(); componentDidUpdate(prevProps: Props) { if (this.client) { @@ -122,7 +125,7 @@ class ApollonEditorComponent extends Component { JSON.stringify({ token, collaborator: { name: collaborationName, color: collaborationColor }, - patch, + patch: this.patchVerifier.sign(patch), }) ); } @@ -251,7 +254,10 @@ class ApollonEditorComponent extends Component { this.props.importDiagram(JSON.stringify(diagram)); } if (patch) { - this.props.editor?.importPatch(patch); + const verified = this.patchVerifier.verified(patch); + if (verified.length > 0) { + this.props.editor?.importPatch(verified); + } } if (selection && originator) { this.props.editor?.remoteSelect(originator.name, originator.color, selection.selected, selection.deselected); diff --git a/packages/webapp/src/main/utils/patch-verifier.ts b/packages/webapp/src/main/utils/patch-verifier.ts new file mode 100644 index 00000000..6405ebba --- /dev/null +++ b/packages/webapp/src/main/utils/patch-verifier.ts @@ -0,0 +1,119 @@ +import { Patch } from '@ls1intum/apollon' +import { Operation, ReplaceOperation } from 'fast-json-patch' + + +/** + * A signed replace operation is a replace operation with an additional hash property. + * This enables tracing the origin of a replace operation. + */ +export type SignedReplaceOperation = ReplaceOperation & { hash: string }; + +/** + * A signed operation is either a replace operation with a hash property or any other operation. + * The hash property is used to verify the origin of a replace operation. + * Only replace operations need a hash property. + */ +export type SignedOperation = Exclude> | SignedReplaceOperation; + +/** + * A signed patch is a patch where all replace operations are signed, i.e. + * all the replace operations have a hash allowing for tracing their origin. + */ +export type SignedPatch = SignedOperation[]; + +/** + * @param operation + * @returns true if the operation is a replace operation, false otherwise + */ +export function isReplaceOperation(operation: Operation): operation is ReplaceOperation { + return operation.op === 'replace'; +} + +/** + * @param operation + * @returns true if the operation is a signed operation, false otherwise. A signed operation is either + * a replace operation with a hash property or any other operation. + */ +export function isSignedOperation(operation: Operation): operation is SignedOperation { + return !isReplaceOperation(operation) || 'hash' in operation; +} + +/** + * A patch verifier enables otpimisitc discard of incomging changes.It can be used to sign + * each operation (or opeerations of each patch) and track them. If the server broadcasts changes + * of the same scope (e.g. the same path) before re-broadcasting that particular change, the client + * can safely discard the change as it will (optimistically) be overridden when the server re-broadcasts + * the tracked change. + * + * This greatly helps with stuttering issues due to clients constantly re-applying changes they have + * already applied locally but in a different order. See + * [**this issue**](https://github.com/ls1intum/Apollon_standalone/pull/70) for more details. + */ +export class PatchVerifier { + private waitlist: { [address: string]: string } = {}; + + /** + * Signs an operation and tracks it. Only replace operations are signed and tracked. + * @param operation + * @returns The signed version of the operation (to be sent to the server) + */ + public signOperation(operation: Operation): SignedOperation { + if (isReplaceOperation(operation)) { + const hash = Math.random().toString(36).substring(2, 15); + const path = operation.path; + this.waitlist[path] = hash; + + return { ...operation, hash }; + } else { + return operation; + } + } + + /** + * Signs all operations inside the patch. + * @param patch + * @returns the signed patch (to be sent to the server) + */ + public sign(patch: Patch) { + return patch.map(op => this.signOperation(op)); + } + + /** + * Checks whether the operation should be applied or should it be optimisitcally discarded. + * - If the operation is not a replace operation, it is always applied. + * - If the operation is a replace operation but it is not signed, it is always applied. + * - If the operation is a signed replace operation and no other operation with the same path is tracked, + * it will be applied. + * - Otherwise it will be discarded. + * + * If it receives an operation that is already tracked, it will be discarded, and the + * operation will be untracked (so following operations on the same path will be applied). + * + * @param operation + * @returns true if the operation should be applied, false if it should be discarded. + */ + public isVerifiedOperation(operation: Operation): boolean { + if ( + isReplaceOperation(operation) + && isSignedOperation(operation) + && operation.path in this.waitlist + ) { + if (this.waitlist[operation.path] === operation.hash) { + delete this.waitlist[operation.path]; + } + + return false; + } else { + return true; + } + } + + /** + * Filters an incoming patch, only leaving the operations that should be applied. + * @param patch + * @returns a patch with operations that should be applied. + */ + public verified(patch: Patch): Patch { + return patch.filter(op => this.isVerifiedOperation(op)); + } +}