QNetworkDiskCache: Fix tracking of size during storeItem()

If the file already existed we simply removed the old one without
adjusting the size. So use the removeFile() function which takes care of
that.

Additionally, if the current size was non-null we previously increased
the size (presumably meant to be temporarily but wasn't) and called
expire() which would either:
1. not do anything and return currentCacheSize, if it was not greater
than the max size. This would mean that the size of the file would be
counted twice.
or,
2. discard currentCacheSize, measure the size of the items, and then
remove some items if the total size surpassed the max cache size

Neither of those branches need us to (temporarily) increase
currentCacheSize. It also doesn't attain the (presumed) goal of trying
to keep below the max cache size after having added the new item.

Fixes: QTBUG-95009
Change-Id: I2b5b13ff473a7aa8169cf2aecfea783c97f2d09a
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
(cherry picked from commit d9f80502f6450f0bc8e6d7ca13e1c912ad485599)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

1 files changed, 38 insertions, 10 deletions

diff --git a/tests/auto/network/access/qnetworkdiskcache/tst_qnetworkdiskcache.cpp b/tests/auto/network/access/qnetworkdiskcache/tst_qnetworkdiskcache.cpp
index f15d4dc7ac..5dc2936d25 100644
--- a/tests/auto/network/access/qnetworkdiskcache/tst_qnetworkdiskcache.cpp
+++ b/tests/auto/network/access/qnetworkdiskcache/tst_qnetworkdiskcache.cpp
@@ -35,6 +35,7 @@
 #include <algorithm>
 
 #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
 
@@ -141,7 +142,7 @@ class SubQNetworkDiskCache : public QNetworkDiskCache
 public:
     ~SubQNetworkDiskCache()
     {
-        if (!cacheDirectory().isEmpty())
+        if (!cacheDirectory().isEmpty() && clearOnDestruction)
             clear();
     }
 
@@ -170,6 +171,11 @@ public:
         d->write("Hello World!");
         insert(d);
     }
+
+    void setClearCacheOnDestruction(bool value) { clearOnDestruction = value; }
+
+private:
+    bool clearOnDestruction = true;
 };
 
 tst_QNetworkDiskCache::tst_QNetworkDiskCache()
@@ -241,17 +247,39 @@ void tst_QNetworkDiskCache::prepare()
 // public qint64 cacheSize() const
 void tst_QNetworkDiskCache::cacheSize()
 {
-    SubQNetworkDiskCache cache;
-    cache.setCacheDirectory(tempDir.path());
-    QCOMPARE(cache.cacheSize(), qint64(0));
+    qint64 cacheSize = 0;
+    {
+        SubQNetworkDiskCache cache;
+        cache.setCacheDirectory(tempDir.path());
+        QCOMPARE(cache.cacheSize(), qint64(0));
 
-    QUrl url(EXAMPLE_URL);
-    QNetworkCacheMetaData metaData;
-    metaData.setUrl(url);
-    QIODevice *d = cache.prepare(metaData);
-    cache.insert(d);
-    QVERIFY(cache.cacheSize() > qint64(0));
+        {
+            QUrl url(EXAMPLE_URL);
+            QNetworkCacheMetaData metaData;
+            metaData.setUrl(url);
+            QIODevice *d = cache.prepare(metaData);
+            cache.insert(d);
+            cacheSize = cache.cacheSize();
+            QVERIFY(cacheSize > qint64(0));
+        }
+        // Add a second item, some difference in behavior when the cache is not empty
+        {
+            QUrl url(EXAMPLE_URL2);
+            QNetworkCacheMetaData metaData;
+            metaData.setUrl(url);
+            QIODevice *d = cache.prepare(metaData);
+            cache.insert(d);
+            QVERIFY(cache.cacheSize() > cacheSize);
+            cacheSize = cache.cacheSize();
+        }
 
+        // Don't clear the cache on destruction so we can re-open the cache and test its size.
+        cache.setClearCacheOnDestruction(false);
+    }
+
+    SubQNetworkDiskCache cache;
+    cache.setCacheDirectory(tempDir.path());
+    QCOMPARE(cache.cacheSize(), cacheSize);
     cache.clear();
     QCOMPARE(cache.cacheSize(), qint64(0));
 }