[GitHub] [nifi] nandorsoma commented on a diff in pull request #6382: NIFI-10317 Taking care of NullPointerException if AMQP header value i…

[GitBox <gi...@apache.org>]

nandorsoma commented on code in PR #6382:
URL: https://github.com/apache/nifi/pull/6382#discussion_r1014480991


##########
nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/test/java/org/apache/nifi/amqp/processors/ConsumeAMQPTest.java:
##########
@@ -46,6 +45,17 @@
 
 public class ConsumeAMQPTest {
 
+    @Test
+    public void testMapToStringConversion() {

Review Comment:
   This test is no longer needed since its logic has been incorporated into `validateHeaderWithValueSeparatorForHeaderParameterConsumeAndTransferToSuccess`.



##########
nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/ConsumeAMQP.java:
##########
@@ -213,31 +214,21 @@ private void addAttribute(final Map<String, String> attributes, final String att
         attributes.put(attributeName, value.toString());
     }
 
-    private String buildHeaders(Map<String, Object> headers,  boolean removeCurlyBraces,Character valueSeparatorForHeaders) {
+    private String buildHeaders(Map<String, Object> headers,  boolean removeCurlyBraces, String valueSeparatorForHeaders) {
         if (headers == null) {
             return null;
         }
         String headerString = convertMapToString(headers,valueSeparatorForHeaders);
 
         if (!removeCurlyBraces) {
-            StringBuilder stringBuilder = new StringBuilder();
-            stringBuilder.append("{").append(headerString).append("}");
-            headerString = stringBuilder.toString();
+            headerString = "{" + headerString + "}";
         }
         return headerString;
     }
 
-    public static String convertMapToString(Map<String, Object> headers,Character valueSeparatorForHeaders) {
-        StringBuilder stringBuilder = new StringBuilder();
-        boolean notFirst = false;
-        for (Map.Entry<String, Object> entry : headers.entrySet()) {
-            if (notFirst) {
-                stringBuilder.append(valueSeparatorForHeaders);
-            }
-            stringBuilder.append(entry.getKey()).append("=").append(entry.getValue().toString());
-            notFirst = true;
-        }
-        return stringBuilder.toString();
+    protected static String convertMapToString(Map<String, Object> headers, String valueSeparatorForHeaders) {

Review Comment:
   This method can be private after the proposed changes.



##########
nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/test/java/org/apache/nifi/amqp/processors/ConsumeAMQPTest.java:
##########
@@ -170,8 +180,11 @@ public void validateHeaderWithValueSeparatorForHeaderParameterConsumeAndTransfer
         final Map<String, List<String>> routingMap = Collections.singletonMap("key1", Arrays.asList("queue1", "queue2"));
         final Map<String, String> exchangeToRoutingKeymap = Collections.singletonMap("myExchange", "key1");
         final Map<String, Object> headersMap = new HashMap<>();
-        headersMap.put("foo1","bar,bar");
-        headersMap.put("foo2","bar,bar");
+        headersMap.put("foo1", "bar,bar");
+        headersMap.put("foo2", "bar,bar");
+        headersMap.put("foo3", "null");
+        headersMap.put("foo4", null);
+        String expectedResult = String.format("{%s}", ConsumeAMQP.convertMapToString(headersMap, "|"));

Review Comment:
   I recommend testing against a hardcoded value because this way, we are testing that the processor called this method, but we need to know whether it is doing what we expect.



-- 
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-unsubscribe@nifi.apache.org

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