Dan Vrátil 62a3cb
commit 9c0dc6b3f0826d32eac310b2e7ecd858ca3df681
Dan Vrátil 62a3cb
Author: Dan Vrátil <dvratil@redhat.com>
Dan Vrátil 62a3cb
Date:   Mon Jun 29 22:45:11 2015 +0200
Dan Vrátil 62a3cb
Dan Vrátil 62a3cb
    Don't leak old external payload files
Dan Vrátil 62a3cb
    
Dan Vrátil 62a3cb
    Actually delete old payload files after we increase the payload revision or
Dan Vrátil 62a3cb
    switch from external to internal payload. This caused ~/.local/share/akonadi/file_db_data
Dan Vrátil 62a3cb
    to grow insanely for all users, leaving them with many duplicated files (just with
Dan Vrátil 62a3cb
    different revisions).
Dan Vrátil 62a3cb
    
Dan Vrátil 62a3cb
    It is recommended that users run akonadictl fsck to clean up the leaked payload
Dan Vrátil 62a3cb
    files.
Dan Vrátil 62a3cb
    
Dan Vrátil 62a3cb
    Note that there won't be any more releases of Akonadi 1.13 (and this has been
Dan Vrátil 62a3cb
    fixed in master already), so I strongly recommend distributions to pick this
Dan Vrátil 62a3cb
    patch into their packaging.
Dan Vrátil 62a3cb
    
Dan Vrátil 62a3cb
    BUG: 341884
Dan Vrátil 62a3cb
    CCBUG: 338402
Dan Vrátil 62a3cb
Dan Vrátil 62a3cb
diff --git a/server/src/storage/partstreamer.cpp b/server/src/storage/partstreamer.cpp
Dan Vrátil 62a3cb
index 2ec41fa..71bdca8 100644
Dan Vrátil 62a3cb
--- a/server/src/storage/partstreamer.cpp
Dan Vrátil 62a3cb
+++ b/server/src/storage/partstreamer.cpp
Dan Vrátil 62a3cb
@@ -290,6 +290,12 @@ bool PartStreamer::stream(const QByteArray &command, bool checkExists,
Dan Vrátil 62a3cb
         mDataChanged = true;
Dan Vrátil 62a3cb
     }
Dan Vrátil 62a3cb
 
Dan Vrátil 62a3cb
+    // If the part is external, remember it's current file name
Dan Vrátil 62a3cb
+    QString originalFile;
Dan Vrátil 62a3cb
+    if (part.isValid() && part.external()) {
Dan Vrátil 62a3cb
+        originalFile = PartHelper::resolveAbsolutePath(part.data());
Dan Vrátil 62a3cb
+    }
Dan Vrátil 62a3cb
+
Dan Vrátil 62a3cb
     part.setPartType(partType);
Dan Vrátil 62a3cb
     part.setVersion(partVersion);
Dan Vrátil 62a3cb
     part.setPimItemId(mItem.id());
Dan Vrátil 62a3cb
@@ -306,6 +312,14 @@ bool PartStreamer::stream(const QByteArray &command, bool checkExists,
Dan Vrátil 62a3cb
         *changed = mDataChanged;
Dan Vrátil 62a3cb
     }
Dan Vrátil 62a3cb
 
Dan Vrátil 62a3cb
+    if (!originalFile.isEmpty()) {
Dan Vrátil 62a3cb
+        // If the part was external but is not anymore, or if it's still external
Dan Vrátil 62a3cb
+        // but the filename has changed (revision update), remove the original file
Dan Vrátil 62a3cb
+        if (!part.external() || (part.external() && originalFile != PartHelper::resolveAbsolutePath(part.data()))) {
Dan Vrátil 62a3cb
+            PartHelper::removeFile(originalFile);
Dan Vrátil 62a3cb
+        }
Dan Vrátil 62a3cb
+    }
Dan Vrátil 62a3cb
+
Dan Vrátil 62a3cb
     return ok;
Dan Vrátil 62a3cb
 }
Dan Vrátil 62a3cb
 
Dan Vrátil 62a3cb
diff --git a/server/tests/unittest/partstreamertest.cpp b/server/tests/unittest/partstreamertest.cpp
Dan Vrátil 62a3cb
index 05e3a8a..669bbbc 100644
Dan Vrátil 62a3cb
--- a/server/tests/unittest/partstreamertest.cpp
Dan Vrátil 62a3cb
+++ b/server/tests/unittest/partstreamertest.cpp
Dan Vrátil 62a3cb
@@ -91,6 +91,7 @@ private Q_SLOTS:
Dan Vrátil 62a3cb
         QTest::addColumn<qint64>("expectedPartSize");
Dan Vrátil 62a3cb
         QTest::addColumn<bool>("expectedChanged");
Dan Vrátil 62a3cb
         QTest::addColumn<bool>("isExternal");
Dan Vrátil 62a3cb
+        QTest::addColumn<int>("version");
Dan Vrátil 62a3cb
         QTest::addColumn<PimItem>("pimItem");
Dan Vrátil 62a3cb
 
Dan Vrátil 62a3cb
         PimItem item;
Dan Vrátil 62a3cb
@@ -101,22 +102,22 @@ private Q_SLOTS:
Dan Vrátil 62a3cb
         QVERIFY(item.insert());
Dan Vrátil 62a3cb
 
Dan Vrátil 62a3cb
         // Order of these tests matters!
Dan Vrátil 62a3cb
-        QTest::newRow("item 1, internal") << QByteArray("PLD:DATA") << QByteArray("123") << 3ll << true << false << item;
Dan Vrátil 62a3cb
-        QTest::newRow("item 1, change to external") << QByteArray("PLD:DATA") << QByteArray("123456789") << 9ll << true << true << item;
Dan Vrátil 62a3cb
-        QTest::newRow("item 1, update external") << QByteArray("PLD:DATA") << QByteArray("987654321") << 9ll << true << true << item;
Dan Vrátil 62a3cb
-        QTest::newRow("item 1, external, no change") << QByteArray("PLD:DATA") << QByteArray("987654321") << 9ll << false << true << item;
Dan Vrátil 62a3cb
-        QTest::newRow("item 1, change to internal") << QByteArray("PLD:DATA") << QByteArray("1234") << 4ll << true << false << item;
Dan Vrátil 62a3cb
-        QTest::newRow("item 1, internal, no change") << QByteArray("PLD:DATA") << QByteArray("1234") << 4ll << false << false << item;
Dan Vrátil 62a3cb
+        QTest::newRow("item 1, internal") << QByteArray("PLD:DATA") << QByteArray("123") << 3ll << true << false << -1 << item;
Dan Vrátil 62a3cb
+        QTest::newRow("item 1, change to external") << QByteArray("PLD:DATA") << QByteArray("123456789") << 9ll << true << true << 0 << item;
Dan Vrátil 62a3cb
+        QTest::newRow("item 1, update external") << QByteArray("PLD:DATA") << QByteArray("987654321") << 9ll << true << true << 1 << item;
Dan Vrátil 62a3cb
+        QTest::newRow("item 1, external, no change") << QByteArray("PLD:DATA") << QByteArray("987654321") << 9ll << false << true << 2 << item;
Dan Vrátil 62a3cb
+        QTest::newRow("item 1, change to internal") << QByteArray("PLD:DATA") << QByteArray("1234") << 4ll << true << false << 2 << item;
Dan Vrátil 62a3cb
+        QTest::newRow("item 1, internal, no change") << QByteArray("PLD:DATA") << QByteArray("1234") << 4ll << false << false << 2 << item;
Dan Vrátil 62a3cb
     }
Dan Vrátil 62a3cb
 
Dan Vrátil 62a3cb
     void testStreamer()
Dan Vrátil 62a3cb
     {
Dan Vrátil 62a3cb
-        return;
Dan Vrátil 62a3cb
         QFETCH(QByteArray, expectedPartName);
Dan Vrátil 62a3cb
         QFETCH(QByteArray, expectedData);
Dan Vrátil 62a3cb
         QFETCH(qint64, expectedPartSize);
Dan Vrátil 62a3cb
         QFETCH(bool, expectedChanged);
Dan Vrátil 62a3cb
         QFETCH(bool, isExternal);
Dan Vrátil 62a3cb
+        QFETCH(int, version);
Dan Vrátil 62a3cb
         QFETCH(PimItem, pimItem);
Dan Vrátil 62a3cb
 
Dan Vrátil 62a3cb
         FakeConnection connection;
Dan Vrátil 62a3cb
@@ -160,17 +161,18 @@ private Q_SLOTS:
Dan Vrátil 62a3cb
 
Dan Vrátil 62a3cb
         PimItem item = PimItem::retrieveById(pimItem.id());
Dan Vrátil 62a3cb
         const QVector<Part> parts = item.parts();
Dan Vrátil 62a3cb
-        QVERIFY(parts.count() == 1);
Dan Vrátil 62a3cb
+        QCOMPARE(parts.count(), 1);
Dan Vrátil 62a3cb
         const Part part = parts[0];
Dan Vrátil 62a3cb
         QCOMPARE(part.datasize(), expectedPartSize);
Dan Vrátil 62a3cb
         QCOMPARE(part.external(), isExternal);
Dan Vrátil 62a3cb
+        qDebug() << part.version() << part.data();
Dan Vrátil 62a3cb
         const QByteArray data = part.data();
Dan Vrátil 62a3cb
         if (isExternal) {
Dan Vrátil 62a3cb
             QVERIFY(streamerSpy.count() == 1);
Dan Vrátil 62a3cb
             QVERIFY(streamerSpy.first().count() == 1);
Dan Vrátil 62a3cb
             const Response response = streamerSpy.first().first().value<Akonadi::Server::Response>();
Dan Vrátil 62a3cb
             const QByteArray str = response.asString();
Dan Vrátil 62a3cb
-            const QByteArray expectedResponse = "+ STREAM [FILE " + QByteArray::number(part.id()) + "_r" + QByteArray::number(part.version()) + "]";
Dan Vrátil 62a3cb
+            const QByteArray expectedResponse = "+ STREAM [FILE " + QByteArray::number(part.id()) + "_r" + QByteArray::number(version) + "]";
Dan Vrátil 62a3cb
             QCOMPARE(QString::fromUtf8(str), QString::fromUtf8(expectedResponse));
Dan Vrátil 62a3cb
 
Dan Vrátil 62a3cb
             QFile file(PartHelper::resolveAbsolutePath(data));
Dan Vrátil 62a3cb
@@ -182,7 +184,7 @@ private Q_SLOTS:
Dan Vrátil 62a3cb
             QCOMPARE(fileData, expectedData);
Dan Vrátil 62a3cb
 
Dan Vrátil 62a3cb
             // Make sure no previous versions are left behind in file_db_data
Dan Vrátil 62a3cb
-            for (int i = 0; i < part.version(); ++i) {
Dan Vrátil 62a3cb
+            for (int i = 0; i < version; ++i) {
Dan Vrátil 62a3cb
                 const QByteArray fileName = QByteArray::number(part.id()) + "_r" + QByteArray::number(part.version());
Dan Vrátil 62a3cb
                 const QString filePath = PartHelper::resolveAbsolutePath(fileName);
Dan Vrátil 62a3cb
                 QVERIFY(!QFile::exists(filePath));
Dan Vrátil 62a3cb
@@ -194,7 +196,7 @@ private Q_SLOTS:
Dan Vrátil 62a3cb
             QCOMPARE(data, expectedData);
Dan Vrátil 62a3cb
 
Dan Vrátil 62a3cb
             // Make sure nothing is left behind in file_db_data
Dan Vrátil 62a3cb
-            for (int i = 0; i <= part.version(); ++i) {
Dan Vrátil 62a3cb
+            for (int i = 0; i <= version; ++i) {
Dan Vrátil 62a3cb
                 const QByteArray fileName = QByteArray::number(part.id()) + "_r" + QByteArray::number(part.version());
Dan Vrátil 62a3cb
                 const QString filePath = PartHelper::resolveAbsolutePath(fileName);
Dan Vrátil 62a3cb
                 QVERIFY(!QFile::exists(filePath));