Merge pull request #10194 from akallabeth/sdl_pref_class

Sdl pref class

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()

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;

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());

client/SDL/sdl_prefs.cpp

@@ -35,29 +35,23 @@ namespace fs = std::experimental::filesystem;
 #include <freerdp/version.h>
 #include <winpr/json.h>
 
-#if defined(WITH_WINPR_JSON)
+SdlPref::WINPR_JSONPtr SdlPref::get()
-using WINPR_JSONPtr = std::unique_ptr<WINPR_JSON, decltype(&WINPR_JSON_Delete)>;
-
-static WINPR_JSONPtr get()
 {
-	auto config = sdl_get_pref_file();
+	auto config = get_pref_file();
 
 	std::ifstream ifs(config);
 	std::string content((std::istreambuf_iterator<char>(ifs)), (std::istreambuf_iterator<char>()));
 	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)
-	if (!config)
-		config = get();
-	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<int64_t>(val);
-#else
-	return fallback;
-#endif
 }
 
-std::vector<std::string> sdl_get_pref_array(const std::string& key,
+std::vector<std::string> SdlPref::get_array(const std::string& key,
                                             const std::vector<std::string>& 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<std::string> 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<char, decltype(&free)>;
 	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> SdlPref::instance(const std::string& name)
+{
+	static std::shared_ptr<SdlPref> _instance;
+	if (!_instance || (_instance->get_pref_file() != name))
+		_instance.reset(new SdlPref(name));
+	return _instance;
+}
+
+std::string SdlPref::get_pref_file()
+{
+	return _name;
+}

client/SDL/sdl_prefs.hpp

@@ -21,12 +21,34 @@
 
 #include <string>
 #include <vector>
+#include <memory>
+#include <winpr/json.h>
 
-std::string sdl_get_pref_dir();
+class SdlPref
-std::string sdl_get_pref_file();
+{
+  public:
+	static std::shared_ptr<SdlPref> instance(const std::string& name = SdlPref::get_default_file());
 
-std::string sdl_get_pref_string(const std::string& key, const std::string& fallback = "");
+	std::string get_pref_file();
-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::string get_string(const std::string& key, const std::string& fallback = "");
-std::vector<std::string> sdl_get_pref_array(const std::string& key,
+	int64_t get_int(const std::string& key, int64_t fallback = 0);
-                                            const std::vector<std::string>& fallback = {});
+	bool get_bool(const std::string& key, bool fallback = false);
+	std::vector<std::string> get_array(const std::string& key,
+	                                   const std::vector<std::string>& fallback = {});
+
+  private:
+	using WINPR_JSONPtr = std::unique_ptr<WINPR_JSON, decltype(&WINPR_JSON_Delete)>;
+
+	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 = "");
+};

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")

client/SDL/test/TestSDLPrefs.cpp

@@ -0,0 +1,121 @@
+#include "../sdl_prefs.hpp"
+
+#include <iostream>
+#include <fstream>
+
+#include <winpr/config.h>
+#include <winpr/winpr.h>
+
+#if __has_include(<filesystem>)
+#include <filesystem>
+namespace fs = std::filesystem;
+#elif __has_include(<experimental/filesystem>)
+#include <experimental/filesystem>
+namespace fs = std::experimental::filesystem;
+#else
+#error Could not find system header "<filesystem>" or "<experimental/filesystem>"
+#endif
+
+static std::shared_ptr<SdlPref> 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;
+}

client/SDL/test/sdl-freerdp.json

@@ -0,0 +1,10 @@
+{
+	"string_key": "abc",
+	"int_key": 123,
+	"bool_key": true,
+	"array_key": [
+		"a",
+		"b",
+		"c"
+	]
+}

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)

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,