sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject [sis] 02/02: Partial fix for support of netCDF variables having an unlimited dimension. https://issues.apache.org/jira/browse/SIS-439
Date Tue, 06 Nov 2018 19:24:30 GMT
This is an automated email from the ASF dual-hosted git repository.

desruisseaux pushed a commit to branch geoapi-4.0
in repository https://gitbox.apache.org/repos/asf/sis.git

commit c1779efec550b420839dbca48147064b3be7dd31
Author: Martin Desruisseaux <martin.desruisseaux@geomatys.com>
AuthorDate: Tue Nov 6 20:23:41 2018 +0100

    Partial fix for support of netCDF variables having an unlimited dimension.
    https://issues.apache.org/jira/browse/SIS-439
---
 .../java/org/apache/sis/image/PixelIterator.java   |   8 +-
 .../org/apache/sis/internal/util/Numerics.java     |  34 ++++++
 .../org/apache/sis/internal/util/NumericsTest.java |  15 +++
 .../java/org/apache/sis/internal/netcdf/Axis.java  |   2 +
 .../org/apache/sis/internal/netcdf/DataType.java   |  19 ++-
 .../org/apache/sis/internal/netcdf/Variable.java   |  13 +-
 .../sis/internal/netcdf/impl/ChannelDecoder.java   |  25 ++--
 .../apache/sis/internal/netcdf/impl/Dimension.java |  26 ++--
 .../sis/internal/netcdf/impl/VariableInfo.java     | 135 +++++++++++++++++----
 .../sis/internal/netcdf/impl/package-info.java     |   1 +
 .../sis/internal/netcdf/ucar/VariableWrapper.java  |   8 ++
 .../org/apache/sis/storage/netcdf/NetcdfStore.java |   2 +-
 .../sis/storage/netcdf/NetcdfStoreProvider.java    |   2 +-
 .../org/apache/sis/internal/netcdf/TestCase.java   |   6 +-
 .../sis/internal/storage/io/ChannelDataInput.java  |  16 ++-
 .../internal/storage/io/HyperRectangleReader.java  |  11 +-
 .../org/apache/sis/internal/storage/io/Region.java |  65 ++++++----
 17 files changed, 300 insertions(+), 88 deletions(-)

diff --git a/core/sis-raster/src/main/java/org/apache/sis/image/PixelIterator.java b/core/sis-raster/src/main/java/org/apache/sis/image/PixelIterator.java
index c076236..fc6642d 100644
--- a/core/sis-raster/src/main/java/org/apache/sis/image/PixelIterator.java
+++ b/core/sis-raster/src/main/java/org/apache/sis/image/PixelIterator.java
@@ -30,6 +30,7 @@ import org.apache.sis.util.resources.Errors;
 import org.apache.sis.util.ArgumentChecks;
 
 import static java.lang.Math.floorDiv;
+import static org.apache.sis.internal.util.Numerics.ceilDiv;
 
 
 /**
@@ -170,13 +171,6 @@ public abstract class PixelIterator {
     }
 
     /**
-     * Returns {@code numerator / denominator} rounded toward positive infinity.
-     */
-    private static int ceilDiv(final int numerator, final int denominator) {
-        return -floorDiv(-numerator, denominator);
-    }
-
-    /**
      * Computes the intersection between the given bounds and and {@code subArea} if {@code
subArea} is non-null.
      * If the result is empty, then the width and/or height are set to zero (not negative).
      */
diff --git a/core/sis-utility/src/main/java/org/apache/sis/internal/util/Numerics.java b/core/sis-utility/src/main/java/org/apache/sis/internal/util/Numerics.java
index 78b4ca5..519d83b 100644
--- a/core/sis-utility/src/main/java/org/apache/sis/internal/util/Numerics.java
+++ b/core/sis-utility/src/main/java/org/apache/sis/internal/util/Numerics.java
@@ -147,6 +147,40 @@ public final class Numerics extends Static {
     }
 
     /**
+     * Returns the smallest (closest to negative infinity) long value that is greater than
or equals to x/y.
+     *
+     * @param  x  the dividend.
+     * @param  y  the divisor.
+     * @return x/y rounded toward positive infinity.
+     *
+     * @see Math#floorDiv(int, int)
+     */
+    public static int ceilDiv(final int x, final int y) {
+        int r = x / y;
+        if ((x ^ y) >= 0 && (r * y != x)) {
+            r++;
+        }
+        return r;
+    }
+
+    /**
+     * Returns the smallest (closest to negative infinity) long value that is greater than
or equals to x/y.
+     *
+     * @param  x  the dividend.
+     * @param  y  the divisor.
+     * @return x/y rounded toward positive infinity.
+     *
+     * @see Math#floorDiv(long, long)
+     */
+    public static long ceilDiv(final long x, final long y) {
+        long r = x / y;
+        if ((x ^ y) >= 0 && (r * y != x)) {
+            r++;
+        }
+        return r;
+    }
+
+    /**
      * If the given value is presents in the cache, returns the cached value.
      * Otherwise returns the given value as-is.
      *
diff --git a/core/sis-utility/src/test/java/org/apache/sis/internal/util/NumericsTest.java
b/core/sis-utility/src/test/java/org/apache/sis/internal/util/NumericsTest.java
index bf2889f..dcaa21e 100644
--- a/core/sis-utility/src/test/java/org/apache/sis/internal/util/NumericsTest.java
+++ b/core/sis-utility/src/test/java/org/apache/sis/internal/util/NumericsTest.java
@@ -41,6 +41,21 @@ import static org.junit.Assert.*;
 @SuppressWarnings("UnnecessaryBoxing")
 public final strictfp class NumericsTest extends TestCase {
     /**
+     * Tests {@link Numerics#ceilDiv(int, int)} and {@link Numerics#ceilDiv(long, long)}.
+     */
+    @Test
+    public void testCeilDiv() {
+        assertEquals( 4,  ceilDiv( 12,  3 ));
+        assertEquals( 4L, ceilDiv( 12L, 3L));
+        assertEquals( 3,  ceilDiv(  8,  3 ));
+        assertEquals( 3L, ceilDiv(  8L, 3L));
+        assertEquals(-4,  ceilDiv(-12,  3 ));
+        assertEquals(-4L, ceilDiv(-12L, 3L));
+        assertEquals(-2,  ceilDiv( -8,  3 ));
+        assertEquals(-2L, ceilDiv( -8L, 3L));
+    }
+
+    /**
      * Tests the {@link Numerics#cached(Object)} method.
      */
     @Test
diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Axis.java b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Axis.java
index 5939443..b070054 100644
--- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Axis.java
+++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Axis.java
@@ -42,6 +42,8 @@ import org.apache.sis.util.ArraysExt;
 public final class Axis {
     /**
      * The attributes to use for fetching dimension (in ISO-19115 sense) information, or
{@code null} if unknown.
+     * Example: {@code "geospatial_lat_min"}, {@code "geospatial_lat_resolution"}, {@code
DimensionNameType.ROW}.
+     * This is used by {@link org.apache.sis.storage.netcdf.MetadataReader} for information
purpose only.
      */
     public final AttributeNames.Dimension attributeNames;
 
diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/DataType.java
b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/DataType.java
index b14f320..b86a8cc 100644
--- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/DataType.java
+++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/DataType.java
@@ -37,7 +37,7 @@ import org.apache.sis.util.Numbers;
  *
  * @author  Johann Sorel (Geomatys)
  * @author  Martin Desruisseaux (Geomatys)
- * @version 0.8
+ * @version 1.0
  * @since   0.8
  * @module
  */
@@ -162,6 +162,23 @@ public enum DataType {
     }
 
     /**
+     * Returns the number of bytes for this data type, or 0 if unknown.
+     *
+     * @return number of bytes for this data type, or 0 if unknown.
+     */
+    public final int size() {
+        switch (number) {
+            case Numbers.BYTE:    return Byte.BYTES;
+            case Numbers.SHORT:   return Short.BYTES;
+            case Numbers.INTEGER: // Same as float
+            case Numbers.FLOAT:   return Float.BYTES;
+            case Numbers.LONG:    // Same as double
+            case Numbers.DOUBLE:  return Double.BYTES;
+            default:              return 0;
+        }
+    }
+
+    /**
      * Returns the signed or unsigned variant of this data type.
      * If this data type does not have the requested variant, then this method returns {@code
this}.
      *
diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Variable.java
b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Variable.java
index 3a5d650..3e4f952 100644
--- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Variable.java
+++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Variable.java
@@ -28,7 +28,7 @@ import org.apache.sis.storage.DataStoreException;
  *
  * @author  Martin Desruisseaux (Geomatys)
  * @author  Johann Sorel (Geomatys)
- * @version 0.8
+ * @version 1.0
  * @since   0.3
  * @module
  */
@@ -91,6 +91,14 @@ public abstract class Variable extends NamedElement {
     }
 
     /**
+     * Returns whether this variable can grow. A variable is unlimited if at least one of
its dimension is unlimited.
+     * In netCDF 3 classic format, only the first dimension can be unlimited.
+     *
+     * @return whether this variable can grow.
+     */
+    public abstract boolean isUnlimited();
+
+    /**
      * Returns {@code true} if the given variable can be used for generating an image.
      * This method checks for the following conditions:
      *
@@ -238,6 +246,9 @@ public abstract class Variable extends NamedElement {
         for (int i=shape.length; --i>=0;) {
             buffer.append('[').append(Integer.toUnsignedLong(shape[i])).append(']');
         }
+        if (isUnlimited()) {
+            buffer.append(" (unlimited)");
+        }
         return buffer.toString();
     }
 }
diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/ChannelDecoder.java
b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/ChannelDecoder.java
index e63d5fa..1fa3e4e 100644
--- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/ChannelDecoder.java
+++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/ChannelDecoder.java
@@ -69,7 +69,7 @@ import ucar.nc2.constants.CF;
  *
  * @author  Johann Sorel (Geomatys)
  * @author  Martin Desruisseaux (Geomatys)
- * @version 0.8
+ * @version 1.0
  *
  * @see <a href="http://portal.opengeospatial.org/files/?artifact_id=43734">NetCDF
Classic and 64-bit Offset Format (1.0)</a>
  *
@@ -214,7 +214,7 @@ public final class ChannelDecoder extends Decoder {
      * <ul>
      *   <li>Magic number: 'C','D','F'</li>
      *   <li>Version number: 1 or 2</li>
-     *   <li>Number of records</li>
+     *   <li>Number of records | {@value #STREAMING}</li>
      *   <li>List of netCDF dimensions  (see {@link #readDimensions(int)})</li>
      *   <li>List of global attributes  (see {@link #readAttributes(int)})</li>
      *   <li>List of variables          (see {@link #readVariables(int, Dimension[])})</li>
@@ -226,6 +226,7 @@ public final class ChannelDecoder extends Decoder {
      * @param  listeners  where to send the warnings.
      * @throws IOException if an error occurred while reading the channel.
      * @throws DataStoreException if the content of the given channel is not a netCDF file.
+     * @throws ArithmeticException if a variable is too large.
      */
     public ChannelDecoder(final ChannelDataInput input, final Charset encoding, final GeometryLibrary
geomlib,
             final WarningListeners<DataStore> listeners) throws IOException, DataStoreException
@@ -277,9 +278,9 @@ public final class ChannelDecoder extends Decoder {
                 }
             }
         }
-        attributeMap = attributes;
-        this.variables = variables;
-        variableMap = NamedElement.toCaseInsensitiveNameMap(variables, NAME_LOCALE);
+        this.attributeMap = attributes;
+        this.variables    = variables;
+        this.variableMap  = NamedElement.toCaseInsensitiveNameMap(variables, NAME_LOCALE);
     }
 
     /**
@@ -470,13 +471,14 @@ public final class ChannelDecoder extends Decoder {
         for (int i=0; i<nelems; i++) {
             final String name = readName();
             int length = input.readInt();
-            if (length == 0) {
+            boolean isUnlimited = (length == 0);
+            if (isUnlimited) {
                 length = numrecs;
                 if (length == STREAMING) {
                     throw new DataStoreContentException(errors().getString(Errors.Keys.MissingValueForProperty_1,
"numrecs"));
                 }
             }
-            dimensions[i] = new Dimension(name, length);
+            dimensions[i] = new Dimension(name, length, isUnlimited);
         }
         dimensionMap = Dimension.toCaseInsensitiveNameMap(dimensions, NAME_LOCALE);
         return dimensions;
@@ -565,8 +567,15 @@ public final class ChannelDecoder extends Decoder {
                 }
             }
             variables[j] = new VariableInfo(input, name, varDims, attributes,
-                    DataType.valueOf(input.readInt()), input.readInt(), readOffset());
+                    DataType.valueOf(input.readInt()), input.readInt(), readOffset(), listeners);
         }
+        /*
+         * The VariableInfo constructor determined if the variables are "unlimited" or not.
+         * The number of unlimited variable determines to padding to apply, because of an
+         * historical particularity in netCDF format. Those final adjustment can be done
+         * only after we finished creating all variables.
+         */
+        VariableInfo.complete(variables);
         return variables;
     }
 
diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/Dimension.java
b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/Dimension.java
index 2851128..32ffd2b 100644
--- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/Dimension.java
+++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/Dimension.java
@@ -26,7 +26,7 @@ import org.apache.sis.internal.netcdf.NamedElement;
  *
  * @author  Johann Sorel (Geomatys)
  * @author  Martin Desruisseaux (Geomatys)
- * @version 0.8
+ * @version 1.0
  * @since   0.3
  * @module
  */
@@ -42,14 +42,22 @@ final class Dimension extends NamedElement {
     final int length;
 
     /**
+     * Whether this dimension is the "record" (also known as "unlimited") dimension.
+     * There is at most one record dimension in a well-formed netCDF file.
+     */
+    final boolean isUnlimited;
+
+    /**
      * Creates a new dimension of the given name and length.
      *
-     * @param name    the dimension name.
-     * @param length  the number of grid cell value along this dimension, as an unsigned
number.
+     * @param name         the dimension name.
+     * @param length       the number of grid cell value along this dimension, as an unsigned
number.
+     * @param isUnlimited  whether this dimension is the "record" (also known as "unlimited")
dimension.
      */
-    Dimension(final String name, final int length) {
-        this.name   = name;
-        this.length = length;
+    Dimension(final String name, final int length, final boolean isUnlimited) {
+        this.name        = name;
+        this.length      = length;
+        this.isUnlimited = isUnlimited;
     }
 
     /**
@@ -72,6 +80,10 @@ final class Dimension extends NamedElement {
      */
     @Override
     public String toString() {
-        return name + '[' + length() + ']';
+        final StringBuilder buffer = new StringBuilder(name).append('[').append(length()).append(']');
+        if (isUnlimited) {
+            buffer.append(" (unlimited)");
+        }
+        return buffer.toString();
     }
 }
diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/VariableInfo.java
b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/VariableInfo.java
index 70ca4ce..22cfc32 100644
--- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/VariableInfo.java
+++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/VariableInfo.java
@@ -32,9 +32,9 @@ import org.apache.sis.internal.storage.io.Region;
 import org.apache.sis.storage.DataStoreException;
 import org.apache.sis.storage.DataStoreContentException;
 import org.apache.sis.storage.netcdf.AttributeNames;
+import org.apache.sis.util.logging.WarningListeners;
 import org.apache.sis.util.resources.Errors;
 import org.apache.sis.util.CharSequences;
-import org.apache.sis.util.Numbers;
 import org.apache.sis.math.Vector;
 
 
@@ -45,7 +45,7 @@ import org.apache.sis.math.Vector;
  *
  * @author  Johann Sorel (Geomatys)
  * @author  Martin Desruisseaux (Geomatys)
- * @version 0.8
+ * @version 1.0
  * @since   0.3
  * @module
  */
@@ -78,13 +78,32 @@ final class VariableInfo extends Variable implements Comparable<VariableInfo>
{
     private final String name;
 
     /**
-     * The dimensions of this variable, in order. When iterating over the values stored in
this variable
-     * (a flattened one-dimensional sequence of values), index in the domain of {@code dimensions[0]}
-     * varies faster, followed by index in the domain of {@code dimensions[1]}, <i>etc.</i>
+     * The dimensions of this variable, in the order they appear in netCDF file. When iterating
over the values stored in
+     * this variable (a flattened one-dimensional sequence of values), index in the domain
of {@code dimensions[length-1]}
+     * varies faster, followed by index in the domain of {@code dimensions[length-2]}, <i>etc.</i>
      */
     final Dimension[] dimensions;
 
     /**
+     * The offset (in bytes) to apply for moving to the next record in variable data, or
-1 if unknown.
+     * A record is a <var>n-1</var> dimensional cube spanning all dimensions
except the unlimited one.
+     * This concept is relevant only for {@linkplain #isUnlimited() unlimited variables}.
+     * For other kind of variables, this field is ignored and its value can be arbitrary.
+     *
+     * <p>On construction, this field is initialized to the number of bytes in this
variable, omitting
+     * the unlimited dimension (if any), or 0 if unknown. After {@link #complete(VariableInfo[])}
has
+     * been invoked, this field is modified as below:</p>
+     *
+     * <ul>
+     *   <li>For {@linkplain #isUnlimited() unlimited variables}, this is the number
of bytes to
+     *       skip <strong>after</strong> the data has been read for reading the
next record.</li>
+     *   <li>For other variables, this field is ignored. In current implementation
it is left to
+     *       the size of this variable, and kept for information purpose only.</li>
+     * </ul>
+     */
+    private long offsetToNextRecord;
+
+    /**
      * The attributes associates to the variable, or an empty map if none.
      * Values can be:
      *
@@ -148,8 +167,9 @@ final class VariableInfo extends Variable implements Comparable<VariableInfo>
{
      * @param  dimensions  the dimensions of this variable.
      * @param  attributes  the attributes associates to the variable, or an empty map if
none.
      * @param  dataType    the netCDF type of data, or {@code null} if unknown.
-     * @param  size        the variable size, used for verification purpose only.
+     * @param  size        the variable size. May be inaccurate and ignored.
      * @param  offset      the offset where the variable data begins in the netCDF file.
+     * @param  warnings    where to report warnings, if any.
      */
     VariableInfo(final ChannelDataInput      input,
                  final String                name,
@@ -157,7 +177,8 @@ final class VariableInfo extends Variable implements Comparable<VariableInfo>
{
                  final Map<String,Object>    attributes,
                        DataType              dataType,
                  final int                   size,
-                 final long                  offset) throws DataStoreException
+                 final long                  offset,
+                 final WarningListeners<?>   warnings) throws DataStoreException
     {
         final Object isUnsigned = attributes.get(CDM.UNSIGNED);
         if (isUnsigned != null) {
@@ -170,11 +191,31 @@ final class VariableInfo extends Variable implements Comparable<VariableInfo>
{
         /*
          * The 'size' value is provided in the netCDF files, but doesn't need to be stored
since it
          * is redundant with the dimension lengths and is not large enough for big variables
anyway.
+         * Instead we compute the length ourselves, excluding the unlimited dimension.
+         */
+        if (dataType != null) {
+            offsetToNextRecord = dataType.size();               // May be zero if unknown
data type.
+            for (int i=0; i<dimensions.length; i++) {
+                final Dimension dim = dimensions[i];
+                if (!dim.isUnlimited) {
+                    offsetToNextRecord = Math.multiplyExact(offsetToNextRecord, dim.length());
+                } else if (i != 0) {
+                    // Unlimited dimension, if any, must be first in a netCDF 3 classic format.
+                    throw new DataStoreException(warnings.getLocale(), "netCDF", input.filename,
null);
+                }
+            }
+        }
+        /*
+         * Prepare the object to be used for reading data cube efficiently from the input
channel.
+         * Note that 'dataType' can not be null if 'offsetToNextRecord' is non-zero.
          */
-        if (dataType != null && dataType.number >= Numbers.BYTE && dataType.number
<= Numbers.DOUBLE) {
+        if (offsetToNextRecord != 0) {
             reader = new HyperRectangleReader(dataType.number, input, offset);
         } else {
             reader = null;
+            if (size != -1) {                   // Maximal unsigned value, means possible
overflow.
+                offsetToNextRecord = Integer.toUnsignedLong(size);
+            }
         }
         /*
          * If the "_CoordinateAliasForDimension" attribute is defined, then its value will
be used
@@ -210,6 +251,42 @@ final class VariableInfo extends Variable implements Comparable<VariableInfo>
{
     }
 
     /**
+     * Performs the final adjustment of the {@link #offsetToNextRecord} field of all the
given variables.
+     * This method applies padding except for the special case documented in netCDF specification:
+     * <cite>"In the special case when there is only one {@linkplain #isUnlimited()
record variable}
+     * and it is of type character, byte, or short, no padding is used between record slabs,
+     * so records after the first record do not necessarily start on four-byte boundaries"</cite>.
+     *
+     * <p>After padding has been applied, this method set the {@link #offsetToNextRecord}
of all unlimited
+     * variables to the number of bytes to skip before reading the next record.</p>
+     */
+    static void complete(final VariableInfo[] variables) {
+        final VariableInfo[] unlimited = new VariableInfo[variables.length];
+        int     count     = 0;                  // Number of valid elements in the 'unlimited'
array.
+        long    total     = 0;                  // Sum of the size of all variables having
a unlimited dimension.
+        boolean isUnknown = false;              // True if 'total' is actually unknown.
+        for (final VariableInfo variable : variables) {
+            if (variable.isUnlimited()) {
+                // Apply 4 bytes padding.
+                final long offsetToNextRecord = Math.addExact(variable.offsetToNextRecord,
Integer.BYTES - 1) & ~(Integer.BYTES - 1);
+                unlimited[count++] = variable;
+                isUnknown |= (offsetToNextRecord == 0);
+                total = Math.addExact(total, offsetToNextRecord);
+            }
+        }
+        if (isUnknown) {
+            // If at least one unlimited variable has unknown size, the whole record stride
is unknown.
+            for (int i=0; i<count; i++) {
+                unlimited[i].offsetToNextRecord = -1;
+            }
+        } else if (count == 1) {
+            unlimited[0].offsetToNextRecord = 0;        // Special case cited in method javadoc.
+        } else for (int i=0; i<count; i++) {
+            unlimited[i].offsetToNextRecord = total - unlimited[i].offsetToNextRecord;
+        }
+    }
+
+    /**
      * Returns the name of this variable.
      */
     @Override
@@ -260,6 +337,16 @@ final class VariableInfo extends Variable implements Comparable<VariableInfo>
{
     }
 
     /**
+     * Returns whether this variable can grow. A variable is unlimited if at least one of
its dimension is unlimited.
+     * In netCDF 3 classic format, only the first dimension can be unlimited.
+     */
+    @Override
+    public boolean isUnlimited() {
+        // The constructor verified that only the first dimension is unlimited.
+        return (dimensions.length != 0) && dimensions[0].isUnlimited;
+    }
+
+    /**
      * Returns {@code true} if this variable seems to be a coordinate system axis,
      * determined by comparing its name with the name of all dimensions in the netCDF file.
      */
@@ -416,26 +503,22 @@ final class VariableInfo extends Variable implements Comparable<VariableInfo>
{
             if (reader == null) {
                 throw new DataStoreContentException(unknownType());
             }
-            long length = 1;
-            boolean overflow = false;
-            for (final Dimension dimension : dimensions) {
-                length *= dimension.length();
-                if (length > Integer.MAX_VALUE) {
-                    overflow = true;
-                }
-            }
-            if (overflow) {
-                throw new DataStoreContentException(Errors.format(Errors.Keys.ExcessiveListSize_2,
name, length));
-            }
-            final int dimension = dimensions.length;
-            final long[] size  = new long[dimension];
-            final int [] sub   = new int [dimension];
+            final int    dimension    = dimensions.length;
+            final long[] regionLower  = new long[dimension];
+            final long[] regionUpper  = new long[dimension];
+            final int [] subsamplings = new int [dimension];
             for (int i=0; i<dimension; i++) {
-                sub [i] = 1;
-                size[i] = dimensions[(dimension - 1) - i].length();
+                regionUpper [i] = dimensions[(dimension - 1) - i].length();
+                subsamplings[i] = 1;
+            }
+            final Region region = new Region(regionUpper, regionLower, regionUpper, subsamplings);
+            if (isUnlimited()) {
+                if (offsetToNextRecord < 0) {
+                    throw new DataStoreException(Errors.format(Errors.Keys.CanNotRead_1,
name));
+                }
+                region.skipAfterLastDimension(offsetToNextRecord / dataType.size());
             }
-            values = Vector.create(reader.read(new Region(size, new long[dimension], size,
sub)), dataType.isUnsigned)
-                    .compress(0);
+            values = Vector.create(reader.read(region), dataType.isUnsigned).compress(0);
         }
         return values;
     }
diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/package-info.java
b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/package-info.java
index 3a34638..e06d20d 100644
--- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/package-info.java
+++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/package-info.java
@@ -25,6 +25,7 @@
  * <ul>
  *   <li><a href="http://www.opengeospatial.org/standards/netcdf">NetCDF standards
on OGC web site</a></li>
  *   <li><a href="http://portal.opengeospatial.org/files/?artifact_id=43734">NetCDF
Classic and 64-bit Offset Format (1.0)</a></li>
+ *   <li><a href="https://www.unidata.ucar.edu/software/netcdf/docs/file_format_specifications.html">NetCDF
on UCAR web site.</a></li>
  * </ul>
  *
  * @author  Johann Sorel (Geomatys)
diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/ucar/VariableWrapper.java
b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/ucar/VariableWrapper.java
index b32a352..056124a 100644
--- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/ucar/VariableWrapper.java
+++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/ucar/VariableWrapper.java
@@ -118,6 +118,14 @@ final class VariableWrapper extends Variable {
     }
 
     /**
+     * Returns whether this variable can grow. A variable is unlimited if at least one of
its dimension is unlimited.
+     */
+    @Override
+    public boolean isUnlimited() {
+        return variable.isUnlimited();
+    }
+
+    /**
      * Returns {@code true} if this variable seems to be a coordinate system axis.
      */
     @Override
diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/storage/netcdf/NetcdfStore.java
b/storage/sis-netcdf/src/main/java/org/apache/sis/storage/netcdf/NetcdfStore.java
index 3629377..9af51ca 100644
--- a/storage/sis-netcdf/src/main/java/org/apache/sis/storage/netcdf/NetcdfStore.java
+++ b/storage/sis-netcdf/src/main/java/org/apache/sis/storage/netcdf/NetcdfStore.java
@@ -99,7 +99,7 @@ public class NetcdfStore extends DataStore implements Aggregate {
         location = connector.getStorageAs(URI.class);
         try {
             decoder = NetcdfStoreProvider.decoder(listeners, connector);
-        } catch (IOException e) {
+        } catch (IOException | ArithmeticException e) {
             throw new DataStoreException(e);
         }
         if (decoder == null) {
diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/storage/netcdf/NetcdfStoreProvider.java
b/storage/sis-netcdf/src/main/java/org/apache/sis/storage/netcdf/NetcdfStoreProvider.java
index cd8c5bd..e456e86 100644
--- a/storage/sis-netcdf/src/main/java/org/apache/sis/storage/netcdf/NetcdfStoreProvider.java
+++ b/storage/sis-netcdf/src/main/java/org/apache/sis/storage/netcdf/NetcdfStoreProvider.java
@@ -275,7 +275,7 @@ public class NetcdfStoreProvider extends DataStoreProvider {
         if (input != null) try {
             decoder = new ChannelDecoder(input, connector.getOption(OptionKey.ENCODING),
geomlib, listeners);
             keepOpen = input;
-        } catch (DataStoreException e) {
+        } catch (DataStoreException | ArithmeticException e) {
             final String path = connector.getStorageAs(String.class);
             if (path != null) try {
                 decoder = createByReflection(path, false, geomlib, listeners);
diff --git a/storage/sis-netcdf/src/test/java/org/apache/sis/internal/netcdf/TestCase.java
b/storage/sis-netcdf/src/test/java/org/apache/sis/internal/netcdf/TestCase.java
index bf9e281..03497f3 100644
--- a/storage/sis-netcdf/src/test/java/org/apache/sis/internal/netcdf/TestCase.java
+++ b/storage/sis-netcdf/src/test/java/org/apache/sis/internal/netcdf/TestCase.java
@@ -37,7 +37,8 @@ import static org.junit.Assert.*;
 
 
 /**
- * Base class of netCDF tests. Subclasses shall override the {@link #createDecoder(TestData)}.
+ * Base class of netCDF tests. The base class uses the UCAR decoder, which is taken as a
reference implementation.
+ * Subclasses testing Apache SIS implementation needs to override the {@link #createDecoder(TestData)}.
  *
  * <p>This class is <strong>not</strong> thread safe - do not run subclasses
in parallel.</p>
  *
@@ -60,6 +61,9 @@ public abstract strictfp class TestCase extends org.apache.sis.test.TestCase
{
 
     /**
      * The decoders cached by {@link #selectDataset(TestData)}.
+     * The make is non-empty only during the execution of a single test class.
+     *
+     * @see #closeAllDecoders()
      */
     private static final Map<TestData,Decoder> DECODERS = new EnumMap<>(TestData.class);
 
diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/ChannelDataInput.java
b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/ChannelDataInput.java
index db4e0ca..6e33098 100644
--- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/ChannelDataInput.java
+++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/ChannelDataInput.java
@@ -823,15 +823,23 @@ public class ChannelDataInput extends ChannelData {
      * @return the string decoded from the {@code length} next bytes.
      * @throws IOException if an error occurred while reading the bytes, or if the given
encoding is invalid.
      */
-    public final String readString(final int length, final Charset encoding) throws IOException
{
+    public final String readString(int length, final Charset encoding) throws IOException
{
+        final byte[] array;
+        int position;
         if (buffer.hasArray() && length <= buffer.capacity()) {
             ensureBufferContains(length);
-            final int position = buffer.position(); // Must be after 'ensureBufferContains(int)'.
+            position = buffer.position();           // Must be after 'ensureBufferContains(int)'.
             buffer.position(position + length);     // Before 'new String' for consistency
with the 'else' block in case of UnsupportedEncodingException.
-            return new String(buffer.array(), buffer.arrayOffset() + position, length, encoding);
+            array = buffer.array();
+            position += buffer.arrayOffset();
         } else {
-            return new String(readBytes(length), encoding);
+            array = readBytes(length);
+            position = 0;
         }
+        while (length > 0 && array[position + (length - 1)] == 0) {
+            length--;                               // Skip trailing 0 (end of string in
C/C++ languages).
+        }
+        return new String(array, position, length, encoding);
     }
 
     /**
diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/HyperRectangleReader.java
b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/HyperRectangleReader.java
index e2ddb66..e2b0859 100644
--- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/HyperRectangleReader.java
+++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/HyperRectangleReader.java
@@ -33,7 +33,7 @@ import org.apache.sis.util.Debug;
  *
  * @author  Johann Sorel (Geomatys)
  * @author  Martin Desruisseaux (Geomatys)
- * @version 0.8
+ * @version 1.0
  * @since   0.7
  * @module
  */
@@ -116,14 +116,15 @@ public final class HyperRectangleReader {
      * @throws IOException if an error occurred while transferring data from the channel.
      */
     public Object read(final Region region) throws IOException {
-        final int contiguousDataLength = region.targetLength(region.contiguousDataDimension);
-        final long[] strides = new long[region.getDimension() - region.contiguousDataDimension];
+        final int contiguousDataDimension = region.contiguousDataDimension();
+        final int contiguousDataLength = region.targetLength(contiguousDataDimension);
+        final long[] strides = new long[region.getDimension() - contiguousDataDimension];
         final int[]   cursor = new int[strides.length];
         final int  sizeShift = reader.dataSizeShift();
         long  streamPosition = origin + (region.startAt << sizeShift);
         int    arrayPosition = 0;
         for (int i=0; i<strides.length; i++) {
-            strides[i] = (region.skips[i + region.contiguousDataDimension] + contiguousDataLength)
<< sizeShift;
+            strides[i] = (region.skips[i + contiguousDataDimension] + contiguousDataLength)
<< sizeShift;
             assert (strides[i] > 0) : i;
         }
         try {
@@ -141,7 +142,7 @@ loop:       do {
                      * new row, or a new plane, or a new cube?). This determine how many
bytes we have to
                      * skip.
                      */
-                    if (++cursor[i] < region.targetSize[region.contiguousDataDimension
+ i]) {
+                    if (++cursor[i] < region.targetSize[contiguousDataDimension + i])
{
                         streamPosition += strides[i];
                         arrayPosition  += contiguousDataLength;
                         continue loop;
diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/Region.java
b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/Region.java
index 345aeb3..486d425 100644
--- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/Region.java
+++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/Region.java
@@ -16,6 +16,8 @@
  */
 package org.apache.sis.internal.storage.io;
 
+import org.apache.sis.internal.util.Numerics;
+
 
 /**
  * A sub-area in a <var>n</var>-dimensional hyper-rectangle, optionally with
sub-sampling.
@@ -29,17 +31,12 @@ package org.apache.sis.internal.storage.io;
  *
  * @author  Johann Sorel (Geomatys)
  * @author  Martin Desruisseaux (Geomatys)
- * @version 0.7
+ * @version 1.0
  * @since   0.7
  * @module
  */
 public final class Region {
     /**
-     * Total length of the sequence of values, ignoring sub-area and sub-sampling.
-     */
-    final long sourceLength;
-
-    /**
      * The size after reading only the sub-region at the given sub-sampling.
      * The length of this array is the hyper-rectangle dimension.
      *
@@ -54,7 +51,7 @@ public final class Region {
     final long startAt;
 
     /**
-     * Number of values to skip after having read a values.
+     * Number of values to skip after having read values.
      *
      * <ol>
      *   <li>{@code skips[0]} is the number of values to skip after each single value
on the same line.</li>
@@ -69,12 +66,6 @@ public final class Region {
     final long[] skips;
 
     /**
-     * Number of dimensions for which we can collapse the read operations in a single operation
because their
-     * data are contiguous. This is the index of the first non-zero element in the {@link
#skips} array.
-     */
-    final int contiguousDataDimension;
-
-    /**
      * Creates a new region. It is caller's responsibility to ensure that:
      * <ul>
      *   <li>all arrays have the same length</li>
@@ -82,6 +73,7 @@ public final class Region {
      *   <li>{@code regionLower[i] >= 0} for all <var>i</var></li>
      *   <li>{@code regionLower[i] < regionUpper[i] <= size[i]} for all <var>i</var></li>
      *   <li>{@code subsamplings[i] > 0} for all <var>i</var></li>
+     *   <li>The total length of data to read does not exceed {@link Integer#MAX_VALUE}.</li>
      * </ul>
      *
      * @param size          the number of elements along each dimension.
@@ -97,27 +89,35 @@ public final class Region {
         long stride   = 1;
         long skip     = 0;
         for (int i=0; i<dimension;) {
-            final int  step  =  subsamplings[i];
-            final long lower =  regionLower [i];
-            final long count = (regionUpper [i] - lower + (step-1)) / step;      // (upper-lower)/step
rounded toward up, provided all values are positive.
+            final int  step  = subsamplings[i];
+            final long lower =  regionLower[i];
+            final long count = Numerics.ceilDiv(regionUpper[i] - lower, step);
             final long upper = lower + ((count-1) * step + 1);
             final long span  = size[i];
             assert (count > 0) && (lower >= 0) && (upper > lower)
&& (upper <= span) : i;
             targetSize[i] = Math.toIntExact(count);
 
-            position += stride * lower;
-            skip     += stride * (span - (upper - lower));
-            skips[i] += stride * (step - 1);
-            stride   *= span;
+            position = Math.addExact(position, Math.multiplyExact(stride, lower));
+            skip     = Math.addExact(skip,     Math.multiplyExact(stride, span - (upper -
lower)));
+            skips[i] = Math.addExact(skips[i], Math.multiplyExact(stride, step - 1));
+            stride   = Math.multiplyExact(stride, span);
             skips[++i] = skip;
         }
         startAt = position;
-        sourceLength = stride;
-        int i;
-        for (i=0; i<dimension; i++) {
-            if (skips[i] != 0) break;
-        }
-        contiguousDataDimension = i;
+    }
+
+    /**
+     * Increases the number of bytes that need to be skipped before incrementing the index
+     * in the last dimension of the hyper-cube. Current implementation allows to alter the
+     * reading for the last dimension only, because this is the only dimension needed for
+     * supporting netCDF "unlimited" dimension. Future versions may expand to other dimensions
+     * if needed.
+     *
+     * @param n  number of bytes to skip.
+     */
+    public void skipAfterLastDimension(final long n) {
+        final int i = skips.length - 2;                     // Reminder: skips.length ==
dimension + 1.
+        skips[i] = Math.addExact(skips[i], n);
     }
 
     /**
@@ -130,6 +130,19 @@ public final class Region {
     }
 
     /**
+     * Number of dimensions for which we can collapse the read operations in a single operation
because their
+     * data are contiguous. This is the index of the first non-zero element in the {@link
#skips} array.
+     */
+    final int contiguousDataDimension() {
+        final int dimension = skips.length - 1;
+        int i;
+        for (i=0; i<dimension; i++) {
+            if (skips[i] != 0) break;
+        }
+        return i;
+    }
+
+    /**
      * Returns the total number of values to be read from the sub-region while applying the
sub-sampling.
      * This method takes in account only the given number of dimensions.
      */


Mime
View raw message