Merge pull request #44 from treasure-data/PLT-10795_fix_npe_when_merg…

…e_fields_are_undefined_on_mailchimp Fix NPE when merge fields are undefined on Mailchimp
treasure-data · Sep 26, 2018 · 039be40 · 039be40
2 parents 2333134 + f188f11
commit 039be40
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 23 deletions.
diff --git a/src/main/java/org/embulk/output/mailchimp/MailChimpRecordBuffer.java b/src/main/java/org/embulk/output/mailchimp/MailChimpRecordBuffer.java
@@ -33,9 +33,11 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.TreeMap;
 
+import static java.lang.String.CASE_INSENSITIVE_ORDER;
+import static java.lang.String.format;
 import static org.embulk.output.mailchimp.MailChimpOutputPluginDelegate.PluginTask;
-import static org.embulk.output.mailchimp.helper.MailChimpHelper.containsCaseInsensitive;
 import static org.embulk.output.mailchimp.helper.MailChimpHelper.fromCommaSeparatedString;
 import static org.embulk.output.mailchimp.helper.MailChimpHelper.orderJsonNode;
 import static org.embulk.output.mailchimp.helper.MailChimpHelper.toJsonNode;
@@ -206,26 +208,38 @@ public JsonNode apply(JsonNode input)
 
                 // Update additional merge fields if exist
                 if (task.getMergeFields().isPresent() && !task.getMergeFields().get().isEmpty()) {
-                    for (final Column column : schema.getColumns()) {
-                        if (!"".equals(containsCaseInsensitive(column.getName(), task.getMergeFields().get()))) {
-                            String value = input.hasNonNull(column.getName()) ? input.findValue(column.getName()).asText() : "";
-
-                            // Try to convert to Json from string with the merge field's type is address
-                            if (availableMergeFields.get(column.getName().toLowerCase()).getType()
-                                    .equals(MergeField.MergeFieldType.ADDRESS.getType())) {
-                                JsonNode addressNode = toJsonNode(value);
-                                if (addressNode instanceof NullNode) {
-                                    mergeFields.put(column.getName().toUpperCase(), value);
-                                }
-                                else {
-                                    mergeFields.set(column.getName().toUpperCase(),
-                                                    orderJsonNode(addressNode, AddressMergeFieldAttribute.values()));
-                                }
+                    Map<String, String> columnNameLookup = new TreeMap<>(CASE_INSENSITIVE_ORDER);
+                    for (Column col : schema.getColumns()) {
+                        columnNameLookup.put(col.getName(), col.getName());
+                    }
+                    for (String field : task.getMergeFields().get()) {
+                        if (!columnNameLookup.containsKey(field)) {
+                            LOG.warn(format("Field '%s' is configured on data transfer but cannot be found on any columns.", field));
+                            continue;
+                        }
+                        String columnName = columnNameLookup.get(field);
+                        if (!availableMergeFields.containsKey(columnName.toLowerCase())) {
+                            LOG.warn(format("Field '%s' is configured on data transfer but is not predefined on Mailchimp.", field));
+                            continue;
+                        }
+
+                        String value = input.hasNonNull(columnName) ? input.findValue(columnName).asText() : "";
+
+                        // Try to convert to Json from string with the merge field's type is address
+                        if (availableMergeFields.get(columnName).getType()
+                                .equals(MergeField.MergeFieldType.ADDRESS.getType())) {
+                            JsonNode addressNode = toJsonNode(value);
+                            if (addressNode instanceof NullNode) {
+                                mergeFields.put(columnName.toUpperCase(), value);
                             }
                             else {
-                                mergeFields.put(column.getName().toUpperCase(), value);
+                                mergeFields.set(columnName.toUpperCase(),
+                                        orderJsonNode(addressNode, AddressMergeFieldAttribute.values()));
                             }
                         }
+                        else {
+                            mergeFields.put(columnName.toUpperCase(), value);
+                        }
                     }
                 }
 

diff --git a/src/main/java/org/embulk/output/mailchimp/helper/MailChimpHelper.java b/src/main/java/org/embulk/output/mailchimp/helper/MailChimpHelper.java
@@ -45,15 +45,14 @@ public static String maskEmail(final String email)
      * @param list the list
      * @return the boolean
      */
-    public static String containsCaseInsensitive(final String s, final List<String> list)
+    public static boolean containsCaseInsensitive(final String s, final List<String> list)
     {
         for (String string : list) {
             if (string.equalsIgnoreCase(s)) {
-                return string;
+                return true;
             }
         }
-
-        return "";
+        return false;
     }
 
     /**

diff --git a/src/test/java/org/embulk/output/mailchimp/TestMailChimpHelper.java b/src/test/java/org/embulk/output/mailchimp/TestMailChimpHelper.java
@@ -20,6 +20,7 @@
 import static org.embulk.output.mailchimp.helper.MailChimpHelper.toJsonNode;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 /**
  * Created by thangnc on 4/26/17.
@@ -37,8 +38,7 @@ public void test_maskEmail_validInErrorResponse()
     @Test
     public void test_containsCaseInsensitive_validMergeFields()
     {
-        String expect = "United State";
-        assertEquals("Interest category should match", expect, containsCaseInsensitive("united state",
+        assertTrue("Interest category should match", containsCaseInsensitive("united state",
                                                                                        Arrays.asList("Donating", "United State")));
     }