Merge branch 'master' of ssh://github.com/OpenRefine/OpenRefine

main/pom.xml

@@ -247,6 +247,7 @@
             <verbose>false</verbose>
             <generateGitPropertiesFile>true</generateGitPropertiesFile>
             <generateGitPropertiesFilename>${project.build.outputDirectory}/git.properties</generateGitPropertiesFilename>
+            <failOnNoGitDirectory>false</failOnNoGitDirectory>
             <format>json</format>
             <gitDescribe>
                 <skip>false</skip>

main/src/com/google/refine/expr/functions/strings/Diff.java

@@ -34,6 +34,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 package com.google.refine.expr.functions.strings;
 
 import java.time.OffsetDateTime;
+import java.time.temporal.ChronoUnit;
 import java.util.Properties;
 
 import org.apache.commons.lang3.StringUtils;
@@ -57,43 +58,38 @@ public class Diff implements Function {
                             String unit = ((String) o3).toLowerCase();
                             OffsetDateTime c1 = (OffsetDateTime)o1;
                             OffsetDateTime c2 = (OffsetDateTime)o2;
-
+                            try {
-                            long delta = getNano(c1) - getNano(c2);
                                 if ("nanos".equals(unit)) {
-                                return delta;
+                                    return ChronoUnit.NANOS.between(c2, c1);
                                 }
-                            
-                            delta /= 1000;
                                 if ("milliseconds".equals(unit)) {
-                                return delta;
+                                    return ChronoUnit.MILLIS.between(c2, c1);
                                 }
-                            
-                            delta /= 1000000;
                                 if ("seconds".equals(unit)) {
-                                return delta;
+                                    return ChronoUnit.SECONDS.between(c2, c1);
                                 }
-                            delta /= 60;
                                 if ("minutes".equals(unit)) {
-                                return delta;
+                                    return ChronoUnit.MINUTES.between(c2, c1);
                                 }
-                            delta /= 60;
                                 if ("hours".equals(unit)) {
-                                return delta;
+                                    return ChronoUnit.HOURS.between(c2, c1);
                                 }
-                            long days = delta / 24;
                                 if ("days".equals(unit)) {
-                                return days;
+                                    return ChronoUnit.DAYS.between(c2, c1);
                                 }
                                 if ("weeks".equals(unit)) {
-                                return days / 7;
+                                    return ChronoUnit.WEEKS.between(c2, c1);
                                 }
                                 if ("months".equals(unit)) {
-                                return days / 30;
+                                    return ChronoUnit.MONTHS.between(c2, c1);
                                 }
                                 if ("years".equals(unit)) {
-                                return days / 365;
+                                    return ChronoUnit.YEARS.between(c2, c1);
                                 }
                                 return new EvalError("Unknown time unit " + unit);
+                            } catch (ArithmeticException arithmeticException) {
+                                    return new EvalError("Number of " + unit + " between given dates causes long overflow.");
+                            }
                         } 
                     }
                 }
@@ -115,8 +111,4 @@ public class Diff implements Function {
     public String getReturns() {
         return "string for strings, number for dates";
     }
-    
-    private long getNano(OffsetDateTime odt) {
-        return odt.toEpochSecond() * 1000000000l + odt.toInstant().getNano();
-    }
 }

main/src/com/google/refine/io/FileProjectManager.java

@@ -363,12 +363,14 @@ public class FileProjectManager extends ProjectManager  {
 
         boolean found = false;
 
+        if (file.exists() || file.canRead()) {
 	        try {
 	        	ParsingUtilities.mapper.readerForUpdating(this).readValue(file);
 	            found = true;
 	        } catch(IOException e) {
 	        	logger.warn(e.toString());
 	        }
+        }
 
         return found;
     }

main/tests/server/src/com/google/refine/tests/expr/functions/strings/DiffTests.java

@@ -28,6 +28,7 @@ package com.google.refine.tests.expr.functions.strings;
 
 import java.time.OffsetDateTime;
 import java.time.format.DateTimeFormatter;
+import java.time.ZoneOffset;
 import java.util.Properties;
 
 import org.slf4j.LoggerFactory;
@@ -51,6 +52,12 @@ public class DiffTests extends RefineTest {
     
     private OffsetDateTime odt1;
     private OffsetDateTime odt2;
+    private OffsetDateTime odt3;
+    private OffsetDateTime odt4;
+    private OffsetDateTime odt5;
+    private OffsetDateTime odt6;
+    private OffsetDateTime odt7;
+    private OffsetDateTime odt8;
     
     @Override
     @BeforeTest
@@ -58,6 +65,12 @@ public class DiffTests extends RefineTest {
         logger = LoggerFactory.getLogger(this.getClass());
         odt1 = OffsetDateTime.parse("2011-09-01T10:15:30.123456+01:00", DateTimeFormatter.ISO_OFFSET_DATE_TIME);
         odt2 = OffsetDateTime.parse("2011-12-02T10:16:30.123467+01:00", DateTimeFormatter.ISO_OFFSET_DATE_TIME);
+        odt3 = OffsetDateTime.of(2010, 1, 10, 12, 0, 0, 0, ZoneOffset.UTC);
+        odt4 = OffsetDateTime.of(2999, 1, 10, 12, 0, 0, 0, ZoneOffset.UTC);
+        odt5 = OffsetDateTime.of(2285, 1, 1, 12, 0, 0, 0, ZoneOffset.UTC);
+        odt6 = OffsetDateTime.of(1700, 1, 10, 12, 0, 0, 0, ZoneOffset.UTC);
+        odt7 = OffsetDateTime.of(1923, 4, 19, 12, 0, 0, 0, ZoneOffset.UTC);
+        odt8 = OffsetDateTime.of(1923, 4, 21, 12, 0, 0, 0, ZoneOffset.UTC);
     }
     
     @BeforeMethod
@@ -100,15 +113,43 @@ public class DiffTests extends RefineTest {
     @Test
     public void testDiffOffsetDateTime() {  
         // OffsetDateTime diff:
+        Assert.assertEquals(invoke("diff",odt2,odt3,"years"),Long.valueOf(1));
         Assert.assertEquals(invoke("diff",odt2,odt1,"days"),Long.valueOf(92));
         Assert.assertEquals(invoke("diff",odt2,odt1,"weeks"),Long.valueOf(13));
         Assert.assertEquals(invoke("diff",odt2,odt1,"months"),Long.valueOf(3));
         Assert.assertEquals(invoke("diff",odt2,odt1,"hours"),Long.valueOf(2208));
+        Assert.assertEquals(invoke("diff",odt2,odt1,"minutes"),Long.valueOf(132481));
         Assert.assertEquals(invoke("diff",odt2,odt1,"seconds"),Long.valueOf(7948860));
-        Assert.assertEquals(invoke("diff",odt2,odt1,"milliseconds"),Long.valueOf(7948860000011l));
+        //Changed due to an error in previous version- it was checking for microseconds (1,000,000th of second) instead of milliseconds (1000th of second) 
+        Assert.assertEquals(invoke("diff",odt2,odt1,"milliseconds"),Long.valueOf(7948860000l));
         Assert.assertEquals(invoke("diff",odt2,odt1,"nanos"),Long.valueOf(7948860000011000l));
     }
     
+    @Test
+    public void testDiffOffsetDateTimeEvalErrors() {
+        //Check whether EvalError is returned if time in nanoseconds between two dates is larger than long max in java when nanoseconds are desired unit.
+        Assert.assertTrue(invoke("diff", odt3, odt4, "nanos") instanceof EvalError);
+        Assert.assertTrue(invoke("diff", odt4, odt5, "nanos") instanceof EvalError);
+        //At the same time if different time unit is requested for the same period, it should not result in EvalError.
+        Assert.assertEquals(invoke("diff", odt3, odt4, "milliseconds"), Long.valueOf(-31209840000000l));   
+        Assert.assertEquals(invoke("diff", odt4, odt5, "milliseconds"), Long.valueOf(22532428800000l));
+        Assert.assertEquals(invoke("diff", odt3, odt4, "days"), Long.valueOf(-361225));
+        Assert.assertEquals(invoke("diff", odt4, odt5, "days"), Long.valueOf(260792));
+        //Also, ensure that periods longer (in nanoseconds) than long max preserve continuity when expressed in different time unit, like days.
+        //Example: 1923-04-19 to 1700-01-01 is just below long max when expressed in nanoseconds
+        Assert.assertEquals(invoke("diff", odt6, odt7, "days"), Long.valueOf(-81547));
+        // and 1923-04-21 to 1700-01-01 is slightly above long max when expressed in nanoseconds
+        Assert.assertEquals(invoke("diff", odt6, odt8, "days"), Long.valueOf(-81549));
+    }
+    
+    @Test
+    public void testDiffOffsetDateTimeEvalErrorsForIncorrectTimeUnit() {
+        //In case if incorrect time unit is passed to function, EvalError should be returned
+        Assert.assertTrue(invoke("diff", odt4, odt5, "milis") instanceof EvalError);
+        Assert.assertTrue(invoke("diff", odt4, odt5, "millis") instanceof EvalError);
+        Assert.assertTrue(invoke("diff", odt4, odt5, "yars") instanceof EvalError);
+    }
+    
     @Test
     public void serializeDiff() {
         String json = "{\"description\":\"For strings, returns the portion where they differ. For dates, it returns the difference in given time units\",\"params\":\"o1, o2, time unit (optional)\",\"returns\":\"string for strings, number for dates\"}";