Skip to content

Commit

Permalink
Prevent remote-change events from occurring in push-only mode (#759)
Browse files Browse the repository at this point in the history
When performing `CRDTTree.edit()`, the edits are reflected by
deepcopying the CRDTTreeNodes for the given range.
This commit adds information about `insPrevID` and `insNextID` during
the `deepcopy()` process to ensure that the correct location is
returned from the correct path.

---------

Co-authored-by: Youngteac Hong <susukang98@gmail.com>
  • Loading branch information
chacha912 and hackerwins authored Mar 7, 2024
1 parent 0fc7f74 commit f1627ef
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 3 deletions.
9 changes: 6 additions & 3 deletions src/client/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -954,9 +954,12 @@ export class Client implements Observable<ClientEvent> {
.then((res) => {
const respPack = converter.fromChangePack<P>(res.changePack!);

// (chacha912, hackerwins): If syncLoop already executed with
// NOTE(chacha912, hackerwins): If syncLoop already executed with
// PushPull, ignore the response when the syncMode is PushOnly.
if (respPack.hasChanges() && syncMode === SyncMode.PushOnly) {
if (
respPack.hasChanges() &&
attachment.syncMode === SyncMode.PushOnly
) {
return doc;
}

Expand All @@ -965,7 +968,7 @@ export class Client implements Observable<ClientEvent> {
type: ClientEventType.DocumentSynced,
value: DocumentSyncResultType.Synced,
});
// (chacha912): If a document has been removed, watchStream should
// NOTE(chacha912): If a document has been removed, watchStream should
// be disconnected to not receive an event for that document.
if (doc.getStatus() === DocumentStatus.Removed) {
this.detachInternal(doc.getKey());
Expand Down
84 changes: 84 additions & 0 deletions test/integration/client_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import yorkie, {
DocumentSyncResultType,
DocEventType,
ClientEventType,
Tree,
} from '@yorkie-js-sdk/src/yorkie';
import { EventCollector } from '@yorkie-js-sdk/test/helper/helper';
import {
Expand Down Expand Up @@ -545,4 +546,87 @@ describe.sequential('Client', function () {

await c1.deactivate();
});

it('Should prevent remote changes in push-only mode', async function ({
task,
}) {
const c1 = new yorkie.Client(testRPCAddr);
const c2 = new yorkie.Client(testRPCAddr);
await c1.activate();
await c2.activate();

const docKey = toDocKey(`${task.name}-${new Date().getTime()}`);
const d1 = new yorkie.Document<{ tree: Tree }>(docKey);
const d2 = new yorkie.Document<{ tree: Tree }>(docKey);
await c1.attach(d1);
await c2.attach(d2);

const eventCollectorD1 = new EventCollector();
const eventCollectorD2 = new EventCollector();
const unsub1 = d1.subscribe((event) => {
eventCollectorD1.add(event.type);
});
const unsub2 = d2.subscribe((event) => {
eventCollectorD2.add(event.type);
});

d1.update((root) => {
root.tree = new Tree({
type: 'doc',
children: [
{
type: 'p',
children: [{ type: 'text', value: '12' }],
},
{
type: 'p',
children: [{ type: 'text', value: '34' }],
},
],
});
});
await eventCollectorD2.waitAndVerifyNthEvent(1, DocEventType.RemoteChange);

assert.equal(d1.getRoot().tree.toXML(), '<doc><p>12</p><p>34</p></doc>');
assert.equal(d2.getRoot().tree.toXML(), '<doc><p>12</p><p>34</p></doc>');

d1.update((root: any) => {
root.tree.edit(2, 2, { type: 'text', value: 'a' });
});
await c1.sync();

// Simulate the situation in the runSyncLoop where a pushpull request has been sent
// but a response has not yet been received.
c2.sync();

// In push-only mode, remote-change events should not occur.
c2.pauseRemoteChanges(d2);
let remoteChangeOccured = false;
const unsub3 = d2.subscribe((event) => {
if (event.type === DocEventType.RemoteChange) {
remoteChangeOccured = true;
}
});
await new Promise((res) => {
// TODO(chacha912): We need to clean up this later because it is non-deterministic.
setTimeout(res, 100); // Keep the push-only state.
});
unsub3();
assert.isFalse(remoteChangeOccured);

c2.resumeRemoteChanges(d2);

d2.update((root: any) => {
root.tree.edit(2, 2, { type: 'text', value: 'b' });
});
await eventCollectorD1.waitAndVerifyNthEvent(3, DocEventType.RemoteChange);

assert.equal(d1.getRoot().tree.toXML(), '<doc><p>1ba2</p><p>34</p></doc>');
assert.equal(d2.getRoot().tree.toXML(), '<doc><p>1ba2</p><p>34</p></doc>');

unsub1();
unsub2();
await c1.deactivate();
await c2.deactivate();
});
});

0 comments on commit f1627ef

Please sign in to comment.