Propagate dir: Never write the etag on remote mkdir #7481

It must always only be written once all children are successfully
propagated.

diff --git a/src/libsync/propagateremotemkdir.cpp b/src/libsync/propagateremotemkdir.cpp
index d1a8c1cf4185443d5abcfae6595e5acb1ab05cc6..945d7d393120480aad962bf83a2340bf36bb4c72 100644 (file)
--- a/src/libsync/propagateremotemkdir.cpp
+++ b/src/libsync/propagateremotemkdir.cpp
@@ -223,8 +223,7 @@ void PropagateRemoteMkdir::slotMkcolJobFinished()
         // while files are still uploading
         propagator()->_activeJobList.append(this);
         auto propfindJob = new PropfindJob(_job->account(), _job->path(), this);
-        propfindJob->setProperties(QList<QByteArray>() << "getetag"
-                                                       << "http://owncloud.org/ns:id");
+        propfindJob->setProperties(QList<QByteArray>() << "http://owncloud.org/ns:id");
         QObject::connect(propfindJob, &PropfindJob::result, this, &PropagateRemoteMkdir::propfindResult);
         QObject::connect(propfindJob, &PropfindJob::finishedWithError, this, &PropagateRemoteMkdir::propfindError);
         propfindJob->start();
@@ -260,9 +259,6 @@ void PropagateRemoteMkdir::slotEncryptFolderFinished()
 void PropagateRemoteMkdir::propfindResult(const QVariantMap &result)
 {
     propagator()->_activeJobList.removeOne(this);
-    if (result.contains("getetag")) {
-        _item->_etag = result["getetag"].toByteArray();
-    }
     if (result.contains("id")) {
         _item->_fileId = result["id"].toByteArray();
     }
@@ -278,8 +274,13 @@ void PropagateRemoteMkdir::propfindError()
 
 void PropagateRemoteMkdir::success()
 {
+    // Never save the etag on first mkdir.
+    // Only fully propagated directories should have the etag set.
+    auto itemCopy = *_item;
+    itemCopy._etag.clear();
+
     // save the file id already so we can detect rename or remove
-    if (!propagator()->updateMetadata(*_item)) {
+    if (!propagator()->updateMetadata(itemCopy)) {
         done(SyncFileItem::FatalError, tr("Error writing metadata to the database"));
         return;
     }

diff --git a/test/testsyncengine.cpp b/test/testsyncengine.cpp
index eadf56ba2d6c0e23835c16a909c0e7d69ae1fbab..4243897a5d1ef9aa28f12c0d10b3cdaebd45b0a3 100644 (file)
--- a/test/testsyncengine.cpp
+++ b/test/testsyncengine.cpp
@@ -236,6 +236,23 @@ private slots:
         QCOMPARE(finishedSpy.first().first().toBool(), false);
     }
 
+    /** Verify that an incompletely propagated directory doesn't have the server's
+     * etag stored in the database yet. */
+    void testDirEtagAfterIncompleteSync() {
+        FakeFolder fakeFolder{FileInfo{}};
+        QSignalSpy finishedSpy(&fakeFolder.syncEngine(), SIGNAL(finished(bool)));
+        fakeFolder.serverErrorPaths().append("NewFolder/foo");
+        fakeFolder.remoteModifier().mkdir("NewFolder");
+        fakeFolder.remoteModifier().insert("NewFolder/foo");
+        QVERIFY(!fakeFolder.syncOnce());
+
+        SyncJournalFileRecord rec;
+        fakeFolder.syncJournal().getFileRecord(QByteArrayLiteral("NewFolder"), &rec);
+        QVERIFY(rec.isValid());
+        QCOMPARE(rec._etag, QByteArrayLiteral("_invalid_"));
+        QVERIFY(!rec._fileId.isEmpty());
+    }
+
     void testDirDownloadWithError() {
         FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()};
         QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &)));