Skip to content

Commit

Permalink
Updated RecordsRead to return RecordsDelete and initial RecordsWrite …
Browse files Browse the repository at this point in the history
…when record is deleted (#814)

Prior to this change, a 404 status is returned without any data when a
RecordsRead is performed against a deleted record.

But 404 status alone prevents a the RecordsDelete from being imported
into the caller's DWN for the purpose updating the record's state.

This PR updated RecordsRead to return RecordsDelete and initial
RecordsWrite when record is deleted.

---------

Co-authored-by: Liran Cohen <[email protected]>
  • Loading branch information
thehenrytsai and LiranCohen authored Oct 4, 2024
1 parent 4261a46 commit d667090
Show file tree
Hide file tree
Showing 9 changed files with 542 additions and 83 deletions.
429 changes: 370 additions & 59 deletions package-lock.json

Large diffs are not rendered by default.

10 changes: 3 additions & 7 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@
"@types/varint": "6.0.0",
"@typescript-eslint/eslint-plugin": "^7.9.0",
"@typescript-eslint/parser": "^7.9.0",
"c8": "^8.0.0",
"c8": "^10.1.2",
"chai": "4.3.6",
"chai-as-promised": "7.1.1",
"cross-env": "7.0.3",
Expand All @@ -116,7 +116,7 @@
"eslint-plugin-todo-plz": "1.3.0",
"events": "3.3.0",
"istanbul-badges-readme": "1.8.1",
"karma": "6.4.1",
"karma": "^6.4.4",
"karma-chai": "0.1.0",
"karma-chrome-launcher": "3.1.1",
"karma-esbuild": "2.2.5",
Expand All @@ -138,11 +138,7 @@
"util": "0.12.4"
},
"overrides": {
"c8": {
"istanbul-lib-report": {
"make-dir": "^4.0.0"
}
},
"cookie": "^0.7.1",
"@typescript-eslint/eslint-plugin": {
"eslint": "^9.2.0"
}
Expand Down
19 changes: 16 additions & 3 deletions src/handlers/records-read.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type { DidResolver } from '@web5/dids';
import type { Filter } from '../types/query-types.js';
import type { MessageStore } from '../types//message-store.js';
import type { MethodHandler } from '../types/method-handler.js';
import type { RecordsQueryReplyEntry, RecordsReadMessage, RecordsReadReply } from '../types/records-types.js';
import type { RecordsDeleteMessage, RecordsQueryReplyEntry, RecordsReadMessage, RecordsReadReply } from '../types/records-types.js';

import { authenticate } from '../core/auth.js';
import { DataStream } from '../utils/data-stream.js';
Expand Down Expand Up @@ -45,8 +45,8 @@ export class RecordsReadHandler implements MethodHandler {
}

// get the latest active messages matching the supplied filter
// only RecordsWrite messages will be returned due to 'isLatestBaseState' being set to true.
const query: Filter = {
// NOTE: we don't filter by `method` so that we get both RecordsWrite and RecordsDelete messages
interface : DwnInterfaceName.Records,
isLatestBaseState : true,
...Records.convertFilter(message.descriptor.filter)
Expand All @@ -63,7 +63,20 @@ export class RecordsReadHandler implements MethodHandler {
), 400);
}

const matchedRecordsWrite = existingMessages[0] as RecordsQueryReplyEntry;
const matchedMessage = existingMessages[0];

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,
initialWrite
};
}

const matchedRecordsWrite = matchedMessage as RecordsQueryReplyEntry;

try {
await RecordsReadHandler.authorizeRecordsRead(tenant, recordsRead, await RecordsWrite.parse(matchedRecordsWrite), this.messageStore);
} catch (error) {
Expand Down
10 changes: 3 additions & 7 deletions src/interfaces/records-delete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,15 +87,11 @@ export class RecordsDelete extends AbstractMessage<RecordsDeleteMessage> {
// we add the immutable properties from the initial RecordsWrite message in order to use them when querying relevant deletes.
const { protocol, protocolPath, recipient, schema, parentId, dateCreated } = initialWrite.descriptor;

// NOTE: the "trick" not may not be apparent on how a query is able to omit deleted records:
// we intentionally not add index for `isLatestBaseState` at all, this means that upon a successful delete,
// no messages with the record ID will match any query because queries by design filter by `isLatestBaseState = true`,
// `isLatestBaseState` for the initial delete would have been toggled to `false`
const indexes: { [key:string]: string | boolean | undefined } = {
// isLatestBaseState : "true", // intentionally showing that this index is omitted
isLatestBaseState : true,
protocol, protocolPath, recipient, schema, parentId, dateCreated,
contextId : initialWrite.contextId,
author : this.author!,
contextId : initialWrite.contextId,
author : this.author!,
...descriptor
};
removeUndefinedProperties(indexes);
Expand Down
23 changes: 20 additions & 3 deletions src/interfaces/records-write.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1034,22 +1034,39 @@ export class RecordsWrite implements MessageInterface<RecordsWriteMessage> {

/**
* Fetches the initial RecordsWrite of a record.
* @returns The initial RecordsWrite if found; `undefined` if the record is not found.
* @returns The initial RecordsWrite if found; `undefined` otherwise.
*/
public static async fetchInitialRecordsWrite(
messageStore: MessageStore,
tenant: string,
recordId: string
): Promise<RecordsWrite | undefined> {

const initialRecordsWriteMessage = await RecordsWrite.fetchInitialRecordsWriteMessage(messageStore, tenant, recordId);
if (initialRecordsWriteMessage === undefined) {
return undefined;
}

const initialRecordsWrite = await RecordsWrite.parse(initialRecordsWriteMessage);
return initialRecordsWrite;
}

/**
* Fetches the initial RecordsWrite message of a record.
* @returns The initial RecordsWriteMessage if found; `undefined` otherwise.
*/
public static async fetchInitialRecordsWriteMessage(
messageStore: MessageStore,
tenant: string,
recordId: string
): Promise<RecordsWriteMessage | undefined> {
const query = { entryId: recordId };
const { messages } = await messageStore.query(tenant, [query]);

if (messages.length === 0) {
return undefined;
}

const initialRecordsWrite = await RecordsWrite.parse(messages[0] as RecordsWriteMessage);
return initialRecordsWrite;
return messages[0] as RecordsWriteMessage;
}
}
13 changes: 10 additions & 3 deletions src/types/records-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,10 +203,17 @@ export type RecordsReadMessage = {
};

export type RecordsReadReply = GenericMessageReply & {
/**
* The RecordsDelete if the record is deleted.
*/
delete?: 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 & {
/**
* The initial write of the record if the returned RecordsWrite message itself is not the initial write.
*/
initialWrite?: RecordsWriteMessage;
data: Readable;
};
Expand Down
117 changes: 117 additions & 0 deletions tests/scenarios/deleted-record.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
import type { DidResolver } from '@web5/dids';
import type { EventStream } from '../../src/types/subscriptions.js';
import type { DataStore, EventLog, MessageStore, ResumableTaskStore } from '../../src/index.js';

import chaiAsPromised from 'chai-as-promised';
import freeForAllProtocolDefinition from '../vectors/protocol-definitions/free-for-all.json' assert { type: 'json' };
import sinon from 'sinon';

import { TestDataGenerator } from '../utils/test-data-generator.js';
import { TestEventStream } from '../test-event-stream.js';
import { TestStores } from '../test-stores.js';
import { TestStubGenerator } from '../utils/test-stub-generator.js';

import chai, { expect } from 'chai';
import { DataStream, Dwn, Jws, ProtocolsConfigure, RecordsRead } from '../../src/index.js';
import { DidKey, UniversalResolver } from '@web5/dids';
import { Encoder, RecordsDelete, RecordsWrite } from '../../src/index.js';

chai.use(chaiAsPromised);

export function testDeletedRecordScenarios(): void {
describe('End-to-end Scenarios Spanning Features', () => {
let didResolver: DidResolver;
let messageStore: MessageStore;
let dataStore: DataStore;
let resumableTaskStore: ResumableTaskStore;
let eventLog: EventLog;
let eventStream: EventStream;
let dwn: Dwn;

// important to follow the `before` and `after` pattern to initialize and clean the stores in tests
// so that different test suites can reuse the same backend store for testing
before(async () => {
didResolver = new UniversalResolver({ didResolvers: [DidKey] });

const stores = TestStores.get();
messageStore = stores.messageStore;
dataStore = stores.dataStore;
resumableTaskStore = stores.resumableTaskStore;
eventLog = stores.eventLog;
eventStream = TestEventStream.get();

dwn = await Dwn.create({ didResolver, messageStore, dataStore, eventLog, eventStream, resumableTaskStore });
});

beforeEach(async () => {
sinon.restore(); // wipe all previous stubs/spies/mocks/fakes

// clean up before each test rather than after so that a test does not depend on other tests to do the clean up
await messageStore.clear();
await dataStore.clear();
await resumableTaskStore.clear();
await eventLog.clear();
});

after(async () => {
await dwn.close();
});

it('should return the RecordsDelete and initial RecordsWrite when reading a deleted record', async () => {
// Scenario:
// 1. Alice deletes an existing record.
// 2. Alice attempts to read the deleted record.
// Expected outcome: Alice should get a 404 error with the reply containing the deleted record and the initial write of the record.

// 0. Setting up a protocol and write a record
const alice = await TestDataGenerator.generatePersona();
TestStubGenerator.stubDidResolver(didResolver, [alice]);

const protocolDefinition = freeForAllProtocolDefinition;
const protocolsConfigure = await ProtocolsConfigure.create({
signer : Jws.createSigner(alice),
definition : protocolDefinition
});
const protocolsConfigureForAliceReply = await dwn.processMessage(
alice.did,
protocolsConfigure.message
);
expect(protocolsConfigureForAliceReply.status.code).to.equal(202);

const data = Encoder.stringToBytes('some post content');
const { message: recordsWriteMessage } = await RecordsWrite.create({
signer : Jws.createSigner(alice),
protocol : protocolDefinition.protocol,
protocolPath : 'post',
schema : protocolDefinition.types.post.schema,
dataFormat : protocolDefinition.types.post.dataFormats[0],
data,
});
const writeReply = await dwn.processMessage(alice.did, recordsWriteMessage, { dataStream: DataStream.fromBytes(data) });
expect(writeReply.status.code).to.equal(202);

// 1. Alice deletes an existing record.
const recordsDelete = await RecordsDelete.create({
signer : Jws.createSigner(alice),
recordId : recordsWriteMessage.recordId
});

const deleteReply = await dwn.processMessage(alice.did, recordsDelete.message);
expect(deleteReply.status.code).to.equal(202);

// 2. Alice attempts to read the deleted record.
const readData = await RecordsRead.create({
signer : Jws.createSigner(alice),
filter : { recordId: recordsWriteMessage.recordId }
});
const readReply = await dwn.processMessage(alice.did, readData.message);

// Expected outcome: Alice should get a 404 error with the reply containing the deleted record and the initial write of the record.
expect(readReply.status.code).to.equal(404);
expect(readReply.delete).to.exist;
expect(readReply.delete).to.deep.equal(recordsDelete.message);
expect(readReply.initialWrite).to.exist;
expect(readReply.initialWrite).to.deep.equal(recordsWriteMessage);
});
});
}
2 changes: 2 additions & 0 deletions tests/test-suite.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { DataStore, EventLog, EventStream, MessageStore, ResumableTaskStore } from '../src/index.js';

import { testAuthorDelegatedGrant } from './features/author-delegated-grant.spec.js';
import { testDeletedRecordScenarios } from './scenarios/deleted-record.spec.js';
import { testDwnClass } from './dwn.spec.js';
import { testEndToEndScenarios } from './scenarios/end-to-end-tests.spec.js';
import { testEventLog } from './event-log/event-log.spec.js';
Expand Down Expand Up @@ -83,6 +84,7 @@ export class TestSuite {
testResumableTasks();

// scenario tests
testDeletedRecordScenarios();
testEndToEndScenarios();
testMessagesQueryScenarios();
testNestedRoleScenarios();
Expand Down
2 changes: 1 addition & 1 deletion tests/vectors/protocol-definitions/free-for-all.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"published": true,
"types": {
"post": {
"schema": "eph",
"schema": "post",
"dataFormats": [
"application/json"
]
Expand Down

0 comments on commit d667090

Please sign in to comment.