You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@freemarker.apache.org by dd...@apache.org on 2021/12/28 22:46:53 UTC

[freemarker] branch FREEMARKER-35 updated: [FREEMARKER-35] Added workaround to avoid Java bugs when formatting months in standalone text style

This is an automated email from the ASF dual-hosted git repository.

ddekany pushed a commit to branch FREEMARKER-35
in repository https://gitbox.apache.org/repos/asf/freemarker.git


The following commit(s) were added to refs/heads/FREEMARKER-35 by this push:
     new bb4eaaf  [FREEMARKER-35] Added workaround to avoid Java bugs when formatting months in standalone text style
bb4eaaf is described below

commit bb4eaaf4c41c72dfb3ad13dad72a9368b30c3eb2
Author: ddekany <dd...@apache.org>
AuthorDate: Tue Dec 28 23:43:44 2021 +0100

    [FREEMARKER-35] Added workaround to avoid Java bugs when formatting months in standalone text style
---
 .../java/freemarker/template/utility/DateUtil.java | 12 +++
 .../template/utility/JavaTimeBugFlags.java         | 90 ++++++++++++++++++++++
 .../utility/DateUtilsPatternParsingTest.java       | 53 ++++---------
 3 files changed, 115 insertions(+), 40 deletions(-)

diff --git a/src/main/java/freemarker/template/utility/DateUtil.java b/src/main/java/freemarker/template/utility/DateUtil.java
index 0a05db4..f2830ad 100644
--- a/src/main/java/freemarker/template/utility/DateUtil.java
+++ b/src/main/java/freemarker/template/utility/DateUtil.java
@@ -925,6 +925,12 @@ public class DateUtil {
                     } else {
                         textStyle = TextStyle.SHORT_STANDALONE;
                     }
+
+                    if (textStyle == TextStyle.SHORT_STANDALONE
+                            && !JavaTimeBugFlags.hasGoodShortStandaloneMonth(locale)) {
+                        textStyle = TextStyle.SHORT;
+                    }
+
                     builder.appendText(ChronoField.MONTH_OF_YEAR, textStyle);
                 } else {
                     TextStyle textStyle;
@@ -934,6 +940,12 @@ public class DateUtil {
                     } else {
                         textStyle = TextStyle.FULL_STANDALONE;
                     }
+
+                    if (textStyle == TextStyle.FULL_STANDALONE
+                            && !JavaTimeBugFlags.hasGoodShortStandaloneMonth(locale)) {
+                        textStyle = TextStyle.FULL;
+                    }
+
                     builder.appendText(ChronoField.MONTH_OF_YEAR, textStyle);
                 }
                 break;
diff --git a/src/main/java/freemarker/template/utility/JavaTimeBugFlags.java b/src/main/java/freemarker/template/utility/JavaTimeBugFlags.java
new file mode 100644
index 0000000..c5e3a29
--- /dev/null
+++ b/src/main/java/freemarker/template/utility/JavaTimeBugFlags.java
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package freemarker.template.utility;
+
+import java.time.LocalDate;
+import java.time.format.DateTimeFormatterBuilder;
+import java.time.format.TextStyle;
+import java.time.temporal.ChronoField;
+import java.util.Locale;
+import java.util.concurrent.ConcurrentHashMap;
+
+import freemarker.log.Logger;
+
+/**
+ * This is to address https://bugs.openjdk.java.net/browse/JDK-8146356
+ * TODO: Detect if the Java version is high enough to always return a fixed value?
+ */
+class JavaTimeBugFlags {
+    private static final Logger LOG = Logger.getLogger("freemarker.runtime");
+    // There are around 160 predefined locales, so this should be enough even if the application uses some variants.
+    private static final int LEAK_ALERT_CACHE_SIZE = 1024;
+
+    private static final LocalDate PROBE_LOCAL_DATE = LocalDate.of(2021, 12, 1);
+
+    private static final ConcurrentHashMap<Locale, Boolean> HAS_GOOD_SHORT_STANDALONE_MONTH = new ConcurrentHashMap<>();
+    private static final ConcurrentHashMap<Locale, Boolean> HAS_GOOD_FULL_STANDALONE_MONTH = new ConcurrentHashMap<>();
+
+    static boolean hasGoodShortStandaloneMonth(Locale locale) {
+        return hasGoodStandaloneMonth(
+                locale,
+                TextStyle.SHORT_STANDALONE, JavaTimeBugFlags.HAS_GOOD_SHORT_STANDALONE_MONTH);
+    }
+
+    static boolean hasGoodFullStandaloneMonth(Locale locale) {
+        return hasGoodStandaloneMonth(
+                locale,
+                TextStyle.FULL_STANDALONE,
+                JavaTimeBugFlags.HAS_GOOD_FULL_STANDALONE_MONTH);
+    }
+
+    private static boolean hasGoodStandaloneMonth(
+            Locale locale, TextStyle textStyle,
+            ConcurrentHashMap<Locale, Boolean> cacheMap) {
+        Boolean good = cacheMap.get(locale);
+        if (good != null) {
+            return good;
+        }
+
+        if (cacheMap.size() >= LEAK_ALERT_CACHE_SIZE) {
+            boolean triggered = false;
+            synchronized (JavaTimeBugFlags.class) {
+                if (cacheMap.size() >= LEAK_ALERT_CACHE_SIZE) {
+                    triggered = true;
+                    cacheMap.clear();
+                }
+            }
+            if (triggered) {
+                LOG.warn("Global HAS_GOOD_STANDALONE_MONTH cache for " + textStyle
+                        + " has exceeded " + LEAK_ALERT_CACHE_SIZE
+                        + " entries => cache flushed. "
+                        + "Typical cause: Something generates high variety of Locale objects.");
+            }
+        }
+
+        String formattedMonth = new DateTimeFormatterBuilder()
+                .appendText(ChronoField.MONTH_OF_YEAR, textStyle)
+                .toFormatter(locale)
+                .format(PROBE_LOCAL_DATE);
+        good = !formattedMonth.equals("12");
+        cacheMap.put(locale, good);
+        return good;
+    }
+}
diff --git a/src/test/java/freemarker/template/utility/DateUtilsPatternParsingTest.java b/src/test/java/freemarker/template/utility/DateUtilsPatternParsingTest.java
index 41021d0..d77b9ab 100644
--- a/src/test/java/freemarker/template/utility/DateUtilsPatternParsingTest.java
+++ b/src/test/java/freemarker/template/utility/DateUtilsPatternParsingTest.java
@@ -29,23 +29,16 @@ import java.time.ZoneId;
 import java.time.ZoneOffset;
 import java.time.ZonedDateTime;
 import java.time.format.DateTimeFormatter;
-import java.time.format.DateTimeFormatterBuilder;
 import java.time.format.DateTimeParseException;
-import java.time.format.TextStyle;
-import java.time.temporal.ChronoField;
 import java.time.temporal.Temporal;
 import java.util.Date;
-import java.util.List;
 import java.util.Locale;
 import java.util.TimeZone;
-import java.util.stream.Collectors;
 
 import org.apache.commons.lang.StringUtils;
 import org.hamcrest.Matchers;
 import org.junit.Test;
 
-import com.google.common.collect.ImmutableList;
-
 /**
  * Move pattern parsing related tests from {@link DateUtilTest} to here.
  */
@@ -68,39 +61,19 @@ public class DateUtilsPatternParsingTest {
     // Most likely supported on different test systems
     private static final Locale SAMPLE_LOCALE = Locale.US;
 
-    private static final List<Locale> SAMPLE_LOCALES;
-    static {
-        LocalDate localDate = LocalDate.of(2021, 12, 1);
-        SAMPLE_LOCALES = ImmutableList.of(
-                // Locales picked more or less arbitrarily, in alphabetical order
-                Locale.CHINA,
-                new Locale("ar", "AE"),
-                new Locale("fi", "FI"),
-                Locale.GERMAN,
-                new Locale("hi", "IN"),
-                Locale.JAPANESE,
-                Locale.ROOT,
-                new Locale("ru", "RU"),
-                Locale.US,
-                new Locale("th", "TH") // Uses buddhist calendar
-        ).stream()
-                .filter(locale -> !(
-                        new DateTimeFormatterBuilder()
-                                .appendText(ChronoField.MONTH_OF_YEAR, TextStyle.SHORT_STANDALONE)
-                                .toFormatter(locale)
-                                .format(localDate))
-                        .equals("12"))
-                .collect(Collectors.toList());
-        System.out.println("!!T Sample locales: " + SAMPLE_LOCALES); // TODO Remove this
-    }
-
-    @Test
-    public void testHasEnoughSampleLocales() {
-        if (SAMPLE_LOCALES.size() < 4) {
-            throw new AssertionError("Too many locales were filtered out from SAMPLE_LOCALE. " +
-                    "We only have these left: " + SAMPLE_LOCALES);
-        }
-    }
+    private static final Locale[] SAMPLE_LOCALES = new Locale[]{
+            // Locales picked more or less arbitrarily, in alphabetical order
+            Locale.CHINA,
+            new Locale("ar", "AE"),
+            new Locale("fi", "FI"),
+            Locale.GERMAN,
+            new Locale("hi", "IN"),
+            Locale.JAPANESE,
+            Locale.ROOT,
+            new Locale("ru", "RU"),
+            Locale.US,
+            new Locale("th", "TH") // Uses buddhist calendar
+    };
 
     @Test
     public void testBasics() {