diff --git a/docs/source/index.rst b/docs/source/index.rst index d9bcec60..b2db953c 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -18,6 +18,7 @@ Welcome to Apollon's documentation! user/getting-started user/api user/uml-model-helpers + user/realtime-collaboration .. toctree:: :caption: Contributor Guide diff --git a/docs/source/user/realtime-collaboration.rst b/docs/source/user/realtime-collaboration.rst new file mode 100644 index 00000000..1c219623 --- /dev/null +++ b/docs/source/user/realtime-collaboration.rst @@ -0,0 +1,66 @@ +.. _realtime-collaboration: + +################ +Realtime Collaboration +################ + +Apollon supports realtime collaboration by emitting patches when a model is changed, and importing +patches potentially emitted by other Apollon clients. Patches follow the `RFC 6902`_ standard (i.e. `JSON Patch`_), +so they can be applied to Apollon diagrams in any desired language. + +.. code-block:: typescript + + // This method subscribes to model change patches. + // The callback is called whenever a patch is emitted. + editor.subscribeToModelChangePatches(callback: (patch: Patch) => void): number; + + // This method unsubscribes from model change patches. + // The subscriptionId is the return value of the subscribeToModelChangePatches method. + editor.unsubscribeFromModelChangePatches(subscriptionId: number): void; + + // This method imports a patch. This can be used to + // apply patches emitted by other Apollon clients. + editor.importPatch(patch: Patch): void; + +Apollon client takes care of detecting conflicts between clients and resolving them. There is no need for +users to manually implement any reconcilliation mechanism. The only requirements to ensure a convergent state +between all Apollon clients are as follows: + +- Apply all patches on all clients in the same order, +- Apply all patches on all clients, including patches emitted by the same client. + +This means, if client A emits patch P1 and client B emits patch P2, both clients must then apply P1 and P2 in the same order (using `importPatch()`). The order can be picked by the server, but it needs to be the same for all clients. This means client A should also receive P1, although it has emitted P1 itself. Similarly client B should receive P2, although it has emitted P2 itself. + +Apollon clients sign the patches they emit and treat receiving their own patches as confirmation that the patch has been applied and ordered with patches from other clients. They also optimize based on this assumption, to recognize when they are ahead of the rest of the clients on some part of the state: when client A applies the effects of patch P1 locally, its state is ahead until other clients have also applied patch P1, so client A can safely ignore other effects on that same part of the state (as it will get overwritten by patch P1 anyway). + +================ +Displaying Remote Users +================ + +In realtime collaboration, it can be useful to display activities of other users active in the collaboration session within the diagram editor. Apollon provides methods to display other users' selections: + +.. code-block:: typescript + + // This method selects or deselects elements + // on part of a given remote user with given name and color. + // Provide the ids of the elements the remote user + // has selected/deselected. + editor.remoteSelect( + selectorName: string, + selectorColor: string, + select: string[], + deselect?: string[] + ): void; + + // This method clears the list of remote users displayed + // on the diagram editor, except allowed users. + // Use this in case some users disconnect from the collaboration session. + pruneRemoteSelectors( + allowedSelectors: { + name: string; + color: string; + }[] + ): void; + +.. _RFC 6902: https://tools.ietf.org/html/rfc6902 +.. _JSON Patch: http://jsonpatch.com/ \ No newline at end of file diff --git a/package-lock.json b/package-lock.json index 066798be..79820f22 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@ls1intum/apollon", - "version": "3.3.7", + "version": "3.3.9", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@ls1intum/apollon", - "version": "3.3.7", + "version": "3.3.9", "hasInstallScript": true, "license": "MIT", "dependencies": { diff --git a/package.json b/package.json index 16d88ee7..703d8bdc 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@ls1intum/apollon", - "version": "3.3.7", + "version": "3.3.9", "description": "A UML diagram editor.", "keywords": [], "homepage": "https://github.com/ls1intum/apollon#readme", diff --git a/src/main/components/store/model-store.tsx b/src/main/components/store/model-store.tsx index 08145397..f56de96f 100644 --- a/src/main/components/store/model-store.tsx +++ b/src/main/components/store/model-store.tsx @@ -50,6 +50,7 @@ export const createReduxStore = ( patcher && createPatcherReducer(patcher, { transform: (model) => ModelState.fromModel(model) as ModelState, + transformInverse: (state) => ModelState.toModel(state), merge, }); diff --git a/src/main/services/patcher/patch-verifier.ts b/src/main/services/patcher/patch-verifier.ts new file mode 100644 index 00000000..979de6da --- /dev/null +++ b/src/main/services/patcher/patch-verifier.ts @@ -0,0 +1,114 @@ +import { Patch } from './patcher-types'; +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): SignedPatch { + 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)); + } +} diff --git a/src/main/services/patcher/patcher-reducer.ts b/src/main/services/patcher/patcher-reducer.ts index cbede062..9753d924 100644 --- a/src/main/services/patcher/patcher-reducer.ts +++ b/src/main/services/patcher/patcher-reducer.ts @@ -12,6 +12,14 @@ export type PatcherReducerOptions = { */ transform?: (state: T) => U; + /** + * Transforms the state before applying the patch. This is useful + * when the internal store schema differs from the schema exposed to the outside. + * @param state The state in the internal schema. + * @returns The state in the external schema. + */ + transformInverse?: (state: U) => T; + /** * Merges the old state with the new state. This is useful when naive strategies * like `Object.assign` would trigger unwanted side-effects and more context-aware merging @@ -25,6 +33,7 @@ export type PatcherReducerOptions = { const _DefaultOptions = { transform: (state: any) => state, + transformInverse: (state: any) => state, merge: (oldState: any, newState: any) => ({ ...oldState, ...newState }), }; @@ -39,14 +48,17 @@ export function createPatcherReducer( options: PatcherReducerOptions = _DefaultOptions, ): Reducer { const transform = options.transform || _DefaultOptions.transform; + const transformInverse = options.transformInverse || _DefaultOptions.transformInverse; const merge = options.merge || _DefaultOptions.merge; - return (state = {} as U, action) => { + return (state, action) => { const { type, payload } = action; if (type === PatcherActionTypes.PATCH) { - const res = transform(patcher.patch(payload)); + const res = patcher.patch(payload, transformInverse(state as U)); - return merge(state, res); + if (res.patched) { + return merge((state ?? {}) as U, transform(res.result)); + } } return state; diff --git a/src/main/services/patcher/patcher-saga.ts b/src/main/services/patcher/patcher-saga.ts new file mode 100644 index 00000000..b147f805 --- /dev/null +++ b/src/main/services/patcher/patcher-saga.ts @@ -0,0 +1,47 @@ +import { call, debounce, delay, put, select, take } from 'redux-saga/effects'; +import { SagaIterator } from 'redux-saga'; + +import { run } from '../../utils/actions/sagas'; +import { PatcherActionTypes } from './patcher-types'; +import { ModelState } from '../../components/store/model-state'; +import { UMLContainerRepository } from '../uml-container/uml-container-repository'; +import { UMLElement } from '../uml-element/uml-element'; +import { UMLRelationship } from '../uml-relationship/uml-relationship'; +import { recalc } from '../uml-relationship/uml-relationship-saga'; +import { render } from '../layouter/layouter'; + +/** + * Fixes the layout of the diagram after importing a patch. + */ +export function* PatchLayouter(): SagaIterator { + yield run([patchLayout]); +} + +export function* patchLayout(): SagaIterator { + yield debounce(100, PatcherActionTypes.PATCH, recalculateLayouts); +} + +function* recalculateLayouts(): SagaIterator { + const { elements }: ModelState = yield select(); + + const ids = Object.values(elements) + .filter((x) => !x.owner) + .map((x) => x.id); + + if (!ids.length) { + return; + } + + yield put(UMLContainerRepository.append(ids)); + + for (const id of Object.keys(elements)) { + yield delay(0); + if (UMLElement.isUMLElement(elements[id])) { + yield call(render, id); + } + + if (UMLRelationship.isUMLRelationship(elements[id]) && !elements[id].isManuallyLayouted) { + yield call(recalc, id); + } + } +} diff --git a/src/main/services/patcher/patcher.ts b/src/main/services/patcher/patcher.ts index 0da3da65..2b554e59 100644 --- a/src/main/services/patcher/patcher.ts +++ b/src/main/services/patcher/patcher.ts @@ -3,6 +3,7 @@ import { buffer, debounceTime, filter, map, merge, Observable, Subject, Subscrip import { compare } from './compare'; import { Patch, PatchListener } from './patcher-types'; +import { PatchVerifier, SignedPatch } from './patch-verifier'; /** * Compares two objects and returns the difference @@ -26,7 +27,7 @@ export interface PatcherOptions { const _DefaultOptions = { diff: compare, - maxFrequency: 25, + maxFrequency: 60, }; /** @@ -40,6 +41,7 @@ export class Patcher { private continuousRouter = new Subject(); private continuousPatchObservable: Observable; private observable: Observable; + private verifier = new PatchVerifier(); readonly options: PatcherOptions; /** @@ -52,6 +54,13 @@ export class Patcher { maxFrequency: options.maxFrequency || _DefaultOptions.maxFrequency, }; + // TODO: Double check the correctness of this code. + // there are guard rails for handling multiple patches per tick + // or filtering out empty patches, but they are only + // applied to the total observable. If a consumer subscribes + // to discrete patches, for example, they won't get these + // guard rails. This is a potential bug. + // // throttle continuous patches to handle back-pressure. note that // unlike discrete changes, it is ok to miss some continuous changes. @@ -108,22 +117,27 @@ export class Patcher { /** * Applies a patch to the object. Will NOT notify subscribers. * @param patch The patch to apply. - * @returns The new state of the object. + * @returns The whether the state should change, and the new state of the object. */ - patch(patch: Patch): T { + patch(patch: Patch | SignedPatch, state?: T): { patched: boolean; result: T } { this.validate(); - if (patch && patch.length > 0) { - this._snapshot = patch.reduce((state, p, index) => { + const verified = this.verifier.verified(patch); + this._snapshot = state ?? this._snapshot; + + if (verified && verified.length > 0) { + this._snapshot = verified.reduce((state, p, index) => { try { return applyReducer(state, p, index); } catch { return state; } }, this.snapshot); + + return { patched: true, result: this.snapshot }; } - return this.snapshot; + return { patched: false, result: this.snapshot }; } /** @@ -185,7 +199,7 @@ export class Patcher { if (patch && patch.length) { const router = discreteChange ? this.discreteRouter : this.continuousRouter; - router.next(patch); + router.next(this.verifier.sign(patch)); } } diff --git a/src/main/services/saga.ts b/src/main/services/saga.ts index 4037e4eb..5bbbb63e 100644 --- a/src/main/services/saga.ts +++ b/src/main/services/saga.ts @@ -6,11 +6,12 @@ import { UMLContainerSaga } from './uml-container/uml-container-saga'; import { UMLDiagramSaga } from './uml-diagram/uml-diagram-saga'; import { UMLElementSaga } from './uml-element/uml-element-saga'; import { UMLRelationshipSaga } from './uml-relationship/uml-relationship-saga'; +import { PatchLayouter } from './patcher/patcher-saga'; export type SagaContext = { layer: ILayer | null; }; export function* saga(): SagaIterator { - yield composeSaga([Layouter, UMLElementSaga, UMLContainerSaga, UMLRelationshipSaga, UMLDiagramSaga]); + yield composeSaga([Layouter, UMLElementSaga, UMLContainerSaga, UMLRelationshipSaga, UMLDiagramSaga, PatchLayouter]); } diff --git a/src/tests/unit/services/patcher/patcher-reducer-test.ts b/src/tests/unit/services/patcher/patcher-reducer-test.ts index f941f0e6..1d85ed80 100644 --- a/src/tests/unit/services/patcher/patcher-reducer-test.ts +++ b/src/tests/unit/services/patcher/patcher-reducer-test.ts @@ -16,16 +16,27 @@ describe('test patcher reducer.', () => { test('calls given transform function for applying the patch.', () => { const cb = jest.fn((x) => ({ y: x.x })); + const inv = (x: any) => ({ x: x.y }); const patcher = new Patcher(); patcher.initialize({}); - const reducer = createPatcherReducer(patcher, { transform: cb }); + const reducer = createPatcherReducer(patcher, { transform: cb, transformInverse: inv }); const nextState = reducer({ y: 41 }, PatcherRepository.patch([{ op: 'add', path: '/x', value: 42 }])); expect(nextState).toEqual({ y: 42 }); }); + test('passes the state to the patcher.', () => { + const patcher = new Patcher(); + patcher.initialize({}); + + const reducer = createPatcherReducer(patcher); + const nextState = reducer({ y: 41 }, PatcherRepository.patch([{ op: 'add', path: '/x', value: 42 }])); + + expect(nextState).toEqual({ x: 42, y: 41 }); + }); + test('calls given merge function for applying the patch.', () => { const cb = jest.fn((x, y) => ({ ...x, ...y })); diff --git a/src/tests/unit/services/patcher/patcher-saga-test.ts b/src/tests/unit/services/patcher/patcher-saga-test.ts new file mode 100644 index 00000000..2200d70f --- /dev/null +++ b/src/tests/unit/services/patcher/patcher-saga-test.ts @@ -0,0 +1,70 @@ +import { call, debounce, delay, select, take } from 'redux-saga/effects'; + +import { patchLayout } from '../../../../main/services/patcher/patcher-saga'; +import { PatcherActionTypes, PatcherRepository } from '../../../../main/services/patcher'; +import { UMLElementState } from '../../../../main/services/uml-element/uml-element-types'; +import { IUMLRelationship } from '../../../../main/services/uml-relationship/uml-relationship'; +import { render } from '../../../../main/services/layouter/layouter'; +import { recalc } from '../../../../main/services/uml-relationship/uml-relationship-saga'; + +describe('test patcher saga.', () => { + test('it invokes re-renders and re-calcs after a patch.', () => { + const run = patchLayout(); + const debounced = run.next().value; + expect(debounced).toEqual(debounce(100, PatcherActionTypes.PATCH, expect.any(Function))); + + const fork = debounced['payload']['args'][2](); + expect(fork.next(PatcherRepository.patch([{ op: 'add', path: '/x', value: 42 }])).value).toEqual(select()); + + const elements: UMLElementState = { + x: { + type: 'Package', + id: 'x', + name: 'package', + owner: null, + bounds: { x: 0, y: 0, width: 100, height: 100 }, + }, + y: { + type: 'Class', + id: 'y', + name: 'class', + owner: 'x', + bounds: { x: 0, y: 0, width: 100, height: 100 }, + }, + z: { + type: 'Class', + id: 'z', + name: 'class', + owner: null, + bounds: { x: 0, y: 0, width: 100, height: 100 }, + }, + w: { + type: 'ClassInheritance', + id: 'w', + name: '...', + owner: null, + source: { element: 'y', direction: 'Up' }, + target: { element: 'z', direction: 'Down' }, + path: [ + { x: 0, y: 0 }, + { x: 200, y: 100 }, + ], + bounds: { x: 0, y: 0, width: 200, height: 100 }, + } as IUMLRelationship, + }; + + fork.next({ elements }); + + expect(fork.next().value).toEqual(delay(0)); + expect(fork.next().value).toEqual(call(render, 'x')); + + expect(fork.next().value).toEqual(delay(0)); + expect(fork.next().value).toEqual(call(render, 'y')); + + expect(fork.next().value).toEqual(delay(0)); + expect(fork.next().value).toEqual(call(render, 'z')); + + expect(fork.next().value).toEqual(delay(0)); + expect(fork.next().value).toEqual(call(recalc, 'w')); + }); +}); diff --git a/src/tests/unit/services/patcher/patcher-test.ts b/src/tests/unit/services/patcher/patcher-test.ts index 229e9062..095209ff 100644 --- a/src/tests/unit/services/patcher/patcher-test.ts +++ b/src/tests/unit/services/patcher/patcher-test.ts @@ -48,7 +48,7 @@ describe('patcher class.', () => { await sleep(1); expect(cb).toHaveBeenCalledWith([ - { op: 'replace', path: '/x', value: 43 }, + { op: 'replace', path: '/x', value: 43, hash: expect.any(String) }, { op: 'add', path: '/y', value: 'hello' }, ]); }); @@ -114,4 +114,67 @@ describe('patcher class.', () => { expect(cb3).toHaveBeenCalledTimes(2); expect(patcher.snapshot).toEqual({ x: 45 }); }); + + test('does not reapply self-induced patches.', async () => { + let captured: any; + const patcher = new Patcher(); + patcher.initialize({ x: 42 }); + patcher.subscribe((patch) => (captured = patch)); + patcher.check({ x: 43 }); + + await sleep(1); + expect(captured).toBeDefined(); + const res = patcher.patch(captured); + expect(res.patched).toBe(false); + }); + + test('suppresses patches on a changed address until confirmation patch is received.', async () => { + let captured: any; + const patcher = new Patcher(); + patcher.initialize({ x: 42 }); + patcher.subscribe((patch) => (captured = patch)); + patcher.check({ x: 43 }); + + await sleep(1); + expect(captured).toBeDefined(); + + const res1 = patcher.patch([{ op: 'replace', path: '/x', value: 44, hash: '123' }]); + expect(res1.patched).toBe(false); + + const res2 = patcher.patch([{ op: 'replace', path: '/x', value: 45, hash: '123' }]); + expect(res2.patched).toBe(false); + + patcher.patch(captured); + + const res3 = patcher.patch([{ op: 'replace', path: '/x', value: 46, hash: '123' }]); + expect(res3.patched).toBe(true); + expect(res3.result).toEqual({ x: 46 }); + }); + + test('always applies unsigned patches.', async () => { + let captured: any; + const patcher = new Patcher(); + patcher.initialize({ x: 42 }); + patcher.subscribe((patch) => (captured = patch)); + patcher.check({ x: 43 }); + + await sleep(1); + expect(captured).toBeDefined(); + + const res1 = patcher.patch([{ op: 'replace', path: '/x', value: 44, hash: '123' }]); + expect(res1.patched).toBe(false); + + const res2 = patcher.patch([{ op: 'add', path: '/y', value: 45 }]); + expect(res2.patched).toBe(true); + + const res3 = patcher.patch([{ op: 'replace', path: '/x', value: 46, hash: '123' }]); + expect(res3.patched).toBe(false); + }); + + test('can update snapshot on patch.', () => { + const patcher = new Patcher(); + patcher.initialize({ x: 42, y: 43 }); + const res = patcher.patch([{ op: 'replace', path: '/x', value: 44 }], { y: 45 }); + expect(res.result).toEqual({ x: 44, y: 45 }); + }); });