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
The following commit(s) were added to refs/heads/geoapi-4.0 by this push: new 02f7f3f Tune the meaning of "0 occurrence" of features in metadata. Better metadata about features in netCDF metadata. 02f7f3f is described below commit 02f7f3ffa17b23a0f7b747fb7cd5b609210d6d0d Author: Martin Desruisseaux <martin.desruisse...@geomatys.com> AuthorDate: Sun Feb 27 18:38:42 2022 +0100 Tune the meaning of "0 occurrence" of features in metadata. Better metadata about features in netCDF metadata. --- .../org/apache/sis/internal/netcdf/Convention.java | 8 ++-- .../org/apache/sis/internal/netcdf/FeatureSet.java | 2 +- .../apache/sis/internal/netcdf/VariableRole.java | 2 +- .../apache/sis/storage/netcdf/MetadataReader.java | 54 ++++++++++++++-------- .../org/apache/sis/internal/sql/feature/Table.java | 4 +- .../sis/internal/storage/MetadataBuilder.java | 51 +++++++++++--------- .../sis/internal/storage/MetadataBuilderTest.java | 18 +++++--- 7 files changed, 83 insertions(+), 56 deletions(-) diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Convention.java b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Convention.java index b8449c2..f14865a 100644 --- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Convention.java +++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Convention.java @@ -224,8 +224,8 @@ public class Convention { * to confuse them with images.</li> * </ul> * - * <p>The default implementation returns {@link VariableRole#FEATURE} if the given variable may be values - * for one feature property of a feature set. This detection is based on the number of dimensions.</p> + * <p>The default implementation returns {@link VariableRole#FEATURE_PROPERTY} if the given variable may be + * values for one feature property of a feature set. This detection is based on the number of dimensions.</p> * * @param variable the variable for which to get the role. * @return role of the given variable. @@ -236,7 +236,7 @@ public class Convention { } final int n = variable.getNumDimensions(); if (n == 1) { - return VariableRole.FEATURE; + return VariableRole.FEATURE_PROPERTY; } else if (n != 0) { final DataType dataType = variable.getDataType(); int numVectors = 0; // Number of dimension having more than 1 value. @@ -251,7 +251,7 @@ public class Convention { } } if (n == Variable.STRING_DIMENSION && dataType == DataType.CHAR) { - return VariableRole.FEATURE; + return VariableRole.FEATURE_PROPERTY; } } return VariableRole.OTHER; diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/FeatureSet.java b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/FeatureSet.java index da314f7..666ffcb 100644 --- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/FeatureSet.java +++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/FeatureSet.java @@ -304,7 +304,7 @@ final class FeatureSet extends DiscreteSampling { final List<FeatureSet> features = new ArrayList<>(3); // Will usually contain at most one element. final Map<Dimension,Boolean> done = new HashMap<>(); // Whether a dimension has already been used. for (final Variable v : decoder.getVariables()) { - if (v.getRole() != VariableRole.FEATURE) { + if (v.getRole() != VariableRole.FEATURE_PROPERTY) { continue; } /* diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/VariableRole.java b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/VariableRole.java index 97a092d..db5764f 100644 --- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/VariableRole.java +++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/VariableRole.java @@ -46,7 +46,7 @@ public enum VariableRole { /** * The variable is a property of a feature. */ - FEATURE, + FEATURE_PROPERTY, /** * Values of the variable are bounds of values of another variable. diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/storage/netcdf/MetadataReader.java b/storage/sis-netcdf/src/main/java/org/apache/sis/storage/netcdf/MetadataReader.java index 3be066d..405b518 100644 --- a/storage/sis-netcdf/src/main/java/org/apache/sis/storage/netcdf/MetadataReader.java +++ b/storage/sis-netcdf/src/main/java/org/apache/sis/storage/netcdf/MetadataReader.java @@ -21,9 +21,9 @@ import java.net.URISyntaxException; import java.util.Date; import java.util.List; import java.util.Set; -import java.util.LinkedHashSet; import java.util.Map; -import java.util.HashMap; +import java.util.LinkedHashSet; +import java.util.LinkedHashMap; import java.util.Arrays; import java.util.ArrayList; import java.util.Collection; @@ -60,6 +60,7 @@ import org.apache.sis.internal.netcdf.Axis; import org.apache.sis.internal.netcdf.Decoder; import org.apache.sis.internal.netcdf.Variable; import org.apache.sis.internal.netcdf.VariableRole; +import org.apache.sis.internal.netcdf.Dimension; import org.apache.sis.internal.netcdf.Grid; import org.apache.sis.internal.storage.io.IOUtilities; import org.apache.sis.internal.storage.MetadataBuilder; @@ -616,10 +617,9 @@ split: while ((start = CharSequences.skipLeadingWhitespaces(value, start, lengt * @param publisher the publisher names, built by the caller in an opportunist way. */ private void addIdentificationInfo(final Set<InternationalString> publisher) throws IOException, DataStoreException { - boolean hasExtent = false; - Set<String> project = null; - Set<String> standard = null; - boolean hasDataType = false; + boolean hasExtent = false; + Set<String> project = null; + Set<String> standard = null; final Set<String> keywords = new LinkedHashSet<>(); for (final String path : searchPath) { decoder.setSearchPath(path); @@ -630,9 +630,7 @@ split: while ((start = CharSequences.skipLeadingWhitespaces(value, start, lengt addAccessConstraint(forCodeName(Restriction.class, keyword)); } addTopicCategory(forEnumName(TopicCategory.class, stringValue(TOPIC_CATEGORY))); - SpatialRepresentationType dt = forCodeName(SpatialRepresentationType.class, stringValue(DATA_TYPE)); - addSpatialRepresentation(dt); - hasDataType |= (dt != null); + addSpatialRepresentation(forCodeName(SpatialRepresentationType.class, stringValue(DATA_TYPE))); if (!hasExtent) { /* * Takes only ONE extent, because a netCDF file may declare many time the same @@ -643,14 +641,6 @@ split: while ((start = CharSequences.skipLeadingWhitespaces(value, start, lengt } } /* - * Add spatial representation type only if it was not explicitly given in the metadata. - * The call to getGridCandidates() may be relatively costly, so we don't want to invoke - * it without necessity. - */ - if (!hasDataType && decoder.getGridCandidates().length != 0) { - addSpatialRepresentation(SpatialRepresentationType.GRID); - } - /* * For the following properties, use only the first non-empty attribute value found on the search path. */ decoder.setSearchPath(searchPath); @@ -887,7 +877,12 @@ split: while ((start = CharSequences.skipLeadingWhitespaces(value, start, lengt */ @SuppressWarnings("null") private void addContentInfo() { - final Map<List<String>, List<Variable>> contents = new HashMap<>(4); + /* + * Prepare a list of features and coverages, but without writing metadata now. + * We differ metadata writing for giving us a chance to group related contents. + */ + final Set<Dimension> features = new LinkedHashSet<>(); + final Map<List<String>, List<Variable>> coverages = new LinkedHashMap<>(4); for (final Variable variable : decoder.getVariables()) { if (VariableRole.isCoverage(variable)) { final List<org.apache.sis.internal.netcdf.Dimension> dimensions = variable.getGridDimensions(); @@ -895,12 +890,31 @@ split: while ((start = CharSequences.skipLeadingWhitespaces(value, start, lengt for (int i=0; i<names.length; i++) { names[i] = dimensions.get(i).getName(); } - CollectionsExt.addToMultiValuesMap(contents, Arrays.asList(names), variable); + CollectionsExt.addToMultiValuesMap(coverages, Arrays.asList(names), variable); hasGridCoverages = true; + } else if (variable.getRole() == VariableRole.FEATURE_PROPERTY) { + /* + * For feature property, we should take only the first dimension. + * If a second dimension exists, it is for character strings. + */ + features.add(variable.getGridDimensions().get(0)); + } + } + /* + * Now write the metadata. Note that the spatial repersentation types added below are actually + * parts of `DataIdentification` instead of `ContentInformation`, but we add them here because + * we have the information here. + */ + if (!features .isEmpty()) addSpatialRepresentation(SpatialRepresentationType.TEXT_TABLE); + if (!coverages.isEmpty()) addSpatialRepresentation(SpatialRepresentationType.GRID); + for (final Dimension feature : features) { + final String name = feature.getName(); + if (name != null) { + addFeatureType(decoder.nameFactory.createLocalName(decoder.namespace, name), feature.length()); } } final String processingLevel = stringValue(PROCESSING_LEVEL); - for (final List<Variable> group : contents.values()) { + for (final List<Variable> group : coverages.values()) { /* * Instantiate a CoverageDescription for each distinct set of netCDF dimensions * (e.g. longitude,latitude,time). This separation is based on the fact that a diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Table.java b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Table.java index 295cb01..7e5d027 100644 --- a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Table.java +++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Table.java @@ -431,8 +431,8 @@ final class Table extends AbstractFeatureSet { /** * Returns the number of rows, or -1 if unknown. Note that some database drivers returns 0, - * so it is better to consider 0 as "unknown" too. We do not cache this count because it may - * change at any time. + * so it is better to consider 0 as "unknown" too (see {@link FeatureIterator#estimatedSize}). + * We do not cache this count because it may change at any time. * * @param metadata information about the database. * @param distinct whether to count distinct values instead of all values. diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/MetadataBuilder.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/MetadataBuilder.java index c679471..ca6d322 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/MetadataBuilder.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/MetadataBuilder.java @@ -1901,33 +1901,42 @@ parse: for (int i = 0; i < length;) { * Note that the {@link FeatureCatalogBuilder} subclasses can also be used for that chaining. * * @param type the feature type to add, or {@code null} for no-operation. - * @param occurrences number of instances of the given feature type, or a negative value if unknown. Note that if - * this value is 0, it will be considered "unknown", because ISO-19115 consider it invalid. - * However, this is a valid case in practice (it represents an empty dataset at the moment). - * @return the name of the added feature, or {@code null} if none. + * @param occurrences number of instances of the given feature type, or a negative value if unknown. + * Note that ISO-19115 considers 0 as an invalid value. Consequently if 0, the feature is not added. + * @return the name of the added feature (even if not added to the metadata), or {@code null} if none. * * @see FeatureCatalogBuilder#define(FeatureType) */ public final GenericName addFeatureType(final FeatureType type, final long occurrences) { - if (type != null) { - final GenericName name = type.getName(); - if (name != null) { - final DefaultFeatureTypeInfo info = new DefaultFeatureTypeInfo(name); - /* - * Note: Exclude 0 as valid instance count. - * Reason: ISO-19115 consider 0 (empty dataset) as an invalid count. However, in practice, it can happen - * to open data sources that contain no record. They're still valid datasets, because as long as their - * structure is valid, there's no point in raising an error (at our level, without any other context - * information) here. - */ - if (occurrences > 0) { - info.setFeatureInstanceCount(shared((int) Math.min(occurrences, Integer.MAX_VALUE))); - } - addIfNotPresent(featureDescription().getFeatureTypeInfo(), info); - return name; + if (type == null) { + return null; + } + final GenericName name = type.getName(); + addFeatureType(name, occurrences); + return name; + } + + /** + * Adds descriptions for a feature of the given name. + * Storage location is: + * + * <ul> + * <li>{@code metadata/contentInfo/featureTypes/featureTypeName}</li> + * <li>{@code metadata/contentInfo/featureTypes/featureInstanceCount}</li> + * </ul> + * + * @param name name of the feature type to add, or {@code null} for no-operation. + * @param occurrences number of instances of the given feature type, or a negative value if unknown. + * Note that ISO-19115 considers 0 as an invalid value. Consequently if 0, the feature is not added. + */ + public final void addFeatureType(final GenericName name, final long occurrences) { + if (name != null && occurrences != 0) { + final DefaultFeatureTypeInfo info = new DefaultFeatureTypeInfo(name); + if (occurrences > 0) { + info.setFeatureInstanceCount(shared((int) Math.min(occurrences, Integer.MAX_VALUE))); } + addIfNotPresent(featureDescription().getFeatureTypeInfo(), info); } - return null; } /** diff --git a/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/MetadataBuilderTest.java b/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/MetadataBuilderTest.java index 9512a7e..deacdb4 100644 --- a/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/MetadataBuilderTest.java +++ b/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/MetadataBuilderTest.java @@ -42,7 +42,7 @@ import org.opengis.feature.FeatureType; * * @author Martin Desruisseaux (Geomatys) * @author Alexis Manin (Geomatys) - * @version 1.1 + * @version 1.2 * @since 0.8 * @module */ @@ -125,8 +125,8 @@ public final strictfp class MetadataBuilderTest extends TestCase { * Tests {@link MetadataBuilder#addFeatureType(FeatureType, long)}. */ @Test - public void feature_count_should_be_ignored_when_it_is_zero() { - verifyFeatureInstanceCount("Feature count should not be written if it is 0", null, 0); + public void feature_should_be_ignored_when_count_is_zero() { + verifyFeatureInstanceCount("Feature should not be written if count is 0", null, 0); } /** @@ -145,9 +145,13 @@ public final strictfp class MetadataBuilderTest extends TestCase { assertNotNull(name); final DefaultMetadata metadata = builder.build(true); - final ContentInformation content = getSingleton(metadata.getContentInfo()); - assertInstanceOf("Metadata.contentInfo", FeatureCatalogueDescription.class, content); - final FeatureTypeInfo info = getSingleton(((FeatureCatalogueDescription) content).getFeatureTypeInfo()); - assertEquals(errorMessage, expected, info.getFeatureInstanceCount()); + if (valueToInsert == 0) { + assertTrue(metadata.getContentInfo().isEmpty()); + } else { + final ContentInformation content = getSingleton(metadata.getContentInfo()); + assertInstanceOf("Metadata.contentInfo", FeatureCatalogueDescription.class, content); + final FeatureTypeInfo info = getSingleton(((FeatureCatalogueDescription) content).getFeatureTypeInfo()); + assertEquals(errorMessage, expected, info.getFeatureInstanceCount()); + } } }