szaszm commented on code in PR #1751:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1751#discussion_r1682880317
##########
C2.md:
##########
@@ -80,8 +81,10 @@ be requested via C2 DESCRIBE manifest command.
nifi.c2.rest.listener.cacert=<SSL Cert path>
# specify the rest URIs if using RESTSender
-
nifi.c2.rest.url=http://<your-c2-server>/<c2-api-path>/c2-protocol/heartbeat
-
nifi.c2.rest.url.ack=http://<your-c2-server>/<c2-api-path>/c2-protocol/acknowledge
+ nifi.c2.rest.path.base=https://<your-c2-server>/<c2-api-path>
+ # specify either absolute url or relative to the nifi.c2.rest.path.base
url for hearbeat and acknowledge
+ nifi.c2.rest.url=/c2-protocol/heartbeat
+ nifi.c2.rest.url.ack=/c2-protocol/acknowledge
Review Comment:
I think these shouldn't include a leading slash, because they are relative
paths. That is unless MiNiFi Java works the same strange way. Wdyt?
##########
libminifi/include/c2/C2Payload.h:
##########
@@ -79,21 +85,65 @@ enum Direction {
RECEIVE
};
-struct AnnotatedValue : state::response::ValueNode {
- using state::response::ValueNode::ValueNode;
- using state::response::ValueNode::operator=;
+class C2Value {
+ public:
+ friend std::ostream& operator<<(std::ostream& out, const C2Value& val);
+
+ C2Value() = default;
+ C2Value(const C2Value& other) {
+ (*this) = other;
+ }
+ C2Value(C2Value&&) = default;
+ template<typename T>
+ requires(std::constructible_from<state::response::ValueNode, T>)
+ C2Value(T&& value) { value_ =
state::response::ValueNode{std::forward<T>(value)}; } //
NOLINT(runtime/explicit)
+ C2Value(const rapidjson::Value& json_value) { // NOLINT(runtime/explicit)
+ value_.emplace<rapidjson::Document>();
+ get<rapidjson::Document>(value_).CopyFrom(json_value,
get<rapidjson::Document>(value_).GetAllocator());
+ }
+ C2Value(rapidjson::Document&& json_doc) { // NOLINT(runtime/explicit)
Review Comment:
Why not explicit? A rationale after the NOLINT expression would be helpful
for readers IMO.
##########
libminifi/include/core/state/nodes/AssetInformation.h:
##########
@@ -0,0 +1,42 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#pragma once
+
+#include "core/state/nodes/MetricsBase.h"
+#include "utils/file/AssetManager.h"
+#include "core/logging/Logger.h"
+
+namespace org::apache::nifi::minifi::state::response {
+
+class AssetInformation : public ResponseNode {
+ public:
+ AssetInformation();
+ explicit AssetInformation(std::string_view name, const utils::Identifier&
uuid = {}) : ResponseNode(name, uuid) {}
+
+ MINIFIAPI static constexpr const char* Description = "Metric node that
defines hash for all asset identifiers";
+
+ void setAssetManager(std::shared_ptr<utils::file::AssetManager>
asset_manager);
Review Comment:
Could we inject `AssetManager` in the constructor? The fewer possible
states, the less things that can go wrong.
##########
libminifi/include/c2/protocols/RESTProtocol.h:
##########
@@ -43,7 +43,7 @@ class RESTProtocol : public HeartbeatJsonSerializer {
protected:
void initialize(core::controller::ControllerServiceProvider* controller,
const std::shared_ptr<Configure> &configure);
void serializeNestedPayload(rapidjson::Value& target, const C2Payload&
payload, rapidjson::Document::AllocatorType& alloc) override;
- static C2Payload parseJsonResponse(const C2Payload &payload, std::span<const
std::byte> response);
+ C2Payload parseJsonResponse(const C2Payload &payload, std::span<const
std::byte> response);
Review Comment:
Even if it's no longer `static`, it could still be `const`, as it doesn't
seem to modify any members.
--
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]
