This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch geoapi-4.0 in repository https://gitbox.apache.org/repos/asf/sis.git
commit 0a2fde06526eea639180b60a891ca01c038c4f11 Merge: c13530cdad 65cb2c7145 Author: Martin Desruisseaux <[email protected]> AuthorDate: Sun Jun 21 19:00:54 2026 +0200 Merge branch 'fix/geotiff-flaky-test' into geoapi-4.0 https://github.com/apache/sis/pull/44 Changes compared to the original pull request: * `Number` comparison initially added into `Utilities.deepEquals(…)` moved to `Range.equals(…)`. * Rely on `Utilities.deepEquals(…)` for comparing the content of `CategoryList`. * Allow (0 … 256) range to be considered as equivalent to [1 … 255]. * `GeoTiffStoreTest` writes an uncompressed image instead of using the deflate algorithm. * Javadoc. .../main/org/apache/sis/coverage/Category.java | 87 +++++++++++--- .../main/org/apache/sis/coverage/CategoryList.java | 2 +- .../org/apache/sis/coverage/SampleDimension.java | 55 +++++++-- .../main/org/apache/sis/coverage/package-info.java | 3 +- .../org/apache/sis/coverage/CategoryListTest.java | 117 ++++++++++++++++++ .../test/org/apache/sis/coverage/CategoryTest.java | 107 +++++++++++++++++ .../apache/sis/coverage/SampleDimensionTest.java | 132 +++++++++++++++++++++ .../org/apache/sis/image/ImageProcessorTest.java | 45 +++++++ .../storage/geotiff/writer/ReformattedImage.java | 4 +- .../sis/storage/geotiff/GeoTiffStoreTest.java | 128 +++++++++++++++++--- .../org/apache/sis/storage/geotiff/ReaderTest.java | 49 ++++++-- .../storage/geotiff/tiled_without_compression.tiff | Bin 0 -> 2324 bytes .../geotiff/untiled_without_compression.tiff | Bin 0 -> 1240 bytes .../main/org/apache/sis/math/NumberType.java | 47 +++++--- .../org/apache/sis/measure/MeasurementRange.java | 26 +++- .../main/org/apache/sis/measure/Range.java | 123 +++++++++++++++++-- .../main/org/apache/sis/util/ComparisonMode.java | 21 ++-- .../main/org/apache/sis/util/Utilities.java | 15 ++- .../apache/sis/util/internal/shared/Numerics.java | 11 +- .../apache/sis/measure/MeasurementRangeTest.java | 1 + .../test/org/apache/sis/measure/RangeTest.java | 24 ++++ gradle/wrapper/gradle-wrapper.properties | 2 +- 22 files changed, 894 insertions(+), 105 deletions(-) diff --cc endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/Category.java index f88ed0593d,04706fc062..1757c17de8 --- a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/Category.java +++ b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/Category.java @@@ -33,6 -36,6 +33,9 @@@ import org.apache.sis.referencing.opera import org.apache.sis.feature.internal.Resources; import org.apache.sis.util.ArgumentChecks; import org.apache.sis.util.ArraysExt; ++import org.apache.sis.util.ComparisonMode; ++import org.apache.sis.util.LenientComparable; ++import org.apache.sis.util.Utilities; import org.apache.sis.util.iso.Types; @@@ -68,10 -71,10 +71,11 @@@ * <p>All {@code Category} objects are immutable and thread-safe.</p> * * @author Martin Desruisseaux (IRD, Geomatys) -- * @version 1.1 ++ * @author Alexis Manin (Geomatys) ++ * @version 1.7 * @since 1.0 */ - public class Category implements Serializable { + public class Category implements LenientComparable, Serializable { /** * Serial number for inter-operability with different versions. */ @@@ -493,12 -496,12 +497,13 @@@ } /** -- * Returns a hash value for this category. This value needs not remain consistent between -- * different implementations of the same class. ++ * Returns a hash value for this category. ++ * ++ * @return a hash value for this category. */ @Override public int hashCode() { -- return name.hashCode(); ++ return name.hashCode() + range.hashCode(); } /** @@@ -507,31 -510,43 +512,75 @@@ * @param object the object to compare with. * @return {@code true} if the given object is equal to this category. */ + @Override - public boolean equals(final Object object) { + public final boolean equals(final Object object) { + return equals(object, ComparisonMode.STRICT); + } + + /** + * Compares this category with the given object for equality at the given level of strictness. + * + * <ul> + * <li>{@link ComparisonMode#STRICT}: same implementation class, same name, exact range and transfer function.</li> + * <li>{@link ComparisonMode#BY_CONTRACT}: same name, exact range and transfer function; class may differ.</li> + * <li>{@link ComparisonMode#IGNORE_METADATA} and more lenient: range and transfer function only (name is ignored).</li> + * <li>{@link ComparisonMode#APPROXIMATE}: range and transfer function approximately equal.</li> + * </ul> + * + * @param object the object to compare with. + * @param mode the comparison strictness level. + * @return {@code true} if both objects are equal according the given comparison mode. ++ * ++ * @since 1.7 + */ + @Override + public boolean equals(final Object object, final ComparisonMode mode) { - if (object == this) return true; - if (!(object instanceof Category)) return false; - final Category that = (Category) object; - - switch (mode) { - case STRICT: { - if (!getClass().equals(that.getClass())) return false; - } - case BY_CONTRACT: { - if (!name.equals(that.name)) return false; + if (object == this) { + // Slight optimization + return true; + } - if (object != null && getClass().equals(object.getClass())) { ++ if (object instanceof Category) { + final Category that = (Category) object; - if (name.equals(that.name)) { - final NumberRange<?> other = that.range; - /* - * The NumberRange.equals(Object) comparison is not sufficient because it considers all NaN values as equal. - * For the purpose of Category, we need to distinguish the different NaN values. - */ - if (range == other || (range.equals(other) - && doubleToRawLongBits(range.getMinDouble()) == doubleToRawLongBits(other.getMinDouble()) - && doubleToRawLongBits(range.getMaxDouble()) == doubleToRawLongBits(other.getMaxDouble()))) - { - return toConverse.equals(that.toConverse); - } ++ if (mode == ComparisonMode.STRICT) { ++ return object.getClass() == getClass() ++ && name.equals(that.name) ++ && equals(range, that.range, mode) ++ && toConverse.equals(that.toConverse); + } - default: - return Utilities.deepEquals(range, that.range, mode) ++ if (mode.isIgnoringMetadata() || getName().equals(that.getName())) { ++ return equals(getSampleRange(), that.getSampleRange(), mode) + && Utilities.deepEquals(toConverse, that.toConverse, mode); + } } + return false; + } + ++ /** ++ * Compares the given ranges with a distinction between the different NaN values. ++ * The {@link NumberRange#equals(Object, ComparisonMode)} method is not sufficient ++ * because it considers all NaN values as equal. For the purpose of {@link Category}, ++ * we need to distinguish the different NaN values. ++ */ ++ private static boolean equals(final NumberRange<?> range, final NumberRange<?> other, final ComparisonMode mode) { ++ if (range == other) { ++ return true; ++ } ++ return range.equals(other, mode) ++ && sameNaN(range.getMinDouble(), other.getMinDouble()) ++ && sameNaN(range.getMaxDouble(), other.getMaxDouble()); ++ } ++ ++ /** ++ * If the given values are NaN, returns whether they have the same bit patterns. ++ * If the given values are non NaN, returns {@code true} if none of them are NaN. ++ * This method does not verify whether non-NaN values are equal because it depends ++ * on {@link ComparisonMode}. ++ */ ++ private static boolean sameNaN(final double value, final double other) { ++ return doubleToRawLongBits(value) == doubleToRawLongBits(other) ++ || !(Double.isNaN(value) || Double.isNaN(other)); + } + /** * Returns a string representation of this category for debugging purpose. * This string representation may change in any future SIS version. diff --cc endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/CategoryList.java index bba233abf1,75cda8ad26..2156b94847 --- a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/CategoryList.java +++ b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/CategoryList.java @@@ -408,7 -410,7 +408,7 @@@ final class CategoryList extends Abstra */ final MathTransform1D getTransferFunction() { MathTransform1D tr = categories[0].toConverse; // See condition in javadoc. -- for (int i=categories.length; --i >= 1;) { ++ for (int i = categories.length; --i >= 1;) { if (!tr.equals(categories[i].toConverse)) { tr = this; break; diff --cc endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/SampleDimension.java index 4826bb42de,90c462ca0d..88a1575010 --- a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/SampleDimension.java +++ b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/SampleDimension.java @@@ -38,8 -41,8 +38,11 @@@ import org.apache.sis.measure.NumberRan import org.apache.sis.math.MathFunctions; import org.apache.sis.math.NumberType; import org.apache.sis.util.ArgumentChecks; ++import org.apache.sis.util.ComparisonMode; ++import org.apache.sis.util.LenientComparable; import org.apache.sis.util.Numbers; import org.apache.sis.util.Debug; ++import org.apache.sis.util.Utilities; import org.apache.sis.util.collection.Containers; import org.apache.sis.util.resources.Vocabulary; import org.apache.sis.util.resources.Errors; @@@ -278,8 -281,8 +281,8 @@@ public class SampleDimension implement public Set<Number> getNoDataValues() { if (converse != null) { // Null if SampleDimension does not contain at least one quantitative category. final boolean isConverted = transferFunction.isIdentity(); -- final NumberRange<?>[] ranges = new NumberRange<?>[categories.size()]; -- final Class<?>[] rangeTypes = new Class<?>[ranges.length]; ++ final var ranges = new NumberRange<?>[categories.size()]; ++ final var rangeTypes = new Class<?>[ranges.length]; int count = 0; for (final Category category : categories) { final Category converted = category.converted(); @@@ -294,7 -297,7 +297,7 @@@ } if (count != 0) { final NumberType widestType = NumberType.forClasses(rangeTypes).filter(NumberType::isConvertible).orElse(null); -- final Set<Number> noDataValues = new TreeSet<>(SampleDimension::compare); ++ final var noDataValues = new TreeSet<Number>(SampleDimension::compare); for (int i=0; i<count; i++) { final NumberRange<?> range = ranges[i]; final Number minimum = range.getMinValue(); @@@ -304,8 -307,8 +307,9 @@@ if (range.isMaxIncluded()) noDataValues.add(widestType.cast(maximum)); } if (NumberType.isInteger(range.getElementType())) { -- long value = minimum.longValue() + 1; // If value was inclusive, then it has already been added to the set. -- long stop = maximum.longValue() - 1; ++ // If value was inclusive, then it has already been added to the set. ++ long value = Math.incrementExact(minimum.longValue()); ++ long stop = Math.decrementExact(maximum.longValue()); while (value <= stop) { noDataValues.add(widestType.wrapExact(value)); } @@@ -474,6 -477,6 +478,8 @@@ /** * Returns a hash value for this sample dimension. ++ * ++ * @return a hash value for this sample dimension. */ @Override public int hashCode() { @@@ -487,15 -490,29 +493,46 @@@ * @return {@code true} if the given object is equal to this sample dimension. */ @Override - public boolean equals(final Object object) { + public final boolean equals(final Object object) { + return equals(object, ComparisonMode.STRICT); + } + ++ /** ++ * Compares this sample dimension with the given object for equality at the given level of strictness. ++ * The {@linkplain #getName() name} is considered as metadata. ++ * ++ * @param object the object to compare with. ++ * @param mode the comparison strictness level. ++ * @return {@code true} if both objects are equal according the given strictness level. ++ * ++ * @see Category#equals(Object, ComparisonMode) ++ * @since 1.7 ++ */ + @Override - public boolean equals(Object other, ComparisonMode mode) { - if (other == this) return true; - if (!(other instanceof SampleDimension)) return false; - - switch (mode) { - case STRICT: { - if (other.getClass() == getClass()) { - final SampleDimension that = (SampleDimension) other; - return name.equals(that.name) && Objects.equals(background, that.background) && categories.equals(that.categories); - } ++ public boolean equals(final Object object, final ComparisonMode mode) { + if (object == this) { + return true; + } + if (object instanceof SampleDimension) { + final SampleDimension that = (SampleDimension) object; - return name.equals(that.name) && Objects.equals(background, that.background) && categories.equals(that.categories); ++ /* ++ * `transferFunction` does not need to be compared because it is derived from the categories. ++ * It is also difficult to keep `transferFunction` comparison consistent with `categories` ++ * comparison if mode is `APPROXIMATE`, because tolerance threshold may not be the same or ++ * may not be applied on the same coefficients. ++ */ ++ if (mode == ComparisonMode.STRICT) { ++ return object.getClass() == getClass() ++ && name.equals(that.name) ++ && categories.equals(that.categories) ++ && Objects.equals(background, that.background); + } - default: { - final var otherDim = (SampleDimension) other; - return Utilities.deepEquals(this.transferFunction, otherDim.transferFunction, mode) - && Utilities.deepEquals(this.categories, otherDim.categories, mode) - && Utilities.deepEquals(this.background, otherDim.background, mode); ++ if (mode.isIgnoringMetadata() || getName().equals(that.getName())) { ++ return Utilities.deepEquals(getCategories(), that.getCategories(), mode) && ++ Utilities.deepEquals(getBackground(), that.getBackground(), mode); + } } + return false; } /** diff --cc endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/package-info.java index cf2ce3869a,cf2ce3869a..fee606b1b8 --- a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/package-info.java +++ b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/package-info.java @@@ -23,7 -23,7 +23,8 @@@ * {@link org.apache.sis.coverage.grid}. * * @author Martin Desruisseaux (Geomatys) -- * @version 1.5 ++ * @author Alexis Manin (Geomatys) ++ * @version 1.7 * @since 1.0 */ package org.apache.sis.coverage; diff --cc endorsed/src/org.apache.sis.feature/test/org/apache/sis/coverage/CategoryListTest.java index ea32bb2589,4e2ecfeb64..70c9b865bb --- a/endorsed/src/org.apache.sis.feature/test/org/apache/sis/coverage/CategoryListTest.java +++ b/endorsed/src/org.apache.sis.feature/test/org/apache/sis/coverage/CategoryListTest.java @@@ -24,6 -24,7 +24,8 @@@ import org.opengis.referencing.operatio import org.apache.sis.referencing.operation.transform.MathTransforms; import org.apache.sis.math.MathFunctions; import org.apache.sis.measure.NumberRange; + import org.apache.sis.util.ComparisonMode; ++import org.apache.sis.util.Utilities; // Test dependencies import org.junit.jupiter.api.Test; @@@ -31,13 -32,13 +33,16 @@@ import static org.junit.jupiter.api.Ass import static org.apache.sis.test.Assertions.assertMessageContains; import org.apache.sis.test.TestUtilities; import org.apache.sis.test.TestCase; ++import org.apache.sis.util.internal.shared.Numerics; /** * Tests {@link CategoryList}. * * @author Martin Desruisseaux (IRD, Geomatys) ++ * @author Alexis Manin (Geomatys) */ ++@SuppressWarnings("exports") public final class CategoryListTest extends TestCase { /** * Creates a new test case. @@@ -414,4 -415,116 +419,116 @@@ assertTrue(category.toConverse.isIdentity()); } } + + /** + * Tests {@link CategoryList#equals(Object, ComparisonMode)} for all comparison modes. + * Covers null/incompatible-type rejection, empty list, element-wise comparison, + * size mismatch, NaN-ordinal differences, name-only differences, and approximate + * transfer-function tolerance. + */ + @Test + public void testLenientEquality() { + final CategoryList list1 = CategoryList.create(categories(), null); + + // ------------------------------------------------------------------ + // Null and incompatible type. + // ------------------------------------------------------------------ - assertFalse(list1.equals(null, ComparisonMode.STRICT), "null/STRICT"); - assertFalse(list1.equals("not a list", ComparisonMode.APPROXIMATE), "String/APPROXIMATE"); ++ assertFalse(Utilities.deepEquals(list1, null, ComparisonMode.STRICT), "null/STRICT"); ++ assertFalse(Utilities.deepEquals(list1, "not a list", ComparisonMode.APPROXIMATE), "String/APPROXIMATE"); + + // ------------------------------------------------------------------ + // Empty list: same reference → true. + // ------------------------------------------------------------------ - assertTrue(CategoryList.EMPTY.equals(CategoryList.EMPTY, ComparisonMode.STRICT), "empty/STRICT"); ++ assertTrue(Utilities.deepEquals(CategoryList.EMPTY, CategoryList.EMPTY, ComparisonMode.STRICT), "empty/STRICT"); + + // ------------------------------------------------------------------ + // Two lists built from independent category arrays with the same + // configuration. All modes must return true. + // ------------------------------------------------------------------ + final CategoryList list2 = CategoryList.create(categories(), null); - assertTrue(list1.equals(list2, ComparisonMode.STRICT), "same-cfg/STRICT"); - assertTrue(list1.equals(list2, ComparisonMode.BY_CONTRACT), "same-cfg/BY_CONTRACT"); - assertTrue(list1.equals(list2, ComparisonMode.IGNORE_METADATA), "same-cfg/IGNORE_METADATA"); - assertTrue(list1.equals(list2, ComparisonMode.APPROXIMATE), "same-cfg/APPROXIMATE"); ++ assertTrue(Utilities.deepEquals(list1, list2, ComparisonMode.STRICT), "same-cfg/STRICT"); ++ assertTrue(Utilities.deepEquals(list1, list2, ComparisonMode.BY_CONTRACT), "same-cfg/BY_CONTRACT"); ++ assertTrue(Utilities.deepEquals(list1, list2, ComparisonMode.IGNORE_METADATA), "same-cfg/IGNORE_METADATA"); ++ assertTrue(Utilities.deepEquals(list1, list2, ComparisonMode.APPROXIMATE), "same-cfg/APPROXIMATE"); + + // ------------------------------------------------------------------ + // Different size: list of 5 categories vs 3 categories. + // ------------------------------------------------------------------ + final ToNaN toNaN = new ToNaN(); + final Category[] fewer = { + new Category("No data", NumberRange.create( 0, true, 0, true), null, null, toNaN), + new Category("Land", NumberRange.create( 7, true, 7, true), null, null, toNaN), + new Category("Temperature", NumberRange.create( 10, true, 100, false), + (MathTransform1D) MathTransforms.linear(0.1, 5), null, toNaN) + }; + final CategoryList shortList = CategoryList.create(fewer, null); - assertFalse(list1.equals(shortList, ComparisonMode.STRICT), "diff-size/STRICT"); - assertFalse(list1.equals(shortList, ComparisonMode.APPROXIMATE), "diff-size/APPROXIMATE"); ++ assertFalse(Utilities.deepEquals(list1, shortList, ComparisonMode.STRICT), "diff-size/STRICT"); ++ assertFalse(Utilities.deepEquals(list1, shortList, ComparisonMode.APPROXIMATE), "diff-size/APPROXIMATE"); + + // ------------------------------------------------------------------ + // Different NaN ordinal in a qualitative category. + // Replace sample value 0 with a category that forces ordinal 99. + // ------------------------------------------------------------------ + final Category[] diffNaN = { + new Category("No data", NumberRange.create( 0, true, 0, true), null, null, (v) -> 99), + new Category("Land", NumberRange.create( 7, true, 7, true), null, null, toNaN), + new Category("Clouds", NumberRange.create( 3, true, 3, true), null, null, toNaN), + new Category("Temperature", NumberRange.create( 10, true, 100, false), + (MathTransform1D) MathTransforms.linear(0.1, 5), null, toNaN), + new Category("Foo", NumberRange.create(100, true, 120, false), + (MathTransform1D) MathTransforms.linear(-1, 3), null, toNaN) + }; + final CategoryList listDiffNaN = CategoryList.create(diffNaN, null); - assertFalse(list1.equals(listDiffNaN, ComparisonMode.STRICT), "diff-NaN/STRICT"); - assertTrue(list1.equals(listDiffNaN, ComparisonMode.APPROXIMATE), "diff-NaN/APPROXIMATE"); ++ assertFalse(Utilities.deepEquals(list1, listDiffNaN, ComparisonMode.STRICT), "diff-NaN/STRICT"); ++ assertTrue (Utilities.deepEquals(list1, listDiffNaN, ComparisonMode.APPROXIMATE), "diff-NaN/APPROXIMATE"); + + // ------------------------------------------------------------------ + // Different name only in the qualitative "No data" category. + // ------------------------------------------------------------------ + final Category[] diffName = { + new Category("Renamed", NumberRange.create( 0, true, 0, true), null, null, toNaN), + new Category("Land", NumberRange.create( 7, true, 7, true), null, null, toNaN), + new Category("Clouds", NumberRange.create( 3, true, 3, true), null, null, toNaN), + new Category("Temperature", NumberRange.create( 10, true, 100, false), + (MathTransform1D) MathTransforms.linear(0.1, 5), null, toNaN), + new Category("Foo", NumberRange.create(100, true, 120, false), + (MathTransform1D) MathTransforms.linear(-1, 3), null, toNaN) + }; + final CategoryList listDiffName = CategoryList.create(diffName, null); - assertFalse(list1.equals(listDiffName, ComparisonMode.STRICT), "diff-name/STRICT"); - assertTrue (list1.equals(listDiffName, ComparisonMode.IGNORE_METADATA), "diff-name/IGNORE_METADATA"); ++ assertFalse(Utilities.deepEquals(list1, listDiffName, ComparisonMode.STRICT), "diff-name/STRICT"); ++ assertTrue (Utilities.deepEquals(list1, listDiffName, ComparisonMode.IGNORE_METADATA), "diff-name/IGNORE_METADATA"); + + // ------------------------------------------------------------------ + // Significantly different transfer function (scale 0.1 → 0.2). + // ------------------------------------------------------------------ + final Category[] bigDiff = { + new Category("No data", NumberRange.create( 0, true, 0, true), null, null, toNaN), + new Category("Land", NumberRange.create( 7, true, 7, true), null, null, toNaN), + new Category("Clouds", NumberRange.create( 3, true, 3, true), null, null, toNaN), + new Category("Temperature", NumberRange.create( 10, true, 100, false), + (MathTransform1D) MathTransforms.linear(0.2, 5), null, toNaN), + new Category("Foo", NumberRange.create(100, true, 120, false), + (MathTransform1D) MathTransforms.linear(-1, 3), null, toNaN) + }; + final CategoryList listBigDiff = CategoryList.create(bigDiff, null); - assertFalse(list1.equals(listBigDiff, ComparisonMode.STRICT), "big-diff/STRICT"); - assertFalse(list1.equals(listBigDiff, ComparisonMode.APPROXIMATE), "big-diff/APPROXIMATE"); ++ assertFalse(Utilities.deepEquals(list1, listBigDiff, ComparisonMode.STRICT), "big-diff/STRICT"); ++ assertFalse(Utilities.deepEquals(list1, listBigDiff, ComparisonMode.APPROXIMATE), "big-diff/APPROXIMATE"); + + // ------------------------------------------------------------------ + // Tiny transfer function difference: offset 5.0 vs 5.0 - 1e-13. + // Relative threshold for offset ≈ 5 is 5E-13 > 1E-13, so within tolerance. + // ------------------------------------------------------------------ + final Category[] tinyDiff = { + new Category("No data", NumberRange.create( 0, true, 0, true), null, null, toNaN), + new Category("Land", NumberRange.create( 7, true, 7, true), null, null, toNaN), + new Category("Clouds", NumberRange.create( 3, true, 3, true), null, null, toNaN), + new Category("Temperature", NumberRange.create( 10, true, 100, false), - (MathTransform1D) MathTransforms.linear(0.1, 5.0 - 1e-13), null, toNaN), ++ (MathTransform1D) MathTransforms.linear(0.1, 5.0 - Numerics.COMPARISON_THRESHOLD), null, toNaN), + new Category("Foo", NumberRange.create(100, true, 120, false), + (MathTransform1D) MathTransforms.linear(-1, 3), null, toNaN) + }; + final CategoryList listTinyDiff = CategoryList.create(tinyDiff, null); - assertFalse(list1.equals(listTinyDiff, ComparisonMode.STRICT), "tiny-diff/STRICT"); - assertTrue (list1.equals(listTinyDiff, ComparisonMode.APPROXIMATE), "tiny-diff/APPROXIMATE"); ++ assertFalse(Utilities.deepEquals(list1, listTinyDiff, ComparisonMode.STRICT), "tiny-diff/STRICT"); ++ assertTrue (Utilities.deepEquals(list1, listTinyDiff, ComparisonMode.APPROXIMATE), "tiny-diff/APPROXIMATE"); + } } diff --cc endorsed/src/org.apache.sis.feature/test/org/apache/sis/coverage/CategoryTest.java index 51f7bf25e8,4313bc79f3..6556d6e9e7 --- a/endorsed/src/org.apache.sis.feature/test/org/apache/sis/coverage/CategoryTest.java +++ b/endorsed/src/org.apache.sis.feature/test/org/apache/sis/coverage/CategoryTest.java @@@ -34,7 -37,7 +37,9 @@@ import org.apache.sis.test.TestCase * Tests {@link Category}. * * @author Martin Desruisseaux (IRD, Geomatys) ++ * @author Alexis Manin (Geomatys) */ ++@SuppressWarnings("exports") public final class CategoryTest extends TestCase { /** * Small tolerance value for comparisons. @@@ -240,4 -243,106 +245,106 @@@ assertTrue (category.toConverse.isIdentity()); assertFalse (category.isQuantitative()); } + + /** + * Tests {@link Category#equals(Object, ComparisonMode)} for all comparison modes. + * Covers qualitative and quantitative categories, name differences, NaN ordinal + * differences, approximate transform comparison, and category-vs-converse inequality. + */ + @Test + public void testLenientEquality() { + final Category qualA = new Category("Water", NumberRange.create(1, true, 1, true), null, null, new ToNaN()); + final Category qualB = new Category("Water", NumberRange.create(1, true, 1, true), null, null, new ToNaN()); + + // Null / incompatible type — all modes must return false. - assertFalse(qualA.equals(null, ComparisonMode.STRICT), "null/STRICT"); ++ assertFalse(qualA.equals(null, ComparisonMode.STRICT), "null/STRICT"); + assertFalse(qualA.equals("not a category", ComparisonMode.APPROXIMATE), "String/APPROXIMATE"); + + // Self-equality — all modes must return true. - assertTrue(qualA.equals(qualA, ComparisonMode.STRICT), "self/STRICT"); - assertTrue(qualA.equals(qualA, ComparisonMode.BY_CONTRACT), "self/BY_CONTRACT"); ++ assertTrue(qualA.equals(qualA, ComparisonMode.STRICT), "self/STRICT"); ++ assertTrue(qualA.equals(qualA, ComparisonMode.BY_CONTRACT), "self/BY_CONTRACT"); + assertTrue(qualA.equals(qualA, ComparisonMode.IGNORE_METADATA), "self/IGNORE_METADATA"); - assertTrue(qualA.equals(qualA, ComparisonMode.APPROXIMATE), "self/APPROXIMATE"); ++ assertTrue(qualA.equals(qualA, ComparisonMode.APPROXIMATE), "self/APPROXIMATE"); + + // Qualitative: same configuration (fresh ToNaN, same sample → same NaN ordinal). + assertTrue(qualA.equals(qualB, ComparisonMode.STRICT), "qual-same/STRICT"); + assertTrue(qualA.equals(qualB, ComparisonMode.BY_CONTRACT), "qual-same/BY_CONTRACT"); + assertTrue(qualA.equals(qualB, ComparisonMode.IGNORE_METADATA), "qual-same/IGNORE_METADATA"); + assertTrue(qualA.equals(qualB, ComparisonMode.APPROXIMATE), "qual-same/APPROXIMATE"); + + // Qualitative: same sample range but different NaN ordinal. + // Force ordinal 99 for qualC; standard ToNaN for sample value 1 yields ordinal 1. + final Category qualC = new Category("Water", NumberRange.create(1, true, 1, true), null, null, (v) -> 99); + assertFalse(qualA.equals(qualC, ComparisonMode.STRICT), "qual-diffNaN/STRICT"); - assertTrue(qualA.equals(qualC, ComparisonMode.APPROXIMATE), "qual-diffNaN/APPROXIMATE"); ++ assertTrue (qualA.equals(qualC, ComparisonMode.APPROXIMATE), "qual-diffNaN/APPROXIMATE"); + + // Qualitative: same NaN ordinal but different name. + final Category qualD = new Category("Land", NumberRange.create(1, true, 1, true), null, null, new ToNaN()); + assertFalse(qualA.equals(qualD, ComparisonMode.STRICT), "qual-diffName/STRICT"); + assertFalse(qualA.equals(qualD, ComparisonMode.BY_CONTRACT), "qual-diffName/BY_CONTRACT"); + assertTrue (qualA.equals(qualD, ComparisonMode.IGNORE_METADATA), "qual-diffName/IGNORE_METADATA"); + assertTrue (qualA.equals(qualD, ComparisonMode.APPROXIMATE), "qual-diffName/APPROXIMATE"); + + // Category vs its converse (ConvertedCategory): must be false in all modes. + final Category qualConverse = qualA.converse; + assertNotSame(qualA, qualConverse); + assertFalse(qualA.equals(qualConverse, ComparisonMode.STRICT), "vs-converse/STRICT"); + assertFalse(qualA.equals(qualConverse, ComparisonMode.BY_CONTRACT), "vs-converse/BY_CONTRACT"); + assertFalse(qualA.equals(qualConverse, ComparisonMode.IGNORE_METADATA), "vs-converse/IGNORE_METADATA"); + assertFalse(qualA.equals(qualConverse, ComparisonMode.APPROXIMATE), "vs-converse/APPROXIMATE"); + + // ------------------------------------------------------------------ + // Quantitative categories with linear transfer function. + // ------------------------------------------------------------------ + final MathTransform1D tf1 = (MathTransform1D) MathTransforms.linear(0.1, 5.0); + final Category quantA = new Category("Temperature", NumberRange.create(10, true, 100, false), tf1, null, null); + final Category quantB = new Category("Temperature", NumberRange.create(10, true, 100, false), tf1, null, null); + + // Same configuration — all modes must return true. + assertTrue(quantA.equals(quantB, ComparisonMode.STRICT), "quant-same/STRICT"); + assertTrue(quantA.equals(quantB, ComparisonMode.BY_CONTRACT), "quant-same/BY_CONTRACT"); + assertTrue(quantA.equals(quantB, ComparisonMode.IGNORE_METADATA), "quant-same/IGNORE_METADATA"); + assertTrue(quantA.equals(quantB, ComparisonMode.APPROXIMATE), "quant-same/APPROXIMATE"); + + // Different name only. + final Category quantC = new Category("Renamed", NumberRange.create(10, true, 100, false), tf1, null, null); + assertFalse(quantA.equals(quantC, ComparisonMode.STRICT), "quant-diffName/STRICT"); + assertFalse(quantA.equals(quantC, ComparisonMode.BY_CONTRACT), "quant-diffName/BY_CONTRACT"); + assertTrue (quantA.equals(quantC, ComparisonMode.IGNORE_METADATA), "quant-diffName/IGNORE_METADATA"); + assertTrue (quantA.equals(quantC, ComparisonMode.APPROXIMATE), "quant-diffName/APPROXIMATE"); + + // Significantly different transfer function: scale 0.1 vs 0.2. + final MathTransform1D tf2 = (MathTransform1D) MathTransforms.linear(0.2, 5.0); + final Category quantD = new Category("Temperature", NumberRange.create(10, true, 100, false), tf2, null, null); + assertFalse(quantA.equals(quantD, ComparisonMode.STRICT), "quant-bigDiff/STRICT"); + assertFalse(quantA.equals(quantD, ComparisonMode.APPROXIMATE), "quant-bigDiff/APPROXIMATE"); + + // Tiny transfer function difference: offset 5.0 vs 5.0 - 1e-13. + // Relative threshold for offset ≈ 5.0 is 5.0 * 1E-13 = 5E-13 > 1E-13, so within tolerance. + final MathTransform1D tf3 = (MathTransform1D) MathTransforms.linear(0.1, 5.0 - 1e-13); + final Category quantE = new Category("Temperature", NumberRange.create(10, true, 100, false), tf3, null, null); + assertFalse(quantA.equals(quantE, ComparisonMode.STRICT), "quant-tinyDiff/STRICT"); + assertTrue (quantA.equals(quantE, ComparisonMode.APPROXIMATE), "quant-tinyDiff/APPROXIMATE"); + + // ------------------------------------------------------------------ + // Category with MeasurementRange (identity transform, unit-bearing range). + // ------------------------------------------------------------------ + final MathTransform1D identity = (MathTransform1D) MathTransforms.identity(1); + final Category measA = new Category("Celsius", + MeasurementRange.create(10f, true, 30f, true, Units.CELSIUS), + identity, Units.CELSIUS, null); + final Category measB = new Category("Celsius", + MeasurementRange.create(10f, true, 30f, true, Units.CELSIUS), + identity, Units.CELSIUS, null); + + // Same unit — equal in STRICT. + assertTrue(measA.equals(measB, ComparisonMode.STRICT), "meas-sameUnit/STRICT"); + + // Different units: CELSIUS vs KELVIN — ranges differ, must be false. + final Category measK = new Category("Kelvin", + MeasurementRange.create(10f, true, 30f, true, Units.KELVIN), + identity, Units.KELVIN, null); + assertFalse(measA.equals(measK, ComparisonMode.STRICT), "meas-diffUnit/STRICT"); + assertFalse(measA.equals(measK, ComparisonMode.APPROXIMATE), "meas-diffUnit/APPROXIMATE"); + } } diff --cc endorsed/src/org.apache.sis.feature/test/org/apache/sis/coverage/SampleDimensionTest.java index a2d38f0628,5587a1cae5..1ee960f556 --- a/endorsed/src/org.apache.sis.feature/test/org/apache/sis/coverage/SampleDimensionTest.java +++ b/endorsed/src/org.apache.sis.feature/test/org/apache/sis/coverage/SampleDimensionTest.java @@@ -25,6 -25,7 +25,9 @@@ import org.apache.sis.referencing.opera import org.apache.sis.math.MathFunctions; import org.apache.sis.measure.NumberRange; import org.apache.sis.measure.Units; + import org.apache.sis.util.ComparisonMode; ++import org.apache.sis.util.LenientComparable; // For javadoc ++import org.apache.sis.util.internal.shared.Numerics; // Test dependencies import org.junit.jupiter.api.Test; @@@ -36,7 -37,7 +39,9 @@@ import org.apache.sis.test.TestCase * Tests {@link SampleDimension}. * * @author Martin Desruisseaux (IRD, Geomatys) ++ * @author Alexis Manin (Geomatys) */ ++@SuppressWarnings("exports") public final class SampleDimensionTest extends TestCase { /** * Creates a new test case. @@@ -223,4 -224,125 +228,131 @@@ assertEquals("Clouds", categories.remove(1).getName().toString()); assertEquals(2, categories.size()); } + ++ /** ++ * Tests {@link SampleDimension#equals(Object, ComparisonMode)} with null or incompatible instances. ++ */ + @Test ++ @SuppressWarnings({"ObjectEqualsNull", "IncompatibleEquals"}) + public void testNullAndIncompatibleTypesAreNeverEqual() { + final var base = new SampleDimension.Builder().setName("base").build(); + assertFalse(base.equals(null, ComparisonMode.STRICT)); + assertFalse(base.equals("other", ComparisonMode.STRICT)); + assertFalse(base.equals(null)); + assertFalse(base.equals("other")); + assertFalse(base.equals(null, ComparisonMode.APPROXIMATE)); + assertFalse(base.equals("other", ComparisonMode.APPROXIMATE)); + } + + /** - * Verify strict equality behavior, and ensure standard {@link Object#equals(Object)} is consistent with strict equality. ++ * Tests {@link SampleDimension#equals(Object, ComparisonMode)} with strict equality behavior. ++ * Also ensures that the standard {@link Object#equals(Object)} is consistent with strict equality. + * - * @see org.apache.sis.util.LenientComparable#equals(Object, ComparisonMode) ++ * @see LenientComparable#equals(Object, ComparisonMode) + * @see ComparisonMode#STRICT + */ + @Test + public void testStrictEquality() { + final var base = new SampleDimension.Builder() + .addQualitative("Clouds", 1) + .addQualitative("Lands", 2) + .setName("base") + .build(); + final var strictlyEqToBase = new SampleDimension.Builder() + .addQualitative("Clouds", 1) + .addQualitative("Lands", 2) + .setName("base") + .build(); + + // Same categories and background as dim1, but different dimension name. + final var renamed = new SampleDimension.Builder() + .addQualitative("Clouds", 1) + .addQualitative("Lands", 2) + .setName("Different name") + .build(); + + // Ensure a sample dimension is strictly equal to itself + assertTrue (base.equals(base, ComparisonMode.STRICT), "A sample dimension must be strictly equal to itself"); + assertTrue (base.equals(base), "A sample dimension must be equal to itself"); + + // Ensure strict comparison work as expected + assertTrue (base.equals(strictlyEqToBase, ComparisonMode.STRICT), "identical dimensions must be equal"); + assertTrue (strictlyEqToBase.equals(base, ComparisonMode.STRICT), "equality must be symmetric"); + assertEquals(base.hashCode(), strictlyEqToBase.hashCode(), "hashCode must be consistent with equals"); + + // Dimensions that differ only in name must NOT be equal under STRICT. + assertFalse(base.equals(renamed, ComparisonMode.STRICT), "different names must produce inequality"); + assertFalse(renamed.equals(base, ComparisonMode.STRICT), "inequality must be symmetric"); + } + + /** - * Verifies the {@link org.apache.sis.util.LenientComparable#equals(Object, ComparisonMode)} contract - * for {@link ComparisonMode#APPROXIMATE}. ++ * Tests {@link SampleDimension#equals(Object, ComparisonMode)} with approximate comparison mode. ++ * The intend is to verify the compliance of {@link ComparisonMode#APPROXIMATE} with the contract ++ * documented in {@link LenientComparable#equals(Object, ComparisonMode)}. + * In approximate mode the dimension name is not compared, so two dimensions that differ + * only by name must be considered equal, while a different transfer function must not. + */ + @Test - public void testEqualsApproximate() { ++ public void testApproximateEquality() { + final var base = new SampleDimension.Builder() + .setBackground(null, 0) + .addQualitative("Clouds", 1) + .addQuantitative("Temperature", 10, 200, 0.1, 5.0, Units.CELSIUS) + .setName("Base") + .build(); + final var strictlyEqualToBase = new SampleDimension.Builder() + .setBackground(null, 0) + .addQualitative("Clouds", 1) + .addQuantitative("Temperature", 10, 200, 0.1, 5.0, Units.CELSIUS) + .setName("Base") + .build(); + // Same structure as base but with an explicit different dimension name. + final SampleDimension renamed = new SampleDimension.Builder() + .setBackground(null, 0) + .addQualitative("Clouds", 1) + .addQuantitative("Temperature", 10, 200, 0.1, 5.0, Units.CELSIUS) + .setName("Renamed") + .build(); + + // Different scale in the transfer function — must not be equal even approximately. + final SampleDimension differentScale = new SampleDimension.Builder() + .setBackground(null, 0) + .addQualitative("Clouds", 1) + .addQuantitative("Temperature", 10, 200, 0.2, 5.0, Units.CELSIUS) + .build(); + + // Tiny difference in offset (differs by less than the APPROXIMATE threshold): should be equal. + final SampleDimension tinyOffsetDiff = new SampleDimension.Builder() + .setBackground(null, 0) + .addQualitative("Clouds", 1) - .addQuantitative("Temperature", 10, 200, 0.1, 5.0 - 1e-13, Units.CELSIUS) ++ .addQuantitative("Temperature", 10, 200, 0.1, 5.0 - Numerics.COMPARISON_THRESHOLD, Units.CELSIUS) + .build(); + + assertTrue(base.equals(base, ComparisonMode.APPROXIMATE), + "A sample dimension should be approximately equal to itself"); + // Different dimension name is ignored under APPROXIMATE — must be equal. + assertTrue (base.equals(strictlyEqualToBase, ComparisonMode.APPROXIMATE), + "two strictly equal sample dimensions should also be approximately equal"); + assertTrue (strictlyEqualToBase.equals(base, ComparisonMode.APPROXIMATE), + "two strictly equal sample dimensions should also be approximately equal"); + + // Different dimension name is ignored under APPROXIMATE — must be equal. + assertTrue (base.equals(renamed, ComparisonMode.APPROXIMATE), + "name difference should be ignored on Sample dimension approximate equality"); + assertTrue (renamed.equals(base, ComparisonMode.APPROXIMATE), + "name difference should be ignored on Sample dimension approximate equality"); + + // The same pair must NOT be equal under STRICT because names differ. + assertFalse(base.equals(renamed, ComparisonMode.STRICT), "STRICT must detect name difference"); + + // Different scale in the transfer function → not equal even approximately. + assertFalse(base.equals(differentScale, ComparisonMode.APPROXIMATE), "different scale must produce inequality"); + assertFalse(differentScale.equals(base, ComparisonMode.APPROXIMATE), "different scale must produce inequality"); + + // A very little difference in transfer function offset should still mark both dimensions as approximately equal + assertTrue (base.equals(tinyOffsetDiff, ComparisonMode.APPROXIMATE), + "A tiny offset difference should not fail approximate equality"); + assertTrue (tinyOffsetDiff.equals(base, ComparisonMode.APPROXIMATE), + "A tiny offset difference should not fail approximate equality"); + } } diff --cc endorsed/src/org.apache.sis.feature/test/org/apache/sis/image/ImageProcessorTest.java index bf2fa4c1fe,f817f65f40..7ce57aa8b5 --- a/endorsed/src/org.apache.sis.feature/test/org/apache/sis/image/ImageProcessorTest.java +++ b/endorsed/src/org.apache.sis.feature/test/org/apache/sis/image/ImageProcessorTest.java @@@ -21,6 -23,6 +21,7 @@@ import java.util.stream.IntStream import java.awt.Shape; import java.awt.Rectangle; import java.awt.image.Raster; ++import java.awt.image.SampleModel; import java.awt.image.BufferedImage; import java.awt.image.RenderedImage; import org.opengis.referencing.operation.MathTransform; @@@ -31,12 -35,12 +32,14 @@@ import static org.junit.jupiter.api.Ass import org.apache.sis.image.processing.isoline.IsolinesTest; import org.apache.sis.test.TestCase; import static org.apache.sis.test.Assertions.assertSingleton; ++import static org.apache.sis.feature.Assertions.assertPixelsEqual; /** * Tests {@link ImageProcessor}. * * @author Martin Desruisseaux (Geomatys) ++ * @author Alexis Manin (Geomatys) */ @SuppressWarnings("exports") public final class ImageProcessorTest extends TestCase { @@@ -110,4 -114,42 +113,46 @@@ IsolinesTest.verifyIsolineFromMultiCells(assertSingleton(r.values())); } while ((parallel = !parallel) == true); } + + /** - * Verify that {@link ImageProcessor#reformat(RenderedImage, SampleModel) reformat} properly adapt tile size - * according to given parameters. ++ * Tests {@link ImageProcessor#reformat(RenderedImage, SampleModel)} with a change of tile size. ++ * The reformat operation shall properly adapt tile size according to given parameters. + */ + @Test - public void changeTileSize() { - changeTileSize(12, 12, 4, 2); ++ public void testReformatWithTileSizeChange() { ++ changeTileSize(12, 12, 4, 2); + changeTileSize(64, 64, 32, 32); - changeTileSize(50, 50, 5, 5); ++ changeTileSize(50, 50, 5, 5); + } + - private void changeTileSize(int sourceImageWidth, int sourceImageHeight, int targetTileWidth, int targetTileHeight) { ++ /** ++ * Implementation of {@link #testReformatWithTileSizeChange()} with the given image and tile sizes. ++ */ ++ private void changeTileSize(final int sourceImageWidth, final int sourceImageHeight, ++ final int targetTileWidth, final int targetTileHeight) ++ { + // Fill source image + final var image = new BufferedImage(sourceImageWidth, sourceImageHeight, BufferedImage.TYPE_BYTE_GRAY); + final var canvas = image.getRaster(); - for (int y = 0 ; y < image.getHeight() ; y++) { - for (int x = 0 ; x < image.getWidth() ; x++) { ++ for (int y = image.getHeight(); --y >= 0;) { ++ for (int x = image.getWidth(); --x >= 0;) { + canvas.setSample(x, y, 0, x*y); + } + } + + // Prepare target image layout + final var tileModel = image.getSampleModel().createCompatibleSampleModel(targetTileWidth, targetTileHeight); - final var preferredTileSize = new Dimension(tileModel.getWidth(), tileModel.getHeight()); - processor.setImageLayout(new ImageLayout(tileModel, preferredTileSize, true, false, true, null)); ++ processor.setImageLayout(ImageLayout.DEFAULT.withSampleModel(tileModel, true)); + + // Execute and verify twice: sequential then parallel ++ final var imageBounds = new Rectangle(image.getWidth(), image.getHeight()); + boolean parallel = false; - final var imageBounds = new Rectangle(0, 0, image.getWidth(), image.getHeight()); + do { + processor.setExecutionMode(parallel ? ImageProcessor.Mode.SEQUENTIAL : ImageProcessor.Mode.PARALLEL); + final RenderedImage reformatted = processor.reformat(image, null); ++ assertEquals(targetTileWidth, reformatted.getTileWidth(), "Reformatted image tile width"); ++ assertEquals(targetTileHeight, reformatted.getTileHeight(), "Reformatted image tile height"); + assertPixelsEqual(image, imageBounds, reformatted, imageBounds); - assertEquals(tileModel.getWidth(), reformatted.getTileWidth(), "Reformatted image tile width"); - assertEquals(tileModel.getHeight(), reformatted.getTileHeight(), "Reformatted image tile height"); + } while ((parallel = !parallel) == true); + } } diff --cc endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/writer/ReformattedImage.java index 4894d16b5b,4894d16b5b..0c05b9a1a6 --- a/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/writer/ReformattedImage.java +++ b/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/writer/ReformattedImage.java @@@ -44,11 -44,11 +44,11 @@@ import org.apache.sis.io.stream.HyperRe */ public final class ReformattedImage { /** -- * Divisor of tile sizes mandated by the TIFF specification. ++ * Divisor of tile sizes mandated by the <abbr>TIFF</abbr> specification. * All tile sizes must be a multiple of this value. * This constant must be a power of 2. */ -- private static final int TILE_DIVISOR = 16; ++ public static final int TILE_DIVISOR = 16; /** * Number of color bands before the extra bands. This number does not include the alpha channel, diff --cc endorsed/src/org.apache.sis.storage.geotiff/test/org/apache/sis/storage/geotiff/GeoTiffStoreTest.java index 2b5804e8a5,ecad02e598..93be7446d2 --- a/endorsed/src/org.apache.sis.storage.geotiff/test/org/apache/sis/storage/geotiff/GeoTiffStoreTest.java +++ b/endorsed/src/org.apache.sis.storage.geotiff/test/org/apache/sis/storage/geotiff/GeoTiffStoreTest.java @@@ -16,23 -16,18 +16,25 @@@ */ package org.apache.sis.storage.geotiff; +import java.util.Random; +import java.util.Iterator; +import java.io.IOException; +import java.io.InputStream; import java.io.ByteArrayOutputStream; + import java.nio.ByteBuffer; import java.nio.file.Path; import java.nio.file.Files; +import java.nio.file.StandardOpenOption; import java.awt.Dimension; import java.awt.Rectangle; import java.awt.image.BufferedImage; import java.awt.image.RenderedImage; -import org.apache.sis.storage.StorageConnector; -import org.apache.sis.util.Utilities; +import javax.imageio.ImageIO; +import javax.imageio.ImageReader; +import javax.imageio.stream.ImageInputStream; import org.opengis.referencing.cs.AxisDirection; import org.opengis.referencing.operation.TransformException; ++import org.apache.sis.util.Utilities; import org.apache.sis.geometry.Envelopes; import org.apache.sis.geometry.GeneralEnvelope; import org.apache.sis.image.DataType; @@@ -50,15 -43,16 +52,17 @@@ import org.apache.sis.coverage.grid.Gri import org.apache.sis.referencing.CRS; import org.apache.sis.referencing.operation.transform.MathTransforms; import org.apache.sis.referencing.operation.matrix.Matrix4; ++import static org.apache.sis.storage.geotiff.writer.ReformattedImage.TILE_DIVISOR; // Test dependencies -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; - import static org.junit.jupiter.api.Assertions.*; import static org.apache.sis.test.Assertions.assertSingleton; -import static org.apache.sis.feature.Assertions.assertGridToCornerEquals; + import static org.apache.sis.feature.Assertions.assertPixelsEqual; +import static org.apache.sis.feature.Assertions.assertGridToCornerEquals; +import org.apache.sis.image.OverviewImageTest; import org.apache.sis.test.TestCase; +import org.apache.sis.test.TestUtilities; import org.apache.sis.referencing.crs.HardCodedCRS; import org.apache.sis.referencing.operation.HardCodedConversions; @@@ -71,17 -65,16 +75,34 @@@ import static org.opengis.test.Assertio * This class tests indirectly (via {@link GeoTiffStore}) the {@link Reader} and {@link Writer} classes. * * @author Martin Desruisseaux (Geomatys) + * @author Estelle Idée (Geomatys) ++ * @author Alexis Manin (Geomatys) */ @SuppressWarnings("exports") public final class GeoTiffStoreTest extends TestCase { /** * Name of a test file for an untiled image with a single band in gray-scale. ++ * The image is uncompressed for avoiding <abbr>JVM</abbr>-dependent variations. ++ */ ++ static final String UNTILED_WITHOUT_COMPRESSION = "untiled_without_compression.tiff"; ++ ++ /** ++ * Name of a test file for an image similar to {@link #UNTILED_WITHOUT_COMPRESSION} but tiled. ++ * The image is uncompressed for avoiding <abbr>JVM</abbr>-dependent variations. ++ */ ++ static final String TILED_WITHOUT_COMPRESSION = "tiled_without_compression.tiff"; ++ ++ /** ++ * Name of a test file for an untiled image with a single band in gray-scale. ++ * This image has been encoded with the default compression and predictor. ++ * Note that the stream of bytes encoded with {@link Compression#DEFLATE} ++ * may varies depending on the <abbr>JVM</abbr>. */ static final String UNTILED = "untiled.tiff"; /** * Name of a test file for an image similar to {@link #UNTILED} but tiled. ++ * This image has been encoded with the default compression and predictor. */ static final String TILED = "tiled.tiff"; @@@ -143,43 -136,55 +164,59 @@@ } /** -- * Writes an image and compare with the {@code "untiled.tiff"} file. ++ * Writes an image and compares with the {@code "untiled_without_compression.tiff"} file. ++ * Then, reads back the image and performs some validations. * * @throws Exception if a referencing or I/O error occurred. */ @Test public void testWriteUntiled() throws Exception { - testWrite(UNTILED, new Rectangle(32, 16), null, 1054); - testWrite(new Rectangle(32, 16), null); ++ testWriteAndRead(UNTILED_WITHOUT_COMPRESSION, new Rectangle(32, 16), null, 1240); } /** -- * Writes an image and compare with the {@code "tiled.tiff"} file. ++ * Writes an image and compares with the {@code "tiled_without_compression.tiff"} file. ++ * Then, reads back the image and performs some validations. * * @throws Exception if a referencing or I/O error occurred. */ @Test public void testWriteTiled() throws Exception { final var tileSize = new Dimension(16, 16); // TIFF tile size must be multiple of 16. - testWrite(TILED, new Rectangle(tileSize.width * 3, tileSize.height * 2), tileSize, 2334); - testWrite(new Rectangle(tileSize.width * 3, tileSize.height * 2), tileSize); ++ testWriteAndRead(TILED_WITHOUT_COMPRESSION, new Rectangle(tileSize.width * 3, tileSize.height * 2), tileSize, 2324); + } + + /** - * Writes an image and compare with the {@code "tiled.tiff"} file. - * <p> - * This test differs from {@link #testWriteTiled()} because it requests a tile size not accepted as is by geotiff. - * The aim of this test is to ensure that Geotiff writer will adapt tile size according to the Tiff standard. - * It requests tiles of size 19, and expect the Geotiff writer to adapt request to write tiles of size 16 or 32. - * </p> ++ * Writes an image and validates the result. ++ * This test differs from {@link #testWriteTiled()} because it requests a tile size not accepted as-is by GeoTIFF. ++ * The aim of this test is to ensure that GeoTIFF writer will adapt tile size according to the TIFF standard. ++ * It requests tiles of size 7, and expects the GeoTIFF writer to adapt request for writing tiles of size 16 or 32. ++ * ++ * @throws Exception if a referencing or I/O error occurred. + */ + @Test - public void testWriteTiledAdapted() throws Exception { - final var tileSize = new Dimension(7, 7); - testWrite(new Rectangle(64, 64), tileSize); ++ public void testWriteResizedTiles() throws Exception { ++ testWriteAndRead(null, new Rectangle(64, 64), new Dimension(7, 7), 4964); } /** * Implementation of {@link #testWriteUntiled()} and {@link #testWriteTiled()}. ++ * The image is written with no compression for avoiding variations in compression algorithms. * - * @param filename name of the file which contain the expected image. ++ * @param filename name of the file which contain the expected image, or {@code null} if none. * @param bounds bounds of the image to create. * @param tileSize size of the tiles, or {@code null} for the image size. + * @param length expected length in bytes. */ - private static void testWrite(final String filename, final Rectangle bounds, final Dimension tileSize, final int length) - private static void testWrite(final Rectangle bounds, final Dimension tileSize) - throws TransformException, DataStoreException ++ private static void testWriteAndRead(final String filename, final Rectangle bounds, final Dimension tileSize, final int length) + throws TransformException, DataStoreException, IOException { /* * We need a CRS which has no EPSG code for ensuring that the test write the same GeoTIFF keys * with or without the presence of an EPSG database on machine which is building this project. */ -- var crs = HardCodedConversions.mercator(HardCodedCRS.JUPITER); -- var geographicArea = new GeneralEnvelope(HardCodedCRS.JUPITER); ++ final var crs = HardCodedConversions.mercator(HardCodedCRS.JUPITER); ++ final var geographicArea = new GeneralEnvelope(HardCodedCRS.JUPITER); geographicArea.setRange(0, 132, 145); // Range of longitude values. geographicArea.setRange(1, 30, 42); // Range of latitude values. final GridCoverage coverage = new GridCoverageBuilder() @@@ -188,18 -193,71 +225,79 @@@ .flipGridAxis(1) .build(); - final var buffer = new ByteArrayOutputStream(); - try (DataStore ds = DataStores.openWritable(buffer, "geotiff")) { + final var buffer = new ByteArrayOutputStream(length); - try (DataStore ds = DataStores.openWritable(buffer, "geotiff")) { ++ final var source = new StorageConnector(buffer); ++ source.setOption(Compression.OPTION_KEY, Compression.NONE); ++ try (DataStore ds = DataStores.openWritable(source, "geotiff")) { assertInstanceOf(GeoTiffStore.class, ds).append(coverage, null); } - final byte[] actual = buffer.toByteArray(); - final byte[] expected; - try (InputStream in = GeoTiffStoreTest.class.getResourceAsStream(filename)) { - assertNotNull(in, filename); - expected = in.readAllBytes(); - } - assertArrayEquals(expected, actual); - try (var store = new GeoTiffStore(new GeoTiffStoreProvider(), new StorageConnector(ByteBuffer.wrap(actual)))) { - var coverageToValidate = store.components().get(0).read(null); - final var expectedGridGeom = coverage.getGridGeometry(); - final var actualGridGeom = coverageToValidate.getGridGeometry(); - assertTrue( - Utilities.equalsApproximately(expectedGridGeom, actualGridGeom), - () -> String.format( - "Written grid geometry differs from original one.%nOriginal:%n%s%nWritten:%n%s%n", - expectedGridGeom, actualGridGeom - ) - ); - - assertTrue( - Utilities.equalsApproximately(expectedGridGeom, actualGridGeom), - () -> String.format( - "Written grid geometry differs from original one.%nOriginal:%n%s%nWritten:%n%s%n", - expectedGridGeom, actualGridGeom - ) - ); + assertEquals(length, actual.length); ++ if (filename != null) { ++ final byte[] expected; ++ try (InputStream in = GeoTiffStoreTest.class.getResourceAsStream(filename)) { ++ assertNotNull(in, filename); ++ expected = in.readAllBytes(); ++ } ++ assertArrayEquals(expected, actual); ++ } ++ /* ++ * At this point, the test of the writer is considered as completed since we compared the writer output ++ * against the expected stream of bytes in a file. The remaining of this method is test of the reader, ++ * unless we had no file to compare with. ++ */ ++ try (var store = new GeoTiffStore(null, new StorageConnector(ByteBuffer.wrap(actual)))) { ++ final var coverageToValidate = assertSingleton(store.components()).read(null); ++ assertEqualsApproximately( ++ coverage.getGridGeometry(), ++ coverageToValidate.getGridGeometry(), ++ "Written grid geometry differs from original one."); + - final var expectedSampleDims = coverage.getSampleDimensions(); - final var actualSampleDims = coverageToValidate.getSampleDimensions(); - assertTrue( - Utilities.equalsApproximately(expectedSampleDims, actualSampleDims), - () -> String.format( - "Written Sample dimensions differ from original one.%nOriginal:%n%s%nWritten:%n%s%n", - expectedSampleDims, actualSampleDims - ) - ); ++ assertEqualsApproximately( ++ coverage.getSampleDimensions(), ++ coverageToValidate.getSampleDimensions(), ++ "Written Sample dimensions differ from original one."); + + final var actualRendering = coverageToValidate.render(null); + assertPixelsEqual(coverage.render(null), null, actualRendering, null); - // If user requested a tiled dataset, we must ensure the written Geotiff file has been tiled - if (tileSize != null && (tileSize.getWidth() < bounds.getWidth() || tileSize.getHeight() < bounds.getHeight())) { - assertTiling(actualRendering, tileSize, 16); ++ if (tileSize != null) { ++ // If user requested a tiled dataset, we must ensure the written GeoTIFF file has been tiled. ++ validateTileSize("width", tileSize.width, actualRendering.getTileWidth()); ++ validateTileSize("height", tileSize.height, actualRendering.getTileHeight()); + } + } + } + + /** - * Represent the side of the tile being evaluated. Either width (X) or height (Y). ++ * Asserts that the given object are approximately equal. ++ * ++ * @param expected the expected object. ++ * @param actual the actual object. ++ * @param message message to show if the objects are not approximately equal. + */ - private enum TileAxis { width, height } ++ private static void assertEqualsApproximately(final Object expected, final Object actual, final String message) { ++ assertTrue(Utilities.equalsApproximately(expected, actual), ++ () -> String.format("%s%nOriginal:%n%s%nWritten:%n%s%n", message, expected, actual)); ++ } + + /** - * Verify that given image tiling respects user tiling request, modulo a given restriction. - * The restriction maps Tiff standard requirement for tile size to be multiple of a given factor. - * </br> - * It means that if user requests a tile size of 3, but the restriction factor is 2, - * then we expect the image to use a tile size of either 2 or 4, - * which are the nearest enclosing multiples of 2 for request 3. ++ * Verifies that given tile width or height is compliant with <abbr>TIFF</abbr> standard. ++ * The {@code sourceSize} and {@code writtenSize} arguments shall be the rendered image's ++ * {@linkplain RenderedImage#getTileWidth() tile width} or {@linkplain RenderedImage#getTileHeight() tile height}. ++ * ++ * <p>If the source image had a tile size which was already compliant with <abbr>TIFF</abbr> requirement, ++ * then the image should have been written. Otherwise, there is no guarantee about the size of the tiles. ++ * It will not necessarily be the multiple of 16 immediately before or after the original tile size.</p> + * - * @param actualRendering The image to control tiling on. - * @param tileSize The tile size requested by user. - * @param tileSizeMultiple A factor to use to adapted requested tile size. ++ * @param axis which side of the tiling is being tested. Used for assertion error message formatting. ++ * @param sourceSize the tile size along tested side in the image to write. ++ * @param writtenSize the tile size along tested side in the image which has been written. + */ - private static void assertTiling(RenderedImage actualRendering, Dimension tileSize, int tileSizeMultiple) { - assertTileSize(TileAxis.width, actualRendering.getWidth(), actualRendering.getTileWidth(), tileSize.width, tileSizeMultiple); - assertTileSize(TileAxis.height, actualRendering.getHeight(), actualRendering.getTileHeight(), tileSize.height, tileSizeMultiple); ++ private static void validateTileSize(final String axis, final int sourceSize, final int writtenSize) { ++ assertEquals(0, writtenSize % TILE_DIVISOR, () -> axis + " tile size is not a multiple of " + TILE_DIVISOR); ++ if ((sourceSize % TILE_DIVISOR) == 0) { ++ assertEquals(sourceSize, writtenSize, () -> axis + " should have keep the original tile size"); ++ } } /** diff --cc endorsed/src/org.apache.sis.storage.geotiff/test/org/apache/sis/storage/geotiff/ReaderTest.java index 3534cb5a70,3534cb5a70..1a6b96cf4e --- a/endorsed/src/org.apache.sis.storage.geotiff/test/org/apache/sis/storage/geotiff/ReaderTest.java +++ b/endorsed/src/org.apache.sis.storage.geotiff/test/org/apache/sis/storage/geotiff/ReaderTest.java @@@ -76,6 -76,6 +76,16 @@@ public class ReaderTest extends TestCas return image.getTile(image.getMinTileX(), image.getMinTileY()); } ++ /** ++ * Tests the tile matrix set or an untiled image. ++ * ++ * @throws DataStoreException if an error occurred while creating the data store. ++ */ ++ @Test ++ public void testUntiledWithoutCompression() throws DataStoreException { ++ readAndVerify(false, GeoTiffStoreTest.UNTILED_WITHOUT_COMPRESSION); ++ } ++ /** * Tests the tile matrix set or an untiled image. * @@@ -83,11 -83,11 +93,17 @@@ */ @Test public void testUntiled() throws DataStoreException { -- try (GeoTiffStore ds = createStore(GeoTiffStoreTest.UNTILED)) { -- final GridCoverageResource resource = assertInstanceOf(GridCoverageResource.class, assertSingleton(ds.components())); -- assertInstanceOf(ProjectedCRS.class, resource.getGridGeometry().getCoordinateReferenceSystem()); -- assertTrue(assertInstanceOf(TiledResource.class, resource).getTileMatrixSets().isEmpty()); -- } ++ readAndVerify(false, GeoTiffStoreTest.UNTILED); ++ } ++ ++ /** ++ * Tests the tile matrix set. ++ * ++ * @throws DataStoreException if an error occurred while creating the data store. ++ */ ++ @Test ++ public void testTileMatrixSetWithoutCompression() throws DataStoreException { ++ readAndVerify(true, GeoTiffStoreTest.TILED_WITHOUT_COMPRESSION); } /** @@@ -97,13 -97,13 +113,28 @@@ */ @Test public void testTileMatrixSet() throws DataStoreException { -- try (GeoTiffStore ds = createStore(GeoTiffStoreTest.TILED)) { ++ readAndVerify(true, GeoTiffStoreTest.TILED); ++ } ++ ++ /** ++ * Implementation of {@link #testUntiled()} and {@link #testTiled()}. ++ * ++ * @param tiled whether the file to read is tiled. ++ * @param filename the file to read. ++ * @throws DataStoreException if an error occurred while creating the data store. ++ */ ++ private void readAndVerify(final boolean tiled, final String filename) throws DataStoreException { ++ try (GeoTiffStore ds = createStore(filename)) { final GridCoverageResource resource = assertInstanceOf(GridCoverageResource.class, assertSingleton(ds.components())); assertInstanceOf(ProjectedCRS.class, resource.getGridGeometry().getCoordinateReferenceSystem()); -- ++ if (!tiled) { ++ assertTrue(assertInstanceOf(TiledResource.class, resource).getTileMatrixSets().isEmpty()); ++ return; ++ } ++ final String name = filename.substring(0, filename.lastIndexOf('.')); final TileMatrixSet pyramid = assertSingleton(assertInstanceOf(TiledResource.class, resource).getTileMatrixSets()); assertSame(resource.getGridGeometry().getCoordinateReferenceSystem(), pyramid.getCoordinateReferenceSystem()); -- assertEquals("tiled:1:TMS", pyramid.getIdentifier().toString()); ++ assertEquals(name + ":1:TMS", pyramid.getIdentifier().toString()); assertFalse(pyramid.getEnvelope().isEmpty()); final SortedMap<GenericName, ? extends TileMatrix> matrices = pyramid.getTileMatrices(); @@@ -113,7 -113,7 +144,7 @@@ assertTrue(matrices.subMap(matrices.firstKey(), matrices.lastKey()).isEmpty()); final TileMatrix matrix = assertSingleton(matrices.values()); -- assertEquals("tiled:1:TMS:L0", matrix.getIdentifier().toFullyQualifiedName().toString()); ++ assertEquals(name + ":1:TMS:L0", matrix.getIdentifier().toFullyQualifiedName().toString()); assertEquals("TMS:L0", matrix.getIdentifier().toString()); assertEquals(assertSingleton(matrices.keySet()), matrix.getIdentifier()); assertArrayEquals(resource.getGridGeometry().getResolution(false), matrix.getResolution()); diff --cc endorsed/src/org.apache.sis.storage.geotiff/test/org/apache/sis/storage/geotiff/tiled_without_compression.tiff index 0000000000,0000000000..25636e35a6 new file mode 100644 Binary files differ diff --cc endorsed/src/org.apache.sis.storage.geotiff/test/org/apache/sis/storage/geotiff/untiled_without_compression.tiff index 0000000000,0000000000..c9908b20e5 new file mode 100644 Binary files differ diff --cc endorsed/src/org.apache.sis.util/main/org/apache/sis/math/NumberType.java index 2f7dddfe17,2f7dddfe17..c6e2850669 --- a/endorsed/src/org.apache.sis.util/main/org/apache/sis/math/NumberType.java +++ b/endorsed/src/org.apache.sis.util/main/org/apache/sis/math/NumberType.java @@@ -47,7 -47,7 +47,7 @@@ import org.apache.sis.util.internal.sha * </ul> * * @author Martin Desruisseaux (IRD, Geomatys) -- * @version 1.6 ++ * @version 1.7 * * @see org.apache.sis.util.Numbers * @see org.apache.sis.image.DataType @@@ -402,13 -402,13 +402,17 @@@ public enum NumberType * @param types the primitive types or {@link Number} classes for which to get a common enumeration value. * @return the enumeration value for the given types, or {@code null}. */ -- private static NumberType forClasses(final boolean optional, final Class<?>... types) { -- NumberType widest = VOID; ++ private static NumberType forClasses(final boolean optional, final Class<?>[] types) { ++ NumberType widest = VOID; // Value of size 0 bit (not the same as null value). boolean found = false; if (types != null) { for (final Class<?> type : types) { -- final NumberType other = forClass(type).orElse(null); -- if (other == null) { ++ final NumberType other = valueOrNull(type); ++ if (other == null) { // Means unknown type (not null type). ++ if (Number.class.isAssignableFrom(type)) { ++ widest = NUMBER; ++ continue; ++ } if (optional) return null; throw new IllegalArgumentException(Errors.format(Errors.Keys.NotANumericalType_1, type)); } @@@ -549,9 -549,9 +553,6 @@@ * @return whether this type is considered narrower than the specified type. */ public boolean isNarrowerThan(final NumberType other) { -- if (other == CHARACTER) { -- return other.isWiderThan(this); -- } return other.ordinal() < NUMBER.ordinal() && ordinal() < other.ordinal() && category <= other.category; // Integers considered narrower than floating-point types. @@@ -572,9 -572,9 +573,6 @@@ * @return whether this type is considered wider than the specified type. */ public boolean isWiderThan(final NumberType other) { -- if (other == CHARACTER) { -- return other.isNarrowerThan(this); -- } return ordinal() < NUMBER.ordinal() && ordinal() > other.ordinal() && category >= other.category; // Floating-point types considered wider than integers. @@@ -685,6 -685,6 +683,25 @@@ throw new ArithmeticException(Errors.format(Errors.Keys.CanNotConvertValue_2, expected, wrapper)); } ++ /** ++ * Returns whether the two given numbers are equal when compared as instances of this type. ++ * The comparison may be inexact if this type is not wide enough for the widest argument value. ++ * ++ * <p><b>Example:</b> ++ * {@link #INTEGER} compares the values returned by the {@link Number#intValue()} method. ++ * It means that if two {@link Long} instances are compared using {@link #INTEGER}, ++ * then only the lowest {@value Integer#SIZE} bits are compared.</p> ++ * ++ * @param v1 first value to compare, or {@code null}. ++ * @param v2 second value to compare, or {@code null}. ++ * @return whether the two values are equal when compared as instances of this type. ++ * ++ * @since 1.7 ++ */ ++ public boolean equals(final Number v1, final Number v2) { ++ return Objects.equals(cast(v1), cast(v2)); ++ } ++ /** * Casts a value to the specified type without checking whether is would cause precision lost. * This method makes the following choice: @@@ -701,7 -701,7 +718,7 @@@ * If this type is not wide enough, then the behavior depends on the implementation of the corresponding * {@code Number.fooValue()} method - typically, the value is just rounded or truncated. * -- * @param number the number to cast, or {@code null}. ++ * @param value the number to cast, or {@code null}. * @return the number cast to this type, or {@code null} if the given number was null. * @throws UnsupportedOperationException if this type cannot cast numbers. * @throws NullPointerException if {@code number} is null. @@@ -710,9 -710,9 +727,9 @@@ * * @see org.apache.sis.util.Numbers#cast(Number, Class) */ -- public Number cast(Number number) { -- if (number == null) { -- return number; ++ public Number cast(Number value) { ++ if (value == null || wrapper.isInstance(value)) { ++ return value; } throw new UnsupportedOperationException(Errors.format(Errors.Keys.UnknownType_1, this)); } diff --cc endorsed/src/org.apache.sis.util/main/org/apache/sis/measure/MeasurementRange.java index 4c36100b25,8b0638edbf..18a051811b --- a/endorsed/src/org.apache.sis.util/main/org/apache/sis/measure/MeasurementRange.java +++ b/endorsed/src/org.apache.sis.util/main/org/apache/sis/measure/MeasurementRange.java @@@ -55,7 -57,7 +57,8 @@@ import org.apache.sis.util.resources.Er * encouraged to make sure that subclasses remain immutable for more predictable behavior. * * @author Martin Desruisseaux (IRD) -- * @version 0.6 ++ * @author Alexis Manin (Geomatys) ++ * @version 1.7 * * @param <E> the type of range elements as a subclass of {@link Number}. * @@@ -64,6 -66,6 +67,7 @@@ * * @since 0.3 */ ++@SuppressWarnings("EqualsAndHashcode") public class MeasurementRange<E extends Number & Comparable<? super E>> extends NumberRange<E> { /** * Serial number for inter-operability with different versions. @@@ -471,16 -473,17 +475,26 @@@ } /** -- * Compares this measurement range with the specified object for equality. Two {@code MeasurementRange} instances -- * are considered equal if they met all conditions {@linkplain Range#equals(Object) documented in the parent class} -- * and their {@link #unit()} are equal in the sense of {@link Objects#equals(Object, Object)}. ++ * Compares this measurement range with the specified object for equality according the specified criterion. ++ * Two {@code MeasurementRange} instances are considered equal if they met all conditions documented in the ++ * {@linkplain Range#equals(Object, ComparisonMode) parent class} and their {@linkplain #unit() units} are equal. * Note that this comparison does not distinguish the various {@link Float#NaN} or {@link Double#NaN} bit patterns. * * @return {@inheritDoc} ++ * ++ * @since 1.7 */ @Override - public boolean equals(final Object object) { - return super.equals(object) && Objects.equals(unit, ((MeasurementRange<?>) object).unit); - public boolean equals(final Object object, ComparisonMode mode) { - return super.equals(object, mode) - && Utilities.deepEquals(unit, ((MeasurementRange<?>) object).unit, mode); ++ public boolean equals(final Object object, final ComparisonMode mode) { ++ if (super.equals(object, mode) && object instanceof MeasurementRange<?>) { ++ final var other = (MeasurementRange<?>) object; ++ if (mode == ComparisonMode.STRICT) { ++ return Objects.equals(unit, other.unit); ++ } else { ++ return Utilities.deepEquals(unit(), other.unit(), mode); ++ } ++ } ++ return false; } /** diff --cc endorsed/src/org.apache.sis.util/main/org/apache/sis/measure/Range.java index 016ff2169c,ab67f954a7..3b00e5ae9c --- a/endorsed/src/org.apache.sis.util/main/org/apache/sis/measure/Range.java +++ b/endorsed/src/org.apache.sis.util/main/org/apache/sis/measure/Range.java @@@ -25,8 -25,11 +25,11 @@@ import javax.measure.Unit import org.apache.sis.math.NumberType; import org.apache.sis.util.ArgumentChecks; import org.apache.sis.util.ArgumentCheckByAssertion; + import org.apache.sis.util.ComparisonMode; import org.apache.sis.util.Emptiable; + import org.apache.sis.util.LenientComparable; -import org.apache.sis.util.Utilities; import org.apache.sis.util.internal.shared.Strings; ++import org.apache.sis.util.internal.shared.Numerics; import org.apache.sis.util.collection.CheckedContainer; @@@ -81,7 -84,7 +84,8 @@@ * @author Joe White * @author Martin Desruisseaux (Geomatys) * @author Jody Garnett (for parameterized type inspiration) -- * @version 1.6 ++ * @author Alexis Manin (Geomatys) ++ * @version 1.7 * * @param <E> the type of range elements, typically a {@link Number} subclass or {@link java.util.Date}. * @@@ -639,27 -642,12 +643,124 @@@ public class Range<E extends Comparable * @return {@code true} if the given object is equal to this range. */ @Override - public boolean equals(final Object object) { + public final boolean equals(final Object object) { + return equals(object, ComparisonMode.STRICT); + } + ++ /** ++ * Compares this range with the given object for equality with a tolerance specified by the given mode. ++ * This method performs the comparison described in {@link #equals(Object)} except for the following: ++ * ++ * <ul> ++ * <li>{@link ComparisonMode#IGNORE_METADATA} or more tolerant: ++ * ignore the {@linkplain #getElementType() element types}. ++ * Numbers are converted to the same type before comparison.</li> ++ * <li>{@link ComparisonMode#APPROXIMATE} or more tolerant: compare the minimum and maximum ++ * values as double-precision floating point numbers with an arbitrary tolerance threshold. ++ * The threshold may change in any future version of Apache <abbr>SIS</abbr>.</li> ++ * <li>{@link ComparisonMode#ALLOW_VARIANT} or more tolerant: allow mixing different boundary types. ++ * For example, {@code (0 … 256)} is equivalent to {@code [1 … 255]} in the case of integer types ++ * (but not for floating point types).</li> ++ * </ul> ++ * ++ * @param object the object to compare with this range for equality. ++ * @param mode the strictness level of the comparison. ++ * @return {@code true} if the given object is equal to this range at the given strictness level. ++ * ++ * @since 1.7 ++ */ ++ @Override ++ public boolean equals(final Object object, final ComparisonMode mode) { + if (object == this) { + return true; + } - if (object != null && object.getClass() == getClass()) { - final Range<?> other = (Range<?>) object; - if (Objects.equals(elementType, other.elementType)) { ++ if (mode == ComparisonMode.STRICT) { ++ if (object != null && object.getClass() == getClass()) { ++ final var other = (Range<?>) object; ++ if (elementType == other.elementType) { ++ if (isEmpty()) { ++ return other.isEmpty(); ++ } ++ return isMinIncluded == other.isMinIncluded && ++ isMaxIncluded == other.isMaxIncluded && ++ Objects.equals(minValue, other.minValue) && ++ Objects.equals(maxValue, other.maxValue); ++ } ++ } ++ } else if (object instanceof Range<?>) { ++ final var other = (Range<?>) object; ++ if (mode.isIgnoringMetadata() || getElementType() == getElementType()) { + if (isEmpty()) { + return other.isEmpty(); + } - return Objects.equals(minValue, other.minValue) && - Objects.equals(maxValue, other.maxValue) && - isMinIncluded == other.isMinIncluded && - isMaxIncluded == other.isMaxIncluded; ++ return epsilonEqual(getMinValue(), isMinIncluded(), other.getMinValue(), other.isMinIncluded(), +1, mode) && ++ epsilonEqual(getMaxValue(), isMaxIncluded(), other.getMaxValue(), other.isMaxIncluded(), -1, mode); ++ } ++ } ++ return false; ++ } ++ ++ /** ++ * Returns {@code true} if the given values are equal according the specified strictness level. ++ * In {@code APPROXIMATE} or {@code DEBUG} mode, this method will derive a relative comparison ++ * threshold from the {@link Numerics#COMPARISON_THRESHOLD} constant. ++ * ++ * @param value1 the first value to compare. ++ * @param value2 the second value to compare. ++ * @param isInc1 whether {@code value1} is inclusive. ++ * @param isInc2 whether {@code value2} is inclusive. ++ * @param toIncl value to add to an integer for making it inclusive. ++ * @param mode the strictness level to use for comparing the numbers. ++ * @return {@code true} if both values are considered equal for the given comparison mode. ++ */ ++ private static boolean epsilonEqual(final Object value1, final boolean isInc1, ++ final Object value2, final boolean isInc2, ++ final int toIncl, final ComparisonMode mode) ++ { ++ if (Objects.equals(value1, value2)) { ++ return isInc1 == isInc2; ++ } ++ if (isInc1 == isInc2 || mode.ordinal() >= ComparisonMode.ALLOW_VARIANT.ordinal()) { ++ if (mode.isIgnoringMetadata() && value1 instanceof Number && value2 instanceof Number) { ++ final var n1 = (Number) value1; ++ final var n2 = (Number) value2; ++ NumberType t1 = NumberType.forNumberClass(n1.getClass()); ++ NumberType t2 = NumberType.forNumberClass(n2.getClass()); ++ if (mode.isApproximate()) { ++ double v1 = n1.doubleValue(); ++ double v2 = n2.doubleValue(); ++ if (isInc1 != isInc2) { ++ /* ++ * The comparison mode allows variants. If at least one value is an integer, ++ * increment or decrement that value for having the same inclusiveness status ++ * as the other value. ++ */ if (t1.isInteger()) v1 += (isInc1 ? -toIncl : toIncl); ++ else if (t2.isInteger()) v2 += (isInc2 ? -toIncl : toIncl); ++ } ++ /* ++ * Compare as floating point numbers regardless the value types (even integers). ++ * This is for avoiding the difficulty of keeping the result consistent with all ++ * combinations of "is included" flags and comparison modes. The comparison stay ++ * nevertheless strict for type `int` or smaller, and become approximate only for ++ * type `long` or larger (`BigInteger`). ++ */ ++ return Numerics.epsilonEqual(v1, v2, mode); ++ } else { ++ if (t2.isWiderThan(t1)) { ++ // Conservatively use `double` for avoiding accuracy lost if converting from `int`. ++ t1 = (t2 == NumberType.FLOAT) ? NumberType.DOUBLE : t2; ++ } ++ return t1.equals(n1, n2); ++ } + } + } + return false; + } + /** * Returns a hash code value for this range. ++ * ++ * @return a hash code value for this range. */ @Override public int hashCode() { @@@ -706,6 -729,6 +807,8 @@@ * If this range is a {@link MeasurementRange}, then the {@linkplain Unit unit of measurement} * is appended to the above string representation except for empty ranges. * ++ * @return an unlocalized string representation of this range. ++ * * @see RangeFormat * @see <a href="https://en.wikipedia.org/wiki/ISO_31-11";>Wikipedia: ISO 31-11</a> */ diff --cc endorsed/src/org.apache.sis.util/main/org/apache/sis/util/ComparisonMode.java index 793a65c9a8,793a65c9a8..221f0ba4ad --- a/endorsed/src/org.apache.sis.util/main/org/apache/sis/util/ComparisonMode.java +++ b/endorsed/src/org.apache.sis.util/main/org/apache/sis/util/ComparisonMode.java @@@ -54,15 -54,15 +54,15 @@@ import org.opengis.referencing.operatio */ public enum ComparisonMode { /** -- * All attributes of the compared objects shall be strictly equal. This comparison mode ++ * All attributes of the compared objects shall be strictly equal. This criterion * is equivalent to the {@link Object#equals(Object)} method, and must be compliant with -- * the contract documented in that method. In particular, this comparison mode shall be -- * consistent with {@link Object#hashCode()} and be symmetric ({@code A.equals(B)} implies -- * {@code B.equals(A)}). ++ * the contract documented in that method. In particular, the comparison algorithm shall ++ * be consistent with {@link Object#hashCode()} and be symmetric ({@code A.equals(B)} ++ * implies {@code B.equals(A)}). * * <h4>Implementation note</h4> -- * In the <abbr>SIS</abbr> implementations, this comparison mode usually have the following -- * characteristics (not always, this is only typical): ++ * In the <abbr>SIS</abbr> implementations, the comparison algorithm for this criterion ++ * usually has the following characteristics: * * <ul> * <li>The objects being compared need to be the same implementation class.</li> @@@ -96,9 -96,9 +96,9 @@@ BY_CONTRACT, /** -- * Only the attributes relevant to the object functionality are compared. Attributes that -- * are only informative can be ignored. This comparison mode is typically less strict than -- * {@link #BY_CONTRACT}. ++ * Only the attributes relevant to the object functionality are compared. ++ * Attributes that are only informative can be ignored. ++ * This criterion is typically less strict than {@link #BY_CONTRACT}. * * <h4>Application to coordinate reference systems</h4> * If the objects being compared are {@link CoordinateReferenceSystem} instances, @@@ -189,6 -189,6 +189,9 @@@ * <b>Example:</b> two geographic <abbr>CRS</abbr>s with the same attributes but with * (<var>latitude</var>, <var>longitude</var>) axes in one case and * (<var>longitude</var>, <var>latitude</var>) axes in the other case.</li> ++ * <li>Two ranges of integer values containing the same set of numbers, ++ * but specified with different bound inclusiveness. ++ * <b>Example:</b> {@code (0 … 256)} and {@code [1 … 255]}.</li> * </ul> * * @since 0.7 diff --cc endorsed/src/org.apache.sis.util/main/org/apache/sis/util/Utilities.java index 4587b71572,4cdf227ea3..b236c50b22 --- a/endorsed/src/org.apache.sis.util/main/org/apache/sis/util/Utilities.java +++ b/endorsed/src/org.apache.sis.util/main/org/apache/sis/util/Utilities.java @@@ -113,13 -114,13 +113,16 @@@ public final class Utilities } /** -- * Convenience method for testing two objects for equality using the given level of strictness. -- * If at least one of the given objects implement the {@link LenientComparable} interface, then -- * the comparison is performed using the {@link LenientComparable#equals(Object, ComparisonMode)} -- * method. Otherwise this method performs the same work as the -- * {@link Objects#deepEquals(Object, Object)} convenience method. ++ * Tests two objects for equality using the given level of strictness. ++ * This method applies the following rules, in order: * -- * <p>If both arguments are arrays or collections, then the elements are compared recursively.</p> ++ * <ul> ++ * <li>If at least one of the given objects implement the {@link LenientComparable} interface, then the ++ * comparison is delegated to the {@link LenientComparable#equals(Object, ComparisonMode)} method.</li> ++ * <li>Otherwise, if both arguments are {@code Map} entries, {@link Map}s, {@link Collection}s or arrays, ++ * then this method invoke {@code deepEquals(…)} for the elements of above-cited containers.</li> ++ * <li>Otherwise, this method delegates to {@link Objects#deepEquals(Object, Object)}.</li> ++ * </ul> * * @param object1 the first object to compare, or {@code null}. * @param object2 the second object to compare, or {@code null}. diff --cc endorsed/src/org.apache.sis.util/main/org/apache/sis/util/internal/shared/Numerics.java index a9de7d6262,bd7b23c21b..b015ade95b --- a/endorsed/src/org.apache.sis.util/main/org/apache/sis/util/internal/shared/Numerics.java +++ b/endorsed/src/org.apache.sis.util/main/org/apache/sis/util/internal/shared/Numerics.java @@@ -65,7 -69,7 +65,7 @@@ public final class Numerics * matrices as equal. This value has been determined empirically in order to allow * {@code org.apache.sis.referencing.operation.transform.ConcatenatedTransform} to * detect the cases where two {@link org.apache.sis.referencing.operation.transform.LinearTransform} -- * are equal for practical purpose. This threshold can be used as below:</p> ++ * are equal for practical purposes. This threshold can be used as below:</p> * * {@snippet lang="java" : * Matrix m1 = ...; @@@ -77,7 -81,7 +77,7 @@@ * * By extension, the same threshold value is used for comparing other floating point values. * -- * <p>The current value is set to the smallest power of 10 which allow the ++ * <p>The current value is set to the smallest power of 10 which allows the * {@code org.apache.sis.test.integration.ConsistencyTest} to pass.</p> * * @see org.apache.sis.referencing.internal.shared.Formulas#LINEAR_TOLERANCE @@@ -385,6 -374,6 +385,7 @@@ /** * Returns {@code true} if the given values are approximately equal, up to the given comparison threshold. ++ * NaN values are considered equal to all other NaN values, even if the threshold is also NaN. * * @param v1 the first value to compare. * @param v2 the second value to compare. @@@ -411,7 -400,11 +412,13 @@@ public static boolean epsilonEqual(final double v1, final double v2, final ComparisonMode mode) { if (mode.isApproximate()) { final double mg = max(abs(v1), abs(v2)); + /* - * If one of the numbers to compare is not finite, it is not possible to compare them using an epsilon. - * In such cases, we must fall back to the standard equal to check if their bit representation is the same. ++ * Infinite magnitude gives infinite tolerance threshold, which causes all finite numbers ++ * to be considered equal to the infinite number. For preventing these false positives, ++ * we must avoid the use of `epsilonEqual(…)` with infinite numbers. Note that there is ++ * no need to check for NaN values because `epsilonEqual(…)` is already NaN-safe. + */ - if (Double.isFinite(mg)) { + if (mg != Double.POSITIVE_INFINITY) { return epsilonEqual(v1, v2, COMPARISON_THRESHOLD * mg); } } diff --cc endorsed/src/org.apache.sis.util/test/org/apache/sis/measure/MeasurementRangeTest.java index baf35e864a,baf35e864a..72a720f789 --- a/endorsed/src/org.apache.sis.util/test/org/apache/sis/measure/MeasurementRangeTest.java +++ b/endorsed/src/org.apache.sis.util/test/org/apache/sis/measure/MeasurementRangeTest.java @@@ -30,6 -30,6 +30,7 @@@ import static org.apache.sis.test.Asser * * @author Martin Desruisseaux (IRD) */ ++@SuppressWarnings("exports") public final class MeasurementRangeTest extends TestCase { /** * Creates a new test case. diff --cc endorsed/src/org.apache.sis.util/test/org/apache/sis/measure/RangeTest.java index d5d6a7e9b0,d5d6a7e9b0..dc9e3185a0 --- a/endorsed/src/org.apache.sis.util/test/org/apache/sis/measure/RangeTest.java +++ b/endorsed/src/org.apache.sis.util/test/org/apache/sis/measure/RangeTest.java @@@ -18,6 -18,6 +18,7 @@@ package org.apache.sis.measure import java.util.Locale; import java.time.Instant; ++import org.apache.sis.util.ComparisonMode; // Test dependencies import org.junit.jupiter.api.Test; @@@ -33,6 -33,6 +34,7 @@@ import org.apache.sis.test.TestCase * @author Joe White * @author Martin Desruisseaux (Geomatys) */ ++@SuppressWarnings("exports") public final class RangeTest extends TestCase { /** * Creates a new test case. @@@ -389,6 -389,6 +391,28 @@@ assertTrue(range5.equals(range6)); } ++ /** ++ * Tests the {@link Range#equals(Object, ComparisonMode)} method. ++ */ ++ @Test ++ public void testLenientEquality() { ++ final Range<Integer> inclusive = new Range<>(Integer.class, 10, true, 20, true); ++ final Range<Integer> exclusive = new Range<>(Integer.class, 10, true, 21, false); ++ final Range<Float> asFloats = new Range<>( Float.class, 10f, true, 20f, true); ++ final Range<Integer> different = new Range<>(Integer.class, 10, true, 22, false); ++ assertTrue (inclusive.equals(exclusive, ComparisonMode.ALLOW_VARIANT)); ++ assertTrue (exclusive.equals(inclusive, ComparisonMode.ALLOW_VARIANT)); ++ assertTrue (exclusive.equals(asFloats, ComparisonMode.ALLOW_VARIANT)); ++ assertFalse(inclusive.equals(different, ComparisonMode.ALLOW_VARIANT)); ++ assertFalse(exclusive.equals(different, ComparisonMode.ALLOW_VARIANT)); ++ assertFalse(inclusive.equals(exclusive, ComparisonMode.APPROXIMATE)); ++ assertFalse(exclusive.equals(asFloats, ComparisonMode.APPROXIMATE)); ++ assertTrue (inclusive.equals(asFloats, ComparisonMode.APPROXIMATE)); ++ assertFalse(inclusive.equals(asFloats, ComparisonMode.BY_CONTRACT)); ++ assertTrue (inclusive.equals(asFloats, ComparisonMode.IGNORE_METADATA)); ++ assertFalse(exclusive.equals(asFloats, ComparisonMode.IGNORE_METADATA)); ++ } ++ /** * Tests serialization. */
