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 5915cf55f76e409a5fd88453df6b2ffc61ddfd86 Author: Martin Desruisseaux <[email protected]> AuthorDate: Fri Jul 17 12:43:14 2026 +0200 chore(Storage): add a test to investigate data discovery in NetCDF files generated by GDAL using Transverse mercator projection. Fix the cause, which was a mismatched number of dimensions between the data cube and the matrix in the `GeoTransform` attribute. Fix the CRS definitions, which was using wrong names for some elements such as the base CRS of a projected CRS. Co-authored-by: Alexis Manin <[email protected]> --- .../sis/coverage/grid/GridDerivationTest.java | 2 +- .../apache/sis/coverage/grid/GridGeometryTest.java | 9 +- .../test/org/apache/sis/feature/Assertions.java | 13 + .../main/org/apache/sis/referencing/CRS.java | 19 +- .../operation/transform/CopyTransform.java | 4 +- .../operation/transform/MathTransforms.java | 33 +- .../apache/sis/storage/netcdf/base/CRSBuilder.java | 8 +- .../apache/sis/storage/netcdf/base/Convention.java | 16 +- .../org/apache/sis/storage/netcdf/base/Grid.java | 4 +- .../sis/storage/netcdf/base/GridMapping.java | 601 ++++++++++++--------- .../sis/storage/netcdf/classic/ChannelDecoder.java | 2 +- .../sis/storage/netcdf/internal/Resources.java | 5 + .../storage/netcdf/internal/Resources.properties | 1 + .../netcdf/internal/Resources_fr.properties | 1 + .../sis/storage/netcdf/MetadataReaderTest.java | 6 +- .../storage/netcdf/NetcdfStoreProviderTest.java | 1 + .../apache/sis/storage/netcdf/NetcdfStoreTest.java | 80 ++- .../transverse-mercator-wrong-geotransform.nc | Bin 0 -> 2676 bytes .../org/apache/sis/storage/DataStoreTestCase.java | 72 +++ .../test/org/apache/sis/test/LoggingWatcher.java | 34 +- 20 files changed, 591 insertions(+), 320 deletions(-) diff --git a/endorsed/src/org.apache.sis.feature/test/org/apache/sis/coverage/grid/GridDerivationTest.java b/endorsed/src/org.apache.sis.feature/test/org/apache/sis/coverage/grid/GridDerivationTest.java index 12f7dc725f..70269fab5d 100644 --- a/endorsed/src/org.apache.sis.feature/test/org/apache/sis/coverage/grid/GridDerivationTest.java +++ b/endorsed/src/org.apache.sis.feature/test/org/apache/sis/coverage/grid/GridDerivationTest.java @@ -41,7 +41,6 @@ import org.apache.sis.referencing.operation.matrix.Matrices; import org.apache.sis.referencing.operation.matrix.Matrix3; import org.apache.sis.referencing.operation.matrix.Matrix4; import org.apache.sis.referencing.operation.transform.MathTransforms; -import static org.apache.sis.coverage.grid.GridGeometryTest.assertExtentEquals; // Test dependencies import org.junit.jupiter.api.Test; @@ -50,6 +49,7 @@ import org.apache.sis.test.TestCase; import org.apache.sis.referencing.crs.HardCodedCRS; import org.apache.sis.referencing.operation.HardCodedConversions; import static org.apache.sis.referencing.Assertions.assertEnvelopeEquals; +import static org.apache.sis.feature.Assertions.assertExtentEquals; import static org.apache.sis.feature.Assertions.assertGridToCenterEquals; import static org.apache.sis.feature.Assertions.assertGridToCornerEquals; diff --git a/endorsed/src/org.apache.sis.feature/test/org/apache/sis/coverage/grid/GridGeometryTest.java b/endorsed/src/org.apache.sis.feature/test/org/apache/sis/coverage/grid/GridGeometryTest.java index 028e0f15e0..d2faf4e56a 100644 --- a/endorsed/src/org.apache.sis.feature/test/org/apache/sis/coverage/grid/GridGeometryTest.java +++ b/endorsed/src/org.apache.sis.feature/test/org/apache/sis/coverage/grid/GridGeometryTest.java @@ -64,6 +64,7 @@ import static org.apache.sis.feature.Assertions.assertGridToCornerEquals; // Specific to the geoapi-3.1 and geoapi-4.0 branches: import org.opengis.referencing.ObjectDomain; import static org.opengis.test.Assertions.assertAxisDirectionsEqual; +import static org.apache.sis.feature.Assertions.assertExtentEquals; /** @@ -80,14 +81,6 @@ public final class GridGeometryTest extends TestCase { public GridGeometryTest() { } - /** - * Verifies grid extent coordinates. - */ - static void assertExtentEquals(final long[] low, final long[] high, final GridExtent extent) { - assertArrayEquals(low, extent.getLow() .getCoordinateValues(), "extent.low"); - assertArrayEquals(high, extent.getHigh().getCoordinateValues(), "extent.high"); - } - /** * Verifies the shift between the two {@code gridToCRS} transforms. * This method should be invoked when the transforms are linear. diff --git a/endorsed/src/org.apache.sis.feature/test/org/apache/sis/feature/Assertions.java b/endorsed/src/org.apache.sis.feature/test/org/apache/sis/feature/Assertions.java index 52d73444f0..63f0721364 100644 --- a/endorsed/src/org.apache.sis.feature/test/org/apache/sis/feature/Assertions.java +++ b/endorsed/src/org.apache.sis.feature/test/org/apache/sis/feature/Assertions.java @@ -22,6 +22,7 @@ import java.awt.image.Raster; import java.awt.image.RenderedImage; import org.opengis.referencing.operation.Matrix; import org.apache.sis.image.PixelIterator; +import org.apache.sis.coverage.grid.GridExtent; import org.apache.sis.coverage.grid.GridGeometry; import org.apache.sis.coverage.grid.PixelInCell; @@ -45,6 +46,18 @@ public final class Assertions { private Assertions() { } + /** + * Verifies grid extent coordinates. + * + * @param low the expected low coordinate values. + * @param high the expected high coordinate values. + * @param actual the extent to validate. + */ + public static void assertExtentEquals(final long[] low, final long[] high, final GridExtent actual) { + assertArrayEquals(low, actual.getLow() .getCoordinateValues(), "low"); + assertArrayEquals(high, actual.getHigh().getCoordinateValues(), "high"); + } + /** * Verifies the matrix of the transform of the grid geometry to pixel corner. * diff --git a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/CRS.java b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/CRS.java index fa2e0ab237..320141476b 100644 --- a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/CRS.java +++ b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/CRS.java @@ -1111,12 +1111,15 @@ public final class CRS { /** * Returns the geodetic reference frame used by the given coordinate reference system. - * If the given <abbr>CRS</abbr> is an instance of {@link GeodeticCRS}, then this method returns the - * <abbr>CRS</abbr>'s datum. Otherwise, if the given <abbr>CRS</abbr> is an instance of {@link CompoundCRS}, - * then this method searches for the first geodetic component. Otherwise, this method returns an empty value. + * If the given <abbr>CRS</abbr> is a {@link GeodeticCRS}, then this method returns the <abbr>CRS</abbr>'s datum. + * If the given <abbr>CRS</abbr> is a {@link DerivedCRS} and if its base <abbr>CRS</abbr> is a {@link GeodeticCRS}, + * then this method returns the base <abbr>CRS</abbr>'s datum. + * Otherwise, if the given <abbr>CRS</abbr> is a {@link CompoundCRS}, + * then this method performs the above checks on each component. + * Otherwise, this method returns an empty value. * * @param crs the coordinate reference system for which to get the geodetic reference frame, or {@code null}. - * @return the geodetic reference frame, or an empty value if none. + * @return the geodetic reference frame of the given <abbr>CRS</abbr>, or an empty value if none. * * @see DatumOrEnsemble#getEllipsoid(CoordinateReferenceSystem) * @see DatumOrEnsemble#getPrimeMeridian(CoordinateReferenceSystem) @@ -1124,10 +1127,14 @@ public final class CRS { * * @since 1.6 */ - public static Optional<GeodeticDatum> getGeodeticReferenceFrame(final CoordinateReferenceSystem crs) { + public static Optional<GeodeticDatum> getGeodeticReferenceFrame(CoordinateReferenceSystem crs) { + if (crs instanceof DerivedCRS) { + crs = ((DerivedCRS) crs).getBaseCRS(); + } if (crs instanceof GeodeticCRS) { return Optional.ofNullable(DatumOrEnsemble.asDatum((GeodeticCRS) crs)); - } else if (crs instanceof CompoundCRS) { + } + if (crs instanceof CompoundCRS) { for (CoordinateReferenceSystem component : ((CompoundCRS) crs).getComponents()) { final Optional<GeodeticDatum> datum = getGeodeticReferenceFrame(component); if (datum.isPresent()) { diff --git a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/CopyTransform.java b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/CopyTransform.java index 4202647372..cc07585453 100644 --- a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/CopyTransform.java +++ b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/CopyTransform.java @@ -49,7 +49,7 @@ final class CopyTransform extends AbstractLinearTransform { private final int srcDim; /** - * The indices of coordinates to copy in the source array. + * The indices of source coordinates to copy in the source array. * The length of this array is the target dimension. */ private final int[] indices; @@ -59,7 +59,7 @@ final class CopyTransform extends AbstractLinearTransform { * * @param srcDim the dimension of source coordinates. * Must be greater than the highest value in {@code indices}. - * @param indices the indices of coordinates to copy in the target array. + * @param indices the indices of source coordinates to copy in the target array. * The length of this array is the number of target dimensions. */ CopyTransform(final int srcDim, final int[] indices) { diff --git a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/MathTransforms.java b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/MathTransforms.java index ae4e96a9b0..e15829a938 100644 --- a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/MathTransforms.java +++ b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/MathTransforms.java @@ -61,7 +61,7 @@ import org.opengis.coordinate.MismatchedDimensionException; * GeoAPI factory interfaces instead. * * @author Martin Desruisseaux (Geomatys) - * @version 1.5 + * @version 1.7 * * @see MathTransformFactory * @@ -75,7 +75,7 @@ public final class MathTransforms { } /** - * Returns an identity transform of the specified dimension. + * Returns an identity transform of the specified number of dimensions. * * <p>Special cases:</p> * <ul> @@ -91,6 +91,35 @@ public final class MathTransforms { return IdentityTransform.create(dimension); } + /** + * Returns a transform of the specified number of dimensions which swaps the two first axes. + * The most typical use case is for switching between (<var>latitude</var>, <var>longitude</var>) + * and (<var>longitude</var>, <var>latitude</var>) axis order of geographic <abbr>CRS</abbr>. + * + * <h4>Warning: use in last resort only</h4> + * While switching the axis order of geographic <abbr>CRS</abbr> seems a very frequent problem, + * the need to invoke this method should be very rare. Apache <abbr>SIS</abbr> already handles + * axis order changes when deriving Coordinate Operations between Coordinate Reference Systems (<abbr>CRS</abbr>). + * If a user's code needs to change axis order manually, this is often the symptom of a design problem such as + * data without {@link org.opengis.referencing.crs.CoordinateReferenceSystem} or with an incorrect <abbr>CRS</abbr>, + * or computation that does not use another <abbr>CRS</abbr> for describing their desired coordinate system. + * + * @param dimension number of dimensions of the transform to be returned. + * @return a transform of the specified dimension which swaps the two first axes. + * + * @since 1.7 + */ + public static LinearTransform swapTwoFirstAxes(final int dimension) { + ArgumentChecks.ensurePositive("dimension", dimension); + if (dimension < 2) { + return IdentityTransform.create(dimension); + } + final int[] indices = ArraysExt.range(0, dimension); + indices[0] = 1; + indices[1] = 0; + return new CopyTransform(dimension, indices); + } + /** * Creates an affine transform which applies the same translation for all dimensions. * For each dimension, input values <var>x</var> are converted into output values <var>y</var> diff --git a/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/base/CRSBuilder.java b/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/base/CRSBuilder.java index 55cad0d6f9..e3eee51839 100644 --- a/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/base/CRSBuilder.java +++ b/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/base/CRSBuilder.java @@ -294,7 +294,7 @@ abstract class CRSBuilder<D extends Datum, CS extends CoordinateSystem> { * We should have at most one builder of each class. But if we nevertheless have more, * add to the most recently used builder. If there is no builder, create a new one. */ - for (int i=components.size(); --i >= 0;) { + for (int i = components.size(); --i >= 0;) { final CRSBuilder<?,?> builder = components.get(i); if (addTo.isInstance(builder) || i == alternative) { builder.add(axis); @@ -418,7 +418,7 @@ previous: for (int i = components.size(); --i >= 0;) { * Using a predefined CS allows us to get more complete definitions (minimum and maximum values, etc.). */ if (coordinateSystem != null) { - for (int i=dimension; --i >= 0;) { + for (int i = dimension; --i >= 0;) { final Axis expected = axes[i]; if (expected == null || !expected.isSameUnitAndDirection(coordinateSystem.getAxis(i))) { coordinateSystem = null; @@ -441,7 +441,7 @@ previous: for (int i = components.size(); --i >= 0;) { */ if (grid) { final CoordinateSystem cs = referenceSystem.getCoordinateSystem(); - for (int i=cs.getDimension(); --i >= 0;) { + for (int i = cs.getDimension(); --i >= 0;) { final CoordinateSystemAxis axis = cs.getAxis(i); if (axis.getRangeMeaning() == RangeMeaning.WRAPAROUND) { final NumberRange<?> range = axes[i].read().range(); // Vector is cached. @@ -473,7 +473,7 @@ previous: for (int i = components.size(); --i >= 0;) { final Map<String,?> properties; if (coordinateSystem == null) { // Fallback if the coordinate system is not predefined. - final StringJoiner joiner = new StringJoiner(" "); + final var joiner = new StringJoiner(" "); final CSFactory csFactory = decoder.getCSFactory(); final var iso = new CoordinateSystemAxis[dimension]; for (int i=0; i<iso.length; i++) { diff --git a/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/base/Convention.java b/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/base/Convention.java index 60798cdb08..25b0f7b7d1 100644 --- a/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/base/Convention.java +++ b/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/base/Convention.java @@ -481,12 +481,12 @@ public class Convention { * * @see <a href="http://cfconventions.org/cf-conventions/cf-conventions.html#grid-mappings-and-projections";>CF-conventions</a> */ - public Map<String,Object> projection(final Node node) { + public Map<String, Object> projection(final Node node) { final String method = node.getAttributeAsString(CF.GRID_MAPPING_NAME); if (method == null) { return null; } - final var definition = new LinkedHashMap<String,Object>(); + final var definition = new LinkedHashMap<String, Object>(); definition.put(CF.GRID_MAPPING_NAME, method); for (final String name : node.getAttributeNames()) try { final String nameLC = name.toLowerCase(Decoder.DATA_LOCALE); @@ -560,9 +560,11 @@ public class Convention { } /** - * Returns an identification of default geodetic components to use if no corresponding information is found in the - * netCDF file. The default implementation returns <q>Unknown datum based upon the GRS 1980 ellipsoid</q>. - * Note that the GRS 1980 ellipsoid is close to WGS 84 ellipsoid. + * Returns an identification of default geodetic components to use if no corresponding information is found + * in the netCDF file. The prime meridian shall be Greenwich or the international meridian. + * + * <p>The default implementation returns <q>Unknown datum based upon the GRS 1980 ellipsoid</q>. + * Note that the <abbr>GRS</abbr> 1980 ellipsoid is close to <abbr>WGS</abbr> 84 ellipsoid.</p> * * <h4>Maintenance note</h4> * If this default is changed, search also for "GRS 1980" strings in {@link CRSBuilder} class. @@ -710,7 +712,7 @@ public class Convention { * @param data the variable for which to get no-data values. * @return no-data values with bitmask of their roles or textual descriptions. */ - public Map<Number,Object> nodataValues(final Variable data) { + public Map<Number, Object> nodataValues(final Variable data) { final var pads = new LinkedHashMap<Number,Object>(); for (int i=0; i < NODATA_ATTRIBUTES.length; i++) { final String name = NODATA_ATTRIBUTES[i]; @@ -788,7 +790,7 @@ public class Convention { * @param data the variable for which to get the colors. * @return colors to use for each category, or {@code null} for the default. */ - public Function<Category,Color[]> getColors(final Variable data) { + public Function<Category, Color[]> getColors(final Variable data) { return null; } } diff --git a/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/base/Grid.java b/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/base/Grid.java index d5163847a4..2cbeae8384 100644 --- a/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/base/Grid.java +++ b/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/base/Grid.java @@ -419,12 +419,12 @@ public abstract class Grid extends NamedElement { */ final int[] gridDimensionIndices = new int[nonLinears.size()]; Arrays.fill(gridDimensionIndices, -1); - for (int i=0; i<gridDimensionIndices.length; i++) { + for (int i=0; i < gridDimensionIndices.length; i++) { final int tgtDim = deferred[i]; final Axis axis = axes[tgtDim]; findFree: for (int srcDim : axis.gridDimensionIndices) { // In preference order (will take only one). srcDim = lastSrcDim - srcDim; // Convert netCDF order to "natural" order. - for (int j=affine.getNumRow(); --j>=0;) { + for (int j = affine.getNumRow(); --j>=0;) { if (affine.getElement(j, srcDim) != 0) { continue findFree; } diff --git a/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/base/GridMapping.java b/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/base/GridMapping.java index 253642a975..22a3b3a02e 100644 --- a/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/base/GridMapping.java +++ b/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/base/GridMapping.java @@ -40,6 +40,7 @@ import org.opengis.parameter.ParameterValueGroup; import org.opengis.parameter.ParameterNotFoundException; import org.opengis.referencing.IdentifiedObject; import org.opengis.referencing.cs.CoordinateSystem; +import org.opengis.referencing.crs.CRSFactory; import org.opengis.referencing.crs.ProjectedCRS; import org.opengis.referencing.crs.GeographicCRS; import org.opengis.referencing.crs.CoordinateReferenceSystem; @@ -47,6 +48,7 @@ import org.opengis.referencing.operation.TransformException; import org.opengis.referencing.operation.OperationMethod; import org.opengis.referencing.operation.MathTransform; import org.opengis.referencing.operation.Conversion; +import org.opengis.referencing.operation.Matrix; import org.opengis.referencing.datum.DatumFactory; import org.opengis.referencing.datum.GeodeticDatum; import org.opengis.referencing.datum.PrimeMeridian; @@ -58,9 +60,7 @@ import org.apache.sis.referencing.crs.AbstractCRS; import org.apache.sis.referencing.cs.AxesConvention; import org.apache.sis.referencing.cs.CoordinateSystems; import org.apache.sis.referencing.datum.DatumOrEnsemble; -import org.apache.sis.referencing.datum.BursaWolfParameters; import org.apache.sis.referencing.datum.DefaultGeodeticDatum; -import org.apache.sis.referencing.operation.matrix.Matrix3; import org.apache.sis.referencing.operation.matrix.Matrices; import org.apache.sis.referencing.operation.matrix.MatrixSIS; import org.apache.sis.referencing.operation.transform.MathTransforms; @@ -85,10 +85,12 @@ import org.apache.sis.util.resources.Errors; import org.apache.sis.util.resources.IndexedResourceBundle; import org.apache.sis.io.wkt.WKTFormat; import org.apache.sis.io.wkt.Warnings; +import org.apache.sis.util.Exceptions; import org.apache.sis.math.NumberType; import org.apache.sis.measure.Units; // Specific to the geoapi-3.1 and geoapi-4.0 branches: +import org.opengis.referencing.crs.DerivedCRS; import org.opengis.referencing.datum.DatumEnsemble; @@ -96,8 +98,11 @@ import org.opengis.referencing.datum.DatumEnsemble; * Helper object for creating a {@link GridGeometry} instance defined by attributes on a variable. * Those attributes are defined by <abbr>CF</abbr>-conventions, but some other non-<abbr>CF</abbr> * attributes are also recognized (e.g. <abbr>GDAL</abbr> and <abbr>ESRI</abbr> conventions). - * This class uses a different approach than {@link CRSBuilder}, + * + * <p>This class uses a different approach than {@link CRSBuilder}, * which creates Coordinate Reference Systems by inspecting coordinate system axes. + * The two approaches are complementary, as this {@code GridMapping} class usually creates a + * two-dimensional <abbr>CRS</abbr>. The other dimensions need to be taken from {@link CRSBuilder}.</p> * * @author Martin Desruisseaux (Geomatys) * @@ -168,6 +173,12 @@ final class GridMapping { */ private MathTransform gridToCRS; + /** + * The pixel in cell convention of the method that decoded the {@link #gridToCRS} field. + * This is null if the pixel in cell convention was not inferred. + */ + private PixelInCell anchorFromConvention; + /** * Whether the {@link #crs} was defined by a WKT string. */ @@ -189,9 +200,10 @@ final class GridMapping { * only one set of grid mapping attributes for the whole file. * * @param variable the variable for which to create a grid geometry. + * @return grid geometry information, or {@code null} if not found. */ static GridMapping forVariable(final Variable variable) { - final Map<String,GridMapping> gridMapping = variable.decoder.gridMapping; + final Map<String, GridMapping> gridMapping = variable.decoder.gridMapping; for (final String name : variable.decoder.convention().nameOfMappingNode(variable)) { GridMapping gm = gridMapping.get(name); if (gm != null) { @@ -204,7 +216,7 @@ final class GridMapping { if (!gridMapping.containsKey(name)) { final Node mapping = variable.decoder.findNode(name); if (mapping != null) { - gm = parse(mapping); + gm = tryAllConventions(mapping); } gridMapping.put(name, gm); // Store even if null. if (gm != null) { @@ -219,7 +231,7 @@ final class GridMapping { final String name = variable.getName(); GridMapping gm = gridMapping.get(name); if (gm == null && !gridMapping.containsKey(name)) { - gm = parse(variable); + gm = tryAllConventions(variable); gridMapping.put(name, gm); // Store even if null. } return gm; @@ -228,195 +240,173 @@ final class GridMapping { /** * Parses the map projection parameters defined as attribute associated to the given variable. * This method tries to parse <abbr>CF</abbr>-compliant attributes, potentially mixed with - * non-standard extensions (for example <abbr>GDAL</abbr>). + * non-standard extensions (for example <abbr>GDAL</abbr> and <abbr>ESRI</abbr>). + * + * @param variable the variable from which to get the map projection parameters. + * @return grid geometry information, or {@code null} if not found or if an error occurred. */ - private static GridMapping parse(final Node mapping) { - final var gm = new GridMapping(mapping); + @SuppressWarnings("UseSpecificCatch") + private static GridMapping tryAllConventions(final Node mapping) { + var gm = new GridMapping(mapping); // Tries CF-convention first, and if it doesn't work, try GDAL convention. - return (gm.parseProjectionParameters() || gm.parseGeoTransform() || gm.parseESRI()) ? gm : null; + int i = 0; + boolean stop = false; + Exception warning = null; + do { + try { + switch (i++) { + case 0: stop = gm.parseProjectionParameters(); break; + case 1: stop = gm.parseGDAL(); break; + case 2: stop = gm.parseESRI(); break; + default: stop = true; gm = null; break; + } + } catch (Exception e) { // Checked exceptions | ClassCastException | IllegalArgumentException and more. + e = Exceptions.unwrap(e); + if (warning == null) warning = e; + else warning.addSuppressed(e); + } + } while (!stop); + if (warning != null) { + cannotCreateGridOrCRS(mapping, warning, warning instanceof TransformException); + } + return gm; } /** * Sets the <abbr>CRS</abbr> and "grid to <abbr>CRS</abbr>" from the <abbr>CF</abbr> conventions. * If this method does not find the expected attributes, then it does nothing. * + * <p>The <abbr>CRS</abbr> may also be specified in <abbr>WKT</abbr> form in attributes which are themselves + * specified by different conventions (<abbr>CF</abbr>, <abbr>GDAL</abbr>, <abbr>ESRI</abbr>). + * If the <abbr>CRS</abbr> is specified both by <abbr>WKT</abbr> and by attributes on a "grid mapping" variable, + * then the grid mapping attributes have precedence as initially mandated by the <abbr>CF</abbr> conventions. + * Note: the latter rule has been relaxed by <abbr>CF</abbr> issue #222, but <abbr>SIS</abbr> implementation + * continues to follow the original specification.</p> + * + * <p>The <abbr>CRS</abbr> created by this method is two-dimensional. + * The addition of vertical or temporal axes must be done by the caller.</p> + * * @return whether this method found grid geometry attributes. + * @throws ClassCastException if an attribute value is not of the expected type. + * @throws FactoryException if an error occurred during the attempt to initialize {@link #crs}. + * @throws TransformException if an error occurred during the attempt to initialize {@link #gridToCRS}. * * @see <a href="http://cfconventions.org/cf-conventions/cf-conventions.html#grid-mappings-and-projections";>CF-conventions</a> + * @see <a href="https://github.com/cf-convention/cf-conventions/issues/222";>Allow CRS WKT to represent the <abbr>CRS</abbr> + * without requiring reader to compare with grid mapping parameters</a> */ - private boolean parseProjectionParameters() { - final Map<String, Object> definition = mapping.decoder.convention().projection(mapping); - if (definition != null) try { - // Take only one WKT for now. We will parse the other one later. - final var alreadyParsedWKT = new ArrayList<String>(2); - if (crs == null) setOrVerifyWKT(definition, CRS_WKT, alreadyParsedWKT); - if (crs == null) setOrVerifyWKT(definition, SPATIAL_REF, alreadyParsedWKT); - /* - * Fetch now numerical values that are not map projection parameters. - * This step needs to be done before to try to set parameter values. - */ - final Object greenwichLongitude = definition.remove(Convention.LONGITUDE_OF_PRIME_MERIDIAN); - /* - * Prepare the block of projection parameters. The set of legal parameter depends on the map projection. - * We assume that all numerical values are map projection parameters. Character sequences (assumed to be - * component names) are handled later. The CF-conventions use parameter names that are slightly different - * than OGC names, but Apache SIS implementations of map projections know how to handle them, including - * the redundant parameters like "inverse_flattening" and "earth_radius". - */ - final String mappingName = (String) definition.remove(CF.GRID_MAPPING_NAME); - final OperationMethod method = mapping.decoder.findOperationMethod(mappingName); - final ParameterValueGroup parameters = method.getParameters().createValue(); - for (final Iterator<Map.Entry<String,Object>> it = definition.entrySet().iterator(); it.hasNext();) { - final Map.Entry<String,Object> entry = it.next(); - final String name = entry.getKey(); - final Object value = entry.getValue(); - try { - if (value instanceof Number || value instanceof double[] || value instanceof float[]) { - it.remove(); - parameters.parameter(name).setValue(value); - } else if (value instanceof String) { - final var text = (String) value; - if (name.endsWith(Convention.NAME_SUFFIX)) { - continue; - } - switch (name) { - case CRS_WKT: - case SPATIAL_REF: continue; // Will be parsed after this loop. - case "geotransform": { // "GeoTransform" made lower-case. - if (parseGeoTransform(null, text)) { - it.remove(); - } - continue; + private boolean parseProjectionParameters() throws FactoryException, TransformException { + final Decoder decoder = mapping.decoder; + final Map<String, Object> definition = decoder.convention().projection(mapping); + if (definition == null) { + return false; + } + /* + * Search in advance for a CRS that we can use as a fallback. We need only one such CRS for now, + * the other CRS definitions will be parsed later in this method. We need this fallback in order + * to provide default names to geodetic objects in the common case where the `definition` map + * does not contain these names. + */ + final var alreadyParsedWKT = new ArrayList<String>(2); + if (crs == null) setOrVerifyWKT(alreadyParsedWKT, definition, CRS_WKT); + if (crs == null) setOrVerifyWKT(alreadyParsedWKT, definition, SPATIAL_REF); + final CoordinateReferenceSystem fromWKT = crs; + /* + * Fetch now numerical values that are not map projection parameters. + * This step needs to be done before to try to set parameter values. + */ + final Object greenwichLongitude = definition.remove(Convention.LONGITUDE_OF_PRIME_MERIDIAN); + final String mappingName = (String) definition.remove(CF.GRID_MAPPING_NAME); + /* + * Prepare the group of projection parameters. The set of legal parameter depends on the map projection. + * We assume that all numerical values are map projection parameters. Character sequences (assumed to be + * component names) are handled later. The CF-conventions use parameter names that are slightly different + * than OGC names, but Apache SIS implementations of map projections know how to handle them, including + * the redundant parameters like "inverse_flattening" and "earth_radius". + */ + final OperationMethod method = decoder.findOperationMethod(mappingName); + final ParameterValueGroup parameters = method.getParameters().createValue(); + for (final Iterator<Map.Entry<String, Object>> it = definition.entrySet().iterator(); it.hasNext();) { + final Map.Entry<String, Object> entry = it.next(); + final String name = entry.getKey(); + final Object value = entry.getValue(); + try { + if (value instanceof Number || value instanceof double[] || value instanceof float[]) { + it.remove(); + parameters.parameter(name).setValue(value); + } else if (value instanceof String) { + final var text = (String) value; + if (name.endsWith(Convention.NAME_SUFFIX)) { + continue; + } + switch (name) { + case CRS_WKT: + case SPATIAL_REF: continue; // Will be parsed after this loop. + case "geotransform": { // "GeoTransform" made lower-case. + if (parseGeoTransform(text)) { + it.remove(); } - } - /* - * In principle we should ignore non-numeric parameters. But in practice, some badly encoded - * netCDF files store parameters as strings instead of numbers. If the parameter name is - * known to the projection method, try to parse the character string. - */ - final ParameterValue<?> parameter; - try { - parameter = parameters.parameter(name); - } catch (IllegalArgumentException e) { continue; } - final Class<?> type = parameter.getDescriptor().getValueClass(); - if (NumberType.isReal(type)) { - it.remove(); - parameter.setValue(Double.parseDouble(text)); - } else if (NumberType.isReal(type.getComponentType())) { - it.remove(); - parameter.setValue(parseDoubles(text), null); - } } - } catch (IllegalArgumentException ex) { // Includes NumberFormatException. - warning(mapping, - ex, - null, // Default to `Resources` bundle. - Resources.Keys.CanNotSetProjectionParameter_5, - mapping.decoder.getFilename(), - mapping.getName(), - name, - value, - ex.getLocalizedMessage()); + /* + * In principle, we should ignore non-numeric parameters. But in practice, some badly encoded + * netCDF files store parameters as strings instead of numbers. If the parameter name is known + * to the projection method, try to parse the character string. + */ + final ParameterValue<?> parameter; + try { + parameter = parameters.parameter(name); + } catch (ParameterNotFoundException e) { + // No warning because it may be normal. + continue; + } + final Class<?> type = parameter.getDescriptor().getValueClass(); + if (NumberType.isReal(type)) { + it.remove(); + parameter.setValue(Double.parseDouble(text)); + } else if (NumberType.isReal(type.getComponentType())) { + it.remove(); + parameter.setValue(parseDoubles(text), null); + } } + } catch (IllegalArgumentException ex) { // Includes NumberFormatException. + warning(mapping, + ex, + null, // Default to `Resources` bundle. + Resources.Keys.CanNotSetProjectionParameter_5, + decoder.getFilename(), + mapping.getName(), + name, + value, + ex.getLocalizedMessage()); } - /* - * In principle, projection parameters do not include the semi-major and semi-minor axis lengths. - * But if those information are provided, then we use them for building the geodetic reference frame. - * Otherwise a default reference frame will be used. - */ - final CoordinateReferenceSystem fromWKT = crs; - final boolean geographic = (method instanceof PseudoPlateCarree); - final GeographicCRS baseCRS = createBaseCRS(mapping.decoder, fromWKT, parameters, definition, greenwichLongitude, geographic); - final MathTransform baseToCRS; - if (geographic) { - // Only swap axis order from (latitude, longitude) to (longitude, latitude). - baseToCRS = MathTransforms.linear(new Matrix3(0, 1, 0, 1, 0, 0, 0, 0, 1)); - crs = baseCRS; - } else { - // Create a projected CRS. - Supplier<Object> nameFallback = () -> (fromWKT instanceof ProjectedCRS) ? - ((ProjectedCRS) fromWKT).getConversionFromBase().getName() : mapping.getName(); - Map<String,?> properties = properties(definition, Convention.CONVERSION_NAME, nameFallback, false); - final Decoder decoder = mapping.decoder; - final Conversion conversion = decoder.getCoordinateOperationFactory() - .createDefiningConversion(properties, method, parameters); - - nameFallback = () -> (fromWKT != null) ? fromWKT.getName() : conversion.getName(); - properties = properties(definition, Convention.PROJECTED_CRS_NAME, nameFallback, true); - final ProjectedCRS projected = decoder.getCRSFactory() - .createProjectedCRS(properties, baseCRS, conversion, decoder.getStandardProjectedCS()); - - baseToCRS = projected.getConversionFromBase().getMathTransform(); - crs = projected; - } - /* - * The CF-Convention said that even if a WKT definition is provided, other attributes shall be present - * and have precedence over the WKT definition. Consequently, the purpose of WKT in netCDF files is not - * obvious (except for CompoundCRS). - */ - if (fromWKT != null) verifyCRS(fromWKT); - setOrVerifyWKT(definition, CRS_WKT, alreadyParsedWKT); - setOrVerifyWKT(definition, SPATIAL_REF, alreadyParsedWKT); - /* - * Report all projection parameters that have not been used. If the map is not rendered - * at expected location, it may be because we have ignored some important parameters. - */ - definition.remove(CF.LONG_NAME); - if (!definition.isEmpty()) { - warningInMapping(mapping, null, Resources.Keys.UnknownProjectionParameters_3, - String.join(", ", definition.keySet())); - } - /* - * Build the "grid to CRS" if present. This is not defined by CF-convention, - * but may be present in some non-CF conventions. - */ - if (gridToCRS == null) { - gridToCRS = mapping.decoder.convention().gridToCRS(mapping, baseToCRS); - // Map pixel corners by `convention().gridToCRS(…)` contract. - } else { - gridToCRS = MathTransforms.concatenate(gridToCRS, baseToCRS); - } - return true; - } catch (ClassCastException | IllegalArgumentException | FactoryException | TransformException e) { - cannotCreateGridOrCRS(mapping, e, false); } - return false; - } - - /** - * Creates the geographic CRS from axis length specified in the given map projection parameters. - * The returned CRS will always have (latitude, longitude) axes in that order and in degrees. - * - * @param decoder the decoder from which to get factories, conventions and listeners. - * @param fromWKT the CRS parsed from WKT if any. Used for fetching default object names. - * @param parameters parameters from which to get ellipsoid axis lengths. Will not be modified. - * @param definition map from which to get element names. Elements used will be removed. - * @param isMainCRS whether the returned <abbr>CRS</abbr> will be the main one. - */ - private static GeographicCRS createBaseCRS(final Decoder decoder, - final CoordinateReferenceSystem fromWKT, - final ParameterValueGroup parameters, - final Map<String,Object> definition, - final Object greenwichLongitude, - final boolean isMainCRS) - throws FactoryException - { - final DatumFactory datumFactory = decoder.getDatumFactory(); + /* + * In principle, projection parameters do not include the semi-major and semi-minor axis lengths. + * But if those information are provided, then we use them for building the geodetic reference frame. + * Otherwise, a default reference frame will be used. + */ final CommonCRS defaultDefinitions = decoder.convention().defaultHorizontalCRS(false); - boolean isSpecified = false; + final DatumFactory datumFactory = decoder.getDatumFactory(); + boolean hasBuiltSomeCustomObjects = false; /* * Prime meridian built from "longitude_of_prime_meridian". */ final PrimeMeridian meridian; if (greenwichLongitude instanceof Number) { final double longitude = ((Number) greenwichLongitude).doubleValue(); - Supplier<Object> nameFallback = () -> DatumOrEnsemble.getPrimeMeridian(fromWKT) - .<Object>map(PrimeMeridian::getName).orElse(longitude == 0 ? "Greenwich" : null); - Map<String,?> properties = properties(definition, Convention.PRIME_MERIDIAN_NAME, nameFallback, false); + final Map<String,?> properties = properties(definition, false, Convention.PRIME_MERIDIAN_NAME, () -> { + // Fallback if `definition` does not contain a name for the prime meridian. + PrimeMeridian template = DatumOrEnsemble.getPrimeMeridian(fromWKT).orElse(null); + if (template == null) { + if (longitude != 0) return null; + template = defaultDefinitions.primeMeridian(); + } + return template.getName(); + }); meridian = datumFactory.createPrimeMeridian(properties, longitude, Units.DEGREE); - isSpecified = true; + hasBuiltSomeCustomObjects = true; } else { meridian = defaultDefinitions.primeMeridian(); } @@ -430,10 +420,16 @@ final class GridMapping { try { final ParameterValue<?> p = parameters.parameter(Constants.SEMI_MAJOR); final Unit<Length> axisUnit = p.getUnit().asType(Length.class); - final double semiMajor = p.doubleValue(); + final double semiMajor = p.doubleValue(); + boolean isIvfDefinitive; + try { + isIvfDefinitive = parameters.parameter(Constants.IS_IVF_DEFINITIVE).booleanValue(); + } catch (ParameterNotFoundException e) { + // Ignore - may be normal if the map projection is not an Apache SIS implementation. + isIvfDefinitive = false; + } final double secondDefiningParameter; final boolean isSphere; - final boolean isIvfDefinitive = parameters.parameter(Constants.IS_IVF_DEFINITIVE).booleanValue(); if (isIvfDefinitive) { secondDefiningParameter = parameters.parameter(Constants.INVERSE_FLATTENING).doubleValue(); isSphere = (secondDefiningParameter == 0) || Double.isInfinite(secondDefiningParameter); @@ -441,7 +437,8 @@ final class GridMapping { secondDefiningParameter = parameters.parameter(Constants.SEMI_MINOR).doubleValue(axisUnit); isSphere = secondDefiningParameter == semiMajor; } - final Supplier<Object> nameFallback = () -> { // Default ellipsoid name if not specified. + final Map<String,?> properties = properties(definition, false, Convention.ELLIPSOID_NAME, () -> { + // Fallback if `definition` does not contain a name for the ellipsoid. return DatumOrEnsemble.getEllipsoid(fromWKT).<Object>map(Ellipsoid::getName).orElseGet(() -> { final Locale locale = decoder.getLocale(); final String name = Vocabulary.forLocale(locale).getString(isSphere ? Vocabulary.Keys.Sphere : Vocabulary.Keys.Ellipsoid); @@ -452,31 +449,33 @@ final class GridMapping { new FieldPosition(0)) .append(" km").toString(); }); - }; - final Map<String,?> properties = properties(definition, Convention.ELLIPSOID_NAME, nameFallback, false); + }); if (isIvfDefinitive) { ellipsoid = datumFactory.createFlattenedSphere(properties, semiMajor, secondDefiningParameter, axisUnit); } else { ellipsoid = datumFactory.createEllipsoid(properties, semiMajor, secondDefiningParameter, axisUnit); } - isSpecified = true; - } catch (ParameterNotFoundException | IllegalStateException e) { - // Ignore - may be normal if the map projection is not an Apache SIS implementation. + hasBuiltSomeCustomObjects = true; + } catch (IllegalStateException e) { + warningInMapping(mapping, e, Resources.Keys.MissingEllipsoid_3, e.getLocalizedMessage()); ellipsoid = defaultDefinitions.ellipsoid(); } /* * Geodetic reference frame built from "towgs84" and above properties. + * The class of the "towgs84" entry will be verified by the datum constuctor. */ - final Object bursaWolf = definition.remove(Convention.TOWGS84); final GeodeticDatum datum; DatumEnsemble<GeodeticDatum> ensemble = null; - if (isSpecified || bursaWolf != null) { - Supplier<Object> nameFallback = () -> CRS.getGeodeticReferenceFrame(fromWKT).map(GeodeticDatum::getName).orElse(null); - Map<String,Object> properties = properties(definition, Convention.GEODETIC_DATUM_NAME, nameFallback, false); - if (bursaWolf instanceof BursaWolfParameters) { + final Object bursaWolf = definition.remove(Convention.TOWGS84); + if (hasBuiltSomeCustomObjects || bursaWolf != null) { + Map<String, Object> properties = properties(definition, false, Convention.GEODETIC_DATUM_NAME, () -> { + // Fallback if `definition` does not contain a name for the geodetic reference frame. + return CRS.getGeodeticReferenceFrame(fromWKT).map(GeodeticDatum::getName).orElse(null); + }); + if (bursaWolf != null) { properties = new HashMap<>(properties); properties.put(DefaultGeodeticDatum.BURSA_WOLF_KEY, bursaWolf); - isSpecified = true; + hasBuiltSomeCustomObjects = true; } datum = datumFactory.createGeodeticDatum(properties, ellipsoid, meridian); } else { @@ -486,19 +485,83 @@ final class GridMapping { } } /* - * Geographic CRS from all above properties. + * Geographic or projected CRS built from above properties. + * The geographic CRS will always have (latitude, longitude) axes in that order and in degrees. + * The swapping to (longitude, latitude) axis order will be done by the `baseToCRS` transform. */ - if (isSpecified) { - Supplier<Object> nameFallback = () -> (fromWKT != null ? fromWKT : datum).getName(); - Map<String,?> properties = properties(definition, Convention.GEOGRAPHIC_CRS_NAME, nameFallback, isMainCRS); - return decoder.getCRSFactory().createGeographicCRS( - properties, - datum, - ensemble, - defaultDefinitions.geographic().getCoordinateSystem()); + final boolean wantGeographicCRS = (method instanceof PseudoPlateCarree); + GeographicCRS baseCRS = defaultDefinitions.geographic(); + final CRSFactory crsFactory = decoder.getCRSFactory(); + if (hasBuiltSomeCustomObjects) { + final Map<String,?> properties = properties(definition, wantGeographicCRS, Convention.GEOGRAPHIC_CRS_NAME, () -> { + // Fallback if `definition` does not contain a name for the geodetic CRS. + IdentifiedObject base = CRS.getHorizontalComponent(fromWKT); + if (base == null) { + base = datum; + } else if (base instanceof DerivedCRS) { + base = ((DerivedCRS) fromWKT).getBaseCRS(); + } + return base.getName(); + }); + baseCRS = crsFactory.createGeographicCRS(properties, datum, ensemble, baseCRS.getCoordinateSystem()); + } + // Only swap axis order from (latitude, longitude) to (longitude, latitude). + MathTransform baseToCRS = MathTransforms.swapTwoFirstAxes(2); + if (wantGeographicCRS) { + crs = baseCRS; } else { - return defaultDefinitions.geographic(); + /* + * For any "projection" other than Pseudo Plate Carrée, we will create a projected CRS, + * which requires a `Conversion` object built from the values in the `parameters` group. + * Reminder: this parameter group has been created from a subset of `definition` at the + * beginning of this method. + */ + Map<String,?> properties = properties(definition, false, Convention.CONVERSION_NAME, () -> { + if (fromWKT instanceof ProjectedCRS) { + return ((ProjectedCRS) fromWKT).getConversionFromBase().getName(); + } + return mapping.getName(); // Variable on which projection parameters are defined as attributes. + }); + final Conversion conversion = decoder.getCoordinateOperationFactory() + .createDefiningConversion(properties, method, parameters); + /* + * Projected CRS. The "base to CRS" transform is the conversion from base directly. + */ + properties = properties(definition, true, Convention.PROJECTED_CRS_NAME, () -> { + return (fromWKT != null) ? fromWKT.getName() : conversion.getName(); + }); + final ProjectedCRS projected = crsFactory.createProjectedCRS(properties, baseCRS, conversion, decoder.getStandardProjectedCS()); + baseToCRS = MathTransforms.concatenate(baseToCRS, projected.getConversionFromBase().getMathTransform()); + crs = projected; } + /* + * The CF-Convention said that even if a WKT definition is provided, other attributes shall be present + * and have precedence over the WKT definition. Consequently, the purpose of WKT in netCDF files is not + * obvious (except for CompoundCRS). + */ + if (fromWKT != null) verifyCRS(fromWKT); + setOrVerifyWKT(alreadyParsedWKT, definition, CRS_WKT); + setOrVerifyWKT(alreadyParsedWKT, definition, SPATIAL_REF); + /* + * Report all projection parameters that have not been used. If the map is not rendered + * at expected location, it may be because we have ignored some important parameters. + */ + definition.remove(CF.LONG_NAME); + if (!definition.isEmpty()) { + warningInMapping(mapping, null, Resources.Keys.UnknownProjectionParameters_3, + String.join(", ", definition.keySet())); + } + /* + * Build the "grid to CRS" if present. This is not defined by CF-convention, + * but may be present in some non-CF conventions. + */ + if (gridToCRS == null) { + gridToCRS = decoder.convention().gridToCRS(mapping, baseToCRS); + // Map pixel corners by `convention().gridToCRS(…)` contract. + } else { + gridToCRS = MathTransforms.concatenate(gridToCRS, baseToCRS); + } + return true; } /** @@ -507,14 +570,14 @@ final class GridMapping { * fetched from the value of the attribute named {@code nameAttribute}. * * @param definition map containing the attribute values. + * @param takeComment whether to consume the {@code comment} attribute. * @param nameAttribute name of the attribute from which to get the name. * @param nameFallback can return {@link String}, {@link Identifier} or {@code null}. - * @param takeComment whether to consume the {@code comment} attribute. */ - private static Map<String,Object> properties(final Map<String,Object> definition, - final String nameAttribute, - final Supplier<?> nameFallback, - final boolean takeComment) + private static Map<String, Object> properties(final Map<String, Object> definition, + final boolean takeComment, + final String nameAttribute, + final Supplier<?> nameFallback) { Object name = definition.remove(nameAttribute); if (name == null) { @@ -539,21 +602,21 @@ final class GridMapping { * If {@link #crs} is null, it is set to the parsing result. Otherwise, the current {@link #crs} has precedence * but the parsed <abbr>CRS</abbr> is compared and a warning is logged if an inconsistency is found. * - * @param definition map containing the attribute values. - * @param attributeName name of the attribute to consume in the definition map. - * @param done <abbr>WKT</abbr> already parsed, for avoiding repetition. + * @param alreadyParsedWKT <abbr>WKT</abbr> already parsed, for avoiding repetition. + * @param definition map containing the attribute values. + * @param attributeName name of the attribute to consume in the definition map. */ @SuppressWarnings("UseSpecificCatch") - private void setOrVerifyWKT(final Map<String,Object> definition, final String attributeName, final List<String> done) { + private void setOrVerifyWKT(final List<String> alreadyParsedWKT, final Map<String, Object> definition, final String attributeName) { Object value = definition.remove(attributeName); if (value instanceof String) { String wkt = ((String) value).strip(); - for (String previous : done) { + for (String previous : alreadyParsedWKT) { if (wkt.equalsIgnoreCase(previous)) { return; } } - done.add(wkt); + alreadyParsedWKT.add(wkt); CoordinateReferenceSystem fromWKT; try { fromWKT = createFromWKT((String) value); @@ -587,56 +650,55 @@ final class GridMapping { } /** - * Tries to parse a CRS and affine transform from GDAL GeoTransform coefficients. - * Those coefficients are not in the usual order expected by matrix, affine - * transforms or TFW files. The relationship from pixel/line (P,L) coordinates - * to CRS are: + * Tries to parse a <abbr>CRS</abbr> and affine transform from <abbr>GDAL</abbr> GeoTransform coefficients. + * This is used for parsing the <abbr>GDAL</abbr>'s {@value #GEOTRANSFORM} attribute. + * The result is stored in the {@link #crs} and {@link #gridToCRS} fields. + * + * @return whether this method found grid geometry attributes. + */ + private boolean parseGDAL() throws ParseException { + boolean found = parseGeoTransform(mapping.getAttributeAsString(GEOTRANSFORM)); + final String wkt = mapping.getAttributeAsString(SPATIAL_REF); + if (wkt != null) { + crs = createFromWKT(wkt); + isWKT = true; + found = true; + } + return found; + } + + /** + * Tries to parse an affine transform from <abbr>GDAL</abbr> GeoTransform coefficients. + * Those coefficients are not in the order usually found in matrices, affine transforms or <abbr>TFW</abbr> files. + * The relationship from pixel/line (P,L) coordinates to <abbr>CRS</abbr> are: * * {@snippet lang="java" : * X = c[0] + P*c[1] + L*c[2]; * Y = c[3] + P*c[4] + L*c[5]; * } * + * The result is stored in the {@link #gridToCRS} field. + * * @return whether this method found grid geometry attributes. */ - private boolean parseGeoTransform() { - return parseGeoTransform(mapping.getAttributeAsString(SPATIAL_REF), - mapping.getAttributeAsString(GEOTRANSFORM)); - } - - /** - * Implementation of {@link #parseGeoTransform()} with given attribute values. - * Used for parsing the <abbr>GDAL</abbr>'s {@value #GEOTRANSFORM} attribute. - * Results is stored in {@link #gridToCRS}. - */ @SuppressWarnings("UseSpecificCatch") - private boolean parseGeoTransform(final String wkt, final String gtr) { - boolean grid = false; - boolean done = false; - try { - if (wkt != null) { - crs = createFromWKT(wkt); - isWKT = true; - done = true; - } - if (gtr != null) { - grid = true; - final double[] c = parseDoubles(gtr); - if (c.length != 6) { - throw new DataStoreContentException(mapping.errors().getString(Errors.Keys.UnexpectedArrayLength_2, 6, c.length)); - } + private boolean parseGeoTransform(final String gtr) { + if (gtr != null) { + final double[] c = parseDoubles(gtr); + if (c.length == 6) { /* * GDAL convention maps pixel corners and see the data as if it was an image. * The row which is visually on the top is handled as if its index was zero, * ignoring the fact that this is usually the last row in a netCDF variable. */ gridToCRS = new AffineTransform2D(c[1], c[4], c[2], c[5], c[0], c[3]); // X_DIMENSION, Y_DIMENSION - done = true; + anchorFromConvention = PixelInCell.CELL_CORNER; + return true; } - } catch (Exception e) { - cannotCreateGridOrCRS(mapping, e, grid); + var e = new DataStoreContentException(mapping.errors().getString(Errors.Keys.UnexpectedArrayLength_2, 6, c.length)); + cannotCreateGridOrCRS(mapping, e, true); } - return done; + return false; } /** @@ -650,12 +712,11 @@ final class GridMapping { /** * Tries to parse the Coordinate Reference System using ESRI conventions or other non-CF conventions. - * This method is invoked as a fallback if {@link #parseGeoTransform()} found no grid geometry. + * This method is invoked as a fallback if {@link #parseGDAL()} found no grid geometry. * * @return whether this method found grid geometry attributes. */ - @SuppressWarnings("UseSpecificCatch") - private boolean parseESRI() { + private boolean parseESRI() throws ParseException, FactoryException { String code = mapping.getAttributeAsString("ESRI_pe_string"); isWKT = (code != null); if (code == null) { @@ -671,21 +732,16 @@ final class GridMapping { * The CRS parsings below need to take those differences in account, except axis order which is tested in * the `adaptGridCRS(…)` method. */ - try { - if (isWKT) { - crs = createFromWKT(code); - } else { - crs = CRS.forCode(Constants.EPSG + ':' + code); - } - } catch (Exception e) { - cannotCreateGridOrCRS(mapping, e, false); - return false; + if (isWKT) { + crs = createFromWKT(code); + } else { + crs = CRS.forCode(Constants.EPSG + ':' + code); } return true; } /** - * Creates a coordinate reference system by parsing a Well Known Text (WKT) string. + * Creates a coordinate reference system by parsing a Well Known Text (<abbr>WKT</abbr>) string. * The WKT is presumed to use the GDAL flavor of WKT 1, and warnings are redirected to decoder listeners. */ private CoordinateReferenceSystem createFromWKT(final String wkt) throws ParseException { @@ -756,21 +812,21 @@ final class GridMapping { * @return the transform for the given variable. */ private MathTransform gridToCRS(final Variable variable) { - MathTransform implicitG2C = gridToCRS; - if (implicitG2C != null) { + MathTransform flipped = gridToCRS; + if (flipped != null) { final int yDim = variable.getNumDimensions() - (1 + SOURCE_AXIS_TO_FLIP); if (yDim >= 0) { final long height = variable.getGridDimensions().get(yDim).length(); if (height >= 0) { // Negative if undetermined length. - final int srcDim = gridToCRS.getSourceDimensions(); + final int srcDim = flipped.getSourceDimensions(); final MatrixSIS m = Matrices.createIdentity(srcDim + 1); m.setElement(SOURCE_AXIS_TO_FLIP, SOURCE_AXIS_TO_FLIP, -1); m.setElement(SOURCE_AXIS_TO_FLIP, srcDim, height); - implicitG2C = MathTransforms.concatenate(MathTransforms.linear(m), implicitG2C); + flipped = MathTransforms.concatenate(MathTransforms.linear(m), flipped); } } } - return implicitG2C; + return flipped; } /** @@ -830,9 +886,9 @@ final class GridMapping { */ CoordinateReferenceSystem explicitCRS = crs; MathTransform explicitG2C = gridToCRS(variable); - if (explicitG2C != null) { + if (anchorFromConvention != null) { // GDAL "GeoTransform" uses pixel corner convention. - anchor = PixelInCell.CELL_CORNER; + anchor = anchorFromConvention; } int firstAffectedCoordinate = 0; boolean isSameGrid = true; @@ -849,11 +905,13 @@ final class GridMapping { * * This is where the potential difference between EPSG axis order and grid axis order is handled. * If we cannot find which component to replace, assume that grid mapping describes the first dimensions. - * We have no guarantees that this latter assumption is right, but it seems to match common practice. + * We have no guarantees that this latter assumption is correct, but it seems to match common practice. */ + Matrix swapAxisOrder = null; final CoordinateSystem cs = implicitCRS.getCoordinateSystem(); firstAffectedCoordinate = AxisDirections.indexOfColinear(cs, explicitCRS.getCoordinateSystem()); if (firstAffectedCoordinate < 0) { + final CoordinateReferenceSystem beforeSwap = explicitCRS; explicitCRS = AbstractCRS.castOrCopy(explicitCRS).forConvention(AxesConvention.RIGHT_HANDED); firstAffectedCoordinate = AxisDirections.indexOfColinear(cs, explicitCRS.getCoordinateSystem()); if (firstAffectedCoordinate < 0) { @@ -862,6 +920,13 @@ final class GridMapping { explicitCRS = crs; // If specified by WKT, use the CRS verbatim. } } + if (explicitCRS != beforeSwap) try { + swapAxisOrder = CoordinateSystems.swapAndScaleAxes(beforeSwap.getCoordinateSystem(), + explicitCRS.getCoordinateSystem()); + } catch (IllegalArgumentException | IncommensurableException e) { + cannotCreateGridOrCRS(variable, e, false); + return null; + } } /* * Replace the grid CRS (or a component of it) by the CRS parsed from WKT or EPSG code with same (if possible) @@ -878,17 +943,17 @@ final class GridMapping { if (isSameGrid) { explicitCRS = implicitCRS; // Keep existing instance if appropriate. } - } - /* - * If we have run the `AbstractCRS.castOrCopy(…).forConvention(…)` code above, the axis order of the CRS - * may be different than the axis order which was assumed when the "grid to CRS" transform was built. - */ - if (explicitCRS != crs && explicitG2C != null) try { - var swap = CoordinateSystems.swapAndScaleAxes(crs.getCoordinateSystem(), explicitCRS.getCoordinateSystem()); - explicitG2C = MathTransforms.concatenate(explicitG2C, MathTransforms.linear(swap)); - } catch (IllegalArgumentException | IncommensurableException e) { - cannotCreateGridOrCRS(variable, e, false); - return null; + /* + * If we have run the `AbstractCRS.castOrCopy(…).forConvention(…)` code above, + * the axis order of the CRS may become different than the axis order which was + * assumed when the "grid to CRS" transform was built. + */ + if (swapAxisOrder != null) { + MathTransform swap = MathTransforms.linear(swapAxisOrder); + int numTrailingCoordinates = explicitG2C.getTargetDimensions() - firstAffectedCoordinate; + swap = MathTransforms.passThrough(firstAffectedCoordinate, swap, numTrailingCoordinates); + explicitG2C = MathTransforms.concatenate(explicitG2C, swap); + } } } /* diff --git a/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/classic/ChannelDecoder.java b/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/classic/ChannelDecoder.java index 8c8c1ecd36..8e9536d59b 100644 --- a/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/classic/ChannelDecoder.java +++ b/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/classic/ChannelDecoder.java @@ -1065,7 +1065,7 @@ nextVar: for (final VariableInfo variable : variables) { * and we would not need to check for variables having dimension names. However, in practice there is * incomplete attributes, so we check for other dimensions even if the above loop did some work. */ - for (int i=variable.dimensions.length; --i >= 0;) { // Reverse of netCDF order. + for (int i = variable.dimensions.length; --i >= 0;) { // Reverse of netCDF order. final DimensionInfo dimension = variable.dimensions[i]; if (usedDimensions.add(dimension)) { final List<VariableInfo> axis = dimToAxes.get(dimension); // Should have only 1 element. diff --git a/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/internal/Resources.java b/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/internal/Resources.java index 26b5fa05ae..d9d5583fde 100644 --- a/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/internal/Resources.java +++ b/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/internal/Resources.java @@ -185,6 +185,11 @@ public class Resources extends IndexedResourceBundle { */ public static final short MismatchedVariableType_3 = 13; + /** + * Missing or incomplete ellipsoid on the “{1}” variable of netCDF file “{0}”: {2} + */ + public static final short MissingEllipsoid_3 = 31; + /** * Missing attribute “{2}” on the “{1}” variable of netCDF file “{0}”. */ diff --git a/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/internal/Resources.properties b/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/internal/Resources.properties index 2c64238ed4..567b430342 100644 --- a/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/internal/Resources.properties +++ b/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/internal/Resources.properties @@ -41,6 +41,7 @@ GridLongitudeSpanTooWide_2 = The grid spans {0}\u00b0 of longitude, which MismatchedAttributeLength_5 = Attributes \u201c{1}\u201d and \u201c{2}\u201d on variable \u201c{0}\u201d have different lengths: {3} and {4} respectively. MismatchedVariableSize_3 = The declared size of variable \u201c{1}\u201d in netCDF file \u201c{0}\u201d is {2,number} bytes greater than expected. MismatchedVariableType_3 = Variables \u201c{1}\u201d and \u201c{2}\u201d in netCDF file \u201c{0}\u201d does not have the same type. +MissingEllipsoid_3 = Missing or incomplete ellipsoid on the \u201c{1}\u201d variable of netCDF file \u201c{0}\u201d: {2} MissingVariableAttribute_3 = Missing attribute \u201c{2}\u201d on the \u201c{1}\u201d variable of netCDF file \u201c{0}\u201d. ResamplingIntervalNotFound_2 = Variable \u201c{1}\u201d or netCDF file \u201c{0}\u201d has a different size than its coordinate system, but no resampling interval is specified. UnexpectedAxisCount_4 = Reference system of type \u2018{1}\u2019 cannot have {2}\u00a0axes. The axes found in the \u201c{0}\u201d netCDF file are: {3}. diff --git a/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/internal/Resources_fr.properties b/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/internal/Resources_fr.properties index 6e94aba15d..24f66085b3 100644 --- a/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/internal/Resources_fr.properties +++ b/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/internal/Resources_fr.properties @@ -46,6 +46,7 @@ GridLongitudeSpanTooWide_2 = La grille s\u2019\u00e9tend sur {0}\u00b0 de MismatchedAttributeLength_5 = Les attributs \u201c{1}\u201d et \u201c{2}\u201d de la variable \u201c{0}\u201d ont des longueurs diff\u00e9rentes\u00a0: {3} et {4} respectivement. MismatchedVariableSize_3 = La longueur d\u00e9clar\u00e9e de la variable \u00ab\u202f{1}\u202f\u00bb dans le fichier netCDF \u00ab\u202f{0}\u202f\u00bb d\u00e9passe de {2,number} octets la valeur attendue. MismatchedVariableType_3 = Les variables \u00ab\u202f{1}\u202f\u00bb et \u00ab\u202f{2}\u202f\u00bb dans le fichier netCDF \u00ab\u202f{0}\u202f\u00bb ne sont pas du m\u00eame type. +MissingEllipsoid_3 = L\u2019ellipso\u00efde est manquant ou incomplet sur la variable \u201c{1}\u201d du fichier netCDF \u201c{0}\u201d\u00a0: {2} MissingVariableAttribute_3 = Il manque l\u2019attribut \u201c{2}\u201d sur la variable \u201c{1}\u201d du fichier netCDF \u201c{0}\u201d. ResamplingIntervalNotFound_2 = La variable \u00ab\u202f{1}\u202f\u00bb du fichier netCDF \u00ab\u202f{0}\u202f\u00bb a une taille diff\u00e9rente de celle de son syst\u00e8me de coordonn\u00e9es, mais l\u2019intervalle d\u2019\u00e9chantillonnage n\u2019a pas \u00e9t\u00e9 sp\u00e9cifi\u00e9. UnexpectedAxisCount_4 = Les syst\u00e8mes de r\u00e9f\u00e9rence de type \u2018{1}\u2019 ne peuvent pas avoir {2}\u00a0axes. Les axes trouv\u00e9s dans le fichier netCDF \u00ab\u202f{0}\u202f\u00bb sont\u00a0: {3}. diff --git a/endorsed/src/org.apache.sis.storage.netcdf/test/org/apache/sis/storage/netcdf/MetadataReaderTest.java b/endorsed/src/org.apache.sis.storage.netcdf/test/org/apache/sis/storage/netcdf/MetadataReaderTest.java index 689854ae78..ae23a1d98d 100644 --- a/endorsed/src/org.apache.sis.storage.netcdf/test/org/apache/sis/storage/netcdf/MetadataReaderTest.java +++ b/endorsed/src/org.apache.sis.storage.netcdf/test/org/apache/sis/storage/netcdf/MetadataReaderTest.java @@ -48,11 +48,13 @@ import org.opengis.test.dataset.TestData; /** - * Tests {@link MetadataReader}. This tests uses the SIS embedded implementation and the UCAR library - * for reading netCDF attributes. + * Tests {@link MetadataReader}. + * This tests uses the <abbr>SIS</abbr> embedded implementation + * and the <abbr>UCAR</abbr> library for reading netCDF attributes. * * @author Martin Desruisseaux (Geomatys) */ +@SuppressWarnings("exports") public final class MetadataReaderTest extends TestCase { /** * Creates a new test case. diff --git a/endorsed/src/org.apache.sis.storage.netcdf/test/org/apache/sis/storage/netcdf/NetcdfStoreProviderTest.java b/endorsed/src/org.apache.sis.storage.netcdf/test/org/apache/sis/storage/netcdf/NetcdfStoreProviderTest.java index c9856d40bf..51580075d1 100644 --- a/endorsed/src/org.apache.sis.storage.netcdf/test/org/apache/sis/storage/netcdf/NetcdfStoreProviderTest.java +++ b/endorsed/src/org.apache.sis.storage.netcdf/test/org/apache/sis/storage/netcdf/NetcdfStoreProviderTest.java @@ -41,6 +41,7 @@ import org.opengis.test.dataset.TestData; * * @author Martin Desruisseaux (Geomatys) */ +@SuppressWarnings("exports") public final class NetcdfStoreProviderTest extends TestCase { /** * Creates a new test case. diff --git a/endorsed/src/org.apache.sis.storage.netcdf/test/org/apache/sis/storage/netcdf/NetcdfStoreTest.java b/endorsed/src/org.apache.sis.storage.netcdf/test/org/apache/sis/storage/netcdf/NetcdfStoreTest.java index a2ea937da1..01c42e47fe 100644 --- a/endorsed/src/org.apache.sis.storage.netcdf/test/org/apache/sis/storage/netcdf/NetcdfStoreTest.java +++ b/endorsed/src/org.apache.sis.storage.netcdf/test/org/apache/sis/storage/netcdf/NetcdfStoreTest.java @@ -16,16 +16,27 @@ */ package org.apache.sis.storage.netcdf; +import java.net.URL; import org.opengis.metadata.Metadata; +import org.opengis.referencing.crs.CoordinateReferenceSystem; +import org.opengis.referencing.crs.ProjectedCRS; +import org.opengis.referencing.operation.MathTransform; +import org.apache.sis.referencing.operation.transform.LinearTransform; +import org.apache.sis.referencing.CRS; +import org.apache.sis.coverage.grid.GridExtent; +import org.apache.sis.coverage.grid.GridGeometry; +import org.apache.sis.coverage.grid.PixelInCell; import org.apache.sis.storage.StorageConnector; import org.apache.sis.storage.DataStoreException; +import org.apache.sis.storage.GridCoverageResource; import org.apache.sis.system.Loggers; import org.apache.sis.util.Version; // Test dependencies import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.*; -import org.apache.sis.test.TestCaseWithLogs; +import static org.apache.sis.feature.Assertions.assertExtentEquals; +import org.apache.sis.storage.DataStoreTestCase; // Specific to the geoapi-3.1 and geoapi-4.0 branches: import org.opengis.test.dataset.TestData; @@ -35,8 +46,10 @@ import org.opengis.test.dataset.TestData; * Tests {@link NetcdfStore}. * * @author Martin Desruisseaux (Geomatys) + * @author Alexis Manin (Geomatys) */ -public final class NetcdfStoreTest extends TestCaseWithLogs { +@SuppressWarnings("exports") +public final class NetcdfStoreTest extends DataStoreTestCase { /** * Creates a new test case. */ @@ -45,13 +58,39 @@ public final class NetcdfStoreTest extends TestCaseWithLogs { } /** - * Returns a new netCDF store to test. + * Returns a new netCDF store opened on a dataset specified by an enumeration value. * - * @param dataset the name of the datastore to load. + * @param dataset the name of the dataset to open. + * @return netCDF data store opened for the given dataset. * @throws DataStoreException if an error occurred while reading the netCDF file. */ - private static NetcdfStore create(final TestData dataset) throws DataStoreException { - return new NetcdfStore(null, new StorageConnector(dataset.location())); + private NetcdfStore create(final TestData dataset) throws DataStoreException { + return create(dataset.name(), dataset.location()); + } + + /** + * Returns a new netCDF store opened on a dataset specified by a resource name. + * + * @param dataset the name of a file in the same directory as this test class. + * @throws DataStoreException if an error occurred while reading the netCDF file. + */ + private NetcdfStore create(final String dataset) throws DataStoreException { + return create(dataset, NetcdfStoreTest.class.getResource(dataset)); + } + + /** + * Returns a new netCDF store to test from the given <abbr>URL</abbr>. + * + * @param name the name of the dataset to open, for assertion message. + * @param dataset the <abbr>URL</abbr> of the dataset to open. + * @return netCDF data store opened for the given dataset. + * @throws DataStoreException if an error occurred while reading the netCDF file. + */ + private NetcdfStore create(final String name, final URL dataset) throws DataStoreException { + assertNotNull(dataset, name); + final var store = new NetcdfStore(null, new StorageConnector(dataset)); + listen(store); + return store; } /** @@ -86,4 +125,33 @@ public final class NetcdfStoreTest extends TestCaseWithLogs { assertEquals(4, version.getMinor()); loggings.assertNoUnexpectedLog(); } + + /** + * Tests the reading of a file a <abbr>CRS</abbr> declared using <abbr>GDAL</abbr>-specific properties. + * + * @throws DataStoreException if an error occurred while reading the netCDF file. + */ + @Test + public void testFileWithGDALEncoding() throws DataStoreException { + try (NetcdfStore store = create("transverse-mercator-wrong-geotransform.nc")) { + final var r = assertInstanceOf(GridCoverageResource.class, store.findResource("evi")); + final GridGeometry gg = r.getGridGeometry(); + assertEquals(3, gg.getDimension()); + + final GridExtent extent = gg.getExtent(); + assertExtentEquals(new long[3], new long[] {2, 3, 2}, extent); + + final MathTransform gridToCRS = gg.getGridToCRS(PixelInCell.CELL_CORNER); + assertEquals(3, gridToCRS.getSourceDimensions()); + assertEquals(3, gridToCRS.getTargetDimensions()); + assertFalse(gridToCRS instanceof LinearTransform); + + final CoordinateReferenceSystem crs = gg.getCoordinateReferenceSystem(); + assertInstanceOf(ProjectedCRS.class, CRS.getHorizontalComponent(crs)); + assertNotNull(CRS.getTemporalComponent(crs)); + } + // The reader should have emitted a warning about mismatched `GeoTransform` values. + loggings.assertNextLogContains("GeoTransform", "evi", "transverse-mercator-wrong-geotransform.nc"); + loggings.assertNoUnexpectedLog(); + } } diff --git a/endorsed/src/org.apache.sis.storage.netcdf/test/org/apache/sis/storage/netcdf/transverse-mercator-wrong-geotransform.nc b/endorsed/src/org.apache.sis.storage.netcdf/test/org/apache/sis/storage/netcdf/transverse-mercator-wrong-geotransform.nc new file mode 100644 index 0000000000..a1851f4d54 Binary files /dev/null and b/endorsed/src/org.apache.sis.storage.netcdf/test/org/apache/sis/storage/netcdf/transverse-mercator-wrong-geotransform.nc differ diff --git a/endorsed/src/org.apache.sis.storage/test/org/apache/sis/storage/DataStoreTestCase.java b/endorsed/src/org.apache.sis.storage/test/org/apache/sis/storage/DataStoreTestCase.java new file mode 100644 index 0000000000..277161153c --- /dev/null +++ b/endorsed/src/org.apache.sis.storage/test/org/apache/sis/storage/DataStoreTestCase.java @@ -0,0 +1,72 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.sis.storage; + +import java.util.logging.Logger; +import org.apache.sis.storage.event.StoreListener; +import org.apache.sis.storage.event.WarningEvent; + +// Test dependencies +import org.apache.sis.test.TestCaseWithLogs; + + +/** + * Base class for {@link DataStore} tests which may emit warnings. + * + * @author Martin Desruisseaux (Geomatys) + */ +@SuppressWarnings("exports") +public abstract class DataStoreTestCase extends TestCaseWithLogs implements StoreListener<WarningEvent> { + /** + * Creates a new test case which will listen to logs emitted by the given logger. + * + * @param logger the logger to listen to. + */ + protected DataStoreTestCase(Logger logger) { + super(logger); + } + + /** + * Creates a new test case which will listen to logs emitted by the logger of the given name. + * + * @param logger name of the logger to listen. + */ + protected DataStoreTestCase(String logger) { + super(logger); + } + + /** + * Performs an unconditional redirection of the warnings emitted by the given store to the JUnit extension. + * The {@link #eventOccured(WarningEvent)} method will be invoked for each warning. + * + * @param store the data store to listen to. + */ + protected final void listen(DataStore store) { + store.addListener(WarningEvent.class, this); + } + + /** + * Forwards the warning emitted by the data store to the JUnit extension that we use for checking warnings. + * This is defined as an ordinary method rather than a lambda function for making easy to set debugger break point. + * + * @param event the warning emitted by the data store. + */ + @Override + public final void eventOccured(WarningEvent event) { + loggings.accept(event.getDescription()); + } +} diff --git a/endorsed/src/org.apache.sis.util/test/org/apache/sis/test/LoggingWatcher.java b/endorsed/src/org.apache.sis.util/test/org/apache/sis/test/LoggingWatcher.java index 0b1addf8c0..28c39b2b8b 100644 --- a/endorsed/src/org.apache.sis.util/test/org/apache/sis/test/LoggingWatcher.java +++ b/endorsed/src/org.apache.sis.util/test/org/apache/sis/test/LoggingWatcher.java @@ -22,6 +22,7 @@ import java.util.Queue; import java.util.LinkedList; import java.util.ConcurrentModificationException; import java.util.Objects; +import java.util.function.Consumer; import java.util.logging.Filter; import java.util.logging.Level; import java.util.logging.Logger; @@ -74,7 +75,7 @@ import org.junit.jupiter.api.extension.ExtensionContext; * * @author Martin Desruisseaux (Geomatys) */ -public final class LoggingWatcher implements BeforeEachCallback, AfterEachCallback, Filter { +public final class LoggingWatcher implements BeforeEachCallback, AfterEachCallback, Filter, Consumer<LogRecord> { /** * Name of the lock to use in a JUnit {@code ResourceLock} annotation. * Tests that are executed in a single thread can be run in parallel @@ -265,26 +266,37 @@ public final class LoggingWatcher implements BeforeEachCallback, AfterEachCallba */ LoggingWatcher owner = null; synchronized (logger) { - if (allFilters == null) { // Should never be null, but sometime happens for an unknown reason. - return true; - } - for (final LoggingWatcher w : allFilters) { - if (w.isMultiThread || w.threadId == record.getLongThreadID()) { - owner = w; - break; + if (allFilters != null) { // Should never be null when executed by JUnit. + for (final LoggingWatcher w : allFilters) { + if (w.isMultiThread || w.threadId == record.getLongThreadID()) { + owner = w; + break; + } } + } else { + owner = this; // Not executed by JUnit. Assume sequential execution. } } if (owner == null) { return true; } - synchronized (owner) { - owner.messages.add(new Message(owner.formatter.formatMessage(record), record.getThrown())); - } + owner.accept(record); } return TestCase.VERBOSE; } + /** + * Unconditionally adds the logging message to the {@link #messages} list. + * Contrarily to {@link #isLoggable(LogRecord)}, this method does not check + * ownership or the log level. + * + * @param record the log record to add to the {@link #messages} list. + */ + @Override + public synchronized void accept(final LogRecord record) { + messages.add(new Message(formatter.formatMessage(record), record.getThrown())); + } + /** * Skips the next log messages if it contains all the given keywords. * This method is used instead of {@link #assertNextLogContains(String...)} when a log message may or
