This is an automated email from the ASF dual-hosted git repository.
desruisseaux pushed a commit to branch geoapi-4.0
in repository https://gitbox.apache.org/repos/asf/sis.git
The following commit(s) were added to refs/heads/geoapi-4.0 by this push:
new 9f766a14c9 Consolidation of the conversion of temporal objects before
comparison. It fixes a `NullPointerException` when there is no converter
available.
9f766a14c9 is described below
commit 9f766a14c9c4365cdd27e6516e98a3e8e9532214
Author: Martin Desruisseaux <[email protected]>
AuthorDate: Mon Jan 26 13:07:30 2026 +0100
Consolidation of the conversion of temporal objects before comparison.
It fixes a `NullPointerException` when there is no converter available.
---
.../org/apache/sis/filter/ComparisonFilter.java | 51 ++++----
.../main/org/apache/sis/temporal/TimeMethods.java | 131 +++++++++++++++------
.../main/org/apache/sis/converter/ClassPair.java | 2 +-
.../apache/sis/converter/ConverterRegistry.java | 2 +-
.../org/apache/sis/converter/DateConverter.java | 14 ++-
.../org/apache/sis/converter/StringConverter.java | 1 +
.../org/apache/sis/converter/SystemRegistry.java | 2 +-
.../main/org/apache/sis/system/Loggers.java | 3 +-
8 files changed, 141 insertions(+), 65 deletions(-)
diff --git
a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/ComparisonFilter.java
b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/ComparisonFilter.java
index 471fd683fd..db3a7cf787 100644
---
a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/ComparisonFilter.java
+++
b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/ComparisonFilter.java
@@ -19,8 +19,6 @@ package org.apache.sis.filter;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.time.DateTimeException;
-import java.time.temporal.Temporal;
-import java.util.Date;
import java.util.List;
import java.util.Collection;
import java.util.Objects;
@@ -170,15 +168,17 @@ abstract class ComparisonFilter<R> extends
BinaryFunctionWidening<R, Object, Obj
final Filter<R> result =
Optimization.OnFilter.super.optimize(optimization);
if (result instanceof ComparisonFilter<?>) {
final var optimized = (ComparisonFilter<R>) result;
- final var numeric = optimized.new Numeric();
- if (numeric.evaluator != null) {
- return numeric;
- }
- final Class<?> type1, type2;
- if (isTemporal(type1 = getResultClass(expression1)) &&
isTemporal(type2 = getResultClass(expression2))) {
- final TimeMethods<?> methods = TimeMethods.forTypes(type1,
type2);
- if (methods != null) {
- return optimized.new
Time(methods.predicate(temporalTest(), type2));
+ final Class<?> t1, t2;
+ if (isSpecialized(t1 = getResultClass(expression1)) &&
+ isSpecialized(t2 = getResultClass(expression2)))
+ {
+ final var numeric = optimized.new Numeric();
+ if (numeric.evaluator != null) {
+ return numeric;
+ }
+ final var temporal = new Time<>(TimeMethods.forTypes(t1, t2),
t2);
+ if (temporal.evaluator != null) {
+ return temporal;
}
}
}
@@ -186,12 +186,12 @@ abstract class ComparisonFilter<R> extends
BinaryFunctionWidening<R, Object, Obj
}
/**
- * Returns whether the given type is considered temporal for the purpose
of the comparison filter.
- * This is necessary for avoiding the comparable objects such as {@link
String} are wrongly handled
- * by {@link TimeMethods}.
+ * Returns whether the given type is non-null and something more
specialized than {@code Object}.
+ * This is used for avoiding unnecessary class-loading of {@link Numeric}
and {@link Time} when
+ * they are sure to be unsuccessful.
*/
- private static boolean isTemporal(final Class<?> type) {
- return (type != null) && (Temporal.class.isAssignableFrom(type) ||
Date.class.isAssignableFrom(type));
+ private static boolean isSpecialized(final Class<?> type) {
+ return (type != null) && (type != Object.class);
}
/**
@@ -222,15 +222,17 @@ abstract class ComparisonFilter<R> extends
BinaryFunctionWidening<R, Object, Obj
/**
* An optimized versions of this filter for the case where the operands
are temporal.
*/
- private final class Time extends Node implements Filter<R> {
+ private final class Time<T,S> extends Node implements Filter<R> {
/** For cross-version compatibility during (de)serialization. */
private static final long serialVersionUID = -5132906457258846016L;
/** The function which performs the comparisons. */
- @SuppressWarnings("serial") final BiPredicate<?,?> evaluator;
+ @SuppressWarnings("serial") final BiPredicate<T,S> evaluator;
- /** Creates a new filter. */
- Time(final BiPredicate<?,?> evaluator) {this.evaluator = evaluator;}
+ /** Creates a new filter. Callers must verifies that {@link
#evaluator} is non-null. */
+ Time(final TimeMethods<T> methods, final Class<S> otherType) {
+ evaluator = (methods != null) ? methods.predicate(temporalTest(),
otherType) : null;
+ }
/** Delegates to the enclosing class.*/
@Override public CodeList<?> getOperatorType() {return
ComparisonFilter.this.getOperatorType();}
@@ -239,13 +241,14 @@ abstract class ComparisonFilter<R> extends
BinaryFunctionWidening<R, Object, Obj
@Override protected Collection<?> getChildren() {return
ComparisonFilter.this.getChildren();}
/** Determines if the test represented by this filter passes with the
given operands. */
- @SuppressWarnings("unchecked")
@Override public boolean test(final R candidate) {
- final Object left = expression1.apply(candidate);
+ @SuppressWarnings("unchecked")
+ final T left = (T) expression1.apply(candidate);
if (left != null) {
- final Object right = expression2.apply(candidate);
+ @SuppressWarnings("unchecked")
+ final S right = (S) expression2.apply(candidate);
if (right != null) {
- return ((BiPredicate) evaluator).test(left, right);
+ return evaluator.test(left, right);
}
}
return false;
diff --git
a/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/temporal/TimeMethods.java
b/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/temporal/TimeMethods.java
index 03fde3581c..5bbf882899 100644
---
a/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/temporal/TimeMethods.java
+++
b/endorsed/src/org.apache.sis.metadata/main/org/apache/sis/temporal/TimeMethods.java
@@ -20,6 +20,7 @@ import java.util.Map;
import java.util.Date;
import java.util.Calendar;
import java.util.Optional;
+import java.util.logging.Logger;
import java.util.function.Supplier;
import java.util.function.Function;
import java.util.function.BiFunction;
@@ -47,7 +48,10 @@ import java.lang.reflect.Modifier;
import java.io.Serializable;
import java.io.ObjectStreamException;
import org.apache.sis.util.Classes;
+import org.apache.sis.util.ObjectConverters;
+import org.apache.sis.util.UnconvertibleObjectException;
import org.apache.sis.util.internal.shared.Strings;
+import org.apache.sis.util.logging.Logging;
import org.apache.sis.util.resources.Errors;
@@ -70,6 +74,11 @@ import org.apache.sis.util.resources.Errors;
* @author Martin Desruisseaux (Geomatys)
*/
public final class TimeMethods<T> implements Serializable {
+ /**
+ * Logger for warnings.
+ */
+ private static final Logger LOGGER =
Logger.getLogger("org.apache.sis.temporal");
+
/**
* The test to apply: equal, before or after.
*
@@ -160,8 +169,21 @@ public final class TimeMethods<T> implements Serializable {
/**
* Converter from an object of arbitrary class to an object of class
{@code <T>}, or {@code null} if none.
* The function may return {@code null} if the given object is an instance
of unsupported type.
+ *
+ * <p><b>Design note:</b> the purpose of this field overlaps with {@link
org.apache.sis.util.ObjectConverters}.
+ * Those converters are not yet registered as {@link
org.apache.sis.util.ObjectConverter}s because they expect
+ * {@code Object} in argument (which is too generic for {@code
ObjectConverter}) for performance reasons,
+ * conversions may lost information, and we may report these lost with
{@link #ignoringField(ChronoField)}
+ * in the future. Another difference is that the function returns {@code
null} instead of throwing an exception
+ * if an operand is not recognized.</p>
+ *
+ * @see #toOffsetDateTime(Object)
+ * @see #toLocalDateTime(Object)
+ * @see #toLocalDate(Object)
+ * @see #toLocalTime(Object)
+ * @see #toInstant(Object)
*/
- public final transient Function<Object, T> converter;
+ private final transient Function<Object, T> converter;
/**
* Predicate to execute for testing the ordering between temporal objects.
@@ -176,7 +198,7 @@ public final class TimeMethods<T> implements Serializable {
*
* @see #now()
*/
- public final transient Supplier<T> now;
+ private final transient Supplier<T> now;
/**
* Function to execute for getting another temporal object with the given
timezone, or {@code null} if none.
@@ -188,7 +210,7 @@ public final class TimeMethods<T> implements Serializable {
*
* @see #withZone(Object, ZoneId, boolean)
*/
- public final transient BiFunction<T, ZoneId, Temporal> withZone;
+ private final transient BiFunction<T, ZoneId, Temporal> withZone;
/**
* Whether the temporal object have a time zone, explicitly or implicitly.
@@ -226,24 +248,67 @@ public final class TimeMethods<T> implements Serializable
{
this.isDynamic = isDynamic;
}
+ /**
+ * Returns a converter from {@code otherType} to {@link #type},
+ * or {@code null} if there is no need for converter.
+ *
+ * @param <S> compile-time value of {@code otherType}.
+ * @param otherType the type of values expected as input by the
converter.
+ * @return converter from {@code otherType} to {@link #type}, or {@code
null}.
+ */
+ @SuppressWarnings("unchecked")
+ private <S> Function<? super S, T> converter(final Class<S> otherType) {
+ if (type.isAssignableFrom(otherType)) {
+ return null;
+ }
+ /*
+ * Check for more specific conversions before to fallback on
`converter` because the most specific
+ * cases are likely to be more efficient. We must exclude the
`java.util.Date` type because of the
+ * complication of the `java.sql.Date` and `java.sql.Time` subtypes.
+ */
+ if (otherType != Object.class &&
!Date.class.isAssignableFrom(otherType)) try {
+ final Function<? super S, ? extends T> castOrConvert =
ObjectConverters.find(otherType, type);
+ return (S other) -> {
+ try {
+ return castOrConvert.apply(other);
+ } catch (UnconvertibleObjectException e) {
+ Logging.ignorableException(LOGGER, TimeMethods.class,
"converter", e);
+ return null;
+ }
+ };
+ } catch (UnconvertibleObjectException e) {
+ Logging.ignorableException(LOGGER, TimeMethods.class, "converter",
e);
+ }
+ if (converter != null) {
+ return converter;
+ }
+ @SuppressWarnings("LocalVariableHidesMemberVariable")
+ final Class<T> type = this.type;
+ return (S other) -> type.isInstance(other) ? (T) other : null;
+ }
+
/**
* Returns the predicate to use for this test.
* The expected type of the first operand is always {@code <T>}.
* The expected type of the second operand will be either {@code <T>} or
{@code Object},
* depending on the value of {@code t2}.
*
- * @param test the test to apply (before, after and/or equal).
- * @param t2 expected class of the second operand.
+ * @param <S> compile-time value of {@code otherType}.
+ * @param test the test to apply (before, after and/or equal).
+ * @param otherType expected class of the second operand.
* @return the predicate for the requested test.
*/
- public final BiPredicate<T,?> predicate(final Test test, final Class<?>
t2) {
+ @SuppressWarnings("unchecked")
+ public final <S> BiPredicate<T,S> predicate(final Test test, final
Class<S> otherType) {
final BiPredicate<T,T> predicate = test.predicate(this);
- if (type.isAssignableFrom(t2)) {
- return predicate;
+ final Function<? super S, T> castOrConvert = converter(otherType);
+ if (castOrConvert == null) {
+ return (BiPredicate) predicate;
}
- @SuppressWarnings("LocalVariableHidesMemberVariable")
- Function<Object, T> converter = this.converter;
- return (T self, Object other) -> predicate.test(self,
converter.apply(other));
+ return (T self, S other) -> {
+ final T converted = castOrConvert.apply(other);
+ return (converted != null) && predicate.test(self, converted);
+ };
}
/**
@@ -305,6 +370,14 @@ public final class TimeMethods<T> implements Serializable {
return null;
}
+ /**
+ * Returns whether the given type is considered temporal. This is
necessary for avoiding that
+ * comparable objects such as {@link String} are wrongly handled by {@code
TimeMethods}.
+ */
+ private static boolean isTemporal(final Class<?> type) {
+ return (type != null) && (Temporal.class.isAssignableFrom(type) ||
Date.class.isAssignableFrom(type));
+ }
+
/**
* Returns {@code true} if both arguments are non-null and the specified
comparison evaluates to {@code true}.
* The type of the objects being compared is determined dynamically, which
has a performance cost.
@@ -432,7 +505,7 @@ adapt: if (type != other.getClass()) {
final TimeMethods<?> methods = forTypes(self.getClass(),
other.getClass(), false);
if (methods != null && !methods.isDynamic) {
assert methods.type.isInstance(self) : self;
- return ((TimeMethods) methods).convertAndCompareObject(test,
self, other);
+ return ((TimeMethods) methods).convertAndCompare(test, self,
other);
}
throw new
DateTimeException(Errors.format(Errors.Keys.CannotCompareInstanceOf_2,
self.getClass(), other.getClass()));
}
@@ -460,16 +533,20 @@ adapt: if (type != other.getClass()) {
/**
* Compares an object of class {@code <T>} with a temporal object of
arbitrary class.
+ * The other object is typically the beginning or ending instant of a
period and may
+ * be converted to the {@code <T>} type before comparison.
+ *
+ * <p>The type of the {@code other} argument should be {@link
TemporalAccessor},
+ * but the method signature uses {@code Object} for accepting also {@link
Date}.</p>
*
* @param test the test to apply (before, after and/or equal).
* @param self the object on which to invoke the method identified by
{@code test}.
- * @param other the argument to give to the test method call.
+ * @param other the argument to give to the test method call after
conversion.
* @return the result of performing the comparison identified by {@code
test}.
- * @throws ClassCastException if {@code self} or {@code other} is not an
instance of {@code type}.
* @throws DateTimeException if the two objects cannot be compared.
*/
@SuppressWarnings("unchecked")
- private boolean convertAndCompareObject(final Test test, final T self,
final Object other) {
+ public boolean convertAndCompare(final Test test, final T self, final
Object other) {
if (converter != null) {
final T converted = converter.apply(other);
if (converted != null) {
@@ -483,21 +560,6 @@ adapt: if (type != other.getClass()) {
throw new
DateTimeException(Errors.format(Errors.Keys.CannotCompareInstanceOf_2,
self.getClass(), other.getClass()));
}
- /**
- * Compares an object of class {@code <T>} with a temporal object of
arbitrary class.
- * The other object is typically the beginning or ending instant of a
period and may
- * be converted to the {@code <T>} type before comparison.
- *
- * @param test the test to apply (before, after and/or equal).
- * @param self the object on which to invoke the method identified by
{@code test}.
- * @param other the argument to give to the test method call.
- * @return the result of performing the comparison identified by {@code
test}.
- * @throws DateTimeException if the two objects cannot be compared.
- */
- public final boolean convertAndCompare(final Test test, final T self,
final TemporalAccessor other) {
- return convertAndCompareObject(test, self, other);
- }
-
/**
* Returns the given object as a temporal accessor.
*
@@ -549,7 +611,10 @@ adapt: if (type != other.getClass()) {
* @return set of comparison methods for operands of the given types, or
{@code null} if not found.
*/
public static TimeMethods<?> forTypes(final Class<?> self, final Class<?>
other) {
- return forTypes(self, other, true);
+ if (isTemporal(self) && isTemporal(other)) {
+ return forTypes(self, other, true);
+ }
+ return null;
}
/**
@@ -857,7 +922,7 @@ adapt: if (type != other.getClass()) {
} catch (UnsupportedOperationException e) {
/*
* java.sql.Date and java.sql.Time cannot be converted to
Instant because a part
- * of their coordinates on the timeline is undefined. For
example in the case of
+ * of their coordinates on the timeline is undefined. For
example, in the case of
* java.sql.Date the hours, minutes and seconds are
unspecified (which is not the
* same thing as assuming that those values are zero).
*/
@@ -950,7 +1015,7 @@ adapt: if (type != other.getClass()) {
*
* <ul>
* <li>{@link ChronoField#OFFSET_SECONDS}: time zone is ignored.</li>
- * <li>{@link ChronoField#SECOND_OF_DAY}: time of dat and time zone are
ignored.</li>
+ * <li>{@link ChronoField#SECOND_OF_DAY}: time of {@code DateTime} and
time zone are ignored.</li>
* </ul>
*
* @param field the field which is ignored.
diff --git
a/endorsed/src/org.apache.sis.util/main/org/apache/sis/converter/ClassPair.java
b/endorsed/src/org.apache.sis.util/main/org/apache/sis/converter/ClassPair.java
index ecaab798df..814b30a143 100644
---
a/endorsed/src/org.apache.sis.util/main/org/apache/sis/converter/ClassPair.java
+++
b/endorsed/src/org.apache.sis.util/main/org/apache/sis/converter/ClassPair.java
@@ -136,7 +136,7 @@ class ClassPair<S,T> implements Serializable {
@Override
public boolean equals(final Object other) {
if (other instanceof ClassPair<?,?>) {
- final ClassPair<?,?> that = (ClassPair<?,?>) other;
+ final var that = (ClassPair<?,?>) other;
return sourceClass == that.sourceClass &&
targetClass == that.targetClass;
}
diff --git
a/endorsed/src/org.apache.sis.util/main/org/apache/sis/converter/ConverterRegistry.java
b/endorsed/src/org.apache.sis.util/main/org/apache/sis/converter/ConverterRegistry.java
index e59c2562b9..5e9b3694f4 100644
---
a/endorsed/src/org.apache.sis.util/main/org/apache/sis/converter/ConverterRegistry.java
+++
b/endorsed/src/org.apache.sis.util/main/org/apache/sis/converter/ConverterRegistry.java
@@ -442,7 +442,7 @@ public class ConverterRegistry {
public <S,T> ObjectConverter<? super S, ? extends T> find(final Class<S>
sourceClass, final Class<T> targetClass)
throws UnconvertibleObjectException
{
- final ClassPair<S,T> key = new ClassPair<>(sourceClass, targetClass);
+ final var key = new ClassPair<S,T>(sourceClass, targetClass);
synchronized (converters) {
ObjectConverter<? super S, ? extends T> converter = get(key);
if (converter != null) {
diff --git
a/endorsed/src/org.apache.sis.util/main/org/apache/sis/converter/DateConverter.java
b/endorsed/src/org.apache.sis.util/main/org/apache/sis/converter/DateConverter.java
index 966386cf04..70eeac72a2 100644
---
a/endorsed/src/org.apache.sis.util/main/org/apache/sis/converter/DateConverter.java
+++
b/endorsed/src/org.apache.sis.util/main/org/apache/sis/converter/DateConverter.java
@@ -27,7 +27,15 @@ import org.apache.sis.math.FunctionProperty;
/**
* Handles conversions from {@link Date} to various objects.
* Note that there is no converter between {@link String} and {@link
java.util.Date}.
- * The {@link java.time.Instant} class should be used instead.
+ * For conversions from {@code String}, use the {@link java.time.Instant}
class instead.
+ *
+ * <h2>Unconvertible dates</h2>
+ * While {@link java.util.Date} is generally convertible to {@link
java.time.Instant},
+ * the {@link java.sql.Date} and {@link java.sql.Time} subclasses are not
convertible:
+ * e.g. {@link java.sql.Date#toInstant()} throws {@link
UnsupportedOperationException}.
+ * We could handle this special case in {@link SystemRegistry} for detecting
early that
+ * a conversion will never succeed, but it does not seem worth to add this
complexity
+ * for now, especially since users could override the above-cited method.
*
* <h2>Immutability and thread safety</h2>
* This base class and all inner classes are immutable, and thus inherently
thread-safe.
@@ -129,7 +137,7 @@ abstract class DateConverter<T> extends
SystemConverter<Date,T> {
}
/**
- * From {@code Date} to SQL {@code Date}.
+ * From {@code Date} to <abbr>SQL</abbr> {@code Date}.
* The inverse of this converter is the identity conversion.
*/
public static final class SQL extends DateConverter<java.sql.Date> {
@@ -149,7 +157,7 @@ abstract class DateConverter<T> extends
SystemConverter<Date,T> {
}
/**
- * From {@code Date} to SQL {@code Timestamp}.
+ * From {@code Date} to <abbr>SQL</abbr> {@code Timestamp}.
* The inverse of this converter is the identity conversion.
*/
public static final class Timestamp extends
DateConverter<java.sql.Timestamp> {
diff --git
a/endorsed/src/org.apache.sis.util/main/org/apache/sis/converter/StringConverter.java
b/endorsed/src/org.apache.sis.util/main/org/apache/sis/converter/StringConverter.java
index 1cfd70527f..9981e939a8 100644
---
a/endorsed/src/org.apache.sis.util/main/org/apache/sis/converter/StringConverter.java
+++
b/endorsed/src/org.apache.sis.util/main/org/apache/sis/converter/StringConverter.java
@@ -85,6 +85,7 @@ abstract class StringConverter<T> extends
SystemConverter<String, T> {
*
* @param targetClass the {@linkplain #getTargetClass() target class}.
*/
+ @SuppressWarnings("OverridableMethodCallInConstructor")
StringConverter(final Class<T> targetClass) {
super(String.class, targetClass);
inverse = createInverse();
diff --git
a/endorsed/src/org.apache.sis.util/main/org/apache/sis/converter/SystemRegistry.java
b/endorsed/src/org.apache.sis.util/main/org/apache/sis/converter/SystemRegistry.java
index b6f9c47eb3..9ccec8df9e 100644
---
a/endorsed/src/org.apache.sis.util/main/org/apache/sis/converter/SystemRegistry.java
+++
b/endorsed/src/org.apache.sis.util/main/org/apache/sis/converter/SystemRegistry.java
@@ -28,7 +28,7 @@ import org.apache.sis.system.Modules;
/**
- * The Apache SIS system-wide {@link ConverterRegistry}.
+ * The Apache <abbr>SIS</abbr> system-wide {@link ConverterRegistry}.
* This class serves two purposes:
*
* <ul>
diff --git
a/endorsed/src/org.apache.sis.util/main/org/apache/sis/system/Loggers.java
b/endorsed/src/org.apache.sis.util/main/org/apache/sis/system/Loggers.java
index a5b6928ddf..e330fb4ef5 100644
--- a/endorsed/src/org.apache.sis.util/main/org/apache/sis/system/Loggers.java
+++ b/endorsed/src/org.apache.sis.util/main/org/apache/sis/system/Loggers.java
@@ -42,8 +42,7 @@ public final class Loggers {
public static final String ROOT = "org.apache.sis";
/**
- * The logger for Apache SIS internal operations. The name of this logger
does not match the package name
- * of the classes using it, because this logger name does not have the
{@code "internal"} part in it.
+ * The logger for Apache SIS internal operations.
*/
public static final String SYSTEM = "org.apache.sis.system";
