#365 - RecordsDelete should return 404 if record does not exist or is already deleted

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@tbd54566975/dwn-sdk-js",
-  "version": "0.0.31",
+  "version": "0.0.32",
   "description": "A reference implementation of https://identity.foundation/decentralized-web-node/spec/",
   "type": "module",
   "types": "./dist/types/src/index.d.ts",
@@ -144,4 +144,4 @@
     "url": "https://github.com/TBD54566975/dwn-sdk-js/issues"
   },
   "homepage": "https://github.com/TBD54566975/dwn-sdk-js#readme"
-}
+}

src/interfaces/records/handlers/records-delete.ts

@@ -9,7 +9,7 @@ import { deleteAllOlderMessagesButKeepInitialWrite } from '../records-interface.
 import { MessageReply } from '../../../core/message-reply.js';
 import { RecordsDelete } from '../messages/records-delete.js';
 import { RecordsWrite } from '../messages/records-write.js';
-import { DwnInterfaceName, Message } from '../../../core/message.js';
+import { DwnInterfaceName, DwnMethodName, Message } from '../../../core/message.js';
 
 export class RecordsDeleteHandler implements MethodHandler {
 
@@ -54,28 +54,31 @@ export class RecordsDeleteHandler implements MethodHandler {
       newestMessage = newestExistingMessage;
     }
 
-    // write the incoming message to DB if incoming message is newest
-    let messageReply: MessageReply;
-    if (incomingMessageIsNewest) {
-      const indexes = await constructIndexes(tenant, recordsDelete);
-
-      await this.messageStore.put(tenant, message, indexes);
-
-      const messageCid = await Message.getCid(message);
-      await this.eventLog.append(tenant, messageCid);
-
-      messageReply = new MessageReply({
-        status: { code: 202, detail: 'Accepted' }
-      });
-    } else {
-      messageReply = new MessageReply({
+    if (!incomingMessageIsNewest) {
+      return new MessageReply({
         status: { code: 409, detail: 'Conflict' }
       });
     }
 
+    // return Not Found if record does not exist or is already deleted
+    if (newestExistingMessage === undefined || newestExistingMessage.descriptor.method === DwnMethodName.Delete) {
+      return new MessageReply({
+        status: { code: 404, detail: 'Not Found' }
+      });
+    }
+
+    const indexes = await constructIndexes(tenant, recordsDelete);
+    await this.messageStore.put(tenant, message, indexes);
+
+    const messageCid = await Message.getCid(message);
+    await this.eventLog.append(tenant, messageCid);
+
     // delete all existing messages that are not newest, except for the initial write
     await deleteAllOlderMessagesButKeepInitialWrite(tenant, existingMessages, newestMessage, this.messageStore, this.dataStore, this.eventLog);
 
+    const messageReply = new MessageReply({
+      status: { code: 202, detail: 'Accepted' }
+    });
     return messageReply;
   };
 }

tests/interfaces/records/handlers/records-delete.spec.ts

@@ -58,7 +58,7 @@ describe('RecordsDeleteHandler.handle()', () => {
       await dwn.close();
     });
 
-    it('should handle RecordsDelete successfully', async () => {
+    it('should handle RecordsDelete successfully and return 404 if deleting a deleted record', async () => {
       const alice = await DidKeyResolver.generate();
 
       // insert data
@@ -89,6 +89,28 @@ describe('RecordsDeleteHandler.handle()', () => {
       const reply2 = await dwn.processMessage(alice.did, queryData.message);
       expect(reply2.status.code).to.equal(200);
       expect(reply2.entries?.length).to.equal(0);
+
+      // testing deleting a deleted record
+      const recordsDelete2 = await RecordsDelete.create({
+        recordId                    : message.recordId,
+        authorizationSignatureInput : Jws.createSignatureInput(alice)
+      });
+
+      const recordsDelete2Reply = await dwn.processMessage(alice.did, recordsDelete2.message);
+      expect(recordsDelete2Reply.status.code).to.equal(404);
+    });
+
+    it('should return 404 if deleting a non-existent record', async () => {
+      const alice = await DidKeyResolver.generate();
+
+      // testing deleting a non-existent record
+      const recordsDelete = await RecordsDelete.create({
+        recordId                    : 'nonExistentRecordId',
+        authorizationSignatureInput : Jws.createSignatureInput(alice)
+      });
+
+      const deleteReply = await dwn.processMessage(alice.did, recordsDelete.message);
+      expect(deleteReply.status.code).to.equal(404);
     });
 
     it('should be disallowed if there is a newer RecordsWrite already in the DWN ', async () => {
@@ -130,7 +152,7 @@ describe('RecordsDeleteHandler.handle()', () => {
       expect(reply.entries![0].encodedData).to.equal(expectedEncodedData);
     });
 
-    it('should be able to delete and rewrite the same data', async () => {
+    it('should be able to delete then rewrite the same data', async () => {
       const alice = await DidKeyResolver.generate();
       const data = Encoder.stringToBytes('test');
       const dataCid = await Cid.computeDagPbCidFromBytes(data);