diff --git a/packages/runner/src/cell.ts b/packages/runner/src/cell.ts index 6957073de..4fb7af75d 100644 --- a/packages/runner/src/cell.ts +++ b/packages/runner/src/cell.ts @@ -41,6 +41,7 @@ import type { IReadOptions, } from "./storage/interface.ts"; import { fromURI } from "./uri-utils.ts"; +import { getEntityId } from "./doc-map.ts"; /** * This is the regular Cell interface, generated by DocImpl.asCell(). @@ -355,7 +356,9 @@ class StreamCell implements Stream { } getRaw(options?: IReadOptions): any { - return (this.tx?.status().status === "ready" ? this.tx : this.runtime.edit()) + return (this.tx?.status().status === "ready" + ? this.tx + : this.runtime.edit()) .readValueOrThrow(this.link, options); } @@ -364,7 +367,10 @@ class StreamCell implements Stream { } getDoc(): DocImpl { - return this.runtime.documentMap.getDocByEntityId(this.link.space, this.link.id); + return this.runtime.documentMap.getDocByEntityId( + this.link.space, + this.link.id, + ); } withTx(_tx?: IExtendedStorageTransaction): Stream { @@ -374,7 +380,7 @@ class StreamCell implements Stream { class RegularCell implements Cell { private readOnlyReason: string | undefined; - + constructor( public readonly runtime: IRuntime, private readonly link: NormalizedFullLink, @@ -433,7 +439,8 @@ class RegularCell implements Cell { if (this.schema) { // Check if schema allows objects const allowsObject = this.schema.type === "object" || - (Array.isArray(this.schema.type) && this.schema.type.includes("object")) || + (Array.isArray(this.schema.type) && + this.schema.type.includes("object")) || (this.schema.anyOf && this.schema.anyOf.some((s) => typeof s === "object" && s.type === "object" @@ -571,7 +578,10 @@ class RegularCell implements Cell { return createQueryResultProxy( this.runtime, tx ?? this.tx ?? this.runtime.edit(), - { ...this.link, path: [...this.path, ...subPath.map((p) => p.toString())] as string[] }, + { + ...this.link, + path: [...this.path, ...subPath.map((p) => p.toString())] as string[], + }, ); } @@ -606,11 +616,16 @@ class RegularCell implements Cell { } getDoc(): DocImpl { - return this.runtime.documentMap.getDocByEntityId(this.link.space, this.link.id); + return this.runtime.documentMap.getDocByEntityId( + this.link.space, + this.link.id, + ); } getRaw(options?: IReadOptions): any { - return (this.tx?.status().status === "ready" ? this.tx : this.runtime.edit()) + return (this.tx?.status().status === "ready" + ? this.tx + : this.runtime.edit()) .readValueOrThrow(this.link, options); } @@ -643,10 +658,18 @@ class RegularCell implements Cell { > | undefined; getSourceCell(schema?: JSONSchema): Cell | undefined { - const sourceCellId = + let sourceCellId = (this.tx?.status().status === "ready" ? this.tx : this.runtime.edit()) - .readOrThrow({ ...this.link, path: ["source"] }); - if (!sourceCellId) return undefined; + .readOrThrow({ ...this.link, path: ["source"] }) as string | undefined; + if (!sourceCellId || typeof sourceCellId !== "string") { + return undefined; + } + if (sourceCellId.startsWith('{"/":')) { + sourceCellId = toURI(JSON.parse(sourceCellId)); + } + if (!sourceCellId.startsWith("of:")) { + throw new Error("Source cell ID must start with 'of:'"); + } return createCell(this.runtime, { space: this.link.space, path: [], @@ -662,12 +685,16 @@ class RegularCell implements Cell { if (sourceLink.path.length > 0) { throw new Error("Source cell must have empty path for now"); } - this.tx.writeOrThrow({ ...this.link, path: ["source"] }, sourceLink.id); + this.tx.writeOrThrow( + { ...this.link, path: ["source"] }, + JSON.stringify(getEntityId(sourceLink.id)), + ); } freeze(reason: string): void { this.readOnlyReason = reason; - this.runtime.documentMap.getDocByEntityId(this.link.space, this.link.id)?.freeze(reason); + this.runtime.documentMap.getDocByEntityId(this.link.space, this.link.id) + ?.freeze(reason); } isFrozen(): boolean { @@ -678,7 +705,10 @@ class RegularCell implements Cell { // Keep old format for backward compatibility return { cell: { - "/": (this.link.id.startsWith("data:") ? this.link.id : fromURI(this.link.id)), + "/": + (this.link.id.startsWith("data:") + ? this.link.id + : fromURI(this.link.id)), }, path: this.path as (string | number)[], }; diff --git a/packages/runner/src/doc.ts b/packages/runner/src/doc.ts index 9d72b32af..0de26c9ff 100644 --- a/packages/runner/src/doc.ts +++ b/packages/runner/src/doc.ts @@ -339,14 +339,21 @@ export function createDoc( // Send notification if shim storage manager is available if (runtime.storage.shim) { + let before = previousValue; + let after = newValue; + for (const key of ["value", ...path.map(String)].reverse()) { + before = { [key]: before }; + after = { [key]: after }; + } + const change: IMemoryChange = { address: { id: toURI(entityId), type: "application/json", path: ["value", ...path.map(String)], }, - before: previousValue, - after: newValue, + before: before, + after: after, }; const notification: ICommitNotification = { @@ -413,8 +420,8 @@ export function createDoc( type: "application/json", path: ["source"], }, - before: JSON.stringify(sourceCell?.entityId), - after: JSON.stringify(cell?.entityId), + before: { source: JSON.stringify(sourceCell?.entityId) }, + after: { source: JSON.stringify(cell?.entityId) }, }; const notification: ICommitNotification = { diff --git a/packages/runner/src/reactive-dependencies.ts b/packages/runner/src/reactive-dependencies.ts index 895af996a..5a1443f32 100644 --- a/packages/runner/src/reactive-dependencies.ts +++ b/packages/runner/src/reactive-dependencies.ts @@ -110,17 +110,11 @@ export function determineTriggeredActions( if (startPath.length > 0) { // If we're starting from a specific path, filter the subscribers to only - // include those that start with that path. + // include those that can be affected by that path. subscribers = subscribers.map(({ action, paths }) => ({ action, paths: paths.filter((path) => arraysOverlap(path, startPath)), })).filter(({ paths }) => paths.length > 0); - - // And prepend path to data, so we don't have to special case this. - for (const key of startPath.toReversed()) { - before = { [key]: before } as JSONValue; - after = { [key]: after } as JSONValue; - } } // Sort subscribers by last/longest path first. diff --git a/packages/runner/src/runtime.ts b/packages/runner/src/runtime.ts index cc774c4c7..10e8dc87c 100644 --- a/packages/runner/src/runtime.ts +++ b/packages/runner/src/runtime.ts @@ -1,3 +1,4 @@ +import { isDeno } from "@commontools/utils/env"; import type { JSONSchema, Module, @@ -45,7 +46,9 @@ import { Runner } from "./runner.ts"; import { registerBuiltins } from "./builtins/index.ts"; import { StaticCache } from "@commontools/static"; -const DEFAULT_USE_REAL_TRANSACTIONS = false; +const DEFAULT_USE_REAL_TRANSACTIONS = isDeno() + ? ["1", "true", "on", "yes"].includes(Deno.env.get("USE_REAL_TRANSACTIONS")!) + : false; export type { IExtendedStorageTransaction, IStorageProvider, MemorySpace }; diff --git a/packages/runner/src/storage/interface.ts b/packages/runner/src/storage/interface.ts index 31a1242e4..d64cbbbb5 100644 --- a/packages/runner/src/storage/interface.ts +++ b/packages/runner/src/storage/interface.ts @@ -559,13 +559,13 @@ export interface ITransactionReader { * }) * assert(w.ok) * - * assert(tx.read({ type, id, path:: ['author'] }).ok === undefined) - * assert(tx.read({ type, id, path:: ['author', 'address'] }).error.name === 'NotFoundError') + * assert(tx.read({ type, id, path: ['author'] }).ok === undefined) + * assert(tx.read({ type, id, path: ['author', 'address'] }).error.name === 'NotFoundError') * // JS specific getters are not supported - * assert(tx.read({ the, of, at: ['content', 'length'] }).ok.is === undefined) - * assert(tx.read({ the, of, at: ['title'] }).ok.is === "Hello world") + * assert(tx.read({ type, id, path: ['content', 'length'] }).ok?.value === undefined) + * assert(tx.read({ type, id, path: ['title'] }).ok?.value === "Hello world") * // Referencing non-existing facts produces errors - * assert(tx.read({ the: 'bad/mime' , of, at: ['author'] }).error.name === 'NotFoundError') + * assert(tx.read({ type: 'bad/mime', id, path: ['author'] }).error.name === 'NotFoundError') * ``` * * @param address - Memory address to read from @@ -656,7 +656,10 @@ export type CommitError = export interface INotFoundError extends Error { name: "NotFoundError"; + source: IAttestation; + address: IMemoryAddress; path?: MemoryAddressPathComponent[]; + from(space: MemorySpace): INotFoundError; } /** @@ -682,13 +685,15 @@ export type ReadError = | INotFoundError | InactiveTransactionError | IInvalidDataURIError - | IUnsupportedMediaTypeError; + | IUnsupportedMediaTypeError + | ITypeMismatchError; export type WriteError = | INotFoundError | IUnsupportedMediaTypeError | InactiveTransactionError - | IReadOnlyAddressError; + | IReadOnlyAddressError + | ITypeMismatchError; export type ReaderError = InactiveTransactionError; @@ -700,19 +705,6 @@ export type WriterError = export interface IStorageTransactionComplete extends Error { name: "StorageTransactionCompleteError"; } -export interface INotFoundError extends Error { - name: "NotFoundError"; - - /** - * Source in which address could not be resolved. - */ - source: IAttestation; - - /** - * Address that we could not resolve. - */ - address: IMemoryAddress; -} /** * Represents adddress within the memory space which is like pointer inside the @@ -859,6 +851,28 @@ export interface IReadOnlyAddressError extends Error { from(space: MemorySpace): IReadOnlyAddressError; } +/** + * Error returned when attempting to access a property on a non-object value. + * This is different from NotFound (document doesn't exist) and Inconsistency + * (state changed). This error indicates a type mismatch that would persist + * even if the transaction were retried. + */ +export interface ITypeMismatchError extends Error { + name: "TypeMismatchError"; + + /** + * The address being accessed. + */ + address: IMemoryAddress; + + /** + * The actual type encountered. + */ + actualType: string; + + from(space: MemorySpace): ITypeMismatchError; +} + /** * Describes either observed or desired state of the memory at a specific * address. diff --git a/packages/runner/src/storage/transaction-explainer.md b/packages/runner/src/storage/transaction-explainer.md new file mode 100644 index 000000000..247a04d38 --- /dev/null +++ b/packages/runner/src/storage/transaction-explainer.md @@ -0,0 +1,172 @@ +# Storage Transaction Abstraction Explainer + +The storage transaction system provides ACID-like guarantees for reading and +writing data across memory spaces. Think of it as a database transaction but for +a distributed, content-addressed memory system. + +## Core Architecture + +The transaction system is built in three layers: + +1. **StorageTransaction** + (`packages/runner/src/storage/transaction.ts:63-107`) - The main API surface + that manages transaction lifecycle +2. **Journal** (`packages/runner/src/storage/transaction/journal.ts:52-100`) - + Tracks all reads/writes and enforces consistency +3. **Chronicle** + (`packages/runner/src/storage/transaction/chronicle.ts:51-223`) - Manages + changes per memory space with conflict detection + +## How It Works + +### Transaction Lifecycle + +A transaction moves through three states: + +1. **Ready** - Active and accepting reads/writes +2. **Pending** - Commit initiated, waiting for storage confirmation +3. **Done** - Completed (successfully or with error) + +```typescript +// Create a new transaction +const transaction = create(storageManager); + +// Read and write operations +const readResult = transaction.read({ + space: "user", + id: "123", + type: "application/json", + path: ["name"], +}); +const writeResult = transaction.write({ + space: "user", + id: "123", + type: "application/json", + path: ["age"], +}, 25); + +// Commit changes +const commitResult = await transaction.commit(); +``` + +### Key Design Principles + +**1. Write Isolation**: A transaction can only write to one memory space. This +prevents distributed consistency issues: + +```typescript +// First write locks the transaction to "user" space +transaction.write({ space: "user", ... }, data); + +// This will fail with WriteIsolationError +transaction.write({ space: "project", ... }, data); +``` + +**2. Read Consistency**: All reads capture "invariants" - assumptions about the +state when read. If any invariant is violated before commit, the transaction +fails: + +```typescript +// Transaction reads age = 30 +const age = transaction.read({ ...address, path: ["age"] }); + +// Another client changes age to 31 + +// This transaction's commit will fail due to inconsistency +``` + +**3. Optimistic Updates**: Writes within a transaction see their own changes +immediately: + +```typescript +transaction.write({ ...address, path: ["name"] }, "Alice"); +const name = transaction.read({ ...address, path: ["name"] }); // Returns "Alice" +``` + +## Internal Mechanics + +### Journal (`packages/runner/src/storage/transaction/journal.ts`) + +The Journal manages transaction state and coordinates between readers/writers: + +- Maintains separate Chronicle instances per memory space +- Tracks all read/write activity for debugging and replay +- Enforces single-writer constraint +- Handles transaction abort/close lifecycle + +### Chronicle (`packages/runner/src/storage/transaction/chronicle.ts`) + +Each Chronicle manages changes for one memory space: + +- **History**: Tracks all read invariants to detect conflicts +- **Novelty**: Stores pending writes not yet committed +- **Rebase**: Merges overlapping writes intelligently + +Key operations: + +1. **Read** - Checks novelty first (uncommitted writes), then replica, captures + invariant +2. **Write** - Validates against current state, stores in novelty +3. **Commit** - Verifies all invariants still hold, builds final transaction + +### Attestation System + +All reads and writes produce "attestations" - immutable records of observed or +desired state: + +```typescript +interface IAttestation { + address: IMemoryAddress; // What was read/written + value?: JSONValue; // The value (undefined = deleted) +} +``` + +## Error Handling + +The system uses Result types extensively with specific errors: + +- **TransactionCompleteError** - Operation on finished transaction +- **WriteIsolationError** - Attempting to write to second space +- **StorageTransactionInconsistent** - Read invariant violated (underlying state + changed) +- **NotFoundError** - Document or path doesn't exist +- **TypeMismatchError** - Accessing properties on non-objects (e.g., trying to + read `string.property`) +- **ReadOnlyAddressError** - Writing to data: URIs + +The distinction between these errors is important: + +- **NotFoundError** is transient - the operation would succeed if the + document/path existed +- **TypeMismatchError** is persistent - the operation will always fail unless + the data type changes +- **StorageTransactionInconsistent** indicates retry is needed - the underlying + data changed + +## Advanced Features + +**1. Data URIs**: Read-only inline data using `data:` URLs + +**2. Path-based Access**: Read/write nested JSON paths: + +```typescript +transaction.read({ ...address, path: ["user", "profile", "name"] }); +``` + +**3. Activity Tracking**: All operations recorded for debugging: + +```typescript +for (const activity of transaction.journal.activity()) { + // Log reads and writes +} +``` + +**4. Consistency Validation**: Chronicle's commit method +(`packages/runner/src/storage/transaction/chronicle.ts:176-222`) carefully +validates that all read invariants still hold before building the final +transaction. + +This architecture ensures strong consistency guarantees while allowing +optimistic updates within a transaction boundary, making it suitable for +collaborative editing scenarios where multiple clients may be modifying data +concurrently. diff --git a/packages/runner/src/storage/transaction-implementation-guide.md b/packages/runner/src/storage/transaction-implementation-guide.md new file mode 100644 index 000000000..4ad9739f3 --- /dev/null +++ b/packages/runner/src/storage/transaction-implementation-guide.md @@ -0,0 +1,381 @@ +# Storage Transaction Implementation Guide + +This guide provides detailed implementation knowledge for developers working on +the storage transaction system. It covers state machines, error handling, and +key implementation details. + +## State Machines + +### Transaction State Machine + +The `StorageTransaction` class manages three distinct states: + +```typescript +// packages/runner/src/storage/transaction.ts:35-57 +type EditableState = { + status: "ready"; + storage: IStorageManager; + journal: Journal.Journal; + writer: ITransactionWriter | null; // Cached writer instance +}; + +type SumbittedState = { + status: "pending"; + journal: Journal.Journal; + promise: Promise>; +}; + +type CompleteState = { + status: "done"; + journal: Journal.Journal; + result: Result; +}; +``` + +**State Transitions:** + +``` +ready → pending → done + ↓ ↓ + └──────→ done (with error) +``` + +### Journal State Machine + +The `Journal` class has two states: + +```typescript +// packages/runner/src/storage/transaction/journal.ts:28-43 +interface OpenState { + status: "open"; + storage: IStorageManager; + branches: Map; + readers: Map; + writers: Map; + activity: Activity[]; +} + +interface ClosedState { + status: "closed"; + reason: Result< + JournalArchive, + IStorageTransactionAborted | IStorageTransactionInconsistent + >; + branches: Map; + activity: Activity[]; +} +``` + +## Critical Implementation Details + +### 1. State Mutation Pattern + +The codebase uses a specific pattern for state management: + +```typescript +// packages/runner/src/storage/transaction.ts:64-69 +class StorageTransaction { + static mutate(transaction: StorageTransaction, state: State) { + transaction.#state = state; + } + static use(transaction: StorageTransaction): State { + return transaction.#state; + } +} +``` + +This pattern ensures controlled state access and makes state transitions +explicit. + +### 2. Writer Isolation Enforcement + +The writer isolation is enforced at two levels: + +**Transaction Level** (`packages/runner/src/storage/transaction.ts:163-206`): + +- Caches a single writer reference in `EditableState.writer` +- Returns cached writer for same space +- Fails with `WriteIsolationError` for different space + +**Journal Level** +(`packages/runner/src/storage/transaction/journal.ts:209-232`): + +- Maintains `writers` Map but effectively allows only one +- First writer creation succeeds, stored in map +- Subsequent requests for same space return cached instance + +### 3. Chronicle's Dual Storage + +Each `Chronicle` maintains two separate data structures: + +```typescript +// packages/runner/src/storage/transaction/chronicle.ts:225-353 +class History { + #model: Map = new Map(); + // Stores read invariants for consistency checking +} + +class Novelty { + #model: Map = new Map(); + // Stores uncommitted writes +} +``` + +**Key insight**: History uses overlapping detection to maintain minimal set of +invariants, while Novelty merges writes to avoid redundancy. + +### 4. Address Handling + +Memory addresses have three components: + +- `id`: Entity URI +- `type`: Media type (e.g., "application/json") +- `path`: JSON path within the value + +Special handling for data URIs: + +```typescript +// packages/runner/src/storage/transaction/chronicle.ts:102-104 +if (Address.isInline(address)) { + return { error: new ReadOnlyAddressError(address) }; +} +``` + +## Error States and Handling + +### 1. Transaction-Level Errors + +**InactiveTransactionError** - Base type for operations on inactive +transactions: + +- `TransactionCompleteError`: Transaction already committed/aborted +- `TransactionAborted`: Transaction was explicitly aborted +- `StorageTransactionInconsistent`: Invariant violation detected + +**WriteIsolationError** (`packages/runner/src/storage/transaction.ts:317-331`): + +```typescript +// Triggered when attempting to write to different space +if (writer.did() !== space) { + return { + error: new WriteIsolationError({ open: writer.did(), requested: space }), + }; +} +``` + +### 2. Chronicle-Level Errors + +**StorageTransactionInconsistent**: + +- Detected during invariant validation +- Compares expected vs actual values at overlapping addresses +- Critical for maintaining consistency guarantees +- Indicates underlying state changed - retry needed + +**NotFoundError**: + +- Document or path doesn't exist +- Returned when writing to nested path of non-existent document +- Transient error - would succeed if document existed + +**TypeMismatchError**: + +- Attempting to access properties on non-objects +- E.g., reading `"string".property` or `null.field` +- Persistent error - will always fail unless data type changes + +**ReadOnlyAddressError**: + +- Prevents writes to `data:` URIs +- Checked early in write path + +### 3. Error Handling Strategy + +The error types follow a clear strategy: + +1. **Transient Errors** (may succeed on retry): + - `NotFoundError`: Create the missing document/path + - `StorageTransactionInconsistent`: Retry with updated state + +2. **Persistent Errors** (will always fail): + - `TypeMismatchError`: Data structure incompatibility + - `ReadOnlyAddressError`: Immutable addresses + - `WriteIsolationError`: Transaction design constraint + +3. **State Errors** (transaction unusable): + - `TransactionCompleteError`: Already finished + - `TransactionAborted`: Explicitly cancelled + +### 4. Commit Failure Modes + +The commit process (`packages/runner/src/storage/transaction.ts:260-299`) can +fail at multiple stages: + +1. **Pre-commit validation**: + - Transaction not in "ready" state + - Journal close fails due to inconsistency + +2. **During commit**: + - Replica commit fails (network, authorization) + - Upstream conflicts detected + +3. **State update on failure**: + + ```typescript + mutate(transaction, { + status: "done", + journal: ready.journal, + result: { error }, + }); + ``` + +## Key Algorithms + +### 1. Consistency Checking + +Chronicle's History class +(`packages/runner/src/storage/transaction/chronicle.ts:287-344`) implements +sophisticated invariant management: + +```typescript +claim(attestation: IAttestation): Result { + const obsolete = new Set(); + + for (const candidate of this) { + if (Address.intersects(attestation.address, candidate.address)) { + // Check consistency at more specific path + const address = longerPath(attestation, candidate); + + if (!valuesMatch(attestation, candidate, address)) { + return { error: new StateInconsistency(...) }; + } + + // Remove redundant invariants + if (isParentOf(attestation, candidate)) { + obsolete.add(candidate); + } + } + } + + // Update invariant set + this.put(attestation); + obsolete.forEach(inv => this.delete(inv)); +} +``` + +### 2. Write Merging + +Novelty class (`packages/runner/src/storage/transaction/chronicle.ts:383-420`) +intelligently merges overlapping writes. + +**Important**: When writing to a nested path, Chronicle first checks if novelty +contains a write that would create the document before returning NotFoundError. +This allows operations like: + +```typescript +// Create document +tx.write({ id: "doc:1", path: [] }, { items: ["a", "b", "c"] }); +// Modify nested path in same transaction +tx.write({ id: "doc:1", path: ["items", "1"] }, "B"); +``` + +The write algorithm: + +```typescript +claim(invariant: IAttestation): Result { + const candidates = this.edit(invariant.address); + + // Try to merge into existing parent + for (const candidate of candidates) { + if (Address.includes(candidate.address, invariant.address)) { + return write(candidate, invariant.address, invariant.value); + } + } + + // Remove obsolete children + for (const candidate of candidates) { + if (Address.includes(invariant.address, candidate.address)) { + candidates.delete(candidate); + } + } + + candidates.put(invariant); +} +``` + +### 3. Rebase Operation + +The rebase operation +(`packages/runner/src/storage/transaction/chronicle.ts:475-496`) applies pending +writes to a source attestation: + +```typescript +rebase(source: IAttestation): Result { + let merged = source; + + for (const change of this.#model.values()) { + if (Address.includes(source.address, change.address)) { + const { error, ok } = write(merged, change.address, change.value); + if (error) return { error }; + merged = ok; + } + } + + return { ok: merged }; +} +``` + +## Performance Considerations + +1. **Chronicle Lazy Initialization**: Chronicles are created on-demand per space + (`packages/runner/src/storage/transaction/journal.ts:158-167`) + +2. **Address String Keys**: Uses `${id}/${type}` for efficient lookups + +3. **Minimal Invariant Set**: History actively prunes redundant invariants + +4. **Single Writer Cache**: Avoids map lookups for common case + +## Testing Considerations + +When testing changes: + +1. **State Transition Coverage**: Ensure all state transitions are tested, + especially error paths + +2. **Concurrency Scenarios**: Test invariant violations from concurrent + modifications + +3. **Address Edge Cases**: + - Empty paths vs nested paths + - Overlapping reads/writes + - Data URI handling + +4. **Error Recovery**: Verify transaction state after each error type + +## Common Pitfalls + +1. **Forgetting State Updates**: Always use `mutate()` to update transaction + state + +2. **Missing Error Propagation**: Chronicle errors must bubble up through + Journal to Transaction + +3. **Invariant Comparison**: Use `JSON.stringify` for deep equality (see + `packages/runner/src/storage/transaction/chronicle.ts:307`) + +4. **Resource Cleanup**: Ensure Chronicles are properly closed even on error + paths + +## Extension Points + +To add new functionality: + +1. **New Error Types**: Extend error interfaces in `interface.ts` + +2. **Additional Metadata**: Extend `Activity` type for new tracking needs + +3. **Custom Validation**: Hook into Chronicle's `commit()` method + +4. **State Observers**: Subscribe to `IStorageSubscriptionCapability` + notifications diff --git a/packages/runner/src/storage/transaction-shim.ts b/packages/runner/src/storage/transaction-shim.ts index 9f8680d9c..393637175 100644 --- a/packages/runner/src/storage/transaction-shim.ts +++ b/packages/runner/src/storage/transaction-shim.ts @@ -9,6 +9,7 @@ import type { InactiveTransactionError, INotFoundError, IReadActivity, + IReadOnlyAddressError, IReadOptions, IStorageSubscription, IStorageSubscriptionCapability, @@ -18,6 +19,7 @@ import type { ITransactionJournal, ITransactionReader, ITransactionWriter, + ITypeMismatchError, IUnsupportedMediaTypeError, JSONValue, MemoryAddressPathComponent, @@ -31,6 +33,7 @@ import type { StorageTransactionFailed, StorageTransactionStatus, Unit, + URI, Write, WriteError, WriterError, @@ -57,7 +60,8 @@ export function uriToEntityId(uri: string): EntityId { function validateParentPath( value: any, path: readonly MemoryAddressPathComponent[], -): INotFoundError | null { + id: URI, +): INotFoundError | ITypeMismatchError | null { const pathLength = path.length; if (pathLength === 0) { @@ -66,13 +70,22 @@ function validateParentPath( // Check if the document itself exists and is an object for first-level writes if (pathLength === 1) { - if (value === undefined || !isRecord(value)) { + if (value === undefined) { const pathError: INotFoundError = new Error( - `Cannot access path [${String(path[0])}] - document is not a record`, + `Cannot access path [${String(path[0])}] - document does not exist`, ) as INotFoundError; pathError.name = "NotFoundError"; return pathError; } + if (!isRecord(value)) { + const typeError: ITypeMismatchError = new Error( + `Cannot access path [${String(path[0])}] - document is not a record`, + ) as ITypeMismatchError; + typeError.name = "TypeMismatchError"; + typeError.address = { id, type: "application/json", path }; + typeError.actualType = typeof value; + return typeError; + } return null; } @@ -83,19 +96,29 @@ function validateParentPath( let parentIndex = 0; for (parentIndex = 0; parentIndex < lastIndex; parentIndex++) { if (!isRecord(parentValue)) { - parentValue = undefined; - break; + // Found a non-record in the path + const typeError: ITypeMismatchError = new Error( + `Cannot access path [${path.join(", ")}] - [${ + path.slice(0, parentIndex + 1).join(", ") + }] is not a record`, + ) as ITypeMismatchError; + typeError.name = "TypeMismatchError"; + typeError.address = { + id, + type: "application/json", + path: path.slice(0, parentIndex + 1), + }; + typeError.actualType = parentValue === null ? "null" : typeof parentValue; + return typeError; } parentValue = parentValue[path[parentIndex] as keyof typeof parentValue]; } - if ( - value === undefined || parentValue === undefined || !isRecord(parentValue) - ) { + if (value === undefined || parentValue === undefined) { const pathError: INotFoundError = new Error( `Cannot access path [${path.join(", ")}] - parent path [${ path.slice(0, lastIndex).join(", ") - }] does not exist or is not a record`, + }] does not exist`, ) as INotFoundError; pathError.name = "NotFoundError"; @@ -103,6 +126,20 @@ function validateParentPath( if (parentIndex > 0) pathError.path = path.slice(0, parentIndex - 1); return pathError; + } else if (!isRecord(parentValue)) { + const typeError: ITypeMismatchError = new Error( + `Cannot access path [${path.join(", ")}] - parent path [${ + path.slice(0, lastIndex).join(", ") + }] is not a record`, + ) as ITypeMismatchError; + typeError.name = "TypeMismatchError"; + typeError.address = { + id, + type: "application/json", + path: path.slice(0, lastIndex), + }; + typeError.actualType = parentValue === null ? "null" : typeof parentValue; + return typeError; } return null; @@ -201,7 +238,11 @@ class TransactionReader implements ITransactionReader { try { const json = getJSONFromDataURI(address.id); - const validationError = validateParentPath(json, address.path); + const validationError = validateParentPath( + json, + address.path, + address.id, + ); if (validationError) { return { ok: undefined, error: validationError }; } @@ -257,7 +298,7 @@ class TransactionReader implements ITransactionReader { const [first, ...rest] = address.path; if (first === "value") { // Validate parent path exists and is a record for nested writes/reads - const validationError = validateParentPath(doc.get(), rest); + const validationError = validateParentPath(doc.get(), rest, address.id); if (validationError) { return { ok: undefined, error: validationError }; } @@ -282,8 +323,8 @@ class TransactionReader implements ITransactionReader { const sourceCell = doc.sourceCell; let value: string | undefined = undefined; if (sourceCell) { - // Convert EntityId to URI string - value = `of:${JSON.parse(JSON.stringify(sourceCell.entityId))["/"]}`; + // Convert EntityId to string + value = JSON.stringify(sourceCell.entityId); } const read: Read = { address, @@ -322,6 +363,8 @@ class TransactionWriter extends TransactionReader | INotFoundError | InactiveTransactionError | IUnsupportedMediaTypeError + | ITypeMismatchError + | IReadOnlyAddressError > { if (address.type !== "application/json") { const error = new Error( @@ -339,8 +382,8 @@ class TransactionWriter extends TransactionReader if (address.id.startsWith("data:")) { const error = new Error( "Cannot write to data URI", - ) as IUnsupportedMediaTypeError; - error.name = "UnsupportedMediaTypeError"; + ) as IReadOnlyAddressError; + error.name = "ReadOnlyAddressError"; return { ok: undefined, error, @@ -378,7 +421,11 @@ class TransactionWriter extends TransactionReader const [first, ...rest] = address.path; if (first === "value") { // Validate parent path exists and is a record for nested writes - const validationError = validateParentPath(doc.get(), rest); + const validationError = validateParentPath( + doc.get(), + rest, + address.id, + ); if (validationError) { return { ok: undefined, error: validationError }; } @@ -404,19 +451,17 @@ class TransactionWriter extends TransactionReader notFoundError.name = "NotFoundError"; return { ok: undefined, error: notFoundError }; } - // Value must be a URI string (of:...) - if (typeof value !== "string" || !value.startsWith("of:")) { + // Value must be a JSON EntityId string + if (typeof value !== "string" || !value.startsWith('{"/":"')) { const notFoundError: INotFoundError = new Error( - `Value for 'source' must be a URI string (of:...)`, + `Value for 'source' must be a JSON string`, ) as INotFoundError; notFoundError.name = "NotFoundError"; return { ok: undefined, error: notFoundError }; } - // Get the source doc in the same space - const sourceEntityId = uriToEntityId(value); const sourceDoc = this.runtime.documentMap.getDocByEntityId( address.space, - sourceEntityId, + value, false, ); if (!sourceDoc) { @@ -612,7 +657,11 @@ export class ExtendedStorageTransaction implements IExtendedStorageTransaction { options?: IReadOptions, ): JSONValue | undefined { const readResult = this.tx.read(address, options); - if (readResult.error && readResult.error.name !== "NotFoundError") { + if ( + readResult.error && + readResult.error.name !== "NotFoundError" && + readResult.error.name !== "TypeMismatchError" + ) { throw readResult.error; } return readResult.ok?.value; @@ -644,9 +693,15 @@ export class ExtendedStorageTransaction implements IExtendedStorageTransaction { value: JSONValue | undefined, ): void { const writeResult = this.tx.write(address, value); - if (writeResult.error && writeResult.error.name === "NotFoundError") { + if ( + writeResult.error && + (writeResult.error.name === "NotFoundError" || + writeResult.error.name === "TypeMismatchError") + ) { // Create parent entries if needed - const lastValidPath = writeResult.error.path; + const lastValidPath = writeResult.error.name === "NotFoundError" + ? writeResult.error.path + : undefined; const valueObj = lastValidPath ? this.readValueOrThrow({ ...address, path: lastValidPath }, { meta: ignoreReadForScheduling, diff --git a/packages/runner/src/storage/transaction/attestation.ts b/packages/runner/src/storage/transaction/attestation.ts index fa8ad2654..54588df58 100644 --- a/packages/runner/src/storage/transaction/attestation.ts +++ b/packages/runner/src/storage/transaction/attestation.ts @@ -5,6 +5,7 @@ import type { INotFoundError, ISpaceReplica, IStorageTransactionInconsistent, + ITypeMismatchError, IUnsupportedMediaTypeError, JSONValue, MemorySpace, @@ -49,13 +50,16 @@ export class UnsupportedMediaTypeError extends Error * Takes `source` attestation, `address` and `value` and produces derived * attestation with `value` set to a property that given `address` leads to * in the `source`. Fails with inconsitency error if provided `address` leads - * to a non-object target. + * to a non-object target, or NotFound error if the document doesn't exist. */ export const write = ( source: IAttestation, address: IMemoryAddress, value: JSONValue | undefined, -): Result => { +): Result< + IAttestation, + IStorageTransactionInconsistent | INotFoundError | ITypeMismatchError +> => { const path = address.path.slice(source.address.path.length); if (path.length === 0) { return { ok: { ...source, value } }; @@ -73,8 +77,17 @@ export const write = ( if (error) { return { error }; } else { - const type = ok.value === null ? "null" : typeof ok.value; - if (type === "object") { + const type = ok.value === null + ? "null" + : Array.isArray(ok.value) + ? "array" + : typeof ok.value; + if ( + type === "object" || + (type === "array" && + ((Number.isInteger(Number(key)) && Number(key) >= 0) || + key === "length")) + ) { const target = ok.value as Record; // If target value is same as desired value this write is a noop @@ -90,10 +103,12 @@ export const write = ( return { ok: patch }; } else { + // Type mismatch - trying to write property on non-object return { - error: new WriteInconsistency( - { address: { ...address, path }, value }, + error: new TypeMismatchError( address, + type, + "write", ), }; } @@ -141,7 +156,10 @@ export const write = ( export const read = ( source: IAttestation, address: IMemoryAddress, -) => resolve(source, address); +): Result< + IAttestation, + IStorageTransactionInconsistent | INotFoundError | ITypeMismatchError +> => resolve(source, address); /** * Takes a source fact {@link State} and derives an attestion describing it's @@ -187,10 +205,21 @@ export const claim = ( export const resolve = ( source: IAttestation, address: IMemoryAddress, -): Result => { +): Result< + IAttestation, + IStorageTransactionInconsistent | INotFoundError | ITypeMismatchError +> => { const { path } = address; let at = source.address.path.length - 1; let value = source.value; + + // If the source value is undefined (document doesn't exist), return NotFound + if (source.value === undefined && path.length > source.address.path.length) { + return { + error: new NotFound(source, address), + }; + } + while (++at < path.length) { const key = path[at]; if (typeof value === "object" && value != null) { @@ -199,14 +228,14 @@ export const resolve = ( ? undefined : (value as Record)[key]; } else { + // Type mismatch - trying to access property on non-object + const actualType = value === null ? "null" : typeof value; return { - error: new ReadInconsistency({ - address: { - ...address, - path: path.slice(0, at), - }, - value, - }, address), + error: new TypeMismatchError( + { ...address, path: path.slice(0, at + 1) }, + actualType, + "read", + ), }; } } @@ -304,86 +333,59 @@ export const load = ( export class NotFound extends RangeError implements INotFoundError { override name = "NotFoundError" as const; + declare readonly source: IAttestation; + declare readonly address: IMemoryAddress; constructor( - public source: IAttestation, - public address: IMemoryAddress, - public space?: MemorySpace, - ) { - const message = [ - `Can not resolve the "${address.type}" of "${address.id}" at "${ - address.path.join(".") - }"`, - space ? ` from "${space}"` : "", - `, because encountered following non-object at ${ - source.address.path.join(".") - }:`, - source.value === undefined ? source.value : JSON.stringify(source.value), - ].join(""); - - super(message); - } - - from(space: MemorySpace) { - return new NotFound(this.source, this.address, space); - } -} - -export class WriteInconsistency extends RangeError - implements IStorageTransactionInconsistent { - override name = "StorageTransactionInconsistent" as const; - - constructor( - public source: IAttestation, - public address: IMemoryAddress, - public space?: MemorySpace, + source: IAttestation, + address: IMemoryAddress, ) { - const message = [ - `Transaction consistency violated: cannot write the "${address.type}" of "${address.id}" at "${ - address.path.join(".") - }"`, - space ? ` in space "${space}"` : "", - `. Write operation expected an object at path "${ - source.address.path.join(".") - }" but encountered: ${ - source.value === undefined ? "undefined" : JSON.stringify(source.value) - }`, - ].join(""); + let message: string; + + // Document doesn't exist + if (source.value === undefined && source.address.path.length === 0) { + message = `Document not found: ${address.id}`; + } // Path doesn't exist within document + else { + message = `Cannot access path [${address.path.join(", ")}] - ${ + source.value === undefined + ? "document does not exist" + : "path does not exist" + }`; + } super(message); + this.source = source; + this.address = address; } from(space: MemorySpace) { - return new WriteInconsistency(this.source, this.address, space); + // Return the same error instance as it doesn't use space in the message + return this; } } -export class ReadInconsistency extends RangeError - implements IStorageTransactionInconsistent { - override name = "StorageTransactionInconsistent" as const; +export class TypeMismatchError extends TypeError implements ITypeMismatchError { + override name = "TypeMismatchError" as const; + declare readonly address: IMemoryAddress; + declare readonly actualType: string; constructor( - public source: IAttestation, - public address: IMemoryAddress, - public space?: MemorySpace, + address: IMemoryAddress, + actualType: string, + operation: "read" | "write", ) { - const message = [ - `Transaction consistency violated: cannot read "${address.type}" of "${address.id}" at "${ - address.path.join(".") - }"`, - space ? ` in space "${space}"` : "", - `. Read operation expected an object at path "${ - source.address.path.join(".") - }" but encountered: ${ - source.value === undefined ? "undefined" : JSON.stringify(source.value) - }`, - ].join(""); - + const message = `Cannot ${operation} property at path [${ + address.path.join(", ") + }] - expected object but found ${actualType}`; super(message); + this.address = address; + this.actualType = actualType; } from(space: MemorySpace) { - return new ReadInconsistency(this.source, this.address, space); + // Return the same error instance as it doesn't use space in the message + return this; } } diff --git a/packages/runner/src/storage/transaction/chronicle.ts b/packages/runner/src/storage/transaction/chronicle.ts index 379386e1d..de91d72f9 100644 --- a/packages/runner/src/storage/transaction/chronicle.ts +++ b/packages/runner/src/storage/transaction/chronicle.ts @@ -2,10 +2,12 @@ import type { IAttestation, IInvalidDataURIError, IMemoryAddress, + INotFoundError, IReadOnlyAddressError, ISpaceReplica, IStorageTransactionInconsistent, ITransaction, + ITypeMismatchError, IUnsupportedMediaTypeError, JSONValue, MemorySpace, @@ -18,8 +20,10 @@ import { claim, InvalidDataURIError, load, + NotFound, read, StateInconsistency, + TypeMismatchError, UnsupportedMediaTypeError, write, } from "./attestation.ts"; @@ -29,7 +33,12 @@ import * as Edit from "./edit.ts"; export const open = (replica: ISpaceReplica) => new Chronicle(replica); -export { InvalidDataURIError, UnsupportedMediaTypeError }; +export { + InvalidDataURIError, + NotFound, + TypeMismatchError, + UnsupportedMediaTypeError, +}; export class ReadOnlyAddressError extends Error implements IReadOnlyAddressError { @@ -43,7 +52,7 @@ export class ReadOnlyAddressError extends Error this.address = address; } - from(space: MemorySpace) { + from(_space: MemorySpace) { return this; } } @@ -96,20 +105,33 @@ export class Chronicle { value?: JSONValue, ): Result< IAttestation, - IStorageTransactionInconsistent | ReadOnlyAddressError + | IStorageTransactionInconsistent + | ReadOnlyAddressError + | INotFoundError + | ITypeMismatchError > { // Check if address is inline (data: URI) - these are read-only if (Address.isInline(address)) { return { error: new ReadOnlyAddressError(address) }; } + // Load the fact from replica + const state = this.load(address); + const loaded = attest(state); + // Validate against current state (replica + any overlapping novelty) - const loaded = attest(this.load(address)); const rebase = this.rebase(loaded); if (rebase.error) { return rebase; } + // Check if document exists when trying to write to nested path + // Only return NotFound if we're accessing a path on a non-existent document + // and there's no novelty write that would have created it + if (rebase.ok.value === undefined && address.path.length > 0) { + return { error: new NotFound(rebase.ok, address) }; + } + const { error } = write(rebase.ok, address, value); if (error) { return { error }; @@ -120,12 +142,14 @@ export class Chronicle { read( address: IMemoryAddress, - options?: { meta?: unknown }, + _options?: { meta?: unknown }, ): Result< IAttestation, | IStorageTransactionInconsistent | IInvalidDataURIError | IUnsupportedMediaTypeError + | INotFoundError + | ITypeMismatchError > { // Handle data URIs if (Address.isInline(address)) { @@ -146,7 +170,14 @@ export class Chronicle { // If we have not read nor written into overlapping memory address so // we'll read it from the local replica. - const loaded = attest(this.load(address)); + const state = this.load(address); + + // Check if document exists when trying to read from nested path + if (state.is === undefined && address.path.length > 0) { + return { error: new NotFound(attest(state), address) }; + } + + const loaded = attest(state); const { error, ok: invariant } = read(loaded, address); if (error) { return { error }; @@ -196,6 +227,25 @@ export class Chronicle { const source = attest(loaded); const { error, ok: merged } = changes.rebase(source); if (error) { + // During commit, NotFound and TypeMismatch errors should be treated as inconsistencies + // because we're trying to apply changes to something that has changed state + if (error.name === "NotFoundError") { + return { + error: new StateInconsistency({ + address: changes.address, + expected: "document to exist", + actual: undefined, + }), + }; + } else if (error.name === "TypeMismatchError") { + return { + error: new StateInconsistency({ + address: error.address, + expected: "object", + actual: error.actualType, + }), + }; + } return { error }; } // // If merged value is `undefined` and loaded fact was retraction @@ -383,7 +433,10 @@ class Novelty { */ claim( invariant: IAttestation, - ): Result { + ): Result< + IAttestation, + IStorageTransactionInconsistent | INotFoundError | ITypeMismatchError + > { const candidates = this.edit(invariant.address); for (const candidate of candidates) { @@ -475,7 +528,10 @@ class Changes { rebase( source: IAttestation, - ): Result { + ): Result< + IAttestation, + IStorageTransactionInconsistent | INotFoundError | ITypeMismatchError + > { let merged = source; for (const change of this.#model.values()) { if (Address.includes(source.address, change.address)) { diff --git a/packages/runner/test/attestation.test.ts b/packages/runner/test/attestation.test.ts index 63514a4d5..9189eca05 100644 --- a/packages/runner/test/attestation.test.ts +++ b/packages/runner/test/attestation.test.ts @@ -113,9 +113,11 @@ describe("Attestation Module", () => { }, "value"); expect(result.error).toBeDefined(); - expect(result.error?.name).toBe("StorageTransactionInconsistent"); - expect(result.error?.message).toContain("cannot write"); - expect(result.error?.message).toContain("expected an object"); + expect(result.error?.name).toBe("TypeMismatchError"); + expect(result.error?.message).toContain("Cannot write property"); + expect(result.error?.message).toContain( + "expected object but found string", + ); }); it("should fail when path leads through primitive", () => { @@ -136,7 +138,7 @@ describe("Attestation Module", () => { }, true); expect(result.error).toBeDefined(); - expect(result.error?.name).toBe("StorageTransactionInconsistent"); + expect(result.error?.name).toBe("TypeMismatchError"); }); it("should handle array modifications", () => { @@ -154,6 +156,112 @@ describe("Attestation Module", () => { expect(result.ok).toBeDefined(); expect(result.ok?.value).toEqual({ items: ["a", "modified", "c"] }); }); + + it("should allow writing to array with index 0", () => { + const source = { + address: { id: "test:array-0", type: "application/json", path: [] }, + value: { items: ["first", "second", "third"] }, + } as const; + + const result = Attestation.write(source, { + id: "test:array-0", + type: "application/json", + path: ["items", "0"], + }, "replaced"); + + expect(result.ok).toBeDefined(); + expect(result.ok?.value).toEqual({ items: ["replaced", "second", "third"] }); + }); + + it("should allow writing to array 'length' property", () => { + const source = { + address: { id: "test:array-length", type: "application/json", path: [] }, + value: { items: ["a", "b", "c", "d", "e"] }, + } as const; + + const result = Attestation.write(source, { + id: "test:array-length", + type: "application/json", + path: ["items", "length"], + }, 3); + + expect(result.ok).toBeDefined(); + expect(result.ok?.value).toEqual({ items: ["a", "b", "c"] }); + }); + + it("should fail when writing to array with negative index", () => { + const source = { + address: { id: "test:array-negative", type: "application/json", path: [] }, + value: { items: ["a", "b", "c"] }, + } as const; + + const result = Attestation.write(source, { + id: "test:array-negative", + type: "application/json", + path: ["items", "-1"], + }, "value"); + + expect(result.error).toBeDefined(); + expect(result.error?.name).toBe("TypeMismatchError"); + expect(result.error?.message).toContain("Cannot write property"); + expect(result.error?.message).toContain("expected object but found array"); + }); + + it("should fail when writing to array with non-integer numeric key", () => { + const source = { + address: { id: "test:array-float", type: "application/json", path: [] }, + value: { items: ["a", "b", "c"] }, + } as const; + + const result = Attestation.write(source, { + id: "test:array-float", + type: "application/json", + path: ["items", "1.5"], + }, "value"); + + expect(result.error).toBeDefined(); + expect(result.error?.name).toBe("TypeMismatchError"); + expect(result.error?.message).toContain("Cannot write property"); + expect(result.error?.message).toContain("expected object but found array"); + }); + + it("should fail when writing to array with string key (not 'length')", () => { + const source = { + address: { id: "test:array-string", type: "application/json", path: [] }, + value: { items: ["a", "b", "c"] }, + } as const; + + const result = Attestation.write(source, { + id: "test:array-string", + type: "application/json", + path: ["items", "someProperty"], + }, "value"); + + expect(result.error).toBeDefined(); + expect(result.error?.name).toBe("TypeMismatchError"); + expect(result.error?.message).toContain("Cannot write property"); + expect(result.error?.message).toContain("expected object but found array"); + }); + + it("should handle writing to large array indices", () => { + const source = { + address: { id: "test:array-large", type: "application/json", path: [] }, + value: { items: ["a", "b", "c"] }, + } as const; + + const result = Attestation.write(source, { + id: "test:array-large", + type: "application/json", + path: ["items", "10"], + }, "sparse"); + + expect(result.ok).toBeDefined(); + const resultValue = result.ok?.value as { items: any[] }; + expect(resultValue.items[10]).toBe("sparse"); + expect(resultValue.items.length).toBe(11); + expect(resultValue.items[3]).toBeUndefined(); + expect(resultValue.items[9]).toBeUndefined(); + }); }); describe("read function", () => { @@ -231,9 +339,11 @@ describe("Attestation Module", () => { }); expect(result.error).toBeDefined(); - expect(result.error?.name).toBe("StorageTransactionInconsistent"); - expect(result.error?.message).toContain("cannot read"); - expect(result.error?.message).toContain("encountered: 42"); + expect(result.error?.name).toBe("TypeMismatchError"); + expect(result.error?.message).toContain("Cannot read property"); + expect(result.error?.message).toContain( + "expected object but found number", + ); }); it("should fail when reading through null", () => { @@ -249,7 +359,7 @@ describe("Attestation Module", () => { }); expect(result.error).toBeDefined(); - expect(result.error?.name).toBe("StorageTransactionInconsistent"); + expect(result.error?.name).toBe("TypeMismatchError"); }); it("should handle array access", () => { @@ -309,7 +419,7 @@ describe("Attestation Module", () => { }); expect(result.error).toBeDefined(); - expect(result.error?.name).toBe("StorageTransactionInconsistent"); + expect(result.error?.name).toBe("NotFoundError"); }); }); @@ -517,7 +627,7 @@ describe("Attestation Module", () => { }); expect(result.error).toBeDefined(); - expect(result.error?.name).toBe("StorageTransactionInconsistent"); + expect(result.error?.name).toBe("TypeMismatchError"); }); it("should handle partial source paths", () => { @@ -553,11 +663,9 @@ describe("Attestation Module", () => { const error = new Attestation.NotFound(source, address); expect(error.name).toBe("NotFoundError"); - expect(error.message).toContain( - 'Can not resolve the "application/json" of "test:1"', + expect(error.message).toBe( + "Cannot access path [data, property] - path does not exist", ); - expect(error.message).toContain("data.property"); - expect(error.message).toContain("non-object at data"); expect(error.source).toBe(source); expect(error.address).toBe(address); }); @@ -576,70 +684,51 @@ describe("Attestation Module", () => { const error = new Attestation.NotFound(source, address); const withSpace = error.from(space); - expect(withSpace.space).toBe(space); - expect(withSpace.message).toContain(`from "${space}"`); + // NotFound error now returns the same instance from .from() + expect(withSpace).toBe(error); + expect(withSpace.message).toBe( + "Cannot access path [property] - path does not exist", + ); }); }); - describe("WriteInconsistency", () => { - it("should create descriptive error message", () => { - const source = { - address: { id: "test:1", type: "application/json", path: ["data"] }, - value: 42, - } as const; + describe("TypeMismatchError", () => { + it("should create descriptive error message for write operation", () => { const address = { id: "test:1", type: "application/json", path: ["data", "property"], } as const; - const error = new Attestation.WriteInconsistency(source, address); - - expect(error.name).toBe("StorageTransactionInconsistent"); - expect(error.message).toContain("cannot write"); - expect(error.message).toContain("data.property"); - expect(error.message).toContain("expected an object"); - expect(error.message).toContain("encountered: 42"); - }); - - it("should support space context", () => { - const source = { - address: { id: "test:1", type: "application/json", path: [] }, - value: "string", - } as const; - const address = { - id: "test:1", - type: "application/json", - path: ["property"], - } as const; - - const error = new Attestation.WriteInconsistency(source, address); - const withSpace = error.from(space); + const error = new Attestation.TypeMismatchError( + address, + "number", + "write", + ); - expect(withSpace.space).toBe(space); - expect(withSpace.message).toContain(`in space "${space}"`); + expect(error.name).toBe("TypeMismatchError"); + expect(error.message).toContain("Cannot write property"); + expect(error.message).toContain("[data, property]"); + expect(error.message).toContain("expected object but found number"); }); - }); - describe("ReadInconsistency", () => { - it("should create descriptive error message", () => { - const source = { - address: { id: "test:1", type: "application/json", path: ["user"] }, - value: null, - } as const; + it("should create descriptive error message for read operation", () => { const address = { id: "test:1", type: "application/json", path: ["user", "name"], } as const; - const error = new Attestation.ReadInconsistency(source, address); + const error = new Attestation.TypeMismatchError( + address, + "null", + "read", + ); - expect(error.name).toBe("StorageTransactionInconsistent"); - expect(error.message).toContain("cannot read"); - expect(error.message).toContain("user.name"); - expect(error.message).toContain("expected an object"); - expect(error.message).toContain("encountered: null"); + expect(error.name).toBe("TypeMismatchError"); + expect(error.message).toContain("Cannot read property"); + expect(error.message).toContain("[user, name]"); + expect(error.message).toContain("expected object but found null"); }); }); diff --git a/packages/runner/test/chronicle.test.ts b/packages/runner/test/chronicle.test.ts index 715ddd505..76418368b 100644 --- a/packages/runner/test/chronicle.test.ts +++ b/packages/runner/test/chronicle.test.ts @@ -431,7 +431,7 @@ describe("Chronicle", () => { }, "value"); expect(writeResult.error).toBeDefined(); - expect(writeResult.error?.name).toBe("StorageTransactionInconsistent"); + expect(writeResult.error?.name).toBe("TypeMismatchError"); }); it("should handle reading invalid nested paths", () => { @@ -452,7 +452,7 @@ describe("Chronicle", () => { }); expect(result.error).toBeDefined(); - expect(result.error?.name).toBe("StorageTransactionInconsistent"); + expect(result.error?.name).toBe("TypeMismatchError"); }); it("should handle writing to invalid nested paths", () => { @@ -473,7 +473,7 @@ describe("Chronicle", () => { ); expect(result.error).toBeDefined(); - expect(result.error?.name).toBe("StorageTransactionInconsistent"); + expect(result.error?.name).toBe("TypeMismatchError"); }); it("should handle deleting properties with undefined", () => { @@ -954,7 +954,7 @@ describe("Chronicle", () => { }, "Alice"); expect(writeResult.error).toBeDefined(); - expect(writeResult.error?.name).toBe("StorageTransactionInconsistent"); + expect(writeResult.error?.name).toBe("TypeMismatchError"); }); it("should fail write when nested data conflicts with non-existent fact", () => { @@ -967,7 +967,7 @@ describe("Chronicle", () => { }, "some value"); expect(writeResult.error).toBeDefined(); - expect(writeResult.error?.name).toBe("StorageTransactionInconsistent"); + expect(writeResult.error?.name).toBe("NotFoundError"); }); it("should fail commit when read invariants change after initial read", async () => { @@ -1245,7 +1245,8 @@ describe("Chronicle", () => { "Cannot write to read-only address", ); expect(result.error!.message).toContain(address.id); - expect(result.error!.address).toEqual(address); + expect((result.error as Chronicle.ReadOnlyAddressError).address) + .toEqual(address); }); it("should fail to write to nested path in data URI", () => { @@ -1260,7 +1261,8 @@ describe("Chronicle", () => { expect(result.error).toBeDefined(); expect(result.error!.name).toBe("ReadOnlyAddressError"); - expect(result.error!.address).toEqual(address); + expect((result.error as Chronicle.ReadOnlyAddressError).address) + .toEqual(address); }); it("should fail to write undefined (delete) to data URI", () => { @@ -1597,9 +1599,9 @@ describe("Chronicle", () => { const result = chronicle.read(address); expect(result.error).toBeDefined(); - expect(result.error!.name).toBe("StorageTransactionInconsistent"); - expect(result.error!.message).toContain("cannot read"); - expect(result.error!.message).toContain("expected an object"); + expect(result.error!.name).toBe("TypeMismatchError"); + expect(result.error!.message).toContain("Cannot read property"); + expect(result.error!.message).toContain("expected object"); }); it("should not interfere with regular replica reads", () => { diff --git a/packages/runner/test/journal.test.ts b/packages/runner/test/journal.test.ts index 71cb35829..9a0e21296 100644 --- a/packages/runner/test/journal.test.ts +++ b/packages/runner/test/journal.test.ts @@ -632,7 +632,7 @@ describe("Journal", () => { }); expect(result.error).toBeDefined(); - expect(result.error?.name).toBe("StorageTransactionInconsistent"); + expect(result.error?.name).toBe("TypeMismatchError"); }); it("should handle writing to invalid nested paths", () => { @@ -653,7 +653,7 @@ describe("Journal", () => { ); expect(result.error).toBeDefined(); - expect(result.error?.name).toBe("StorageTransactionInconsistent"); + expect(result.error?.name).toBe("TypeMismatchError"); }); it("should handle deleting properties with undefined", () => { diff --git a/packages/runner/test/reactive-dependencies.test.ts b/packages/runner/test/reactive-dependencies.test.ts index 6e829c46a..a2eb9fb05 100644 --- a/packages/runner/test/reactive-dependencies.test.ts +++ b/packages/runner/test/reactive-dependencies.test.ts @@ -871,8 +871,8 @@ describe("determineTriggeredActions", () => { const result = determineTriggeredActions( dependencies, - { name: "Alice" }, - { name: "Bob" }, + { user: { name: "Alice" }, post: { title: "Hello" } }, + { user: { name: "Bob" }, post: { title: "Hello" } }, ["user"], ); expect(result).toEqual([action1]); @@ -886,8 +886,8 @@ describe("determineTriggeredActions", () => { const result = determineTriggeredActions( dependencies, - { c: 1 }, - { c: 2 }, + { a: { b: { c: 1 } } }, + { a: { b: { c: 2 } } }, ["a", "b"], ); expect(result).toEqual([action1]); @@ -901,8 +901,8 @@ describe("determineTriggeredActions", () => { const result = determineTriggeredActions( dependencies, - { title: "Old" }, - { title: "New" }, + { post: { title: "Old" }, user: { name: "Alice" } }, + { post: { title: "New" }, user: { name: "Alice" } }, ["post"], ); expect(result).toEqual([]); @@ -918,8 +918,8 @@ describe("determineTriggeredActions", () => { const result = determineTriggeredActions( dependencies, - { profile: { name: "Alice" }, settings: { theme: "dark" } }, - { profile: { name: "Bob" }, settings: { theme: "dark" } }, + { users: { "123": { profile: { name: "Alice" }, settings: { theme: "dark" } } } }, + { users: { "123": { profile: { name: "Bob" }, settings: { theme: "dark" } } } }, ["users", "123"], ); expect(result).toEqual([action1]); @@ -934,7 +934,7 @@ describe("determineTriggeredActions", () => { const result = determineTriggeredActions( dependencies, undefined, - { b: 1 }, + { a: { b: 1 } }, ["a"], ); expect(result).toEqual([action1]); @@ -953,12 +953,20 @@ describe("determineTriggeredActions", () => { const result = determineTriggeredActions( dependencies, { - profile: { name: "Alice" }, - settings: { theme: "dark" }, + users: { + "123": { + profile: { name: "Alice" }, + settings: { theme: "dark" }, + }, + }, }, { - profile: { name: "Alice Updated" }, // Changed - settings: { theme: "dark" }, + users: { + "123": { + profile: { name: "Alice Updated" }, // Changed + settings: { theme: "dark" }, + }, + }, }, ["users", "123"], ); diff --git a/packages/runner/test/storage-subscription.test.ts b/packages/runner/test/storage-subscription.test.ts index 88e6e06b1..435926e92 100644 --- a/packages/runner/test/storage-subscription.test.ts +++ b/packages/runner/test/storage-subscription.test.ts @@ -90,8 +90,8 @@ describe("Storage Subscription", () => { space, id: entityId, type: "application/json", - path: ["value"], - }, value); + path: [], + }, { value }); await tx.commit(); @@ -101,14 +101,8 @@ describe("Storage Subscription", () => { expect(commit.space).toBe(space); expect(commit.source).toBe(tx.tx); - expect([...commit.changes]).toContainEqual({ - address: { - id: entityId, - type: "application/json", - path: ["value"], - }, - after: value, - before: undefined, + expect([...commit.changes].map((c) => c.after)).toContainEqual({ + value, }); }); @@ -135,14 +129,8 @@ describe("Storage Subscription", () => { expect(commit.space).toBe(space); expect(commit.source).toBe(tx.tx); - expect([...commit.changes]).toContainEqual({ - address: { - id: cell.getAsNormalizedFullLink().id, - type: "application/json", - path: ["value"], - }, - before: undefined, - after: { test: "data" }, + expect([...commit.changes].map((c) => c.after)).toContainEqual({ + value: { test: "data" }, }); }); @@ -181,18 +169,12 @@ describe("Storage Subscription", () => { expect(commit2.source).toBe(tx.tx); // Both should have the same changes - const expectedChange = { - address: { - id: cell.getAsNormalizedFullLink().id, - type: "application/json", - path: ["value"], - }, - after: { value: 42 }, - before: undefined, - }; - - expect([...commit1.changes]).toContainEqual(expectedChange); - expect([...commit2.changes]).toContainEqual(expectedChange); + expect([...commit1.changes].map((c) => c.after)).toContainEqual({ + value: { value: 42 }, + }); + expect([...commit2.changes].map((c) => c.after)).toContainEqual({ + value: { value: 42 }, + }); }); }); @@ -310,7 +292,7 @@ describe("Storage Subscription", () => { const fact = Fact.assert({ the: "application/json", of: entityId, - is: { data: "to be pulled" }, + is: { value: { data: "to be pulled" } }, }); await memory.transact({ changes: Changes.from([fact]) }); @@ -332,14 +314,8 @@ describe("Storage Subscription", () => { expect(pull.type).toBe("pull"); expect(pull.space).toBe(space); - expect([...pull.changes]).toContainEqual({ - address: { - id: entityId, - type: "application/json", - path: ["value"], - }, - before: undefined, - after: { data: "to be pulled" }, + expect([...pull.changes].map((c) => c.after)).toContainEqual({ + value: { data: "to be pulled" }, }); }); }); @@ -552,9 +528,9 @@ describe("Storage Subscription", () => { expect(commit.space).toBe(space); expect([...commit.changes].length).toBeGreaterThanOrEqual(1); const change = [...commit.changes][0]; - expect(change.address.path).toEqual(["source"]); - expect(change.after).toEqual(JSON.stringify(cell.entityId)); - expect(change.before).toBeUndefined(); + // The actual value contains the full document where source field has the JSON string + expect(change.after).toEqual({ source: JSON.stringify(cell.entityId) }); + expect((change.before as any)?.source).toBeUndefined(); }); }); }); diff --git a/packages/runner/test/storage-transaction-shim.test.ts b/packages/runner/test/storage-transaction-shim.test.ts index adb9276a4..7efc6d8df 100644 --- a/packages/runner/test/storage-transaction-shim.test.ts +++ b/packages/runner/test/storage-transaction-shim.test.ts @@ -11,6 +11,7 @@ import type { } from "../src/storage/interface.ts"; import { createDoc } from "../src/doc.ts"; import { ShimStorageManager } from "../src/storage/transaction-shim.ts"; +import { getEntityId } from "../src/doc-map.ts"; const signer = await Identity.fromPassphrase("test operator"); const space = signer.did(); @@ -33,6 +34,7 @@ describe("StorageTransaction", () => { }); it("should create a transaction and read/write values", async () => { + if (!runtime.storage.shim) return; const transaction = runtime.edit(); // Check initial status @@ -44,8 +46,8 @@ describe("StorageTransaction", () => { space, id: "of:test-entity", type: "application/json", - path: ["value"], - }, {}); + path: [], + }, { value: {} }); expect(rootWriteResult.ok).toBeDefined(); @@ -118,6 +120,7 @@ describe("StorageTransaction", () => { }); it("should handle transaction abort", async () => { + if (!runtime.storage.shim) return; const transaction = runtime.edit(); // Abort the transaction @@ -153,14 +156,16 @@ describe("StorageTransaction", () => { space, id: "of:test-entity", type: "application/json", - path: ["value"], - }, { name: "test" }); + path: [], + }, { value: { name: "test" } }); expect(result.error).toBeUndefined(); expect(result.ok).toBeDefined(); }); it("should fail writing to nested path when document is not a record", () => { + if (!runtime.storage.shim) return; + const transaction = runtime.edit(); // First write a non-record value to the document @@ -168,8 +173,8 @@ describe("StorageTransaction", () => { space, id: "of:test-entity", type: "application/json", - path: ["value"], - }, "not a record"); + path: [], + }, { value: "not a record" }); // Try to write to a nested path const result = transaction.write({ @@ -180,12 +185,12 @@ describe("StorageTransaction", () => { }, "value"); expect(result.error).toBeDefined(); - expect(result.error!.message).toContain( - "not a record", - ); + expect(result.error!.name).toBe("TypeMismatchError"); }); it("should fail writing to deeply nested path when parent is not a record", () => { + if (!runtime.storage.shim) return; + const transaction = runtime.edit(); // First write a record with a non-record value at "a" @@ -193,8 +198,8 @@ describe("StorageTransaction", () => { space, id: "of:test-entity", type: "application/json", - path: ["value"], - }, { a: "not a record" }); + path: [], + }, { value: { a: "not a record" } }); // Try to write to a deeply nested path const result = transaction.write({ @@ -205,9 +210,7 @@ describe("StorageTransaction", () => { }, "value"); expect(result.error).toBeDefined(); - expect(result.error!.message).toContain( - "parent path [a] does not exist or is not a record", - ); + expect(result.error!.name).toBe("TypeMismatchError"); }); it("should allow writing to nested path when parent is a record", () => { @@ -218,8 +221,8 @@ describe("StorageTransaction", () => { space, id: "of:test-entity", type: "application/json", - path: ["value"], - }, { a: {} }); + path: [], + }, { value: { a: {} } }); // Write to a nested path const result = transaction.write({ @@ -241,8 +244,8 @@ describe("StorageTransaction", () => { space, id: "of:test-entity", type: "application/json", - path: ["value"], - }, { a: { b: { c: {} } } }); + path: [], + }, { value: { a: { b: { c: {} } } } }); // Write to a deeply nested path const result = transaction.write({ @@ -257,6 +260,7 @@ describe("StorageTransaction", () => { }); it("should fail writing to nested path when parent path doesn't exist", () => { + if (!runtime.storage.shim) return; const transaction = runtime.edit(); // First write a record to the document @@ -264,8 +268,8 @@ describe("StorageTransaction", () => { space, id: "of:test-entity", type: "application/json", - path: ["value"], - }, { existing: "value" }); + path: [], + }, { value: { existing: "value" } }); expect(writeResult.ok).toBeDefined(); // Try to write to a path where parent doesn't exist @@ -277,8 +281,9 @@ describe("StorageTransaction", () => { }, "value"); expect(result.error).toBeDefined(); + expect(result.error!.name).toBe("NotFoundError"); expect(result.error!.message).toContain( - "parent path [missing] does not exist or is not a record", + "parent path [missing] does not exist", ); }); @@ -290,8 +295,8 @@ describe("StorageTransaction", () => { space, id: "of:test-entity", type: "application/json", - path: ["value"], - }, { a: { b: { c: "not a record" } } }); + path: [], + }, { value: { a: { b: { c: "not a record" } } } }); // Try to write to a deeply nested path where parent is not a record const result = transaction.write({ @@ -302,9 +307,7 @@ describe("StorageTransaction", () => { }, "deep value"); expect(result.error).toBeDefined(); - expect(result.error!.name).toBe("NotFoundError"); - // Should set path to ["a", "b"] (the last valid parent path) - expect((result.error as INotFoundError).path).toEqual(["a", "b"]); + expect(result.error!.name).toBe("TypeMismatchError"); }); }); @@ -320,16 +323,16 @@ describe("StorageTransaction", () => { space, id: doc1Id, type: "application/json", - path: ["value"], - }, { foo: 1 }).ok, + path: [], + }, { value: { foo: 1 } }).ok, ).toBeDefined(); expect( transaction.write({ space, id: doc2Id, type: "application/json", - path: ["value"], - }, { bar: 2 }).ok, + path: [], + }, { value: { bar: 2 } }).ok, ).toBeDefined(); // Set doc1's sourceCell to doc2 const setSource = transaction.write({ @@ -337,7 +340,7 @@ describe("StorageTransaction", () => { id: doc1Id, type: "application/json", path: ["source"], - }, doc2Id); + }, JSON.stringify(getEntityId(doc2Id))); expect(setSource.ok).toBeDefined(); // Read back the sourceCell const readSource = transaction.read({ @@ -347,10 +350,12 @@ describe("StorageTransaction", () => { path: ["source"], }); expect(readSource.ok).toBeDefined(); - expect(readSource.ok?.value).toBe(doc2Id); + expect(readSource.ok?.value).toBe(JSON.stringify(getEntityId(doc2Id))); }); it("should error if path beyond 'source' is used", () => { + if (!runtime.storage.shim) return; + const transaction = runtime.edit(); const doc1Id = "of:doc1"; expect( @@ -380,6 +385,8 @@ describe("StorageTransaction", () => { }); it("should error if source doc does not exist", () => { + if (!runtime.storage.shim) return; + const transaction = runtime.edit(); const doc1Id = "of:doc1"; expect( @@ -401,6 +408,8 @@ describe("StorageTransaction", () => { }); it("should error if value for 'source' is not a URI string", () => { + if (!runtime.storage.shim) return; + const transaction = runtime.edit(); const doc1Id = "of:doc1"; expect( @@ -431,8 +440,8 @@ describe("StorageTransaction", () => { space, id: "of:test-entity", type: "application/json", - path: ["value"], - }, { foo: 123 }); + path: [], + }, { value: { foo: 123 } }); expect(writeResult.ok).toBeDefined(); expect(writeResult.error).toBeUndefined(); @@ -464,14 +473,16 @@ describe("StorageTransaction", () => { expect(notFound).toBeUndefined(); // Should throw for other errors (e.g., unsupported media type) - expect(() => - transaction.readOrThrow({ - space, - id: "of:test-entity", - type: "unsupported/type", - path: ["value"], - }) - ).toThrow("Unsupported media type"); + if (runtime.storage.shim) { // Only shim doesn't support other types + expect(() => + transaction.readOrThrow({ + space, + id: "of:test-entity", + type: "unsupported/type", + path: ["value"], + }) + ).toThrow("Unsupported media type"); + } }); }); @@ -510,6 +521,8 @@ describe("DocImpl shim notifications", () => { describe("setAtPath with transaction parameter", () => { it("should accept transaction parameter and work correctly", () => { + if (!runtime.storage.shim) return; + const entityId = { "/": "transaction-test-entity" }; const value = { initial: "value" }; @@ -522,6 +535,8 @@ describe("DocImpl shim notifications", () => { }); it("should handle nested paths with transaction parameter", () => { + if (!runtime.storage.shim) return; + const entityId = { "/": "nested-transaction-test-entity" }; const value = { initial: "value" }; @@ -539,6 +554,8 @@ describe("DocImpl shim notifications", () => { }); it("should handle array paths with transaction parameter", () => { + if (!runtime.storage.shim) return; + const entityId = { "/": "array-transaction-test-entity" }; const value = { items: [] }; @@ -551,6 +568,8 @@ describe("DocImpl shim notifications", () => { }); it("should handle schema validation with transaction parameter", () => { + if (!runtime.storage.shim) return; + const entityId = { "/": "schema-transaction-test-entity" }; const value = { name: "test" }; @@ -568,6 +587,8 @@ describe("DocImpl shim notifications", () => { }); it("should handle root path setting with transaction parameter", () => { + if (!runtime.storage.shim) return; + const entityId = { "/": "root-transaction-test-entity" }; const value = { initial: "value" }; @@ -582,6 +603,8 @@ describe("DocImpl shim notifications", () => { describe("storage notifications with shim storage manager", () => { it("should send notifications when shim storage manager is available", () => { + if (!runtime.storage.shim) return; + const entityId = { "/": "notification-test-entity" }; const value = { initial: "value" }; @@ -595,7 +618,6 @@ describe("DocImpl shim notifications", () => { }, }); - const previousValue = doc.getAtPath(["initial"]); doc.setAtPath(["initial"], "updated", undefined, tx.tx); expect(notifications).toHaveLength(1); @@ -610,11 +632,17 @@ describe("DocImpl shim notifications", () => { "value", "initial", ]); - expect(notifications[0].changes[0].before).toBe("value"); - expect(notifications[0].changes[0].after).toBe("updated"); + expect(notifications[0].changes[0].before).toEqual({ + value: { initial: "value" }, + }); + expect(notifications[0].changes[0].after).toEqual({ + value: { initial: "updated" }, + }); }); it("should send notifications for nested path changes", () => { + if (!runtime.storage.shim) return; + const entityId = { "/": "nested-notification-test-entity" }; const value = { user: { name: "John" } }; @@ -636,11 +664,17 @@ describe("DocImpl shim notifications", () => { "user", "name", ]); - expect(notifications[0].changes[0].before).toBe("John"); - expect(notifications[0].changes[0].after).toBe("Jane"); + expect(notifications[0].changes[0].before).toEqual({ + value: { user: { name: "John" } }, + }); + expect(notifications[0].changes[0].after).toEqual({ + value: { user: { name: "Jane" } }, + }); }); it("should send notifications for root value changes", () => { + if (!runtime.storage.shim) return; + const entityId = { "/": "root-notification-test-entity" }; const value = { initial: "value" }; @@ -659,11 +693,15 @@ describe("DocImpl shim notifications", () => { expect(notifications).toHaveLength(1); expect(notifications[0].changes[0].address.path).toEqual(["value"]); - expect(notifications[0].changes[0].before).toEqual({ initial: "value" }); - expect(notifications[0].changes[0].after).toEqual(newValue); + expect(notifications[0].changes[0].before).toEqual({ + value: { initial: "value" }, + }); + expect(notifications[0].changes[0].after).toEqual({ value: newValue }); }); it("should not send notifications when value doesn't change", () => { + if (!runtime.storage.shim) return; + const entityId = { "/": "no-change-notification-test-entity" }; const value = { name: "test" }; @@ -684,6 +722,8 @@ describe("DocImpl shim notifications", () => { }); it("should handle multiple notifications in sequence", () => { + if (!runtime.storage.shim) return; + const entityId = { "/": "multiple-notification-test-entity" }; const value = { counter: 0 }; @@ -702,14 +742,22 @@ describe("DocImpl shim notifications", () => { doc.setAtPath(["counter"], 3, undefined, tx.tx); expect(notifications).toHaveLength(3); - expect(notifications[0].changes[0].after).toBe(1); - expect(notifications[1].changes[0].after).toBe(2); - expect(notifications[2].changes[0].after).toBe(3); + expect(notifications[0].changes[0].after).toEqual({ + value: { counter: 1 }, + }); + expect(notifications[1].changes[0].after).toEqual({ + value: { counter: 2 }, + }); + expect(notifications[2].changes[0].after).toEqual({ + value: { counter: 3 }, + }); }); }); describe("integration with existing functionality", () => { it("should work with updates callbacks and storage notifications", () => { + if (!runtime.storage.shim) return; + const entityId = { "/": "integration-test-entity" }; const value = { status: "initial" }; @@ -747,6 +795,8 @@ describe("DocImpl shim notifications", () => { }); it("should handle frozen documents with transaction parameter", () => { + if (!runtime.storage.shim) return; + const entityId = { "/": "frozen-transaction-test-entity" }; const value = { data: "initial" }; @@ -759,6 +809,8 @@ describe("DocImpl shim notifications", () => { }); it("should work with ephemeral documents", () => { + if (!runtime.storage.shim) return; + const entityId = { "/": "ephemeral-transaction-test-entity" }; const value = { data: "initial" }; @@ -829,7 +881,7 @@ describe("Subscription Shim", () => { it("should remove subscriptions that return done: true", () => { let callCount = 0; const subscription = { - next(notification: any) { + next(_notification: any) { callCount++; return { done: true }; // Stop after first call }, @@ -857,7 +909,7 @@ describe("Subscription Shim", () => { it("should remove subscriptions that throw errors", () => { let callCount = 0; const subscription = { - next(notification: any) { + next(_notification: any) { callCount++; throw new Error("Test error"); }, @@ -1015,6 +1067,7 @@ describe("root value rewriting", () => { }); it("should not rewrite non-empty paths", () => { + if (!runtime.storage.shim) return; const transaction = runtime.edit(); // First create a document @@ -1045,6 +1098,8 @@ describe("root value rewriting", () => { }); it("should not rewrite non-objects", () => { + if (!runtime.storage.shim) return; // In real tx, this is allowed + const transaction = runtime.edit(); // Write non-object to empty path @@ -1136,6 +1191,6 @@ describe("data: URI behaviors", () => { } as IMemorySpaceAddress; const result = transaction.write(address, {}); expect(result.error).toBeDefined(); - expect(result.error?.name).toBe("UnsupportedMediaTypeError"); + expect(result.error?.name).toBe("ReadOnlyAddressError"); }); }); diff --git a/packages/runner/test/storage.test.ts b/packages/runner/test/storage.test.ts index 8c77d8250..2b0b01e8d 100644 --- a/packages/runner/test/storage.test.ts +++ b/packages/runner/test/storage.test.ts @@ -48,7 +48,10 @@ describe("Storage", () => { const testValue = { data: "test" }; testCell.send(testValue); - await testCell.sync(); + await tx.commit(); + tx = runtime.edit(); + + if (runtime.storage.shim) await testCell.sync(); const query = storageManager .mount(space) @@ -60,6 +63,7 @@ describe("Storage", () => { const [fact] = query.facts; + expect(fact).toBeDefined(); expect(fact.is).toEqual({ value: testValue }); }); @@ -78,7 +82,12 @@ describe("Storage", () => { }; testCell.send(testValue); - await testCell.sync(); + // Commit transaction to persist data + await tx.commit(); + await runtime.idle(); + tx = runtime.edit(); + + if (runtime.storage.shim) await testCell.sync(); const entry = storageManager.open(space).get( refCell.getAsNormalizedFullLink().id, @@ -101,7 +110,12 @@ describe("Storage", () => { }; testCell.send(testValue); - await testCell.sync(); + // Commit transaction to persist data + await tx.commit(); + await runtime.idle(); + tx = runtime.edit(); + + if (runtime.storage.shim) await testCell.sync(); const refCellURI = refCell.getAsNormalizedFullLink().id; const entry = storageManager.open(space).get(refCellURI); @@ -111,12 +125,15 @@ describe("Storage", () => { describe("doc updates", () => { it("should persist doc updates", async () => { - await testCell.sync(); - testCell.send("value 1"); testCell.send("value 2"); - await runtime.storage.synced(); + // Commit transaction to persist data + await tx.commit(); + await runtime.idle(); + tx = runtime.edit(); + + if (runtime.storage.shim) await testCell.sync(); const query = storageManager .mount(space) @@ -184,6 +201,8 @@ describe("Storage", () => { describe("ephemeral docs", () => { it("should not be loaded from storage", async () => { + if (!runtime.storage.shim) return; + const ephemeralCell = runtime.getCell( space, "ephemeral", @@ -204,11 +223,15 @@ describe("Storage", () => { describe("doc updates", () => { it("should persist doc updates with schema", async () => { - await testCell.sync(); - testCell.send("value 1"); testCell.send("value 2"); + await tx.commit(); + await runtime.idle(); + tx = runtime.edit(); + + if (runtime.storage.shim) await testCell.sync(); + await runtime.storage.synced(); const query = storageManager diff --git a/packages/runner/test/transaction-notfound.test.ts b/packages/runner/test/transaction-notfound.test.ts new file mode 100644 index 000000000..d0be6997c --- /dev/null +++ b/packages/runner/test/transaction-notfound.test.ts @@ -0,0 +1,204 @@ +import { describe, it } from "@std/testing/bdd"; +import { expect } from "@std/expect"; +import * as Transaction from "../src/storage/transaction.ts"; +import type { + ISpaceReplica, + IStorageManager, + MemorySpace, +} from "../src/storage/interface.ts"; +import { unclaimed } from "@commontools/memory/fact"; + +// Mock replica that simulates non-existent documents +class MockReplica implements ISpaceReplica { + private data = new Map(); + + constructor(private space: MemorySpace) {} + + did() { + return this.space; + } + + get(entry: { the: string; of: string }) { + const key = `${entry.of}:${entry.the}`; + return this.data.get(key); + } + + commit() { + return Promise.resolve({ ok: {} as any }); + } + + // Helper to set test data + setData(id: string, type: string, value: any) { + const key = `${id}:${type}`; + this.data.set(key, { the: type, of: id, is: value }); + } +} + +// Mock storage manager +class MockStorageManager implements IStorageManager { + id = "test-storage"; + replicas = new Map(); + + open(space: MemorySpace) { + let replica = this.replicas.get(space); + if (!replica) { + replica = new MockReplica(space); + this.replicas.set(space, replica); + } + return { replica } as any; + } + + edit() { + return Transaction.create(this); + } + + subscribe() {} +} + +describe("Transaction NotFound Behavior", () => { + const testSpace: MemorySpace = "did:test:space"; + + it("should return NotFoundError when reading from non-existent document", () => { + const storage = new MockStorageManager(); + const tx = storage.edit(); + + const result = tx.read({ + space: testSpace, + id: "doc:1", + type: "application/json", + path: ["value", "name"], + }); + + expect(result.error).toBeDefined(); + expect(result.error?.name).toBe("NotFoundError"); + expect(result.error?.message).toBe("Document not found: doc:1"); + }); + + it("should return NotFoundError when writing to non-existent document with nested path", () => { + const storage = new MockStorageManager(); + const tx = storage.edit(); + + const result = tx.write({ + space: testSpace, + id: "doc:1", + type: "application/json", + path: ["value", "name"], + }, "Alice"); + + expect(result.error).toBeDefined(); + expect(result.error?.name).toBe("NotFoundError"); + expect(result.error?.message).toBe("Document not found: doc:1"); + }); + + it("should succeed when writing to non-existent document with empty path", () => { + const storage = new MockStorageManager(); + const tx = storage.edit(); + + const result = tx.write({ + space: testSpace, + id: "doc:1", + type: "application/json", + path: [], + }, { name: "Alice" }); + + expect(result.ok).toBeDefined(); + expect(result.error).toBeUndefined(); + }); + + it("should return NotFoundError when reading non-existent path in existing document", () => { + const storage = new MockStorageManager(); + const replica = storage.open(testSpace).replica as MockReplica; + replica.setData("doc:1", "application/json", { age: 30 }); + + const tx = storage.edit(); + + const result = tx.read({ + space: testSpace, + id: "doc:1", + type: "application/json", + path: ["name"], + }); + + // Reading a non-existent property returns undefined, not NotFound + expect(result.ok).toBeDefined(); + expect(result.ok?.value).toBeUndefined(); + }); + + it("should return TypeMismatchError when traversing through non-object", () => { + const storage = new MockStorageManager(); + const replica = storage.open(testSpace).replica as MockReplica; + replica.setData("doc:1", "application/json", { name: "Alice" }); + + const tx = storage.edit(); + + const result = tx.read({ + space: testSpace, + id: "doc:1", + type: "application/json", + path: ["name", "length"], // Trying to access property of a string + }); + + expect(result.error).toBeDefined(); + expect(result.error?.name).toBe("TypeMismatchError"); + }); + + it("should handle writes creating new documents", () => { + const storage = new MockStorageManager(); + const tx = storage.edit(); + + // Write to root path of non-existent document should succeed + const writeResult = tx.write({ + space: testSpace, + id: "doc:new", + type: "application/json", + path: [], + }, { name: "Bob", age: 25 }); + + expect(writeResult.ok).toBeDefined(); + + // Now read it back + const readResult = tx.read({ + space: testSpace, + id: "doc:new", + type: "application/json", + path: ["name"], + }); + + expect(readResult.ok).toBeDefined(); + expect(readResult.ok?.value).toBe("Bob"); + }); + + it("should propagate NotFound through transaction reader", () => { + const storage = new MockStorageManager(); + const tx = storage.edit(); + const reader = tx.reader(testSpace); + + expect(reader.ok).toBeDefined(); + + const result = reader.ok!.read({ + id: "doc:missing", + type: "application/json", + path: ["value"], + }); + + expect(result.error).toBeDefined(); + expect(result.error?.name).toBe("NotFoundError"); + }); + + it("should propagate NotFound through transaction writer", () => { + const storage = new MockStorageManager(); + const tx = storage.edit(); + const writer = tx.writer(testSpace); + + expect(writer.ok).toBeDefined(); + + const result = writer.ok!.write({ + id: "doc:missing", + type: "application/json", + path: ["nested", "value"], + }, "test"); + + expect(result.error).toBeDefined(); + expect(result.error?.name).toBe("NotFoundError"); + }); +}); diff --git a/packages/runner/test/transaction.test.ts b/packages/runner/test/transaction.test.ts index f41005944..cd7913bf0 100644 --- a/packages/runner/test/transaction.test.ts +++ b/packages/runner/test/transaction.test.ts @@ -196,7 +196,10 @@ describe("StorageTransaction", () => { expect(booleanResult.ok).toBeDefined(); // Test nested object metadata - const nestedMeta = { config: { nested: { value: "deep" } }, array: [1, 2, 3] }; + const nestedMeta = { + config: { nested: { value: "deep" } }, + array: [1, 2, 3], + }; const nestedResult = transaction.read(address, { meta: nestedMeta }); expect(nestedResult.ok).toBeDefined(); @@ -464,7 +467,7 @@ describe("StorageTransaction", () => { of: "user:consistency", is: { name: "Initial", version: 1 }, }); - + const initialCommit = await replica.commit({ facts: [v1], claims: [], @@ -495,7 +498,7 @@ describe("StorageTransaction", () => { is: { name: "Modified", version: 2 }, cause: v1, }); - + const modifyCommit = await replica.commit({ facts: [v2], claims: [], @@ -503,7 +506,10 @@ describe("StorageTransaction", () => { expect(modifyCommit.ok).toBeDefined(); // Verify the replica state actually changed - const updatedState = replica.get({ the: "application/json", of: "user:consistency" }); + const updatedState = replica.get({ + the: "application/json", + of: "user:consistency", + }); expect(updatedState?.is).toEqual({ name: "Modified", version: 2 }); // Now attempt to commit - should fail due to read invariant violation @@ -609,7 +615,7 @@ describe("StorageTransaction", () => { const result = transaction.read(nestedAddress); expect(result.error).toBeDefined(); - expect(result.error?.name).toBe("StorageTransactionInconsistent"); + expect(result.error?.name).toBe("TypeMismatchError"); }); it("should handle writing to invalid nested paths", () => { @@ -631,7 +637,7 @@ describe("StorageTransaction", () => { const result = transaction.write(nestedAddress, "value"); expect(result.error).toBeDefined(); - expect(result.error?.name).toBe("StorageTransactionInconsistent"); + expect(result.error?.name).toBe("TypeMismatchError"); }); }); @@ -701,4 +707,4 @@ describe("StorageTransaction", () => { } }); }); -}); \ No newline at end of file +});