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

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to