From f31da67a3c66243794c93ff764e5afdeefa8674b Mon Sep 17 00:00:00 2001 From: Uwe Seimet Date: Thu, 1 Feb 2024 18:46:43 +0100 Subject: [PATCH] Add remaining properties, update unit tests --- cpp/base/property_handler.cpp | 10 ++--- cpp/base/property_handler.h | 2 +- cpp/command/command_dispatcher.cpp | 2 + cpp/command/command_executor.cpp | 63 +++++++++++++++++------------- cpp/command/command_executor.h | 2 + cpp/test/property_handler_test.cpp | 6 +++ cpp/test/s2pctl_display_test.cpp | 2 +- 7 files changed, 52 insertions(+), 35 deletions(-) diff --git a/cpp/base/property_handler.cpp b/cpp/base/property_handler.cpp index 1970df12..c903b23e 100644 --- a/cpp/base/property_handler.cpp +++ b/cpp/base/property_handler.cpp @@ -29,16 +29,16 @@ void PropertyHandler::Init(const string &filenames, const property_map &cmd_prop // Always parse the optional global property file if (exists(path(GLOBAL_CONFIGURATION))) { - ParsePropertyFile(GLOBAL_CONFIGURATION, true); + ParsePropertyFile(properties, GLOBAL_CONFIGURATION, true); } // When there is no explicit property file list parse the local property file if (filenames.empty()) { - ParsePropertyFile(GetHomeDir() + "/" + LOCAL_CONFIGURATION, true); + ParsePropertyFile(properties, GetHomeDir() + "/" + LOCAL_CONFIGURATION, true); } else { for (const auto &filename : Split(filenames, ',')) { - ParsePropertyFile(filename, false); + ParsePropertyFile(properties, filename, false); } } @@ -59,7 +59,7 @@ void PropertyHandler::Init(const string &filenames, const property_map &cmd_prop } } -void PropertyHandler::ParsePropertyFile(const string &filename, bool default_file) +void PropertyHandler::ParsePropertyFile(property_map &properties, const string &filename, bool default_file) { ifstream property_file(filename); if (property_file.fail() && !default_file) { @@ -79,7 +79,7 @@ void PropertyHandler::ParsePropertyFile(const string &filename, bool default_fil throw parser_exception(fmt::format("Invalid property '{}'", property)); } - property_cache[kv[0]] = kv[1]; + properties[kv[0]] = kv[1]; } } } diff --git a/cpp/base/property_handler.h b/cpp/base/property_handler.h index 6e0f9f15..cebf35d6 100644 --- a/cpp/base/property_handler.h +++ b/cpp/base/property_handler.h @@ -40,7 +40,7 @@ class PropertyHandler void Init(const string&, const property_map&); - void ParsePropertyFile(const string&, bool); + void ParsePropertyFile(property_map&, const string&, bool); property_map GetProperties(const string& = "") const; string GetProperty(string_view) const; void AddProperty(const string &key, const string &value) diff --git a/cpp/command/command_dispatcher.cpp b/cpp/command/command_dispatcher.cpp index 6a1adcf9..9e37f91a 100644 --- a/cpp/command/command_dispatcher.cpp +++ b/cpp/command/command_dispatcher.cpp @@ -37,6 +37,7 @@ bool CommandDispatcher::DispatchCommand(const CommandContext &context, PbResult return context.ReturnLocalizedError(LocalizationKey::ERROR_LOG_LEVEL, log_level); } else { + PropertyHandler::Instance().AddProperty("log_level", log_level); return context.ReturnSuccessStatus(); } @@ -46,6 +47,7 @@ bool CommandDispatcher::DispatchCommand(const CommandContext &context, PbResult return false; } else { + PropertyHandler::Instance().AddProperty("image_folder", GetParam(command, "folder")); return context.WriteSuccessResult(result); } diff --git a/cpp/command/command_executor.cpp b/cpp/command/command_executor.cpp index 005b0d4d..cd460724 100644 --- a/cpp/command/command_executor.cpp +++ b/cpp/command/command_executor.cpp @@ -92,8 +92,10 @@ bool CommandExecutor::ProcessCmd(const CommandContext &context) if (const string error = SetReservedIds(GetParam(command, "ids")); !error.empty()) { return context.ReturnErrorStatus(error); } - - return context.ReturnSuccessStatus(); + else { + PropertyHandler::Instance().AddProperty("reserved_ids", Join(reserved_ids, ",")); + return context.ReturnSuccessStatus(); + } } case CHECK_AUTHENTICATION: @@ -275,32 +277,7 @@ bool CommandExecutor::Attach(const CommandContext &context, const PbDeviceDefini } #endif - const string &identifier = fmt::format("device.{0}:{1}.", device->GetId(), device->GetLun()); - PropertyHandler::Instance().AddProperty(identifier + "type", device->GetTypeString()); - PropertyHandler::Instance().AddProperty(identifier + "product", - device->GetVendor() + ":" + device->GetProduct() + ":" + device->GetRevision()); - vector p; - const auto disk = dynamic_pointer_cast(device); - if (disk) { - if (disk->GetConfiguredSectorSize()) { - PropertyHandler::Instance().AddProperty(identifier + "block_size", - to_string(disk->GetConfiguredSectorSize())); - } - } - if (disk && !disk->GetFilename().empty()) { - string filename = disk->GetFilename(); - if (filename.starts_with(context.GetDefaultFolder())) { - filename = filename.substr(context.GetDefaultFolder().length() + 1); - } - PropertyHandler::Instance().AddProperty(identifier + "params", filename); - } - else if (!device->GetParams().empty()) { - vector p; - for (const auto& [param, value] : device->GetParams()) { - p.emplace_back(param + "=" + value); - } - PropertyHandler::Instance().AddProperty(identifier + "params", Join(p, ":")); - } + SetUpDeviceProperties(context, device); string msg = "Attached "; if (device->IsReadOnly()) { @@ -412,6 +389,36 @@ void CommandExecutor::DetachAll() const } } +void CommandExecutor::SetUpDeviceProperties(const CommandContext &context, shared_ptr device) +{ + const string &identifier = fmt::format("device.{0}:{1}.", device->GetId(), device->GetLun()); + PropertyHandler::Instance().AddProperty(identifier + "type", device->GetTypeString()); + PropertyHandler::Instance().AddProperty(identifier + "product", + device->GetVendor() + ":" + device->GetProduct() + ":" + device->GetRevision()); + vector p; + const auto disk = dynamic_pointer_cast(device); + if (disk) { + if (disk->GetConfiguredSectorSize()) { + PropertyHandler::Instance().AddProperty(identifier + "block_size", + to_string(disk->GetConfiguredSectorSize())); + } + } + if (disk && !disk->GetFilename().empty()) { + string filename = disk->GetFilename(); + if (filename.starts_with(context.GetDefaultFolder())) { + filename = filename.substr(context.GetDefaultFolder().length() + 1); + } + PropertyHandler::Instance().AddProperty(identifier + "params", filename); + } + else if (!device->GetParams().empty()) { + vector p; + for (const auto& [param, value] : device->GetParams()) { + p.emplace_back(param + "=" + value); + } + PropertyHandler::Instance().AddProperty(identifier + "params", Join(p, ":")); + } +} + string CommandExecutor::SetReservedIds(string_view ids) { set ids_to_reserve; diff --git a/cpp/command/command_executor.h b/cpp/command/command_executor.h index b1372588..b7bcdf2a 100644 --- a/cpp/command/command_executor.h +++ b/cpp/command/command_executor.h @@ -56,6 +56,8 @@ class CommandExecutor static bool ValidateIdAndLun(const CommandContext&, int, int); static bool SetProductData(const CommandContext&, const PbDeviceDefinition&, PrimaryDevice&); + void SetUpDeviceProperties(const CommandContext&, shared_ptr); + mutex& GetExecutionLocker() { return execution_locker; diff --git a/cpp/test/property_handler_test.cpp b/cpp/test/property_handler_test.cpp index fe2893e7..39d5daf9 100644 --- a/cpp/test/property_handler_test.cpp +++ b/cpp/test/property_handler_test.cpp @@ -44,6 +44,7 @@ TEST(PropertyHandlerTest, Init) const string &properties1 = R"(key1=value1 key2=value2 +device.3.params=params3 )"; const string &properties2 = R"(key3=value3 @@ -58,10 +59,15 @@ key2=value2 property_map cmd_properties; cmd_properties["key1"] = "value2"; + cmd_properties["device.1.params"] = "params1"; + cmd_properties["device.2:1.params"] = "params2"; property_handler = SetUpProperties(properties1, properties2, cmd_properties); EXPECT_EQ("value2", property_handler.GetProperty("key1")); EXPECT_EQ("value2", property_handler.GetProperty("key2")); EXPECT_EQ("value3", property_handler.GetProperty("key3")); + EXPECT_EQ("params1", property_handler.GetProperty("device.1:0.params")); + EXPECT_EQ("params2", property_handler.GetProperty("device.2:1.params")); + EXPECT_EQ("params3", property_handler.GetProperty("device.3:0.params")); EXPECT_THROW(SetUpProperties(properties3), parser_exception); } diff --git a/cpp/test/s2pctl_display_test.cpp b/cpp/test/s2pctl_display_test.cpp index d69e31a8..680ae8a5 100644 --- a/cpp/test/s2pctl_display_test.cpp +++ b/cpp/test/s2pctl_display_test.cpp @@ -325,7 +325,7 @@ TEST(S2pCtlDisplayTest, DisplayPropertiesInfo) const string s = display.DisplayPropertiesInfo(info); EXPECT_FALSE(s.empty()); - EXPECT_NE(string::npos, s.find("settings")); + EXPECT_NE(string::npos, s.find("s2p properties")); } TEST(S2pCtlDisplayTest, DisplayOperationInfo)