From 01b4c1e4ec2a4a4a545a1329015903bb2a93b238 Mon Sep 17 00:00:00 2001 From: Richard Zowalla Date: Sat, 29 Aug 2026 19:13:44 +0200 Subject: [PATCH] [OPENJPA-2983] Resolve EXTRACT temporal fields by constant, not by name CriteriaBuilderImpl.extract() derived the DateTimeExtractField from field.toString().toUpperCase(), which depends on the default locale and on the textual representation the specification constants happen to use. Map the LocalDateField / LocalTimeField / LocalDateTimeField constants to their DateTimeExtractField equivalents by identity instead, and reject any other temporal field with a meaningful message instead of letting Enum.valueOf fail. TestExtractTemporalField pins every entry of the new mapping table, which is where the risk now sits: it is a characterisation test of the 16 hand written pairs, not a reproduction of the original defect, since the name based lookup produced the same result for all constants once Locale.ROOT was passed. --- .../criteria/CriteriaBuilderImpl.java | 42 +++++++- .../persistence/criteria/Expressions.java | 4 + .../criteria/TestExtractTemporalField.java | 100 ++++++++++++++++++ 3 files changed, 142 insertions(+), 4 deletions(-) create mode 100644 openjpa-persistence/src/test/java/org/apache/openjpa/persistence/criteria/TestExtractTemporalField.java diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/CriteriaBuilderImpl.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/CriteriaBuilderImpl.java index 2200b10a88..864bce194b 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/CriteriaBuilderImpl.java +++ b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/CriteriaBuilderImpl.java @@ -29,8 +29,8 @@ import java.time.temporal.Temporal; import java.util.Collection; import java.util.Collections; +import java.util.IdentityHashMap; import java.util.List; -import java.util.Locale; import java.util.Map; import java.util.Set; @@ -44,6 +44,9 @@ import jakarta.persistence.criteria.From; import jakarta.persistence.criteria.Join; import jakarta.persistence.criteria.ListJoin; +import jakarta.persistence.criteria.LocalDateField; +import jakarta.persistence.criteria.LocalDateTimeField; +import jakarta.persistence.criteria.LocalTimeField; import jakarta.persistence.criteria.MapJoin; import jakarta.persistence.criteria.Nulls; import jakarta.persistence.criteria.Order; @@ -83,6 +86,34 @@ */ public class CriteriaBuilderImpl implements OpenJPACriteriaBuilder, ExpressionParser { private static final long serialVersionUID = 1L; + + /** + * The temporal fields EXTRACT accepts, keyed by the constants defined by the specification. + * These constants are not enums, but they are the only instances their classes hand out, so + * they are matched by identity rather than by their (locale sensitive, lower case) name. + */ + private static final Map, DateTimeExtractField> EXTRACT_FIELDS; + static { + Map, DateTimeExtractField> fields = new IdentityHashMap<>(); + fields.put(LocalDateField.YEAR, DateTimeExtractField.YEAR); + fields.put(LocalDateField.QUARTER, DateTimeExtractField.QUARTER); + fields.put(LocalDateField.MONTH, DateTimeExtractField.MONTH); + fields.put(LocalDateField.WEEK, DateTimeExtractField.WEEK); + fields.put(LocalDateField.DAY, DateTimeExtractField.DAY); + fields.put(LocalTimeField.HOUR, DateTimeExtractField.HOUR); + fields.put(LocalTimeField.MINUTE, DateTimeExtractField.MINUTE); + fields.put(LocalTimeField.SECOND, DateTimeExtractField.SECOND); + fields.put(LocalDateTimeField.YEAR, DateTimeExtractField.YEAR); + fields.put(LocalDateTimeField.QUARTER, DateTimeExtractField.QUARTER); + fields.put(LocalDateTimeField.MONTH, DateTimeExtractField.MONTH); + fields.put(LocalDateTimeField.WEEK, DateTimeExtractField.WEEK); + fields.put(LocalDateTimeField.DAY, DateTimeExtractField.DAY); + fields.put(LocalDateTimeField.HOUR, DateTimeExtractField.HOUR); + fields.put(LocalDateTimeField.MINUTE, DateTimeExtractField.MINUTE); + fields.put(LocalDateTimeField.SECOND, DateTimeExtractField.SECOND); + EXTRACT_FIELDS = Collections.unmodifiableMap(fields); + } + private MetamodelImpl _model; public OpenJPACriteriaBuilder setMetaModel(MetamodelImpl model) { @@ -1202,10 +1233,13 @@ public Expression replace(Expression str, String substring, Stri @Override public Expression extract(TemporalField field, Expression temporal) { - String fieldName = field.toString().toUpperCase(Locale.ROOT); - DateTimeExtractField extractField = DateTimeExtractField.valueOf(fieldName); + DateTimeExtractField extractField = EXTRACT_FIELDS.get(field); + if (extractField == null) { + throw new IllegalArgumentException("Unsupported temporal field for EXTRACT: " + field); + } @SuppressWarnings("unchecked") - Class resultType = fieldName.equals("SECOND") ? (Class) Double.class : (Class) Integer.class; + Class resultType = (Class) (extractField == DateTimeExtractField.SECOND + ? Double.class : Integer.class); return new Expressions.ExtractField<>(resultType, extractField, temporal); } diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/Expressions.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/Expressions.java index 02d247f265..4d03e40d80 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/Expressions.java +++ b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/Expressions.java @@ -414,6 +414,10 @@ public ExtractField(Class resultType, DateTimeExtractField field, Expression< this.field = field; } + public DateTimeExtractField getField() { + return field; + } + @Override public Value toValue(ExpressionFactory factory, CriteriaQueryImpl q) { Value value = factory.getDateTimeField(field, Expressions.toValue(e, factory, q)); diff --git a/openjpa-persistence/src/test/java/org/apache/openjpa/persistence/criteria/TestExtractTemporalField.java b/openjpa-persistence/src/test/java/org/apache/openjpa/persistence/criteria/TestExtractTemporalField.java new file mode 100644 index 0000000000..815a3055c7 --- /dev/null +++ b/openjpa-persistence/src/test/java/org/apache/openjpa/persistence/criteria/TestExtractTemporalField.java @@ -0,0 +1,100 @@ +/* + * 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 org.apache.openjpa.persistence.criteria; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThrows; + +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.LocalTime; +import java.time.temporal.Temporal; + +import jakarta.persistence.criteria.Expression; +import jakarta.persistence.criteria.LocalDateField; +import jakarta.persistence.criteria.LocalDateTimeField; +import jakarta.persistence.criteria.LocalTimeField; +import jakarta.persistence.criteria.TemporalField; + +import org.apache.openjpa.kernel.exps.DateTimeExtractField; +import org.junit.Test; + +/** + * Tests that EXTRACT resolves each temporal field constant of the specification to its + * kernel equivalent, rather than deriving it from the constant's textual representation. + */ +public class TestExtractTemporalField { + + private final CriteriaBuilderImpl cb = new CriteriaBuilderImpl(); + + @Test + public void testLocalDateFields() { + assertExtracts(DateTimeExtractField.YEAR, LocalDateField.YEAR, LocalDate.class); + assertExtracts(DateTimeExtractField.QUARTER, LocalDateField.QUARTER, LocalDate.class); + assertExtracts(DateTimeExtractField.MONTH, LocalDateField.MONTH, LocalDate.class); + assertExtracts(DateTimeExtractField.WEEK, LocalDateField.WEEK, LocalDate.class); + assertExtracts(DateTimeExtractField.DAY, LocalDateField.DAY, LocalDate.class); + } + + @Test + public void testLocalTimeFields() { + assertExtracts(DateTimeExtractField.HOUR, LocalTimeField.HOUR, LocalTime.class); + assertExtracts(DateTimeExtractField.MINUTE, LocalTimeField.MINUTE, LocalTime.class); + assertExtracts(DateTimeExtractField.SECOND, LocalTimeField.SECOND, LocalTime.class); + } + + @Test + public void testLocalDateTimeFields() { + assertExtracts(DateTimeExtractField.YEAR, LocalDateTimeField.YEAR, LocalDateTime.class); + assertExtracts(DateTimeExtractField.QUARTER, LocalDateTimeField.QUARTER, LocalDateTime.class); + assertExtracts(DateTimeExtractField.MONTH, LocalDateTimeField.MONTH, LocalDateTime.class); + assertExtracts(DateTimeExtractField.WEEK, LocalDateTimeField.WEEK, LocalDateTime.class); + assertExtracts(DateTimeExtractField.DAY, LocalDateTimeField.DAY, LocalDateTime.class); + assertExtracts(DateTimeExtractField.HOUR, LocalDateTimeField.HOUR, LocalDateTime.class); + assertExtracts(DateTimeExtractField.MINUTE, LocalDateTimeField.MINUTE, LocalDateTime.class); + assertExtracts(DateTimeExtractField.SECOND, LocalDateTimeField.SECOND, LocalDateTime.class); + } + + /** + * DATE and TIME are extracted as a part rather than as a field; OpenJPA supports them in + * JPQL only, so the criteria API must reject them with a meaningful error. + */ + @Test + public void testUnsupportedFieldIsRejected() { + assertThrows(IllegalArgumentException.class, + () -> extract(LocalDateTimeField.DATE, LocalDateTime.class)); + assertThrows(IllegalArgumentException.class, + () -> extract(LocalDateTimeField.TIME, LocalDateTime.class)); + assertThrows(IllegalArgumentException.class, + () -> extract(new TemporalField() { }, LocalDate.class)); + } + + private void assertExtracts(DateTimeExtractField expected, + TemporalField field, Class temporalType) { + Expression e = extract(field, temporalType); + assertSame(field.toString(), expected, ((Expressions.ExtractField) e).getField()); + assertEquals(field.toString(), + expected == DateTimeExtractField.SECOND ? Double.class : Integer.class, e.getJavaType()); + } + + private Expression extract(TemporalField field, Class temporalType) { + return cb.extract(field, new Expressions.Constant<>(temporalType, null)); + } +}