From a831164d657b4352079ac667d03937042e225787 Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Fri, 21 Jul 2017 11:28:15 +0200 Subject: [PATCH] Revert "Revert "Discovery: consider also the "shared by me" as shared"" This reverts commit efa7821dd28491ad0b93fec53be6d4ad0f94f19f. This reverts the revert, but also add a check that the server version is bigger than 10.0 Issue #4788 --- src/libsync/discoveryphase.cpp | 22 +++++++++++++++++++++- src/libsync/syncfilestatus.cpp | 14 +++++++------- src/libsync/syncfilestatus.h | 8 ++++---- src/libsync/syncfilestatustracker.cpp | 2 +- test/syncenginetestutils.h | 4 +++- test/testsyncfilestatustracker.cpp | 7 ++++++- 6 files changed, 42 insertions(+), 15 deletions(-) diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp index 16b98faeb..dcad77390 100644 --- a/src/libsync/discoveryphase.cpp +++ b/src/libsync/discoveryphase.cpp @@ -276,6 +276,10 @@ void DiscoverySingleDirectoryJob::start() << "http://owncloud.org/ns:checksums"; if (_isRootPath) props << "http://owncloud.org/ns:data-fingerprint"; + if (_account->serverVersionInt() >= Account::makeServerVersion(10, 0, 0)) { + // Server older than 10.0 have performances issue if we ask for the share-types on every PROPFIND + props << "http://owncloud.org/ns:share-types"; + } lsColJob->setProperties(props); @@ -371,9 +375,25 @@ static csync_vio_file_stat_t *propertyMapToFileStat(const QMap if (!checksum.isEmpty()) { file_stat->checksumHeader = strdup(checksum.constData()); } + } else if (property == "share-types" && !value.isEmpty()) { + // Since QMap is sorted, "share-types" is always "permissions". + if (file_stat->remotePerm[0] == '\0' || !(file_stat->fields & CSYNC_VIO_FILE_STAT_FIELDS_PERM)) { + qWarning() << "Server returned a share type, but no permissions?"; + } else { + // S means shared with me. + // But for our purpose, we want to know if the file is shared. It does not matter + // if we are the owner or not. + // Piggy back on the persmission field 'S' + if (!std::strchr(file_stat->remotePerm, 'S')) { + if (std::strlen(file_stat->remotePerm) < sizeof(file_stat->remotePerm) - 1) { + std::strcat(file_stat->remotePerm, "S"); + } else { + qWarning() << "permissions too large" << file_stat->remotePerm; + } + } + } } } - return file_stat; } diff --git a/src/libsync/syncfilestatus.cpp b/src/libsync/syncfilestatus.cpp index 12a997786..8d1a5933f 100644 --- a/src/libsync/syncfilestatus.cpp +++ b/src/libsync/syncfilestatus.cpp @@ -17,13 +17,13 @@ namespace OCC { SyncFileStatus::SyncFileStatus() : _tag(StatusNone) - , _sharedWithMe(false) + , _shared(false) { } SyncFileStatus::SyncFileStatus(SyncFileStatusTag tag) : _tag(tag) - , _sharedWithMe(false) + , _shared(false) { } @@ -37,14 +37,14 @@ SyncFileStatus::SyncFileStatusTag SyncFileStatus::tag() const return _tag; } -void SyncFileStatus::setSharedWithMe(bool isShared) +void SyncFileStatus::setShared(bool isShared) { - _sharedWithMe = isShared; + _shared = isShared; } -bool SyncFileStatus::sharedWithMe() const +bool SyncFileStatus::shared() const { - return _sharedWithMe; + return _shared; } QString SyncFileStatus::toSocketAPIString() const @@ -71,7 +71,7 @@ QString SyncFileStatus::toSocketAPIString() const statusString = QLatin1String("ERROR"); break; } - if (canBeShared && _sharedWithMe) { + if (canBeShared && _shared) { statusString += QLatin1String("+SWM"); } diff --git a/src/libsync/syncfilestatus.h b/src/libsync/syncfilestatus.h index 27006a572..e4743f7a3 100644 --- a/src/libsync/syncfilestatus.h +++ b/src/libsync/syncfilestatus.h @@ -44,19 +44,19 @@ public: void set(SyncFileStatusTag tag); SyncFileStatusTag tag() const; - void setSharedWithMe(bool isShared); - bool sharedWithMe() const; + void setShared(bool isShared); + bool shared() const; QString toSocketAPIString() const; private: SyncFileStatusTag _tag; - bool _sharedWithMe; + bool _shared; }; inline bool operator==(const SyncFileStatus &a, const SyncFileStatus &b) { - return a.tag() == b.tag() && a.sharedWithMe() == b.sharedWithMe(); + return a.tag() == b.tag() && a.shared() == b.shared(); } inline bool operator!=(const SyncFileStatus &a, const SyncFileStatus &b) diff --git a/src/libsync/syncfilestatustracker.cpp b/src/libsync/syncfilestatustracker.cpp index e643099b2..9d4fa6e37 100644 --- a/src/libsync/syncfilestatustracker.cpp +++ b/src/libsync/syncfilestatustracker.cpp @@ -286,7 +286,7 @@ SyncFileStatus SyncFileStatusTracker::resolveSyncAndErrorStatus(const QString &r ASSERT(sharedFlag != UnknownShared, "The shared status needs to have been fetched from a SyncFileItem or the DB at this point."); if (sharedFlag == Shared) - status.setSharedWithMe(true); + status.setShared(true); return status; } diff --git a/test/syncenginetestutils.h b/test/syncenginetestutils.h index b90ff480b..062b62cea 100644 --- a/test/syncenginetestutils.h +++ b/test/syncenginetestutils.h @@ -289,6 +289,7 @@ public: QString etag = generateEtag(); QByteArray fileId = generateFileId(); QByteArray checksums; + QByteArray extraDavProperties; qint64 size = 0; char contentChar = 'W'; @@ -360,6 +361,7 @@ public: xml.writeTextElement(ocUri, QStringLiteral("permissions"), fileInfo.isShared ? QStringLiteral("SRDNVCKW") : QStringLiteral("RDNVCKW")); xml.writeTextElement(ocUri, QStringLiteral("id"), fileInfo.fileId); xml.writeTextElement(ocUri, QStringLiteral("checksums"), fileInfo.checksums); + buffer.write(fileInfo.extraDavProperties); xml.writeEndElement(); // prop xml.writeTextElement(davUri, QStringLiteral("status"), "HTTP/1.1 200 OK"); xml.writeEndElement(); // propstat @@ -826,7 +828,7 @@ public: OCC::SyncEngine &syncEngine() const { return *_syncEngine; } FileModifier &localModifier() { return _localModifier; } - FileModifier &remoteModifier() { return _fakeQnam->currentRemoteState(); } + FileInfo &remoteModifier() { return _fakeQnam->currentRemoteState(); } FileInfo currentLocalState() { QDir rootDir{_tempDir.path()}; FileInfo rootTemplate; diff --git a/test/testsyncfilestatustracker.cpp b/test/testsyncfilestatustracker.cpp index bcb6ae205..741ae1dc6 100644 --- a/test/testsyncfilestatustracker.cpp +++ b/test/testsyncfilestatustracker.cpp @@ -411,11 +411,14 @@ private slots: void sharedStatus() { SyncFileStatus sharedUpToDateStatus(SyncFileStatus::StatusUpToDate); - sharedUpToDateStatus.setSharedWithMe(true); + sharedUpToDateStatus.setShared(true); FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; fakeFolder.remoteModifier().insert("S/s0"); fakeFolder.remoteModifier().appendByte("S/s1"); + fakeFolder.remoteModifier().insert("B/b3"); + fakeFolder.remoteModifier().find("B/b3")->extraDavProperties = "0"; + StatusPushSpy statusSpy(fakeFolder.syncEngine()); fakeFolder.scheduleSync(); @@ -435,6 +438,8 @@ private slots: QEXPECT_FAIL("", "We currently only know if a new file is shared on the second sync, after a PROPFIND.", Continue); QCOMPARE(statusSpy.statusOf("S/s0"), sharedUpToDateStatus); QCOMPARE(statusSpy.statusOf("S/s1"), sharedUpToDateStatus); + QCOMPARE(statusSpy.statusOf("B/b1").shared(), false); + QCOMPARE(statusSpy.statusOf("B/b3"), sharedUpToDateStatus); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); }