QNetworkDiskCache: use QSaveFile

QTemporaryFile is not designed to promote temporary files
to non-temporary files. So, it, quite importantly, does not
care if the content of the files are flushed to disk before
renaming it into its 'final' destination. This is what
QSaveFile is for.

This was much more time-consuming than intended since I had to debug
this quirk about calling size().

Fixes: QTBUG-111397
Change-Id: I15b300f6a5748ad3a0be983545ea621269a12ecd
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
(cherry picked from commit e5f295c8a458dcd336f7cf3768ca62aded69e659)
Reviewed-by: Marc Mutz <marc.mutz@qt.io>

3 files changed, 24 insertions, 46 deletions

diff --git a/src/network/access/qnetworkdiskcache.cpp b/src/network/access/qnetworkdiskcache.cpp
index 0f29156318..e604a6b210 100644
--- a/src/network/access/qnetworkdiskcache.cpp
+++ b/src/network/access/qnetworkdiskcache.cpp
@@ -56,7 +56,6 @@
 #include <memory>
 
 #define CACHE_POSTFIX QLatin1String(".d")
-#define PREPARED_SLASH QLatin1String("prepared/")
 #define CACHE_VERSION 8
 #define DATA_DIR QLatin1String("data")
 
@@ -206,13 +205,13 @@ QIODevice *QNetworkDiskCache::prepare(const QNetworkCacheMetaData &metaData)
         cacheItem->data.open(QBuffer::ReadWrite);
         device = &(cacheItem->data);
     } else {
-        QString templateName = d->tmpCacheFileName();
+        QString fileName = d->cacheFileName(cacheItem->metaData.url());
         QT_TRY {
-            cacheItem->file = new QTemporaryFile(templateName, &cacheItem->data);
+            cacheItem->file = new QSaveFile(fileName, &cacheItem->data);
         } QT_CATCH(...) {
             cacheItem->file = nullptr;
         }
-        if (!cacheItem->file || !cacheItem->file->open()) {
+        if (!cacheItem->file || !cacheItem->file->open(QFileDevice::WriteOnly)) {
             qWarning("QNetworkDiskCache::prepare() unable to open temporary file");
             cacheItem.reset();
             return nullptr;
@@ -252,7 +251,6 @@ void QNetworkDiskCache::insert(QIODevice *device)
 void QNetworkDiskCachePrivate::prepareLayout()
 {
     QDir helper;
-    helper.mkpath(cacheDirectory + PREPARED_SLASH);
 
     //Create directory and subdirectories 0-F
     helper.mkpath(dataDirectory);
@@ -281,9 +279,8 @@ void QNetworkDiskCachePrivate::storeItem(QCacheItem *cacheItem)
 
     currentCacheSize = q->expire();
     if (!cacheItem->file) {
-        QString templateName = tmpCacheFileName();
-        cacheItem->file = new QTemporaryFile(templateName, &cacheItem->data);
-        if (cacheItem->file->open()) {
+        cacheItem->file = new QSaveFile(fileName, &cacheItem->data);
+        if (cacheItem->file->open(QFileDevice::WriteOnly)) {
             cacheItem->writeHeader(cacheItem->file);
             cacheItem->writeCompressedData(cacheItem->file);
         }
@@ -291,13 +288,15 @@ void QNetworkDiskCachePrivate::storeItem(QCacheItem *cacheItem)
 
     if (cacheItem->file
         && cacheItem->file->isOpen()
-        && cacheItem->file->error() == QFile::NoError) {
-        cacheItem->file->setAutoRemove(false);
-        // ### use atomic rename rather then remove & rename
-        if (cacheItem->file->rename(fileName))
-            currentCacheSize += cacheItem->file->size();
-        else
-            cacheItem->file->setAutoRemove(true);
+        && cacheItem->file->error() == QFileDevice::NoError) {
+        // We have to call size() here instead of inside the if-body because
+        // commit() invalidates the file-engine, and size() will create a new
+        // one, pointing at an empty filename.
+        qint64 size = cacheItem->file->size();
+        if (cacheItem->file->commit())
+            currentCacheSize += size;
+        // Delete and unset the QSaveFile, it's invalid now.
+        delete std::exchange(cacheItem->file, nullptr);
     }
     if (cacheItem->metaData.url() == lastItem.metaData.url())
         lastItem.reset();
@@ -542,16 +541,6 @@ qint64 QNetworkDiskCache::expire()
         QString name = i.value();
         QFile file(name);
 
-        if (name.contains(PREPARED_SLASH)) {
-            for (QCacheItem *item : qAsConst(d->inserting)) {
-                if (item && item->file && item->file->fileName() == name) {
-                    delete item->file;
-                    item->file = nullptr;
-                    break;
-                }
-            }
-        }
-
         qint64 size = file.size();
         file.remove();
         totalSize -= size;
@@ -604,12 +593,6 @@ QString QNetworkDiskCachePrivate::uniqueFileName(const QUrl &url)
     return pathFragment;
 }
 
-QString QNetworkDiskCachePrivate::tmpCacheFileName() const
-{
-    //The subdirectory is presumed to be already read for use.
-    return cacheDirectory + PREPARED_SLASH + QLatin1String("XXXXXX") + CACHE_POSTFIX;
-}
-
 /*!
     Generates fully qualified path of cached resource from a URL.
  */
@@ -660,7 +643,7 @@ enum
     CurrentCacheVersion = CACHE_VERSION
 };
 
-void QCacheItem::writeHeader(QFile *device) const
+void QCacheItem::writeHeader(QFileDevice *device) const
 {
     QDataStream out(device);
 
@@ -672,7 +655,7 @@ void QCacheItem::writeHeader(QFile *device) const
     out << compressed;
 }
 
-void QCacheItem::writeCompressedData(QFile *device) const
+void QCacheItem::writeCompressedData(QFileDevice *device) const
 {
     QDataStream out(device);
 
@@ -683,7 +666,7 @@ void QCacheItem::writeCompressedData(QFile *device) const
     Returns \c false if the file is a cache file,
     but is an older version and should be removed otherwise true.
  */
-bool QCacheItem::read(QFile *device, bool readData)
+bool QCacheItem::read(QFileDevice *device, bool readData)
 {
     reset();
 
diff --git a/src/network/access/qnetworkdiskcache_p.h b/src/network/access/qnetworkdiskcache_p.h
index 34be28f9b4..6b1af7f4fd 100644
--- a/src/network/access/qnetworkdiskcache_p.h
+++ b/src/network/access/qnetworkdiskcache_p.h
@@ -56,20 +56,16 @@
 
 #include <qbuffer.h>
 #include <qhash.h>
-#include <qtemporaryfile.h>
+#include <qsavefile.h>
 
 QT_REQUIRE_CONFIG(networkdiskcache);
 
 QT_BEGIN_NAMESPACE
 
-class QFile;
-
 class QCacheItem
 {
 public:
-    QCacheItem() : file(nullptr)
-    {
-    }
+    QCacheItem() = default;
     ~QCacheItem()
     {
         reset();
@@ -77,7 +73,7 @@ public:
 
     QNetworkCacheMetaData metaData;
     QBuffer data;
-    QTemporaryFile *file;
+    QSaveFile *file = nullptr;
     inline qint64 size() const
         { return file ? file->size() : data.size(); }
 
@@ -87,9 +83,9 @@ public:
         delete file;
         file = nullptr;
     }
-    void writeHeader(QFile *device) const;
-    void writeCompressedData(QFile *device) const;
-    bool read(QFile *device, bool readData);
+    void writeHeader(QFileDevice *device) const;
+    void writeCompressedData(QFileDevice *device) const;
+    bool read(QFileDevice *device, bool readData);
 
     bool canCompress() const;
 };
@@ -105,7 +101,6 @@ public:
 
     static QString uniqueFileName(const QUrl &url);
     QString cacheFileName(const QUrl &url) const;
-    QString tmpCacheFileName() const;
     bool removeFile(const QString &file);
     void storeItem(QCacheItem *item);
     void prepareLayout();
diff --git a/tests/auto/network/access/qnetworkdiskcache/tst_qnetworkdiskcache.cpp b/tests/auto/network/access/qnetworkdiskcache/tst_qnetworkdiskcache.cpp
index 5dc2936d25..006a529c6d 100644
--- a/tests/auto/network/access/qnetworkdiskcache/tst_qnetworkdiskcache.cpp
+++ b/tests/auto/network/access/qnetworkdiskcache/tst_qnetworkdiskcache.cpp
@@ -37,7 +37,7 @@
 #define EXAMPLE_URL "http://user:pass@localhost:4/#foo"
 #define EXAMPLE_URL2 "http://user:pass@localhost:4/bar"
 //cached objects are organized into these many subdirs
-#define NUM_SUBDIRECTORIES 16
+#define NUM_SUBDIRECTORIES 15
 
 class tst_QNetworkDiskCache : public QObject
 {