Skip to content

Commit

Permalink
Fix record author bug (#345)
Browse files Browse the repository at this point in the history 
* Improve Record class test structure and add failing test for newly discovered bug
* Rename remoteTarget to remoteOrigin
* Correct typo in error message
* Improve semantics of Record class

---------

Signed-off-by: Frank Hinek <[email protected]>
  • Loading branch information
@frankhinek @finn-block
frankhinek authored and finn-block committed Mar 19, 2024
1 parent 9f956ca commit 69bb912
Show file tree
Hide file tree
Showing 4 changed files with 431 additions and 258 deletions.
2 changes: 1 addition & 1 deletion packages/agent/src/dwn-manager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ export class DwnManager {
dwnReply = await this.agent.rpcClient.sendDwnRequest(dwnRpcRequest as DwnRpcRequest);
break;
} catch(error: unknown) {
const message = (error instanceof Error) ? error.message : 'Uknown error';
const message = (error instanceof Error) ? error.message : 'Unknown error';
errorMessages.push({ url: dwnUrl, message });
}
}
Expand Down
40 changes: 18 additions & 22 deletions packages/api/src/dwn-api.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,10 +270,6 @@ export class DwnApi {
createFrom: async (request: RecordsCreateFromRequest): Promise<RecordsWriteResponse> => {
const { author: inheritedAuthor, ...inheritedProperties } = request.record.toJSON();

// Remove target from inherited properties since target is being explicitly defined in method parameters.
delete inheritedProperties.target;


// If `data` is being updated then `dataCid` and `dataSize` must not be present.
if (request.data !== undefined) {
delete inheritedProperties.dataCid;
Expand Down Expand Up @@ -373,18 +369,18 @@ export class DwnApi {
*/
author : RecordsWrite.getAuthor(entry),
/**
* Set the `target` DID to currently connected DID so that subsequent calls to
* Set the `connectedDid` to currently connected DID so that subsequent calls to
* {@link Record} instance methods, such as `record.update()` are executed on the
* local DWN even if the record was returned by a query of a remote DWN.
*/
target : this.connectedDid,
connectedDid : this.connectedDid,
/**
* If the record was returned by a query of a remote DWN, set the `remoteTarget` to
* the DID of the DWN that returned the record. The `remoteTarget` will be used to
* determine which DWN to send subsequent read requests to in the event the data payload
* exceeds the threshold for being returned with queries.
* If the record was returned by a query of a remote DWN, set the `remoteOrigin` to
* the DID of the DWN that returned the record. The `remoteOrigin` property will be used
* to determine which DWN to send subsequent read requests to in the event the data
* payload exceeds the threshold for being returned with queries.
*/
remoteTarget : request.from,
remoteOrigin : request.from,
...entry as RecordsWriteMessage
};
const record = new Record(this.agent, recordOptions);
Expand Down Expand Up @@ -433,18 +429,18 @@ export class DwnApi {
*/
author : RecordsWrite.getAuthor(responseRecord),
/**
* Set the `target` DID to currently connected DID so that subsequent calls to
* Set the `connectedDid` to currently connected DID so that subsequent calls to
* {@link Record} instance methods, such as `record.update()` are executed on the
* local DWN even if the record was read from a remote DWN.
*/
target : this.connectedDid,
connectedDid : this.connectedDid,
/**
* If the record was returned by a query of a remote DWN, set the `remoteTarget` to
* the DID of the DWN that returned the record. The `remoteTarget` will be used to
* determine which DWN to send subsequent read requests to in the event the data payload
* exceeds the threshold for being returned with queries.
* If the record was returned by reading from a remote DWN, set the `remoteOrigin` to
* the DID of the DWN that returned the record. The `remoteOrigin` property will be used
* to determine which DWN to send subsequent read requests to in the event the data
* payload must be read again (e.g., if the data stream is consumed).
*/
remoteTarget : request.from,
remoteOrigin : request.from,
...responseRecord,
};

Expand Down Expand Up @@ -490,14 +486,14 @@ export class DwnApi {
* Assume the author is the connected DID since the record was just written to the
* local DWN.
*/
author : this.connectedDid,
encodedData : dataBlob,
author : this.connectedDid,
/**
* Set the `target` DID to currently connected DID so that subsequent calls to
* Set the `connectedDid` to currently connected DID so that subsequent calls to
* {@link Record} instance methods, such as `record.update()` are executed on the
* local DWN.
*/
target : this.connectedDid,
connectedDid : this.connectedDid,
encodedData : dataBlob,
...responseMessage,
};

Expand Down
79 changes: 40 additions & 39 deletions packages/api/src/record.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ import { dataToBlob } from './utils.js';
*/
export type RecordOptions = RecordsWriteMessage & {
author: string;
target: string;
connectedDid: string;
encodedData?: string | Blob;
data?: Readable | ReadableStream;
remoteTarget?: string;
remoteOrigin?: string;
};

/**
Expand All @@ -37,7 +37,6 @@ export type RecordModel = RecordsWriteDescriptor
& {
author: string;
recordId?: string;
target: string;
}

/**
Expand Down Expand Up @@ -68,29 +67,29 @@ export type RecordUpdateOptions = {
* @beta
*/
export class Record implements RecordModel {
// mutable properties

/** Record's author DID */
author: string;

/** Record's target DID */
target: string;

// Record instance metadata.
private _agent: Web5Agent;
private _connectedDid: string;
private _encodedData?: Blob;
private _readableStream?: Readable;
private _remoteOrigin?: string;

// Private variables for DWN `RecordsWrite` message properties.
private _author: string;
private _attestation?: RecordsWriteMessage['attestation'];
private _contextId?: string;
private _descriptor: RecordsWriteDescriptor;
private _encodedData?: Blob;
private _encryption?: RecordsWriteMessage['encryption'];
private _readableStream?: Readable;
private _recordId: string;
private _remoteTarget?: string;

// Immutable DWN Record properties.
// Getters for immutable DWN Record properties.

/** Record's signatures attestation */
get attestation(): RecordsWriteMessage['attestation'] { return this._attestation; }

/** DID that signed the record. */
get author(): string { return this._author; }

/** Record's context ID */
get contextId() { return this._contextId; }

Expand Down Expand Up @@ -127,7 +126,7 @@ export class Record implements RecordModel {
/** Record's schema */
get schema() { return this._descriptor.schema; }

// Mutable DWN Record properties.
// Getters for mutable DWN Record properties.

/** Record's CID */
get dataCid() { return this._descriptor.dataCid; }
Expand All @@ -150,15 +149,19 @@ export class Record implements RecordModel {
constructor(agent: Web5Agent, options: RecordOptions) {
this._agent = agent;

/** Store the target and author DIDs that were used to create the message to use for subsequent
* updates, reads, etc. */
this.author = options.author;
this.target = options.target;
/** Store the author DID that originally signed the message as a convenience for developers, so
* that they don't have to decode the signer's DID from the JWS. */
this._author = options.author;

/** Store the currently `connectedDid` so that subsequent message signing is done with the
* connected DID's keys and DWN requests target the connected DID's DWN. */
this._connectedDid = options.connectedDid;

/** If the record was queried from a remote DWN, the `remoteTarget` DID will be defined. This
* value is used to send subsequent read requests to the same remote DWN in the event the
* record's data payload was too large to be returned in query results. */
this._remoteTarget = options.remoteTarget;
/** If the record was queried or read from a remote DWN, the `remoteOrigin` DID will be
* defined. This value is used to send subsequent read requests to the same remote DWN in the
* event the record's data payload was too large to be returned in query results. or must be
* read again (e.g., if the data stream is consumed). */
this._remoteOrigin = options.remoteOrigin;

// RecordsWriteMessage properties.
this._attestation = options.attestation;
Expand Down Expand Up @@ -264,14 +267,13 @@ export class Record implements RecordModel {
self._readableStream = NodeStream.fromWebReadable({ readableStream: self._encodedData.stream() });

} else if (!NodeStream.isReadable({ readable: self._readableStream })) {
/** If `encodedData` is not set, then the Record was instantiated by `dwn.records.read()`
* or was too large to be returned in `dwn.records.query()` results. In either case, the
* data is not available in-memory and must be fetched from either: */
self._readableStream = self._remoteTarget ?
// 1. ...a remote DWN if the record was queried from a remote DWN.
await self.readRecordData({ target: self._remoteTarget, isRemote: true }) :
// 2. ...a local DWN if the record was queried from the local DWN.
await self.readRecordData({ target: self.target, isRemote: false });
/** If the data stream for this `Record` instance has already been partially or fully
* consumed, then the data must be fetched again from either: */
self._readableStream = self._remoteOrigin ?
// A. ...a remote DWN if the record was originally queried from a remote DWN.
await self.readRecordData({ target: self._remoteOrigin, isRemote: true }) :
// B. ...a local DWN if the record was originally queried from the local DWN.
await self.readRecordData({ target: self._connectedDid, isRemote: false });
}

if (!self._readableStream) {
Expand Down Expand Up @@ -305,7 +307,7 @@ export class Record implements RecordModel {
async send(target: string): Promise<ResponseStatus> {
const { reply: { status } } = await this._agent.sendDwnRequest({
messageType : DwnInterfaceName.Records + DwnMethodName.Write,
author : this.author,
author : this._connectedDid,
dataStream : await this.data.blob(),
target : target,
messageOptions : this.toJSON(),
Expand Down Expand Up @@ -338,8 +340,7 @@ export class Record implements RecordModel {
published : this.published,
recipient : this.recipient,
recordId : this.id,
schema : this.schema,
target : this.target,
schema : this.schema
};
}

Expand Down Expand Up @@ -415,11 +416,11 @@ export class Record implements RecordModel {
};

const agentResponse = await this._agent.processDwnRequest({
author : this.author,
author : this._connectedDid,
dataStream : dataBlob,
messageOptions,
messageType : DwnInterfaceName.Records + DwnMethodName.Write,
target : this.target,
target : this._connectedDid,
});

const { message, reply: { status } } = agentResponse;
Expand All @@ -440,7 +441,7 @@ export class Record implements RecordModel {
}

/**
* Fetches the record's data from the source DWN.
* Fetches the record's data from the specified DWN.
*
* This private method is called when the record data is not available in-memory
* and needs to be fetched from either a local or a remote DWN.
Expand All @@ -456,7 +457,7 @@ export class Record implements RecordModel {
*/
private async readRecordData({ target, isRemote }: { target: string, isRemote: boolean }) {
const readRequest = {
author : this.author,
author : this._connectedDid,
messageOptions : { filter: { recordId: this.id } },
messageType : DwnInterfaceName.Records + DwnMethodName.Read,
target,
Expand Down
Loading

0 comments on commit 69bb912

Please sign in to comment.