diff --git a/pj_plugins/CMakeLists.txt b/pj_plugins/CMakeLists.txt index f052da2..4c94110 100644 --- a/pj_plugins/CMakeLists.txt +++ b/pj_plugins/CMakeLists.txt @@ -386,7 +386,7 @@ target_compile_definitions(plugin_catalog_test PRIVATE ) target_compile_options(plugin_catalog_test PRIVATE ${PJ_WARNING_FLAGS}) target_link_libraries(plugin_catalog_test PRIVATE - pj_plugin_catalog GTest::gtest_main + pj_plugin_catalog pj_plugin_runtime_catalog GTest::gtest_main ) add_dependencies(plugin_catalog_test mock_data_source_plugin mock_json_parser_plugin mock_toolbox_plugin mock_dialog_plugin missing_id_data_source_plugin diff --git a/pj_plugins/include/pj_plugins/host/plugin_runtime_catalog.hpp b/pj_plugins/include/pj_plugins/host/plugin_runtime_catalog.hpp index 5129a33..eb2ec09 100644 --- a/pj_plugins/include/pj_plugins/host/plugin_runtime_catalog.hpp +++ b/pj_plugins/include/pj_plugins/host/plugin_runtime_catalog.hpp @@ -72,6 +72,13 @@ class PluginRuntimeCatalog { // Replaces the directory scanned by scanDirectory() and reload(). void setPluginDir(std::filesystem::path plugin_dir); + // Replaces the ordered list of directories scanned by scanDirectory() and + // reload(). Directories are scanned in order and de-duplicated by manifest id: + // when the same plugin id appears in more than one directory, the first + // (highest-priority) directory wins and the later copies are skipped with an + // info diagnostic. Empty entries are ignored. + void setPluginDirs(std::vector plugin_dirs); + // Replaces the optional diagnostic sink. void setDiagnosticSink(DiagnosticSink sink); @@ -135,6 +142,11 @@ class PluginRuntimeCatalog { [[nodiscard]] std::string listAvailableEncodings() const; private: + // Scans every directory in plugin_dirs_ (in order) and returns the loadable + // descriptors de-duplicated by manifest id (first directory wins). Reports + // scan diagnostics and one info diagnostic per skipped duplicate. + [[nodiscard]] std::vector collectDeduplicatedPlugins() const; + // Loads a descriptor using the family-specific loader. bool loadAndRegister(const PluginDescriptor& descriptor); @@ -159,7 +171,7 @@ class PluginRuntimeCatalog { // Emits diagnostics produced by DSO discovery. void reportScanDiagnostics(const PluginScanResult& scan) const; - std::filesystem::path plugin_dir_; + std::vector plugin_dirs_; DiagnosticSink sink_; std::string diagnostic_source_; std::vector data_sources_; diff --git a/pj_plugins/src/plugin_runtime_catalog.cpp b/pj_plugins/src/plugin_runtime_catalog.cpp index c6e045b..f43a9a6 100644 --- a/pj_plugins/src/plugin_runtime_catalog.cpp +++ b/pj_plugins/src/plugin_runtime_catalog.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include "pj_base/data_source_protocol.h" @@ -60,29 +61,60 @@ std::vector constPtrs(const std::vector& plugins, uint6 PluginRuntimeCatalog::PluginRuntimeCatalog( std::filesystem::path plugin_dir, DiagnosticSink sink, std::string diagnostic_source) - : plugin_dir_(std::move(plugin_dir)), sink_(std::move(sink)), diagnostic_source_(std::move(diagnostic_source)) {} + : sink_(std::move(sink)), diagnostic_source_(std::move(diagnostic_source)) { + if (!plugin_dir.empty()) { + plugin_dirs_.push_back(std::move(plugin_dir)); + } +} void PluginRuntimeCatalog::setPluginDir(std::filesystem::path plugin_dir) { - plugin_dir_ = std::move(plugin_dir); + plugin_dirs_.clear(); + if (!plugin_dir.empty()) { + plugin_dirs_.push_back(std::move(plugin_dir)); + } +} + +void PluginRuntimeCatalog::setPluginDirs(std::vector plugin_dirs) { + plugin_dirs_ = std::move(plugin_dirs); } void PluginRuntimeCatalog::setDiagnosticSink(DiagnosticSink sink) { sink_ = std::move(sink); } +std::vector PluginRuntimeCatalog::collectDeduplicatedPlugins() const { + std::vector winners; + std::unordered_set seen_ids; + for (const std::filesystem::path& dir : plugin_dirs_) { + if (dir.empty()) { + continue; + } + auto scan = scanPluginDsos(dir); + if (!scan) { + report(DiagnosticLevel::kError, {}, scan.error()); + continue; + } + reportScanDiagnostics(*scan); + for (const PluginDescriptor& descriptor : scan->plugins) { + if (!seen_ids.insert(descriptor.id).second) { + report( + DiagnosticLevel::kInfo, descriptor.id, + descriptor.dso_path.string() + ": ignoring duplicate plugin id \"" + descriptor.id + + "\" (already provided by a higher-priority folder)"); + continue; + } + winners.push_back(descriptor); + } + } + return winners; +} + void PluginRuntimeCatalog::scanDirectory() { data_sources_.clear(); message_parsers_.clear(); toolbox_plugins_.clear(); - auto scan = scanPluginDsos(plugin_dir_); - if (!scan) { - report(DiagnosticLevel::kError, {}, scan.error()); - return; - } - reportScanDiagnostics(*scan); - - for (const PluginDescriptor& descriptor : scan->plugins) { + for (const PluginDescriptor& descriptor : collectDeduplicatedPlugins()) { if (!loadAndRegister(descriptor)) { report( DiagnosticLevel::kError, descriptor.id, @@ -92,16 +124,11 @@ void PluginRuntimeCatalog::scanDirectory() { } bool PluginRuntimeCatalog::reload() { - auto scan = scanPluginDsos(plugin_dir_); - if (!scan) { - report(DiagnosticLevel::kError, {}, scan.error()); - return false; - } - reportScanDiagnostics(*scan); + const std::vector plugins = collectDeduplicatedPlugins(); std::vector on_disk; - on_disk.reserve(scan->plugins.size()); - for (const PluginDescriptor& descriptor : scan->plugins) { + on_disk.reserve(plugins.size()); + for (const PluginDescriptor& descriptor : plugins) { on_disk.push_back(canonicalPath(descriptor.dso_path)); } @@ -121,7 +148,7 @@ bool PluginRuntimeCatalog::reload() { drop_missing(message_parsers_, "MessageParser"); drop_missing(toolbox_plugins_, "Toolbox"); - for (const PluginDescriptor& descriptor : scan->plugins) { + for (const PluginDescriptor& descriptor : plugins) { const std::string path = canonicalPath(descriptor.dso_path); const auto disk_mtime = safeMtime(descriptor.dso_path); if (disk_mtime == std::filesystem::file_time_type{}) { diff --git a/pj_plugins/tests/plugin_catalog_test.cpp b/pj_plugins/tests/plugin_catalog_test.cpp index 9b9a101..b6f8b9e 100644 --- a/pj_plugins/tests/plugin_catalog_test.cpp +++ b/pj_plugins/tests/plugin_catalog_test.cpp @@ -11,6 +11,8 @@ #include #include +#include "pj_plugins/host/plugin_runtime_catalog.hpp" + namespace PJ { namespace { @@ -178,5 +180,47 @@ TEST_F(PluginCatalogTest, FamilyToStringRoundTrip) { EXPECT_EQ(toString(PluginFamily::kUnknown), "unknown"); } +TEST_F(PluginCatalogTest, RuntimeCatalogDedupsDuplicateIdFirstFolderWins) { + // The same data-source DSO (manifest id "mock-data-source") placed in two + // folders: setPluginDirs scans them in order and must load it exactly once, + // from the first (higher-priority) folder. + const std::filesystem::path dir_a = dir_ / "a"; + const std::filesystem::path dir_b = dir_ / "b"; + std::filesystem::create_directories(dir_a); + std::filesystem::create_directories(dir_b); + std::filesystem::copy_file(PJ_MOCK_DATA_SOURCE_PLUGIN_PATH, dir_a / pluginFileName("ds")); + std::filesystem::copy_file(PJ_MOCK_DATA_SOURCE_PLUGIN_PATH, dir_b / pluginFileName("ds")); + + PluginRuntimeCatalog catalog; + catalog.setPluginDirs({dir_a, dir_b}); + catalog.scanDirectory(); + + ASSERT_EQ(catalog.dataSources().size(), 1U); + EXPECT_EQ(catalog.dataSources()[0].id, "mock-data-source"); + // The winner must come from the first folder. Compare at the filesystem level + // (std::filesystem::equivalent) so it holds regardless of how the stored path + // is spelled — Windows back/forward slashes, drive-letter case, canonicalisation. + const std::filesystem::path winner(catalog.dataSources()[0].path); + EXPECT_TRUE(std::filesystem::equivalent(winner.parent_path(), dir_a)) + << "winner " << winner << " is not in the first folder " << dir_a; +} + +TEST_F(PluginCatalogTest, RuntimeCatalogLoadsDistinctIdsFromMultipleFolders) { + // Different plugins in different folders all load. + const std::filesystem::path dir_a = dir_ / "a"; + const std::filesystem::path dir_b = dir_ / "b"; + std::filesystem::create_directories(dir_a); + std::filesystem::create_directories(dir_b); + std::filesystem::copy_file(PJ_MOCK_DATA_SOURCE_PLUGIN_PATH, dir_a / pluginFileName("ds")); + std::filesystem::copy_file(PJ_MOCK_TOOLBOX_PLUGIN_PATH, dir_b / pluginFileName("tb")); + + PluginRuntimeCatalog catalog; + catalog.setPluginDirs({dir_a, dir_b}); + catalog.scanDirectory(); + + EXPECT_EQ(catalog.dataSources().size(), 1U); + EXPECT_EQ(catalog.toolboxes().size(), 1U); +} + } // namespace } // namespace PJ