Make schema evaluation null-proof; avoid adding null values in data extension. (#3723)

Null values in non-null cells should normally not happen, but sometimes they do and
we shouldn't fail miserably in those cases.

Closes #2880.

Co-authored-by: Tom Morris <tfmorris@gmail.com>

extensions/wikidata/src/org/openrefine/wikidata/schema/WbDateVariable.java

@@ -53,6 +53,9 @@ public class WbDateVariable extends WbVariableExpr<TimeValue> {
     @Override
     public TimeValue fromCell(Cell cell, ExpressionContext ctxt)
             throws SkipSchemaExpressionException {
+        if (cell == null || cell.value == null) {
+            throw new SkipSchemaExpressionException();
+        }
         try {
             // parsed dates are accepted by converting them to strings
             return WbDateConstant.parse(cell.value.toString());

extensions/wikidata/src/org/openrefine/wikidata/schema/WbLocationVariable.java

@@ -46,6 +46,9 @@ public class WbLocationVariable extends WbVariableExpr<GlobeCoordinatesValue> {
     @Override
     public GlobeCoordinatesValue fromCell(Cell cell, ExpressionContext ctxt)
             throws SkipSchemaExpressionException {
+        if (cell == null || cell.value == null) {
+            throw new SkipSchemaExpressionException();
+        }
         String expr = cell.value.toString();
         try {
             return WbLocationConstant.parse(expr);

extensions/wikidata/src/org/openrefine/wikidata/schema/WbStringVariable.java

@@ -56,7 +56,7 @@ public class WbStringVariable extends WbVariableExpr<StringValue> {
     @Override
     public StringValue fromCell(Cell cell, ExpressionContext ctxt)
             throws SkipSchemaExpressionException {
-        if (!cell.value.toString().isEmpty()) {
+        if (cell != null && !cell.value.toString().isEmpty()) {
         	String stringValue = cell.value.toString();
         	if (cell.value instanceof Double && ((Double)cell.value) % 1 == 0) {
         		stringValue = Long.toString(((Double)cell.value).longValue());

extensions/wikidata/tests/src/org/openrefine/wikidata/schema/WbDateVariableTest.java

@@ -61,6 +61,16 @@ public class WbDateVariableTest extends WbVariableTest<TimeValue> {
         isSkipped("  2018-XX");
         isSkipped("invalid format");
     }
+    
+    @Test
+    public void testNullStringValue() {
+        isSkipped((String) null);
+    }
+    
+    @Test
+    public void testNullCell() {
+        isSkipped((Cell) null);
+    }
 
     @Test
     public void testNumber() {

extensions/wikidata/tests/src/org/openrefine/wikidata/schema/WbItemVariableTest.java

@@ -82,6 +82,16 @@ public class WbItemVariableTest extends WbVariableTest<ItemIdValue> {
     public void testUnreconciledCell() {
         isSkipped("some value");
     }
+    
+    @Test
+    public void testNullCell() {
+        isSkipped((Cell) null);
+    }
+    
+    @Test
+    public void testNullStringValue() {
+        isSkipped((String) null);
+    }
 
     @Test
     public void testSerialize() {

extensions/wikidata/tests/src/org/openrefine/wikidata/schema/WbLanguageVariableTest.java

@@ -26,6 +26,8 @@ package org.openrefine.wikidata.schema;
 import org.openrefine.wikidata.testing.JacksonSerializationTest;
 import org.testng.annotations.Test;
 
+import com.google.refine.model.Cell;
+
 public class WbLanguageVariableTest extends WbVariableTest<String> {
 
     @Override
@@ -46,6 +48,11 @@ public class WbLanguageVariableTest extends WbVariableTest<String> {
         isSkipped((String) null);
         isSkipped("");
     }
+    
+    @Test
+    public void testNullCell() {
+        isSkipped((Cell) null);
+    }
 
     @Test
     public void testSerialize() {

extensions/wikidata/tests/src/org/openrefine/wikidata/schema/WbLocationVariableTest.java

@@ -28,6 +28,8 @@ import org.testng.annotations.Test;
 import org.wikidata.wdtk.datamodel.helpers.Datamodel;
 import org.wikidata.wdtk.datamodel.interfaces.GlobeCoordinatesValue;
 
+import com.google.refine.model.Cell;
+
 public class WbLocationVariableTest extends WbVariableTest<GlobeCoordinatesValue> {
 
     @Override
@@ -62,6 +64,15 @@ public class WbLocationVariableTest extends WbVariableTest<GlobeCoordinatesValue
     public void testEmpty() {
         isSkipped("");
     }
+    
+    @Test
+    public void testNullStringValue() {
+        isSkipped((String) null);
+    }
+    
+    public void testNullCell() {
+        isSkipped((Cell) null);
+    }
 
     @Test
     public void testSerialize() {

extensions/wikidata/tests/src/org/openrefine/wikidata/schema/WbStringVariableTest.java

@@ -40,6 +40,19 @@ public class WbStringVariableTest extends WbVariableTest<StringValue> {
     public void testEmpty() {
         isSkipped("");
     }
+    
+    /**
+     * This should not normally happen: cell values should
+     * never be null (only whole cells can be null).
+     * But better safe than sorry!
+     */
+    public void testNullStringValue() {
+        isSkipped((String) null);
+    }
+    
+    public void testNullCell() {
+        isSkipped((Cell) null);
+    }
 
     @Test
     public void testSimpleString() {

main/src/com/google/refine/model/changes/DataExtensionChange.java

@@ -270,7 +270,7 @@ public class DataExtensionChange implements Change {
                 }
                 cell = new Cell(rc.name, recon);
             } else {
-                cell = new Cell((Serializable) value, null);
+                cell = value == null ? null : new Cell((Serializable) value, null);
             }
             
             row.setCell(_firstNewCellIndex + c, cell);

main/tests/server/src/com/google/refine/operations/recon/ExtendDataOperationTests.java

@@ -249,7 +249,7 @@ public class ExtendDataOperationTests extends RefineTest {
       		  "{"
   			+ "\"rows\": {"
   			+ "    \"Q794\": {\"P297\": [{\"str\": \"IR\"}]},"
-  			+ "    \"Q863\": {\"P297\": [{\"str\": \"TJ\"}]},"
+  			+ "    \"Q863\": {\"P297\": []},"
   			+ "    \"Q30\": {\"P297\": [{\"str\": \"US\"}]},"
   			+ "    \"Q17\": {\"P297\": [{\"str\": \"JP\"}]}"
   			+ "},"
@@ -270,7 +270,7 @@ public class ExtendDataOperationTests extends RefineTest {
         // Inspect rows
         Assert.assertTrue("IR".equals(project.rows.get(0).getCellValue(1)), "Bad country code for Iran.");
         Assert.assertTrue("JP".equals(project.rows.get(1).getCellValue(1)), "Bad country code for Japan.");
-        Assert.assertTrue("TJ".equals(project.rows.get(2).getCellValue(1)), "Bad country code for Tajikistan.");
+        Assert.assertNull(project.rows.get(2).getCell(1), "Expected a null country code.");
         Assert.assertTrue("US".equals(project.rows.get(3).getCellValue(1)), "Bad country code for United States.");
 
         // Make sure we did not create any recon stats for that column (no reconciled value)