Copilot commented on code in PR #2266:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2266#discussion_r4047969661
##########
extensions/opc/src/PutOPCProcessor.cpp:
##########
@@ -281,9 +246,21 @@ void PutOPCProcessor::onTrigger(core::ProcessContext&
context, core::ProcessSess
return;
}
- if (!readParentNodeId()) {
- context.yield();
- return;
+ if (parent_node_defined_ && id_type_ == opc::OPCNodeIDType::Path &&
!path_node_id_resolved_) {
+ std::vector<opc::NodeId> translated_node_ids;
+ auto sc = connection_->translateBrowsePathsToNodeIdsRequest(node_id_,
translated_node_ids, namespace_idx_, path_reference_types_, logger_);
+ if (sc != UA_STATUSCODE_GOOD) {
+ logger_->log_error("Failed to translate path '{}' to a node id: {}",
node_id_, UA_StatusCode_name(sc));
+ context.yield();
+ return;
+ }
+ if (translated_node_ids.size() != 1) {
+ logger_->log_error("Path '{}' resolved to {} node ids; exactly one
target node is required for put", node_id_, translated_node_ids.size());
+ context.yield();
+ return;
+ }
+ node_ = std::move(translated_node_ids[0]);
+ path_node_id_resolved_ = true;
}
Review Comment:
This permanently caches the parent path translation, although `reconnect()`
never invalidates `path_node_id_resolved_`. If the OPC server restarts or the
path is deleted and recreated with a different server-assigned NodeId,
subsequent creates continue using the stale parent; the previous implementation
resolved the path on every trigger. Resolve this path per trigger, or
invalidate the cached NodeId whenever the connection is re-established.
##########
extensions/opc/src/OPCCommon.cpp:
##########
@@ -584,4 +593,27 @@ UA_StatusCode Client::readHistory(HistoryReadTypeOption
history_type, const UA_N
return UA_Client_HistoryRead_raw(client_, &node_id, callback, start_time,
end_time, UA_STRING_NULL, false, 0, UA_TIMESTAMPSTORETURN_SOURCE,
callback_context);
}
+std::expected<opc::NodeId, std::string> buildNodeId(opc::OPCNodeIDType
id_type, UA_UInt16 namespace_idx, const std::string& node_id) {
+ switch (id_type) {
+ case opc::OPCNodeIDType::String:
+ return opc::NodeId{UA_NODEID_STRING_ALLOC(namespace_idx,
node_id.c_str())};
+ case opc::OPCNodeIDType::Int:
+ try {
+ return opc::NodeId{UA_NODEID_NUMERIC(namespace_idx,
std::stoi(node_id))};
+ } catch(const std::exception&) {
+ auto error_msg = utils::string::join_pack(node_id, " cannot be used as
an int type node ID");
+ return std::unexpected{error_msg};
+ }
Review Comment:
Numeric OPC NodeIds are `UA_UInt32`, but `std::stoi` parses only the signed
`int` range and its negative result is then converted to unsigned.
Consequently, a valid ID such as `4294967295` is rejected while `-1` silently
addresses that same ID; trailing text such as `666junk` is also accepted. Parse
the complete input into the unsigned 32-bit range before constructing the
NodeId.
--
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]
