From 632f500f3ef133419fb43e2e87c72fd35d713091 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 16 May 2024 15:35:33 +0200 Subject: [PATCH 1/4] [client,sdl] refactor SdlPref * Wrap in class * Allow overriding default configuration file for tests --- client/SDL/sdl_freerdp.cpp | 2 +- client/SDL/sdl_kbd.cpp | 4 +-- client/SDL/sdl_prefs.cpp | 66 +++++++++++++++++--------------------- client/SDL/sdl_prefs.hpp | 36 +++++++++++++++++---- 4 files changed, 62 insertions(+), 46 deletions(-) diff --git a/client/SDL/sdl_freerdp.cpp b/client/SDL/sdl_freerdp.cpp index 7bb9e6364..d4cdf27f1 100644 --- a/client/SDL/sdl_freerdp.cpp +++ b/client/SDL/sdl_freerdp.cpp @@ -1561,7 +1561,7 @@ static void print_config_file_help() std::cout << " The SDL client supports some user defined configuration options." << std::endl; std::cout << " Settings are stored in JSON format" << std::endl; std::cout << " The location is a per user file. Location for current user is " - << sdl_get_pref_file() << std::endl; + << SdlPref::instance()->get_pref_file() << std::endl; std::cout << " The XDG_CONFIG_HOME environment variable can be used to override the base directory." << std::endl; diff --git a/client/SDL/sdl_kbd.cpp b/client/SDL/sdl_kbd.cpp index f45b75855..cd285b968 100644 --- a/client/SDL/sdl_kbd.cpp +++ b/client/SDL/sdl_kbd.cpp @@ -402,7 +402,7 @@ uint32_t sdlInput::prefToMask() { "KMOD_GUI", KMOD_GUI } }; uint32_t mod = KMOD_NONE; - for (const auto& val : sdl_get_pref_array("SDL_KeyModMask", { "KMOD_RSHIFT" })) + for (const auto& val : SdlPref::instance()->get_array("SDL_KeyModMask", { "KMOD_RSHIFT" })) { auto it = mapping.find(val); if (it != mapping.end()) @@ -480,7 +480,7 @@ static UINT32 sdl_scancode_to_rdp(Uint32 scancode) uint32_t sdlInput::prefKeyValue(const std::string& key, uint32_t fallback) { - auto item = sdl_get_pref_string(key); + auto item = SdlPref::instance()->get_string(key); if (item.empty()) return fallback; auto val = sdl_scancode_val(item.c_str()); diff --git a/client/SDL/sdl_prefs.cpp b/client/SDL/sdl_prefs.cpp index e7e696b6a..f73393906 100644 --- a/client/SDL/sdl_prefs.cpp +++ b/client/SDL/sdl_prefs.cpp @@ -35,29 +35,23 @@ namespace fs = std::experimental::filesystem; #include #include -#if defined(WITH_WINPR_JSON) -using WINPR_JSONPtr = std::unique_ptr; - -static WINPR_JSONPtr get() +SdlPref::WINPR_JSONPtr SdlPref::get() { - auto config = sdl_get_pref_file(); + auto config = get_pref_file(); std::ifstream ifs(config); std::string content((std::istreambuf_iterator(ifs)), (std::istreambuf_iterator())); return { WINPR_JSON_ParseWithLength(content.c_str(), content.size()), WINPR_JSON_Delete }; } -static WINPR_JSON* get_item(const std::string& key) +WINPR_JSON* SdlPref::get_item(const std::string& key) { - static WINPR_JSONPtr config{ nullptr, WINPR_JSON_Delete }; - if (!config) - config = get(); - if (!config) + if (!_config) return nullptr; - return WINPR_JSON_GetObjectItem(config.get(), key.c_str()); + return WINPR_JSON_GetObjectItem(_config.get(), key.c_str()); } -static std::string item_to_str(WINPR_JSON* item, const std::string& fallback = "") +std::string SdlPref::item_to_str(WINPR_JSON* item, const std::string& fallback) { if (!item || !WINPR_JSON_IsString(item)) return fallback; @@ -66,47 +60,33 @@ static std::string item_to_str(WINPR_JSON* item, const std::string& fallback = " return {}; return str; } -#endif -std::string sdl_get_pref_string(const std::string& key, const std::string& fallback) +std::string SdlPref::get_string(const std::string& key, const std::string& fallback) { -#if defined(WITH_WINPR_JSON) auto item = get_item(key); return item_to_str(item, fallback); -#else - return fallback; -#endif } -bool sdl_get_pref_bool(const std::string& key, bool fallback) +bool SdlPref::get_bool(const std::string& key, bool fallback) { -#if defined(WITH_WINPR_JSON) auto item = get_item(key); if (!item || !WINPR_JSON_IsBool(item)) return fallback; return WINPR_JSON_IsTrue(item); -#else - return fallback; -#endif } -int64_t sdl_get_pref_int(const std::string& key, int64_t fallback) +int64_t SdlPref::get_int(const std::string& key, int64_t fallback) { -#if defined(WITH_WINPR_JSON) auto item = get_item(key); if (!item || !WINPR_JSON_IsNumber(item)) return fallback; auto val = WINPR_JSON_GetNumberValue(item); return static_cast(val); -#else - return fallback; -#endif } -std::vector sdl_get_pref_array(const std::string& key, +std::vector SdlPref::get_array(const std::string& key, const std::vector& fallback) { -#if defined(WITH_WINPR_JSON) auto item = get_item(key); if (!item || !WINPR_JSON_IsArray(item)) return fallback; @@ -119,12 +99,13 @@ std::vector sdl_get_pref_array(const std::string& key, } return values; -#else - return fallback; -#endif } -std::string sdl_get_pref_dir() +SdlPref::SdlPref(const std::string& file) : _name(file), _config(get()) +{ +} + +std::string SdlPref::get_pref_dir() { using CStringPtr = std::unique_ptr; CStringPtr path(GetKnownPath(KNOWN_PATH_XDG_CONFIG_HOME), free); @@ -137,9 +118,22 @@ std::string sdl_get_pref_dir() return config.string(); } -std::string sdl_get_pref_file() +std::string SdlPref::get_default_file() { - fs::path config{ sdl_get_pref_dir() }; + fs::path config{ SdlPref::get_pref_dir() }; config /= "sdl-freerdp.json"; return config.string(); } + +std::shared_ptr SdlPref::instance(const std::string& name) +{ + static std::shared_ptr _instance; + if (!_instance || (_instance->get_pref_file() != name)) + _instance.reset(new SdlPref(name)); + return _instance; +} + +std::string SdlPref::get_pref_file() +{ + return _name; +} diff --git a/client/SDL/sdl_prefs.hpp b/client/SDL/sdl_prefs.hpp index d7924f28c..225d5a1f7 100644 --- a/client/SDL/sdl_prefs.hpp +++ b/client/SDL/sdl_prefs.hpp @@ -21,12 +21,34 @@ #include #include +#include +#include -std::string sdl_get_pref_dir(); -std::string sdl_get_pref_file(); +class SdlPref +{ + public: + static std::shared_ptr instance(const std::string& name = SdlPref::get_default_file()); -std::string sdl_get_pref_string(const std::string& key, const std::string& fallback = ""); -int64_t sdl_get_pref_int(const std::string& key, int64_t fallback = 0); -bool sdl_get_pref_bool(const std::string& key, bool fallback = false); -std::vector sdl_get_pref_array(const std::string& key, - const std::vector& fallback = {}); \ No newline at end of file + std::string get_pref_file(); + + std::string get_string(const std::string& key, const std::string& fallback = ""); + int64_t get_int(const std::string& key, int64_t fallback = 0); + bool get_bool(const std::string& key, bool fallback = false); + std::vector get_array(const std::string& key, + const std::vector& fallback = {}); + + private: + using WINPR_JSONPtr = std::unique_ptr; + + std::string _name; + WINPR_JSONPtr _config; + + SdlPref(const std::string& file); + + WINPR_JSON* get_item(const std::string& key); + WINPR_JSONPtr get(); + + static std::string get_pref_dir(); + static std::string get_default_file(); + static std::string item_to_str(WINPR_JSON* item, const std::string& fallback = ""); +}; From 3f97fb59ad11e24aee78dc26d83eaf0a214b1a05 Mon Sep 17 00:00:00 2001 From: Ondrej Holy Date: Mon, 29 Apr 2024 16:01:12 +0200 Subject: [PATCH 2/4] [client,sdl] add test case for SDL prefs Let's add test case for SDL prefs functionality. This is to test the JSON wrapper to ensure it doesn't break something. --- client/SDL/CMakeLists.txt | 4 + client/SDL/test/CMakeLists.txt | 36 +++++++++ client/SDL/test/TestSDLPrefs.cpp | 121 +++++++++++++++++++++++++++++++ client/SDL/test/sdl-freerdp.json | 10 +++ 4 files changed, 171 insertions(+) create mode 100644 client/SDL/test/CMakeLists.txt create mode 100644 client/SDL/test/TestSDLPrefs.cpp create mode 100644 client/SDL/test/sdl-freerdp.json diff --git a/client/SDL/CMakeLists.txt b/client/SDL/CMakeLists.txt index 526655b57..c17ccd73e 100644 --- a/client/SDL/CMakeLists.txt +++ b/client/SDL/CMakeLists.txt @@ -119,3 +119,7 @@ set_property(TARGET ${PROJECT_NAME} PROPERTY FOLDER "Client/SDL") install(TARGETS ${PROJECT_NAME} DESTINATION ${CMAKE_INSTALL_BINDIR} COMPONENT client) add_subdirectory(man) + +if(BUILD_TESTING) + add_subdirectory(test) +endif() diff --git a/client/SDL/test/CMakeLists.txt b/client/SDL/test/CMakeLists.txt new file mode 100644 index 000000000..cc336833f --- /dev/null +++ b/client/SDL/test/CMakeLists.txt @@ -0,0 +1,36 @@ +set(MODULE_NAME "TestSDL") +set(MODULE_PREFIX "TEST_SDL") + +set(${MODULE_PREFIX}_DRIVER ${MODULE_NAME}.cpp) + +set(${MODULE_PREFIX}_TESTS TestSDLPrefs.cpp) + +create_test_sourcelist(${MODULE_PREFIX}_SRCS + ${${MODULE_PREFIX}_DRIVER} + ${${MODULE_PREFIX}_TESTS}) + +add_executable(${MODULE_NAME} ${${MODULE_PREFIX}_SRCS}) + +set(${MODULE_PREFIX}_LIBS freerdp winpr sdl_prefs) + +target_link_libraries(${MODULE_NAME} ${${MODULE_PREFIX}_LIBS}) + +set_target_properties(${MODULE_NAME} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${TESTING_OUTPUT_DIRECTORY}") + +set(TEST_SRC_AREA "${CMAKE_CURRENT_SOURCE_DIR}") +set(TEST_BIN_AREA "${CMAKE_CURRENT_BINARY_DIR}") + +if (WIN32) + string(REPLACE "\\" "\\\\" TEST_SRC_AREA "${TEST_SRC_AREA}") + string(REPLACE "\\" "\\\\" TEST_BIN_AREA "${TEST_BIN_AREA}") +endif() + +add_compile_definitions(TEST_SRC_AREA="${TEST_SRC_AREA}") +add_compile_definitions(TEST_BIN_AREA="${TEST_BIN_AREA}") + +foreach(test ${${MODULE_PREFIX}_TESTS}) + get_filename_component(TestName ${test} NAME_WE) + add_test(${TestName} ${TESTING_OUTPUT_DIRECTORY}/${MODULE_NAME} ${TestName}) +endforeach() + +set_property(TARGET ${MODULE_NAME} PROPERTY FOLDER "FreeRDP/Client/Test") diff --git a/client/SDL/test/TestSDLPrefs.cpp b/client/SDL/test/TestSDLPrefs.cpp new file mode 100644 index 000000000..db4a75302 --- /dev/null +++ b/client/SDL/test/TestSDLPrefs.cpp @@ -0,0 +1,121 @@ +#include "../sdl_prefs.hpp" + +#include +#include + +#include +#include + +#if __has_include() +#include +namespace fs = std::filesystem; +#elif __has_include() +#include +namespace fs = std::experimental::filesystem; +#else +#error Could not find system header "" or "" +#endif + +static std::shared_ptr instance() +{ +#if !defined(TEST_SRC_AREA) +#error "Please define TEST_SRC_AREA to the source directory of this test case" +#endif + fs::path src(TEST_SRC_AREA); + src /= "sdl-freerdp.json"; + if (!fs::exists(src)) + { + std::cerr << "[ERROR] test configuration file '" << src << "' does not exist" << std::endl; + return nullptr; + } + + return SdlPref::instance(src.string()); +} + +int TestSDLPrefs(int argc, char* argv[]) +{ + WINPR_UNUSED(argc); + WINPR_UNUSED(argv); + +#if defined(WITH_WINPR_JSON) + printf("implementation: json\n"); +#else + printf("implementation: fallback\n"); +#endif + +#if defined(WITH_WINPR_JSON) + printf("config: %s\n", instance()->get_pref_file().c_str()); +#endif + + auto string_value = instance()->get_string("string_key", "cba"); +#if defined(WITH_WINPR_JSON) + if (string_value != "abc") + return -1; +#else + if (string_value != "cba") + return -1; +#endif + + auto string_value_nonexistent = instance()->get_string("string_key_nonexistent", "cba"); + if (string_value_nonexistent != "cba") + return -1; + + auto int_value = instance()->get_int("int_key", 321); +#if defined(WITH_WINPR_JSON) + if (int_value != 123) + return -1; +#else + if (int_value != 321) + return -1; +#endif + + auto int_value_nonexistent = instance()->get_int("int_key_nonexistent", 321); + if (int_value_nonexistent != 321) + return -1; + + auto bool_value = instance()->get_bool("bool_key", false); +#if defined(WITH_WINPR_JSON) + if (!bool_value) + return -1; +#else + if (bool_value) + return -1; +#endif + + auto bool_value_nonexistent = instance()->get_bool("bool_key_nonexistent", false); + if (bool_value_nonexistent) + return -1; + + auto array_value = instance()->get_array("array_key", { "c", "b", "a" }); + if (array_value.size() != 3) + return -1; +#if defined(WITH_WINPR_JSON) + if (array_value[0] != "a") + return -1; + if (array_value[1] != "b") + return -1; + if (array_value[2] != "c") + return -1; +#else + if (array_value[0] != "c") + return -1; + if (array_value[1] != "b") + return -1; + if (array_value[2] != "a") + return -1; +#endif + + auto array_value_nonexistent = + instance()->get_array("array_key_nonexistent", { "c", "b", "a" }); + if (array_value_nonexistent.size() != 3) + return -1; + + if (array_value_nonexistent[0] != "c") + return -1; + if (array_value_nonexistent[1] != "b") + return -1; + if (array_value_nonexistent[2] != "a") + return -1; + + return 0; +} diff --git a/client/SDL/test/sdl-freerdp.json b/client/SDL/test/sdl-freerdp.json new file mode 100644 index 000000000..17facb32e --- /dev/null +++ b/client/SDL/test/sdl-freerdp.json @@ -0,0 +1,10 @@ +{ + "string_key": "abc", + "int_key": 123, + "bool_key": true, + "array_key": [ + "a", + "b", + "c" + ] +} \ No newline at end of file From ac69cf346e8b0ae8ae5845a64a84507f5187af25 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 16 May 2024 16:48:21 +0200 Subject: [PATCH 3/4] [cmake] add missing include include(CMakeDependentOption) before use. --- cmake/JsonDetect.cmake | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmake/JsonDetect.cmake b/cmake/JsonDetect.cmake index 1066421f1..994e3039d 100644 --- a/cmake/JsonDetect.cmake +++ b/cmake/JsonDetect.cmake @@ -1,3 +1,5 @@ +include(CMakeDependentOption) + option(WITH_JSON_DISABLED "Build without any JSON support" OFF) CMAKE_DEPENDENT_OPTION(WITH_CJSON_REQUIRED "Build with cJSON (fail if not found)" OFF "NOT WITH_JSON_DISABLED" OFF) CMAKE_DEPENDENT_OPTION(WITH_JSONC_REQUIRED "Build with JSON-C (fail if not found)" OFF "NOT WITH_JSON_DISABLED" OFF) From e5bb2f98f8fc7cc2fa1c0538b0e0707c7fae0d52 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 16 May 2024 17:01:07 +0200 Subject: [PATCH 4/4] [ci,nightly] add json-c alternative to debian --- packaging/deb/freerdp-nightly/control | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packaging/deb/freerdp-nightly/control b/packaging/deb/freerdp-nightly/control index f31abe0ea..0aad18bcc 100644 --- a/packaging/deb/freerdp-nightly/control +++ b/packaging/deb/freerdp-nightly/control @@ -44,7 +44,7 @@ Build-Depends: libpam0g-dev, uuid-dev, libxml2-dev, - libcjson-dev, + libjson-c-dev | libcjson-dev, libsdl2-2.0-0, libsdl2-dev, libsdl2-ttf-dev,