diff --git a/CHANGELOG.md b/CHANGELOG.md index 23959c6..2fafd62 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,9 +32,54 @@ API surface). (`scripts/check_deprecated_markers.sh`) enforcing that every `[[deprecated("...")]]` marker names a replacement and a target removal version. +- **Keyed, shareable model instances.** A model declares a nested `PrimaryKey` + alias; actions declare which field carries it (`BRIDGE_KEY_FROM`) or that + their result establishes it (`BRIDGE_KEY_FROM_RESULT`). + `BridgeHandler` joins a server-side directory keyed on + `(typeId, primary)`, so handlers in one process — or in two clients over one + `RemoteServer` — reach the same instance. Adds `attach()`, `primary()` and + `instances()` to the handler, the `primary`/`shared` envelope fields and the + `attach`/`assign`/`instances` wire kinds, all additive. See + `docs/spec/core/shared_instances.md`. +- **Instance subscriptions.** `BridgeHandler::subscribe(cb)`, keyed on the + result/state type, fires whenever an `R` is produced on the instance the + handler is attached to — by any handler attached to it. Fan-out is per + `Bridge`; there is no server-initiated push. See + `docs/spec/core/bridge.md#subscription-semantics`. + +### Changed + +- **`RemoteServer::closeConnection` now releases one reference per attachment + rather than erasing every model in the scope.** Required by cross-client + instance sharing: otherwise one client's disconnect destroys an instance + another client is still attached to. A connection scope records an + attachment *count* per instance for the same reason. Unshared instances have + exactly one attacher, so their lifetime is unchanged. +- `examples/bank` is reshaped onto stateful, keyed models: `AccountModel` holds + one account in memory keyed by account id, and the new `CustomerModel` takes + the per-owner half (`ListAccounts`/`OpenAccount`). + +### Removed + +- **The reactive-draft mechanism** — `BridgeHandler::set<&A::field>`, + `reset`, the action-keyed `subscribe`, and their in-flight coalescing. + Its job is done better by a stateful model holding the draft itself, and + `subscribe` now means instance subscriptions (above). `morph::flows::FlowSession` + already owned its own draft tuple and now gates on `ActionValidator` and + dispatches directly; its public API, the `w-*`/`app-*` schema and + `WizardView.qml` are unchanged. `ActionValidator` keeps its server-side + validation role and loses only its draft-readiness one. Pre-1.0, per + `docs/spec/VERSIONING.md`. ### Fixed +- `CustomerModel::execute(ListAccounts)` dereferenced `QuerySingle`'s optional + unchecked (inherited verbatim from the old `AccountModel`); it now throws + `NotFound`. +- A flaky assertion in `tests/test_concurrency_invariants.cpp`: `succeeded > 0` + during backend churn is a scheduling race, not an invariant — under a + thread-serialising tool the switcher can cancel every in-flight call. The + same structural property is now asserted against the quiesced bridge. - Stale pre-JSON "5-part"/"6-part protocol" wording in `docs/ARCHITECTURE.md` and a test comment — the wire has been a JSON `Envelope` since it superseded the pipe-delimited protocol. diff --git a/README.md b/README.md index f5f4162..4cd3716 100644 --- a/README.md +++ b/README.md @@ -120,7 +120,7 @@ and the call site above are unchanged. `Bridge` and share it. - **`BridgeHandler`** — your typed, GUI-facing handle to one model type `M`. It registers `M` on the bridge on construction and deregisters on destruction - (RAII). This is the object you call `.execute(...)` / `.subscribe(...)` on. + (RAII). This is the object you call `.execute(...)` / `.subscribe(...)` on. Create one per model type, wherever in the UI you need to talk to that model. ## Multiple models across multiple files @@ -382,14 +382,19 @@ morph is a young, actively developed library with thorough test coverage of its core. It is honest about the following boundaries — read the per-subsystem specs in [`docs/spec/`](docs/spec) before relying on any of these in production: -- **Security is app-supplied.** The wire protocol has no version negotiation, - no message-size or timeout bounds, and no built-in authentication; `Context` - identity is unauthenticated and `RemoteServer` model ids are guessable - sequential integers with control messages unauthorized. `RemoteServer` - assumes a trusted, authenticated transport — it is not a hardened - public-internet server as shipped. +- **Security is app-supplied.** There is no built-in authentication: `Context` + identity is whatever the client claims until an `IAuthorizer` verifies it, and + the default authorizer allows everything. Protocol version negotiation, + message-size and timeout bounds, opaque model ids, and register/per-instance + authorization hooks all ship (see `docs/spec/security.md`), but they are + **opt-in** — a server that configures none of them assumes a trusted, + authenticated transport and is not a hardened public-internet server. - **`Completion` is a leaf callback primitive**, not a composable future: one handler per outcome, no `T→U` chaining, no `co_await`, no cancellation. +- **Instance subscriptions are best-effort and in-process.** `subscribe` + fans out to handlers on the same `Bridge`; there is no server-initiated push, + so two separate clients sharing an instance do not see each other's results + until they ask again. No replay, no durability, no coalescing. - **Exact numbers are fixed-width.** `Rational` is an `int64` pair; `+`/`-`/`*` can overflow (undefined behaviour) rather than returning an error, and high decimal precision shrinks the representable magnitude. Wire input is *clamped*, @@ -399,8 +404,10 @@ in [`docs/spec/`](docs/spec) before relying on any of these in production: the store/log divergence gap by opting into `IModelHolder::setOutboxManaged` + `journal::OutboxRelay` (see `docs/spec/journal/journal.md`); a model that doesn't opt in keeps the default fire-after-success append. -- **Offline durability is bring-your-own.** Only an in-memory queue ships; the - crash-safety story depends on a durable queue you implement. +- **Offline durability is opt-in.** `FileOfflineQueue` (NDJSON, always built) + and `SqliteOfflineQueue` (`MORPH_BUILD_OFFLINE_SQLITE`) both persist across + restarts; the in-memory queue remains the default, so crash-safety depends on + selecting a durable one. - **Registration is global and macro-driven** (per-TU, static-init, string type ids); there is no runtime deregistration and unknown ids fail at runtime, not compile time. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 557e37a..24b52b8 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -310,7 +310,7 @@ Because these are mutually exclusive per topology, recording is automatically se **`Loggable`** (`morph::model::Loggable::{Yes,No}`) is a strong-typed opt-out on the existing `BRIDGE_REGISTER_ACTION` macro (an optional 4th argument; no separate registration macro). Default is `Yes` — every action is recorded unless explicitly marked `Loggable::No` (typically pure queries like `GetAccount`/`ListAccounts`). Hand-written `ActionTraits` specialisations that predate this member (as used in several tests) are unaffected: `morph::model::detail::actionLoggable()` defaults to `Yes` when the member is absent, via a `HasLoggableFlag` concept exactly like `ActionValidator`'s `HasValidate`. -**`ActionLogPolicy::coalesce`** (default `false`) decides whether repeated executions of the same action against the same entity should collapse to the latest occurrence at a checkpoint, or whether every occurrence is a distinct, permanent fact. This matters because the fielded/reactive `set<...>` mechanism (see "Subscriptions and fielded actions" below) can already fire the same action many times in a row — without coalescing, every keystroke-driven re-fire would become a permanent log entry. `false` is correct for anything resembling a business event (a deposit); `true` is for drafts/settings where only the final value matters. +**`ActionLogPolicy::coalesce`** (default `false`) decides whether repeated executions of the same action against the same entity should collapse to the latest occurrence at a checkpoint, or whether every occurrence is a distinct, permanent fact. This matters because a form driving one action per edit can fire the same action many times in a row — without coalescing, every keystroke-driven re-fire would become a permanent log entry. `false` is correct for anything resembling a business event (a deposit); `true` is for drafts/settings where only the final value matters. **`SessionLog`** (`journal.hpp`) is where coalescing actually happens. It keeps full, uncoalesced history in memory (the raw material for `undoLast()`), and `checkpoint(durableSink)` reduces everything appended since the last checkpoint by `(modelType, entityKey, actionType)` — keeping only the latest entry where `coalesce == true`, every entry otherwise — before forwarding the reduced set to the real sink. `undoLast()` needs no inverse operations: it drops the most recent entry and calls `journal::replay()` over what remains, reusing the same `ActionDispatcher`/`ModelRegistryFactory` `RemoteServer` already relies on for dispatch. This is not a workaround — a model's entire state genuinely is "initial state plus its ordered actions replayed," so reconstructing it by replay is the direct statement of that fact, not a special case. @@ -412,52 +412,51 @@ handler.execute(MyAction{21}) .onError([](std::exception_ptr e) { /* runs on GUI thread */ }); ``` -## Subscriptions and fielded actions +## Instance subscriptions -The framework offers two complementary surfaces for invoking actions from the GUI: +The framework offers two complementary surfaces for talking to a model: -1. **One-shot**: `handler.execute(action) → Completion`. The full action is built in the GUI and sent in a single call. Suitable for actions that fire on a button click ("delete this order", "submit form"). -2. **Fielded / reactive**: `handler.subscribe(cb)` + `handler.set<&A::field>(value)`. Field values stream into a client-side draft, a per-action validator decides when the draft is ready, and the framework dispatches `model.execute(draft)` and pushes the result to the subscriber. Suitable for forms where each widget edits one field and the GUI should respond live as the user types. +1. **One-shot**: `handler.execute(action) -> Completion`. The action is built + at the call site and dispatched in one call. +2. **Observing**: `handler.subscribe(cb)` fires whenever an `R` is produced + on *the instance this handler is attached to* — by this handler, by another + handler sharing that instance, or by another screen entirely. Suitable for a + view that renders some model state and must stay current when anything + changes it. -### API - -```cpp -// Per-action validator — template specialisation, no Model coupling. -template -struct morph::model::ActionValidator { - static bool ready(const A&) noexcept { return true; } // default: one-shot -}; +A subscription names the **result/state type**, not an action. The subscriber +describes what it renders rather than what somebody else must call to produce +it, so adding an action that also yields an `R` never breaks an existing +subscriber. -// User specialises (or uses the macro) for actions with fielded readiness. -BRIDGE_REGISTER_VALIDATOR(FormAction, [](const FormAction& a) { - return a.a != 0.0 && a.b != 0.0 && a.c != 0.0; -}) -``` +### API ```cpp -morph::bridge::BridgeHandler handler{bridge, &guiExec}; +morph::bridge::BridgeHandler screen{bridge, guiExec}; +morph::bridge::BridgeHandler sidebar{bridge, guiExec}; -handler.subscribe([](double sum) { renderTotal(sum); }); +screen.attach(42); +sidebar.attach(42); // same instance -handler.set<&FormAction::a>(3.0); -handler.set<&FormAction::b>(5.0); -handler.set<&FormAction::c>(7.0); // validator passes → execute → callback fires +screen.subscribe([](AccountInfo info) { renderBalance(info); }); -handler.unsubscribe(); -handler.reset(); +sidebar.execute(Deposit{.amountMinor = 5000}); +// -> screen's callback runs: it never had to know Deposit exists ``` ### Behavior | Aspect | Default | |---|---| -| **Validator default** | `ActionValidator::ready` returns `true` for any action without a specialisation. First `set<>` triggers a fire. | -| **Re-fire** | Every `set<>` that lands a `ready()==true` state dispatches the action again — live recomputation. The draft persists between fires. | -| **Draft persistence** | Drafts survive successive fires, `unsubscribe`, and `Bridge::switchBackend`. Destroyed with the handler or via `reset()`. | -| **In-flight coalescing** | If patches land while a previous execute is in flight, exactly one re-fire is queued for when it completes — running with the latest draft snapshot. Further patches during the same flight collapse into that single pending re-fire. Matches typical reactive-UI behaviour. | -| **Subscriber cardinality** | One subscriber per `(handler, Action type)`. `subscribe(cb)` replaces any prior callback. | -| **No-subscriber fire** | If `set<>` triggers an execute but no subscriber is installed, the action still runs and the result is silently dropped. | -| **Subscription thread** | Callbacks always run on the executor passed at handler construction (`guiExec`). | +| **Keying** | On the result type `R`. Any action producing an `R` notifies. | +| **Scope** | The instance the handler is currently attached to. Matched at publish time, so a subscription follows a re-pointed handler. | +| **Subscriber cardinality** | One callback per `(handler, R)`. `subscribe(cb)` replaces any prior callback. | +| **Echo** | The originating handler is notified too — no "was this mine" bookkeeping in subscribers. | +| **Failures** | A failed action notifies nobody. | +| **Ordering** | Per instance, guaranteed by that instance's strand. Nothing is guaranteed between instances. | +| **Durability** | None. Best-effort and unbuffered: no replay, no cursor, no coalescing. | +| **Callback thread** | Always the executor passed at handler construction. | + ## Exact values, units, and schema-driven forms @@ -546,7 +545,7 @@ required empty-capable field is engaged" — `Quantity`, `Choice`, `Timestamp`, or any type with a `hasValue()`) intended as the body of the action's `validate()` — which the existing `ActionValidator` resolution picks up automatically. One declaration then drives the schema's `required` array, the -client-side submit gate, and the fielded-action readiness check. +client-side submit gate, and the server-side readiness check. ### `morph::time::Timestamp` and `morph::forms::Choice` — dates and combo boxes @@ -703,7 +702,7 @@ documented behavior. | Glaze for JSON | Reflects aggregate types automatically; no hand-written serialisation per action. | | `CompletionState` internal only | Keeps the public API free of state-handling machinery; implementation can change without breaking callers. | | JSON `Envelope` wire protocol | Self-describing and forward-compatible (unknown keys ignored); carries a `callId` so async WebSocket replies can be correlated back to pending `Completion` objects. | -| Client-side drafts for fielded actions | Avoids new wire messages, server-side draft state, and a server push channel. Patches never leave the client; only the full action is sent when the validator passes. | +| Subscriptions keyed on the result type, scoped to the instance | A subscriber is a renderer: it names the state it draws, not the actions that produce it, so a new action never breaks it. Scoping to the instance is what lets two screens on one shared model see each other's work without a query-invalidation vocabulary. | | `ActionValidator` is action-typed, not model-typed | Different actions on the same model have different readiness requirements; pinning the predicate to the action keeps GUI code oblivious to model internals. | | `set(value)` over `set(&Action::f, value)` | Member-pointer NTTP encodes both the action type and the field type; the call site stays terse without losing type safety. | | `Rational` wire codec canonicalises on read | Wire input is untrusted; every deserialised value passes the reducing constructor, so invariants hold no matter what a client sends. | diff --git a/docs/spec/core/bridge.md b/docs/spec/core/bridge.md index 0f6eea0..3fcc749 100644 --- a/docs/spec/core/bridge.md +++ b/docs/spec/core/bridge.md @@ -49,10 +49,9 @@ registers on construction, deregisters on destruction. (compile-time dispatch) and `executeJson(actionType, bodyJson)` (runtime dispatch via `ActionExecuteRegistry`). -For GUI-led workflows, `subscribe(cb)` registers a result callback, -`set<&Action::field>(value)` fills one field of an in-progress draft, and -`reset()` discards the draft. When all required fields are filled, the -handler automatically fires the action. +For GUI-led workflows, `subscribe(cb)` observes *the instance the handler is +attached to*: it fires whenever an `R` is produced there, by any handler +attached to it. `unsubscribe()` drops the callback. ## `HandlerBinding` @@ -213,8 +212,8 @@ destruction. Non-copyable. **Construction** takes a `Bridge&` and a GUI executor. Optionally accepts a pre-built `HandlerBinding` (for dependency injection). It captures the bridge's `liveness()` weak token into `_bridgeAlive`, holds a strong `Bridge&`, and -stores a `SubscriberState` shared pointer that supports the field-by-field and -subscription APIs. +and carries the sharing policy as its second template argument (see +`BridgeHandler` below). **Destruction** deregisters the binding via `Bridge::deregisterHandler` — but only if `_bridgeAlive.lock()` still succeeds. If the `Bridge` was already @@ -259,52 +258,23 @@ the reactive `set<>` path rather than trusting the raw wire body: if it returns `false` the executor throws `std::invalid_argument` and the completion resolves through `onError` (a proper error reply upstream) — the handler is never invoked with an invalid action. This closes a gap where the - request/reply path skipped the readiness/validity check that the reactive - `set<>` path (`tryFireImpl`) already performs. `ActionValidator::ready` + request/reply path skipped the readiness/validity check the dispatch paths + perform. `ActionValidator::ready` auto-detects a `bool validate() const` member and defaults to `true`, so actions without a validator dispatch exactly as before (backward compatible). -**`subscribe(cb)`** / **`subscribe(cb, errCb)`** stores a -result (and optional error) callback keyed by -`ActionTraits::typeId()`. Callbacks execute on the GUI executor. +**`unsubscribe()`** removes this handler's callback for result type `R`. -**`unsubscribe()`** clears both result and error callbacks for the -action type. - -**`set<&Action::field>(value)`** updates one field of the in-progress draft. -Uses `MemberPointerTraits` to recover the action and field types from the -pointer-to-member. After setting the value, recomputes any declared computed -fields on the snapshot (`morph::forms::recomputeAll`, [forms.md](../forms/forms.md), -a no-op for actions with no `computedFields`), then checks -`ActionValidator::ready(snapshot)`. If all required fields are -present, fires the action via `Bridge::executeVia` and delivers the result +**`subscribe(cb)`** registers `cb` against this handler's binding. The +subscription is matched at publish time by comparing the binding's current +instance, so it follows the handler when it re-points. Delivers the result to the registered `sink` callback. If a flight is already in progress, marks `pending = true` and refires when the current flight completes (debounce-like coalescence). On failure, the registered `errSink` is invoked; if none is registered, the error is logged via `morph::log::logError` -(tagged `[subscription:]`) rather than silently dropped. - -**`reset()`** discards the in-progress draft. **`guiExecutor()`** returns the executor passed at construction. -### SubscriberState - -```cpp -struct SubscriberState { - std::mutex mtx; - Bridge* bridge; - std::shared_ptr binding; - IExecutor* guiExec; - std::unordered_map entries; -}; -``` - -Keys are `std::string_view` pointing to `ActionTraits::typeId()` string -literals (static storage duration — keys never dangle). Each `SubscriberEntry` -holds a `draft` (`std::any` of the action struct), `sink`, `errSink`, and -`running`/`pending` flags for flight tracking. - ## `ActionExecuteRegistry` Process-level singleton (`instance()`). Maps `(modelTypeId, actionTypeId)` @@ -369,54 +339,55 @@ struct MemberPointerTraits { }; ``` -Compile-time decomposition of a pointer-to-data-member type. Used by -`BridgeHandler::set` to recover both the action type (`A`) -and the field type (`V`) from a single non-type template parameter, so -callers write `handler.set<&MyAction::c>(7.0)` with no redundant type -arguments. +Compile-time decomposition of a pointer-to-data-member type. Recovers both the +class (`A`) and the member type (`V`) from a single non-type template +parameter, so a caller names a field as `&MyAction::c` with no redundant type +arguments. Used by `morph::flows::FlowSession::set<>` +([workflows_navigation.md](../forms/workflows_navigation.md)) and by +`morph::forms`' computed-field declarations. ## Subscription semantics -The fielded/reactive surface (`subscribe`, `set`, `unsubscribe`, `reset`) is -built on the per-handler `SubscriberState`. Exactly one `SubscriberEntry` -exists per action type, keyed by `ActionTraits::typeId()`. The rules: - -- **One subscriber per `(handler, action type)`.** `subscribe(cb)` (or the - two-argument `subscribe(cb, errCb)`) *replaces* any callback previously - registered for `A` on this handler — there is no fan-out. The two-argument - overload additionally stores the error sink; the one-argument overload leaves - the existing `errSink` untouched. -- **Fire without a subscriber.** A `set<>`-triggered fire runs whether or not a - subscriber is installed. If no `sink` is registered when the result arrives, - the result is simply dropped (the entry exists because `set<>` created the - draft, but its `sink` is empty). Errors are different: with no `errSink` the - error is logged via `morph::log::logError` tagged `[subscription:]`, - never silently dropped. -- **Default validator fires on the first `set<>`.** `ActionValidator::ready` - returns `true` only for an action that has *neither* a - `BRIDGE_REGISTER_VALIDATOR` specialisation *nor* a `bool validate() const` - member — for such an action the very first `set<>` puts the - (single-field-populated) draft into a ready state and dispatches immediately. - Actions that need several fields before firing add a `validate()` member (the - preferred, macro-free path — auto-detected via the `HasValidate` concept) or - specialise the validator. -- **Every ready `set<>` re-fires.** Each `set<>` recomputes any declared - computed fields on the draft snapshot (`morph::forms::recomputeAll`, - [forms.md](../forms/forms.md), a no-op for actions with no - `computedFields`) before the `ready()` check, then — landing a - `ready()==true` snapshot — dispatches the action again with the recomputed - value already in place: live recomputation. This recompute is client-side - and **not authoritative** (for display only); every dispatch path below - recomputes it again, authoritatively, before `Model::execute`. Rapid patches - coalesce: while a flight is running, further `set<>` calls set - `pending=true`, and exactly one re-fire with the latest snapshot is issued - when the in-flight completion resolves (`consumeFlight`). -- **Draft lifetime.** A draft is created lazily on the first `set<>` for its - action type and persists across fires, across `unsubscribe()`, and across - `Bridge::switchBackend` (the draft lives in the handler's `SubscriberState`, - not the backend). It is destroyed only when the handler is destroyed or when - `reset()` is called. `unsubscribe()` clears both callbacks but leaves - the draft intact. +`subscribe(cb)` is keyed on the **result/state type**, not on an action. It +fires whenever an `R` is produced on the instance the handler is attached to — +by this handler, by another handler sharing that instance, or by another screen +entirely. The subscriber names *what it renders*, not what somebody else must +call to produce it, so adding an action that also yields an `R` never breaks an +existing subscriber. + +- **Scope is the instance, not the model type.** A subscription is matched at + publish time by comparing the binding's *current* instance, so re-pointing a + handler ([`attach`](#bridgehandlermodel-sharing)) moves its subscriptions with + it. A handler with no primary hears only its own results, because nothing else + is attached to what it holds. +- **One callback per `(handler, R)`.** Subscribing again replaces the previous + callback; there is no fan-out within a handler. +- **The originating handler is notified too.** Suppressing the echo would force + every subscriber to special-case "was this mine", which is exactly the + bookkeeping this replaces. +- **Callbacks run on the handler's executor**, as `.then` does. Two handlers in + one process with different executors each get their callback where they asked + for it. +- **Ordering is per instance.** Every action on an instance runs on that + instance's strand, so notifications are naturally ordered; nothing is + guaranteed *between* instances. +- **Failed actions notify nobody.** A notification is produced from a successful + result only. +- **Delivery is best-effort and unbuffered.** There is no replay, no cursor, no + checkpointing, and no coalescing — a model that emits at high frequency must + throttle itself. Sinks are snapshotted under the registry lock and invoked + outside it, so a subscriber that re-enters the bridge cannot deadlock. +- **Only copy-constructible results are published.** A result type that cannot + be copied is delivered to its caller's `Completion` as usual but is never + boxed for subscribers. +- **Fan-out is per `Bridge`, i.e. per client process.** This is the one place + subscriptions are narrower than instance sharing: an instance really is shared + across clients ([shared_instances.md](shared_instances.md)), but a result + produced by *another client* on that instance does not reach this client's + subscribers — there is no server-initiated frame, and both transports would + need an unsolicited-message path to carry one. Two handlers in one process see + each other's work; two clients do not, and must re-read to notice a change. + ## Thread safety @@ -424,16 +395,14 @@ exists per action type, keyed by `ActionTraits::typeId()`. The rules: `_backendMtx`, `_mtx`, and `_sessionMtx`, with `executeVia` taking its backend snapshot under the short, dedicated `_backendMtx` rather than `_mtx`). -`BridgeHandler`'s individual mutating operations — `set`, `subscribe`, -`unsubscribe`, `reset` — are each internally safe: they take the -`SubscriberState::mtx` while touching the entry map. Result and error callbacks -never run under that mutex; they are marshalled to the `guiExec` executor -passed at construction, and `tryFireImpl` captures a `weak_ptr` -so a callback that fires after the handler is destroyed is a no-op. The -intended usage is nonetheless **single-GUI-thread affinity**: a handler and its -subscriptions belong to one GUI thread, and interleaving concurrent `set<>` -storms from multiple threads onto the same handler, while memory-safe, has no -defined ordering guarantee beyond the per-operation locking. +`subscribe`/`unsubscribe` mutate the bridge's subscription registry under +`_subMtx`. Callbacks never run under that mutex: `publishResult` snapshots the +matching sinks under the lock and invokes them outside it, marshalled to the +`guiExec` executor passed at construction, so a subscriber that re-enters the +bridge cannot deadlock. A subscription holds a `weak_ptr` to its binding, so one +belonging to a destroyed handler is skipped and pruned rather than dangling. The +intended usage remains **single-GUI-thread affinity**: a handler and its +subscriptions belong to one GUI thread. ## Lifetime & ownership @@ -496,11 +465,11 @@ make teardown order-independent.) | dtor | `~BridgeHandler()` | Deregisters via `Bridge::deregisterHandler`, but only if the bridge's liveness token is still alive; a no-op if the `Bridge` was already destroyed. | | `execute` | `Completion execute(Action)` | Typed dispatch through the bridge. | | `executeJson` | `Completion executeJson(string_view actionType, string_view bodyJson)` | Type-erased dispatch through `ActionExecuteRegistry`. | -| `subscribe(cb)` | `void subscribe(function)` | Result callback. | -| `subscribe(cb, errCb)` | `void subscribe(function, function)` | Result + error callbacks. | -| `unsubscribe` | `void unsubscribe()` | Clears both callbacks. | -| `set` | `void set(MemberPointerTraits::ValueType value)` | Field-by-field update; auto-fires when ready. | -| `reset` | `void reset()` | Discards in-progress draft. | +| `subscribe(cb)` | `void subscribe(function)` | Fire `cb` whenever an `R` is produced on the attached instance. | +| `unsubscribe` | `void unsubscribe()` | Drops this handler's callback for `R`. | +| `attach(key)` | `void attach(const PrimaryKeyOf&)` | Attaches/re-points a shared handler. | +| `primary()` | `optional> primary()` | The handler's current primary, or empty. | +| `instances()` | `Completion>> instances()` | Snapshot of live shared keys. | | `guiExecutor` | `IExecutor* guiExecutor() const noexcept` | Returns the callback executor. | | `binding` | `const shared_ptr& binding() const` | Returns the underlying binding. | @@ -518,13 +487,13 @@ make teardown order-independent.) | Decision | Choice | Why | |---|---|---| | Binding storage | **`vector>`** | `Bridge` does not own the bindings — `BridgeHandler` holds the `shared_ptr`. Weak references let `switchBackend` and the reconnect handler skip dead bindings without keeping handlers alive. (Handler *teardown* after the bridge is made safe separately, by the `_liveness` token — not by this weak storage.) | -| Teardown order | **`shared_ptr _liveness` + per-handler `weak_ptr`** | Makes bridge-vs-handler destruction order-independent: a handler outliving its bridge skips deregistration instead of dereferencing a dangling `Bridge&`. Normal `execute`/`set` still require the bridge to outlive its handlers. | +| Teardown order | **`shared_ptr _liveness` + per-handler `weak_ptr`** | Makes bridge-vs-handler destruction order-independent: a handler outliving its bridge skips deregistration instead of dereferencing a dangling `Bridge&`. Normal `execute`/`subscribe` still require the bridge to outlive its handlers. | | Backend pointer | **Short snapshot under the dedicated `_backendMtx`** | `executeVia()` reads the backend through a `loadBackend()` helper that copies the `shared_ptr` under `_backendMtx` (never `_mtx`), so it never blocks on `switchBackend()`'s `_mtx`. | | Session storage | **Separate `_sessionMtx` from `_mtx`** | Session access is a hot path (every `executeVia` reads it). A separate mutex avoids contention with handler registration/switchBackend. | | Reconnect handler | **Liveness guard + weak‑backend guard + stale check; cleared in `~Bridge`** | The lambda captures a `weak_ptr` to `_liveness` and a `weak_ptr`. On invocation it first locks the liveness token — if the `Bridge` is gone it returns without touching `this` (no use-after-free). It then checks `pinned == loadBackend()` — if a switch occurred since the handler was installed, the reconnect is ignored. `~Bridge` and `switchBackend` also clear the outgoing backend's handler via `setReconnectHandler(nullptr)`; the liveness guard covers a reconnect already in flight when teardown races it. | -| Fielded actions | **`SubscriberState` shared across `BridgeHandler` copy-unsafe design** | The handler is non-copyable; the subscriber state is `shared_ptr` so `tryFireImpl` can capture a `weak_ptr` and survive handler destruction. Flight tracking (`running`/`pending`) coalesces rapid `set` calls. | +| Subscription keying | **On the result type, and against the binding rather than an instance id** | A subscriber is a renderer: it cares about the state it draws, not about which of several actions produced it, so a new action yielding the same type never breaks it. Storing against the binding makes a subscription follow a re-pointed handler, which is what "tell me about the account I am looking at" requires. | | Action readiness | **`ActionValidator::ready(snapshot)`** | Framework-agnostic validation — each action struct defines its own required-field semantics. The bridge never interprets action fields. | -| Local-path validation enforcement | **`localOp` checks `ActionValidator::ready` before `Model::execute`** | Closes the gap where an `Action` built by hand and dispatched via `BridgeHandler::execute()` (bypassing the reactive `set<>` gate) reached the model unvalidated; mirrors `ActionDispatcher::registerAction`'s server-side runner (`registry.md`). Backward compatible: `ready()` defaults to `true` for actions with no validator. | +| Local-path validation enforcement | **`localOp` checks `ActionValidator::ready` before `Model::execute`** | Closes the gap where an `Action` built by hand and dispatched via `BridgeHandler::execute()` (without a client-side gate) reached the model unvalidated; mirrors `ActionDispatcher::registerAction`'s server-side runner (`registry.md`). Backward compatible: `ready()` defaults to `true` for actions with no validator. | | Subscription keys | **`string_view` into static storage** | `ActionTraits::typeId()` returns `constexpr` string literals with static duration. The `unordered_map` holds non-owning keys; no allocation, no lifetime issues. | | `executeJson` | **Separate registry, not a vtable** | The action type is unknown at the call site. A flat `unordered_map<(modelId, actionId), Executor, PairKeyHash>` lets any translation unit register its actions without central registration or RTTI. | | `registerActionExecutorOnce` | **`inline` definition in header** | The function is forward-declared in `registry.hpp` (`morph::model::detail`) but defined `inline` in `bridge.hpp`, after `ActionExecuteRegistry`. `inline` lets that definition be instantiated in every TU that transitively includes `bridge.hpp` without an ODR/link violation. The registration runs from the anonymous-namespace initializer the macro emits. Because the definition lives only in `bridge.hpp`, any TU expanding `BRIDGE_REGISTER_ACTION` must include it (directly or transitively) or the link fails with an unresolved symbol. | diff --git a/docs/spec/core/shared_instances.md b/docs/spec/core/shared_instances.md new file mode 100644 index 0000000..15ce382 --- /dev/null +++ b/docs/spec/core/shared_instances.md @@ -0,0 +1,389 @@ +# Keyed, shareable model instances — design + +## Contents + +- [The gap this closes](#the-gap-this-closes) +- [What morph already has](#what-morph-already-has) +- [Declaring a primary key](#declaring-a-primary-key) +- [Where the key comes from](#where-the-key-comes-from) +- [`AllowShared` — opting a handler into sharing](#allowshared--opting-a-handler-into-sharing) +- [Re-pointing, not re-keying](#re-pointing-not-re-keying) +- [The instance directory](#the-instance-directory) +- [Enumerating live instances](#enumerating-live-instances) +- [Wire protocol changes](#wire-protocol-changes) +- [Ownership and authorization](#ownership-and-authorization) +- [Lifetime and the A7 connection-scope change](#lifetime-and-the-a7-connection-scope-change) +- [API reference](#api-reference) +- [Design decisions](#design-decisions) +- [Failure modes](#failure-modes) +- [Limitations](#limitations) +- [Non-goals](#non-goals) +- [Cross-references](#cross-references) + +## The gap this closes + +`Bridge::registerHandler()` unconditionally constructs a fresh +`HandlerBinding` and calls `registerModelWithContext(...)`. Every +`BridgeHandler` is therefore a new model instance, always. There is no way to +name an instance, no way to ask for one that already exists, and no way to find +out which ones are alive. + +The cost is visible in `examples/bank`: five controllers each construct a +`BridgeHandler`, so the desktop GUI holds five +`AccountModel` instances — and, because each model lazily opens its own +`Lightweight::DataMapper`, five SQLite connections — for what is logically one +thing. + +Once models hold state ([the bank example](../../../examples/bank/README.md)) +the problem stops being wasteful and starts being wrong: five instances of +account 42 are five divergent copies of that account's balance. + +## What morph already has + +Most of the mechanism is present and only needs connecting. + +- **A stable per-instance identity already exists.** `HandlerBinding::contextKey` + is documented as "stable identity of this model instance (e.g. an account + id)" and already travels in the `register` wire envelope + ([wire.md](wire.md)). It is used **only** for journal entity keys + and server-side log attachment; `wire.md`'s design-decision table states + explicitly that `contextKey` plays no part in instance routing. The vocabulary + is there; the routing is not. +- **Structural trait detection is an established pattern.** + [views.md](../forms/views.md) detects `kind`, `query`, `title`, `rowKey` + and friends "via a `requires`-expression, not inheritance or a marker base". + A model's key type is declared the same way. +- **Per-instance authorization exists.** `IAuthorizer::authorizeInstance` is + consulted on every `execute` and every `deregister`, carrying the instance id + and its recorded owner ([session.md](../session/session.md)). +- **One strand per instance** already gives a shared instance the serialisation + it needs; sharing an instance changes nothing about how its actions run. + +## Declaring a primary key + +A model declares its key type as a nested alias. Declaring it is what makes the +model keyed; a model without it behaves exactly as an unkeyed model always has. + +```cpp +class AccountModel { +public: + using PrimaryKey = std::int64_t; // detected structurally + // ... +}; +``` + +`PrimaryKey` must be a type morph can carry on the wire and use as a map key: +an integral type or `std::string`. The key type is what +[`instances()`](#enumerating-live-instances) is typed on, so it is visible in +user code as `AccountModel::PrimaryKey`. + +## Where the key comes from + +A keyed action declares which of its fields carries the key. Different actions +spell it differently, which is why the declaration is per action rather than per +model: + +```cpp +BRIDGE_KEY_FROM(GetAccount, &GetAccount::id) +BRIDGE_KEY_FROM(Deposit, &Deposit::accountId) +BRIDGE_KEY_FROM(CloseAccount, &CloseAccount::id) +``` + +An action that *creates* the entity has no key to carry — it produces one. Such +an action sources the key from its **result**, exactly as a database insert +returns its generated primary key: + +```cpp +BRIDGE_KEY_FROM_RESULT(OpenAccount, &dto::AccountInfo::id) +``` + +Actions with neither declaration are **keyless**, and most actions are: they run +on whichever instance the handler is currently attached to and say nothing about +identity. This is the common case, not the exception — a stateful model's whole +point is that its actions operate on state the instance already holds. + +A handler may also attach explicitly, without going through an action: + +```cpp +handler.attach(42); +``` + +## `AllowShared` — opting a handler into sharing + +Sharing eligibility is a **static** property of the handler, expressed as a +template parameter. The primary key is discovered **dynamically**, from +whichever action first supplies one. + +```cpp +BridgeHandler a1{bridge, gui}; // no primary yet +BridgeHandler a2{bridge, gui}; // no primary yet +BridgeHandler a3{bridge, gui}; // opts out + +a1.execute(GetAccount{.id = 42}); // sets primary 42 → modelId 1 +a2.execute(GetAccount{.id = 42}); // primary 42 already live → attaches to modelId 1 +a3.execute(GetAccount{.id = 42}); // sets primary 42 → modelId 2, deliberately separate +``` + +`BridgeHandler` — the spelling every existing call site uses — is +`BridgeHandler`, and behaves byte-for-byte as it does today: +its own instance, never entered into the directory, never handed to anyone +else. This is what keeps the feature backward compatible. + +Because a shared handler starts with no primary, **it registers nothing at +construction**. The `register` envelope is deferred until the handler knows +which instance it wants. A handler that only ever runs keyless actions gets a +private instance created on first execute; see +[Failure modes](#failure-modes). + +## Re-pointing, not re-keying + +A primary is **not** write-once. A handler already attached to account 42 may +execute a keyed action naming 43: + +```cpp +a1.execute(GetAccount{.id = 42}); // attached to the instance for 42 +a1.execute(GetAccount{.id = 43}); // now attached to the instance for 43 +``` + +This **re-points the handler** to the instance holding key 43, creating that +instance if it does not exist. The instance for 42 is untouched — it keeps its +identity and its state, and survives if any other handler is still attached. + +Instances never mutate their own identity. That is a deliberate simplification +and it is what makes the rule total: because a key always maps to exactly one +instance and an instance never changes key, there is no collision case to +resolve, no merge semantics to define, and no window in which the directory +disagrees with itself. Re-pointing gives the account-switching behaviour a GUI +actually wants, without any of that. + +Re-pointing a handler does **not** cancel its in-flight calls. A call dispatched +against instance 1 completes against instance 1 and delivers its result +normally; only subsequent calls go to the new instance. + +## The instance directory + +The directory lives **server-side**, in `RemoteServer`, so instances are +reusable across clients. Two connections that attach to key 42 reach the same +instance and see each other's state — that is the point of (a), and it is what +distinguishes this from a client-side handle cache. + +The directory maps `(modelTypeId, primaryKey) → ModelId`, held under the same +`_regMtx` that guards `_models`/`_owners`, so directory membership can never +desync from instance existence — the same invariant the connection-scope map +already maintains ([backend.md](backend.md), "Connection scopes"). + +Only instances created by an `AllowShared` handler are entered. A plain +handler's instance is invisible to the directory and unreachable by key. + +In local mode (`LocalBackend`) the directory lives in the backend rather than +the server, with identical semantics. The call site is unchanged between the +two, as morph requires everywhere. + +## Enumerating live instances + +```cpp +handler.instances() // Completion> + .then([](std::vector keys) { /* {42, 43, 71} */ }); +``` + +**This must be asynchronous.** The directory is server state, so in remote mode +answering it is a round trip. morph's core rule is that a call site is identical +local and remote, so the local implementation returns an already-resolved +`Completion` rather than the API returning a bare `std::vector` that only works +in-process. + +The result is a snapshot, not a live view, and it is stale the moment it +arrives — another client may attach or release between the reply being built +and the callback running. Callers must treat a returned key as "was live +recently", never as a guarantee that a subsequent `attach` finds the same +instance. + +Only *shared* instances are enumerable. Plain handlers' instances are absent by +construction. + +## Wire protocol changes + +Four additive changes. All are compatible with the additive-only evolution +policy in [wire.md](wire.md), and the lenient decoding that A6 +established means an older peer ignores what it does not understand. + +- **`register` grows `primary` and `shared`.** `primary` is the key as a string + (integral keys are decimal-encoded); `shared` is a bool. A `register` with + `shared: true` is a *register-or-attach*: the server returns the existing + `ModelId` for that `(typeId, primary)` if one is live, otherwise creates one + and enters it in the directory. `shared: false` or absent is today's + behaviour exactly. +- **A new `attach` request.** Re-points an existing binding at a different + primary without tearing down and recreating it, returning the target + `ModelId`. Semantically a `deregister` + `register` pair, made atomic so a + re-pointing handler cannot lose its slot to `LimitPolicy::maxLiveModels` in + between. +- **A new `assign` request.** Files an already-live `modelId` under a primary + key, in place. This is what makes a result-sourced key work without losing + state: an action that creates its own entity runs on a not-yet-keyed + instance, and only the reply carries the generated key, so the instance the + action ran on is promoted rather than abandoned for a fresh one. The existing + holder of a key always wins — promoting onto a taken key is a silent no-op, + never a displacement. +- **A new `instances` request.** Takes a model type id, replies with the live + primary keys for it. Subject to `authorize` like any other request; see below. + +`contextKey` keeps its current meaning and is **not** overloaded as the primary. +The two coincide in practice — a keyed model will normally set `contextKey` to +its primary so journal entries carry the entity key — but conflating them would +silently change behaviour for anyone already setting `contextKey` for journal +purposes, which the framework's opt-in discipline forbids. + +## Ownership and authorization + +`RemoteServer` records an `ownerPrincipal` for each instance at register time +and consults `authorizeInstance` on every execute, with the documented typical +policy being `ownerPrincipal.empty() || ownerPrincipal == ctx.principal` +([session.md](../session/session.md)). + +Under that policy, a second client attaching to an instance the first client +created would be **rejected**. Cross-client sharing and per-instance ownership +are therefore mutually exclusive, and the design says so rather than letting an +authorizer silently defeat the feature: + +- **A shared instance is ownerless.** Its `ownerPrincipal` is empty, so the + standard policy admits every principal, and gating access to it is the job of + `authorize` (per model type and action) or of the model itself. +- **`authorizeRegister` still gates creation.** An authorizer that refuses + `register` for a model type refuses it whether or not the request is shared. +- **`instances` is gated by `authorize`** for the model type with an empty + action id, so an authorizer can refuse enumeration without refusing use. It + leaks the set of live keys to anyone permitted to call it, which is a + meaningful disclosure for key spaces that are themselves sensitive — the + security spec must call this out. + +An application that needs per-instance ownership on a shared model must enforce +it inside the model, from `Context::principal`, which is the same advice +`security.md` already gives for security-critical checks. + +## Lifetime and the A7 connection-scope change + +This changed shipped A7 behaviour, which nothing in the §A–§E program did. It was +unavoidable. + +`closeConnection(cid)` used to erase every model recorded in `cid`'s scope. With +cross-client sharing that would destroy an instance another live client is still +attached to, so a scope entry is a **reference**, not ownership: + +- Each attach — from any connection — increments an instance's attach count. +- `deregister`, handler destruction, and `closeConnection` each decrement. +- The instance is destroyed when the count reaches zero, at which point it + leaves the directory. +- `closeConnection` remains idempotent and still bypasses `IAuthorizer`; it + decrements once per scope entry regardless of how many handlers a single + connection had attached. + +Unshared instances have exactly one attacher by construction, so their lifetime +is unchanged: count reaches zero on the same event that erases them today. + +An optional idle grace period (keep a zero-count shared instance alive for *n* +seconds in case another client re-attaches) is **not** part of this work. +Default behaviour is immediate destruction at zero. + +`LimitPolicy::maxLiveModels` counts instances, not attachments, so sharing +strictly reduces pressure on it. + +## API reference + +| Symbol | Signature | Meaning | +|---|---|---| +| `Model::PrimaryKey` | nested type alias | Declares the model keyed. Integral or `std::string`. Detected structurally. | +| `morph::bridge::AllowShared` | tag type | Second template argument of `BridgeHandler`. Opts the handler into the directory. | +| `morph::bridge::NoSharing` | tag type | The default. Today's isolated-instance behaviour. | +| `BRIDGE_KEY_FROM(A, &A::field)` | macro | Declares that action `A` carries its model's key in `field`. | +| `BRIDGE_KEY_FROM_RESULT(A, &R::field)` | macro | Declares that `A`'s *result* establishes the key. | +| `handler.attach(key)` | `void` | Attaches (or re-points) without executing an action. | +| `handler.primary()` | `std::optional` | The handler's current primary; empty if unattached. | +| `handler.instances()` | `Completion>` | Snapshot of live shared keys for this model type. | + +## Design decisions + +- **Sharing is static, identity is dynamic.** Whether a handler *may* share is a + compile-time property, so it is visible at the declaration and cannot vary per + call. Which instance it shares is a runtime property, because that is what the + user is choosing at runtime. +- **The directory is server-side.** A client-side cache would solve the bank's + five-connections problem but not the one that matters — two clients diverging + on the same entity. Server-side is the whole reason this needs wire changes. +- **Instances never change key.** Re-pointing the handler achieves the same user + goal with none of the collision, merge, or directory-consistency cases. +- **`contextKey` is not reused as the primary.** It has a shipped meaning; + overloading it would change behaviour for existing users silently. +- **Shared instances are ownerless.** The alternative — teaching + `authorizeInstance` about a set of owners — makes a simple, shipped, verified + hook substantially more complex to serve a case the model layer can handle. +- **`instances()` is async even locally.** Local/remote call-site symmetry is + the framework's most load-bearing promise; a synchronous convenience overload + would be the first place it broke. + +## Failure modes + +- **A shared handler that runs a keyless action while unattached** gets a + private instance with no primary, which can never enter the directory. It is + then an `AllowShared` handler that shares nothing. The recommended discipline + is *keyed action, or `attach`, first*; the implementation should make this + observable — `primary()` returning empty after a successful execute is the + signal — rather than silently degrading. +- **Attaching to a key whose entity does not exist.** The directory happily + creates an instance; hydration fails inside the model and the action completes + through `onError`. The instance must not be left in the directory in a + half-hydrated state, so a failed *first* action on a freshly created shared + instance releases it. +- **`instances()` raced against `attach`.** Documented as inherent: the snapshot + is stale on arrival. An `attach` to a key from a stale list is not an error — + it simply creates the instance again. +- **Re-pointing with calls in flight.** In-flight calls complete against the old + instance. A caller that assumes `.then` runs against the newly attached + instance is wrong; the spec must state the ordering explicitly. +- **`closeConnection` under-counting.** If a connection attaches the same + instance from two handlers, the scope must record two references, or closing + it leaks one. This is the main correctness risk in the A7 change and needs a + dedicated test. + +## Limitations + +- **No activation on demand from persistence.** An instance exists because a + handler asked for it, not because the key exists in a database. There is no + "the instance always exists, virtually" guarantee. +- **No idle deactivation.** Zero attachments destroys immediately; there is no + keep-alive, no LRU, no eviction policy. +- **No key derived from the session principal.** A per-user model must be + attached explicitly after login. +- **One key per instance.** No secondary keys, no alternate indexes, no + querying the directory by anything but model type. +- **Enumeration is per model type and unfiltered.** No paging, no predicate; a + model type with very many live instances returns all of them. +- **`instances()` discloses live keys** to any principal `authorize` admits. + +## Non-goals + +- **Not virtual actors.** No perpetual existence, no placement, no clustering, + no activation-on-message-to-a-cold-key. The directory tracks what exists; it + does not conjure it. +- **No cross-instance transactions.** Two shared instances are two strands; an + operation spanning both is a domain concern, as it is today. +- **No state persistence provider.** What an instance holds and how it is loaded + is entirely the model's business — see + [the bank example](../../../examples/bank/README.md). + +## Cross-references + +- [the bank example](../../../examples/bank/README.md) — the demonstrator; a key + identifies nothing until models hold state. +- [bridge.md's subscription semantics](bridge.md#subscription-semantics) — how attached handlers + learn that a shared instance changed. +- [bridge.md](bridge.md) — `HandlerBinding`, `contextKey`, + `registerHandler`, and `switchBackend`'s re-registration path. +- [backend.md](backend.md) — `RemoteServer`, connection scopes + (A7), and `LimitPolicy`. +- [wire.md](wire.md) — the envelope, additive evolution, and the + `contextKey`-vs-`modelId` decision this proposal preserves. +- [session.md](../session/session.md) — `authorizeInstance`, + `authorizeRegister`, and the recorded owner principal. +- [security.md](../security.md) — the per-instance ownership hook and the + trust boundary the ownerless-shared-instance decision sits inside. diff --git a/docs/spec/forms/forms.md b/docs/spec/forms/forms.md index 83f2031..1ed5fbf 100644 --- a/docs/spec/forms/forms.md +++ b/docs/spec/forms/forms.md @@ -1128,15 +1128,13 @@ table below); an action that declares no `computedFields` emits neither key. ### Where the value is authoritative -`recomputeAll` runs at four call sites: +`recomputeAll` runs at three call sites, all authoritative: -1. `BridgeHandler::set<>`'s reactive path (`tryFireImpl`, [bridge.md](../core/bridge.md)) - — live, client-side, **not authoritative**, for display only. -2. `ActionExecuteRegistry::registerAction`'s executor (the client-bridge JSON +1. `ActionExecuteRegistry::registerAction`'s executor (the client-bridge JSON dispatch path behind `BridgeHandler::executeJson`, [bridge.md](../core/bridge.md)). -3. `Bridge::executeVia`'s `localOp` (the in-process execution path `LocalBackend` +2. `Bridge::executeVia`'s `localOp` (the in-process execution path `LocalBackend` uses for every `execute()`/`executeJson` call, [bridge.md](../core/bridge.md)). -4. `ActionDispatcher::registerAction`'s runner (the server-side execution path +3. `ActionDispatcher::registerAction`'s runner (the server-side execution path `RemoteServer` uses for `SimulatedRemoteBackend` and the Qt WebSocket transport, [registry.md](../core/registry.md)). @@ -1394,7 +1392,7 @@ wrong or un-merged schema rather than fail loudly. | [rational.md](../util/rational.md) | Exact `Rational` values; the `num`/`den` in each `x-unitAlternatives` entry are a `Rational` numerator/denominator, which is why unit switches recompute exactly. Also the comparison/equality `greater`/`greaterOrEqual`/`less`/`lessOrEqual`/`equals` use for numeric fields, so client and server compare identical values. | | [security.md](../security.md) | The dispatcher's trust boundary — why `required` gates only the client and handlers must re-validate. | | [session.md](../session/session.md) | `Context::locale`, the server-side hook for data (not chrome) localisation — the one place `session::current()->locale` participates, for `Choice` option-row labels. | -| [bridge.md](../core/bridge.md) | The reactive `set<>`/`tryFireImpl` live recompute, and the `ActionExecuteRegistry`/`executeVia` authoritative recompute sites. | +| [bridge.md](../core/bridge.md) | The `ActionExecuteRegistry`/`executeVia` authoritative recompute sites. | | [registry.md](../core/registry.md) | `ActionDispatcher::registerAction`'s runner — the server-side authoritative recompute site. | ## Out of scope diff --git a/docs/spec/forms/workflows_navigation.md b/docs/spec/forms/workflows_navigation.md index 06a2457..4d9218b 100644 --- a/docs/spec/forms/workflows_navigation.md +++ b/docs/spec/forms/workflows_navigation.md @@ -211,17 +211,14 @@ public: wording ("the renderer resolves each prefill path ... and issues the corresponding `set<>`") — `FlowSession` does not push prefill itself. - **`Steps...` must be pairwise distinct.** Each step type occupies one slot - of both `BridgeHandler`'s per-action-type draft (bridge.md's - "Subscription semantics": exactly one `SubscriberEntry` per action type) - and `FlowSession`'s own `std::get(_drafts)` tuple lookup, which requires + of `FlowSession`'s own `std::get(_drafts)` tuple lookup, which requires a unique type. Reusing the same action type as two steps of one wizard is not supported (`static_assert`-enforced). -- **Backend-switch behaviour is inherited, not reimplemented.** Because - `set<>`/`subscribe<>` are the real `BridgeHandler` calls, an in-flight - step's fire cancelled by `Bridge::switchBackend` surfaces - `BackendChangedError` on `FlowSession`'s `onError` callback exactly as - bridge.md documents for a lone fielded subscriber, and the draft survives - the switch. +- **Backend-switch behaviour is inherited, not reimplemented.** A step is + dispatched with the ordinary `BridgeHandler::execute`, so an in-flight step + cancelled by `Bridge::switchBackend` surfaces `BackendChangedError` on + `FlowSession`'s `onError` callback exactly as bridge.md documents for any + other caller. The draft survives the switch because `FlowSession` owns it. ## The Qt/QML reference renderer @@ -377,9 +374,8 @@ the typed template API (see [Design decisions](#design-decisions)). screen this document reserves but does not yet implement, and the precedent (`CollectionView.qml` shipping in `src/qt/forms`) this spec's `WizardView.qml` placement follows. -- [../core/bridge.md](../core/bridge.md) — `BridgeHandler::set<>`/`subscribe<>`/ - `unsubscribe<>`, `ActionValidator::ready`, draft persistence across fires - and backend switches, and `BackendChangedError` — the mechanism +- [../core/bridge.md](../core/bridge.md) — `BridgeHandler::execute`, + `ActionValidator::ready`, and `BackendChangedError` — the mechanism `FlowSession` extends to span a sequence without adding a new dispatch path. - [../core/registry.md](../core/registry.md) — `ActionTraits::typeId()` and the `BRIDGE_REGISTER_ACTION` pattern `BRIDGE_REGISTER_WIZARD`/`BRIDGE_REGISTER_APP` diff --git a/docs/superpowers/2026-07-06-reactive-forms-bridge.md b/docs/superpowers/2026-07-06-reactive-forms-bridge.md index 0d776c8..3547786 100644 --- a/docs/superpowers/2026-07-06-reactive-forms-bridge.md +++ b/docs/superpowers/2026-07-06-reactive-forms-bridge.md @@ -134,11 +134,9 @@ calls `controller.submitIfValid(actionType, previewLine)` directly. independently, last-writer-wins on `resultText`. If this proves janky, a ~150 ms debounce timer in `DynamicForm.qml` is a self-contained follow-up. -- `BridgeHandler`'s compile-time fielded-draft API - (`set<&Action::field>`/`subscribe`) is not usable generically: - glaze plain-aggregate reflection exposes only field names, not - compile-time member pointers. The generic path goes through - `executeJson` instead. +- A compile-time, per-field API is not usable generically: glaze + plain-aggregate reflection exposes only field names, not compile-time + member pointers. The generic path goes through `executeJson` instead. - The options combo-box fetch is not reactive; it runs once per form from `Component.onCompleted`. - `examples/forms/main.cpp` (schema dump / `--emit-html` / REPL) has no GUI diff --git a/docs/todo.md b/docs/todo.md index 4b5e15b..9b01ff1 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -1,10 +1,137 @@ -# Production-hardening program — status +# Program tracker + +Two programs live here: + +- **[§F — Stateful models](#f-stateful-models-shipped)** came out of + [issue #18](https://github.com/LASTRADA-Software/morph/issues/18) ("compare + against Axelor / Jmix / Causeway / Orleans and find out what we are missing"). + All three items have shipped. +- **§A–§E — Production hardening & GUI generation** is **shipped**, kept for + the rationale (priority, dependency order) that motivated the work. + Present-tense designs for those are in [`docs/spec/`](spec). + +--- + +# §F. Stateful models (shipped) + +## What the issue #18 survey actually found + +The four projects named in the issue are not comparable to morph as a set: +Axelor, Jmix and Causeway are full-stack Java business-application platforms; +Orleans is a .NET distributed actor runtime. Reading their feature lists side by +side produces a backlog of ~25 items, most of which morph should never build — +it does not own a database, a process engine, or an IDE. + +So the survey was done the other way round: against **use cases in +`examples/bank`**, morph's largest worked example, asking where a developer hits +a wall rather than which boxes are unticked. That produced one finding that +subsumes most of the others. + +**morph's models are anonymous and stateless, so the model layer is doing +nothing.** Every bank model's only member is the `std::optional` it +inherits from `WithMapper` — a connection, not domain state. The per-instance +strand that morph advertises as its core service therefore protects nothing; +every action is a full round trip to SQLite; and because a `BridgeHandler` +registers one instance, the desktop GUI holds five `AccountModel` instances (and +five SQLite connections) for what is logically one thing. + +Orleans is the comparator that names this directly — a grain is *identity + +behaviour + state*, and morph has only behaviour. But the fix is not to become +an actor runtime. It is to make morph's existing model layer do the job it +already claims: hold state, be identified, be reachable. + +## Accepted items + +### F1 — Reshape `examples/bank` onto stateful models · P0 · shipped + +Split the per-domain models into models keyed by the entity they are named +after, holding that entity in memory, hydrated on activation and written +through on mutation. This is first deliberately: it is what introduces the main +idea of the library, and F2/F3 are unverifiable without it — a primary key +identifies nothing when instances carry nothing. + +`AccountModel` holds one account in memory, keyed by account id; `CustomerModel` +took the per-owner half (`ListAccounts`/`OpenAccount`), which was never +account-scoped. Cross-model ledger writes settle on another model's connection, +so `bank/db/row_versions.hpp` bumps a per-row counter and cached readers +re-hydrate on a stale version — the example's sharpest edge, shown rather than +arranged away. WASM shadow models and the five GUI controllers moved with it. +See [`examples/bank/README.md`](../examples/bank/README.md), "Stateful, keyed +models". + +### F2 — Keyed, shareable model instances · P0 · shipped + +A model declares a `PrimaryKey`; actions declare which field carries it (or that +their *result* establishes it); `BridgeHandler` opts a handler +into a **server-side** instance directory, so instances are reusable across +clients. `instances()` enumerates the live keys. A keyed action re-points a +handler rather than re-keying an instance, so key collisions do not arise. + +Carried a change to shipped behaviour: A7's `closeConnection` now decrements a +reference count rather than erasing, since otherwise one client's disconnect +destroys an instance another client is using. + +See [`spec/core/shared_instances.md`](spec/core/shared_instances.md). + +### F3 — Instance subscriptions · P1 · shipped + +`subscribe(cb)` keyed on the **result/state** type, firing whenever an `R` is +produced on the instance the handler is attached to — by this handler, by +another handler sharing it, or by another screen entirely. + +**Removed** the reactive-draft mechanism (`set<&A::field>`, `reset`, the old +action-keyed `subscribe`, in-flight coalescing), whose job stateful models do +better by holding the draft themselves. `morph::flows::FlowSession` already +owned its own draft tuple and merely mirrored into the handler's, so it now +gates on `ActionValidator` and dispatches directly; its public API, the +`w-*`/`app-*` schema, and `WizardView.qml` are unchanged. + +`ActionValidator` survives with its A1 server-side role, losing only its +draft-readiness one. See +[`spec/core/bridge.md`](spec/core/bridge.md#subscription-semantics) and +[`ARCHITECTURE.md`](ARCHITECTURE.md#instance-subscriptions). + +**Not included:** cross-*client* push. Fan-out is per `Bridge`, so two handlers +in one process — the case the bank GUI actually has — see each other's work, +while two separate clients do not. A server-initiated `notify` frame would need +both transports to grow an unsolicited-message path; that is its own item. + +## Considered and refused + +Recorded so the survey does not get re-run and so the boundary is explicit. + +| Not building | Why | +|---|---| +| **Entity metamodel / naked objects** (Causeway) | morph's unit is the *action*, deliberately. Rows plus row-bound actions already are an object UI; what is missing is metadata on the row, not a second parallel metamodel. | +| **Server-authoritative per-instance action availability** ("see it, use it, do it") | Real gap — `CloseAccount` is implemented, tested and exposed in no GUI because the client cannot ask "may I?". Judged not worth the surface area now. | +| **Query invalidation / live lists** | Superseded in part by F3, which gives shared instances a change channel without a query-invalidation vocabulary. | +| **Paging / sorting contract on view schemas** | `views.md` already records "no server-side query language" as a non-goal; paging stays a field on the query action. | +| **Result-type presentation metadata** (money, enum labels, badge severity) | Every GUI controller hand-writes a `toMap()` projection. Genuine duplication, but it is a forms-layer concern, not a model-layer one. | +| **Field- and row-level permissions** (Jmix, Causeway SecMan) | They own the ORM and the whole app; morph owns a seam. Per-principal schema redaction would break the one-cached-schema-per-type design. The model is the right place, and `bank`'s `loadOwned` guard shows it works. | +| **BPM / BPMN engine** (Axelor, Jmix) | A process engine without a store is meaningless, and morph does not own the store. | +| **Grain call filters / interceptor pipeline** (Orleans) | morph already has validator + authorizer + `observe` + journal on every dispatch path. No observed friction. | +| **Clustering, silos, placement, grain versioning, stateless workers, distributed ACID transactions** (Orleans) | morph is a UI bridge with one server, not a distributed runtime. | +| **Managed streams with cursors and checkpoints** (Orleans) | F3 is best-effort and unbuffered by design; durability here is a distributed-runtime concern. | +| **Reporting engine** (BIRT, JasperReports), **ORM + schema migration**, **IDE Studio** | Out of identity. morph owns neither the store nor the tooling. | +| **Runtime custom fields / dynamic attributes** (Axelor, Jmix) | Hard against compile-time reflection, and no observed need. | +| **REST / GraphQL facade, blob transfer, tabular export, multi-tenancy discriminator** | Plausible, no observed need. Revisit when one exists. | + +## Also surfaced + +- **`README.md`'s "Status & limitations" is stale.** It still claims the wire + protocol has no version negotiation, that `RemoteServer` model ids are + guessable sequential integers, and that only an in-memory offline queue + ships — all fixed by §A/§B below. Corrected as part of this program. + +--- + +# Production hardening & GUI generation (shipped) This tracked a design-approved, prioritized checklist of production-hardening -and GUI-generation work. **Every item below has shipped.** The authoritative, -present-tense design for each is in `docs/spec/`; this file is now a changelog -of what landed and why, kept for the rationale (priority, dependency order) -that motivated the work. +and GUI-generation work. **Every item in §A–§E has shipped.** The authoritative, +present-tense design for each is in `docs/spec/`; this section is a record of +what landed and why, kept for the rationale (priority, dependency order) that +motivated the work. Readiness depended on deployment mode: @@ -72,6 +199,10 @@ dropped socket now reclaims its models instead of stranding them until process exit. `closeConnection` is server housekeeping — it bypasses `IAuthorizer` by design, not a synthesized wire `deregister`. See `spec/core/backend.md#connection-scopes`. +> **F2 changes this.** Cross-client instance sharing requires `closeConnection` +> to decrement a reference count rather than erase. See +> [`spec/core/shared_instances.md`](spec/core/shared_instances.md). + --- ## B. Durability & data-integrity (both modes, if you persist) @@ -202,6 +333,9 @@ stays renderer-agnostic) is now documented in `BRIDGE_REGISTER_WIZARD`/`BRIDGE_REGISTER_APP`) and the `src/qt/forms` `WizardView.qml` reference renderer plus the demo's `AppShell.qml`. + > **F3 reworks this.** `FlowSession` is built on the reactive-draft mechanism + > F3 removes; it must be re-expressed as a stateful, keyed flow model first. + ### Ecosystem - **E-G9 — Renderer toolkit** · P1 · shipped — `spec/forms/forms.md` @@ -219,12 +353,11 @@ stays renderer-agnostic) is now documented in ## Notes -- Every item landed opt-in or backward compatible by default — none change - existing behavior unless enabled. -- `docs/planned/` no longer holds any implemented-item specs; the - authoritative current-state specs are entirely in `docs/spec/`. -- Two items surfaced *by* this program, not originally on it, and fixed before - it closed out: C3's fuzz harness found two real bugs in `morph::wire`'s +- Every §A–§E item landed opt-in or backward compatible by default — none change + existing behavior unless enabled. **§F breaks this pattern deliberately:** F2 + changes A7's cleanup semantics and F3 removes a public API. +- Two items surfaced *by* the §A–§E program, not originally on it, and fixed + before it closed out: C3's fuzz harness found two real bugs in `morph::wire`'s glaze-based parsing (a heap-buffer-overflow reachable by a 5-byte input, and a case where `RemoteServer`'s own error reply didn't round-trip through `encode`/`decode`). See `docs/spec/testing_strategy.md`'s "Known findings" diff --git a/examples/bank/CMakeLists.txt b/examples/bank/CMakeLists.txt index 698629c..112b315 100644 --- a/examples/bank/CMakeLists.txt +++ b/examples/bank/CMakeLists.txt @@ -50,6 +50,7 @@ add_library(bank_lib STATIC src/core/money.cpp src/db/schema.cpp src/models/account_model.cpp + src/models/customer_model.cpp src/models/auth_model.cpp src/models/transaction_model.cpp src/models/payee_model.cpp @@ -97,6 +98,7 @@ if(MORPH_BUILD_TESTS) tests/test_remote.cpp tests/test_offline.cpp tests/test_relations.cpp + tests/test_stateful_account.cpp ) target_include_directories(bank_tests PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/tests) target_link_libraries(bank_tests PRIVATE bank_lib Catch2::Catch2WithMain) diff --git a/examples/bank/README.md b/examples/bank/README.md index adb33d2..34375aa 100644 --- a/examples/bank/README.md +++ b/examples/bank/README.md @@ -37,13 +37,43 @@ GUI / CLI ──actions/results (plain DTOs)──▶ morph Bridge ──▶ Mod `entities.hpp`), the shared `WithMapper` mixin (one lazily-opened `DataMapper` per model), `user_ops.hpp` (principal→`user_id` resolution), and reusable `ledger_ops.hpp` (relation-aware debit/credit/post-entry + the `loadOwned` ownership guard). -- **`include/bank/models/` + `src/models/`** — one model per banking domain. The - `BRIDGE_REGISTER_*` macros live in the **model header** so every `.execute()` call - site sees the `ActionTraits` specialisation. +- **`include/bank/models/` + `src/models/`** — the models. The `BRIDGE_REGISTER_*` + macros live in the **model header** so every `.execute()` call site sees the + `ActionTraits` specialisation. `AccountModel` and `CustomerModel` are **stateful + and keyed** (see below); the remaining models are still per-domain and stateless. - **`src/db/schema.cpp`** — all `LIGHTWEIGHT_SQL_MIGRATION` table definitions. - **`include/bank/app/` + `src/app/`** — `App`: shared worker pool, GUI executor, `Bridge`, database setup, and login (which sets the bridge's default session). +### Stateful, keyed models + +`AccountModel` holds **one account, in memory**, for the lifetime of the instance. It +declares `using PrimaryKey = std::int64_t`, so morph keys instances by account id, and +`GetAccount`/`CloseAccount` declare that they carry that key (`BRIDGE_KEY_FROM`). Two +`BridgeHandler` handlers naming the same account — in one +GUI, or in two clients over one `RemoteServer` — reach a single instance and a single +balance. + +This is the shape morph is built around, and it is what makes the per-model strand +load-bearing: the instance owns mutable state, so its unsynchronised read-modify-write +is correct precisely because no two actions on one instance ever overlap. + +`CustomerModel` is the per-*owner* half that `AccountModel` used to also be doing: +`ListAccounts` and `OpenAccount` were never account-scoped, which is why both DTOs +carry an `owner` while `GetAccount`/`CloseAccount` carry an account id. It is keyed by +owner username. + +SQLite stays authoritative. The instance is a cache with identity: hydrated on first +use, written through on every mutation, dropped when the instance goes away. + +**The honest edge.** `Transfer`, bill payment and loan disbursement move money across +two accounts inside a single `SqlTransaction` owned by a *different* model, because +morph has no cross-instance transaction and this example must not imply it does. Those +writes land behind a cached row's back, so every balance write bumps a counter in +[`bank/db/row_versions.hpp`](include/bank/db/row_versions.hpp) and a cached reader +re-hydrates when the version it captured is stale. A real deployment would use the +store's own row version instead. + ### Why per-model `DataMapper`? morph runs each model on its own strand (single-threaded), so a model can own its own @@ -93,7 +123,7 @@ integers), so the GUI and CLI are unaffected; models map the relation values to | **Auth** | register, login, change password, `WhoAmI` (session introspection) | | **Account** | open (checking/savings/credit), list, get, close; overdraft, interest | | **Transaction** | deposit, withdraw, **atomic transfer**, paginated history | -| **Payee** | add (with IBAN validation + `set<>` streaming), remove, list | +| **Payee** | add (with IBAN validation), remove, list | | **Payment** | one-off bill pay, scheduled payments, standing orders, cancel | | **Card** | issue debit/credit, freeze/unfreeze/cancel, set limit, change PIN | | **Loan** | apply (disburse), amortization schedule, repay, payoff | @@ -102,7 +132,7 @@ integers), so the GUI and CLI are unaffected; models map the relation values to | **Statement** | date-ranged credit/debit summary across all accounts | morph features exercised: `Completion` then/onError, **sessions** (principal scoping + -authorization), **validation** (`validate()` + the `set<>`/`subscribe<>` streaming +authorization), **validation** (`validate()` + the dispatch-path validator form flow), **local ↔ remote parity**, a custom **`IAuthorizer`**, and the **offline queue + `SyncWorker`** replay path. diff --git a/examples/bank/gui/controllers/AccountController.hpp b/examples/bank/gui/controllers/AccountController.hpp index d71345b..84959aa 100644 --- a/examples/bank/gui/controllers/AccountController.hpp +++ b/examples/bank/gui/controllers/AccountController.hpp @@ -9,7 +9,7 @@ #ifndef Q_MOC_RUN #include -#include "bank/models/account_model.hpp" +#include "bank/models/customer_model.hpp" #endif namespace bankgui { @@ -37,7 +37,7 @@ class AccountController : public BankController { void accountsChanged(); private: - morph::bridge::BridgeHandler _model; + morph::bridge::BridgeHandler _model; QVariantList _accounts; QString _totalBalance{QStringLiteral("—")}; int _openCount{0}; diff --git a/examples/bank/gui/controllers/CardController.hpp b/examples/bank/gui/controllers/CardController.hpp index fe0b34e..4b0f083 100644 --- a/examples/bank/gui/controllers/CardController.hpp +++ b/examples/bank/gui/controllers/CardController.hpp @@ -10,7 +10,7 @@ #ifndef Q_MOC_RUN #include -#include "bank/models/account_model.hpp" +#include "bank/models/customer_model.hpp" #include "bank/models/card_model.hpp" #endif @@ -44,7 +44,7 @@ class CardController : public BankController { void reloadCards(); void reloadAccounts(); - morph::bridge::BridgeHandler _accountModel; + morph::bridge::BridgeHandler _accountModel; morph::bridge::BridgeHandler _cardModel; QVariantList _cards; QVariantList _accounts; diff --git a/examples/bank/gui/controllers/LoanController.hpp b/examples/bank/gui/controllers/LoanController.hpp index 0187860..2890dbe 100644 --- a/examples/bank/gui/controllers/LoanController.hpp +++ b/examples/bank/gui/controllers/LoanController.hpp @@ -10,7 +10,7 @@ #ifndef Q_MOC_RUN #include -#include "bank/models/account_model.hpp" +#include "bank/models/customer_model.hpp" #include "bank/models/loan_model.hpp" #endif @@ -47,7 +47,7 @@ class LoanController : public BankController { void reloadAccounts(); morph::bridge::BridgeHandler _loanModel; - morph::bridge::BridgeHandler _accountModel; + morph::bridge::BridgeHandler _accountModel; QVariantList _loans; QVariantList _accounts; QVariantList _schedule; diff --git a/examples/bank/gui/controllers/PayeeController.hpp b/examples/bank/gui/controllers/PayeeController.hpp index d1b49ab..2cfd352 100644 --- a/examples/bank/gui/controllers/PayeeController.hpp +++ b/examples/bank/gui/controllers/PayeeController.hpp @@ -10,7 +10,7 @@ #ifndef Q_MOC_RUN #include -#include "bank/models/account_model.hpp" +#include "bank/models/customer_model.hpp" #include "bank/models/payee_model.hpp" #include "bank/models/payment_model.hpp" #endif @@ -46,7 +46,7 @@ class PayeeController : public BankController { void reloadAccounts(); morph::bridge::BridgeHandler _payeeModel; - morph::bridge::BridgeHandler _accountModel; + morph::bridge::BridgeHandler _accountModel; morph::bridge::BridgeHandler _paymentModel; QVariantList _payees; QVariantList _accounts; diff --git a/examples/bank/gui/controllers/TransactionController.hpp b/examples/bank/gui/controllers/TransactionController.hpp index a4b72f0..7f2d66c 100644 --- a/examples/bank/gui/controllers/TransactionController.hpp +++ b/examples/bank/gui/controllers/TransactionController.hpp @@ -10,7 +10,7 @@ #ifndef Q_MOC_RUN #include -#include "bank/models/account_model.hpp" +#include "bank/models/customer_model.hpp" #include "bank/models/transaction_model.hpp" #endif @@ -48,7 +48,7 @@ class TransactionController : public BankController { void reloadHistory(); [[nodiscard]] int selectedCurrency() const; - morph::bridge::BridgeHandler _accountModel; + morph::bridge::BridgeHandler _accountModel; morph::bridge::BridgeHandler _txnModel; QVariantList _accounts; QVariantList _history; diff --git a/examples/bank/gui_wasm/CMakeLists.txt b/examples/bank/gui_wasm/CMakeLists.txt index cd9e5e5..83a8b26 100644 --- a/examples/bank/gui_wasm/CMakeLists.txt +++ b/examples/bank/gui_wasm/CMakeLists.txt @@ -26,6 +26,7 @@ qt_add_executable(bank_gui_wasm # In-memory model implementations (persistence-free): src/models/auth_model_wasm.cpp src/models/account_model_wasm.cpp + src/models/customer_model_wasm.cpp src/models/transaction_model_wasm.cpp src/models/card_model_wasm.cpp src/models/payee_model_wasm.cpp diff --git a/examples/bank/gui_wasm/include/bank/models/account_model.hpp b/examples/bank/gui_wasm/include/bank/models/account_model.hpp index eb30441..df32e45 100644 --- a/examples/bank/gui_wasm/include/bank/models/account_model.hpp +++ b/examples/bank/gui_wasm/include/bank/models/account_model.hpp @@ -2,21 +2,36 @@ #pragma once // WASM shadow of include/bank/models/account_model.hpp (in-memory backend). +// Kept in lockstep with the desktop model: one account per instance, held in +// memory, keyed by account id. -#include +#include #include +#include +#include +#include #include "bank/dto/account_dto.hpp" +#include "bank/wasm/store.hpp" namespace bank { -/// @brief Opens, lists, inspects, and closes customer accounts (in-memory). +/// @brief One customer account, cached in the instance (in-memory backend). class AccountModel { public: - dto::AccountInfo execute(const dto::OpenAccount& action); - dto::AccountList execute(const dto::ListAccounts& action); + /// @brief Account id. Declaring this alias is what makes the model keyed. + using PrimaryKey = std::int64_t; + dto::AccountInfo execute(const dto::GetAccount& action); dto::CommandResult execute(const dto::CloseAccount& action); + +private: + void hydrate(std::int64_t accountId); + + wasm::AccountRow _row{}; + std::string _owner; + std::int64_t _loadedId = 0; + std::uint64_t _seenVersion = 0; }; } // namespace bank @@ -24,11 +39,10 @@ class AccountModel { using bank::AccountModel; using bank::dto::CloseAccount; using bank::dto::GetAccount; -using bank::dto::ListAccounts; -using bank::dto::OpenAccount; BRIDGE_REGISTER_MODEL(AccountModel, "AccountModel") -BRIDGE_REGISTER_ACTION(AccountModel, OpenAccount, "OpenAccount") -BRIDGE_REGISTER_ACTION(AccountModel, ListAccounts, "ListAccounts") BRIDGE_REGISTER_ACTION(AccountModel, GetAccount, "GetAccount") BRIDGE_REGISTER_ACTION(AccountModel, CloseAccount, "CloseAccount") + +BRIDGE_KEY_FROM(GetAccount, &GetAccount::id); +BRIDGE_KEY_FROM(CloseAccount, &CloseAccount::id); diff --git a/examples/bank/gui_wasm/include/bank/models/customer_model.hpp b/examples/bank/gui_wasm/include/bank/models/customer_model.hpp new file mode 100644 index 0000000..e969cf0 --- /dev/null +++ b/examples/bank/gui_wasm/include/bank/models/customer_model.hpp @@ -0,0 +1,36 @@ +// SPDX-License-Identifier: Apache-2.0 +#pragma once + +// WASM shadow of include/bank/models/customer_model.hpp (in-memory backend). + +#include +#include +#include +#include + +#include "bank/dto/account_dto.hpp" + +namespace bank { + +/// @brief One customer: lists and opens the accounts they own (in-memory). +class CustomerModel { +public: + /// @brief Owner username. Declaring this alias is what makes the model keyed. + using PrimaryKey = std::string; + + dto::AccountInfo execute(const dto::OpenAccount& action); + dto::AccountList execute(const dto::ListAccounts& action); +}; + +} // namespace bank + +using bank::CustomerModel; +using bank::dto::ListAccounts; +using bank::dto::OpenAccount; + +BRIDGE_REGISTER_MODEL(CustomerModel, "CustomerModel") +BRIDGE_REGISTER_ACTION(CustomerModel, OpenAccount, "OpenAccount") +BRIDGE_REGISTER_ACTION(CustomerModel, ListAccounts, "ListAccounts") + +BRIDGE_KEY_FROM(ListAccounts, &ListAccounts::owner); +BRIDGE_KEY_FROM(OpenAccount, &OpenAccount::owner); diff --git a/examples/bank/gui_wasm/include/bank/wasm/store_ops.hpp b/examples/bank/gui_wasm/include/bank/wasm/store_ops.hpp index 2c87dff..0045277 100644 --- a/examples/bank/gui_wasm/include/bank/wasm/store_ops.hpp +++ b/examples/bank/gui_wasm/include/bank/wasm/store_ops.hpp @@ -1,6 +1,8 @@ // SPDX-License-Identifier: Apache-2.0 #pragma once +#include "bank/db/row_versions.hpp" + #include #include #include @@ -109,6 +111,7 @@ inline TxnRow postEntry(Db& db, const AccountRow& account, TxnDirection directio inline TxnRow applyCredit(Db& db, AccountRow& account, std::int64_t amountMinor, TxnKind kind, std::int64_t counterpartyId, const std::string& description) { account.balanceMinor += amountMinor; + db::bumpRowVersion(static_cast(account.id)); db.accounts.update(account); return postEntry(db, account, TxnDirection::Credit, kind, amountMinor, counterpartyId, description); } @@ -122,6 +125,7 @@ inline TxnRow applyDebit(Db& db, AccountRow& account, std::int64_t amountMinor, throw InsufficientFunds{"amount exceeds available balance plus overdraft"}; } account.balanceMinor = projected; + db::bumpRowVersion(static_cast(account.id)); db.accounts.update(account); return postEntry(db, account, TxnDirection::Debit, kind, amountMinor, counterpartyId, description); } diff --git a/examples/bank/gui_wasm/src/models/account_model_wasm.cpp b/examples/bank/gui_wasm/src/models/account_model_wasm.cpp index eb74080..1bc7eb0 100644 --- a/examples/bank/gui_wasm/src/models/account_model_wasm.cpp +++ b/examples/bank/gui_wasm/src/models/account_model_wasm.cpp @@ -1,16 +1,17 @@ // SPDX-License-Identifier: Apache-2.0 // -// In-memory implementation of AccountModel for the WASM build. Mirrors the -// behaviour of src/models/account_model.cpp but persists to bank::wasm::Db. +// In-memory implementation of the stateful AccountModel for the WASM build. +// Mirrors src/models/account_model.cpp: one account per instance, cached in the +// instance, re-hydrated from bank::wasm::Db only when the row version moves. #include "bank/models/account_model.hpp" -#include #include #include "bank/core/errors.hpp" #include "bank/core/principal.hpp" #include "bank/core/types.hpp" +#include "bank/db/row_versions.hpp" #include "bank/wasm/store.hpp" #include "bank/wasm/store_ops.hpp" @@ -18,16 +19,6 @@ namespace bank { namespace { -std::string generateAccountNumber() { - static thread_local std::mt19937_64 rng{std::random_device{}()}; - std::uniform_int_distribution digit{0, 9}; - std::string number = "DE"; - for (int idx = 0; idx < 20; ++idx) { - number.push_back(static_cast('0' + digit(rng))); - } - return number; -} - dto::AccountInfo toInfo(const wasm::AccountRow& rec, const std::string& owner) { return dto::AccountInfo{ .id = static_cast(rec.id), @@ -42,66 +33,35 @@ dto::AccountInfo toInfo(const wasm::AccountRow& rec, const std::string& owner) { }; } -int defaultInterestBps(int kind) { - return kind == static_cast(AccountKind::Savings) ? 150 : 0; -} - } // namespace -dto::AccountInfo AccountModel::execute(const dto::OpenAccount& action) { - if (!action.validate()) { - throw ValidationError{"invalid account kind/currency/overdraft"}; - } - const std::string owner = resolveOwner(action.owner); - if (owner.empty()) { - throw Unauthorized{"no session principal to own the account"}; - } - auto& db = wasm::sharedDb(); - - wasm::AccountRow rec; - rec.userId = wasm::requireUserId(db, owner); - rec.number = generateAccountNumber(); - rec.kind = action.kind; - rec.currency = action.currency; - rec.balanceMinor = 0; - rec.overdraftMinor = action.overdraftMinor; - rec.status = static_cast(AccountStatus::Open); - rec.interestBps = defaultInterestBps(action.kind); - rec.id = db.accounts.insert(rec); - return toInfo(rec, owner); -} - -dto::AccountList AccountModel::execute(const dto::ListAccounts& action) { - const std::string owner = resolveOwner(action.owner); - if (owner.empty()) { - throw Unauthorized{"no session principal to list accounts for"}; - } - auto& db = wasm::sharedDb(); - const auto userId = wasm::requireUserId(db, owner); - - dto::AccountList out; - for (const auto& rec : db.accounts.where([&](const wasm::AccountRow& a) { return a.userId == userId; })) { - out.accounts.push_back(toInfo(rec, owner)); +void AccountModel::hydrate(std::int64_t accountId) { + const std::string owner = sessionPrincipal(); + const auto current = db::rowVersion(accountId); + if (_loadedId == accountId && _owner == owner && _seenVersion == current) { + return; } - return out; + auto& store = wasm::sharedDb(); + _row = wasm::loadOwnedAccount(store, accountId, wasm::requireUserId(store, owner)); + _owner = owner; + _loadedId = accountId; + _seenVersion = current; } dto::AccountInfo AccountModel::execute(const dto::GetAccount& action) { - const std::string owner = sessionPrincipal(); - auto& db = wasm::sharedDb(); - auto rec = wasm::loadOwnedAccount(db, action.id, wasm::requireUserId(db, owner)); - return toInfo(rec, owner); + hydrate(action.id); + return toInfo(_row, _owner); } dto::CommandResult AccountModel::execute(const dto::CloseAccount& action) { - const std::string owner = sessionPrincipal(); - auto& db = wasm::sharedDb(); - auto rec = wasm::loadOwnedAccount(db, action.id, wasm::requireUserId(db, owner)); - if (rec.balanceMinor != 0) { + hydrate(action.id); + if (_row.balanceMinor != 0) { return dto::CommandResult{.ok = false, .message = "account balance must be zero before closing"}; } - rec.status = static_cast(AccountStatus::Closed); - db.accounts.update(rec); + _row.status = static_cast(AccountStatus::Closed); + wasm::sharedDb().accounts.update(_row); + db::bumpRowVersion(action.id); + _seenVersion = db::rowVersion(action.id); return dto::CommandResult{.ok = true, .message = "account closed"}; } diff --git a/examples/bank/gui_wasm/src/models/customer_model_wasm.cpp b/examples/bank/gui_wasm/src/models/customer_model_wasm.cpp new file mode 100644 index 0000000..0c51fbd --- /dev/null +++ b/examples/bank/gui_wasm/src/models/customer_model_wasm.cpp @@ -0,0 +1,90 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// In-memory implementation of CustomerModel for the WASM build — the per-owner +// half of what used to be one AccountModel. Mirrors src/models/customer_model.cpp. + +#include "bank/models/customer_model.hpp" + +#include +#include + +#include "bank/core/errors.hpp" +#include "bank/core/principal.hpp" +#include "bank/core/types.hpp" +#include "bank/wasm/store.hpp" +#include "bank/wasm/store_ops.hpp" + +namespace bank { + +namespace { + +std::string generateAccountNumber() { + static thread_local std::mt19937_64 rng{std::random_device{}()}; + std::uniform_int_distribution digit{0, 9}; + std::string number = "DE"; + for (int idx = 0; idx < 20; ++idx) { + number.push_back(static_cast('0' + digit(rng))); + } + return number; +} + +dto::AccountInfo toInfo(const wasm::AccountRow& rec, const std::string& owner) { + return dto::AccountInfo{ + .id = static_cast(rec.id), + .owner = owner, + .number = rec.number, + .kind = rec.kind, + .currency = rec.currency, + .balanceMinor = rec.balanceMinor, + .overdraftMinor = rec.overdraftMinor, + .status = rec.status, + .interestBps = rec.interestBps, + }; +} + +int defaultInterestBps(int kind) { return kind == static_cast(AccountKind::Savings) ? 150 : 0; } + +} // namespace + +// execute() is morph's dispatch contract; every model spells it as a member. +// NOLINTNEXTLINE(readability-convert-member-functions-to-static) +dto::AccountInfo CustomerModel::execute(const dto::OpenAccount& action) { + if (!action.validate()) { + throw ValidationError{"invalid account kind/currency/overdraft"}; + } + const std::string owner = resolveOwner(action.owner); + if (owner.empty()) { + throw Unauthorized{"no session principal to own the account"}; + } + auto& store = wasm::sharedDb(); + + wasm::AccountRow rec; + rec.userId = wasm::requireUserId(store, owner); + rec.number = generateAccountNumber(); + rec.kind = action.kind; + rec.currency = action.currency; + rec.balanceMinor = 0; + rec.overdraftMinor = action.overdraftMinor; + rec.status = static_cast(AccountStatus::Open); + rec.interestBps = defaultInterestBps(action.kind); + rec.id = store.accounts.insert(rec); + return toInfo(rec, owner); +} + +// NOLINTNEXTLINE(readability-convert-member-functions-to-static) +dto::AccountList CustomerModel::execute(const dto::ListAccounts& action) { + const std::string owner = resolveOwner(action.owner); + if (owner.empty()) { + throw Unauthorized{"no session principal to list accounts for"}; + } + auto& store = wasm::sharedDb(); + const auto userId = wasm::requireUserId(store, owner); + + dto::AccountList out; + for (const auto& rec : store.accounts.where([&](const wasm::AccountRow& row) { return row.userId == userId; })) { + out.accounts.push_back(toInfo(rec, owner)); + } + return out; +} + +} // namespace bank diff --git a/examples/bank/include/bank/db/account_mapping.hpp b/examples/bank/include/bank/db/account_mapping.hpp new file mode 100644 index 0000000..f615a02 --- /dev/null +++ b/examples/bank/include/bank/db/account_mapping.hpp @@ -0,0 +1,36 @@ +// SPDX-License-Identifier: Apache-2.0 +#pragma once + +#include +#include + +#include "bank/db/entities.hpp" +#include "bank/dto/account_dto.hpp" + +/// @file +/// Shared `AccountRecord` -> `AccountInfo` projection. Split out of +/// `account_model.cpp` when the model divided into `AccountModel` (per account) +/// and `CustomerModel` (per owner), so both halves project rows identically. + +namespace bank::db { + +/// @brief Translates a persisted `AccountRecord` into the wire `AccountInfo` DTO. +/// @param rec Persisted account row. +/// @param owner Resolved owner username — the wire DTO carries the username +/// rather than the internal `user_id` the record stores. +/// @return The wire projection of @p rec. +[[nodiscard]] inline dto::AccountInfo toAccountInfo(const AccountRecord& rec, const std::string& owner) { + return dto::AccountInfo{ + .id = static_cast(rec.id.Value()), + .owner = owner, + .number = std::string{rec.number.Value().str()}, + .kind = rec.kind.Value(), + .currency = rec.currency.Value(), + .balanceMinor = rec.balanceMinor.Value(), + .overdraftMinor = rec.overdraftMinor.Value(), + .status = rec.status.Value(), + .interestBps = rec.interestBps.Value(), + }; +} + +} // namespace bank::db diff --git a/examples/bank/include/bank/db/ledger_ops.hpp b/examples/bank/include/bank/db/ledger_ops.hpp index e8048d7..d750618 100644 --- a/examples/bank/include/bank/db/ledger_ops.hpp +++ b/examples/bank/include/bank/db/ledger_ops.hpp @@ -11,6 +11,7 @@ #include "bank/core/errors.hpp" #include "bank/core/types.hpp" #include "bank/db/entities.hpp" +#include "bank/db/row_versions.hpp" #include "bank/db/user_ops.hpp" /// @file @@ -120,6 +121,7 @@ inline TxnRecord applyCredit(Lightweight::DataMapper& mapper, AccountRecord& acc std::int64_t amountMinor, TxnKind kind, std::int64_t counterpartyId, const std::string& description) { account.balanceMinor = account.balanceMinor.Value() + amountMinor; + bumpRowVersion(static_cast(account.id.Value())); mapper.Update(account); return postEntry(mapper, account, TxnDirection::Credit, kind, amountMinor, counterpartyId, description); } @@ -135,6 +137,7 @@ inline TxnRecord applyDebit(Lightweight::DataMapper& mapper, AccountRecord& acco throw InsufficientFunds{"amount exceeds available balance plus overdraft"}; } account.balanceMinor = projected; + bumpRowVersion(static_cast(account.id.Value())); mapper.Update(account); return postEntry(mapper, account, TxnDirection::Debit, kind, amountMinor, counterpartyId, description); } diff --git a/examples/bank/include/bank/db/row_versions.hpp b/examples/bank/include/bank/db/row_versions.hpp new file mode 100644 index 0000000..e9918ab --- /dev/null +++ b/examples/bank/include/bank/db/row_versions.hpp @@ -0,0 +1,74 @@ +// SPDX-License-Identifier: Apache-2.0 +#pragma once + +#include +#include +#include + +/// @file +/// Per-row version counters, so a *stateful* model can tell when a row it is +/// holding in memory was changed by somebody else. +/// +/// A keyed `AccountModel` instance owns one account and keeps its row in memory +/// — that is what makes reads free and what makes sharing the instance worth +/// anything. But the bank's ledger operations (transfer, bill payment, loan +/// disbursement) deliberately move money inside a single `SqlTransaction` owned +/// by a *different* model, because morph has no cross-instance transaction and +/// this example must not pretend otherwise (see +/// docs/planned/stateful_bank_example.md). Those writes land in SQLite behind +/// the cached row's back. +/// +/// This is the smallest honest fix: every writer bumps the account's version, +/// and a cached reader re-hydrates when the version it captured is stale. It is +/// process-wide because the models it coordinates share one process; a real +/// deployment would use the store's own row version or an optimistic-concurrency +/// column instead. + +namespace bank::db { + +/// @brief Process-wide monotonic version counters keyed by account id. +/// +/// Thread-safe: models run on their own strands, but different models run +/// concurrently, so bumps and reads genuinely race. +class RowVersions { +public: + /// @brief Returns the process-wide instance. + /// @return Reference to the singleton. + static RowVersions& instance() { + static RowVersions inst; + return inst; + } + + /// @brief Records that the row for @p accountId changed. + /// @param accountId Account whose row was written. + void bump(std::int64_t accountId) { + std::scoped_lock const lock{_mtx}; + _versions[accountId] += 1; + } + + /// @brief Current version of @p accountId's row. + /// @param accountId Account to query. + /// @return A counter that changes whenever the row is written; `0` if never written. + [[nodiscard]] std::uint64_t version(std::int64_t accountId) { + std::scoped_lock const lock{_mtx}; + auto iter = _versions.find(accountId); + return iter == _versions.end() ? 0U : iter->second; + } + +private: + std::mutex _mtx; + std::unordered_map _versions; +}; + +/// @brief Convenience wrapper for `RowVersions::instance().bump(accountId)`. +/// @param accountId Account whose row was written. +inline void bumpRowVersion(std::int64_t accountId) { RowVersions::instance().bump(accountId); } + +/// @brief Convenience wrapper for `RowVersions::instance().version(accountId)`. +/// @param accountId Account to query. +/// @return The account's current row version. +[[nodiscard]] inline std::uint64_t rowVersion(std::int64_t accountId) { + return RowVersions::instance().version(accountId); +} + +} // namespace bank::db diff --git a/examples/bank/include/bank/models/account_model.hpp b/examples/bank/include/bank/models/account_model.hpp index 881cff4..8dfade4 100644 --- a/examples/bank/include/bank/models/account_model.hpp +++ b/examples/bank/include/bank/models/account_model.hpp @@ -1,34 +1,54 @@ // SPDX-License-Identifier: Apache-2.0 #pragma once -#include +#include +#include #include +#include +#include #include "bank/db/db_model.hpp" +#include "bank/db/entities.hpp" #include "bank/dto/account_dto.hpp" /// @file -/// The Account model. A plain, single-threaded C++ class — morph runs each -/// instance on its own strand, so the model never deals with concurrency. It -/// owns one Lightweight `DataMapper` (lazily opened on first use, on the strand -/// thread) and translates between wire DTOs and the `AccountRecord` entity. +/// The Account model — one customer account, **held in memory** for the lifetime +/// of the instance. +/// +/// This is the shape morph is designed around and the reason the framework runs +/// each model on its own strand: the instance owns mutable state, so an +/// unsynchronised read-modify-write of `_row` is correct precisely because no +/// two actions on the same instance ever overlap. A stateless model (as every +/// bank model used to be) makes that strand protect nothing. +/// +/// The model declares `PrimaryKey`, so morph keys instances by account id: two +/// `BridgeHandler` handlers naming the same account — +/// in one GUI or in two clients — reach one instance and see one balance. +/// SQLite stays authoritative; the instance is a cache with identity, hydrated +/// on first use and written through on every mutation. namespace bank { -/// @brief Opens, lists, inspects, and closes customer accounts. +/// @brief One customer account: its row, cached, with reads served from memory. class AccountModel : private db::WithMapper { public: - /// @brief Opens a new account; returns the freshly created account. - dto::AccountInfo execute(const dto::OpenAccount& action); - - /// @brief Lists accounts owned by the requested (or session) owner. - dto::AccountList execute(const dto::ListAccounts& action); + /// @brief Account id. Declaring this alias is what makes the model keyed. + using PrimaryKey = std::int64_t; - /// @brief Returns one account by id, or throws `NotFound`. + /// @brief Returns the account, hydrating from SQLite only when needed. dto::AccountInfo execute(const dto::GetAccount& action); /// @brief Closes a zero-balance account; returns ok/message. dto::CommandResult execute(const dto::CloseAccount& action); + +private: + /// @brief Loads `_row` for @p accountId if it is absent, stale, or for another account. + void hydrate(std::int64_t accountId); + + db::AccountRecord _row{}; ///< the account itself — in memory, not re-queried + std::string _owner; ///< resolved owner username for `_row` + std::int64_t _loadedId = 0; ///< which account `_row` holds; 0 = none + std::uint64_t _seenVersion = 0; ///< row version `_row` was hydrated at }; } // namespace bank @@ -37,18 +57,16 @@ class AccountModel : private db::WithMapper { // Registration lives in the header (not the .cpp) on purpose: the // BRIDGE_REGISTER_ACTION macro specialises `morph::model::ActionTraits`, // and every translation unit that calls `handler.execute(Action{...})` needs -// that specialisation visible to deduce the result type. The macros must sit at -// global scope and token-paste unqualified identifiers, so the types are pulled -// in with using-declarations first. The static registration runs once per -// including TU; the underlying registry assignment is idempotent. +// that specialisation visible to deduce the result type. using bank::AccountModel; using bank::dto::CloseAccount; using bank::dto::GetAccount; -using bank::dto::ListAccounts; -using bank::dto::OpenAccount; BRIDGE_REGISTER_MODEL(AccountModel, "AccountModel") -BRIDGE_REGISTER_ACTION(AccountModel, OpenAccount, "OpenAccount") -BRIDGE_REGISTER_ACTION(AccountModel, ListAccounts, "ListAccounts", ::morph::model::Loggable::No) BRIDGE_REGISTER_ACTION(AccountModel, GetAccount, "GetAccount", ::morph::model::Loggable::No) BRIDGE_REGISTER_ACTION(AccountModel, CloseAccount, "CloseAccount") + +// Both actions name the account they act on, so a shared handler attaches (or +// re-points) to that account's instance on the way through. +BRIDGE_KEY_FROM(GetAccount, &GetAccount::id); +BRIDGE_KEY_FROM(CloseAccount, &CloseAccount::id); diff --git a/examples/bank/include/bank/models/customer_model.hpp b/examples/bank/include/bank/models/customer_model.hpp new file mode 100644 index 0000000..718bc98 --- /dev/null +++ b/examples/bank/include/bank/models/customer_model.hpp @@ -0,0 +1,51 @@ +// SPDX-License-Identifier: Apache-2.0 +#pragma once + +#include +#include +#include +#include + +#include "bank/db/db_model.hpp" +#include "bank/dto/account_dto.hpp" + +/// @file +/// The Customer model — the *per-owner* half of what used to be one +/// `AccountModel` doing two unrelated jobs. +/// +/// `ListAccounts` and `OpenAccount` were never account-scoped operations: they +/// are scoped by owner, which is exactly why both DTOs carry an `owner` field +/// while `GetAccount`/`CloseAccount` carry an account id. Splitting the model by +/// the entity it is actually about is what gives each half a primary key that +/// identifies something. + +namespace bank { + +/// @brief One customer: lists and opens the accounts they own. +class CustomerModel : private db::WithMapper { +public: + /// @brief Owner username. Declaring this alias is what makes the model keyed. + using PrimaryKey = std::string; + + /// @brief Opens a new account for the requested (or session) owner. + dto::AccountInfo execute(const dto::OpenAccount& action); + + /// @brief Lists accounts owned by the requested (or session) owner. + dto::AccountList execute(const dto::ListAccounts& action); +}; + +} // namespace bank + +using bank::CustomerModel; +using bank::dto::ListAccounts; +using bank::dto::OpenAccount; + +BRIDGE_REGISTER_MODEL(CustomerModel, "CustomerModel") +BRIDGE_REGISTER_ACTION(CustomerModel, OpenAccount, "OpenAccount") +BRIDGE_REGISTER_ACTION(CustomerModel, ListAccounts, "ListAccounts", ::morph::model::Loggable::No) + +// Keyed by owner. An empty `owner` means "the session principal", which is not +// a key the directory can share on, so such a call simply runs on whatever +// instance the handler already holds. +BRIDGE_KEY_FROM(ListAccounts, &ListAccounts::owner); +BRIDGE_KEY_FROM(OpenAccount, &OpenAccount::owner); diff --git a/examples/bank/src/cli/main.cpp b/examples/bank/src/cli/main.cpp index 3ba0026..abcb1b3 100644 --- a/examples/bank/src/cli/main.cpp +++ b/examples/bank/src/cli/main.cpp @@ -36,7 +36,7 @@ #include "bank/dto/payment_dto.hpp" #include "bank/dto/statement_dto.hpp" #include "bank/dto/transaction_dto.hpp" -#include "bank/models/account_model.hpp" +#include "bank/models/customer_model.hpp" #include "bank/models/auth_model.hpp" #include "bank/models/budget_model.hpp" #include "bank/models/card_model.hpp" @@ -79,7 +79,7 @@ void runScenario(morph::bridge::Bridge& bridge, morph::exec::MainThreadExecutor& std::println("\n========== {} ==========", label); morph::bridge::BridgeHandler auth{bridge, &gui}; - morph::bridge::BridgeHandler accounts{bridge, &gui}; + morph::bridge::BridgeHandler accounts{bridge, &gui}; morph::bridge::BridgeHandler txns{bridge, &gui}; morph::bridge::BridgeHandler payees{bridge, &gui}; morph::bridge::BridgeHandler payments{bridge, &gui}; diff --git a/examples/bank/src/models/account_model.cpp b/examples/bank/src/models/account_model.cpp index 887aeb4..e36a3b5 100644 --- a/examples/bank/src/models/account_model.cpp +++ b/examples/bank/src/models/account_model.cpp @@ -4,115 +4,56 @@ #include #include -#include #include #include "bank/core/errors.hpp" #include "bank/core/principal.hpp" #include "bank/core/types.hpp" +#include "bank/db/account_mapping.hpp" #include "bank/db/ledger_ops.hpp" -#include "bank/db/user_ops.hpp" +#include "bank/db/row_versions.hpp" namespace bank { -namespace { - -/// Generates a pseudo-IBAN-ish account number: "DE" + 20 digits. Good enough -/// for an example; not a real check-digit-valid IBAN. -std::string generateAccountNumber() { - static thread_local std::mt19937_64 rng{std::random_device{}()}; - std::uniform_int_distribution digit{0, 9}; - std::string number = "DE"; - for (int idx = 0; idx < 20; ++idx) { - number.push_back(static_cast('0' + digit(rng))); - } - return number; -} - -/// Translates a persisted `AccountRecord` into the wire `AccountInfo` DTO. -/// @p owner is the resolved owner username (the wire DTO carries the username -/// rather than the internal `user_id` the record stores). -dto::AccountInfo toInfo(const db::AccountRecord& rec, const std::string& owner) { - return dto::AccountInfo{ - .id = static_cast(rec.id.Value()), - .owner = owner, - .number = std::string{rec.number.Value().str()}, - .kind = rec.kind.Value(), - .currency = rec.currency.Value(), - .balanceMinor = rec.balanceMinor.Value(), - .overdraftMinor = rec.overdraftMinor.Value(), - .status = rec.status.Value(), - .interestBps = rec.interestBps.Value(), - }; -} - -/// Default annual interest for savings accounts (1.5% = 150 bps); others earn 0. -int defaultInterestBps(int kind) { return kind == static_cast(AccountKind::Savings) ? 150 : 0; } - -} // namespace - -dto::AccountInfo AccountModel::execute(const dto::OpenAccount& action) { - if (!action.validate()) { - throw ValidationError{"invalid account kind/currency/overdraft"}; - } - const std::string owner = resolveOwner(action.owner); - if (owner.empty()) { - throw Unauthorized{"no session principal to own the account"}; - } - - db::AccountRecord rec; - db::setReference(rec.user, db::requireUserId(mapper(), owner)); - rec.number = Light::SqlAnsiString<34>{generateAccountNumber()}; - rec.kind = action.kind; - rec.currency = action.currency; - rec.balanceMinor = 0; - rec.overdraftMinor = action.overdraftMinor; - rec.status = static_cast(AccountStatus::Open); - rec.interestBps = defaultInterestBps(action.kind); - - mapper().Create(rec); - return toInfo(rec, owner); -} - -dto::AccountList AccountModel::execute(const dto::ListAccounts& action) { - const std::string owner = resolveOwner(action.owner); - if (owner.empty()) { - throw Unauthorized{"no session principal to list accounts for"}; - } - - // Load the owner and walk the `UserRecord::accounts` HasMany relation rather - // than issuing a manual `WHERE user_id = ?` — the relation resolves the join - // for us and returns the user's accounts directly. - const auto userId = db::requireUserId(mapper(), owner); - auto user = mapper().QuerySingle(userId).value(); - - dto::AccountList out; - out.accounts.reserve(user.accounts.Count()); - for (const auto& account : user.accounts.All()) { - out.accounts.push_back(toInfo(*account, owner)); +void AccountModel::hydrate(std::int64_t accountId) { + const std::string owner = sessionPrincipal(); + // Re-read only when we have to: a different account, nothing cached yet, a + // different principal asking, or somebody else wrote the row (a transfer or + // bill payment settling on another model's connection — see + // db/row_versions.hpp). Otherwise the answer is already in memory, which is + // the entire point of a stateful model. + const auto current = db::rowVersion(accountId); + if (_loadedId == accountId && _owner == owner && _seenVersion == current) { + return; } - return out; + _row = db::loadOwned(mapper(), accountId, owner, "account"); + _owner = owner; + _loadedId = accountId; + _seenVersion = current; } dto::AccountInfo AccountModel::execute(const dto::GetAccount& action) { - const std::string owner = sessionPrincipal(); - auto rec = db::loadOwned(mapper(), action.id, owner, "account"); - return toInfo(rec, owner); + hydrate(action.id); + return db::toAccountInfo(_row, _owner); } dto::CommandResult AccountModel::execute(const dto::CloseAccount& action) { - auto rec = db::loadOwned(mapper(), action.id, sessionPrincipal(), "account"); + hydrate(action.id); // Best-effort zero-balance guard. The balance is read on this model's own // connection, so a deposit committing on another model's connection between // this read and the Update could leave a Closed account holding funds — the // same cross-connection window documented in ledger_ops.hpp. A production // ledger would close the account inside the same transaction that settles // its balance, or gate on an atomic conditional update. - if (rec.balanceMinor.Value() != 0) { + if (_row.balanceMinor.Value() != 0) { return dto::CommandResult{.ok = false, .message = "account balance must be zero before closing"}; } - rec.status = static_cast(AccountStatus::Closed); - mapper().Update(rec); + _row.status = static_cast(AccountStatus::Closed); + mapper().Update(_row); + // Write through, then publish the new version so any other cached holder of + // this row re-hydrates rather than serving a stale status. + db::bumpRowVersion(action.id); + _seenVersion = db::rowVersion(action.id); return dto::CommandResult{.ok = true, .message = "account closed"}; } diff --git a/examples/bank/src/models/customer_model.cpp b/examples/bank/src/models/customer_model.cpp new file mode 100644 index 0000000..0dae7f6 --- /dev/null +++ b/examples/bank/src/models/customer_model.cpp @@ -0,0 +1,84 @@ +// SPDX-License-Identifier: Apache-2.0 + +#include "bank/models/customer_model.hpp" + +#include +#include +#include +#include + +#include "bank/core/errors.hpp" +#include "bank/core/principal.hpp" +#include "bank/core/types.hpp" +#include "bank/db/account_mapping.hpp" +#include "bank/db/ledger_ops.hpp" +#include "bank/db/user_ops.hpp" + +namespace bank { + +namespace { + +/// Generates a pseudo-IBAN-ish account number: "DE" + 20 digits. Good enough +/// for an example; not a real check-digit-valid IBAN. +std::string generateAccountNumber() { + static thread_local std::mt19937_64 rng{std::random_device{}()}; + std::uniform_int_distribution digit{0, 9}; + std::string number = "DE"; + for (int idx = 0; idx < 20; ++idx) { + number.push_back(static_cast('0' + digit(rng))); + } + return number; +} + +/// Default annual interest for savings accounts (1.5% = 150 bps); others earn 0. +int defaultInterestBps(int kind) { return kind == static_cast(AccountKind::Savings) ? 150 : 0; } + +} // namespace + +dto::AccountInfo CustomerModel::execute(const dto::OpenAccount& action) { + if (!action.validate()) { + throw ValidationError{"invalid account kind/currency/overdraft"}; + } + const std::string owner = resolveOwner(action.owner); + if (owner.empty()) { + throw Unauthorized{"no session principal to own the account"}; + } + + db::AccountRecord rec; + db::setReference(rec.user, db::requireUserId(mapper(), owner)); + rec.number = Light::SqlAnsiString<34>{generateAccountNumber()}; + rec.kind = action.kind; + rec.currency = action.currency; + rec.balanceMinor = 0; + rec.overdraftMinor = action.overdraftMinor; + rec.status = static_cast(AccountStatus::Open); + rec.interestBps = defaultInterestBps(action.kind); + + mapper().Create(rec); + return db::toAccountInfo(rec, owner); +} + +dto::AccountList CustomerModel::execute(const dto::ListAccounts& action) { + const std::string owner = resolveOwner(action.owner); + if (owner.empty()) { + throw Unauthorized{"no session principal to list accounts for"}; + } + + // Load the owner and walk the `UserRecord::accounts` HasMany relation rather + // than issuing a manual `WHERE user_id = ?` — the relation resolves the join + // for us and returns the user's accounts directly. + const auto userId = db::requireUserId(mapper(), owner); + auto user = mapper().QuerySingle(userId); + if (!user.has_value()) { + throw NotFound{"owner not found"}; + } + + dto::AccountList out; + out.accounts.reserve(user->accounts.Count()); + for (const auto& account : user->accounts.All()) { + out.accounts.push_back(db::toAccountInfo(*account, owner)); + } + return out; +} + +} // namespace bank diff --git a/examples/bank/tests/test_account.cpp b/examples/bank/tests/test_account.cpp index 4f3cc6e..a40083b 100644 --- a/examples/bank/tests/test_account.cpp +++ b/examples/bank/tests/test_account.cpp @@ -10,6 +10,7 @@ #include "bank/core/types.hpp" #include "bank/dto/account_dto.hpp" #include "bank/models/account_model.hpp" +#include "bank/models/customer_model.hpp" #include "bank_test_support.hpp" using bank::testing::await; @@ -29,9 +30,10 @@ TEST_CASE("AccountModel opens, lists, fetches and closes accounts", "[account]") app.login("alice-account-basic"); morph::bridge::BridgeHandler accounts{app.bridge(), app.gui()}; + morph::bridge::BridgeHandler accountsOwner{app.bridge(), app.gui()}; SECTION("open populates an account with a number and zero balance") { - auto info = await(accounts.execute(bank::dto::OpenAccount{ + auto info = await(accountsOwner.execute(bank::dto::OpenAccount{ .owner = "", .kind = static_cast(bank::AccountKind::Checking), .currency = static_cast(bank::Currency::EUR), @@ -49,7 +51,7 @@ TEST_CASE("AccountModel opens, lists, fetches and closes accounts", "[account]") } SECTION("savings accounts get a non-zero interest rate") { - auto info = await(accounts.execute(bank::dto::OpenAccount{ + auto info = await(accountsOwner.execute(bank::dto::OpenAccount{ .kind = static_cast(bank::AccountKind::Savings), .currency = static_cast(bank::Currency::USD), }), @@ -58,10 +60,10 @@ TEST_CASE("AccountModel opens, lists, fetches and closes accounts", "[account]") } SECTION("list returns only the session owner's accounts") { - await(accounts.execute(bank::dto::OpenAccount{.kind = 0, .currency = 0}), app.guiLoop()); - await(accounts.execute(bank::dto::OpenAccount{.kind = 1, .currency = 0}), app.guiLoop()); + await(accountsOwner.execute(bank::dto::OpenAccount{.kind = 0, .currency = 0}), app.guiLoop()); + await(accountsOwner.execute(bank::dto::OpenAccount{.kind = 1, .currency = 0}), app.guiLoop()); - auto list = await(accounts.execute(bank::dto::ListAccounts{}), app.guiLoop()); + auto list = await(accountsOwner.execute(bank::dto::ListAccounts{}), app.guiLoop()); REQUIRE(list.accounts.size() >= 2); for (const auto& acct : list.accounts) { REQUIRE(acct.owner == "alice-account-basic"); @@ -69,14 +71,14 @@ TEST_CASE("AccountModel opens, lists, fetches and closes accounts", "[account]") } SECTION("get returns the same account that was opened") { - auto opened = await(accounts.execute(bank::dto::OpenAccount{.kind = 0, .currency = 0}), app.guiLoop()); + auto opened = await(accountsOwner.execute(bank::dto::OpenAccount{.kind = 0, .currency = 0}), app.guiLoop()); auto fetched = await(accounts.execute(bank::dto::GetAccount{.id = opened.id}), app.guiLoop()); REQUIRE(fetched.id == opened.id); REQUIRE(fetched.number == opened.number); } SECTION("closing a zero-balance account succeeds") { - auto opened = await(accounts.execute(bank::dto::OpenAccount{.kind = 0, .currency = 0}), app.guiLoop()); + auto opened = await(accountsOwner.execute(bank::dto::OpenAccount{.kind = 0, .currency = 0}), app.guiLoop()); auto result = await(accounts.execute(bank::dto::CloseAccount{.id = opened.id}), app.guiLoop()); REQUIRE(result.ok); @@ -89,6 +91,7 @@ TEST_CASE("AccountModel reports errors through onError", "[account]") { bank::app::App app{dbConnectionForTests()}; app.login("bob-account-errors"); morph::bridge::BridgeHandler accounts{app.bridge(), app.gui()}; + morph::bridge::BridgeHandler accountsOwner{app.bridge(), app.gui()}; SECTION("fetching a non-existent account throws NotFound") { REQUIRE_THROWS_AS(await(accounts.execute(bank::dto::GetAccount{.id = 999999}), app.guiLoop()), bank::NotFound); @@ -99,7 +102,7 @@ TEST_CASE("AccountModel reports errors through onError", "[account]") { // so morph's ActionValidator gate catches this before AccountModel:: // execute() runs -- see docs/spec/forms/forms.md, "Security / trust // boundary". - REQUIRE_THROWS_AS(await(accounts.execute(bank::dto::OpenAccount{.kind = 0, .currency = 99}), app.guiLoop()), + REQUIRE_THROWS_AS(await(accountsOwner.execute(bank::dto::OpenAccount{.kind = 0, .currency = 99}), app.guiLoop()), morph::model::ValidationError); } } diff --git a/examples/bank/tests/test_budget.cpp b/examples/bank/tests/test_budget.cpp index ca1367c..cac89d4 100644 --- a/examples/bank/tests/test_budget.cpp +++ b/examples/bank/tests/test_budget.cpp @@ -12,7 +12,7 @@ #include "bank/dto/account_dto.hpp" #include "bank/dto/budget_dto.hpp" #include "bank/dto/transaction_dto.hpp" -#include "bank/models/account_model.hpp" +#include "bank/models/customer_model.hpp" #include "bank/models/budget_model.hpp" #include "bank/models/transaction_model.hpp" #include "bank_test_support.hpp" @@ -32,7 +32,7 @@ std::string testConnection() { TEST_CASE("BudgetModel upserts budgets and computes spending", "[budget]") { bank::app::App app{testConnection()}; app.login("laura-budget"); - morph::bridge::BridgeHandler accounts{app.bridge(), app.gui()}; + morph::bridge::BridgeHandler accounts{app.bridge(), app.gui()}; morph::bridge::BridgeHandler txns{app.bridge(), app.gui()}; morph::bridge::BridgeHandler budgets{app.bridge(), app.gui()}; diff --git a/examples/bank/tests/test_card.cpp b/examples/bank/tests/test_card.cpp index 5d70733..2a712a9 100644 --- a/examples/bank/tests/test_card.cpp +++ b/examples/bank/tests/test_card.cpp @@ -10,7 +10,7 @@ #include "bank/core/types.hpp" #include "bank/dto/account_dto.hpp" #include "bank/dto/card_dto.hpp" -#include "bank/models/account_model.hpp" +#include "bank/models/customer_model.hpp" #include "bank/models/card_model.hpp" #include "bank_test_support.hpp" @@ -28,7 +28,7 @@ std::string testConnection() { TEST_CASE("CardModel issues and manages cards", "[card]") { bank::app::App app{testConnection()}; app.login("judy-card"); - morph::bridge::BridgeHandler accounts{app.bridge(), app.gui()}; + morph::bridge::BridgeHandler accounts{app.bridge(), app.gui()}; morph::bridge::BridgeHandler cards{app.bridge(), app.gui()}; const auto account = await(accounts.execute(bank::dto::OpenAccount{.kind = 0, .currency = 0}), app.guiLoop()).id; diff --git a/examples/bank/tests/test_loan.cpp b/examples/bank/tests/test_loan.cpp index eaaa73b..ffb2fee 100644 --- a/examples/bank/tests/test_loan.cpp +++ b/examples/bank/tests/test_loan.cpp @@ -13,6 +13,7 @@ #include "bank/dto/account_dto.hpp" #include "bank/dto/loan_dto.hpp" #include "bank/models/account_model.hpp" +#include "bank/models/customer_model.hpp" #include "bank/models/loan_model.hpp" #include "bank_test_support.hpp" @@ -32,10 +33,11 @@ TEST_CASE("LoanModel disburses, schedules, and repays", "[loan]") { bank::app::App app{testConnection()}; app.login("ken-loan"); morph::bridge::BridgeHandler accounts{app.bridge(), app.gui()}; + morph::bridge::BridgeHandler accountsOwner{app.bridge(), app.gui()}; morph::bridge::BridgeHandler loans{app.bridge(), app.gui()}; const auto account = - await(accounts.execute(bank::dto::OpenAccount{.kind = 0, .currency = 0}), app.guiLoop()).id; + await(accountsOwner.execute(bank::dto::OpenAccount{.kind = 0, .currency = 0}), app.guiLoop()).id; SECTION("applying disburses the principal into the account") { auto loan = await(loans.execute(bank::dto::ApplyLoan{.accountId = account, diff --git a/examples/bank/tests/test_offline.cpp b/examples/bank/tests/test_offline.cpp index 05b5848..05c9e76 100644 --- a/examples/bank/tests/test_offline.cpp +++ b/examples/bank/tests/test_offline.cpp @@ -19,6 +19,7 @@ #include "bank/dto/account_dto.hpp" #include "bank/dto/transaction_dto.hpp" #include "bank/models/account_model.hpp" +#include "bank/models/customer_model.hpp" #include "bank/models/transaction_model.hpp" #include "bank_test_support.hpp" @@ -38,10 +39,11 @@ TEST_CASE("Offline deposits are queued and replayed on reconnect", "[offline]") bank::app::App app{testConnection()}; app.login("peter-offline"); morph::bridge::BridgeHandler accounts{app.bridge(), app.gui()}; + morph::bridge::BridgeHandler accountsOwner{app.bridge(), app.gui()}; morph::bridge::BridgeHandler txns{app.bridge(), app.gui()}; const auto acct = - await(accounts.execute(bank::dto::OpenAccount{.kind = 0, .currency = 0}), app.guiLoop()).id; + await(accountsOwner.execute(bank::dto::OpenAccount{.kind = 0, .currency = 0}), app.guiLoop()).id; // --- While "offline": park deposits in the durable queue instead of sending. morph::offline::InMemoryOfflineQueue queue; diff --git a/examples/bank/tests/test_payee.cpp b/examples/bank/tests/test_payee.cpp index 378f442..21267c9 100644 --- a/examples/bank/tests/test_payee.cpp +++ b/examples/bank/tests/test_payee.cpp @@ -57,25 +57,32 @@ TEST_CASE("PayeeModel add/list/remove scoped to the owner", "[payee]") { } } -TEST_CASE("PayeeModel form-style streaming via subscribe/set", "[payee][subscribe]") { +TEST_CASE("PayeeModel notifies subscribers of the state it produces", "[payee][subscribe]") { bank::app::App app{testConnection()}; app.login("heidi-form"); morph::bridge::BridgeHandler payees{app.bridge(), app.gui()}; + // Subscribing names the result type, so the observer never has to know + // which action produced it. std::atomic fired{false}; std::int64_t newId = 0; - payees.subscribe([&](bank::dto::PayeeInfo info) { + payees.subscribe([&](const bank::dto::PayeeInfo& info) { newId = info.id; fired.store(true); }); - // Setting the name alone does not satisfy validate() (no IBAN yet)... - payees.set<&bank::dto::AddPayee::name>("Streamed Payee"); - app.guiLoop().runFor(std::chrono::milliseconds{50}); + // An incomplete payee fails validate() (no IBAN), so the dispatch-path + // validator rejects it and no subscriber is notified. + std::atomic rejected{false}; + payees.execute(bank::dto::AddPayee{.name = "Streamed Payee", .iban = ""}) + .onError([&](const std::exception_ptr&) { rejected.store(true); }); + REQUIRE(waitUntil([&] { return rejected.load(); }, std::chrono::milliseconds{2000}, + std::chrono::milliseconds{5}, app.guiLoop())); REQUIRE_FALSE(fired.load()); - // ...completing the IBAN makes the draft ready and fires the action. - payees.set<&bank::dto::AddPayee::iban>("FR1420041010050500013M02606"); + // A complete one succeeds and the subscription fires. + await(payees.execute(bank::dto::AddPayee{.name = "Streamed Payee", .iban = "FR1420041010050500013M02606"}), + app.guiLoop()); REQUIRE(waitUntil([&] { return fired.load(); }, std::chrono::milliseconds{2000}, std::chrono::milliseconds{5}, app.guiLoop())); REQUIRE(newId > 0); diff --git a/examples/bank/tests/test_payment.cpp b/examples/bank/tests/test_payment.cpp index f91c5c4..52c7830 100644 --- a/examples/bank/tests/test_payment.cpp +++ b/examples/bank/tests/test_payment.cpp @@ -14,6 +14,7 @@ #include "bank/dto/payee_dto.hpp" #include "bank/dto/payment_dto.hpp" #include "bank/models/account_model.hpp" +#include "bank/models/customer_model.hpp" #include "bank/models/payee_model.hpp" #include "bank/models/payment_model.hpp" #include "bank/models/transaction_model.hpp" @@ -35,12 +36,13 @@ TEST_CASE("PaymentModel pays bills, schedules, and cancels", "[payment]") { bank::app::App app{testConnection()}; app.login("ivan-pay"); morph::bridge::BridgeHandler accounts{app.bridge(), app.gui()}; + morph::bridge::BridgeHandler accountsOwner{app.bridge(), app.gui()}; morph::bridge::BridgeHandler payees{app.bridge(), app.gui()}; morph::bridge::BridgeHandler txns{app.bridge(), app.gui()}; morph::bridge::BridgeHandler payments{app.bridge(), app.gui()}; const auto account = - await(accounts.execute(bank::dto::OpenAccount{.kind = 0, .currency = 0}), app.guiLoop()).id; + await(accountsOwner.execute(bank::dto::OpenAccount{.kind = 0, .currency = 0}), app.guiLoop()).id; await(txns.execute(bank::dto::Deposit{.accountId = account, .amountMinor = 50000}), app.guiLoop()); const auto payee = await(payees.execute(bank::dto::AddPayee{.name = "Electric Co", .iban = "DE89370400440532013000"}), diff --git a/examples/bank/tests/test_relations.cpp b/examples/bank/tests/test_relations.cpp index eb6c614..3ac5219 100644 --- a/examples/bank/tests/test_relations.cpp +++ b/examples/bank/tests/test_relations.cpp @@ -28,7 +28,7 @@ #include "bank/dto/payee_dto.hpp" #include "bank/dto/payment_dto.hpp" #include "bank/dto/transaction_dto.hpp" -#include "bank/models/account_model.hpp" +#include "bank/models/customer_model.hpp" #include "bank/models/card_model.hpp" #include "bank/models/payee_model.hpp" #include "bank/models/payment_model.hpp" @@ -52,7 +52,7 @@ TEST_CASE("ORM relations: BelongsTo navigation and HasMany inverses", "[relation bank::app::App app{dbConnectionForTests()}; app.login(principal); // provisions the users row - morph::bridge::BridgeHandler accounts{app.bridge(), app.gui()}; + morph::bridge::BridgeHandler accounts{app.bridge(), app.gui()}; morph::bridge::BridgeHandler cards{app.bridge(), app.gui()}; morph::bridge::BridgeHandler payees{app.bridge(), app.gui()}; morph::bridge::BridgeHandler payments{app.bridge(), app.gui()}; diff --git a/examples/bank/tests/test_remote.cpp b/examples/bank/tests/test_remote.cpp index d59a98b..927a671 100644 --- a/examples/bank/tests/test_remote.cpp +++ b/examples/bank/tests/test_remote.cpp @@ -20,6 +20,7 @@ #include "bank/core/types.hpp" #include "bank/dto/account_dto.hpp" #include "bank/models/account_model.hpp" +#include "bank/models/customer_model.hpp" #include "bank_test_support.hpp" using bank::testing::await; @@ -73,9 +74,10 @@ TEST_CASE("AccountModel runs unchanged over a remote backend", "[remote]") { bank::testing::ensurePrincipal("olivia-remote"); morph::bridge::BridgeHandler accounts{bridge, &gui}; + morph::bridge::BridgeHandler accountsOwner{bridge, &gui}; SECTION("opening an account round-trips through JSON serialisation") { - auto info = await(accounts.execute(bank::dto::OpenAccount{ + auto info = await(accountsOwner.execute(bank::dto::OpenAccount{ .kind = static_cast(bank::AccountKind::Savings), .currency = static_cast(bank::Currency::GBP), }), @@ -86,7 +88,7 @@ TEST_CASE("AccountModel runs unchanged over a remote backend", "[remote]") { } SECTION("the authorizer rejects the forbidden action") { - auto info = await(accounts.execute(bank::dto::OpenAccount{.kind = 0, .currency = 0}), gui); + auto info = await(accountsOwner.execute(bank::dto::OpenAccount{.kind = 0, .currency = 0}), gui); REQUIRE_THROWS(await(accounts.execute(bank::dto::CloseAccount{.id = info.id}), gui)); } } diff --git a/examples/bank/tests/test_stateful_account.cpp b/examples/bank/tests/test_stateful_account.cpp new file mode 100644 index 0000000..328e9b9 --- /dev/null +++ b/examples/bank/tests/test_stateful_account.cpp @@ -0,0 +1,135 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// Tests for the stateful, keyed AccountModel (F1). +// +// The point of the reshape is that an account instance *holds* its row rather +// than re-querying it, and that two handlers naming the same account reach one +// instance. A stateless model would satisfy none of these assertions +// non-vacuously — the balance would simply be re-read from SQLite every time. +// +// See docs/planned/stateful_bank_example.md. + +#include +#include +#include +#include +#include + +#include "bank/app/app.hpp" +#include "bank/core/types.hpp" +#include "bank/dto/account_dto.hpp" +#include "bank/dto/transaction_dto.hpp" +#include "bank/models/account_model.hpp" +#include "bank/models/customer_model.hpp" +#include "bank/models/transaction_model.hpp" +#include "bank_test_support.hpp" + +using bank::testing::await; +using morph::bridge::AllowShared; +using morph::bridge::BridgeHandler; + +namespace { + +std::string statefulTestConnection() { + bank::testing::ensureDatabase(); + return "DRIVER=SQLite3;Database=" + (std::filesystem::temp_directory_path() / "morph_bank_tests.db").string(); +} + +/// Opens a checking account for the logged-in principal and returns its id. +std::int64_t openChecking(bank::app::App& app, BridgeHandler& customer) { + auto info = await(customer.execute(bank::dto::OpenAccount{ + .kind = static_cast(bank::AccountKind::Checking), + .currency = static_cast(bank::Currency::USD), + .overdraftMinor = 0, + }), + app.guiLoop()); + return info.id; +} + +} // namespace + +TEST_CASE("two shared handlers on one account reach one instance", "[stateful-account]") { + bank::app::App app{statefulTestConnection()}; + app.login("sid-shared-instance"); + + BridgeHandler customer{app.bridge(), app.gui()}; + const auto acct = openChecking(app, customer); + + BridgeHandler screen{app.bridge(), app.gui()}; + BridgeHandler sidebar{app.bridge(), app.gui()}; + + // Each keyed action attaches its handler to the account it names, so both + // land on the same instance rather than on two copies of the same row. + REQUIRE(await(screen.execute(bank::dto::GetAccount{.id = acct}), app.guiLoop()).id == acct); + REQUIRE(await(sidebar.execute(bank::dto::GetAccount{.id = acct}), app.guiLoop()).id == acct); + + REQUIRE(screen.primary().value_or(-1) == acct); + REQUIRE(sidebar.primary().value_or(-1) == acct); + + // The directory holds exactly one entry for the account both handlers named. + REQUIRE(await(screen.instances(), app.guiLoop()) == std::vector{acct}); +} + +TEST_CASE("a cached account re-hydrates after another model moves money", "[stateful-account]") { + bank::app::App app{statefulTestConnection()}; + app.login("tess-stale-cache"); + + BridgeHandler customer{app.bridge(), app.gui()}; + const auto acct = openChecking(app, customer); + + BridgeHandler account{app.bridge(), app.gui()}; + BridgeHandler txns{app.bridge(), app.gui()}; + + REQUIRE(await(account.execute(bank::dto::GetAccount{.id = acct}), app.guiLoop()).balanceMinor == 0); + + // TransactionModel owns the atomic write and settles on its *own* SQLite + // connection, so the cached row above is now stale. The version bump is what + // makes the next read notice — without it a stateful model would happily + // serve money that no longer exists. + await(txns.execute(bank::dto::Deposit{.accountId = acct, .amountMinor = 12345}), app.guiLoop()); + + REQUIRE(await(account.execute(bank::dto::GetAccount{.id = acct}), app.guiLoop()).balanceMinor == 12345); +} + +TEST_CASE("a plain account handler keeps its own instance", "[stateful-account]") { + bank::app::App app{statefulTestConnection()}; + app.login("percy-private-instance"); + + BridgeHandler customer{app.bridge(), app.gui()}; + const auto acct = openChecking(app, customer); + + BridgeHandler shared{app.bridge(), app.gui()}; + BridgeHandler priv{app.bridge(), app.gui()}; + + REQUIRE(await(shared.execute(bank::dto::GetAccount{.id = acct}), app.guiLoop()).id == acct); + REQUIRE(await(priv.execute(bank::dto::GetAccount{.id = acct}), app.guiLoop()).id == acct); + + // Both answer correctly — they read the same row — but only the opted-in + // handler is in the directory, so the plain one is invisible to it. + REQUIRE(await(shared.instances(), app.guiLoop()).size() == 1); +} + +TEST_CASE("closing through the cached instance still enforces the zero-balance rule", "[stateful-account]") { + bank::app::App app{statefulTestConnection()}; + app.login("cass-close-guard"); + + BridgeHandler customer{app.bridge(), app.gui()}; + const auto acct = openChecking(app, customer); + + BridgeHandler account{app.bridge(), app.gui()}; + BridgeHandler txns{app.bridge(), app.gui()}; + + await(txns.execute(bank::dto::Deposit{.accountId = acct, .amountMinor = 500}), app.guiLoop()); + + // The guard reads the cached row, so it only holds because the deposit + // invalidated that cache. + auto refused = await(account.execute(bank::dto::CloseAccount{.id = acct}), app.guiLoop()); + REQUIRE_FALSE(refused.ok); + + await(txns.execute(bank::dto::Withdraw{.accountId = acct, .amountMinor = 500}), app.guiLoop()); + auto accepted = await(account.execute(bank::dto::CloseAccount{.id = acct}), app.guiLoop()); + REQUIRE(accepted.ok); + + REQUIRE(await(account.execute(bank::dto::GetAccount{.id = acct}), app.guiLoop()).status == + static_cast(bank::AccountStatus::Closed)); +} diff --git a/examples/bank/tests/test_statement.cpp b/examples/bank/tests/test_statement.cpp index 9235537..4e1a159 100644 --- a/examples/bank/tests/test_statement.cpp +++ b/examples/bank/tests/test_statement.cpp @@ -11,7 +11,7 @@ #include "bank/dto/account_dto.hpp" #include "bank/dto/statement_dto.hpp" #include "bank/dto/transaction_dto.hpp" -#include "bank/models/account_model.hpp" +#include "bank/models/customer_model.hpp" #include "bank/models/statement_model.hpp" #include "bank/models/transaction_model.hpp" #include "bank_test_support.hpp" @@ -31,7 +31,7 @@ std::string testConnection() { TEST_CASE("StatementModel aggregates credits and debits across accounts", "[statement]") { bank::app::App app{testConnection()}; app.login("nina-stmt"); - morph::bridge::BridgeHandler accounts{app.bridge(), app.gui()}; + morph::bridge::BridgeHandler accounts{app.bridge(), app.gui()}; morph::bridge::BridgeHandler txns{app.bridge(), app.gui()}; morph::bridge::BridgeHandler statements{app.bridge(), app.gui()}; diff --git a/examples/bank/tests/test_transaction.cpp b/examples/bank/tests/test_transaction.cpp index 9c20580..b1884eb 100644 --- a/examples/bank/tests/test_transaction.cpp +++ b/examples/bank/tests/test_transaction.cpp @@ -11,6 +11,7 @@ #include "bank/dto/account_dto.hpp" #include "bank/dto/transaction_dto.hpp" #include "bank/models/account_model.hpp" +#include "bank/models/customer_model.hpp" #include "bank/models/transaction_model.hpp" #include "bank_test_support.hpp" @@ -24,9 +25,9 @@ std::string testConnection() { } /// Opens a fresh checking account in the given currency and returns its id. -std::int64_t openAccount(bank::app::App& app, morph::bridge::BridgeHandler& accounts, +std::int64_t openAccount(bank::app::App& app, morph::bridge::BridgeHandler& customer, bank::Currency currency = bank::Currency::USD) { - auto info = await(accounts.execute(bank::dto::OpenAccount{ + auto info = await(customer.execute(bank::dto::OpenAccount{ .kind = static_cast(bank::AccountKind::Checking), .currency = static_cast(currency), .overdraftMinor = 0, @@ -41,9 +42,10 @@ TEST_CASE("TransactionModel deposit / withdraw adjust balances and ledger", "[tr bank::app::App app{testConnection()}; app.login("erin-txn"); morph::bridge::BridgeHandler accounts{app.bridge(), app.gui()}; + morph::bridge::BridgeHandler accountsOwner{app.bridge(), app.gui()}; morph::bridge::BridgeHandler txns{app.bridge(), app.gui()}; - const std::int64_t acct = openAccount(app, accounts); + const std::int64_t acct = openAccount(app, accountsOwner); SECTION("deposit increases the balance and records a credit") { auto entry = @@ -79,10 +81,11 @@ TEST_CASE("TransactionModel transfer is atomic and balance-preserving", "[transa bank::app::App app{testConnection()}; app.login("frank-transfer"); morph::bridge::BridgeHandler accounts{app.bridge(), app.gui()}; + morph::bridge::BridgeHandler accountsOwner{app.bridge(), app.gui()}; morph::bridge::BridgeHandler txns{app.bridge(), app.gui()}; - const std::int64_t src = openAccount(app, accounts); - const std::int64_t dst = openAccount(app, accounts); + const std::int64_t src = openAccount(app, accountsOwner); + const std::int64_t dst = openAccount(app, accountsOwner); await(txns.execute(bank::dto::Deposit{.accountId = src, .amountMinor = 10000}), app.guiLoop()); SECTION("a valid transfer moves money and conserves the total") { diff --git a/include/morph/core/backend.hpp b/include/morph/core/backend.hpp index 41b2b9f..353b390 100644 --- a/include/morph/core/backend.hpp +++ b/include/morph/core/backend.hpp @@ -53,6 +53,22 @@ struct ActionCall { ::morph::session::Context session; }; +/// @brief The two identities a model instance can carry, passed together. +/// +/// Bundled into one struct rather than passed as two adjacent `string_view` +/// parameters because they are trivially swappable at a call site and mean +/// entirely different things: transposing them would silently file journal +/// entries under the directory key and share instances under the log's entity +/// key. Keeping them named at every call site makes that mistake unwritable. +struct InstanceIdentity { + /// @brief Entity key for the action log; empty if none. See `journal::LogEntry::entityKey`. + std::string_view contextKey; + + /// @brief Canonical string encoding of the primary key; empty if the + /// instance is anonymous and therefore unshareable. + std::string_view primary; +}; + /// @brief Abstract interface for execution backends (local, remote, …). /// /// A backend owns model instances and dispatches actions against them. @@ -88,7 +104,103 @@ struct IBackend { return registerModel(typeId, std::move(factory)); } + /// @brief Registers or attaches to the shared instance holding @p primary. + /// + /// A *register-or-attach*: if an instance for `(typeId, primary)` is already + /// live in the backend's shared directory, its id is returned and its attach + /// count incremented — no new instance is created and @p factory is not + /// called. Otherwise a new instance is created, entered in the directory, + /// and returned with an attach count of one. + /// + /// An empty @p primary means "no identity": the call degrades to + /// `registerModelWithContext`, producing a private instance that never + /// enters the directory and can never be shared. + /// + /// The default implementation ignores @p primary and forwards to + /// `registerModelWithContext`, so a backend that has not implemented sharing + /// keeps its existing one-instance-per-caller behaviour rather than silently + /// handing two callers the same instance. + /// + /// @param typeId String type-id of the model. + /// @param factory Callable that constructs the `IModelHolder` (local path only). + /// @param identity Entity key for the action log plus the directory primary key. + /// @return Id of the shared (or newly created) instance. + virtual ::morph::exec::detail::ModelId registerModelShared( + const std::string& typeId, std::function()> factory, + InstanceIdentity identity) { + return registerModelWithContext(typeId, std::move(factory), identity.contextKey); + } + + /// @brief Re-points from @p current to the shared instance holding @p primary. + /// + /// The default implementation releases @p current (when non-zero) and then + /// calls `registerModelShared`, which is exactly right for an in-process + /// backend. Backends behind a wire protocol override this with the single + /// `attach` request so a re-pointing client cannot lose its slot to + /// `LimitPolicy::maxLiveModels` between the release and the acquire. + /// + /// @param typeId String type-id of the model. + /// @param factory Callable that constructs the `IModelHolder` (local path only). + /// @param identity Entity key for the action log plus the directory primary key. + /// @param current Instance currently held, or `ModelId{0}` if none. + /// @return Id of the instance now attached to. + virtual ::morph::exec::detail::ModelId attachModel( + const std::string& typeId, std::function()> factory, + InstanceIdentity identity, ::morph::exec::detail::ModelId current) { + if (current.v != 0U) { + deregisterModel(current); + } + return registerModelShared(typeId, std::move(factory), identity); + } + + /// @brief Enters an already-live instance into the directory under @p primary. + /// + /// The *promotion* half of keyed instances, and what makes a result-sourced + /// key work without losing state: an action that creates its own entity runs + /// on an instance that does not yet have a key, and the key only exists once + /// the result comes back. Re-pointing to a freshly created instance would + /// strand everything the create just did, so instead the instance the action + /// ran on is given the generated key in place. + /// + /// A no-op when @p primary is empty, when @p mid is not live, or when another + /// instance already holds that key — the existing holder always wins, so a + /// promotion can never silently displace a directory entry other handlers + /// are already attached to. + /// + /// @param mid Live instance to promote. + /// @param typeId Model type id — the directory's first key component. + /// @param primary Canonical string encoding of the key to file it under. + virtual void assignPrimary(::morph::exec::detail::ModelId mid, const std::string& typeId, + std::string_view primary) { + (void)mid; + (void)typeId; + (void)primary; + } + + /// @brief Lists the primary keys of live shared instances of @p typeId. + /// + /// Only instances created through `registerModelShared`/`attachModel` with a + /// non-empty primary appear; a private instance is invisible to the + /// directory by construction. The result is a snapshot and is stale the + /// moment it is returned. + /// + /// Synchronous, matching `registerModel`, which already blocks on remote + /// backends. The asynchronous surface users see is + /// `BridgeHandler::instances()`, which wraps this in a `Completion` so the + /// call site reads identically local and remote. + /// + /// @param typeId String type-id to enumerate. + /// @return Canonical key strings of the live shared instances; empty by default. + virtual std::vector listInstances(const std::string& typeId) { + (void)typeId; + return {}; + } + /// @brief Removes the model identified by @p mid from the backend. + /// + /// For a shared instance this *decrements* its attach count and destroys the + /// instance only when the count reaches zero, so one caller releasing an + /// instance never tears it out from under another that is still attached. virtual void deregisterModel(::morph::exec::detail::ModelId mid) = 0; /// @brief Dispatches @p call against the model identified by @p mid. @@ -195,11 +307,100 @@ class LocalBackend : public detail::IBackend { return mid; } - /// @brief Removes the model with @p mid. Thread-safe. - /// @param mid Id returned by a prior `registerModel()` call. + /// @brief Registers or attaches to the shared instance holding @p primary. + /// + /// An empty @p primary bypasses the directory entirely and produces a + /// private instance, exactly as `registerModel` does. + /// @param typeId String type-id of the model — the directory's first key component. + /// @param factory Callable that constructs the `IModelHolder`; not called on an attach. + /// @param identity Entity key for the action log plus the directory primary key. + /// @return Id of the shared (or newly created) instance. + ::morph::exec::detail::ModelId registerModelShared( + const std::string& typeId, std::function()> factory, + detail::InstanceIdentity identity) override { + if (identity.primary.empty()) { + return registerModelWithContext(typeId, std::move(factory), identity.contextKey); + } + ::morph::observe::detail::emitMetric(::morph::observe::Metric::registerCount, 1.0); + DirectoryKey dirKey{typeId, std::string{identity.primary}}; + std::scoped_lock const lock{_regMtx}; + if (auto found = _directory.find(dirKey); found != _directory.end()) { + _attachCount[found->second] += 1; + return found->second; + } + ::morph::exec::detail::ModelId const mid{_nextId.fetch_add(1) + 1}; + auto holder = factory(); + if (holder->isBackendChangeAware()) { + _changeAware.insert(mid); + } + _models[mid] = std::move(holder); + _directory.emplace(dirKey, mid); + _sharedKeyOf.emplace(mid, std::move(dirKey)); + _attachCount[mid] = 1; + return mid; + } + + /// @brief Enters an already-live instance into the directory under @p primary. Thread-safe. + /// @param mid Live instance to promote. + /// @param typeId Model type id — the directory's first key component. + /// @param primary Canonical string encoding of the key to file it under. + void assignPrimary(::morph::exec::detail::ModelId mid, const std::string& typeId, + std::string_view primary) override { + if (primary.empty()) { + return; + } + std::scoped_lock const lock{_regMtx}; + if (!_models.contains(mid)) { + return; + } + DirectoryKey dirKey{typeId, std::string{primary}}; + if (_directory.contains(dirKey)) { + return; + } + if (auto prevIter = _sharedKeyOf.find(mid); prevIter != _sharedKeyOf.end()) { + _directory.erase(prevIter->second); + _sharedKeyOf.erase(prevIter); + } + _directory.emplace(dirKey, mid); + _sharedKeyOf.emplace(mid, std::move(dirKey)); + _attachCount.try_emplace(mid, 1); + } + + /// @brief Lists the primary keys of live shared instances of @p typeId. Thread-safe. + /// @param typeId String type-id to enumerate. + /// @return Canonical key strings of the live shared instances, in unspecified order. + std::vector listInstances(const std::string& typeId) override { + std::vector keys; + std::scoped_lock const lock{_regMtx}; + for (const auto& [dirKey, mid] : _directory) { + if (dirKey.first == typeId) { + keys.push_back(dirKey.second); + } + } + return keys; + } + + /// @brief Removes the model with @p mid, or releases one attachment to it. Thread-safe. + /// + /// A private instance is erased outright. A shared instance has its attach + /// count decremented and is erased — and removed from the directory — only + /// when that count reaches zero, so releasing one handler never destroys an + /// instance another handler still holds. + /// @param mid Id returned by a prior `registerModel()`/`registerModelShared()` call. void deregisterModel(::morph::exec::detail::ModelId mid) override { ::morph::observe::detail::emitMetric(::morph::observe::Metric::deregisterCount, 1.0); std::scoped_lock const lock{_regMtx}; + if (auto refIter = _attachCount.find(mid); refIter != _attachCount.end()) { + refIter->second -= 1; + if (refIter->second > 0) { + return; + } + _attachCount.erase(refIter); + if (auto keyIter = _sharedKeyOf.find(mid); keyIter != _sharedKeyOf.end()) { + _directory.erase(keyIter->second); + _sharedKeyOf.erase(keyIter); + } + } _models.erase(mid); _changeAware.erase(mid); } @@ -367,6 +568,17 @@ class LocalBackend : public detail::IBackend { // `notifyBackendChanged()` never needs to inspect a model it doesn't have // to. Always a subset of `_models`' keys. std::unordered_set<::morph::exec::detail::ModelId, ::morph::exec::detail::ModelIdHash> _changeAware; + // Shared-instance directory: (typeId, primary) -> ModelId, plus the reverse + // lookup and per-instance attach count. All three are maintained under + // _regMtx alongside _models, so directory membership can never desync from + // instance existence — the same invariant RemoteServer's connection scopes + // maintain. Only instances registered with a non-empty primary appear here; + // a private instance has no entry in any of the three, which is exactly what + // makes deregisterModel's decrement path a no-op for it. + using DirectoryKey = std::pair; + std::unordered_map _directory; + std::unordered_map<::morph::exec::detail::ModelId, DirectoryKey, ::morph::exec::detail::ModelIdHash> _sharedKeyOf; + std::unordered_map<::morph::exec::detail::ModelId, std::size_t, ::morph::exec::detail::ModelIdHash> _attachCount; std::atomic _nextId{0}; std::mutex _pendingMtx; std::vector>>> _pending; diff --git a/include/morph/core/bridge.hpp b/include/morph/core/bridge.hpp index bc1f61b..1b93c13 100644 --- a/include/morph/core/bridge.hpp +++ b/include/morph/core/bridge.hpp @@ -7,9 +7,12 @@ #include #include #include +#include #include #include #include +#include +#include #include #include #include @@ -18,6 +21,7 @@ #include "../session/session.hpp" #include "backend.hpp" #include "completion.hpp" +#include "model_key.hpp" #include "registry.hpp" namespace morph::bridge { @@ -106,9 +110,8 @@ namespace detail { /// @brief Compile-time decomposition of a pointer-to-data-member type. /// -/// Used by `BridgeHandler::set` to recover both the action -/// type and the field type from a single non-type template parameter, so -/// callers write `handler.set<&MyAction::c>(7.0)` with no redundant type +/// Recovers both the class and the member type from a single non-type template +/// parameter, so callers name a field as `&MyAction::c` with no redundant type /// arguments. /// /// @tparam T The pointer-to-member type (e.g. `double MyAction::*`). @@ -145,6 +148,21 @@ struct HandlerBinding { /// can attach a log to the instance it creates. See `IBackend::registerModelWithContext`. std::string contextKey; + /// @brief Canonical string encoding of this instance's primary key. + /// + /// Empty until a keyed action or an explicit `attach` supplies one. Only + /// meaningful when `shared` is set; a private binding never consults it. + /// Mutated only under `Bridge::_mtx`. + std::string primary; + + /// @brief Whether this binding participates in the shared instance directory. + /// + /// Set once at construction from `BridgeHandler`'s `Sharing` template + /// argument and never changed. A shared binding defers its backend + /// registration until it has a primary, so `currentId` stays `0` — and + /// `executeVia` fails fast on "handler not bound" — until then. + bool shared = false; + /// @brief Current `ModelId` value in the active backend (0 = unbound). std::atomic currentId{0}; }; @@ -229,6 +247,213 @@ class Bridge { _handlers.push_back(binding); } + /// @brief Creates a shared, initially **unattached** binding for `Model`. + /// + /// Unlike `registerHandler()`, this registers nothing on the backend: + /// a shared handler has no instance until a keyed action or an explicit + /// `attach` tells it which one it wants. The binding is tracked from the + /// start so `switchBackend()` knows about it, but it stays unbound + /// (`currentId == 0`) until then. + /// + /// @tparam Model Concrete model type. Must have a registered `ModelTraits`. + /// @return Shared pointer to the new, unattached binding. + template + std::shared_ptr registerSharedHandler() { + auto binding = std::make_shared(); + binding->typeId = std::string{::morph::model::ModelTraits::typeId()}; + binding->modelFactory = [] { return ::morph::model::detail::ModelFactory::create(); }; + binding->shared = true; + std::scoped_lock const lock{_mtx}; + _handlers.push_back(binding); + return binding; + } + + /// @brief Attaches (or re-points) @p binding to the shared instance for @p primary. + /// + /// Idempotent: attaching to the primary a binding already holds is a no-op, + /// so a keyed action repeated against the same instance costs nothing. A + /// different primary re-points the binding — the previous instance is + /// released and survives only if another handler still holds it. + /// + /// The binding's `contextKey` is set to @p primary as well, so a keyed + /// model's journal entries carry the entity key without the caller + /// arranging it separately. + /// + /// @tparam Model Concrete model type. + /// @param binding Shared binding, as returned by `registerSharedHandler()`. + /// @param primary Canonical string encoding of the primary key to attach to. + template + void attachHandler(const std::shared_ptr& binding, std::string primary) { + std::scoped_lock const lock{_mtx}; + if (binding->primary == primary && binding->currentId.load() != 0U) { + return; + } + binding->contextKey = primary; + // The default `attachModel` releases the current instance before + // acquiring the new one. If the acquire then fails -- a transport error, + // a server at `maxLiveModels` -- the binding must not keep pointing at + // the id it just gave up, or the next execute dispatches to a released + // instance and gets a confusing "model not found" instead of the + // documented "handler not bound". Unbind first, publish only on success. + auto const previous = ::morph::exec::detail::ModelId{binding->currentId.load()}; + binding->currentId.store(0); + binding->primary.clear(); + auto newId = loadBackend()->attachModel(binding->typeId, binding->modelFactory, + {.contextKey = binding->contextKey, .primary = primary}, previous); + binding->primary = std::move(primary); + binding->currentId.store(newId.v); + } + + /// @brief Gives @p binding an anonymous instance if it does not have one yet. + /// + /// Used before a result-keyed action: such an action generates the key it + /// will be filed under, so it has to run *somewhere* first. The instance it + /// gets is private (empty primary, invisible to the directory) until + /// `assignHandlerPrimary` promotes it in place once the key is known. + /// @param binding Shared binding to bind. + void ensureBound(const std::shared_ptr& binding) { + std::scoped_lock const lock{_mtx}; + if (binding->currentId.load() != 0U) { + return; + } + auto newId = loadBackend()->registerModelShared(binding->typeId, binding->modelFactory, + {.contextKey = binding->contextKey, .primary = {}}); + binding->currentId.store(newId.v); + } + + /// @brief Files @p binding's current instance under @p primary, in place. + /// + /// The instance keeps everything the creating action just did — nothing is + /// re-created and nothing is stranded. A no-op if another instance already + /// holds that key; the existing holder always wins. + /// @tparam Model Concrete model type. + /// @param binding Shared binding whose instance is being promoted. + /// @param primary Canonical string encoding of the key to file it under. + template + void assignHandlerPrimary(const std::shared_ptr& binding, std::string primary) { + std::scoped_lock const lock{_mtx}; + uint64_t const raw = binding->currentId.load(); + if (raw == 0U || primary.empty()) { + return; + } + loadBackend()->assignPrimary(::morph::exec::detail::ModelId{raw}, binding->typeId, primary); + binding->contextKey = primary; + binding->primary = std::move(primary); + } + + /// @brief Returns @p binding's current primary key, or empty if unattached. + /// @param binding Binding to inspect. + /// @return Canonical key string, or an empty string when unattached. + [[nodiscard]] std::string bindingPrimary(const std::shared_ptr& binding) { + std::scoped_lock const lock{_mtx}; + return binding->primary; + } + + /// @brief Lists the live shared primary keys of `Model` on the active backend. + /// @tparam Model Concrete model type. + /// @return Canonical key strings, in unspecified order. + template + [[nodiscard]] std::vector listInstancesOf() { + return loadBackend()->listInstances(std::string{::morph::model::ModelTraits::typeId()}); + } + + /// @brief Registers a result-type subscription for @p binding. + /// + /// The subscription is stored against the *binding*, not against a fixed + /// instance id, and is matched at publish time by comparing the binding's + /// current instance. Re-pointing a handler therefore moves its subscriptions + /// with it, which is what makes "tell me about the account I am looking at" + /// keep working when the user switches accounts. + /// + /// @param binding Handler binding that owns the subscription. + /// @param type Result type being subscribed to. + /// @param sink Type-erased delivery callback; receives the boxed result. + /// @param exec Executor the callback is delivered on. + void addSubscription(const std::shared_ptr& binding, std::type_index type, + std::function sink, ::morph::exec::IExecutor* exec) { + std::scoped_lock const lock{_subMtx}; + for (auto& entry : _subscriptions) { + auto owner = entry.binding.lock(); + if (owner && owner.get() == binding.get() && entry.type == type) { + entry.sink = std::move(sink); // one callback per (handler, result type) + entry.exec = exec; + return; + } + } + _subscriptions.push_back({.binding = binding, .type = type, .sink = std::move(sink), .exec = exec}); + _subscriptionCount.store(_subscriptions.size(), std::memory_order_relaxed); + } + + /// @brief Removes @p binding's subscription for @p type, if any. + /// @param binding Handler binding that owns the subscription. + /// @param type Result type to stop hearing about. + void removeSubscription(const std::shared_ptr& binding, std::type_index type) { + std::scoped_lock const lock{_subMtx}; + std::erase_if(_subscriptions, [&](const InstanceSubscription& entry) { + auto owner = entry.binding.lock(); + return !owner || (owner.get() == binding.get() && entry.type == type); + }); + _subscriptionCount.store(_subscriptions.size(), std::memory_order_relaxed); + } + + /// @brief Whether any subscription is currently registered on this bridge. + /// + /// A single relaxed atomic load, so the overwhelmingly common case — a + /// process with no subscribers at all — pays nothing per result. Without + /// this, every successful action would build a `std::type_index`, copy its + /// result into a `std::any`, take `_subMtx` and walk the (empty) + /// subscription list before its `Completion` could resolve: a throughput + /// regression for every existing caller, on the hot path, to serve a feature + /// they are not using. + /// @return `true` if at least one subscription exists. + [[nodiscard]] bool hasSubscribers() const noexcept { + return _subscriptionCount.load(std::memory_order_relaxed) != 0U; + } + + /// @brief Delivers @p value to every subscriber attached to instance @p mid. + /// + /// Called for every successful action result. Subscribers are matched on + /// *the instance the result was produced on*, so a handler hears about work + /// another handler — or, with a shared instance, another screen entirely — + /// did on the model it is attached to. + /// + /// The producing handler is notified too: suppressing the echo would force + /// every subscriber to special-case "was this mine", which is exactly the + /// bookkeeping the feature exists to remove. + /// + /// Sinks are snapshotted under the lock and invoked outside it, so a + /// subscriber that re-enters the bridge cannot deadlock. + /// + /// @param mid Instance the result was produced on. + /// @param type Result type produced. + /// @param value Boxed result. + void publishResult(::morph::exec::detail::ModelId mid, std::type_index type, const std::any& value) { + std::vector, ::morph::exec::IExecutor*>> targets; + { + std::scoped_lock const lock{_subMtx}; + // Prune while we are already holding the lock and walking the list: + // a handler that is destroyed without unsubscribing would otherwise + // leave its entry behind until some *other* handler happened to call + // add/removeSubscription, which in a long-lived app with many + // transient handlers is never. + std::erase_if(_subscriptions, [](const InstanceSubscription& entry) { return entry.binding.expired(); }); + _subscriptionCount.store(_subscriptions.size(), std::memory_order_relaxed); + for (const auto& entry : _subscriptions) { + auto owner = entry.binding.lock(); + if (owner && entry.type == type && owner->currentId.load() == mid.v && entry.sink) { + targets.emplace_back(entry.sink, entry.exec); + } + } + } + for (auto& [sink, exec] : targets) { + if (exec != nullptr) { + exec->post([sink, value] { sink(value); }); + } else { + sink(value); + } + } + } + /// @brief Installs a default session context that `executeVia` stamps onto the /// `ActionCall` of every subsequent call. /// @@ -291,8 +516,19 @@ class Bridge { if (!binding) { continue; } - auto newId = newShared->registerModelWithContext(binding->typeId, binding->modelFactory, - binding->contextKey); + // A shared binding that never attached has no instance to + // re-create: it stays live and unbound, and acquires one on + // the new backend the first time it is attached. + if (binding->shared && binding->primary.empty()) { + live.push_back(weak); + continue; + } + auto newId = binding->shared + ? newShared->registerModelShared( + binding->typeId, binding->modelFactory, + {.contextKey = binding->contextKey, .primary = binding->primary}) + : newShared->registerModelWithContext(binding->typeId, binding->modelFactory, + binding->contextKey); staged.emplace_back(binding, newId.v); live.push_back(weak); } @@ -376,12 +612,18 @@ class Bridge { /// @param binding Binding returned by `registerHandler()`. /// @param action Action to execute (moved in). /// @param cbExec Executor on which the `Completion` callbacks are posted. + /// @param onResult Optional observer run on the typed result *before* it is + /// moved into the returned `Completion` and before the + /// caller's own `.then`. Used to adopt a result-sourced + /// primary key so the binding is already promoted by the time + /// user code sees the result; empty for every other call. /// @return Completion that resolves with the typed result or an exception /// (including `ValidationError` on `LocalBackend` when the action /// fails its validator). template ::morph::async::Completion::Result> executeVia( - const std::shared_ptr& binding, Action action, ::morph::exec::IExecutor* cbExec) { + const std::shared_ptr& binding, Action action, ::morph::exec::IExecutor* cbExec, + std::function::Result&)> onResult = {}) { using R = ::morph::model::ActionTraits::Result; auto backend = loadBackend(); @@ -404,9 +646,8 @@ class Bridge { call.localOp = [sharedAction](::morph::model::detail::IModelHolder& holder) -> std::shared_ptr { // Enforce the action's validator on the local execution path too, so // a caller that constructs an Action by hand and calls - // BridgeHandler::execute() directly — bypassing the - // reactive set<>/tryFireImpl gate that already checks ready() — is - // rejected the same way a hand-built wire envelope is rejected by + // BridgeHandler::execute() directly is rejected the + // same way a hand-built wire envelope is rejected by // ActionDispatcher::registerAction's runner (registry.hpp). No JSON is // involved on this path, so there is no declared-precision // reconciliation step here (that only applies to decoded wire @@ -459,7 +700,8 @@ class Bridge { } auto anyCompletion = backend->execute(::morph::exec::detail::ModelId{raw}, std::move(call), cbExec); anyCompletion - .then([typedState](const std::shared_ptr& vAny) { + .then([typedState, onResult = std::move(onResult), this, raw, + alive = liveness()](const std::shared_ptr& vAny) { // Guard the value-forwarding: if R's move/copy throws (or the cast // is somehow wrong), route the exception to the typed completion's // error sink instead of letting it escape the callback executor — @@ -468,7 +710,24 @@ class Bridge { // and std::terminate. Mirrors the forwarding guard in remote.hpp's // SimulatedRemoteBackend::execute. See docs/spec/bridge.md. try { - typedState->setValue(std::move(*static_cast(vAny.get()))); + auto* const typedResult = static_cast(vAny.get()); + // Runs before the value is moved out and before the caller's + // own .then, so a result-sourced primary key is adopted by + // the binding before any user code observes the result. + if (onResult) { + onResult(*typedResult); + } + // Fan the result out to everything attached to this + // instance before the value is moved away. Guarded on the + // bridge's liveness token: a completion can in principle + // resolve after the Bridge is gone. + if constexpr (std::is_copy_constructible_v) { + if (hasSubscribers() && !alive.expired()) { + publishResult(::morph::exec::detail::ModelId{raw}, std::type_index{typeid(R)}, + std::any{*typedResult}); + } + } + typedState->setValue(std::move(*typedResult)); } catch (...) { typedState->setException(std::current_exception()); } @@ -478,7 +737,7 @@ class Bridge { } private: - template + template friend class BridgeHandler; /// @brief Weak observer of this bridge's lifetime, handed to each handler. @@ -544,30 +803,68 @@ class Bridge { std::vector> _handlers; mutable std::mutex _sessionMtx; ::morph::session::Context _defaultSession; + // Instance subscriptions. Held against the binding rather than a fixed + // instance id so a re-pointed handler keeps its subscriptions; matched at + // publish time by comparing the binding's current instance. + struct InstanceSubscription { + std::weak_ptr binding; + std::type_index type; + std::function sink; + ::morph::exec::IExecutor* exec = nullptr; + }; + std::mutex _subMtx; + std::vector _subscriptions; + // Mirrors _subscriptions.size() for the lock-free hasSubscribers() probe. + // Maintained under _subMtx; read relaxed off it. A stale-by-one read is + // harmless: publishResult re-checks under the lock and finds nothing. + std::atomic _subscriptionCount{0}; // Destroyed with the Bridge; handlers hold weak_ptrs to it (see liveness()). std::shared_ptr _liveness{std::make_shared()}; }; +/// @brief `BridgeHandler` sharing policy: private, one instance per handler. +/// +/// The default, and what every pre-existing call site gets. Such a handler +/// registers its own instance at construction and never enters the shared +/// directory, so two `BridgeHandler` objects are two independent models — +/// byte-for-byte the behaviour morph has always had. +struct NoSharing {}; + +/// @brief `BridgeHandler` sharing policy: joins the shared instance directory. +/// +/// A shared handler registers **nothing** at construction. It acquires an +/// instance the first time a keyed action or an explicit `attach()` names a +/// primary key, and every other `AllowShared` handler naming that same key — +/// in this process or, for a remote backend, in any other client — reaches the +/// same instance. Releasing the last such handler destroys it. +/// +/// A shared handler that only ever runs *keyless* actions never attaches, and +/// its `execute` fails fast with "handler not bound": there is no instance to +/// run against and inventing a private one would silently defeat the sharing +/// the caller asked for. Attach first — see docs/planned/shared_model_instances.md. +struct AllowShared {}; + /// @brief RAII wrapper that binds a single model type to a `Bridge`. /// /// On construction, registers a `HandlerBinding` on the bridge. On destruction, /// deregisters it automatically. The handler is non-copyable. /// -/// @par Fielded actions and subscriptions -/// Beyond the one-shot `execute(action) -> Completion` API, the handler -/// offers a streaming surface for actions whose values arrive field-by-field -/// from a GUI (typically one widget per field): +/// @par Instance subscriptions +/// Beyond the one-shot `execute(action) -> Completion` API, a handler can +/// observe *the instance it is attached to*: /// -/// - `subscribe(cb)` stashes a result callback for action type `A`. -/// - `set<&A::field>(value)` updates one field of the in-progress draft of `A`. -/// - `unsubscribe()` drops the callback. -/// - `reset()` discards the in-progress draft of `A`. +/// - `subscribe(cb)` fires whenever an `R` is produced on that instance, by +/// any handler attached to it. +/// - `unsubscribe()` drops the callback. /// /// @tparam Model Concrete model type. -template +template // NOLINTNEXTLINE(cppcoreguidelines-special-member-functions) class BridgeHandler { public: + /// @brief Whether this handler participates in the shared instance directory. + static constexpr bool kShared = std::is_same_v; + /// @brief Constructs and registers the handler using the default model factory. /// /// @param bridge The bridge to register on. @@ -576,11 +873,9 @@ class BridgeHandler { : _bridge{bridge}, _bridgeAlive{bridge.liveness()}, _guiExec{guiExec}, - _binding{bridge.template registerHandler()}, - _subs{std::make_shared()} { - _subs->bridge = &_bridge; - _subs->binding = _binding; - _subs->guiExec = _guiExec; + _binding{makeBinding(bridge)} { + static_assert(!kShared || ::morph::model::KeyedModel, + "BridgeHandler requires Model to declare a PrimaryKey alias"); } /// @brief Constructs the handler with a pre-built binding (for dependency injection). @@ -592,12 +887,8 @@ class BridgeHandler { : _bridge{bridge}, _bridgeAlive{bridge.liveness()}, _guiExec{guiExec}, - _binding{std::move(binding)}, - _subs{std::make_shared()} { + _binding{std::move(binding)} { _bridge.registerHandler(_binding); - _subs->bridge = &_bridge; - _subs->binding = _binding; - _subs->guiExec = _guiExec; } /// @brief Deregisters the binding from the bridge. @@ -627,7 +918,101 @@ class BridgeHandler { /// @return Completion that resolves on the GUI executor. template ::morph::async::Completion::Result> execute(Action action) { - return _bridge.template executeVia(_binding, std::move(action), _guiExec); + if constexpr (kShared && ::morph::model::detail::PayloadKeyed) { + // The action names its instance: attach (or re-point) before + // dispatching, so the call lands on the instance it asked for. + _bridge.template attachHandler(_binding, ::morph::model::ActionKeyTraits::key(action)); + } + if constexpr (kShared && ::morph::model::detail::ResultKeyed) { + // The action *creates* the instance and its result carries the + // generated key, exactly as a database insert returns its primary + // key. Adopt it before any user callback observes the result, so a + // .then() can immediately run further actions on the new instance. + using R = ::morph::model::ActionTraits::Result; + // The action generates its own key, so it must run before the key + // exists. Give the handler an anonymous instance to run on, then + // promote *that* instance once the reply names it — re-pointing to a + // fresh one instead would strand whatever the create just did. + _bridge.ensureBound(_binding); + auto* const bridgePtr = &_bridge; + auto binding = _binding; + return _bridge.template executeVia( + _binding, std::move(action), _guiExec, [bridgePtr, binding](const R& result) { + bridgePtr->template assignHandlerPrimary( + binding, ::morph::model::ActionKeyTraits::template keyOfResult(result)); + }); + } else { + return _bridge.template executeVia(_binding, std::move(action), _guiExec); + } + } + + /// @brief Attaches (or re-points) this handler to the instance for @p key. + /// + /// Creates the instance if no live instance holds @p key, otherwise joins the + /// existing one. Re-pointing an already-attached handler releases the old + /// instance, which survives only if another handler still holds it. + /// + /// The primary is deliberately **not** write-once: naming a different key is + /// how a screen switches which entity it is looking at. Instances never + /// change their own identity — the *handler* moves — so a key always maps to + /// exactly one instance and no collision case can arise. + /// + /// @tparam M Defaulted to `Model`; never named explicitly. Present only so the + /// signature is instantiated lazily, since `PrimaryKeyOf` is + /// ill-formed for an unkeyed model. + /// @param key Primary key of the instance to attach to. + template + void attach(const ::morph::model::PrimaryKeyOf& key) + requires kShared + { + _bridge.template attachHandler(_binding, ::morph::model::keyToString(key)); + } + + /// @brief This handler's current primary key, or `nullopt` if unattached. + /// @tparam M Defaulted to `Model`; never named explicitly. See `attach`. + /// @return The attached key, or `nullopt` before the first attach. + template + [[nodiscard]] std::optional<::morph::model::PrimaryKeyOf> primary() + requires kShared + { + auto raw = _bridge.bindingPrimary(_binding); + if (raw.empty()) { + return std::nullopt; + } + return ::morph::model::keyFromString<::morph::model::PrimaryKeyOf>(raw); + } + + /// @brief Snapshot of the live shared instance keys for `Model`. + /// + /// Asynchronous even in local mode: the directory is backend state, and in + /// remote mode answering costs a round trip. Returning a bare `std::vector` + /// would work in-process and force a different call site everywhere else, + /// breaking the local/remote symmetry the framework is built on. + /// + /// The result is a snapshot, stale the moment it arrives — another client may + /// attach or release before the callback runs. Treat a returned key as "was + /// live recently", never as a guarantee that a later `attach` finds the same + /// instance. + /// + /// @tparam M Defaulted to `Model`; never named explicitly. See `attach`. + /// @return Completion resolving on the GUI executor with the live keys. + template + [[nodiscard]] ::morph::async::Completion>> instances() + requires kShared + { + using Key = ::morph::model::PrimaryKeyOf; + auto state = std::make_shared<::morph::async::detail::CompletionState>>(); + ::morph::async::Completion> comp{state, _guiExec}; + try { + std::vector keys; + for (const auto& raw : _bridge.template listInstancesOf()) { + keys.push_back(::morph::model::keyFromString(raw)); + } + state->setValue(std::move(keys)); + } catch (...) { + state->setException(std::current_exception()); + } + return comp; } /// @brief Type-erased execute: looks up the action by its registered @@ -648,73 +1033,49 @@ class BridgeHandler { actionType, this, bodyJson); } + /// @brief Creates this handler's binding: deferred when shared, immediate otherwise. + /// @param bridge Bridge to create the binding on. + /// @return The new binding. + static std::shared_ptr makeBinding(Bridge& bridge) { + if constexpr (kShared) { + return bridge.template registerSharedHandler(); + } else { + return bridge.template registerHandler(); + } + } + /// @brief The executor used to deliver this handler's `Completion` callbacks. /// @return The GUI/callback executor passed at construction. [[nodiscard]] ::morph::exec::IExecutor* guiExecutor() const noexcept { return _guiExec; } - /// @brief Subscribes to results of action type @p Action. + /// @brief Subscribes to results of type @p R produced on the attached instance. /// - /// @tparam Action Concrete action type registered with `BRIDGE_REGISTER_ACTION`. - /// @param cb Callable receiving the action's `Result` by value on the GUI executor. - template - void subscribe(std::function::Result)> cb) { - using R = ::morph::model::ActionTraits::Result; - auto wrapper = [cb = std::move(cb)](const std::any& boxed) { cb(std::any_cast(boxed)); }; - std::scoped_lock lock{_subs->mtx}; - _subs->entries[::morph::model::ActionTraits::typeId()].sink = std::move(wrapper); - } - - /// @brief Subscribes to both results and errors of action type @p Action. + /// Fires whenever an `R` is produced on the instance this handler is + /// attached to — by this handler, by another handler sharing the instance, + /// or by another screen entirely. The subscriber names *what it renders*, + /// not what somebody else must call to produce it, so adding an action that + /// also yields an `R` never breaks an existing subscriber. /// - /// @tparam Action Concrete action type registered with `BRIDGE_REGISTER_ACTION`. - /// @param cb Result callback invoked on success. - /// @param errCb Error callback invoked on failure (replaces orphan logging). - template - void subscribe(std::function::Result)> cb, - std::function errCb) { - subscribe(std::move(cb)); - std::scoped_lock lock{_subs->mtx}; - _subs->entries[::morph::model::ActionTraits::typeId()].errSink = std::move(errCb); + /// One callback per `(handler, R)`: subscribing again replaces the previous + /// one. Callbacks are delivered on this handler\'s executor. Failed actions + /// notify nobody; delivery is best-effort and unbuffered, with no replay. + /// + /// @tparam R Result/state type to observe. + /// @param cb Callable receiving the value by value on the GUI executor. + template + void subscribe(std::function cb) { + _bridge.addSubscription( + _binding, std::type_index{typeid(R)}, + [cb = std::move(cb)](const std::any& boxed) { cb(std::any_cast(boxed)); }, _guiExec); } - /// @brief Removes the subscriber for action type @p Action. - template + /// @brief Removes this handler\'s subscription for @p R. + /// @tparam R Result/state type to stop hearing about. + template void unsubscribe() { - std::scoped_lock lock{_subs->mtx}; - auto iter = _subs->entries.find(::morph::model::ActionTraits::typeId()); - if (iter != _subs->entries.end()) { - iter->second.sink = nullptr; - iter->second.errSink = nullptr; - } - } - - /// @brief Sets one field of the in-progress draft and fires the action if ready. - /// - /// @tparam FieldPtr Pointer-to-data-member of the action struct (encodes both action and field type). - /// @param value New value for the field. - template - void set(detail::MemberPointerTraits::ValueType value) { - using A = detail::MemberPointerTraits::ClassType; - { - std::scoped_lock lock{_subs->mtx}; - auto& entry = _subs->entries[::morph::model::ActionTraits::typeId()]; - if (!entry.draft.has_value()) { - entry.draft = A{}; - } - std::any_cast(entry.draft).*FieldPtr = std::move(value); - } - tryFireImpl(_subs, ::morph::model::ActionTraits::typeId()); + _bridge.removeSubscription(_binding, std::type_index{typeid(R)}); } - /// @brief Discards the in-progress draft for action @p Action. - template - void reset() { - std::scoped_lock lock{_subs->mtx}; - auto iter = _subs->entries.find(::morph::model::ActionTraits::typeId()); - if (iter != _subs->entries.end()) { - iter->second.draft.reset(); - } - } /// @brief Returns the underlying `HandlerBinding`. /// @@ -722,132 +1083,10 @@ class BridgeHandler { [[nodiscard]] const std::shared_ptr& binding() const { return _binding; } private: - struct SubscriberEntry { - std::any draft; - std::function sink; - std::function errSink; - bool running{false}; - bool pending{false}; - }; - struct SubscriberState { - std::mutex mtx; - Bridge* bridge{nullptr}; - std::shared_ptr binding; - ::morph::exec::IExecutor* guiExec{nullptr}; - /// @note Keys are `std::string_view` pointing to string literals from - /// `ActionTraits::typeId()` — all call sites pass compile-time strings - /// with static storage duration, so the map's keys never dangle. - std::unordered_map entries; - }; - - struct PostExec { - std::function sink; - std::function errSink; - bool refire{false}; - }; - - static PostExec consumeFlight(SubscriberState& state, std::string_view typeId) { - PostExec out; - std::scoped_lock lock{state.mtx}; - auto& entry = state.entries.find(typeId)->second; - out.sink = entry.sink; - out.errSink = entry.errSink; - entry.running = false; - if (entry.pending) { - entry.pending = false; - out.refire = true; - } - return out; - } - - static void logUnhandledError(std::string_view typeId, const std::exception_ptr& err) { - try { - std::rethrow_exception(err); - } catch (const std::exception& exc) { - ::morph::log::logError(std::string{"[subscription:"} + std::string{typeId} + - "] unhandled exception: " + exc.what()); - } catch (...) { - ::morph::log::logError(std::string{"[subscription:"} + std::string{typeId} + - "] unhandled unknown exception"); - } - } - - template - static void tryFireImpl(const std::shared_ptr& state, std::string_view typeId) { - // Every caller holds the `SubscriberState` alive for the duration of this - // call, so no liveness check is needed on entry. The async continuations - // below may outlive the subscription, so they each re-check through this - // weak_ptr instead. - const std::weak_ptr weak{state}; - using R = ::morph::model::ActionTraits::Result; - - Action snapshot; - { - std::scoped_lock lock{state->mtx}; - auto iter = state->entries.find(typeId); - if (iter == state->entries.end() || !iter->second.draft.has_value()) { - return; - } - if (iter->second.running) { - iter->second.pending = true; - return; - } - snapshot = std::any_cast(iter->second.draft); - // Recompute every declared computed field live, on the snapshot, - // before the readiness check and fire -- so a validator that - // inspects a computed field sees the freshly-derived value, and - // the fired action already carries it. No-op for actions with no - // computedFields. This is a live, non-authoritative recompute for - // display; the dispatch paths (bridge.hpp's ActionExecuteRegistry - // executor and localOp, registry.hpp's ActionDispatcher runner) - // recompute it again, authoritatively. See docs/spec/forms/forms.md. - ::morph::forms::recomputeAll(snapshot); - } - if (!::morph::model::ActionValidator::ready(snapshot)) { - return; - } - { - std::scoped_lock lock{state->mtx}; - state->entries[typeId].running = true; - } - - state->bridge->template executeVia(state->binding, std::move(snapshot), state->guiExec) - .then([weak, typeId](R result) { - auto inner = weak.lock(); - if (!inner) { - return; - } - auto outcome = consumeFlight(*inner, typeId); - if (outcome.sink) { - std::any boxed{std::move(result)}; - outcome.sink(boxed); - } - if (outcome.refire) { - tryFireImpl(inner, typeId); - } - }) - .onError([weak, typeId](const std::exception_ptr& err) { - auto inner = weak.lock(); - if (!inner) { - return; - } - auto outcome = consumeFlight(*inner, typeId); - if (outcome.errSink) { - outcome.errSink(err); - } else { - logUnhandledError(typeId, err); - } - if (outcome.refire) { - tryFireImpl(inner, typeId); - } - }); - } - Bridge& _bridge; std::weak_ptr _bridgeAlive; // expires when _bridge is destroyed ::morph::exec::IExecutor* _guiExec; std::shared_ptr _binding; - std::shared_ptr _subs; }; /// Out-of-line definition of ActionExecuteRegistry::registerAction. diff --git a/include/morph/core/model_key.hpp b/include/morph/core/model_key.hpp new file mode 100644 index 0000000..f51390e --- /dev/null +++ b/include/morph/core/model_key.hpp @@ -0,0 +1,169 @@ +// SPDX-License-Identifier: Apache-2.0 + +#pragma once +#include +#include +#include +#include +#include +#include +#include + +/// @file +/// Primary keys for model instances. +/// +/// A model type declares itself *keyed* by exposing a nested `PrimaryKey` type +/// alias; the alias is detected structurally (a `requires`-expression), never by +/// inheritance or a marker base — the same detection style +/// `morph::views::ViewTraits` uses for a view's `kind`/`query` members. A model +/// without the alias is unkeyed and behaves exactly as it always has. +/// +/// Actions say *which of their fields carries* that key via `BRIDGE_KEY_FROM`, +/// or — for an action that creates the entity rather than naming it — which +/// field of their *result* establishes it, via `BRIDGE_KEY_FROM_RESULT`. Actions +/// with neither declaration are keyless and run against whichever instance the +/// handler is already attached to; that is the common case. +/// +/// Keys travel the wire as strings (`wire::Envelope::primary`) regardless of +/// their C++ type, so the directory in `RemoteServer` needs exactly one map type +/// rather than one per key type. See docs/planned/shared_model_instances.md. + +namespace morph::model { + +/// @brief Key types a model may declare as its `PrimaryKey`. +/// +/// Restricted to integral types and `std::string` because a key must round-trip +/// losslessly through the wire's string encoding and be usable as a map key. +/// `bool` is excluded: it carries one bit of identity, which is never a +/// meaningful primary key and is far more likely to be a mistake. +template +concept ModelKey = + (std::integral && !std::same_as, bool>) || std::same_as, std::string>; + +/// @brief Satisfied by model types that declare a `PrimaryKey` alias. +/// +/// Declaring the alias is what opts a model into keyed, shareable instances. +template +concept KeyedModel = requires { typename M::PrimaryKey; } && ModelKey; + +/// @brief The declared key type of a keyed model. +/// @tparam M Keyed model type. +template +using PrimaryKeyOf = M::PrimaryKey; + +/// @brief Encodes a primary key as its canonical wire string. +/// +/// Integral keys are decimal; `std::string` keys pass through unchanged. The +/// encoding is total — every valid key has exactly one representation — so two +/// clients naming the same key always land on the same directory entry. +/// @tparam K Key type satisfying `ModelKey`. +/// @param key Key value to encode. +/// @return The canonical string form of @p key. +template +[[nodiscard]] std::string keyToString(const K& key) { + if constexpr (std::same_as, std::string>) { + return key; + } else { + return std::to_string(key); + } +} + +/// @brief Decodes a primary key from its canonical wire string. +/// +/// @tparam K Key type satisfying `ModelKey`. +/// @param text Canonical string form, as produced by `keyToString`. +/// @return The decoded key. +/// @throws std::runtime_error if @p text is not a valid encoding of a `K` +/// (non-numeric text, trailing garbage, or a value out of range). A key +/// that cannot be decoded is a protocol error, not a value to clamp: +/// silently yielding 0 would route the caller to the wrong instance. +template +[[nodiscard]] K keyFromString(std::string_view text) { + if constexpr (std::same_as, std::string>) { + return std::string{text}; + } else { + K value{}; + // from_chars is a [first, last) pointer API; a string_view's data()+size() + // is the only way to express its end, and is exactly what the standard + // intends here. + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) + auto [ptr, errc] = std::from_chars(text.data(), text.data() + text.size(), value); + const auto* const last = text.data() + text.size(); // NOLINT(cppcoreguidelines-pro-bounds-pointer-arithmetic) + if (errc != std::errc{} || ptr != last) { + throw std::runtime_error("invalid primary key encoding: '" + std::string{text} + "'"); + } + return value; + } +} + +/// @brief Declares where an action's model key comes from. +/// +/// The primary template is the *keyless* case, which is the default and the +/// common one: such an action says nothing about identity and runs against +/// whichever instance its handler is already attached to. Specialise via +/// `BRIDGE_KEY_FROM` or `BRIDGE_KEY_FROM_RESULT` rather than by hand. +/// @tparam Action Concrete action type. +template +struct ActionKeyTraits { + /// @brief Whether this action carries or establishes its model's key. + static constexpr bool hasKey = false; + + /// @brief Whether the key comes from the action's result rather than its payload. + static constexpr bool fromResult = false; +}; + +namespace detail { + +/// @brief Satisfied by actions whose key is carried in the action payload. +template +concept PayloadKeyed = ActionKeyTraits::hasKey && !ActionKeyTraits::fromResult; + +/// @brief Satisfied by actions whose key is established by the action's result. +template +concept ResultKeyed = ActionKeyTraits::hasKey && ActionKeyTraits::fromResult; + +} // namespace detail + +} // namespace morph::model + +// NOLINTBEGIN(cppcoreguidelines-macro-usage) — declaration macros are the intended public API, +// matching BRIDGE_REGISTER_MODEL/ACTION in registry.hpp: they must emit a template +// specialisation at global scope, which no function template can do. + +/// @brief Declares that action `A` carries its model's primary key in `MEMBER`. +/// +/// `MEMBER` is a pointer-to-data-member of `A` (e.g. `&GetAccount::id`) whose +/// type satisfies `morph::model::ModelKey`. Executing such an action on a +/// shareable handler attaches (or re-points) that handler to the instance +/// holding the named key, creating it if no instance holds it yet. +/// +/// Must appear at global scope, in exactly one translation unit, like the other +/// `BRIDGE_REGISTER_*` macros. +#define BRIDGE_KEY_FROM(A, MEMBER) \ + template <> \ + struct morph::model::ActionKeyTraits { \ + static constexpr bool hasKey = true; \ + static constexpr bool fromResult = false; \ + static std::string key(const A& action) { return morph::model::keyToString(action.*MEMBER); } \ + } + +/// @brief Declares that action `A`'s *result* establishes its model's primary key. +/// +/// For actions that create the entity rather than name it: the key is not in the +/// request, it is generated and returned, exactly as a database insert returns +/// its generated primary key. `MEMBER` is a pointer-to-data-member of `A`'s +/// result type (e.g. `&AccountInfo::id`). +/// +/// Must appear at global scope, in exactly one translation unit. +#define BRIDGE_KEY_FROM_RESULT(A, MEMBER) \ + template <> \ + struct morph::model::ActionKeyTraits { \ + static constexpr bool hasKey = true; \ + static constexpr bool fromResult = true; \ + template \ + static std::string keyOfResult(const R& result) { \ + return morph::model::keyToString(result.*MEMBER); \ + } \ + } + +// NOLINTEND(cppcoreguidelines-macro-usage) diff --git a/include/morph/core/remote.hpp b/include/morph/core/remote.hpp index db06ba6..2a37aa2 100644 --- a/include/morph/core/remote.hpp +++ b/include/morph/core/remote.hpp @@ -428,9 +428,14 @@ class RemoteServer : public std::enable_shared_from_this { if (scopeIter == _connectionScopes.end()) { return; } - for (const auto& mid : scopeIter->second) { - _models.erase(mid); - _owners.erase(mid); + for (const auto& [mid, refs] : scopeIter->second) { + // Release exactly as many references as this connection held. A + // shared instance another connection is still attached to survives; + // a private one (count 1, no directory entry) is erased outright, + // which is byte-for-byte the previous behaviour. + for (std::size_t idx = 0; idx < refs; ++idx) { + releaseInstanceLocked(mid); + } _modelConnection.erase(mid); } _connectionScopes.erase(scopeIter); @@ -582,6 +587,205 @@ class RemoteServer : public std::enable_shared_from_this { } private: + /// @brief Releases one reference to @p mid, destroying it at zero. Caller holds `_regMtx`. + /// + /// A private instance has no `_attachCount` entry and is erased outright — + /// byte-for-byte the pre-sharing behaviour. A shared instance is erased, and + /// removed from the directory, only when its last attachment goes away, so + /// one client's `deregister` or dropped connection never tears an instance + /// out from under another client still using it. + /// @param mid Instance to release. + /// @return `true` if this call destroyed the instance. + bool releaseInstanceLocked(::morph::exec::detail::ModelId mid) { + if (auto refIter = _attachCount.find(mid); refIter != _attachCount.end()) { + refIter->second -= 1; + if (refIter->second > 0) { + return false; + } + _attachCount.erase(refIter); + if (auto keyIter = _sharedKeyOf.find(mid); keyIter != _sharedKeyOf.end()) { + _directory.erase(keyIter->second); + _sharedKeyOf.erase(keyIter); + } + } + _models.erase(mid); + _owners.erase(mid); + return true; + } + + /// @brief Drops one of @p cid's references to @p mid, then releases the instance. + /// Caller holds `_regMtx`. + /// @param mid Instance to release. + /// @param cid Connection whose reference is being dropped; `0` for unscoped. + void releaseScopedLocked(::morph::exec::detail::ModelId mid, ConnectionId cid) { + if (cid != 0) { + if (auto scopeIter = _connectionScopes.find(cid); scopeIter != _connectionScopes.end()) { + if (auto refIter = scopeIter->second.find(mid); refIter != scopeIter->second.end()) { + refIter->second -= 1; + if (refIter->second == 0) { + scopeIter->second.erase(refIter); + } + } + } + } + if (releaseInstanceLocked(mid)) { + _modelConnection.erase(mid); + } + } + + /// @brief Records a new attachment of @p mid to @p cid. Caller holds `_regMtx`. + /// @param mid Instance being attached. + /// @param cid Connection attaching it; `0` for unscoped (records nothing). + /// @return `false` if @p cid's scope was already closed, in which case nothing was recorded. + bool noteScopeAttachLocked(::morph::exec::detail::ModelId mid, ConnectionId cid) { + if (cid == 0) { + return true; + } + auto scopeIter = _connectionScopes.find(cid); + if (scopeIter == _connectionScopes.end()) { + return false; + } + scopeIter->second[mid] += 1; + _modelConnection[mid] = cid; + return true; + } + + /// @brief Attaches a configured `LogProvider`'s log to a freshly created holder. + /// @param holder Newly created instance. + /// @param env Envelope carrying `typeId` and `contextKey`. + void attachLogIfConfigured(::morph::model::detail::IModelHolder& holder, const ::morph::wire::Envelope& env) { + if (env.contextKey.empty()) { + return; + } + LogProvider provider; + { + std::scoped_lock const lock{_logProviderMtx}; + provider = _logProvider; + } + if (provider) { + if (auto log = provider(env.typeId, env.contextKey)) { + holder.attachActionLog(std::move(log), env.contextKey); + } + } + } + + /// @brief Acquires (or creates) the shared instance for `(typeId, primary)` and replies. + /// + /// The register-or-attach core shared by the `register` branch (when + /// `shared` is set) and by `attach`. A shared instance is recorded with an + /// **empty owner principal**: `IAuthorizer::authorizeInstance`'s documented + /// `ownerPrincipal == ctx.principal` policy would otherwise reject every + /// client but the one that created it, defeating cross-client sharing + /// outright. Gating access to a shared model is therefore `authorize`'s job + /// (per type and action) or the model's own — see docs/spec/security.md. + /// + /// @param env Decoded request; uses `typeId`, `primary`, `contextKey`, `callId`. + /// @param reply Reply sink; always invoked exactly once. + /// @param cid Connection scope, or `0` for unscoped. + /// @param releaseCurrent Instance to release first (an `attach` re-point), or `ModelId{0}`. + void acquireSharedInstance(const ::morph::wire::Envelope& env, const std::function& reply, + ConnectionId cid, ::morph::exec::detail::ModelId releaseCurrent) { + LimitPolicy limits; + { + std::scoped_lock const lock{_limitsMtx}; + limits = _limits; + } + DirectoryKey dirKey{env.typeId, env.primary}; + { + std::scoped_lock const lock{_regMtx}; + if (releaseCurrent.v != 0U) { + releaseScopedLocked(releaseCurrent, cid); + } + if (auto found = _directory.find(dirKey); found != _directory.end()) { + auto const mid = found->second; + _attachCount[mid] += 1; + if (!noteScopeAttachLocked(mid, cid)) { + releaseInstanceLocked(mid); + reply(::morph::wire::encode(::morph::wire::makeErr("connection closed", env.callId))); + return; + } + reply(::morph::wire::encode(::morph::wire::makeOk(env.callId, {}, mid.v))); + return; + } + } + // Directory miss. Construct outside the lock, exactly as the private + // register path does, then re-check under the insert lock: a concurrent + // request for the same key may have won the race while we built ours. + auto holder = _registry.create(env.typeId); + attachLogIfConfigured(*holder, env); + ::morph::exec::detail::ModelId const fresh{nextOpaqueId()}; + { + std::scoped_lock const lock{_regMtx}; + if (auto found = _directory.find(dirKey); found != _directory.end()) { + auto const mid = found->second; + _attachCount[mid] += 1; + if (!noteScopeAttachLocked(mid, cid)) { + releaseInstanceLocked(mid); + reply(::morph::wire::encode(::morph::wire::makeErr("connection closed", env.callId))); + return; + } + reply(::morph::wire::encode(::morph::wire::makeOk(env.callId, {}, mid.v))); + return; + } + if (limits.maxLiveModels != 0 && _models.size() >= limits.maxLiveModels) { + reply(::morph::wire::encode(::morph::wire::makeErr("too many models", env.callId))); + return; + } + if (!noteScopeAttachLocked(fresh, cid)) { + reply(::morph::wire::encode(::morph::wire::makeErr("connection closed", env.callId))); + return; + } + _models[fresh] = std::move(holder); + _owners[fresh] = std::string{}; // shared instances are ownerless, by design + _directory.emplace(dirKey, fresh); + _sharedKeyOf.emplace(fresh, std::move(dirKey)); + _attachCount[fresh] = 1; + } + reply(::morph::wire::encode(::morph::wire::makeOk(env.callId, {}, fresh.v))); + } + + /// @brief Files a live instance under a primary key, in place. + /// + /// The existing holder of a key always wins: promoting onto a key another + /// instance already holds is a silent no-op rather than a displacement, so a + /// promotion can never steal an entry handlers are already attached to. + /// @param env Decoded request; uses `typeId`, `primary`, `modelId`. + void applyAssignLocked(const ::morph::wire::Envelope& env) { + ::morph::exec::detail::ModelId const mid{env.modelId}; + if (env.primary.empty() || !_models.contains(mid)) { + return; + } + DirectoryKey dirKey{env.typeId, env.primary}; + if (_directory.contains(dirKey)) { + return; + } + if (auto prevIter = _sharedKeyOf.find(mid); prevIter != _sharedKeyOf.end()) { + _directory.erase(prevIter->second); + _sharedKeyOf.erase(prevIter); + } + _directory.emplace(dirKey, mid); + _sharedKeyOf.emplace(mid, std::move(dirKey)); + _attachCount.try_emplace(mid, 1); + } + + /// @brief Answers an `instances` request with the live shared keys of a type. + /// @param env Decoded request; uses `typeId` and `callId`. + /// @param reply Reply sink; always invoked exactly once. + void handleInstances(const ::morph::wire::Envelope& env, const std::function& reply) { + std::vector keys; + { + std::scoped_lock const lock{_regMtx}; + for (const auto& [dirKey, mid] : _directory) { + if (dirKey.first == env.typeId) { + keys.push_back(dirKey.second); + } + } + } + std::string body; + (void)glz::write_json(keys, body); + reply(::morph::wire::encode(::morph::wire::makeOk(env.callId, std::move(body)))); + } + // One flat switch over the wire's `kind` discriminator. Splitting it would // scatter the authorization sequence each branch depends on across helpers, // with no reader benefit. @@ -598,7 +802,8 @@ class RemoteServer : public std::enable_shared_from_this { // the existing register/execute validation runs — while `deregister` // (and any other kind) still flows through unchanged, so a client can // still tear its models down cleanly during the drain window. - if ((env.kind == "register" || env.kind == "execute") && _shuttingDown.load(std::memory_order_acquire)) { + if ((env.kind == "register" || env.kind == "execute" || env.kind == "attach") && + _shuttingDown.load(std::memory_order_acquire)) { reply(::morph::wire::encode(::morph::wire::makeErr("server shutting down", env.callId))); return; } @@ -645,19 +850,15 @@ class RemoteServer : public std::enable_shared_from_this { reply(::morph::wire::encode(::morph::wire::makeErr("unauthorized", env.callId))); return; } - auto holder = _registry.create(env.typeId); - if (!env.contextKey.empty()) { - LogProvider provider; - { - std::scoped_lock const lock{_logProviderMtx}; - provider = _logProvider; - } - if (provider) { - if (auto log = provider(env.typeId, env.contextKey)) { - holder->attachActionLog(std::move(log), env.contextKey); - } - } + // A `shared` register naming a primary is a register-or-attach + // against the directory; everything below is the private path, + // unchanged. + if (env.shared && !env.primary.empty()) { + acquireSharedInstance(env, reply, cid, ::morph::exec::detail::ModelId{0}); + return; } + auto holder = _registry.create(env.typeId); + attachLogIfConfigured(*holder, env); // Record the owner principal for per-instance authorization: // env.session's principal is already the verified identity // stamped above (empty if the authorizer does not @@ -702,7 +903,7 @@ class RemoteServer : public std::enable_shared_from_this { if (scopeIter == _connectionScopes.end()) { scopeAlreadyClosed = true; } else { - scopeIter->second.insert(mid); + scopeIter->second[mid] += 1; _modelConnection[mid] = cid; } } @@ -725,6 +926,47 @@ class RemoteServer : public std::enable_shared_from_this { return; } reply(::morph::wire::encode(::morph::wire::makeOk(env.callId, {}, mid.v))); + } else if (env.kind == "attach") { + if (env.typeId.empty()) { + throw std::runtime_error("attach requires a typeId"); + } + if (auto verified = _authorizer->authenticate(env.session)) { + env.session.principal = std::move(*verified); + } else { + env.session.principal.clear(); + } + if (!_authorizer->authorizeRegister(env.session, env.typeId)) { + reply(::morph::wire::encode(::morph::wire::makeErr("unauthorized", env.callId))); + return; + } + acquireSharedInstance(env, reply, cid, ::morph::exec::detail::ModelId{env.modelId}); + } else if (env.kind == "assign") { + if (env.typeId.empty()) { + throw std::runtime_error("assign requires a typeId"); + } + { + std::scoped_lock const lock{_regMtx}; + applyAssignLocked(env); + } + reply(::morph::wire::encode(::morph::wire::makeOk(env.callId, {}, env.modelId))); + } else if (env.kind == "instances") { + if (env.typeId.empty()) { + throw std::runtime_error("instances requires a typeId"); + } + if (auto verified = _authorizer->authenticate(env.session)) { + env.session.principal = std::move(*verified); + } else { + env.session.principal.clear(); + } + // Enumeration is a read channel over the directory: gate it with + // `authorize` for the model type (empty action id) so a deployer + // can refuse listing without refusing use. It discloses the live + // key set to anyone admitted — see docs/spec/security.md. + if (!_authorizer->authorize(env.session, env.typeId, {})) { + reply(::morph::wire::encode(::morph::wire::makeErr("unauthorized", env.callId))); + return; + } + handleInstances(env, reply); } else if (env.kind == "deregister") { ::morph::observe::detail::emitMetric(::morph::observe::Metric::deregisterCount, 1.0); ::morph::exec::detail::ModelId const mid{env.modelId}; @@ -749,17 +991,25 @@ class RemoteServer : public std::enable_shared_from_this { } { std::scoped_lock const lock{_regMtx}; - _models.erase(mid); - _owners.erase(mid); - // Keep the connection scope's membership set in sync: an - // explicit wire deregister removes the id from its scope - // too, so a later closeConnection never double-erases it. + releaseInstanceLocked(mid); + // Keep the connection scope's membership in sync: an + // explicit wire deregister drops one of this connection's + // references, so a later closeConnection never + // double-releases it. if (auto connIter = _modelConnection.find(mid); connIter != _modelConnection.end()) { if (auto scopeIter = _connectionScopes.find(connIter->second); scopeIter != _connectionScopes.end()) { - scopeIter->second.erase(mid); + auto refIter = scopeIter->second.find(mid); + if (refIter != scopeIter->second.end()) { + refIter->second -= 1; + if (refIter->second == 0) { + scopeIter->second.erase(refIter); + } + } + } + if (!_models.contains(mid)) { + _modelConnection.erase(connIter); } - _modelConnection.erase(connIter); } } reply(::morph::wire::encode(::morph::wire::makeOk(env.callId))); @@ -1024,13 +1274,26 @@ class RemoteServer : public std::enable_shared_from_this { // and the scoped handle(msg, reply, cid) overload). Guarded by _regMtx — // the same lock as _models/_owners — so scope membership can never desync // from instance existence. - std::unordered_map> + // Value is a *count* per instance, not a set: one connection may attach the + // same shared instance from two handlers, and closing the connection must + // release both references or the instance leaks. A private instance always + // has a count of exactly 1. + std::unordered_map> _connectionScopes; // Owning connection recorded per scoped instance; absent means unscoped // (registered via the two-argument handle()/handleInline()). std::unordered_map<::morph::exec::detail::ModelId, ConnectionId, ::morph::exec::detail::ModelIdHash> _modelConnection; + // Shared-instance directory: (typeId, primary) -> ModelId, its reverse, and + // the cross-connection attach count. Guarded by _regMtx alongside + // _models/_owners so directory membership can never desync from instance + // existence. Only instances registered with `shared` set and a non-empty + // primary appear; a private instance has no entry in any of the three. + using DirectoryKey = std::pair; + std::unordered_map _directory; + std::unordered_map<::morph::exec::detail::ModelId, DirectoryKey, ::morph::exec::detail::ModelIdHash> _sharedKeyOf; + std::unordered_map<::morph::exec::detail::ModelId, std::size_t, ::morph::exec::detail::ModelIdHash> _attachCount; std::atomic _nextId{0}; std::atomic _nextConnectionId{0}; std::atomic _minVersion{::morph::wire::kProtocolVersion}; @@ -1115,6 +1378,89 @@ class SimulatedRemoteBackend : public detail::IBackend { throw std::runtime_error("register failed: " + reply.message); } + /// @brief Registers or attaches to the server's shared instance for @p identity. + /// + /// Sends a `shared` register, so the server returns the live instance for + /// `(typeId, primary)` when one exists rather than creating a second. An + /// empty primary degrades to the private path. + /// @param typeId String type-id sent in the `register` message. + /// @param factory Ignored — the server constructs via its own registry. + /// @param identity Entity key for the action log plus the directory primary key. + /// @return `ModelId` of the shared (or newly created) instance. + /// @throws std::runtime_error if the server replies with an error. + ::morph::exec::detail::ModelId registerModelShared( + const std::string& typeId, std::function()> factory, + detail::InstanceIdentity identity) override { + if (identity.primary.empty()) { + return registerModelWithContext(typeId, std::move(factory), identity.contextKey); + } + auto reply = + ::morph::wire::decode(_server.handleInline(::morph::wire::encode(::morph::wire::makeRegisterShared( + typeId, std::string{identity.primary}, std::string{identity.contextKey})))); + if (reply.kind == "ok") { + return ::morph::exec::detail::ModelId{reply.modelId}; + } + throw std::runtime_error("register failed: " + reply.message); + } + + /// @brief Re-points from @p current to the server's shared instance for @p identity. + /// + /// One `attach` request rather than a deregister/register pair, so the + /// re-pointing client cannot lose its slot to `LimitPolicy::maxLiveModels` + /// between releasing the old instance and acquiring the new one. + /// @param typeId String type-id sent in the `attach` message. + /// @param factory Ignored — the server constructs via its own registry. + /// @param identity Entity key for the action log plus the directory primary key. + /// @param current Instance currently held, or `ModelId{0}` if none. + /// @return `ModelId` of the instance now attached to. + /// @throws std::runtime_error if the server replies with an error. + ::morph::exec::detail::ModelId attachModel( + const std::string& typeId, std::function()> factory, + detail::InstanceIdentity identity, ::morph::exec::detail::ModelId current) override { + if (identity.primary.empty()) { + if (current.v != 0U) { + deregisterModel(current); + } + return registerModelWithContext(typeId, std::move(factory), identity.contextKey); + } + auto reply = ::morph::wire::decode(_server.handleInline( + ::morph::wire::encode(::morph::wire::makeAttach(typeId, std::string{identity.primary}, current.v)))); + if (reply.kind == "ok") { + return ::morph::exec::detail::ModelId{reply.modelId}; + } + throw std::runtime_error("attach failed: " + reply.message); + } + + /// @brief Files a live server-side instance under @p primary. + /// @param mid Live instance to promote. + /// @param typeId Model type id. + /// @param primary Canonical string encoding of the key to file it under. + void assignPrimary(::morph::exec::detail::ModelId mid, const std::string& typeId, + std::string_view primary) override { + if (primary.empty() || mid.v == 0U) { + return; + } + (void)_server.handleInline( + ::morph::wire::encode(::morph::wire::makeAssign(typeId, std::string{primary}, mid.v))); + } + + /// @brief Asks the server for the live shared primary keys of @p typeId. + /// @param typeId String type-id to enumerate. + /// @return Canonical key strings of the live shared instances. + /// @throws std::runtime_error if the server replies with an error. + std::vector listInstances(const std::string& typeId) override { + auto reply = + ::morph::wire::decode(_server.handleInline(::morph::wire::encode(::morph::wire::makeInstances(typeId)))); + if (reply.kind != "ok") { + throw std::runtime_error("instances failed: " + reply.message); + } + std::vector keys; + if (auto errCode = glz::read_json(keys, reply.body)) { + throw std::runtime_error("instances decode failed: " + glz::format_error(errCode, reply.body)); + } + return keys; + } + /// @brief Deregisters the model on the server. Processed inline; safe from any thread. /// @param mid Id of the model to deregister. void deregisterModel(::morph::exec::detail::ModelId mid) override { diff --git a/include/morph/core/wire.hpp b/include/morph/core/wire.hpp index de8f872..43cb618 100644 --- a/include/morph/core/wire.hpp +++ b/include/morph/core/wire.hpp @@ -46,6 +46,14 @@ inline constexpr std::uint32_t kProtocolVersion = 1; /// - `"register"` — client requests model creation. Uses `typeId`, and /// optionally `contextKey` (the new instance's stable /// identity, e.g. an account id — see `RemoteServer::setLogProvider`). +/// With `shared` set, additionally uses `primary` and becomes +/// a register-or-attach against the shared directory. +/// - `"attach"` — client re-points at a different `primary` of `typeId`, +/// releasing `modelId` if non-zero. Replies `ok` with the +/// target instance's id in `modelId`. +/// - `"assign"` — client files live `modelId` under `primary` of `typeId`. +/// - `"instances"` — client asks for the live shared primary keys of `typeId`. +/// Replies `ok` with a JSON array of key strings in `body`. /// - `"deregister"` — client destroys an instance. Uses `modelId`. /// - `"execute"` — client dispatches an action. Uses `callId`, `modelId`, /// `modelType`, `actionType`, `body`, and optionally `session`. @@ -69,6 +77,29 @@ struct Envelope { /// Ignored on every kind other than `register`. std::string contextKey; + /// @brief Primary key of the instance being registered or attached to. + /// + /// Carried on `register` (when `shared` is set) and on `attach`, always as + /// the key's canonical string encoding (`morph::model::keyToString`) + /// whatever its C++ type, so the server's directory needs one map type + /// rather than one per key type. Empty means "no primary" — the instance is + /// anonymous and cannot be shared. Ignored on every other kind. + /// + /// Distinct from `contextKey`, which continues to mean only "entity key for + /// the action log". A keyed model will normally set both to the same value, + /// but conflating the fields would change behaviour for callers already + /// setting `contextKey` for journal purposes. + std::string primary; + + /// @brief Whether a `register` should join the shared instance directory. + /// + /// `true` makes the request a *register-or-attach*: the server returns the + /// existing instance for `(typeId, primary)` if one is live, otherwise + /// creates it and enters it in the directory. `false` (the default, and the + /// value every pre-existing client sends) is today's behaviour exactly — a + /// private instance, invisible to the directory. + bool shared = false; + /// @brief Existing model instance id for `deregister`, `execute`, `ok(register)`. uint64_t modelId = 0; @@ -124,6 +155,79 @@ inline Envelope makeRegister(std::string typeId, std::string contextKey = {}) { return env; } +/// @brief Builds a shared (register-or-attach) `register` envelope. +/// +/// The server returns the live instance for `(typeId, primary)` if one exists, +/// otherwise creates it and enters it in the shared directory. A shared instance +/// is recorded with no owner principal, so `IAuthorizer::authorizeInstance`'s +/// documented `ownerPrincipal == ctx.principal` policy does not lock the second +/// client out of an instance the first created — see +/// docs/planned/shared_model_instances.md. +/// +/// @param typeId Model type id to register or attach to. +/// @param primary Canonical string encoding of the instance's primary key. +/// @param contextKey Optional entity key for the action log (default: none). +inline Envelope makeRegisterShared(std::string typeId, std::string primary, std::string contextKey = {}) { + Envelope env; + env.kind = "register"; + env.typeId = std::move(typeId); + env.primary = std::move(primary); + env.contextKey = std::move(contextKey); + env.shared = true; + return env; +} + +/// @brief Builds an `attach` envelope — re-points a client at a different primary. +/// +/// Semantically a `deregister` + shared `register` pair, made a single request +/// so a re-pointing handler cannot lose its slot to `LimitPolicy::maxLiveModels` +/// between releasing the old instance and acquiring the new one. +/// +/// @param typeId Model type id. +/// @param primary Canonical string encoding of the primary key to attach to. +/// @param modelId Instance the client is currently attached to; `0` if none. +inline Envelope makeAttach(std::string typeId, std::string primary, uint64_t modelId = 0) { + Envelope env; + env.kind = "attach"; + env.typeId = std::move(typeId); + env.primary = std::move(primary); + env.modelId = modelId; + env.shared = true; + return env; +} + +/// @brief Builds an `assign` envelope — files a live instance under a primary key. +/// +/// The promotion half of keyed instances: an action that creates its own entity +/// runs on a not-yet-keyed instance, and only the reply carries the generated +/// key. Assigning promotes that same instance in place, so nothing the create +/// did is stranded on a throwaway. +/// @param typeId Model type id. +/// @param primary Canonical string encoding of the key to file the instance under. +/// @param modelId Live instance to promote. +inline Envelope makeAssign(std::string typeId, std::string primary, uint64_t modelId) { + Envelope env; + env.kind = "assign"; + env.typeId = std::move(typeId); + env.primary = std::move(primary); + env.modelId = modelId; + env.shared = true; + return env; +} + +/// @brief Builds an `instances` envelope — asks for the live shared keys of a type. +/// +/// The reply's `body` is a JSON array of canonical key strings. Only instances +/// created through a shared `register`/`attach` are listed; a private instance +/// is invisible to the directory by construction. +/// @param typeId Model type id to enumerate. +inline Envelope makeInstances(std::string typeId) { + Envelope env; + env.kind = "instances"; + env.typeId = std::move(typeId); + return env; +} + /// @brief Builds a `deregister` envelope. inline Envelope makeDeregister(uint64_t modelId) { Envelope env; diff --git a/include/morph/forms/flows.hpp b/include/morph/forms/flows.hpp index 05e5471..4266066 100644 --- a/include/morph/forms/flows.hpp +++ b/include/morph/forms/flows.hpp @@ -208,7 +208,7 @@ class FlowSession { explicit FlowSession(::morph::bridge::BridgeHandler& handler, std::function onError = nullptr) : _handler{handler}, _onError{std::move(onError)} { - subscribeCurrent(); + beginStep(); } /// @brief Flags this session as gone, then unsubscribes the current step. @@ -222,7 +222,6 @@ class FlowSession { /// touching a partially- or fully-destroyed object. ~FlowSession() { _alive->store(false, std::memory_order_release); - unsubscribeCurrent(); } FlowSession(const FlowSession&) = delete; @@ -244,11 +243,22 @@ class FlowSession { if (::morph::model::ActionTraits::typeId() != currentActionType()) { throw std::logic_error{"FlowSession::set<>: field belongs to an action that is not the current step"}; } + A draft{}; + std::size_t stepIndex = 0; { std::scoped_lock const lock{_mtx}; - std::get(_drafts).*FieldPtr = value; + std::get(_drafts).*FieldPtr = std::move(value); + draft = std::get(_drafts); + stepIndex = _activeStep; + } + // The readiness gate that used to live in the handler's draft machinery + // lives here now: the flow already owns the draft, so it can decide when + // the step is complete and dispatch it itself. Note there is no + // in-flight coalescing — each ready `set<>` dispatches, where the old + // handler-side draft collapsed patches landing during a flight. + if (::morph::model::ActionValidator::ready(draft)) { + fireStep(std::move(draft), stepIndex); } - _handler.template set(std::move(value)); } /// @brief Moves to the next step, if the current step has already produced @@ -264,7 +274,6 @@ class FlowSession { if (!ready || finished()) { return false; } - unsubscribeCurrent(); ++_index; { std::scoped_lock const lock{_mtx}; @@ -276,7 +285,7 @@ class FlowSession { _activeStep = _index; } if (!finished()) { - subscribeCurrent(); + beginStep(); } return true; } @@ -289,14 +298,13 @@ class FlowSession { if (_index == 0) { return false; } - unsubscribeCurrent(); --_index; { std::scoped_lock const lock{_mtx}; _currentReady = true; // this step already produced a result once, or it could not have been left _activeStep = _index; } - subscribeCurrent(); + beginStep(); return true; } @@ -374,23 +382,25 @@ class FlowSession { _currentReady = true; } - /// @brief Installs the result/error sinks for step @p A. + /// @brief Dispatches step @p A's completed draft and routes its outcome. /// /// Both closures capture `_alive` (a copy of the `shared_ptr`, so it - /// outlives `this` if the two race) and check it before touching - /// anything on `this` — see `~FlowSession()`'s doc comment for why - /// `unsubscribe()` alone is not enough. + /// outlives `this` if the two race) and check it before touching anything on + /// `this` — a completion can still resolve after the flow is destroyed. + /// @tparam A Step action type. + /// @param draft The completed action to execute. + /// @param stepIndex Index of the step this dispatch belongs to. template - void installSubscription(std::size_t stepIndex) { + void fireStep(A draft, std::size_t stepIndex) { auto alive = _alive; - _handler.template subscribe( - [this, alive, stepIndex](::morph::model::ActionTraits::Result result) { + _handler.execute(std::move(draft)) + .then([this, alive, stepIndex](::morph::model::ActionTraits::Result result) { if (!alive->load(std::memory_order_acquire)) { return; } this->template captureResult(result, stepIndex); - }, - [this, alive, stepIndex](std::exception_ptr err) { + }) + .onError([this, alive, stepIndex](const std::exception_ptr& err) { if (!alive->load(std::memory_order_acquire)) { return; } @@ -425,22 +435,13 @@ class FlowSession { } } - void subscribeCurrent() { - // Publish which step the callbacks about to be installed belong to. - // Read back under the same mutex by every callback, so one that fires - // after the flow has moved on can recognise itself as stale. - std::size_t stepIndex = 0; - { - std::scoped_lock const lock{_mtx}; - _activeStep = _index; - stepIndex = _index; - } - detail::forStep(_index, - [this, stepIndex] { this->template installSubscription(stepIndex); }); - } - - void unsubscribeCurrent() { - detail::forStep(_index, [this] { _handler.template unsubscribe(); }); + /// @brief Publishes which step is now current. + /// + /// Read back under the same mutex by every dispatch callback, so one that + /// resolves after the flow has moved on recognises itself as stale. + void beginStep() { + std::scoped_lock const lock{_mtx}; + _activeStep = _index; } ::morph::bridge::BridgeHandler& _handler; diff --git a/include/morph/net/socket_backend.hpp b/include/morph/net/socket_backend.hpp index d2108cc..aed7039 100644 --- a/include/morph/net/socket_backend.hpp +++ b/include/morph/net/socket_backend.hpp @@ -145,6 +145,79 @@ class SocketBackend : public ::morph::backend::detail::IBackend { throw std::runtime_error("register failed: " + reply.message); } + /// @brief Sends a shared (register-or-attach) `register` and blocks for the reply. + /// + /// An empty primary degrades to the private path. Same synchronous-call + /// constraint as `registerModel`. + /// @param typeId String type-id of the model. + /// @param factory Ignored — the server constructs via its own registry. + /// @param identity Entity key for the action log plus the directory primary key. + /// @return `ModelId` of the shared (or newly created) instance. + /// @throws std::runtime_error if the server replies with an error or the socket is down. + ::morph::exec::detail::ModelId registerModelShared( + const std::string& typeId, std::function()> factory, + ::morph::backend::detail::InstanceIdentity identity) override { + if (identity.primary.empty()) { + return registerModelWithContext(typeId, std::move(factory), identity.contextKey); + } + return sendControlForId(::morph::wire::makeRegisterShared(typeId, std::string{identity.primary}, + std::string{identity.contextKey}), + "register"); + } + + /// @brief Sends an `attach` and blocks for the reply, re-pointing from @p current. + /// @param typeId String type-id of the model. + /// @param factory Ignored — the server constructs via its own registry. + /// @param identity Entity key for the action log plus the directory primary key. + /// @param current Instance currently held, or `ModelId{0}` if none. + /// @return `ModelId` of the instance now attached to. + /// @throws std::runtime_error if the server replies with an error or the socket is down. + ::morph::exec::detail::ModelId attachModel( + const std::string& typeId, std::function()> factory, + ::morph::backend::detail::InstanceIdentity identity, ::morph::exec::detail::ModelId current) override { + if (identity.primary.empty()) { + if (current.v != 0U) { + deregisterModel(current); + } + return registerModelWithContext(typeId, std::move(factory), identity.contextKey); + } + return sendControlForId(::morph::wire::makeAttach(typeId, std::string{identity.primary}, current.v), "attach"); + } + + /// @brief Files a live server-side instance under @p primary. + /// @param mid Live instance to promote. + /// @param typeId Model type id. + /// @param primary Canonical string encoding of the key to file it under. + void assignPrimary(::morph::exec::detail::ModelId mid, const std::string& typeId, + std::string_view primary) override { + if (primary.empty() || mid.v == 0U) { + return; + } + (void)sendControlForId(::morph::wire::makeAssign(typeId, std::string{primary}, mid.v), "assign"); + } + + /// @brief Asks the server for the live shared primary keys of @p typeId. + /// @param typeId String type-id to enumerate. + /// @return Canonical key strings of the live shared instances. + /// @throws std::runtime_error if the server replies with an error or the socket is down. + std::vector listInstances(const std::string& typeId) override { + std::string replyJson; + try { + replyJson = sendSync(::morph::wire::encode(::morph::wire::makeInstances(typeId))); + } catch (const std::exception& exc) { + throw std::runtime_error(std::string{"instances failed: "} + exc.what()); + } + auto reply = ::morph::wire::decode(replyJson); + if (reply.kind != "ok") { + throw std::runtime_error("instances failed: " + reply.message); + } + std::vector keys; + if (auto errCode = glz::read_json(keys, reply.body)) { + throw std::runtime_error("instances decode failed: " + glz::format_error(errCode, reply.body)); + } + return keys; + } + /// @brief Sends a `deregister` message fire-and-forget (does not wait for a reply). /// @param mid Id of the model to remove on the server. void deregisterModel(::morph::exec::detail::ModelId mid) override { @@ -160,6 +233,25 @@ class SocketBackend : public ::morph::backend::detail::IBackend { } } + /// @brief Sends one synchronous control envelope and returns the replied `modelId`. + /// @param env Envelope to send. + /// @param what Verb name used in the error message. + /// @return `ModelId` carried by the `ok` reply. + /// @throws std::runtime_error if the server errors or the socket is down. + ::morph::exec::detail::ModelId sendControlForId(const ::morph::wire::Envelope& env, std::string_view what) { + std::string replyJson; + try { + replyJson = sendSync(::morph::wire::encode(env)); + } catch (const std::exception& exc) { + throw std::runtime_error(std::string{what} + " failed: " + exc.what()); + } + auto reply = ::morph::wire::decode(replyJson); + if (reply.kind == "ok") { + return ::morph::exec::detail::ModelId{reply.modelId}; + } + throw std::runtime_error(std::string{what} + " failed: " + reply.message); + } + /// @brief Sends an `execute` message and returns a `Completion` resolved on reply. /// @param mid Target model id on the server. /// @param call Bundled action; `serializeAction` and `deserializeResult` are used. diff --git a/include/morph/qt/qt_websocket_backend.hpp b/include/morph/qt/qt_websocket_backend.hpp index d7d1208..3ce9a95 100644 --- a/include/morph/qt/qt_websocket_backend.hpp +++ b/include/morph/qt/qt_websocket_backend.hpp @@ -107,6 +107,42 @@ class QtWebSocketBackend : public ::morph::backend::detail::IBackend { const std::string& typeId, std::function()> factory) override; + /// @brief Sends a shared (register-or-attach) `register` and blocks for the reply. + /// + /// An empty primary degrades to the private path. + /// @param typeId String type-id of the model. + /// @param factory Ignored — model construction is delegated to the server. + /// @param identity Entity key for the action log plus the directory primary key. + /// @return `ModelId` of the shared (or newly created) instance. + /// @throws std::runtime_error if the server errors or the socket is not connected. + ::morph::exec::detail::ModelId registerModelShared( + const std::string& typeId, std::function()> factory, + ::morph::backend::detail::InstanceIdentity identity) override; + + /// @brief Sends an `attach` and blocks for the reply, re-pointing from @p current. + /// @param typeId String type-id of the model. + /// @param factory Ignored — model construction is delegated to the server. + /// @param identity Entity key for the action log plus the directory primary key. + /// @param current Instance currently held, or `ModelId{0}` if none. + /// @return `ModelId` of the instance now attached to. + /// @throws std::runtime_error if the server errors or the socket is not connected. + ::morph::exec::detail::ModelId attachModel( + const std::string& typeId, std::function()> factory, + ::morph::backend::detail::InstanceIdentity identity, ::morph::exec::detail::ModelId current) override; + + /// @brief Files a live server-side instance under @p primary. + /// @param mid Live instance to promote. + /// @param typeId Model type id. + /// @param primary Canonical string encoding of the key to file it under. + void assignPrimary(::morph::exec::detail::ModelId mid, const std::string& typeId, + std::string_view primary) override; + + /// @brief Asks the server for the live shared primary keys of @p typeId. + /// @param typeId String type-id to enumerate. + /// @return Canonical key strings of the live shared instances. + /// @throws std::runtime_error if the server errors or the socket is not connected. + std::vector listInstances(const std::string& typeId) override; + /// @brief Sends a `deregister` message fire-and-forget (does not wait for a reply). /// /// No acknowledgement is awaited, which avoids a nested `QEventLoop` during diff --git a/src/qt/qt_websocket_backend.cpp b/src/qt/qt_websocket_backend.cpp index 4f6af27..a258c34 100644 --- a/src/qt/qt_websocket_backend.cpp +++ b/src/qt/qt_websocket_backend.cpp @@ -142,6 +142,69 @@ ::morph::wire::ProtocolNegotiationResult QtWebSocketBackend::negotiateProtocolVe return ::morph::wire::interpretHelloReply(::morph::wire::decode(replyJson)); } +namespace { + +/// @brief Decodes a synchronous control reply and returns its `modelId`. +/// @param replyJson Raw reply text. +/// @param what Verb name used in the error message. +/// @return The replied `ModelId`. +/// @throws std::runtime_error if the reply is an `err`. +::morph::exec::detail::ModelId modelIdFromReply(const std::string& replyJson, std::string_view what) { + auto reply = ::morph::wire::decode(replyJson); + if (reply.kind == "ok") { + return ::morph::exec::detail::ModelId{reply.modelId}; + } + throw std::runtime_error(std::string{what} + " failed: " + reply.message); +} + +} // namespace + +::morph::exec::detail::ModelId QtWebSocketBackend::registerModelShared( + const std::string& typeId, std::function()> factory, + ::morph::backend::detail::InstanceIdentity identity) { + if (identity.primary.empty()) { + return registerModelWithContext(typeId, std::move(factory), identity.contextKey); + } + return modelIdFromReply(sendSync(::morph::wire::encode(::morph::wire::makeRegisterShared( + typeId, std::string{identity.primary}, std::string{identity.contextKey}))), + "register"); +} + +::morph::exec::detail::ModelId QtWebSocketBackend::attachModel( + const std::string& typeId, std::function()> factory, + ::morph::backend::detail::InstanceIdentity identity, ::morph::exec::detail::ModelId current) { + if (identity.primary.empty()) { + if (current.v != 0U) { + deregisterModel(current); + } + return registerModelWithContext(typeId, std::move(factory), identity.contextKey); + } + return modelIdFromReply( + sendSync(::morph::wire::encode(::morph::wire::makeAttach(typeId, std::string{identity.primary}, current.v))), + "attach"); +} + +void QtWebSocketBackend::assignPrimary(::morph::exec::detail::ModelId mid, const std::string& typeId, + std::string_view primary) { + if (primary.empty() || mid.v == 0U) { + return; + } + (void)modelIdFromReply( + sendSync(::morph::wire::encode(::morph::wire::makeAssign(typeId, std::string{primary}, mid.v))), "assign"); +} + +std::vector QtWebSocketBackend::listInstances(const std::string& typeId) { + auto reply = ::morph::wire::decode(sendSync(::morph::wire::encode(::morph::wire::makeInstances(typeId)))); + if (reply.kind != "ok") { + throw std::runtime_error("instances failed: " + reply.message); + } + std::vector keys; + if (auto errCode = glz::read_json(keys, reply.body)) { + throw std::runtime_error("instances decode failed: " + glz::format_error(errCode, reply.body)); + } + return keys; +} + void QtWebSocketBackend::deregisterModel(::morph::exec::detail::ModelId mid) { // Fire-and-forget — avoids a nested QEventLoop during destructor which can // trigger Qt asserts. The server does no connection-scoped cleanup, so an diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index f8a2b03..6ccaa78 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -45,6 +45,7 @@ add_executable(morph_tests test_coverage_extra.cpp test_coverage_push95.cpp test_server_limits.cpp + test_shared_instances.cpp test_wire_hardening.cpp test_protocol_version.cpp test_limit_policy.cpp diff --git a/tests/net/test_socket_backend.cpp b/tests/net/test_socket_backend.cpp index e8c8bcd..9829182 100644 --- a/tests/net/test_socket_backend.cpp +++ b/tests/net/test_socket_backend.cpp @@ -34,6 +34,35 @@ BRIDGE_REGISTER_MODEL(SbEchoModel, "SbEchoModel") BRIDGE_REGISTER_ACTION(SbEchoModel, SbEchoAction, "SbEchoAction") BRIDGE_REGISTER_ACTION(SbEchoModel, SbEchoFail, "SbEchoFail") +// A stateful, keyed model: two clients naming the same key must reach one +// instance and see one counter. A stateless echo model could not tell the +// difference between sharing and not sharing. +// +// External linkage as above: glaze reflection and the BRIDGE_REGISTER_* macros +// both require it. +// NOLINTBEGIN(misc-use-internal-linkage) +struct SbBump { + std::int64_t id = 0; + int by = 0; +}; +struct SbTotal { + int value = 0; +}; + +struct SbCounterModel { + using PrimaryKey = std::int64_t; + int value = 0; + SbTotal execute(const SbBump& act) { + value += act.by; + return {.value = value}; + } +}; + +BRIDGE_REGISTER_MODEL(SbCounterModel, "SbCounterModel") +BRIDGE_REGISTER_ACTION(SbCounterModel, SbBump, "SbBump") +BRIDGE_KEY_FROM(SbBump, &SbBump::id); +// NOLINTEND(misc-use-internal-linkage) + struct SbSlowAction { int value = 0; }; @@ -164,6 +193,74 @@ TEST_CASE("SocketBackend: two backends share one server with isolated model stat REQUIRE(lastB.load() == 22); } +TEST_CASE("SocketBackend: two clients sharing a key reach one instance over the wire", + "[net][socket_backend][shared-instances]") { + morph::exec::ThreadPoolExecutor serverPool{2}; + auto server = std::make_shared(serverPool); + morph::net::SocketServer wsServer{*server, 0}; + REQUIRE(wsServer.listen()); + + std::string const url = "ws://127.0.0.1:" + std::to_string(static_cast(wsServer.port())); + auto backendA = std::make_unique(url); + auto backendB = std::make_unique(url); + REQUIRE(backendA->waitForConnected()); + REQUIRE(backendB->waitForConnected()); + + morph::exec::ThreadPoolExecutor cbPool{2}; + morph::bridge::Bridge bridgeA{std::move(backendA)}; + morph::bridge::Bridge bridgeB{std::move(backendB)}; + morph::bridge::BridgeHandler fromA{bridgeA, &cbPool}; + morph::bridge::BridgeHandler fromB{bridgeB, &cbPool}; + + // Two genuinely separate clients, two sockets, one server-side directory. + std::atomic lastA{-1}; + fromA.execute(SbBump{.id = 77, .by = 10}).then([&](const SbTotal& res) { lastA.store(res.value); }).onError([](const std::exception_ptr&) {}); + spinUntil([&] { return lastA.load() != -1; }); + REQUIRE(lastA.load() == 10); + + std::atomic lastB{-1}; + fromB.execute(SbBump{.id = 77, .by = 5}).then([&](const SbTotal& res) { lastB.store(res.value); }).onError([](const std::exception_ptr&) {}); + spinUntil([&] { return lastB.load() != -1; }); + // 15, not 5: the second client attached to the first client's instance. + REQUIRE(lastB.load() == 15); + + std::atomic keyCount{-1}; + fromB.instances().then([&](const std::vector& keys) { keyCount.store(static_cast(keys.size())); }) + .onError([](const std::exception_ptr&) {}); + spinUntil([&] { return keyCount.load() != -1; }); + REQUIRE(keyCount.load() == 1); +} + +TEST_CASE("SocketBackend: a plain handler keeps its own instance over the wire", "[net][socket_backend]") { + morph::exec::ThreadPoolExecutor serverPool{2}; + auto server = std::make_shared(serverPool); + morph::net::SocketServer wsServer{*server, 0}; + REQUIRE(wsServer.listen()); + + std::string const url = "ws://127.0.0.1:" + std::to_string(static_cast(wsServer.port())); + auto shared = std::make_unique(url); + auto priv = std::make_unique(url); + REQUIRE(shared->waitForConnected()); + REQUIRE(priv->waitForConnected()); + + morph::exec::ThreadPoolExecutor cbPool{2}; + morph::bridge::Bridge sharedBridge{std::move(shared)}; + morph::bridge::Bridge privBridge{std::move(priv)}; + morph::bridge::BridgeHandler joined{sharedBridge, &cbPool}; + morph::bridge::BridgeHandler alone{privBridge, &cbPool}; + + std::atomic lastShared{-1}; + joined.execute(SbBump{.id = 88, .by = 30}).then([&](const SbTotal& res) { lastShared.store(res.value); }).onError([](const std::exception_ptr&) {}); + spinUntil([&] { return lastShared.load() != -1; }); + REQUIRE(lastShared.load() == 30); + + std::atomic lastPriv{-1}; + alone.execute(SbBump{.id = 88, .by = 1}).then([&](const SbTotal& res) { lastPriv.store(res.value); }).onError([](const std::exception_ptr&) {}); + spinUntil([&] { return lastPriv.load() != -1; }); + // Opted out, so it registered its own instance and counts from zero. + REQUIRE(lastPriv.load() == 1); +} + TEST_CASE("SocketBackend: registerModel on a never-connected socket throws, does not hang", "[net][socket_backend][disconnect]") { // Port 1 is reserved (root-only) on Linux/macOS and never listening — the diff --git a/tests/test_computed_fields.cpp b/tests/test_computed_fields.cpp index b07dcf4..962ad9e 100644 --- a/tests/test_computed_fields.cpp +++ b/tests/test_computed_fields.cpp @@ -240,26 +240,36 @@ TEST_CASE("BridgeHandler::set<> recomputes total live before firing", "[bridge][ } }); - handler.set<&CFLineItem::qty>(Rational{Numerator{3}, Denominator{1}, dp2}); - handler.set<&CFLineItem::price>(Rational{Numerator{2}, Denominator{1}, dp2}); + // `total` is left unengaged on purpose: recomputeAll overwrites it from + // qty*price on the dispatch path, which is what this asserts. + handler.execute(CFLineItem{.qty = Rational{Numerator{3}, Denominator{1}, dp2}, + .price = Rational{Numerator{2}, Denominator{1}, dp2}, + .total = {}}); REQUIRE(morph::testing::waitUntil([&] { return haveTotal.load(); })); std::scoped_lock lock{totalMtx}; CHECK(lastTotal == Rational{6, dp2}); } -TEST_CASE("BridgeHandler::set<> does not fire before both computed inputs are engaged", "[bridge][computed]") { +TEST_CASE("an action with a computed input missing fails its validator", "[bridge][computed]") { morph::exec::ThreadPoolExecutor pool{2}; SyncExecutor cbExec; morph::bridge::Bridge bridge{std::make_unique(pool)}; morph::bridge::BridgeHandler handler{bridge, &cbExec}; std::atomic fired{false}; + std::atomic failed{false}; handler.subscribe([&](CFLineItem /*unused*/) { fired.store(true); }); - handler.set<&CFLineItem::qty>(Rational{Numerator{3}, Denominator{1}, dp2}); - std::this_thread::sleep_for(std::chrono::milliseconds{50}); - CHECK_FALSE(fired.load()); // price still missing -> total unengaged -> validate() is false + // price is missing, so total stays unengaged and validate() is false. The + // validator gate now sits on the dispatch path rather than in a client-side + // draft, so the action is rejected instead of simply never firing -- and a + // failed action notifies no subscriber. + handler.execute(CFLineItem{.qty = Rational{Numerator{3}, Denominator{1}, dp2}, .price = {}, .total = {}}) + .onError([&](const std::exception_ptr&) { failed.store(true); }); + + REQUIRE(morph::testing::waitUntil([&] { return failed.load(); })); + CHECK_FALSE(fired.load()); } // --------------------------------------------------------------------------- diff --git a/tests/test_concurrency_invariants.cpp b/tests/test_concurrency_invariants.cpp index 70c8352..e6e0622 100644 --- a/tests/test_concurrency_invariants.cpp +++ b/tests/test_concurrency_invariants.cpp @@ -302,9 +302,20 @@ TEST_CASE("morph::bridge::Bridge: concurrent executeVia under repeated switchBac REQUIRE(resolved.load() == totalActions); REQUIRE(succeeded.load() + failed.load() == totalActions); - // Some actions must succeed — if every single one failed something is - // structurally broken (the snapshot-and-dispatch path never resolved). - REQUIRE(succeeded.load() > 0); + + // The structural claim is that the snapshot-and-dispatch path still + // *resolves successfully* after repeated backend switching — not that it + // wins any particular race. Assert it against the now-quiesced bridge + // rather than against the churn above: under a thread-serialising tool + // (Valgrind runs every thread on one core) the switcher can legitimately + // cancel all 200 in-flight calls, which made a `succeeded > 0` check on the + // churn a coin flip rather than an invariant. + std::atomic afterSwitching{0}; + constexpr int settledActions = 5; + for (int idx = 0; idx < settledActions; ++idx) { + handler.execute(LoadCountAction{1}).then([&](int) { afterSwitching.fetch_add(1); }); + } + REQUIRE(waitUntil([&] { return afterSwitching.load() == settledActions; }, 10s)); } // ── morph::offline::NetworkMonitor: stop() called from onOnline does not deadlock ───────────── diff --git a/tests/test_coverage_extra.cpp b/tests/test_coverage_extra.cpp index 35cfbe9..ffffed6 100644 --- a/tests/test_coverage_extra.cpp +++ b/tests/test_coverage_extra.cpp @@ -52,29 +52,17 @@ TEST_CASE("BridgeHandler::unsubscribe on type with no entry is a no-op", "[bridg morph::bridge::Bridge bridge{std::make_unique(pool)}; morph::bridge::BridgeHandler handler{bridge, &cb}; - // No prior subscribe → the unsubscribe path finds no entry and exercises - // the False arm of `if (iter != _subs->entries.end())` at line 311. - REQUIRE_NOTHROW(handler.unsubscribe()); + // No prior subscribe, so the removal path finds nothing to erase. + REQUIRE_NOTHROW(handler.unsubscribe()); } -TEST_CASE("BridgeHandler::reset on type with no entry is a no-op", "[bridge]") { +TEST_CASE("BridgeHandler::unsubscribe is idempotent", "[bridge]") { morph::exec::ThreadPoolExecutor pool{1}; CovSyncExecutor cb; morph::bridge::Bridge bridge{std::make_unique(pool)}; morph::bridge::BridgeHandler handler{bridge, &cb}; - REQUIRE_NOTHROW(handler.reset()); -} - -TEST_CASE("BridgeHandler::set on a field reuses an existing draft", "[bridge]") { - morph::exec::ThreadPoolExecutor pool{1}; - CovSyncExecutor cb; - morph::bridge::Bridge bridge{std::make_unique(pool)}; - morph::bridge::BridgeHandler handler{bridge, &cb}; - - // First set creates the draft entry → line 327 True arm. Second set finds - // the entry already there → False arm. - handler.set<&CovAction::v>(1); - handler.set<&CovAction::v>(2); - REQUIRE(true); + handler.subscribe([](int) {}); + REQUIRE_NOTHROW(handler.unsubscribe()); + REQUIRE_NOTHROW(handler.unsubscribe()); } diff --git a/tests/test_coverage_gaps.cpp b/tests/test_coverage_gaps.cpp index a783620..dd8d494 100644 --- a/tests/test_coverage_gaps.cpp +++ b/tests/test_coverage_gaps.cpp @@ -6,7 +6,9 @@ #include #include +#include #include +#include #include #include #include @@ -423,116 +425,26 @@ BRIDGE_REGISTER_VALIDATOR(SlowSubAction, [](const SlowSubAction& act) { return a BRIDGE_REGISTER_VALIDATOR(ThrowSubAction, [](const ThrowSubAction& act) { return act.trigger != 0; }) BRIDGE_REGISTER_VALIDATOR(WeirdSubAction, [](const WeirdSubAction& act) { return act.trigger != 0; }) -TEST_CASE("morph::bridge::BridgeHandler: handler destroyed mid-flight makes weak-lock continuations no-op", - "[coverage][bridge]") { - // Covers tryFireImpl's outer weak.lock() check (489-490) and the - // then-continuation's inner weak.lock() (518-519). The action sleeps long - // enough for us to destroy the handler before the continuation runs. - morph::exec::ThreadPoolExecutor pool{2}; - SyncExecutor cbExec; - morph::bridge::Bridge bridge{std::make_unique(pool)}; - - { - morph::bridge::BridgeHandler handler{bridge, &cbExec}; - std::atomic got{-1}; - handler.subscribe([&](int value) { got.store(value); }); - handler.set<&SlowSubAction::seq>(7); - // Don't wait — drop the handler immediately so the continuation lands - // after SubscriberState is destroyed. - } - // Let the pool drain so the dispatched op finishes (no-op via weak). - std::this_thread::sleep_for(120ms); - REQUIRE(true); -} - -TEST_CASE("morph::bridge::BridgeHandler: onError continuation no-ops when SubscriberState already gone", - "[coverage][bridge]") { - // Covers the onError-continuation's weak.lock() failure path (532-533). - morph::exec::ThreadPoolExecutor pool{2}; - SyncExecutor cbExec; - morph::bridge::Bridge bridge{std::make_unique(pool)}; - - { - morph::bridge::BridgeHandler handler{bridge, &cbExec}; - handler.subscribe([](int) {}); // sink installed but action throws below - // Use ThrowSubAction which throws synchronously inside the strand; the - // onError continuation lands after the handler dies. - handler.subscribe([](int) {}); - handler.set<&ThrowSubAction::trigger>(1); - } - std::this_thread::sleep_for(60ms); - REQUIRE(true); -} - -// ── bridge.hpp: logUnhandledError non-std::exception branch (lines 480-482) - -TEST_CASE("morph::bridge::BridgeHandler: logUnhandledError covers non-std::exception branch", "[coverage][bridge]") { - // No errSink installed → outcome.errSink is empty → onError continuation - // calls logUnhandledError, which rethrows; the action's exception is a - // non-std type, so the catch(...) arm fires. - morph::exec::ThreadPoolExecutor pool{2}; +TEST_CASE("morph::bridge::BridgeHandler: unsubscribe with no entry is a no-op", "[coverage][bridge]") { + morph::exec::ThreadPoolExecutor pool{1}; SyncExecutor cbExec; morph::bridge::Bridge bridge{std::make_unique(pool)}; morph::bridge::BridgeHandler handler{bridge, &cbExec}; - LogGuard guard; - std::atomic sawUnknown{false}; - morph::log::setLogger([&](morph::log::LogLevel /*lvl*/, std::string_view msg) { - if (msg.contains("unknown")) { - sawUnknown.store(true); - } - }); - - handler.subscribe([](int) {}); // sink only, no errSink - handler.set<&WeirdSubAction::trigger>(1); - - REQUIRE(waitFor([&] { return sawUnknown.load(); })); + REQUIRE_NOTHROW(handler.unsubscribe()); } -TEST_CASE("morph::bridge::BridgeHandler: logUnhandledError covers std::exception branch", "[coverage][bridge]") { - // Same shape but with a std::exception payload; covers lines 476-479 by - // making sure the catch(const std::exception&) arm runs in addition to - // the catch(...) arm above. - morph::exec::ThreadPoolExecutor pool{2}; +TEST_CASE("morph::bridge::Bridge: publishResult with no subscribers is a no-op", "[coverage][bridge]") { + morph::exec::ThreadPoolExecutor pool{1}; SyncExecutor cbExec; morph::bridge::Bridge bridge{std::make_unique(pool)}; morph::bridge::BridgeHandler handler{bridge, &cbExec}; - LogGuard guard; - std::atomic sawStd{false}; - morph::log::setLogger([&](morph::log::LogLevel /*lvl*/, std::string_view msg) { - if (msg.contains("threw inside action")) { - sawStd.store(true); - } - }); - - handler.subscribe([](int) {}); // sink only, no errSink - handler.set<&ThrowSubAction::trigger>(1); - - REQUIRE(waitFor([&] { return sawStd.load(); })); + // Nothing is subscribed, so the fan-out loop finds no matching entry. + REQUIRE_NOTHROW(bridge.publishResult(::morph::exec::detail::ModelId{1}, std::type_index{typeid(int)}, + std::any{int{0}})); } -// ── bridge.hpp: refire-after-error path (lines 541-542) - -// ── bridge.hpp: unsubscribe/reset early-return branches (lines 374, 416) - -TEST_CASE("morph::bridge::BridgeHandler: unsubscribe with no entry is a no-op", "[coverage][bridge]") { - morph::exec::ThreadPoolExecutor pool{2}; - SyncExecutor cbExec; - morph::bridge::Bridge bridge{std::make_unique(pool)}; - morph::bridge::BridgeHandler handler{bridge, &cbExec}; - REQUIRE_NOTHROW(handler.unsubscribe()); // entries empty → false arm at 374 -} - -TEST_CASE("morph::bridge::BridgeHandler: reset with no entry is a no-op", "[coverage][bridge]") { - morph::exec::ThreadPoolExecutor pool{2}; - SyncExecutor cbExec; - morph::bridge::Bridge bridge{std::make_unique(pool)}; - morph::bridge::BridgeHandler handler{bridge, &cbExec}; - REQUIRE_NOTHROW(handler.reset()); // false arm at 416 -} - -// ── bridge.hpp: deregisterHandler search lambda's mismatch arm (line 163) TEST_CASE("morph::bridge::Bridge: deregisterHandler skips other bindings", "[coverage][bridge]") { // With two live handlers, deregistering one walks past the other in the @@ -663,34 +575,6 @@ TEST_CASE("morph::bridge::Bridge: switchBackend purges weak_ptr bindings whose o // ── bridge.hpp: tryFireImpl returns when draft is absent (lines 498-499) -TEST_CASE("morph::bridge::BridgeHandler: tryFireImpl bails when draft has been reset", "[coverage][bridge]") { - // After reset<>(), the entry's draft is empty. A subsequent tryFireImpl - // would observe `!iter->second.draft.has_value()` and return. - // We can't call tryFireImpl directly (private), but we can drive a - // refire path: set fires → in flight → reset clears draft → continuation - // returns refire=true → tryFireImpl re-enters → entry exists but draft - // empty → 498-499. - morph::exec::ThreadPoolExecutor pool{2}; - SyncExecutor cbExec; - morph::bridge::Bridge bridge{std::make_unique(pool)}; - morph::bridge::BridgeHandler handler{bridge, &cbExec}; - - std::atomic sinkCount{0}; - handler.subscribe([&](int) { sinkCount.fetch_add(1); }); - - // First set fires SlowSubAction (sleeps 40ms in the model). - handler.set<&SlowSubAction::seq>(7); - // Pile a second set on while the first is still in flight — pending=true. - handler.set<&SlowSubAction::seq>(8); - // Drop the draft. When the first dispatch's continuation lands, refire - // is queued and the recursive tryFireImpl sees an empty draft → 498-499. - handler.reset(); - - // Let everything settle. Either path is fine — we just need the recursive - // tryFireImpl to be invoked with the empty draft. - std::this_thread::sleep_for(120ms); -} - // ── model.hpp: morph::model::detail::IModelHolder::into() throws std::bad_cast (lines 70-71) TEST_CASE("morph::model::detail::IModelHolder::into() throws std::bad_cast", "[coverage][model]") { @@ -699,29 +583,3 @@ TEST_CASE("morph::model::detail::IModelHolder::into() throws std::bad_cas // sanity: into() still works REQUIRE_NOTHROW(holder->template into()); } - -TEST_CASE("morph::bridge::BridgeHandler: refire fires again after a failed action", "[coverage][bridge]") { - // Burst two set<>s on a throwing action: the first kicks off the dispatch, - // the second sets pending=true while running. On error, consumeFlight - // returns refire=true → tryFireImpl is re-invoked from the onError arm. - morph::exec::ThreadPoolExecutor pool{2}; - SyncExecutor cbExec; - morph::bridge::Bridge bridge{std::make_unique(pool)}; - morph::bridge::BridgeHandler handler{bridge, &cbExec}; - - std::atomic errorsSeen{0}; - handler.subscribe([](int) {}, [&](const std::exception_ptr&) { errorsSeen.fetch_add(1); }); - - // First trigger — dispatch starts, may already be in flight. - handler.set<&ThrowSubAction::trigger>(1); - // Second trigger while the first is most likely still in flight on the strand; - // marks pending=true → onError consumeFlight returns refire=true → fires - // tryFireImpl again, which dispatches a second time → second error. - handler.set<&ThrowSubAction::trigger>(2); - - // We may or may not race the in-flight window; at minimum we expect one - // error (no refire) or two errors (refire happened). Wait for the refire - // path with a longer budget; if we never see two, that's fine — the test - // still exercised consumeFlight. - REQUIRE(waitFor([&] { return errorsSeen.load() >= 1; })); -} diff --git a/tests/test_example.cpp b/tests/test_example.cpp index ee2b1cc..4fa2b40 100644 --- a/tests/test_example.cpp +++ b/tests/test_example.cpp @@ -40,14 +40,14 @@ TEST_CASE("Example Model", "[model]") { .then([&](ActionOutput output) { REQUIRE(output.result == 6.0); }) .onError([](const std::exception_ptr&) { FAIL("Action execution should not have thrown an exception"); }); + // Subscribing names the *result* type, so the observer never has to know + // which action produced it. std::atomic fired{false}; - handler.subscribe([&](ActionOutput output) { + handler.subscribe([&](ActionOutput output) { REQUIRE(output.result == 6.0); fired.store(true); }); - handler.set<&ActionInput::a>(1.0); - handler.set<&ActionInput::b>(2.0); - handler.set<&ActionInput::c>(3.0); + handler.execute(ActionInput{.a = 1.0, .b = 2.0, .c = 3.0}); std::this_thread::sleep_for(std::chrono::milliseconds{50}); REQUIRE(fired.load() == true); diff --git a/tests/test_remote_connection_scope.cpp b/tests/test_remote_connection_scope.cpp index 4b8dc7b..da58f74 100644 --- a/tests/test_remote_connection_scope.cpp +++ b/tests/test_remote_connection_scope.cpp @@ -470,3 +470,203 @@ TEST_CASE("morph::backend::RemoteServer: repeated registers on a closed scope ne } REQUIRE(server->health().liveModels == 0U); } + +// ── Shared instance directory: the server side of keyed instances ──────────── +// +// The directory keys on the `(typeId, primary)` strings the envelope carries, +// so it needs no keyed model type to exercise — that is a client-side concept. +// These drive RemoteServer directly, which is the only way to reach the +// refcount-and-scope interactions that make cross-client sharing safe. + +TEST_CASE("morph::backend::RemoteServer: two connections sharing a key reach one instance", + "[remote][connection-scope][shared-instances]") { + morph::exec::ThreadPoolExecutor pool{2}; + auto& env = csEnv(); + auto server = std::make_shared(pool, env.dispatcher, env.registry); + + auto cidA = server->openConnection(); + auto cidB = server->openConnection(); + + WaitReply regA; + server->handle(morph::wire::encode(morph::wire::makeRegisterShared("CS_SquareModel", "42")), std::ref(regA), cidA); + REQUIRE(regA.await()); + REQUIRE(regA.env.kind == "ok"); + + WaitReply regB; + server->handle(morph::wire::encode(morph::wire::makeRegisterShared("CS_SquareModel", "42")), std::ref(regB), cidB); + REQUIRE(regB.await()); + REQUIRE(regB.env.kind == "ok"); + + // One instance, not two: the second register attached to the first's. + REQUIRE(regB.env.modelId == regA.env.modelId); + REQUIRE(server->health().liveModels == 1U); + + // Closing one connection releases only *its* reference — the instance must + // survive for the connection still attached. This is the A7 change. + server->closeConnection(cidA); + REQUIRE(server->health().liveModels == 1U); + + morph::wire::Envelope execReq; + execReq.kind = "execute"; + execReq.modelId = regA.env.modelId; + execReq.modelType = "CS_SquareModel"; + execReq.actionType = "CS_SquareAction"; + execReq.body = R"({"x":5})"; + WaitReply stillThere; + server->handle(morph::wire::encode(execReq), std::ref(stillThere)); + REQUIRE(stillThere.await()); + REQUIRE(stillThere.env.kind == "ok"); + REQUIRE(stillThere.env.body == "25"); + + // The last reference goes, and so does the instance. + server->closeConnection(cidB); + REQUIRE(server->health().liveModels == 0U); +} + +TEST_CASE("morph::backend::RemoteServer: instances lists live shared keys", + "[remote][connection-scope][shared-instances]") { + morph::exec::ThreadPoolExecutor pool{2}; + auto& env = csEnv(); + auto server = std::make_shared(pool, env.dispatcher, env.registry); + auto cid = server->openConnection(); + + // Unrolled rather than looped: each Catch2 REQUIRE expands to branches, and + // a loop around them trips the cognitive-complexity gate for no benefit. + WaitReply regSeven; + server->handle(morph::wire::encode(morph::wire::makeRegisterShared("CS_SquareModel", "7")), std::ref(regSeven), + cid); + REQUIRE(regSeven.await()); + REQUIRE(regSeven.env.kind == "ok"); + + WaitReply regNine; + server->handle(morph::wire::encode(morph::wire::makeRegisterShared("CS_SquareModel", "9")), std::ref(regNine), cid); + REQUIRE(regNine.await()); + REQUIRE(regNine.env.kind == "ok"); + + // A private register is invisible to the directory by construction. + WaitReply priv; + server->handle(morph::wire::encode(morph::wire::makeRegister("CS_SquareModel")), std::ref(priv), cid); + REQUIRE(priv.await()); + + WaitReply listed; + server->handle(morph::wire::encode(morph::wire::makeInstances("CS_SquareModel")), std::ref(listed), cid); + REQUIRE(listed.await()); + REQUIRE(listed.env.kind == "ok"); + std::vector keys; + REQUIRE_FALSE(glz::read_json(keys, listed.env.body)); + std::ranges::sort(keys); + REQUIRE(keys == std::vector{"7", "9"}); +} + +TEST_CASE("morph::backend::RemoteServer: attach re-points and releases the old instance", + "[remote][connection-scope][shared-instances]") { + morph::exec::ThreadPoolExecutor pool{2}; + auto& env = csEnv(); + auto server = std::make_shared(pool, env.dispatcher, env.registry); + auto cid = server->openConnection(); + + WaitReply first; + server->handle(morph::wire::encode(morph::wire::makeRegisterShared("CS_SquareModel", "1")), std::ref(first), cid); + REQUIRE(first.await()); + REQUIRE(first.env.kind == "ok"); + REQUIRE(server->health().liveModels == 1U); + + WaitReply moved; + server->handle(morph::wire::encode(morph::wire::makeAttach("CS_SquareModel", "2", first.env.modelId)), + std::ref(moved), cid); + REQUIRE(moved.await()); + REQUIRE(moved.env.kind == "ok"); + REQUIRE(moved.env.modelId != first.env.modelId); + // Nobody else held key 1, so re-pointing destroyed it rather than leaking. + REQUIRE(server->health().liveModels == 1U); +} + +TEST_CASE("morph::backend::RemoteServer: assign files a live instance under a key", + "[remote][connection-scope][shared-instances]") { + morph::exec::ThreadPoolExecutor pool{2}; + auto& env = csEnv(); + auto server = std::make_shared(pool, env.dispatcher, env.registry); + auto cid = server->openConnection(); + + // A private instance, as a create-style action would run on. + WaitReply anon; + server->handle(morph::wire::encode(morph::wire::makeRegister("CS_SquareModel")), std::ref(anon), cid); + REQUIRE(anon.await()); + REQUIRE(anon.env.kind == "ok"); + + WaitReply promoted; + server->handle(morph::wire::encode(morph::wire::makeAssign("CS_SquareModel", "100", anon.env.modelId)), + std::ref(promoted), cid); + REQUIRE(promoted.await()); + REQUIRE(promoted.env.kind == "ok"); + + // It is the *same* instance, now reachable by key — nothing was recreated. + WaitReply attached; + server->handle(morph::wire::encode(morph::wire::makeRegisterShared("CS_SquareModel", "100")), std::ref(attached), + cid); + REQUIRE(attached.await()); + REQUIRE(attached.env.modelId == anon.env.modelId); + +} + +TEST_CASE("morph::backend::RemoteServer: assign never displaces the incumbent holder of a key", + "[remote][connection-scope][shared-instances]") { + morph::exec::ThreadPoolExecutor pool{2}; + auto& env = csEnv(); + auto server = std::make_shared(pool, env.dispatcher, env.registry); + auto cid = server->openConnection(); + + WaitReply incumbent; + server->handle(morph::wire::encode(morph::wire::makeRegisterShared("CS_SquareModel", "200")), std::ref(incumbent), + cid); + REQUIRE(incumbent.await()); + REQUIRE(incumbent.env.kind == "ok"); + + // A second instance promoting onto the taken key is a silent no-op: the + // incumbent always wins, so no already-attached client is redirected. + WaitReply other; + server->handle(morph::wire::encode(morph::wire::makeRegister("CS_SquareModel")), std::ref(other), cid); + REQUIRE(other.await()); + WaitReply clash; + server->handle(morph::wire::encode(morph::wire::makeAssign("CS_SquareModel", "200", other.env.modelId)), + std::ref(clash), cid); + REQUIRE(clash.await()); + REQUIRE(clash.env.kind == "ok"); + + WaitReply again; + server->handle(morph::wire::encode(morph::wire::makeRegisterShared("CS_SquareModel", "200")), std::ref(again), cid); + REQUIRE(again.await()); + REQUIRE(again.env.modelId == incumbent.env.modelId); +} + +TEST_CASE("morph::backend::RemoteServer: the new kinds reject an empty typeId", + "[remote][connection-scope][shared-instances]") { + morph::exec::ThreadPoolExecutor pool{2}; + auto& env = csEnv(); + auto server = std::make_shared(pool, env.dispatcher, env.registry); + + for (const auto& request : {morph::wire::makeAttach("", "1"), morph::wire::makeAssign("", "1", 7), + morph::wire::makeInstances("")}) { + WaitReply reply; + server->handle(morph::wire::encode(request), std::ref(reply)); + REQUIRE(reply.await()); + REQUIRE(reply.env.kind == "err"); + } +} + +TEST_CASE("morph::backend::RemoteServer: a shared register on a closed scope is refused", + "[remote][connection-scope][shared-instances]") { + morph::exec::ThreadPoolExecutor pool{2}; + auto& env = csEnv(); + auto server = std::make_shared(pool, env.dispatcher, env.registry); + + auto cid = server->openConnection(); + server->closeConnection(cid); + + WaitReply reg; + server->handle(morph::wire::encode(morph::wire::makeRegisterShared("CS_SquareModel", "5")), std::ref(reg), cid); + REQUIRE(reg.await()); + REQUIRE(reg.env.kind == "err"); + REQUIRE(reg.env.message == "connection closed"); + REQUIRE(server->health().liveModels == 0U); +} diff --git a/tests/test_shared_instances.cpp b/tests/test_shared_instances.cpp new file mode 100644 index 0000000..35259d4 --- /dev/null +++ b/tests/test_shared_instances.cpp @@ -0,0 +1,424 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// Tests for keyed, shareable model instances (F2). +// +// A model declares a `PrimaryKey` alias; actions declare which field carries it +// (`BRIDGE_KEY_FROM`) or that their result establishes it +// (`BRIDGE_KEY_FROM_RESULT`); `BridgeHandler` joins a +// server-side directory keyed on `(typeId, primary)`. These tests pin the four +// properties the design turns on: two shared handlers naming one key reach one +// instance, a plain handler never does, the instance survives until the last +// attachment goes away, and all of it behaves identically local and remote. +// +// See docs/planned/shared_model_instances.md. + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "test_support.hpp" + +namespace { + +// Model/action types need external linkage for glaze reflection, so they live +// at namespace scope below rather than inside the test cases. + +} // namespace + +/// A counter whose value lives *in the instance* — the whole point of keying. +// Model, action and result types need **external** linkage: glaze's +// plain-aggregate reflection cannot see into an anonymous namespace, and the +// BRIDGE_REGISTER_* macros specialise templates at global scope. +// NOLINTBEGIN(misc-use-internal-linkage) +/// A stateless model would make every one of these tests vacuous. +struct ShiCounterState { + std::int64_t value = 0; +}; + +struct ShiAddTo { + std::int64_t id = 0; + std::int64_t amount = 0; +}; + +struct ShiRead { + std::int64_t id = 0; +}; + +/// Keyless: runs against whatever instance the handler already holds. +struct ShiPeek { + int unused = 0; +}; + +/// Creates the entity, so its key cannot be in the request — it comes back in +/// the reply, exactly as a database insert returns its generated primary key. +struct ShiCreate { + std::int64_t initial = 0; +}; + +struct ShiCreated { + std::int64_t id = 0; + std::int64_t value = 0; +}; + +/// Like ShiCreate, but the caller picks the id the reply will carry — the only +/// way to force a key collision through the public API. +struct ShiCreateAs { + std::int64_t wantId = 0; + std::int64_t initial = 0; +}; + +struct ShiCounterModel { + using PrimaryKey = std::int64_t; + + std::int64_t value = 0; + + ShiCounterState execute(const ShiAddTo& act) { + value += act.amount; + return {.value = value}; + } + [[nodiscard]] ShiCounterState execute(const ShiRead& /*act*/) const { return {.value = value}; } + [[nodiscard]] ShiCounterState execute(const ShiPeek& /*act*/) const { return {.value = value}; } + + ShiCreated execute(const ShiCreate& act) { + static std::atomic nextId{9000}; + value = act.initial; + return {.id = nextId.fetch_add(1), .value = value}; + } + + ShiCreated execute(const ShiCreateAs& act) { + value = act.initial; + return {.id = act.wantId, .value = value}; + } +}; + +BRIDGE_REGISTER_MODEL(ShiCounterModel, "SHI_CounterModel") +BRIDGE_REGISTER_ACTION(ShiCounterModel, ShiAddTo, "SHI_AddTo") +BRIDGE_REGISTER_ACTION(ShiCounterModel, ShiRead, "SHI_Read") +BRIDGE_REGISTER_ACTION(ShiCounterModel, ShiPeek, "SHI_Peek") +BRIDGE_REGISTER_ACTION(ShiCounterModel, ShiCreate, "SHI_Create") +BRIDGE_REGISTER_ACTION(ShiCounterModel, ShiCreateAs, "SHI_CreateAs") + +BRIDGE_KEY_FROM(ShiAddTo, &ShiAddTo::id); +BRIDGE_KEY_FROM(ShiRead, &ShiRead::id); +BRIDGE_KEY_FROM_RESULT(ShiCreate, &ShiCreated::id); +BRIDGE_KEY_FROM_RESULT(ShiCreateAs, &ShiCreated::id); +// NOLINTEND(misc-use-internal-linkage) + +namespace { + +using morph::bridge::AllowShared; +using morph::bridge::Bridge; +using morph::bridge::BridgeHandler; + +/// Drives a `Completion` to resolution and returns the value. +/// +/// Polls rather than assuming synchronous resolution: `LocalBackend` resolves on +/// the caller's thread here, but `SimulatedRemoteBackend` posts to a worker pool, +/// and the same test bodies run against both. +template +T settle(morph::async::Completion comp) { + auto out = std::make_shared(); + auto done = std::make_shared>(false); + auto failed = std::make_shared>(false); + std::move(comp) + .then([out, done](T value) { + *out = std::move(value); + done->store(true); + }) + .onError([failed, done](const std::exception_ptr&) { + failed->store(true); + done->store(true); + }); + REQUIRE(morph::testing::waitUntil([&] { return done->load(); })); + REQUIRE_FALSE(failed->load()); + return *out; +} + +std::unique_ptr makeLocal(morph::exec::IExecutor& pool) { + return std::make_unique(pool); +} + +} // namespace + +TEST_CASE("two AllowShared handlers naming one key reach one instance", "[shared-instances]") { + morph::testing::InlineExecutor exec; + Bridge bridge{makeLocal(exec)}; + + BridgeHandler first{bridge, &exec}; + BridgeHandler second{bridge, &exec}; + + // A keyed action attaches the handler on the way through. + REQUIRE(settle(first.execute(ShiAddTo{.id = 42, .amount = 10})).value == 10); + // The second handler names the same key, so it lands on the same counter + // and observes the first handler's work. + REQUIRE(settle(second.execute(ShiAddTo{.id = 42, .amount = 5})).value == 15); + REQUIRE(settle(first.execute(ShiRead{.id = 42})).value == 15); + + REQUIRE(first.primary().value_or(-1) == 42); + REQUIRE(second.primary().value_or(-1) == 42); +} + +TEST_CASE("a plain handler never joins the directory", "[shared-instances]") { + morph::testing::InlineExecutor exec; + Bridge bridge{makeLocal(exec)}; + + BridgeHandler shared{bridge, &exec}; + BridgeHandler priv{bridge, &exec}; + + REQUIRE(settle(shared.execute(ShiAddTo{.id = 7, .amount = 100})).value == 100); + // Same key, but this handler opted out — it gets its own instance, starting + // from zero, exactly as every pre-existing call site does. + REQUIRE(settle(priv.execute(ShiAddTo{.id = 7, .amount = 1})).value == 1); + // …and the shared instance is untouched by it. + REQUIRE(settle(shared.execute(ShiRead{.id = 7})).value == 100); +} + +TEST_CASE("different keys are different instances", "[shared-instances]") { + morph::testing::InlineExecutor exec; + Bridge bridge{makeLocal(exec)}; + + BridgeHandler handler{bridge, &exec}; + REQUIRE(settle(handler.execute(ShiAddTo{.id = 1, .amount = 3})).value == 3); + // A different key is a different counter, not a re-labelled one. + REQUIRE(settle(handler.execute(ShiAddTo{.id = 2, .amount = 8})).value == 8); + + // Re-pointing away released the *last* attachment to instance 1, so it was + // destroyed: lifetime is refcounted, not cached. Coming back therefore finds + // a fresh counter. Keeping it alive is what a second handler is for — see + // the re-pointing test below. + REQUIRE(settle(handler.execute(ShiRead{.id = 1})).value == 0); +} + +TEST_CASE("a keyed action re-points the handler rather than re-keying the instance", "[shared-instances]") { + morph::testing::InlineExecutor exec; + Bridge bridge{makeLocal(exec)}; + + BridgeHandler mover{bridge, &exec}; + BridgeHandler pinned{bridge, &exec}; + + settle(mover.execute(ShiAddTo{.id = 100, .amount = 50})); + settle(pinned.execute(ShiRead{.id = 100})); // pin instance 100 alive + + // The primary is deliberately not write-once: naming another key moves the + // *handler*, leaving instance 100 and its state intact for `pinned`. + settle(mover.execute(ShiAddTo{.id = 200, .amount = 1})); + REQUIRE(mover.primary().value_or(-1) == 200); + REQUIRE(settle(pinned.execute(ShiPeek{})).value == 50); +} + +TEST_CASE("an instance outlives any single handler and dies with the last one", "[shared-instances]") { + morph::testing::InlineExecutor exec; + Bridge bridge{makeLocal(exec)}; + + BridgeHandler survivor{bridge, &exec}; + settle(survivor.execute(ShiAddTo{.id = 9, .amount = 4})); + { + BridgeHandler transient{bridge, &exec}; + settle(transient.execute(ShiAddTo{.id = 9, .amount = 6})); + REQUIRE(settle(transient.execute(ShiRead{.id = 9})).value == 10); + } // transient releases one attachment — the instance must survive + + REQUIRE(settle(survivor.execute(ShiPeek{})).value == 10); + + { + // Once the last handler goes, the instance and its directory entry go + // with it, so a later attach starts from a fresh counter. + BridgeHandler lastOne{bridge, &exec}; + settle(lastOne.execute(ShiRead{.id = 9})); + } +} + +TEST_CASE("releasing every handler drops the instance and its directory entry", "[shared-instances]") { + morph::testing::InlineExecutor exec; + Bridge bridge{makeLocal(exec)}; + + { + BridgeHandler only{bridge, &exec}; + settle(only.execute(ShiAddTo{.id = 500, .amount = 77})); + REQUIRE(settle(only.instances()).size() == 1); + } + + BridgeHandler fresh{bridge, &exec}; + REQUIRE(settle(fresh.instances()).empty()); + // A fresh attach to the same key starts from zero — the old instance is gone. + REQUIRE(settle(fresh.execute(ShiRead{.id = 500})).value == 0); +} + +TEST_CASE("instances() lists the live shared keys", "[shared-instances]") { + morph::testing::InlineExecutor exec; + Bridge bridge{makeLocal(exec)}; + + BridgeHandler alpha{bridge, &exec}; + BridgeHandler beta{bridge, &exec}; + BridgeHandler hidden{bridge, &exec}; + + settle(alpha.execute(ShiAddTo{.id = 11, .amount = 1})); + settle(beta.execute(ShiAddTo{.id = 22, .amount = 1})); + settle(hidden.execute(ShiAddTo{.id = 33, .amount = 1})); // private: not listed + + auto keys = settle(alpha.instances()); + std::ranges::sort(keys); + REQUIRE(keys == std::vector{11, 22}); +} + +TEST_CASE("explicit attach binds without executing an action", "[shared-instances]") { + morph::testing::InlineExecutor exec; + Bridge bridge{makeLocal(exec)}; + + BridgeHandler handler{bridge, &exec}; + REQUIRE_FALSE(handler.primary().has_value()); + + handler.attach(64); + REQUIRE(handler.primary().value_or(-1) == 64); + // A keyless action now has an instance to run against. + REQUIRE(settle(handler.execute(ShiPeek{})).value == 0); +} + +TEST_CASE("an unattached shared handler fails a keyless action fast", "[shared-instances]") { + morph::testing::InlineExecutor exec; + Bridge bridge{makeLocal(exec)}; + + BridgeHandler handler{bridge, &exec}; + + // No primary yet: there is no instance to run against, and quietly inventing + // a private one would defeat the sharing the caller asked for. + bool failed = false; + handler.execute(ShiPeek{}).onError([&](const std::exception_ptr&) { failed = true; }); + REQUIRE(failed); +} + +TEST_CASE("sharing works identically across a remote backend", "[shared-instances]") { + morph::testing::InlineExecutor exec; + morph::exec::ThreadPoolExecutor pool{2}; + auto server = std::make_shared(pool); + + // Two independent Bridges over one server stand in for two clients: the + // directory lives server-side precisely so they meet on one instance. + Bridge clientA{std::make_unique(*server)}; + Bridge clientB{std::make_unique(*server)}; + + BridgeHandler fromA{clientA, &exec}; + BridgeHandler fromB{clientB, &exec}; + + REQUIRE(settle(fromA.execute(ShiAddTo{.id = 314, .amount = 20})).value == 20); + // The second *client* — not merely the second handler — sees the first's work. + REQUIRE(settle(fromB.execute(ShiAddTo{.id = 314, .amount = 2})).value == 22); + REQUIRE(settle(fromB.instances()) == std::vector{314}); +} + +TEST_CASE("a remote plain handler still gets its own instance", "[shared-instances]") { + morph::testing::InlineExecutor exec; + morph::exec::ThreadPoolExecutor pool{2}; + auto server = std::make_shared(pool); + Bridge bridge{std::make_unique(*server)}; + + BridgeHandler shared{bridge, &exec}; + BridgeHandler priv{bridge, &exec}; + + settle(shared.execute(ShiAddTo{.id = 8, .amount = 30})); + REQUIRE(settle(priv.execute(ShiAddTo{.id = 8, .amount = 1})).value == 1); + REQUIRE(settle(shared.execute(ShiRead{.id = 8})).value == 30); +} + +TEST_CASE("a result-sourced key promotes the instance the create ran on", "[shared-instances]") { + morph::testing::InlineExecutor exec; + Bridge bridge{makeLocal(exec)}; + + BridgeHandler creator{bridge, &exec}; + auto created = settle(creator.execute(ShiCreate{.initial = 5})); + + // The handler adopts the generated key before any user callback sees the + // result, so a .then() could immediately act on the new instance. + REQUIRE(created.id != 0); + REQUIRE(creator.primary().value_or(-1) == created.id); + + // Crucially the *same* instance was filed under that key rather than a fresh + // one being created for it: everything the create did is still there. This + // is the whole reason promotion exists instead of re-pointing. + REQUIRE(settle(creator.execute(ShiPeek{})).value == 5); + + // …and it is now reachable by key like any other shared instance. + BridgeHandler latecomer{bridge, &exec}; + latecomer.attach(created.id); + REQUIRE(settle(latecomer.execute(ShiPeek{})).value == 5); + REQUIRE(settle(latecomer.instances()).size() == 1); +} + +TEST_CASE("a result-sourced key promotes across a remote backend", "[shared-instances]") { + morph::testing::InlineExecutor exec; + morph::exec::ThreadPoolExecutor pool{2}; + auto server = std::make_shared(pool); + Bridge bridge{std::make_unique(*server)}; + + BridgeHandler creator{bridge, &exec}; + auto created = settle(creator.execute(ShiCreate{.initial = 11})); + REQUIRE(creator.primary().value_or(-1) == created.id); + REQUIRE(settle(creator.execute(ShiPeek{})).value == 11); +} + +TEST_CASE("promoting onto a key another instance holds leaves the incumbent alone", "[shared-instances]") { + morph::testing::InlineExecutor exec; + Bridge bridge{makeLocal(exec)}; + + // Hold key 4242 with a real instance carrying state. + BridgeHandler incumbent{bridge, &exec}; + settle(incumbent.execute(ShiAddTo{.id = 4242, .amount = 99})); + + // A creating action whose generated key collides with it must not displace + // the incumbent: the existing holder always wins, so no handler silently + // ends up pointing at a different instance than the one it created. + BridgeHandler collider{bridge, &exec}; + settle(collider.execute(ShiCreateAs{.wantId = 4242, .initial = 7})); + + REQUIRE(settle(incumbent.execute(ShiPeek{})).value == 99); + REQUIRE(settle(incumbent.instances()) == std::vector{4242}); +} + +TEST_CASE("attaching to the key a handler already holds is a no-op", "[shared-instances]") { + morph::testing::InlineExecutor exec; + Bridge bridge{makeLocal(exec)}; + + BridgeHandler handler{bridge, &exec}; + settle(handler.execute(ShiAddTo{.id = 55, .amount = 8})); + + // Re-attaching to the same key must not release and re-acquire the + // instance, which would silently discard its state. + handler.attach(55); + handler.attach(55); + REQUIRE(settle(handler.execute(ShiPeek{})).value == 8); +} + +TEST_CASE("instances() is empty before anything is attached", "[shared-instances]") { + morph::testing::InlineExecutor exec; + Bridge bridge{makeLocal(exec)}; + BridgeHandler handler{bridge, &exec}; + REQUIRE(settle(handler.instances()).empty()); +} + +TEST_CASE("primary keys round-trip through their canonical encoding", "[shared-instances]") { + REQUIRE(morph::model::keyToString(-17) == "-17"); + REQUIRE(morph::model::keyFromString("-17") == -17); + REQUIRE(morph::model::keyToString("abc") == "abc"); + REQUIRE(morph::model::keyFromString("abc") == "abc"); + // A malformed key is a protocol error, never a value to clamp: decoding it + // to 0 would silently route the caller to the wrong instance. + REQUIRE_THROWS(morph::model::keyFromString("12x")); + REQUIRE_THROWS(morph::model::keyFromString("")); +} + +TEST_CASE("keyed models and keyed actions are detected structurally", "[shared-instances]") { + STATIC_REQUIRE(morph::model::KeyedModel); + STATIC_REQUIRE_FALSE(morph::model::KeyedModel); + STATIC_REQUIRE(morph::model::detail::PayloadKeyed); + STATIC_REQUIRE_FALSE(morph::model::detail::PayloadKeyed); + STATIC_REQUIRE_FALSE(morph::model::ActionKeyTraits::hasKey); +} diff --git a/tests/test_subscription.cpp b/tests/test_subscription.cpp index 577738a..12455fc 100644 --- a/tests/test_subscription.cpp +++ b/tests/test_subscription.cpp @@ -1,573 +1,312 @@ // SPDX-License-Identifier: Apache-2.0 +// +// Tests for instance subscriptions (F3). +// +// `subscribe(cb)` is keyed on the **result/state type** and fires whenever an +// `R` is produced on the instance the handler is attached to — by this handler, +// by another handler sharing the instance, or by another screen entirely. It +// replaces the reactive-draft mechanism (`set<&A::field>`, `reset`, and an +// action-keyed `subscribe`), whose job a stateful model does better by holding +// the draft itself; see docs/planned/instance_subscriptions.md. +// +// The subscriber names *what it renders*, not what somebody else must call to +// produce it, so adding an action that also yields an `R` never breaks an +// existing subscriber. +#include +#include +#include +#include #include #include -#include #include #include -#include -#include -#include -#include #include #include -#include -#include #include "test_support.hpp" +// ── Fixture: a stateful counter, so a subscription reports real shared state ── -// ── Test fixture: a model with two action types ───────────────────────────── -// -// FormAction takes five doubles; the validator requires all three of {a, b, c} -// to be non-zero before the action may fire. d and e are optional inputs. -struct FormAction { - double a = 0.0; - double b = 0.0; - double c = 0.0; - double d = 0.0; - double e = 0.0; -}; - -// SimpleAction has no validator override — default morph::model::ActionValidator returns true, -// so it should fire on the first set<>. -struct SimpleAction { - int x = 0; +/// The state type subscribers name. Produced by more than one action, which is +// Model, action and result types need **external** linkage: glaze's +// plain-aggregate reflection cannot see into an anonymous namespace, and the +// BRIDGE_REGISTER_* macros specialise templates at global scope. +// NOLINTBEGIN(misc-use-internal-linkage) +/// exactly the case result-keyed subscription exists to serve. +struct SubCounterState { + std::int64_t value = 0; }; -struct ThrowAction { - int trigger = 0; +/// A second, unrelated state type — used to prove types do not cross-talk. +struct SubLabelState { + std::string text; }; -// FlakyAction throws only when `mode == 0`, otherwise returns mode * 2. Used to test -// re-fire after error: first fire throws, second fire (after setting mode=1) succeeds. -struct FlakyAction { - int mode = -1; +struct SubBump { + std::int64_t id = 0; + std::int64_t by = 0; }; -// A bundle of non-numeric field types to verify set<> with strings and nested structs. -struct Inner { - int n = 0; -}; -struct MixedAction { - std::string name; - Inner inner; - int count = 0; +struct SubRead { + std::int64_t id = 0; }; -// A slow action whose `execute` sleeps long enough that bursting set<>() during -// the in-flight call exercises the coalescing path explicitly. -struct SlowAction { - int seq = 0; +struct SubLabel { + std::int64_t id = 0; }; -struct FormModel { - double execute(FormAction action) { return action.a + action.b + action.c + action.d + action.e; } - int execute(SimpleAction action) { return action.x * 10; } - int execute(ThrowAction /*unused*/) { throw std::runtime_error("boom"); } - int execute(FlakyAction action) { - if (action.mode == 0) { - throw std::runtime_error("flaky"); - } - return action.mode * 2; - } - std::string execute(const MixedAction& action) { - return action.name + ":" + std::to_string(action.inner.n) + ":" + std::to_string(action.count); - } - int execute(SlowAction action) { - std::this_thread::sleep_for(std::chrono::milliseconds{40}); - return action.seq; - } +struct SubExplode { + std::int64_t id = 0; }; -BRIDGE_REGISTER_MODEL(FormModel, "Test_FormModel") -BRIDGE_REGISTER_ACTION(FormModel, FormAction, "Test_FormAction") -BRIDGE_REGISTER_ACTION(FormModel, SimpleAction, "Test_SimpleAction") -BRIDGE_REGISTER_ACTION(FormModel, ThrowAction, "Test_ThrowAction") -BRIDGE_REGISTER_ACTION(FormModel, FlakyAction, "Test_FlakyAction") -BRIDGE_REGISTER_ACTION(FormModel, MixedAction, "Test_MixedAction") -BRIDGE_REGISTER_ACTION(FormModel, SlowAction, "Test_SlowAction") - -BRIDGE_REGISTER_VALIDATOR(FormAction, [](const FormAction& a) { - return a.a != 0.0 && a.b != 0.0 && a.c != 0.0; -}) -BRIDGE_REGISTER_VALIDATOR(ThrowAction, [](const ThrowAction& a) { return a.trigger != 0; }) -BRIDGE_REGISTER_VALIDATOR(FlakyAction, [](const FlakyAction& a) { return a.mode >= 0; }) -BRIDGE_REGISTER_VALIDATOR(MixedAction, [](const MixedAction& a) { - return !a.name.empty() && a.inner.n != 0 && a.count != 0; -}) - -using SyncExecutor = morph::testing::InlineExecutor; +struct SubCounterModel { + using PrimaryKey = std::int64_t; -namespace { + std::int64_t value = 0; -template -void waitFor(Pred pred, std::chrono::milliseconds budget = std::chrono::milliseconds{2000}) { - auto deadline = std::chrono::steady_clock::now() + budget; - while (!pred() && std::chrono::steady_clock::now() < deadline) { - std::this_thread::sleep_for(std::chrono::milliseconds{2}); + SubCounterState execute(const SubBump& act) { + value += act.by; + return {.value = value}; } -} + [[nodiscard]] SubCounterState execute(const SubRead& /*act*/) const { return {.value = value}; } + [[nodiscard]] static SubLabelState execute(const SubLabel& /*act*/) { return {.text = "label"}; } + static SubCounterState execute(const SubExplode& /*act*/) { throw std::runtime_error{"boom"}; } +}; -} // namespace +BRIDGE_REGISTER_MODEL(SubCounterModel, "SUB_CounterModel") +BRIDGE_REGISTER_ACTION(SubCounterModel, SubBump, "SUB_Bump") +BRIDGE_REGISTER_ACTION(SubCounterModel, SubRead, "SUB_Read") +BRIDGE_REGISTER_ACTION(SubCounterModel, SubLabel, "SUB_Label") +BRIDGE_REGISTER_ACTION(SubCounterModel, SubExplode, "SUB_Explode") -TEST_CASE("Subscription: default validator fires on first set", "[bridge][subscription]") { - morph::exec::ThreadPoolExecutor pool{2}; - SyncExecutor cbExec; - morph::bridge::Bridge bridge{std::make_unique(pool)}; - morph::bridge::BridgeHandler handler{bridge, &cbExec}; +BRIDGE_KEY_FROM(SubBump, &SubBump::id); +BRIDGE_KEY_FROM(SubRead, &SubRead::id); +BRIDGE_KEY_FROM(SubLabel, &SubLabel::id); +BRIDGE_KEY_FROM(SubExplode, &SubExplode::id); +// NOLINTEND(misc-use-internal-linkage) - std::atomic seen{-1}; - handler.subscribe([&](int result) { seen.store(result); }); +namespace { - handler.set<&SimpleAction::x>(7); +using morph::bridge::AllowShared; +using morph::bridge::Bridge; +using morph::bridge::BridgeHandler; - waitFor([&] { return seen.load() != -1; }); - REQUIRE(seen.load() == 70); +/// Runs a completion to resolution, ignoring its value. +template +void drain(morph::async::Completion comp) { + auto done = std::make_shared>(false); + std::move(comp).then([done](const T&) { done->store(true); }).onError([done](const std::exception_ptr&) { + done->store(true); + }); + REQUIRE(morph::testing::waitUntil([&] { return done->load(); })); } -TEST_CASE("Subscription: custom validator gates fire until ready", "[bridge][subscription]") { - morph::exec::ThreadPoolExecutor pool{2}; - SyncExecutor cbExec; - morph::bridge::Bridge bridge{std::make_unique(pool)}; - morph::bridge::BridgeHandler handler{bridge, &cbExec}; - - std::atomic calls{0}; - std::atomic last{0.0}; - handler.subscribe([&](double sum) { - last.store(sum); - calls.fetch_add(1); - }); +std::unique_ptr makeLocal(morph::exec::IExecutor& pool) { + return std::make_unique(pool); +} - // a alone is not ready → no fire - handler.set<&FormAction::a>(1.0); - std::this_thread::sleep_for(std::chrono::milliseconds{30}); - REQUIRE(calls.load() == 0); +} // namespace - // a + b still not enough → no fire - handler.set<&FormAction::b>(2.0); - std::this_thread::sleep_for(std::chrono::milliseconds{30}); - REQUIRE(calls.load() == 0); +TEST_CASE("a subscriber hears results produced by its own handler", "[bridge][subscription]") { + morph::testing::InlineExecutor exec; + Bridge bridge{makeLocal(exec)}; + BridgeHandler handler{bridge, &exec}; - // a + b + c → validator passes, action fires - handler.set<&FormAction::c>(4.0); + std::int64_t seen = 0; + int fires = 0; + handler.subscribe([&](SubCounterState state) { + seen = state.value; + ++fires; + }); - waitFor([&] { return calls.load() >= 1; }); - REQUIRE(calls.load() == 1); - REQUIRE(last.load() == 7.0); + drain(handler.execute(SubBump{.id = 1, .by = 7})); + REQUIRE(fires == 1); + REQUIRE(seen == 7); } -TEST_CASE("Subscription: re-fires on subsequent sets after ready", "[bridge][subscription]") { - morph::exec::ThreadPoolExecutor pool{2}; - SyncExecutor cbExec; - morph::bridge::Bridge bridge{std::make_unique(pool)}; - morph::bridge::BridgeHandler handler{bridge, &cbExec}; - - std::mutex resultsMtx; - std::vector results; - handler.subscribe([&](double sum) { - std::scoped_lock lock{resultsMtx}; - results.push_back(sum); - }); - - // Cross readiness on the third set - handler.set<&FormAction::a>(1.0); - handler.set<&FormAction::b>(1.0); - handler.set<&FormAction::c>(1.0); // first fire = 3.0 - waitFor([&] { - std::scoped_lock lock{resultsMtx}; - return !results.empty(); - }); +TEST_CASE("a subscriber hears another handler's work on the shared instance", "[bridge][subscription]") { + morph::testing::InlineExecutor exec; + Bridge bridge{makeLocal(exec)}; - // Now the draft already passes validation; every subsequent set re-fires - // once the in-flight call settles. Push more values and observe results - // converge to the latest snapshot. - handler.set<&FormAction::d>(10.0); - handler.set<&FormAction::e>(100.0); + BridgeHandler watcher{bridge, &exec}; + BridgeHandler actor{bridge, &exec}; - waitFor([&] { - std::scoped_lock lock{resultsMtx}; - return !results.empty() && results.back() == 113.0; - }); + watcher.attach(10); + std::int64_t seen = -1; + watcher.subscribe([&](SubCounterState state) { seen = state.value; }); - std::scoped_lock lock{resultsMtx}; - REQUIRE(!results.empty()); - REQUIRE(results.front() == 3.0); - REQUIRE(results.back() == 113.0); - // Coalescing means we expect fewer fires than sets: at most one per - // "round", not one per set. - REQUIRE(results.size() <= 3); + // A different handler, on the same instance: the watcher does not need to + // know that SubBump exists, only that SubCounterState is what it renders. + drain(actor.execute(SubBump{.id = 10, .by = 3})); + REQUIRE(seen == 3); } -TEST_CASE("Subscription: error sink receives model exceptions", "[bridge][subscription]") { - morph::exec::ThreadPoolExecutor pool{2}; - SyncExecutor cbExec; - morph::bridge::Bridge bridge{std::make_unique(pool)}; - morph::bridge::BridgeHandler handler{bridge, &cbExec}; - - std::atomic errFired{false}; - handler.subscribe( - [](int /*unused*/) {}, - [&](const std::exception_ptr& err) { - try { - std::rethrow_exception(err); - } catch (const std::runtime_error&) { - errFired.store(true); - } - }); - - handler.set<&ThrowAction::trigger>(1); - - waitFor([&] { return errFired.load(); }); - REQUIRE(errFired.load()); -} +TEST_CASE("a subscriber hears nothing from a different instance", "[bridge][subscription]") { + morph::testing::InlineExecutor exec; + Bridge bridge{makeLocal(exec)}; -TEST_CASE("Subscription: unsubscribe stops further results", "[bridge][subscription]") { - morph::exec::ThreadPoolExecutor pool{2}; - SyncExecutor cbExec; - morph::bridge::Bridge bridge{std::make_unique(pool)}; - morph::bridge::BridgeHandler handler{bridge, &cbExec}; + BridgeHandler watcher{bridge, &exec}; + BridgeHandler elsewhere{bridge, &exec}; - std::atomic calls{0}; - handler.subscribe([&](int /*unused*/) { calls.fetch_add(1); }); + watcher.attach(20); + bool fired = false; + watcher.subscribe([&](SubCounterState) { fired = true; }); - handler.set<&SimpleAction::x>(1); - waitFor([&] { return calls.load() >= 1; }); - REQUIRE(calls.load() == 1); + drain(elsewhere.execute(SubBump{.id = 21, .by = 1})); + REQUIRE_FALSE(fired); +} - handler.unsubscribe(); +TEST_CASE("a subscription follows its handler when it re-points", "[bridge][subscription]") { + morph::testing::InlineExecutor exec; + Bridge bridge{makeLocal(exec)}; - handler.set<&SimpleAction::x>(2); - // give the worker a chance to fire (action still executes; the result is - // dropped because no sink). The count should stay at 1. - std::this_thread::sleep_for(std::chrono::milliseconds{50}); - REQUIRE(calls.load() == 1); -} + BridgeHandler watcher{bridge, &exec}; + BridgeHandler actor{bridge, &exec}; -TEST_CASE("Subscription: distinct action types do not interfere", "[bridge][subscription]") { - morph::exec::ThreadPoolExecutor pool{2}; - SyncExecutor cbExec; - morph::bridge::Bridge bridge{std::make_unique(pool)}; - morph::bridge::BridgeHandler handler{bridge, &cbExec}; - - std::atomic simple{-1}; - std::atomic form{-1.0}; - handler.subscribe([&](int result) { simple.store(result); }); - handler.subscribe([&](double sum) { form.store(sum); }); - - handler.set<&SimpleAction::x>(3); - handler.set<&FormAction::a>(1.0); - handler.set<&FormAction::b>(2.0); - handler.set<&FormAction::c>(3.0); // FormAction now ready - - waitFor([&] { return simple.load() != -1 && form.load() != -1.0; }); - REQUIRE(simple.load() == 30); - REQUIRE(form.load() == 6.0); -} + watcher.attach(30); + std::int64_t seen = -1; + watcher.subscribe([&](SubCounterState state) { seen = state.value; }); -TEST_CASE("Subscription: reset clears the draft", "[bridge][subscription]") { - morph::exec::ThreadPoolExecutor pool{2}; - SyncExecutor cbExec; - morph::bridge::Bridge bridge{std::make_unique(pool)}; - morph::bridge::BridgeHandler handler{bridge, &cbExec}; - - std::atomic last{-1.0}; - handler.subscribe([&](double sum) { last.store(sum); }); - - handler.set<&FormAction::a>(1.0); - handler.set<&FormAction::b>(2.0); - handler.set<&FormAction::c>(3.0); - waitFor([&] { return last.load() == 6.0; }); - - handler.reset(); - - // After reset, just setting `a` alone shouldn't fire — validator needs all of a/b/c. - last.store(-1.0); - handler.set<&FormAction::a>(5.0); - std::this_thread::sleep_for(std::chrono::milliseconds{50}); - REQUIRE(last.load() == -1.0); - - // Re-fill and confirm the draft genuinely restarted from defaults. - handler.set<&FormAction::b>(5.0); - handler.set<&FormAction::c>(5.0); - waitFor([&] { return last.load() != -1.0; }); - REQUIRE(last.load() == 15.0); -} + // "Tell me about the account I am looking at" must keep working when the + // user switches accounts, so the subscription moves with the handler. + watcher.attach(31); + drain(actor.execute(SubBump{.id = 31, .by = 5})); + REQUIRE(seen == 5); -TEST_CASE("Subscription: result is dropped silently with no sink installed", "[bridge][subscription]") { - morph::exec::ThreadPoolExecutor pool{2}; - SyncExecutor cbExec; - morph::bridge::Bridge bridge{std::make_unique(pool)}; - morph::bridge::BridgeHandler handler{bridge, &cbExec}; - - // No subscribe() — set<> still triggers execute, but result drops. - handler.set<&SimpleAction::x>(42); - std::this_thread::sleep_for(std::chrono::milliseconds{50}); - // No assertion required; the test passes if nothing crashes and no orphan - // error is logged (because there is no exception, just a discarded result). - SUCCEED("no-sink fire completed without UAF or hang"); + seen = -1; + drain(actor.execute(SubBump{.id = 30, .by = 9})); + REQUIRE(seen == -1); // the instance it left behind is no longer its business } -TEST_CASE("Subscription: callbacks no-op after handler destruction", "[bridge][subscription]") { - morph::exec::ThreadPoolExecutor pool{2}; - SyncExecutor cbExec; - morph::bridge::Bridge bridge{std::make_unique(pool)}; +TEST_CASE("distinct result types do not interfere", "[bridge][subscription]") { + morph::testing::InlineExecutor exec; + Bridge bridge{makeLocal(exec)}; + BridgeHandler handler{bridge, &exec}; + handler.attach(40); - std::atomic calls{0}; - { - morph::bridge::BridgeHandler handler{bridge, &cbExec}; - handler.subscribe([&](int /*unused*/) { calls.fetch_add(1); }); - handler.set<&SimpleAction::x>(1); - waitFor([&] { return calls.load() >= 1; }); - } - // Handler has been destroyed. Give any racing completion callbacks a window - // to fire — they should now see a null weak_ptr and no-op. - std::this_thread::sleep_for(std::chrono::milliseconds{50}); - REQUIRE(calls.load() == 1); -} + int counters = 0; + int labels = 0; + handler.subscribe([&](SubCounterState) { ++counters; }); + handler.subscribe([&](const SubLabelState&) { ++labels; }); -TEST_CASE("Subscription: second subscribe replaces the first", "[bridge][subscription]") { - morph::exec::ThreadPoolExecutor pool{2}; - SyncExecutor cbExec; - morph::bridge::Bridge bridge{std::make_unique(pool)}; - morph::bridge::BridgeHandler handler{bridge, &cbExec}; - - std::atomic firstCalls{0}; - std::atomic secondCalls{0}; - handler.subscribe([&](int /*unused*/) { firstCalls.fetch_add(1); }); - handler.subscribe([&](int /*unused*/) { secondCalls.fetch_add(1); }); - - handler.set<&SimpleAction::x>(1); - waitFor([&] { return secondCalls.load() >= 1; }); - // Only the latest subscriber sees results. - REQUIRE(secondCalls.load() == 1); - REQUIRE(firstCalls.load() == 0); -} + drain(handler.execute(SubBump{.id = 40, .by = 1})); + REQUIRE(counters == 1); + REQUIRE(labels == 0); -TEST_CASE("Subscription: set before subscribe drops the result", "[bridge][subscription]") { - morph::exec::ThreadPoolExecutor pool{2}; - SyncExecutor cbExec; - morph::bridge::Bridge bridge{std::make_unique(pool)}; - morph::bridge::BridgeHandler handler{bridge, &cbExec}; - - // Fire before any subscriber is installed — result is computed and dropped. - handler.set<&SimpleAction::x>(1); - std::this_thread::sleep_for(std::chrono::milliseconds{30}); - - // Now install a subscriber. It must not see the prior dropped result. - std::atomic calls{0}; - std::atomic last{-1}; - handler.subscribe([&](int val) { - last.store(val); - calls.fetch_add(1); - }); - std::this_thread::sleep_for(std::chrono::milliseconds{30}); - REQUIRE(calls.load() == 0); - - // The next set still fires from the preserved draft (x already 1 from earlier), - // but at this point the value is whatever the next set lands. - handler.set<&SimpleAction::x>(5); - waitFor([&] { return calls.load() >= 1; }); - REQUIRE(calls.load() == 1); - REQUIRE(last.load() == 50); + drain(handler.execute(SubLabel{.id = 40})); + REQUIRE(counters == 1); + REQUIRE(labels == 1); } -TEST_CASE("Subscription: works with string and nested-struct fields", "[bridge][subscription]") { - morph::exec::ThreadPoolExecutor pool{2}; - SyncExecutor cbExec; - morph::bridge::Bridge bridge{std::make_unique(pool)}; - morph::bridge::BridgeHandler handler{bridge, &cbExec}; - - std::atomic fired{false}; - std::string seen; - std::mutex seenMtx; - handler.subscribe([&](std::string result) { - std::scoped_lock lock{seenMtx}; - seen = std::move(result); - fired.store(true); - }); +TEST_CASE("every action producing the type notifies the subscriber", "[bridge][subscription]") { + morph::testing::InlineExecutor exec; + Bridge bridge{makeLocal(exec)}; + BridgeHandler handler{bridge, &exec}; + handler.attach(50); - handler.set<&MixedAction::name>(std::string{"alpha"}); - handler.set<&MixedAction::inner>(Inner{42}); - handler.set<&MixedAction::count>(7); + int fires = 0; + handler.subscribe([&](SubCounterState) { ++fires; }); - waitFor([&] { return fired.load(); }); - std::scoped_lock lock{seenMtx}; - REQUIRE(seen == "alpha:42:7"); + drain(handler.execute(SubBump{.id = 50, .by = 1})); + drain(handler.execute(SubRead{.id = 50})); // a different action, same state type + REQUIRE(fires == 2); } -TEST_CASE("Subscription: unsubscribe preserves the draft", "[bridge][subscription]") { - morph::exec::ThreadPoolExecutor pool{2}; - SyncExecutor cbExec; - morph::bridge::Bridge bridge{std::make_unique(pool)}; - morph::bridge::BridgeHandler handler{bridge, &cbExec}; - - std::atomic last{-1.0}; - handler.subscribe([&](double sum) { last.store(sum); }); - - handler.set<&FormAction::a>(1.0); - handler.set<&FormAction::b>(2.0); - handler.set<&FormAction::c>(3.0); - waitFor([&] { return last.load() == 6.0; }); - - handler.unsubscribe(); - // The draft is still intact. Mutate one more field; the action will fire - // again (no subscriber → result drops) but the draft state persists. - handler.set<&FormAction::d>(10.0); - std::this_thread::sleep_for(std::chrono::milliseconds{30}); - - // Re-subscribe; the next set must fire against the *preserved* draft state - // (a/b/c/d already populated), producing a + b + c + d + e = 16 + e. - last.store(-1.0); - handler.subscribe([&](double sum) { last.store(sum); }); - handler.set<&FormAction::e>(100.0); - waitFor([&] { return last.load() != -1.0; }); - REQUIRE(last.load() == 116.0); -} +TEST_CASE("subscribing again replaces the previous callback", "[bridge][subscription]") { + morph::testing::InlineExecutor exec; + Bridge bridge{makeLocal(exec)}; + BridgeHandler handler{bridge, &exec}; + handler.attach(60); -TEST_CASE("Subscription: works under morph::backend::SimulatedRemoteBackend", "[bridge][subscription][remote]") { - morph::exec::ThreadPoolExecutor serverPool{2}; - auto server = std::make_shared(serverPool); - SyncExecutor cbExec; - morph::bridge::Bridge bridge{std::make_unique(*server)}; - morph::bridge::BridgeHandler handler{bridge, &cbExec}; + int first = 0; + int second = 0; + handler.subscribe([&](SubCounterState) { ++first; }); + handler.subscribe([&](SubCounterState) { ++second; }); - std::atomic last{-1.0}; - handler.subscribe([&](double sum) { last.store(sum); }); + drain(handler.execute(SubBump{.id = 60, .by = 1})); + REQUIRE(first == 0); + REQUIRE(second == 1); +} - handler.set<&FormAction::a>(2.0); - handler.set<&FormAction::b>(3.0); - handler.set<&FormAction::c>(5.0); +TEST_CASE("unsubscribe stops further delivery", "[bridge][subscription]") { + morph::testing::InlineExecutor exec; + Bridge bridge{makeLocal(exec)}; + BridgeHandler handler{bridge, &exec}; + handler.attach(70); - waitFor([&] { return last.load() == 10.0; }, std::chrono::milliseconds{4000}); - REQUIRE(last.load() == 10.0); -} + int fires = 0; + handler.subscribe([&](SubCounterState) { ++fires; }); + drain(handler.execute(SubBump{.id = 70, .by = 1})); + REQUIRE(fires == 1); -TEST_CASE("Subscription: draft survives switchBackend", "[bridge][subscription][switch]") { - morph::exec::ThreadPoolExecutor pool1{2}; - morph::exec::ThreadPoolExecutor pool2{2}; - SyncExecutor cbExec; - morph::bridge::Bridge bridge{std::make_unique(pool1)}; - morph::bridge::BridgeHandler handler{bridge, &cbExec}; - - std::atomic last{-1.0}; - handler.subscribe([&](double sum) { last.store(sum); }); - - // Build a partial draft — validator does not pass yet, so nothing fires. - handler.set<&FormAction::a>(1.0); - handler.set<&FormAction::b>(2.0); - std::this_thread::sleep_for(std::chrono::milliseconds{30}); - REQUIRE(last.load() == -1.0); - - // Switch backends mid-edit. The draft lives in the handler, so it survives. - bridge.switchBackend(std::make_unique(pool2)); - - // Complete the draft. The fire must reach the new backend. - handler.set<&FormAction::c>(3.0); - waitFor([&] { return last.load() != -1.0; }); - REQUIRE(last.load() == 6.0); + handler.unsubscribe(); + drain(handler.execute(SubBump{.id = 70, .by = 1})); + REQUIRE(fires == 1); } -TEST_CASE("Subscription: re-fires after a failed execute", "[bridge][subscription]") { - morph::exec::ThreadPoolExecutor pool{2}; - SyncExecutor cbExec; - morph::bridge::Bridge bridge{std::make_unique(pool)}; - morph::bridge::BridgeHandler handler{bridge, &cbExec}; - - std::atomic okFired{0}; - std::atomic errFired{0}; - std::atomic lastOk{-1}; - handler.subscribe( - [&](int val) { - lastOk.store(val); - okFired.fetch_add(1); - }, - [&](const std::exception_ptr& /*unused*/) { errFired.fetch_add(1); }); - - // mode=0 → execute throws → errFired increments, running flag must reset. - handler.set<&FlakyAction::mode>(0); - waitFor([&] { return errFired.load() >= 1; }); - REQUIRE(errFired.load() == 1); - REQUIRE(okFired.load() == 0); - - // mode=5 → execute succeeds → okFired increments. If the previous error - // path failed to clear `running`, this set would silently be queued as - // `pending` and never actually run. - handler.set<&FlakyAction::mode>(5); - waitFor([&] { return okFired.load() >= 1; }); - REQUIRE(okFired.load() == 1); - REQUIRE(lastOk.load() == 10); - REQUIRE(errFired.load() == 1); -} +TEST_CASE("a failed action notifies nobody", "[bridge][subscription]") { + morph::testing::InlineExecutor exec; + Bridge bridge{makeLocal(exec)}; + BridgeHandler handler{bridge, &exec}; + handler.attach(80); -TEST_CASE("Subscription: unhandled errors route to the framework logger", "[bridge][subscription]") { - morph::exec::ThreadPoolExecutor pool{2}; - SyncExecutor cbExec; - morph::bridge::Bridge bridge{std::make_unique(pool)}; - morph::bridge::BridgeHandler handler{bridge, &cbExec}; - - // Save and restore the global logger around the test so we don't pollute - // other tests with our spy. - auto savedLevel = morph::log::getLogLevel(); - morph::log::setLogLevel(morph::log::LogLevel::debug); - std::mutex captureMtx; - std::string captured; - morph::log::setLogger([&](morph::log::LogLevel /*lvl*/, std::string_view msg) { - std::scoped_lock lock{captureMtx}; - captured.append(msg).append("\n"); - }); + bool fired = false; + handler.subscribe([&](SubCounterState) { fired = true; }); - // Subscribe with success-only (no errCb). The action throws — without our - // fallback, the error would be silently swallowed because the framework's - // internal .onError attachment suppresses CompletionState's orphan log. - handler.subscribe([](int /*unused*/) {}); - handler.set<&ThrowAction::trigger>(1); + drain(handler.execute(SubExplode{.id = 80})); + REQUIRE_FALSE(fired); +} - waitFor([&] { - std::scoped_lock lock{captureMtx}; - return captured.contains("[subscription:"); - }); +TEST_CASE("delivery stops once the subscribing handler is destroyed", "[bridge][subscription]") { + morph::testing::InlineExecutor exec; + Bridge bridge{makeLocal(exec)}; + BridgeHandler actor{bridge, &exec}; + actor.attach(90); + int fires = 0; { - std::scoped_lock lock{captureMtx}; - REQUIRE(captured.contains("[subscription:Test_ThrowAction]")); - REQUIRE(captured.contains("boom")); + BridgeHandler watcher{bridge, &exec}; + watcher.attach(90); + watcher.subscribe([&](SubCounterState) { ++fires; }); + drain(actor.execute(SubBump{.id = 90, .by = 1})); + REQUIRE(fires == 1); } - morph::log::setLogger(nullptr); - morph::log::setLogLevel(savedLevel); + drain(actor.execute(SubBump{.id = 90, .by = 1})); + REQUIRE(fires == 1); } -TEST_CASE("Subscription: bursts coalesce while a fire is in flight", "[bridge][subscription]") { - morph::exec::ThreadPoolExecutor pool{2}; - SyncExecutor cbExec; - morph::bridge::Bridge bridge{std::make_unique(pool)}; - morph::bridge::BridgeHandler handler{bridge, &cbExec}; - - std::atomic fires{0}; - std::atomic lastSeen{-1}; - handler.subscribe([&](int val) { - lastSeen.store(val); - fires.fetch_add(1); - }); +TEST_CASE("a private handler's results stay private", "[bridge][subscription]") { + morph::testing::InlineExecutor exec; + Bridge bridge{makeLocal(exec)}; - // First set kicks off a fire that will sleep ~40ms inside execute. - handler.set<&SlowAction::seq>(1); + BridgeHandler watcher{bridge, &exec}; + BridgeHandler priv{bridge, &exec}; - // Burst many sets while that fire is in flight. With one-running + - // one-pending coalescing, the in-flight call completes, then a single - // re-fire runs with the latest snapshot (seq=10). - for (int idx = 2; idx <= 10; ++idx) { - handler.set<&SlowAction::seq>(idx); - } + watcher.attach(100); + bool fired = false; + watcher.subscribe([&](SubCounterState) { fired = true; }); + + // The plain handler has its own instance, so nothing it does is on the + // instance the watcher is attached to. + drain(priv.execute(SubBump{.id = 100, .by = 1})); + REQUIRE_FALSE(fired); +} + +TEST_CASE("instance subscriptions work under SimulatedRemoteBackend", "[bridge][subscription][remote]") { + morph::testing::InlineExecutor exec; + morph::exec::ThreadPoolExecutor pool{2}; + auto server = std::make_shared(pool); + Bridge bridge{std::make_unique(*server)}; + + BridgeHandler watcher{bridge, &exec}; + BridgeHandler actor{bridge, &exec}; + watcher.attach(110); - waitFor([&] { return lastSeen.load() == 10; }, std::chrono::milliseconds{4000}); + auto seen = std::make_shared>(-1); + watcher.subscribe([seen](SubCounterState state) { seen->store(state.value); }); - REQUIRE(lastSeen.load() == 10); - // 10 set<>() calls; without coalescing we'd see up to 10 fires. The - // invariant is "strictly fewer fires than sets, and the last fire used - // the latest snapshot". In practice this is typically 2 (the first + - // one coalesced re-fire), but we test the invariant, not the exact count. - REQUIRE(fires.load() >= 1); - REQUIRE(fires.load() < 10); + drain(actor.execute(SubBump{.id = 110, .by = 4})); + REQUIRE(morph::testing::waitUntil([&] { return seen->load() == 4; })); }