From f4414dd72ba588cdea2d8b117a98c08c098d92a4 Mon Sep 17 00:00:00 2001 From: Ian Chua Date: Thu, 16 Jul 2026 18:18:07 +0800 Subject: [PATCH] fix: collapse plugincapabilityid --- src/slic3r/GUI/PluginsConfigDialog.cpp | 2 +- src/slic3r/GUI/PluginsDialog.cpp | 155 +++++++--------- src/slic3r/GUI/PluginsDialog.hpp | 13 +- src/slic3r/Utils/NetworkAgentFactory.cpp | 6 +- src/slic3r/plugin/PluginConfig.cpp | 174 ++++++++---------- src/slic3r/plugin/PluginConfig.hpp | 138 ++------------ src/slic3r/plugin/PluginManager.cpp | 36 ++-- src/slic3r/plugin/PluginManager.hpp | 28 +-- src/slic3r/plugin/PluginResolver.cpp | 26 ++- src/slic3r/plugin/PluginResolver.hpp | 3 +- src/slic3r/plugin/PythonPluginInterface.hpp | 22 +++ .../test_plugin_capability_config.cpp | 44 +++-- tests/slic3rutils/test_plugin_config.cpp | 130 +++++++++---- tests/slic3rutils/test_plugin_lifecycle.cpp | 12 +- 14 files changed, 361 insertions(+), 428 deletions(-) diff --git a/src/slic3r/GUI/PluginsConfigDialog.cpp b/src/slic3r/GUI/PluginsConfigDialog.cpp index 421207df35..2f36e192e9 100644 --- a/src/slic3r/GUI/PluginsConfigDialog.cpp +++ b/src/slic3r/GUI/PluginsConfigDialog.cpp @@ -156,7 +156,7 @@ void PluginsConfigDialog::send_capability_config(const PluginCapabilityId& id) response["custom_html"] = ""; response["error"] = ""; - const auto cap = PluginManager::instance().get_plugin_capability(id.plugin_key, id.name, id.type, false); + const auto cap = PluginManager::instance().get_plugin_capability(id, false); if (!cap || preset == nullptr) { response["error"] = into_u8(_L("This capability is no longer available.")); call_web_handler(response); diff --git a/src/slic3r/GUI/PluginsDialog.cpp b/src/slic3r/GUI/PluginsDialog.cpp index ad90b74582..cddc090c84 100644 --- a/src/slic3r/GUI/PluginsDialog.cpp +++ b/src/slic3r/GUI/PluginsDialog.cpp @@ -39,9 +39,9 @@ namespace Slic3r { namespace GUI { namespace { -const wxString kDeletePluginTitle = _L("Delete Plugin"); -const wxString kUnsubscribeTitle = _L("Unsubscribe"); -const wxString kOverwritePluginTitle = _L("Overwrite Plugin"); +const wxString kDeletePluginTitle = _L("Delete Plugin"); +const wxString kUnsubscribeTitle = _L("Unsubscribe"); +const wxString kOverwritePluginTitle = _L("Overwrite Plugin"); std::string s_selected_plugin_install_action = "explore"; struct PluginContextAction @@ -61,9 +61,7 @@ struct PluginAvailableActions struct PluginCapabilityView { - std::string name; - std::string type_label; - std::string type_key; + PluginCapabilityId id; bool enabled = false; bool can_toggle = false; bool can_run = false; @@ -90,7 +88,7 @@ struct PluginDialogItem std::string version; std::string installed_version; std::string latest_version; - std::string sort_version; // Version shown in the row (installed if installed, else latest); used by the Version sort. + std::string sort_version; // Version shown in the row (installed if installed, else latest); used by the Version sort. std::string type_label; std::string type_key; std::string sharing_token; @@ -103,7 +101,7 @@ struct PluginDialogItem PluginUpdateStatus update_status = PluginUpdateStatus::Normal; std::string error_text; bool has_error = false; - bool is_loaded = false; + bool is_loaded = false; bool loading = false; bool is_cloud_plugin = false; @@ -157,7 +155,7 @@ void refresh_plugin_catalog_blocking(bool fetch_cloud) std::vector not_found, unauthorized; const std::vector current_cloud_catalog = fetch_cloud ? std::vector{} : - current_cloud_catalog_snapshot(); + current_cloud_catalog_snapshot(); manager.rescan_plugins(); @@ -176,15 +174,13 @@ void refresh_plugin_catalog_blocking(bool fetch_cloud) return; for (const auto& uuid : not_found) - plater->get_notification_manager()->push_notification( - NotificationType::CustomNotification, - NotificationManager::NotificationLevel::RegularNotificationLevel, - format(_L("Plugin %s is no longer available."), uuid)); + plater->get_notification_manager()->push_notification(NotificationType::CustomNotification, + NotificationManager::NotificationLevel::RegularNotificationLevel, + format(_L("Plugin %s is no longer available."), uuid)); for (const auto& uuid : unauthorized) - plater->get_notification_manager()->push_notification( - NotificationType::CustomNotification, - NotificationManager::NotificationLevel::RegularNotificationLevel, - format(_L("Plugin %s access is unauthorized."), uuid)); + plater->get_notification_manager()->push_notification(NotificationType::CustomNotification, + NotificationManager::NotificationLevel::RegularNotificationLevel, + format(_L("Plugin %s access is unauthorized."), uuid)); }); } @@ -207,9 +203,9 @@ nlohmann::json build_plugin_payload_item(const PluginDialogItem& dialog_item) nlohmann::json caps = nlohmann::json::array(); for (const PluginCapabilityView& capability : dialog_item.capabilities) { nlohmann::json c; - c["name"] = capability.name; - c["type"] = capability.type_label; - c["type_key"] = capability.type_key; + c["name"] = capability.id.name; + c["type"] = plugin_capability_type_display_name(capability.id.type); + c["type_key"] = plugin_capability_type_to_string(capability.id.type); c["enabled"] = capability.enabled; c["can_toggle"] = capability.can_toggle; c["can_run"] = capability.can_run; @@ -223,9 +219,8 @@ nlohmann::json build_plugin_payload_item(const PluginDialogItem& dialog_item) // rows the config view has no use for. std::vector config_ids; for (const PluginCapabilityView& capability : dialog_item.capabilities) - if (!capability.name.empty()) - config_ids.push_back(PluginCapabilityId{plugin_capability_type_from_string(capability.type_key), - capability.name, dialog_item.plugin_key}); + if (!capability.id.name.empty()) + config_ids.push_back(capability.id); payload_item["config_capabilities"] = PluginConfig::capabilities_payload(config_ids); nlohmann::json changelog = nlohmann::json::array(); @@ -319,13 +314,13 @@ PluginDialogItem build_plugin_dialog_item(const PluginDescriptor& descriptor) PluginDialogItem item; PluginManager& manager = PluginManager::instance(); - item.plugin_key = descriptor.plugin_key; - item.display_name = descriptor.name; - item.description = !descriptor.is_metadata_valid() && descriptor.has_error() ? - descriptor.normalized_error() : - (descriptor.description.empty() ? "No description." : descriptor.description); - item.author = descriptor.author; - item.version = descriptor.version; + item.plugin_key = descriptor.plugin_key; + item.display_name = descriptor.name; + item.description = !descriptor.is_metadata_valid() && descriptor.has_error() ? + descriptor.normalized_error() : + (descriptor.description.empty() ? "No description." : descriptor.description); + item.author = descriptor.author; + item.version = descriptor.version; // The cloud merge overwrites `version` with the latest cloud version, so prefer the preserved local // one, falling back to `version` for local-only / pre-merge descriptors. item.installed_version = descriptor.has_local_package() ? @@ -338,10 +333,9 @@ PluginDialogItem build_plugin_dialog_item(const PluginDescriptor& descriptor) const std::vector> capabilities = manager.get_plugin_capabilities(descriptor.plugin_key, PluginCapabilityType::Unknown, /*only_enabled=*/false); - const PluginCapabilityType primary_type = capabilities.empty() ? PluginCapabilityType::Unknown : - capabilities.front()->type(); - item.type_label = plugin_capability_type_to_string(primary_type); - item.type_key = plugin_capability_type_to_string(primary_type); + const PluginCapabilityType primary_type = capabilities.empty() ? PluginCapabilityType::Unknown : capabilities.front()->type(); + item.type_label = plugin_capability_type_to_string(primary_type); + item.type_key = plugin_capability_type_to_string(primary_type); // "types" is the display-only compatibility list. Cloud plugins show the raw labels the // service returned (which may not map to real capability types); local plugins derive them // from the capabilities actually loaded. @@ -369,9 +363,7 @@ PluginDialogItem build_plugin_dialog_item(const PluginDescriptor& descriptor) item.loading = manager.is_plugin_load_in_progress(descriptor.plugin_key); item.has_script_capability = false; for (const auto& capability : capabilities) { - item.capabilities.push_back({capability->name(), plugin_capability_type_display_name(capability->type()), - plugin_capability_type_to_string(capability->type()), capability->is_enabled(), true, false, - capability->config_ui_available()}); + item.capabilities.push_back({capability->identity(), capability->is_enabled(), true, false, capability->config_ui_available()}); if (capability->type() == PluginCapabilityType::Script) item.has_script_capability = true; } @@ -390,12 +382,12 @@ PluginDialogItem build_plugin_dialog_item(const PluginDescriptor& descriptor) item.available_actions = evaluate_action_policy(item); const bool has_enabled_script = std::any_of(item.capabilities.begin(), item.capabilities.end(), [](const PluginCapabilityView& capability) { - return capability.type_key == "script" && capability.enabled; + return capability.id.type == PluginCapabilityType::Script && capability.enabled; }); item.can_run_script = descriptor.is_metadata_valid() && !descriptor.has_error() && item.has_script_capability && item.is_loaded && !item.loading && has_enabled_script; for (PluginCapabilityView& capability : item.capabilities) { - capability.can_run = item.can_run_script && capability.type_key == "script" && capability.enabled; + capability.can_run = item.can_run_script && capability.id.type == PluginCapabilityType::Script && capability.enabled; } return item; } @@ -518,21 +510,21 @@ void PluginsDialog::on_script_message(const nlohmann::json& payload) open_plugin_hub(); } else if (command == "set_plugin_sort") { set_plugin_sort(payload.value("sort_key", ""), payload.value("sort_order", "")); - } else if (command == "get_capability_config") { - send_capability_config(payload.value("plugin_key", ""), - plugin_capability_type_from_string(payload.value("capability_type", "")), - payload.value("capability_name", "")); - } else if (command == "save_capability_config") { + } else if (command == "get_capability_config" || command == "save_capability_config" || command == "restore_capability_config") { + const PluginCapabilityId id{plugin_capability_type_from_string(payload.value("capability_type", "")), + payload.value("capability_name", ""), payload.value("plugin_key", "")}; + if (command == "get_capability_config") { + send_capability_config(id); + return; + } + if (command == "restore_capability_config") { + restore_capability_config(id); + return; + } + // `config` is a JSON string from the default editor's textarea, or an already-structured // value from a capability's custom UI. Both land here; save_capability_config sorts it out. - save_capability_config(payload.value("plugin_key", ""), - plugin_capability_type_from_string(payload.value("capability_type", "")), - payload.value("capability_name", ""), - payload.contains("config") ? payload.at("config") : nlohmann::json::object()); - } else if (command == "restore_capability_config") { - restore_capability_config(payload.value("plugin_key", ""), - plugin_capability_type_from_string(payload.value("capability_type", "")), - payload.value("capability_name", "")); + save_capability_config(id, payload.contains("config") ? payload.at("config") : nlohmann::json::object()); } else if (command == "set_plugin_install_action") { const std::string action = payload.value("action", ""); if (action == "explore" || action == "install-local") @@ -583,15 +575,6 @@ bool PluginsDialog::get_descriptor(const std::string& plugin_key, PluginDescript return manager.try_get_plugin_descriptor(plugin_key, descriptor) && descriptor.is_invalid_package(); } -std::shared_ptr PluginsDialog::get_capability(const std::string& plugin_key, - PluginCapabilityType type, - const std::string& capability_name) const -{ - // only_enabled=false: this is an existence check used to gate both enabling and disabling a - // capability from the dialog, so a currently-disabled capability must still resolve. - return PluginManager::instance().get_plugin_capability(plugin_key, capability_name, type, /*only_enabled=*/false); -} - void PluginsDialog::refresh_plugin_catalog_async(const wxString& title, const wxString& message, bool fetch_cloud) { run_with_dialog([fetch_cloud]() { refresh_plugin_catalog_blocking(fetch_cloud); }, [this]() { send_plugins(); }, title, message); @@ -695,8 +678,10 @@ void PluginsDialog::toggle_plugin(const std::string& plugin_key, bool enabled) show_status(wxString::Format(_L("Activating \"%s\"..."), plugin_display_name(plugin_key)), "info"); } -void PluginsDialog::toggle_plugin_capability(const std::string& plugin_key, PluginCapabilityType type, - const std::string& capability_name, bool enabled) +void PluginsDialog::toggle_plugin_capability(const std::string& plugin_key, + PluginCapabilityType type, + const std::string& capability_name, + bool enabled) { if (plugin_key.empty() || capability_name.empty() || type == PluginCapabilityType::Unknown) return; @@ -708,7 +693,7 @@ void PluginsDialog::toggle_plugin_capability(const std::string& plugin_key, Plug return; } - if (!get_capability(plugin_key, type, capability_name)) { + if (!PluginManager::instance().get_plugin_capability({type, capability_name, plugin_key}, /*only_enabled=*/false)) { BOOST_LOG_TRIVIAL(warning) << "Cannot toggle missing plugin capability: " << plugin_key << " | " << capability_name; send_plugins(); return; @@ -717,11 +702,11 @@ void PluginsDialog::toggle_plugin_capability(const std::string& plugin_key, Plug PluginManager& manager = PluginManager::instance(); if (enabled) { BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << ": Enabling plugin capability: " << plugin_key << " | " << capability_name; - manager.set_capability_enabled(plugin_key, capability_name, true); + manager.set_capability_enabled({type, capability_name, plugin_key}, true); } else { // check if the capability is currently in use here. BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << ": Disabling plugin capability: " << plugin_key << " | " << capability_name; - manager.set_capability_enabled(plugin_key, capability_name, false); + manager.set_capability_enabled({type, capability_name, plugin_key}, false); } send_plugins(); @@ -896,39 +881,29 @@ wxString PluginsDialog::plugin_display_name(const std::string& plugin_key) const return from_u8(plugin_key); } -void PluginsDialog::send_capability_config(const std::string& plugin_key, - PluginCapabilityType type, - const std::string& capability_name) -{ - call_web_handler(PluginConfig::get_config_response({type, capability_name, plugin_key})); -} +void PluginsDialog::send_capability_config(const PluginCapabilityId& id) { call_web_handler(PluginConfig::get_config_response(id)); } -void PluginsDialog::save_capability_config(const std::string& plugin_key, - PluginCapabilityType type, - const std::string& capability_name, - const nlohmann::json& config) +void PluginsDialog::save_capability_config(const PluginCapabilityId& id, const nlohmann::json& config) { - const nlohmann::json response = PluginConfig::save_config_response({type, capability_name, plugin_key}, config); + const nlohmann::json response = PluginConfig::save_config_response(id, config); call_web_handler(response); if (response.value("ok", false)) show_status(_L("Configuration saved."), "success"); } -void PluginsDialog::restore_capability_config(const std::string& plugin_key, - PluginCapabilityType type, - const std::string& capability_name) +void PluginsDialog::restore_capability_config(const PluginCapabilityId& id) { // Destructive, so confirm first. The confirmation stays here rather than in PluginConfig: it needs // a parent window. const int rc = wxMessageBox(wxString::Format(_L("Restore the default configuration for \"%s\"?\n\n" "This discards the settings currently saved for this capability."), - from_u8(capability_name)), + from_u8(id.name)), _L("Restore defaults"), wxYES_NO | wxNO_DEFAULT | wxICON_WARNING, this); if (rc != wxYES) return; - const nlohmann::json response = PluginConfig::restore_config_response({type, capability_name, plugin_key}); + const nlohmann::json response = PluginConfig::restore_config_response(id); call_web_handler(response); if (response.value("ok", false)) @@ -937,7 +912,6 @@ void PluginsDialog::restore_capability_config(const std::string& plugin_key, void PluginsDialog::run_script_plugin(const std::string& plugin_key, const std::string& capability_name) { - PluginManager& manager = PluginManager::instance(); std::string error; @@ -981,7 +955,7 @@ void PluginsDialog::run_script_plugin(const std::string& plugin_key, const std:: manager.clear_plugin_error(plugin_key); send_plugins(); - const bool skipped = result.status == PluginResult::Skipped; + const bool skipped = result.status == PluginResult::Skipped; const wxString fallback = skipped ? _L("Script plugin skipped.") : _L("Script plugin finished."); const wxString message = result.message.empty() ? fallback : from_u8(result.message); show_status(message, skipped ? "info" : "success"); @@ -1002,9 +976,8 @@ void PluginsDialog::update_plugin(const std::string& plugin_key) std::string error; bool updated = false; try { - updated = run_with_dialog_wait( - [plugin_key, &error]() { return PluginManager::instance().update_cloud_plugin(plugin_key, error); }, - _L("Updating plugin"), _L("Updating") + ": " + name); + updated = run_with_dialog_wait([plugin_key, &error]() { return PluginManager::instance().update_cloud_plugin(plugin_key, error); }, + _L("Updating plugin"), _L("Updating") + ": " + name); } catch (const std::exception& ex) { error = ex.what(); } catch (...) { @@ -1144,8 +1117,7 @@ void PluginsDialog::reinstall_local_plugin(const std::string& plugin_key) manager.load_plugin(plugin_key, false); std::string error; - if (!manager.wait_for_plugin_load(plugin_key, std::chrono::minutes(5), error) || - !manager.is_plugin_loaded(plugin_key)) + if (!manager.wait_for_plugin_load(plugin_key, std::chrono::minutes(5), error) || !manager.is_plugin_loaded(plugin_key)) return {false, error.empty() ? "Plugin failed to load." : error}; if (!was_loaded && !manager.unload_plugin(plugin_key)) @@ -1177,7 +1149,7 @@ void PluginsDialog::reinstall_cloud_plugin(const PluginDescriptor& plugin) return; PluginManager& manager = PluginManager::instance(); - const bool was_loaded = PluginManager::instance().is_plugin_loaded(plugin_key); + const bool was_loaded = PluginManager::instance().is_plugin_loaded(plugin_key); std::string error; if (plugin.has_local_package()) { @@ -1203,8 +1175,7 @@ void PluginsDialog::reinstall_cloud_plugin(const PluginDescriptor& plugin) PluginManager& manager = PluginManager::instance(); manager.load_plugin(plugin_key); std::string error; - if (!manager.wait_for_plugin_load(plugin_key, std::chrono::minutes(5), error) || - !manager.is_plugin_loaded(plugin_key)) + if (!manager.wait_for_plugin_load(plugin_key, std::chrono::minutes(5), error) || !manager.is_plugin_loaded(plugin_key)) return {false, error.empty() ? "Plugin failed to load." : error}; return {true, {}}; }, diff --git a/src/slic3r/GUI/PluginsDialog.hpp b/src/slic3r/GUI/PluginsDialog.hpp index 5b0678dbd3..b041d8db71 100644 --- a/src/slic3r/GUI/PluginsDialog.hpp +++ b/src/slic3r/GUI/PluginsDialog.hpp @@ -29,6 +29,7 @@ class wxTimer; namespace Slic3r { class PluginCapabilityInterface; +struct PluginCapabilityId; enum class PluginCapabilityType; namespace GUI { @@ -58,7 +59,6 @@ private: nlohmann::json build_plugins_payload() const; bool get_descriptor(const std::string& plugin_key, Slic3r::PluginDescriptor& descriptor) const; - std::shared_ptr get_capability(const std::string& plugin_key, PluginCapabilityType type, const std::string& capability_name) const; void refresh_plugin_catalog_async(const wxString& title, const wxString& message, bool fetch_cloud); void refresh_plugins(); @@ -70,15 +70,12 @@ private: bool install_plugin_package(const std::string& package_path); bool install_cloud_plugin(const std::string& uuid, const std::string& version, const wxString& name); void run_script_plugin(const std::string& plugin_key, const std::string& capability_name); - // Config tab. Both are scoped to (plugin_key, type, capability_name): a request naming a + // Config tab. Both are scoped to the full capability ID: a request naming a // capability that is gone or not configurable is refused rather than served from, or written // to, some other entry. - void send_capability_config(const std::string& plugin_key, PluginCapabilityType type, const std::string& capability_name); - void save_capability_config(const std::string& plugin_key, - PluginCapabilityType type, - const std::string& capability_name, - const nlohmann::json& config); - void restore_capability_config(const std::string& plugin_key, PluginCapabilityType type, const std::string& capability_name); + void send_capability_config(const PluginCapabilityId& id); + void save_capability_config(const PluginCapabilityId& id, const nlohmann::json& config); + void restore_capability_config(const PluginCapabilityId& id); // Pushes a one-line result into the web footer status bar (level: "success" | "warn" | "error" | "info"), // used for every plugin/capability operation instead of a modal box so the dialog stays non-disruptive. void show_status(const wxString& message, const char* level); diff --git a/src/slic3r/Utils/NetworkAgentFactory.cpp b/src/slic3r/Utils/NetworkAgentFactory.cpp index 2763435519..3883d99f2e 100644 --- a/src/slic3r/Utils/NetworkAgentFactory.cpp +++ b/src/slic3r/Utils/NetworkAgentFactory.cpp @@ -238,8 +238,8 @@ void NetworkAgentFactory::register_python_printer_agent(const std::string& plugi { PluginManager& plugin_manager = PluginManager::instance(); - auto cap = plugin_manager.get_plugin_capability(plugin_key, capability_name, PluginCapabilityType::PrinterConnection, - /*only_enabled=*/false); + auto cap = plugin_manager.get_plugin_capability({PluginCapabilityType::PrinterConnection, capability_name, plugin_key}, + /*only_enabled=*/false); if (!cap) { BOOST_LOG_TRIVIAL(warning) << "Printer-agent capability '" << capability_name << "' not found for plugin '" << plugin_key << "'"; return; @@ -305,7 +305,7 @@ void NetworkAgentFactory::register_python_printer_agent(const std::string& plugi // Re-resolve without holding the registry mutex to avoid lock inversion and reentrancy. // only_enabled defaults to true here, so a capability that changed identity (reload) or was // merely disabled both surface the same way: current_cap no longer equals cap. - auto current_cap = plugin_manager.get_plugin_capability(plugin_key, capability_name, PluginCapabilityType::PrinterConnection); + auto current_cap = plugin_manager.get_plugin_capability({PluginCapabilityType::PrinterConnection, capability_name, plugin_key}); if (current_cap != cap) { BOOST_LOG_TRIVIAL(debug) << "Printer-agent capability '" << capability_name << "' from plugin '" << plugin_key << "' changed or was disabled during registration"; diff --git a/src/slic3r/plugin/PluginConfig.cpp b/src/slic3r/plugin/PluginConfig.cpp index 9acce3ed86..1efbb7dfc6 100644 --- a/src/slic3r/plugin/PluginConfig.cpp +++ b/src/slic3r/plugin/PluginConfig.cpp @@ -26,6 +26,7 @@ namespace { constexpr const char* KEY_PLUGIN = "plugin_key"; constexpr const char* KEY_CAPABILITY = "capability"; +constexpr const char* KEY_TYPE = "capability_type"; constexpr const char* KEY_VERSION = "plugin_version"; constexpr const char* KEY_CAP_CONFIG = "cap_config"; @@ -35,23 +36,24 @@ std::string string_field(const nlohmann::json& entry, const char* key) return it != entry.end() && it->is_string() ? it->get() : std::string(); } -bool is_recognized_entry(const nlohmann::json& entry, CapabilityConfigId& id) +bool is_recognized_entry(const nlohmann::json& entry, PluginCapabilityId& id) { if (!entry.is_object()) return false; id.plugin_key = string_field(entry, KEY_PLUGIN); - id.capability = string_field(entry, KEY_CAPABILITY); - return !id.plugin_key.empty() && !id.capability.empty(); + id.name = string_field(entry, KEY_CAPABILITY); + id.type = plugin_capability_type_from_string(string_field(entry, KEY_TYPE)); + return !id.empty(); } -CapabilityConfigEntry decode_entry(const CapabilityConfigId& id, const nlohmann::json& entry) +CapabilityConfigEntry decode_entry(const PluginCapabilityId& id, const nlohmann::json& entry) { CapabilityConfigEntry result; result.id = id; result.plugin_version = string_field(entry, KEY_VERSION); const auto cap_it = entry.find(KEY_CAP_CONFIG); - result.cap_config = cap_it != entry.end() ? *cap_it : nlohmann::json::object(); + result.config = cap_it != entry.end() ? *cap_it : nlohmann::json::object(); return result; } @@ -65,11 +67,6 @@ std::string running_plugin_version(const std::string& plugin_key) return descriptor.installed_version.empty() ? descriptor.version : descriptor.installed_version; } -CapabilityConfigId make_id(const PluginCapabilityId& id) -{ - return CapabilityConfigId{id.plugin_key, id.name}; -} - // Null wherever the plugin host runs without the GUI app (the unit tests). wxGetApp() dereferences // the app unconditionally, so ask wxWidgets instead. const PresetBundle* active_preset_bundle() @@ -81,31 +78,15 @@ const PresetBundle* active_preset_bundle() // PluginLoader stamps both halves onto the instance when it materializes the capability, so the // caller never supplies them and cannot name another capability's entry. Empty means the instance // was never materialized: refuse rather than read or clobber a wrong entry. -std::pair capability_identity(const PluginCapabilityInterface& capability, const char* api_name) +PluginCapabilityId capability_identity(const PluginCapabilityInterface& capability, const char* api_name) { - std::pair id{capability.audit_plugin_key(), capability.name()}; - if (id.first.empty() || id.second.empty()) + const PluginCapabilityId id = capability.identity(); + if (id.empty()) throw std::runtime_error(std::string(api_name) + "() is only available on a capability loaded by the plugin host"); return id; } -// The capability type identifies the preset that owns its override. If a plugin raises while -// reporting its type, resolve against the base config instead of failing the capability config API. -PluginCapabilityId capability_full_identity(const PluginCapabilityInterface& capability, const char* api_name) -{ - const auto [plugin_key, capability_name] = capability_identity(capability, api_name); - - PluginCapabilityType type = PluginCapabilityType::Unknown; - try { - type = capability.get_type(); - } catch (const std::exception& ex) { - BOOST_LOG_TRIVIAL(warning) << "Capability '" << capability_name << "' of plugin '" << plugin_key - << "': get_type() failed (" << ex.what() << "); reading the base config only"; - } - return {type, capability_name, plugin_key}; -} - } // namespace CapabilityConfigDocument CapabilityConfigDocument::from_entries(const nlohmann::json& entries) @@ -115,8 +96,8 @@ CapabilityConfigDocument CapabilityConfigDocument::from_entries(const nlohmann:: return document; for (const nlohmann::json& entry : entries) { - CapabilityConfigId id; - if (is_recognized_entry(entry, id)) + PluginCapabilityId id; + if (is_recognized_entry(entry, id) || (!id.plugin_key.empty() && !id.name.empty())) document.m_entries[id] = entry; else document.m_opaque_entries.push_back(entry); @@ -131,41 +112,56 @@ CapabilityConfigDocument CapabilityConfigDocument::from_root_json(const nlohmann return entries != root.end() ? from_entries(*entries) : CapabilityConfigDocument(); } -std::optional CapabilityConfigDocument::find(const CapabilityConfigId& id) const +std::optional CapabilityConfigDocument::find(const PluginCapabilityId& id) const { const auto it = m_entries.find(id); - if (it == m_entries.end()) - return std::nullopt; - return decode_entry(it->first, it->second); + if (it != m_entries.end()) + return decode_entry(it->first, it->second); + + // Legacy config.json entries have no capability type. Keep them addressable by the new + // typed API until that capability is saved again. + if (id.type != PluginCapabilityType::Unknown) { + const auto legacy = m_entries.find({PluginCapabilityType::Unknown, id.name, id.plugin_key}); + if (legacy != m_entries.end()) + return decode_entry(id, legacy->second); + } + return std::nullopt; } -bool CapabilityConfigDocument::contains(const CapabilityConfigId& id) const +bool CapabilityConfigDocument::contains(const PluginCapabilityId& id) const { - return m_entries.find(id) != m_entries.end(); + return find(id).has_value(); } bool CapabilityConfigDocument::upsert(CapabilityConfigEntry entry) { - if (entry.id.plugin_key.empty() || entry.id.capability.empty()) + if (entry.id.empty()) return false; + if (entry.id.type != PluginCapabilityType::Unknown) + m_entries.erase({PluginCapabilityType::Unknown, entry.id.name, entry.id.plugin_key}); + nlohmann::json serialized = nlohmann::json::object(); const auto existing = m_entries.find(entry.id); if (existing != m_entries.end() && existing->second.is_object()) serialized = existing->second; serialized[KEY_PLUGIN] = entry.id.plugin_key; - serialized[KEY_CAPABILITY] = entry.id.capability; + serialized[KEY_CAPABILITY] = entry.id.name; + serialized[KEY_TYPE] = plugin_capability_type_to_string(entry.id.type); serialized[KEY_VERSION] = entry.plugin_version; - serialized[KEY_CAP_CONFIG] = entry.cap_config; + serialized[KEY_CAP_CONFIG] = entry.config; m_entries[entry.id] = std::move(serialized); return true; } -bool CapabilityConfigDocument::erase(const CapabilityConfigId& id) +bool CapabilityConfigDocument::erase(const PluginCapabilityId& id) { - return m_entries.erase(id) != 0; + bool erased = m_entries.erase(id) != 0; + if (id.type != PluginCapabilityType::Unknown) + erased = m_entries.erase({PluginCapabilityType::Unknown, id.name, id.plugin_key}) != 0 || erased; + return erased; } bool CapabilityConfigDocument::empty() const @@ -260,39 +256,34 @@ bool PluginConfig::save() return true; } -void PluginConfig::save_config(const std::string& plugin_key, - const std::string& capability_name, - const std::string& version, - const nlohmann::json& config) -{ save_config({plugin_key, capability_name, version, config}); } - -bool PluginConfig::store_capability_config(const std::string& plugin_key, - const std::string& capability_name, - const nlohmann::json& config) +bool PluginConfig::store_capability_config(const PluginCapabilityId& id, const nlohmann::json& config) { - save_config({plugin_key, capability_name, running_plugin_version(plugin_key), config}); + if (id.empty()) + return false; + + save_config({id, running_plugin_version(id.plugin_key), config}); return save(); } -void PluginConfig::save_config(const BaseConfig& config) +void PluginConfig::save_config(const CapabilityConfigEntry& config) { - if (config.empty()) { - BOOST_LOG_TRIVIAL(error) << "PluginConfig: refusing to store a config without a plugin key and capability name"; + if (config.id.empty()) { + BOOST_LOG_TRIVIAL(error) << "PluginConfig: refusing to store a config without a complete capability identity"; return; } std::lock_guard lock(m_mutex); - m_dirty = m_document.upsert(CapabilityConfigEntry{{config.plugin_key, config.capability_name}, config.plugin_version, config.config}) || m_dirty; + m_dirty = m_document.upsert(config) || m_dirty; } -bool PluginConfig::erase_capability_config(const std::string& plugin_key, const std::string& capability_name) +bool PluginConfig::erase_capability_config(const PluginCapabilityId& id) { - if (plugin_key.empty() || capability_name.empty()) + if (id.empty()) return false; { std::lock_guard lock(m_mutex); - if (!m_document.erase({plugin_key, capability_name})) + if (!m_document.erase(id)) return true; m_dirty = true; } @@ -300,19 +291,16 @@ bool PluginConfig::erase_capability_config(const std::string& plugin_key, const return save(); } -BaseConfig PluginConfig::get_config(const std::string& plugin_key, const std::string& capability_name) const +std::optional PluginConfig::get_config(const PluginCapabilityId& id) const { std::lock_guard lock(m_mutex); - const auto entry = m_document.find({plugin_key, capability_name}); - if (!entry) - return BaseConfig(); - return BaseConfig{entry->id.plugin_key, entry->id.capability, entry->plugin_version, entry->cap_config}; + return m_document.find(id); } -bool PluginConfig::has_config(const std::string& plugin_key, const std::string& capability_name) const +bool PluginConfig::has_config(const PluginCapabilityId& id) const { std::lock_guard lock(m_mutex); - return m_document.contains({plugin_key, capability_name}); + return m_document.contains(id); } bool PluginConfig::dirty() const @@ -358,22 +346,22 @@ EffectiveCapabilityConfig PresetPluginConfigService::get_effective_config(const const PluginCapabilityId& id) const { EffectiveCapabilityConfig result; - result.id = make_id(id); + result.id = id; result.running_plugin_version = running_plugin_version(id.plugin_key); - const BaseConfig base = PluginManager::instance().get_config().get_config(id.plugin_key, id.name); - result.has_base_config = !base.empty(); + const auto base = PluginManager::instance().get_config().get_config(id); + result.has_base_config = base.has_value(); if (const auto entry = overrides.find(result.id)) { result.has_preset_override = true; - result.config = entry->cap_config; + result.config = entry->config; result.stored_plugin_version = entry->plugin_version; return result; } if (result.has_base_config) { - result.config = base.config; - result.stored_plugin_version = base.plugin_version; + result.config = base->config; + result.stored_plugin_version = base->plugin_version; } return result; } @@ -382,20 +370,19 @@ MutationResult PresetPluginConfigService::set_preset_override(CapabilityConfigDo const PluginCapabilityId& id, const nlohmann::json& value) const { - MutationResult result; - const CapabilityConfigId config_id = make_id(id); - const std::string version = running_plugin_version(id.plugin_key); + MutationResult result; + const std::string version = running_plugin_version(id.plugin_key); // A no-op is a successful unchanged result: re-saving the displayed value must not mark the // preset dirty. - const auto existing = overrides.find(config_id); - if (existing && existing->cap_config == value && existing->plugin_version == version) { + const auto existing = overrides.find(id); + if (existing && existing->config == value && existing->plugin_version == version) { result.ok = true; result.effective = get_effective_config(overrides, id); return result; } - overrides.upsert({config_id, version, value}); + overrides.upsert({id, version, value}); result.ok = true; result.changed = true; @@ -408,7 +395,7 @@ MutationResult PresetPluginConfigService::remove_preset_override(CapabilityConfi { MutationResult result; result.ok = true; - result.changed = overrides.erase(make_id(id)); + result.changed = overrides.erase(id); result.effective = get_effective_config(overrides, id); return result; } @@ -458,21 +445,19 @@ nlohmann::json capability_get_config(const PluginCapabilityInterface& capability { // Shares its resolution with the dialogs, so a capability reads back exactly the config its UI // showed as effective. Stored in neither layer: an empty object, indexable unconditionally. - return active_capability_config(capability_full_identity(capability, "get_config")).config; + return active_capability_config(capability_identity(capability, "get_config")).config; } std::string capability_get_config_version(const PluginCapabilityInterface& capability) { // Must resolve through the same layer as get_config(), or a plugin would migrate one layer's // config by another's version stamp. - return active_capability_config(capability_full_identity(capability, "get_config_version")).stored_plugin_version; + return active_capability_config(capability_identity(capability, "get_config_version")).stored_plugin_version; } bool capability_save_config(const PluginCapabilityInterface& capability, const nlohmann::json& config) { - const auto [plugin_key, capability_name] = capability_identity(capability, "save_config"); - - return PluginManager::instance().get_config().store_capability_config(plugin_key, capability_name, config); + return PluginManager::instance().get_config().store_capability_config(capability_identity(capability, "save_config"), config); } nlohmann::json PluginConfig::capabilities_payload(const std::vector& caps) @@ -480,7 +465,7 @@ nlohmann::json PluginConfig::capabilities_payload(const std::vectorconfig; if (cap->config_ui_available()) { // A raising or empty get_config_ui() costs the capability only its custom UI: report the @@ -564,7 +550,7 @@ nlohmann::json PluginConfig::save_config_response(const PluginCapabilityId& id, response["ok"] = false; response["error"] = ""; - const auto cap = PluginManager::instance().get_plugin_capability(id.plugin_key, id.name, id.type, false); + const auto cap = PluginManager::instance().get_plugin_capability(id, false); if (!cap) { BOOST_LOG_TRIVIAL(warning) << "Refusing to save config for a capability that is no longer loaded. plugin_key=" << id.plugin_key << " capability_name=" << id.name; @@ -583,7 +569,7 @@ nlohmann::json PluginConfig::save_config_response(const PluginCapabilityId& id, } } - if (!PluginManager::instance().get_config().store_capability_config(id.plugin_key, id.name, parsed)) { + if (!PluginManager::instance().get_config().store_capability_config(id, parsed)) { BOOST_LOG_TRIVIAL(error) << "Failed to write the plugin config file. plugin_key=" << id.plugin_key << " capability_name=" << id.name; response["error"] = GUI::into_u8(_L("The configuration could not be written to disk. Your changes were not saved.")); @@ -594,7 +580,8 @@ nlohmann::json PluginConfig::save_config_response(const PluginCapabilityId& id, // Echo back what was persisted, not what the user typed, so the editor reloads from the store. response["ok"] = true; - response["config"] = PluginManager::instance().get_config().get_config(id.plugin_key, id.name).config; + if (const auto stored = PluginManager::instance().get_config().get_config(id)) + response["config"] = stored->config; return response; } @@ -610,7 +597,7 @@ nlohmann::json PluginConfig::restore_config_response(const PluginCapabilityId& i response["ok"] = false; response["error"] = ""; - const auto cap = PluginManager::instance().get_plugin_capability(id.plugin_key, id.name, id.type, false); + const auto cap = PluginManager::instance().get_plugin_capability(id, false); if (!cap) { BOOST_LOG_TRIVIAL(warning) << "Refusing to restore config for a capability that is no longer loaded. plugin_key=" << id.plugin_key << " capability_name=" << id.name; @@ -643,7 +630,7 @@ nlohmann::json PluginConfig::restore_config_response(const PluginCapabilityId& i return response; } - if (!PluginManager::instance().get_config().store_capability_config(id.plugin_key, id.name, defaults)) { + if (!PluginManager::instance().get_config().store_capability_config(id, defaults)) { BOOST_LOG_TRIVIAL(error) << "Failed to write the plugin config file while restoring defaults. plugin_key=" << id.plugin_key << " capability_name=" << id.name; response["error"] = GUI::into_u8(_L("The configuration could not be written to disk. Nothing was changed.")); @@ -654,7 +641,8 @@ nlohmann::json PluginConfig::restore_config_response(const PluginCapabilityId& i << " capability_name=" << id.name; response["ok"] = true; - response["config"] = PluginManager::instance().get_config().get_config(id.plugin_key, id.name).config; + if (const auto stored = PluginManager::instance().get_config().get_config(id)) + response["config"] = stored->config; return response; } diff --git a/src/slic3r/plugin/PluginConfig.hpp b/src/slic3r/plugin/PluginConfig.hpp index 9056137124..9d8b105af1 100644 --- a/src/slic3r/plugin/PluginConfig.hpp +++ b/src/slic3r/plugin/PluginConfig.hpp @@ -4,44 +4,23 @@ #include #include #include -#include +#include #include #include #include #include -#include #include #define PLUGIN_CONFIG_DIR "config.json" namespace Slic3r { -class PluginCapabilityInterface; class Preset; -struct PluginCapabilityId; - -struct CapabilityConfigId -{ - std::string plugin_key; - std::string capability; - - friend bool operator<(const CapabilityConfigId& lhs, const CapabilityConfigId& rhs) - { - return lhs.plugin_key < rhs.plugin_key || - (lhs.plugin_key == rhs.plugin_key && lhs.capability < rhs.capability); - } - - friend bool operator==(const CapabilityConfigId& lhs, const CapabilityConfigId& rhs) - { - return lhs.plugin_key == rhs.plugin_key && lhs.capability == rhs.capability; - } -}; - struct CapabilityConfigEntry { - CapabilityConfigId id; + PluginCapabilityId id; std::string plugin_version; - nlohmann::json cap_config = nlohmann::json::object(); + nlohmann::json config = nlohmann::json::object(); }; class CapabilityConfigDocument @@ -52,63 +31,28 @@ public: static CapabilityConfigDocument from_root_json(const nlohmann::json& root); static CapabilityConfigDocument from_entries(const nlohmann::json& entries); - std::optional find(const CapabilityConfigId& id) const; - bool contains(const CapabilityConfigId& id) const; + std::optional find(const PluginCapabilityId& id) const; + bool contains(const PluginCapabilityId& id) const; bool upsert(CapabilityConfigEntry entry); - bool erase(const CapabilityConfigId& id); + bool erase(const PluginCapabilityId& id); bool empty() const; nlohmann::json serialize_entries() const; nlohmann::json root_json() const; private: - std::map m_entries; + std::map m_entries; std::vector m_opaque_entries; }; -/* -Example config.json shape -{ - "config": [ - { - "plugin_key": "some_name", - "capability": "capability_name", - "plugin_version": "1.0.0", - "cap_config": { "plugin": "specific", "stuff": "here" } - } - ] -} -*/ - -struct BaseConfig { - std::string plugin_key; - std::string capability_name; - std::string plugin_version; - - nlohmann::json config; - - // True for the default-constructed instance returned by get_config() on a miss. - bool empty() const { return plugin_key.empty() || capability_name.empty(); } -}; - -// A preset keeps its plugin capability overrides as one raw JSON string in this ordinary -// ConfigOptionString, so the whole preset lifecycle — load, save, diff/dirty, inheritance, 3MF, -// sync — carries it for free. The plugin layer is the only thing that gives that string meaning. inline constexpr const char* PLUGIN_OVERRIDES_OPTION_KEY = "plugin_preference_overrides"; -// The preset's raw override text, or "" when it stores none. std::string plugin_overrides_of(const Preset& preset); - -// An empty string is a valid, empty document. Returns false and fills `error` when the text is -// present but is not a JSON array of entries; the caller then shows it and edits nothing. bool parse_plugin_overrides(const std::string& raw, CapabilityConfigDocument& document, std::string& error); - -// The document as compact JSON text, and "" once it holds no entries. Empty text — rather than a -// removed option — records "cleared here" against an inheriting parent that has overrides. std::string serialize_plugin_overrides(const CapabilityConfigDocument& document); struct EffectiveCapabilityConfig { - CapabilityConfigId id; + PluginCapabilityId id; nlohmann::json config = nlohmann::json::object(); bool has_preset_override = false; @@ -125,11 +69,6 @@ struct MutationResult EffectiveCapabilityConfig effective; }; -// Resolves a capability's effective config as `preset override -> base config -> none`, and mutates -// the override layer. It works on a CapabilityConfigDocument the caller owns, never on a Preset and -// never on the base config file, which keeps the two layers from writing to each other: -// PluginConfigField holds the document and feeds the edited text back through the normal field/dirty -// pipeline, so the preset is written the way every other setting is. class PresetPluginConfigService { public: @@ -142,64 +81,28 @@ public: const PluginCapabilityId& id) const; }; -// The same resolution against the preset that is active right now, rather than a document the -// caller holds: this is what a running capability reads through the Python config API. It falls -// back to the base config when no active preset bundle is available. EffectiveCapabilityConfig active_capability_config(const PluginCapabilityId& id); -// Store for every plugin capability's configuration, persisted as a single config.json alongside the -// installed plugins. The shape of `cap_config` belongs to the plugin; this class only round-trips it. -// -// A capability is identified by (plugin_key, capability_name). `plugin_version` records which version -// last wrote the entry, so an upgraded plugin can spot a stale config and migrate it. It is -// deliberately not part of the identity: upgrading a plugin must not reset the user's settings. -// -// Plugin code runs on worker threads, so every entry point is mutex-guarded. class PluginConfig { public: static const std::string plugin_config_file() { return (boost::filesystem::path(get_orca_plugins_dir()) / PLUGIN_CONFIG_DIR).string(); } - - // A missing or malformed file leaves the store empty rather than throwing: a bad plugin config - // must not block startup. void load(); - - // Rewrites config.json atomically. False means the config on disk is unchanged. bool save(); - void save_config(const std::string& plugin_key, const std::string& capability_name, const std::string& version, const nlohmann::json& config); - void save_config(const BaseConfig& config); + void save_config(const CapabilityConfigEntry& config); - // Replaces one capability's cap_config and writes config.json straight away, stamping the entry - // with the plugin version currently running. Every other entry is round-tripped untouched, so - // saving one capability cannot disturb another's config. - bool store_capability_config(const std::string& plugin_key, const std::string& capability_name, const nlohmann::json& config); - bool erase_capability_config(const std::string& plugin_key, const std::string& capability_name); + bool store_capability_config(const PluginCapabilityId& id, const nlohmann::json& config); + bool erase_capability_config(const PluginCapabilityId& id); - // A default-constructed BaseConfig (see BaseConfig::empty) when there is no stored config. - BaseConfig get_config(const std::string& plugin_key, const std::string& capability_name) const; - bool has_config(const std::string& plugin_key, const std::string& capability_name) const; + std::optional get_config(const PluginCapabilityId& id) const; + bool has_config(const PluginCapabilityId& id) const; bool dirty() const; - // ---- Webview-facing helpers, shared by PluginsDialog's Config tab and PluginsConfigDialog ---- - // Static because a capability's config is addressed globally by (plugin_key, capability_name) - // through PluginManager's store, not through any one PluginConfig instance. The caller owns the - // UI: it confirms destructive restores and shows status toasts. - - // The config sidebar's rows, in the order given. Capabilities no longer loaded are skipped — the - // sidebar only offers what can actually be configured. static nlohmann::json capabilities_payload(const std::vector& caps); - - // One capability's stored config, plus its custom HTML UI when it provides one. - static nlohmann::json get_config_response(const PluginCapabilityId& id); - - // Persists one capability's config. `config` is either text from the default editor (re-parsed - // here, so malformed JSON can never reach config.json) or a structured value from a custom UI. + static nlohmann::json get_config_response(const PluginCapabilityId& id);tom UI. static nlohmann::json save_config_response(const PluginCapabilityId& id, const nlohmann::json& config); - - // Overwrites one capability's stored config with its get_default_config(). The caller must have - // confirmed with the user first — this does not ask. static nlohmann::json restore_config_response(const PluginCapabilityId& id); private: @@ -208,21 +111,8 @@ private: bool m_dirty = false; }; -// Host implementations behind the capability-level Python config API (bound onto every capability -// class in PythonPluginBridge). The capability addresses only itself: the (plugin_key, -// capability_name) pair is read off the instance the call arrived on, never passed in from Python, -// so a capability cannot reach another capability's config. Throws std::runtime_error (RuntimeError -// in Python) on an unmaterialized instance. - -// The cap_config resolved as the config UI presents it: the active preset's override when it has -// one, otherwise the config stored here (see active_capability_config). Empty object when neither. nlohmann::json capability_get_config(const PluginCapabilityInterface& capability); -// The plugin version that wrote the config get_config() returns — the same layer it came from — so a -// plugin can migrate a stale cap_config. Empty when the capability has no stored config. std::string capability_get_config_version(const PluginCapabilityInterface& capability); -// Replaces cap_config and persists. Always writes the store, never a preset: presets are the user's -// to edit, and a plugin saving from a worker thread cannot mark one dirty. A capability whose active -// preset overrides it will therefore keep reading that override back, not what it saved. bool capability_save_config(const PluginCapabilityInterface& capability, const nlohmann::json& config); } // namespace Slic3r diff --git a/src/slic3r/plugin/PluginManager.cpp b/src/slic3r/plugin/PluginManager.cpp index 1d43e58bfc..2dee799575 100644 --- a/src/slic3r/plugin/PluginManager.cpp +++ b/src/slic3r/plugin/PluginManager.cpp @@ -511,21 +511,18 @@ std::vector> PluginManager::get_plugi return result; } -std::shared_ptr PluginManager::get_plugin_capability(const std::string& plugin_key, - const std::string& capability_name, - PluginCapabilityType type, - bool only_enabled) const +std::shared_ptr PluginManager::get_plugin_capability(const PluginCapabilityId& id, bool only_enabled) const { std::lock_guard lock(m_mutex); - const Plugin* plugin = find_plugin_locked(plugin_key); + const Plugin* plugin = find_plugin_locked(id.plugin_key); if (plugin == nullptr) return nullptr; for (const auto& capability : plugin->capabilities) { - if (!capability || capability->name() != capability_name) + if (!capability || capability->name() != id.name) continue; - if (type != PluginCapabilityType::Unknown && capability->type() != type) + if (id.type != PluginCapabilityType::Unknown && capability->type() != id.type) continue; if (only_enabled && !capability->is_enabled()) continue; @@ -740,7 +737,7 @@ void PluginManager::load_plugin(const std::string& plugin_key, bool skip_deps, s for (const std::string& capability_name : capabilities_to_enable) { if (std::find(loaded_capability_names.begin(), loaded_capability_names.end(), capability_name) != loaded_capability_names.end()) - set_capability_enabled(plugin_id, capability_name, true); + set_capability_enabled({PluginCapabilityType::Unknown, capability_name, plugin_id}, true); } run_on_load_callbacks(plugin_id); @@ -852,10 +849,11 @@ void PluginManager::load_plugin_impl(const std::string& plugin_key, bool skip_de auto config = cap->get_default_config(); if (config.empty()) continue; - if (m_config.has_config(plugin_key, cap->name())) + const PluginCapabilityId id = cap->identity(); + if (m_config.has_config(id)) continue; - m_config.save_config(plugin_key, cap->name(), plugin.descriptor.installed_version, config); + m_config.save_config({id, plugin.descriptor.installed_version, config}); } bool committed = false; @@ -1007,8 +1005,7 @@ void PluginManager::unload_cloud_plugins() } // ── Enable state ──────────────────────────────────────────────────────────────────────────── - -void PluginManager::set_capability_enabled(const std::string& plugin_key, const std::string& capability_name, bool enabled) +void PluginManager::set_capability_enabled(const PluginCapabilityId& id, bool enabled) { PluginCapabilityId changed; bool did_change = false; @@ -1016,15 +1013,18 @@ void PluginManager::set_capability_enabled(const std::string& plugin_key, const { std::lock_guard lock(m_mutex); - Plugin* plugin = find_plugin_locked(plugin_key); + Plugin* plugin = find_plugin_locked(id.plugin_key); if (plugin == nullptr || !plugin->is_loaded()) return; for (const auto& capability : plugin->capabilities) { - if (!capability || capability->name() != capability_name || capability->is_enabled() == enabled) + if (!capability || capability->name() != id.name || + (id.type != PluginCapabilityType::Unknown && capability->type() != id.type) || + capability->is_enabled() == enabled) continue; + capability->set_enabled(enabled); - changed = PluginCapabilityId{capability->type(), capability->name(), plugin_key}; + changed = capability->identity(); did_change = true; break; } @@ -1033,7 +1033,7 @@ void PluginManager::set_capability_enabled(const std::string& plugin_key, const if (!did_change) return; - write_loaded_plugin_install_state(plugin_key); + write_loaded_plugin_install_state(id.plugin_key); if (enabled) run_on_capability_load_callbacks(changed); @@ -1043,6 +1043,8 @@ void PluginManager::set_capability_enabled(const std::string& plugin_key, const void PluginManager::write_loaded_plugin_install_state(const std::string& plugin_key) { + std::lock_guard state_lock(m_install_state_mutex); + PluginDescriptor descriptor; std::vector> capabilities; { @@ -1950,7 +1952,7 @@ ExecutionResult PluginManager::run_script_capability(const std::string& plugin_k return {}; } - auto cap = get_plugin_capability(plugin_key, capability_name, PluginCapabilityType::Script); + auto cap = get_plugin_capability({PluginCapabilityType::Script, capability_name, plugin_key}); if (!cap) return {}; diff --git a/src/slic3r/plugin/PluginManager.hpp b/src/slic3r/plugin/PluginManager.hpp index 153e8cefc9..84a5c062e1 100644 --- a/src/slic3r/plugin/PluginManager.hpp +++ b/src/slic3r/plugin/PluginManager.hpp @@ -29,20 +29,6 @@ namespace Slic3r { class OrcaCloudServiceAgent; -// Identity of a single capability, as published to lifecycle subscribers. Purely a message -// payload — capabilities are looked up by (plugin_key, name) linear scan, so unlike the registry -// key this replaces, it needs no hash. Equality is provided for the plugin-config side, which -// de-duplicates the "capabilities in use" list (PluginResolver) with std::unique. -struct PluginCapabilityId -{ - PluginCapabilityType type = PluginCapabilityType::Unknown; - std::string name; - std::string plugin_key; // owning package - - bool operator==(const PluginCapabilityId& o) const - { return type == o.type && name == o.name && plugin_key == o.plugin_key; } -}; - // One discovered plugin package: one .py/.whl file -> one descriptor + one Python module + // N materialized capabilities. // @@ -157,10 +143,9 @@ public: const std::string& plugin_key = "", // "" => all plugins PluginCapabilityType type = PluginCapabilityType::Unknown, // Unknown => all types bool only_enabled = true) const; - std::shared_ptr get_plugin_capability(const std::string& plugin_key, - const std::string& capability_name, - PluginCapabilityType type = PluginCapabilityType::Unknown, - bool only_enabled = true) const; + std::shared_ptr get_plugin_capability(const PluginCapabilityId& id, bool only_enabled = true) const; + + // Try to resolve from just capability name and type from presets. std::shared_ptr get_plugin_capability(const std::string& capability_name, PluginCapabilityType type = PluginCapabilityType::Unknown, bool only_enabled = true) const; @@ -177,7 +162,7 @@ public: bool cancel_plugin_load(const std::string& plugin_key); std::string get_plugin_load_error(const std::string& plugin_key) const; - void set_capability_enabled(const std::string& plugin_key, const std::string& capability_name, bool enabled); + void set_capability_enabled(const PluginCapabilityId& id, bool enabled); // Sets the cloud user whose _subscribed/{user_id} directory is scanned and installed into. void set_cloud_user(const std::string& user_id); @@ -290,6 +275,9 @@ private: std::map m_load_errors; mutable std::condition_variable m_load_cv; + // Serialize sidecar snapshots so concurrent capability toggles cannot write stale state out of order. + mutable std::mutex m_install_state_mutex; + std::map> m_callbacks; std::map> m_capability_callbacks; @@ -346,7 +334,7 @@ void execute_capabilities_from_refs(const ConfigOptionStrings& capabilities, // only_enabled = false so that "not loaded" and "loaded but disabled" stay distinguishable // and each keeps its own diagnostic. - auto cap = plugin_mgr.get_plugin_capability(plugin_key, cap_name, type, /*only_enabled=*/false); + auto cap = plugin_mgr.get_plugin_capability({type, cap_name, plugin_key}, /*only_enabled=*/false); if (!cap) { BOOST_LOG_TRIVIAL(warning) << tag << ": no loaded capability '" << cap_name << "' for plugin '" << plugin_key << "'; skipping"; continue; diff --git a/src/slic3r/plugin/PluginResolver.cpp b/src/slic3r/plugin/PluginResolver.cpp index 2972b9edb3..4ba58ee550 100644 --- a/src/slic3r/plugin/PluginResolver.cpp +++ b/src/slic3r/plugin/PluginResolver.cpp @@ -31,7 +31,10 @@ namespace { // The tracked option in `preset` whose value references `ref`'s capability, or "" when none does. // Doubles as the "Jump to" target and as the signal that the plugin is still required: a missing // plugin with no referencing option is dropped from the set. -std::string find_option_for_capability(Preset::Type type, const Preset& preset, const PluginCapabilityRef& ref) +std::string find_option_for_capability(Preset::Type type, + const Preset& preset, + const PluginCapabilityRef& ref, + PluginCapabilityType capability_type = PluginCapabilityType::Unknown) { if (type != Preset::TYPE_PRINT && type != Preset::TYPE_PRINTER && type != Preset::TYPE_FILAMENT) return {}; @@ -42,13 +45,15 @@ std::string find_option_for_capability(Preset::Type type, const Preset& preset, if (def == nullptr) return {}; + const std::string expected_type = plugin_capability_type_to_string(capability_type); const auto matches_ref = [&ref](const std::string& value) { return value == ref.capability_name; }; for (const std::string& field : preset.config.keys()) { const ConfigOptionDef* opt_def = def->get(field); - if (opt_def == nullptr || !opt_def->is_plugin_backed()) + if (opt_def == nullptr || !opt_def->is_plugin_backed() || + (capability_type != PluginCapabilityType::Unknown && opt_def->plugin_type != expected_type)) continue; const ConfigOption* option = preset.config.option(field); @@ -70,8 +75,12 @@ std::string find_option_for_capability(Preset::Type type, const Preset& preset, // printer_agent stores AgentInfo::id, so a missing plugin cannot be reverse-mapped through // the runtime registry. If no regular printer plugin field matched, assume printer_agent. - if (type == Preset::Type::TYPE_PRINTER && preset.config.has("printer_agent")) + if (type == Preset::Type::TYPE_PRINTER && preset.config.has("printer_agent")) { + const ConfigOptionDef* agent_def = def->get("printer_agent"); + if (agent_def != nullptr && agent_def->is_plugin_backed() && + (capability_type == PluginCapabilityType::Unknown || agent_def->plugin_type == expected_type)) return "printer_agent"; + } return {}; } @@ -129,10 +138,13 @@ void collect_capabilities_in_use(Preset::Type type, const Preset& preset, std::v if (!PluginManager::instance().try_get_plugin_descriptor(key, descriptor)) continue; - // The loaded capability is also what supplies the type: the manifest ref does not carry one. + // The manifest ref does not carry a type, so require the live capability type to match the + // type declared by the option that references it. for (const auto& capability : PluginManager::instance().get_plugin_capabilities(descriptor.plugin_key, PluginCapabilityType::Unknown, false)) - if (capability && capability->name() == ref.capability_name) - out.push_back(PluginCapabilityId{capability->type(), capability->name(), capability->audit_plugin_key()}); + if (capability && capability->type() != PluginCapabilityType::Unknown && + capability->name() == ref.capability_name && + !find_option_for_capability(type, preset, ref, capability->type()).empty()) + out.push_back(capability->identity()); } } @@ -206,7 +218,7 @@ static std::pair loaded_capability_state(const std::string& plugin_k if (!mgr.is_plugin_loaded(plugin_key)) return {false, false}; - const auto capability = mgr.get_plugin_capability(plugin_key, ref.capability_name, PluginCapabilityType::Unknown, + const auto capability = mgr.get_plugin_capability({PluginCapabilityType::Unknown, ref.capability_name, plugin_key}, /*only_enabled=*/false); if (!capability) return {false, false}; diff --git a/src/slic3r/plugin/PluginResolver.hpp b/src/slic3r/plugin/PluginResolver.hpp index 00997edaaf..2f856a7599 100644 --- a/src/slic3r/plugin/PluginResolver.hpp +++ b/src/slic3r/plugin/PluginResolver.hpp @@ -4,8 +4,7 @@ #include // PluginCapabilityRef, parse_capability_ref #include // Preset::Type #include -#include // PluginCapabilityId -#include // PluginCapabilityType +#include // PluginCapabilityId, PluginCapabilityType #include #include #include diff --git a/src/slic3r/plugin/PythonPluginInterface.hpp b/src/slic3r/plugin/PythonPluginInterface.hpp index 82c38018bd..4a6df06441 100644 --- a/src/slic3r/plugin/PythonPluginInterface.hpp +++ b/src/slic3r/plugin/PythonPluginInterface.hpp @@ -14,6 +14,27 @@ namespace Slic3r { enum class PluginCapabilityType { PrinterConnection = 0, Automation, Analysis, Importer, Exporter, Visualization, Script, SlicingPipeline, Unknown }; +struct PluginCapabilityId +{ + PluginCapabilityType type = PluginCapabilityType::Unknown; + std::string name; + std::string plugin_key; + + bool empty() const { return type == PluginCapabilityType::Unknown || name.empty() || plugin_key.empty(); } + friend bool operator==(const PluginCapabilityId& lhs, const PluginCapabilityId& rhs) + { + return lhs.type == rhs.type && lhs.name == rhs.name && lhs.plugin_key == rhs.plugin_key; + } + friend bool operator<(const PluginCapabilityId& lhs, const PluginCapabilityId& rhs) + { + if (lhs.plugin_key != rhs.plugin_key) + return lhs.plugin_key < rhs.plugin_key; + if (lhs.name != rhs.name) + return lhs.name < rhs.name; + return lhs.type < rhs.type; + } +}; + inline std::string plugin_capability_type_to_string(PluginCapabilityType type) { switch (type) { @@ -157,6 +178,7 @@ public: // from Python can tell which capability they are serving. const std::string& name() const { return m_name; } PluginCapabilityType type() const { return m_type; } + PluginCapabilityId identity() const { return {m_type, m_name, m_audit_plugin_key}; } void set_resolved_identity(std::string name, PluginCapabilityType type) { m_name = std::move(name); diff --git a/tests/slic3rutils/test_plugin_capability_config.cpp b/tests/slic3rutils/test_plugin_capability_config.cpp index 814abb027e..40034d2591 100644 --- a/tests/slic3rutils/test_plugin_capability_config.cpp +++ b/tests/slic3rutils/test_plugin_capability_config.cpp @@ -43,7 +43,8 @@ py::module_ import_orca_module() py::object make_capability(const std::string& class_name, const std::string& body, const std::string& plugin_key, - const std::string& capability_name) + const std::string& capability_name, + PluginCapabilityType type = PluginCapabilityType::Script) { // Import first: it brings the interpreter up, and any py:: object built before it would touch a // Python that does not exist yet. @@ -57,7 +58,6 @@ py::object make_capability(const std::string& class_name, if (!plugin_key.empty()) { auto iface = instance.cast>(); - const PluginCapabilityType type = iface->get_type(); iface->set_audit_plugin_key(plugin_key); iface->set_resolved_identity(capability_name, type); } @@ -77,6 +77,11 @@ json py_get_config(const py::object& cap) { return json::parse(cap.attr("get_con bool py_save_config(const py::object& cap, const json& value) { return cap.attr("save_config")(value.dump()).cast(); } +PluginCapabilityId capability_id(PluginCapabilityType type, const char* name, const char* plugin_key) +{ + return {type, name, plugin_key}; +} + } // namespace TEST_CASE("Capability config API is exposed on every Python capability", "[PluginConfig][Python]") @@ -116,9 +121,10 @@ TEST_CASE("get_config returns only cap_config and save_config persists it", "[Pl REQUIRE(py_save_config(cap, json{{"speed", 5}, {"name", "fast"}})); - const BaseConfig stored = host_config().get_config("plugin_a", "cap_a"); - REQUIRE_FALSE(stored.empty()); - CHECK(stored.config == json{{"speed", 5}, {"name", "fast"}}); + const auto stored = host_config().get_config(capability_id(PluginCapabilityType::Script, "cap_a", "plugin_a")); + REQUIRE(stored); + CHECK(stored->id == capability_id(PluginCapabilityType::Script, "cap_a", "plugin_a")); + CHECK(stored->config == json{{"speed", 5}, {"name", "fast"}}); // Python reads back exactly cap_config — no host metadata. const json reloaded = py_get_config(cap); @@ -141,7 +147,7 @@ TEST_CASE("save_config rejects a string that is not valid JSON", "[PluginConfig] // Refusing unparseable text must leave the previously stored config alone. CHECK_FALSE(cap.attr("save_config")("{not json").cast()); - CHECK(host_config().get_config("plugin_a", "cap_a").config == json{{"keep", "me"}}); + CHECK(host_config().get_config(capability_id(PluginCapabilityType::Script, "cap_a", "plugin_a"))->config == json{{"keep", "me"}}); } TEST_CASE("Saving one capability's config does not touch another's", "[PluginConfig][Python]") @@ -162,9 +168,19 @@ TEST_CASE("Saving one capability's config does not touch another's", "[PluginCon REQUIRE(py_save_config(a_cap1, json{{"value", 99}})); - CHECK(host_config().get_config("plugin_a", "cap_a").config == json{{"value", 99}}); - CHECK(host_config().get_config("plugin_a", "cap_b").config == json{{"value", 2}}); - CHECK(host_config().get_config("plugin_b", "cap_a").config == json{{"value", 3}}); + CHECK(host_config().get_config(capability_id(PluginCapabilityType::Script, "cap_a", "plugin_a"))->config == json{{"value", 99}}); + CHECK(host_config().get_config(capability_id(PluginCapabilityType::Script, "cap_b", "plugin_a"))->config == json{{"value", 2}}); + CHECK(host_config().get_config(capability_id(PluginCapabilityType::Script, "cap_a", "plugin_b"))->config == json{{"value", 3}}); + + // A shared name under one plugin must remain isolated when the capability type differs. + py::object importer = make_capability("IsoCapImporter", body, "plugin_a", "cap_a", PluginCapabilityType::Importer); + REQUIRE(py_save_config(importer, json{{"value", 4}})); + CHECK(host_config().get_config(capability_id(PluginCapabilityType::Script, "cap_a", "plugin_a"))->config == json{{"value", 99}}); + CHECK(host_config().get_config(capability_id(PluginCapabilityType::Importer, "cap_a", "plugin_a"))->config == json{{"value", 4}}); + + host_config().load(); + CHECK(host_config().get_config(capability_id(PluginCapabilityType::Script, "cap_a", "plugin_a"))->config == json{{"value", 99}}); + CHECK(host_config().get_config(capability_id(PluginCapabilityType::Importer, "cap_a", "plugin_a"))->config == json{{"value", 4}}); CHECK(py_get_config(a_cap2).at("value") == 2); CHECK(py_get_config(b_cap1).at("value") == 3); @@ -213,7 +229,7 @@ TEST_CASE("A capability that omits the config UI hooks gets the default editor", CHECK(iface->get_config_ui().empty()); REQUIRE(py_save_config(bare, json{{"speed", 5}})); - CHECK(host_config().get_config("plugin_a", "cap_a").config == json{{"speed", 5}}); + CHECK(host_config().get_config(capability_id(PluginCapabilityType::Script, "cap_a", "plugin_a"))->config == json{{"speed", 5}}); } TEST_CASE("get_default_config supplies the value Restore defaults writes back", "[PluginConfig][Python]") @@ -289,10 +305,10 @@ TEST_CASE("Restoring defaults overwrites only the target capability", "[PluginCo // What PluginsDialog::restore_capability_config does: ask the capability, store the answer. auto iface = as_interface(target); - REQUIRE(host_config().store_capability_config("plugin_a", "cap_a", iface->get_default_config())); + REQUIRE(host_config().store_capability_config(capability_id(PluginCapabilityType::Script, "cap_a", "plugin_a"), iface->get_default_config())); - CHECK(host_config().get_config("plugin_a", "cap_a").config == json{{"speed", 1}}); - CHECK(host_config().get_config("plugin_b", "cap_a").config == json{{"speed", 99}}); + CHECK(host_config().get_config(capability_id(PluginCapabilityType::Script, "cap_a", "plugin_a"))->config == json{{"speed", 1}}); + CHECK(host_config().get_config(capability_id(PluginCapabilityType::Script, "cap_a", "plugin_b"))->config == json{{"speed", 99}}); } TEST_CASE("A raising get_default_config leaves the stored config untouched", "[PluginConfig][Python]") @@ -312,7 +328,7 @@ TEST_CASE("A raising get_default_config leaves the stored config untouched", "[P CHECK_THROWS_AS(iface->get_default_config(), py::error_already_set); // The dialog stores nothing when the hook throws: a broken plugin must not wipe user settings. - CHECK(host_config().get_config("plugin_a", "cap_a").config == json{{"keep", "me"}}); + CHECK(host_config().get_config(capability_id(PluginCapabilityType::Script, "cap_a", "plugin_a"))->config == json{{"keep", "me"}}); } TEST_CASE("A raising config UI hook surfaces as an exception the host can catch", "[PluginConfig][Python]") diff --git a/tests/slic3rutils/test_plugin_config.cpp b/tests/slic3rutils/test_plugin_config.cpp index 4ee18e85b2..99de5e53db 100644 --- a/tests/slic3rutils/test_plugin_config.cpp +++ b/tests/slic3rutils/test_plugin_config.cpp @@ -2,6 +2,7 @@ #include #include +#include #include "plugin_test_utils.hpp" @@ -17,6 +18,11 @@ using json = nlohmann::json; namespace { +PluginCapabilityId capability_id(PluginCapabilityType type, const char* name, const char* plugin_key) +{ + return {type, name, plugin_key}; +} + json read_config_file() { boost::nowide::ifstream ifs(PluginConfig::plugin_config_file().c_str()); @@ -40,24 +46,24 @@ TEST_CASE("PluginConfig creates, reads back and persists a capability config", " ScopedDataDir data_dir_guard("plugin-config-roundtrip"); PluginConfig config; + const PluginCapabilityId id = capability_id(PluginCapabilityType::Script, "cap_a", "plugin_a"); // A capability nobody has configured yet reads as an empty record rather than throwing. - CHECK_FALSE(config.has_config("plugin_a", "cap_a")); - CHECK(config.get_config("plugin_a", "cap_a").empty()); + CHECK_FALSE(config.has_config(id)); + CHECK_FALSE(config.get_config(id)); - REQUIRE(config.store_capability_config("plugin_a", "cap_a", json{{"speed", 5}})); + REQUIRE(config.store_capability_config(id, json{{"speed", 5}})); - const BaseConfig stored = config.get_config("plugin_a", "cap_a"); - REQUIRE_FALSE(stored.empty()); - CHECK(stored.plugin_key == "plugin_a"); - CHECK(stored.capability_name == "cap_a"); - CHECK(stored.config == json{{"speed", 5}}); - CHECK(config.has_config("plugin_a", "cap_a")); + const auto stored = config.get_config(id); + REQUIRE(stored); + CHECK(stored->id == id); + CHECK(stored->config == json{{"speed", 5}}); + CHECK(config.has_config(id)); // store_capability_config writes through, so a fresh instance (a restart, in effect) sees it. PluginConfig reloaded; reloaded.load(); - CHECK(reloaded.get_config("plugin_a", "cap_a").config == json{{"speed", 5}}); + CHECK(reloaded.get_config(id)->config == json{{"speed", 5}}); } TEST_CASE("PluginConfig updates only the target capability's cap_config", "[PluginConfig]") @@ -65,23 +71,44 @@ TEST_CASE("PluginConfig updates only the target capability's cap_config", "[Plug ScopedDataDir data_dir_guard("plugin-config-isolation"); PluginConfig config; - // The identity is the (plugin_key, capability) pair, so all three below are separate records. - REQUIRE(config.store_capability_config("plugin_a", "cap_a", json{{"value", 1}})); - REQUIRE(config.store_capability_config("plugin_a", "cap_b", json{{"value", 2}})); - REQUIRE(config.store_capability_config("plugin_b", "cap_a", json{{"value", 3}})); + const PluginCapabilityId a_a = capability_id(PluginCapabilityType::Script, "cap_a", "plugin_a"); + const PluginCapabilityId a_b = capability_id(PluginCapabilityType::Script, "cap_b", "plugin_a"); + const PluginCapabilityId b_a = capability_id(PluginCapabilityType::Script, "cap_a", "plugin_b"); + // The identity is the full (type, capability, plugin_key) tuple, so all three below are separate records. + REQUIRE(config.store_capability_config(a_a, json{{"value", 1}})); + REQUIRE(config.store_capability_config(a_b, json{{"value", 2}})); + REQUIRE(config.store_capability_config(b_a, json{{"value", 3}})); - REQUIRE(config.store_capability_config("plugin_a", "cap_a", json{{"value", 99}})); + REQUIRE(config.store_capability_config(a_a, json{{"value", 99}})); - CHECK(config.get_config("plugin_a", "cap_a").config == json{{"value", 99}}); - CHECK(config.get_config("plugin_a", "cap_b").config == json{{"value", 2}}); - CHECK(config.get_config("plugin_b", "cap_a").config == json{{"value", 3}}); + CHECK(config.get_config(a_a)->config == json{{"value", 99}}); + CHECK(config.get_config(a_b)->config == json{{"value", 2}}); + CHECK(config.get_config(b_a)->config == json{{"value", 3}}); // The same holds on disk, not just in memory. PluginConfig reloaded; reloaded.load(); - CHECK(reloaded.get_config("plugin_a", "cap_a").config == json{{"value", 99}}); - CHECK(reloaded.get_config("plugin_a", "cap_b").config == json{{"value", 2}}); - CHECK(reloaded.get_config("plugin_b", "cap_a").config == json{{"value", 3}}); + CHECK(reloaded.get_config(a_a)->config == json{{"value", 99}}); + CHECK(reloaded.get_config(a_b)->config == json{{"value", 2}}); + CHECK(reloaded.get_config(b_a)->config == json{{"value", 3}}); +} + +TEST_CASE("PluginConfig isolates same-name capabilities by type", "[PluginConfig]") +{ + ScopedDataDir data_dir_guard("plugin-config-type-isolation"); + const PluginCapabilityId script = capability_id(PluginCapabilityType::Script, "shared", "plugin_a"); + const PluginCapabilityId importer = capability_id(PluginCapabilityType::Importer, "shared", "plugin_a"); + + PluginConfig config; + REQUIRE(config.store_capability_config(script, json{{"value", "script"}})); + REQUIRE(config.store_capability_config(importer, json{{"value", "importer"}})); + CHECK(config.get_config(script)->config == json{{"value", "script"}}); + CHECK(config.get_config(importer)->config == json{{"value", "importer"}}); + + PluginConfig reloaded; + reloaded.load(); + CHECK(reloaded.get_config(script)->config == json{{"value", "script"}}); + CHECK(reloaded.get_config(importer)->config == json{{"value", "importer"}}); } TEST_CASE("PluginConfig serializes the documented on-disk schema", "[PluginConfig]") @@ -89,7 +116,8 @@ TEST_CASE("PluginConfig serializes the documented on-disk schema", "[PluginConfi ScopedDataDir data_dir_guard("plugin-config-schema"); PluginConfig config; - REQUIRE(config.store_capability_config("plugin_a", "cap_a", json{{"speed", 5}})); + const PluginCapabilityId id = capability_id(PluginCapabilityType::Script, "cap_a", "plugin_a"); + REQUIRE(config.store_capability_config(id, json{{"speed", 5}})); // Locks the field names: an existing config.json must keep loading after any future change. const json root = read_config_file(); @@ -100,10 +128,11 @@ TEST_CASE("PluginConfig serializes the documented on-disk schema", "[PluginConfi const json& entry = root.at("config").front(); CHECK(entry.at("plugin_key") == "plugin_a"); CHECK(entry.at("capability") == "cap_a"); + CHECK(entry.at("capability_type") == "script"); CHECK(entry.at("cap_config") == json{{"speed", 5}}); CHECK(entry.contains("plugin_version")); // Only cap_config is user data; the rest of the record is host-managed. - CHECK(entry.size() == 4); + CHECK(entry.size() == 5); } TEST_CASE("PluginConfig keeps a capability's config after its plugin goes away", "[PluginConfig]") @@ -112,14 +141,14 @@ TEST_CASE("PluginConfig keeps a capability's config after its plugin goes away", { PluginConfig config; - REQUIRE(config.store_capability_config("plugin_a", "cap_a", json{{"token", "keep me"}})); + REQUIRE(config.store_capability_config(capability_id(PluginCapabilityType::Script, "cap_a", "plugin_a"), json{{"token", "keep me"}})); } // config.json is deliberately not keyed to installed plugins: a record outlives its plugin and is // still there on reinstall. Asserts no cleanup path silently drops it. PluginConfig after_removal; after_removal.load(); - CHECK(after_removal.get_config("plugin_a", "cap_a").config == json{{"token", "keep me"}}); + CHECK(after_removal.get_config(capability_id(PluginCapabilityType::Script, "cap_a", "plugin_a"))->config == json{{"token", "keep me"}}); } TEST_CASE("PluginConfig treats a missing config file as an empty store", "[PluginConfig]") @@ -130,7 +159,7 @@ TEST_CASE("PluginConfig treats a missing config file as an empty store", "[Plugi PluginConfig config; REQUIRE_NOTHROW(config.load()); - CHECK_FALSE(config.has_config("plugin_a", "cap_a")); + CHECK_FALSE(config.has_config(capability_id(PluginCapabilityType::Script, "cap_a", "plugin_a"))); CHECK_FALSE(config.dirty()); } @@ -143,7 +172,7 @@ TEST_CASE("PluginConfig survives a malformed config file", "[PluginConfig]") PluginConfig config; REQUIRE_NOTHROW(config.load()); // a bad config must not block startup - CHECK_FALSE(config.has_config("plugin_a", "cap_a")); + CHECK_FALSE(config.has_config(capability_id(PluginCapabilityType::Script, "cap_a", "plugin_a"))); } SECTION("valid JSON without the entries array") @@ -153,7 +182,7 @@ TEST_CASE("PluginConfig survives a malformed config file", "[PluginConfig]") PluginConfig config; REQUIRE_NOTHROW(config.load()); - CHECK_FALSE(config.has_config("plugin_a", "cap_a")); + CHECK_FALSE(config.has_config(capability_id(PluginCapabilityType::Script, "cap_a", "plugin_a"))); } SECTION("entries without an identity are skipped, the rest still load") @@ -161,26 +190,46 @@ TEST_CASE("PluginConfig survives a malformed config file", "[PluginConfig]") ScopedDataDir data_dir_guard("plugin-config-partial"); write_config_file(R"({"config": [ {"cap_config": {"orphan": true}}, - {"plugin_key": "plugin_a", "capability": "cap_a", "plugin_version": "1.0.0", "cap_config": {"kept": true}} + {"plugin_key": "plugin_a", "capability": "cap_a", "capability_type": "script", "plugin_version": "1.0.0", "cap_config": {"kept": true}} ]})"); PluginConfig config; REQUIRE_NOTHROW(config.load()); - CHECK(config.get_config("plugin_a", "cap_a").config == json{{"kept", true}}); - CHECK(config.get_config("plugin_a", "cap_a").plugin_version == "1.0.0"); + CHECK(config.get_config(capability_id(PluginCapabilityType::Script, "cap_a", "plugin_a"))->config == json{{"kept", true}}); + CHECK(config.get_config(capability_id(PluginCapabilityType::Script, "cap_a", "plugin_a"))->plugin_version == "1.0.0"); } SECTION("an entry with no cap_config reads as an empty object") { ScopedDataDir data_dir_guard("plugin-config-nocap"); write_config_file(R"({"config": [ - {"plugin_key": "plugin_a", "capability": "cap_a", "plugin_version": "1.0.0"} + {"plugin_key": "plugin_a", "capability": "cap_a", "capability_type": "script", "plugin_version": "1.0.0"} ]})"); PluginConfig config; REQUIRE_NOTHROW(config.load()); - REQUIRE(config.has_config("plugin_a", "cap_a")); - CHECK(config.get_config("plugin_a", "cap_a").config == json::object()); + REQUIRE(config.has_config(capability_id(PluginCapabilityType::Script, "cap_a", "plugin_a"))); + CHECK(config.get_config(capability_id(PluginCapabilityType::Script, "cap_a", "plugin_a"))->config == json::object()); + } + + SECTION("legacy entries without capability_type remain addressable") + { + ScopedDataDir data_dir_guard("plugin-config-notype"); + write_config_file(R"({"config": [ + {"plugin_key": "plugin_a", "capability": "cap_a", "plugin_version": "1.0.0", "cap_config": {"old": true}} + ]})"); + + PluginConfig config; + REQUIRE_NOTHROW(config.load()); + const auto id = capability_id(PluginCapabilityType::Script, "cap_a", "plugin_a"); + REQUIRE(config.has_config(id)); + CHECK(config.get_config(id)->config == json{{"old", true}}); + + REQUIRE(config.store_capability_config(id, json{{"migrated", true}})); + const json root = read_config_file(); + REQUIRE(root.at("config").size() == 1); + CHECK(root.at("config").front().at("capability_type") == "script"); + CHECK(root.at("config").front().at("cap_config") == json{{"migrated", true}}); } } @@ -189,12 +238,12 @@ TEST_CASE("PluginConfig refuses to store a record without an identity", "[Plugin ScopedDataDir data_dir_guard("plugin-config-identity"); PluginConfig config; - config.save_config(BaseConfig{"", "cap_a", "1.0.0", json::object()}); - config.save_config(BaseConfig{"plugin_a", "", "1.0.0", json::object()}); + config.save_config(CapabilityConfigEntry{capability_id(PluginCapabilityType::Script, "cap_a", ""), "1.0.0", json::object()}); + config.save_config(CapabilityConfigEntry{capability_id(PluginCapabilityType::Script, "", "plugin_a"), "1.0.0", json::object()}); // Neither could ever be looked up again, so neither is kept. - CHECK_FALSE(config.has_config("", "cap_a")); - CHECK_FALSE(config.has_config("plugin_a", "")); + CHECK_FALSE(config.has_config(capability_id(PluginCapabilityType::Script, "cap_a", ""))); + CHECK_FALSE(config.has_config(capability_id(PluginCapabilityType::Script, "", "plugin_a"))); CHECK_FALSE(config.dirty()); } @@ -206,9 +255,10 @@ TEST_CASE("PluginConfig preserves unknown keys inside cap_config", "[PluginConfi const json nested = json{{"nested", {{"deep", json::array({1, 2, 3})}}}, {"flag", false}, {"name", "x"}}; PluginConfig config; - REQUIRE(config.store_capability_config("plugin_a", "cap_a", nested)); + const PluginCapabilityId id = capability_id(PluginCapabilityType::Script, "cap_a", "plugin_a"); + REQUIRE(config.store_capability_config(id, nested)); PluginConfig reloaded; reloaded.load(); - CHECK(reloaded.get_config("plugin_a", "cap_a").config == nested); + CHECK(reloaded.get_config(id)->config == nested); } diff --git a/tests/slic3rutils/test_plugin_lifecycle.cpp b/tests/slic3rutils/test_plugin_lifecycle.cpp index 5f8a45452b..f91d15e662 100644 --- a/tests/slic3rutils/test_plugin_lifecycle.cpp +++ b/tests/slic3rutils/test_plugin_lifecycle.cpp @@ -122,9 +122,7 @@ bool load_and_wait(PluginManager& manager, std::shared_ptr find_capability(PluginManager& manager, const std::string& plugin_key, const std::string& name) -{ - return manager.get_plugin_capability(plugin_key, name, PluginCapabilityType::Unknown, /*only_enabled=*/false); -} +{ return manager.get_plugin_capability({PluginCapabilityType::Unknown, name, plugin_key}, /*only_enabled=*/false); } std::vector> capabilities_of(PluginManager& manager, const std::string& plugin_key) { @@ -173,7 +171,7 @@ TEST_CASE("A discovered script plugin loads and materializes its capability", "[ CHECK(echo->is_enabled()); CHECK(echo->audit_plugin_key() == "Echo_Plugin"); - CHECK(manager.get_plugin_capability("Echo_Plugin", "Echo", PluginCapabilityType::Script) == echo); + CHECK(manager.get_plugin_capability({PluginCapabilityType::Script, "Echo", "Echo_Plugin"}) == echo); manager.unload_plugin("Echo_Plugin"); } @@ -244,7 +242,7 @@ TEST_CASE("Unloading a plugin drops the package and its capabilities", "[PluginL CHECK_FALSE(manager.is_plugin_loaded("Echo_Plugin")); CHECK(manager.get_plugin_capabilities("Echo_Plugin").empty()); - CHECK(manager.get_plugin_capability("Echo_Plugin", "Echo", PluginCapabilityType::Script) == nullptr); + CHECK(manager.get_plugin_capability({PluginCapabilityType::Script, "Echo", "Echo_Plugin"}) == nullptr); // The package stays discovered, but nothing capability-shaped survives the unload. const PluginDescriptor descriptor = descriptor_of(manager, "Echo_Plugin"); @@ -403,7 +401,7 @@ TEST_CASE("Disabling a capability round-trips through the sidecar and survives a REQUIRE(find_capability(manager, "Echo_Plugin", "Echo")->is_enabled()); // Disabling writes the choice through to .install_state.json. - manager.set_capability_enabled("Echo_Plugin", "Echo", false); + manager.set_capability_enabled({PluginCapabilityType::Unknown, "Echo", "Echo_Plugin"}, false); CHECK_FALSE(find_capability(manager, "Echo_Plugin", "Echo")->is_enabled()); PluginInstallState persisted; @@ -440,7 +438,7 @@ TEST_CASE("A capability disabled after load stays disabled when rediscovered and std::string error; REQUIRE(load_and_wait(manager, "Echo_Plugin", error)); - manager.set_capability_enabled("Echo_Plugin", "Echo", false); + manager.set_capability_enabled({PluginCapabilityType::Unknown, "Echo", "Echo_Plugin"}, false); REQUIRE(manager.unload_plugin("Echo_Plugin")); // Rediscover, as the app does when a plugin is toggled off and back on. The enable flags the