From 431931467510a03816231bda16a53ba46cbce3c9 Mon Sep 17 00:00:00 2001 From: Tom Morris Date: Thu, 2 Aug 2012 21:41:19 +0000 Subject: [PATCH] FIXED - task 594: Date diff function doesn't work for two Calendar objects http://code.google.com/p/google-refine/issues/detail?id=594 git-svn-id: http://google-refine.googlecode.com/svn/trunk@2519 7d457c2a-affb-35e4-300a-418c747d4874 --- .../refine/expr/functions/strings/Diff.java | 19 +++-- .../expr/functions/strings/DiffTests.java | 85 +++++++++++++++++++ 2 files changed, 99 insertions(+), 5 deletions(-) create mode 100644 main/tests/server/src/com/google/refine/tests/expr/functions/strings/DiffTests.java diff --git a/main/src/com/google/refine/expr/functions/strings/Diff.java b/main/src/com/google/refine/expr/functions/strings/Diff.java index d2ca23c79..0e2c90d25 100644 --- a/main/src/com/google/refine/expr/functions/strings/Diff.java +++ b/main/src/com/google/refine/expr/functions/strings/Diff.java @@ -41,6 +41,7 @@ import org.apache.commons.lang.StringUtils; import org.json.JSONException; import org.json.JSONWriter; +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; @@ -49,11 +50,11 @@ public class Diff implements Function { @Override public Object call(Properties bindings, Object[] args) { - if (args.length >= 2 && args.length <= 3) { + if (args.length >= 2) { Object o1 = args[0]; Object o2 = args[1]; if (o1 != null && o2 != null) { - if (o1 instanceof String && o2 instanceof String) { + if (args.length == 2 && o1 instanceof String && o2 instanceof String) { return StringUtils.difference((String) o1,(String) o2); } else if ((o1 instanceof Date || o1 instanceof Calendar) && args.length == 3) { Object o3 = args[2]; @@ -61,7 +62,14 @@ public class Diff implements Function { try { String unit = ((String) o3).toLowerCase(); Date c1 = (o1 instanceof Date) ? (Date) o1 : ((Calendar) o1).getTime(); - Date c2 = (o2 instanceof Date) ? (Date) o2 : CalendarParser.parse((o2 instanceof String) ? (String) o2 : o2.toString()).getTime(); + Date c2; + if (o2 instanceof Date) { + c2 = (Date) o2; + } else if (o2 instanceof Calendar) { + c2 = ((Calendar) o2).getTime(); + } else { + c2 = CalendarParser.parse((o2 instanceof String) ? (String) o2 : o2.toString()).getTime(); + } long delta = (c1.getTime() - c2.getTime()) / 1000; if ("seconds".equals(unit)) { return delta; @@ -87,14 +95,15 @@ public class Diff implements Function { if ("years".equals(unit)) { return days / 365; } + return new EvalError("Unknown time unit " + unit); } catch (CalendarParserException e) { - // we should throw at this point because it's important to know that date parsing failed + return new EvalError(e); } } } } } - return null; + return new EvalError("Unexpected arguments - expecting either 2 strings or 2 dates and a unit string"); } @Override diff --git a/main/tests/server/src/com/google/refine/tests/expr/functions/strings/DiffTests.java b/main/tests/server/src/com/google/refine/tests/expr/functions/strings/DiffTests.java new file mode 100644 index 000000000..a209d4dde --- /dev/null +++ b/main/tests/server/src/com/google/refine/tests/expr/functions/strings/DiffTests.java @@ -0,0 +1,85 @@ +package com.google.refine.tests.expr.functions.strings; + +import java.util.Calendar; +import java.util.Properties; + +import org.slf4j.LoggerFactory; +import org.testng.Assert; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.BeforeTest; +import org.testng.annotations.Test; + +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.ControlFunctionRegistry; +import com.google.refine.grel.Function; +import com.google.refine.tests.RefineTest; + + +public class DiffTests extends RefineTest { + + static Properties bindings; + private Calendar date1; + private Calendar date2; + + @Override + @BeforeTest + public void init() { + logger = LoggerFactory.getLogger(this.getClass()); + try { + date1 = CalendarParser.parse("2012-08-02"); + date2 = CalendarParser.parse("2012-10-02"); + } catch (CalendarParserException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + Assert.fail(); + } + } + + @BeforeMethod + public void SetUp() { + bindings = new Properties(); + } + + @AfterMethod + public void TearDown() { + bindings = null; + } + + /** + * Lookup a control function by name and invoke it with a variable number of args + */ + private static Object invoke(String name,Object... args) { + // registry uses static initializer, so no need to set it up + Function function = ControlFunctionRegistry.getFunction(name); + if (function == null) { + throw new IllegalArgumentException("Unknown function "+name); + } + if (args == null) { + return function.call(bindings,new Object[0]); + } else { + return function.call(bindings,args); + } + } + + @Test + public void testDiffInvalidParams() { + Assert.assertTrue(invoke("diff") instanceof EvalError); + Assert.assertTrue(invoke("diff", "one","two","three") instanceof EvalError); + Assert.assertTrue(invoke("diff", date1,date2) instanceof EvalError); + Assert.assertTrue(invoke("diff", date1,date2,"foo") instanceof EvalError); + } + + @Test + public void testDiff() { + Assert.assertEquals((String)(invoke("diff", "onetwo","onetwothree")),"three"); + Assert.assertEquals(invoke("diff",date2,date1,"days"),Long.valueOf(61)); + Assert.assertEquals(invoke("diff",date2,date1,"weeks"),Long.valueOf(8)); + Assert.assertEquals(invoke("diff",date2,date1,"months"),Long.valueOf(2)); + Assert.assertEquals(invoke("diff",date2,date1,"hours"),Long.valueOf(1464)); + Assert.assertEquals(invoke("diff",date2,date1,"seconds"),Long.valueOf(5270400)); + + } +}