Update CacheSettings structure.

mykhailo-kuchma · mykhailo-kuchma · commit c1ccaf7468d8 · 2019-12-05T18:43:58.000+02:00
* Add disk_path_mutable.
* Deprecate the disk_path in favor of a new disk_path_mutable.
* Update the SDK to use a new disk_path_mutable.
* When disk_path_mutable is empty, disk_path is used (backwards compatibility).

Resolves: OLPEDGE-1039

Signed-off-by: Mykhailo Kuchma &lt;ext-mykhailo.kuchma@here.com&gt;
diff --git a/olp-cpp-sdk-core/include/olp/core/cache/CacheSettings.h b/olp-cpp-sdk-core/include/olp/core/cache/CacheSettings.h
@@ -23,6 +23,8 @@
 
 #include <boost/optional.hpp>
 
+#include <olp/core/porting/deprecated.h>
+
 namespace olp {
 
 namespace cache {
@@ -40,8 +42,15 @@ struct CacheSettings {
   /**
    * @brief Path to save contents on disk
    */
+  OLP_SDK_DEPRECATED("Use disk_path_mutable instead. Will be removed 03.2020")
   boost::optional<std::string> disk_path = boost::none;
 
+  /**
+   * @brief Path to the mutable (read-write) cache, where SDK will cache and
+   * lookup the content. User should have write permissions.
+   */
+  boost::optional<std::string> disk_path_mutable = boost::none;
+
   /**
    * @brief Set the upper limit of disk space to use for persistent stores in
    * bytes. Default is 32 MB. Set it to std::uint64_t(-1) in order to never
diff --git a/olp-cpp-sdk-core/src/cache/DefaultCache.cpp b/olp-cpp-sdk-core/src/cache/DefaultCache.cpp
@@ -17,7 +17,14 @@
  * License-Filename: LICENSE
  */
 
+#include "olp/core/porting/warning_disable.h"
+
+PORTING_PUSH_WARNINGS()
+PORTING_CLANG_GCC_DISABLE_WARNING("-Wdeprecated-declarations")
+// Generated class methods use a deprecated field and generate warning
 #include "olp/core/cache/DefaultCache.h"
+PORTING_POP_WARNINGS()
+
 #include "DiskCache.h"
 #include "InMemoryCache.h"
 #include "olp/core/logging/Log.h"
@@ -57,6 +64,16 @@ bool StoreExpiry(const std::string& key, olp::cache::DiskCache& disk_cache,
                      olp::cache::InMemoryCache::DefaultTimeProvider()()));
 }
 
+void ValidateDiskPath(olp::cache::CacheSettings& settings) {
+  if (!settings.disk_path_mutable) {
+    PORTING_PUSH_WARNINGS()
+    PORTING_CLANG_GCC_DISABLE_WARNING("-Wdeprecated-declarations")
+    PORTING_MSVC_DISABLE_WARNINGS(4996)
+    settings.disk_path_mutable = settings.disk_path;
+    PORTING_POP_WARNINGS()
+  }
+}
+
 }  // namespace
 
 namespace olp {
@@ -104,7 +121,7 @@ bool DefaultCache::Clear() {
   }
   if (SetupStorage() != DefaultCache::StorageOpenResult::Success) {
     OLP_SDK_LOG_DEBUG_F(kLogTag, "Failed to reopen the diskcache %s",
-                        settings_.disk_path.get().c_str());
+                        settings_.disk_path_mutable.get().c_str());
     return false;
   }
   return true;
@@ -281,7 +298,11 @@ DefaultCache::StorageOpenResult DefaultCache::SetupStorage() {
   if (settings_.max_memory_cache_size > 0) {
     memory_cache_.reset(new InMemoryCache(settings_.max_memory_cache_size));
   }
-  if (settings_.disk_path) {
+
+  // Temporary code for backwards compatibility.
+  ValidateDiskPath(settings_);
+
+  if (settings_.disk_path_mutable) {
     StorageSettings storage_settings;
     storage_settings.max_disk_storage = settings_.max_disk_storage;
     storage_settings.max_chunk_size = settings_.max_chunk_size;
@@ -290,12 +311,12 @@ DefaultCache::StorageOpenResult DefaultCache::SetupStorage() {
     storage_settings.max_file_size = settings_.max_file_size;
 
     disk_cache_ = std::make_unique<DiskCache>();
-    auto status =
-        disk_cache_->Open(settings_.disk_path.get(), settings_.disk_path.get(),
-                          storage_settings, OpenOptions::Default);
+    auto status = disk_cache_->Open(settings_.disk_path_mutable.get(),
+                                    settings_.disk_path_mutable.get(),
+                                    storage_settings, OpenOptions::Default);
     if (status == OpenResult::Fail) {
       disk_cache_.reset();
-      settings_.disk_path = boost::none;
+      settings_.disk_path_mutable = boost::none;
       result = OpenDiskPathFailure;
     }
   }
diff --git a/olp-cpp-sdk-core/tests/cache/DefaultCacheTest.cpp b/olp-cpp-sdk-core/tests/cache/DefaultCacheTest.cpp
@@ -29,7 +29,7 @@
 
 TEST(DefaultCacheTest, BasicTest) {
   olp::cache::CacheSettings settings;
-  settings.disk_path = olp::utils::Dir::TempDirectory() + "/unittest";
+  settings.disk_path_mutable = olp::utils::Dir::TempDirectory() + "/unittest";
   olp::cache::DefaultCache cache(settings);
   ASSERT_EQ(olp::cache::DefaultCache::Success, cache.Open());
   ASSERT_TRUE(cache.Clear());
@@ -129,7 +129,7 @@ TEST(DefaultCacheTest, RemoveWithPrefix) {
 TEST(DefaultCacheTest, BasicDiskTest) {
   olp::cache::CacheSettings settings;
   settings.max_memory_cache_size = 0;
-  settings.disk_path = olp::utils::Dir::TempDirectory() + "/unittest";
+  settings.disk_path_mutable = olp::utils::Dir::TempDirectory() + "/unittest";
   olp::cache::DefaultCache cache(settings);
   ASSERT_EQ(olp::cache::DefaultCache::Success, cache.Open());
   ASSERT_TRUE(cache.Clear());
@@ -146,7 +146,7 @@ TEST(DefaultCacheTest, BasicDiskTest) {
 TEST(DefaultCacheTest, ExpiredDiskTest) {
   olp::cache::CacheSettings settings;
   settings.max_memory_cache_size = 0;
-  settings.disk_path = olp::utils::Dir::TempDirectory() + "/unittest";
+  settings.disk_path_mutable = olp::utils::Dir::TempDirectory() + "/unittest";
   olp::cache::DefaultCache cache(settings);
   ASSERT_EQ(olp::cache::DefaultCache::Success, cache.Open());
   ASSERT_TRUE(cache.Clear());
@@ -196,7 +196,7 @@ TEST(DefaultCacheTest, ExpiredMemTest) {
 
 TEST(DefaultCacheTest, BadPath) {
   olp::cache::CacheSettings settings;
-  settings.disk_path = std::string("/////this/is/a/bad/path");
+  settings.disk_path_mutable = std::string("/////this/is/a/bad/path");
   olp::cache::DefaultCache cache(settings);
   ASSERT_EQ(olp::cache::DefaultCache::OpenDiskPathFailure, cache.Open());
 
@@ -212,7 +212,7 @@ TEST(DefaultCacheTest, BadPath) {
 
 TEST(DefaultCacheTest, AlreadyInUsePath) {
   olp::cache::CacheSettings settings;
-  settings.disk_path = olp::utils::Dir::TempDirectory() + "/unittest";
+  settings.disk_path_mutable = olp::utils::Dir::TempDirectory() + "/unittest";
   olp::cache::DefaultCache cache(settings);
   ASSERT_EQ(olp::cache::DefaultCache::Success, cache.Open());
 
diff --git a/tests/integration/olp-cpp-sdk-dataservice-read/CatalogClientCacheTest.cpp b/tests/integration/olp-cpp-sdk-dataservice-read/CatalogClientCacheTest.cpp
@@ -39,15 +39,15 @@ class CatalogClientCacheTest : public CatalogClientTestBase {
       }
       case CacheType::DISK: {
         settings.max_memory_cache_size = 0;
-        settings.disk_path =
+        settings.disk_path_mutable =
             olp::utils::Dir::TempDirectory() + kClientTestCacheDir;
-        ClearCache(settings.disk_path.get());
+        ClearCache(settings.disk_path_mutable.get());
         break;
       }
       case CacheType::BOTH: {
-        settings.disk_path =
+        settings.disk_path_mutable =
             olp::utils::Dir::TempDirectory() + kClientTestCacheDir;
-        ClearCache(settings.disk_path.get());
+        ClearCache(settings.disk_path_mutable.get());
         break;
       }
       case CacheType::NONE: {
diff --git a/tests/integration/olp-cpp-sdk-dataservice-read/VersionedLayerClientCacheTest.cpp b/tests/integration/olp-cpp-sdk-dataservice-read/VersionedLayerClientCacheTest.cpp
@@ -43,15 +43,15 @@ class VersionedLayerClientCacheTest : public CatalogClientTestBase {
       }
       case CacheType::DISK: {
         settings.max_memory_cache_size = 0;
-        settings.disk_path =
+        settings.disk_path_mutable =
             olp::utils::Dir::TempDirectory() + kClientTestCacheDir;
-        ClearCache(settings.disk_path.get());
+        ClearCache(settings.disk_path_mutable.get());
         break;
       }
       case CacheType::BOTH: {
-        settings.disk_path =
+        settings.disk_path_mutable =
             olp::utils::Dir::TempDirectory() + kClientTestCacheDir;
-        ClearCache(settings.disk_path.get());
+        ClearCache(settings.disk_path_mutable.get());
         break;
       }
       case CacheType::NONE: {
diff --git a/tests/integration/olp-cpp-sdk-dataservice-read/VolatileLayerClientCacheTest.cpp b/tests/integration/olp-cpp-sdk-dataservice-read/VolatileLayerClientCacheTest.cpp
@@ -43,15 +43,15 @@ class VolatileLayerClientCacheTest : public CatalogClientTestBase {
       }
       case CacheType::DISK: {
         settings.max_memory_cache_size = 0;
-        settings.disk_path =
+        settings.disk_path_mutable =
             olp::utils::Dir::TempDirectory() + kClientTestCacheDir;
-        ClearCache(settings.disk_path.get());
+        ClearCache(settings.disk_path_mutable.get());
         break;
       }
       case CacheType::BOTH: {
-        settings.disk_path =
+        settings.disk_path_mutable =
             olp::utils::Dir::TempDirectory() + kClientTestCacheDir;
-        ClearCache(settings.disk_path.get());
+        ClearCache(settings.disk_path_mutable.get());
         break;
       }
       case CacheType::NONE: {
diff --git a/tests/performance/MemoryTest.cpp b/tests/performance/MemoryTest.cpp
@@ -415,7 +415,7 @@ TestConfiguration ShortRunningTestWithDiskCache() {
   if (location.empty()) {
     location = utils::Dir::TempDirectory() + "/performance_test";
   }
-  settings.disk_path = location;
+  settings.disk_path_mutable = location;
 
   TestConfiguration configuration;
   configuration.configuration_name = "short_test_disk_cache";

Original file line number	Diff line number	Diff line change
`@@ -415,7 +415,7 @@ TestConfiguration ShortRunningTestWithDiskCache() {`
`415`	`415`	`if (location.empty()) {`
`416`	`416`	`location = utils::Dir::TempDirectory() + "/performance_test";`
`417`	`417`	`}`
`418`		`- settings.disk_path = location;`
	`418`	`+ settings.disk_path_mutable = location;`
`419`	`419`
`420`	`420`	`TestConfiguration configuration;`
`421`	`421`	`configuration.configuration_name = "short_test_disk_cache";`