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
commit face96522ca3fefcdb0dd47a192ee582285c5993 Author: Martin Desruisseaux <[email protected]> AuthorDate: Wed Sep 15 15:15:02 2021 +0200 Fix a `NullPointerException` when arithmetic operation results in a `Quantity` having no pre-defined specialized interface. --- .../java/org/apache/sis/measure/DerivedScalar.java | 15 +++++---- .../java/org/apache/sis/measure/Quantities.java | 25 ++++++++++++-- .../main/java/org/apache/sis/measure/Scalar.java | 9 +++-- .../org/apache/sis/measure/ScalarFallback.java | 6 ++++ .../java/org/apache/sis/measure/SystemUnit.java | 18 ++++++---- .../java/org/apache/sis/measure/UnitServices.java | 39 +++++++++++++++------- .../org/apache/sis/measure/QuantitiesTest.java | 12 +++++++ 7 files changed, 94 insertions(+), 30 deletions(-) diff --git a/core/sis-utility/src/main/java/org/apache/sis/measure/DerivedScalar.java b/core/sis-utility/src/main/java/org/apache/sis/measure/DerivedScalar.java index f4f313c..0df8485 100644 --- a/core/sis-utility/src/main/java/org/apache/sis/measure/DerivedScalar.java +++ b/core/sis-utility/src/main/java/org/apache/sis/measure/DerivedScalar.java @@ -35,14 +35,14 @@ import org.apache.sis.util.ArgumentChecks; * It is a design similar to {@link org.opengis.referencing.crs.DerivedCRS}</p> * * @author Martin Desruisseaux (Geomatys) - * @version 1.0 + * @version 1.1 * * @param <Q> the concrete subtype. * * @since 1.0 * @module */ -abstract class DerivedScalar<Q extends Quantity<Q>> extends Scalar<Q> { +class DerivedScalar<Q extends Quantity<Q>> extends Scalar<Q> { /** * For cross-version compatibility. */ @@ -103,7 +103,10 @@ abstract class DerivedScalar<Q extends Quantity<Q>> extends Scalar<Q> { * } */ @Override - abstract Quantity<Q> create(double newValue, Unit<Q> newUnit); + Quantity<Q> create(double newValue, Unit<Q> newUnit) { + assert newUnit == getSystemUnit() : newUnit; + return new DerivedScalar<>(this, newValue); + } /** * Returns the system unit of measurement. @@ -155,10 +158,10 @@ abstract class DerivedScalar<Q extends Quantity<Q>> extends Scalar<Q> { } ArgumentChecks.ensureNonNull("unit", newUnit); // "unit" is the parameter name used in public API. /* - * Do not invoke 'this.create(double, Unit)' because the contract in this subclass + * Do not invoke `this.create(double, Unit)` because the contract in this subclass * restricts the above method to cases where the given unit is the system unit. - * Furthermore we need to let 'Quantities.create(…)' re-evaluate whether we need - * a 'DerivedScalar' instance or whether 'Scalar' would be sufficient. + * Furthermore we need to let `Quantities.create(…)` re-evaluate whether we need + * a `DerivedScalar` instance or whether `Scalar` would be sufficient. */ return Quantities.create(derivedUnit.getConverterTo(newUnit).convert(derivedValue), newUnit); } diff --git a/core/sis-utility/src/main/java/org/apache/sis/measure/Quantities.java b/core/sis-utility/src/main/java/org/apache/sis/measure/Quantities.java index 2fe287d..3c47d62 100644 --- a/core/sis-utility/src/main/java/org/apache/sis/measure/Quantities.java +++ b/core/sis-utility/src/main/java/org/apache/sis/measure/Quantities.java @@ -93,12 +93,24 @@ public final class Quantities extends Static { * even if we skip the conversion to system unit. Since the vast majority of units fall in this * category, it is worth to do this optimization. * - * (Note: despite its name, above 'isLinear()' method actually has an 'isScale()' behavior). + * (Note: despite its name, above `isLinear()` method actually has an `isScale()` behavior). */ if (factory != null) { return factory.create(value, unit); } else { - return ScalarFallback.factory(value, unit, ((SystemUnit<Q>) system).quantity); + final Class<Q> type = ((SystemUnit<Q>) system).quantity; + if (type != null) { + return ScalarFallback.factory(value, unit, type); + } else { + /* + * This cast should be safe because `type` should be null only in contexts where the user + * can not expect a more specific type. For example it may be the result of an arithmetic + * operation, in which case the return value in method signature is `Quantity<?>`. + */ + @SuppressWarnings("unchecked") + final Q quantity = (Q) new Scalar<>(value, unit); + return quantity; + } } } /* @@ -112,7 +124,14 @@ public final class Quantities extends Static { return quantity; } } - return DerivedScalar.Fallback.factory(value, unit, system, c, ((SystemUnit<Q>) system).quantity); + final Class<Q> type = ((SystemUnit<Q>) system).quantity; + if (type != null) { + return DerivedScalar.Fallback.factory(value, unit, system, c, type); + } else { + @SuppressWarnings("unchecked") // Same reason than for `new Scalar(…)`. + final Q quantity = (Q) new DerivedScalar<>(value, unit, system, c); + return quantity; + } } else { throw new IllegalArgumentException(Errors.format(Errors.Keys.UnsupportedImplementation_1, unit.getClass())); } diff --git a/core/sis-utility/src/main/java/org/apache/sis/measure/Scalar.java b/core/sis-utility/src/main/java/org/apache/sis/measure/Scalar.java index 9d67f3b..5a784c3 100644 --- a/core/sis-utility/src/main/java/org/apache/sis/measure/Scalar.java +++ b/core/sis-utility/src/main/java/org/apache/sis/measure/Scalar.java @@ -39,7 +39,7 @@ import org.apache.sis.internal.util.Numerics; * @since 0.8 * @module */ -abstract class Scalar<Q extends Quantity<Q>> extends Number implements Quantity<Q>, Comparable<Q> { +class Scalar<Q extends Quantity<Q>> extends Number implements Quantity<Q>, Comparable<Q> { /** * For cross-version compatibility. */ @@ -57,6 +57,7 @@ abstract class Scalar<Q extends Quantity<Q>> extends Number implements Quantity< /** * Creates a new scalar for the given value. + * Callers should ensure that the unit argument is non-null. */ Scalar(final double value, final Unit<Q> unit) { this.value = value; @@ -82,7 +83,9 @@ abstract class Scalar<Q extends Quantity<Q>> extends Number implements Quantity< * * @see Quantities#create(double, Unit) */ - abstract Quantity<Q> create(double newValue, Unit<Q> newUnit); + Quantity<Q> create(double newValue, Unit<Q> newUnit) { + return new Scalar<>(newValue, newUnit); + } /** * Returns a quantity quantity of same type than this quantity but with a different value and/or unit. @@ -218,7 +221,7 @@ abstract class Scalar<Q extends Quantity<Q>> extends Number implements Quantity< * Conversion from this quantity to system unit was a scale factor (see assumption documented * in this method javadoc) and given conversion is also a scale factor. Consequently conversion * from the new quantity unit to system unit will still be a scale factor, in which case this - * 'Scalar' class is still appropriate. + * `Scalar` class is still appropriate. */ return create(newValue, newUnit); } else { diff --git a/core/sis-utility/src/main/java/org/apache/sis/measure/ScalarFallback.java b/core/sis-utility/src/main/java/org/apache/sis/measure/ScalarFallback.java index abf9d3c..b2d9bb3 100644 --- a/core/sis-utility/src/main/java/org/apache/sis/measure/ScalarFallback.java +++ b/core/sis-utility/src/main/java/org/apache/sis/measure/ScalarFallback.java @@ -40,6 +40,7 @@ final class ScalarFallback<Q extends Quantity<Q>> extends Scalar<Q> implements I /** * Creates a new scalar for the given value and unit of measurement. + * Callers should ensure that all the arguments are non-null. */ private ScalarFallback(final double value, final Unit<Q> unit, final Class<Q> type) { super(value, unit); @@ -56,6 +57,11 @@ final class ScalarFallback<Q extends Quantity<Q>> extends Scalar<Q> implements I /** * Creates a new {@link ScalarFallback} instance implementing the given quantity type. + * Callers should ensure that all the arguments are non-null. + * + * @param value the numerical value. + * @param unit unit of measurement. + * @param type interface implemented by proxy instances. */ @SuppressWarnings("unchecked") static <Q extends Quantity<Q>> Q factory(final double value, final Unit<Q> unit, final Class<Q> type) { diff --git a/core/sis-utility/src/main/java/org/apache/sis/measure/SystemUnit.java b/core/sis-utility/src/main/java/org/apache/sis/measure/SystemUnit.java index 4bc56e8..cc34fa5 100644 --- a/core/sis-utility/src/main/java/org/apache/sis/measure/SystemUnit.java +++ b/core/sis-utility/src/main/java/org/apache/sis/measure/SystemUnit.java @@ -43,7 +43,7 @@ import org.apache.sis.math.Fraction; * without scale factor or offset. * * @author Martin Desruisseaux (MPO, Geomatys) - * @version 1.0 + * @version 1.1 * * @param <Q> the kind of quantity to be measured using this units. * @@ -63,6 +63,8 @@ final class SystemUnit<Q extends Quantity<Q>> extends AbstractUnit<Q> implements /** * The type of quantity that uses this unit, or {@code null} if unknown. + * This field should be null only when this unit is the result of an arithmetic + * operation and that result can not be mapped to a known {@link Quantity} subtype. */ final Class<Q> quantity; @@ -140,7 +142,7 @@ final class SystemUnit<Q extends Quantity<Q>> extends AbstractUnit<Q> implements } /* * Try to create a unit symbol as the concatenation of the symbols of the two units, - * with the operation symbol between them. If we can not, 'symbol' will stay null. + * with the operation symbol between them. If we can not, `symbol` will stay null. */ String symbol = null; if (operation != 0) { @@ -425,7 +427,7 @@ final class SystemUnit<Q extends Quantity<Q>> extends AbstractUnit<Q> implements if (quantity != null) { /* * Use the cache only if this unit has a non-null quantity type. Do not use the cache even - * in read-only mode when 'quantity' is null because we would be unable to guarantee that + * in read-only mode when `quantity` is null because we would be unable to guarantee that * the parameterized type <Q> is correct. */ final Object existing = UnitRegistry.putIfAbsent(symbol, alt); @@ -439,7 +441,7 @@ final class SystemUnit<Q extends Quantity<Q>> extends AbstractUnit<Q> implements throw new IllegalArgumentException(Errors.format(Errors.Keys.ElementAlreadyPresent_1, symbol)); } /* - * This method may be invoked for a new quantity, after a call to 'asType(Class)'. + * This method may be invoked for a new quantity, after a call to `asType(Class)`. * Try to register the new unit for that Quantity. But if another unit is already * registered for that Quantity, this is not necessarily an error. */ @@ -502,7 +504,7 @@ final class SystemUnit<Q extends Quantity<Q>> extends AbstractUnit<Q> implements result = result.transform(c); /* * If the system unit product is an Apache SIS implementation, try to infer a unit symbol - * to be given to our customized 'transform' method. Otherwise fallback on standard API. + * to be given to our customized `transform` method. Otherwise fallback on standard API. */ result = inferSymbol(result, operation, other); } @@ -621,11 +623,15 @@ final class SystemUnit<Q extends Quantity<Q>> extends AbstractUnit<Q> implements */ @Override public Quantity<Q> create(final Number value, final Unit<Q> unit) { + ArgumentChecks.ensureNonNull("value", value); + ArgumentChecks.ensureNonNull("unit", unit); final double v = AbstractConverter.doubleValue(value); if (factory != null) { return factory.create(v, unit); - } else { + } else if (quantity != null) { return ScalarFallback.factory(v, unit, quantity); + } else { + return new Scalar<>(v, unit); } } } diff --git a/core/sis-utility/src/main/java/org/apache/sis/measure/UnitServices.java b/core/sis-utility/src/main/java/org/apache/sis/measure/UnitServices.java index 365582b..0d03c16 100644 --- a/core/sis-utility/src/main/java/org/apache/sis/measure/UnitServices.java +++ b/core/sis-utility/src/main/java/org/apache/sis/measure/UnitServices.java @@ -44,7 +44,7 @@ import org.apache.sis.util.Characters; * without direct dependency. A {@code UnitServices} instance can be obtained by call to {@link #current()}. * * @author Martin Desruisseaux (Geomatys) - * @version 0.8 + * @version 1.1 * @since 0.8 * @module */ @@ -226,25 +226,40 @@ public class UnitServices extends ServiceProvider implements SystemOfUnitsServic * * @param <Q> compile-time value of the {@code type} argument. * @param type type of the desired the quantity. - * @return the service to obtain {@link Quantity} instances, or {@code null} if none. + * @return the service to obtain {@link Quantity} instances. * * @see Quantities#create(double, Unit) */ @Override public <Q extends Quantity<Q>> QuantityFactory<Q> getQuantityFactory(final Class<Q> type) { + ArgumentChecks.ensureNonNull("type", type); QuantityFactory<Q> factory = Units.get(type); if (factory == null) { - factory = new QuantityFactory<Q>() { - @Override - public Quantity<Q> create(final Number value, final Unit<Q> unit) { - return ScalarFallback.factory(AbstractConverter.doubleValue(value), unit, type); - } + if (type != null) { + factory = new QuantityFactory<Q>() { + @Override + public Quantity<Q> create(final Number value, final Unit<Q> unit) { + return ScalarFallback.factory(AbstractConverter.doubleValue(value), unit, type); + } - @Override - public Unit<Q> getSystemUnit() { - return null; - } - }; + @Override + public Unit<Q> getSystemUnit() { + return null; + } + }; + } else { + factory = new QuantityFactory<Q>() { + @Override + public Quantity<Q> create(final Number value, final Unit<Q> unit) { + return new Scalar<>(AbstractConverter.doubleValue(value), unit); + } + + @Override + public Unit<Q> getSystemUnit() { + return null; + } + }; + } } return factory; } diff --git a/core/sis-utility/src/test/java/org/apache/sis/measure/QuantitiesTest.java b/core/sis-utility/src/test/java/org/apache/sis/measure/QuantitiesTest.java index 4a38e79..cfc077c 100644 --- a/core/sis-utility/src/test/java/org/apache/sis/measure/QuantitiesTest.java +++ b/core/sis-utility/src/test/java/org/apache/sis/measure/QuantitiesTest.java @@ -112,6 +112,18 @@ public final strictfp class QuantitiesTest extends TestCase { } /** + * Tests a multiply operation that result in a quantity for which we have no specialized sub-interface. + * + * @since 1.1 + */ + @Test + public void testUnspecialized() { + final Quantity<?> quantity = Quantities.create(3, Units.CENTIMETRE).multiply(Quantities.create(4, Units.SECOND)); + assertEquals("value", 12, quantity.getValue().doubleValue(), STRICT); + assertEquals("unit", "cm⋅s", quantity.getUnit().toString()); + } + + /** * Tests {@link Scalar#equals(Object)} and {@link Scalar#hashCode()}. * This test uses a unit without specific {@link Scalar} subclass, in order to * verify that tested methods work even though the {@link ScalarFallback} proxy.
