[GitHub] [nifi] sedadgn commented on a change in pull request #5458: NIFI-7865 amqp$header is splitted in the wrong way for "," and "}"
sedadgn commented on a change in pull request #5458:
URL: https://github.com/apache/nifi/pull/5458#discussion_r784726662
##
File path:
nifi-commons/nifi-utils/src/main/java/org/apache/nifi/processor/util/StandardValidators.java
##
@@ -980,4 +980,25 @@ public ValidationResult validate(final String subject,
final String value, final
return new
ValidationResult.Builder().subject(subject).input(value).explanation(reason).valid(reason
== null).build();
}
}
+
+public static final Validator SINGLE_CHAR_VALIDATOR = (subject, input,
context) -> {
Review comment:
I moved the method
--
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
[GitHub] [nifi] sedadgn commented on a change in pull request #5458: NIFI-7865 amqp$header is splitted in the wrong way for "," and "}"
sedadgn commented on a change in pull request #5458: URL: https://github.com/apache/nifi/pull/5458#discussion_r757455310 ## File path: nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/pom.xml ## @@ -21,6 +21,8 @@ language governing permissions and limitations under the License. --> 5.8.0 +1.8 Review comment: > Hey @sedadgn! Thank you for implementing the alternative approach. I've found few things, can you check them? None of them is a big thing so I also think we are really close to merging. One slight thing. There are a lot of inconsistently placed empty lines, missing spaces. They are not considered blocker issues in the community but can I ask you to clean them? I'd appreciate if you could do. @nandorsoma Thank you for your review. I wrote few comment. I made few changing according your comment. Best Regards -- 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
[GitHub] [nifi] sedadgn commented on a change in pull request #5458: NIFI-7865 amqp$header is splitted in the wrong way for "," and "}"
sedadgn commented on a change in pull request #5458:
URL: https://github.com/apache/nifi/pull/5458#discussion_r757453945
##
File path:
nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/PublishAMQP.java
##
@@ -240,20 +254,20 @@ private BasicProperties
extractAmqpPropertiesFromFlowFile(FlowFile flowFile) {
updateBuilderFromAttribute(flowFile, "userId", builder::userId);
updateBuilderFromAttribute(flowFile, "appId", builder::appId);
updateBuilderFromAttribute(flowFile, "clusterId", builder::clusterId);
-updateBuilderFromAttribute(flowFile, "headers", headers ->
builder.headers(validateAMQPHeaderProperty(headers)));
+updateBuilderFromAttribute(flowFile, "headers", headers ->
builder.headers(validateAMQPHeaderProperty(headers,headerSeparator)));
return builder.build();
}
/**
* Will validate if provided amqpPropValue can be converted to a {@link
Map}.
- * Should be passed in the format: amqp$headers=key=value,key=value etc.
- *
+ * Should be passed in the format: amqp$headers=key=value
+ * @param splitValue is used to split for property
* @param amqpPropValue the value of the property
* @return {@link Map} if valid otherwise null
*/
-private Map validateAMQPHeaderProperty(String
amqpPropValue) {
-String[] strEntries = amqpPropValue.split(",");
+private Map validateAMQPHeaderProperty(String
amqpPropValue,Character splitValue) {
+String[] strEntries =
amqpPropValue.split(Pattern.quote(String.valueOf(splitValue)));
Review comment:
Pattern.quote create the escaped version of the regex representing.
For Example '|'.
Also you can look at this.
https://stackoverflow.com/questions/10796160/splitting-a-java-string-by-the-pipe-symbol-using-split
##
File path:
nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/test/java/org/apache/nifi/amqp/processors/ConsumeAMQPTest.java
##
@@ -161,6 +165,138 @@ public void
validateSuccessfulConsumeAndTransferToSuccess() throws Exception {
}
}
+@Test
+public void
validateHeaderWithValueSeparatorForHeaderParameterConsumeAndTransferToSuccess()
throws Exception {
+final Map> routingMap =
Collections.singletonMap("key1", Arrays.asList("queue1", "queue2"));
+final Map exchangeToRoutingKeymap =
Collections.singletonMap("myExchange", "key1");
+final Map headersMap = new HashMap<>();
+headersMap.put("foo1","bar,bar");
+headersMap.put("foo2","bar,bar");
+
+AMQP.BasicProperties.Builder builderBasicProperties = new
AMQP.BasicProperties.Builder();
+builderBasicProperties.headers(headersMap);
+
+final Connection connection = new
TestConnection(exchangeToRoutingKeymap, routingMap);
+
+try (AMQPPublisher sender = new AMQPPublisher(connection,
mock(ComponentLog.class))) {
+sender.publish("hello".getBytes(), builderBasicProperties.build(),
"key1", "myExchange");
+
+ConsumeAMQP proc = new LocalConsumeAMQP(connection);
+TestRunner runner = initTestRunner(proc);
+runner.setProperty(ConsumeAMQP.HEADER_SEPARATOR, "|");
+runner.run();
+final MockFlowFile successFF =
runner.getFlowFilesForRelationship(PublishAMQP.REL_SUCCESS).get(0);
+assertNotNull(successFF);
+successFF.assertAttributeEquals("amqp$routingKey", "key1");
+successFF.assertAttributeEquals("amqp$exchange", "myExchange");
+String headers = successFF.getAttribute("amqp$headers");
+Map properties =
Splitter.on("|").withKeyValueSeparator("=").split(headers.substring(1,headers.length()-1));
+Assert.assertEquals(headersMap,properties);
+
+}
+}
+@Test
+public void validateWithNotValidHeaderSeparatorParameter() {
+final Map> routingMap =
Collections.singletonMap("key1", Arrays.asList("queue1", "queue2"));
+final Map exchangeToRoutingKeymap =
Collections.singletonMap("myExchange", "key1");
+final Connection connection = new
TestConnection(exchangeToRoutingKeymap, routingMap);
+ConsumeAMQP proc = new LocalConsumeAMQP(connection);
+TestRunner runner = initTestRunner(proc);
+runner.setProperty(ConsumeAMQP.HEADER_SEPARATOR, "|,");
+runner.assertNotValid();
+}
+
+@Test
+public void
validateHeaderWithRemoveCurlyBracesParameterConsumeAndTransferToSuccess()
throws Exception {
+final Map> routingMap =
Collections.singletonMap("key1", Arrays.asList("queue1", "queue2"));
+final Map exchangeToRoutingKeymap =
Collections.singletonMap("myExchange", "key1");
+final Map headersMap = new HashMap<>();
+headersMap.put("key1","(bar,bar)");
+AMQP.BasicProperties.Builder builderBasicProperties = new
AMQP.BasicProperties.Builder();
+
[GitHub] [nifi] sedadgn commented on a change in pull request #5458: NIFI-7865 amqp$header is splitted in the wrong way for "," and "}"
sedadgn commented on a change in pull request #5458:
URL: https://github.com/apache/nifi/pull/5458#discussion_r751285499
##
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
attributes, final String att
attributes.put(attributeName, value.toString());
}
+private String buildHeaders(Map 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 are right. I added the control for first and last characters
--
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
[GitHub] [nifi] sedadgn commented on a change in pull request #5458: NIFI-7865 amqp$header is splitted in the wrong way for "," and "}"
sedadgn commented on a change in pull request #5458:
URL: https://github.com/apache/nifi/pull/5458#discussion_r751283372
##
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
attributes, final String att
attributes.put(attributeName, value.toString());
}
+private String buildHeaders(Map 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:
You are right. I changed the parameter names to
ESCAPE_COMMA_VALUE_IN_HEADER and UNESCAPE_COMMA_VALUE_IN_HEADER.
--
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
[GitHub] [nifi] sedadgn commented on a change in pull request #5458: NIFI-7865 amqp$header is splitted in the wrong way for "," and "}"
sedadgn commented on a change in pull request #5458: URL: https://github.com/apache/nifi/pull/5458#discussion_r751282543 ## 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: in PublishAMQP According to this parameter, I unescape the escaped value from ConsumeAMQP. I changed the description. I hope it is more understandable. -- 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
[GitHub] [nifi] sedadgn commented on a change in pull request #5458: NIFI-7865 amqp$header is splitted in the wrong way for "," and "}"
sedadgn commented on a change in pull request #5458:
URL: https://github.com/apache/nifi/pull/5458#discussion_r751279398
##
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:
Our scenario is as follows:
our publisher -> nifi consumer -> transform-> nifi publisher -> our consumer
so now it can work with different publishers. It can also work on its own
when no changes are made to the newly added parameters. But when the
ESCAPE_COMMA_VALUE_IN_HEADER parameter in ConsumeAMQP is true, the
UNESCAPE_COMMA_VALUE_IN_HEADER parameter in PublishAMQP must also be true to
get the comma value as it is entered.
--
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
[GitHub] [nifi] sedadgn commented on a change in pull request #5458: NIFI-7865 amqp$header is splitted in the wrong way for "," and "}"
sedadgn commented on a change in pull request #5458:
URL: https://github.com/apache/nifi/pull/5458#discussion_r740815019
##
File path:
nifi-commons/nifi-properties/src/main/java/org/apache/nifi/util/StringUtils.java
##
@@ -510,4 +514,27 @@ public static String toTitleCase(String input) {
.map(word -> Character.toTitleCase(word.charAt(0)) +
word.substring(1))
.collect(Collectors.joining(" "));
}
+
+/**
+ * Escape {@code str} by replacing occurrences of {@code charToEscape}
with {@code escapeChar+charToEscape}
+ * @param str the input string
+ * @param escapeChar the character used for escaping
+ * @param charToEscape the character that needs to be escaped
+ * @return the escaped string
+ */
+public static String escapeString(String str, char escapeChar, char
charToEscape) {
+if (str == null || str.isEmpty()) {
+return null;
+}
+StringBuilder result = new StringBuilder();
+for (int i=0; i
[GitHub] [nifi] sedadgn commented on a change in pull request #5458: NIFI-7865 amqp$header is splitted in the wrong way for "," and "}"
sedadgn commented on a change in pull request #5458:
URL: https://github.com/apache/nifi/pull/5458#discussion_r740815019
##
File path:
nifi-commons/nifi-properties/src/main/java/org/apache/nifi/util/StringUtils.java
##
@@ -510,4 +514,27 @@ public static String toTitleCase(String input) {
.map(word -> Character.toTitleCase(word.charAt(0)) +
word.substring(1))
.collect(Collectors.joining(" "));
}
+
+/**
+ * Escape {@code str} by replacing occurrences of {@code charToEscape}
with {@code escapeChar+charToEscape}
+ * @param str the input string
+ * @param escapeChar the character used for escaping
+ * @param charToEscape the character that needs to be escaped
+ * @return the escaped string
+ */
+public static String escapeString(String str, char escapeChar, char
charToEscape) {
+if (str == null || str.isEmpty()) {
+return null;
+}
+StringBuilder result = new StringBuilder();
+for (int i=0; i
[GitHub] [nifi] sedadgn commented on a change in pull request #5458: NIFI-7865 amqp$header is splitted in the wrong way for "," and "}"
sedadgn commented on a change in pull request #5458:
URL: https://github.com/apache/nifi/pull/5458#discussion_r740815019
##
File path:
nifi-commons/nifi-properties/src/main/java/org/apache/nifi/util/StringUtils.java
##
@@ -510,4 +514,27 @@ public static String toTitleCase(String input) {
.map(word -> Character.toTitleCase(word.charAt(0)) +
word.substring(1))
.collect(Collectors.joining(" "));
}
+
+/**
+ * Escape {@code str} by replacing occurrences of {@code charToEscape}
with {@code escapeChar+charToEscape}
+ * @param str the input string
+ * @param escapeChar the character used for escaping
+ * @param charToEscape the character that needs to be escaped
+ * @return the escaped string
+ */
+public static String escapeString(String str, char escapeChar, char
charToEscape) {
+if (str == null || str.isEmpty()) {
+return null;
+}
+StringBuilder result = new StringBuilder();
+for (int i=0; i
[GitHub] [nifi] sedadgn commented on a change in pull request #5458: NIFI-7865 amqp$header is splitted in the wrong way for "," and "}"
sedadgn commented on a change in pull request #5458:
URL: https://github.com/apache/nifi/pull/5458#discussion_r738035135
##
File path:
nifi-commons/nifi-properties/src/main/java/org/apache/nifi/util/StringUtils.java
##
@@ -510,4 +514,27 @@ public static String toTitleCase(String input) {
.map(word -> Character.toTitleCase(word.charAt(0)) +
word.substring(1))
.collect(Collectors.joining(" "));
}
+
+/**
+ * Escape {@code str} by replacing occurrences of {@code charToEscape}
with {@code escapeChar+charToEscape}
+ * @param str the input string
+ * @param escapeChar the character used for escaping
+ * @param charToEscape the character that needs to be escaped
+ * @return the escaped string
+ */
+public static String escapeString(String str, char escapeChar, char
charToEscape) {
+if (str == null || str.isEmpty()) {
+return null;
+}
+StringBuilder result = new StringBuilder();
+for (int i=0; i
[GitHub] [nifi] sedadgn commented on a change in pull request #5458: NIFI-7865 amqp$header is splitted in the wrong way for "," and "}"
sedadgn commented on a change in pull request #5458:
URL: https://github.com/apache/nifi/pull/5458#discussion_r736481331
##
File path:
nifi-commons/nifi-properties/src/main/java/org/apache/nifi/util/StringUtils.java
##
@@ -510,4 +514,20 @@ public static String toTitleCase(String input) {
.map(word -> Character.toTitleCase(word.charAt(0)) +
word.substring(1))
.collect(Collectors.joining(" "));
}
+
+public static String escapeString(String str, char escapeChar, char
charToEscape) {
+if (str == null) {
Review comment:
The idea was, that escape followed by unescape always produces the
original string. E.g. unescape(escape("a,b" )) == "a,b". If the original string
is already escaped, then the end result is an escaped string. E.g.
unescape(escape("a,\\,b" )) == "a,\\,b". That would not work if we perform the
check you suggested.
We can add the check if you want, to. For our specific problems, it would
work either way.
--
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
[GitHub] [nifi] sedadgn commented on a change in pull request #5458: NIFI-7865 amqp$header is splitted in the wrong way for "," and "}"
sedadgn commented on a change in pull request #5458:
URL: https://github.com/apache/nifi/pull/5458#discussion_r736481331
##
File path:
nifi-commons/nifi-properties/src/main/java/org/apache/nifi/util/StringUtils.java
##
@@ -510,4 +514,20 @@ public static String toTitleCase(String input) {
.map(word -> Character.toTitleCase(word.charAt(0)) +
word.substring(1))
.collect(Collectors.joining(" "));
}
+
+public static String escapeString(String str, char escapeChar, char
charToEscape) {
+if (str == null) {
Review comment:
The idea was, that escape followed by unescape always produces the
original string. E.g. unescape(escape("a,b" )) == "a,b". If the original string
is already escaped, then the end result is an escaped string. E.g.
unescape(escape("a,\\,b" )) == "a,\,b". That would not work if we perform the
check you suggested.
We can add the check if you want, to. For our specific problems, it would
work either way.
--
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
[GitHub] [nifi] sedadgn commented on a change in pull request #5458: NIFI-7865 amqp$header is splitted in the wrong way for "," and "}"
sedadgn commented on a change in pull request #5458:
URL: https://github.com/apache/nifi/pull/5458#discussion_r736481331
##
File path:
nifi-commons/nifi-properties/src/main/java/org/apache/nifi/util/StringUtils.java
##
@@ -510,4 +514,20 @@ public static String toTitleCase(String input) {
.map(word -> Character.toTitleCase(word.charAt(0)) +
word.substring(1))
.collect(Collectors.joining(" "));
}
+
+public static String escapeString(String str, char escapeChar, char
charToEscape) {
+if (str == null) {
Review comment:
The idea was, that escape followed by unescape always produces the
original string. E.g. unescape(escape("a,b" )) == "a,b". If the original string
is already escaped, then the end result is an escaped string. E.g.
unescape(escape("a,\,b" )) == "a,\,b". That would not work if we perform the
check you suggested.
We can add the check if you want, to. For our specific problems, it would
work either way.
--
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
[GitHub] [nifi] sedadgn commented on a change in pull request #5458: NIFI-7865 amqp$header is splitted in the wrong way for "," and "}"
sedadgn commented on a change in pull request #5458: URL: https://github.com/apache/nifi/pull/5458#discussion_r736476239 ## File path: nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/ConsumeAMQP.java ## @@ -99,6 +102,28 @@ .required(true) .build(); Review comment: The comma is special, because it is used as the separator (for join in ConsumeAMQP and for split in PublishAMQP). Curly braces were added by ConsumeAMQP as first and last character. No other characters have special meaning. -- 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
[GitHub] [nifi] sedadgn commented on a change in pull request #5458: NIFI-7865 amqp$header is splitted in the wrong way for "," and "}"
sedadgn commented on a change in pull request #5458:
URL: https://github.com/apache/nifi/pull/5458#discussion_r736470809
##
File path:
nifi-commons/nifi-properties/src/main/java/org/apache/nifi/util/StringUtils.java
##
@@ -510,4 +514,20 @@ public static String toTitleCase(String input) {
.map(word -> Character.toTitleCase(word.charAt(0)) +
word.substring(1))
.collect(Collectors.joining(" "));
}
+
Review comment:
I am unsure how to do that. The method does not replace, just add a
character. I added a javadoc to explain the parameters instead. Maybe that
helps!
--
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
