nandorsoma commented on a change in pull request #5458:
URL: https://github.com/apache/nifi/pull/5458#discussion_r750791129
##########
File path:
nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/ConsumeAMQP.java
##########
@@ -192,6 +220,48 @@ private void addAttribute(final Map<String, String>
attributes, final String att
attributes.put(attributeName, value.toString());
}
+ private String buildHeaders(Map<String, Object> headers, boolean
escapeComma, boolean removeCurlyBraces) {
+ if (headers == null) {
+ return null;
+ }
+ if (escapeComma && removeCurlyBraces) {
+ return headers.keySet().stream()
+ .map(key -> key + "=" +
escapeString(headers.get(key).toString()))
+ .collect(Collectors.joining(", "));
+ } else if (escapeComma) {
+ return headers.keySet().stream()
+ .map(key -> key + "=" +
escapeString(headers.get(key).toString()))
+ .collect(Collectors.joining(", ", "{", "}"));
+ } else if (removeCurlyBraces) {
+ String headerString = headers.toString();
+ return headerString.substring(1, headerString.length() - 1);
Review comment:
You could make this part a bit fault tolerant by removing the first and
last characters only if they are really curly braces. What do you think?
##########
File path:
nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/PublishAMQP.java
##########
@@ -99,6 +101,15 @@
.addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
.build();
+ static final PropertyDescriptor UNESCAPE_COMMA = new
PropertyDescriptor.Builder()
Review comment:
I was a bit confused about the name of the property. The goal is to
escape a comma in the header so the processor won't threat that comma in the
header as a separator. Wouldn't IGNORE_ESCAPED_COMMA_IN_HEADER reflect that
intention better?
##########
File path:
nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/ConsumeAMQP.java
##########
@@ -192,6 +220,48 @@ private void addAttribute(final Map<String, String>
attributes, final String att
attributes.put(attributeName, value.toString());
}
+ private String buildHeaders(Map<String, Object> headers, boolean
escapeComma, boolean removeCurlyBraces) {
+ if (headers == null) {
+ return null;
+ }
+ if (escapeComma && removeCurlyBraces) {
+ return headers.keySet().stream()
+ .map(key -> key + "=" +
escapeString(headers.get(key).toString()))
Review comment:
On the publish side, based on the property, the intention is to allow
comma in the header. It doesn't specify that we want to allow it in the key or
in the value. Now I'm able to put an escaped comma into the key on the
Publisher side, but here only the key will be escaped.
So, while on the publisher side the property looks like that:
```
amqp$headers=foo\,foo=(bar\,bar)
```
After processing in the consumer's success flowFile it will look like that:
```
amqp$headers=foo,foo=(bar\,bar)
```
I think you either need to specify that escaped comma is only allowed in the
value or you need to escape the key here. I prefer the first one, because comma
in the key is undesirable I think.
##########
File path:
nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/ConsumeAMQP.java
##########
@@ -99,6 +103,28 @@
.required(true)
.build();
+ static final PropertyDescriptor ESCAPE_COMMA = new
PropertyDescriptor.Builder()
+ .name("escape.comma")
+ .displayName("Escape Comma")
+ .description("When there is a comma in the header itself, with the
help of this parameter, the header's own commas are escaped. "
+ + "When this parameter is selected true, the Unescape Comma
parameter in PublishAMQP processor must be set to true ")
Review comment:
Based on the second sentence for the first sight it seems like this
feature is only working between NiFi publishers and consumers. What happens
when NiFi consumes a message that is produced by a different publisher? Is it a
valid scenario?
--
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: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
