Revert "Revert "Discovery: consider also the "shared by me" as shared""
authorOlivier Goffart <ogoffart@woboq.com>
Fri, 21 Jul 2017 09:28:15 +0000 (11:28 +0200)
committerOlivier Goffart <olivier@woboq.com>
Tue, 25 Jul 2017 10:11:33 +0000 (12:11 +0200)
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
src/libsync/syncfilestatus.cpp
src/libsync/syncfilestatus.h
src/libsync/syncfilestatustracker.cpp
test/syncenginetestutils.h
test/testsyncfilestatustracker.cpp

index 16b98faeb2c2da9e5d5d8ceb438e4920c25d6870..dcad77390c8cf847fb51be67cedcfc5243e28d69 100644 (file)
@@ -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<QString, QString>
             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;
 }
 
index 12a9977860aa51cf09fc970c9cbc002b2bdd8b9e..8d1a5933fe6c8a327ce23ac9324bb7be3708bb82 100644 (file)
 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");
     }
 
index 27006a57227fc4900c88a18f0489dfd2fa062dde..e4743f7a3f5293b9840e9223110c3f0a4d7c716b 100644 (file)
@@ -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)
index e643099b29ca2b11f41b0e77d3286dca74f241d2..9d4fa6e37de29bc2cc24289e92671aa47caef4e9 100644 (file)
@@ -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;
 }
index b90ff480b1d3ba11620cac028ea52b36a2dd5fd4..062b62cea74cb40b6fa004167b465f99c91a7df7 100644 (file)
@@ -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;
index bcb6ae2058e0e0e11aef3f49fe9466bd0e1bdfbd..741ae1dc6d5bebe45fd58f9c83c31934aa592313 100644 (file)
@@ -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 = "<oc:share-types><oc:share-type>0</oc:share-type></oc:share-types>";
+
         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());
     }