From 4b1b897a140589426c9325f040feee741798c167 Mon Sep 17 00:00:00 2001 From: Mikaël Capelle Date: Mon, 1 Jun 2020 17:12:20 +0200 Subject: Use a single extractFiles() method and avoir member variables when not needed. --- src/installationmanager.h | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) (limited to 'src/installationmanager.h') diff --git a/src/installationmanager.h b/src/installationmanager.h index cb355641..3d0dd35f 100644 --- a/src/installationmanager.h +++ b/src/installationmanager.h @@ -244,7 +244,7 @@ private: * @return true if the extraction was successful, false if the extraciton was * cancelled. If an error occured, an exception is thrown. */ - bool extractFiles(QDir extractPath); + bool extractFiles(QString extractPath, QString title, bool showFilenames); private: @@ -260,16 +260,10 @@ private: Archive *m_ArchiveHandler; QString m_CurrentFile; - QString m_ErrorMessage; // Map from entries in the tree that is used by the installer and absolute // paths to temporary files: std::map, QString> m_CreatedFiles; - - QProgressDialog *m_InstallationProgress { nullptr }; - int m_Progress; - QString m_ProgressFile; - std::set m_TempFilesToDelete; QString m_URL; -- cgit v1.3.1 From 046af63dd55ee4b04d79cb2188f169bd8292aa19 Mon Sep 17 00:00:00 2001 From: Mikaël Capelle Date: Mon, 1 Jun 2020 17:14:30 +0200 Subject: Remove non-used declarations. Move queryPassword to lambda. --- src/installationmanager.cpp | 10 ++-------- src/installationmanager.h | 8 -------- 2 files changed, 2 insertions(+), 16 deletions(-) (limited to 'src/installationmanager.h') diff --git a/src/installationmanager.cpp b/src/installationmanager.cpp index 00f06af1..a60e60eb 100644 --- a/src/installationmanager.cpp +++ b/src/installationmanager.cpp @@ -125,12 +125,6 @@ void InstallationManager::setURL(QString const &url) m_URL = url; } -void InstallationManager::queryPassword(QString *password) -{ - *password = QInputDialog::getText(nullptr, tr("Password required"), - tr("Password"), QLineEdit::Password); -} - bool InstallationManager::extractFiles(QString extractPath, QString title, bool showFilenames) { QProgressDialog *installationProgress = new QProgressDialog(m_ParentWidget); @@ -630,8 +624,8 @@ IPluginInstaller::EInstallResult InstallationManager::install(const QString &fil // open the archive and construct the directory tree the installers work on bool archiveOpen = m_ArchiveHandler->open( fileName.toStdWString(), [this]() { - QString password; - queryPassword(&password); + QString password = QInputDialog::getText(m_ParentWidget, tr("Password required"), + tr("Password"), QLineEdit::Password); return password.toStdWString(); }); if (!archiveOpen) { diff --git a/src/installationmanager.h b/src/installationmanager.h index 3d0dd35f..67e58356 100644 --- a/src/installationmanager.h +++ b/src/installationmanager.h @@ -189,14 +189,6 @@ public: private: - void queryPassword(QString *password); - void updateProgress(float percentage); - void updateProgressFile(const QString &fileName); - void report7ZipError(const QString &errorMessage); - - // Recursive worker function for mapToArchive (takes raw reference for "speed"). - bool unpackSingleFile(const QString &fileName); - MOBase::IPluginInstaller::EInstallResult doInstall(MOBase::GuessedValue &modName, QString gameName, int modID, const QString &version, const QString &newestVersion, int categoryID, int fileCategoryID, const QString &repository); -- cgit v1.3.1 From 2c20a4115d0fdc4ce81787d8d78bd06fbf838b25 Mon Sep 17 00:00:00 2001 From: Mikaël Capelle Date: Mon, 1 Jun 2020 21:06:52 +0200 Subject: Fix opening and extraction of archives with passwords and/or encrypted filenames. --- src/installationmanager.cpp | 33 ++++++++++++++++++++++++++------- src/installationmanager.h | 13 +++++++++++-- 2 files changed, 37 insertions(+), 9 deletions(-) (limited to 'src/installationmanager.h') diff --git a/src/installationmanager.cpp b/src/installationmanager.cpp index 639dc88a..d2262d13 100644 --- a/src/installationmanager.cpp +++ b/src/installationmanager.cpp @@ -105,6 +105,12 @@ InstallationManager::InstallationManager() break; } }); + + // Connect the query password slot - This is the only way I found to be able to query user + // from a separate thread. We use a BlockingQueuedConnection so that calling passwordRequested() + // will block until the end of the slot. + connect(this, &InstallationManager::passwordRequested, + this, &InstallationManager::queryPassword, Qt::BlockingQueuedConnection); } InstallationManager::~InstallationManager() @@ -125,6 +131,11 @@ void InstallationManager::setURL(QString const &url) m_URL = url; } +void InstallationManager::queryPassword() { + m_Password = QInputDialog::getText(m_ParentWidget, tr("Password required"), + tr("Password"), QLineEdit::Password); +} + bool InstallationManager::extractFiles(QString extractPath, QString title, bool showFilenames) { QProgressDialog *installationProgress = new QProgressDialog(m_ParentWidget); @@ -625,11 +636,22 @@ IPluginInstaller::EInstallResult InstallationManager::install(const QString &fil m_ArchiveHandler->close(); // open the archive and construct the directory tree the installers work on + bool archiveOpen = m_ArchiveHandler->open( - fileName.toStdWString(), [this]() { - QString password = QInputDialog::getText(m_ParentWidget, tr("Password required"), - tr("Password"), QLineEdit::Password); - return password.toStdWString(); + fileName.toStdWString(), [this]() -> std::wstring { + m_Password = QString(); + + // Note: If we are not in the Qt event thread, we cannot use queryPassword() directly, + // so we emit passwordRequested() that is connected to queryPassword(). The connection is + // made using Qt::BlockingQueuedConnection, so the emit "call" is actually blocking. We + // cannot use emit if we are in the even thread, otherwize we have a deadlock. + if (QThread::currentThread() != QApplication::instance()->thread()) { + emit passwordRequested(); + } + else { + queryPassword(); + } + return m_Password.toStdWString(); }); if (!archiveOpen) { log::debug("integrated archiver can't open {}: {} ({})", @@ -755,8 +777,6 @@ IPluginInstaller::EInstallResult InstallationManager::install(const QString &fil return installResult; } - - QString InstallationManager::getErrorString(Archive::Error errorCode) { switch (errorCode) { @@ -791,7 +811,6 @@ QString InstallationManager::getErrorString(Archive::Error errorCode) } } - void InstallationManager::registerInstaller(IPluginInstaller *installer) { m_Installers.push_back(installer); diff --git a/src/installationmanager.h b/src/installationmanager.h index 67e58356..328272e1 100644 --- a/src/installationmanager.h +++ b/src/installationmanager.h @@ -204,10 +204,16 @@ private: void postInstallCleanup(); +private slots: + + /** + * @brief Query user for password and update the m_Password field. + */ + void queryPassword(); + signals: - void progressUpdate(float percentage); - void progressUpdate(QString const fileName); + void passwordRequested(); private: @@ -253,6 +259,9 @@ private: Archive *m_ArchiveHandler; QString m_CurrentFile; + // Current password: + QString m_Password; + // Map from entries in the tree that is used by the installer and absolute // paths to temporary files: std::map, QString> m_CreatedFiles; -- cgit v1.3.1 From b8a1365dacd7ffa8705bbb3dcaf2e2dc6613e6fb Mon Sep 17 00:00:00 2001 From: Mikaël Capelle Date: Tue, 2 Jun 2020 12:40:46 +0200 Subject: Update after change to CreateArchive. Remove unused archive handler from self-updater. --- src/archivefiletree.cpp | 12 ++++++------ src/archivefiletree.h | 6 +++--- src/installationmanager.cpp | 11 +++-------- src/installationmanager.h | 5 ++--- src/selfupdater.cpp | 27 --------------------------- src/selfupdater.h | 2 -- 6 files changed, 14 insertions(+), 49 deletions(-) (limited to 'src/installationmanager.h') diff --git a/src/archivefiletree.cpp b/src/archivefiletree.cpp index 1f410b34..4bedeb41 100644 --- a/src/archivefiletree.cpp +++ b/src/archivefiletree.cpp @@ -120,8 +120,8 @@ public: // Overrides: /** * */ - void mapToArchive(Archive* archive) const override { - mapToArchive(*this, "", archive->getFileList()); + void mapToArchive(Archive &archive) const override { + mapToArchive(*this, "", archive.getFileList()); } protected: @@ -214,9 +214,9 @@ private: mutable std::vector m_Files; }; -std::shared_ptr ArchiveFileTree::makeTree(Archive* archive) +std::shared_ptr ArchiveFileTree::makeTree(Archive const& archive) { - auto const& data = archive->getFileList(); + auto const& data = archive.getFileList(); std::vector files; files.reserve(data.size()); @@ -255,6 +255,6 @@ void mapToArchive(std::vector const& data, It begin, It end) { } } -void ArchiveFileTree::mapToArchive(Archive* archive, std::vector> const& entries) { - ::mapToArchive(archive->getFileList(), entries.cbegin(), entries.cend()); +void ArchiveFileTree::mapToArchive(Archive &archive, std::vector> const& entries) { + ::mapToArchive(archive.getFileList(), entries.cbegin(), entries.cend()); } diff --git a/src/archivefiletree.h b/src/archivefiletree.h index 0041acd6..e4a5cbdd 100644 --- a/src/archivefiletree.h +++ b/src/archivefiletree.h @@ -37,7 +37,7 @@ public: * * @return a file tree representing the given archive. */ - static std::shared_ptr makeTree(Archive* archive); + static std::shared_ptr makeTree(Archive const& archive); /** * @brief Update the given archive to reflect change in this tree. @@ -48,7 +48,7 @@ public: * @param archive The archive to update. Must be the one used to * create the tree. */ - virtual void mapToArchive(Archive *archive) const = 0; + virtual void mapToArchive(Archive &archive) const = 0; /** * @brief Update the given archive to prepare for the extraction @@ -61,7 +61,7 @@ public: * @param entries List of entries to mark for extraction. All the entries must * come from a tree created with the given archive. */ - static void mapToArchive(Archive* archive, std::vector> const& entries); + static void mapToArchive(Archive &archive, std::vector> const& entries); protected: diff --git a/src/installationmanager.cpp b/src/installationmanager.cpp index 0a97225e..dad5b436 100644 --- a/src/installationmanager.cpp +++ b/src/installationmanager.cpp @@ -64,10 +64,6 @@ using namespace MOBase; using namespace MOShared; -typedef Archive* (*CreateArchiveType)(); - - - template static T resolveFunction(QLibrary &lib, const char *name) { @@ -116,7 +112,6 @@ InstallationManager::InstallationManager() InstallationManager::~InstallationManager() { - delete m_ArchiveHandler; } void InstallationManager::setParentWidget(QWidget *widget) @@ -238,7 +233,7 @@ QStringList InstallationManager::extractFiles(std::vectorisFile(); }); // Update the archive: - ArchiveFileTree::mapToArchive(m_ArchiveHandler, files); + ArchiveFileTree::mapToArchive(*m_ArchiveHandler, files); // Retrieve the file path: QStringList result; @@ -663,7 +658,7 @@ IPluginInstaller::EInstallResult InstallationManager::install(const QString &fil ON_BLOCK_EXIT(std::bind(&InstallationManager::postInstallCleanup, this)); std::shared_ptr filesTree = - archiveOpen ? ArchiveFileTree::makeTree(m_ArchiveHandler) : nullptr; + archiveOpen ? ArchiveFileTree::makeTree(*m_ArchiveHandler) : nullptr; IPluginInstaller::EInstallResult installResult = IPluginInstaller::RESULT_NOTATTEMPTED; std::sort(m_Installers.begin(), m_Installers.end(), [] (IPluginInstaller *LHS, IPluginInstaller *RHS) { @@ -707,7 +702,7 @@ IPluginInstaller::EInstallResult InstallationManager::install(const QString &fil // stops at this root): p->detach(); - p->mapToArchive(m_ArchiveHandler); + p->mapToArchive(*m_ArchiveHandler); // Clean the created files: cleanCreatedFiles(filesTree); diff --git a/src/installationmanager.h b/src/installationmanager.h index 328272e1..1b0c6d73 100644 --- a/src/installationmanager.h +++ b/src/installationmanager.h @@ -256,10 +256,9 @@ private: std::vector m_Installers; std::set m_SupportedExtensions; - Archive *m_ArchiveHandler; + // Archive management: + std::unique_ptr m_ArchiveHandler; QString m_CurrentFile; - - // Current password: QString m_Password; // Map from entries in the tree that is used by the installer and absolute diff --git a/src/selfupdater.cpp b/src/selfupdater.cpp index 501c7ed1..dfe11880 100644 --- a/src/selfupdater.cpp +++ b/src/selfupdater.cpp @@ -71,45 +71,18 @@ along with Mod Organizer. If not, see . using namespace MOBase; using namespace MOShared; - -typedef Archive* (*CreateArchiveType)(); - - -template static T resolveFunction(QLibrary &lib, const char *name) -{ - T temp = reinterpret_cast(lib.resolve(name)); - if (temp == nullptr) { - throw std::runtime_error(QObject::tr("invalid 7-zip32.dll: %1").arg(lib.errorString()).toLatin1().constData()); - } - return temp; -} - - SelfUpdater::SelfUpdater(NexusInterface *nexusInterface) : m_Parent(nullptr) , m_Interface(nexusInterface) , m_Reply(nullptr) , m_Attempts(3) { - QLibrary archiveLib(QCoreApplication::applicationDirPath() + "\\dlls\\archive.dll"); - if (!archiveLib.load()) { - throw MyException(tr("archive.dll not loaded: \"%1\"").arg(archiveLib.errorString())); - } - - CreateArchiveType CreateArchiveFunc = resolveFunction(archiveLib, "CreateArchive"); - - m_ArchiveHandler = CreateArchiveFunc(); - if (!m_ArchiveHandler->isValid()) { - throw MyException(InstallationManager::getErrorString(m_ArchiveHandler->getLastError())); - } - m_MOVersion = createVersionInfo(); } SelfUpdater::~SelfUpdater() { - delete m_ArchiveHandler; } void SelfUpdater::setUserInterface(QWidget *widget) diff --git a/src/selfupdater.h b/src/selfupdater.h index 0c81efc5..8ff14326 100644 --- a/src/selfupdater.h +++ b/src/selfupdater.h @@ -140,8 +140,6 @@ private: bool m_Canceled; int m_Attempts; - Archive *m_ArchiveHandler; - GitHub m_GitHub; QJsonObject m_UpdateCandidate; -- cgit v1.3.1 From e49ce3ed21b10b9af21caaabd8c2f1339fe59076 Mon Sep 17 00:00:00 2001 From: Mikaël Capelle Date: Fri, 5 Jun 2020 20:36:35 +0200 Subject: Use QEventLoop to process events when extracting instead of manual loop with processEvents. --- src/installationmanager.cpp | 45 +++++++++++++++++++++++---------------------- src/installationmanager.h | 13 +++++++++++++ 2 files changed, 36 insertions(+), 22 deletions(-) (limited to 'src/installationmanager.h') diff --git a/src/installationmanager.cpp b/src/installationmanager.cpp index f4b19c71..7f4ad864 100644 --- a/src/installationmanager.cpp +++ b/src/installationmanager.cpp @@ -155,24 +155,28 @@ bool InstallationManager::extractFiles(QString extractPath, QString title, bool // extracted... installationProgress->setAutoReset(false); - installationProgress->show(); + // Connect signals emitted by the extraction callback to the progress dialog slots: + connect(this, &InstallationManager::progressUpdate, installationProgress, &QProgressDialog::setValue); + connect(this, &InstallationManager::progressFileChange, installationProgress, &QProgressDialog::setLabelText); - auto start = std::chrono::system_clock::now(); + // Cancelling progress only cancel the extraction, we do not force exiting the event-loop: + connect(installationProgress, &QProgressDialog::canceled, [this]() { m_ArchiveHandler->cancel(); }); + + installationProgress->show(); // Variable updated by the callbacks: - int progress = 0; - QString progressFile; QString errorMessage; // The callbacks: - auto progressCallback = [&progress](auto progressType, uint64_t current, uint64_t total) { + auto progressCallback = [this](auto progressType, uint64_t current, uint64_t total) { if (progressType == Archive::ProgressType::EXTRACTION) { - progress = static_cast(100 * current / total); + int progress = static_cast(100 * current / total); + emit progressUpdate(progress); } }; - Archive::FileChangeCallback fileChangeCallback = [&progressFile](auto changeType, std::wstring const& file) { + Archive::FileChangeCallback fileChangeCallback = [this](auto changeType, std::wstring const& file) { if (changeType == Archive::FileChangeType::EXTRACTION_START) { - progressFile = QString::fromStdWString(file); + emit progressFileChange(QString::fromStdWString(file)); } }; auto errorCallback = [&errorMessage, this](std::wstring const& message) { @@ -180,6 +184,8 @@ bool InstallationManager::extractFiles(QString extractPath, QString title, bool errorMessage = QString::fromStdWString(message); }; + auto start = std::chrono::system_clock::now(); + // unpack only the files we need for the installer QFuture future = QtConcurrent::run([&]() -> bool { return m_ArchiveHandler->extract( @@ -190,20 +196,15 @@ bool InstallationManager::extractFiles(QString extractPath, QString title, bool ); }); - // Wait for the extraction to complete (or be cancelled): - do { - if (installationProgress->wasCanceled()) { - m_ArchiveHandler->cancel(); - installationProgress->reset(); - } - if (progress != installationProgress->value()) { - installationProgress->setValue(progress); - } - if (showFilenames && progressFile != installationProgress->labelText()) { - installationProgress->setLabelText(progressFile); - } - QCoreApplication::processEvents(); - } while (!future.isFinished()); + // Future watcher to exit the loop: + QFutureWatcher futureWatcher; + futureWatcher.setFuture(future); + + QEventLoop loop(this); + connect(&futureWatcher, &QFutureWatcher::finished, &loop, &QEventLoop::quit); + + // Wait for future to complete: + loop.exec(); // Check the result: if (!future.result()) { diff --git a/src/installationmanager.h b/src/installationmanager.h index 1b0c6d73..1d94151f 100644 --- a/src/installationmanager.h +++ b/src/installationmanager.h @@ -213,8 +213,21 @@ private slots: signals: + /** + * @brief Emitted when a password is requested from the archive wrapper. + */ void passwordRequested(); + /** + * @brief Progress update from the extraction. + */ + void progressUpdate(int percentage); + + /** + * @brief File change update from the extraction. + */ + void progressFileChange(QString const& value); + private: struct ByPriority { -- cgit v1.3.1