You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sis.apache.org by de...@apache.org on 2021/09/15 13:15:58 UTC
[sis] 02/02: Fix a `NullPointerException` when arithmetic operation
results in a `Quantity` having no pre-defined specialized interface.
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 <ma...@geomatys.com>
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.