Use single instance of remote handler per sync

Pro:
- allows use of destructor to clean up
- remote handler can store file location and reuse it

Con:
- upload can not be called without previously calling download
  - currently not needed
This commit is contained in:
Stefan Forstenlechner 2024-06-19 20:14:27 +02:00
parent fc165f68fd
commit 2c03738580
5 changed files with 27 additions and 30 deletions

View file

@ -92,6 +92,7 @@ DatabaseWidget::DatabaseWidget(QSharedPointer<Database> db, QWidget* parent)
, m_tagView(new TagView(this))
, m_saveAttempts(0)
, m_remoteSettings(new RemoteSettings(m_db, this))
, m_remoteHandler(nullptr)
, m_entrySearcher(new EntrySearcher(false))
{
Q_ASSERT(m_db);
@ -1085,7 +1086,7 @@ void DatabaseWidget::syncWithRemote(const RemoteParams* params)
setDisabled(true);
emit databaseSyncInProgress();
QScopedPointer<RemoteHandler> remoteHandler(new RemoteHandler(this));
m_remoteHandler.reset(new RemoteHandler(this));
RemoteHandler::RemoteResult result;
result.success = false;
result.errorMessage = tr("Remote Sync did not contain any download or upload commands.");
@ -1094,7 +1095,7 @@ void DatabaseWidget::syncWithRemote(const RemoteParams* params)
if (!params->downloadCommand.isEmpty()) {
emit updateSyncProgress(25, tr("Downloading..."));
// Start a download first then merge and upload in the callback
result = remoteHandler->download(params);
result = m_remoteHandler->download(params);
if (result.success) {
QString error;
QSharedPointer<Database> remoteDb = QSharedPointer<Database>::create();
@ -1134,10 +1135,9 @@ void DatabaseWidget::syncDatabaseWithLockedDatabase(const QString& filePath, con
void DatabaseWidget::uploadAndFinishSync(const RemoteParams* params, RemoteHandler::RemoteResult result)
{
QScopedPointer<RemoteHandler> remoteHandler(new RemoteHandler(this));
if (result.success && !params->uploadCommand.isEmpty()) {
emit updateSyncProgress(75, tr("Uploading..."));
result = remoteHandler->upload(result.filePath, params);
result = m_remoteHandler->upload(params);
}
finishSync(params, result);
@ -1145,8 +1145,7 @@ void DatabaseWidget::uploadAndFinishSync(const RemoteParams* params, RemoteHandl
void DatabaseWidget::finishSync(const RemoteParams* params, RemoteHandler::RemoteResult result)
{
QScopedPointer<RemoteHandler> remoteHandler(new RemoteHandler(this));
remoteHandler->cleanup(result.filePath);
m_remoteHandler.reset();
setDisabled(false);
emit updateSyncProgress(-1, "");
if (result.success) {

View file

@ -321,6 +321,7 @@ private:
int m_saveAttempts;
QScopedPointer<RemoteSettings> m_remoteSettings;
QScopedPointer<RemoteHandler> m_remoteHandler;
// Search state
QScopedPointer<EntrySearcher> m_entrySearcher;

View file

@ -187,17 +187,14 @@ void DatabaseSettingsWidgetRemote::testDownload()
if (!result.success) {
m_ui->messageWidget->showMessage(tr("Download failed with error: %1").arg(result.errorMessage),
MessageWidget::Error);
remoteHandler->cleanup(result.filePath);
return;
}
if (!QFile::exists(result.filePath)) {
m_ui->messageWidget->showMessage(tr("Download finished, but file %1 could not be found.").arg(result.filePath),
MessageWidget::Error);
remoteHandler->cleanup(result.filePath);
return;
}
remoteHandler->cleanup(result.filePath);
m_ui->messageWidget->showMessage(tr("Download successful."), MessageWidget::Positive);
}

View file

@ -32,6 +32,14 @@ RemoteHandler::RemoteHandler(QObject* parent)
{
}
RemoteHandler::~RemoteHandler()
{
QFileInfo file(m_tempFileLocation);
if (file.absoluteDir().exists() && file.absoluteDir().dirName().startsWith(PREFIX)) {
file.absoluteDir().removeRecursively();
}
}
void RemoteHandler::setRemoteProcessFunc(std::function<QScopedPointer<RemoteProcess>(QObject*)> func)
{
m_createRemoteProcess = std::move(func);
@ -39,7 +47,7 @@ void RemoteHandler::setRemoteProcessFunc(std::function<QScopedPointer<RemoteProc
RemoteHandler::RemoteResult RemoteHandler::download(const RemoteParams* params)
{
return AsyncTask::runAndWaitForFuture([params] {
return AsyncTask::runAndWaitForFuture([this, params] {
RemoteResult result;
if (!params) {
result.success = false;
@ -48,8 +56,8 @@ RemoteHandler::RemoteResult RemoteHandler::download(const RemoteParams* params)
}
QString error;
auto filePath = getTempFileLocation(&error);
result.filePath = filePath;
m_tempFileLocation = getTempFileLocation(&error);
result.filePath = m_tempFileLocation;
if (!error.isEmpty()) {
result.success = false;
result.errorMessage = error;
@ -57,7 +65,7 @@ RemoteHandler::RemoteResult RemoteHandler::download(const RemoteParams* params)
}
auto remoteProcess = m_createRemoteProcess(nullptr); // use nullptr parent, otherwise there is a warning
remoteProcess->setTempFileLocation(filePath);
remoteProcess->setTempFileLocation(m_tempFileLocation);
remoteProcess->start(params->downloadCommand);
if (!params->downloadInput.isEmpty()) {
remoteProcess->write(params->downloadInput + "\n");
@ -74,7 +82,7 @@ RemoteHandler::RemoteResult RemoteHandler::download(const RemoteParams* params)
if (finished && statusCode == 0) {
// Check if the file actually downloaded
QFileInfo fileInfo(filePath);
QFileInfo fileInfo(m_tempFileLocation);
if (!fileInfo.exists() || fileInfo.size() == 0) {
result.success = false;
result.errorMessage = tr("Command `%1` failed to download database.").arg(params->downloadCommand);
@ -96,11 +104,11 @@ RemoteHandler::RemoteResult RemoteHandler::download(const RemoteParams* params)
});
}
RemoteHandler::RemoteResult RemoteHandler::upload(const QString& filePath, const RemoteParams* params)
RemoteHandler::RemoteResult RemoteHandler::upload(const RemoteParams* params)
{
return AsyncTask::runAndWaitForFuture([filePath, params] {
return AsyncTask::runAndWaitForFuture([this, params] {
RemoteResult result;
result.filePath = filePath;
result.filePath = m_tempFileLocation;
if (!params) {
result.success = false;
result.errorMessage = tr("Invalid database pointer or upload parameters provided.");
@ -108,7 +116,7 @@ RemoteHandler::RemoteResult RemoteHandler::upload(const QString& filePath, const
}
auto remoteProcess = m_createRemoteProcess(nullptr); // use nullptr parent, otherwise there is a warning
remoteProcess->setTempFileLocation(filePath);
remoteProcess->setTempFileLocation(m_tempFileLocation);
remoteProcess->start(params->uploadCommand);
if (!params->uploadInput.isEmpty()) {
remoteProcess->write(params->uploadInput + "\n");
@ -169,11 +177,3 @@ QString RemoteHandler::getTempFileLocation(QString* error)
return tempFileLocation;
}
void RemoteHandler::cleanup(QString& tempFileLocation)
{
QFileInfo file(tempFileLocation);
if (file.absoluteDir().exists() && file.absoluteDir().dirName().startsWith(PREFIX)) {
file.absoluteDir().removeRecursively();
}
}

View file

@ -30,7 +30,7 @@ class RemoteHandler : public QObject
public:
explicit RemoteHandler(QObject* parent = nullptr);
~RemoteHandler() override = default;
~RemoteHandler() override;
struct RemoteResult
{
@ -42,14 +42,14 @@ public:
};
RemoteResult download(const RemoteParams* params);
RemoteResult upload(const QString& filePath, const RemoteParams* params);
void cleanup(QString& tempFileLocation);
RemoteResult upload(const RemoteParams* params);
// Used for testing only
static void setRemoteProcessFunc(std::function<QScopedPointer<RemoteProcess>(QObject*)> func);
private:
QString m_tempFileLocation;
static QString getTempFileLocation(QString* error);
static std::function<QScopedPointer<RemoteProcess>(QObject*)> m_createRemoteProcess;