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 4cba6e649c62b3a8a6a4eb330f8f7f376a12a665
Merge: e13ceef 7b8a6a6
Author: Martin Desruisseaux <martin.desruisse...@geomatys.com>
AuthorDate: Fri Dec 31 02:51:25 2021 +0100
Merge remote-tracking branch 'origin/fix/projection_name_clash' into
geoapi-4.0 together with correction for the bug demonstrated by the new test.
.../sis/feature/builder/FeatureTypeBuilder.java | 13 ++-
.../org/apache/sis/util/resources/Vocabulary.java | 5 +
.../sis/util/resources/Vocabulary.properties | 1 +
.../sis/util/resources/Vocabulary_fr.properties | 1 +
.../org/apache/sis/storage/sql/SQLStoreTest.java | 4 +-
.../java/org/apache/sis/storage/FeatureQuery.java | 113 +++++++++++++++------
.../org/apache/sis/storage/FeatureQueryTest.java | 26 ++++-
7 files changed, 129 insertions(+), 34 deletions(-)
diff --cc
core/sis-feature/src/main/java/org/apache/sis/feature/builder/FeatureTypeBuilder.java
index 55457bc,55457bc..3ab4bdc
---
a/core/sis-feature/src/main/java/org/apache/sis/feature/builder/FeatureTypeBuilder.java
+++
b/core/sis-feature/src/main/java/org/apache/sis/feature/builder/FeatureTypeBuilder.java
@@@ -692,6 -692,6 +692,8 @@@ public class FeatureTypeBuilder extend
/**
* Creates a new {@code AttributeType} builder initialized to the same
characteristics than the given template.
++ * If the new attribute duplicates an existing one (for example if the
same template is used many times),
++ * caller should use the returned builder for modifying some attributes.
*
* @param <V> the compile-time type of values in the {@code
template} argument.
* @param template an existing attribute type to use as a template.
@@@ -795,7 -795,7 +797,9 @@@
/**
* Creates a new {@code FeatureAssociationRole} builder initialized to
the same characteristics
-- * than the given template.
++ * than the given template. If the new association duplicates an existing
one (for example if the
++ * same template is used many times), caller should use the returned
builder for modifying some
++ * associations.
*
* @param template an existing feature association to use as a template.
* @return a builder for an {@code FeatureAssociationRole}, initialized
with the values of the given template.
@@@ -813,6 -813,6 +817,7 @@@
/**
* Adds the given property in the feature type properties.
* The given property shall be an instance of one of the following types:
++ *
* <ul>
* <li>{@link AttributeType}, in which case this method delegate to
{@link #addAttribute(AttributeType)}.</li>
* <li>{@link FeatureAssociationRole}, in which case this method
delegate to {@link #addAssociation(FeatureAssociationRole)}.</li>
@@@ -820,8 -820,8 +825,12 @@@
* this builder does not create new operations.</li>
* </ul>
*
++ * This method does not verify if the given property duplicates an
existing property.
++ * If the same template is used many times, then the caller should use
the returned builder
++ * for modifying some properties.
++ *
* @param template the property to add to the feature type.
-- * @return a builder initialized to the given builder.
++ * @return a builder initialized to the given template.
* In the {@code Operation} case, the builder is a read-only
accessor on the operation properties.
*
* @see #properties()
diff --cc
core/sis-utility/src/main/java/org/apache/sis/util/resources/Vocabulary.java
index 97b2e11,a3fa6f8..0f7d994
---
a/core/sis-utility/src/main/java/org/apache/sis/util/resources/Vocabulary.java
+++
b/core/sis-utility/src/main/java/org/apache/sis/util/resources/Vocabulary.java
@@@ -1285,6 -1265,6 +1285,11 @@@ public final class Vocabulary extends I
public static final short Unnamed = 208;
/**
++ * Unnamed #{0}
++ */
++ public static final short Unnamed_1 = 266;
++
++ /**
* Unspecified
*/
public static final short Unspecified = 209;
diff --cc
core/sis-utility/src/main/java/org/apache/sis/util/resources/Vocabulary.properties
index 3737547,23535c1..62e15b9
---
a/core/sis-utility/src/main/java/org/apache/sis/util/resources/Vocabulary.properties
+++
b/core/sis-utility/src/main/java/org/apache/sis/util/resources/Vocabulary.properties
@@@ -259,6 -255,6 +259,7 @@@ TypeOfResource = Type of resou
Units = Units
Unknown = Unknown
Unnamed = Unnamed
++Unnamed_1 = Unnamed #{0}
Unspecified = Unspecified
Untitled = Untitled
UnavailableContent = Unavailable content.
diff --cc
core/sis-utility/src/main/java/org/apache/sis/util/resources/Vocabulary_fr.properties
index 959ad86,873cf3d..8440b29
---
a/core/sis-utility/src/main/java/org/apache/sis/util/resources/Vocabulary_fr.properties
+++
b/core/sis-utility/src/main/java/org/apache/sis/util/resources/Vocabulary_fr.properties
@@@ -266,6 -262,6 +266,7 @@@ TypeOfResource = Type de resso
Units = Unit\u00e9s
Unknown = Inconnu
Unnamed = Sans nom
++Unnamed_1 = Sans nom \u2116{0}
Unspecified = Non-sp\u00e9cifi\u00e9
Untitled = Sans titre
UnavailableContent = Contenu non-disponible.
diff --cc
storage/sis-sqlstore/src/test/java/org/apache/sis/storage/sql/SQLStoreTest.java
index cd2cb95,094ceb4..5756e44
---
a/storage/sis-sqlstore/src/test/java/org/apache/sis/storage/sql/SQLStoreTest.java
+++
b/storage/sis-sqlstore/src/test/java/org/apache/sis/storage/sql/SQLStoreTest.java
@@@ -380,7 -369,7 +380,7 @@@ public final strictfp class SQLStoreTes
*/
final FeatureSet parks = dataset.findResource("Parks");
final FeatureQuery query = new FeatureQuery();
-- query.setProjection(new
FeatureQuery.NamedExpression(FF.property(desiredProperty)));
++ query.setProjection(FF.property(desiredProperty));
query.setSortBy(FF.sort(FF.property("country"),
SortOrder.DESCENDING),
FF.sort(FF.property(desiredProperty),
SortOrder.ASCENDING));
final FeatureSet subset = parks.subset(query);
@@@ -541,7 -501,7 +541,7 @@@
final FeatureQuery query = new FeatureQuery();
query.setSortBy(FF.sort(FF.property("native_name"),
SortOrder.DESCENDING));
query.setSelection(FF.equal(FF.property("country"),
FF.literal("FRA")));
-- query.setProjection(new
FeatureQuery.NamedExpression(FF.property("native_name")));
++ query.setProjection(FF.property("native_name"));
final FeatureSet frenchParks = parks.subset(query);
/*
* Verify the feature type.
diff --cc
storage/sis-storage/src/main/java/org/apache/sis/storage/FeatureQuery.java
index ffee909,ffee909..9368d53
--- a/storage/sis-storage/src/main/java/org/apache/sis/storage/FeatureQuery.java
+++ b/storage/sis-storage/src/main/java/org/apache/sis/storage/FeatureQuery.java
@@@ -17,6 -17,6 +17,8 @@@
package org.apache.sis.storage;
import java.util.Arrays;
++import java.util.Set;
++import java.util.HashSet;
import java.util.Map;
import java.util.LinkedHashMap;
import java.util.Objects;
@@@ -35,9 -35,9 +37,10 @@@ import org.apache.sis.internal.storage.
import org.apache.sis.filter.DefaultFilterFactory;
import org.apache.sis.filter.Optimization;
import org.apache.sis.util.ArgumentChecks;
--import org.apache.sis.util.Classes;
++import org.apache.sis.util.CharSequences;
import org.apache.sis.util.collection.Containers;
import org.apache.sis.util.iso.Names;
++import org.apache.sis.util.resources.Vocabulary;
// Branch-dependent imports
import org.opengis.feature.Feature;
@@@ -218,9 -218,9 +221,12 @@@ public class FeatureQuery extends Quer
for (int i=0; i<properties.length; i++) {
final NamedExpression c = properties[i];
ArgumentChecks.ensureNonNullElement("properties", i, c);
-- final Object key = c.alias != null ? c.alias : c.expression;
++ Object key = (c.alias != null) ? c.alias : c.expression;
final Integer p = uniques.putIfAbsent(key, i);
if (p != null) {
++ if (key instanceof Expression) {
++ key = label((Expression) key);
++ }
throw new
IllegalArgumentException(Resources.format(Resources.Keys.DuplicatedQueryProperty_3,
key, p, i));
}
}
@@@ -396,6 -396,6 +402,9 @@@
* An expression to be retrieved by a {@code Query}, together with the
name to assign to it.
* In relational database terminology, subset of columns is called
<cite>projection</cite>.
* Columns can be given to the {@link
FeatureQuery#setProjection(NamedExpression[])} method.
++ *
++ * @version 1.2
++ * @since 1.1
*/
public static class NamedExpression implements Serializable {
/**
@@@ -451,30 -451,30 +460,6 @@@
}
/**
-- * Adds in the given builder the type of results computed by this
column.
-- *
-- * @param column index of this column. Used for error message
only.
-- * @param valueType the type of features to be evaluated by the
expression in this column.
-- * @param addTo where to add the type of properties evaluated
by expression in this column.
-- * @throws IllegalArgumentException if this method can operate only
on some feature types
-- * and the given type is not one of them.
-- * @throws InvalidFilterValueException if this method can not
determine the result type of the expression
-- * in this column. It may be because that expression is
backed by an unsupported implementation.
-- *
-- * @see FeatureQuery#expectedType(FeatureType)
-- */
-- final void expectedType(final int column, final FeatureType
valueType, final FeatureTypeBuilder addTo) {
-- final PropertyTypeBuilder resultType =
FeatureExpression.expectedType(expression, valueType, addTo);
-- if (resultType == null) {
-- throw new
InvalidFilterValueException(Resources.format(Resources.Keys.InvalidExpression_2,
--
expression.getFunctionName().toInternationalString(), column));
-- }
-- if (alias != null && !alias.equals(resultType.getName())) {
-- resultType.setName(alias);
-- }
-- }
--
-- /**
* Returns a hash code value for this column.
*
* @return a hash code value.
@@@ -524,7 -524,7 +509,7 @@@
} else if (expression instanceof ValueReference<?,?>) {
buffer.append(((ValueReference<?,?>) expression).getXPath());
} else {
-- buffer.append(Classes.getShortClassName(expression)); //
Class name with enclosing class if any.
++ buffer.append(expression.getFunctionName());
}
if (alias != null) {
buffer.append(" AS “").append(alias).append('”');
@@@ -533,6 -533,6 +518,23 @@@
}
/**
++ * Returns a label for the given expression for reporting to human (e.g.
in exception messages).
++ * This method uses the value reference (XPath) or literal value if
applicable, truncated to an
++ * arbitrary length.
++ */
++ private static String label(final Expression<?,?> expression) {
++ final String text;
++ if (expression instanceof Literal<?,?>) {
++ text = String.valueOf(((Literal<?,?>) expression).getValue());
++ } else if (expression instanceof ValueReference<?,?>) {
++ text = ((ValueReference<?,?>) expression).getXPath();
++ } else {
++ return expression.getFunctionName().toString();
++ }
++ return CharSequences.shortSentence(text, 40).toString();
++ }
++
++ /**
* Applies this query on the given feature set.
* This method is invoked by the default implementation of {@link
FeatureSet#subset(Query)}.
* The default implementation executes the query using the default {@link
java.util.stream.Stream} methods.
@@@ -565,6 -565,6 +567,13 @@@
/**
* Returns the type of values evaluated by this query when executed on
features of the given type.
++ * If some expressions have no name, default names are computed as below:
++ *
++ * <ul>
++ * <li>If the expression is an instance of {@link ValueReference},
++ * the tip of the {@linkplain ValueReference#getXPath()
x-path}.</li>
++ * <li>Otherwise the localized string "Unnamed #1" with increasing
numbers.</li>
++ * </ul>
*
* @param valueType the type of features to be evaluated by the
expressions in this query.
* @return type resulting from expressions evaluation (never null).
@@@ -577,9 -577,9 +586,55 @@@
if (projection == null) {
return valueType; // All columns included: result is of
the same type.
}
++ int unnamedNumber = 0; // Sequential number for unnamed
expressions.
++ Set<String> names = null; // Names already used, for avoiding
collisions.
final FeatureTypeBuilder ftb = new
FeatureTypeBuilder().setName(valueType.getName());
-- for (int i=0; i<projection.length; i++) {
-- projection[i].expectedType(i, valueType, ftb);
++ for (int column = 0; column < projection.length; column++) {
++ /*
++ * For each property, get the expected type (mandatory) and its
name (optional).
++ * A default name will be computed if no alias were explicitly
given by user.
++ */
++ GenericName name = projection[column].alias;
++ final Expression<?,?> expression = projection[column].expression;
++ final PropertyTypeBuilder resultType =
FeatureExpression.expectedType(expression, valueType, ftb);
++ if (resultType == null) {
++ throw new
InvalidFilterValueException(Resources.format(Resources.Keys.InvalidExpression_2,
++
expression.getFunctionName().toInternationalString(), column));
++ }
++ if (name == null) {
++ /*
++ * Build a list of aliases declared by the user, for making
sure that we do not collide with them.
++ * No check for `GenericName` collision here because it was
already verified by `setProjection(…)`.
++ * We may have collision of their `String` representations
however, which is okay.
++ */
++ if (names == null) {
++ names = new
HashSet<>(Containers.hashMapCapacity(projection.length));
++ for (final NamedExpression p : projection) {
++ if (p.alias != null) {
++ names.add(p.alias.tip().toString());
++ }
++ }
++ }
++ /*
++ * The rules for creating a default name are documented in
this method Javadoc.
++ * Note that despite the use of `Vocabulary` resources, the
name will be unlocalized
++ * (for easier programmatic use) because `GenericName`
implementation is designed for
++ * providing localized names only if explicitly requested.
++ */
++ CharSequence text = null;
++ if (expression instanceof ValueReference<?,?>) {
++ String xpath = ((ValueReference<?,?>)
expression).getXPath().trim();
++ xpath = xpath.substring(xpath.lastIndexOf('/') + 1);
// Works also if '/' is not found.
++ if (!(xpath.isEmpty() || names.contains(xpath))) {
++ text = xpath;
++ }
++ }
++ if (text == null) do {
++ text =
Vocabulary.formatInternational(Vocabulary.Keys.Unnamed_1, ++unnamedNumber);
++ } while (!names.add(text.toString()));
++ name = Names.createLocalName(null, null, text);
++ }
++ resultType.setName(name);
}
return ftb.build();
}
diff --cc
storage/sis-storage/src/test/java/org/apache/sis/storage/FeatureQueryTest.java
index 6918ac9,f675e4f..7f6af73
---
a/storage/sis-storage/src/test/java/org/apache/sis/storage/FeatureQueryTest.java
+++
b/storage/sis-storage/src/test/java/org/apache/sis/storage/FeatureQueryTest.java
@@@ -18,6 -18,6 +18,7 @@@ package org.apache.sis.storage
import java.util.List;
import java.util.Arrays;
++import java.util.Iterator;
import java.util.stream.Collectors;
import org.apache.sis.feature.builder.FeatureTypeBuilder;
import org.apache.sis.internal.storage.MemoryFeatureSet;
@@@ -43,7 -44,7 +45,9 @@@ import org.opengis.filter.SortProperty
* Tests {@link FeatureQuery} and (indirectly) {@link FeatureSubset}.
*
* @author Johann Sorel (Geomatys)
-- * @version 1.1
++ * @author Alexis Manin (Geomatys)
++ * @author Martin Desruisseaux (Geomatys)
++ * @version 1.2
* @since 1.0
* @module
*/
@@@ -209,4 -210,15 +213,24 @@@ public final strictfp class FeatureQuer
final PropertyType p =
TestUtilities.getSingleton(result.getType().getProperties(true));
assertEquals("value2", p.getName().toString());
}
+
++ /**
++ * Tests the creation of default column names when no alias where
explicitly specified.
++ * Note that the string representations of default names shall be
unlocalized.
++ *
++ * @throws DataStoreException if an error occurred while executing the
query.
++ */
+ @Test
- public void testColumnNameClash() throws DataStoreException {
++ public void testDefaultColumnName() throws DataStoreException {
+ final FilterFactory<Feature,?,?> ff =
DefaultFilterFactory.forFeatures();
+ query.setProjection(
+ ff.add(ff.property("value1", Number.class), ff.literal(1)),
+ ff.add(ff.property("value2", Number.class), ff.literal(1)));
- final FeatureSet subset = featureSet.subset(query); // NOTE: I would
expect an error here
++ final FeatureSet subset = featureSet.subset(query);
+ final FeatureType type = subset.getType();
- assertEquals(2, type.getProperties(true).size());
++ final Iterator<? extends PropertyType> properties =
type.getProperties(true).iterator();
++ assertEquals("Unnamed #1", properties.next().getName().toString());
++ assertEquals("Unnamed #2", properties.next().getName().toString());
++ assertFalse(properties.hasNext());
+ }
}
