Sharing: fix overlay icons directly after sharing a file

src/gui/folder.cpp

@@ -841,6 +841,11 @@ void Folder::slotTransmissionProgress(const ProgressInfo &pi)
 // a item is completed: count the errors and forward to the ProgressDispatcher
 void Folder::slotItemCompleted(const SyncFileItemPtr &item)
 {
+    if (item->_instruction == CSYNC_INSTRUCTION_NONE || item->_instruction == CSYNC_INSTRUCTION_UPDATE_METADATA) {
+        // We only care about the updates that deserve to be shown in the UI
+        return;
+    }
+
     // add new directories or remove gone away dirs to the watcher
     if (item->isDirectory() && item->_instruction == CSYNC_INSTRUCTION_NEW) {
         FolderMan::instance()->addMonitorPath(alias(), path() + item->_file);

src/gui/sharemanager.cpp

@@ -15,25 +15,41 @@
 #include "sharemanager.h"
 #include "ocssharejob.h"
 #include "account.h"
+#include "folderman.h"
+#include "accountstate.h"
 
 #include <QUrl>
 #include <QJsonDocument>
 #include <QJsonObject>
 #include <QJsonArray>
 
-namespace {
-struct CreateShare
-{
-    QString path;
-    OCC::Share::ShareType shareType;
-    QString shareWith;
-    OCC::Share::Permissions permissions;
-};
-} // anonymous namespace
-Q_DECLARE_METATYPE(CreateShare)
-
 namespace OCC {
 
+/**
+ * When a share is modified, we need to tell the folders so they can adjust overlay icons
+ */
+static void updateFolder(const AccountPtr &account, const QString &path)
+{
+    foreach (Folder *f, FolderMan::instance()->map()) {
+        if (f->accountState()->account() != account)
+            continue;
+        auto folderPath = f->remotePath();
+        if (path.startsWith(folderPath) && (path == folderPath || folderPath.endsWith('/') || path[folderPath.size()] == '/')) {
+            // Workaround the fact that the server does not invalidate the etags of parent directories
+            // when something is shared.
+            auto relative = path.midRef(folderPath.size());
+            if (relative.startsWith('/'))
+                relative = relative.mid(1);
+            f->journalDb()->avoidReadFromDbOnNextSync(relative.toString());
+
+            // Schedule a sync so it can update the remote permission flag and let the socket API
+            // know about the shared icon.
+            f->scheduleThisFolderSoon();
+        }
+    }
+}
+
+
 Share::Share(AccountPtr account,
     const QString &id,
     const QString &path,
@@ -54,6 +70,11 @@ AccountPtr Share::account() const
     return _account;
 }
 
+QString Share::path() const
+{
+    return _path;
+}
+
 QString Share::getId() const
 {
     return _id;
@@ -99,6 +120,8 @@ void Share::deleteShare()
 void Share::slotDeleted()
 {
     emit shareDeleted();
+
+    updateFolder(_account, _path);
 }
 
 void Share::slotOcsError(int statusCode, const QString &message)
@@ -258,6 +281,8 @@ void ShareManager::slotLinkShareCreated(const QJsonDocument &reply)
     QSharedPointer<LinkShare> share(parseLinkShare(data));
 
     emit linkShareCreated(share);
+
+    updateFolder(_account, share->path());
 }
 
 
@@ -267,51 +292,34 @@ void ShareManager::createShare(const QString &path,
     const Share::Permissions permissions)
 {
     auto job = new OcsShareJob(_account);
-
-    // Store values that we need for creating this share later.
-    CreateShare continuation;
-    continuation.path = path;
-    continuation.shareType = shareType;
-    continuation.shareWith = shareWith;
-    continuation.permissions = permissions;
-    _jobContinuation[job] = QVariant::fromValue(continuation);
-
-    connect(job, &OcsShareJob::shareJobFinished, this, &ShareManager::slotCreateShare);
     connect(job, &OcsJob::ocsError, this, &ShareManager::slotOcsError);
+    connect(job, &OcsShareJob::shareJobFinished, this,
+        [=](const QJsonDocument &reply) {
+            // Find existing share permissions (if this was shared with us)
+            Share::Permissions existingPermissions = SharePermissionDefault;
+            foreach (const QJsonValue &element, reply.object()["ocs"].toObject()["data"].toArray()) {
+                auto map = element.toObject();
+                if (map["file_target"] == path)
+                    existingPermissions = Share::Permissions(map["permissions"].toInt());
+            }
+
+            // Limit the permissions we request for a share to the ones the item
+            // was shared with initially.
+            auto perm = permissions;
+            if (permissions == SharePermissionDefault) {
+                perm = existingPermissions;
+            } else if (existingPermissions != SharePermissionDefault) {
+                perm &= existingPermissions;
+            }
+
+            OcsShareJob *job = new OcsShareJob(_account);
+            connect(job, &OcsShareJob::shareJobFinished, this, &ShareManager::slotShareCreated);
+            connect(job, &OcsJob::ocsError, this, &ShareManager::slotOcsError);
+            job->createShare(path, shareType, shareWith, permissions);
+        });
     job->getSharedWithMe();
 }
 
-void ShareManager::slotCreateShare(const QJsonDocument &reply)
-{
-    if (!_jobContinuation.contains(sender()))
-        return;
-
-    CreateShare cont = _jobContinuation[sender()].value<CreateShare>();
-    if (cont.path.isEmpty())
-        return;
-    _jobContinuation.remove(sender());
-
-    // Find existing share permissions (if this was shared with us)
-    Share::Permissions existingPermissions = SharePermissionDefault;
-    foreach (const QJsonValue &element, reply.object()["ocs"].toObject()["data"].toArray()) {
-        auto map = element.toObject();
-        if (map["file_target"] == cont.path)
-            existingPermissions = Share::Permissions(map["permissions"].toInt());
-    }
-
-    // Limit the permissions we request for a share to the ones the item
-    // was shared with initially.
-    if (cont.permissions == SharePermissionDefault) {
-        cont.permissions = existingPermissions;
-    } else if (existingPermissions != SharePermissionDefault) {
-        cont.permissions &= existingPermissions;
-    }
-
-    OcsShareJob *job = new OcsShareJob(_account);
-    connect(job, &OcsShareJob::shareJobFinished, this, &ShareManager::slotShareCreated);
-    connect(job, &OcsJob::ocsError, this, &ShareManager::slotOcsError);
-    job->createShare(cont.path, cont.shareType, cont.shareWith, cont.permissions);
-}
 
 void ShareManager::slotShareCreated(const QJsonDocument &reply)
 {
@@ -320,6 +328,8 @@ void ShareManager::slotShareCreated(const QJsonDocument &reply)
     QSharedPointer<Share> share(parseShare(data));
 
     emit shareCreated(share);
+
+    updateFolder(_account, share->path());
 }
 
 void ShareManager::fetchShares(const QString &path)

src/gui/sharemanager.h

@@ -64,6 +64,8 @@ class Share : public QObject
      */
     AccountPtr account() const;
 
+    QString path() const;
+
     /*
      * Get the id
      */
@@ -293,13 +295,10 @@ private slots:
     void slotLinkShareCreated(const QJsonDocument &reply);
     void slotShareCreated(const QJsonDocument &reply);
     void slotOcsError(int statusCode, const QString &message);
-    void slotCreateShare(const QJsonDocument &reply);
-
 private:
     QSharedPointer<LinkShare> parseLinkShare(const QJsonObject &data);
     QSharedPointer<Share> parseShare(const QJsonObject &data);
 
-    QMap<QObject *, QVariant> _jobContinuation;
     AccountPtr _account;
 };
 }

src/libsync/syncengine.cpp

@@ -571,7 +571,6 @@ int SyncEngine::treewalkFile(csync_file_stat_t *file, csync_file_stat_t *other,
         dir = SyncFileItem::None;
         // For directories, metadata-only updates will be done after all their files are propagated.
         if (!isDirectory) {
-            emit syncItemDiscovered(*item);
 
             // Update the database now already:  New remote fileid or Etag or RemotePerm
             // Or for files that were detected as "resolved conflict".
@@ -608,6 +607,9 @@ int SyncEngine::treewalkFile(csync_file_stat_t *file, csync_file_stat_t *other,
                 }
 
                 _journal->setFileRecordMetadata(item->toSyncJournalFileRecordWithInode(filePath));
+
+                // This might have changed the shared flag, so we must notify SyncFileStatusTracker for example
+                emit itemCompleted(item);
             } else {
                 // The local tree is walked first and doesn't have all the info from the server.
                 // Update only outdated data from the disk.
@@ -683,8 +685,6 @@ int SyncEngine::treewalkFile(csync_file_stat_t *file, csync_file_stat_t *other,
     }
 
     _syncItemMap.insert(key, item);
-
-    emit syncItemDiscovered(*item);
     return re;
 }
 

src/libsync/syncengine.h

@@ -105,8 +105,6 @@ class OWNCLOUDSYNC_EXPORT SyncEngine : public QObject
     // During update, before reconcile
     void rootEtag(QString);
 
-    // before actual syncing (after update+reconcile) for each item
-    void syncItemDiscovered(const SyncFileItem &);
     // after the above signals. with the items that actually need propagating
     void aboutToPropagate(SyncFileItemVector &);
 

test/testsyncengine.cpp

@@ -16,7 +16,7 @@ bool itemDidComplete(const QSignalSpy &spy, const QString &path)
     for(const QList<QVariant> &args : spy) {
         auto item = args[0].value<SyncFileItemPtr>();
         if (item->destination() == path)
-            return true;
+            return item->_instruction != CSYNC_INSTRUCTION_NONE && item->_instruction != CSYNC_INSTRUCTION_UPDATE_METADATA;
     }
     return false;
 }

test/testsyncfilestatustracker.cpp

@@ -436,6 +436,8 @@ private slots:
         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>";
+        fakeFolder.remoteModifier().find("A/a1")->isShared = true; // becomes shared
+        fakeFolder.remoteModifier().find("A", true); // change the etags of the parent
 
         StatusPushSpy statusSpy(fakeFolder.syncEngine());
 
@@ -458,6 +460,7 @@ private slots:
         QCOMPARE(statusSpy.statusOf("S/s1"), sharedUpToDateStatus);
         QCOMPARE(statusSpy.statusOf("B/b1").shared(), false);
         QCOMPARE(statusSpy.statusOf("B/b3"), sharedUpToDateStatus);
+        QCOMPARE(statusSpy.statusOf("A/a1"), sharedUpToDateStatus);
 
         QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
     }