Merge pull request twitter#102 from miguno/issue65

Issue 65: LzoJsonRecordReader and JsonLoader may fail with NullPointerException

src/java/com/twitter/elephantbird/mapreduce/input/LzoJsonRecordReader.java

@@ -89,14 +89,23 @@ public boolean nextKeyValue() throws IOException, InterruptedException {
   public static boolean decodeLineToJson(JSONParser parser, Text line, MapWritable value) {
     try {
       JSONObject jsonObj = (JSONObject)parser.parse(line.toString());
-      for (Object key: jsonObj.keySet()) {
-        Text mapKey = new Text(key.toString());
-        Text mapValue = new Text();
-        if (jsonObj.get(key) != null) {
-          mapValue.set(jsonObj.get(key).toString());
+      if (jsonObj != null) {
+        for (Object key: jsonObj.keySet()) {
+          Text mapKey = new Text(key.toString());
+          Text mapValue = new Text();
+          if (jsonObj.get(key) != null) {
+            mapValue.set(jsonObj.get(key).toString());
+          }
+
+          value.put(mapKey, mapValue);
         }
-
-        value.put(mapKey, mapValue);
+      }
+      else {
+          // JSONParser#parse(String) may return a null reference, e.g. when
+          // the input parameter is the string "null".  A single line with
+          // "null" is not valid JSON though.
+          LOG.warn("Could not json-decode string: " + line);
+          return false;
       }
       return true;
     } catch (ParseException e) {

src/java/com/twitter/elephantbird/pig/load/JsonLoader.java

@@ -95,9 +95,19 @@ protected Tuple parseStringToTuple(String line) {
     try {
       Map<String, String> values = Maps.newHashMap();
       JSONObject jsonObj = (JSONObject)jsonParser_.parse(line);
-      for (Object key: jsonObj.keySet()) {
-        Object value = jsonObj.get(key);
-        values.put(key.toString(), value != null ? value.toString() : null);
+      if (jsonObj != null) {
+        for (Object key: jsonObj.keySet()) {
+          Object value = jsonObj.get(key);
+          values.put(key.toString(), value != null ? value.toString() : null);
+        }
+      }
+      else {
+          // JSONParser#parse(String) may return a null reference, e.g. when
+          // the input parameter is the string "null".  A single line with
+          // "null" is not valid JSON though.
+          LOG.warn("Could not json-decode string: " + line);
+          incrCounter(JsonLoaderCounters.LinesParseError, 1L);
+          return null;
       }
       return tupleFactory_.newTuple(values);
     } catch (ParseException e) {

src/test/com/twitter/elephantbird/mapreduce/input/TestLzoJsonRecordReader.java

@@ -0,0 +1,31 @@
+package com.twitter.elephantbird.mapreduce.input;
+
+import junit.framework.TestCase;
+import org.apache.hadoop.io.MapWritable;
+import org.apache.hadoop.io.Text;
+import org.json.simple.parser.JSONParser;
+import org.junit.Test;
+
+/**
+ * Test the LzoJsonRecordReader, make sure it reads the data properly.
+ */
+public class TestLzoJsonRecordReader extends TestCase {
+
+  /**
+   * {@link LzoJsonRecordReader#decodeLineToJson(JSONParser, Text, MapWritable)}
+   * must not choke on lines containing the word "null" (i.e. not the null
+   * value but the string "null").
+   *
+   * This can happen when the original input line to JSONParser contains "null"
+   * as a string.  In this case {@link JSONParser#parse(java.io.Reader)} will
+   * return a null reference.
+   *
+   */
+  @Test
+  public void testNullString() {
+      Text line = new Text("null");
+      boolean result = LzoJsonRecordReader.decodeLineToJson(new JSONParser(), line, new MapWritable());
+      assertEquals("Parsing line with contents 'null'", false, result);
+  }
+
+}

src/test/com/twitter/elephantbird/pig/load/TestJsonLoader.java

@@ -0,0 +1,32 @@
+package com.twitter.elephantbird.pig.load;
+
+import junit.framework.TestCase;
+import org.apache.pig.data.Tuple;
+import org.json.simple.parser.JSONParser;
+import org.junit.Test;
+
+/**
+ * Test the JsonLoader, make sure it reads the data properly.
+ */
+
+public class TestJsonLoader extends TestCase {
+
+  /**
+   * {@link JsonLoader#parseStringToTuple(String)} must not choke on lines
+   * containing the word "null" (i.e. not the null value but the string
+   * "null").
+   *
+   * This can happen when the original input line to JSONParser contains "null"
+   * as a string.  In this case {@link JSONParser#parse(java.io.Reader)} will
+   * return a null reference.
+   *
+   */
+  @Test
+  public void testNullString() {
+      String nullString = "null";
+      JsonLoader jsonLoader = new JsonLoader();
+      Tuple result = jsonLoader.parseStringToTuple(nullString);
+      assertEquals("Parsing line with contents 'null'", null, result);
+  }
+
+}