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 2020/06/08 00:38:35 UTC
[freemarker] 01/02: 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.
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 <dd...@apache.org>
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