Detect missing server data during discovery #7112

This has two positive effects:
- We can put the error on the particular file that has missing data
- We can sync all other files

src/libsync/discovery.cpp

@@ -367,6 +367,27 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo(
     item->_directDownloadUrl = serverEntry.directDownloadUrl;
     item->_directDownloadCookies = serverEntry.directDownloadCookies;
 
+    // Check for missing server data
+    {
+        QStringList missingData;
+        if (serverEntry.size == -1)
+            missingData.append(tr("size"));
+        if (serverEntry.remotePerm.isNull())
+            missingData.append(tr("permissions"));
+        if (serverEntry.etag.isEmpty())
+            missingData.append(tr("etag"));
+        if (serverEntry.fileId.isEmpty())
+            missingData.append(tr("file id"));
+        if (!missingData.isEmpty()) {
+            item->_instruction = CSYNC_INSTRUCTION_ERROR;
+            item->_status = SyncFileItem::NormalError;
+            _childIgnored = true;
+            item->_errorString = tr("server reported no %1").arg(missingData.join(QLatin1String(", ")));
+            emit _discoveryData->itemDiscovered(item);
+            return;
+        }
+    }
+
     // The file is known in the db already
     if (dbEntry.isValid()) {
         if (serverEntry.isDirectory != dbEntry.isDirectory()) {

src/libsync/discoveryphase.cpp

@@ -332,16 +332,6 @@ void DiscoverySingleDirectoryJob::directoryListingIteratedSlot(QString file, con
         propertyMapToRemoteInfo(map, result);
         if (result.isDirectory)
             result.size = 0;
-        if (result.size == -1
-            || result.remotePerm.isNull()
-            || result.etag.isEmpty()
-            || result.fileId.isEmpty()) {
-            _error = tr("The server file discovery reply is missing data.");
-            qCWarning(lcDiscovery)
-                << "Missing properties:" << file << result.isDirectory << result.size
-                << result.modtime << result.remotePerm.toString()
-                << result.etag << result.fileId;
-        }
 
         if (_isExternalStorage && result.remotePerm.hasPermission(RemotePermissions::IsMounted)) {
             /* All the entries in a external storage have 'M' in their permission. However, for all

test/testremotediscovery.cpp

@@ -12,6 +12,16 @@
 
 using namespace OCC;
 
+SyncFileItemPtr findItem(const QSignalSpy &spy, const QString &path)
+{
+    for (const QList<QVariant> &args : spy) {
+        auto item = args[0].value<SyncFileItemPtr>();
+        if (item->destination() == path)
+            return item;
+    }
+    return SyncFileItemPtr(new SyncFileItem);
+}
+
 struct FakeBrokenXmlPropfindReply : FakePropfindReply {
     FakeBrokenXmlPropfindReply(FileInfo &remoteRootFileInfo, QNetworkAccessManager::Operation op,
                                const QNetworkRequest &request, QObject *parent)
@@ -38,7 +48,6 @@ struct MissingPermissionsPropfindReply : FakePropfindReply {
 enum ErrorKind : int {
     // Lower code are corresponding to HTML error code
     InvalidXML = 1000,
-    MissingPermissions,
     Timeout,
 };
 
@@ -64,7 +73,6 @@ private slots:
         // 200 should be an error since propfind should return 207
         QTest::newRow("200") << 200 << httpErrorMessage;
         QTest::newRow("InvalidXML") << +InvalidXML << "error while reading directory 'B' : Unknown error";
-        QTest::newRow("MissingPermissions") << +MissingPermissions << "error while reading directory 'B' : The server file discovery reply is missing data.";
         QTest::newRow("Timeout") << +Timeout << "error while reading directory 'B' : Operation canceled";
     }
 
@@ -94,8 +102,6 @@ private slots:
             if (req.attribute(QNetworkRequest::CustomVerbAttribute) == "PROPFIND" && req.url().path().endsWith("/B")) {
                 if (errorKind == InvalidXML) {
                     return new FakeBrokenXmlPropfindReply(fakeFolder.remoteModifier(), op, req, this);
-                } else if (errorKind == MissingPermissions) {
-                    return new MissingPermissionsPropfindReply(fakeFolder.remoteModifier(), op, req, this);
                 } else if (errorKind == Timeout) {
                     return new FakeHangingReply(op, req, this);
                 } else if (errorKind < 1000) {
@@ -125,6 +131,37 @@ private slots:
             QCOMPARE(fakeFolder.currentRemoteState().children["C"], fakeFolder.currentLocalState().children["C"]);
         }
     }
+
+    void testMissingData()
+    {
+        FakeFolder fakeFolder{ FileInfo() };
+        fakeFolder.remoteModifier().insert("good");
+        fakeFolder.remoteModifier().insert("noetag");
+        fakeFolder.remoteModifier().find("noetag")->etag.clear();
+        fakeFolder.remoteModifier().insert("nofileid");
+        fakeFolder.remoteModifier().find("nofileid")->fileId.clear();
+        fakeFolder.remoteModifier().mkdir("nopermissions");
+        fakeFolder.remoteModifier().insert("nopermissions/A");
+
+        fakeFolder.setServerOverride([&](QNetworkAccessManager::Operation op, const QNetworkRequest &req, QIODevice *)
+                -> QNetworkReply *{
+            if (req.attribute(QNetworkRequest::CustomVerbAttribute) == "PROPFIND" && req.url().path().endsWith("nopermissions"))
+                return new MissingPermissionsPropfindReply(fakeFolder.remoteModifier(), op, req, this);
+            return nullptr;
+        });
+
+        QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &)));
+        QVERIFY(!fakeFolder.syncOnce());
+
+        QCOMPARE(findItem(completeSpy, "good")->_instruction, CSYNC_INSTRUCTION_NEW);
+        QCOMPARE(findItem(completeSpy, "noetag")->_instruction, CSYNC_INSTRUCTION_ERROR);
+        QCOMPARE(findItem(completeSpy, "nofileid")->_instruction, CSYNC_INSTRUCTION_ERROR);
+        QCOMPARE(findItem(completeSpy, "nopermissions")->_instruction, CSYNC_INSTRUCTION_NEW);
+        QCOMPARE(findItem(completeSpy, "nopermissions/A")->_instruction, CSYNC_INSTRUCTION_ERROR);
+        QVERIFY(findItem(completeSpy, "noetag")->_errorString.contains("etag"));
+        QVERIFY(findItem(completeSpy, "nofileid")->_errorString.contains("file id"));
+        QVERIFY(findItem(completeSpy, "nopermissions/A")->_errorString.contains("permissions"));
+    }
 };
 
 QTEST_GUILESS_MAIN(TestRemoteDiscovery)