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

Return 204 when there is no data #790

Merged
merged 9 commits into from
Aug 8, 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
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@
"lint": "eslint . --max-warnings 0",
"lint:fix": "eslint . --fix",
"circular-dependency-check": "depcruise src",
"test:node": "npm run compile-validators && tsc && c8 mocha \"dist/esm/tests/**/*.spec.js\"",
"test:node": "npm run compile-validators && tsc && c8 node --enable-source-maps node_modules/.bin/mocha \"dist/esm/tests/**/*.spec.js\"",
"test:browser": "npm run compile-validators && cross-env karma start karma.conf.cjs",
"test:browser-debug": "npm run compile-validators && cross-env karma start karma.conf.debug.cjs",
"license-check": "license-report --only=prod > license-report.json && node ./build/license-check.cjs",
Expand Down
7 changes: 6 additions & 1 deletion src/handlers/records-write.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,12 @@ export class RecordsWriteHandler implements MethodHandler {
}

const messageReply = {
status: { code: 202, detail: 'Accepted' }
// In order to discern between something that was accepted as a queryable write and something that was accepted
// as an initial state we use separate response codes. See https://github.com/TBD54566975/dwn-sdk-js/issues/695
// for more details.
status: (newMessageIsInitialWrite && dataStream === undefined) ?
andresuribe87 marked this conversation as resolved.
Show resolved Hide resolved
{ code: 204, detail: 'No Content' } :
{ code: 202, detail: 'Accepted' }
};

// delete all existing messages of the same record that are not newest, except for the initial write
Expand Down
2 changes: 1 addition & 1 deletion tests/handlers/messages-subscribe.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ export function testMessagesSubscribeHandler(): void {
}
});
const grantWriteReply = await dwn.processMessage(alice.did, grantWrite.message, { dataStream });
expect(grantWriteReply.status.code).to.equal(202);
expect(grantWriteReply.status.code).to.equal(204);


// bob attempts to use the `MessagesQuery` grant on an `MessagesSubscribe` message
Expand Down
18 changes: 10 additions & 8 deletions tests/handlers/records-write.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ export function testRecordsWriteHandler(): void {

// simulate synchronize of pruned initial `RecordsWrite`
const reply = await dwn.processMessage(alice.did, recordsWrite.message);
expect(reply.status.code).to.equal(202);
expect(reply.status.code).to.equal(204);

// verify `RecordsWrite` inserted is not returned with a query
const recordsQueryMessageData = await TestDataGenerator.generateRecordsQuery({
Expand Down Expand Up @@ -3009,6 +3009,7 @@ export function testRecordsWriteHandler(): void {

// bob learns of metadata (ie. dataCid) of alice's secret data,
// attempts to gain unauthorized access by writing to alice's DWN through open protocol referencing the dataCid without supplying the data
// which he is allowed to do, the DWN will treat the operation as an initial-write or a record that has a later and different state.
const imageRecordsWrite = await TestDataGenerator.generateRecordsWrite({
author : bob,
protocol,
Expand All @@ -3019,8 +3020,8 @@ export function testRecordsWriteHandler(): void {
dataSize,
recipient : alice.did
});
const imageReply = await dwn.processMessage(alice.did, imageRecordsWrite.message, { dataStream: imageRecordsWrite.dataStream });
expect(imageReply.status.code).to.equal(202); // allows write but is not readable or queryable
const imageReply = await dwn.processMessage(alice.did, imageRecordsWrite.message);
expect(imageReply.status.code).to.equal(204); // allows write but is not readable or queryable
thehenrytsai marked this conversation as resolved.
Show resolved Hide resolved

// verify the record is not able to be read
const bobRecordsReadData = await RecordsRead.create({
Expand Down Expand Up @@ -3101,6 +3102,7 @@ export function testRecordsWriteHandler(): void {

// bob learns of metadata (ie. dataCid) of alice's secret data,
// attempts to gain unauthorized access by writing to alice's DWN through open protocol referencing the dataCid without supplying the data
// which he is allowed to do, the DWN will treat the operation as an initial-write or a record that has a later and different state.
const imageRecordsWrite = await TestDataGenerator.generateRecordsWrite({
author : bob,
protocol,
Expand All @@ -3111,8 +3113,8 @@ export function testRecordsWriteHandler(): void {
dataSize,
recipient : alice.did
});
const imageReply = await dwn.processMessage(alice.did, imageRecordsWrite.message, { dataStream: imageRecordsWrite.dataStream });
expect(imageReply.status.code).to.equal(202); // allows write but is not readable or queryable
const imageReply = await dwn.processMessage(alice.did, imageRecordsWrite.message);
expect(imageReply.status.code).to.equal(204); // allows write but is not readable or queryable

// verify the record is not able to be read
const bobRecordsReadData = await RecordsRead.create({
Expand Down Expand Up @@ -4071,7 +4073,7 @@ export function testRecordsWriteHandler(): void {
data,
});
const prunedRecordsWriteReply = await dwn.processMessage(alice.did, prunedRecordsWrite.message);
expect(prunedRecordsWriteReply.status.code).to.equal(202);
expect(prunedRecordsWriteReply.status.code).to.equal(204);

// Update record to published, omitting dataStream
const recordsWrite = await TestDataGenerator.generateFromRecordsWrite({
Expand Down Expand Up @@ -4099,7 +4101,7 @@ export function testRecordsWriteHandler(): void {
data,
});
const prunedRecordsWriteReply = await dwn.processMessage(alice.did, prunedRecordsWrite.message);
expect(prunedRecordsWriteReply.status.code).to.equal(202);
expect(prunedRecordsWriteReply.status.code).to.equal(204);

// Update record to published, omitting dataStream
const recordsWrite = await TestDataGenerator.generateFromRecordsWrite({
Expand Down Expand Up @@ -4144,7 +4146,7 @@ export function testRecordsWriteHandler(): void {
dataSize : 4
});
const bobWriteReply = await dwn.processMessage(bob.did, bobWriteData.message); // intentionally missing data stream
expect(bobWriteReply.status.code).to.equal(202); // NOTE: allows write here but does not allow read or query later
expect(bobWriteReply.status.code).to.equal(204); // NOTE: allows write here but does not allow read or query later

const aliceQueryWriteAfterBobWriteData = await TestDataGenerator.generateRecordsQuery({
author : alice,
Expand Down
1 change: 1 addition & 0 deletions tests/utils/test-data-generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,7 @@ export class TestDataGenerator {

/**
* Generates a RecordsWrite message for testing.
* `dataBytes` & `dataStream` returned will be `undefined` as long as `dataCid` or `dataSize` is given.
* Implementation currently uses `RecordsWrite.create()`.
* @param input.attesters Attesters of the message. Will NOT be generated if not given.
* @param input.data Data that belongs to the record. Generated when not given only if `dataCid` and `dataSize` are also not given.
Expand Down
Loading