Re: [PR] MINIFICPP-2376 Add support for parameter context inheritance [nifi-minifi-cpp]

["fgerlits (via GitHub)" <gi...@apache.org>]

fgerlits commented on code in PR #1888:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1888#discussion_r1909114892


##########
libminifi/src/core/flow/StructuredConfiguration.cpp:
##########
@@ -139,6 +139,41 @@ std::unique_ptr<core::ProcessGroup> StructuredConfiguration::getRootFrom(const N
   }
 }
 
+namespace {
+bool hasInheritanceCycle(const ParameterContext& parameter_context, std::unordered_set<std::string>& visited_parameter_contexts, std::unordered_set<std::string>& current_stack) {
+  if (current_stack.contains(parameter_context.getName())) {
+    return true;
+  }
+
+  if (visited_parameter_contexts.contains(parameter_context.getName())) {
+    return false;
+  }
+
+  current_stack.insert(parameter_context.getName());
+  visited_parameter_contexts.insert(parameter_context.getName());
+
+  for (const auto& inherited_parameter_context : parameter_context.getInheritedParameterContexts()) {
+    if (hasInheritanceCycle(*inherited_parameter_context, visited_parameter_contexts, current_stack)) {
+      return true;
+    }
+  }
+
+  current_stack.erase(parameter_context.getName());
+
+  return false;
+}
+}  // namespace
+
+void StructuredConfiguration::verifyNoInheritanceCycles() const {
+  std::unordered_set<std::string> visited_parameter_contexts;
+  std::unordered_set<std::string> current_stack;
+  for (const auto& [parameter_context_name, parameter_context] : parameter_contexts_) {
+    if (hasInheritanceCycle(*parameter_context, visited_parameter_contexts, current_stack)) {
+      throw std::invalid_argument("Circular references in Parameter Context inheritance are not allowed. Inheritance cycle was detected in parameter context '" + parameter_context->getName() + "'");

Review Comment:
   very minor, but we could use `parameter_context_name` instead of `parameter_context->getName()`



##########
CONFIGURE.md:
##########
@@ -139,12 +139,25 @@ Processor properties in flow configurations can be parameterized using parameter
  - `#` character can be used to escape the parameter syntax. E.g. if the `parameterName` parameter's value is `xxx` then `#{parameterName}` will be replaced with `xxx`, `##{parameterName}` will be replaced with `#{parameterName}`, and `#####{parameterName}` will be replaced with `##xxx`.
  - Sensitive parameters can only be assigned to sensitive properties and non-sensitive parameters can only be assigned to non-sensitive properties.
 
+Parameter contexts can be inherited from multiple other parameter contexts. The order of the parameter inheritance matters in the way that if a parameter is present in multiple parameter contexts, the parameter value assigned to the first parameter in the inheritance order will be used. No circular inheritance is allowed.
+
 An example for using parameters in a JSON configuration file:
 
 ```json
 {
     "parameterContexts": [
         {
+            "identifier": "235e6b47-ea22-45cd-a472-545801db98e6",
+            "name": "common-parameter-context",
+            "description": "Common parameter context",
+            "parameters": [
+                {
+                    "name": "common_timeout",
+                    "description": "Common timeout seconds",
+                    "sensitive": false,
+                    "value": "30"
+                }
+            ],
             "identifier": "804e6b47-ea22-45cd-a472-545801db98e6",

Review Comment:
   I think a `}, {` is missing between these two parameter context definitions



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org