Add main toolbar service boundary

2026-06-03 12:42:23 +02:00
parent 62561624ed
commit fb111dcdc9
5 changed files with 218 additions and 32 deletions
--- a/docs/modernization/debt.md
+++ b/docs/modernization/debt.md
@@ -52,7 +52,7 @@ agent or engineer to remove them without reconstructing context from chat.
 | DEBT-0032 | Open | Modernization | Layer menu command planning now consumes pure `pp_app_core` through `App::init_menu_layer` and `pano_cli plan-layer-menu`, but live execution still calls legacy `Canvas::clear`, `App::dialog_layer_rename`, `NodePanelLayer::merge`, and reads `Canvas::I` animation/layer state directly | Preserve existing Layer menu behavior while layer commands move toward document/app services | `pp_app_core_document_layer_tests`; `pano_cli plan-layer-menu --command merge --current-index 2 --lower-name Paint`; `pano_cli plan-layer-menu --command rename --no-current-layer`; `ctest --preset desktop-fast --build-config Debug` | Layer clear, rename, merge-down execution, animation gating, and selected-layer state are owned by document/app services with Layer-menu callbacks acting only as UI adapters |
 | DEBT-0033 | Open | Modernization | Tools menu and floating-panel planning now consumes pure `pp_app_core` through `App::init_menu_tools`, `pano_cli plan-tools-menu`, and `pano_cli plan-tools-panel`, but live execution still constructs legacy `NodePanelFloating` panels, mutates legacy panel nodes, clears `CanvasModeGrid`, resets `NodeCanvas` camera state, opens legacy shortcuts UI, and calls the iOS SonarPen bridge directly | Preserve current Tools menu behavior while UI shell actions move toward app/UI/platform services | `pp_app_core_tools_menu_tests`; `pano_cli plan-tools-menu --command shortcuts`; `pano_cli plan-tools-panel --panel layers`; `pano_cli plan-tools-panel --panel animation --already-visible`; `ctest --preset desktop-fast --build-config Debug` | Tools panel creation, submenu routing, grid clear, camera reset, shortcuts dialog, and SonarPen dispatch are owned by app/UI/platform services with `App::init_menu_tools` acting only as a UI adapter |
 | DEBT-0034 | Open | Modernization | About menu command planning now consumes pure `pp_app_core` through `App::init_menu_about` and `pano_cli plan-about-menu`, but live execution still opens legacy About/manual/what's-new dialogs, invokes the injected crash hook, and runs the legacy Canvas stroke performance test directly | Preserve About menu behavior while dialogs and diagnostics move toward app/UI/platform services | `pp_app_core_about_menu_tests`; `pano_cli plan-about-menu --command news --version-major 2 --version-minor 5 --version-fix 7`; `pano_cli plan-about-menu --command performance --no-canvas`; `ctest --preset desktop-fast --build-config Debug` | About/manual/what's-new dialog dispatch, crash-test dispatch, and performance-test execution are owned by app/UI/platform services with `App::init_menu_about` acting only as a UI adapter |
-| DEBT-0035 | Open | Modernization | Main toolbar/status command planning now consumes pure `pp_app_core` through `App::init_toolbar_main` and `pano_cli plan-main-toolbar`, but live execution still opens legacy open/save/settings/message-box dialogs, mutates legacy `ActionManager` history, and clears the legacy `Canvas` directly | Preserve reachable toolbar/status behavior while app shell commands move toward app/document/UI services | `pp_app_core_main_toolbar_tests`; `pano_cli plan-main-toolbar --command undo --undo-count 2`; `pano_cli plan-main-toolbar --command clear-canvas --no-canvas`; `ctest --preset desktop-fast --build-config Debug` | Open/save/settings/message-box routing, undo/redo/clear-history execution, and canvas-clear execution are owned by app/document/UI services with `App::init_toolbar_main` acting only as a UI adapter |
+| DEBT-0035 | Open | Modernization | Main toolbar/status command planning and execution dispatch now consume pure `pp_app_core` through `App::init_toolbar_main`, `pano_cli plan-main-toolbar`, and the `MainToolbarServices` boundary, but the live adapter still opens legacy open/save/settings/message-box dialogs, mutates legacy `ActionManager` history, and clears the legacy `Canvas` directly | Preserve reachable toolbar/status behavior while app shell commands move toward app/document/UI services | `pp_app_core_main_toolbar_tests`; `pano_cli plan-main-toolbar --command undo --undo-count 2`; `pano_cli plan-main-toolbar --command clear-canvas --no-canvas`; `ctest --preset desktop-fast --build-config Debug` | Open/save/settings/message-box routing, undo/redo/clear-history execution, and canvas-clear execution are owned by injected app/document/UI services with `App::init_toolbar_main` acting only as a UI adapter and no legacy toolbar adapter |

 ## Closed Debt

--- a/docs/modernization/roadmap.md
+++ b/docs/modernization/roadmap.md
@@ -537,8 +537,10 @@ legacy `ActionManager` stack execution continues.
 `pano_cli plan-main-toolbar` exposes app-core planning for the live main
 toolbar/status-bar shell, including open/save dialogs, undo/redo availability,
 clear-history availability, clear-canvas no-canvas blocking, message-box
-creation, and settings dialog routing before legacy dialogs, `ActionManager`,
-and `Canvas` execution continue.
+creation, and settings dialog routing. `pp_app_core` now also owns a
+`MainToolbarServices` executor boundary, so `App::init_toolbar_main` dispatches
+through a legacy adapter before legacy dialogs, `ActionManager`, and `Canvas`
+execution continue.
 `pano_cli plan-quick-operation` exposes app-core planning for quick brush/color
 slot selection versus popup opening, plus quick mini-state restore/reset
 validation used by the live quick panel before legacy `Brush`, color picker,
@@ -1293,7 +1295,8 @@ Results:
  planning as JSON automation.
 - `pp_app_core_main_toolbar_tests` passed, covering live toolbar/status direct
  dialog routing, undo/redo availability, clear-history availability, no-canvas
-  clear blocking, and negative history metric rejection.
+  clear blocking, negative history metric rejection, and dispatch through the
+  `MainToolbarServices` executor boundary without invoking no-op actions.
 - `pano_cli_plan_main_toolbar_undo_smoke`,
  `pano_cli_plan_main_toolbar_redo_empty_smoke`,
  `pano_cli_plan_main_toolbar_clear_canvas_no_canvas_smoke`, and
--- a/src/app_core/main_toolbar.h
+++ b/src/app_core/main_toolbar.h
@@ -43,6 +43,20 @@ struct MainToolbarPlan {
    bool no_op = false;
 };

+class MainToolbarServices {
+public:
+    virtual ~MainToolbarServices() = default;
+
+    virtual void show_open_dialog() = 0;
+    virtual void show_save_dialog() = 0;
+    virtual void invoke_undo() = 0;
+    virtual void invoke_redo() = 0;
+    virtual void clear_history() = 0;
+    virtual void clear_canvas() = 0;
+    virtual void show_message_box() = 0;
+    virtual void show_settings_dialog() = 0;
+};
+
 [[nodiscard]] inline pp::foundation::Result<MainToolbarPlan> plan_main_toolbar_command(
    MainToolbarCommand command,
    int undo_count = 0,
@@ -143,4 +157,40 @@ struct MainToolbarPlan {
        pp::foundation::Status::invalid_argument("unknown main toolbar command"));
 }

+[[nodiscard]] inline pp::foundation::Status execute_main_toolbar_plan(
+    const MainToolbarPlan& plan,
+    MainToolbarServices& services)
+{
+    switch (plan.action) {
+    case MainToolbarAction::show_open_dialog:
+        services.show_open_dialog();
+        return pp::foundation::Status::success();
+    case MainToolbarAction::show_save_dialog:
+        services.show_save_dialog();
+        return pp::foundation::Status::success();
+    case MainToolbarAction::invoke_undo:
+        services.invoke_undo();
+        return pp::foundation::Status::success();
+    case MainToolbarAction::invoke_redo:
+        services.invoke_redo();
+        return pp::foundation::Status::success();
+    case MainToolbarAction::clear_history:
+        services.clear_history();
+        return pp::foundation::Status::success();
+    case MainToolbarAction::clear_canvas:
+        services.clear_canvas();
+        return pp::foundation::Status::success();
+    case MainToolbarAction::show_message_box:
+        services.show_message_box();
+        return pp::foundation::Status::success();
+    case MainToolbarAction::show_settings_dialog:
+        services.show_settings_dialog();
+        return pp::foundation::Status::success();
+    case MainToolbarAction::no_op_unavailable:
+        return pp::foundation::Status::success();
+    }
+
+    return pp::foundation::Status::invalid_argument("unknown main toolbar action");
+}
+
 } // namespace pp::app
--- a/src/app_layout.cpp
+++ b/src/app_layout.cpp
@@ -249,6 +249,74 @@ void apply_tools_panel_chrome(NodePanelFloating& panel, const pp::app::ToolsPane
    panel.m_droppable = plan.droppable;
 }

+class LegacyMainToolbarServices final : public pp::app::MainToolbarServices {
+public:
+    explicit LegacyMainToolbarServices(App& app) noexcept
+        : app_(app)
+    {
+    }
+
+    void show_open_dialog() override
+    {
+        app_.dialog_open();
+    }
+
+    void show_save_dialog() override
+    {
+        app_.dialog_save();
+    }
+
+    void invoke_undo() override
+    {
+        ActionManager::undo();
+    }
+
+    void invoke_redo() override
+    {
+        ActionManager::redo();
+    }
+
+    void clear_history() override
+    {
+        ActionManager::clear();
+    }
+
+    void clear_canvas() override
+    {
+        if (!app_.canvas || !app_.canvas->m_canvas)
+            return;
+
+        app_.canvas->m_canvas->clear({ 0.0F, 0.0F, 0.0F, 0.0F });
+    }
+
+    void show_message_box() override
+    {
+        app_.msgbox = new NodeMessageBox();
+        app_.msgbox->set_manager(&app_.layout);
+        app_.msgbox->init();
+        app_.layout[app_.main_id]->add_child(app_.msgbox);
+    }
+
+    void show_settings_dialog() override
+    {
+        app_.settings = new NodeSettings();
+        app_.settings->set_manager(&app_.layout);
+        app_.settings->init();
+        app_.layout[app_.main_id]->add_child(app_.settings);
+    }
+
+private:
+    App& app_;
+};
+
+void execute_main_toolbar_plan(App& app, const pp::app::MainToolbarPlan& plan)
+{
+    LegacyMainToolbarServices services(app);
+    const auto status = pp::app::execute_main_toolbar_plan(plan, services);
+    if (!status.ok())
+        LOG("Main toolbar action failed: %s", status.message);
+}
+
 } // namespace

 void App::title_update()
@@ -284,8 +352,8 @@ void App::init_toolbar_main()
        button->on_click = [this, button](Node*) {
            const auto plan = pp::app::plan_main_toolbar_command(
                pp::app::MainToolbarCommand::open_document);
-            if (plan && plan.value().action == pp::app::MainToolbarAction::show_open_dialog)
-                dialog_open();
+            if (plan)
+                execute_main_toolbar_plan(*this, plan.value());
        };
    }
    if (auto* button = layout[main_id]->find<NodeButton>("btn-save"))
@@ -293,8 +361,8 @@ void App::init_toolbar_main()
        button->on_click = [this, button](Node*) {
            const auto plan = pp::app::plan_main_toolbar_command(
                pp::app::MainToolbarCommand::save_document);
-            if (plan && plan.value().action == pp::app::MainToolbarAction::show_save_dialog)
-                dialog_save();
+            if (plan)
+                execute_main_toolbar_plan(*this, plan.value());
        };
    }
    if (auto* button = layout[main_id]->find<NodeButtonCustom>("btn-undo"))
@@ -303,8 +371,8 @@ void App::init_toolbar_main()
            const auto plan = pp::app::plan_main_toolbar_command(
                pp::app::MainToolbarCommand::undo,
                static_cast<int>(ActionManager::I.m_actions.size()));
-            if (plan && plan.value().action == pp::app::MainToolbarAction::invoke_undo)
-                ActionManager::undo();
+            if (plan)
+                execute_main_toolbar_plan(*this, plan.value());
        };
    }
    if (auto* button = layout[main_id]->find<NodeButtonCustom>("btn-redo"))
@@ -314,8 +382,8 @@ void App::init_toolbar_main()
                pp::app::MainToolbarCommand::redo,
                0,
                static_cast<int>(ActionManager::I.m_redos.size()));
-            if (plan && plan.value().action == pp::app::MainToolbarAction::invoke_redo)
-                ActionManager::redo();
+            if (plan)
+                execute_main_toolbar_plan(*this, plan.value());
        };
    }
    if (auto* button = layout[main_id]->find<NodeButtonCustom>("btn-clean-memory"))
@@ -326,8 +394,8 @@ void App::init_toolbar_main()
                static_cast<int>(ActionManager::I.m_actions.size()),
                static_cast<int>(ActionManager::I.m_redos.size()),
                static_cast<int>(ActionManager::I.m_memory));
-            if (plan && plan.value().action == pp::app::MainToolbarAction::clear_history)
-                ActionManager::clear();
+            if (plan)
+                execute_main_toolbar_plan(*this, plan.value());
        };
    }
    if (auto* button = layout[main_id]->find<NodeButton>("btn-clear"))
@@ -340,12 +408,8 @@ void App::init_toolbar_main()
                0,
                0,
                static_cast<bool>(canvas));
-            if (plan && plan.value().action == pp::app::MainToolbarAction::clear_canvas)
-                canvas->m_canvas->clear({
-                    0.0F,
-                    0.0F,
-                    0.0F,
-                    0.0F });
+            if (plan)
+                execute_main_toolbar_plan(*this, plan.value());
        };
    }
    if (auto* button = layout[main_id]->find<NodeButton>("btn-popup"))
@@ -353,12 +417,8 @@ void App::init_toolbar_main()
        button->on_click = [this](Node*) {
            const auto plan = pp::app::plan_main_toolbar_command(
                pp::app::MainToolbarCommand::show_message_box);
-            if (!plan || plan.value().action != pp::app::MainToolbarAction::show_message_box)
-                return;
-            msgbox = new NodeMessageBox();
-            msgbox->set_manager(&layout);
-            msgbox->init();
-            layout[main_id]->add_child(msgbox);
+            if (plan)
+                execute_main_toolbar_plan(*this, plan.value());
        };
    }
    if (auto* button = layout[main_id]->find<NodeButtonCustom>("btn-settings"))
@@ -366,12 +426,8 @@ void App::init_toolbar_main()
        button->on_click = [this](Node*) {
            const auto plan = pp::app::plan_main_toolbar_command(
                pp::app::MainToolbarCommand::show_settings);
-            if (!plan || plan.value().action != pp::app::MainToolbarAction::show_settings_dialog)
-                return;
-            settings = new NodeSettings();
-            settings->set_manager(&layout);
-            settings->init();
-            layout[main_id]->add_child(settings);
+            if (plan)
+                execute_main_toolbar_plan(*this, plan.value());
        };
    }
 }
--- a/tests/app_core/main_toolbar_tests.cpp
+++ b/tests/app_core/main_toolbar_tests.cpp
@@ -3,6 +3,39 @@

 namespace {

+class FakeMainToolbarServices final : public pp::app::MainToolbarServices {
+public:
+    void show_open_dialog() override { open_dialogs += 1; }
+    void show_save_dialog() override { save_dialogs += 1; }
+    void invoke_undo() override { undo_calls += 1; }
+    void invoke_redo() override { redo_calls += 1; }
+    void clear_history() override { clear_history_calls += 1; }
+    void clear_canvas() override { clear_canvas_calls += 1; }
+    void show_message_box() override { message_boxes += 1; }
+    void show_settings_dialog() override { settings_dialogs += 1; }
+
+    [[nodiscard]] int total_calls() const noexcept
+    {
+        return open_dialogs
+            + save_dialogs
+            + undo_calls
+            + redo_calls
+            + clear_history_calls
+            + clear_canvas_calls
+            + message_boxes
+            + settings_dialogs;
+    }
+
+    int open_dialogs = 0;
+    int save_dialogs = 0;
+    int undo_calls = 0;
+    int redo_calls = 0;
+    int clear_history_calls = 0;
+    int clear_canvas_calls = 0;
+    int message_boxes = 0;
+    int settings_dialogs = 0;
+};
+
 void direct_dialog_commands_are_available(pp::tests::Harness& harness)
 {
    const auto open = pp::app::plan_main_toolbar_command(pp::app::MainToolbarCommand::open_document);
@@ -101,6 +134,49 @@ void rejects_negative_history_metrics(pp::tests::Harness& harness)
    PP_EXPECT(harness, !pp::app::plan_main_toolbar_command(pp::app::MainToolbarCommand::clear_history, 0, 0, -1));
 }

+void executor_dispatches_to_service_boundary(pp::tests::Harness& harness)
+{
+    FakeMainToolbarServices services;
+
+    auto open = pp::app::plan_main_toolbar_command(pp::app::MainToolbarCommand::open_document);
+    PP_EXPECT(harness, open);
+    if (open) {
+        const auto status = pp::app::execute_main_toolbar_plan(open.value(), services);
+        PP_EXPECT(harness, status.ok());
+        PP_EXPECT(harness, services.open_dialogs == 1);
+    }
+
+    auto undo = pp::app::plan_main_toolbar_command(pp::app::MainToolbarCommand::undo, 1);
+    PP_EXPECT(harness, undo);
+    if (undo) {
+        const auto status = pp::app::execute_main_toolbar_plan(undo.value(), services);
+        PP_EXPECT(harness, status.ok());
+        PP_EXPECT(harness, services.undo_calls == 1);
+    }
+
+    auto clear_canvas = pp::app::plan_main_toolbar_command(
+        pp::app::MainToolbarCommand::clear_canvas,
+        0,
+        0,
+        0,
+        true);
+    PP_EXPECT(harness, clear_canvas);
+    if (clear_canvas) {
+        const auto status = pp::app::execute_main_toolbar_plan(clear_canvas.value(), services);
+        PP_EXPECT(harness, status.ok());
+        PP_EXPECT(harness, services.clear_canvas_calls == 1);
+    }
+
+    auto redo_empty = pp::app::plan_main_toolbar_command(pp::app::MainToolbarCommand::redo);
+    PP_EXPECT(harness, redo_empty);
+    if (redo_empty) {
+        const int calls_before = services.total_calls();
+        const auto status = pp::app::execute_main_toolbar_plan(redo_empty.value(), services);
+        PP_EXPECT(harness, status.ok());
+        PP_EXPECT(harness, services.total_calls() == calls_before);
+    }
+}
+
 } // namespace

 int main()
@@ -110,5 +186,6 @@ int main()
    harness.run("history commands reuse history breakpoints", history_commands_reuse_history_breakpoints);
    harness.run("canvas clear requires live canvas", canvas_clear_requires_live_canvas);
    harness.run("rejects negative history metrics", rejects_negative_history_metrics);
+    harness.run("executor dispatches to service boundary", executor_dispatches_to_service_boundary);
    return harness.finish();
 }