Просмотр исходного кода

libsync: Fix a case in which canceling the sync would result in some folder never being sync'ed

The problem occurs because of the sorting of items when we have things like

bigfolder
bigfolder/bigsubfolder
bigfolder-2

Then, because dashes come before slash in ascii, the bigfolder-2 would come before its content
and the propagator would thinkg bigfolder is empty and save the etag before it is processed

Should fix issue #2832
Olivier Goffart 11 лет назад
Родитель
Сommit
b9161aa06c
4 измененных файлов с 97 добавлено и 1 удалено
  1. 2 0
      src/libsync/owncloudpropagator.cpp
  2. 27 1
      src/libsync/syncfileitem.h
  3. 1 0
      test/CMakeLists.txt
  4. 67 0
      test/testsyncfileitem.h

+ 2 - 0
src/libsync/owncloudpropagator.cpp

@@ -252,6 +252,8 @@ PropagateItemJob* OwncloudPropagator::createJob(const SyncFileItem& item) {
 
 void OwncloudPropagator::start(const SyncFileItemVector& items)
 {
+    Q_ASSERT(std::is_sorted(items.begin(), items.end()));
+
     /* This builds all the job needed for the propagation.
      * Each directories is a PropagateDirectory job, which contains the files in it.
      * In order to do that we loop over the items. (which are sorted by destination)

+ 27 - 1
src/libsync/syncfileitem.h

@@ -64,7 +64,33 @@ public:
 
     friend bool operator<(const SyncFileItem& item1, const SyncFileItem& item2) {
         // Sort by destination
-        return item1.destination() < item2.destination();
+        auto d1 = item1.destination();
+        auto d2 = item2.destination();
+
+        // But this we need to order it so the slash come first. It should be this order:
+        //  "foo", "foo/bar", "foo-bar"
+        // This is important since we assume that the contents of a folder directly follows
+        // its contents
+
+        auto data1 = d1.constData();
+        auto data2 = d2.constData();
+
+        // Find the lenght of the largest prefix
+        int prefixL = 0;
+        auto minSize = std::min(d1.size(), d2.size());
+        while (prefixL < minSize && data1[prefixL] == data2[prefixL]) { prefixL++; }
+
+        if (prefixL == d1.size())
+            return true;
+        if (prefixL == d2.size())
+            return false;
+
+        if (data1[prefixL] == '/')
+            return true;
+        if (data2[prefixL] == '/')
+            return false;
+
+        return data1[prefixL] < data2[prefixL];
     }
 
     QString destination() const {

+ 1 - 0
test/CMakeLists.txt

@@ -29,6 +29,7 @@ owncloud_add_test(CSyncSqlite "")
 owncloud_add_test(NetrcParser ../src/cmd/netrcparser.cpp)
 owncloud_add_test(OwnSql "")
 owncloud_add_test(SyncJournalDB "")
+owncloud_add_test(SyncFileItem "")
 
 
 

+ 67 - 0
test/testsyncfileitem.h

@@ -0,0 +1,67 @@
+/*
+ *    This software is in the public domain, furnished "as is", without technical
+ *       support, and with no warranty, express or implied, as to its usefulness for
+ *          any purpose.
+ *          */
+
+#ifndef MIRALL_TESTSYNCFILEITEM_H
+#define MIRALL_TESTSYNCFILEITEM_H
+
+#include <QtTest>
+
+#include "syncfileitem.h"
+
+using namespace OCC;
+
+class TestSyncFileItem : public QObject
+{
+    Q_OBJECT
+
+private slots:
+    void initTestCase() {
+    }
+
+    void cleanupTestCase() {
+    }
+
+    void testComparator_data() {
+        QTest::addColumn<SyncFileItem>("a");
+        QTest::addColumn<SyncFileItem>("b");
+        QTest::addColumn<SyncFileItem>("c");
+
+        auto i = [](const QString &file) {
+            SyncFileItem itm;
+            itm._file = file;
+            return itm;
+        };
+
+        QTest::newRow("a1") << i("client") << i("client/build") << i("client-build") ;
+        QTest::newRow("a2") << i("test/t1") << i("test/t2") << i("test/t3") ;
+        QTest::newRow("a3") << i("ABCD") << i("abcd") << i("zzzz");
+
+        SyncFileItem movedItem1;
+        movedItem1._file = "folder/source/file.f";
+        movedItem1._renameTarget = "folder/destination/file.f";
+        movedItem1._instruction = CSYNC_INSTRUCTION_RENAME;
+
+        QTest::newRow("move1") << i("folder/destination") << movedItem1 << i("folder/destination-2");
+        QTest::newRow("move2") << i("folder/destination/1") << movedItem1 << i("folder/source");
+        QTest::newRow("move3") << i("abc") << movedItem1 << i("ijk");
+    }
+
+    void testComparator() {
+        QFETCH( SyncFileItem , a );
+        QFETCH( SyncFileItem , b );
+        QFETCH( SyncFileItem , c );
+
+        QVERIFY(a < b);
+        QVERIFY(b < c);
+        QVERIFY(a < c);
+
+        QVERIFY(!(b < a));
+        QVERIFY(!(c < b));
+        QVERIFY(!(c < a));
+    }
+};
+
+#endif