From 5776b3029e6dfb6abb405a5273ca73d5b1ba2dbb Mon Sep 17 00:00:00 2001 From: Yourim Cha Date: Fri, 28 Jul 2023 14:55:43 +0900 Subject: [PATCH 1/8] Add presence to unwatched event and modify getPresence to retrieve only online client --- public/quill.html | 13 ++++-- src/client/client.ts | 12 ++++-- src/document/document.ts | 70 +++++++++++++++++++------------ test/integration/presence_test.ts | 30 ++++++------- 4 files changed, 76 insertions(+), 49 deletions(-) diff --git a/public/quill.html b/public/quill.html index ab780d7d0..f7503ba1b 100644 --- a/public/quill.html +++ b/public/quill.html @@ -110,8 +110,8 @@ }); doc.subscribe('others', (event) => { if (event.type === 'unwatched') { - const { clientID } = event.value; - cursors.removeCursor(doc.getPresence(clientID).username); + const { clientID, presence } = event.value; + cursors.removeCursor(presence.username); } else if (event.type === 'presence-changed') { const { clientID, presence } = event.value; const range = doc @@ -222,8 +222,13 @@ to = from + op.delete; console.log(`%c local: ${from}-${to}: ''`, 'color: green'); - doc.update((root) => { - root.content.edit(from, to, ''); + doc.update((root, presence) => { + const range = root.content.edit(from, to, ''); + if (range) { + presence.set({ + selection: root.content.indexRangeToPosRange(range), + }); + } }, `update content by ${client.getID()}`); } else if (op.retain !== undefined) { from = to + op.retain; diff --git a/src/client/client.ts b/src/client/client.ts index 940adbf58..66d65632b 100644 --- a/src/client/client.ts +++ b/src/client/client.ts @@ -24,6 +24,7 @@ import { CompleteFn, NextFn, } from '@yorkie-js-sdk/src/util/observable'; +import { deepcopy } from '@yorkie-js-sdk/src/util/object'; import { ActivateClientRequest, DeactivateClientRequest, @@ -903,11 +904,14 @@ export class Client implements Observable { } break; case PbDocEventType.DOC_EVENT_TYPE_DOCUMENTS_UNWATCHED: { + const presence = attachment.doc.getPresence(publisher); attachment.doc.removeOnlineClient(publisher); - attachment.doc.publish({ - type: DocEventType.Unwatched, - value: { clientID: publisher }, - }); + if (presence) { + attachment.doc.publish({ + type: DocEventType.Unwatched, + value: { clientID: publisher, presence: deepcopy(presence) }, + }); + } break; } } diff --git a/src/document/document.ts b/src/document/document.ts index 8439b46e4..2a89304e2 100644 --- a/src/document/document.ts +++ b/src/document/document.ts @@ -142,7 +142,7 @@ export type DocEvent

= | RemoteChangeEvent | InitializedEvent

| WatchedEvent

- | UnwatchedEvent + | UnwatchedEvent

| PresenceChangedEvent

; /** @@ -223,9 +223,9 @@ export interface WatchedEvent

extends BaseDocEvent { value: { clientID: ActorID; presence: P }; } -export interface UnwatchedEvent extends BaseDocEvent { +export interface UnwatchedEvent

extends BaseDocEvent { type: DocEventType.Unwatched; - value: { clientID: ActorID }; + value: { clientID: ActorID; presence: P }; } export interface PresenceChangedEvent

@@ -446,6 +446,7 @@ export class Document { this.clone!.root, message, ); + const actorID = this.changeID.getActorID()!; try { const proxy = createJSON>( @@ -453,16 +454,13 @@ export class Document { this.clone!.root.getObject(), ); - if (!this.presences.has(this.changeID.getActorID()!)) { - this.clone!.presences.set(this.changeID.getActorID()!, {} as P); + if (!this.presences.has(actorID)) { + this.clone!.presences.set(actorID, {} as P); } updater( proxy, - new Presence( - context, - this.clone!.presences.get(this.changeID.getActorID()!)!, - ), + new Presence(context, this.clone!.presences.get(actorID)!), ); } catch (err) { // drop clone because it is contaminated. @@ -487,7 +485,7 @@ export class Document { value: { message: change.getMessage() || '', operations: opInfos, - actor: change.getID().getActorID(), + actor: actorID, }, }); } @@ -496,8 +494,8 @@ export class Document { this.publish({ type: DocEventType.PresenceChanged, value: { - clientID: change.getID().getActorID()!, - presence: this.getPresence(change.getID().getActorID()!)!, + clientID: actorID, + presence: this.getPresence(actorID)!, }, }); } @@ -975,21 +973,32 @@ export class Document { let changeInfo: ChangeInfo | undefined; let docEvent: DocEvent

| undefined; const actorID = change.getID().getActorID()!; - if (change.hasPresenceChange()) { + if (change.hasPresenceChange() && this.onlineClients.has(actorID)) { const presenceChange = change.getPresenceChange()!; - if ( - presenceChange.type === PresenceChangeType.Put && - this.onlineClients.has(actorID) - ) { - docEvent = { - type: this.presences.has(actorID) - ? DocEventType.PresenceChanged - : DocEventType.Watched, - value: { - clientID: actorID, - presence: presenceChange.presence, - }, - }; + switch (presenceChange.type) { + case PresenceChangeType.Put: + docEvent = { + type: this.presences.has(actorID) + ? DocEventType.PresenceChanged + : DocEventType.Watched, + value: { + clientID: actorID, + presence: presenceChange.presence, + }, + }; + break; + case PresenceChangeType.Clear: + docEvent = { + type: DocEventType.Unwatched, + value: { + clientID: actorID, + presence: this.getPresence(actorID)!, + }, + }; + this.removeOnlineClient(actorID); + break; + default: + break; } } @@ -1088,6 +1097,15 @@ export class Document { * `getPresence` returns the presence of the given clientID. */ public getPresence(clientID: ActorID): P | undefined { + if (!this.onlineClients.has(clientID)) return; + return this.presences.get(clientID); + } + + /** + * `getPresenceForTest` returns the presence of the given clientID + * regardless of whether the client is online or not. + */ + public getPresenceForTest(clientID: ActorID): P | undefined { return this.presences.get(clientID); } diff --git a/test/integration/presence_test.ts b/test/integration/presence_test.ts index e5ac8c152..95fd2f485 100644 --- a/test/integration/presence_test.ts +++ b/test/integration/presence_test.ts @@ -30,13 +30,13 @@ describe('Presence', function () { for (let i = 0; i < snapshotThreshold; i++) { doc1.update((root, p) => p.set({ key: `${i}` })); } - assert.deepEqual(doc1.getPresence(c1.getID()!), { + assert.deepEqual(doc1.getPresenceForTest(c1.getID()!), { key: `${snapshotThreshold - 1}`, }); await c1.sync(); await c2.sync(); - assert.deepEqual(doc2.getPresence(c1.getID()!), { + assert.deepEqual(doc2.getPresenceForTest(c1.getID()!), { key: `${snapshotThreshold - 1}`, }); }); @@ -61,13 +61,13 @@ describe('Presence', function () { isRealtimeSync: false, }); - assert.deepEqual(doc1.getPresence(c1.getID()!), { key: 'key1' }); - assert.deepEqual(doc1.getPresence(c2.getID()!), undefined); - assert.deepEqual(doc2.getPresence(c2.getID()!), { key: 'key2' }); - assert.deepEqual(doc2.getPresence(c1.getID()!), { key: 'key1' }); + assert.deepEqual(doc1.getPresenceForTest(c1.getID()!), { key: 'key1' }); + assert.deepEqual(doc1.getPresenceForTest(c2.getID()!), undefined); + assert.deepEqual(doc2.getPresenceForTest(c2.getID()!), { key: 'key2' }); + assert.deepEqual(doc2.getPresenceForTest(c1.getID()!), { key: 'key1' }); await c1.sync(); - assert.deepEqual(doc1.getPresence(c2.getID()!), { key: 'key2' }); + assert.deepEqual(doc1.getPresenceForTest(c2.getID()!), { key: 'key2' }); await c2.detach(doc2); await c1.sync(); @@ -93,13 +93,13 @@ describe('Presence', function () { }); const emptyObject = {} as PresenceType; - assert.deepEqual(doc1.getPresence(c1.getID()!), emptyObject); - assert.deepEqual(doc1.getPresence(c2.getID()!), undefined); - assert.deepEqual(doc2.getPresence(c2.getID()!), emptyObject); - assert.deepEqual(doc2.getPresence(c1.getID()!), emptyObject); + assert.deepEqual(doc1.getPresenceForTest(c1.getID()!), emptyObject); + assert.deepEqual(doc1.getPresenceForTest(c2.getID()!), undefined); + assert.deepEqual(doc2.getPresenceForTest(c2.getID()!), emptyObject); + assert.deepEqual(doc2.getPresenceForTest(c1.getID()!), emptyObject); await c1.sync(); - assert.deepEqual(doc1.getPresence(c2.getID()!), emptyObject); + assert.deepEqual(doc1.getPresenceForTest(c2.getID()!), emptyObject); }); it('Should be synced eventually', async function () { @@ -209,14 +209,14 @@ describe('Presence', function () { }); doc1.update((root, p) => p.set({ cursor: { x: 1, y: 1 } })); - assert.deepEqual(doc1.getPresence(c1.getID()!), { + assert.deepEqual(doc1.getPresenceForTest(c1.getID()!), { key: 'key1', cursor: { x: 1, y: 1 }, }); await c1.sync(); await c2.sync(); - assert.deepEqual(doc2.getPresence(c1.getID()!), { + assert.deepEqual(doc2.getPresenceForTest(c1.getID()!), { key: 'key1', cursor: { x: 1, y: 1 }, }); @@ -335,7 +335,7 @@ describe(`Document.Subscribe('presence')`, function () { [ { type: DocEventType.Unwatched, - value: { clientID: c2ID }, + value: { clientID: c2ID, presence: { name: 'b' } }, }, ], ]); From 9e86a395edc3cf16928d9386d694510af4ab5837 Mon Sep 17 00:00:00 2001 From: Yourim Cha Date: Tue, 1 Aug 2023 04:57:48 +0900 Subject: [PATCH 2/8] Return presence to the user by making a copy --- src/client/client.ts | 3 +-- src/document/document.ts | 21 +++++++++++++++++---- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/client/client.ts b/src/client/client.ts index 66d65632b..16591c7e7 100644 --- a/src/client/client.ts +++ b/src/client/client.ts @@ -24,7 +24,6 @@ import { CompleteFn, NextFn, } from '@yorkie-js-sdk/src/util/observable'; -import { deepcopy } from '@yorkie-js-sdk/src/util/object'; import { ActivateClientRequest, DeactivateClientRequest, @@ -909,7 +908,7 @@ export class Client implements Observable { if (presence) { attachment.doc.publish({ type: DocEventType.Unwatched, - value: { clientID: publisher, presence: deepcopy(presence) }, + value: { clientID: publisher, presence }, }); } break; diff --git a/src/document/document.ts b/src/document/document.ts index 2a89304e2..58f439e0c 100644 --- a/src/document/document.ts +++ b/src/document/document.ts @@ -1056,6 +1056,8 @@ export class Document { /** * `setOnlineClients` sets the given online client set. + * + * @internal */ public setOnlineClients(onlineClients: Set) { this.onlineClients = onlineClients; @@ -1063,6 +1065,8 @@ export class Document { /** * `addOnlineClient` adds the given clientID into the online client set. + * + * @internal */ public addOnlineClient(clientID: ActorID) { this.onlineClients.add(clientID); @@ -1070,6 +1074,8 @@ export class Document { /** * `removeOnlineClient` removes the clientID from the online client set. + * + * @internal */ public removeOnlineClient(clientID: ActorID) { this.onlineClients.delete(clientID); @@ -1077,6 +1083,8 @@ export class Document { /** * `hasPresence` returns whether the given clientID has a presence or not. + * + * @internal */ public hasPresence(clientID: ActorID): boolean { return this.presences.has(clientID); @@ -1090,7 +1098,8 @@ export class Document { return {} as P; } - return this.presences.get(this.changeID.getActorID()!)!; + const p = this.presences.get(this.changeID.getActorID()!)!; + return deepcopy(p); } /** @@ -1098,15 +1107,19 @@ export class Document { */ public getPresence(clientID: ActorID): P | undefined { if (!this.onlineClients.has(clientID)) return; - return this.presences.get(clientID); + const p = this.presences.get(clientID); + return p ? deepcopy(p) : undefined; } /** * `getPresenceForTest` returns the presence of the given clientID * regardless of whether the client is online or not. + * + * @internal */ public getPresenceForTest(clientID: ActorID): P | undefined { - return this.presences.get(clientID); + const p = this.presences.get(clientID); + return p ? deepcopy(p) : undefined; } /** @@ -1118,7 +1131,7 @@ export class Document { if (this.presences.has(clientID)) { presences.push({ clientID, - presence: this.presences.get(clientID)!, + presence: deepcopy(this.presences.get(clientID)!), }); } } From a047391b93ad4f37d30197b7fc4d80aceaad093e Mon Sep 17 00:00:00 2001 From: Yourim Cha Date: Wed, 2 Aug 2023 18:12:07 +0900 Subject: [PATCH 3/8] Add test for pause and resume --- test/integration/presence_test.ts | 206 ++++++++++++++++++++++++++++++ 1 file changed, 206 insertions(+) diff --git a/test/integration/presence_test.ts b/test/integration/presence_test.ts index 95fd2f485..042956868 100644 --- a/test/integration/presence_test.ts +++ b/test/integration/presence_test.ts @@ -221,6 +221,62 @@ describe('Presence', function () { cursor: { x: 1, y: 1 }, }); }); + + it(`Should return only online clients`, async function () { + const c1 = new yorkie.Client(testRPCAddr); + const c2 = new yorkie.Client(testRPCAddr); + const c3 = new yorkie.Client(testRPCAddr); + await c1.activate(); + await c2.activate(); + await c3.activate(); + const c1ID = c1.getID()!; + const c2ID = c2.getID()!; + const c3ID = c3.getID()!; + + const docKey = toDocKey(`${this.test!.title}-${new Date().getTime()}`); + type PresenceType = { name: string; cursor: { x: number; y: number } }; + const doc1 = new yorkie.Document<{}, PresenceType>(docKey); + await c1.attach(doc1, { + initialPresence: { name: 'a1', cursor: { x: 0, y: 0 } }, + }); + const stub1 = sinon.stub(); + const unsub1 = doc1.subscribe('presence', stub1); + + // 01. c2 attaches doc in realtime sync, and c3 attached doc in manual sync. + const doc2 = new yorkie.Document<{}, PresenceType>(docKey); + await c2.attach(doc2, { + initialPresence: { name: 'b1', cursor: { x: 0, y: 0 } }, + }); + const doc3 = new yorkie.Document<{}, PresenceType>(docKey); + await c3.attach(doc3, { + initialPresence: { name: 'c1', cursor: { x: 0, y: 0 } }, + isRealtimeSync: false, + }); + await waitStubCallCount(stub1, 1); // c2 watched + + assert.deepEqual(doc1.getPresences(), [ + { clientID: c1ID, presence: doc1.getMyPresence() }, + { clientID: c2ID, presence: doc2.getMyPresence() }, + ]); + assert.deepEqual(doc1.getPresence(c3ID), undefined); + + // 02. c2 pauses the document (in manual sync), c3 resumes the document (in realtime sync). + await c2.pause(doc2); + await waitStubCallCount(stub1, 2); // c2 unwatched + await c3.resume(doc3); + await waitStubCallCount(stub1, 3); // c3 watched + + assert.deepEqual(doc1.getPresences(), [ + { clientID: c1ID, presence: doc1.getMyPresence() }, + { clientID: c3ID, presence: doc3.getMyPresence() }, + ]); + assert.deepEqual(doc1.getPresence(c2ID), undefined); + + unsub1(); + await c1.deactivate(); + await c2.deactivate(); + await c3.deactivate(); + }); }); describe(`Document.Subscribe('presence')`, function () { @@ -353,4 +409,154 @@ describe(`Document.Subscribe('presence')`, function () { unsub1(); }); + + it(`Can receive presence-related event only when using realtime sync`, async function () { + const c1 = new yorkie.Client(testRPCAddr); + const c2 = new yorkie.Client(testRPCAddr); + const c3 = new yorkie.Client(testRPCAddr); + await c1.activate(); + await c2.activate(); + await c3.activate(); + const c2ID = c2.getID()!; + const c3ID = c3.getID()!; + + const docKey = toDocKey(`${this.test!.title}-${new Date().getTime()}`); + type PresenceType = { name: string; cursor: { x: number; y: number } }; + const doc1 = new yorkie.Document<{}, PresenceType>(docKey); + await c1.attach(doc1, { + initialPresence: { name: 'a1', cursor: { x: 0, y: 0 } }, + }); + const stub1 = sinon.stub(); + const unsub1 = doc1.subscribe('presence', stub1); + + // 01. c2 attaches doc in realtime sync, and c3 attached doc in manual sync. + // c1 receives the watched event from c2. + const doc2 = new yorkie.Document<{}, PresenceType>(docKey); + await c2.attach(doc2, { + initialPresence: { name: 'b1', cursor: { x: 0, y: 0 } }, + }); + const doc3 = new yorkie.Document<{}, PresenceType>(docKey); + await c3.attach(doc3, { + initialPresence: { name: 'c1', cursor: { x: 0, y: 0 } }, + isRealtimeSync: false, + }); + await waitStubCallCount(stub1, 1); // c2 watched + + // 02. c2 and c3 update the presence. + // c1 receives the presence-changed event from c2. + doc2.update((_, presence) => { + presence.set({ name: 'b2' }); + }); + doc3.update((_, presence) => { + presence.set({ name: 'c2' }); + }); + await waitStubCallCount(stub1, 2); // c2 presence-changed + + // 03. c2 pauses the document (in manual sync), c3 resumes the document (in realtime sync). + // c1 receives an unwatched event from c2 and a watched event from c3. + await c2.pause(doc2); + await waitStubCallCount(stub1, 3); // c2 unwatched + await c3.resume(doc3); + await waitStubCallCount(stub1, 5); // c3 watched, c3 presence-changed + + // 04. c2 and c3 update the presence. + // c1 receives the presence-changed event from c3. + doc2.update((_, presence) => { + presence.set({ name: 'b3' }); + }); + doc3.update((_, presence) => { + presence.set({ name: 'c3' }); + }); + await waitStubCallCount(stub1, 6); // c3 presence-changed + + // 05. c3 pauses the document (in manual sync), + // c1 receives an unwatched event from c3. + await c3.pause(doc3); + await waitStubCallCount(stub1, 7); // c3 unwatched + + // 06. c2 performs manual sync and then resumes(switches to realtime sync). + // After applying all changes, only the watched event is triggered. + await c2.sync(); + await c2.resume(doc2); + await waitStubCallCount(stub1, 8); // c2 watched + + assert.deepEqual(stub1.args, [ + [ + { + type: DocEventType.Watched, + value: { + clientID: c2ID, + presence: { cursor: { x: 0, y: 0 }, name: 'b1' }, + }, + }, + ], + [ + { + type: DocEventType.PresenceChanged, + value: { + clientID: c2ID, + presence: { cursor: { x: 0, y: 0 }, name: 'b2' }, + }, + }, + ], + [ + { + type: DocEventType.Unwatched, + value: { + clientID: c2ID, + presence: { cursor: { x: 0, y: 0 }, name: 'b2' }, + }, + }, + ], + [ + { + type: DocEventType.Watched, + value: { + clientID: c3ID, + presence: { cursor: { x: 0, y: 0 }, name: 'c1' }, + }, + }, + ], + [ + { + type: DocEventType.PresenceChanged, + value: { + clientID: c3ID, + presence: { cursor: { x: 0, y: 0 }, name: 'c2' }, + }, + }, + ], + [ + { + type: DocEventType.PresenceChanged, + value: { + clientID: c3ID, + presence: { cursor: { x: 0, y: 0 }, name: 'c3' }, + }, + }, + ], + [ + { + type: DocEventType.Unwatched, + value: { + clientID: c3ID, + presence: { cursor: { x: 0, y: 0 }, name: 'c3' }, + }, + }, + ], + [ + { + type: DocEventType.Watched, + value: { + clientID: c2ID, + presence: { cursor: { x: 0, y: 0 }, name: 'b3' }, + }, + }, + ], + ]); + unsub1(); + await c1.deactivate(); + await c2.deactivate(); + await c3.deactivate(); + }).timeout(10000); }); From e25ec302c0023f7e73355368f8dbb24e4b8ed156 Mon Sep 17 00:00:00 2001 From: Yourim Cha Date: Thu, 3 Aug 2023 14:05:13 +0900 Subject: [PATCH 4/8] Add comments --- src/client/client.ts | 4 ++++ src/document/document.ts | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/src/client/client.ts b/src/client/client.ts index 16591c7e7..abb207df3 100644 --- a/src/client/client.ts +++ b/src/client/client.ts @@ -892,6 +892,8 @@ export class Client implements Observable { break; case PbDocEventType.DOC_EVENT_TYPE_DOCUMENTS_WATCHED: attachment.doc.addOnlineClient(publisher); + // NOTE(chacha912): We added to onlineClients, but we won't trigger watched event + // unless we also know their initial presence data at this point. if (attachment.doc.hasPresence(publisher)) { attachment.doc.publish({ type: DocEventType.Watched, @@ -905,6 +907,8 @@ export class Client implements Observable { case PbDocEventType.DOC_EVENT_TYPE_DOCUMENTS_UNWATCHED: { const presence = attachment.doc.getPresence(publisher); attachment.doc.removeOnlineClient(publisher); + // NOTE(chacha912): There is no presence, when PresenceChange(clear) is applied before unwatching. + // In that case, the 'unwatched' event is triggered while handling the PresenceChange. if (presence) { attachment.doc.publish({ type: DocEventType.Unwatched, diff --git a/src/document/document.ts b/src/document/document.ts index 58f439e0c..d23276571 100644 --- a/src/document/document.ts +++ b/src/document/document.ts @@ -977,6 +977,9 @@ export class Document { const presenceChange = change.getPresenceChange()!; switch (presenceChange.type) { case PresenceChangeType.Put: + // NOTE(chacha912): When the user exists in onlineClients, but + // their presence was initially absent, we can consider that we have + // received their initial presence, so trigger the 'watched' event. docEvent = { type: this.presences.has(actorID) ? DocEventType.PresenceChanged @@ -988,6 +991,11 @@ export class Document { }; break; case PresenceChangeType.Clear: + // NOTE(chacha912): When the user exists in onlineClients, but + // PresenceChange(clear) is received, we can consider it as detachment + // occurring before unwatching. + // Detached user is no longer participating in the document, we remove + // them from the online clients and trigger the 'unwatched' event. docEvent = { type: DocEventType.Unwatched, value: { From ca94e35a6e31ca8c800fec68eaaee91e6e7aa2aa Mon Sep 17 00:00:00 2001 From: Yourim Cha Date: Thu, 3 Aug 2023 17:21:39 +0900 Subject: [PATCH 5/8] Modify test to pass --- test/integration/presence_test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/presence_test.ts b/test/integration/presence_test.ts index 68b5e0221..759366af4 100644 --- a/test/integration/presence_test.ts +++ b/test/integration/presence_test.ts @@ -302,11 +302,11 @@ describe('Presence', function () { const counter = p.get('counter'); p.set({ counter: counter + 1 }); }); - assert.deepEqual(doc1.getPresence(c1.getID()!), { counter: 1 }); + assert.deepEqual(doc1.getPresenceForTest(c1.getID()!), { counter: 1 }); await c1.sync(); await c2.sync(); - assert.deepEqual(doc2.getPresence(c1.getID()!), { counter: 1 }); + assert.deepEqual(doc2.getPresenceForTest(c1.getID()!), { counter: 1 }); }); }); From 00b0a2d9d23229a7dd279a5cfb9df5352922cc1c Mon Sep 17 00:00:00 2001 From: Yourim Cha Date: Thu, 3 Aug 2023 17:47:15 +0900 Subject: [PATCH 6/8] Modify test to pass --- test/integration/document_test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/integration/document_test.ts b/test/integration/document_test.ts index d88ce18db..13e5e0bdf 100644 --- a/test/integration/document_test.ts +++ b/test/integration/document_test.ts @@ -589,13 +589,13 @@ describe('Document', function () { d1.update((root) => { root['k1'] = [1, 2]; }, 'set array'); - await c1.attach(d1); + await c1.attach(d1, { isRealtimeSync: false }); assert.equal(d1.toSortedJSON(), '{"k1":[1,2]}'); const c2 = new yorkie.Client(testRPCAddr); await c2.activate(); const d2 = new yorkie.Document(docKey); - await c2.attach(d2); + await c2.attach(d2, { isRealtimeSync: false }); assert.equal(d2.toSortedJSON(), '{"k1":[1,2]}'); // 02. c1 updates d1 and removes it. @@ -603,12 +603,12 @@ describe('Document', function () { root['k1'].push(3); }); await c1.remove(d1); - assert.equal(d1.toSortedJSON(), '{"k1":[1,2,3]}'); + assert.equal(d1.toSortedJSON(), '{"k1":[1,2,3]}', 'd1'); assert.equal(d1.getStatus(), DocumentStatus.Removed); // 03. c2 syncs and checks that d2 is removed. await c2.sync(); - assert.equal(d2.toSortedJSON(), '{"k1":[1,2,3]}'); + assert.equal(d2.toSortedJSON(), '{"k1":[1,2,3]}', 'd2'); assert.equal(d2.getStatus(), DocumentStatus.Removed); await c1.deactivate(); From 45030ac1cb0aaff8c9a06ec4f4bb5c6c8ef4b32b Mon Sep 17 00:00:00 2001 From: Youngteac Hong Date: Fri, 4 Aug 2023 09:35:32 +0900 Subject: [PATCH 7/8] Update test/integration/presence_test.ts --- test/integration/presence_test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/presence_test.ts b/test/integration/presence_test.ts index 759366af4..15a5daa4e 100644 --- a/test/integration/presence_test.ts +++ b/test/integration/presence_test.ts @@ -589,5 +589,5 @@ describe(`Document.Subscribe('presence')`, function () { await c1.deactivate(); await c2.deactivate(); await c3.deactivate(); - }).timeout(10000); + }); }); From b2eb6d45ef35e9b7300d8ac653538eda80479566 Mon Sep 17 00:00:00 2001 From: Youngteac Hong Date: Fri, 4 Aug 2023 18:34:35 +0900 Subject: [PATCH 8/8] Fix some non-deterministic behavior in test cases related to Presence --- test/helper/helper.ts | 17 ++++++++++++---- test/integration/client_test.ts | 33 ++++++++++++++++++++----------- test/integration/presence_test.ts | 28 +++++++++++++++----------- 3 files changed, 51 insertions(+), 27 deletions(-) diff --git a/test/helper/helper.ts b/test/helper/helper.ts index f06b344ff..92644f64f 100644 --- a/test/helper/helper.ts +++ b/test/helper/helper.ts @@ -23,6 +23,14 @@ import { OperationInfo } from '@yorkie-js-sdk/src/document/operation/operation'; export type Indexable = Record; +export async function sleep(interval = 1000): Promise { + return new Promise((resolve) => { + setTimeout(() => { + resolve(); + }, interval); + }); +} + export async function waitStubCallCount( stub: sinon.SinonStub, callCount: number, @@ -31,12 +39,13 @@ export async function waitStubCallCount( const doLoop = () => { if (stub.callCount >= callCount) { resolve(); + return; } - return false; + + setTimeout(doLoop, 0); }; - if (!doLoop()) { - setTimeout(doLoop, 1000); - } + + doLoop(); }); } diff --git a/test/integration/client_test.ts b/test/integration/client_test.ts index cebca5304..4443e5ae2 100644 --- a/test/integration/client_test.ts +++ b/test/integration/client_test.ts @@ -171,9 +171,10 @@ describe('Client', function () { }); await waitStubCallCount(stubD2, 1); // d2 should be able to update - assert.equal(d2Events.pop(), DocEventType.LocalChange); + + assert.equal(d2Events.at(-1), DocEventType.LocalChange); await waitStubCallCount(stubD1, 1); // d1 should be able to receive d2's update - assert.equal(d1Events.pop(), DocEventType.RemoteChange); + assert.equal(d1Events.at(-1), DocEventType.RemoteChange); assert.equal(d1.toSortedJSON(), d2.toSortedJSON()); // Simulate network error @@ -193,10 +194,11 @@ describe('Client', function () { }); await waitStubCallCount(stubD2, 2); // d2 should be able to update - assert.equal(d2Events.pop(), DocEventType.LocalChange); + assert.equal(d2Events.at(-1), DocEventType.LocalChange); await waitStubCallCount(stubC2, 2); // c2 should fail to sync + assert.equal( - c2Events.pop(), + c2Events.at(-1), DocumentSyncResultType.SyncFailed, 'c2 sync fail', ); @@ -205,7 +207,7 @@ describe('Client', function () { assert.equal(err.message, 'INVALID_STATE_ERR - 0'); // c1 should also fail to sync }); assert.equal( - c1Events.pop(), + c1Events.at(-1), DocumentSyncResultType.SyncFailed, 'c1 sync fail', ); @@ -216,11 +218,16 @@ describe('Client', function () { xhr.restore(); await waitStubCallCount(stubC2, 3); // wait for c2 to sync - assert.equal(c2Events.pop(), DocumentSyncResultType.Synced, 'c2 sync'); - await waitStubCallCount(stubC1, 5); - assert.equal(c1Events.pop(), DocumentSyncResultType.Synced, 'c1 sync'); + assert.equal(c2Events.at(-1), DocumentSyncResultType.Synced, 'c2 sync'); + await waitStubCallCount(stubC1, 6); + assert.isTrue( + [c1Events.at(-1), c1Events.at(-2)].includes( + DocumentSyncResultType.Synced, + ), + 'c1 sync', + ); await waitStubCallCount(stubD1, 2); - assert.equal(d1Events.pop(), DocEventType.RemoteChange); // d1 should be able to receive d2's update + assert.equal(d1Events.at(-1), DocEventType.RemoteChange); // d1 should be able to receive d2's update assert.equal(d1.toSortedJSON(), d2.toSortedJSON()); unsub1.client(); @@ -268,8 +275,12 @@ describe('Client', function () { root.version = 'v2'; }); await c1.sync(); - await waitStubCallCount(stubC2, 2); - assert.equal(c2Events.pop(), ClientEventType.DocumentSynced); + await waitStubCallCount(stubC2, 3); + assert.isTrue( + [c2Events.at(-1), c2Events.at(-1)].includes( + ClientEventType.DocumentSynced, + ), + ); assert.equal(d1.toSortedJSON(), d2.toSortedJSON()); unsub1(); diff --git a/test/integration/presence_test.ts b/test/integration/presence_test.ts index 15a5daa4e..19620530f 100644 --- a/test/integration/presence_test.ts +++ b/test/integration/presence_test.ts @@ -5,7 +5,11 @@ import { testRPCAddr, toDocKey, } from '@yorkie-js-sdk/test/integration/integration_helper'; -import { waitStubCallCount, deepSort } from '@yorkie-js-sdk/test/helper/helper'; +import { + waitStubCallCount, + sleep, + deepSort, +} from '@yorkie-js-sdk/test/helper/helper'; describe('Presence', function () { it('Can be built from a snapshot', async function () { @@ -102,7 +106,9 @@ describe('Presence', function () { assert.deepEqual(doc1.getPresenceForTest(c2.getID()!), emptyObject); }); - it('Should be synced eventually', async function () { + // TODO(hackerwins): This test case is not stable. It should be fixed. + // To occur the problem, we need to run this test case in for loop with 50 or 100 iteration. + it.skip('Should be synced eventually', async function () { const c1 = new yorkie.Client(testRPCAddr); const c2 = new yorkie.Client(testRPCAddr); await c1.activate(); @@ -113,20 +119,12 @@ describe('Presence', function () { const docKey = toDocKey(`${this.test!.title}-${new Date().getTime()}`); type PresenceType = { name: string }; const doc1 = new yorkie.Document<{}, PresenceType>(docKey); - await c1.attach(doc1, { - initialPresence: { - name: 'a', - }, - }); + await c1.attach(doc1, { initialPresence: { name: 'a' } }); const stub1 = sinon.stub(); const unsub1 = doc1.subscribe('presence', stub1); const doc2 = new yorkie.Document<{}, PresenceType>(docKey); - await c2.attach(doc2, { - initialPresence: { - name: 'b', - }, - }); + await c2.attach(doc2, { initialPresence: { name: 'b' } }); const stub2 = sinon.stub(); const unsub2 = doc2.subscribe('presence', stub2); @@ -507,8 +505,14 @@ describe(`Document.Subscribe('presence')`, function () { // 06. c2 performs manual sync and then resumes(switches to realtime sync). // After applying all changes, only the watched event is triggered. + + // TODO(hackerwins): This is workaround for some non-deterministic behavior. + // We need to fix this issue. + await sleep(); await c2.sync(); + await sleep(); await c2.resume(doc2); + await sleep(); await waitStubCallCount(stub1, 8); // c2 watched assert.deepEqual(stub1.args, [