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.

Sign up for GitHub

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

Fix record author bug #345

Merged
merged 4 commits into from
Dec 12, 2023
Merged

Fix record author bug #345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b919cfc
Improve Record class test structure and add failing test for newly di…
frankhinek Dec 9, 2023
585be3f
Rename remoteTarget to remoteOrigin
frankhinek Dec 9, 2023
23d86b7
Correct typo in error message
frankhinek Dec 9, 2023
33f666a
Improve semantics of Record class
frankhinek Dec 9, 2023
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 packages/agent/src/dwn-manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@
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';

Check warning on line 228 in packages/agent/src/dwn-manager.ts

View check run for this annotation

Codecov / codecov/patch

packages/agent/src/dwn-manager.ts#L228 

Added line #L228 was not covered by tests
errorMessages.push({ url: dwnUrl, message });
}
}
Expand Down
40 changes: 18 additions & 22 deletions packages/api/src/dwn-api.ts
View file
Open in desktop
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
Open in desktop
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