From 5f3682a47d7cf6dbc7a3cd0b048bec421d82bb92 Mon Sep 17 00:00:00 2001
From: Christian Kamm <mail@ckamm.de>
Date: Mon, 23 Sep 2019 15:52:57 +0200
Subject: [PATCH] Propagate dir: Never write the etag on remote mkdir #7481

It must always only be written once all children are successfully
propagated.
---
 src/libsync/propagateremotemkdir.cpp | 13 +++++++------
 test/testsyncengine.cpp              | 17 +++++++++++++++++
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/src/libsync/propagateremotemkdir.cpp b/src/libsync/propagateremotemkdir.cpp
index 9a266fadf2..64b3d48265 100644
--- a/src/libsync/propagateremotemkdir.cpp
+++ b/src/libsync/propagateremotemkdir.cpp
@@ -114,8 +114,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();
@@ -128,9 +127,6 @@ void PropagateRemoteMkdir::slotMkcolJobFinished()
 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();
     }
@@ -146,8 +142,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 96cd665eb3..d60f0e3eaf 100644
--- 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 &)));