SDK should not cache data with FetchOption::OnlineOnly requests. (#705)

* SDK should not cache data with FetchOption::OnlineOnly requests.
Add Check if fetch_option is not OnlineOnly, and cache only in that case.
Update tests. Add CacheWithUpdate option.

Resolves:OLPEDGE-1659

Signed-off-by: Liubov Didkivska <[email protected]>

Author: Liubov Didkivska

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

@@ -57,7 +57,7 @@ ApiClientLookup::ApiClientResponse ApiClientLookup::LookupApi(
     client::OlpClientSettings settings) {
   repository::ApiCacheRepository repository(catalog, settings.cache);
 
-  if (options != OnlineOnly) {
+  if (options != OnlineOnly && options != CacheWithUpdate) {
     auto url = repository.Get(service, service_version);
     if (url) {
       OLP_SDK_LOG_INFO_F(kLogTag, "LookupApi(%s, %s) -> from cache",
@@ -107,7 +107,9 @@ ApiClientLookup::ApiClientResponse ApiClientLookup::LookupApi(
   }
 
   const auto& service_url = api_result.at(0).GetBaseUrl();
-  repository.Put(service, service_version, service_url);
+  if (options != OnlineOnly && options != CacheWithUpdate) {
+    repository.Put(service, service_version, service_url);
+  }
   OLP_SDK_LOG_INFO_F(kLogTag, "LookupApi(%s/%s): %s - OK, service_url=%s",
                      service.c_str(), service_version.c_str(),
                      catalog.partition.c_str(), service_url.c_str());

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

@@ -49,7 +49,8 @@ inline client::CancellationToken ScheduleFetch(TaskScheduler&& schedule_task,
         schedule_task(request.WithFetchOption(FetchOptions::CacheOnly),
                       std::forward<Callback>(callback));
     auto online_token = schedule_task(
-        std::move(request.WithFetchOption(FetchOptions::OnlineOnly)), nullptr);
+        std::move(request.WithFetchOption(FetchOptions::CacheWithUpdate)),
+        nullptr);
 
     return client::CancellationToken([=]() {
       cache_token.Cancel();
@@ -92,4 +93,4 @@ inline client::CancellationToken AddTask(
 
 }  // namespace read
 }  // namespace dataservice
-}  // namespace olp
+}  // namespace olp

olp-cpp-sdk-dataservice-read/src/repositories/CatalogRepository.cpp

@@ -54,7 +54,7 @@ CatalogResponse CatalogRepository::GetCatalog(
 
   repository::CatalogCacheRepository repository{catalog, settings.cache};
 
-  if (fetch_options != OnlineOnly) {
+  if (fetch_options != OnlineOnly && fetch_options != CacheWithUpdate) {
     auto cached = repository.Get();
     if (cached) {
       OLP_SDK_LOG_INFO_F(kLogTag, "cache catalog '%s' found!",
@@ -79,9 +79,10 @@ CatalogResponse CatalogRepository::GetCatalog(
   auto catalog_response =
       ConfigApi::GetCatalog(client, catalog.ToCatalogHRNString(),
                             request.GetBillingTag(), cancellation_context);
-  if (catalog_response.IsSuccessful()) {
+  if (catalog_response.IsSuccessful() && fetch_options != OnlineOnly) {
     repository.Put(catalog_response.GetResult());
-  } else {
+  }
+  if (!catalog_response.IsSuccessful()) {
     const auto& error = catalog_response.GetError();
     if (error.GetHttpStatusCode() == http::HttpStatusCode::FORBIDDEN) {
       repository.Clear();
@@ -99,7 +100,7 @@ CatalogVersionResponse CatalogRepository::GetLatestVersion(
   repository::CatalogCacheRepository repository(catalog, settings.cache);
 
   auto fetch_option = request.GetFetchOption();
-  if (fetch_option != OnlineOnly) {
+  if (fetch_option != OnlineOnly && fetch_option != CacheWithUpdate) {
     auto cached_version = repository.GetVersion();
     if (cached_version) {
       OLP_SDK_LOG_INFO_F(kLogTag, "cache catalog '%s' found!",
@@ -127,9 +128,10 @@ CatalogVersionResponse CatalogRepository::GetLatestVersion(
   auto version_response = MetadataApi::GetLatestCatalogVersion(
       client, -1, request.GetBillingTag(), cancellation_context);
 
-  if (version_response.IsSuccessful()) {
+  if (version_response.IsSuccessful() && fetch_option != OnlineOnly) {
     repository.PutVersion(version_response.GetResult());
-  } else {
+  }
+  if (!version_response.IsSuccessful()) {
     const auto& error = version_response.GetError();
     if (error.GetHttpStatusCode() == http::HttpStatusCode::FORBIDDEN) {
       repository.Clear();

olp-cpp-sdk-dataservice-read/src/repositories/DataRepository.cpp

@@ -141,7 +141,7 @@ DataResponse DataRepository::GetBlobData(
 
   repository::DataCacheRepository repository(catalog, settings.cache);
 
-  if (fetch_option != OnlineOnly) {
+  if (fetch_option != OnlineOnly && fetch_option != CacheWithUpdate) {
     auto cached_data = repository.Get(layer, data_handle.value());
     if (cached_data) {
       OLP_SDK_LOG_INFO_F(kLogTag, "cache data '%s' found!",
@@ -174,9 +174,11 @@ DataResponse DataRepository::GetBlobData(
         data_request.GetBillingTag(), cancellation_context);
   }
 
-  if (blob_response.IsSuccessful()) {
+  if (blob_response.IsSuccessful() && fetch_option != OnlineOnly) {
     repository.Put(blob_response.GetResult(), layer, data_handle.value());
-  } else {
+  }
+
+  if (!blob_response.IsSuccessful()) {
     const auto& error = blob_response.GetError();
     if (error.GetHttpStatusCode() == http::HttpStatusCode::FORBIDDEN) {
       OLP_SDK_LOG_INFO_F(kLogTag, "clear '%s' cache",

olp-cpp-sdk-dataservice-read/src/repositories/PartitionsRepository.cpp

@@ -124,7 +124,7 @@ PartitionsResponse PartitionsRepository::GetPartitions(
 
   repository::PartitionsCacheRepository repository(catalog, settings.cache);
 
-  if (fetch_option != OnlineOnly) {
+  if (fetch_option != OnlineOnly && fetch_option != CacheWithUpdate) {
     auto cached_partitions = repository.Get(request, layer);
     if (cached_partitions) {
       OLP_SDK_LOG_INFO_F(kLogTag, "cache data '%s' found!",
@@ -173,12 +173,13 @@ PartitionsResponse PartitionsRepository::GetPartitions(
   // Save all partitions only when downloaded via metadata API
   const bool is_layer_metadata = partition_ids.empty();
 
-  if (response.IsSuccessful()) {
+  if (response.IsSuccessful() && fetch_option != OnlineOnly) {
     OLP_SDK_LOG_INFO_F(kLogTag, "put '%s' to cache",
                        request.CreateKey(layer).c_str());
     repository.Put(request, response.GetResult(), layer, expiry,
                    is_layer_metadata);
-  } else {
+  }
+  if (!response.IsSuccessful()) {
     const auto& error = response.GetError();
     if (error.GetHttpStatusCode() == http::HttpStatusCode::FORBIDDEN) {
       OLP_SDK_LOG_INFO_F(kLogTag, "clear '%s' cache",
@@ -211,7 +212,7 @@ PartitionsResponse PartitionsRepository::GetPartitionById(
 
   auto fetch_option = data_request.GetFetchOption();
 
-  if (fetch_option != OnlineOnly) {
+  if (fetch_option != OnlineOnly && fetch_option != CacheWithUpdate) {
     auto cached_partitions =
         repository.Get(partition_request, partitions, layer);
     if (cached_partitions.GetPartitions().size() == partitions.size()) {
@@ -240,12 +241,13 @@ PartitionsResponse PartitionsRepository::GetPartitionById(
       client, layer, partitions, version, {}, data_request.GetBillingTag(),
       cancellation_context);
 
-  if (query_response.IsSuccessful()) {
+  if (query_response.IsSuccessful() && fetch_option != OnlineOnly) {
     OLP_SDK_LOG_INFO_F(kLogTag, "put '%s' to cache",
                        data_request.CreateKey(layer).c_str());
     repository.Put(partition_request, query_response.GetResult(), layer,
                    boost::none /* TODO: expiration */);
-  } else {
+  }
+  if (!query_response.IsSuccessful()) {
     const auto& error = query_response.GetError();
     if (error.GetHttpStatusCode() == http::HttpStatusCode::FORBIDDEN) {
       OLP_SDK_LOG_INFO_F(kLogTag, "clear '%s' cache",
@@ -353,7 +355,7 @@ PartitionsResponse PartitionsRepository::QueryPartitionForVersionedTile(
     }
   }
 
-  if (fetch_option != OnlineOnly) {
+  if (fetch_option != OnlineOnly && fetch_option != CacheWithUpdate) {
     // add partitions to cache
     repository::PartitionsCacheRepository repository(catalog, settings.cache);
     repository.Put(PartitionsRequest().WithVersion(version), partitions,

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

@@ -98,7 +98,7 @@ TEST(ApiClientLookupTest, LookupApi) {
                                          olp::http::HttpStatusCode::OK),
                                      OLP_SDK_HTTP_RESPONSE_LOOKUP_CONFIG));
     EXPECT_CALL(*cache, Put(cache_key, _, _, _))
-        .Times(1)
+        .Times(0)
         .WillOnce(Return(true));
 
     client::CancellationContext context;

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

@@ -170,7 +170,7 @@ TEST_F(CatalogRepositoryTest, GetLatestVersionOnlineOnlyNotFound) {
   EXPECT_FALSE(response.IsSuccessful());
 }
 
-TEST_F(CatalogRepositoryTest, GetLatestVersionOnlineOnlyFoundAndCacheWritten) {
+TEST_F(CatalogRepositoryTest, GetLatestVersionOnlineOnlyFound) {
   olp::client::CancellationContext context;
 
   auto request = CatalogVersionRequest();
@@ -183,9 +183,9 @@ TEST_F(CatalogRepositoryTest, GetLatestVersionOnlineOnlyFoundAndCacheWritten) {
         return boost::any{};
       });
 
-  EXPECT_CALL(*cache_, Put(Eq(kLatestVersionCacheKey), _, _, _)).Times(1);
+  EXPECT_CALL(*cache_, Put(Eq(kLatestVersionCacheKey), _, _, _)).Times(0);
 
-  EXPECT_CALL(*cache_, Put(Eq(kMetadataCacheKey), _, _, _)).Times(1);
+  EXPECT_CALL(*cache_, Put(Eq(kMetadataCacheKey), _, _, _)).Times(0);
 
   EXPECT_CALL(*network_,
               Send(IsGetRequest(OLP_SDK_URL_LOOKUP_METADATA), _, _, _, _))
@@ -339,7 +339,7 @@ TEST_F(CatalogRepositoryTest, GetLatestVersionTimeouted) {
             response.GetError().GetErrorCode());
 }
 
-TEST_F(CatalogRepositoryTest, GetCatalogOnlineOnlyFoundAndCacheWritten) {
+TEST_F(CatalogRepositoryTest, GetCatalogOnlineOnlyFound) {
   olp::client::CancellationContext context;
 
   auto request = CatalogRequest();
@@ -351,9 +351,9 @@ TEST_F(CatalogRepositoryTest, GetCatalogOnlineOnlyFoundAndCacheWritten) {
         return boost::any{};
       });
 
-  EXPECT_CALL(*cache_, Put(Eq(kCatalogCacheKey), _, _, _)).Times(1);
+  EXPECT_CALL(*cache_, Put(Eq(kCatalogCacheKey), _, _, _)).Times(0);
 
-  EXPECT_CALL(*cache_, Put(Eq(kConfigCacheKey), _, _, _)).Times(1);
+  EXPECT_CALL(*cache_, Put(Eq(kConfigCacheKey), _, _, _)).Times(0);
 
   ON_CALL(*network_, Send(IsGetRequest(OLP_SDK_URL_LOOKUP_CONFIG), _, _, _, _))
       .WillByDefault(ReturnHttpResponse(olp::http::NetworkResponse().WithStatus(

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

@@ -171,7 +171,7 @@ TEST_F(PartitionsRepositoryTest, GetPartitionById) {
                                          olp::http::HttpStatusCode::OK),
                                      kOlpSdkHttpResponseLookupQuery));
 
-    EXPECT_CALL(*cache, Put(Eq(kCacheKeyMetadata), _, _, _)).Times(1);
+    EXPECT_CALL(*cache, Put(Eq(kCacheKeyMetadata), _, _, _)).Times(0);
   };
 
   {
@@ -246,7 +246,7 @@ TEST_F(PartitionsRepositoryTest, GetPartitionById) {
                                          olp::http::HttpStatusCode::OK),
                                      kOlpSdkHttpResponsePartitionById));
 
-    EXPECT_CALL(*cache, Put(Eq(cache_key), _, _, _)).Times(1);
+    EXPECT_CALL(*cache, Put(Eq(cache_key), _, _, _)).Times(0);
 
     auto response = repository::PartitionsRepository::GetPartitionById(
         catalog_hrn, kVersionedLayerId, context,
@@ -274,7 +274,7 @@ TEST_F(PartitionsRepositoryTest, GetPartitionById) {
         .WillOnce(ReturnHttpResponse(olp::http::NetworkResponse().WithStatus(
                                          olp::http::HttpStatusCode::OK),
                                      kOlpSdkHttpResponsePartitionById));
-    EXPECT_CALL(*cache, Put(Eq(cache_key_no_version), _, _, _)).Times(1);
+    EXPECT_CALL(*cache, Put(Eq(cache_key_no_version), _, _, _)).Times(0);
 
     auto response = repository::PartitionsRepository::GetPartitionById(
         catalog_hrn, kVersionedLayerId, context,

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

@@ -177,7 +177,7 @@ TEST_F(DataserviceReadVolatileLayerClientTest, GetPartitionsVersionIsIgnored) {
         "Online request with version in request. Version should be ignored.");
     auto request = olp::dataservice::read::PartitionsRequest();
     request.WithVersion(4);
-    request.WithFetchOption(FetchOptions::OnlineOnly);
+    request.WithFetchOption(FetchOptions::OnlineIfNotFound);
 
     PartitionsResponse partitions_response;
     olp::client::Condition condition;