From 2758633f32a1bc1e71df5bfb4c9d5b6831fac0d6 Mon Sep 17 00:00:00 2001 From: Owen Stephens Date: Wed, 21 Feb 2018 23:10:38 +0000 Subject: [PATCH 1/4] Update toDate and toNumber tests to handle null and empty strings correctly --- .../expr/functions/strings/ToFromConversionTests.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/main/tests/server/src/com/google/refine/tests/expr/functions/strings/ToFromConversionTests.java b/main/tests/server/src/com/google/refine/tests/expr/functions/strings/ToFromConversionTests.java index fbc6d9a9e..5db608229 100644 --- a/main/tests/server/src/com/google/refine/tests/expr/functions/strings/ToFromConversionTests.java +++ b/main/tests/server/src/com/google/refine/tests/expr/functions/strings/ToFromConversionTests.java @@ -98,10 +98,9 @@ public class ToFromConversionTests extends RefineTest { @Test public void testToNumber() { -// Assert.assertTrue(invoke("toNumber") instanceof EvalError); - Assert.assertNull(invoke("toNumber")); -// Assert.assertTrue(invoke("toNumber", (Object) null) instanceof EvalError); - Assert.assertNull(invoke("toNumber", (Object) null)); + Assert.assertTrue(invoke("toNumber") instanceof EvalError); + Assert.assertTrue(invoke("toNumber", (Object) null) instanceof EvalError); + Assert.assertTrue(invoke("toNumber", "") instanceof EvalError); Assert.assertTrue(invoke("toNumber", "string") instanceof EvalError); Assert.assertEquals(invoke("toNumber", "0.0"), 0.0); Assert.assertEquals(invoke("toNumber", "123"), Long.valueOf(123)); @@ -128,9 +127,9 @@ public class ToFromConversionTests extends RefineTest { @Test public void testToDate() throws CalendarParserException { -// Assert.assertTrue(invoke("toDate") instanceof EvalError); - Assert.assertNull(invoke("toDate")); + Assert.assertTrue(invoke("toDate") instanceof EvalError); Assert.assertTrue(invoke("toDate", (Object) null) instanceof EvalError); + Assert.assertTrue(invoke("toDate", "") instanceof EvalError); Assert.assertTrue(invoke("toDate", 1.0) instanceof EvalError); Assert.assertTrue(invoke("toDate", "2012-03-01","xxx") instanceof EvalError); // bad format string Assert.assertTrue(invoke("toDate", "2012-03-01") instanceof GregorianCalendar); From 5aaf4362b0b0879d1a496010af9e06bb47dbd97d Mon Sep 17 00:00:00 2001 From: Owen Stephens Date: Wed, 21 Feb 2018 23:19:54 +0000 Subject: [PATCH 2/4] Return errors not nulls when: wrong number of arguments supplied; first argument zero length/empty string --- main/src/com/google/refine/expr/functions/ToNumber.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/main/src/com/google/refine/expr/functions/ToNumber.java b/main/src/com/google/refine/expr/functions/ToNumber.java index b0cdfb041..4935b3199 100644 --- a/main/src/com/google/refine/expr/functions/ToNumber.java +++ b/main/src/com/google/refine/expr/functions/ToNumber.java @@ -39,6 +39,7 @@ import org.json.JSONException; import org.json.JSONWriter; import com.google.refine.expr.EvalError; +import com.google.refine.grel.ControlFunctionRegistry; import com.google.refine.grel.Function; public class ToNumber implements Function { @@ -58,12 +59,15 @@ public class ToNumber implements Function { try { return Double.parseDouble(s); } catch (NumberFormatException e) { - return new EvalError("Cannot parse to number"); + return new EvalError("Unable to parse as number"); } + } else { + return new EvalError("Unable to parse as number"); } } + } else { + return new EvalError(ControlFunctionRegistry.getFunctionName(this) + " expects one non-null argument"); } - return null; } @Override From 24f1923ff4311736c382e98a5f0b8c5c26097d74 Mon Sep 17 00:00:00 2001 From: Owen Stephens Date: Wed, 21 Feb 2018 23:20:15 +0000 Subject: [PATCH 3/4] Return errors not nulls when: wrong number of arguments supplied; first argument zero length/empty string --- .../google/refine/expr/functions/ToDate.java | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/main/src/com/google/refine/expr/functions/ToDate.java b/main/src/com/google/refine/expr/functions/ToDate.java index b74655cfe..c748be0dc 100644 --- a/main/src/com/google/refine/expr/functions/ToDate.java +++ b/main/src/com/google/refine/expr/functions/ToDate.java @@ -50,29 +50,30 @@ import com.google.refine.expr.EvalError; import com.google.refine.expr.util.CalendarParser; import com.google.refine.expr.util.CalendarParserException; import com.google.refine.grel.Function; +import com.google.refine.grel.ControlFunctionRegistry; import com.google.refine.util.ParsingUtilities; public class ToDate implements Function { @Override public Object call(Properties bindings, Object[] args) { - if (args.length == 0) { - // missing value, can this happen? - return null; - } - Object arg0 = args[0]; String o1; - if (arg0 instanceof Date) { - return arg0; - } else if (arg0 instanceof Calendar) { - return ((Calendar) arg0).getTime(); - } else if (arg0 instanceof Long) { - o1 = ((Long) arg0).toString(); // treat integers as years - } else if (arg0 instanceof String) { - o1 = (String) arg0; + if (args.length == 0) { + return new EvalError(ControlFunctionRegistry.getFunctionName(this) + " expects at least one argument"); } else { - // ignore cell values that aren't strings - return new EvalError("Not a String - cannot parse to date"); + Object arg0 = args[0]; + if (arg0 instanceof Date) { + return arg0; + } else if (arg0 instanceof Calendar) { + return ((Calendar) arg0).getTime(); + } else if (arg0 instanceof Long) { + o1 = ((Long) arg0).toString(); // treat integers as years + } else if (arg0 instanceof String && arg0.toString().trim().length() > 0) { + o1 = (String) arg0; + } else { + // ignore cell values that aren't Date, Calendar, Long or String + return new EvalError("Unable to parse as date"); + } } // "o, boolean month_first (optional)" @@ -98,13 +99,12 @@ public class ToDate implements Function { // } catch (DatatypeConfigurationException e2) { // } } - return new EvalError("Cannot parse to date"); + return new EvalError("Unable to parse as date"); } - } - - // "o, format1, format2 (optional), ..." - Locale locale = Locale.getDefault(); - if (args.length>=2) { + } else if (args.length>=2) { + // "o, format1, format2 (optional), ..." + Locale locale = Locale.getDefault(); + for (int i=1;i Date: Wed, 21 Feb 2018 23:20:47 +0000 Subject: [PATCH 4/4] Use look up for function name --- main/src/com/google/refine/expr/functions/ToString.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/main/src/com/google/refine/expr/functions/ToString.java b/main/src/com/google/refine/expr/functions/ToString.java index 2a3b54b82..0a03ae169 100644 --- a/main/src/com/google/refine/expr/functions/ToString.java +++ b/main/src/com/google/refine/expr/functions/ToString.java @@ -43,6 +43,7 @@ import org.json.JSONException; import org.json.JSONWriter; import com.google.refine.expr.EvalError; +import com.google.refine.grel.ControlFunctionRegistry; import com.google.refine.grel.Function; import com.google.refine.util.StringUtils; @@ -68,7 +69,7 @@ public class ToString implements Function { } } } - return new EvalError("ToString accepts an object and an optional second argument containing a date format string"); + return new EvalError(ControlFunctionRegistry.getFunctionName(this) + " accepts an object and an optional second argument containing a date format string"); }