From 7efd9bf58c71ea77783676b1dc274fcb1d97e741 Mon Sep 17 00:00:00 2001 From: Frank Hinek Date: Sat, 18 Nov 2023 19:30:25 -0500 Subject: [PATCH] Fix bug with `record.data.*` after record queries (#298) * Improve Type safety now that RecordsReadReply is part of DwnResponse * Improve test coverage by ensuring that the DWN query record size limit is imported from DWN SDK * Fix bug caused by RecordsRead now taking a filter property and add tests --- packages/agent/tests/dwn-manager.spec.ts | 5 +- packages/api/src/dwn-api.ts | 12 ++- packages/api/src/record.ts | 7 +- packages/api/tests/dwn-api.spec.ts | 53 ++++++++++++ packages/api/tests/record.spec.ts | 101 ++++++++++++++++++++--- 5 files changed, 152 insertions(+), 26 deletions(-) diff --git a/packages/agent/tests/dwn-manager.spec.ts b/packages/agent/tests/dwn-manager.spec.ts index 105110058..d695c74d8 100644 --- a/packages/agent/tests/dwn-manager.spec.ts +++ b/packages/agent/tests/dwn-manager.spec.ts @@ -8,7 +8,6 @@ import { EventsGetReply, EventsGetMessage, MessagesGetReply, - RecordsReadReply, RecordsQueryReply, UnionMessageReply, MessagesGetMessage, @@ -377,7 +376,7 @@ describe('DwnManager', () => { expect(readMessage).to.have.property('authorization'); expect(readMessage).to.have.property('descriptor'); - const readReply = readResponse.reply as RecordsReadReply; + const readReply = readResponse.reply; expect(readReply).to.have.property('status'); expect(readReply.status.code).to.equal(200); expect(readReply).to.have.property('record'); @@ -552,7 +551,7 @@ describe('DwnManager', () => { expect(readMessage.descriptor.method).to.equal('Read'); expect(readMessage.descriptor.interface).to.equal('Records'); - const readReply = response.reply as RecordsReadReply; + const readReply = response.reply; expect(readReply.record).to.exist; const record = readReply.record as unknown as RecordsWriteMessage & { data: ReadableStream }; diff --git a/packages/api/src/dwn-api.ts b/packages/api/src/dwn-api.ts index db5008024..359b8050e 100644 --- a/packages/api/src/dwn-api.ts +++ b/packages/api/src/dwn-api.ts @@ -440,13 +440,11 @@ export class DwnApi { /** * Writes a record to the DWN * - * As a convenience, the Record instance returned will cache a copy of the data if the - * data size, in bytes, is less than the DWN 'max data size allowed to be encoded' - * parameter of 10KB. This is done to maintain consistency with other DWN methods, - * like RecordsQuery, that include relatively small data payloads when returning - * RecordsWrite message properties. Regardless of data size, methods such as - * `record.data.stream()` will return the data when called even if it requires fetching - * from the DWN datastore. + * As a convenience, the Record instance returned will cache a copy of the data. This is done + * to maintain consistency with other DWN methods, like RecordsQuery, that include relatively + * small data payloads when returning RecordsWrite message properties. Regardless of data + * size, methods such as `record.data.stream()` will return the data when called even if it + * requires fetching from the DWN datastore. */ write: async (request: RecordsWriteRequest): Promise => { const messageOptions: Partial = { diff --git a/packages/api/src/record.ts b/packages/api/src/record.ts index f95de7a66..88c7fc3e9 100644 --- a/packages/api/src/record.ts +++ b/packages/api/src/record.ts @@ -1,7 +1,6 @@ import type { Web5Agent } from '@web5/agent'; import type { Readable } from 'readable-stream'; import type { - RecordsReadReply, RecordsWriteMessage, RecordsWriteOptions, RecordsWriteDescriptor, @@ -191,12 +190,12 @@ export class Record implements RecordModel { // If neither of the above are true, then the record must be fetched from the DWN. this._readableStream = this._agent.processDwnRequest({ author : this.author, - messageOptions : { recordId: this.id }, + messageOptions : { filter: { recordId: this.id } }, messageType : DwnInterfaceName.Records + DwnMethodName.Read, target : this.target, }) - .then(response => response.reply as RecordsReadReply) - .then(reply => reply.record.data as Readable) + .then(response => response.reply) + .then(reply => reply.record.data) .catch(error => { throw new Error(`Error encountered while attempting to read data: ${error.message}`); }); } diff --git a/packages/api/tests/dwn-api.spec.ts b/packages/api/tests/dwn-api.spec.ts index 39a179067..68de357f7 100644 --- a/packages/api/tests/dwn-api.spec.ts +++ b/packages/api/tests/dwn-api.spec.ts @@ -392,6 +392,59 @@ describe('DwnApi', () => { expect(deleteResult.status.code).to.equal(401); expect(deleteResult.status.detail).to.include('message failed authorization'); }); + + it('deletes records that were authored/signed by another DID', async () => { + /** + * WHAT IS BEING TESTED? + * + * We are testing whether a record authored/signed by one party (Alice) can be written to + * another party's DWN (Bob), and that recipient (Bob) is able to delete the record. + * + * TEST SETUP STEPS: + * 1. Configure the email protocol on Bob's local DWN. + */ + const { status: bobProtocolStatus, protocol: bobProtocol } = await dwnBob.protocols.configure({ + message: { + definition: emailProtocolDefinition + } + }); + expect(bobProtocolStatus.code).to.equal(202); + /** + * 2. Configure the email protocol on Bob's remote DWN. + */ + const { status: bobRemoteProtocolStatus } = await bobProtocol.send(bobDid.did); + expect(bobRemoteProtocolStatus.code).to.equal(202); + /** + * 3. Alice creates a record, but doesn't store it locally. + */ + const { status: createStatus, record: testRecord} = await dwnAlice.records.create({ + store : false, + data : 'test', + message : { + protocol : 'http://email-protocol.xyz', + protocolPath : 'email', + schema : 'http://email-protocol.xyz/schema/email', + dataFormat : 'text/plain' + } + }); + expect(createStatus.code).to.equal(202); + expect(testRecord.author).to.equal(aliceDid.did); + /** + * 4. Alice writes the record to Bob's remote DWN. + */ + const { status: sendStatus } = await testRecord.send(bobDid.did); + expect(sendStatus.code).to.equal(202); + /** + * 5. Bob deletes the record from his remote DWN. + */ + const deleteResult = await dwnBob.records.delete({ + from : bobDid.did, + message : { + recordId: testRecord.id + } + }); + expect(deleteResult.status.code).to.equal(202); + }); }); }); diff --git a/packages/api/tests/record.spec.ts b/packages/api/tests/record.spec.ts index 0411e0010..0c13bcbfb 100644 --- a/packages/api/tests/record.spec.ts +++ b/packages/api/tests/record.spec.ts @@ -12,6 +12,7 @@ import chaiAsPromised from 'chai-as-promised'; import { utils as didUtils } from '@web5/dids'; import { TestManagedAgent } from '@web5/agent'; import { + DwnConstant, RecordsWrite, DwnMethodName, DwnInterfaceName, @@ -254,10 +255,10 @@ describe('Record', () => { it('returns large data payloads after dwn.records.write()', async () => { // Generate data that exceeds the DWN encoded data limit to ensure that the data will have to be fetched // with a RecordsRead when record.data.blob() is executed. - const dataJson = TestDataGenerator.randomJson(11_000); + const dataJson = TestDataGenerator.randomJson(DwnConstant.maxDataSizeAllowedToBeEncoded + 1000); const inputDataBytes = new TextEncoder().encode(JSON.stringify(dataJson)); - // Write the 11KB record to agent-connected DWN. + // Write the large record to agent-connected DWN. const { record, status } = await dwn.records.write({ data: dataJson }); expect(status.code).to.equal(202); @@ -271,13 +272,39 @@ describe('Record', () => { expect(readDataBytes).to.deep.equal(inputDataBytes); }); + it('returns large data payloads after dwn.records.query()', async () => { + /** Generate data that exceeds the DWN encoded data limit to ensure that the data will have to + * be fetched with a RecordsRead when record.data.blob() is executed. */ + const dataJson = TestDataGenerator.randomJson(DwnConstant.maxDataSizeAllowedToBeEncoded + 1000); + const inputDataBytes = new TextEncoder().encode(JSON.stringify(dataJson)); + + // Write the large record to agent-connected DWN. + const { record, status } = await dwn.records.write({ data: dataJson }); + expect(status.code).to.equal(202); + + // Query for the record that was just created. + const { records: queryRecords, status: queryRecordStatus } = await dwn.records.query({ + message: { filter: { recordId: record!.id }} + }); + expect(queryRecordStatus.code).to.equal(200); + + // Confirm that the size, in bytes, of the data read as a Blob matches the original input data. + const [ queryRecord ] = queryRecords; + const queriedDataBlob = await queryRecord.data.blob(); + expect(queriedDataBlob.size).to.equal(inputDataBytes.length); + + // Convert the Blob into an array and ensure it matches the input data, byte for byte. + const queriedDataBytes = new Uint8Array(await queriedDataBlob.arrayBuffer()); + expect(queriedDataBytes).to.deep.equal(inputDataBytes); + }); + it('returns large data payloads after dwn.records.read()', async () => { // Generate data that exceeds the DWN encoded data limit to ensure that the data will have to be fetched // with a RecordsRead when record.data.blob() is executed. - const dataJson = TestDataGenerator.randomJson(11_000); + const dataJson = TestDataGenerator.randomJson(DwnConstant.maxDataSizeAllowedToBeEncoded + 1000); const inputDataBytes = new TextEncoder().encode(JSON.stringify(dataJson)); - // Write the 11KB record to agent-connected DWN. + // Write the large record to agent-connected DWN. const { record, status } = await dwn.records.write({ data: dataJson }); expect(status.code).to.equal(202); @@ -346,10 +373,10 @@ describe('Record', () => { it('returns large data payloads after dwn.records.write()', async () => { // Generate data that exceeds the DWN encoded data limit to ensure that the data will have to be fetched // with a RecordsRead when record.data.json() is executed. - const dataJson = TestDataGenerator.randomJson(11_000); + const dataJson = TestDataGenerator.randomJson(DwnConstant.maxDataSizeAllowedToBeEncoded + 1000); const inputDataBytes = new TextEncoder().encode(JSON.stringify(dataJson)); - // Write the 11KB record to agent-connected DWN. + // Write the large record to agent-connected DWN. const { record, status } = await dwn.records.write({ data: dataJson }); expect(status.code).to.equal(202); @@ -363,13 +390,39 @@ describe('Record', () => { expect(readDataBytes).to.deep.equal(inputDataBytes); }); + it('returns large data payloads after dwn.records.query()', async () => { + /** Generate data that exceeds the DWN encoded data limit to ensure that the data will have to + * be fetched with a RecordsRead when record.data.json() is executed. */ + const dataJson = TestDataGenerator.randomJson(DwnConstant.maxDataSizeAllowedToBeEncoded + 1000); + const inputDataBytes = new TextEncoder().encode(JSON.stringify(dataJson)); + + // Write the large record to agent-connected DWN. + const { record, status } = await dwn.records.write({ data: dataJson }); + expect(status.code).to.equal(202); + + // Query for the record that was just created. + const { records: queryRecords, status: queryRecordStatus } = await dwn.records.query({ + message: { filter: { recordId: record!.id }} + }); + expect(queryRecordStatus.code).to.equal(200); + + // Confirm that the size, in bytes, of the data read as JSON matches the original input data. + const [ queryRecord ] = queryRecords; + const queriedDataBlob = await queryRecord!.data.json(); + + // Convert the JSON to bytes and ensure it matches the input data, byte for byte. + const queriedDataBytes = new TextEncoder().encode(JSON.stringify(queriedDataBlob)); + expect(queriedDataBytes.length).to.equal(inputDataBytes.length); + expect(queriedDataBytes).to.deep.equal(inputDataBytes); + }); + it('returns large data payloads after dwn.records.read()', async () => { // Generate data that exceeds the DWN encoded data limit to ensure that the data will have to be fetched // with a RecordsRead when record.data.json() is executed. - const dataJson = TestDataGenerator.randomJson(11_000); + const dataJson = TestDataGenerator.randomJson(DwnConstant.maxDataSizeAllowedToBeEncoded + 1000); const inputDataBytes = new TextEncoder().encode(JSON.stringify(dataJson)); - // Write the 11KB record to agent-connected DWN. + // Write the large record to agent-connected DWN. const { record, status } = await dwn.records.write({ data: dataJson }); expect(status.code).to.equal(202); @@ -434,9 +487,9 @@ describe('Record', () => { it('returns large data payloads after dwn.records.write()', async () => { // Generate data that exceeds the DWN encoded data limit to ensure that the data will have to be fetched // with a RecordsRead when record.data.text() is executed. - const dataText = TestDataGenerator.randomString(11_000); + const dataText = TestDataGenerator.randomString(DwnConstant.maxDataSizeAllowedToBeEncoded + 1000); - // Write the 11KB record to agent-connected DWN. + // Write the large record to agent-connected DWN. const { record, status } = await dwn.records.write({ data: dataText }); expect(status.code).to.equal(202); @@ -449,12 +502,36 @@ describe('Record', () => { expect(readDataText).to.deep.equal(dataText); }); + it('returns large data payloads after dwn.records.query()', async () => { + /** Generate data that exceeds the DWN encoded data limit to ensure that the data will have to + * be fetched with a RecordsRead when record.data.blob() is executed. */ + const dataText = TestDataGenerator.randomString(DwnConstant.maxDataSizeAllowedToBeEncoded + 1000); + + // Write the large record to agent-connected DWN. + const { record, status } = await dwn.records.write({ data: dataText }); + expect(status.code).to.equal(202); + + // Query for the record that was just created. + const { records: queryRecords, status: queryRecordStatus } = await dwn.records.query({ + message: { filter: { recordId: record!.id }} + }); + expect(queryRecordStatus.code).to.equal(200); + + // Confirm that the length of the data read as text matches the original input data. + const [ queryRecord ] = queryRecords; + const queriedDataText = await queryRecord!.data.text(); + expect(queriedDataText.length).to.equal(dataText.length); + + // Ensure the text returned matches the input data, char for char. + expect(queriedDataText).to.deep.equal(dataText); + }); + it('returns large data payloads after dwn.records.read()', async () => { // Generate data that exceeds the DWN encoded data limit to ensure that the data will have to be fetched // with a RecordsRead when record.data.text() is executed. - const dataText = TestDataGenerator.randomString(11_000); + const dataText = TestDataGenerator.randomString(DwnConstant.maxDataSizeAllowedToBeEncoded + 1000); - // Write the 11KB record to agent-connected DWN. + // Write the large record to agent-connected DWN. const { record, status } = await dwn.records.write({ data: dataText }); expect(status.code).to.equal(202);