Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated RecordsRead to return RecordsDelete and initial RecordsWrite when record is deleted #814

Merged
merged 3 commits into from
Oct 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;

LiranCohen marked this conversation as resolved.
Show resolved Hide resolved
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