Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions pj_marketplace/include/pj_marketplace/extension_manager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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.
Expand All @@ -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_;
Expand Down
32 changes: 27 additions & 5 deletions pj_marketplace/src/core/ExtensionManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ void ExtensionManager::initComponents() {
}

static constexpr const char* kManifestFileName = "manifest.json";
static constexpr const char* kPendingUninstallMarker = ".pj_pending_uninstall";

// ---------------------------------------------------------------------------
// Public interface
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;
Expand Down
11 changes: 10 additions & 1 deletion pj_marketplace/src/ui/marketplace_window.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
}
Expand Down Expand Up @@ -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); });
Expand Down
39 changes: 39 additions & 0 deletions pj_marketplace/tests/extension_manager_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 <gtest/gtest.h>

Expand Down Expand Up @@ -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
// ---------------------------------------------------------------------------
Expand Down
Loading