diff --git a/pj_marketplace/include/pj_marketplace/extension_manager.hpp b/pj_marketplace/include/pj_marketplace/extension_manager.hpp index 65c7362a..9cbc83e7 100644 --- a/pj_marketplace/include/pj_marketplace/extension_manager.hpp +++ b/pj_marketplace/include/pj_marketplace/extension_manager.hpp @@ -21,7 +21,10 @@ class DownloadManager; // DownloadManager, then registers the extension immediately // - On Windows: extracts to a staging directory (.pending/) because in-use DLLs // cannot be overwritten; the extension becomes active after the next restart +// - On Windows uninstall: if the directory cannot be removed (DLL still mapped), +// schedules it for deletion at the next startup via applyPendingUninstalls() // - At startup: applies any pending staged installs via applyPendingInstalls() +// and deletes any directories deferred from a previous uninstall via applyPendingUninstalls() // - Discovers installed extensions by scanning extensions_dir and reading manifest.json // // All constructor dependencies are injected, so tests can pass a DownloadManager stub @@ -69,6 +72,11 @@ class ExtensionManager : public QObject { // because staging is never used, but it is safe to call on any platform. void applyPendingInstalls(); + // Deletes any extension directories that could not be removed during a previous + // uninstall() because their DLL was still loaded (Windows only). + // Should be called once at application startup. Safe to call on any platform. + void applyPendingUninstalls(); + bool isInstalled(const QString& id) const; // Compares the registry version against the installed one using QVersionNumber, @@ -90,6 +98,10 @@ class ExtensionManager : public QObject { void uninstallFinished(const QString& id, bool success); void uninstallError(const QString& id, const QString& error_message); + // Emitted on Windows when the extension is deregistered but its directory could not + // be removed (DLL still loaded). The directory will be deleted on the next startup + // via applyPendingUninstalls(). + void uninstallPendingRestart(const QString& id); private: // Called by both constructors to finish setup after members are assigned. @@ -99,6 +111,7 @@ class ExtensionManager : public QObject { void saveState(); void disconnectDlConns(); void savePendingMeta(const Extension& ext); + void schedulePendingUninstall(const QString& path); DownloadManager* downloader_ = nullptr; QString extensions_dir_; diff --git a/pj_marketplace/src/core/ExtensionManager.cpp b/pj_marketplace/src/core/ExtensionManager.cpp index d8a53aed..063e9869 100644 --- a/pj_marketplace/src/core/ExtensionManager.cpp +++ b/pj_marketplace/src/core/ExtensionManager.cpp @@ -41,6 +41,7 @@ void ExtensionManager::initComponents() { } static constexpr const char* kManifestFileName = "manifest.json"; +static constexpr const char* kPendingUninstallMarker = ".pj_pending_uninstall"; // --------------------------------------------------------------------------- // Public interface @@ -188,11 +189,17 @@ void ExtensionManager::uninstall(const QString& extension_id) { const QString dir_path = installed_[extension_id].path; if (!QDir(dir_path).removeRecursively()) { - // On Windows the DLL may still be mapped by the host process (F-14, staging deferred to April+). - // Report the error rather than corrupting the state file with a phantom entry. - emit uninstallError( - extension_id, QString("Could not remove directory \"%1\" — the plugin may still be loaded").arg(dir_path)); - emit uninstallFinished(extension_id, false); + if (PlatformUtils::isWindows()) { + // The DLL is still mapped by the host process. Deregister the extension immediately + // and mark the directory for deletion at the next startup. + schedulePendingUninstall(dir_path); + installed_.remove(extension_id); + emit uninstallPendingRestart(extension_id); + } else { + emit uninstallError( + extension_id, QString("Could not remove directory \"%1\" — the plugin may still be loaded").arg(dir_path)); + emit uninstallFinished(extension_id, false); + } return; } @@ -283,6 +290,16 @@ void ExtensionManager::applyPendingInstalls() { } } +void ExtensionManager::applyPendingUninstalls() { + const QDir dir(extensions_dir_); + for (const QFileInfo& entry : dir.entryInfoList(QDir::Dirs | QDir::NoDotAndDotDot)) { + if (!QFile::exists(entry.absoluteFilePath() + "/" + kPendingUninstallMarker)) { + continue; + } + QDir(entry.absoluteFilePath()).removeRecursively(); + } +} + bool ExtensionManager::isInstalled(const QString& id) const { return installed_.contains(id); } @@ -314,6 +331,11 @@ void ExtensionManager::disconnectDlConns() { disconnect(dl_cancelled_conn_); } +void ExtensionManager::schedulePendingUninstall(const QString& path) { + QFile marker(path + "/" + kPendingUninstallMarker); + marker.open(QIODevice::WriteOnly); // content irrelevant; existence is the signal +} + void ExtensionManager::savePendingMeta(const Extension& ext) { QJsonObject obj; obj["id"] = ext.id; diff --git a/pj_marketplace/src/ui/marketplace_window.cpp b/pj_marketplace/src/ui/marketplace_window.cpp index aa2fc876..9b5f433d 100644 --- a/pj_marketplace/src/ui/marketplace_window.cpp +++ b/pj_marketplace/src/ui/marketplace_window.cpp @@ -42,6 +42,7 @@ MarketplaceWindow::MarketplaceWindow(const QUrl& registry_url, QWidget* parent) setupUi(); setupSignals(); ext_mgr_->applyPendingInstalls(); + ext_mgr_->applyPendingUninstalls(); registry_mgr_->fetchRegistry(registry_url_); // extensions_ is now populated via the fetchFinished signal above. } @@ -115,7 +116,15 @@ void MarketplaceWindow::setupSignals() { ui_->progress_bar_->setVisible(false); populateCards(); setStatus("Extension staged — will be active after restart"); - }); + }); + + connect(ext_mgr_, &ExtensionManager::uninstallPendingRestart, this, + [this](const QString& id) { + pending_restart_ids_.insert(id); + ui_->progress_bar_->setVisible(false); + populateCards(); + setStatus("Extension staged — will be uninstalled after restart"); + }); connect(registry_mgr_, &RegistryManager::fetchError, this, [this](const QString& error) { setStatus("Registry error: " + error, true); }); diff --git a/pj_marketplace/tests/extension_manager_test.cpp b/pj_marketplace/tests/extension_manager_test.cpp index df5fb24a..bf5145bb 100644 --- a/pj_marketplace/tests/extension_manager_test.cpp +++ b/pj_marketplace/tests/extension_manager_test.cpp @@ -9,6 +9,7 @@ // [6] applyPendingInstalls: simulates the Windows post-restart staging path // [7] State persistence: installed state derived from disk across manager restarts // [8] Platform detection: currentPlatform() format and registry key resolution +// [9] applyPendingUninstalls: deferred directory cleanup via marker file #include @@ -665,6 +666,44 @@ TEST_F(ExtensionManagerTest, FreshManagerHasNoInstalledExtensions) { EXPECT_TRUE(mgr_->installedExtensions().isEmpty()); } +// --------------------------------------------------------------------------- +// [9] applyPendingUninstalls — deferred directory cleanup simulation +// +// On Windows, uninstall() writes a .pj_pending_uninstall marker inside the +// extension directory when removeRecursively() fails. On the next startup, +// applyPendingUninstalls() removes every extension directory that carries that +// marker. These tests create that state manually so the cleanup logic can be +// verified on any platform. +// --------------------------------------------------------------------------- + +// A directory containing the marker is removed by applyPendingUninstalls(). +TEST_F(ExtensionManagerTest, ApplyPendingUninstallsRemovesMarkedDirectory) { + const QString ext_path = ext_dir_.path() + "/csv-loader"; + ASSERT_TRUE(QDir().mkpath(ext_path)); + QFile marker(ext_path + "/.pj_pending_uninstall"); + ASSERT_TRUE(marker.open(QIODevice::WriteOnly)); + marker.close(); + + mgr_->applyPendingUninstalls(); + + EXPECT_FALSE(QDir(ext_path).exists()); +} + +// A directory without the marker is left untouched. +TEST_F(ExtensionManagerTest, ApplyPendingUninstallsIgnoresUnmarkedDirectory) { + const QString ext_path = ext_dir_.path() + "/csv-loader"; + ASSERT_TRUE(QDir().mkpath(ext_path)); + + mgr_->applyPendingUninstalls(); + + EXPECT_TRUE(QDir(ext_path).exists()); +} + +// applyPendingUninstalls() is a no-op when extensions_dir contains no sub-directories. +TEST_F(ExtensionManagerTest, ApplyPendingUninstallsIsNoOpForEmptyDirectory) { + mgr_->applyPendingUninstalls(); // must not crash +} + // --------------------------------------------------------------------------- // [8] Platform detection // ---------------------------------------------------------------------------