szaszm commented on code in PR #1331:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1331#discussion_r874697975
##########
extensions/http-curl/processors/InvokeHTTP.cpp:
##########
@@ -264,18 +269,37 @@ bool InvokeHTTP::shouldEmitFlowFile() const {
return ("POST" == method_ || "PUT" == method_ || "PATCH" == method_);
}
-std::optional<std::map<std::string, std::string>>
InvokeHTTP::validateAttributesAgainstHTTPHeaderRules(const
std::map<std::string, std::string>& attributes) const {
- std::map<std::string, std::string> result;
- for (const auto& [attribute_name, attribute_value] : attributes) {
- if (utils::HTTPClient::isValidHttpHeaderField(attribute_name)) {
- result.emplace(attribute_name, attribute_value);
- } else if (invalid_http_header_field_handling_strategy_ ==
InvalidHTTPHeaderFieldHandlingOption::TRANSFORM) {
-
result.emplace(utils::HTTPClient::replaceInvalidCharactersInHttpHeaderFieldName(attribute_name),
attribute_value);
- } else if (invalid_http_header_field_handling_strategy_ ==
InvalidHTTPHeaderFieldHandlingOption::FAIL) {
- return std::nullopt;
- }
+/**
+ * Calls append_header with valid HTTP header keys, based on
attributes_to_send_
+ * @param flow_file
+ * @param append_header Callback to append HTTP header to the request
+ * @return false when the flow file should be routed to failure, true otherwise
+ */
+bool InvokeHTTP::appendHeaders(const core::FlowFile& flow_file,
/*std::invocable<std::string, std::string>*/ auto append_header) {
+ static_assert(std::is_invocable_v<decltype(append_header), std::string,
std::string>);
+ if (!attributes_to_send_) return true;
+ const auto key_fn = [](const std::pair<std::string, std::string>& pair) {
return pair.first; };
+ const auto original_attributes = flow_file.getAttributes();
+ // non-const views, because otherwise it doesn't satisfy viewable_range, and
transform would fail
+ ranges::viewable_range auto matching_attributes = original_attributes
+ | ranges::views::filter([this](const auto& key) { return
utils::regexMatch(key, *attributes_to_send_); }, key_fn);
+ switch (invalid_http_header_field_handling_strategy_.value()) {
+ case InvalidHTTPHeaderFieldHandlingOption::FAIL:
+ if (ranges::any_of(matching_attributes,
std::not_fn(&utils::HTTPClient::isValidHttpHeaderField), key_fn)) return false;
+ for (const auto& header: matching_attributes)
append_header(header.first, header.second);
+ return true;
+ case InvalidHTTPHeaderFieldHandlingOption::DROP:
+ for (const auto& header: matching_attributes |
ranges::views::filter(&utils::HTTPClient::isValidHttpHeaderField, key_fn)) {
+ append_header(header.first, header.second);
+ }
+ return true;
+ case InvalidHTTPHeaderFieldHandlingOption::TRANSFORM:
+ for (const auto& header: matching_attributes) {
+
append_header(utils::HTTPClient::replaceInvalidCharactersInHttpHeaderFieldName(header.first),
header.second);
+ }
+ return true;
}
- return result;
+ return true;
Review Comment:
One certain benefit it that there are no more heap allocations aside from
the return value of `flow_file.getAttributes()` and possibly libcurl internals.
I agree that the code feels more complex, but I think it's because it's more
dense and expressive, not because it's actually more complex. Or at least when
I think about the alternatives for each line or part of code, they are either
similar in complexity, but using more [raw
loops](https://youtu.be/qH6sSOr-yk8?t=130 and ifs, and not expressing the
intent that well, or just more complex. Part of the extra complexity is that I
felt that the logic of a single algorithm was unnecessary scattered between
InvokeHTTP::validateAttributesAgainstHTTPHeaderRules and
HTTPClient::build_header_list, so now "both" are happening here.
--
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]
