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
commit 6edc0d25f9cc5560af8216b84ba8a785d3e1f1b1 Author: ddekany <[email protected]> AuthorDate: Sat Jun 6 22:00:26 2020 +0200 FREEMARKER-35: Minor cleanup. Also prepared the code for the case if in a later Java version the directly supported Temporal subclasses will not be final. Also improved some JavaDoc commends in old code. --- .../freemarker/core/BuiltInsForMultipleTypes.java | 2 +- src/main/java/freemarker/core/Configurable.java | 26 +++++- src/main/java/freemarker/core/Environment.java | 12 +-- src/main/java/freemarker/core/EvalUtil.java | 4 +- .../java/freemarker/core/TemplateDateFormat.java | 6 +- .../freemarker/core/TemplateDateFormatFactory.java | 4 +- .../core/TemplateNumberFormatFactory.java | 4 +- .../freemarker/core/TemplateTemporalFormat.java | 2 +- .../java/freemarker/core/TemplateValueFormat.java | 3 +- .../java/freemarker/core/_CoreTemporalUtils.java | 99 ++++++++++++++++++++++ .../freemarker/template/utility/TemporalUtil.java | 8 +- .../freemarker/core/TemporalConfigurableTest.java | 57 +++++++++++++ 12 files changed, 208 insertions(+), 19 deletions(-) diff --git a/src/main/java/freemarker/core/BuiltInsForMultipleTypes.java b/src/main/java/freemarker/core/BuiltInsForMultipleTypes.java index 7eb65c4..1159d5d 100644 --- a/src/main/java/freemarker/core/BuiltInsForMultipleTypes.java +++ b/src/main/java/freemarker/core/BuiltInsForMultipleTypes.java @@ -616,7 +616,7 @@ class BuiltInsForMultipleTypes { TemporalFormatter(TemplateTemporalModel temporalModel, Environment env) throws TemplateException { this.temporalModel = temporalModel; this.env = env; - this.defaultFormat = env.getTemplateTemporalFormat(temporalModel.getAsTemporal()); + this.defaultFormat = env.getTemplateTemporalFormat(temporalModel.getAsTemporal().getClass()); } @Override diff --git a/src/main/java/freemarker/core/Configurable.java b/src/main/java/freemarker/core/Configurable.java index 08252b7..c891510 100644 --- a/src/main/java/freemarker/core/Configurable.java +++ b/src/main/java/freemarker/core/Configurable.java @@ -1404,6 +1404,20 @@ public class Configurable { return zonedDateTimeFormat == null ? parent.getZonedDateTimeFormat() : zonedDateTimeFormat; } + /** + * Gets the temporal format string for a given {@link Temporal} subclass. + * + * @param temporalClass The class of the object to format. Can't be {@code null}. Currently, the supported classes + * are these: {@link Instant}, {@link LocalDate}, {@link LocalDateTime}, {@link LocalTime}, + * {@link OffsetDateTime}, {@link OffsetTime}, {@link Year}, {@link YearMonth}, {@link ZonedDateTime}. + * + * @return Never {@code null}, maybe {@code ""} though. + * + * @throws NullPointerException If {@link temporalClass} was {@code null} + * @throws IllegalArgumentException If {@link temporalClass} was not a supported {@link Temporal} subclass. + * + * @since 2.3.31 + */ public String getTemporalFormat(Class<? extends Temporal> temporalClass) { if (temporalClass == Instant.class) { return getInstantFormat(); @@ -1417,14 +1431,20 @@ public class Configurable { return getOffsetDateTimeFormat(); } else if (temporalClass == OffsetTime.class) { return getOffsetTimeFormat(); + } else if (temporalClass == ZonedDateTime.class) { + return getZonedDateTimeFormat(); } else if (temporalClass == Year.class) { return getYearFormat(); } else if (temporalClass == YearMonth.class) { return getYearMonthFormat(); - } else if (temporalClass == ZonedDateTime.class) { - return getZonedDateTimeFormat(); } else { - return ""; + Class<? extends Temporal> normTemporalClass = _CoreTemporalUtils.normalizeSupportedTemporalClass(temporalClass); + if (normTemporalClass == temporalClass) { + throw new IllegalArgumentException("There's no temporal format seting for this class: " + + temporalClass.getName()); + } else { + return getTemporalFormat(normTemporalClass); + } } } diff --git a/src/main/java/freemarker/core/Environment.java b/src/main/java/freemarker/core/Environment.java index fea5f7e..3a0a251 100644 --- a/src/main/java/freemarker/core/Environment.java +++ b/src/main/java/freemarker/core/Environment.java @@ -1771,7 +1771,7 @@ public final class Environment extends Configurable { * The blamed expression if an error occurs; only used for error messages. */ String formatTemporalToPlainText(TemplateTemporalModel ttm, String formatString, Expression blamedDateSourceExp) throws TemplateException, TemplateValueFormatException { - TemplateTemporalFormat ttf = getTemplateTemporalFormat(formatString); + TemplateTemporalFormat ttf = getTemplateTemporalFormat(ttm.getAsTemporal().getClass(), formatString, true); try { return EvalUtil.assertFormatResultNotNull(ttf.format(ttm)); } catch (TemplateValueFormatException e) { @@ -1780,7 +1780,7 @@ public final class Environment extends Configurable { } String formatTemporalToPlainText(TemplateTemporalModel ttm, Expression tdmSourceExpr) throws TemplateException { - TemplateTemporalFormat ttf = getTemplateTemporalFormat(configuration.getTemporalFormat(ttm.getAsTemporal().getClass())); + TemplateTemporalFormat ttf = getTemplateTemporalFormat(ttm.getAsTemporal().getClass()); try { return EvalUtil.assertFormatResultNotNull(ttf.format(ttm)); } catch (TemplateValueFormatException e) { @@ -2219,11 +2219,13 @@ public final class Environment extends Configurable { + (sqlDTTZ ? CACHED_TDFS_SQL_D_T_TZ_OFFS : 0); } - TemplateTemporalFormat getTemplateTemporalFormat(Temporal temporal) { - return new TemplateTemporalFormat(getTemporalFormat(temporal.getClass()), getLocale(), getTimeZone()); + TemplateTemporalFormat getTemplateTemporalFormat(Class<? extends Temporal> temporalClass) { + // TODO [FREEMARKER-35] Temporal class keyed cache, invalidated by temporalFormat (instantFormat, localDateFormat, etc.), locale and timeZone change. + return getTemplateTemporalFormat(temporalClass, getTemporalFormat(temporalClass), true); } - private TemplateTemporalFormat getTemplateTemporalFormat(String format) { + private TemplateTemporalFormat getTemplateTemporalFormat(Class<? extends Temporal> temporalClass, String format, boolean cache) { + // TODO [FREEMARKER-35] format keyed cache, invalidated by local and timeZone change. return new TemplateTemporalFormat(format, getLocale(), getTimeZone()); } diff --git a/src/main/java/freemarker/core/EvalUtil.java b/src/main/java/freemarker/core/EvalUtil.java index c086b60..50f32c5 100644 --- a/src/main/java/freemarker/core/EvalUtil.java +++ b/src/main/java/freemarker/core/EvalUtil.java @@ -409,7 +409,7 @@ class EvalUtil { } } else if (tm instanceof TemplateTemporalModel) { TemplateTemporalModel ttm = (TemplateTemporalModel) tm; - TemplateTemporalFormat format = env.getTemplateTemporalFormat(ttm.getAsTemporal()); + TemplateTemporalFormat format = env.getTemplateTemporalFormat(ttm.getAsTemporal().getClass()); try { return assertFormatResultNotNull(format.format(ttm)); } catch (TemplateValueFormatException e) { @@ -452,7 +452,7 @@ class EvalUtil { } } else if (tm instanceof TemplateTemporalModel) { TemplateTemporalModel ttm = (TemplateTemporalModel) tm; - TemplateTemporalFormat format = env.getTemplateTemporalFormat(ttm.getAsTemporal()); + TemplateTemporalFormat format = env.getTemplateTemporalFormat(ttm.getAsTemporal().getClass()); try { return ensureFormatResultString(format.format(ttm), exp, env); } catch (TemplateValueFormatException e) { diff --git a/src/main/java/freemarker/core/TemplateDateFormat.java b/src/main/java/freemarker/core/TemplateDateFormat.java index f5a2fb3..ec75627 100644 --- a/src/main/java/freemarker/core/TemplateDateFormat.java +++ b/src/main/java/freemarker/core/TemplateDateFormat.java @@ -31,7 +31,7 @@ import freemarker.template.TemplateModelException; * formats that can't be represented with Java's existing {@link DateFormat} implementations. * * <p> - * Implementations need not be thread-safe if the {@link TemplateNumberFormatFactory} doesn't recycle them among + * Implementations need not be thread-safe if the {@link TemplateDateFormatFactory} doesn't recycle them among * different {@link Environment}-s. As far as FreeMarker's concerned, instances are bound to a single * {@link Environment}, and {@link Environment}-s are thread-local objects. * @@ -87,7 +87,7 @@ public abstract class TemplateDateFormat extends TemplateValueFormat { * * @return The interpretation of the text either as a {@link Date} or {@link TemplateDateModel}. Typically, a * {@link Date}. {@link TemplateDateModel} is used if you have to attach some application-specific - * meta-information thats also extracted during {@link #formatToPlainText(TemplateDateModel)} (so if you format + * meta-information that's also extracted during {@link #formatToPlainText(TemplateDateModel)} (so if you format * something and then parse it, you get back an equivalent result). It can't be {@code null}. Known issue * (at least in FTL 2): {@code ?date}/{@code ?time}/{@code ?datetime}, when not invoked as a method, can't * return the {@link TemplateDateModel}, only the {@link Date} from inside it, hence the additional @@ -101,7 +101,7 @@ public abstract class TemplateDateFormat extends TemplateValueFormat { public abstract boolean isLocaleBound(); /** - * Tells if this formatter should be re-created if the time zone changes. Currently always {@code true}. + * Tells if this formatter should be re-created if the time zone changes. */ public abstract boolean isTimeZoneBound(); diff --git a/src/main/java/freemarker/core/TemplateDateFormatFactory.java b/src/main/java/freemarker/core/TemplateDateFormatFactory.java index f6651d4..38cae8b 100644 --- a/src/main/java/freemarker/core/TemplateDateFormatFactory.java +++ b/src/main/java/freemarker/core/TemplateDateFormatFactory.java @@ -75,7 +75,9 @@ public abstract class TemplateDateFormatFactory extends TemplateValueFormatFacto * says. * @param env * The runtime environment from which the formatting was called. This is mostly meant to be used for - * {@link Environment#setCustomState(Object, Object)}/{@link Environment#getCustomState(Object)}. + * {@link Environment#setCustomState(Object, Object)}/{@link Environment#getCustomState(Object)}. The + * result shouldn't depend on setting values in the {@link Environment}, as changing other setting + * will not necessarily invalidate the result. * * @throws TemplateValueFormatException * If any problem occurs while parsing/getting the format. Notable subclasses: diff --git a/src/main/java/freemarker/core/TemplateNumberFormatFactory.java b/src/main/java/freemarker/core/TemplateNumberFormatFactory.java index cee3478..684fef2 100644 --- a/src/main/java/freemarker/core/TemplateNumberFormatFactory.java +++ b/src/main/java/freemarker/core/TemplateNumberFormatFactory.java @@ -52,7 +52,9 @@ public abstract class TemplateNumberFormatFactory extends TemplateValueFormatFac * forever (i.e. locale changes in the {@link Environment} must not be followed). * @param env * The runtime environment from which the formatting was called. This is mostly meant to be used for - * {@link Environment#setCustomState(Object, Object)}/{@link Environment#getCustomState(Object)}. + * {@link Environment#setCustomState(Object, Object)}/{@link Environment#getCustomState(Object)}. The + * result shouldn't depend on setting values in the {@link Environment}, as changing other setting + * will not necessarily invalidate the result. * * @throws TemplateValueFormatException * if any problem occurs while parsing/getting the format. Notable subclasses: diff --git a/src/main/java/freemarker/core/TemplateTemporalFormat.java b/src/main/java/freemarker/core/TemplateTemporalFormat.java index c946913..aec3844 100644 --- a/src/main/java/freemarker/core/TemplateTemporalFormat.java +++ b/src/main/java/freemarker/core/TemplateTemporalFormat.java @@ -53,7 +53,7 @@ public class TemplateTemporalFormat extends TemplateValueFormat { } /** - * Tells if this formatter should be re-created if the time zone changes. Currently always {@code true}. + * Tells if this formatter should be re-created if the time zone changes. */ public boolean isTimeZoneBound() { return true; diff --git a/src/main/java/freemarker/core/TemplateValueFormat.java b/src/main/java/freemarker/core/TemplateValueFormat.java index f7bcc0e..488fc8e 100644 --- a/src/main/java/freemarker/core/TemplateValueFormat.java +++ b/src/main/java/freemarker/core/TemplateValueFormat.java @@ -26,7 +26,8 @@ package freemarker.core; public abstract class TemplateValueFormat { /** - * Meant to be used in error messages to tell what format the parsed string didn't fit. + * Meant to be used in error messages to tell what format the parsed string didn't fit, or rarely, what format the + * value to format wasn't compatible with. */ public abstract String getDescription(); diff --git a/src/main/java/freemarker/core/_CoreTemporalUtils.java b/src/main/java/freemarker/core/_CoreTemporalUtils.java new file mode 100644 index 0000000..027c13d --- /dev/null +++ b/src/main/java/freemarker/core/_CoreTemporalUtils.java @@ -0,0 +1,99 @@ +/* + * 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.core; + +import java.lang.reflect.Modifier; +import java.time.Instant; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.LocalTime; +import java.time.OffsetDateTime; +import java.time.OffsetTime; +import java.time.Year; +import java.time.YearMonth; +import java.time.ZonedDateTime; +import java.time.temporal.Temporal; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Stream; + +/** + * For internal use only; don't depend on this, there's no backward compatibility guarantee at all! + * This class is to work around the lack of module system in Java, i.e., so that other FreeMarker packages can + * access things inside this package that users shouldn't. + */ +public class _CoreTemporalUtils { + + private _CoreTemporalUtils() { + // No meant to be instantiated + } + + /** + * {@link Temporal} subclasses directly supperted by FreeMarker. + */ + public static final List<Class<? extends Temporal>> SUPPORTED_TEMPORAL_CLASSES = Arrays.asList( + Instant.class, + LocalDate.class, + LocalDateTime.class, + LocalTime.class, + OffsetDateTime.class, + OffsetTime.class, + ZonedDateTime.class, + Year.class, + YearMonth.class); + + static final boolean SUPPORTED_TEMPORAL_CLASSES_ARE_FINAL = SUPPORTED_TEMPORAL_CLASSES.stream() + .allMatch(cl -> (cl.getModifiers() & Modifier.FINAL) == Modifier.FINAL); + + /** + * Ensures that {@code ==} can be used to check if the class is assignable to one of the {@link Temporal} subclasses + * that FreeMarker directly supports. At least in Java 8 they are all final anyway, but just in case this changes in + * a future Java version, use this method before using {@code ==}. + * + * @since 2.3.31 + */ + public static Class<? extends Temporal> normalizeSupportedTemporalClass(Class<? extends Temporal> temporalClass) { + if (SUPPORTED_TEMPORAL_CLASSES_ARE_FINAL) { + return temporalClass; + } else { + if (Instant.class.isAssignableFrom(temporalClass)) { + return Instant.class; + } else if (LocalDate.class.isAssignableFrom(temporalClass)) { + return LocalDate.class; + } else if (LocalDateTime.class.isAssignableFrom(temporalClass)) { + return LocalDateTime.class; + } else if (LocalTime.class.isAssignableFrom(temporalClass)) { + return LocalTime.class; + } else if (OffsetDateTime.class.isAssignableFrom(temporalClass)) { + return OffsetDateTime.class; + } else if (OffsetTime.class.isAssignableFrom(temporalClass)) { + return OffsetTime.class; + } else if (Year.class.isAssignableFrom(temporalClass)) { + return Year.class; + } else if (YearMonth.class.isAssignableFrom(temporalClass)) { + return YearMonth.class; + } else if (ZonedDateTime.class.isAssignableFrom(temporalClass)) { + return ZonedDateTime.class; + } else { + return temporalClass; + } + } + } +} diff --git a/src/main/java/freemarker/template/utility/TemporalUtil.java b/src/main/java/freemarker/template/utility/TemporalUtil.java index 25756c5..a1c9d73 100644 --- a/src/main/java/freemarker/template/utility/TemporalUtil.java +++ b/src/main/java/freemarker/template/utility/TemporalUtil.java @@ -18,9 +18,13 @@ */ package freemarker.template.utility; +import java.lang.reflect.Modifier; import java.time.Instant; +import java.time.LocalDate; +import java.time.LocalDateTime; import java.time.LocalTime; import java.time.OffsetDateTime; +import java.time.OffsetTime; import java.time.Year; import java.time.YearMonth; import java.time.ZoneOffset; @@ -34,6 +38,7 @@ import java.time.temporal.Temporal; import java.util.Locale; import java.util.TimeZone; import java.util.regex.Pattern; +import java.util.stream.Stream; public class TemporalUtil { private static final Pattern FORMAT_STYLE_PATTERN = Pattern.compile("^(short|medium|long|full)(_(short|medium|long|full))?$"); @@ -136,7 +141,6 @@ public class TemporalUtil { if (temporal instanceof Instant) temporal = ((Instant) temporal).atZone(timeZone == null ? ZoneOffset.UTC : timeZone.toZoneId()); - String[] formatSplt = format.split("_"); DateTimeFormatter dtf; if ("xs".equals(format)) dtf = getXSFormatter(temporal); @@ -145,6 +149,7 @@ public class TemporalUtil { else if (FORMAT_STYLE_PATTERN.matcher(format).matches()) { boolean isYear = temporal instanceof Year; boolean isYearMonth = temporal instanceof YearMonth; + String[] formatSplt = format.split("_"); if (isYear || isYearMonth) { String reducedPattern = DateTimeFormatterBuilder.getLocalizedDateTimePattern(FormatStyle.valueOf(formatSplt[0].toUpperCase()), null, IsoChronology.INSTANCE, locale); if (isYear) @@ -188,4 +193,5 @@ public class TemporalUtil { } return newPattern.toString(); } + } diff --git a/src/test/java/freemarker/core/TemporalConfigurableTest.java b/src/test/java/freemarker/core/TemporalConfigurableTest.java new file mode 100644 index 0000000..92cb575 --- /dev/null +++ b/src/test/java/freemarker/core/TemporalConfigurableTest.java @@ -0,0 +1,57 @@ +/* + * 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.core; + +import static org.junit.Assert.*; + +import java.time.Instant; +import java.time.chrono.ChronoLocalDate; +import java.time.temporal.Temporal; + +import org.junit.Test; + +import freemarker.template.Configuration; + +public class TemporalConfigurableTest { + + @Test + public void testSupportedTemporalClassAreFinal() { + assertTrue( + "FreeMarker was implemented with the assumption that temporal classes are final. While there " + + "are mesures in palce to handle if it's not a case, it would be better to review the code.", + _CoreTemporalUtils.SUPPORTED_TEMPORAL_CLASSES_ARE_FINAL); + } + + @Test + public void testGetTemporalFormat() { + Configuration cfg = new Configuration(Configuration.VERSION_2_3_31); + for (Class<? extends Temporal> supportedTemporalClass : _CoreTemporalUtils.SUPPORTED_TEMPORAL_CLASSES) { + assertNotNull(cfg.getTemporalFormat(supportedTemporalClass)); + } + + try { + assertNotNull(cfg.getTemporalFormat(ChronoLocalDate.class)); + fail(); + } catch (IllegalArgumentException e) { + // Expected + } + } + +} \ No newline at end of file
