exceptionfactory commented on code in PR #9507:
URL: https://github.com/apache/nifi/pull/9507#discussion_r1848650018
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java:
##########
@@ -1257,6 +1274,14 @@ private String getLogString(Map<String, List<String>>
map) {
* Returns a Map of flowfile attributes from the response http headers.
Multivalue headers are naively converted to comma separated strings.
*/
private Map<String, String> convertAttributesFromHeaders(final Response
responseHttp) {
+ return convertAttributesFromHeaders(responseHttp, "");
+ }
Review Comment:
This method overload does not seem necessary since it is a `private` method.
Recommend consolidating all references to a single
`convertAttributesFromHeaders` method.
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java:
##########
@@ -635,6 +646,7 @@ public void migrateProperties(final PropertyConfiguration
config) {
config.renameProperty("Always Output Response",
RESPONSE_GENERATION_REQUIRED.getName());
config.renameProperty("flow-file-naming-strategy",
RESPONSE_FLOW_FILE_NAMING_STRATEGY.getName());
config.renameProperty("Add Response Headers to Request",
RESPONSE_HEADER_REQUEST_ATTRIBUTES_ENABLED.getName());
+ config.renameProperty("Prefix to Added Response Headers to the
Request", RESPONSE_HEADER_REQUEST_ATTRIBUTES_PREFIX.getName());
Review Comment:
This property name migration does not appear to be necessary.
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java:
##########
@@ -1266,7 +1291,7 @@ private Map<String, String>
convertAttributesFromHeaders(final Response response
if (!values.isEmpty()) {
// create a comma separated string from the values, this is
stored in the map
final String value = StringUtils.join(values,
MULTIPLE_HEADER_DELIMITER);
- attributes.put(key, value);
+ attributes.put(trimToEmpty(prefix) + key, value);
Review Comment:
Recommend declaring `final trimmedPrefix = trimToEmpty(prefix)` to avoid
calling this method on every attribute addition.
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java:
##########
@@ -1257,6 +1274,14 @@ private String getLogString(Map<String, List<String>>
map) {
* Returns a Map of flowfile attributes from the response http headers.
Multivalue headers are naively converted to comma separated strings.
*/
private Map<String, String> convertAttributesFromHeaders(final Response
responseHttp) {
+ return convertAttributesFromHeaders(responseHttp, "");
+ }
+
+ /**
+ * Returns a Map of flowfile attributes from the response http headers.
Multivalue headers are naively converted to comma separated strings.
+ * Prefix is passed in to allow differentiation for these new attributes.
+ */
+ private Map<String, String> convertAttributesFromHeaders(final Response
responseHttp, String prefix) {
Review Comment:
```suggestion
private Map<String, String> convertAttributesFromHeaders(final Response
responseHttp, final String prefix) {
```
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java:
##########
@@ -987,6 +992,18 @@ public void onTrigger(ProcessContext context,
ProcessSession session) throws Pro
}
}
+ // If the property to add the response headers to the request
flowfile is true then add them
+ //
+ // This needs to be done after the response flowFile has been
created from the request flowFile
+ // as the added attribute headers may have a prefix added that
doesn't make sense for the response flowFile.
+ if
(context.getProperty(RESPONSE_HEADER_REQUEST_ATTRIBUTES_ENABLED).asBoolean() &&
requestFlowFile != null) {
+ String prefix =
context.getProperty(RESPONSE_HEADER_REQUEST_ATTRIBUTES_PREFIX).evaluateAttributeExpressions(requestFlowFile).getValue();
Review Comment:
```suggestion
final String prefix =
context.getProperty(RESPONSE_HEADER_REQUEST_ATTRIBUTES_PREFIX).evaluateAttributeExpressions(requestFlowFile).getValue();
```
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java:
##########
@@ -987,6 +992,18 @@ public void onTrigger(ProcessContext context,
ProcessSession session) throws Pro
}
}
+ // If the property to add the response headers to the request
flowfile is true then add them
+ //
Review Comment:
This existing comment can be removed
--
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]