Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
4f3f642
Upload: refactor the upload in two classes so the new chuning can be …
ogoffart May 20, 2016
a155810
WIP: new chunking algorithm
ogoffart Aug 2, 2016
fad387b
Chunking-Ng: Resume
ogoffart Aug 2, 2016
79abb8b
ChunkingNg: enable depending on an environment variable
ogoffart Aug 3, 2016
818b585
Chunking-NG: Qt4 compile
ogoffart Aug 3, 2016
c41f6ed
Chunking-NG: use new dav URL for the move
ogoffart Aug 31, 2016
7c75a39
Chunking-NG: Some fixup after feedback from the pull request #5102
ogoffart Sep 10, 2016
4c79ce2
ConnectionValidator: fetch the account name.
ogoffart Sep 16, 2016
28018e8
Chunking-NG: Fix destination URL
ogoffart Sep 16, 2016
c222793
Chunking-NG: use OC-If-Destination-Match instread of If-Match
ogoffart Sep 16, 2016
3c24d5a
Chunking-NG: The MOVE will return the code 204 if the file was alread…
ogoffart Sep 16, 2016
da26e59
Chunking-NG: add some headers when uploading chunks
ogoffart Sep 16, 2016
0960058
Merge remote-tracking branch 'origin/master' into chunking-ng
ogoffart Oct 20, 2016
273590f
ChunkingNG: Use the 'If' header
ogoffart Oct 20, 2016
e33b89c
Chunking-NG: Enable if the server supports it
ogoffart Oct 21, 2016
8ca3eb7
Merge remote-tracking branch 'origin/master' into chunking-ng
ogoffart Oct 31, 2016
456d827
Fix compile after merge
ogoffart Oct 31, 2016
c8014a0
ChunkingNG: Add Test
ogoffart Oct 31, 2016
15f2b91
ChunkingNG: remove stale files when resuming
ogoffart Oct 31, 2016
5377d1e
Chunking-NG: code cleanup after review
ogoffart Nov 14, 2016
92027e8
SyncEngineTestUtils: Do don't allocate a buffer for the whole file
ogoffart Nov 15, 2016
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
2 changes: 2 additions & 0 deletions src/libsync/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ set(libsync_SRCS
propagatorjobs.cpp
propagatedownload.cpp
propagateupload.cpp
propagateuploadv1.cpp
propagateuploadng.cpp
propagateremotedelete.cpp
propagateremotemove.cpp
propagateremotemkdir.cpp
Expand Down
12 changes: 10 additions & 2 deletions src/libsync/account.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,19 @@ AccountPtr Account::sharedFromThis()
return _sharedThis.toStrongRef();
}

QString Account::user() const
{
return _user.isEmpty() ? _credentials->user() : _user;
}

void Account::setUser(const QString &user)
{
_user = user;
}

QString Account::displayName() const
{
auto user = _credentials->user();
QString dn = QString("%1@%2").arg(user, _url.host());
QString dn = QString("%1@%2").arg(user(), _url.host());
int port = url().port();
if (port > 0 && port != 80 && port != 443) {
dn.append(QLatin1Char(':'));
Expand Down
5 changes: 5 additions & 0 deletions src/libsync/account.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ class OWNCLOUDSYNC_EXPORT Account : public QObject {

AccountPtr sharedFromThis();

/// The user that can be used in dav url

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please comment how this relates to the shibboleth user or the user name entered in the wizard

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know exactly. the answer from @DeepDiver1975 on #engineering (17:26) was not enough for me to make a better comment.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment explaining in what situations that would be different from credentials.user()?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know. I was just told it could be different and that i had to do an API call even when using the HTTPCrendentials.

@DeepDiver1975 Can you clarify?

QString user() const;
void setUser(const QString &user);

/// The name of the account as shown in the toolbar
QString displayName() const;

Expand Down Expand Up @@ -198,6 +202,7 @@ protected Q_SLOTS:

QWeakPointer<Account> _sharedThis;
QString _id;
QString _user;
QMap<QString, QVariant> _settingsMap;
QUrl _url;
QList<QSslCertificate> _approvedCerts;
Expand Down
6 changes: 6 additions & 0 deletions src/libsync/capabilities.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,10 @@ QByteArray Capabilities::uploadChecksumType() const
return QByteArray();
}

bool Capabilities::chunkingNg() const
{
return _capabilities["dav"].toMap()["chunking"].toByteArray() >= "1.0";
}


}
1 change: 1 addition & 0 deletions src/libsync/capabilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class OWNCLOUDSYNC_EXPORT Capabilities {
bool sharePublicLinkEnforceExpireDate() const;
int sharePublicLinkExpireDateDays() const;
bool shareResharing() const;
bool chunkingNg() const;

/// returns true if the capabilities report notifications
bool notificationsAvailable() const;
Expand Down
20 changes: 18 additions & 2 deletions src/libsync/connectionvalidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -229,10 +229,26 @@ void ConnectionValidator::slotCapabilitiesRecieved(const QVariantMap &json)
auto caps = json.value("ocs").toMap().value("data").toMap().value("capabilities");
qDebug() << "Server capabilities" << caps;
_account->setCapabilities(caps.toMap());
reportResult(Connected);
return;
fetchUser();
}

void ConnectionValidator::fetchUser()
{

JsonApiJob *job = new JsonApiJob(_account, QLatin1String("ocs/v1.php/cloud/user"), this);
job->setTimeout(timeoutToUseMsec);
QObject::connect(job, SIGNAL(jsonReceived(QVariantMap, int)), this, SLOT(slotUserFetched(QVariantMap)));
job->start();
}

void ConnectionValidator::slotUserFetched(const QVariantMap &json)
{
QString user = json.value("ocs").toMap().value("data").toMap().value("id").toString();
if (!user.isEmpty()) {
_account->setUser(user);
}
reportResult(Connected);
}

void ConnectionValidator::reportResult(Status status)
{
Expand Down
14 changes: 12 additions & 2 deletions src/libsync/connectionvalidator.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ namespace OCC {
* This is a job-like class to check that the server is up and that we are connected.
* There are two entry points: checkServerAndAuth and checkAuthentication
* checkAuthentication is the quick version that only does the propfind
* while checkServerAndAuth is doing the 3 calls.
* while checkServerAndAuth is doing the 4 calls.
*
* We cannot use the capabilites call to test the login and the password because of
* https://github.com/owncloud/core/issues/12930
Expand Down Expand Up @@ -60,7 +60,15 @@ namespace OCC {
+-> checkServerCapabilities (cloud/capabilities)
JsonApiJob
|
+-> slotCapabilitiesRecieved --> X
+-> slotCapabilitiesRecieved -+
|
+-----------------------------------+
|
+-> fetchUser
PropfindJob
|
+-> slotUserFetched --> X

\endcode
*/
class OWNCLOUDSYNC_EXPORT ConnectionValidator : public QObject
Expand Down Expand Up @@ -109,10 +117,12 @@ protected slots:
void slotAuthSuccess();

void slotCapabilitiesRecieved(const QVariantMap&);
void slotUserFetched(const QVariantMap &);

private:
void reportResult(Status status);
void checkServerCapabilities();
void fetchUser();

QStringList _errors;
AccountPtr _account;
Expand Down
20 changes: 18 additions & 2 deletions src/libsync/networkjobs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,24 @@ MkColJob::MkColJob(AccountPtr account, const QString &path, QObject *parent)
{
}

MkColJob::MkColJob(AccountPtr account, const QUrl &url,
const QMap<QByteArray, QByteArray> &extraHeaders, QObject *parent)
: AbstractNetworkJob(account, QString(), parent), _url(url), _extraHeaders(extraHeaders)
{
}

void MkColJob::start()
{
// add 'Content-Length: 0' header (see https://github.com/owncloud/client/issues/3256)
QNetworkRequest req;
req.setRawHeader("Content-Length", "0");
for(auto it = _extraHeaders.constBegin(); it != _extraHeaders.constEnd(); ++it) {
req.setRawHeader(it.key(), it.value());
}

// assumes ownership
QNetworkReply *reply = davRequest("MKCOL", path(), req);
QNetworkReply *reply = _url.isValid() ? davRequest("MKCOL", _url, req)
: davRequest("MKCOL", path(), req);
setReply(reply);
setupConnections(reply);
AbstractNetworkJob::start();
Expand Down Expand Up @@ -264,6 +274,11 @@ LsColJob::LsColJob(AccountPtr account, const QString &path, QObject *parent)
{
}

LsColJob::LsColJob(AccountPtr account, const QUrl &url, QObject *parent)
: AbstractNetworkJob(account, QString(), parent), _url(url)
{
}

void LsColJob::setProperties(QList<QByteArray> properties)
{
_properties = properties;
Expand Down Expand Up @@ -307,7 +322,8 @@ void LsColJob::start()
QBuffer *buf = new QBuffer(this);
buf->setData(xml);
buf->open(QIODevice::ReadOnly);
QNetworkReply *reply = davRequest("PROPFIND", path(), req, buf);
QNetworkReply *reply = _url.isValid() ? davRequest("PROPFIND", _url, req, buf)
: davRequest("PROPFIND", path(), req, buf);
buf->setParent(reply);
setReply(reply);
setupConnections(reply);
Expand Down
6 changes: 6 additions & 0 deletions src/libsync/networkjobs.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ class OWNCLOUDSYNC_EXPORT LsColJob : public AbstractNetworkJob {
Q_OBJECT
public:
explicit LsColJob(AccountPtr account, const QString &path, QObject *parent = 0);
explicit LsColJob(AccountPtr account, const QUrl &url, QObject *parent = 0);
void start() Q_DECL_OVERRIDE;
QHash<QString, qint64> _sizes;

Expand All @@ -87,6 +88,7 @@ private slots:

private:
QList<QByteArray> _properties;
QUrl _url; // Used instead of path() if the url is specified in the constructor
};

/**
Expand Down Expand Up @@ -170,8 +172,12 @@ private slots:
*/
class OWNCLOUDSYNC_EXPORT MkColJob : public AbstractNetworkJob {
Q_OBJECT
QUrl _url; // Only used if the constructor taking a url is taken.
QMap<QByteArray, QByteArray> _extraHeaders;
public:
explicit MkColJob(AccountPtr account, const QString &path, QObject *parent = 0);
explicit MkColJob(AccountPtr account, const QUrl &url,
const QMap<QByteArray, QByteArray> &extraHeaders, QObject *parent = 0);
void start() Q_DECL_OVERRIDE;

signals:
Expand Down
9 changes: 8 additions & 1 deletion src/libsync/owncloudpropagator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,14 @@ PropagateItemJob* OwncloudPropagator::createJob(const SyncFileItemPtr &item) {
job->setDeleteExistingFolder(deleteExisting);
return job;
} else {
auto job = new PropagateUploadFile(this, item);
PropagateUploadFileCommon *job = 0;
static const auto chunkng = qgetenv("OWNCLOUD_CHUNKING_NG");
if (item->_size > chunkSize()
&& (account()->capabilities().chunkingNg() || chunkng == "1") && chunkng != "0") {
job = new PropagateUploadFileNG(this, item);
} else {
job = new PropagateUploadFileV1(this, item);
}
job->setDeleteExisting(deleteExisting);
return job;
}
Expand Down
3 changes: 2 additions & 1 deletion src/libsync/owncloudpropagator.h
Original file line number Diff line number Diff line change
Expand Up @@ -379,10 +379,11 @@ private slots:
#if QT_VERSION < QT_VERSION_CHECK(5, 0, 0)
// access to signals which are protected in Qt4
friend class PropagateDownloadFile;
friend class PropagateUploadFile;
friend class PropagateLocalMkdir;
friend class PropagateLocalRename;
friend class PropagateRemoteMove;
friend class PropagateUploadFileV1;
friend class PropagateUploadFileNG;
#endif
};

Expand Down
5 changes: 4 additions & 1 deletion src/libsync/propagateremotedelete.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,14 @@ DeleteJob::DeleteJob(AccountPtr account, const QString& path, QObject* parent)
: AbstractNetworkJob(account, path, parent)
{ }

DeleteJob::DeleteJob(AccountPtr account, const QUrl& url, QObject* parent)
: AbstractNetworkJob(account, QString(), parent), _url(url)
{ }

void DeleteJob::start()
{
QNetworkRequest req;
setReply(davRequest("DELETE", path(), req));
setReply(_url.isValid() ? davRequest("DELETE", _url, req) : davRequest("DELETE", path(), req));
setupConnections(reply());

if( reply()->error() != QNetworkReply::NoError ) {
Expand Down
2 changes: 2 additions & 0 deletions src/libsync/propagateremotedelete.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ namespace OCC {
*/
class DeleteJob : public AbstractNetworkJob {
Q_OBJECT
QUrl _url; // Only used if the constructor taking a url is taken.
public:
explicit DeleteJob(AccountPtr account, const QString& path, QObject* parent = 0);
explicit DeleteJob(AccountPtr account, const QUrl& url, QObject* parent = 0);

void start() Q_DECL_OVERRIDE;
bool finished() Q_DECL_OVERRIDE;
Expand Down
10 changes: 9 additions & 1 deletion src/libsync/propagateremotemove.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,20 @@ MoveJob::MoveJob(AccountPtr account, const QString& path,
: AbstractNetworkJob(account, path, parent), _destination(destination)
{ }

MoveJob::MoveJob(AccountPtr account, const QUrl& url, const QString &destination,
QMap<QByteArray, QByteArray> extraHeaders, QObject* parent)
: AbstractNetworkJob(account, QString(), parent), _destination(destination), _url(url)
, _extraHeaders(extraHeaders)
{ }

void MoveJob::start()
{
QNetworkRequest req;
req.setRawHeader("Destination", QUrl::toPercentEncoding(_destination, "/"));
setReply(davRequest("MOVE", path(), req));
for(auto it = _extraHeaders.constBegin(); it != _extraHeaders.constEnd(); ++it) {
req.setRawHeader(it.key(), it.value());
}
setReply(_url.isValid() ? davRequest("MOVE", _url, req) : davRequest("MOVE", path(), req));
setupConnections(reply());

if( reply()->error() != QNetworkReply::NoError ) {
Expand Down
4 changes: 4 additions & 0 deletions src/libsync/propagateremotemove.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,12 @@ namespace OCC {
class MoveJob : public AbstractNetworkJob {
Q_OBJECT
const QString _destination;
const QUrl _url; // Only used (instead of path) when the constructor taking an URL is used
QMap<QByteArray, QByteArray> _extraHeaders;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if this should somehow be in AbstractNetworkJob? but maybe it can't if the subclasses are the ones instaciating the reply..

so not important.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had the same thought. Adding the "manual url" override for each job doesn't look nice. Could there be a target() function that returns path() or _url that derived classes could use?

Similarly, maybe extraHeaders could be on the AbstractNetworkJob and subclasses could call applyExtraHeaders on their QNetworkRequests?

Finally, could the url override and extra headers be setters?

public:
explicit MoveJob(AccountPtr account, const QString& path, const QString &destination, QObject* parent = 0);
explicit MoveJob(AccountPtr account, const QUrl& url, const QString &destination,
QMap<QByteArray, QByteArray> _extraHeaders, QObject* parent = 0);

void start() Q_DECL_OVERRIDE;
bool finished() Q_DECL_OVERRIDE;
Expand Down
Loading