QmlDesigner: Improve storage handling in StorageCache

Using an explicit class for the storage not even makes the code easier
to understand but enables the possibility of code optimizations too.

Change-Id: I9239919d3385ba1bf141c23bdc3013b6c1e624ed
Reviewed-by: Tim Jenssen <tim.jenssen@qt.io>

src/plugins/qmldesigner/designercore/metainfo/storagecache.h

@@ -63,16 +63,14 @@ public:
 template<typename Type,
          typename ViewType,
          typename IndexType,
+         typename Storage,
          typename Mutex,
          typename Compare,
          Compare compare = Utils::compare,
-         typename CacheEntry = StorageCacheEntry<Type, ViewType, IndexType>,
+         typename CacheEntry = StorageCacheEntry<Type, ViewType, IndexType>>
-         typename FetchValue = std::function<Type(IndexType)>,
-         typename FetchId = std::function<IndexType(ViewType)>>
 class StorageCache
 {
-    template<typename T, typename V, typename I, typename M, typename C, C c, typename CE, typename, typename>
+    friend StorageCache;
-    friend class StorageCache;
 
     using ResultType = std::conditional_t<std::is_base_of<NonLockingMutex, Mutex>::value, ViewType, Type>;
 
@@ -82,9 +80,8 @@ public:
     using const_iterator = typename CacheEntries::const_iterator;
     using Found = QmlDesigner::Found<const_iterator>;
 
-    StorageCache(FetchValue fetchValue, FetchId fetchId, std::size_t reserveSize = 1024)
+    StorageCache(Storage storage, std::size_t reserveSize = 1024)
-        : m_fetchValue{std::move(fetchValue)}
+        : m_storage{std::move(storage)}
-        , m_fetchId{std::move(fetchId)}
     {
         m_entries.reserve(reserveSize);
         m_indices.reserve(reserveSize);
@@ -165,7 +162,7 @@ public:
                             m_entries.begin(),
                             m_entries.end(),
                             Utils::make_iterator([&](ViewType newView) {
-                                IndexType index = m_fetchId(newView);
+                                IndexType index = m_storage.fetchId(newView);
                                 newCacheEntries.emplace_back(newView, index);
                             }),
                             less);
@@ -213,7 +210,7 @@ public:
         if (!std::is_base_of<NonLockingMutex, Mutex>::value)
             found = find(view);
         if (!found.wasFound) {
-            IndexType index = insertEntry(found.iterator, view, m_fetchId(view));
+            IndexType index = insertEntry(found.iterator, view, m_storage.fetchId(view));
             found.iterator = m_entries.begin() + index;
         }
 
@@ -249,7 +246,7 @@ public:
         std::lock_guard<Mutex> exclusiveLock(m_mutex);
         IndexType index;
 
-        Type value{m_fetchValue(id)};
+        Type value{m_storage.fetchValue(id)};
         index = insertEntry(find(value).iterator, value, id);
 
         return std::move(m_entries[index].value);
@@ -330,8 +327,7 @@ private:
     CacheEntries m_entries;
     std::vector<IndexType> m_indices;
     mutable Mutex m_mutex;
-    FetchValue m_fetchValue;
+    Storage m_storage;
-    FetchId m_fetchId;
 };
 
 } // namespace QmlDesigner

src/plugins/qmldesigner/designercore/metainfo/storagecachefwd.h

@@ -31,14 +31,6 @@ namespace QmlDesigner {
 
 class NonLockingMutex;
 
-template<typename Type,
+template<typename Type, typename ViewType, typename IndexType, typename Storage, typename Mutex, typename Compare, Compare compare, typename CacheEntry>
-         typename ViewType,
-         typename IndexType,
-         typename Mutex,
-         typename Compare,
-         Compare compare,
-         typename CacheEntry,
-         typename FetchValue,
-         typename FetchId>
 class StorageCache;
 } // namespace QmlDesigner

tests/unit/unittest/storagecache-test.cpp

@@ -42,12 +42,21 @@ using uint64 = unsigned long long;
 using QmlDesigner::findInSorted;
 using Utils::compare;
 using Utils::reverseCompare;
-using StorageIdFunction = std::function<int(Utils::SmallStringView)>;
+
-using StorageStringFunction = std::function<Utils::PathString(int)>;
+class StorageAdapter
+{
+public:
+    auto fetchId(Utils::SmallStringView view) { return storage.fetchDirectoryId(view); }
+
+    auto fetchValue(int id) { return storage.fetchDirectoryPath(id); }
+
+    MockFilePathStorage &storage;
+};
 
 using CacheWithMockLocking = QmlDesigner::StorageCache<Utils::PathString,
                                                        Utils::SmallStringView,
                                                        int,
+                                                       StorageAdapter,
                                                        NiceMock<MockMutex>,
                                                        decltype(&Utils::reverseCompare),
                                                        Utils::reverseCompare>;
@@ -55,6 +64,7 @@ using CacheWithMockLocking = QmlDesigner::StorageCache<Utils::PathString,
 using CacheWithoutLocking = QmlDesigner::StorageCache<Utils::PathString,
                                                       Utils::SmallStringView,
                                                       int,
+                                                      StorageAdapter,
                                                       NiceMock<MockMutexNonLocking>,
                                                       decltype(&Utils::reverseCompare),
                                                       Utils::reverseCompare>;
@@ -87,11 +97,8 @@ protected:
 protected:
     NiceMock<SqliteDatabaseMock> databaseMock;
     NiceMock<MockFilePathStorage> mockStorage{databaseMock};
-    StorageIdFunction mockStorageFetchDirectyId{
+    StorageAdapter storageAdapter{mockStorage};
-        [&](Utils::SmallStringView string) { return mockStorage.fetchDirectoryId(string); }};
+    Cache cache{storageAdapter};
-    StorageStringFunction mockStorageFetchDirectyPath{
-        [&](int id) { return mockStorage.fetchDirectoryPath(id); }};
-    Cache cache{mockStorageFetchDirectyPath, mockStorageFetchDirectyId};
     typename Cache::MutexType &mockMutex = cache.mutex();
     Utils::PathString filePath1{"/file/pathOne"};
     Utils::PathString filePath2{"/file/pathTwo"};