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

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

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));
+
+    }
+}