Refactored reply structure to a RecordsRead (#817)

Currently the [reply structure of a `RecordsRead` is quite messy: (duplicated properties, data stream nested in a message, inconsistent naming etc).

This is an attempt at cleaning it up with a flatter structure.

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@tbd54566975/dwn-sdk-js",
-  "version": "0.4.7",
+  "version": "0.5.0",
   "description": "A reference implementation of https://identity.foundation/decentralized-web-node/spec/",
   "repository": {
     "type": "git",

src/handlers/records-read.ts

@@ -65,16 +65,20 @@ export class RecordsReadHandler implements MethodHandler {
 
     const matchedMessage = existingMessages[0];
 
+    // if the matched message is a RecordsDelete, we mark the record as not-found and return both the RecordsDelete and the initial RecordsWrite
+    // TODO: https://github.com/TBD54566975/dwn-sdk-js/issues/819:
+    // Consider performing authorization checks like when records exists before returning RecordsDelete and initial RecordsWrite of a deleted record
     if (matchedMessage.descriptor.method === DwnMethodName.Delete) {
       const recordsDeleteMessage = matchedMessage as RecordsDeleteMessage;
       const initialWrite = await RecordsWrite.fetchInitialRecordsWriteMessage(this.messageStore, tenant, recordsDeleteMessage.descriptor.recordId);
       return {
-        status : { code: 404, detail: 'Not Found' },
-        delete : recordsDeleteMessage,
+        status        : { code: 404, detail: 'Not Found' },
+        recordsDelete : recordsDeleteMessage,
         initialWrite
       };
     }
 
+    // else the matched message is a RecordsWrite
     const matchedRecordsWrite = matchedMessage as RecordsQueryReplyEntry;
 
     try {
@@ -98,27 +102,24 @@ export class RecordsReadHandler implements MethodHandler {
       data = result.dataStream;
     }
 
-    const record = {
-      ...matchedRecordsWrite,
+    const recordsReadReply: RecordsReadReply = {
+      status       : { code: 200, detail: 'OK' },
+      recordsWrite : matchedRecordsWrite,
       data
     };
 
-    // attach initial write if returned RecordsWrite is not initial write
-    if (!await RecordsWrite.isInitialWrite(record)) {
+    // attach initial write if latest RecordsWrite is not initial write
+    if (!await RecordsWrite.isInitialWrite(matchedRecordsWrite)) {
       const initialWriteQueryResult = await this.messageStore.query(
         tenant,
-        [{ recordId: record.recordId, isLatestBaseState: false, method: DwnMethodName.Write }]
+        [{ recordId: matchedRecordsWrite.recordId, isLatestBaseState: false, method: DwnMethodName.Write }]
       );
       const initialWrite = initialWriteQueryResult.messages[0] as RecordsQueryReplyEntry;
-      delete initialWrite.encodedData; // defensive measure but technically optional because we do this when an update RecordsWrite takes place
-      record.initialWrite = initialWrite;
+      delete initialWrite.encodedData; // just defensive because technically should already be deleted when a later RecordsWrite is written
+      recordsReadReply.initialWrite = initialWrite;
     }
 
-    const messageReply: RecordsReadReply = {
-      status: { code: 200, detail: 'OK' },
-      record
-    };
-    return messageReply;
+    return recordsReadReply;
   };
 
   /**

src/types/records-types.ts

@@ -202,21 +202,29 @@ export type RecordsReadMessage = {
   descriptor: RecordsReadDescriptor;
 };
 
+/**
+ * The reply to a RecordsRead message.
+ */
 export type RecordsReadReply = GenericMessageReply & {
+  /**
+   * The latest RecordsWrite message of the record if record exists (not deleted).
+   */
+  recordsWrite?: RecordsWriteMessage;
+
   /**
    * The RecordsDelete if the record is deleted.
    */
-  delete?: RecordsDeleteMessage;
+  recordsDelete?: RecordsDeleteMessage;
 
   /**
    * The initial write of the record if the returned RecordsWrite message itself is not the initial write or if a RecordsDelete is returned.
    */
   initialWrite?: RecordsWriteMessage;
 
-  record?: RecordsWriteMessage & {
-    initialWrite?: RecordsWriteMessage;
-    data: Readable;
-  };
+  /**
+   * The data stream associated with the record if the records exists (not deleted).
+   */
+  data?: Readable;
 };
 
 export type RecordsReadDescriptor = {

tests/features/author-delegated-grant.spec.ts

@@ -505,7 +505,7 @@ export function testAuthorDelegatedGrant(): void {
       });
       const deviceXRecordsReadReply = await dwn.processMessage(bob.did, recordsReadByDeviceX.message);
       expect(deviceXRecordsReadReply.status.code).to.equal(200);
-      expect(deviceXRecordsReadReply.record?.recordId).to.equal(chatRecord.message.recordId);
+      expect(deviceXRecordsReadReply.recordsWrite?.recordId).to.equal(chatRecord.message.recordId);
 
       // Verify that Carol cannot query as Alice by invoking the delegated grant granted to Device X
       const recordsQueryByCarol = await RecordsQuery.create({

tests/features/owner-signature.spec.ts

@@ -78,26 +78,26 @@ export function testOwnerSignature(): void {
 
       const readReply = await dwn.processMessage(bob.did, recordsRead.message);
       expect(readReply.status.code).to.equal(200);
-      expect(readReply.record).to.exist;
-      expect(readReply.record?.descriptor).to.exist;
+      expect(readReply.recordsWrite).to.exist;
+      expect(readReply.recordsWrite?.descriptor).to.exist;
 
       // Alice augments Bob's message as an external owner
-      const { data, ...messageFetched } = readReply.record!; // remove data from message
-      const ownerSignedMessage = await RecordsWrite.parse(messageFetched);
+      const { recordsWrite } = readReply; // remove data from message
+      const ownerSignedMessage = await RecordsWrite.parse(recordsWrite!);
       await ownerSignedMessage.signAsOwner(Jws.createSigner(alice));
 
       // Test that Alice can successfully retain/write Bob's message to her DWN
-      const aliceDataStream = readReply.record!.data;
+      const aliceDataStream = readReply.data!;
       const aliceWriteReply = await dwn.processMessage(alice.did, ownerSignedMessage.message, { dataStream: aliceDataStream });
       expect(aliceWriteReply.status.code).to.equal(202);
 
       // Test that Bob's message can be read from Alice's DWN
       const readReply2 = await dwn.processMessage(alice.did, recordsRead.message);
       expect(readReply2.status.code).to.equal(200);
-      expect(readReply2.record).to.exist;
-      expect(readReply2.record?.descriptor).to.exist;
+      expect(readReply2.recordsWrite).to.exist;
+      expect(readReply2.recordsWrite?.descriptor).to.exist;
 
-      const dataFetched = await DataStream.toBytes(readReply2.record!.data!);
+      const dataFetched = await DataStream.toBytes(readReply2.data!);
       expect(ArrayUtility.byteArraysEqual(dataFetched, dataBytes!)).to.be.true;
     });
 
@@ -144,10 +144,10 @@ export function testOwnerSignature(): void {
       });
       const readReply = await dwn.processMessage(alice.did, recordsRead.message);
       expect(readReply.status.code).to.equal(200);
-      expect(readReply.record).to.exist;
-      expect(readReply.record?.descriptor).to.exist;
+      expect(readReply.recordsWrite).to.exist;
+      expect(readReply.recordsWrite?.descriptor).to.exist;
 
-      const dataFetched = await DataStream.toBytes(readReply.record!.data!);
+      const dataFetched = await DataStream.toBytes(readReply.data!);
       expect(ArrayUtility.byteArraysEqual(dataFetched, bobRecordsWrite.dataBytes!)).to.be.true;
     });
 

tests/features/permissions.spec.ts

@@ -345,7 +345,7 @@ export function testPermissions(): void {
       // 9. Verify that any third-party can fetch the revocation status of the permission grant
       const revocationReadReply2 = await dwn.processMessage(alice.did, revocationRead.message);
       expect(revocationReadReply2.status.code).to.equal(200);
-      expect(revocationReadReply2.record?.recordId).to.equal(revokeWrite.recordsWrite.message.recordId);
+      expect(revocationReadReply2.recordsWrite?.recordId).to.equal(revokeWrite.recordsWrite.message.recordId);
     });
 
     it('should fail if a RecordsPermissionScope in a Request or Grant record is created without a protocol', async () => {

tests/features/protocol-update-action.spec.ts

@@ -165,8 +165,8 @@ export function testProtocolUpdateAction(): void {
       });
       const recordsReadReply = await dwn.processMessage(alice.did, recordsRead.message);
       expect(recordsReadReply.status.code).to.equal(200);
-      expect(recordsReadReply.record?.data).to.exist;
-      const dataFromReply = await DataStream.toBytes(recordsReadReply.record!.data);
+      expect(recordsReadReply.data).to.exist;
+      const dataFromReply = await DataStream.toBytes(recordsReadReply.data!);
       expect(dataFromReply).to.eql(bobFooNewBytes);
 
       // 5. Carol creates a `foo` by invoking the user role.
@@ -350,8 +350,8 @@ export function testProtocolUpdateAction(): void {
       });
       const bobBarReadReply = await dwn.processMessage(alice.did, bobBarRead.message);
       expect(bobBarReadReply.status.code).to.equal(200);
-      expect(bobBarReadReply.record?.data).to.exist;
-      const dataFromBobBarRead = await DataStream.toBytes(bobBarReadReply.record!.data);
+      expect(bobBarReadReply.data).to.exist;
+      const dataFromBobBarRead = await DataStream.toBytes(bobBarReadReply.data!);
       expect(dataFromBobBarRead).to.eql(bobBarNewBytes);
 
       // 7. Verify that Bob cannot update Carol's `bar`.
@@ -423,8 +423,8 @@ export function testProtocolUpdateAction(): void {
       });
       const bobBazReadReply = await dwn.processMessage(alice.did, bobBazRead.message);
       expect(bobBazReadReply.status.code).to.equal(200);
-      expect(bobBazReadReply.record?.data).to.exist;
-      const dataFromBobBazRead = await DataStream.toBytes(bobBazReadReply.record!.data);
+      expect(bobBazReadReply.data).to.exist;
+      const dataFromBobBazRead = await DataStream.toBytes(bobBazReadReply.data!);
       expect(dataFromBobBazRead).to.eql(bobBazNewBytes);
 
       // 11. Verify that Bob cannot update Carol's `baz`
@@ -534,8 +534,8 @@ export function testProtocolUpdateAction(): void {
       });
       const bobFooReadReply = await dwn.processMessage(alice.did, bobBarRead.message);
       expect(bobFooReadReply.status.code).to.equal(200);
-      expect(bobFooReadReply.record?.data).to.exist;
-      const dataFromBobFooRead = await DataStream.toBytes(bobFooReadReply.record!.data);
+      expect(bobFooReadReply.data).to.exist;
+      const dataFromBobFooRead = await DataStream.toBytes(bobFooReadReply.data!);
       expect(dataFromBobFooRead).to.eql(bobFooNewBytes);
 
       // 5. Verify that Bob cannot update Carol's `foo`.

tests/features/records-tags.spec.ts

@@ -2079,8 +2079,9 @@ export function testRecordsTags(): void {
 
         const tagsRecord1ReadReply = await dwn.processMessage(alice.did, tagsRecord1Read.message);
         expect(tagsRecord1ReadReply.status.code).to.equal(200);
-        expect(tagsRecord1ReadReply.record).to.not.be.undefined;
-        expect(tagsRecord1ReadReply.record!.descriptor.tags).to.deep.equal({ stringTag, numberTag, booleanTag, stringArrayTag, numberArrayTag });
+        expect(tagsRecord1ReadReply.recordsWrite).to.not.be.undefined;
+        expect(tagsRecord1ReadReply.recordsWrite!.descriptor.tags)
+          .to.deep.equal({ stringTag, numberTag, booleanTag, stringArrayTag, numberArrayTag });
       });
 
       it('should overwrite tags when updating a Record', async () => {
@@ -2113,8 +2114,8 @@ export function testRecordsTags(): void {
 
         const tagsRecord1ReadReply = await dwn.processMessage(alice.did, tagsRecord1Read.message);
         expect(tagsRecord1ReadReply.status.code).to.equal(200);
-        expect(tagsRecord1ReadReply.record).to.not.be.undefined;
-        expect(tagsRecord1ReadReply.record!.descriptor.tags).to.deep.equal({
+        expect(tagsRecord1ReadReply.recordsWrite).to.not.be.undefined;
+        expect(tagsRecord1ReadReply.recordsWrite!.descriptor.tags).to.deep.equal({
           stringTag      : 'string-value',
           numberTag      : 54566975,
           booleanTag     : false,
@@ -2148,8 +2149,8 @@ export function testRecordsTags(): void {
 
         const updatedRecordReadReply = await dwn.processMessage(alice.did, tagsRecord1Read.message);
         expect(updatedRecordReadReply.status.code).to.equal(200);
-        expect(updatedRecordReadReply.record).to.not.be.undefined;
-        expect(updatedRecordReadReply.record!.descriptor.tags).to.deep.equal({ newTag: 'new-value' });
+        expect(updatedRecordReadReply.recordsWrite).to.exist;
+        expect(updatedRecordReadReply.recordsWrite!.descriptor.tags).to.deep.equal({ newTag: 'new-value' });
 
         // Sanity: Query for the old tag value should return no results
         const tagsQueryMatchReply2 = await dwn.processMessage(alice.did, tagsQueryMatch.message);

tests/features/resumable-tasks.spec.ts

@@ -138,7 +138,7 @@ export function testResumableTasks(): void {
 
       const readReply = await dwn.processMessage(alice.did, recordsRead.message);
       expect(readReply.status.code).to.equal(200);
-      expect(readReply.record).to.exist;
+      expect(readReply.recordsWrite).to.exist;
 
       // 3. Restart the DWN to trigger the resumable task to be resumed.
       await dwn.close();
@@ -147,7 +147,7 @@ export function testResumableTasks(): void {
       // 4. Verify that the record is deleted.
       const readReply2 = await dwn.processMessage(alice.did, recordsRead.message);
       expect(readReply2.status.code).to.equal(404);
-      expect(readReply2.record).to.be.undefined;
+      expect(readReply2.recordsWrite).to.be.undefined;
     });
 
     it('should only resume tasks that are timed-out up to the batch size when DWN starts', async () => {

tests/handlers/records-delete.spec.ts

@@ -168,7 +168,7 @@ export function testRecordsDeleteHandler(): void {
         const aliceRead1Reply = await dwn.processMessage(alice.did, aliceRead1.message);
         expect(aliceRead1Reply.status.code).to.equal(200);
 
-        const aliceDataFetched = await DataStream.toBytes(aliceRead1Reply.record!.data!);
+        const aliceDataFetched = await DataStream.toBytes(aliceRead1Reply.data!);
         expect(ArrayUtility.byteArraysEqual(aliceDataFetched, data)).to.be.true;
 
         // alice deletes the other record
@@ -194,7 +194,7 @@ export function testRecordsDeleteHandler(): void {
         const bobRead1Reply = await dwn.processMessage(bob.did, bobRead1.message);
         expect(bobRead1Reply.status.code).to.equal(200);
 
-        const bobDataFetched = await DataStream.toBytes(bobRead1Reply.record!.data!);
+        const bobDataFetched = await DataStream.toBytes(bobRead1Reply.data!);
         expect(ArrayUtility.byteArraysEqual(bobDataFetched, data)).to.be.true;
       });