Bug 56595: Also switch the cache in DateUtil.isADateFormat() to ThreadLocals to not...

Also do more simple checks before actually looking at the cache

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1647296 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/src/java/org/apache/poi/ss/usermodel/DateUtil.java b/src/java/org/apache/poi/ss/usermodel/DateUtil.java
index 2bc9f01713af3479e7ecc4ef82b4bdbd97c6c6ef..79512ef10271b5f53943a9bd45af2665446b4a1c 100644 (file)
--- a/src/java/org/apache/poi/ss/usermodel/DateUtil.java
+++ b/src/java/org/apache/poi/ss/usermodel/DateUtil.java
@@ -327,15 +327,30 @@ public class DateUtil {
         return calendar;
     }
 
-
     // variables for performance optimization:
     // avoid re-checking DataUtil.isADateFormat(int, String) if a given format
     // string represents a date format if the same string is passed multiple times.
     // see https://issues.apache.org/bugzilla/show_bug.cgi?id=55611
-    private static int lastFormatIndex = -1;
-    private static String lastFormatString = null;
-    private static boolean cached = false;
+    private static ThreadLocal<Integer> lastFormatIndex = new ThreadLocal<Integer>() {
+        protected Integer initialValue() {
+            return -1;
+        }
+    };
+    private static ThreadLocal<String> lastFormatString = new ThreadLocal<String>();
+    private static ThreadLocal<Boolean> lastCachedResult = new ThreadLocal<Boolean>();
+
+    private static boolean isCached(String formatString, int formatIndex) {
+        String cachedFormatString = lastFormatString.get();
+        return cachedFormatString != null && formatIndex == lastFormatIndex.get()
+                && formatString.equals(cachedFormatString);
+    }
 
+    private static void cache(String formatString, int formatIndex, boolean cached) {
+        lastFormatIndex.set(formatIndex);
+        lastFormatString.set(formatString);
+        lastCachedResult.set(Boolean.valueOf(cached));
+    }
+    
     /**
      * Given a format ID and its format String, will check to see if the
      *  format represents a date format or not.
@@ -350,27 +365,23 @@ public class DateUtil {
      * @see #isInternalDateFormat(int)
      */
 
-    public static synchronized boolean isADateFormat(int formatIndex, String formatString) {
-       
-         if (formatString != null && formatIndex == lastFormatIndex && formatString.equals(lastFormatString)) {
-                       return cached;
-         }
+    public static boolean isADateFormat(int formatIndex, String formatString) {
         // First up, is this an internal date format?
         if(isInternalDateFormat(formatIndex)) {
-            lastFormatIndex = formatIndex;
-            lastFormatString = formatString;
-            cached = true;
+            cache(formatString, formatIndex, true);
             return true;
         }
 
-        // If we didn't get a real string, it can't be
+        // If we didn't get a real string, don't even cache it as we can always find this out quickly
         if(formatString == null || formatString.length() == 0) {
-            lastFormatIndex = formatIndex;
-            lastFormatString = formatString;
-            cached = false;
             return false;
         }
 
+        // check the cache first
+        if (isCached(formatString, formatIndex)) {
+            return lastCachedResult.get();
+        }
+
         String fs = formatString;
         /*if (false) {
             // Normalize the format string. The code below is equivalent
@@ -419,9 +430,7 @@ public class DateUtil {
 
         // short-circuit if it indicates elapsed time: [h], [m] or [s]
         if(date_ptrn4.matcher(fs).matches()){
-            lastFormatIndex = formatIndex;
-            lastFormatString = formatString;
-            cached = true;
+            cache(formatString, formatIndex, true);
             return true;
         }
 
@@ -449,9 +458,7 @@ public class DateUtil {
         // optionally followed by AM/PM
 
         boolean result = date_ptrn3b.matcher(fs).matches();
-        lastFormatIndex = formatIndex;
-        lastFormatString = formatString;
-        cached = result;
+        cache(formatString, formatIndex, result);
         return result;
     }
 

diff --git a/src/testcases/org/apache/poi/ss/usermodel/TestDataFormatter.java b/src/testcases/org/apache/poi/ss/usermodel/TestDataFormatter.java
index 06ad051a38f7d66b7f9ef0a5897defae793b7920..08f21a59004e5f5c9157b1ae0b51f8be246a6cc6 100644 (file)
--- a/src/testcases/org/apache/poi/ss/usermodel/TestDataFormatter.java
+++ b/src/testcases/org/apache/poi/ss/usermodel/TestDataFormatter.java
@@ -21,6 +21,7 @@
 
 package org.apache.poi.ss.usermodel;
 
+import java.io.IOException;
 import java.text.DateFormat;
 import java.util.Calendar;
 import java.util.Date;
@@ -158,7 +159,7 @@ public class TestDataFormatter extends TestCase {
        String p2dp_n1dp_z0 = "00.00;(00.0);0";
        String all2dpTSP = "00.00_x";
        String p2dp_n2dpTSP = "00.00_x;(00.00)_x";
-       String p2dp_n1dpTSP = "00.00_x;(00.0)_x";
+       //String p2dp_n1dpTSP = "00.00_x;(00.0)_x";
        
        assertEquals("12.34", dfUS.formatRawCellContents(12.343, -1, all2dp));
        assertEquals("12.34", dfUS.formatRawCellContents(12.343, -1, p2dp_n1dp));
@@ -493,20 +494,24 @@ public class TestDataFormatter extends TestCase {
         assertEquals(" $-   ", dfUS.formatRawCellContents(0.0, -1, "_-$* #,##0.00_-;-$* #,##0.00_-;_-$* \"-\"??_-;_-@_-"));
     }
     
-    public void testErrors() {
+    public void testErrors() throws IOException {
         DataFormatter dfUS = new DataFormatter(Locale.US, true);
 
         // Create a spreadsheet with some formula errors in it
         Workbook wb = new HSSFWorkbook();
-        Sheet s = wb.createSheet();
-        Row r = s.createRow(0);
-        Cell c = r.createCell(0, Cell.CELL_TYPE_ERROR);
-        
-        c.setCellErrorValue(FormulaError.DIV0.getCode());
-        assertEquals(FormulaError.DIV0.getString(), dfUS.formatCellValue(c));
-        
-        c.setCellErrorValue(FormulaError.REF.getCode());
-        assertEquals(FormulaError.REF.getString(), dfUS.formatCellValue(c));
+        try {
+            Sheet s = wb.createSheet();
+            Row r = s.createRow(0);
+            Cell c = r.createCell(0, Cell.CELL_TYPE_ERROR);
+            
+            c.setCellErrorValue(FormulaError.DIV0.getCode());
+            assertEquals(FormulaError.DIV0.getString(), dfUS.formatCellValue(c));
+            
+            c.setCellErrorValue(FormulaError.REF.getCode());
+            assertEquals(FormulaError.REF.getString(), dfUS.formatCellValue(c));
+        } finally {
+            wb.close();
+        }
     }
 
     /**
@@ -607,4 +612,25 @@ public class TestDataFormatter extends TestCase {
                        assertTrue(e.getMessage().contains("Cannot format given Object as a Number"));
                }
        }
+       
+       public void testIsADateFormat() {
+           // first check some cases that should not be a date, also call multiple times to ensure the cache is used
+           assertFalse(DateUtil.isADateFormat(-1, null));
+           assertFalse(DateUtil.isADateFormat(-1, null));
+        assertFalse(DateUtil.isADateFormat(123, null));
+        assertFalse(DateUtil.isADateFormat(123, ""));
+        assertFalse(DateUtil.isADateFormat(124, ""));
+        assertFalse(DateUtil.isADateFormat(-1, ""));
+        assertFalse(DateUtil.isADateFormat(-1, ""));
+        assertFalse(DateUtil.isADateFormat(-1, "nodateformat"));
+
+        // then also do the same for some valid date formats
+        assertTrue(DateUtil.isADateFormat(0x0e, null));
+        assertTrue(DateUtil.isADateFormat(0x2f, null));
+        assertTrue(DateUtil.isADateFormat(-1, "yyyy"));
+        assertTrue(DateUtil.isADateFormat(-1, "yyyy"));
+        assertTrue(DateUtil.isADateFormat(-1, "dd/mm/yy;[red]dd/mm/yy"));
+        assertTrue(DateUtil.isADateFormat(-1, "dd/mm/yy;[red]dd/mm/yy"));
+        assertTrue(DateUtil.isADateFormat(-1, "[h]"));
+       }
 }