From 0dcfea9fe022b4fc61e2d1d835ab9e9dc5ff521d Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Fri, 18 Oct 2024 19:45:37 -0400 Subject: [PATCH] properly test expected behavior not only expeted results, add commens to code --- packages/api/src/record.ts | 4 +-- packages/api/tests/dwn-api.spec.ts | 48 +++++++++++++++++++++++++----- packages/api/tests/record.spec.ts | 46 ++++++++++++++++++++++++++-- 3 files changed, 86 insertions(+), 12 deletions(-) diff --git a/packages/api/src/record.ts b/packages/api/src/record.ts index 2bce08c99..e93b2463f 100644 --- a/packages/api/src/record.ts +++ b/packages/api/src/record.ts @@ -721,7 +721,7 @@ export class Record implements RecordModel { ...descriptor, ...params, parentContextId, - protocolRole, + protocolRole : protocolRole ?? this._protocolRole, // Use the current protocolRole if not provided. messageTimestamp : dateModified, // Map Record class `dateModified` property to DWN SDK `messageTimestamp` recordId : this._recordId }; @@ -850,7 +850,7 @@ export class Record implements RecordModel { prune : prune, recordId : this._recordId, messageTimestamp : dateModified, - protocolRole : deleteParams?.protocolRole + protocolRole : deleteParams?.protocolRole ?? this._protocolRole // if no protocolRole is provided, use the current protocolRole }; } diff --git a/packages/api/tests/dwn-api.spec.ts b/packages/api/tests/dwn-api.spec.ts index 7ee55cc0e..992d7368b 100644 --- a/packages/api/tests/dwn-api.spec.ts +++ b/packages/api/tests/dwn-api.spec.ts @@ -3,7 +3,7 @@ import type { BearerDid } from '@web5/dids'; import sinon from 'sinon'; import { expect } from 'chai'; import { Web5UserAgent } from '@web5/user-agent'; -import { AgentPermissionsApi, DwnDateSort, DwnProtocolDefinition, getRecordAuthor, Oidc, PlatformAgentTestHarness, WalletConnect } from '@web5/agent'; +import { AgentPermissionsApi, DwnDateSort, DwnInterface, DwnProtocolDefinition, getRecordAuthor, Oidc, PlatformAgentTestHarness, ProcessDwnRequest, SendDwnRequest, WalletConnect } from '@web5/agent'; import { DwnApi } from '../src/dwn-api.js'; import { testDwnUrl } from './utils/test-config.js'; @@ -2082,6 +2082,10 @@ describe('DwnApi', () => { }); it('ensures that a protocolRole used to query is also used to read the data of the resulted records', async () => { + // scenario: Bob has a protocol where he can write notes and add friends who can query and read these notes + // Alice is a friend of Bob and she queries for the notes and reads the data of the notes + // the protocolRole used to query for the notes should also be used to read the data of the notes + const protocol = { ...notesProtocolDefinition, protocol: 'http://example.com/notes' + TestDataGenerator.randomString(15) @@ -2147,12 +2151,26 @@ describe('DwnApi', () => { expect(noteRecords).to.exist; expect(noteRecords).to.have.lengthOf(3); + // spy on sendDwnRequest to ensure that the protocolRole is used to read the data of the notes + const sendDwnRequestSpy = sinon.spy(testHarness.agent, 'sendDwnRequest'); + + // confirm that it starts with 0 calls + expect(sendDwnRequestSpy.callCount).to.equal(0); // Alice attempts to read the data of the notes, which should succeed for (const record of noteRecords) { const readResult = await record.data.text(); const expectedData = recordData.get(record.id); expect(readResult).to.equal(expectedData); } + + // confirm that it was called 3 times + expect(sendDwnRequestSpy.callCount).to.equal(3); + + // confirm that the protocolRole was used to read the data of the notes + expect(sendDwnRequestSpy.getCalls().every(call => + call.args[0].messageType === DwnInterface.RecordsRead && + (call.args[0] as ProcessDwnRequest).messageParams.protocolRole === 'friend' + )).to.be.true; }); }); }); @@ -2522,6 +2540,9 @@ describe('DwnApi', () => { }); it('ensures that a protocolRole used to subscribe is also used to read the data of the resulted records', async () => { + // scenario: Bob has a protocol where he can write notes and add friends who can subscribe and read these notes + // When Alice subscribes to the notes protocol using the role, the role should also be used to read the data of the notes + const protocol = { ...notesProtocolDefinition, protocol: 'http://example.com/notes' + TestDataGenerator.randomString(15) @@ -2555,11 +2576,6 @@ describe('DwnApi', () => { // Alice subscribes to the notes protocol using the role const notes: Map = new Map(); - const subscriptionHandler = async (record: Record) => { - notes.set(record.id, record); - }; - - // alice uses the role to query for the available notes const { status: notesSubscribeStatus, subscription } = await dwnAlice.records.subscribe({ from : bobDid.uri, message : { @@ -2569,7 +2585,10 @@ describe('DwnApi', () => { protocolPath : 'note' } }, - subscriptionHandler + subscriptionHandler: (record) => { + // add to the notes map + notes.set(record.id, record); + } }); expect(notesSubscribeStatus.code).to.equal(200); expect(subscription).to.exist; @@ -2599,11 +2618,26 @@ describe('DwnApi', () => { expect(notes.size).to.equal(3); }); + // spy on sendDwnRequest to ensure that the protocolRole is used to read the data of the notes + const sendDwnRequestSpy = sinon.spy(testHarness.agent, 'sendDwnRequest'); + + // confirm that it starts with 0 calls + expect(sendDwnRequestSpy.callCount).to.equal(0); + // Alice attempts to read the data of the notes, which should succeed for (const record of notes.values()) { const readResult = await record.data.text(); const expectedData = recordData.get(record.id); expect(readResult).to.equal(expectedData); } + + // confirm that it was called 3 times + expect(sendDwnRequestSpy.callCount).to.equal(3); + + // confirm that the protocolRole was used to read the data of the notes + expect(sendDwnRequestSpy.getCalls().every(call => + call.args[0].messageType === DwnInterface.RecordsRead && + (call.args[0] as ProcessDwnRequest).messageParams.protocolRole === 'friend' + )).to.be.true; }); }); }); diff --git a/packages/api/tests/record.spec.ts b/packages/api/tests/record.spec.ts index f4645f0eb..136ca258e 100644 --- a/packages/api/tests/record.spec.ts +++ b/packages/api/tests/record.spec.ts @@ -1,12 +1,12 @@ import type { BearerDid ,PortableDid } from '@web5/dids'; -import type { DwnMessageParams, DwnProtocolDefinition, DwnPublicKeyJwk, DwnSigner } from '@web5/agent'; +import type { DwnMessageParams, DwnProtocolDefinition, DwnPublicKeyJwk, DwnSigner, ProcessDwnRequest, SendDwnRequest } from '@web5/agent'; import sinon from 'sinon'; import { expect } from 'chai'; import { NodeStream } from '@web5/common'; import { utils as didUtils } from '@web5/dids'; import { Web5UserAgent } from '@web5/user-agent'; -import { DwnConstant, DwnDateSort, DwnEncryptionAlgorithm, DwnInterface, DwnKeyDerivationScheme, dwnMessageConstructors, getRecordAuthor, Oidc, PlatformAgentTestHarness, WalletConnect } from '@web5/agent'; +import { DwnConstant, DwnDateSort, DwnEncryptionAlgorithm, DwnInterface, DwnKeyDerivationScheme, dwnMessageConstructors, getRecordAuthor, getRecordProtocolRole, Oidc, PlatformAgentTestHarness, WalletConnect } from '@web5/agent'; import { Record } from '../src/record.js'; import { DwnApi } from '../src/dwn-api.js'; import { dataToBlob } from '../src/utils.js'; @@ -3089,6 +3089,9 @@ describe('Record', () => { }); it('updates a record using a different protocolRole than the one used when querying for/reading the record', async () => { + // scenario: Bob has a notes protocol that has friends who can read/query/subscribe to notes, but coAuthors that can update notes. + // When Alice uses her friend role to query for notes, she cannot update them with that same role. Instead she uses her coAuthor role update. + const protocol = { ...notesProtocolDefinition, protocol: 'http://example.com/notes' + TestDataGenerator.randomString(15) @@ -3196,16 +3199,32 @@ describe('Record', () => { const { status: updateStatus } = await coAuthorNote!.update({ data: 'updated note' }); expect(updateStatus.code).to.equal(202); + // spy on sendDwnRequest to ensure that the protocolRole is used to read the data of the notes + const sendDwnRequestSpy = sinon.spy(testHarness.agent, 'sendDwnRequest'); + + // confirm that it starts with 0 calls + expect(sendDwnRequestSpy.callCount).to.equal(0); + // This is accepted locally but will fail when sending the update to the remote DWN const { status: sendStatus } = await coAuthorNote.send(bobDid.uri); expect(sendStatus.code).to.equal(401); + expect(sendDwnRequestSpy.callCount).to.equal(2); // the first call is for the initialWrite + let record = (sendDwnRequestSpy.secondCall.args[0] as ProcessDwnRequest).rawMessage; + let sendAuthorizationRole = getRecordProtocolRole(record); + expect(sendAuthorizationRole).to.equal('friend'); - // Now update the record with the correct role const { status: updateStatusCoAuthor } = await coAuthorNote!.update({ data: 'updated note', protocolRole: 'note/coAuthor' }); expect(updateStatusCoAuthor.code).to.equal(202); + sendDwnRequestSpy.resetHistory(); + + // Now update the record with the correct role const { status: sendStatusCoAuthor } = await coAuthorNote.send(bobDid.uri); expect(sendStatusCoAuthor.code).to.equal(202); + expect(sendDwnRequestSpy.callCount).to.equal(1); // the initialWrite was already sent and added to the sent-cache, only the update is sent + record = (sendDwnRequestSpy.firstCall.args[0] as ProcessDwnRequest).rawMessage; + sendAuthorizationRole = getRecordProtocolRole(record); + expect(sendAuthorizationRole).to.equal('note/coAuthor'); }); }); @@ -3782,6 +3801,9 @@ describe('Record', () => { }); it('deletes a record using a different protocolRole than the one used when querying for/reading the record', async () => { + // scenario: Bob has a notes protocol that has friends who can read/query/subscribe to notes, but coAuthors that can update/delete notes. + // When Alice uses her friend role to query for notes, she cannot delete them with that same role. Instead she uses her coAuthor role to delete. + const protocol = { ...notesProtocolDefinition, protocol: 'http://example.com/notes' + TestDataGenerator.randomString(15) @@ -3880,18 +3902,36 @@ describe('Record', () => { const coDeleteNote = bobNotesAliceQuery.find((record) => record.id === aliceCoAuthorNoteId); expect(coDeleteNote).to.not.be.undefined; + // spy on sendDwnRequest to ensure that the protocolRole is used to read the data of the notes + const sendDwnRequestSpy = sinon.spy(testHarness.agent, 'sendDwnRequest'); + + // confirm that it starts with 0 calls + expect(sendDwnRequestSpy.callCount).to.equal(0); + const { status: deleteStatus } = await coDeleteNote.delete({ store: false }); expect(deleteStatus.code).to.equal(202); const { status: sendDeleteStatus } = await coDeleteNote.send(bobDid.uri); expect(sendDeleteStatus.code).to.equal(401); + expect(sendDwnRequestSpy.callCount).to.equal(2); // the first call is for the initialWrite + let record = (sendDwnRequestSpy.secondCall.args[0] as ProcessDwnRequest).rawMessage; + let sendAuthorizationRole = getRecordProtocolRole(record); + expect(sendAuthorizationRole).to.equal('friend'); + + sendDwnRequestSpy.resetHistory(); + // Now update the record with the correct role const { status: updateStatusCoAuthor } = await coDeleteNote.delete({ protocolRole: 'note/coAuthor', store: false }); expect(updateStatusCoAuthor.code).to.equal(202, `delete: ${updateStatusCoAuthor.detail}`); const { status: sendStatusCoAuthor } = await coDeleteNote.send(bobDid.uri); expect(sendStatusCoAuthor.code).to.equal(202, `delete send: ${sendStatusCoAuthor.detail}`); + + expect(sendDwnRequestSpy.callCount).to.equal(1); // the initialWrite was already sent and added to the sent-cache, only the update is sent + record = (sendDwnRequestSpy.firstCall.args[0] as ProcessDwnRequest).rawMessage; + sendAuthorizationRole = getRecordProtocolRole(record); + expect(sendAuthorizationRole).to.equal('note/coAuthor'); }); });