diff --git a/.github/actions/spell-check/expect/expect.txt b/.github/actions/spell-check/expect/expect.txt index f1513698f..55298a2bd 100644 --- a/.github/actions/spell-check/expect/expect.txt +++ b/.github/actions/spell-check/expect/expect.txt @@ -2277,6 +2277,7 @@ tcome tcommandline tcommands tcon +TDelegated TDP TEAMPROJECT tearoff diff --git a/doc/cascadia/Json-Utility-API.md b/doc/cascadia/Json-Utility-API.md index 465f3239a..422a550bf 100644 --- a/doc/cascadia/Json-Utility-API.md +++ b/doc/cascadia/Json-Utility-API.md @@ -8,19 +8,20 @@ return a JSON value coerced into the specified type. When reading into existing storage, it returns a boolean indicating whether that storage was modified. If the JSON value cannot be converted to the specified type, an exception will be generated. +For non-nullable type conversions (most POD types), `null` is considered to be an invalid type. ```c++ std::string one; std::optional two; JsonUtils::GetValue(json, one); -// one is populated or unchanged. +// one is populated or an exception is thrown. JsonUtils::GetValue(json, two); -// two is populated, nullopt or unchanged +// two is populated, nullopt or an exception is thrown auto three = JsonUtils::GetValue(json); -// three is populated or zero-initialized +// three is populated or an exception is thrown auto four = JsonUtils::GetValue>(json); // four is populated or nullopt @@ -225,14 +226,14 @@ auto v = JsonUtils::GetValue(json, conv); -|json type invalid|json null|valid -|-|-|- -`T`|❌ exception|🔵 unchanged|✔ converted +`T`|❌ exception|❌ exception|✔ converted `std::optional`|❌ exception|🟨 `nullopt`|✔ converted ### GetValue<T>() (returning) -|json type invalid|json null|valid -|-|-|- -`T`|❌ exception|🟨 `T{}` (zero value)|✔ converted +`T`|❌ exception|❌ exception|✔ converted `std::optional`|❌ exception|🟨 `nullopt`|✔ converted ### GetValueForKey(T&) (type-deducing) @@ -242,14 +243,14 @@ a "key not found" state. The remaining three cases are the same. val type|key not found|_json type invalid_|_json null_|_valid_ -|-|-|-|- -`T`|🔵 unchanged|_❌ exception_|_🔵 unchanged_|_✔ converted_ -`std::optional`|_🔵 unchanged_|_❌ exception_|_🟨 `nullopt`_|_✔ converted_ +`T`|🔵 unchanged|_❌ exception_|_❌ exception_|_✔ converted_ +`std::optional`|🔵 unchanged|_❌ exception_|_🟨 `nullopt`_|_✔ converted_ ### GetValueForKey<T>() (return value) val type|key not found|_json type invalid_|_json null_|_valid_ -|-|-|-|- -`T`|🟨 `T{}` (zero value)|_❌ exception_|_🟨 `T{}` (zero value)_|_✔ converted_ +`T`|🟨 `T{}` (zero value)|_❌ exception_|_❌ exception_|_✔ converted_ `std::optional`|🟨 `nullopt`|_❌ exception_|_🟨 `nullopt`_|_✔ converted_ ### Future Direction diff --git a/src/cascadia/LocalTests_SettingsModel/CommandTests.cpp b/src/cascadia/LocalTests_SettingsModel/CommandTests.cpp index b50dd6059..b3a0951f7 100644 --- a/src/cascadia/LocalTests_SettingsModel/CommandTests.cpp +++ b/src/cascadia/LocalTests_SettingsModel/CommandTests.cpp @@ -144,7 +144,6 @@ namespace SettingsModelLocalTests // of from keybindings. const std::string commands0String{ R"([ - { "name": "command0", "command": { "action": "splitPane", "split": null } }, { "name": "command1", "command": { "action": "splitPane", "split": "vertical" } }, { "name": "command2", "command": { "action": "splitPane", "split": "horizontal" } }, { "name": "command4", "command": { "action": "splitPane" } }, @@ -157,18 +156,8 @@ namespace SettingsModelLocalTests VERIFY_ARE_EQUAL(0u, commands.Size()); auto warnings = implementation::Command::LayerJson(commands, commands0Json); VERIFY_ARE_EQUAL(0u, warnings.size()); - VERIFY_ARE_EQUAL(5u, commands.Size()); + VERIFY_ARE_EQUAL(4u, commands.Size()); - { - auto command = commands.Lookup(L"command0"); - VERIFY_IS_NOT_NULL(command); - VERIFY_IS_NOT_NULL(command.Action()); - VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.Action().Action()); - const auto& realArgs = command.Action().Args().try_as(); - VERIFY_IS_NOT_NULL(realArgs); - // Verify the args have the expected value - VERIFY_ARE_EQUAL(SplitState::Automatic, realArgs.SplitStyle()); - } { auto command = commands.Lookup(L"command1"); VERIFY_IS_NOT_NULL(command); diff --git a/src/cascadia/LocalTests_SettingsModel/KeyBindingsTests.cpp b/src/cascadia/LocalTests_SettingsModel/KeyBindingsTests.cpp index 78ac3ada2..6f74fed4d 100644 --- a/src/cascadia/LocalTests_SettingsModel/KeyBindingsTests.cpp +++ b/src/cascadia/LocalTests_SettingsModel/KeyBindingsTests.cpp @@ -322,7 +322,6 @@ namespace SettingsModelLocalTests void KeyBindingsTests::TestSplitPaneArgs() { const std::string bindings0String{ R"([ - { "keys": ["ctrl+c"], "command": { "action": "splitPane", "split": null } }, { "keys": ["ctrl+d"], "command": { "action": "splitPane", "split": "vertical" } }, { "keys": ["ctrl+e"], "command": { "action": "splitPane", "split": "horizontal" } }, { "keys": ["ctrl+g"], "command": { "action": "splitPane" } }, @@ -335,17 +334,8 @@ namespace SettingsModelLocalTests VERIFY_IS_NOT_NULL(keymap); VERIFY_ARE_EQUAL(0u, keymap->_keyShortcuts.size()); keymap->LayerJson(bindings0Json); - VERIFY_ARE_EQUAL(5u, keymap->_keyShortcuts.size()); + VERIFY_ARE_EQUAL(4u, keymap->_keyShortcuts.size()); - { - KeyChord kc{ true, false, false, static_cast('C') }; - auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); - VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); - const auto& realArgs = actionAndArgs.Args().try_as(); - VERIFY_IS_NOT_NULL(realArgs); - // Verify the args have the expected value - VERIFY_ARE_EQUAL(SplitState::Automatic, realArgs.SplitStyle()); - } { KeyChord kc{ true, false, false, static_cast('D') }; auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc); diff --git a/src/cascadia/TerminalSettingsModel/IInheritable.h b/src/cascadia/TerminalSettingsModel/IInheritable.h index c8c1ee1e4..539b21a54 100644 --- a/src/cascadia/TerminalSettingsModel/IInheritable.h +++ b/src/cascadia/TerminalSettingsModel/IInheritable.h @@ -72,11 +72,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // This is like std::optional, but we can use it in inheritance to determine whether the user explicitly cleared it template - struct NullableSetting - { - std::optional setting{ std::nullopt }; - bool set{ false }; - }; + using NullableSetting = std::optional>; } // Use this macro to quickly implement both getters and the setter for an @@ -93,7 +89,7 @@ public: \ bool Has##name() const \ { \ return _##name.has_value(); \ - }; \ + } \ \ /* Returns the resolved value for this setting */ \ /* fallback: user set value --> inherited value --> system set value */ \ @@ -101,19 +97,19 @@ public: \ { \ const auto val{ _get##name##Impl() }; \ return val ? *val : type{ __VA_ARGS__ }; \ - }; \ + } \ \ /* Overwrite the user set value */ \ void name(const type& value) \ { \ _##name = value; \ - }; \ + } \ \ /* Clear the user set value */ \ void Clear##name() \ { \ _##name = std::nullopt; \ - }; \ + } \ \ private: \ std::optional _##name{ std::nullopt }; \ @@ -137,7 +133,7 @@ private: \ \ /*no value was found*/ \ return std::nullopt; \ - }; + } // This macro is similar to the one above, but is reserved for optional settings // like Profile.Foreground (where null is interpreted @@ -148,51 +144,51 @@ public: \ /* Returns true if the user explicitly set the value, false otherwise*/ \ bool Has##name() const \ { \ - return _##name.set; \ - }; \ + return _##name.has_value(); \ + } \ \ /* Returns the resolved value for this setting */ \ /* fallback: user set value --> inherited value --> system set value */ \ winrt::Windows::Foundation::IReference name() const \ { \ const auto val{ _get##name##Impl() }; \ - if (val.set) \ + if (val) \ { \ - if (val.setting) \ + if (*val) \ { \ - return *val.setting; \ + return **val; \ } \ return nullptr; \ } \ return winrt::Windows::Foundation::IReference{ __VA_ARGS__ }; \ - }; \ + } \ \ /* Overwrite the user set value */ \ void name(const winrt::Windows::Foundation::IReference& value) \ { \ if (value) /*set value is different*/ \ { \ - _##name.setting = value.Value(); \ + _##name = std::optional{ value.Value() }; \ } \ else \ { \ - _##name.setting = std::nullopt; \ + /* note we're setting the _inner_ value */ \ + _##name = std::optional{ std::nullopt }; \ } \ - _##name.set = true; \ - }; \ + } \ \ /* Clear the user set value */ \ void Clear##name() \ { \ - _##name.set = false; \ - }; \ + _##name = std::nullopt; \ + } \ \ private: \ NullableSetting _##name{}; \ NullableSetting _get##name##Impl() const \ { \ /*return user set value*/ \ - if (Has##name()) \ + if (_##name) \ { \ return _##name; \ } \ @@ -201,12 +197,12 @@ private: \ /*iterate through parents to find a value*/ \ for (auto parent : _parents) \ { \ - auto val{ parent->_get##name##Impl() }; \ - if (val.set) \ + if (auto val{ parent->_get##name##Impl() }) \ { \ return val; \ } \ } \ + \ /*no value was found*/ \ - return { std::nullopt, false }; \ - }; + return std::nullopt; \ + } diff --git a/src/cascadia/TerminalSettingsModel/JsonUtils.h b/src/cascadia/TerminalSettingsModel/JsonUtils.h index 2ed3f1bde..13b77637a 100644 --- a/src/cascadia/TerminalSettingsModel/JsonUtils.h +++ b/src/cascadia/TerminalSettingsModel/JsonUtils.h @@ -60,36 +60,34 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils const std::string_view zeroCopyString{ begin, gsl::narrow_cast(end - begin) }; return zeroCopyString; } - - template - struct DeduceOptional - { - using Type = typename std::decay::type; - static constexpr bool IsOptional = false; - }; - - template - struct DeduceOptional> - { - using Type = typename std::decay::type; - static constexpr bool IsOptional = true; - }; - - template - struct DeduceOptional<::winrt::Windows::Foundation::IReference> - { - using Type = typename std::decay::type; - static constexpr bool IsOptional = true; - }; - - template<> - struct DeduceOptional<::winrt::hstring> - { - using Type = typename ::winrt::hstring; - static constexpr bool IsOptional = true; - }; } + template + struct OptionOracle + { + template // universal parameter + static constexpr bool HasValue(U&&) + { + return true; + } + }; + + template + struct OptionOracle> + { + static constexpr std::optional EmptyV() { return std::nullopt; } + static constexpr bool HasValue(const std::optional& o) { return o.has_value(); } + static constexpr auto&& Value(const std::optional& o) { return *o; } + }; + + template + struct OptionOracle<::winrt::Windows::Foundation::IReference> + { + static constexpr ::winrt::Windows::Foundation::IReference EmptyV() { return nullptr; } + static constexpr bool HasValue(const ::winrt::Windows::Foundation::IReference& o) { return static_cast(o); } + static constexpr auto&& Value(const ::winrt::Windows::Foundation::IReference& o) { return o.Value(); } + }; + class DeserializationError : public std::runtime_error { public: @@ -177,13 +175,27 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils // Leverage the wstring converter's validation winrt::hstring FromJson(const Json::Value& json) { + if (json.isNull()) + { + return winrt::hstring{}; + } return winrt::hstring{ til::u8u16(Detail::GetStringView(json)) }; } Json::Value ToJson(const winrt::hstring& val) { + if (val == winrt::hstring{}) + { + return Json::Value::nullSingleton(); + } return til::u16u8(val); } + + bool CanConvert(const Json::Value& json) + { + // hstring has a specific behavior for null, so it can convert it + return ConversionTrait::CanConvert(json) || json.isNull(); + } }; #endif @@ -419,6 +431,56 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils }; #endif + template::type>, typename TOpt = std::optional::type>> + struct OptionalConverter + { + using Oracle = OptionOracle; + TDelegatedConverter delegatedConverter{}; + + TOpt FromJson(const Json::Value& json) + { + if (!json && !delegatedConverter.CanConvert(json)) + { + // If the nested converter can't deal with null, emit an empty optional + // If it can, it probably has specific null behavior that it wants to use. + return Oracle::EmptyV(); + } + TOpt val{ delegatedConverter.FromJson(json) }; + return val; + } + + bool CanConvert(const Json::Value& json) + { + return json.isNull() || delegatedConverter.CanConvert(json); + } + + Json::Value ToJson(const TOpt& val) + { + if (!Oracle::HasValue(val)) + { + return Json::Value::nullSingleton(); + } + return delegatedConverter.ToJson(Oracle::Value(val)); + } + + std::string TypeDescription() const + { + return delegatedConverter.TypeDescription(); + } + }; + + template + struct ConversionTrait> : public OptionalConverter, std::optional> + { + }; + +#ifdef WINRT_Windows_Foundation_H + template + struct ConversionTrait<::winrt::Windows::Foundation::IReference> : public OptionalConverter, ::winrt::Windows::Foundation::IReference> + { + }; +#endif + template struct EnumMapper { @@ -586,31 +648,15 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils template bool GetValue(const Json::Value& json, T& target, Converter&& conv) { - if constexpr (Detail::DeduceOptional::IsOptional) + if (!conv.CanConvert(json)) { - // FOR OPTION TYPES - // - If the json object is set to `null`, then - // we'll instead set the target back to the empty optional. - if (json.isNull()) - { - target = T{}; // zero-construct an empty optional - return true; - } + DeserializationError e{ json }; + e.expectedType = conv.TypeDescription(); + throw e; } - if (json) - { - if (!conv.CanConvert(json)) - { - DeserializationError e{ json }; - e.expectedType = conv.TypeDescription(); - throw e; - } - - target = conv.FromJson(json); - return true; - } - return false; + target = conv.FromJson(json); + return true; } // GetValue, forced return type, manual converter @@ -654,7 +700,7 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils template bool GetValue(const Json::Value& json, T& target) { - return GetValue(json, target, ConversionTrait::Type>{}); + return GetValue(json, target, ConversionTrait::type>{}); } // GetValue, forced return type, with automatic converter @@ -662,7 +708,7 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils std::decay_t GetValue(const Json::Value& json) { std::decay_t local{}; - GetValue(json, local, ConversionTrait::Type>{}); + GetValue(json, local, ConversionTrait::type>{}); return local; // returns zero-initialized or value } @@ -670,14 +716,14 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils template bool GetValueForKey(const Json::Value& json, std::string_view key, T& target) { - return GetValueForKey(json, key, target, ConversionTrait::Type>{}); + return GetValueForKey(json, key, target, ConversionTrait::type>{}); } // GetValueForKey, forced return type, with automatic converter template std::decay_t GetValueForKey(const Json::Value& json, std::string_view key) { - return GetValueForKey(json, key, ConversionTrait::Type>{}); + return GetValueForKey(json, key, ConversionTrait::type>{}); } // Get multiple values for keys (json, k, &v, k, &v, k, &v, ...). @@ -696,15 +742,19 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils template void SetValueForKey(Json::Value& json, std::string_view key, const T& target, Converter&& conv) { - // demand guarantees that it will return a value or throw an exception - *json.demand(&*key.cbegin(), (&*key.cbegin()) + key.size()) = conv.ToJson(target); + // We don't want to write any empty optionals into JSON (right now). + if (OptionOracle::HasValue(target)) + { + // demand guarantees that it will return a value or throw an exception + *json.demand(&*key.cbegin(), (&*key.cbegin()) + key.size()) = conv.ToJson(target); + } } // SetValueForKey, type-deduced, with automatic converter template void SetValueForKey(Json::Value& json, std::string_view key, const T& target) { - SetValueForKey(json, key, target, ConversionTrait::Type>{}); + SetValueForKey(json, key, target, ConversionTrait::type>{}); } }; diff --git a/src/cascadia/TerminalSettingsModel/Profile.cpp b/src/cascadia/TerminalSettingsModel/Profile.cpp index a38d40adc..7017f15e2 100644 --- a/src/cascadia/TerminalSettingsModel/Profile.cpp +++ b/src/cascadia/TerminalSettingsModel/Profile.cpp @@ -293,10 +293,10 @@ void Profile::LayerJson(const Json::Value& json) JsonUtils::GetValueForKey(json, HiddenKey, _Hidden); // Core Settings - _Foreground.set = JsonUtils::GetValueForKey(json, ForegroundKey, _Foreground.setting); - _Background.set = JsonUtils::GetValueForKey(json, BackgroundKey, _Background.setting); - _SelectionBackground.set = JsonUtils::GetValueForKey(json, SelectionBackgroundKey, _SelectionBackground.setting); - _CursorColor.set = JsonUtils::GetValueForKey(json, CursorColorKey, _CursorColor.setting); + JsonUtils::GetValueForKey(json, ForegroundKey, _Foreground); + JsonUtils::GetValueForKey(json, BackgroundKey, _Background); + JsonUtils::GetValueForKey(json, SelectionBackgroundKey, _SelectionBackground); + JsonUtils::GetValueForKey(json, CursorColorKey, _CursorColor); JsonUtils::GetValueForKey(json, ColorSchemeKey, _ColorSchemeName); // TODO:MSFT:20642297 - Use a sentinel value (-1) for "Infinite scrollback" @@ -320,18 +320,11 @@ void Profile::LayerJson(const Json::Value& json) // Padding was never specified as an integer, but it was a common working mistake. // Allow it to be permissive. - JsonUtils::GetValueForKey(json, PaddingKey, _Padding, JsonUtils::PermissiveStringConverter{}); + JsonUtils::GetValueForKey(json, PaddingKey, _Padding, JsonUtils::OptionalConverter>{}); JsonUtils::GetValueForKey(json, ScrollbarStateKey, _ScrollState); - // StartingDirectory is "nullable". But we represent a null starting directory as the empty string - // When null is set in the JSON, we empty initialize startDir (empty string), and set StartingDirectory to that - // Without this, we're accidentally setting StartingDirectory to nullopt instead. - hstring startDir; - if (JsonUtils::GetValueForKey(json, StartingDirectoryKey, startDir)) - { - _StartingDirectory = startDir; - } + JsonUtils::GetValueForKey(json, StartingDirectoryKey, _StartingDirectory); JsonUtils::GetValueForKey(json, IconKey, _Icon); JsonUtils::GetValueForKey(json, BackgroundImageKey, _BackgroundImagePath); @@ -341,7 +334,7 @@ void Profile::LayerJson(const Json::Value& json) JsonUtils::GetValueForKey(json, RetroTerminalEffectKey, _RetroTerminalEffect); JsonUtils::GetValueForKey(json, AntialiasingModeKey, _AntialiasingMode); - _TabColor.set = JsonUtils::GetValueForKey(json, TabColorKey, _TabColor.setting); + JsonUtils::GetValueForKey(json, TabColorKey, _TabColor); JsonUtils::GetValueForKey(json, BellStyleKey, _BellStyle); } diff --git a/src/cascadia/ut_app/JsonUtilsTests.cpp b/src/cascadia/ut_app/JsonUtilsTests.cpp index efaac0da0..7dbaf9fd2 100644 --- a/src/cascadia/ut_app/JsonUtilsTests.cpp +++ b/src/cascadia/ut_app/JsonUtilsTests.cpp @@ -101,6 +101,38 @@ JSON_FLAG_MAPPER(JsonTestFlags) }; }; +struct hstring_like +{ + std::string value; +}; +template<> +struct ConversionTrait +{ + hstring_like FromJson(const Json::Value& json) + { + return { ConversionTrait{}.FromJson(json) }; + } + + bool CanConvert(const Json::Value& json) + { + return json.isNull() || ConversionTrait{}.CanConvert(json); + } + + Json::Value ToJson(const hstring_like& val) + { + if (val.value == "") + { + return Json::Value::nullSingleton(); + } + return val.value; + } + + std::string TypeDescription() const + { + return "string"; + } +}; + namespace TerminalAppUnitTests { class JsonUtilsTests @@ -119,6 +151,11 @@ namespace TerminalAppUnitTests TEST_METHOD(FlagMapper); TEST_METHOD(NestedExceptionDuringKeyParse); + + TEST_METHOD(SetValueHStringLike); + TEST_METHOD(GetValueHStringLike); + + TEST_METHOD(DoubleOptional); }; template @@ -136,9 +173,8 @@ namespace TerminalAppUnitTests //// 1.a. Type Invalid - Exception //// VERIFY_THROWS_SPECIFIC(GetValue(object), DeserializationError, _ReturnTrueForException); - //// 1.b. JSON NULL - Zero Value //// - std::string zeroValueString{}; - VERIFY_ARE_EQUAL(zeroValueString, GetValue(Json::Value::nullSingleton())); + //// 1.b. JSON NULL - Exception //// + VERIFY_THROWS_SPECIFIC(GetValue(Json::Value::nullSingleton()), DeserializationError, _ReturnTrueForException); //// 1.c. Valid - Valid //// VERIFY_ARE_EQUAL(expected, GetValue(object)); @@ -167,9 +203,8 @@ namespace TerminalAppUnitTests VERIFY_THROWS_SPECIFIC(GetValue(object, outputRedHerring), DeserializationError, _ReturnTrueForException); VERIFY_ARE_EQUAL(5, outputRedHerring); // unchanged - //// 1.b. JSON NULL - Unchanged //// - VERIFY_IS_FALSE(GetValue(Json::Value::nullSingleton(), output)); // FALSE = storage not modified! - VERIFY_ARE_EQUAL("sentinel", output); // unchanged + //// 1.b. JSON NULL - Exception //// + VERIFY_THROWS_SPECIFIC(GetValue(Json::Value::nullSingleton(), output), DeserializationError, _ReturnTrueForException); //// 1.c. Valid //// VERIFY_IS_TRUE(GetValue(object, output)); @@ -208,14 +243,14 @@ namespace TerminalAppUnitTests //// 1.a. Type Invalid - Exception //// VERIFY_THROWS_SPECIFIC(GetValueForKey(object, key), DeserializationError, _ReturnTrueForException); - //// 1.b. JSON NULL - Zero Value //// - std::string zeroValueString{}; - VERIFY_ARE_EQUAL(zeroValueString, GetValueForKey(object, nullKey)); + //// 1.b. JSON NULL - Exception //// + VERIFY_THROWS_SPECIFIC(GetValueForKey(object, nullKey), DeserializationError, _ReturnTrueForException); //// 1.c. Valid - Valid //// VERIFY_ARE_EQUAL(expected, GetValueForKey(object, key)); //// 1.d. Not Found - Zero Value //// + std::string zeroValueString{}; VERIFY_ARE_EQUAL(zeroValueString, GetValueForKey(object, invalidKey)); //// 2. Optional //// @@ -253,8 +288,7 @@ namespace TerminalAppUnitTests VERIFY_ARE_EQUAL(5, outputRedHerring); // unchanged //// 1.b. JSON NULL - Unchanged //// - VERIFY_IS_FALSE(GetValueForKey(object, nullKey, output)); // FALSE = storage not modified! - VERIFY_ARE_EQUAL("sentinel", output); // unchanged + VERIFY_THROWS_SPECIFIC(GetValueForKey(object, nullKey, output), DeserializationError, _ReturnTrueForException); //// 1.c. Valid //// VERIFY_IS_TRUE(GetValueForKey(object, key, output)); @@ -505,4 +539,89 @@ namespace TerminalAppUnitTests VERIFY_THROWS_SPECIFIC(GetValueForKey(object, key), DeserializationError, CheckKeyInException); } + void JsonUtilsTests::SetValueHStringLike() + { + // Terminal has a string type (hstring) where null/"" are the same, and + // we want to make sure that optionals of that type serialize "properly". + hstring_like first{ "" }; + hstring_like second{ "second" }; + std::optional third{ { "" } }; + std::optional fourth{ { "fourth" } }; + std::optional fifth{}; + + Json::Value object{ Json::objectValue }; + + SetValueForKey(object, "first", first); + SetValueForKey(object, "second", second); + SetValueForKey(object, "third", third); + SetValueForKey(object, "fourth", fourth); + SetValueForKey(object, "fifth", fifth); + + VERIFY_ARE_EQUAL(Json::Value::nullSingleton(), object["first"]); // real empty value serializes as null + VERIFY_ARE_EQUAL("second", object["second"].asString()); // serializes as a string + VERIFY_ARE_EQUAL(Json::Value::nullSingleton(), object["third"]); // optional populated with real empty value serializes as null + VERIFY_ARE_EQUAL("fourth", object["fourth"].asString()); // serializes as a string + VERIFY_IS_FALSE(object.isMember("fifth")); // does not serialize + } + + void JsonUtilsTests::GetValueHStringLike() + { + Json::Value object{ Json::objectValue }; + object["string"] = "string"; + object["null"] = Json::Value::nullSingleton(); + // object["nonexistent"] can't be set, clearly, to continue not existing + + hstring_like v; + VERIFY_IS_TRUE(GetValueForKey(object, "string", v)); + VERIFY_ARE_EQUAL("string", v.value); // deserializes as string + VERIFY_IS_TRUE(GetValueForKey(object, "null", v)); + VERIFY_ARE_EQUAL("", v.value); // deserializes as real value, but empty + VERIFY_IS_FALSE(GetValueForKey(object, "nonexistent", v)); // does not deserialize + + std::optional optionalV; + // deserializes as populated optional containing string + VERIFY_IS_TRUE(GetValueForKey(object, "string", optionalV)); + VERIFY_IS_TRUE(optionalV.has_value()); + VERIFY_ARE_EQUAL("string", optionalV->value); + + optionalV = std::nullopt; + // deserializes as populated optional containing real empty value + VERIFY_IS_TRUE(GetValueForKey(object, "null", optionalV)); + VERIFY_IS_TRUE(optionalV.has_value()); + VERIFY_ARE_EQUAL("", optionalV->value); + + optionalV = std::nullopt; + // does not deserialize; optional remains nullopt + VERIFY_IS_FALSE(GetValueForKey(object, "nonexistent", optionalV)); + VERIFY_ARE_EQUAL(std::nullopt, optionalV); + } + + void JsonUtilsTests::DoubleOptional() + { + const std::optional> first{ std::nullopt }; // no value + const std::optional> second{ std::optional{ std::nullopt } }; // outer has a value, inner is "no value" + const std::optional> third{ std::optional{ 3 } }; // outer has a value, inner is "no value" + + Json::Value object{ Json::objectValue }; + + SetValueForKey(object, "first", first); + SetValueForKey(object, "second", second); + SetValueForKey(object, "third", third); + + VERIFY_IS_FALSE(object.isMember("first")); + VERIFY_IS_TRUE(object.isMember("second")); + VERIFY_ARE_EQUAL(Json::Value::nullSingleton(), object["second"]); + VERIFY_ARE_EQUAL(Json::Value{ 3 }, object["third"]); + + std::optional> firstOut, secondOut, thirdOut; + VERIFY_IS_FALSE(GetValueForKey(object, "first", firstOut)); + VERIFY_IS_TRUE(GetValueForKey(object, "second", secondOut)); + VERIFY_IS_TRUE(static_cast(secondOut)); + VERIFY_ARE_EQUAL(std::nullopt, *secondOut); // should have come back out as null + + VERIFY_IS_TRUE(GetValueForKey(object, "third", thirdOut)); + VERIFY_IS_TRUE(static_cast(thirdOut)); + VERIFY_IS_TRUE(static_cast(*thirdOut)); + VERIFY_ARE_EQUAL(3, **thirdOut); + } }