Align cache according to requirements.

If the user do not provide a default cache, SDK will create a default one.
Remove CreateDefaultCache from CatalogClient.h

Changed CatalogClientImpl class constructor to take settings by value.

Resolves: OLPEDGE-872
Resolves: OLPEDGE-908

Signed-off-by: Mykhailo Kuchma <[email protected]>
Signed-off-by: Diachenko Mykahilo <[email protected]>

Author: mykhailo-kuchma

olp-cpp-sdk-dataservice-read/include/olp/dataservice/read/CatalogClient.h

@@ -49,15 +49,6 @@ namespace model {
 class Partition;
 }  // namespace model
 
-/**
- * @brief Creates an instance of the default cache with provided settings.
- * @param settings Cache settings.
- *
- * @return DefaultCache instance.
- */
-DATASERVICE_READ_API std::shared_ptr<cache::KeyValueCache> CreateDefaultCache(
-    const cache::CacheSettings& settings = cache::CacheSettings());
-
 class CatalogRequest;
 using CatalogResult = model::Catalog;
 using CatalogResponse = client::ApiResponse<CatalogResult, client::ApiError>;

olp-cpp-sdk-dataservice-read/src/CatalogClient.cpp

@@ -18,27 +18,24 @@
  */
 
 #include "olp/dataservice/read/CatalogClient.h"
-#include <olp/core/cache/DefaultCache.h>
+
+#include "olp/core/cache/DefaultCache.h"
+#include "olp/core/client/OlpClientSettingsFactory.h"
 #include "CatalogClientImpl.h"
 
 namespace olp {
 namespace dataservice {
 namespace read {
 
-std::shared_ptr<cache::KeyValueCache> CreateDefaultCache(
-    const cache::CacheSettings& settings) {
-  auto cache = std::make_shared<cache::DefaultCache>(settings);
-  cache->Open();
-  return std::move(cache);
-}
-
 CatalogClient::CatalogClient(client::HRN catalog,
                              client::OlpClientSettings settings) {
-  auto cache = settings.cache;
-  auto settings_ptr =
-      std::make_shared<client::OlpClientSettings>(std::move(settings));
-  impl_ = std::make_shared<CatalogClientImpl>(
-      std::move(catalog), std::move(settings_ptr), std::move(cache));
+  // If the user did not specify cache, we creade a default one (memory only)
+  if (!settings.cache) {
+    settings.cache = client::OlpClientSettingsFactory::CreateDefaultCache({});
+  }
+
+  impl_ = std::make_shared<CatalogClientImpl>(std::move(catalog),
+                                              std::move(settings));
 }
 
 bool CatalogClient::cancelPendingRequests() {

olp-cpp-sdk-dataservice-read/src/CatalogClientImpl.cpp

@@ -19,9 +19,7 @@
 
 #include "CatalogClientImpl.h"
 
-#include <olp/core/client/OlpClientFactory.h>
 #include <olp/core/logging/Log.h>
-
 #include "PendingRequests.h"
 #include "PrefetchTilesProvider.h"
 #include "repositories/ApiRepository.h"
@@ -40,12 +38,11 @@ namespace {
 constexpr auto kLogTag = "CatalogClientImpl";
 }
 
-CatalogClientImpl::CatalogClientImpl(
-    HRN hrn, std::shared_ptr<OlpClientSettings> settings,
-    std::shared_ptr<cache::KeyValueCache> cache)
-    : catalog_(std::move(hrn)),
-      settings_(settings),
-      api_client_(OlpClientFactory::Create(*settings)) {
+CatalogClientImpl::CatalogClientImpl(HRN catalog, OlpClientSettings settings)
+    : catalog_(std::move(catalog)),
+      settings_(std::make_shared<OlpClientSettings>(std::move(settings))) {
+  auto cache = settings_->cache;
+
   // create repositories, satisfying dependencies.
   auto api_repo = std::make_shared<ApiRepository>(catalog_, settings_, cache);
 
@@ -59,7 +56,7 @@ CatalogClientImpl::CatalogClientImpl(
       catalog_, api_repo, catalog_repo_, partition_repo_, cache);
 
   auto prefetch_repo = std::make_shared<PrefetchTilesRepository>(
-      hrn, api_repo, partition_repo_->GetPartitionsCacheRepository(),
+      catalog_, api_repo, partition_repo_->GetPartitionsCacheRepository(),
       settings_);
 
   prefetch_provider_ = std::make_shared<PrefetchTilesProvider>(

olp-cpp-sdk-dataservice-read/src/CatalogClientImpl.h

@@ -50,9 +50,7 @@ class PendingRequests;
 
 class CatalogClientImpl final {
  public:
-  CatalogClientImpl(client::HRN catalog,
-                    std::shared_ptr<client::OlpClientSettings> settings,
-                    std::shared_ptr<cache::KeyValueCache> cache);
+  CatalogClientImpl(client::HRN catalog, client::OlpClientSettings settings);
 
   ~CatalogClientImpl();
 
@@ -93,7 +91,6 @@ class CatalogClientImpl final {
  private:
   client::HRN catalog_;
   std::shared_ptr<client::OlpClientSettings> settings_;
-  std::shared_ptr<client::OlpClient> api_client_;
   std::shared_ptr<repository::CatalogRepository> catalog_repo_;
   std::shared_ptr<repository::PartitionsRepository> partition_repo_;
   std::shared_ptr<repository::DataRepository> data_repo_;

olp-cpp-sdk-dataservice-read/tests/DataRepositoryTest.cpp

@@ -21,8 +21,10 @@
 
 #include <matchers/NetworkUrlMatchers.h>
 #include <mocks/NetworkMock.h>
+#include <olp/core/cache/KeyValueCache.h>
 #include <olp/core/client/OlpClientFactory.h>
 #include <olp/core/client/OlpClientSettings.h>
+#include <olp/core/client/OlpClientSettingsFactory.h>
 #include <olp/dataservice/read/DataRequest.h>
 #include <repositories/DataRepository.h>
 
@@ -62,7 +64,8 @@ class DataRepositoryTest : public ::testing::Test {
 void DataRepositoryTest::SetUp() {
   network_mock_ = std::make_shared<NetworkMock>();
   settings_ = std::make_shared<olp::client::OlpClientSettings>();
-  settings_->cache = olp::dataservice::read::CreateDefaultCache();
+  settings_->cache =
+      olp::client::OlpClientSettingsFactory::CreateDefaultCache({});
   settings_->network_request_handler = network_mock_;
 }
 

tests/integration/olp-cpp-sdk-dataservice-read/CatalogClientCacheTest.cpp

@@ -54,6 +54,11 @@ class CatalogClientCacheTest : public CatalogClientTestBase {
         ClearCache(settings.disk_path.get());
         break;
       }
+      case CacheType::NONE: {
+        // We don't create a cache here
+        settings_.cache = nullptr;
+        return;
+      }
       default:
         // shouldn't get here
         break;
@@ -435,5 +440,6 @@ TEST_P(CatalogClientCacheTest, GetVolatilePartitionsExpiry) {
 
 INSTANTIATE_TEST_SUITE_P(, CatalogClientCacheTest,
                          ::testing::Values(CacheType::IN_MEMORY,
-                                           CacheType::DISK, CacheType::BOTH));
+                                           CacheType::DISK, CacheType::BOTH,
+                                           CacheType::NONE));
 }  // namespace

tests/integration/olp-cpp-sdk-dataservice-read/CatalogClientTest.cpp

@@ -20,7 +20,9 @@
 #include <gmock/gmock.h>
 #include <matchers/NetworkUrlMatchers.h>
 #include <mocks/NetworkMock.h>
+#include <olp/core/cache/KeyValueCache.h>
 #include <olp/core/client/HRN.h>
+#include <olp/core/client/OlpClientSettingsFactory.h>
 #include <olp/core/http/Network.h>
 #include <olp/core/logging/Log.h>
 #include <olp/core/porting/make_unique.h>
@@ -1032,7 +1034,8 @@ TEST_P(CatalogClientTest, GetDataWithPartitionIdCancelInnerConfig) {
 
   olp::cache::CacheSettings cache_settings;
   cache_settings.max_memory_cache_size = 0;
-  settings_.cache = olp::dataservice::read::CreateDefaultCache(cache_settings);
+  settings_.cache =
+      olp::client::OlpClientSettingsFactory::CreateDefaultCache(cache_settings);
   auto catalog_client =
       std::make_unique<olp::dataservice::read::CatalogClient>(hrn, settings_);
 

tests/integration/olp-cpp-sdk-dataservice-read/CatalogClientTestBase.h

@@ -29,7 +29,7 @@ namespace olp {
 namespace tests {
 namespace integration {
 
-enum class CacheType { IN_MEMORY, DISK, BOTH };
+enum class CacheType { IN_MEMORY, DISK, BOTH, NONE };
 
 std::ostream& operator<<(std::ostream& os, const CacheType cache_type);