Fix Excel date import - Fixes #1908 (#2909)

* Add utility functions to check/convert dates

* Add date tests and refactor to DRY up

* Fix date import - fixes #1908

Change from java.util.Date to OpenRefine 3.0+'s OffsetDateTime
Fixes #1908

* Centralize date conversion

* Moving utility methods to ParsingUtilities

* Fix tests

main/src/com/google/refine/expr/functions/ToDate.java

@@ -202,4 +202,5 @@ public class ToDate implements Function {
     public String getReturns() {
         return "date";
     }
+
 }

main/src/com/google/refine/importers/ExcelImporter.java

@@ -34,7 +34,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 package com.google.refine.importers;
 
 import java.io.File;
-import java.io.FileInputStream;
 import java.io.BufferedInputStream;
 import java.io.IOException;
 import java.io.InputStream;
@@ -249,7 +248,7 @@ public class ExcelImporter extends TabularImportingParserBase {
             double d = cell.getNumericCellValue();
             
             if (DateUtil.isCellDateFormatted(cell)) {
-                value = DateUtil.getJavaDate(d);
+                value = ParsingUtilities.toDate(DateUtil.getJavaDate(d));
                 // TODO: If we had a time datatype, we could use something like the following
                 // to distinguish times from dates (although Excel doesn't really make the distinction)
                 // Another alternative would be to look for values < 0.60

main/src/com/google/refine/importers/OdsImporter.java

@@ -217,7 +217,7 @@ public class OdsImporter extends TabularImportingParserBase {
         } else if ("float".equals(cellType)) {
             value = cell.getDoubleValue();
         } else if ("date".equals(cellType)) {
-            value = cell.getDateValue().toInstant().atOffset(ZoneOffset.UTC);
+            value = ParsingUtilities.toDate(cell.getDateValue());
         } else if ("currency".equals(cellType)) {
             value = cell.getCurrencyValue();
         } else if ("percentage".equals(cellType)) {
@@ -289,5 +289,4 @@ public class OdsImporter extends TabularImportingParserBase {
             return null;
         }
     }
-
 }

main/src/com/google/refine/util/ParsingUtilities.java

@@ -44,6 +44,7 @@ import java.time.ZoneOffset;
 import java.time.format.DateTimeFormatter;
 import java.time.format.DateTimeParseException;
 import java.util.Calendar;
+import java.util.Date;
 import java.util.GregorianCalendar;
 import java.util.Properties;
 import java.util.TimeZone;
@@ -240,6 +241,18 @@ public class ParsingUtilities {
         return cal;
     }
 
+    public static boolean isDate(Object o) {
+        return o instanceof OffsetDateTime;
+    }
+
+    public static OffsetDateTime toDate(Date date) {
+        return date.toInstant().atOffset(ZoneOffset.UTC);
+    }
+
+    public static OffsetDateTime toDate(Calendar date) {
+        return date.toInstant().atOffset(ZoneOffset.UTC);
+    }
+
 	public static ObjectNode evaluateJsonStringToObjectNode(String optionsString) {
 		try {
 			JsonNode tree = mapper.readTree(optionsString);

main/tests/server/src/com/google/refine/importers/ExcelImporterTests.java

@@ -36,6 +36,7 @@ package com.google.refine.importers;
 
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
+import static org.testng.Assert.assertTrue;
 
 import java.io.File;
 import java.io.FileInputStream;
@@ -49,6 +50,7 @@ import java.util.Date;
 
 import org.apache.poi.hssf.usermodel.HSSFWorkbook;
 import org.apache.poi.ss.usermodel.Cell;
+import org.apache.poi.ss.usermodel.CellStyle;
 import org.apache.poi.ss.usermodel.Row;
 import org.apache.poi.ss.usermodel.Sheet;
 import org.apache.poi.ss.usermodel.Workbook;
@@ -134,6 +136,9 @@ public class ExcelImporterTests extends ImporterTest {
         Assert.assertEquals((String)project.rows.get(1).getCellValue(4)," Row 1 Col 5");
         Assert.assertNull((String)project.rows.get(1).getCellValue(5));
 
+        assertTrue(ParsingUtilities.isDate(project.rows.get(1).getCellValue(2))); // Calendar
+        assertTrue(ParsingUtilities.isDate(project.rows.get(1).getCellValue(3))); // Date
+
         verify(options, times(1)).get("ignoreLines");
         verify(options, times(1)).get("headerLines");
         verify(options, times(1)).get("skipDataLines");
@@ -199,6 +204,9 @@ public class ExcelImporterTests extends ImporterTest {
         Assert.assertFalse((Boolean)project.rows.get(1).getCellValue(1));
         Assert.assertTrue((Boolean)project.rows.get(2).getCellValue(1));
 
+        assertTrue(ParsingUtilities.isDate(project.rows.get(1).getCellValue(2))); // Calendar
+        assertTrue(ParsingUtilities.isDate(project.rows.get(1).getCellValue(3))); // Date
+
         Assert.assertEquals((String)project.rows.get(1).getCellValue(4)," Row 1 Col 5");
         Assert.assertNull((String)project.rows.get(1).getCellValue(5));
 
@@ -258,6 +266,9 @@ public class ExcelImporterTests extends ImporterTest {
         Assert.assertFalse((Boolean)project.rows.get(1).getCellValue(1));
         Assert.assertTrue((Boolean)project.rows.get(2).getCellValue(1));
 
+        assertTrue(ParsingUtilities.isDate(project.rows.get(1).getCellValue(2))); // Calendar
+        assertTrue(ParsingUtilities.isDate(project.rows.get(1).getCellValue(3))); // Date
+
         Assert.assertEquals((String)project.rows.get(1).getCellValue(4)," Row 1 Col 5");
         Assert.assertNull((String)project.rows.get(1).getCellValue(5));
 
@@ -302,10 +313,12 @@ public class ExcelImporterTests extends ImporterTest {
         Assert.assertEquals(((Number)project.rows.get(ROWS).getCellValue(0)).doubleValue(),0.0, EPSILON);
         Assert.assertEquals(((Number)project.rows.get(ROWS).getCellValue(COLUMNS)).doubleValue(),1.0, EPSILON);
 
-
         Assert.assertFalse((Boolean)project.rows.get(1).getCellValue(1));
         Assert.assertTrue((Boolean)project.rows.get(2).getCellValue(1));
 
+        assertTrue(ParsingUtilities.isDate(project.rows.get(1).getCellValue(2))); // Calendar
+        assertTrue(ParsingUtilities.isDate(project.rows.get(1).getCellValue(3))); // Date
+
         Assert.assertEquals((String)project.rows.get(1).getCellValue(4)," Row 1 Col 5");
         Assert.assertNull((String)project.rows.get(1).getCellValue(5));
 
@@ -321,37 +334,15 @@ public class ExcelImporterTests extends ImporterTest {
 
         final Workbook wb = xml ? new XSSFWorkbook() : new HSSFWorkbook();
 
+        CellStyle dateStyle = wb.createCellStyle();
+        short dateFormat = wb.createDataFormat().getFormat("yyyy-MM-dd");
+        dateStyle.setDataFormat(dateFormat);
+
         for (int s = 0; s < SHEETS; s++) {
             Sheet sheet = wb.createSheet("Test Sheet " + s);
-
             for (int row = 0; row < ROWS; row++) {
-                int col = 0;
+                createDataRow(sheet, row, dateStyle, 0);
-                Row r = sheet.createRow(row);
-                Cell c;
-
-                c = r.createCell(col++);
-                c.setCellValue(row * 1.1); // double
-
-                c = r.createCell(col++);
-                c.setCellValue(row % 2 == 0); // boolean
-
-                c = r.createCell(col++);
-                c.setCellValue(Calendar.getInstance()); // calendar
-
-                c = r.createCell(col++);
-                c.setCellValue(new Date()); // date
-
-                c = r.createCell(col++);
-                c.setCellValue(" Row " + row + " Col " + col); // string
-
-                c = r.createCell(col++);
-                c.setCellValue(""); // string
-
-                //            HSSFHyperlink hl = new HSSFHyperlink(HSSFHyperlink.LINK_URL);
-                //            hl.setLabel(cellData.text);
-                //            hl.setAddress(cellData.link);
             }
-
         }
 
         File file = null;
@@ -367,48 +358,23 @@ public class ExcelImporterTests extends ImporterTest {
             return null;
         }
         return file;
-		
     }
 
    private static File createSheetsWithDifferentColumns(boolean xml) {
 
         final Workbook wb = xml ? new XSSFWorkbook() : new HSSFWorkbook();
 
+        CellStyle dateStyle = wb.createCellStyle();
+        short dateFormat = wb.createDataFormat().getFormat("yyyy-MM-dd");
+        dateStyle.setDataFormat(dateFormat);
+
         for (int s = 0; s < SHEETS; s++) {
             Sheet sheet = wb.createSheet("Test Sheet " + s);
-
             for (int row = 0; row < ROWS; row++) {
-                int col = 0;
+                createDataRow(sheet, row, dateStyle, s);
-                Row r = sheet.createRow(row);
-                Cell c;
-
-                c = r.createCell(col++);
-                c.setCellValue(row * 1.1); // double
-
-                c = r.createCell(col++);
-                c.setCellValue(row % 2 == 0); // boolean
-
-                c = r.createCell(col++);
-                c.setCellValue(Calendar.getInstance()); // calendar
-
-                c = r.createCell(col++);
-                c.setCellValue(new Date()); // date
-
-                c = r.createCell(col++);
-                c.setCellValue(" Row " + row + " Col " + col); // string
-
-                c = r.createCell(col++);
-                c.setCellValue(""); // string
-
-                // Create extra columns to ensure sheet(i+1) has more columns than sheet(i)
-                for(int i = 0; i < s; i++){
-                    c = r.createCell(col++);
-                    c.setCellValue(i + s);
             }
         }
 
-        }
-
         File file = null;
         try {
             file = File.createTempFile("openrefine-importer-test", xml ? ".xlsx" : ".xls");
@@ -424,4 +390,41 @@ public class ExcelImporterTests extends ImporterTest {
         return file;
     }
 
+private static void createDataRow(Sheet sheet, int row, CellStyle dateCellStyle, int extra_columns) {
+    int col = 0;
+    Row r = sheet.createRow(row);
+    Cell c;
+
+
+    c = r.createCell(col++);
+    c.setCellValue(row * 1.1); // double
+
+    c = r.createCell(col++);
+    c.setCellValue(row % 2 == 0); // boolean
+
+    c = r.createCell(col++);
+    c.setCellValue(Calendar.getInstance()); // calendar
+    c.setCellStyle(dateCellStyle);
+
+    c = r.createCell(col++);
+    c.setCellValue(new Date()); // date
+    c.setCellStyle(dateCellStyle);
+
+    c = r.createCell(col++);
+    c.setCellValue(" Row " + row + " Col " + col); // string
+
+    c = r.createCell(col++);
+    c.setCellValue(""); // string
+
+//    HSSFHyperlink hl = new HSSFHyperlink(HSSFHyperlink.LINK_URL);
+//    hl.setLabel(cellData.text);
+//    hl.setAddress(cellData.link);
+
+    // Create extra columns to ensure sheet(i+1) has more columns than sheet(i)
+    for(int i = 0; i < extra_columns; i++){
+        c = r.createCell(col++);
+        c.setCellValue(i + extra_columns);
+    }
+}
+
 }