consistent-config-caching (#14)

configcat · Jul 21, 2023 · 7f34bd9 · 7f34bd9
1 parent 65206be
commit 7f34bd9
Show file tree

Hide file tree

Showing 12 changed files with 167 additions and 48 deletions.
diff --git a/.github/workflows/cpp-ci.yml b/.github/workflows/cpp-ci.yml
@@ -21,7 +21,7 @@ jobs:
       CTEST_OUTPUT_ON_FAILURE: 1
 
     steps:
-    - uses: actions/checkout@v2
+    - uses: actions/checkout@v3
     - name: Install dependencies on ubuntu
       if: startsWith(matrix.os, 'ubuntu')
       run: |
@@ -58,7 +58,7 @@ jobs:
       CXXFLAGS: "--coverage -fno-inline"
 
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
       - name: Install dependencies
         run: |
           git clone https://github.com/microsoft/vcpkg build/vcpkg

diff --git a/include/configcat/config.h b/include/configcat/config.h
@@ -184,23 +184,27 @@ struct ConfigEntry {
     static constexpr char kConfig[] = "config";
     static constexpr char kETag[] = "etag";
     static constexpr char kFetchTime[] = "fetch_time";
+    static constexpr char kSerializationFormatVersion[] = "v2";
 
     static inline std::shared_ptr<ConfigEntry> empty = std::make_shared<ConfigEntry>(Config::empty, "empty");
 
     ConfigEntry(std::shared_ptr<Config> config = Config::empty,
                 const std::string& eTag = "",
+                const std::string& configJsonString = "{}",
                 double fetchTime = kDistantPast):
             config(config),
             eTag(eTag),
+            configJsonString(configJsonString),
             fetchTime(fetchTime) {
     }
     ConfigEntry(const ConfigEntry&) = delete; // Disable copy
 
-    static std::shared_ptr<ConfigEntry> fromJson(const std::string& jsonString);
-    std::string toJson() const;
+    static std::shared_ptr<ConfigEntry> fromString(const std::string& text);
+    std::string serialize() const;
 
     std::shared_ptr<Config> config;
     std::string eTag;
+    std::string configJsonString;
     double fetchTime;
 };
 

diff --git a/src/config.cpp b/src/config.cpp
@@ -1,6 +1,7 @@
 #include "configcat/config.h"
 #include <nlohmann/json.hpp>
 #include <fstream>
+#include <cmath>
 
 using namespace std;
 using json = nlohmann::json;
@@ -165,20 +166,39 @@ shared_ptr<Config> Config::fromFile(const string& filePath) {
     return config;
 }
 
-shared_ptr<ConfigEntry> ConfigEntry::fromJson(const std::string& jsonString) {
-    json configEntryObj = json::parse(jsonString);
-    auto config = make_shared<Config>();
-    auto configObj = configEntryObj.at(kConfig);
-    configObj.get_to(*config);
-    return make_shared<ConfigEntry>(config, configEntryObj.value(kETag, ""), configEntryObj.value(kFetchTime, kDistantPast));
+shared_ptr<ConfigEntry> ConfigEntry::fromString(const string& text) {
+    if (text.empty())
+        return ConfigEntry::empty;
+
+    auto fetchTimeIndex = text.find('\n');
+    auto eTagIndex = text.find('\n', fetchTimeIndex + 1);
+    if (fetchTimeIndex == string::npos || eTagIndex == string::npos) {
+        throw std::invalid_argument("Number of values is fewer than expected.");
+    }
+
+    auto fetchTimeString = text.substr(0, fetchTimeIndex);
+    double fetchTime;
+    try {
+        fetchTime = std::stod(fetchTimeString);
+    } catch (const std::exception& e) {
+        throw std::invalid_argument("Invalid fetch time: " + fetchTimeString + ". " + e.what());
+    }
+
+    auto eTag = text.substr(fetchTimeIndex + 1, eTagIndex - fetchTimeIndex - 1);
+    if (eTag.empty()) {
+        throw std::invalid_argument("Empty eTag value");
+    }
+
+    auto configJsonString = text.substr(eTagIndex + 1);
+    try {
+        return make_shared<ConfigEntry>(Config::fromJson(configJsonString), eTag, configJsonString, fetchTime / 1000.0);
+    } catch (const std::exception& e) {
+        throw std::invalid_argument("Invalid config JSON: " + configJsonString + ". " + e.what());
+    }
 }
 
-string ConfigEntry::toJson() const {
-    return "{"s +
-       '"' + kConfig + '"' + ":" + (config ? config->toJson() : "{}") +
-       "," + '"' + kETag + '"' + ":" + '"' + eTag + '"' +
-       "," + '"' + kFetchTime + '"' + ":" + to_string(fetchTime) +
-   "}";
+string ConfigEntry::serialize() const {
+    return to_string(static_cast<uint64_t>(floor(fetchTime * 1000))) + "\n" + eTag + "\n" + configJsonString;
 }
 
 } // namespace configcat
diff --git a/src/configfetcher.cpp b/src/configfetcher.cpp
@@ -178,7 +178,7 @@ FetchResponse ConfigFetcher::fetch(const std::string& eTag) {
             try {
                 auto config = Config::fromJson(response.text);
                 LOG_DEBUG << "Fetch was successful: new config fetched.";
-                return FetchResponse(fetched, make_shared<ConfigEntry>(config, eTag, getUtcNowSecondsSinceEpoch()));
+                return FetchResponse(fetched, make_shared<ConfigEntry>(config, eTag, response.text, getUtcNowSecondsSinceEpoch()));
             } catch (exception& exception) {
                 LogEntry logEntry(logger, LOG_LEVEL_ERROR, 1105);
                 logEntry <<

diff --git a/src/configservice.cpp b/src/configservice.cpp
@@ -21,7 +21,7 @@ ConfigService::ConfigService(const string& sdkKey,
     pollingMode(options.pollingMode ? options.pollingMode : PollingMode::autoPoll()),
     cachedEntry(ConfigEntry::empty),
     configCache(configCache) {
-    cacheKey = SHA1()(string("cpp_") + ConfigFetcher::kConfigJsonName + "_" + sdkKey);
+    cacheKey = generateCacheKey(sdkKey);
     configFetcher = make_unique<ConfigFetcher>(sdkKey, logger, pollingMode->getPollingIdentifier(), options);
     offline = options.offline;
     startTime = chrono::steady_clock::now();
@@ -110,6 +110,9 @@ void ConfigService::setOffline() {
     LOG_INFO(5200) << "Switched to OFFLINE mode.";
 }
 
+string ConfigService::generateCacheKey(const string& sdkKey) {
+    return SHA1()(sdkKey + "_" + ConfigFetcher::kConfigJsonName + "_" + ConfigEntry::kSerializationFormatVersion);
+}
 tuple<shared_ptr<ConfigEntry>, string> ConfigService::fetchIfOlder(double time, bool preferCache) {
     {
         lock_guard<mutex> lock(fetchMutex);
@@ -189,7 +192,7 @@ shared_ptr<ConfigEntry> ConfigService::readCache() {
         }
 
         cachedEntryString = jsonString;
-        return ConfigEntry::fromJson(jsonString);
+        return ConfigEntry::fromString(jsonString);
     } catch (exception& exception) {
         LOG_ERROR(2200) << "Error occurred while reading the cache. " << exception.what();
         return ConfigEntry::empty;
@@ -198,7 +201,7 @@ shared_ptr<ConfigEntry> ConfigService::readCache() {
 
 void ConfigService::writeCache(const std::shared_ptr<ConfigEntry>& configEntry) {
     try {
-        configCache->write(cacheKey, configEntry->toJson());
+        configCache->write(cacheKey, configEntry->serialize());
     } catch (exception& exception) {
         LOG_ERROR(2201) << "Error occurred while writing the cache. " << exception.what();
     }

diff --git a/src/configservice.h b/src/configservice.h
@@ -37,6 +37,8 @@ class ConfigService {
     void setOffline();
     bool isOffline() { return offline; }
 
+    static std::string generateCacheKey(const std::string& sdkKey);
+
 private:
     // Returns the ConfigEntry object and error message in case of any error.
     std::tuple<std::shared_ptr<ConfigEntry>, std::string> fetchIfOlder(double time, bool preferCache = false);

diff --git a/src/version.h b/src/version.h
@@ -1,3 +1,3 @@
 #pragma once
 
-#define CONFIGCAT_VERSION "2.0.1"
+#define CONFIGCAT_VERSION "3.0.0"
diff --git a/test/test-autopolling.cpp b/test/test-autopolling.cpp
@@ -174,9 +174,13 @@ TEST_F(AutoPollingTest, Cache) {
 }
 
 TEST_F(AutoPollingTest, ReturnCachedConfigWhenCacheIsNotExpired) {
-    auto mockCache = make_shared<SingleValueCache>(R"({"config":)"s
-        + string_format(kTestJsonFormat, R"("test")") + R"(,"etag":"test-etag")"
-        + R"(,"fetch_time":)" + to_string(getUtcNowSecondsSinceEpoch()) + "}");
+    auto jsonString = string_format(kTestJsonFormat, R"("test")");
+    auto mockCache = make_shared<SingleValueCache>(ConfigEntry(
+        Config::fromJson(jsonString),
+        "test-etag",
+        jsonString,
+        getUtcNowSecondsSinceEpoch()).serialize()
+    );
 
     configcat::Response firstResponse = {200, string_format(kTestJsonFormat, R"("test2")")};
     mockHttpSessionAdapter->enqueueResponse(firstResponse);
@@ -212,9 +216,13 @@ TEST_F(AutoPollingTest, ReturnCachedConfigWhenCacheIsNotExpired) {
 TEST_F(AutoPollingTest, FetchConfigWhenCacheIsExpired) {
     auto pollIntervalSeconds = 2;
     auto maxInitWaitTimeSeconds = 1;
-    auto mockCache = make_shared<SingleValueCache>(R"({"config":)"s
-        + string_format(kTestJsonFormat, R"("test")") + R"(,"etag":"test-etag")"
-        + R"(,"fetch_time":)" + to_string(getUtcNowSecondsSinceEpoch() - pollIntervalSeconds) + "}");
+    auto jsonString = string_format(kTestJsonFormat, R"("test")");
+    auto mockCache = make_shared<SingleValueCache>(ConfigEntry(
+        Config::fromJson(jsonString),
+        "test-etag",
+        jsonString,
+        getUtcNowSecondsSinceEpoch() - pollIntervalSeconds).serialize()
+    );
 
     configcat::Response firstResponse = {200, string_format(kTestJsonFormat, R"("test2")")};
     mockHttpSessionAdapter->enqueueResponse(firstResponse);
@@ -232,9 +240,13 @@ TEST_F(AutoPollingTest, FetchConfigWhenCacheIsExpired) {
 TEST_F(AutoPollingTest, initWaitTimeReturnCached) {
     auto pollIntervalSeconds = 60;
     auto maxInitWaitTimeSeconds = 1;
-    auto mockCache = make_shared<SingleValueCache>(R"({"config":)"s
-        + string_format(kTestJsonFormat, R"("test")") + R"(,"etag":"test-etag")"
-        + R"(,"fetch_time":)" + to_string(getUtcNowSecondsSinceEpoch() - 2 * pollIntervalSeconds) + "}");
+    auto jsonString = string_format(kTestJsonFormat, R"("test")");
+    auto mockCache = make_shared<SingleValueCache>(ConfigEntry(
+        Config::fromJson(jsonString),
+        "test-etag",
+        jsonString,
+        getUtcNowSecondsSinceEpoch() - 2 * pollIntervalSeconds).serialize()
+    );
 
     configcat::Response response = {200, string_format(kTestJsonFormat, R"("test2")")};
     constexpr int responseDelay = 5;

diff --git a/test/test-configcache.cpp b/test/test-configcache.cpp
@@ -0,0 +1,62 @@
+#include <gtest/gtest.h>
+#include "mock.h"
+#include "utils.h"
+#include "configservice.h"
+#include "configcat/configcatoptions.h"
+#include "configcat/configcatclient.h"
+
+
+using namespace configcat;
+using namespace std;
+
+TEST(ConfigCacheTest, CacheKey) {
+    EXPECT_EQ("147c5b4c2b2d7c77e1605b1a4309f0ea6684a0c6", ConfigService::generateCacheKey("test1"));
+    EXPECT_EQ("c09513b1756de9e4bc48815ec7a142b2441ed4d5", ConfigService::generateCacheKey("test2"));
+}
+
+TEST(ConfigCacheTest, CachePayload) {
+    double nowInSeconds = 1686756435.8449;
+    std::string etag = "test-etag";
+    ConfigEntry entry(Config::fromJson(kTestJsonString), etag, kTestJsonString, nowInSeconds);
+    EXPECT_EQ("1686756435844\n" + etag + "\n" + kTestJsonString, entry.serialize());
+}
+
+TEST(ConfigCatTest, InvalidCacheContent) {
+    static constexpr char kTestJsonFormat[] = R"({ "f": { "testKey": { "v": %s, "p": [], "r": [] } } })";
+    HookCallbacks hookCallbacks;
+    auto hooks = make_shared<Hooks>();
+    hooks->addOnError([&](const string& error) { hookCallbacks.onError(error); });
+    auto configJsonString = string_format(kTestJsonFormat, R"("test")");
+    auto configCache = make_shared<SingleValueCache>(ConfigEntry(
+        Config::fromJson(configJsonString),
+        "test-etag",
+        configJsonString,
+        getUtcNowSecondsSinceEpoch()).serialize()
+    );
+
+    ConfigCatOptions options;
+    options.pollingMode = PollingMode::manualPoll();
+    options.configCache = configCache;
+    options.hooks = hooks;
+    auto client = ConfigCatClient::get("test", &options);
+
+    EXPECT_EQ("test", client->getValue("testKey", "default"));
+    EXPECT_EQ(0, hookCallbacks.errorCallCount);
+
+    // Invalid fetch time in cache
+    configCache->value = "text\n"s + "test-etag\n" + string_format(kTestJsonFormat, R"("test2")");
+    EXPECT_EQ("test", client->getValue("testKey", "default"));
+    EXPECT_TRUE(hookCallbacks.error.find("Error occurred while reading the cache. Invalid fetch time: text") != std::string::npos);
+
+    // Number of values is fewer than expected
+    configCache->value = std::to_string(getUtcNowSecondsSinceEpoch()) + "\n" + string_format(kTestJsonFormat, R"("test2")");
+    EXPECT_EQ("test", client->getValue("testKey", "default"));
+    EXPECT_TRUE(hookCallbacks.error.find("Error occurred while reading the cache. Number of values is fewer than expected.") != std::string::npos);
+
+    // Invalid config JSON
+    configCache->value = std::to_string(getUtcNowSecondsSinceEpoch()) + "\n" + "test-etag\n" + "wrong-json";
+    EXPECT_EQ("test", client->getValue("testKey", "default"));
+    EXPECT_TRUE(hookCallbacks.error.find("Error occurred while reading the cache. Invalid config JSON: wrong-json.") != std::string::npos);
+
+    ConfigCatClient::close(client);
+}
diff --git a/test/test-configcatclient.cpp b/test/test-configcatclient.cpp
@@ -250,10 +250,11 @@ TEST_F(ConfigCatClientTest, FailingAutoPoll) {
 
 TEST_F(ConfigCatClientTest, FromCacheOnly) {
     auto mockCache = make_shared<InMemoryConfigCache>();
-    auto cacheKey = SHA1()(string("cpp_") + ConfigFetcher::kConfigJsonName + "_" + kTestSdkKey);
-    auto config = Config::fromJson(string_format(kTestJsonFormat, R"("fake")"));
-    auto configEntry = ConfigEntry(config);
-    mockCache->write(cacheKey, configEntry.toJson());
+    auto cacheKey = SHA1()(""s + kTestSdkKey + "_" + ConfigFetcher::kConfigJsonName + "_" + ConfigEntry::kSerializationFormatVersion);
+    auto jsonString = string_format(kTestJsonFormat, R"("fake")");
+    auto config = Config::fromJson(jsonString);
+    auto configEntry = ConfigEntry(config, "test-etag", jsonString, getUtcNowSecondsSinceEpoch());
+    mockCache->write(cacheKey, configEntry.serialize());
     mockHttpSessionAdapter->enqueueResponse({500, ""});
 
     ConfigCatOptions options;
@@ -268,10 +269,11 @@ TEST_F(ConfigCatClientTest, FromCacheOnly) {
 
 TEST_F(ConfigCatClientTest, FromCacheOnlyRefresh) {
     auto mockCache = make_shared<InMemoryConfigCache>();
-    auto cacheKey = SHA1()(string("cpp_") + ConfigFetcher::kConfigJsonName + "_" + kTestSdkKey);
-    auto config = Config::fromJson(string_format(kTestJsonFormat, R"("fake")"));
-    auto configEntry = ConfigEntry(config);
-    mockCache->write(cacheKey, configEntry.toJson());
+    auto cacheKey = SHA1()(""s + kTestSdkKey + "_" + ConfigFetcher::kConfigJsonName + "_" + ConfigEntry::kSerializationFormatVersion);
+    auto jsonString = string_format(kTestJsonFormat, R"("fake")");
+    auto config = Config::fromJson(jsonString);
+    auto configEntry = ConfigEntry(config, "test-etag", jsonString, getUtcNowSecondsSinceEpoch());
+    mockCache->write(cacheKey, configEntry.serialize());
     mockHttpSessionAdapter->enqueueResponse({500, ""});
 
     ConfigCatOptions options;

diff --git a/test/test-hooks.cpp b/test/test-hooks.cpp
@@ -5,8 +5,6 @@
 #include "configcat/configcatclient.h"
 #include "configcat/configcatoptions.h"
 #include "configcat/configcatlogger.h"
-#include "configcat/consolelogger.h"
-#include <thread>
 #include <chrono>
 
 using namespace configcat;
@@ -32,7 +30,11 @@ TEST_F(HooksTest, Init) {
 
     ConfigCatOptions options;
     options.pollingMode = PollingMode::manualPoll();
-    options.configCache = make_shared<SingleValueCache>(R"({"config":)"s + kTestJsonString + R"(,"etag":"test-etag"})");
+    options.configCache = make_shared<SingleValueCache>(ConfigEntry(
+        Config::fromJson(kTestJsonString),
+        "test-etag",
+        kTestJsonString).serialize()
+    );
     options.hooks = hooks;
     auto client = ConfigCatClient::get("test", &options);
 
@@ -70,7 +72,11 @@ TEST_F(HooksTest, Subscribe) {
 
     ConfigCatOptions options;
     options.pollingMode = PollingMode::manualPoll();
-    options.configCache = make_shared<SingleValueCache>(R"({"config":)"s + kTestJsonString + R"(,"etag":"test-etag"})");
+    options.configCache = make_shared<SingleValueCache>(ConfigEntry(
+        Config::fromJson(kTestJsonString),
+        "test-etag",
+        kTestJsonString).serialize()
+    );
     options.hooks = hooks;
     auto client = ConfigCatClient::get("test", &options);
 

diff --git a/test/test-lazyloading.cpp b/test/test-lazyloading.cpp
@@ -105,9 +105,13 @@ TEST_F(LazyLoadingTest, Cache) {
 }
 
 TEST_F(LazyLoadingTest, ReturnCachedConfigWhenCacheIsNotExpired) {
-    auto mockCache = make_shared<SingleValueCache>(R"({"config":)"s
-        + string_format(kTestJsonFormat, R"("test")") + R"(,"etag":"test-etag")"
-        + R"(,"fetch_time":)" + to_string(getUtcNowSecondsSinceEpoch()) + "}");
+    auto jsonString = string_format(kTestJsonFormat, R"("test")");
+    auto mockCache = make_shared<SingleValueCache>(ConfigEntry(
+        Config::fromJson(jsonString),
+        "test-etag",
+        jsonString,
+        getUtcNowSecondsSinceEpoch()).serialize()
+    );
 
     configcat::Response firstResponse = {200, string_format(kTestJsonFormat, R"("test2")")};
     mockHttpSessionAdapter->enqueueResponse(firstResponse);
@@ -132,9 +136,13 @@ TEST_F(LazyLoadingTest, ReturnCachedConfigWhenCacheIsNotExpired) {
 
 TEST_F(LazyLoadingTest, FetchConfigWhenCacheIsExpired) {
     auto cacheTimeToLiveSeconds = 1;
-    auto mockCache = make_shared<SingleValueCache>(R"({"config":)"s
-        + string_format(kTestJsonFormat, R"("test")") + R"(,"etag":"test-etag")"
-        + R"(,"fetch_time":)" + to_string(getUtcNowSecondsSinceEpoch() - cacheTimeToLiveSeconds) + "}");
+    auto jsonString = string_format(kTestJsonFormat, R"("test")");
+    auto mockCache = make_shared<SingleValueCache>(ConfigEntry(
+        Config::fromJson(jsonString),
+        "test-etag",
+        jsonString,
+        getUtcNowSecondsSinceEpoch() - cacheTimeToLiveSeconds).serialize()
+    );
 
     configcat::Response firstResponse = {200, string_format(kTestJsonFormat, R"("test2")")};
     mockHttpSessionAdapter->enqueueResponse(firstResponse);