diff --git a/CMakeLists.txt b/CMakeLists.txt index 113c325d2..d29fb27a1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -601,14 +601,19 @@ if(MORPH_BUILD_FORMS_QML) # not only under morph::qt: forms_controller_core.hpp includes both, and # morph::qt (MORPH_BUILD_QT, which needs Qt WebSockets) is not part of # every install that ships this header. Neither needs anything beyond - # QtCore. + # QtCore. multi_model_bridge_core.hpp/multi_model_forms_controller_core.hpp + # and the owned_local_bridge.hpp detail header they (and the single-model + # pair) share are the same story. target_sources(morph_qt_forms INTERFACE FILE_SET HEADERS BASE_DIRS include FILES + include/morph/qt/bridge/detail/owned_local_bridge.hpp include/morph/qt/bridge/generic_model_bridge_core.hpp + include/morph/qt/bridge/multi_model_bridge_core.hpp include/morph/qt/forms/forms_controller_core.hpp + include/morph/qt/forms/multi_model_forms_controller_core.hpp include/morph/qt/qt_executor.hpp ) endif() diff --git a/docs/spec/core/bridge.md b/docs/spec/core/bridge.md index 3b4439410..675f482b5 100644 --- a/docs/spec/core/bridge.md +++ b/docs/spec/core/bridge.md @@ -1100,6 +1100,7 @@ make teardown order-independent.) | `instance` | `static ActionExecuteRegistry& instance()` | Process-level singleton. | | `registerAction` | `template void registerAction(string_view modelId, string_view actionId)` | Registers an executor that deserializes JSON → `ActionTraits::fromJson`, calls `BridgeHandler::execute<>`, serializes result back. Files **two** entries — one per sharing tag (`NoSharing`, `AllowShared`) — from one generic-lambda template. Defined out-of-line after `BridgeHandler`. | | `execute` | `template Completion execute(string_view modelId, string_view actionId, void* handler, string_view bodyJson) const` | Lookup + invoke, under the caller's own sharing policy. Key is `(modelId, actionId, typeid(Sharing))`. Throws `runtime_error` on an unknown key. | +| `contains` | `template bool contains(string_view modelId, string_view actionId) const noexcept` | Existence check over the same key `execute` looks up, without invoking anything. Since `registerAction` always files both sharing tags together, this answers the same for either `Sharing` for any action registered via `BRIDGE_REGISTER_ACTION`. Backs `BridgeHandler::servesAction`. | ### `Bridge` diff --git a/docs/spec/forms/forms.md b/docs/spec/forms/forms.md index c04e53975..3b9331672 100644 --- a/docs/spec/forms/forms.md +++ b/docs/spec/forms/forms.md @@ -1422,6 +1422,29 @@ renderer for it, Qt/QML, as a reusable component rather than example code. home: its own `LabFormsDemo` QML module carries only `Main.qml` and the `FormsController` subclass naming `lab::LabModel`; `Main.qml` imports `MorphForms` for `DynamicForm`/`I18nCatalog` like any other consumer would. +- **`include/morph/qt/forms/multi_model_forms_controller_core.hpp`** (same + component, same install story) ships + `morph::qt::forms::MultiModelFormsControllerCore`, the + multi-model sibling of `FormsControllerCore` for a rung + whose forms span more than one registered model + (`bookmarks::gui::FormsBridge`, whose forms serve `AuthModel`, + `BookmarkModel` and `TagModel`, is the shipped example). Same + `schemasJson()`/`submitIfValid()`/`fetchOptions()` surface, over + `morph::qt::bridge::MultiModelBridgeCore` + (`include/morph/qt/bridge/multi_model_bridge_core.hpp`) instead of + `GenericModelBridgeCore`: it composes one + `GenericModelBridgeCore` per `Model` in the pack and routes a + submitted action-type id to whichever one serves it, via + `GenericModelBridgeCore::servesAction` (which forwards to + `BridgeHandler::servesAction`) — a pure existence check over + `ActionExecuteRegistry`, the same registry `executeJson` dispatches + through — rather than a hand-written `actionType -> Model` table. Models + are tried in the order the pack declares them; an action id registered on + more than one `Model` in the pack is a configuration bug asserted in debug + builds, not a case routed silently. An unrouted action type resolves + `onError` directly with `"no model in this client serves action ''"`, + the same wording every hand-written router before it agreed on + independently. This is packaging and factoring only: no `x-*` key changed, and a plain single-action form renders identically to before the renderer was extracted. @@ -1435,9 +1458,9 @@ not one, because only one of the two signals is universal: - **`optionsReceived(optionsAction, ok, payload)` is optional.** It exists only on a controller that serves a `Choice` field; a controller that serves none deliberately declares neither it nor `fetchOptions()` - (`bookmarks::gui::BookmarkFormsController` and - `pastebin::gui::FormsBridge` each carry the reasoning: an unused - `fetchOptions()` would be a stub with nothing to call it). Its block gates its + (`bookmarks::gui::FormsBridge` and `pastebin::gui::FormsBridge` each carry + the reasoning: an unused `fetchOptions()` would be a stub with nothing to + call it). Its block gates its **target** on the signal being declared — `form.controller.optionsReceived !== undefined`, else `null` — so a controller that omits it is never connected to and the absence is not a warning. Without the split, every form instance diff --git a/examples/bookmarks/README.md b/examples/bookmarks/README.md index 7250bad5c..78a66f735 100644 --- a/examples/bookmarks/README.md +++ b/examples/bookmarks/README.md @@ -452,15 +452,17 @@ submit button either, and every form is bound to the live controller. The one non-form input on the whole screen is the per-row selection checkbox, which types nothing. -Two pieces of glue carry their own written justification, per rule 2's "(b) +`gui_lib/bookmark_qml_bridges.hpp`'s `FormsBridge` composes +`morph::qt::forms::MultiModelFormsControllerCore` directly, over the `Bridge&`/`IExecutor*` +`AppContext::onReady()` hands it — no rung-owned routing controller sits +between them; the shipped core routes each action-type string to whichever of +the three form-serving models owns it, via a plain existence check over +`ActionExecuteRegistry` rather than a hand-written table. + +One piece of glue carries its own written justification, per rule 2's "(b) pure glue with no domain logic" clause: -- `gui::BookmarkFormsController` — composed over an injected - `Bridge&`/`IExecutor*`, like `morph::qt::forms::FormsControllerCore`'s own - composing constructor, plus the one genuinely new part this rung's own - controller owns — routing an action-type string to whichever of the three - form-serving models owns it, which the shipped core (templated over a - single model) has no equivalent for. - `gui::FormsBridge::onLoginSucceeded` — installs the token the server returned as the shared `Bridge`'s default session, so every subsequent action carries it. Infrastructure wiring, not business logic: it decides diff --git a/examples/bookmarks/gui_lib/bookmark_forms_controller.cpp b/examples/bookmarks/gui_lib/bookmark_forms_controller.cpp deleted file mode 100644 index 6f03288b6..000000000 --- a/examples/bookmarks/gui_lib/bookmark_forms_controller.cpp +++ /dev/null @@ -1,37 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -#include "bookmark_forms_controller.hpp" - -#include -#include - -// submitIfValid() is a template (OnReply/OnError deduced per call site, -// exactly like FormsControllerCore's own) and so stays fully defined in the -// header; this translation unit holds the two things that need exactly one -// non-inline definition — the constructor and the action-type routing table. - -namespace bookmarks::gui { - -BookmarkFormsController::BookmarkFormsController(::morph::bridge::Bridge& bridge, ::morph::exec::IExecutor* executor, - std::string schemasJson) - : _authHandler{bridge, executor}, - _bookmarkHandler{bridge, executor}, - _tagHandler{bridge, executor}, - _schemasJson{std::move(schemasJson)} {} - -::morph::async::Completion BookmarkFormsController::dispatch(const std::string& actionType, - const std::string& bodyJson) { - if (actionType == "Login") { - return _authHandler.executeJson(actionType, bodyJson); - } - if (actionType == "CreateBookmark" || actionType == "EditBookmark" || actionType == "ImportBookmarks") { - return _bookmarkHandler.executeJson(actionType, bodyJson); - } - if (actionType == "RenameTag" || actionType == "MergeTags") { - return _tagHandler.executeJson(actionType, bodyJson); - } - // Reported, never silently dropped: the QML side names action types as - // strings, so a typo has to arrive somewhere a human can read it. - throw std::runtime_error{"no model in this client serves action '" + actionType + "'"}; -} - -} // namespace bookmarks::gui diff --git a/examples/bookmarks/gui_lib/bookmark_forms_controller.hpp b/examples/bookmarks/gui_lib/bookmark_forms_controller.hpp deleted file mode 100644 index a27d05243..000000000 --- a/examples/bookmarks/gui_lib/bookmark_forms_controller.hpp +++ /dev/null @@ -1,143 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -#pragma once - -#include -#include -#include -#include -#include -#include - -#include "bookmarks/models/auth_model.hpp" -#include "bookmarks/models/bookmark_model.hpp" -#include "bookmarks/models/tag_model.hpp" - -namespace bookmarks::gui { - -/// @brief Same schema-driven surface as the shipped -/// `morph::qt::forms::FormsControllerCore` -/// (`schemasJson()`/`submitIfValid()`), composed over an injected -/// `Bridge&`/`IExecutor*` instead of constructing its own -/// `LocalBackend`. This rung still owns a thin controller of its own -/// rather than using the shipped core directly: the core is templated -/// over a *single* model, and this rung's forms span three -/// (`AuthModel`/`BookmarkModel`/`TagModel`, see "The one thing that -/// is genuinely new here" below) — `dispatch()`'s routing has no -/// equivalent on the shipped core. Pure glue, no domain logic -/// (`examples/IMPLEMENTATION.md` rule 2 justification (b)) — the -/// schema/validation/rendering machinery is untouched; only the -/// backend-wiring seam differs. -/// -/// @par The one thing that is genuinely new here: routing -/// The shipped core is a template over a *single* model. This rung's forms -/// span three -/// (`Login` on `AuthModel`, `CreateBookmark`/`EditBookmark`/`ImportBookmarks` -/// on `BookmarkModel`, `RenameTag`/`MergeTags` on `TagModel`), and -/// `BridgeHandler::executeJson` dispatches against the model type it -/// is instantiated for — so something has to map an action-type string to the -/// right handler. `dispatch()` below is that map and nothing else: a -/// six-entry lookup with no conditionals about *what* an action means. An -/// unrouted action type is reported through the caller's own error callback -/// rather than thrown, so a typo in QML surfaces as a message in the status -/// line like every other failure. -/// -/// @par Handler lifetime, and why all three are constructed together -/// All three `BridgeHandler`s are members, so they are constructed together -/// (three registrations, no deregistrations) and destroyed together at -/// shutdown. That is deliberate: `QtWebSocketBackend::deregisterModel` now -/// assigns its fire-and-forget `deregister` envelope a real, tracked callId -/// rather than the `callId == 0` sentinel a subsequent synchronous -/// register/attach/assign call also used to use — closing a race that used -/// to be able to corrupt a freshly constructed handler's binding if it was -/// built on the same connection right after an older one was torn down. This -/// rung's handler-lifetime shape (all three built together, never rebuilt -/// mid-session) predates that fix and was never the shape the race needed -/// anyway: nothing in this rung's client destroys one handler and -/// constructs a different one on the same connection — the whole handler -/// set outlives login, and login only installs a session on the shared -/// `Bridge`. -/// -/// @par No `fetchOptions()` -/// Deliberately absent: it exists on the shipped core to serve a -/// `morph::forms::Choice` field's combo-box options, and none of this -/// rung's DTOs declare a `Choice` field — `CreateBookmark::visibility` is a -/// plain reflected enum, not a server-fetched choice. Adding an unused -/// `fetchOptions()` would be a stub with nothing to call it. -/// -/// @par Array-typed members -/// `CreateBookmark::tags`/`EditBookmark::tags` are `std::vector` -/// and reach `DynamicForm` as `{"type":"array","items":{"type":"string"}}`. -/// The shipped renderer gives that shape a dedicated comma-separated control -/// (`docs/spec/forms/forms.md`, "Array fields") and encodes it as a genuine -/// JSON array, so tagging from the create and edit forms works with no -/// special-casing here: `"work, home"` submits as `["work","home"]`. -/// Array-of-string is the fully supported case, which is the only array shape -/// the forms this controller serves declare — `BulkEdit`'s array of -/// `BookmarkId` is a different shape, and `bookmark_schemas.hpp` explains why -/// it is not one of them. -class BookmarkFormsController { -public: - /// @param bridge The shared `Bridge` `AppContext` owns. - /// @param executor The executor `Completion` callbacks land on. - /// @param schemasJson Pre-assembled `{actionType: schemaJson()}` map, - /// matching `FormsControllerCore`'s own constructor contract — - /// `bookmark_schemas.hpp`'s `bookmarkSchemasJson()` builds the one - /// every shell passes. - BookmarkFormsController(::morph::bridge::Bridge& bridge, ::morph::exec::IExecutor* executor, - std::string schemasJson); - - /// @brief The `{actionType: schema}` JSON supplied at construction. - /// @return A reference to the cached schema-set JSON. - [[nodiscard]] const std::string& schemasJson() const noexcept { return _schemasJson; } - - /// @brief Dispatches @p bodyJson as @p actionType's body via the generic - /// `executeJson` path on whichever model serves @p actionType, - /// invoking @p onReply / @p onError on the GUI thread once the - /// reply arrives. - /// - /// Same body as `FormsControllerCore::submitIfValid` - /// (`include/morph/qt/forms/forms_controller_core.hpp`), with the single - /// handler replaced by `dispatch()`'s routing and a `try`/`catch` around - /// it — `dispatch()` is the only step that can fail synchronously (an - /// unrouted or unregistered action type), and this turns that into the - /// same asynchronous failure shape every other error takes. The - /// `dispatch()` call is sequenced before either lambda is constructed, so - /// @p onError is still intact in the handler. - /// - /// @tparam OnReply Callable invoked with the result JSON (`std::string`) on success. - /// @tparam OnError Callable invoked with the `std::exception_ptr` on failure. - /// @param actionType Registered action type id. - /// @param bodyJson Fully-assembled JSON body for the action. - /// @param onReply Success callback. - /// @param onError Failure callback. - template - void submitIfValid(const std::string& actionType, const std::string& bodyJson, OnReply onReply, OnError onError) { - try { - dispatch(actionType, bodyJson) - .then( - [onReply = std::move(onReply)](std::string resultJson) mutable { onReply(std::move(resultJson)); }) - .onError([onError](const std::exception_ptr& err) mutable { onError(err); }); - } catch (...) { - onError(std::current_exception()); - } - } - -private: - /// @brief Routes @p actionType to the handler for the model that serves - /// it and starts the dispatch. - /// @param actionType Registered action type id. - /// @param bodyJson Fully-assembled JSON body for the action. - /// @return The in-flight completion carrying the result JSON. - /// @throws std::runtime_error if no model in this controller serves - /// @p actionType (or if the action is unknown to the one that - /// does — `BridgeHandler::executeJson`'s own contract). - [[nodiscard]] ::morph::async::Completion dispatch(const std::string& actionType, - const std::string& bodyJson); - - ::morph::bridge::BridgeHandler _authHandler; - ::morph::bridge::BridgeHandler _bookmarkHandler; - ::morph::bridge::BridgeHandler _tagHandler; - std::string _schemasJson; -}; - -} // namespace bookmarks::gui diff --git a/examples/bookmarks/gui_lib/bookmark_qml_bridges.cpp b/examples/bookmarks/gui_lib/bookmark_qml_bridges.cpp index 5c3046b37..daa79ca87 100644 --- a/examples/bookmarks/gui_lib/bookmark_qml_bridges.cpp +++ b/examples/bookmarks/gui_lib/bookmark_qml_bridges.cpp @@ -130,9 +130,9 @@ std::optional decodeLoginResult(const std::string& resultJson) { // ── FormsBridge ───────────────────────────────────────────────────────────── FormsBridge::FormsBridge(::morph::bridge::Bridge& bridge, ::morph::exec::IExecutor* executor, QObject* parent) - : QObject{parent}, _bridge{bridge}, _controller{bridge, executor, bookmarkSchemasJson()} {} + : QObject{parent}, _bridge{bridge}, _core{bridge, executor, bookmarkSchemasJson()} {} -QString FormsBridge::schemasJson() const { return QString::fromStdString(_controller.schemasJson()); } +QString FormsBridge::schemasJson() const { return QString::fromStdString(_core.schemasJson()); } void FormsBridge::onLoginSucceeded(const LoginResult& result) { ::morph::session::Context session; @@ -148,12 +148,13 @@ void FormsBridge::submitIfValid(const QString& actionType, const QString& bodyJs // doc comment for the full argument. `_callbacks.guard(...)` is the // general-purpose gate (`CallbackScope`'s `guard()`, not `Completion`'s // `then(scope, fn)` overload) because the `Completion` these end up on is - // created and attached *inside* `BookmarkFormsController::submitIfValid`, - // one frame further in; what this function hands over is a pair of plain - // callables. Wrapping them here keeps the controller a - // callback-shape-agnostic seam and puts the gate in the class that owns the - // captured `this`, which is where it belongs. - _controller.submitIfValid( + // created and attached *inside* + // `MultiModelFormsControllerCore::submitIfValid`, one frame further in; + // what this function hands over is a pair of plain callables. Wrapping + // them here keeps the controller a callback-shape-agnostic seam and puts + // the gate in the class that owns the captured `this`, which is where it + // belongs. + _core.submitIfValid( actionType.toStdString(), bodyJson.toStdString(), _callbacks.guard([this, actionType](std::string resultJson) { // A successful Login is the one reply this client reads rather // than merely displays: the token has to be installed before diff --git a/examples/bookmarks/gui_lib/bookmark_qml_bridges.hpp b/examples/bookmarks/gui_lib/bookmark_qml_bridges.hpp index 83866e2ee..eec9a3dd9 100644 --- a/examples/bookmarks/gui_lib/bookmark_qml_bridges.hpp +++ b/examples/bookmarks/gui_lib/bookmark_qml_bridges.hpp @@ -19,9 +19,12 @@ #include #include #include +#include -#include "bookmark_forms_controller.hpp" #include "bookmark_presenter.hpp" +#include "bookmarks/models/auth_model.hpp" +#include "bookmarks/models/bookmark_model.hpp" +#include "bookmarks/models/tag_model.hpp" #include "shared_feed_presenter.hpp" #include "tag_presenter.hpp" #endif @@ -44,7 +47,7 @@ /// its own, and that is a deliberate deviation from this task's brief. A /// standalone `AuthBridge` taking `(Bridge&, IExecutor*)` — the presenter /// rule-2 constructor every adapter here has — would have to own a second -/// `BookmarkFormsController`, and therefore a second `BridgeHandler` for +/// `MultiModelFormsControllerCore`, and therefore a second `BridgeHandler` for /// *each* of this rung's three form-serving models: six registered instances /// per client where four is the number `bookmarks::app::App`'s own /// `kMaxLiveModels` comment budgets for. The alternative (handing one @@ -82,24 +85,35 @@ namespace bookmarks::gui { [[nodiscard]] std::optional decodeLoginResult(const std::string& resultJson); #endif -/// @brief QML-facing face of `bookmarks::gui::BookmarkFormsController`, plus -/// this client's one session-installing seam. +/// @brief QML-facing face of +/// `morph::qt::forms::MultiModelFormsControllerCore`, plus this client's one session-installing +/// seam. /// /// Same surface `DynamicForm.qml` expects of a controller — a `schemasJson` /// property, `submitIfValid(actionType, bodyJson)`, and a `replyReceived` /// signal — so the shipped renderer needs no bookmarks-specific knowledge, -/// and one instance serves the login screen and every domain form alike. +/// and one instance serves the login screen and every domain form alike. The +/// composed core routes each action type to whichever of the three models +/// serves it (`docs/spec/forms/forms.md`'s `MultiModelFormsControllerCore` +/// entry); this class adds nothing to that routing, only the one +/// session-installing step `onLoginSucceeded` below performs on a successful +/// `Login`. /// /// @par Why this class holds a `CallbackScope` -/// `submitIfValid` hands the wrapped controller two callbacks that capture +/// `submitIfValid` hands the wrapped core two callbacks that capture /// `this`; the success arm also reaches `_bridge` through `onLoginSucceeded`. -/// They are attached to a `Completion`, which **always** resolves through the -/// executor — never inline, even in `Local` mode -/// (`docs/spec/core/completion.md`) — so an in-flight reply outlives its +/// For a *routed* action type they are attached to a `Completion`, which +/// **always** resolves through the executor — never inline, even in `Local` +/// mode (`docs/spec/core/completion.md`) — so an in-flight reply outlives its /// dispatch call by construction, and this object can be destroyed before the /// reply lands. Both shells own their bridges by `unique_ptr` in `main()` and /// destroy them when the process tears down (`gui/main.cpp`, -/// `gui_wasm/main_wasm.cpp`), which is exactly such a window. +/// `gui_wasm/main_wasm.cpp`), which is exactly such a window. An *unrouted* +/// action type is the one exception: the composed core's `onError` runs +/// synchronously, on `submitIfValid`'s own call frame, since no model ever +/// took the dispatch — still through the same guarded callback, so this is a +/// difference in timing, not in which callback runs or how it is guarded. /// /// The rung's three neighbours are already covered and neither mechanism /// reaches here: `BookmarkPresenter`/`TagPresenter`/`SharedFeedPresenter` @@ -120,6 +134,15 @@ namespace bookmarks::gui { /// is worth: this is a by-construction hazard closed pre-emptively, not a /// crash that was observed here. Nothing in this rung's suite reproduced a /// use-after-free through `FormsBridge`. +/// +/// @par No `fetchOptions()`/`optionsReceived` +/// Deliberately absent, even though the composed +/// `MultiModelFormsControllerCore` itself provides `fetchOptions()`: it +/// exists there to serve a `morph::forms::Choice` field's combo-box +/// options, and none of this rung's DTOs (`bookmarks/dto/*.hpp`) declare a +/// `Choice` field — `CreateBookmark::visibility` is a plain reflected enum, +/// not a server-fetched choice. Adding an unused `Q_INVOKABLE fetchOptions()` +/// here would be a stub with nothing to call it. class FormsBridge : public QObject { Q_OBJECT @@ -175,15 +198,16 @@ class FormsBridge : public QObject { void onLoginSucceeded(const LoginResult& result); ::morph::bridge::Bridge& _bridge; - BookmarkFormsController _controller; + ::morph::qt::forms::MultiModelFormsControllerCore<::morph::bridge::NoSharing, AuthModel, BookmarkModel, TagModel> + _core; /// @brief Lifetime gate for the `this`-capturing reply callbacks /// `submitIfValid` attaches — see this class's own doc comment. /// /// **Declared last on purpose**, and it must stay last: reverse-order - /// member destruction is what makes the gate close before `_controller` - /// (and the three `BridgeHandler`s inside it) is torn down. Anything added - /// to this class goes *above* this line. + /// member destruction is what makes the gate close before `_core` (and + /// the three `BridgeHandler`s inside it) is torn down. Anything added to + /// this class goes *above* this line. /// /// `requestStop()`/`reset()` are deliberately not called anywhere: this /// bridge has no "user navigated away" or "supersede the previous query" diff --git a/examples/bookmarks/gui_lib/bookmark_schemas.hpp b/examples/bookmarks/gui_lib/bookmark_schemas.hpp index 77d67394b..8a35f8d61 100644 --- a/examples/bookmarks/gui_lib/bookmark_schemas.hpp +++ b/examples/bookmarks/gui_lib/bookmark_schemas.hpp @@ -11,13 +11,14 @@ /// @file /// The one schema document every bookmarks form renders from, assembled in -/// one place so every shell that builds a `BookmarkFormsController` — the +/// one place so every shell that builds a `bookmarks::gui::FormsBridge` — the /// desktop client (`gui/main.cpp`), a future WASM client, and the tests — /// builds the *identical* map instead of each assembling its own /// (`examples/TESTING.md`'s "same client code" requirement). Same split -/// `pastebin::gui::pasteSchemasJson()` uses, and for the same reason: -/// `BookmarkFormsController` takes the document as a constructor argument by -/// design, so whatever composes it decides which actions it serves. +/// `pastebin::gui::pasteSchemasJson()` uses, and for the same reason: the +/// composed `morph::qt::forms::MultiModelFormsControllerCore` takes the +/// document as a constructor argument by design, so whatever composes it +/// decides which actions it serves. namespace bookmarks::gui { diff --git a/examples/bookmarks/gui_wasm/main_wasm.cpp b/examples/bookmarks/gui_wasm/main_wasm.cpp index dbf3171c2..817f29938 100644 --- a/examples/bookmarks/gui_wasm/main_wasm.cpp +++ b/examples/bookmarks/gui_wasm/main_wasm.cpp @@ -7,8 +7,9 @@ /// This file is the *only* difference between the browser client and the /// desktop client (`gui/main.cpp`). Everything with behaviour in it — the /// presenters (`gui_lib/bookmark_presenter.hpp`, `gui_lib/tag_presenter.hpp`, -/// `gui_lib/shared_feed_presenter.hpp`), the forms controller -/// (`gui_lib/bookmark_forms_controller.hpp`), the QML adapters +/// `gui_lib/shared_feed_presenter.hpp`), the forms-controller composition +/// (`gui_lib/bookmark_qml_bridges.hpp`'s `FormsBridge`, over the shipped +/// `morph::qt::forms::MultiModelFormsControllerCore`), the QML adapters /// (`gui_lib/bookmark_qml_bridges.hpp`), the schema document /// (`gui_lib/bookmark_schemas.hpp`) and the QML itself (`gui/qml/Main.qml`, /// built into the `Bookmarks` module both binaries link) — is shared diff --git a/examples/bookmarks/tests/test_bookmark_qml_bridges.cpp b/examples/bookmarks/tests/test_bookmark_qml_bridges.cpp index 36c33f993..935f317b8 100644 --- a/examples/bookmarks/tests/test_bookmark_qml_bridges.cpp +++ b/examples/bookmarks/tests/test_bookmark_qml_bridges.cpp @@ -2,9 +2,10 @@ // // The QML-adapter layer's own suite: `FormsBridge`, `BookmarkBridge`, // `TagBridge` and `SharedFeedBridge` (`gui_lib/bookmark_qml_bridges.hpp`) plus -// the action-type routing in `BookmarkFormsController::dispatch` -// (`gui_lib/bookmark_forms_controller.cpp`) — everything that stands between -// the Task 17 presenters and the QML shell. +// the action-type routing `FormsBridge` gets from the composed +// `morph::qt::forms::MultiModelFormsControllerCore` +// (`include/morph/qt/forms/multi_model_forms_controller_core.hpp`) — +// everything that stands between the Task 17 presenters and the QML shell. // // Why this file exists as a *separate* suite from test_bookmark_presenter.cpp: // those adapters are the only place in the rung where a `BookmarkView` becomes @@ -681,14 +682,16 @@ TEST_CASE("BookmarkBridge::bulkArchive maps true to BulkArchiveOp::Archive and f } // ═════════════════════════════════════════════════════════════════════════ -// BookmarkFormsController::dispatch — the six-entry routing table +// MultiModelFormsControllerCore's routing — every action reaches its model // ═════════════════════════════════════════════════════════════════════════ -TEST_CASE("BookmarkFormsController::dispatch routes every one of the six form actions to the model that serves it", +TEST_CASE("FormsBridge routes every one of the six form actions to the model that serves it", "[bookmarks][gui][qml-bridges]") { - // `dispatch()` maps an action-type *string* to one of three - // `BridgeHandler`s. A typo, or a new action added to bookmark_schemas.hpp - // and forgotten here, is not a compile error: the form renders, the button + // The composed MultiModelFormsControllerCore routes an action-type *string* to whichever of + // the three models serves it. A typo, or a new action added to + // bookmark_schemas.hpp and forgotten in one of the three model + // registrations, is not a compile error: the form renders, the button // submits, and the reply is an error message. This case submits all six // ids exactly as the QML string literals spell them. DbFixture fixture; @@ -760,12 +763,12 @@ TEST_CASE("BookmarkFormsController::dispatch routes every one of the six form ac CHECK(tagIdNamed(after, QStringLiteral("home")) == -1); } -TEST_CASE("BookmarkFormsController::dispatch reports an unrouted action type instead of dropping it", - "[bookmarks][gui][qml-bridges]") { - // The exact failure mode the routing table risks: a QML string literal - // that no `if` in `dispatch()` matches. It must surface as a message in - // the status line (BookmarkListView.qml:193 renders `actionType + ": " + - // payload` on `!ok`), never as a submit that silently does nothing. +TEST_CASE("FormsBridge reports an unrouted action type instead of dropping it", "[bookmarks][gui][qml-bridges]") { + // The exact failure mode routing risks: a QML string literal none of the + // three composed models' servesAction() recognises. It must surface as a + // message in the status line (BookmarkListView.qml:193 renders + // `actionType + ": " + payload` on `!ok`), never as a submit that + // silently does nothing. DbFixture fixture; auto rig = makeAuthedRig("alice"); bookmarks::gui::FormsBridge forms{rig->bridge(0), rig->executor()}; diff --git a/examples/kanban/gui_lib/project_admin_presenter.cpp b/examples/kanban/gui_lib/project_admin_presenter.cpp index 1fd5eeac1..b5180c053 100644 --- a/examples/kanban/gui_lib/project_admin_presenter.cpp +++ b/examples/kanban/gui_lib/project_admin_presenter.cpp @@ -38,11 +38,15 @@ void ProjectAdminPresenter::login(const QString& username) { void ProjectAdminPresenter::submitForm(const QString& actionType, const QString& bodyJson) { // `executeJson` is the type-erased counterpart of the typed `execute` // calls below: the schema renderer only ever knows an action by the string - // the schema names it with. This is the actionType->handler table - // `bookmarks::gui::BookmarkFormsController::dispatch` already has: two - // models behind one controller, and the unroutable case reports rather - // than silently dropping — QML names types as strings, so a typo has to - // arrive somewhere a human reads it. + // the schema names it with. This is the same actionType->model shape + // `morph::qt::forms::MultiModelFormsControllerCore` routes generically for + // a rung whose routing has no side effects of its own + // (`bookmarks::gui::FormsBridge` is the shipped example) -- hand-written + // here instead because every branch below also does something the generic + // core cannot: re-decoding the body for a typed signal, installing the + // session, redacting a token. The unroutable case reports rather than + // silently dropping — QML names types as strings, so a typo has to arrive + // somewhere a human reads it. const std::string type = actionType.toStdString(); if (type == "CreateProject") { // Decoded here, not merely relayed, so this path emits the *same* diff --git a/examples/kanban/tests/test_gui_forms_render.cpp b/examples/kanban/tests/test_gui_forms_render.cpp index c6661cfb6..9ca15ed98 100644 --- a/examples/kanban/tests/test_gui_forms_render.cpp +++ b/examples/kanban/tests/test_gui_forms_render.cpp @@ -186,8 +186,8 @@ void pressSubmit(QObject* form) { /// `DynamicForm.qml` declares `onOptionsReceived` in a `Connections` block /// whose `target` is the controller, unconditionally. A controller that serves /// no `morph::forms::Choice` field has no such signal — and deliberately so: -/// `bookmarks::gui::BookmarkFormsController`'s own "No `fetchOptions()`" note -/// records that adding one with nothing to call it would be a stub. So the +/// `bookmarks::gui::FormsBridge`'s own "No `fetchOptions()`/`optionsReceived`" +/// note records that adding one with nothing to call it would be a stub. So the /// engine warns once per form, for every conforming controller in the ladder, /// the moment a *real* controller is attached. The rule-6 smoke test never sees /// it because it attaches none. It is filed against the renderer; tolerated diff --git a/examples/polls/README.md b/examples/polls/README.md index 6b873969a..9fa898018 100644 --- a/examples/polls/README.md +++ b/examples/polls/README.md @@ -530,8 +530,9 @@ Known gaps: now-correct generic path is work nobody has done, not work anybody is blocked on. Both that class's doc comment and `poll_schemas.hpp`'s already say so. -- **`PollFormsController` cannot be a verbatim copy of - `bookmarks::gui::BookmarkFormsController`'s per-model-handler shape.** +- **`PollFormsController` cannot be built over + `morph::qt::forms::MultiModelFormsControllerCore`'s per-model-handler shape** + (the core `bookmarks::gui::FormsBridge` composes for its three models). Every one of bookmarks' three models is plain (`NoSharing`), so which handler object serves a given call never matters there. `PollModel` is `AllowShared` and keyed: an `AllowShared` handler starts unattached and diff --git a/examples/polls/gui_lib/poll_forms_controller.hpp b/examples/polls/gui_lib/poll_forms_controller.hpp index 50d43ff62..e2c6ade86 100644 --- a/examples/polls/gui_lib/poll_forms_controller.hpp +++ b/examples/polls/gui_lib/poll_forms_controller.hpp @@ -19,24 +19,27 @@ namespace polls::gui { /// @brief Owns the *one* `BridgeHandler` a vote-view /// screen dispatches every already-open-poll action through, and /// exposes both the schema-driven `submitIfValid` surface -/// `bookmarks::gui::BookmarkFormsController` established and the -/// typed convenience methods that surface cannot cover. +/// `morph::qt::forms::MultiModelFormsControllerCore` established and +/// the typed convenience methods that surface cannot cover. /// -/// @par Why this is not a verbatim copy of `BookmarkFormsController` -/// `BookmarkFormsController` owns one `BridgeHandler` *per model* (three, for -/// three models) precisely because `BookmarkModel`/`TagModel`/`AuthModel` are -/// all plain (`NoSharing`) — each handler registers its own private instance -/// eagerly at construction, so which handler object serves a given call -/// never matters. `PollModel` is different: it is `AllowShared` and keyed by -/// `pollId` (`poll_model.hpp`'s own doc comment; this rung's shared-instance -/// showcase). An `AllowShared` handler starts **unattached** and only joins -/// the poll's shared instance the first time a payload-keyed action -/// (`OpenPoll`) dispatches through *that specific handler object* — every -/// other action on the same poll must reuse that exact handler, or it hits -/// "handler not bound" (no instance to run against). A second, independently -/// constructed `BridgeHandler` — as -/// `BookmarkFormsController`'s per-model shape would produce if copied -/// verbatim — would need its *own* `OpenPoll` attach before anything routed +/// @par Why this is not built over `MultiModelFormsControllerCore` +/// `MultiModelFormsControllerCore` (the core `bookmarks::gui::FormsBridge` +/// composes) owns one `BridgeHandler` *per model* in its pack precisely +/// because a rung with several form-serving models all plain (`NoSharing`) — +/// bookmarks' `BookmarkModel`/`TagModel`/`AuthModel` are — can have each +/// handler register its own private instance eagerly at construction, so +/// which handler object serves a given call never matters. `PollModel` is +/// different: it is `AllowShared` and keyed by `pollId` (`poll_model.hpp`'s +/// own doc comment; this rung's shared-instance showcase). An `AllowShared` +/// handler starts **unattached** and only joins the poll's shared instance +/// the first time a payload-keyed action (`OpenPoll`) dispatches through +/// *that specific handler object* — every other action on the same poll must +/// reuse that exact handler, or it hits "handler not bound" (no instance to +/// run against). A second, independently constructed +/// `BridgeHandler` — as one more `Model` in a +/// `MultiModelFormsControllerCore` pack would produce, since this rung has +/// only one form-serving model and no routing to do — would need its *own* +/// `OpenPoll` attach before anything routed /// through it could work, doubling the shared instance's live attachment /// count for no benefit and, worse, silently failing every call issued /// before that second attach completed. So this class owns exactly one diff --git a/include/morph/core/bridge.hpp b/include/morph/core/bridge.hpp index bb99cff61..17aad8ba4 100644 --- a/include/morph/core/bridge.hpp +++ b/include/morph/core/bridge.hpp @@ -110,6 +110,30 @@ class ActionExecuteRegistry { return iter->second(handler, bodyJson); } + /// @brief Checks whether an executor is registered for `(modelId, actionId)`, + /// specialised for the caller's own `Sharing` policy, without invoking it. + /// + /// `registerAction` always files both the `NoSharing` and `AllowShared` + /// executors for a given `(Model, Action)` pair (see its own doc comment), + /// so for any action registered via `BRIDGE_REGISTER_ACTION` this answers + /// the same for either `Sharing` -- the template parameter exists for + /// symmetry with `execute` and to stay correct if that ever + /// changes, not because the two currently disagree. + /// @tparam Sharing `NoSharing` or `AllowShared` — the caller's own sharing policy. + /// @param modelId String id of the target model. + /// @param actionId String id of the action to check. + /// @return `true` if `execute(modelId, actionId, ...)` would find an executor. + template + [[nodiscard]] bool contains(std::string_view modelId, std::string_view actionId) const noexcept { + // Same registration-phase latch `execute` closes (see its own doc + // comment): this is a read of `_executors` too, so a registration + // racing a routing-only caller that never calls `execute` (e.g. one + // that only ever probes unrouted actions) must still be caught. + ::morph::model::detail::noteRegistryRead(this == &instance()); + return _executors.contains( + KeyView{.modelId = modelId, .actionId = actionId, .sharing = std::type_index{typeid(Sharing)}}); + } + /// @brief Returns the process-level singleton registry. /// @return Reference to the singleton `ActionExecuteRegistry`. static ActionExecuteRegistry& instance(); @@ -3313,6 +3337,21 @@ class BridgeHandler { actionType, this, bodyJson); } + /// @brief Whether `Model` has an action registered under @p actionId, without + /// invoking it. + /// + /// A pure existence check over `ActionExecuteRegistry` -- the same + /// registry `executeJson` dispatches through -- so a caller holding + /// several handlers for different models can find the one that serves a + /// string action-type id without hand-maintaining that mapping itself + /// (see `morph::qt::bridge::MultiModelBridgeCore`, the shipped example). + /// @param actionId Action type id to check. + /// @return `true` if `executeJson(actionId, ...)` would find a registered action. + [[nodiscard]] bool servesAction(std::string_view actionId) const noexcept { + return ActionExecuteRegistry::instance().contains(::morph::model::ModelTraits::typeId(), + actionId); + } + /// @brief Creates this handler's binding: deferred when shared, immediate otherwise. /// @param bridge Bridge to create the binding on. /// @return The new binding. diff --git a/include/morph/qt/bridge/detail/owned_local_bridge.hpp b/include/morph/qt/bridge/detail/owned_local_bridge.hpp new file mode 100644 index 000000000..fbb73ee11 --- /dev/null +++ b/include/morph/qt/bridge/detail/owned_local_bridge.hpp @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: Apache-2.0 + +#pragma once + +/// @file +/// Private helper shared by every bridge core that offers an "I'll build my +/// own Bridge" convenience constructor (`GenericModelBridgeCore`, +/// `MultiModelBridgeCore`) alongside the one composing over a caller-supplied +/// `Bridge`/executor. Not part of the public API: nothing outside +/// `morph::qt::bridge` names this type. + +#include +#include +#include +#include + +namespace morph::qt::bridge::detail { + +/// @brief The private pool/executor/backend bundle an owning bridge-core +/// constructor builds and owns, so it never has to duplicate this +/// shape per core. +/// +/// Declaration order matters for destruction: `bridge` must tear down before +/// `pool`/`gui`, so it is declared last. +struct OwnedLocalBridge { + /// @brief Backs `bridge`'s `LocalBackend`. + ::morph::exec::ThreadPoolExecutor pool{2}; + /// @brief Delivers `Completion` callbacks on the GUI thread. + ::morph::qt::QtExecutor gui; + /// @brief The always-local `Bridge` every composed handler registers on. + ::morph::bridge::Bridge bridge{std::make_unique<::morph::backend::LocalBackend>(pool)}; +}; + +} // namespace morph::qt::bridge::detail diff --git a/include/morph/qt/bridge/generic_model_bridge_core.hpp b/include/morph/qt/bridge/generic_model_bridge_core.hpp index c00779312..37005ebfa 100644 --- a/include/morph/qt/bridge/generic_model_bridge_core.hpp +++ b/include/morph/qt/bridge/generic_model_bridge_core.hpp @@ -34,9 +34,11 @@ #include #include #include +#include #include #include #include +#include #include namespace morph::qt::bridge { @@ -103,26 +105,23 @@ class GenericModelBridgeCore { .onError([onError = std::move(onError)](const std::exception_ptr& err) mutable { onError(err); }); } -private: - /// @brief The private pool/executor/backend bundle the schema-only - /// constructor builds and owns. Absent (`_owned` unengaged) when the - /// core instead composes over a caller-supplied `Bridge`/executor. - /// - /// Declaration order within the struct matters for destruction: `bridge` - /// must tear down before `pool`/`gui`, so it is declared last. - struct OwnedBridge { - ::morph::exec::ThreadPoolExecutor pool{2}; - ::morph::qt::QtExecutor gui; - ::morph::bridge::Bridge bridge{std::make_unique<::morph::backend::LocalBackend>(pool)}; - }; + /// @brief Whether `Model` has an action registered under @p actionId, + /// without invoking it. Forwards to `BridgeHandler::servesAction`; + /// see there for the full contract. + /// @param actionId Action type id to check. + /// @return `true` if `execute(actionId, ...)` would find a registered action. + [[nodiscard]] bool servesAction(std::string_view actionId) const noexcept { + return _handler.servesAction(actionId); + } +private: // Declaration order matters for destruction: _handler must tear down // before _owned (its bridge and executor, when this core owns them), so // _owned is declared first and _handler after it. When the caller- // supplied-Bridge constructor is used, _owned stays unengaged and // _handler instead references the caller's Bridge/executor directly -- // the caller is responsible for outliving _handler in that case. - std::optional _owned; + std::optional _owned; ::morph::bridge::BridgeHandler _handler; }; diff --git a/include/morph/qt/bridge/multi_model_bridge_core.hpp b/include/morph/qt/bridge/multi_model_bridge_core.hpp new file mode 100644 index 000000000..2f4449033 --- /dev/null +++ b/include/morph/qt/bridge/multi_model_bridge_core.hpp @@ -0,0 +1,168 @@ +// SPDX-License-Identifier: Apache-2.0 + +#pragma once + +/// @file +/// Multi-model sibling of `morph::qt::bridge::GenericModelBridgeCore`: owns +/// (or composes over) one `GenericModelBridgeCore` per `Model` +/// in the pack and routes a string action-type id to whichever one serves it, +/// so an app whose forms span more than one registered model does not have to +/// hand-write that routing table itself. See +/// `morph::qt::forms::MultiModelFormsControllerCore`, which specialises this +/// core for the shipped forms renderer exactly as `FormsControllerCore` +/// specialises `GenericModelBridgeCore`, for the reference shape. +/// +/// The routing is derived, not declared: `GenericModelBridgeCore::servesAction` +/// answers "is this action registered for this Model" directly from +/// `ActionExecuteRegistry` -- the same source of truth `BRIDGE_REGISTER_ACTION` +/// already populates -- so nothing here re-states which action belongs to +/// which model. A hand-written `actionType -> Model` table would just be a +/// second, driftable copy of that registration. +/// +/// Composes one `GenericModelBridgeCore` per `Model` rather than duplicating +/// its `executeJson`-plus-completion-wiring body: routing is the only thing +/// this class adds, and it is a thin layer over the single-model core rather +/// than a parallel reimplementation of it. + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace morph::qt::bridge { + +/// @brief Owns, or composes over, one `GenericModelBridgeCore` +/// per `Model` in the pack, and routes a string action-type id to +/// whichever one serves it. +/// +/// @tparam Sharing The handlers' shared sharing policy -- `morph::bridge::NoSharing` +/// or `morph::bridge::AllowShared`; see `morph::bridge::BridgeHandler`. +/// Named first, unlike `GenericModelBridgeCore`'s `` +/// order: a template parameter pack must be the last template +/// parameter, so with @p Model variadic, @p Sharing cannot +/// also carry a usable default -- a default there would only +/// ever apply when @p Model is empty, which the static +/// assertion below already forbids. Every instantiation must +/// name it explicitly. +/// @tparam Model Two or more registered model types (`BRIDGE_REGISTER_MODEL`), +/// in the order their cores are tried when routing an +/// action-type id. A single model has no routing to do -- +/// use `GenericModelBridgeCore` instead. +template +class MultiModelBridgeCore { + static_assert(sizeof...(Model) >= 2, + "MultiModelBridgeCore needs at least two Model types; a single model has no " + "routing to do -- use GenericModelBridgeCore instead."); + +public: + /// @brief Constructs the core with its own private, always-local `Bridge` + /// (`ThreadPoolExecutor` + `QtExecutor` + `LocalBackend`), shared + /// by every model's core. + /// + /// Use the `(Bridge&, IExecutor*)` overload instead when the app already + /// has a `Bridge` (remote/socket mode, or one shared across multiple + /// presenters) that this core should compose over rather than duplicate. + /// Each element composes over that one shared `Bridge` via its own + /// `(Bridge&, IExecutor*)` constructor -- never its owning default + /// constructor, which would otherwise give every `Model` an independent, + /// unshared `Bridge` of its own. + MultiModelBridgeCore() : _owned{std::in_place} { emplaceAll(_owned->bridge, &_owned->gui); } + + /// @brief Constructs the core over a caller-supplied `Bridge`/executor, + /// instead of building a private, always-local one. + /// + /// Every model's core registers on @p bridge exactly as the owning + /// constructor's internal ones do, so `execute` dispatches through + /// whatever backend @p bridge currently has installed -- including a + /// backend @p bridge switches to later via `Bridge::switchBackend`, since + /// every registered handler re-registers itself automatically. + /// + /// @param bridge The bridge every model's core registers on. Must + /// outlive this core. + /// @param guiExec Executor used to deliver `Completion` callbacks (e.g. a + /// `QtExecutor` for the GUI thread). Must outlive this + /// core. + MultiModelBridgeCore(::morph::bridge::Bridge& bridge, ::morph::exec::IExecutor* guiExec) { + emplaceAll(bridge, guiExec); + } + + /// @brief Routes @p bodyJson as @p actionType's body to whichever + /// `Model` in the pack serves it, invoking @p onReply / @p onError + /// on the GUI thread once the reply (or the routing failure) + /// arrives. + /// + /// Models are tried in the order the pack declares them; the first whose + /// `GenericModelBridgeCore::servesAction` recognises @p actionType + /// dispatches it, via that core's own `execute`. If none do, @p onError + /// is invoked directly (no throw, no round trip) with a + /// `std::runtime_error` naming the unrouted action, on the same + /// synchronous call frame -- exactly the shape a caller's own + /// `try`/`catch`-around-a-throwing-router used to produce, without the + /// throw. + /// + /// Action ids are expected to be unique per action struct across the + /// whole pack; if two `Model`s were ever registered under the same + /// actionType, routing would silently resolve to whichever is declared + /// first here, the same way a hand-written `actionType -> Model` table + /// would if two of its entries collided. + /// @tparam OnReply Callable invoked with the result JSON (`std::string`) on success. + /// @tparam OnError Callable invoked with the `std::exception_ptr` on failure. + /// @param actionType Registered action type id. + /// @param bodyJson Fully-assembled JSON body for the action. + /// @param onReply Success callback. + /// @param onError Failure callback. + template + void execute(const std::string& actionType, const std::string& bodyJson, OnReply onReply, OnError onError) { + const bool routed = std::apply( + [&](auto&... core) { return (tryOne(*core, actionType, bodyJson, onReply, onError) || ...); }, _cores); + if (!routed) { + onError(std::make_exception_ptr( + std::runtime_error{"no model in this client serves action '" + actionType + "'"})); + } + } + +private: + void emplaceAll(::morph::bridge::Bridge& bridge, ::morph::exec::IExecutor* guiExec) { + std::apply([&](auto&... core) { (core.emplace(bridge, guiExec), ...); }, _cores); + } + + /// @brief Dispatches through @p core if it serves @p actionType, moving + /// @p onReply / @p onError into its own completion wiring. + /// + /// A free function template over `Core` rather than a method, since it + /// touches no member of this class -- it is the one place @p onReply / + /// @p onError are actually moved, and that must happen at most once + /// across the whole pack, which the `||` fold in `execute()` guarantees + /// by short-circuiting on the first `true`. + template + static bool tryOne(Core& core, const std::string& actionType, const std::string& bodyJson, OnReply& onReply, + OnError& onError) { + if (!core.servesAction(actionType)) { + return false; + } + core.execute(actionType, bodyJson, std::move(onReply), std::move(onError)); + return true; + } + + // Declaration order matters for destruction: _cores must tear down + // before _owned (its bridge and executor, when this core owns them), so + // _owned is declared first and _cores after it -- same reasoning as + // GenericModelBridgeCore. Each element is wrapped in optional because + // GenericModelBridgeCore is neither copyable nor movable (it holds a + // BridgeHandler, whose deleted copy constructor suppresses the implicit + // move too), so it cannot be constructed as a tuple-constructor argument + // the way a movable type could; emplaceAll() constructs each one in place + // instead, right after _owned (when engaged) is available to hand it a + // Bridge&. Every element is engaged for this object's entire lifetime + // after construction -- optional is used here purely as + // deferred-construction storage, not to express an absent core. + std::optional _owned; + std::tuple>...> _cores; +}; + +} // namespace morph::qt::bridge diff --git a/include/morph/qt/forms/multi_model_forms_controller_core.hpp b/include/morph/qt/forms/multi_model_forms_controller_core.hpp new file mode 100644 index 000000000..d727fec4c --- /dev/null +++ b/include/morph/qt/forms/multi_model_forms_controller_core.hpp @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: Apache-2.0 + +#pragma once + +/// @file +/// Multi-model sibling of `morph::qt::forms::FormsControllerCore`: same +/// `schemasJson()`/`submitIfValid()`/`fetchOptions()` surface +/// `DynamicForm.qml` expects of a controller, over +/// `morph::qt::bridge::MultiModelBridgeCore` instead of +/// `morph::qt::bridge::GenericModelBridgeCore` -- for a rung whose forms span +/// more than one registered model, and whose action-type-to-model routing +/// that implies. + +#include +#include +#include + +namespace morph::qt::forms { + +/// @brief `DynamicForm.qml`-facing facade over +/// `morph::qt::bridge::MultiModelBridgeCore`, naming its one routed +/// dispatch operation `submitIfValid`/`fetchOptions` instead of +/// `execute`, and carrying the `{actionType: schema}` document +/// `DynamicForm.qml` renders from -- a forms-specific concept the +/// composed core deliberately does not know about. +/// +/// @tparam Sharing Forwarded to `MultiModelBridgeCore` unchanged. Named +/// first, for the same reason `MultiModelBridgeCore` +/// documents on its own `Sharing` parameter: it precedes a +/// variadic pack, which must be the last template parameter, +/// so it cannot also carry a usable default. +/// @tparam Model Two or more registered model types +/// (`BRIDGE_REGISTER_MODEL`) whose actions the shipped +/// `DynamicForm.qml` renders, in the order their handlers are +/// tried when routing an action-type id. +template +class MultiModelFormsControllerCore { +public: + /// @brief Constructs the core with its own private, always-local `Bridge`. + /// See `MultiModelBridgeCore`'s own constructor for the full + /// contract. + /// @param schemasJson The full schema set the QML renderer will parse. + explicit MultiModelFormsControllerCore(std::string schemasJson) : _schemasJson{std::move(schemasJson)} {} + + /// @brief Constructs the core over a caller-supplied `Bridge`/executor. + /// See `MultiModelBridgeCore`'s own constructor for the full + /// contract. + /// @param bridge The bridge every model's handler registers on. Must + /// outlive this core. + /// @param guiExec Executor used to deliver `Completion` callbacks. + /// Must outlive this core. + /// @param schemasJson The full schema set the QML renderer will parse. + MultiModelFormsControllerCore(::morph::bridge::Bridge& bridge, ::morph::exec::IExecutor* guiExec, + std::string schemasJson) + : _core{bridge, guiExec}, _schemasJson{std::move(schemasJson)} {} + + /// @brief The `{actionType: schema}` JSON supplied at construction. + /// @return A reference to the cached schema-set JSON. + [[nodiscard]] const std::string& schemasJson() const noexcept { return _schemasJson; } + + /// @brief Routes @p bodyJson as @p actionType's body to whichever `Model` + /// serves it, invoking @p onReply / @p onError on the GUI thread + /// once the reply (or the routing failure) arrives. + /// @tparam OnReply Callable invoked with the result JSON (`std::string`) on success. + /// @tparam OnError Callable invoked with the `std::exception_ptr` on failure. + /// @param actionType Registered action type id. + /// @param bodyJson Fully-assembled JSON body for the action. + /// @param onReply Success callback. + /// @param onError Failure callback. + template + void submitIfValid(const std::string& actionType, const std::string& bodyJson, OnReply onReply, OnError onError) { + _core.execute(actionType, bodyJson, std::move(onReply), std::move(onError)); + } + + /// @brief Executes @p optionsAction with @p bodyJson to fetch a `Choice` + /// field's combo-box options, via the same routed dispatch + /// `submitIfValid` uses. @p optionsAction is a parameter rather + /// than a hardcoded id, and @p bodyJson is a true pass-through (not + /// always `"{}"`), so a dependent `Choice` (`x-optionsDependsOn`) + /// can send `{parentField: value, ...}` instead of an empty body. + /// @tparam OnReply Callable invoked with the options-action result JSON on success. + /// @tparam OnError Callable invoked with the `std::exception_ptr` on failure. + /// @param optionsAction Registered action type id that serves the options. + /// @param bodyJson Fully-assembled JSON body for the options action + /// (`"{}"` for an independent `Choice`). + /// @param onReply Success callback. + /// @param onError Failure callback. + template + void fetchOptions(const std::string& optionsAction, const std::string& bodyJson, OnReply onReply, + OnError onError) { + _core.execute(optionsAction, bodyJson, std::move(onReply), std::move(onError)); + } + +private: + ::morph::qt::bridge::MultiModelBridgeCore _core; + std::string _schemasJson; +}; + +} // namespace morph::qt::forms diff --git a/scripts/branch_partial_allowlist.json b/scripts/branch_partial_allowlist.json index 03b0ad8fe..e2bbb1129 100644 --- a/scripts/branch_partial_allowlist.json +++ b/scripts/branch_partial_allowlist.json @@ -115,7 +115,7 @@ }, { "file": "include/morph/core/bridge.hpp", - "line": 2124, + "line": 2148, "source": "if (_executeDeadline.count() > 0 && _timeoutScheduler) {", "reason": "Unreachable by construction (core audit finding B6). `setExecuteDeadline` (this file) is the only writer of both `_executeDeadline` and `_timeoutScheduler`, and always creates `_timeoutScheduler` in the same call that sets `_executeDeadline` positive (`_executeDeadline = deadline; if (_executeDeadline.count() > 0 && !_timeoutScheduler) { _timeoutScheduler = std::make_shared<...>(); }`, both under `_executeDeadlineMtx`); nothing anywhere resets `_timeoutScheduler` back to null -- the class's own doc comment on `setExecuteDeadline` says so explicitly (\"setting the deadline back to 0 stops new calls from arming it but does not tear the thread down\"). So `_executeDeadline > 0 && !_timeoutScheduler` cannot happen at this line once any positive deadline has ever been set." }, diff --git a/src/qt/forms/CMakeLists.txt b/src/qt/forms/CMakeLists.txt index 8e9fd67aa..382e6d87e 100644 --- a/src/qt/forms/CMakeLists.txt +++ b/src/qt/forms/CMakeLists.txt @@ -112,4 +112,14 @@ if(MORPH_BUILD_TESTS AND NOT EMSCRIPTEN) apply_sanitizers(morph_forms_controller_core_tests ${AF_SANITIZER}) endif() add_test(NAME forms_controller_core COMMAND morph_forms_controller_core_tests) + + # Same rationale, for MultiModelBridgeCore/MultiModelFormsControllerCore. + qt_add_executable(morph_multi_model_bridge_core_tests tests/test_multi_model_bridge_core.cpp) + target_link_libraries(morph_multi_model_bridge_core_tests PRIVATE morph::qt_forms Catch2::Catch2 + morph_test_log_level) + + if(DEFINED AF_SANITIZER) + apply_sanitizers(morph_multi_model_bridge_core_tests ${AF_SANITIZER}) + endif() + add_test(NAME multi_model_bridge_core COMMAND morph_multi_model_bridge_core_tests) endif() diff --git a/src/qt/forms/tests/.clang-tidy b/src/qt/forms/tests/.clang-tidy new file mode 100644 index 000000000..ec0f195a2 --- /dev/null +++ b/src/qt/forms/tests/.clang-tidy @@ -0,0 +1,58 @@ +# Checks suppressed inside this test directory only, on the same terms as the +# framework's own tests/.clang-tidy: an entry here is a check whose finding is +# *test idiom* -- a property of Catch2, or of the BRIDGE_REGISTER_MODEL/ACTION +# macros' own requirements -- rather than a defect. One entry per check, with +# the reason it cannot fire on anything worth fixing. + +# bugprone-chained-comparison: `CHECK(a == b)` expands to +# `catchAssertionHandler.handleExpr(Catch::Decomposer() <= a == b)`, so the +# check sees `v0 <= v1 == v2` and reports a chained comparison. Nothing in the +# test source compares three things: the `<=` it objects to is Catch2's own +# capture hook, and both fixes it suggests -- parenthesise, or split with a +# logical operator -- would defeat the expression decomposition that makes a +# failing assertion print its two operands. Catch2 says so itself; the macro +# definition carries `/* NOLINT(bugprone-chained-comparison) */` on that very +# line in 3.15.3. CI pins catch2 3.4.0 -- ubuntu-24.04's package, which +# predates that comment -- which is why the finding reaches CI here and not on +# a workstation with a current Catch2. + +# misc-use-internal-linkage: every hit is a Catch2 fixture -- a model or action +# struct used only by the TEST_CASEs beside it -- and the fix-it it proposes +# does not compile. BRIDGE_REGISTER_MODEL/ACTION expand to +# `template <> struct morph::model::ModelTraits {...}`, and an explicit +# specialisation written with a qualified name must appear in a namespace +# enclosing `morph::model`; from inside an anonymous namespace clang rejects it +# with "class template specialization of 'ModelTraits' not in a namespace +# enclosing 'model'". glaze's reflection-based get_name() likewise needs these +# types to have external linkage -- same reasoning tests/.clang-tidy's own +# entry for this check gives, and the same reason this directory's own +# test_forms_controller_core.cpp already carries the idiom unflagged (its +# lines simply hadn't been part of a diff before test_multi_model_bridge_core.cpp +# introduced a second file with the same shape). + +# readability-convert-member-functions-to-static: fires on the `execute` +# methods of fixture models. A stub action handler ignores its own state, +# which is exactly what a production model never does -- the non-static +# signature is the interface `BridgeHandler::execute` dispatches +# through, not an oversight. Same reasoning, same precedent file, as the +# `misc-use-internal-linkage` entry above. +# +# Directory-scoped, and no wider, for the same reason tests/.clang-tidy and +# every examples/*/tests/.clang-tidy copy is: clang-tidy offers no finer +# granularity than a directory, so this sits here rather than at `src/qt/`, +# where it would also cover non-test sources. `InheritParentConfig` keeps +# every other check the repository-root .clang-tidy enables; exactly these +# entries are subtracted, and only here. +# +# This test directory predates the gate that used to keep the +# bugprone-chained-comparison copy in step across every Catch2 test directory +# (see tests/.clang-tidy's own note -- "the gate that checked this was +# removed"); it never received its own copy, which is why test_forms_controller_core.cpp +# carried these idioms unflagged for as long as it did. + +Checks: > + -bugprone-chained-comparison, + -misc-use-internal-linkage, + -readability-convert-member-functions-to-static + +InheritParentConfig: true diff --git a/src/qt/forms/tests/test_multi_model_bridge_core.cpp b/src/qt/forms/tests/test_multi_model_bridge_core.cpp new file mode 100644 index 000000000..bd0749b97 --- /dev/null +++ b/src/qt/forms/tests/test_multi_model_bridge_core.cpp @@ -0,0 +1,216 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// Catch2 (own main, controls QCoreApplication lifetime like +// tests/qt/test_qt_websocket.cpp) coverage of MultiModelBridgeCore and +// MultiModelFormsControllerCore: routing an action-type id to whichever of +// several registered models serves it, generically over servesAction/ +// executeJson, proving the core that examples/bookmarks' FormsBridge now +// composes directly. + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +// Deliberately at file scope, NOT inside an anonymous namespace: glaze's +// reflection forms an `extern const T external;` declaration for each +// registered type, which requires external linkage -- same constraint +// test_forms_controller_core.cpp's own file-scope models exist for. This is +// its own standalone executable (morph_multi_model_bridge_core_tests), never +// linked into the shared morph_tests binary, so there is no ODR risk from +// other translation units reusing these names. + +struct PingAction { + std::string text; +}; + +class PingModel { +public: + std::string execute(const PingAction& action) { return "ping: " + action.text; } +}; + +BRIDGE_REGISTER_MODEL(PingModel, "MultiCore_PingModel") +BRIDGE_REGISTER_ACTION(PingModel, PingAction, "MultiCore_PingAction") + +struct PongAction { + std::string text; +}; + +struct PongOptions {}; + +struct PongOptionsResult { + std::string value; +}; + +class PongModel { +public: + std::string execute(const PongAction& action) { return "pong: " + action.text; } + PongOptionsResult execute(const PongOptions& /*options*/) { return PongOptionsResult{.value = "pong-option"}; } +}; + +BRIDGE_REGISTER_MODEL(PongModel, "MultiCore_PongModel") +BRIDGE_REGISTER_ACTION(PongModel, PongAction, "MultiCore_PongAction") +BRIDGE_REGISTER_ACTION(PongModel, PongOptions, "MultiCore_PongOptions") + +// A third model, used only by the three-model routing case below, so that +// case is not indistinguishable from the two-model ones. +struct BuzzAction { + std::string text; +}; + +class BuzzModel { +public: + std::string execute(const BuzzAction& action) { return "buzz: " + action.text; } +}; + +BRIDGE_REGISTER_MODEL(BuzzModel, "MultiCore_BuzzModel") +BRIDGE_REGISTER_ACTION(BuzzModel, BuzzAction, "MultiCore_BuzzAction") + +namespace { + +void pumpUntil(const std::function& done, int maxIterations = 300) { + for (int idx = 0; idx < maxIterations && !done(); ++idx) { + QCoreApplication::processEvents(QEventLoop::AllEvents, 10); + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } +} + +/// @brief One dispatch round trip, run to completion. +struct RunResult { + std::string reply; ///< The success payload, empty on failure. + std::exception_ptr error; ///< Set on failure, null on success. + + /// @return `error`'s `what()`, or `""` if this run succeeded. + [[nodiscard]] std::string errorText() const { + if (!error) { + return {}; + } + try { + std::rethrow_exception(error); + } catch (const std::exception& e) { + return e.what(); + } + } +}; + +/// @brief Invokes @p dispatch with a matching `(onReply, onError)` pair, pumps +/// the event loop until one of them settles it, and returns the +/// outcome -- the `atomic`/`pumpUntil`/capture boilerplate every +/// case below would otherwise repeat. +/// @param dispatch Callable taking `(onReply, onError)`, e.g. +/// `[&](auto onReply, auto onError){ core.execute("Action", "{}", onReply, onError); }`. +template +[[nodiscard]] RunResult runSync(Dispatch&& dispatch) { + std::atomic done{false}; + RunResult result; + std::forward(dispatch)( + [&](std::string resultJson) { + result.reply = std::move(resultJson); + done.store(true); + }, + [&](const std::exception_ptr& err) { + result.error = err; + done.store(true); + }); + pumpUntil([&] { return done.load(); }); + return result; +} + +} // namespace + +TEST_CASE("MultiModelBridgeCore routes to whichever Model serves the action, in either order", + "[multi_model_bridge_core]") { + morph::qt::bridge::MultiModelBridgeCore core; + + CHECK(runSync([&](auto onReply, auto onError) { + core.execute("MultiCore_PongAction", R"({"text":"hi"})", onReply, onError); + }).reply == R"("pong: hi")"); + + CHECK(runSync([&](auto onReply, auto onError) { + core.execute("MultiCore_PingAction", R"({"text":"there"})", onReply, onError); + }).reply == R"("ping: there")"); +} + +TEST_CASE("MultiModelBridgeCore routes across three Models, not just the first two tried", + "[multi_model_bridge_core]") { + // Exercises every position in the pack as both the match and a + // fall-through, on this exact three-Model instantiation: routing to the + // first (Ping) never even probes the rest, routing to the last (Buzz) + // means the first two must have declined it first, and the unrouted case + // means all three did. + morph::qt::bridge::MultiModelBridgeCore core; + + CHECK(runSync([&](auto onReply, auto onError) { + core.execute("MultiCore_PingAction", R"({"text":"first"})", onReply, onError); + }).reply == R"("ping: first")"); + + CHECK(runSync([&](auto onReply, auto onError) { + core.execute("MultiCore_PongAction", R"({"text":"middle"})", onReply, onError); + }).reply == R"("pong: middle")"); + + CHECK(runSync([&](auto onReply, auto onError) { + core.execute("MultiCore_BuzzAction", R"({"text":"last"})", onReply, onError); + }).reply == R"("buzz: last")"); + + const RunResult unrouted = + runSync([&](auto onReply, auto onError) { core.execute("NoSuchAction", "{}", onReply, onError); }); + CHECK(unrouted.errorText() == "no model in this client serves action 'NoSuchAction'"); +} + +TEST_CASE("MultiModelBridgeCore reports an unrouted action type via onError, not a thrown exception", + "[multi_model_bridge_core]") { + morph::qt::bridge::MultiModelBridgeCore core; + + // Delivered synchronously, on the same call frame -- runSync's pumpUntil + // returns on its very first check, since an unrouted action never reaches + // a Completion. + const RunResult result = + runSync([&](auto onReply, auto onError) { core.execute("NoSuchAction", "{}", onReply, onError); }); + CHECK(result.error != nullptr); + CHECK(result.errorText() == "no model in this client serves action 'NoSuchAction'"); +} + +TEST_CASE("MultiModelBridgeCore composes over a caller-supplied Bridge/executor", "[multi_model_bridge_core]") { + morph::exec::ThreadPoolExecutor pool{2}; + morph::qt::QtExecutor gui; + morph::bridge::Bridge bridge{std::make_unique(pool)}; + + morph::qt::bridge::MultiModelBridgeCore core{bridge, &gui}; + + CHECK(runSync([&](auto onReply, auto onError) { + core.execute("MultiCore_PingAction", R"({"text":"composed"})", onReply, onError); + }).reply == R"("ping: composed")"); +} + +TEST_CASE("MultiModelFormsControllerCore submits and fetches options through the routed dispatch", + "[multi_model_bridge_core]") { + morph::qt::forms::MultiModelFormsControllerCore core{ + R"({"MultiCore_PingAction":{},"MultiCore_PongAction":{}})"}; + CHECK(core.schemasJson() == R"({"MultiCore_PingAction":{},"MultiCore_PongAction":{}})"); + + CHECK(runSync([&](auto onReply, auto onError) { + core.submitIfValid("MultiCore_PingAction", R"({"text":"submit"})", onReply, onError); + }).reply == R"("ping: submit")"); + + const RunResult options = runSync( + [&](auto onReply, auto onError) { core.fetchOptions("MultiCore_PongOptions", "{}", onReply, onError); }); + CHECK(options.reply.contains("pong-option")); +} + +int main(int argc, char** argv) { + const QCoreApplication app{argc, argv}; + Catch::Session session; + return morph::testkit::runSession(session, argc, argv); +} diff --git a/src/qt/forms/tests/tst_DynamicFormChoicelessController.qml b/src/qt/forms/tests/tst_DynamicFormChoicelessController.qml index f94fc8a39..34d0a017c 100644 --- a/src/qt/forms/tests/tst_DynamicFormChoicelessController.qml +++ b/src/qt/forms/tests/tst_DynamicFormChoicelessController.qml @@ -5,8 +5,8 @@ // // `optionsReceived` only exists on a controller that serves a Choice. A // controller that serves none does not declare it, and deliberately does not: -// `bookmarks::gui::BookmarkFormsController`'s own "No `fetchOptions()`" note -// records that adding one with nothing to call it would be a stub. So the +// `bookmarks::gui::FormsBridge`'s own "No `fetchOptions()`/`optionsReceived`" +// note records that adding one with nothing to call it would be a stub. So the // sanctioned shape used to warn once per form instance, the moment a real // controller was attached: // diff --git a/tests/test_bridge_execute_json.cpp b/tests/test_bridge_execute_json.cpp index 11b49d3cb..3822e9558 100644 --- a/tests/test_bridge_execute_json.cpp +++ b/tests/test_bridge_execute_json.cpp @@ -79,6 +79,32 @@ TEST_CASE("ActionExecuteRegistry: unknown action type throws", "[bridge][execute REQUIRE_THROWS_AS(handler.executeJson("NoSuchAction", "{}"), std::runtime_error); } +TEST_CASE("BridgeHandler::servesAction is true for a registered action and false for an unregistered one", + "[bridge][execute-json]") { + morph::exec::ThreadPoolExecutor pool{2}; + SyncExecutor cbExec; + morph::bridge::Bridge bridge{std::make_unique(pool)}; + const morph::bridge::BridgeHandler handler{bridge, &cbExec}; + + CHECK(handler.servesAction("Test_ExecJson_AddNumbers")); + CHECK_FALSE(handler.servesAction("NoSuchAction")); + // A pure existence check: unlike executeJson, it must not throw for an + // action this Model does not serve. + CHECK_NOTHROW(handler.servesAction("NoSuchAction")); +} + +TEST_CASE("ActionExecuteRegistry::contains agrees with execute for both Sharing policies", "[bridge][execute-json]") { + // registerAction files both the NoSharing and AllowShared executors for + // every registered (Model, Action) pair (see registerAction's own doc + // comment), so contains must answer the same for either tag, + // matching whichever the caller's own handler is instantiated with. + auto& registry = morph::bridge::ActionExecuteRegistry::instance(); + CHECK(registry.contains("Test_ExecJson_MathModel", "Test_ExecJson_AddNumbers")); + CHECK(registry.contains("Test_ExecJson_MathModel", "Test_ExecJson_AddNumbers")); + CHECK_FALSE(registry.contains("Test_ExecJson_MathModel", "NoSuchAction")); + CHECK_FALSE(registry.contains("NoSuchModel", "Test_ExecJson_AddNumbers")); +} + // ── Coverage: registerAction guarded forwarding (bridge.hpp L845-847, L849) ── // These target the two uncovered regions in ActionExecuteRegistry::registerAction's // executor lambda: