Copilot commented on code in PR #1983:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1983#discussion_r2161128990
##########
extensions/expression-language/ProcessContextExpr.cpp:
##########
@@ -73,9 +73,12 @@ std::map<std::string, std::string>
ProcessContextExpr::getDynamicProperties(cons
for (auto& [dynamic_property_name, dynamic_property_value] : dynamic_props) {
Review Comment:
The cache for compiled expressions isn’t invalidated if a dynamic property
is changed via `setDynamicProperty`. You should clear or update the
corresponding entry in `cached_dynamic_expressions_` when properties are reset
or removed to avoid stale evaluations.
##########
extensions/expression-language/ProcessContextExpr.cpp:
##########
@@ -73,9 +73,12 @@ std::map<std::string, std::string>
ProcessContextExpr::getDynamicProperties(cons
for (auto& [dynamic_property_name, dynamic_property_value] : dynamic_props) {
if (!cached_dynamic_expressions_.contains(dynamic_property_name)) {
auto expression = expression::compile(dynamic_property_value);
- expression::Parameters p(this, flow_file);
- dynamic_property_value = expression(p).asString();
+ expression::Parameters parameters(this, flow_file);
+ dynamic_property_value = expression(parameters).asString();
cached_dynamic_expressions_.emplace(std::string{dynamic_property_name},
std::move(expression));
+ } else {
+ expression::Parameters parameters(this, flow_file);
+ dynamic_property_value =
cached_dynamic_expressions_[dynamic_property_name](parameters).asString();
Review Comment:
[nitpick] Avoid two lookups (`contains` + `operator[]`). Consider using
`auto it = cached_dynamic_expressions_.find(dynamic_property_name);` to check
existence and reuse `it` for lookup or insertion.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
