You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2020/04/25 15:25:39 UTC

[GitHub] [nifi] ottobackwards opened a new pull request #4234: NIFI-7394 InvokeHTTP Processor multipart/form-data support

ottobackwards opened a new pull request #4234:
URL: https://github.com/apache/nifi/pull/4234


   Add support for sending Multipart/FORM data to InvokeHTTP.
   
   By using dynamic properties with a prefix naming scheme, allow
   definition of the parts, including the name to give the Flowfile content
   part, and optionally it's file name.
   
   Thank you for submitting a contribution to Apache NiFi.
   
   Please provide a short description of the PR here:
   
   #### Description of PR
   
   _Enables X functionality; fixes bug NIFI-YYYY._
   
   In order to streamline the review of the contribution we ask you
   to ensure the following steps have been taken:
   
   ### For all changes:
   - [x] Is there a JIRA ticket associated with this PR? Is it referenced 
        in the commit message?
   
   - [x] Does your PR title start with **NIFI-XXXX** where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
   
   - [x] Has your PR been rebased against the latest commit within the target branch (typically `master`)?
   
   - [x] Is your initial contribution a single, squashed commit? _Additional commits in response to PR reviewer feedback should be made on this branch and pushed to allow change tracking. Do not `squash` or use `--force` when pushing to allow for clean monitoring of changes._
   
   ### For code changes:
   - [x] Have you ensured that the full suite of tests is executed via `mvn -Pcontrib-check clean install` at the root `nifi` folder?
   - [x] Have you written or updated unit tests to verify your changes?
   - [-] Have you verified that the full build is successful on both JDK 8 and JDK 11?
   - [-] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? 
   - [-] If applicable, have you updated the `LICENSE` file, including the main `LICENSE` file under `nifi-assembly`?
   - [-] If applicable, have you updated the `NOTICE` file, including the main `NOTICE` file found under `nifi-assembly`?
   - [-] If adding new Properties, have you added `.displayName` in addition to .name (programmatic access) for each of the new properties?
   
   ### For documentation related changes:
   - [X] Have you ensured that format looks appropriate for the output in which it is rendered?
   
   ### Note:
   Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] ottobackwards commented on a change in pull request #4234: NIFI-7394 InvokeHTTP Processor multipart/form-data support

Posted by GitBox <gi...@apache.org>.
ottobackwards commented on a change in pull request #4234:
URL: https://github.com/apache/nifi/pull/4234#discussion_r416689504



##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java
##########
@@ -131,9 +135,19 @@
     @WritesAttribute(attribute = "invokehttp.java.exception.message", description = "The Java exception message raised when the processor fails"),
     @WritesAttribute(attribute = "user-defined", description = "If the 'Put Response Body In Attribute' property is set then whatever it is set to "
         + "will become the attribute key and the value would be the body of the HTTP response.")})
-@DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
-                    description = "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
-                            + "the Attribute Expression Language set in the value of the Dynamic Property.")
+@DynamicProperties ({
+    @DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
+                + "the Attribute Expression Language set in the value of the Dynamic Property."),
+    @DynamicProperty(name = "post.form.<NAME>", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "When the HTTP Method is POST, dynamic properties with the property name in the form of post.form.<NAME>,"
+                + " where the <NAME> will be the form data name, will be used to fill out the multipart form parts."
+                + "  If the value is the literal 'FLOWFILE_CONTENT', that form part will have the flowfile's data."
+                + "  The property name post.form.filename will set the filename to set for the flowfile content."

Review comment:
       What I did was add two properties:  the content name ( the form data name ) and a boolean to set the file name.   If that is set, we use the flow file file name.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] ottobackwards commented on a change in pull request #4234: NIFI-7394 InvokeHTTP Processor multipart/form-data support

Posted by GitBox <gi...@apache.org>.
ottobackwards commented on a change in pull request #4234:
URL: https://github.com/apache/nifi/pull/4234#discussion_r416051173



##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java
##########
@@ -131,9 +135,19 @@
     @WritesAttribute(attribute = "invokehttp.java.exception.message", description = "The Java exception message raised when the processor fails"),
     @WritesAttribute(attribute = "user-defined", description = "If the 'Put Response Body In Attribute' property is set then whatever it is set to "
         + "will become the attribute key and the value would be the body of the HTTP response.")})
-@DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
-                    description = "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
-                            + "the Attribute Expression Language set in the value of the Dynamic Property.")
+@DynamicProperties ({
+    @DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
+                + "the Attribute Expression Language set in the value of the Dynamic Property."),
+    @DynamicProperty(name = "post.form.<NAME>", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "When the HTTP Method is POST, dynamic properties with the property name in the form of post.form.<NAME>,"
+                + " where the <NAME> will be the form data name, will be used to fill out the multipart form parts."
+                + "  If the value is the literal 'FLOWFILE_CONTENT', that form part will have the flowfile's data."

Review comment:
       I agree that this is more awkward than I would like, but that is a general problem with the was that properties work.
   
   A form can have an arbitrary number of fields or parts.  It also may not be for uploading a document.  So to make it broadly useful you have to be able to define multiple fields.  Given the way that flowfiles work, I think only being able to define 1 'file upload' field for the flow file is good enough.
   
   I thought that using this technique, like I did for the ExecuteStreamCommand to support arguments and fix the escaping issue, would allow for having _N_ parts, with EL support available.  Now, new properties for the things that I have as known constant values _is_ a better way to go, but I was hesitant to add new properties.
   
   I think the most flexible solution is to use the dynamic parameters, with new parameters for the content  Content form name, Content file name.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] ottobackwards commented on a change in pull request #4234: NIFI-7394 InvokeHTTP Processor multipart/form-data support

Posted by GitBox <gi...@apache.org>.
ottobackwards commented on a change in pull request #4234:
URL: https://github.com/apache/nifi/pull/4234#discussion_r416054076



##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java
##########
@@ -131,9 +135,19 @@
     @WritesAttribute(attribute = "invokehttp.java.exception.message", description = "The Java exception message raised when the processor fails"),
     @WritesAttribute(attribute = "user-defined", description = "If the 'Put Response Body In Attribute' property is set then whatever it is set to "
         + "will become the attribute key and the value would be the body of the HTTP response.")})
-@DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
-                    description = "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
-                            + "the Attribute Expression Language set in the value of the Dynamic Property.")
+@DynamicProperties ({
+    @DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
+                + "the Attribute Expression Language set in the value of the Dynamic Property."),
+    @DynamicProperty(name = "post.form.<NAME>", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "When the HTTP Method is POST, dynamic properties with the property name in the form of post.form.<NAME>,"
+                + " where the <NAME> will be the form data name, will be used to fill out the multipart form parts."
+                + "  If the value is the literal 'FLOWFILE_CONTENT', that form part will have the flowfile's data."
+                + "  The property name post.form.filename will set the filename to set for the flowfile content."
+                + "  If send message body is false, the properties will be ignored.")

Review comment:
       All post.form properties are ignored if send message body is false. That is more in line with how the current code worked.  I can see a logical argument that if there are forms set, you should send those and only let the send body apply to sending the flowfile.  What do you think?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] ottobackwards commented on a change in pull request #4234: NIFI-7394 InvokeHTTP Processor multipart/form-data support

Posted by GitBox <gi...@apache.org>.
ottobackwards commented on a change in pull request #4234:
URL: https://github.com/apache/nifi/pull/4234#discussion_r416052749



##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java
##########
@@ -131,9 +135,19 @@
     @WritesAttribute(attribute = "invokehttp.java.exception.message", description = "The Java exception message raised when the processor fails"),
     @WritesAttribute(attribute = "user-defined", description = "If the 'Put Response Body In Attribute' property is set then whatever it is set to "
         + "will become the attribute key and the value would be the body of the HTTP response.")})
-@DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
-                    description = "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
-                            + "the Attribute Expression Language set in the value of the Dynamic Property.")
+@DynamicProperties ({
+    @DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
+                + "the Attribute Expression Language set in the value of the Dynamic Property."),
+    @DynamicProperty(name = "post.form.<NAME>", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "When the HTTP Method is POST, dynamic properties with the property name in the form of post.form.<NAME>,"
+                + " where the <NAME> will be the form data name, will be used to fill out the multipart form parts."
+                + "  If the value is the literal 'FLOWFILE_CONTENT', that form part will have the flowfile's data."
+                + "  The property name post.form.filename will set the filename to set for the flowfile content."

Review comment:
       ok, so you are saying to remove that, and have it set to the flow file's file name.  And document it?  and if they _don't_ want to set a file name, what do we do?  Have a post file name with form data properties ( boolean)?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] ottobackwards commented on a change in pull request #4234: NIFI-7394 InvokeHTTP Processor multipart/form-data support

Posted by GitBox <gi...@apache.org>.
ottobackwards commented on a change in pull request #4234:
URL: https://github.com/apache/nifi/pull/4234#discussion_r416688879



##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java
##########
@@ -131,9 +135,19 @@
     @WritesAttribute(attribute = "invokehttp.java.exception.message", description = "The Java exception message raised when the processor fails"),
     @WritesAttribute(attribute = "user-defined", description = "If the 'Put Response Body In Attribute' property is set then whatever it is set to "
         + "will become the attribute key and the value would be the body of the HTTP response.")})
-@DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
-                    description = "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
-                            + "the Attribute Expression Language set in the value of the Dynamic Property.")
+@DynamicProperties ({
+    @DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
+                + "the Attribute Expression Language set in the value of the Dynamic Property."),
+    @DynamicProperty(name = "post.form.<NAME>", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "When the HTTP Method is POST, dynamic properties with the property name in the form of post.form.<NAME>,"
+                + " where the <NAME> will be the form data name, will be used to fill out the multipart form parts."
+                + "  If the value is the literal 'FLOWFILE_CONTENT', that form part will have the flowfile's data."

Review comment:
       This is what i did




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] ottobackwards commented on a change in pull request #4234: NIFI-7394 InvokeHTTP Processor multipart/form-data support

Posted by GitBox <gi...@apache.org>.
ottobackwards commented on a change in pull request #4234:
URL: https://github.com/apache/nifi/pull/4234#discussion_r416061201



##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java
##########
@@ -131,9 +135,19 @@
     @WritesAttribute(attribute = "invokehttp.java.exception.message", description = "The Java exception message raised when the processor fails"),
     @WritesAttribute(attribute = "user-defined", description = "If the 'Put Response Body In Attribute' property is set then whatever it is set to "
         + "will become the attribute key and the value would be the body of the HTTP response.")})
-@DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
-                    description = "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
-                            + "the Attribute Expression Language set in the value of the Dynamic Property.")
+@DynamicProperties ({
+    @DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
+                + "the Attribute Expression Language set in the value of the Dynamic Property."),
+    @DynamicProperty(name = "post.form.<NAME>", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "When the HTTP Method is POST, dynamic properties with the property name in the form of post.form.<NAME>,"
+                + " where the <NAME> will be the form data name, will be used to fill out the multipart form parts."
+                + "  If the value is the literal 'FLOWFILE_CONTENT', that form part will have the flowfile's data."
+                + "  The property name post.form.filename will set the filename to set for the flowfile content."
+                + "  If send message body is false, the properties will be ignored.")

Review comment:
       Actually, the more I think of it, the possibility of just sending form data and no file is real.  This should be changed.  Although I don't know if that wrecks the send body idea. Thoughts?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] ottobackwards commented on a change in pull request #4234: NIFI-7394 InvokeHTTP Processor multipart/form-data support

Posted by GitBox <gi...@apache.org>.
ottobackwards commented on a change in pull request #4234:
URL: https://github.com/apache/nifi/pull/4234#discussion_r416690002



##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java
##########
@@ -131,9 +135,19 @@
     @WritesAttribute(attribute = "invokehttp.java.exception.message", description = "The Java exception message raised when the processor fails"),
     @WritesAttribute(attribute = "user-defined", description = "If the 'Put Response Body In Attribute' property is set then whatever it is set to "
         + "will become the attribute key and the value would be the body of the HTTP response.")})
-@DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
-                    description = "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
-                            + "the Attribute Expression Language set in the value of the Dynamic Property.")
+@DynamicProperties ({
+    @DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
+                + "the Attribute Expression Language set in the value of the Dynamic Property."),
+    @DynamicProperty(name = "post.form.<NAME>", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "When the HTTP Method is POST, dynamic properties with the property name in the form of post.form.<NAME>,"
+                + " where the <NAME> will be the form data name, will be used to fill out the multipart form parts."
+                + "  If the value is the literal 'FLOWFILE_CONTENT', that form part will have the flowfile's data."
+                + "  The property name post.form.filename will set the filename to set for the flowfile content."
+                + "  If send message body is false, the properties will be ignored.")

Review comment:
       This is done.  They can send just forms, just the flow file _as a form_, or the flowfile




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] markap14 commented on a change in pull request #4234: NIFI-7394 InvokeHTTP Processor multipart/form-data support

Posted by GitBox <gi...@apache.org>.
markap14 commented on a change in pull request #4234:
URL: https://github.com/apache/nifi/pull/4234#discussion_r416029131



##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java
##########
@@ -1039,10 +1084,39 @@ public void writeTo(BufferedSink sink) throws IOException {
                 }
 
                 @Override
-                public long contentLength(){
+                public long contentLength() {
                     return useChunked ? -1 : requestFlowFile.getSize();
                 }
             };
+
+            if (propertyDescriptors.size() > 0) {
+                // we have form data
+                MultipartBody.Builder builder = new Builder().setType(MultipartBody.FORM);
+                String contentKey = null;
+                String contentFileName = null;
+
+                // loop through the dynamic form parameters
+                // we can't add the content before we know if there is a file name or not, so we
+                // get the keys and do them after this
+                for (final Map.Entry<String, PropertyDescriptor> entry : propertyDescriptors.entrySet()) {
+                    final String propValue = context.getProperty(entry.getValue().getName()).evaluateAttributeExpressions(requestFlowFile).getValue();
+                    if (propValue.equals(FLOWFILE_CONTENT_FORM_MARKER)) {
+                        contentKey = entry.getKey();
+                    } else if (entry.getValue().getName().equals(FLOWFILE_CONTENT_FORM_FILE_NAME)) {
+                        contentFileName = propValue;
+                    } else {
+                        builder.addFormDataPart(entry.getKey(), propValue);
+                    }
+                }
+                if (contentKey != null) {
+                    ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();

Review comment:
       It appears that this. is buffering the content of the entire FlowFile into a ByteArrayOutputStream -- and then doing nothing with it. Was this just an artifact of a refactoring and was missed or something?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] ottobackwards commented on a change in pull request #4234: NIFI-7394 InvokeHTTP Processor multipart/form-data support

Posted by GitBox <gi...@apache.org>.
ottobackwards commented on a change in pull request #4234:
URL: https://github.com/apache/nifi/pull/4234#discussion_r416033229



##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java
##########
@@ -1039,10 +1084,39 @@ public void writeTo(BufferedSink sink) throws IOException {
                 }
 
                 @Override
-                public long contentLength(){
+                public long contentLength() {
                     return useChunked ? -1 : requestFlowFile.getSize();
                 }
             };
+
+            if (propertyDescriptors.size() > 0) {
+                // we have form data
+                MultipartBody.Builder builder = new Builder().setType(MultipartBody.FORM);
+                String contentKey = null;
+                String contentFileName = null;
+
+                // loop through the dynamic form parameters
+                // we can't add the content before we know if there is a file name or not, so we
+                // get the keys and do them after this
+                for (final Map.Entry<String, PropertyDescriptor> entry : propertyDescriptors.entrySet()) {
+                    final String propValue = context.getProperty(entry.getValue().getName()).evaluateAttributeExpressions(requestFlowFile).getValue();
+                    if (propValue.equals(FLOWFILE_CONTENT_FORM_MARKER)) {
+                        contentKey = entry.getKey();
+                    } else if (entry.getValue().getName().equals(FLOWFILE_CONTENT_FORM_FILE_NAME)) {
+                        contentFileName = propValue;
+                    } else {
+                        builder.addFormDataPart(entry.getKey(), propValue);
+                    }
+                }
+                if (contentKey != null) {
+                    ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();

Review comment:
       ok all set, pushed.  Sorry about that.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] ottobackwards commented on a change in pull request #4234: NIFI-7394 InvokeHTTP Processor multipart/form-data support

Posted by GitBox <gi...@apache.org>.
ottobackwards commented on a change in pull request #4234:
URL: https://github.com/apache/nifi/pull/4234#discussion_r416051173



##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java
##########
@@ -131,9 +135,19 @@
     @WritesAttribute(attribute = "invokehttp.java.exception.message", description = "The Java exception message raised when the processor fails"),
     @WritesAttribute(attribute = "user-defined", description = "If the 'Put Response Body In Attribute' property is set then whatever it is set to "
         + "will become the attribute key and the value would be the body of the HTTP response.")})
-@DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
-                    description = "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
-                            + "the Attribute Expression Language set in the value of the Dynamic Property.")
+@DynamicProperties ({
+    @DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
+                + "the Attribute Expression Language set in the value of the Dynamic Property."),
+    @DynamicProperty(name = "post.form.<NAME>", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "When the HTTP Method is POST, dynamic properties with the property name in the form of post.form.<NAME>,"
+                + " where the <NAME> will be the form data name, will be used to fill out the multipart form parts."
+                + "  If the value is the literal 'FLOWFILE_CONTENT', that form part will have the flowfile's data."

Review comment:
       I agree that this is more awkward than I would like, but that is a general problem with the way that properties work.
   
   A form can have an arbitrary number of fields or parts.  It also may not be for uploading a document.  So to make it broadly useful you have to be able to define multiple fields.  Given the way that flowfiles work, I think only being able to define 1 'file upload' field for the flow file is good enough.
   
   I thought that using this technique, like I did for the ExecuteStreamCommand to support arguments and fix the escaping issue, would allow for having _N_ parts, with EL support available.  Now, new properties for the things that I have as known constant values _is_ a better way to go, but I was hesitant to add new properties.
   
   I think the most flexible solution is to use the dynamic parameters, with new parameters for the content  Content form name, Content file name.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] ottobackwards commented on pull request #4234: NIFI-7394 InvokeHTTP Processor multipart/form-data support

Posted by GitBox <gi...@apache.org>.
ottobackwards commented on pull request #4234:
URL: https://github.com/apache/nifi/pull/4234#issuecomment-620665819


   @markap14 I believe I have addressed everything, and this should be ready for re-review.  With the amount of changes, I'm sure it can still be better. 


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] markap14 commented on a change in pull request #4234: NIFI-7394 InvokeHTTP Processor multipart/form-data support

Posted by GitBox <gi...@apache.org>.
markap14 commented on a change in pull request #4234:
URL: https://github.com/apache/nifi/pull/4234#discussion_r416624881



##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java
##########
@@ -131,9 +135,19 @@
     @WritesAttribute(attribute = "invokehttp.java.exception.message", description = "The Java exception message raised when the processor fails"),
     @WritesAttribute(attribute = "user-defined", description = "If the 'Put Response Body In Attribute' property is set then whatever it is set to "
         + "will become the attribute key and the value would be the body of the HTTP response.")})
-@DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
-                    description = "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
-                            + "the Attribute Expression Language set in the value of the Dynamic Property.")
+@DynamicProperties ({
+    @DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
+                + "the Attribute Expression Language set in the value of the Dynamic Property."),
+    @DynamicProperty(name = "post.form.<NAME>", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "When the HTTP Method is POST, dynamic properties with the property name in the form of post.form.<NAME>,"
+                + " where the <NAME> will be the form data name, will be used to fill out the multipart form parts."
+                + "  If the value is the literal 'FLOWFILE_CONTENT', that form part will have the flowfile's data."
+                + "  The property name post.form.filename will set the filename to set for the flowfile content."

Review comment:
       Correct. I would add a property that indicates whether or not to send the filename (probably would default to true??)




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] ottobackwards commented on a change in pull request #4234: NIFI-7394 InvokeHTTP Processor multipart/form-data support

Posted by GitBox <gi...@apache.org>.
ottobackwards commented on a change in pull request #4234:
URL: https://github.com/apache/nifi/pull/4234#discussion_r416066144



##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java
##########
@@ -131,9 +135,19 @@
     @WritesAttribute(attribute = "invokehttp.java.exception.message", description = "The Java exception message raised when the processor fails"),
     @WritesAttribute(attribute = "user-defined", description = "If the 'Put Response Body In Attribute' property is set then whatever it is set to "
         + "will become the attribute key and the value would be the body of the HTTP response.")})
-@DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
-                    description = "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
-                            + "the Attribute Expression Language set in the value of the Dynamic Property.")
+@DynamicProperties ({
+    @DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
+                + "the Attribute Expression Language set in the value of the Dynamic Property."),
+    @DynamicProperty(name = "post.form.<NAME>", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "When the HTTP Method is POST, dynamic properties with the property name in the form of post.form.<NAME>,"
+                + " where the <NAME> will be the form data name, will be used to fill out the multipart form parts."
+                + "  If the value is the literal 'FLOWFILE_CONTENT', that form part will have the flowfile's data."

Review comment:
       Also, we could change the name pattern to be "nifi.post.form"




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] markap14 commented on a change in pull request #4234: NIFI-7394 InvokeHTTP Processor multipart/form-data support

Posted by GitBox <gi...@apache.org>.
markap14 commented on a change in pull request #4234:
URL: https://github.com/apache/nifi/pull/4234#discussion_r416636401



##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java
##########
@@ -131,9 +135,19 @@
     @WritesAttribute(attribute = "invokehttp.java.exception.message", description = "The Java exception message raised when the processor fails"),
     @WritesAttribute(attribute = "user-defined", description = "If the 'Put Response Body In Attribute' property is set then whatever it is set to "
         + "will become the attribute key and the value would be the body of the HTTP response.")})
-@DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
-                    description = "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
-                            + "the Attribute Expression Language set in the value of the Dynamic Property.")
+@DynamicProperties ({
+    @DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
+                + "the Attribute Expression Language set in the value of the Dynamic Property."),
+    @DynamicProperty(name = "post.form.<NAME>", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "When the HTTP Method is POST, dynamic properties with the property name in the form of post.form.<NAME>,"
+                + " where the <NAME> will be the form data name, will be used to fill out the multipart form parts."
+                + "  If the value is the literal 'FLOWFILE_CONTENT', that form part will have the flowfile's data."

Review comment:
       Or, if you want to be even more clever about not breaking backward compatibility you could use the notation "post:form:<name>" because other user-defined properties are treated as headers, and colons are not legal in header names (https://www.w3.org/Protocols/rfc2616/rfc2616-sec2.html#sec2 specifies what a 'token' is and https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html specifies that a header name is simply a 'token').




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] markap14 commented on a change in pull request #4234: NIFI-7394 InvokeHTTP Processor multipart/form-data support

Posted by GitBox <gi...@apache.org>.
markap14 commented on a change in pull request #4234:
URL: https://github.com/apache/nifi/pull/4234#discussion_r416624003



##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java
##########
@@ -131,9 +135,19 @@
     @WritesAttribute(attribute = "invokehttp.java.exception.message", description = "The Java exception message raised when the processor fails"),
     @WritesAttribute(attribute = "user-defined", description = "If the 'Put Response Body In Attribute' property is set then whatever it is set to "
         + "will become the attribute key and the value would be the body of the HTTP response.")})
-@DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
-                    description = "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
-                            + "the Attribute Expression Language set in the value of the Dynamic Property.")
+@DynamicProperties ({
+    @DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
+                + "the Attribute Expression Language set in the value of the Dynamic Property."),
+    @DynamicProperty(name = "post.form.<NAME>", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "When the HTTP Method is POST, dynamic properties with the property name in the form of post.form.<NAME>,"
+                + " where the <NAME> will be the form data name, will be used to fill out the multipart form parts."
+                + "  If the value is the literal 'FLOWFILE_CONTENT', that form part will have the flowfile's data."
+                + "  The property name post.form.filename will set the filename to set for the flowfile content."
+                + "  If send message body is false, the properties will be ignored.")

Review comment:
       I think it makes perfect sense to allow sending of form parameters but not the FlowFile content.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] markap14 commented on a change in pull request #4234: NIFI-7394 InvokeHTTP Processor multipart/form-data support

Posted by GitBox <gi...@apache.org>.
markap14 commented on a change in pull request #4234:
URL: https://github.com/apache/nifi/pull/4234#discussion_r416038768



##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java
##########
@@ -131,9 +135,19 @@
     @WritesAttribute(attribute = "invokehttp.java.exception.message", description = "The Java exception message raised when the processor fails"),
     @WritesAttribute(attribute = "user-defined", description = "If the 'Put Response Body In Attribute' property is set then whatever it is set to "
         + "will become the attribute key and the value would be the body of the HTTP response.")})
-@DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
-                    description = "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
-                            + "the Attribute Expression Language set in the value of the Dynamic Property.")
+@DynamicProperties ({
+    @DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
+                + "the Attribute Expression Language set in the value of the Dynamic Property."),
+    @DynamicProperty(name = "post.form.<NAME>", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "When the HTTP Method is POST, dynamic properties with the property name in the form of post.form.<NAME>,"
+                + " where the <NAME> will be the form data name, will be used to fill out the multipart form parts."
+                + "  If the value is the literal 'FLOWFILE_CONTENT', that form part will have the flowfile's data."
+                + "  The property name post.form.filename will set the filename to set for the flowfile content."
+                + "  If send message body is false, the properties will be ignored.")

Review comment:
       Which properties will be ignored? All post.form.<name> properties or just the one with a value of FLOWFILE_CONTENT and the post.form.filename?

##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java
##########
@@ -131,9 +135,19 @@
     @WritesAttribute(attribute = "invokehttp.java.exception.message", description = "The Java exception message raised when the processor fails"),
     @WritesAttribute(attribute = "user-defined", description = "If the 'Put Response Body In Attribute' property is set then whatever it is set to "
         + "will become the attribute key and the value would be the body of the HTTP response.")})
-@DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
-                    description = "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
-                            + "the Attribute Expression Language set in the value of the Dynamic Property.")
+@DynamicProperties ({
+    @DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
+                + "the Attribute Expression Language set in the value of the Dynamic Property."),
+    @DynamicProperty(name = "post.form.<NAME>", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "When the HTTP Method is POST, dynamic properties with the property name in the form of post.form.<NAME>,"
+                + " where the <NAME> will be the form data name, will be used to fill out the multipart form parts."
+                + "  If the value is the literal 'FLOWFILE_CONTENT', that form part will have the flowfile's data."
+                + "  The property name post.form.filename will set the filename to set for the flowfile content."

Review comment:
       It's a bit of bad form to have a user-defined property with a well-known name. If the name is well-known it should not be user-defined. To be honest, I wouldn't even make this a configurable option. If the user wants a different filename sent, they should change the filename attribute of the FlowFile. This is how it's handled in almost all processors that deal with filenames. E.g., PutFile, PutHDFS, etc. 

##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java
##########
@@ -131,9 +135,19 @@
     @WritesAttribute(attribute = "invokehttp.java.exception.message", description = "The Java exception message raised when the processor fails"),
     @WritesAttribute(attribute = "user-defined", description = "If the 'Put Response Body In Attribute' property is set then whatever it is set to "
         + "will become the attribute key and the value would be the body of the HTTP response.")})
-@DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
-                    description = "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
-                            + "the Attribute Expression Language set in the value of the Dynamic Property.")
+@DynamicProperties ({
+    @DynamicProperty(name = "Header Name", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "Send request header with a key matching the Dynamic Property Key and a value created by evaluating "
+                + "the Attribute Expression Language set in the value of the Dynamic Property."),
+    @DynamicProperty(name = "post.form.<NAME>", value = "Attribute Expression Language", expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES,
+        description =
+            "When the HTTP Method is POST, dynamic properties with the property name in the form of post.form.<NAME>,"
+                + " where the <NAME> will be the form data name, will be used to fill out the multipart form parts."
+                + "  If the value is the literal 'FLOWFILE_CONTENT', that form part will have the flowfile's data."

Review comment:
       What do you think about instead adding a new, optional property along the lines of "Content Multipart/Form Name", along with maybe another property that indicates whether or not use Multipart/Form encoding?
   Going that route, I think, may make the configuration a little bit annoying. But the reason that I suggest it is two-fold:
   1) As-is, this PR changes the behavior of the user-defined properties. If someone already was using a post.form.XYZ property we'd change behavior. This is only a slight concern given that it would be a pretty weird HTTP Header to send :)
   2) It's difficult to remember the exact string contents "FLOWFILE_CONTENT" and if there's ever a typo, the consequences could be pretty bad. The user will think everything was properly uploaded and have no idea that they are not actually sending the FlowFile's content but are sending some string such as FLOWFIEL_CONTENT every time.

##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java
##########
@@ -1025,11 +1060,20 @@ private Request configureRequest(final ProcessContext context, final ProcessSess
 
     private RequestBody getRequestBodyToSend(final ProcessSession session, final ProcessContext context, final FlowFile requestFlowFile) {
         if(context.getProperty(PROP_SEND_BODY).asBoolean()) {
-            return new RequestBody() {
+            String evalContentType = context.getProperty(PROP_CONTENT_TYPE)
+                .evaluateAttributeExpressions(requestFlowFile).getValue();
+            final String contentType = StringUtils.isBlank(evalContentType) ? DEFAULT_CONTENT_TYPE : evalContentType;
+            HashMap<String,PropertyDescriptor> propertyDescriptors = new HashMap<>();

Review comment:
       Best to declare as a `Map<String, PropertyDescriptor>` rather than `HashMap`




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] ottobackwards commented on a change in pull request #4234: NIFI-7394 InvokeHTTP Processor multipart/form-data support

Posted by GitBox <gi...@apache.org>.
ottobackwards commented on a change in pull request #4234:
URL: https://github.com/apache/nifi/pull/4234#discussion_r416030191



##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java
##########
@@ -1039,10 +1084,39 @@ public void writeTo(BufferedSink sink) throws IOException {
                 }
 
                 @Override
-                public long contentLength(){
+                public long contentLength() {
                     return useChunked ? -1 : requestFlowFile.getSize();
                 }
             };
+
+            if (propertyDescriptors.size() > 0) {
+                // we have form data
+                MultipartBody.Builder builder = new Builder().setType(MultipartBody.FORM);
+                String contentKey = null;
+                String contentFileName = null;
+
+                // loop through the dynamic form parameters
+                // we can't add the content before we know if there is a file name or not, so we
+                // get the keys and do them after this
+                for (final Map.Entry<String, PropertyDescriptor> entry : propertyDescriptors.entrySet()) {
+                    final String propValue = context.getProperty(entry.getValue().getName()).evaluateAttributeExpressions(requestFlowFile).getValue();
+                    if (propValue.equals(FLOWFILE_CONTENT_FORM_MARKER)) {
+                        contentKey = entry.getKey();
+                    } else if (entry.getValue().getName().equals(FLOWFILE_CONTENT_FORM_FILE_NAME)) {
+                        contentFileName = propValue;
+                    } else {
+                        builder.addFormDataPart(entry.getKey(), propValue);
+                    }
+                }
+                if (contentKey != null) {
+                    ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();

Review comment:
       I am sorry, I thought I had changed that, let me fix it
   




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] ottobackwards commented on pull request #4234: NIFI-7394 InvokeHTTP Processor multipart/form-data support

Posted by GitBox <gi...@apache.org>.
ottobackwards commented on pull request #4234:
URL: https://github.com/apache/nifi/pull/4234#issuecomment-620144998


   @markap14, it is an artifact.  My first implementation did do that, but I knew it was wrong.  I refactored to the current way after looking through the Multipart api a bit.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] markap14 commented on pull request #4234: NIFI-7394 InvokeHTTP Processor multipart/form-data support

Posted by GitBox <gi...@apache.org>.
markap14 commented on pull request #4234:
URL: https://github.com/apache/nifi/pull/4234#issuecomment-621340558


   Thanks @ottobackwards for improving this - and for working through it with me to ensure that we come up with a solution that we're all happy with. I think this is a huge improvement and will be much appreciated throughout the community! +1 have merged to master.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] ottobackwards edited a comment on pull request #4234: NIFI-7394 InvokeHTTP Processor multipart/form-data support

Posted by GitBox <gi...@apache.org>.
ottobackwards edited a comment on pull request #4234:
URL: https://github.com/apache/nifi/pull/4234#issuecomment-620144998


   @markap14, it is an artifact.  My first implementation did do that, but I knew it was wrong.  I refactored to the current way after looking through the Multipart api a bit more.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] ottobackwards commented on a change in pull request #4234: NIFI-7394 InvokeHTTP Processor multipart/form-data support

Posted by GitBox <gi...@apache.org>.
ottobackwards commented on a change in pull request #4234:
URL: https://github.com/apache/nifi/pull/4234#discussion_r416054817



##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java
##########
@@ -1025,11 +1060,20 @@ private Request configureRequest(final ProcessContext context, final ProcessSess
 
     private RequestBody getRequestBodyToSend(final ProcessSession session, final ProcessContext context, final FlowFile requestFlowFile) {
         if(context.getProperty(PROP_SEND_BODY).asBoolean()) {
-            return new RequestBody() {
+            String evalContentType = context.getProperty(PROP_CONTENT_TYPE)
+                .evaluateAttributeExpressions(requestFlowFile).getValue();
+            final String contentType = StringUtils.isBlank(evalContentType) ? DEFAULT_CONTENT_TYPE : evalContentType;
+            HashMap<String,PropertyDescriptor> propertyDescriptors = new HashMap<>();

Review comment:
       yikes, good point. done




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] ottobackwards commented on pull request #4234: NIFI-7394 InvokeHTTP Processor multipart/form-data support

Posted by GitBox <gi...@apache.org>.
ottobackwards commented on pull request #4234:
URL: https://github.com/apache/nifi/pull/4234#issuecomment-621349469


   That is great @markap14!  That is the whole point of the reviews, we are lucky to have them.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org