-
Notifications
You must be signed in to change notification settings - Fork 1
feat: add internal-message enqueue and snapshot incarnation APIs #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -116,6 +116,7 @@ import { waitFor, Worker } from "./worker.js" | |
| import { EffectWorker } from "./effect-worker.js" | ||
| import type { WakeUpRole } from "./wake-up.js" | ||
| import { withDatabaseDeadline } from "./database/deadline.js" | ||
| import type { DatabaseConnection } from "./database/types.js" | ||
|
|
||
| interface RegisteredActor { | ||
| actorClass: ActorClass | ||
|
|
@@ -124,6 +125,13 @@ interface RegisteredActor { | |
| queries: ReadonlySet<string> | ||
| } | ||
|
|
||
| export interface SnapshotWithIncarnation<ActorType extends Actor> { | ||
| snapshot: ActorSnapshot<ActorType> | ||
| instanceId: string | ||
| revision: string | ||
| createdAtMs: number | ||
| } | ||
|
|
||
| interface PayloadProjectionSnapshot { | ||
| state: JsonObject | ||
| instanceId: string | ||
|
|
@@ -683,6 +691,27 @@ export class SolidObjectsRuntime { | |
| argumentsValue: {}, | ||
| authorizationContext: options.authorizationContext, | ||
| }) | ||
| const built = await this.buildSnapshotWithIncarnation(reference) | ||
| return built.snapshot | ||
| } | ||
|
|
||
| async snapshotWithIncarnation<ActorType extends Actor>( | ||
| reference: ActorReferenceCore<ActorType>, | ||
| options: SnapshotOptions = {}, | ||
| ): Promise<SnapshotWithIncarnation<ActorType>> { | ||
| await this.authorize({ | ||
| kind: "query", | ||
| reference, | ||
| operation: "__snapshot__", | ||
| argumentsValue: {}, | ||
| authorizationContext: options.authorizationContext, | ||
| }) | ||
| return this.buildSnapshotWithIncarnation(reference) | ||
| } | ||
|
|
||
| private async buildSnapshotWithIncarnation<ActorType extends Actor>( | ||
| reference: ActorReferenceCore<ActorType>, | ||
| ): Promise<SnapshotWithIncarnation<ActorType>> { | ||
| const registered = this.fetchActor(reference.actorType) | ||
| const instance = await this.repository.findInstanceByIdentity( | ||
| reference.actorType, | ||
|
|
@@ -718,7 +747,12 @@ export class SolidObjectsRuntime { | |
| ) { | ||
| throw new QueryMutatedState("snapshot getters must not mutate actor state or stage work") | ||
| } | ||
| return readonlyCopy(snapshot) as ActorSnapshot<ActorType> | ||
| return { | ||
| snapshot: readonlyCopy(snapshot) as ActorSnapshot<ActorType>, | ||
| instanceId: instance?.id ?? "0", | ||
| revision: String(instance?.state_revision ?? 0), | ||
| createdAtMs: Number(instance?.created_at_ms ?? 0), | ||
| } | ||
| } | ||
|
|
||
| async subscriptionSnapshot(options: { | ||
|
|
@@ -1443,6 +1477,66 @@ export class SolidObjectsRuntime { | |
| } | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The new Rule Used: What: Disallow the use of Prompt To Fix With AIThis is a comment left during a code review.
Path: src/runtime.ts
Line: 1478
Comment:
**Generic default violates type rule**
The new `Result = unknown` annotation violates the repository requirement to use concrete TypeScript types, forcing consumers that omit the generic to narrow or assert the message result. Replace it with the concrete result type supported by message result storage.
**Rule Used:** What: Disallow the use of `unknown` in TypeScript ... ([source](https://app.greptile.com/craftsmanfounder/-/custom-context?memory=af673ae0-6488-4c8b-8b4a-1bfea4eb4de7))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||
|
|
||
| async enqueueInternalMessage<Result = unknown>(options: { | ||
| actorType: string | ||
| actorId: string | ||
| operation: string | ||
| argumentsValue?: JsonObject | ||
| idempotencyKey?: string | ||
| }): Promise<MessageReference<Result>> { | ||
| const { actorType, actorId, operation, argumentsValue = {}, idempotencyKey } = options | ||
| const actor = this.fetchActor(actorType) | ||
| if (!actor.operations.has(operation)) { | ||
| throw new UnknownOperation(`unknown operation ${JSON.stringify(operation)}`) | ||
| } | ||
| const argumentsObject = jsonObject(argumentsValue, { maxBytes: this.settings.maxPayloadBytes }) | ||
| const message = await this.repository.enqueue({ | ||
| actorType, | ||
| actorId, | ||
| operation, | ||
| deliveryMode: "internal", | ||
| arguments: argumentsObject, | ||
| initialState: initialStateFor(actor.definition), | ||
| stateVersion: actor.definition.stateVersion, | ||
| ...(idempotencyKey === undefined ? {} : { idempotencyKey }), | ||
| }) | ||
| this.announceInternalMessage(message) | ||
| return this.messageReferenceFromRow<Result>(message) | ||
| } | ||
|
|
||
| async enqueueInternalMessageInTransaction( | ||
| connection: DatabaseConnection, | ||
| options: { | ||
| actorType: string | ||
| actorId: string | ||
| operation: string | ||
| argumentsValue?: JsonObject | ||
| idempotencyKey?: string | ||
| }, | ||
| ): Promise<MessageRow> { | ||
| const { actorType, actorId, operation, argumentsValue = {}, idempotencyKey } = options | ||
| const actor = this.fetchActor(actorType) | ||
| if (!actor.operations.has(operation)) { | ||
| throw new UnknownOperation(`unknown operation ${JSON.stringify(operation)}`) | ||
| } | ||
| const argumentsObject = jsonObject(argumentsValue, { maxBytes: this.settings.maxPayloadBytes }) | ||
| return this.repository.enqueueInTransaction(connection, { | ||
| actorType, | ||
| actorId, | ||
| operation, | ||
| deliveryMode: "internal", | ||
| arguments: argumentsObject, | ||
| initialState: initialStateFor(actor.definition), | ||
| stateVersion: actor.definition.stateVersion, | ||
| ...(idempotencyKey === undefined ? {} : { idempotencyKey }), | ||
| }) | ||
| } | ||
|
|
||
| announceInternalMessage(message: MessageRow): void { | ||
| this.emitInstrumentation("message.enqueued", messageInstrumentation(message)) | ||
| this.wakeUp("actors") | ||
| } | ||
|
|
||
| private async enqueue<Result>(options: { | ||
| reference: ActorReferenceCore<Actor> | ||
| operation: string | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| export const VERSION = "0.13.3" | ||
| export const VERSION = "0.14.0" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| import { afterEach, describe, expect, it } from "vitest" | ||
| import { Actor } from "../src/actor.js" | ||
| import { createRuntime, type SolidObjectsRuntime } from "../src/runtime.js" | ||
| import type { SolidObjectsConfiguration } from "../src/configuration.js" | ||
| import { sqlite } from "../src/database/sqlite.js" | ||
| import { UnknownOperation } from "../src/errors.js" | ||
|
|
||
| class Counter extends Actor { | ||
| static override readonly actorType = "Counter" | ||
|
|
||
| count = 0 | ||
|
|
||
| increment({ amount = 1 }: { amount?: number } = {}): number { | ||
| this.count += amount | ||
| return this.count | ||
| } | ||
| } | ||
|
|
||
| let runtime: SolidObjectsRuntime | undefined | ||
|
|
||
| afterEach(async () => { | ||
| await runtime?.close() | ||
| runtime = undefined | ||
| }) | ||
|
|
||
| describe("enqueueInternalMessage", () => { | ||
| it("processes an internal message even when authorizeMessage rejects every request", async () => { | ||
| runtime = createRuntime(configuredSettings({ authorizeMessage: () => false })) | ||
| runtime.register(Counter) | ||
| await runtime.install() | ||
|
|
||
| await runtime.enqueueInternalMessage({ | ||
| actorType: Counter.actorType, | ||
| actorId: "c1", | ||
| operation: "increment", | ||
| argumentsValue: { amount: 5 }, | ||
| }) | ||
|
|
||
| expect(await runtime.worker().runUntilIdle()).toBe(1) | ||
| await expect(runtime.ref(Counter, "c1").snapshot()).resolves.toMatchObject({ count: 5 }) | ||
| }) | ||
|
|
||
| it("still rejects an operation the actor does not define", async () => { | ||
| runtime = createRuntime(configuredSettings()) | ||
| runtime.register(Counter) | ||
| await runtime.install() | ||
|
|
||
| await expect( | ||
| runtime.enqueueInternalMessage({ | ||
| actorType: Counter.actorType, | ||
| actorId: "c1", | ||
| operation: "missing", | ||
| }), | ||
| ).rejects.toBeInstanceOf(UnknownOperation) | ||
| }) | ||
| }) | ||
|
|
||
| describe("enqueueInternalMessageInTransaction", () => { | ||
| it("enqueues inside a caller-supplied transaction and runs after announce", async () => { | ||
| runtime = createRuntime(configuredSettings()) | ||
| runtime.register(Counter) | ||
| await runtime.install() | ||
| const activeRuntime = runtime | ||
|
|
||
| const message = await activeRuntime.settings.database.transaction((connection) => | ||
| activeRuntime.enqueueInternalMessageInTransaction(connection, { | ||
| actorType: Counter.actorType, | ||
| actorId: "c1", | ||
| operation: "increment", | ||
| argumentsValue: { amount: 3 }, | ||
| }), | ||
| ) | ||
| activeRuntime.announceInternalMessage(message) | ||
|
|
||
| expect(await activeRuntime.worker().runUntilIdle()).toBe(1) | ||
| await expect(activeRuntime.ref(Counter, "c1").snapshot()).resolves.toMatchObject({ count: 3 }) | ||
| }) | ||
|
|
||
| it("rolls back with the caller's transaction if the surrounding work fails", async () => { | ||
| runtime = createRuntime(configuredSettings()) | ||
| runtime.register(Counter) | ||
| await runtime.install() | ||
| const activeRuntime = runtime | ||
|
|
||
| await expect( | ||
| activeRuntime.settings.database.transaction(async (connection) => { | ||
| await activeRuntime.enqueueInternalMessageInTransaction(connection, { | ||
| actorType: Counter.actorType, | ||
| actorId: "c1", | ||
| operation: "increment", | ||
| argumentsValue: { amount: 3 }, | ||
| }) | ||
| throw new Error("caller failed after enqueueing") | ||
| }), | ||
| ).rejects.toThrow("caller failed after enqueueing") | ||
|
|
||
| expect(await activeRuntime.worker().runUntilIdle()).toBe(0) | ||
| }) | ||
| }) | ||
|
|
||
| function configuredSettings( | ||
| overrides: Partial<SolidObjectsConfiguration> = {}, | ||
| ): SolidObjectsConfiguration { | ||
| return { | ||
| database: sqlite({ path: ":memory:" }), | ||
| authorizeMessage: () => true, | ||
| authorizeQuery: () => true, | ||
| authorizeDestroy: () => true, | ||
| pollingIntervalMilliseconds: 1, | ||
| syncPollingIntervalMilliseconds: 1, | ||
| maxAttempts: 2, | ||
| ...overrides, | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When an actor is destroyed and recreated within the same database-clock millisecond, both incarnations receive the same
createdAtMs, so a stale derived write can pass the advertised incarnation fence and overwrite or publish data as though it came from the current incarnation.Prompt To Fix With AI