From 226455a7d6029292c810e67d80e129e2e20f83da Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Thu, 29 Aug 2024 15:08:08 -0400 Subject: [PATCH] Conflicting Sync error. (#857) When I added the one-shot `sync()` method I put in a guard to prevent it from being called while an interval was running. This caused a bug in the interval, so I've modified the code to account for it. Also updated some type docs to get rid of the warning. --- .changeset/tall-birds-dress.md | 8 +++++ packages/agent/src/connect.ts | 20 +++++++++++- packages/agent/src/sync-engine-level.ts | 3 +- packages/agent/tests/connect.spec.ts | 12 +++++-- .../agent/tests/sync-engine-level.spec.ts | 31 +++++++++++++------ packages/api/src/web5.ts | 25 +++++++++++++-- packages/api/tests/web5.spec.ts | 6 ++-- 7 files changed, 84 insertions(+), 21 deletions(-) create mode 100644 .changeset/tall-birds-dress.md diff --git a/.changeset/tall-birds-dress.md b/.changeset/tall-birds-dress.md new file mode 100644 index 000000000..6eaf78707 --- /dev/null +++ b/.changeset/tall-birds-dress.md @@ -0,0 +1,8 @@ +--- +"@web5/agent": patch +"@web5/identity-agent": patch +"@web5/proxy-agent": patch +"@web5/user-agent": patch +--- + +Sync vs StartSync conflicting error. diff --git a/packages/agent/src/connect.ts b/packages/agent/src/connect.ts index 24baf15cc..884c15b96 100644 --- a/packages/agent/src/connect.ts +++ b/packages/agent/src/connect.ts @@ -185,9 +185,27 @@ export type ConnectPermissionRequest = { permissionScopes: DwnPermissionScope[]; }; +/** + * Shorthand for the types of permissions that can be requested. + */ export type Permission = 'write' | 'read' | 'delete' | 'query' | 'subscribe'; -function createPermissionRequestForProtocol(definition: DwnProtocolDefinition, permissions: Permission[]): ConnectPermissionRequest { +/** + * The options for creating a permission request for a given protocol. + */ +export type ProtocolPermissionOptions = { + /** The protocol definition for the protocol being requested */ + definition: DwnProtocolDefinition; + + /** The permissions being requested for the protocol */ + permissions: Permission[]; +}; + +/** + * Creates a set of Dwn Permission Scopes to request for a given protocol. + * If no permissions are provided, the default is to request all permissions (write, read, delete, query, subscribe). + */ +function createPermissionRequestForProtocol({ definition, permissions }: ProtocolPermissionOptions): ConnectPermissionRequest { const requests: DwnPermissionScope[] = []; // In order to enable sync, we must request permissions for `MessagesQuery`, `MessagesRead` and `MessagesSubscribe` diff --git a/packages/agent/src/sync-engine-level.ts b/packages/agent/src/sync-engine-level.ts index 5908fba49..72336b3b3 100644 --- a/packages/agent/src/sync-engine-level.ts +++ b/packages/agent/src/sync-engine-level.ts @@ -288,7 +288,8 @@ export class SyncEngineLevel implements SyncEngine { } try { - await this.sync(); + await this.push(); + await this.pull(); } catch (error: any) { this.stopSync(); reject(error); diff --git a/packages/agent/tests/connect.spec.ts b/packages/agent/tests/connect.spec.ts index c56dc8720..286f7234d 100644 --- a/packages/agent/tests/connect.spec.ts +++ b/packages/agent/tests/connect.spec.ts @@ -822,7 +822,9 @@ describe('web5 connect', function () { } }; - const permissionRequests = WalletConnect.createPermissionRequestForProtocol(protocol, []); + const permissionRequests = WalletConnect.createPermissionRequestForProtocol({ + definition: protocol, permissions: [] + }); expect(permissionRequests.protocolDefinition).to.deep.equal(protocol); expect(permissionRequests.permissionScopes.length).to.equal(3); // only includes the sync permissions @@ -846,7 +848,9 @@ describe('web5 connect', function () { } }; - const permissionRequests = WalletConnect.createPermissionRequestForProtocol(protocol, ['write', 'read']); + const permissionRequests = WalletConnect.createPermissionRequestForProtocol({ + definition: protocol, permissions: ['write', 'read'] + }); expect(permissionRequests.protocolDefinition).to.deep.equal(protocol); @@ -871,7 +875,9 @@ describe('web5 connect', function () { } }; - const permissionRequests = WalletConnect.createPermissionRequestForProtocol(protocol, ['write', 'read', 'delete', 'query', 'subscribe']); + const permissionRequests = WalletConnect.createPermissionRequestForProtocol({ + definition: protocol, permissions: ['write', 'read', 'delete', 'query', 'subscribe'] + }); expect(permissionRequests.protocolDefinition).to.deep.equal(protocol); diff --git a/packages/agent/tests/sync-engine-level.spec.ts b/packages/agent/tests/sync-engine-level.spec.ts index 35fa68a08..85003f6ec 100644 --- a/packages/agent/tests/sync-engine-level.spec.ts +++ b/packages/agent/tests/sync-engine-level.spec.ts @@ -2415,23 +2415,28 @@ describe('SyncEngineLevel', () => { }); describe('startSync()', () => { - it('calls sync() in each interval', async () => { + it('calls pull() and push() in each interval', async () => { await testHarness.agent.sync.registerIdentity({ did: alice.did.uri, }); - const syncSpy = sinon.stub(SyncEngineLevel.prototype, 'sync'); - syncSpy.resolves(); + const pullSpy = sinon.stub(SyncEngineLevel.prototype as any, 'pull'); + pullSpy.resolves(); + + const pushSpy = sinon.stub(SyncEngineLevel.prototype as any, 'push'); + pushSpy.resolves(); const clock = sinon.useFakeTimers(); testHarness.agent.sync.startSync({ interval: '500ms' }); await clock.tickAsync(1_400); // just under 3 intervals - syncSpy.restore(); + pullSpy.restore(); + pushSpy.restore(); clock.restore(); - expect(syncSpy.callCount).to.equal(2, 'push'); + expect(pullSpy.callCount).to.equal(2, 'push'); + expect(pushSpy.callCount).to.equal(2, 'pull'); }); it('does not call sync() again until a sync round finishes', async () => { @@ -2441,24 +2446,30 @@ describe('SyncEngineLevel', () => { const clock = sinon.useFakeTimers(); - const syncSpy = sinon.stub(SyncEngineLevel.prototype, 'sync'); - syncSpy.returns(new Promise((resolve) => { + const pullSpy = sinon.stub(SyncEngineLevel.prototype as any, 'pull'); + pullSpy.returns(new Promise((resolve) => { clock.setTimeout(() => { resolve(); }, 1_500); // more than the interval })); + const pushSpy = sinon.stub(SyncEngineLevel.prototype as any, 'push'); + pushSpy.resolves(); + testHarness.agent.sync.startSync({ interval: '500ms' }); await clock.tickAsync(1_400); // less time than the push - expect(syncSpy.callCount).to.equal(1, 'sync'); + expect(pullSpy.callCount).to.equal(1, 'pull'); + expect(pullSpy.callCount).to.equal(1, 'push'); await clock.tickAsync(600); //remaining time for a 2nd sync - expect(syncSpy.callCount).to.equal(2, 'sync'); + expect(pullSpy.callCount).to.equal(2, 'pull'); + expect(pushSpy.callCount).to.equal(2, 'push'); - syncSpy.restore(); + pullSpy.restore(); + pushSpy.restore(); clock.restore(); }); }); diff --git a/packages/api/src/web5.ts b/packages/api/src/web5.ts index 75468f6d6..b93e857bf 100644 --- a/packages/api/src/web5.ts +++ b/packages/api/src/web5.ts @@ -36,12 +36,28 @@ export type DidCreateOptions = { dwnEndpoints?: string[]; } +/** + * Represents a permission request for a protocol definition. + */ export type ConnectPermissionRequest = { + /** + * The protocol definition for the protocol being requested. + */ protocolDefinition: DwnProtocolDefinition; + /** + * The permissions being requested for the protocol. If none are provided, the default is to request all permissions. + */ permissions?: Permission[]; } +/** + * Options for connecting to a Web5 agent. This includes the ability to connect to an external wallet + */ export type ConnectOptions = Omit & { + /** + * The permissions that are being requested for the connected DID. + * This is used to create the {@link ConnectPermissionRequest} for the wallet connect flow. + */ permissionRequests: ConnectPermissionRequest[]; } @@ -286,9 +302,12 @@ export class Web5 { // No connected identity found and connectOptions are provided, attempt to import a delegated DID from an external wallet try { const { permissionRequests, ...connectOptions } = walletConnectOptions; - const walletPermissionRequests = permissionRequests.map(({ protocolDefinition, permissions }) => WalletConnect.createPermissionRequestForProtocol(protocolDefinition, permissions ?? [ - 'read', 'write', 'delete', 'query', 'subscribe' - ])); + const walletPermissionRequests = permissionRequests.map(({ protocolDefinition, permissions }) => WalletConnect.createPermissionRequestForProtocol({ + definition : protocolDefinition, + permissions : permissions ?? [ + 'read', 'write', 'delete', 'query', 'subscribe' + ]} + )); const { delegatePortableDid, connectedDid, delegateGrants } = await WalletConnect.initClient({ ...connectOptions, diff --git a/packages/api/tests/web5.spec.ts b/packages/api/tests/web5.spec.ts index ceb4791e9..5f8981d4d 100644 --- a/packages/api/tests/web5.spec.ts +++ b/packages/api/tests/web5.spec.ts @@ -846,7 +846,7 @@ describe('web5 api', () => { const call = requestPermissionsSpy.getCall(0); // since no explicit permissions were provided, all permissions should be requested - expect(call.args[1]).to.have.members([ + expect(call.args[0].permissions).to.have.members([ 'read', 'write', 'delete', 'query', 'subscribe' ]); } @@ -920,14 +920,14 @@ describe('web5 api', () => { const call1 = requestPermissionsSpy.getCall(0); // since no explicit permissions were provided for the first protocol, all permissions should be requested - expect(call1.args[1]).to.have.members([ + expect(call1.args[0].permissions).to.have.members([ 'read', 'write', 'delete', 'query', 'subscribe' ]); const call2 = requestPermissionsSpy.getCall(1); // only the provided permissions should be requested for the second protocol - expect(call2.args[1]).to.have.members([ + expect(call2.args[0].permissions).to.have.members([ 'read', 'write' ]); }