fix: collapse plugincapabilityid

This commit is contained in:
Ian Chua
2026-07-16 18:18:07 +08:00
parent b6f98d9592
commit f4414dd72b
14 changed files with 361 additions and 428 deletions

View File

@@ -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<std::shared_ptr<PluginCapabilityInterface>>();
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<bool>(); }
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<bool>());
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]")

View File

@@ -2,6 +2,7 @@
#include <libslic3r/Utils.hpp>
#include <slic3r/plugin/PluginConfig.hpp>
#include <slic3r/plugin/PluginManager.hpp>
#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);
}

View File

@@ -122,9 +122,7 @@ bool load_and_wait(PluginManager& manager,
std::shared_ptr<PluginCapabilityInterface> 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<std::shared_ptr<PluginCapabilityInterface>> 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