Commit fba3079e authored by Harald Sitter's avatar Harald Sitter
Browse files

completely rejigger the way resuming works on smb

the resuming logic is super duplicating and was also somewhat
divergent between the duplicates.

to resolve this there's now a new unified couple of helpers that
specifically deal with establishing context on a file transfer. this
actually maybe should eventually move to KIO proper. it seems to me that
every implementation needs to do more or less the same thing except for
the IO specifics

- shouldResume is called relatively early on and establishes the
context. the context is entirely dependent on the resume configuration
at hand. if KIO::Resume is set we resume in-place, otherwise we may have
an intermediate .part file. if neither is applicable then we straight up
need to overwrite. this effectively hides the details of the
intermediate url switch. the actual transfer logic in smb_dir gets a
context back with the destination configured to where it should write to
(not necessarily where the file will end up at in the end)
- concludeResumeHasError is call...
parent 0cbdeb1f
Pipeline #86954 passed with stage
in 2 minutes and 1 second
......@@ -6,3 +6,7 @@ Source: https://invent.kde.org/network/kio-extras
Files: smb/kdsoap-ws-discovery-client/*
Copyright: Casper Meijn
License: GPL-3.0-or-later
Files: smb/autotests/fixtures/*
Copyright: none
License: CC0-1.0
......@@ -12,6 +12,7 @@ include(ECMAddTests)
ecm_add_tests(
smburltest.cpp
transfertest.cpp
shouldresumetest.cpp
LINK_LIBRARIES
Qt5::Test
kio_smb_static
......
/*
SPDX-License-Identifier: GPL-2.0-only OR GPL-3.0-only OR LicenseRef-KDE-Accepted-GPL
SPDX-FileCopyrightText: 2021 Harald Sitter <sitter@kde.org>
*/
#include <utility>
#include <QProcess>
#include <QTest>
#include <KIO/Job>
#include "transfer_resume.h"
struct FakeWorker {
struct FakeError {
const int id;
const QString message;
};
void error(int id, const QString &message)
{
m_errors.push_back(FakeError{id, message});
}
bool configValue(const QString &key, bool defaultValue) const
{
return m_config.value(key, defaultValue);
}
bool canResume(KIO::filesize_t offset)
{
Q_UNUSED(offset); // in reality this is a user query, always assume the user says yes for now
return true;
}
void debugErrors()
{
for (const auto &error : std::as_const(m_errors)) {
qDebug() << "ERROR{" << KIO::buildErrorString(error.id, error.message) << "}";
}
}
QVector<FakeError> m_errors;
QHash<QString, bool> m_config = {{"MarkPartial", true}};
};
class ShouldResumeTest : public QObject
{
Q_OBJECT
private:
QTemporaryDir m_tmpDir;
private Q_SLOTS:
void initTestCase()
{
QVERIFY(m_tmpDir.isValid());
const QString fixturesPath = QFINDTESTDATA("fixtures/.");
QProcess proc;
proc.setProcessChannelMode(QProcess::ForwardedChannels);
proc.start("cp", {"-rv", fixturesPath, m_tmpDir.path()});
QVERIFY(proc.waitForFinished());
QCOMPARE(proc.exitCode(), 0);
}
QString tmpPath(const QString &subpath)
{
return QDir(m_tmpDir.path()).filePath(subpath);
}
QUrl tmpUrl(const QString &subpath)
{
return QUrl::fromLocalFile(tmpPath(subpath));
}
void noResumeButPartial()
{
// NB: this has no fixture ;)
FakeWorker worker;
auto url = tmpUrl("noResumeButPartial/thing");
auto partUrl = tmpUrl("noResumeButPartial/thing.part");
auto resume = Transfer::shouldResume<QFileResumeIO>(url, KIO::JobFlags(), &worker);
QVERIFY(resume.has_value());
QCOMPARE(resume->resuming, false);
QCOMPARE(resume->destination, partUrl);
QCOMPARE(resume->completeDestination, url);
QCOMPARE(resume->partDestination, partUrl);
QDir().mkdir(tmpPath("noResumeButPartial"));
QFile part(partUrl.toLocalFile());
QVERIFY(part.open(QFile::WriteOnly));
part.write("");
QCOMPARE(Transfer::concludeResumeHasError<QFileResumeIO>(false, resume.value(), &worker), false);
QVERIFY(QFileInfo::exists(url.toLocalFile()));
QVERIFY(!QFileInfo::exists(partUrl.toLocalFile()));
}
void noResumeAndNoPartial()
{
// NB: this has no fixture ;)
FakeWorker worker;
worker.m_config["MarkPartial"] = false;
auto url = tmpUrl("noResumeAndNoPartial/thing");
auto resume = Transfer::shouldResume<QFileResumeIO>(url, KIO::JobFlags(), &worker);
worker.debugErrors();
QVERIFY(resume.has_value());
QCOMPARE(resume->resuming, false);
QCOMPARE(resume->destination, url);
QCOMPARE(resume->completeDestination, url);
QCOMPARE(resume->partDestination, QUrl());
QCOMPARE(Transfer::concludeResumeHasError<QFileResumeIO>(false, resume.value(), &worker), false);
}
void resume()
{
FakeWorker worker;
auto url = tmpUrl("resume/thing");
auto partUrl = tmpUrl("resume/thing.part");
auto resume = Transfer::shouldResume<QFileResumeIO>(url, KIO::JobFlags(), &worker);
QVERIFY(resume.has_value());
QCOMPARE(resume->resuming, true);
QCOMPARE(resume->destination, partUrl);
QCOMPARE(resume->completeDestination, url);
QCOMPARE(resume->partDestination, partUrl);
QCOMPARE(Transfer::concludeResumeHasError<QFileResumeIO>(false, resume.value(), &worker), false);
QVERIFY(QFileInfo::exists(url.toLocalFile()));
QVERIFY(!QFileInfo::exists(partUrl.toLocalFile()));
}
void resumeInPlace()
{
FakeWorker worker;
auto url = tmpUrl("resumeInPlace/thing");
auto resume = Transfer::shouldResume<QFileResumeIO>(url, KIO::Resume, &worker);
QVERIFY(resume.has_value());
QCOMPARE(resume->resuming, true);
QCOMPARE(resume->destination, url);
QCOMPARE(resume->completeDestination, url);
QCOMPARE(resume->partDestination, url);
QCOMPARE(Transfer::concludeResumeHasError<QFileResumeIO>(false, resume.value(), &worker), false);
QVERIFY(QFileInfo::exists(url.toLocalFile()));
}
void noResumeInPlace()
{
FakeWorker worker;
auto url = tmpUrl("resumeInPlace/thing"); // intentionally the same path this scenario errors out
auto resume = Transfer::shouldResume<QFileResumeIO>(url, KIO::JobFlags(), &worker);
QVERIFY(!resume.has_value());
QCOMPARE(worker.m_errors.size(), 1);
QCOMPARE(worker.m_errors.at(0).id, KIO::ERR_FILE_ALREADY_EXIST);
}
};
QTEST_GUILESS_MAIN(ShouldResumeTest)
#include "shouldresumetest.moc"
/*
SPDX-License-Identifier: GPL-2.0-only OR GPL-3.0-only OR LicenseRef-KDE-Accepted-GPL
SPDX-FileCopyrightText: 2020 Harald Sitter <sitter@kde.org>
SPDX-FileCopyrightText: 2020-2021 Harald Sitter <sitter@kde.org>
*/
#include <QTest>
......@@ -133,6 +133,16 @@ private Q_SLOTS:
QCOMPARE(SMBUrl(QUrl("file:///home/foo/bar")).getType(), SMBURLTYPE_UNKNOWN);
QCOMPARE(SMBUrl(QUrl("sftp://me@localhost/foo/bar")).getType(), SMBURLTYPE_UNKNOWN);
}
void testFileParts()
{
// We use SMBUrl for transfers from and to local files as well, make sure it behaves accordingly.
SMBUrl url(QUrl("file:///foo"));
QCOMPARE(QUrl("file:///foo"), url);
QCOMPARE(QUrl("file:///foo.part"), url.partUrl());
// Clearly not a file should not work
QCOMPARE(QUrl(), SMBUrl(QUrl("file:///")).partUrl());
}
};
QTEST_GUILESS_MAIN(SMBUrlTest)
......
......@@ -71,6 +71,7 @@ class SMBSlave : public QObject, public KIO::SlaveBase
{
Q_OBJECT
friend class SMBCDiscoverer;
friend class SMBResumeIO;
SlaveFrontend m_frontend { *this };
SMBContext m_context { new SMBAuthenticator(m_frontend) };
......
......@@ -17,6 +17,7 @@
#include <future>
#include "transfer.h"
#include "transfer_resume.h"
void SMBSlave::copy(const QUrl &src, const QUrl &dst, int permissions, KIO::JobFlags flags)
{
......@@ -187,35 +188,17 @@ void SMBSlave::smbCopyGet(const QUrl &ksrc, const QUrl &kdst, int permissions, K
}
}
bool bResume = false;
const QFileInfo partInfo(dstFile + QLatin1String(".part"));
const bool bPartExists = partInfo.exists();
const bool bMarkPartial = configValue(QStringLiteral("MarkPartial"), true);
if (bMarkPartial && bPartExists && partInfo.size() > 0) {
if (partInfo.isDir()) {
error(ERR_IS_DIRECTORY, partInfo.absoluteFilePath());
return;
}
bResume = canResume(partInfo.size());
auto optionalResume = Transfer::shouldResume<QFileResumeIO>(kdst, flags, this);
if (!optionalResume.has_value()) { // had error
return;
}
if (bPartExists && !bResume) // get rid of an unwanted ".part" file
QFile::remove(partInfo.absoluteFilePath());
const auto resume = optionalResume.value();
// open the output file...
QFile::OpenMode mode;
QString filename;
if (bResume) {
filename = partInfo.absoluteFilePath();
mode = QFile::WriteOnly | QFile::Append;
} else {
filename = (bMarkPartial ? partInfo.absoluteFilePath() : dstFile);
mode = QFile::WriteOnly | QFile::Truncate;
}
const QFile::OpenMode mode = resume.resuming ? (QFile::WriteOnly | QFile::Append) : (QFile::WriteOnly | QFile::Truncate);
QFile file(filename);
if (!bResume) {
QFile file(resume.destination.path());
if (!resume.resuming) {
QFile::Permissions perms;
if (permissions == -1) {
perms = QFile::ReadOwner | QFile::WriteOwner;
......@@ -229,7 +212,7 @@ void SMBSlave::smbCopyGet(const QUrl &ksrc, const QUrl &kdst, int permissions, K
qCDebug(KIO_SMB_LOG) << "could not write to" << dstFile;
switch (file.error()) {
case QFile::OpenError:
if (bResume) {
if (resume.resuming) {
error(ERR_CANNOT_RESUME, kdst.toDisplayString());
} else {
error(ERR_CANNOT_OPEN_FOR_WRITING, kdst.toDisplayString());
......@@ -272,9 +255,9 @@ void SMBSlave::smbCopyGet(const QUrl &ksrc, const QUrl &kdst, int permissions, K
errNum = errno;
} else {
errNum = 0;
if (bResume) {
qCDebug(KIO_SMB_LOG) << "seeking to size" << partInfo.size();
off_t offset = smbc_lseek(srcfd, partInfo.size(), SEEK_SET);
if (resume.resuming) {
qCDebug(KIO_SMB_LOG) << "seeking to size" << resume.destinationOffset;
off_t offset = smbc_lseek(srcfd, resume.destinationOffset, SEEK_SET);
if (offset == -1) {
error(KIO::ERR_CANNOT_SEEK, src.toDisplayString());
smbc_close(srcfd);
......@@ -343,29 +326,9 @@ void SMBSlave::smbCopyGet(const QUrl &ksrc, const QUrl &kdst, int permissions, K
smbc_close(srcfd);
// Handle error condition.
if (isErr) {
const QString sPart = partInfo.absoluteFilePath();
if (bMarkPartial) {
const int size = configValue(QStringLiteral("MinimumKeepSize"), DEFAULT_MINIMUM_KEEP_SIZE);
if (partInfo.size() < size) {
QFile::remove(sPart);
}
}
return;
}
// Rename partial file to its original name.
if (bMarkPartial) {
const QString sPart = partInfo.absoluteFilePath();
// Remove old dest file if it exists..
if (dstInfo.exists()) {
QFile::remove(dstFile);
}
if (!QFile::rename(sPart, dstFile)) {
qCDebug(KIO_SMB_LOG) << "failed to rename" << sPart << "to" << dstFile;
error(ERR_CANNOT_RENAME_PARTIAL, sPart);
return;
}
if (Transfer::concludeResumeHasError<QFileResumeIO>(isErr, resume, this)) {
return; // NB: error() called inside if applicable
}
// set modification time (if applicable)
......@@ -410,39 +373,20 @@ void SMBSlave::smbCopyPut(const QUrl &ksrc, const QUrl &kdst, int permissions, K
totalSize(static_cast<filesize_t>(srcInfo.size()));
bool bResume = false;
bool bPartExists = false;
const bool bMarkPartial = configValue(QStringLiteral("MarkPartial"), true);
const SMBUrl dstOrigUrl(kdst);
if (bMarkPartial) {
const int errNum = cache_stat(dstOrigUrl.partUrl(), &st);
bPartExists = (errNum == 0);
if (bPartExists) {
if (!(flags & KIO::Overwrite) && !(flags & KIO::Resume)) {
bResume = canResume(st.st_size);
} else {
bResume = (flags & KIO::Resume);
}
}
}
int dstfd = -1;
int errNum = cache_stat(dstOrigUrl, &st);
if (errNum == 0 && !(flags & KIO::Overwrite) && !(flags & KIO::Resume)) {
if (S_ISDIR(st.st_mode)) {
error(KIO::ERR_IS_DIRECTORY, dstOrigUrl.toDisplayString());
} else {
error(KIO::ERR_FILE_ALREADY_EXIST, dstOrigUrl.toDisplayString());
}
const std::optional<TransferContext> resumeOptional = Transfer::shouldResume<SMBResumeIO>(dstOrigUrl, flags, this);
if (!resumeOptional.has_value()) { // had an error
return;
}
const TransferContext &resume = resumeOptional.value();
KIO::filesize_t processed_size = 0;
const SMBUrl dstUrl(bMarkPartial ? dstOrigUrl.partUrl() : dstOrigUrl);
const SMBUrl dstUrl(resume.destination);
if (bResume) {
int dstfd = -1;
int errNum = 0;
if (resume.resuming) {
// append if resuming
qCDebug(KIO_SMB_LOG) << "resume" << dstUrl;
dstfd = smbc_open(dstUrl.toSmbcUrl(), O_RDWR, 0);
......@@ -485,7 +429,6 @@ void SMBSlave::smbCopyPut(const QUrl &ksrc, const QUrl &kdst, int permissions, K
}
bool isErr = false;
if (processed_size == 0 || srcFile.seek(processed_size)) {
// Perform the copy
TransferSegment segment(srcInfo.size());
......@@ -521,27 +464,9 @@ void SMBSlave::smbCopyPut(const QUrl &ksrc, const QUrl &kdst, int permissions, K
return;
}
// Handle error condition.
if (isErr) {
if (bMarkPartial) {
const int size = configValue(QStringLiteral("MinimumKeepSize"), DEFAULT_MINIMUM_KEEP_SIZE);
const int errNum = cache_stat(dstUrl, &st);
if (errNum == 0 && st.st_size < size) {
smbc_unlink(dstUrl.toSmbcUrl());
}
}
return;
}
// Rename partial file to its original name.
if (bMarkPartial) {
smbc_unlink(dstOrigUrl.toSmbcUrl());
if (smbc_rename(dstUrl.toSmbcUrl(), dstOrigUrl.toSmbcUrl()) < 0) {
qCDebug(KIO_SMB_LOG) << "failed to rename" << dstUrl << "to" << dstOrigUrl << "->" << strerror(errno);
error(ERR_CANNOT_RENAME_PARTIAL, dstUrl.toDisplayString());
return;
}
}
if (Transfer::concludeResumeHasError<SMBResumeIO>(isErr, resume, this)) {
return; // NB: error() called inside if applicable
}
applyMTimeSMBC(dstOrigUrl);
......
/*
SPDX-License-Identifier: GPL-2.0-or-later
SPDX-FileCopyrightText: 2000 Caldera Systems Inc.
SPDX-FileCopyrightText: 2020 Harald Sitter <sitter@kde.org>
SPDX-FileCopyrightText: 2020-2021 Harald Sitter <sitter@kde.org>
SPDX-FileContributor: Matthew Peterson <mpeterson@caldera.com>
*/
......@@ -179,7 +179,10 @@ SMBUrlType SMBUrl::getType() const
SMBUrl SMBUrl::partUrl() const
{
if (m_type == SMBURLTYPE_SHARE_OR_PATH && !fileName().isEmpty()) {
const bool isRemoteFile = m_type == SMBURLTYPE_SHARE_OR_PATH && !fileName().isEmpty();
const bool isLocalFile = scheme() == QLatin1String("file") && !fileName().isEmpty();
// Mind that filename doesn't necessarily mean it is a file.
if (isRemoteFile || isLocalFile) {
SMBUrl url(*this);
url.setPath(path() + QLatin1String(".part"));
return url;
......
/*
SPDX-License-Identifier: GPL-2.0-only OR GPL-3.0-only OR LicenseRef-KDE-Accepted-GPL
SPDX-FileCopyrightText: 2021 Harald Sitter <sitter@kde.org>
*/
#pragma once
#include <optional>
#include <QFileInfo>
#include <kio/ioslave_defaults.h>
#include "kio_smb.h"
// Carries the context of a file transfer.
struct TransferContext {
// When resuming a file. This is false when starting a new .part!
// To establish if a partial file is used the completeDestination should be compared with the partDestination.
const bool resuming;
// The intermediate destination
const SMBUrl destination;
// The part destination. This is null when not using a partial file.
const SMBUrl partDestination;
// The complete destination i.e. the final destination i.e. the place where the file will be once all is said and done
const SMBUrl completeDestination;
// The offest to resume from in the destination. Naturally only should be used when resuming is true.
const off_t destinationOffset = -1;
};
// Simple encapsulation for SMB resume IO for use with shouldResume.
// This hides the specific IO concern from the resume logic such that it can be used with either SMB IO or local IO.
class SMBResumeIO
{
public:
explicit SMBResumeIO(const SMBUrl &url)
: m_url(url)
// m_stat implicitly init'd by the stat for m_exists
, m_exists(SMBSlave::cache_stat(m_url, &m_stat) == 0)
{
}
bool exists() const
{
return m_exists;
}
off_t size() const
{
return m_stat.st_size;
}
bool isDir() const
{
return S_ISDIR(m_stat.st_mode);
}
bool remove()
{
return smbc_unlink(m_url.toSmbcUrl());
}
bool renameTo(const SMBUrl &newUrl)
{
smbc_unlink(newUrl.toSmbcUrl());
if (smbc_rename(m_url.toSmbcUrl(), newUrl.toSmbcUrl()) < 0) {
qCDebug(KIO_SMB_LOG) << "SMB failed to rename" << m_url << "to" << newUrl << "->" << strerror(errno);
return false;
}
return true;
}
private:
const SMBUrl m_url;
struct stat m_stat {
};
bool m_exists;
};
// Simple encapsulation for local resume IO for use with shouldResume.
// This hides the specific IO concern from the resume logic such that it can be used with either SMB IO or local IO.
class QFileResumeIO : public QFileInfo
{
public:
explicit QFileResumeIO(const SMBUrl &url)
: QFileInfo(url.path())
{
qDebug() << url.path();
}
bool remove()
{
return QFile::remove(filePath());
}
bool renameTo(const SMBUrl &newUrl)
{
QFile::remove(newUrl.path());
if (!QFile::rename(filePath(), newUrl.path())) {
qCDebug(KIO_SMB_LOG) << "failed to rename" << filePath() << "to" << newUrl.path();
return false;
}
return true;
}
private:
const SMBUrl m_url;
};
namespace Transfer
{
// Check if we should resume the upload to destination.
// This returns nullopt when an error has ocurred. The error() function is called internally.
// NB: WorkerInterface is intentionally duck-typed so we can unit test with a mock entity that looks like a SlaveBase but isn't one.
// Similarly ResumeIO is duck-typed so we can use QFileInfo as as base class in one implementation but not the other,
// allowing us to cut down on boilerplate call-forwarding code.
template<typename ResumeIO, typename WorkerInterface>
Q_REQUIRED_RESULT std::optional<TransferContext> shouldResume(const SMBUrl &destination, KIO::JobFlags flags, WorkerInterface *worker)
{
// Resumption has two presentations:
// a) partial resumption - when a .part file is left behind and we pick up where that part left off
// b) in-place resumption - when we are expected to append to the actual destination file without
// .part temporary in between (FIXME behavior is largely unclear and the below logic is possibly not correct
// https://invent.kde.org/frameworks/kio/-/issues/9)
const bool markPartial = worker->configValue(QStringLiteral("MarkPartial"), true);
if (const ResumeIO destIO(destination); destIO.exists()) {
if (const bool resume = static_cast<bool>(flags & KIO::Resume); resume && destIO.exists()) {
// We are resuming the destination file directly!
return TransferContext{resume, destination, destination, destination, destIO.size()};
}
// Not a resume operation -> if we also were not told to overwrite then we can't process this copy at all
// because the ultimate destination already exists.
if (!(flags & KIO::Overwrite)) {
worker->error(destIO.isDir() ? KIO::ERR_IS_DIRECTORY : KIO::ERR_FILE_ALREADY_EXIST, destination.toDisplayString());
return std::nullopt;
}
}
if (markPartial) {
const SMBUrl partUrl = destination.partUrl();
if (ResumeIO partIO(partUrl); partIO.exists() && worker->canResume(partIO.size())) {
return TransferContext{true, partUrl, partUrl, destination, partIO.size()};
}
return TransferContext{false, partUrl, partUrl, destination}; // new part file without offsets or resume
}
// The part file is not enabled or present, neither is KIO::Resume enabled and the dest file present -> regular
// transfer without resuming of anything.
return TransferContext{false, destination, QUrl(), destination};
}
// Concludes the resuming. This ought to be called after writing to the destination has
// completed. Destination should be closed. isError is the potential error state. When isError is true,
// the partial file may get discarded (depending on it existing and having an insufficient size).
// The return value is true when an error has occurred. When isError was true this can only ever return true.
template<typename ResumeIO, typename WorkerInterface>
Q_REQUIRED_RESULT bool concludeResumeHasError(bool isError, const TransferContext &resume, WorkerInterface *worker)
{
qDebug() << "concluding" << resume.destination << resume.partDestination << resume.completeDestination;
if (resume.destination == resume.completeDestination) {
return isError;
}
// Handle error condition.
if (isError) {
const off_t minimumSize = worker->configValue(QStringLiteral("MinimumKeepSize"), DEFAULT_MINIMUM_KEEP_SIZE);
// TODO should this be partdestination?
if (ResumeIO destIO(resume.destination); destIO.exists() && destIO.size() < minimumSize) {
destIO.remove();
}
return true;
}
// Rename partial file to its original name. The ResumeIO takes care of potential removing of the destination.
if (ResumeIO partIO(resume.partDestination); !partIO.renameTo(resume.completeDestination)) {
worker->