Commit 0905b073 authored by Bart De Vries's avatar Bart De Vries Committed by Nate Graham
Browse files

Importer: convert all file operations to allow remote URLs

Summary:
All file operations are updated to allow remote URLs, both on the source
and destination side.  This consists mainly of converting local file/dir
operations to KIO calls.
Remote source was already mostly covered.
To support remote destinations, a few adaptations were needed:
 - If destination is remote or a remote server mounted locally, then do
   not create the temporary directory as a subdir of the destination,
   but create it locally using default QTemporaryDir().
 - When comparing file contents, if destination is remote, use
   KIO::storedGet to get the file contents in one go.  This is much
   simpler and more robust than using asynchronous KIO::open calls,
   but has the drawback that the file is read in one go instead of in
   chunks.

Test Plan:
Added unit tests for remote source and destination.
Also tested manually by running gwenview_importer with a combination
of remote and local destinations, covering smb and sftp protocols, as
well as remote server mounted locally (e.g. cifs mount).
Also tested on multiple files with the same name existing in source
directory; both identical and different in content.

Reviewers: #gwenview, ngraham

Reviewed By: #gwenview, ngraham

Subscribers: ngraham, #gwenview

Tags: #gwenview

Differential Revision: https://phabricator.kde.org/D24875
parent d3acacad
......@@ -25,6 +25,8 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Cambridge, MA 02110-1301, USA
#include <QFile>
#include <QFileInfo>
#include <QUrl>
#include <QBuffer>
#include <QScopedPointer>
// KDE
#include <QDebug>
......@@ -42,44 +44,69 @@ namespace FileUtils
bool contentsAreIdentical(const QUrl& url1, const QUrl& url2, QWidget* authWindow)
{
// FIXME: Support remote urls
KIO::StatJob *statJob = KIO::mostLocalUrl(url1);
KJobWidgets::setWindow(statJob, authWindow);
if (!statJob->exec()) {
qWarning() << "Unable to stat" << url1;
return false;
KIO::StatJob *statJob;
KIO::StoredTransferJob *fileJob;
QScopedPointer<QIODevice> file1, file2;
QByteArray file1Bytes, file2Bytes;
if (url1.isLocalFile()) {
statJob = KIO::mostLocalUrl(url1);
KJobWidgets::setWindow(statJob, authWindow);
if (!statJob->exec()) {
qWarning() << "Unable to stat" << url1;
return false;
}
file1.reset(new QFile(statJob->mostLocalUrl().toLocalFile()));
} else { // file1 is remote
fileJob = KIO::storedGet(url1, KIO::NoReload ,KIO::HideProgressInfo);
KJobWidgets::setWindow(fileJob, authWindow);
if (!fileJob->exec()) {
qWarning() << "Can't read" << url1;
return false;
}
file1Bytes = QByteArray(fileJob->data());
file1.reset(new QBuffer(&file1Bytes));
}
QFile file1(statJob->mostLocalUrl().toLocalFile());
if (!file1.open(QIODevice::ReadOnly)) {
if (!file1->open(QIODevice::ReadOnly)) {
// Can't read url1, assume it's different from url2
qWarning() << "Can't read" << url1;
return false;
}
statJob = KIO::mostLocalUrl(url2);
KJobWidgets::setWindow(statJob, authWindow);
if (!statJob->exec()) {
qWarning() << "Unable to stat" << url2;
return false;
if (url2.isLocalFile()) {
statJob = KIO::mostLocalUrl(url2);
KJobWidgets::setWindow(statJob, authWindow);
if (!statJob->exec()) {
qWarning() << "Unable to stat" << url2;
return false;
}
file2.reset(new QFile(statJob->mostLocalUrl().toLocalFile()));
} else { // file2 is remote
fileJob = KIO::storedGet(url2, KIO::NoReload, KIO::HideProgressInfo);
KJobWidgets::setWindow(fileJob, authWindow);
if (!fileJob->exec()) {
qWarning() << "Can't read" << url2;
return false;
}
file2Bytes = QByteArray(fileJob->data());
file2.reset(new QBuffer(&file2Bytes));
}
QFile file2(statJob->mostLocalUrl().toLocalFile());
if (!file2.open(QIODevice::ReadOnly)) {
if (!file2->open(QIODevice::ReadOnly)) {
// Can't read url2, assume it's different from url1
qWarning() << "Can't read" << url2;
return false;
}
const int CHUNK_SIZE = 4096;
while (!file1.atEnd() && !file2.atEnd()) {
QByteArray url1Array = file1.read(CHUNK_SIZE);
QByteArray url2Array = file2.read(CHUNK_SIZE);
while (!file1->atEnd() && !file2->atEnd()) {
QByteArray url1Array = file1->read(CHUNK_SIZE);
QByteArray url2Array = file2->read(CHUNK_SIZE);
if (url1Array != url2Array) {
return false;
}
}
if (file1.atEnd() && file2.atEnd()) {
if (file1->atEnd() && file2->atEnd()) {
return true;
} else {
qWarning() << "One file ended before the other";
......@@ -131,7 +158,7 @@ RenameResult rename(const QUrl& src, const QUrl& dst_, QWidget* authWindow)
}
// Rename
KIO::Job* job = KIO::rename(src, dst, KIO::HideProgressInfo);
KIO::Job* job = KIO::moveAs(src, dst, KIO::HideProgressInfo);
KJobWidgets::setWindow(job, authWindow);
if (!job->exec()) {
result = RenameFailed;
......
......@@ -33,6 +33,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Cambridge, MA 02110-1301, USA
#include <KIO/DeleteJob>
#include <KIO/MkpathJob>
#include <KIO/Job>
#include <kio/jobclasses.h>
#include <KIO/JobUiDelegate>
#include <KJobWidgets>
#include <KLocalizedString>
......@@ -44,6 +45,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Cambridge, MA 02110-1301, USA
#include <fileutils.h>
#include <filenameformater.h>
#include <lib/timeutils.h>
#include <lib/urlutils.h>
#include <QDir>
namespace Gwenview
......@@ -56,6 +58,7 @@ struct ImporterPrivate
std::unique_ptr<FileNameFormater> mFileNameFormater;
QUrl mTempImportDirUrl;
QTemporaryDir* mTempImportDir;
QUrl mDestinationDirUrl;
/* @defgroup reset Should be reset in start()
* @{ */
......@@ -73,28 +76,34 @@ struct ImporterPrivate
bool createImportDir(const QUrl& url)
{
Q_ASSERT(url.isLocalFile());
// FIXME: Support remote urls
if (!QDir().mkpath(url.toLocalFile())) {
KIO::Job* job = KIO::mkpath(url, QUrl(), KIO::HideProgressInfo);
KJobWidgets::setWindow(job, mAuthWindow);
if (!job->exec()) {
emit q->error(i18n("Could not create destination folder."));
return false;
}
QString tempDirPath = url.toLocalFile() + "/.gwenview_importer-XXXXXX";
mTempImportDir = new QTemporaryDir(tempDirPath);
// Check if local and fast url. The check for fast url is needed because
// otherwise the retrieved date will not be correct: see implementation
// of TimeUtils::dateTimeForFileItem
if (UrlUtils::urlIsFastLocalFile(url)) {
QString tempDirPath = url.toLocalFile() + "/.gwenview_importer-XXXXXX";
mTempImportDir = new QTemporaryDir(tempDirPath);
} else {
mTempImportDir = new QTemporaryDir();
}
if (!mTempImportDir->isValid()) {
emit q->error(i18n("Could not create temporary upload folder."));
return false;
}
mTempImportDirUrl = QUrl::fromLocalFile(mTempImportDir->path() + '/');
if (!mTempImportDirUrl.isValid()) {
emit q->error(i18n("Could not create temporary upload folder."));
return false;
}
}
return true;
}
......@@ -117,7 +126,7 @@ struct ImporterPrivate
void renameImportedUrl(const QUrl& src)
{
QUrl dst = src.resolved(QUrl(".."));
QUrl dst = mDestinationDirUrl;
QString fileName;
if (mFileNameFormater.get()) {
KFileItem item(src);
......@@ -131,7 +140,7 @@ struct ImporterPrivate
} else {
fileName = src.fileName();
}
dst.setPath(dst.path() + fileName);
dst.setPath(dst.path() + '/' + fileName);
FileUtils::RenameResult result;
// Create additional subfolders if needed (e.g. when extra slashes in FileNameFormater)
......@@ -192,6 +201,7 @@ void Importer::setAutoRenameFormat(const QString& format)
void Importer::start(const QList<QUrl>& list, const QUrl& destination)
{
d->mDestinationDirUrl = destination;
d->mUrlList = list;
d->mImportedUrlList.clear();
d->mSkippedUrlList.clear();
......
......@@ -41,7 +41,7 @@ gv_add_unit_test(timeutilstest)
gv_add_unit_test(placetreemodeltest testutils.cpp)
gv_add_unit_test(urlutilstest)
gv_add_unit_test(historymodeltest)
gv_add_unit_test(importertest
gv_add_unit_test(importertest testutils.cpp
${importer_SOURCE_DIR}/importer.cpp
${importer_SOURCE_DIR}/fileutils.cpp
${importer_SOURCE_DIR}/filenameformater.cpp
......
......@@ -110,6 +110,62 @@ void ImporterTest::testSuccessfulImport()
}
}
void ImporterTest::testSuccessfulImportRemote()
{
// First test import from local to remote
QUrl remoteUrl = setUpRemoteTestDir();
if (!remoteUrl.isValid()) {
QSKIP("Not running this test: failed to setup remote test dir.");
}
Importer importer(nullptr);
QSignalSpy maximumChangedSpy(&importer, SIGNAL(maximumChanged(int)));
QSignalSpy errorSpy(&importer, SIGNAL(error(QString)));
QList<QUrl> list = mDocumentList;
QEventLoop loop;
connect(&importer, &Importer::importFinished, &loop, &QEventLoop::quit);
importer.start(list, remoteUrl);
loop.exec();
QCOMPARE(maximumChangedSpy.count(), 1);
QCOMPARE(maximumChangedSpy.takeFirst().at(0).toInt(), list.count() * 100);
QCOMPARE(errorSpy.count(), 0);
QCOMPARE(importer.importedUrlList().count(), list.count());
QCOMPARE(importer.importedUrlList(), list);
QCOMPARE(importer.skippedUrlList().count(), 0);
QCOMPARE(importer.renamedCount(), 0);
for (const QUrl & src : qAsConst(list)) {
QUrl dst = remoteUrl;
dst.setPath(dst.path() + '/' + src.fileName());
QVERIFY(FileUtils::contentsAreIdentical(src, dst));
}
// Secondly test import from remote back to local
QUrl localUrl = QUrl::fromLocalFile(mTempDir->path() + "/foo");
importer.start(list, localUrl);
loop.exec();
QCOMPARE(maximumChangedSpy.count(), 1);
QCOMPARE(maximumChangedSpy.takeFirst().at(0).toInt(), list.count() * 100);
QCOMPARE(errorSpy.count(), 0);
QCOMPARE(importer.importedUrlList().count(), list.count());
QCOMPARE(importer.importedUrlList(), list);
QCOMPARE(importer.skippedUrlList().count(), 0);
QCOMPARE(importer.renamedCount(), 0);
for (const QUrl & src : qAsConst(list)) {
QUrl dst = localUrl;
dst.setPath(dst.path() + '/' + src.fileName());
QVERIFY(FileUtils::contentsAreIdentical(src, dst));
}
}
void ImporterTest::testSkippedUrlList()
{
QUrl destUrl = QUrl::fromLocalFile(mTempDir->path() + "/foo");
......
......@@ -38,6 +38,7 @@ private Q_SLOTS:
void init();
void testContentsAreIdentical();
void testSuccessfulImport();
void testSuccessfulImportRemote();
void testAutoRenameFormat();
void testReadOnlyDestination();
void testFileNameFormater();
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment