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:
[email protected]
