fgerlits commented on code in PR #1940:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1940#discussion_r2028977279
##########
extensions/opc/src/opc.cpp:
##########
@@ -325,72 +332,80 @@ void Client::traverse(UA_NodeId nodeId, const
std::function<nodeFoundCallBackFun
}
}
-bool Client::exists(UA_NodeId nodeId) {
+bool Client::exists(UA_NodeId node_id) {
bool retval = false;
- auto callback = [&retval](Client& /*client*/, const UA_ReferenceDescription*
/*ref*/, const std::string& /*pat*/) -> bool {
+ auto callback = [&retval](const UA_ReferenceDescription* /*ref*/, const
std::string& /*pat*/) -> bool {
retval = true;
return false; // If any node is found, the given node exists, so traverse
can be stopped
};
- traverse(nodeId, callback, "", 1);
+ traverse(node_id, callback, "", 1);
return retval;
}
-UA_StatusCode Client::translateBrowsePathsToNodeIdsRequest(const std::string&
path, std::vector<UA_NodeId>& foundNodeIDs, const
std::shared_ptr<core::logging::Logger>& logger) {
- logger->log_trace("Trying to find node id for {}", path.c_str());
+UA_StatusCode Client::translateBrowsePathsToNodeIdsRequest(const std::string&
path, std::vector<UA_NodeId>& found_node_ids, int32_t namespace_index,
+ const std::vector<UA_UInt32>& path_reference_types, const
std::shared_ptr<core::logging::Logger>& logger) {
+ logger->log_trace("Trying to find node ids for {}", path.c_str());
- auto tokens = utils::string::split(path, "/");
- std::vector<UA_UInt32> ids;
- for (size_t i = 0; i < tokens.size(); ++i) {
- UA_UInt32 val = (i ==0) ? UA_NS0ID_ORGANIZES : UA_NS0ID_HASCOMPONENT;
- ids.push_back(val);
- }
+ auto tokens = utils::string::splitAndTrimRemovingEmpty(path, "/");
- UA_BrowsePath browsePath;
- UA_BrowsePath_init(&browsePath);
- browsePath.startingNode = UA_NODEID_NUMERIC(0, UA_NS0ID_OBJECTSFOLDER);
+ UA_BrowsePath browse_path;
+ UA_BrowsePath_init(&browse_path);
+ browse_path.startingNode = UA_NODEID_NUMERIC(0, UA_NS0ID_OBJECTSFOLDER);
- browsePath.relativePath.elements =
reinterpret_cast<UA_RelativePathElement*>(UA_Array_new(tokens.size(),
&UA_TYPES[UA_TYPES_RELATIVEPATHELEMENT]));
- browsePath.relativePath.elementsSize = tokens.size();
+ browse_path.relativePath.elements =
reinterpret_cast<UA_RelativePathElement*>(UA_Array_new(tokens.size(),
&UA_TYPES[UA_TYPES_RELATIVEPATHELEMENT]));
+ browse_path.relativePath.elementsSize = tokens.size();
+
+ std::vector<UA_UInt32> ids;
+ ids.push_back(UA_NS0ID_ORGANIZES);
+ for (size_t i = 0; i < tokens.size() - 1; ++i) {
Review Comment:
I know we check this in `onSchedule`, but a `gsl_Expects(!tokens.empty())`
before this line would make it look safe as well as being safe.
##########
extensions/opc/src/opcbase.cpp:
##########
@@ -24,85 +24,121 @@
#include "core/Processor.h"
#include "core/ProcessSession.h"
#include "core/Core.h"
+#include "utils/ProcessorConfigUtils.h"
namespace org::apache::nifi::minifi::processors {
- void BaseOPCProcessor::onSchedule(core::ProcessContext& context,
core::ProcessSessionFactory&) {
- logger_->log_trace("BaseOPCProcessor::onSchedule");
+void BaseOPCProcessor::onSchedule(core::ProcessContext& context,
core::ProcessSessionFactory&) {
+ logger_->log_trace("BaseOPCProcessor::onSchedule");
- applicationURI_.clear();
- certBuffer_.clear();
- keyBuffer_.clear();
- password_.clear();
- username_.clear();
- trustBuffers_.clear();
+ application_uri_.clear();
+ cert_buffer_.clear();
+ key_buffer_.clear();
+ password_.clear();
+ username_.clear();
+ trust_buffers_.clear();
- context.getProperty(OPCServerEndPoint, endPointURL_);
- context.getProperty(ApplicationURI, applicationURI_);
+ context.getProperty(OPCServerEndPoint, endpoint_url);
+ context.getProperty(ApplicationURI, application_uri_);
+ context.getProperty(Username, username_);
+ context.getProperty(Password, password_);
- if (context.getProperty(Username, username_) !=
context.getProperty(Password, password_)) {
- throw Exception(PROCESS_SCHEDULE_EXCEPTION, "Both or neither of Username
and Password should be provided!");
- }
+ if (username_.empty() != password_.empty()) {
+ throw Exception(PROCESS_SCHEDULE_EXCEPTION, "Both or neither of Username
and Password should be provided!");
+ }
- auto certificatePathRes = context.getProperty(CertificatePath, certpath_);
- auto keyPathRes = context.getProperty(KeyPath, keypath_);
- context.getProperty(TrustedPath, trustpath_);
- if (certificatePathRes != keyPathRes) {
- throw Exception(PROCESS_SCHEDULE_EXCEPTION, "All or none of Certificate
path and Key path should be provided!");
- }
+ context.getProperty(CertificatePath, certpath_);
+ context.getProperty(KeyPath, keypath_);
+ context.getProperty(TrustedPath, trustpath_);
+ if (certpath_.empty() != keypath_.empty()) {
+ throw Exception(PROCESS_SCHEDULE_EXCEPTION, "All or none of Certificate
path and Key path should be provided!");
+ }
- if (certpath_.empty()) {
- return;
- }
- if (applicationURI_.empty()) {
- throw Exception(PROCESS_SCHEDULE_EXCEPTION, "Application URI must be
provided if Certificate path is provided!");
- }
+ if (certpath_.empty()) {
+ return;
+ }
+ if (application_uri_.empty()) {
+ throw Exception(PROCESS_SCHEDULE_EXCEPTION, "Application URI must be
provided if Certificate path is provided!");
+ }
- std::ifstream input_cert(certpath_, std::ios::binary);
- if (input_cert.good()) {
- certBuffer_ =
std::vector<char>(std::istreambuf_iterator<char>(input_cert), {});
- }
- std::ifstream input_key(keypath_, std::ios::binary);
- if (input_key.good()) {
- keyBuffer_ =
std::vector<char>(std::istreambuf_iterator<char>(input_key), {});
- }
+ std::ifstream input_cert(certpath_, std::ios::binary);
+ if (input_cert.good()) {
+ cert_buffer_ =
std::vector<char>(std::istreambuf_iterator<char>(input_cert), {});
+ }
+ std::ifstream input_key(keypath_, std::ios::binary);
+ if (input_key.good()) {
+ key_buffer_ = std::vector<char>(std::istreambuf_iterator<char>(input_key),
{});
+ }
- if (certBuffer_.empty()) {
- auto error_msg = utils::string::join_pack("Failed to load cert from
path: ", certpath_);
- throw Exception(PROCESS_SCHEDULE_EXCEPTION, error_msg);
- }
- if (keyBuffer_.empty()) {
- auto error_msg = utils::string::join_pack("Failed to load key from path:
", keypath_);
+ if (cert_buffer_.empty()) {
+ auto error_msg = utils::string::join_pack("Failed to load cert from path:
", certpath_);
+ throw Exception(PROCESS_SCHEDULE_EXCEPTION, error_msg);
+ }
+ if (key_buffer_.empty()) {
+ auto error_msg = utils::string::join_pack("Failed to load key from path:
", keypath_);
+ throw Exception(PROCESS_SCHEDULE_EXCEPTION, error_msg);
+ }
+
+ auto trusted_cert_paths =
utils::string::splitAndTrimRemovingEmpty(trustpath_, ",");
+ for (const auto& trust_path : trusted_cert_paths) {
+ std::ifstream input_trust(trust_path, std::ios::binary);
+ if (input_trust.good()) {
+
trust_buffers_.push_back(std::vector<char>(std::istreambuf_iterator<char>(input_trust),
{}));
+ } else {
+ auto error_msg = utils::string::join_pack("Failed to load trusted server
certs from path: ", trust_path);
throw Exception(PROCESS_SCHEDULE_EXCEPTION, error_msg);
}
+ }
+}
- if (!trustpath_.empty()) {
- std::ifstream input_trust(trustpath_, std::ios::binary);
- if (input_trust.good()) {
-
trustBuffers_.push_back(std::vector<char>(std::istreambuf_iterator<char>(input_trust),
{}));
- } else {
- auto error_msg = utils::string::join_pack("Failed to load trusted
server certs from path: ", trustpath_);
- throw Exception(PROCESS_SCHEDULE_EXCEPTION, error_msg);
- }
- }
+bool BaseOPCProcessor::reconnect() {
+ if (connection_ == nullptr) {
+ connection_ = opc::Client::createClient(logger_, application_uri_,
cert_buffer_, key_buffer_, trust_buffers_);
}
- bool BaseOPCProcessor::reconnect() {
- if (connection_ == nullptr) {
- connection_ = opc::Client::createClient(logger_, applicationURI_,
certBuffer_, keyBuffer_, trustBuffers_);
- }
+ if (connection_->isConnected()) {
+ return true;
+ }
- if (connection_->isConnected()) {
- return true;
+ auto sc = connection_->connect(endpoint_url, username_, password_);
+ if (sc != UA_STATUSCODE_GOOD) {
+ logger_->log_error("Failed to connect: {}!", UA_StatusCode_name(sc));
+ return false;
+ }
+ logger_->log_debug("Successfully connected.");
+ return true;
+}
+
+void BaseOPCProcessor::readPathReferenceTypes(core::ProcessContext& context,
const std::string& node_id) {
+ std::string value;
+ context.getProperty(PathReferenceTypes, value);
+ if (value.empty()) {
+ return;
+ }
+ auto path_reference_types = utils::string::splitAndTrimRemovingEmpty(value,
"/");
+ if (path_reference_types.size() !=
utils::string::splitAndTrimRemovingEmpty(node_id, "/").size() - 1) {
Review Comment:
I may be overly paranoid about subtracting from unsigned numbers, but I'd
prefer `if (path_reference_types.size() + 1 !=
utils::string::splitAndTrimRemovingEmpty(node_id, "/").size())`
--
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-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
