kvs: Rename reset_key to set_value_to_default

score/kvs/docs/detailed_design/class_diagram.puml

@@ -40,7 +40,7 @@ package "kvs_builder" {
 package "kvs_api" {
     +interface KvsApi {
         +reset()
-        +reset_key(key : string)
+        +set_value_to_default(key : string)
         +get_all_keys() : list<string>
         +key_exists(key : string) : bool
         +get_value(key : string) : KvsValue
@@ -66,7 +66,7 @@ package "kvs" {
         +parameters() : KvsParameters
 
         +reset()
-        +reset_key(key : string)
+        +set_value_to_default(key : string)
         +get_all_keys() : list<string>
         +key_exists(key : string) : bool
         +get_value(key : string) : KvsValue

score/kvs/docs/detailed_design/class_diagram_cpp.puml

@@ -47,7 +47,7 @@ interface IKvsBackend <<interface>> {
 
 interface IKvs <<interface>> {
     + Reset() : Result<void>
-    + ResetKey(key : string) : Result<void>
+    + SetValueToDefault(key : string) : Result<void>
     + GetAllKeys() : Result<vector<string>>
     + KeyExists(key : string) : Result<bool>
     + GetValue(key : string) : Result<KvsValue>
@@ -104,7 +104,7 @@ class JsonBackend <<implementation>> {
 
 class KvsMock  <<mock>> {
     + Reset() : Result<void>
-    + ResetKey(key : string) : Result<void>
+    + SetValueToDefault(key : string) : Result<void>
     + GetAllKeys() : Result<vector<string>>
     + KeyExists(key : string) : Result<bool>
     + GetValue(key : string) : Result<KvsValue>
@@ -128,7 +128,7 @@ class Kvs <<implementation>> {
     - backend : IKvsBackend
     __
     + Reset() : Result<void>
-    + ResetKey(key : string) : Result<void>
+    + SetValueToDefault(key : string) : Result<void>
     + GetAllKeys() : Result<vector<string>>
     + KeyExists(key : string) : Result<bool>
     + GetValue(key : string) : Result<KvsValue>

score/kvs/error.cpp

@@ -55,8 +55,6 @@ std::string_view KvsErrorDomain::MessageFor(const score::result::ErrorCode& code
             return "Authentication failed";
         case ErrorCode::kKeyNotFound:
             return "Key not found";
-        case ErrorCode::kKeyDefaultNotFound:
-            return "Key default value not found";
         case ErrorCode::kSerializationFailed:
             return "Serialization failed";
         case ErrorCode::kInvalidSnapshotId:

score/kvs/error.h

@@ -68,9 +68,6 @@ enum class ErrorCode : score::result::ErrorCode
     /// Key not found.
     kKeyNotFound = 14,
 
-    /// Key default value not found.
-    kKeyDefaultNotFound = 15,
-
     /// Serialization failed.
     kSerializationFailed = 16,
 
@@ -121,8 +118,6 @@ enum class ErrorCode : score::result::ErrorCode
         [[deprecated("Use kAuthenticationFailed instead. This name is removed in a future release.")]] =
             kAuthenticationFailed,
     KeyNotFound [[deprecated("Use kKeyNotFound instead. This name is removed in a future release.")]] = kKeyNotFound,
-    KeyDefaultNotFound [[deprecated("Use kKeyDefaultNotFound instead. This name is removed in a future release.")]] =
-        kKeyDefaultNotFound,
     SerializationFailed [[deprecated("Use kSerializationFailed instead. This name is removed in a future release.")]] =
         kSerializationFailed,
     InvalidSnapshotId [[deprecated("Use kInvalidSnapshotId instead. This name is removed in a future release.")]] =

score/kvs/kvs.cpp

@@ -261,21 +261,30 @@ score::Result<KvsValue> Kvs::GetDefaultValue(const std::string_view key) const n
     return search->second;
 }
 
-score::Result<void> Kvs::ResetKey(const std::string_view key) noexcept
+score::Result<void> Kvs::SetValueToDefault(const std::string_view key) noexcept
 {
-    std::unique_lock<std::mutex> lock_kvs{kvs_mutex_, std::try_to_lock};
-    if (!lock_kvs.owns_lock())
+    // Validate key.
+    if (!IsKeyValid(key))
     {
-        return score::MakeUnexpected(ErrorCode::kMutexLockFailed);
+        logger_->LogError() << "Provided key is not valid" << key;
+        return score::MakeUnexpected(ErrorCode::kInvalidKey);
     }
 
-    if (default_values_.find(std::string(key)) == default_values_.end())
+    // Get a default value.
+    auto default_value_result{GetDefaultValue(key)};
+    if (!default_value_result.has_value())
     {
-        return score::MakeUnexpected(ErrorCode::kKeyDefaultNotFound);
+        return score::MakeUnexpected(static_cast<ErrorCode>(*default_value_result.error()));
+    }
+
+    // Set value.
+    std::unique_lock<std::mutex> lock{kvs_mutex_, std::try_to_lock};
+    if (!lock.owns_lock())
+    {
+        return score::MakeUnexpected(ErrorCode::kMutexLockFailed);
     }
 
-    // Return value ignored: a key that was never written needs no reset.
-    score::cpp::ignore = kvs_.erase(std::string(key));
+    kvs_.insert_or_assign(std::string(key), std::move(*default_value_result));
     return {};
 }
 

score/kvs/kvs.h

@@ -229,15 +229,13 @@ class Kvs final
     /// @return The default value on success, or an ErrorCode on failure.
     score::Result<KvsValue> GetDefaultValue(const std::string_view key) const noexcept;
 
-    /// @brief Resets a specified key to its default value.
+    /// @brief Sets a specified key to its default value.
     ///
-    /// If no default value is available, it returns an ErrorCode and doesn't delete the key.
-    /// If no key was ever written, but a default value is available, it returns successfully.
-    /// If a key was written and it has a default value, it deletes the written key.
+    /// If no default value is available, it returns an ErrorCode.
     ///
-    /// @param key The key to be reset to its default value.
+    /// @param key The key to be set to its default value.
     /// @return Nothing on success, or an ErrorCode on failure.
-    score::Result<void> ResetKey(const std::string_view key) noexcept;
+    score::Result<void> SetValueToDefault(const std::string_view key) noexcept;
 
     /// @brief Checks if the specified key wasn't set yet and uses its default value.
     /// @param key The key to check.
@@ -381,13 +379,6 @@ class Kvs final
         return GetDefaultValue(key);
     }
 
-    /// @deprecated Use `ResetKey()` instead.
-    [[deprecated("Use ResetKey() instead. This name is removed in a future release.")]]
-    score::Result<void> reset_key(const std::string_view key) noexcept
-    {
-        return ResetKey(key);
-    }
-
     /// @deprecated Use `IsValueDefault()` instead.
     [[deprecated("Use IsValueDefault() instead. This name is removed in a future release.")]]
     score::Result<bool> is_value_default(const std::string_view key) const noexcept

score/kvs/kvs_deprecated_api_test.cpp

@@ -46,7 +46,6 @@ static_assert(static_cast<score::result::ErrorCode>(ErrorCode::kOutOfStorageSpac
 static_assert(static_cast<score::result::ErrorCode>(ErrorCode::kQuotaExceeded) == 12, "");
 static_assert(static_cast<score::result::ErrorCode>(ErrorCode::kAuthenticationFailed) == 13, "");
 static_assert(static_cast<score::result::ErrorCode>(ErrorCode::kKeyNotFound) == 14, "");
-static_assert(static_cast<score::result::ErrorCode>(ErrorCode::kKeyDefaultNotFound) == 15, "");
 static_assert(static_cast<score::result::ErrorCode>(ErrorCode::kSerializationFailed) == 16, "");
 static_assert(static_cast<score::result::ErrorCode>(ErrorCode::kInvalidSnapshotId) == 17, "");
 static_assert(static_cast<score::result::ErrorCode>(ErrorCode::kConversionFailed) == 18, "");
@@ -97,7 +96,6 @@ static_assert(ErrorCode::OutOfStorageSpace == ErrorCode::kOutOfStorageSpace, "")
 static_assert(ErrorCode::QuotaExceeded == ErrorCode::kQuotaExceeded, "");
 static_assert(ErrorCode::AuthenticationFailed == ErrorCode::kAuthenticationFailed, "");
 static_assert(ErrorCode::KeyNotFound == ErrorCode::kKeyNotFound, "");
-static_assert(ErrorCode::KeyDefaultNotFound == ErrorCode::kKeyDefaultNotFound, "");
 static_assert(ErrorCode::SerializationFailed == ErrorCode::kSerializationFailed, "");
 static_assert(ErrorCode::InvalidSnapshotId == ErrorCode::kInvalidSnapshotId, "");
 static_assert(ErrorCode::ConversionFailed == ErrorCode::kConversionFailed, "");
@@ -227,11 +225,6 @@ TEST(kvs_deprecated_api, kvs_write_wrappers)
     EXPECT_EQ(std::get<int32_t>(kvs.GetValue("old_key").value().GetValue()), 1);
     EXPECT_EQ(std::get<int32_t>(kvs.GetValue("new_key").value().GetValue()), 1);
 
-    ASSERT_TRUE(kvs.SetValue("default", KvsValue(9)));
-    ASSERT_FALSE(kvs.IsValueDefault("default").value());
-    ASSERT_TRUE(kvs.reset_key("default"));
-    EXPECT_TRUE(kvs.IsValueDefault("default").value());
-
     ASSERT_TRUE(kvs.remove_key("old_key"));
     EXPECT_FALSE(kvs.KeyExists("old_key").value());
     EXPECT_TRUE(kvs.KeyExists("new_key").value());

score/kvs/kvs_error_test.cpp

@@ -40,7 +40,6 @@ TEST(kvs_MessageFor, MessageFor)
         {ErrorCode::kQuotaExceeded, "Quota exceeded"},
         {ErrorCode::kAuthenticationFailed, "Authentication failed"},
         {ErrorCode::kKeyNotFound, "Key not found"},
-        {ErrorCode::kKeyDefaultNotFound, "Key default value not found"},
         {ErrorCode::kSerializationFailed, "Serialization failed"},
         {ErrorCode::kInvalidSnapshotId, "Invalid snapshot ID"},
         {ErrorCode::kConversionFailed, "Conversion failed"},

score/kvs/kvs_test.cpp

@@ -840,7 +840,7 @@ TEST(kvs_get_default_value, get_default_value_failure)
     CleanupEnvironment();
 }
 
-TEST(kvs_reset_key, reset_key_success)
+TEST(kvs_set_value_to_default, set_value_to_default_success)
 {
     PrepareEnvironment();
     seed_defaults(R"({"default": {"t": "i32", "v": 5}, "kvs": {"t": "f64", "v": 42.0}})");
@@ -857,25 +857,35 @@ TEST(kvs_reset_key, reset_key_success)
     auto exists_result{result.value().KeyExists("kvs")};
     ASSERT_TRUE(exists_result);
     ASSERT_TRUE(exists_result.value());
-    // Reset a key.
-    auto reset_key_result{result.value().ResetKey("kvs")};
-    EXPECT_TRUE(reset_key_result);
+    // Set a written key to its default value.
+    auto set_value_to_default_result{result.value().SetValueToDefault("kvs")};
+    EXPECT_TRUE(set_value_to_default_result);
     exists_result = result.value().KeyExists("kvs");
     ASSERT_TRUE(exists_result);
-    EXPECT_FALSE(exists_result.value());
-    auto is_default_result{result.value().IsValueDefault("kvs")};
-    ASSERT_TRUE(is_default_result);
-    EXPECT_TRUE(is_default_result.value());
-    EXPECT_TRUE(result.value().GetDefaultValue("kvs"));
+    EXPECT_TRUE(exists_result.value());
+    auto value_result{result.value().GetValue("kvs")};
+    ASSERT_TRUE(value_result);
+    ASSERT_EQ(value_result.value().GetType(), KvsValue::Type::kF64);
+    EXPECT_EQ(std::get<double>(value_result.value().GetValue()), 42.0);
 
-    // Reset a non written key which has default value.
-    reset_key_result = result.value().ResetKey("default");
-    EXPECT_TRUE(reset_key_result);
+    // Set a non written key which has default value.
+    exists_result = result.value().KeyExists("default");
+    ASSERT_TRUE(exists_result);
+    ASSERT_FALSE(exists_result.value());
+    set_value_to_default_result = result.value().SetValueToDefault("default");
+    EXPECT_TRUE(set_value_to_default_result);
+    exists_result = result.value().KeyExists("default");
+    ASSERT_TRUE(exists_result);
+    EXPECT_TRUE(exists_result.value());
+    value_result = result.value().GetValue("default");
+    ASSERT_TRUE(value_result);
+    ASSERT_EQ(value_result.value().GetType(), KvsValue::Type::kI32);
+    EXPECT_EQ(std::get<int32_t>(value_result.value().GetValue()), 5);
 
     CleanupEnvironment();
 }
 
-TEST(kvs_reset_key, reset_key_failure)
+TEST(kvs_set_value_to_default, set_value_to_default_failure)
 {
     PrepareEnvironment();
 
@@ -888,12 +898,12 @@ TEST(kvs_reset_key, reset_key_failure)
                           kDefaultMaxSnapshots)};
     ASSERT_TRUE(result);
 
-    // Reset a non-existing key.
-    auto reset_key_result{result.value().ResetKey("non_existing_key")};
-    EXPECT_FALSE(reset_key_result);
-    EXPECT_EQ(reset_key_result.error(), ErrorCode::kKeyDefaultNotFound);
+    // Set a non-existing key to default.
+    auto set_value_to_default_result{result.value().SetValueToDefault("non_existing_key")};
+    EXPECT_FALSE(set_value_to_default_result);
+    EXPECT_EQ(set_value_to_default_result.error(), ErrorCode::kKeyNotFound);
 
-    // Reset a key without default value.
+    // Set a key without default value.
     result = Kvs::Open(kInstanceId,
                        kSnapshotId,
                        KvsDefaults::kRequired,
@@ -906,9 +916,9 @@ TEST(kvs_reset_key, reset_key_failure)
     auto get_default_result{result.value().GetDefaultValue("kvs")};
     ASSERT_FALSE(get_default_result);
     EXPECT_EQ(get_default_result.error(), ErrorCode::kKeyNotFound);
-    reset_key_result = result.value().ResetKey("kvs");
-    EXPECT_FALSE(reset_key_result);
-    EXPECT_EQ(reset_key_result.error(), ErrorCode::kKeyDefaultNotFound);
+    set_value_to_default_result = result.value().SetValueToDefault("kvs");
+    EXPECT_FALSE(set_value_to_default_result);
+    EXPECT_EQ(set_value_to_default_result.error(), ErrorCode::kKeyNotFound);
 
     // Mutex locked.
     result = Kvs::Open(kInstanceId,
@@ -920,9 +930,42 @@ TEST(kvs_reset_key, reset_key_failure)
                        kDefaultMaxSnapshots);
     ASSERT_TRUE(result);
     std::unique_lock<std::mutex> lock{result.value().kvs_mutex_};
-    reset_key_result = result.value().ResetKey("kvs");
-    EXPECT_FALSE(reset_key_result);
-    EXPECT_EQ(static_cast<ErrorCode>(*reset_key_result.error()), ErrorCode::kMutexLockFailed);
+    set_value_to_default_result = result.value().SetValueToDefault("default");
+    EXPECT_FALSE(set_value_to_default_result);
+    EXPECT_EQ(static_cast<ErrorCode>(*set_value_to_default_result.error()), ErrorCode::kMutexLockFailed);
+
+    CleanupEnvironment();
+}
+
+TEST(kvs_set_value_to_default, set_value_to_default_failure_invalid_key)
+{
+    PrepareEnvironment();
+    auto result{Kvs::Open(kInstanceId,
+                          kSnapshotId,
+                          KvsDefaults::kRequired,
+                          KvsLoad::kRequired,
+                          std::string(kDataDir),
+                          std::nullopt,
+                          kDefaultMaxSnapshots)};
+    ASSERT_TRUE(result);
+
+    // Keys are validated before the default value lookup.
+    // None of these keys has a default value, so a lookup first would return kKeyNotFound.
+    std::vector<std::string> invalid_keys{InvalidCharacterKeys()};
+    invalid_keys.emplace_back("");
+    invalid_keys.emplace_back(kMaxKeyLengthBytes + 1U, 'k');
+    const auto keys_before{result.value().GetAllKeys()};
+    ASSERT_TRUE(keys_before);
+    for (const auto& key : invalid_keys)
+    {
+        auto set_value_to_default_result{result.value().SetValueToDefault(key)};
+        ASSERT_FALSE(set_value_to_default_result) << "key: " << key;
+        EXPECT_EQ(static_cast<ErrorCode>(*set_value_to_default_result.error()), ErrorCode::kInvalidKey)
+            << "key: " << key;
+    }
+    const auto keys_after{result.value().GetAllKeys()};
+    ASSERT_TRUE(keys_after);
+    EXPECT_EQ(keys_after.value(), keys_before.value());
 
     CleanupEnvironment();
 }

score/kvs/rust_kvs/error_code.rs

@@ -64,9 +64,6 @@ pub enum ErrorCode {
     /// Key not found
     KeyNotFound,
 
-    // Key has no default value
-    KeyDefaultNotFound,
-
     /// Serialization failed
     SerializationFailed(String),
 

score/kvs/rust_kvs/examples/defaults.rs

@@ -12,7 +12,7 @@
 // *******************************************************************************
 //! Example for default values.
 //! - Creating KVS instance using `KvsBuilder` with `defaults` modes.
-//! - Default-specific APIs: `get_default_value`, `is_value_default`.
+//! - Default-specific APIs: `get_default_value`, `is_value_default`, `set_value_to_default`.
 //! - Key-value operations behavior on defaults available.
 
 use rust_kvs::prelude::*;
@@ -109,7 +109,7 @@ fn main() -> Result<(), ErrorCode> {
         println!("All KVS keys: {all_keys:?}");
 
         let k3_key = "k3";
-        let k3_value = kvs.get_value(k3_key)?;
+        let k3_value = kvs.get_value(k3_key);
         println!("{k3_key:?} = {k3_value:?}");
 
         if !kvs.key_exists(k3_key)? {
@@ -120,15 +120,17 @@ fn main() -> Result<(), ErrorCode> {
     }
 
     {
-        println!("-> `reset_key` usage");
+        println!("-> `set_value_to_default` usage");
 
         let k1_key = "k1";
-        kvs.reset_key(k1_key)?;
-        if !kvs.key_exists(k1_key)? {
-            println!("{k1_key} key not found");
+        kvs.set_value_to_default(k1_key)?;
+        let k1_value = kvs.get_value(k1_key)?;
+        println!("{k1_key:?} = {k1_value:?}");
+        if kvs.key_exists(k1_key)? {
+            println!("{k1_key} key exists");
         }
-        if kvs.is_value_default(k1_key)? {
-            println!("{k1_key} default available");
+        if !kvs.is_value_default(k1_key)? {
+            println!("{k1_key} is not default");
         }
 
         println!();