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]

Reply via email to