From 0ef2c092c3c1a623ecc33599d4b0665ecd77d786 Mon Sep 17 00:00:00 2001 From: Henry Tsai Date: Fri, 1 Mar 2024 10:15:52 -0800 Subject: [PATCH] #690 - Allowed `dataFormat` to be modified (#697) --- Q_AND_A.md | 9 ++ src/interfaces/records-delete.ts | 4 +- src/interfaces/records-write.ts | 10 +- src/utils/hd-key.ts | 1 + tests/handlers/records-write.spec.ts | 148 ++++++++++++++---- tests/scenarios/subscriptions.spec.ts | 25 ++- .../protocol-definitions/social-media.json | 6 +- 7 files changed, 151 insertions(+), 52 deletions(-) diff --git a/Q_AND_A.md b/Q_AND_A.md index 69708cb5b..652dee314 100644 --- a/Q_AND_A.md +++ b/Q_AND_A.md @@ -117,6 +117,15 @@ This will be addressed in a future upgrade and we've created an issue to track it. https://github.com/TBD54566975/dwn-sdk-js/issues/668 - last updated (2024/01/22) +- Why are we not notifying deletes in a subscription that uses mutable property as a filter (e.g. `published`, `dataFormat`). + + (Last update: 2024/02/29) + + We are happy to revisit but the current behavior is due to the following arguments: + + - Philosophical argument: the subscription filter is subscribing to events/messages that matches the filter, not changes to an earlier state. + - Practical argument: this requires more thought and coding and is a lower priority until we have real-world usage and feedback. + ## Data Store - Is it possible to implement the Data Store interface purely using a blob/binary data storage service such as Amazon S3, Azure Blob Storage, or Google Cloud Storage? diff --git a/src/interfaces/records-delete.ts b/src/interfaces/records-delete.ts index 702cbee0f..82be10fdc 100644 --- a/src/interfaces/records-delete.ts +++ b/src/interfaces/records-delete.ts @@ -79,7 +79,7 @@ export class RecordsDelete extends AbstractMessage { const descriptor = { ...message.descriptor }; // 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, dataFormat, dateCreated } = initialWrite.descriptor; + 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, @@ -87,7 +87,7 @@ export class RecordsDelete extends AbstractMessage { // `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 - protocol, protocolPath, recipient, schema, parentId, dataFormat, dateCreated, + protocol, protocolPath, recipient, schema, parentId, dateCreated, contextId : initialWrite.contextId, author : this.author!, ...descriptor diff --git a/src/interfaces/records-write.ts b/src/interfaces/records-write.ts index a1e3a49be..3f8c574f9 100644 --- a/src/interfaces/records-write.ts +++ b/src/interfaces/records-write.ts @@ -126,6 +126,12 @@ export type KeyEncryptionInput = { export type CreateFromOptions = { recordsWriteMessage: RecordsWriteMessage, data?: Uint8Array; + + /** + * The data format of the new data. If not given, the data format from the existing message will be used. + */ + dataFormat?: string; + published?: boolean; messageTimestamp?: string; datePublished?: string; @@ -400,7 +406,6 @@ export class RecordsWrite implements MessageInterface { protocol : sourceMessage.descriptor.protocol, protocolPath : sourceMessage.descriptor.protocolPath, schema : sourceMessage.descriptor.schema, - dataFormat : sourceMessage.descriptor.dataFormat, parentContextId : Records.getParentContextFromOfContextId(sourceMessage.contextId), // mutable properties below messageTimestamp : options.messageTimestamp ?? currentTime, @@ -409,6 +414,7 @@ export class RecordsWrite implements MessageInterface { data : options.data, dataCid : options.data ? undefined : sourceMessage.descriptor.dataCid, // if data not given, use base message dataCid dataSize : options.data ? undefined : sourceMessage.descriptor.dataSize, // if data not given, use base message dataSize + dataFormat : options.dataFormat ?? sourceMessage.descriptor.dataFormat, protocolRole : options.protocolRole, delegatedGrant : options.delegatedGrant, // finally still need signers @@ -910,7 +916,7 @@ export class RecordsWrite implements MessageInterface { * @throws {Error} if immutable properties between two RecordsWrite message */ public static verifyEqualityOfImmutableProperties(existingWriteMessage: RecordsWriteMessage, newMessage: RecordsWriteMessage): boolean { - const mutableDescriptorProperties = ['dataCid', 'dataSize', 'datePublished', 'published', 'messageTimestamp']; + const mutableDescriptorProperties = ['dataCid', 'dataSize', 'dataFormat', 'datePublished', 'published', 'messageTimestamp']; // get distinct property names that exist in either the existing message given or new message let descriptorPropertyNames: string[] = []; diff --git a/src/utils/hd-key.ts b/src/utils/hd-key.ts index 7d17f88ab..25f462ae8 100644 --- a/src/utils/hd-key.ts +++ b/src/utils/hd-key.ts @@ -9,6 +9,7 @@ export enum KeyDerivationScheme { DataFormats = 'dataFormats', ProtocolContext = 'protocolContext', ProtocolPath = 'protocolPath', + /** * Key derivation using the `schema` value for Flat-space records. */ diff --git a/tests/handlers/records-write.spec.ts b/tests/handlers/records-write.spec.ts index cec22d664..65c830d55 100644 --- a/tests/handlers/records-write.spec.ts +++ b/tests/handlers/records-write.spec.ts @@ -232,6 +232,38 @@ export function testRecordsWriteHandler(): void { .to.equal(newerWrite.message.descriptor.dataCid); // expecting unchanged }); + it('#690 - should allow data format of a flat-space record to be updated to any value', async () => { + const initialWriteData = await TestDataGenerator.generateRecordsWrite(); + const tenant = initialWriteData.author.did; + + TestStubGenerator.stubDidResolver(didResolver, [initialWriteData.author]); + + const initialWriteReply = await dwn.processMessage(tenant, initialWriteData.message, { dataStream: initialWriteData.dataStream }); + expect(initialWriteReply.status.code).to.equal(202); + + const newDataFormat = 'any-new-data-format'; + const newDataBytes = TestDataGenerator.randomBytes(100); + const updateWrite = await RecordsWrite.createFrom({ + recordsWriteMessage : initialWriteData.message, + dataFormat : newDataFormat, + signer : Jws.createSigner(initialWriteData.author), + data : newDataBytes + }); + + const newDataStream = DataStream.fromBytes(newDataBytes); + const updateReply = await dwn.processMessage(tenant, updateWrite.message, { dataStream: newDataStream }); + expect(updateReply.status.code).to.equal(202); + + // verify the data format of the record is updated + const recordsRead = await RecordsRead.create({ + filter : { recordId: initialWriteData.message.recordId }, + signer : Jws.createSigner(initialWriteData.author), + }); + const recordsReadReply = await dwn.processMessage(tenant, recordsRead.message); + expect(recordsReadReply.status.code).to.equal(200); + expect(recordsReadReply.record?.descriptor.dataFormat).to.equal(newDataFormat); + }); + it('should not allow changes to immutable properties', async () => { const initialWriteData = await TestDataGenerator.generateRecordsWrite(); const tenant = initialWriteData.author.did; @@ -272,20 +304,6 @@ export function testRecordsWriteHandler(): void { expect(reply.status.code).to.equal(400); expect(reply.status.detail).to.contain('schema is an immutable property'); - - // dataFormat test - childMessageData = await TestDataGenerator.generateRecordsWrite({ - author : initialWriteData.author, - recordId, - schema, - dateCreated, - dataFormat : 'should-not-be-allowed-to-change' - }); - - reply = await dwn.processMessage(tenant, childMessageData.message, { dataStream: childMessageData.dataStream }); - - expect(reply.status.code).to.equal(400); - expect(reply.status.detail).to.contain('dataFormat is an immutable property'); }); it('should inherit data from previous RecordsWrite given a matching dataCid and dataSize and no dataStream', async () => { @@ -894,7 +912,6 @@ export function testRecordsWriteHandler(): void { // modify write2 by referencing the `dataCid` in write1 (which should not be allowed) const write2Change = await TestDataGenerator.generateRecordsWrite({ author : alice, - // immutable properties just inherit from the message given recipient : write2.message.descriptor.recipient, recordId : write2.message.recordId, dateCreated : write2.message.descriptor.dateCreated, @@ -949,7 +966,6 @@ export function testRecordsWriteHandler(): void { // modify write2 by referencing the `dataCid` in write1 (which should not be allowed) const write2Change = await TestDataGenerator.generateRecordsWrite({ author : alice, - // immutable properties just inherit from the message given recipient : write2.message.descriptor.recipient, recordId : write2.message.recordId, dateCreated : write2.message.descriptor.dateCreated, @@ -2626,7 +2642,7 @@ export function testRecordsWriteHandler(): void { expect(reply.status.detail).to.contain(DwnErrorCode.ProtocolAuthorizationParentlessIncorrectProtocolPath); }); - it('should fail authorization if given `dataFormat` is mismatching with the dataFormats in protocol definition', async () => { + it('#690 - should only allow data format of a protocol-space record to be updated to any value allowed by the protocol configuration', async () => { const alice = await TestDataGenerator.generateDidKeyPersona(); const protocolDefinition = socialMediaProtocolDefinition; @@ -2640,9 +2656,9 @@ export function testRecordsWriteHandler(): void { const protocolConfigureReply = await dwn.processMessage(alice.did, protocolConfig.message); expect(protocolConfigureReply.status.code).to.equal(202); - // write record with matching dataFormat - const data = Encoder.stringToBytes('any data'); - const recordsWriteMatch = await TestDataGenerator.generateRecordsWrite({ + // write image record + const data = TestDataGenerator.randomBytes(100); + const imageRecordsWrite = await TestDataGenerator.generateRecordsWrite({ author : alice, recipient : alice.did, protocol, @@ -2651,23 +2667,95 @@ export function testRecordsWriteHandler(): void { dataFormat : protocolDefinition.types.image.dataFormats[0], data }); - const replyMatch = await dwn.processMessage(alice.did, recordsWriteMatch.message, { dataStream: recordsWriteMatch.dataStream }); - expect(replyMatch.status.code).to.equal(202); + const writeReply = await dwn.processMessage(alice.did, imageRecordsWrite.message, { dataStream: imageRecordsWrite.dataStream }); + expect(writeReply.status.code).to.equal(202); + + // update the image to a not-allowed data format + const newDataBytes = TestDataGenerator.randomBytes(100); + const notAllowedUpdateWrite = await RecordsWrite.createFrom({ + recordsWriteMessage : imageRecordsWrite.message, + dataFormat : `not-allowed-data-format`, + signer : Jws.createSigner(alice), + data : newDataBytes + }); + + const newDataStream = DataStream.fromBytes(newDataBytes); + const notAllowedUpdateWriteReply = await dwn.processMessage(alice.did, notAllowedUpdateWrite.message, { dataStream: newDataStream }); + expect(notAllowedUpdateWriteReply.status.code).to.equal(400); + expect(notAllowedUpdateWriteReply.status.detail).to.contain(DwnErrorCode.ProtocolAuthorizationIncorrectDataFormat); + + + // update the image to a different allowed dataFormat + const updateWrite = await RecordsWrite.createFrom({ + recordsWriteMessage : imageRecordsWrite.message, + dataFormat : protocolDefinition.types.image.dataFormats[1], + signer : Jws.createSigner(alice), + data : newDataBytes + }); + + const updateReply = await dwn.processMessage(alice.did, updateWrite.message, { dataStream: newDataStream }); + expect(updateReply.status.code).to.equal(202); + + // verify the data format of the record is updated + const recordsRead = await RecordsRead.create({ + filter : { recordId: imageRecordsWrite.message.recordId }, + signer : Jws.createSigner(alice), + }); + const recordsReadReply = await dwn.processMessage(alice.did, recordsRead.message); + expect(recordsReadReply.status.code).to.equal(200); + expect(recordsReadReply.record?.descriptor.dataFormat).to.equal(protocolDefinition.types.image.dataFormats[1]); + }); + + it('#690 - should allow any data format for a record if protocol definition does not explicitly specify the list of allowed data formats', async () => { + const alice = await TestDataGenerator.generateDidKeyPersona(); + + const protocolDefinition = minimalProtocolDefinition; + const protocol = protocolDefinition.protocol; + + const protocolConfig = await TestDataGenerator.generateProtocolsConfigure({ + author : alice, + protocolDefinition : protocolDefinition, + }); - // write record with mismatch dataFormat - const recordsWriteMismatch = await TestDataGenerator.generateRecordsWrite({ + const protocolConfigureReply = await dwn.processMessage(alice.did, protocolConfig.message); + expect(protocolConfigureReply.status.code).to.equal(202); + + // write image record + const data = TestDataGenerator.randomBytes(100); + const imageRecordsWrite = await TestDataGenerator.generateRecordsWrite({ author : alice, recipient : alice.did, protocol, - protocolPath : 'image', - schema : protocolDefinition.types.image.schema, - dataFormat : 'not/allowed/dataFormat', + protocolPath : 'foo', + schema : 'any-schema', + dataFormat : 'any-data-format', data }); + const writeReply = await dwn.processMessage(alice.did, imageRecordsWrite.message, { dataStream: imageRecordsWrite.dataStream }); + expect(writeReply.status.code).to.equal(202); + + // update the image to a different data format + const newDataFormat = 'any-new-data-format'; + const newDataBytes = TestDataGenerator.randomBytes(100); + const updateWrite = await RecordsWrite.createFrom({ + recordsWriteMessage : imageRecordsWrite.message, + dataFormat : newDataFormat, + signer : Jws.createSigner(alice), + data : newDataBytes + }); - const replyMismatch = await dwn.processMessage(alice.did, recordsWriteMismatch.message, { dataStream: recordsWriteMismatch.dataStream }); - expect(replyMismatch.status.code).to.equal(400); - expect(replyMismatch.status.detail).to.contain(DwnErrorCode.ProtocolAuthorizationIncorrectDataFormat); + const newDataStream = DataStream.fromBytes(newDataBytes); + const updateReply = await dwn.processMessage(alice.did, updateWrite.message, { dataStream: newDataStream }); + expect(updateReply.status.code).to.equal(202); + + // verify the data format of the record is updated + const recordsRead = await RecordsRead.create({ + filter : { recordId: imageRecordsWrite.message.recordId }, + signer : Jws.createSigner(alice), + }); + const recordsReadReply = await dwn.processMessage(alice.did, recordsRead.message); + expect(recordsReadReply.status.code).to.equal(200); + expect(recordsReadReply.record?.descriptor.dataFormat).to.equal(newDataFormat); }); it('should fail authorization if record schema is not allowed at the hierarchical level attempted for the RecordsWrite', async () => { diff --git a/tests/scenarios/subscriptions.spec.ts b/tests/scenarios/subscriptions.spec.ts index 05200da1b..9b1973109 100644 --- a/tests/scenarios/subscriptions.spec.ts +++ b/tests/scenarios/subscriptions.spec.ts @@ -754,10 +754,12 @@ export function testSubscriptionScenarios(): void { }); it('filters by dataFormat', async () => { - // scenario: alice stores different file types and needs events relating to `image/jpeg` - // alice creates 3 files, one of them `image/jpeg` - // alice queries for `image/jpeg` retrieving the one message - // alice adds another image to query for using the prior image as a cursor + // Scenario: Alice subscribes events relating to `image/jpeg` after which a number of record messages of various data formats are processed + // 1. Alice subscribes for `image/jpeg` records + // 2. Alice creates 3 files, one of them `image/jpeg` + // 3. Alice receives the one `image/jpeg` message + // 4. Alice adds another image + // 5. Alice receives the other `image/jpeg` message const alice = await TestDataGenerator.generateDidKeyPersona(); @@ -767,6 +769,7 @@ export function testSubscriptionScenarios(): void { imageMessages.push(await Message.getCid(message)); }; + // alice subscribes to image/jpeg changes const imageSubscription = await TestDataGenerator.generateEventsSubscribe({ author : alice, filters : [{ dataFormat: 'image/jpeg' }] @@ -800,7 +803,6 @@ export function testSubscriptionScenarios(): void { const imageDataReply = await dwn.processMessage(alice.did, imageData.message, { dataStream: imageData.dataStream }); expect(imageDataReply.status.code).to.equal(202); - // wait for messages to emit and handler to process await Time.minimalSleep(); expect(imageMessages.length).to.equal(1); @@ -814,21 +816,12 @@ export function testSubscriptionScenarios(): void { const imageData2Reply = await dwn.processMessage(alice.did, imageData2.message, { dataStream: imageData2.dataStream }); expect(imageData2Reply.status.code).to.equal(202); - // delete the first image - const deleteImageData = await TestDataGenerator.generateRecordsDelete({ - author : alice, - recordId : imageData.message.recordId, - }); - const deleteImageDataReply = await dwn.processMessage(alice.did, deleteImageData.message); - expect(deleteImageDataReply.status.code).to.equal(202); - // wait for messages to emit and handler to process await Time.minimalSleep(); - expect(imageMessages.length).to.equal(3); + expect(imageMessages.length).to.equal(2); // check that the new image and the delete messages were emitted expect(imageMessages).to.include.members([ - await Message.getCid(imageData2.message), - await Message.getCid(deleteImageData.message) + await Message.getCid(imageData2.message) ]); });; diff --git a/tests/vectors/protocol-definitions/social-media.json b/tests/vectors/protocol-definitions/social-media.json index 246a56489..5b28d8d63 100644 --- a/tests/vectors/protocol-definitions/social-media.json +++ b/tests/vectors/protocol-definitions/social-media.json @@ -17,7 +17,9 @@ "image": { "schema": "imageSchema", "dataFormats": [ - "image/jpeg" + "image/jpeg", + "image/gif", + "image/png" ] }, "caption": { @@ -85,4 +87,4 @@ } } } -} +} \ No newline at end of file