Issue: 4556521 Unified Configuration: missing fields

- Exsposed the config registry for use by other parts of the code
- Refactored the display of non-default configurations to scan the
  config registry

Signed-off-by: Oren Shemesh <[email protected]>

Author: orshemesh-grok

contrib/jenkins_tests/gtest.sh

@@ -72,20 +72,20 @@ fi
 eval "${sudo_cmd} pkill -9 ${prj_service} 2>/dev/null || true"
 eval "${sudo_cmd} ${install_dir}/sbin/${prj_service} --console -v5 &"
 
-# Test with full coverage of the new config
-eval "${sudo_cmd} $timeout_exe env XLIO_USE_NEW_CONFIG=1 XLIO_CONFIG_FILE=${WORKSPACE}/tests/gtest/xlio_config_full_coverage.json GTEST_TAP=2 LD_PRELOAD=$gtest_lib $gtest_app $gtest_opt --gtest_filter=-xlio_*:-ultra* --gtest_output=xml:${WORKSPACE}/${prefix}/test-basic.xml"
+# Test with full coverage of the config-file feature
+eval "${sudo_cmd} $timeout_exe env WORKSPACE=${WORKSPACE} XLIO_USE_NEW_CONFIG=1 XLIO_CONFIG_FILE=${WORKSPACE}/tests/gtest/xlio_config_full_coverage.json GTEST_TAP=2 LD_PRELOAD=$gtest_lib $gtest_app $gtest_opt --gtest_filter=-xlio_*:-ultra* --gtest_output=xml:${WORKSPACE}/${prefix}/test-basic.xml"
 rc=$(($rc+$?))
 
 # Exclude EXTRA API tests IPv6
-eval "${sudo_cmd} $timeout_exe env GTEST_TAP=2 LD_PRELOAD=$gtest_lib $gtest_app $gtest_opt_ipv6 --gtest_filter=-xlio_*:-ultra* --gtest_output=xml:${WORKSPACE}/${prefix}/test-basic-ipv6.xml"
+eval "${sudo_cmd} $timeout_exe env WORKSPACE=${WORKSPACE} GTEST_TAP=2 LD_PRELOAD=$gtest_lib $gtest_app $gtest_opt_ipv6 --gtest_filter=-xlio_*:-ultra* --gtest_output=xml:${WORKSPACE}/${prefix}/test-basic-ipv6.xml"
 rc=$(($rc+$?))
 
 # Verify Delegated TCP Timers tests
-eval "${sudo_cmd} $timeout_exe env XLIO_RX_POLL_ON_TX_TCP=1 XLIO_TCP_ABORT_ON_CLOSE=1 XLIO_TCP_CTL_THREAD=delegate GTEST_TAP=2 LD_PRELOAD=$gtest_lib $gtest_app $gtest_opt --gtest_filter=-xlio*:-ultra* --gtest_output=xml:${WORKSPACE}/${prefix}/test-delegate.xml"
+eval "${sudo_cmd} $timeout_exe env WORKSPACE=${WORKSPACE} XLIO_RX_POLL_ON_TX_TCP=1 XLIO_TCP_ABORT_ON_CLOSE=1 XLIO_TCP_CTL_THREAD=delegate GTEST_TAP=2 LD_PRELOAD=$gtest_lib $gtest_app $gtest_opt --gtest_filter=-xlio*:-ultra* --gtest_output=xml:${WORKSPACE}/${prefix}/test-delegate.xml"
 rc=$(($rc+$?))
 
 # Verify Delegated TCP Timers tests IPv6
-eval "${sudo_cmd} $timeout_exe env XLIO_RX_POLL_ON_TX_TCP=1 XLIO_TCP_ABORT_ON_CLOSE=1 XLIO_TCP_CTL_THREAD=delegate GTEST_TAP=2 LD_PRELOAD=$gtest_lib $gtest_app $gtest_opt_ipv6 --gtest_filter=-xlio*:-ultra* --gtest_output=xml:${WORKSPACE}/${prefix}/test-delegate-ipv6.xml"
+eval "${sudo_cmd} $timeout_exe env WORKSPACE=${WORKSPACE} XLIO_RX_POLL_ON_TX_TCP=1 XLIO_TCP_ABORT_ON_CLOSE=1 XLIO_TCP_CTL_THREAD=delegate GTEST_TAP=2 LD_PRELOAD=$gtest_lib $gtest_app $gtest_opt_ipv6 --gtest_filter=-xlio*:-ultra* --gtest_output=xml:${WORKSPACE}/${prefix}/test-delegate-ipv6.xml"
 rc=$(($rc+$?))
 
 if [[ -z "${MANUAL_RUN}" ]]; then

src/core/Makefile.am

@@ -197,6 +197,7 @@ libxlio_la_SOURCES := \
 	\
 	$(top_builddir)/src/core/config/descriptor_providers/xlio_config_schema.h \
 	\
+	config_printer.cpp \
 	libxlio.c \
 	main.cpp \
 	\
@@ -344,6 +345,7 @@ libxlio_la_SOURCES := \
 	util/data_updater.h \
 	\
 	$(top_builddir)/third_party/legacy_config_parser/config_parser.h \
+	config_printer.h \
 	main.h \
 	xlio.h \
 	xlio_extra.h \

src/core/config/config_registry.cpp

@@ -117,6 +117,8 @@ void config_registry::initialize_registry(std::queue<std::unique_ptr<loader>> &&
                                      key + "': expected " +
                                      get_user_friendly_type_name(param_desc.type()) + ", got " +
                                      get_user_friendly_type_name(value.type()));
+            } catch (const std::runtime_error &e) {
+                throw_xlio_exception("In '" + loader->source() + "': " + e.what());
             }
         }
 

src/core/config/config_registry.h

@@ -69,64 +69,41 @@ class config_registry {
     std::vector<std::string> get_sources() const;
 
     /**
-     * @brief Gets default value for non-integer types
+     * @brief Gets default value
      * @tparam T Value type
      * @param key Configuration parameter key
      * @return Default value for the parameter
      */
-    template <typename T>
-    typename std::enable_if<!is_integer<T>::value, T>::type get_default_value(
-        const std::string &key) const
-    {
-        return get_value_impl<T>(
-            key, [this](const std::string &k) { return get_default_value_as_any(k); });
-    }
-
-    /**
-     * @brief Gets default value for integer types with bounds checking
-     * @tparam T Integer type
-     * @param key Configuration parameter key
-     * @return Default value for the parameter
-     * @note For enums, use int instead since C++14 doesn't support bound checking for enums
-     */
-    template <typename T>
-    typename std::enable_if<is_integer<T>::value, T>::type get_default_value(
-        const std::string &key) const
+    template <typename T> T get_default_value(const std::string &key) const
     {
         return get_value_impl<T>(
             key, [this](const std::string &k) { return get_default_value_as_any(k); });
     }
 
     /**
-     * @brief Gets configured value for non-integer types
+     * @brief Gets configured value. For integer types, also does bounds checking
      * @tparam T Value type
      * @param key Configuration parameter key
      * @return Current value for the parameter
      */
-    template <typename T>
-    typename std::enable_if<!is_integer<T>::value, T>::type get_value(const std::string &key) const
+    template <typename T> T get_value(const std::string &key) const
     {
         return get_value_impl<T>(key, [this](const std::string &k) { return get_value_as_any(k); });
     }
 
     /**
-     * @brief Gets configured value for integer types with bounds checking
-     * @tparam T Integer type
+     * @brief Gets configured value as any
      * @param key Configuration parameter key
-     * @return Current value for the parameter
-     * @note For enums, use int instead since C++14 doesn't support bound checking for enums
+     * @return Current value for the parameter, as any
      */
-    template <typename T>
-    typename std::enable_if<is_integer<T>::value, T>::type get_value(const std::string &key) const
-    {
-        return get_value_impl<T>(key, [this](const std::string &k) { return get_value_as_any(k); });
-    }
+    std::experimental::any get_value_as_any(const std::string &key) const;
+
+    const config_descriptor &get_config_descriptor() const { return m_config_descriptor; }
 
 private:
     std::map<std::string, std::experimental::any> m_config_data;
     config_descriptor m_config_descriptor;
     std::vector<std::string> m_sources;
-    std::experimental::any get_value_as_any(const std::string &key) const;
     std::experimental::any get_default_value_as_any(const std::string &key) const;
     void initialize_registry(std::queue<std::unique_ptr<loader>> &&value_loaders,
                              std::unique_ptr<descriptor_provider> descriptor_provider);

src/core/config/descriptor_providers/json_descriptor_provider.cpp

@@ -152,6 +152,8 @@ std::unique_ptr<parameter_descriptor> json_descriptor_provider::create_descripto
     // Create parameter descriptor with default value
     auto descriptor = std::make_unique<parameter_descriptor>(*analysis.default_value);
 
+    descriptor->set_title(analysis.title);
+
     // Apply constraints if present
     if (analysis.needs_constraint_validation()) {
         apply_constraints(descriptor.get(), analysis.constraint_cfg);

src/core/config/descriptor_providers/schema_analyzer.cpp

@@ -11,6 +11,7 @@
 #include <stdexcept>
 #include <functional>
 #include <algorithm>
+#include "vlogger/vlogger.h"
 
 static void for_each_oneof_option(json_object *one_of_field,
                                   std::function<void(json_object *)> func)
@@ -161,6 +162,25 @@ std::experimental::optional<std::experimental::any> schema_analyzer::determine_d
     return json_utils::to_any_value(default_field);
 }
 
+std::experimental::optional<std::string> schema_analyzer::determine_title()
+{
+    json_object *title_field =
+        json_utils::try_get_field(m_property_obj, config_strings::schema::JSON_TITLE);
+    if (!title_field) {
+        // Enforce title definition only for leafs and arrays. Objects are exempt.
+        if (determine_value_type() != typeid(json_object *)) {
+            throw_xlio_exception("Title must be a defined for: " + m_path + " - " +
+                                 std::to_string(json_object_get_type(title_field)));
+        }
+        return std::experimental::nullopt;
+    }
+    if (json_object_get_type(title_field) != json_type_string) {
+        throw_xlio_exception("Title must be a string for: " + m_path + " - " +
+                             std::to_string(json_object_get_type(title_field)));
+    }
+    return std::experimental::optional<std::string>(json_object_get_string(title_field));
+}
+
 memory_size_extension_config_t schema_analyzer::analyze_memory_size_extension_config()
 {
     if (!has_memory_size_flag()) {
@@ -397,6 +417,7 @@ schema_analyzer::analysis_result::analysis_result(schema_analyzer &analyzer)
     : json_property_type(analyzer.determine_property_type())
     , value_type(analyzer.determine_value_type())
     , default_value(analyzer.determine_default_value(value_type))
+    , title(analyzer.determine_title())
     , memory_cfg(analyzer.analyze_memory_size_extension_config())
     , constraint_cfg(analyzer.analyze_constraint_config())
     , enum_cfg(analyzer.analyze_enum_mapping_config())

src/core/config/descriptor_providers/schema_analyzer.h

@@ -70,6 +70,8 @@ class schema_analyzer {
         std::type_index value_type; /**< C++ type for the parameter value */
         std::experimental::optional<std::experimental::any>
             default_value; /**< Default value ready for use */
+        std::experimental::optional<std::string>
+            title; /**< Title of the parameter as defined in schema */
 
         // Pre-parsed component configurations
         memory_size_extension_config_t memory_cfg; /**< Memory size transformation configuration */
@@ -120,6 +122,7 @@ class schema_analyzer {
     std::type_index determine_value_type();
     std::experimental::optional<std::experimental::any> determine_default_value(
         std::type_index type);
+    std::experimental::optional<std::string> determine_title();
 
     // Component configuration methods
     memory_size_extension_config_t analyze_memory_size_extension_config();

src/core/config/descriptors/config_descriptor.cpp

@@ -80,6 +80,11 @@ std::type_index config_descriptor::get_parent_expected_type(const std::string &k
     return typeid(std::map<std::string, std::experimental::any>);
 }
 
+const config_descriptor::parameter_map_t &config_descriptor::get_parameter_map() const
+{
+    return parameter_map;
+}
+
 /**
  * @brief Calculates the Levenshtein distance between two strings.
  *

src/core/config/descriptors/config_descriptor.h

@@ -20,6 +20,8 @@
  */
 class config_descriptor {
 public:
+    typedef std::map<std::string, parameter_descriptor> parameter_map_t;
+
     /**
      * @brief Default constructor
      */
@@ -55,11 +57,18 @@ class config_descriptor {
      */
     std::type_index get_parent_expected_type(const std::string &key) const;
 
+    /**
+     * @brief Gets the parameter map, allowing external users to iterate
+     *        over all parameters
+     * @return The parameter map
+     */
+    const parameter_map_t &get_parameter_map() const;
+
 private:
     /**
      * @brief Map from parameter name to its descriptor
      */
-    std::map<std::string, parameter_descriptor> parameter_map;
+    parameter_map_t parameter_map;
 
     /**
      * @brief Set of all parameter keys for efficient prefix-based lookups

src/core/config/descriptors/parameter_descriptor.cpp

@@ -12,6 +12,17 @@
 #include <climits>
 #include <sstream>
 #include <cctype>
+#include <numeric>
+
+void parameter_descriptor::set_title(const std::experimental::optional<std::string> &title)
+{
+    m_title = title;
+}
+
+const std::experimental::optional<std::string> &parameter_descriptor::get_title() const
+{
+    return m_title;
+}
 
 /**
  * @brief Parse memory size string with suffixes (e.g., "4GB", "512MB", "1024KB", "1024B", "5G")
@@ -151,6 +162,7 @@ parameter_descriptor::parameter_descriptor(const parameter_descriptor &pd)
     , m_string_mapping(pd.m_string_mapping)
     , m_value_transformer(pd.m_value_transformer)
     , m_type(pd.m_type)
+    , m_title(pd.m_title)
 {
 }
 
@@ -164,6 +176,11 @@ void parameter_descriptor::set_string_mappings(const std::map<std::string, int64
     }
 }
 
+bool parameter_descriptor::has_string_mappings() const
+{
+    return !m_string_mapping.empty();
+}
+
 void parameter_descriptor::set_value_transformer(value_transformer_t transformer)
 {
     m_value_transformer = std::move(transformer);
@@ -196,7 +213,19 @@ std::experimental::any parameter_descriptor::convert_string_to_int64(const std::
         return it->second;
     }
 
-    throw std::experimental::bad_any_cast();
+    // If no string mappings are defined, throw an exception which has no further information
+    if (m_string_mapping.empty()) {
+        throw std::experimental::bad_any_cast();
+    }
+
+    // We have string mappings but value is not one of them - make an effort and create a nice error
+    // message
+    std::string valid_values = std::accumulate(
+        next(m_string_mapping.begin()), m_string_mapping.end(), m_string_mapping.begin()->first,
+        [](const std::string &a, const auto &b) { return a + "," + b.first; });
+
+    throw std::runtime_error("Invalid value for " + m_title.value_or("") + ": " + val +
+                             ", not one of: [" + valid_values + "]");
 }
 
 std::experimental::any parameter_descriptor::get_value(bool val) const
@@ -226,6 +255,23 @@ std::experimental::any parameter_descriptor::get_value(const std::string &val) c
     throw std::experimental::bad_any_cast();
 }
 
+std::string parameter_descriptor::convert_int64_to_mapped_string_or(
+    int64_t val, const std::string &default_value) const
+{
+    if (m_string_mapping.empty()) {
+        return std::to_string(val);
+    }
+
+    for (const auto &mapping : m_string_mapping) {
+        if ((mapping.second.type() == typeid(int64_t)) &&
+            (std::experimental::any_cast<int64_t>(mapping.second) == val)) {
+            return mapping.first;
+        }
+    }
+
+    return default_value;
+}
+
 std::experimental::any parameter_descriptor::get_value(int64_t val) const
 {
     if (m_type != typeid(int64_t)) {