From c66891c19c5cc7786ea3f45ada678bf15deda9e2 Mon Sep 17 00:00:00 2001 From: "Justin R. Wilson" Date: Thu, 18 Apr 2024 10:25:28 -0500 Subject: [PATCH] `Service_Participant::type_object_encoding` returns incorrect value Problem ------- `Service_Participant::type_object_encoding` doesn't return the value that is stored in the ConfigStore. Solution -------- Finish the support for enums in the ConfigStore and use it. --- dds/DCPS/ConfigStoreImpl.h | 63 +++++++++++++++++-- dds/DCPS/Service_Participant.cpp | 46 ++++---------- docs/news.d/type-object-encoding.rst | 5 ++ tests/unit-tests/dds/DCPS/ConfigStoreImpl.cpp | 23 ++++++- .../dds/DCPS/Service_Participant.cpp | 15 +++++ 5 files changed, 111 insertions(+), 41 deletions(-) create mode 100644 docs/news.d/type-object-encoding.rst create mode 100644 tests/unit-tests/dds/DCPS/Service_Participant.cpp diff --git a/dds/DCPS/ConfigStoreImpl.h b/dds/DCPS/ConfigStoreImpl.h index 948a4ad71fb..29ddb7948aa 100644 --- a/dds/DCPS/ConfigStoreImpl.h +++ b/dds/DCPS/ConfigStoreImpl.h @@ -172,7 +172,6 @@ class OpenDDS_Dcps_Export ConfigStoreImpl : public ConfigStore { const EnumList decoder[]) { bool found = false; - // Encode the default. String value_as_string; for (size_t idx = 0; decoder[idx].name; ++idx) { if (decoder[idx].value == value) { @@ -184,8 +183,8 @@ class OpenDDS_Dcps_Export ConfigStoreImpl : public ConfigStore { if (!found && log_level >= LogLevel::Warning) { ACE_ERROR((LM_WARNING, - ACE_TEXT("(%P|%t) WARNING: ConfigStoreImpl::get: ") - ACE_TEXT("failed to convert default value to string\n"))); + "(%P|%t) WARNING: ConfigStoreImpl::get: " + "failed to convert default value to string\n")); } const String actual = get(key, value_as_string); @@ -197,14 +196,66 @@ class OpenDDS_Dcps_Export ConfigStoreImpl : public ConfigStore { if (log_level >= LogLevel::Warning) { ACE_ERROR((LM_WARNING, - ACE_TEXT("(%P|%t) WARNING: ConfigStoreImpl::get: ") - ACE_TEXT("failed to encode (%C) to enumerated value, defaulting to (%C)\n"), - actual.c_str(), value_as_string.c_str())); + "(%P|%t) WARNING: ConfigStoreImpl::get: " + "for %C, failed to encode (%C) to enumerated value, defaulting to (%C)\n", + key, actual.c_str(), value_as_string.c_str())); } return value; } + template + void set(const char* key, + T value, + const EnumList decoder[]) + { + bool found = false; + String value_as_string; + for (size_t idx = 0; decoder[idx].name; ++idx) { + if (decoder[idx].value == value) { + value_as_string = decoder[idx].name; + found = true; + break; + } + } + + if (!found) { + if (log_level >= LogLevel::Warning) { + ACE_ERROR((LM_WARNING, + "(%P|%t) WARNING: ConfigStoreImpl::set: " + "for %C, failed to convert enum value to string\n", + key)); + } + return; + } + + set(key, value_as_string); + } + + template + void set(const char* key, + const String& value, + const EnumList decoder[]) + { + bool found = false; + // Sanity check. + String value_as_string; + for (size_t idx = 0; decoder[idx].name; ++idx) { + if (value == decoder[idx].name) { + set(key, value); + found = true; + break; + } + } + + if (!found && log_level >= LogLevel::Warning) { + ACE_ERROR((LM_WARNING, + "(%P|%t) WARNING: ConfigStoreImpl::set: " + "for %C, %C is not a valid enum value\n", + key, value.c_str())); + } + } + void set(const char* key, const TimeDuration& value, TimeFormat format); diff --git a/dds/DCPS/Service_Participant.cpp b/dds/DCPS/Service_Participant.cpp index a28d80eca26..ccd3cb5bdc4 100644 --- a/dds/DCPS/Service_Participant.cpp +++ b/dds/DCPS/Service_Participant.cpp @@ -2283,51 +2283,31 @@ Service_Participant::get_type_object(DDS::DomainParticipant_ptr participant, return XTypes::TypeObject(); } +namespace { + const EnumList type_object_encoding_kinds[] = + { + { Service_Participant::Encoding_Normal, "Normal" }, + { Service_Participant::Encoding_WriteOldFormat, "WriteOldFormat" }, + { Service_Participant::Encoding_ReadOldFormat, "ReadOldFormat" }, + { Service_Participant::Encoding_Normal, 0 } + }; +} + Service_Participant::TypeObjectEncoding Service_Participant::type_object_encoding() const { - String encoding_str = "Normal"; - config_store_->get(COMMON_DCPS_TYPE_OBJECT_ENCODING, encoding_str); - - struct NameValue { - const char* name; - TypeObjectEncoding value; - }; - static const NameValue entries[] = { - {"Normal", Encoding_Normal}, - {"WriteOldFormat", Encoding_WriteOldFormat}, - {"ReadOldFormat", Encoding_ReadOldFormat}, - }; - for (size_t i = 0; i < sizeof entries / sizeof entries[0]; ++i) { - if (0 == std::strcmp(entries[i].name, encoding_str.c_str())) { - return entries[i].value; - } - } - ACE_ERROR((LM_ERROR, "(%P|%t) ERROR: Service_Participant::type_object_encoding: " - "invalid encoding %C\n", encoding_str.c_str())); - - return Encoding_Normal; + return config_store_->get(COMMON_DCPS_TYPE_OBJECT_ENCODING, Encoding_Normal, type_object_encoding_kinds); } void Service_Participant::type_object_encoding(TypeObjectEncoding encoding) { - switch (encoding) { - case Encoding_Normal: - config_store_->set_string(COMMON_DCPS_TYPE_OBJECT_ENCODING, "Normal"); - break; - case Encoding_WriteOldFormat: - config_store_->set_string(COMMON_DCPS_TYPE_OBJECT_ENCODING, "WriteOldFormat"); - break; - case Encoding_ReadOldFormat: - config_store_->set_string(COMMON_DCPS_TYPE_OBJECT_ENCODING, "ReadOldFormat"); - break; - } + config_store_->set(COMMON_DCPS_TYPE_OBJECT_ENCODING, encoding, type_object_encoding_kinds); } void Service_Participant::type_object_encoding(const char* encoding) { - config_store_->set_string(COMMON_DCPS_TYPE_OBJECT_ENCODING, encoding); + config_store_->set(COMMON_DCPS_TYPE_OBJECT_ENCODING, encoding, type_object_encoding_kinds); } unsigned int diff --git a/docs/news.d/type-object-encoding.rst b/docs/news.d/type-object-encoding.rst new file mode 100644 index 00000000000..72311540e3e --- /dev/null +++ b/docs/news.d/type-object-encoding.rst @@ -0,0 +1,5 @@ +.. news-prs: 4593 + +.. news-start-section: Fixes +- Fix bug where ``Service_Participant::type_object_encoding`` doesn't return configured value. +.. news-end-section diff --git a/tests/unit-tests/dds/DCPS/ConfigStoreImpl.cpp b/tests/unit-tests/dds/DCPS/ConfigStoreImpl.cpp index f4bd82f7b56..d066c8410b6 100644 --- a/tests/unit-tests/dds/DCPS/ConfigStoreImpl.cpp +++ b/tests/unit-tests/dds/DCPS/ConfigStoreImpl.cpp @@ -242,7 +242,8 @@ TEST(dds_DCPS_ConfigStoreImpl, set_get_StringList) enum MyConfigStoreEnum { ALPHA, BETA, - GAMMA + GAMMA, + DELTA }; TEST(dds_DCPS_ConfigStoreImpl, set_get_Enum) @@ -257,8 +258,26 @@ TEST(dds_DCPS_ConfigStoreImpl, set_get_Enum) ConfigTopic_rch topic = make_rch(); ConfigStoreImpl store(topic); + // Get the default if there is no entry. EXPECT_EQ(store.get("key", GAMMA, kinds), GAMMA); - store.set("key", "beta"); + + // Default not in helper works. + EXPECT_EQ(store.get("key", DELTA, kinds), DELTA); + + // Setting with enum works. + store.set("key", ALPHA, kinds); + EXPECT_EQ(store.get("key", GAMMA, kinds), ALPHA); + + // Setting with string works. + store.set("key", "beta", kinds); + EXPECT_EQ(store.get("key", GAMMA, kinds), BETA); + + // Setting with enum that is not in helper does nothing. + store.set("key", DELTA, kinds); + EXPECT_EQ(store.get("key", GAMMA, kinds), BETA); + + // Setting with enum that is not in helper does nothing. + store.set("key", "delta", kinds); EXPECT_EQ(store.get("key", GAMMA, kinds), BETA); } diff --git a/tests/unit-tests/dds/DCPS/Service_Participant.cpp b/tests/unit-tests/dds/DCPS/Service_Participant.cpp new file mode 100644 index 00000000000..1a9739b63cc --- /dev/null +++ b/tests/unit-tests/dds/DCPS/Service_Participant.cpp @@ -0,0 +1,15 @@ +#include + +#include + +using namespace OpenDDS::DCPS; + +TEST(dds_DCPS_Service_Participant, type_object_encoding) { + Service_Participant sp; + + EXPECT_EQ(sp.type_object_encoding(), Service_Participant::Encoding_Normal); + sp.type_object_encoding(Service_Participant::Encoding_WriteOldFormat); + EXPECT_EQ(sp.type_object_encoding(), Service_Participant::Encoding_WriteOldFormat); + sp.type_object_encoding("ReadOldFormat"); + EXPECT_EQ(sp.type_object_encoding(), Service_Participant::Encoding_ReadOldFormat); +}