You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sis.apache.org by de...@apache.org on 2017/04/13 13:11:41 UTC
svn commit: r1791248 - in /sis/branches/JDK8:
core/sis-feature/src/test/java/org/apache/sis/feature/
storage/sis-storage/src/main/java/org/apache/sis/storage/
storage/sis-storage/src/test/java/org/apache/sis/storage/
Author: desruisseaux
Date: Thu Apr 13 13:11:41 2017
New Revision: 1791248
URL: http://svn.apache.org/viewvc?rev=1791248&view=rev
Log:
Bug fix: when adding "A" and "myNameSpace:A" properties, then asking for "A", FeatureNaming should find the former.
Modified:
sis/branches/JDK8/core/sis-feature/src/test/java/org/apache/sis/feature/DefaultFeatureTypeTest.java
sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/storage/FeatureNaming.java
sis/branches/JDK8/storage/sis-storage/src/test/java/org/apache/sis/storage/FeatureNamingTest.java
Modified: sis/branches/JDK8/core/sis-feature/src/test/java/org/apache/sis/feature/DefaultFeatureTypeTest.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-feature/src/test/java/org/apache/sis/feature/DefaultFeatureTypeTest.java?rev=1791248&r1=1791247&r2=1791248&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-feature/src/test/java/org/apache/sis/feature/DefaultFeatureTypeTest.java [UTF-8] (original)
+++ sis/branches/JDK8/core/sis-feature/src/test/java/org/apache/sis/feature/DefaultFeatureTypeTest.java [UTF-8] Thu Apr 13 13:11:41 2017
@@ -48,6 +48,14 @@ import org.opengis.feature.AttributeType
@DependsOn(DefaultAttributeTypeTest.class)
public final strictfp class DefaultFeatureTypeTest extends TestCase {
/**
+ * Convenience method returning the given name in a a property map
+ * to be given to {@link AbstractIdentifiedType} constructor.
+ */
+ private static Map<String,?> name(final Object name) {
+ return singletonMap(AbstractIdentifiedType.NAME_KEY, name);
+ }
+
+ /**
* Creates a simple feature type without super-types.
* The feature contains the following attributes:
*
@@ -62,10 +70,7 @@ public final strictfp class DefaultFeatu
final Map<String,Object> identification = new HashMap<>();
final DefaultAttributeType<String> city = DefaultAttributeTypeTest.city(identification);
final DefaultAttributeType<Integer> population = DefaultAttributeTypeTest.population(identification);
-
- identification.clear();
- assertNull(identification.put(DefaultFeatureType.NAME_KEY, "City"));
- return new DefaultFeatureType(identification, false, null, city, population);
+ return new DefaultFeatureType(name("City"), false, null, city, population);
}
/**
@@ -81,7 +86,7 @@ public final strictfp class DefaultFeatu
* @return the feature for an university city.
*/
public static DefaultFeatureType universityCity() {
- return new DefaultFeatureType(singletonMap(DefaultFeatureType.NAME_KEY, "University city"), false,
+ return new DefaultFeatureType(name("University city"), false,
new DefaultFeatureType[] {city()}, DefaultAttributeTypeTest.universities());
}
@@ -98,7 +103,7 @@ public final strictfp class DefaultFeatu
* @return the feature for a capital.
*/
public static DefaultFeatureType capital() {
- return new DefaultFeatureType(singletonMap(DefaultFeatureType.NAME_KEY, "Capital"), false,
+ return new DefaultFeatureType(name("Capital"), false,
new DefaultFeatureType[] {city()}, DefaultAttributeTypeTest.parliament());
}
@@ -121,10 +126,8 @@ public final strictfp class DefaultFeatu
assertNull(identification.put(DefaultFeatureType.NAME_KEY + "_fr", "M�tropole"));
return new DefaultFeatureType(identification, false,
new DefaultFeatureType[] {city()},
- new DefaultAttributeType<>(singletonMap(DefaultAttributeType.NAME_KEY, "region"),
- CharSequence.class, 1, 1, null),
- new DefaultAttributeType<>(singletonMap(DefaultAttributeType.NAME_KEY, "isGlobal"),
- Boolean.class, 1, 1, null));
+ new DefaultAttributeType<>(name("region"), CharSequence.class, 1, 1, null),
+ new DefaultAttributeType<>(name("isGlobal"), Boolean.class, 1, 1, null));
}
/**
@@ -142,13 +145,13 @@ public final strictfp class DefaultFeatu
private static DefaultFeatureType worldMetropolis(final DefaultFeatureType metropolis,
final DefaultFeatureType universityCity, final DefaultAttributeType<?> temperature, final Class<?> regionType)
{
- return new DefaultFeatureType(singletonMap(DefaultFeatureType.NAME_KEY, "World metropolis"), false,
- new DefaultFeatureType[] { // Super types
+ return new DefaultFeatureType(name("World metropolis"), false,
+ new DefaultFeatureType[] { // Super types
metropolis,
universityCity
},
- new DefaultAttributeType<?>[] { // Properties
- new DefaultAttributeType<>(singletonMap(DefaultAttributeType.NAME_KEY, "region"), regionType, 1, 1, null),
+ new DefaultAttributeType<?>[] { // Properties
+ new DefaultAttributeType<>(name("region"), regionType, 1, 1, null),
temperature
});
}
@@ -267,12 +270,10 @@ public final strictfp class DefaultFeatu
final int minimumOccurs, final int maximumOccurs)
{
final DefaultAttributeType<String> festival = new DefaultAttributeType<>(
- singletonMap(DefaultAttributeType.NAME_KEY, "festival"),
- String.class, minimumOccurs, maximumOccurs, null);
+ name("festival"), String.class, minimumOccurs, maximumOccurs, null);
final DefaultFeatureType complex = new DefaultFeatureType(
- singletonMap(DefaultAttributeType.NAME_KEY, "Festival"),
- false, null, city, population, festival);
+ name("Festival"), false, null, city, population, festival);
assertUnmodifiable(complex);
final Collection<PropertyType> properties = complex.getProperties(false);
@@ -300,16 +301,12 @@ public final strictfp class DefaultFeatu
@Test
@DependsOnMethod("testSimple")
public void testNameCollision() {
- final DefaultAttributeType<String> city = new DefaultAttributeType<>(
- singletonMap(DefaultAttributeType.NAME_KEY, "name"), String.class, 1, 1, null);
- final DefaultAttributeType<Integer> cityId = new DefaultAttributeType<>(
- singletonMap(DefaultAttributeType.NAME_KEY, "name"), Integer.class, 1, 1, null);
- final DefaultAttributeType<Integer> population = new DefaultAttributeType<>(
- singletonMap(DefaultAttributeType.NAME_KEY, "population"), Integer.class, 1, 1, null);
+ final DefaultAttributeType<String> city = new DefaultAttributeType<>(name("name"), String.class, 1, 1, null);
+ final DefaultAttributeType<Integer> cityId = new DefaultAttributeType<>(name("name"), Integer.class, 1, 1, null);
+ final DefaultAttributeType<Integer> population = new DefaultAttributeType<>(name("population"), Integer.class, 1, 1, null);
- final Map<String,String> identification = singletonMap(DefaultAttributeType.NAME_KEY, "City");
try {
- final Object t = new DefaultFeatureType(identification, false, null, city, population, cityId);
+ final Object t = new DefaultFeatureType(name("City"), false, null, city, population, cityId);
fail("Duplicated attribute names shall not be allowed:\n" + t);
} catch (IllegalArgumentException e) {
final String message = e.getMessage();
@@ -329,17 +326,16 @@ public final strictfp class DefaultFeatu
public void testQualifiedNames() {
final NameFactory factory = DefaultFactories.forBuildin(NameFactory.class);
final DefaultAttributeType<String> city = new DefaultAttributeType<>(
- singletonMap(DefaultAttributeType.NAME_KEY, factory.createGenericName(null, "ns1", "name")),
+ name(factory.createGenericName(null, "ns1", "name")),
String.class, 1, 1, null);
final DefaultAttributeType<Integer> cityId = new DefaultAttributeType<>(
- singletonMap(DefaultAttributeType.NAME_KEY, factory.createGenericName(null, "ns2", "name")),
+ name(factory.createGenericName(null, "ns2", "name")),
Integer.class, 1, 1, null);
final DefaultAttributeType<Integer> population = new DefaultAttributeType<>(
- singletonMap(DefaultAttributeType.NAME_KEY, factory.createGenericName(null, "ns1", "population")),
+ name(factory.createGenericName(null, "ns1", "population")),
Integer.class, 1, 1, null);
final DefaultFeatureType feature = new DefaultFeatureType(
- singletonMap(DefaultAttributeType.NAME_KEY, "City"),
- false, null, city, cityId, population);
+ name("City"), false, null, city, cityId, population);
final Iterator<PropertyType> it = feature.getProperties(false).iterator();
assertSame ("properties[0]", city, it.next());
@@ -362,13 +358,35 @@ public final strictfp class DefaultFeatu
}
/**
+ * Tests two names having the same tip, but where only one of the two names have a namespace.
+ *
+ * @since 0.8
+ */
+ @Test
+ @DependsOnMethod("testQualifiedNames")
+ public void testQualifiedAndUnqualifiedNames() {
+ final NameFactory factory = DefaultFactories.forBuildin(NameFactory.class);
+ final DefaultAttributeType<String> a1 = new DefaultAttributeType<>(
+ name(factory.createGenericName(null, "sis", "identifier")),
+ String.class, 1, 1, null);
+ final DefaultAttributeType<String> a2 = new DefaultAttributeType<>(
+ name(factory.createGenericName(null, "identifier")),
+ String.class, 1, 1, null);
+ final DefaultFeatureType feature = new DefaultFeatureType(
+ name("City"), false, null, a1, a2);
+
+ assertSame("sis:identifier", a1, feature.getProperty("sis:identifier"));
+ assertSame( "identifier", a2, feature.getProperty( "identifier"));
+ }
+
+ /**
* Tests inclusion of a property of kind operation.
*/
@Test
public void testOperationProperty() {
- final Map<String,String> featureName = singletonMap(DefaultFeatureType.NAME_KEY, "Identified city");
- final Map<String,String> identifierName = singletonMap(DefaultAttributeType.NAME_KEY, "identifier");
- final DefaultFeatureType[] parent = {city()};
+ final Map<String,?> featureName = name("Identified city");
+ final Map<String,?> identifierName = name("identifier");
+ final DefaultFeatureType[] parent = {city()};
final DefaultFeatureType city = new DefaultFeatureType(featureName, false,
parent, new LinkOperation(identifierName, parent[0].getProperty("city")));
assertPropertiesEquals(city, true, "city", "population", "identifier");
@@ -428,9 +446,9 @@ public final strictfp class DefaultFeatu
final DefaultFeatureType metropolis = metropolis();
final DefaultFeatureType capital = capital(); // Tested by 'testComplex()'.
final DefaultFeatureType metroCapital = new DefaultFeatureType(
- singletonMap(DefaultFeatureType.NAME_KEY, "Metropolis and capital"), false,
+ name("Metropolis and capital"), false,
new DefaultFeatureType[] {metropolis, capital},
- new DefaultAttributeType<>(singletonMap(DefaultAttributeType.NAME_KEY, "country"),
+ new DefaultAttributeType<>(name("country"),
String.class, 1, 1, null));
assertUnmodifiable(metroCapital);
@@ -508,7 +526,7 @@ public final strictfp class DefaultFeatu
@DependsOnMethod("testPropertyOverride")
public void testPropertyDuplication() {
DefaultFeatureType city = city();
- city = new DefaultFeatureType(singletonMap(DefaultFeatureType.NAME_KEY, "New-City"),
+ city = new DefaultFeatureType(name("New-City"),
false, new DefaultFeatureType[] {city()}, city.getProperty("city"));
assertPropertiesEquals(city, false);
Modified: sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/storage/FeatureNaming.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/storage/FeatureNaming.java?rev=1791248&r1=1791247&r2=1791248&view=diff
==============================================================================
--- sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/storage/FeatureNaming.java [UTF-8] (original)
+++ sis/branches/JDK8/storage/sis-storage/src/main/java/org/apache/sis/storage/FeatureNaming.java [UTF-8] Thu Apr 13 13:11:41 2017
@@ -100,11 +100,12 @@ import org.apache.sis.internal.storage.R
public class FeatureNaming<E> {
/**
* All aliases found for all names given to the {@link #add(DataStore, GenericName, Object)} method.
- * Keys are aliases (never the explicitely given names) and values are names for which the key is an alias.
+ * Keys are aliases and values are the complete names for which the key is an alias.
* Each {@code List<String>} instance contains exactly one name if there is no ambiguity.
* If the list contains more than one name, this means that the alias is ambiguous.
*
- * <p>Note that this map always have less entries than {@link #values} map.</p>
+ * <p>For saving space in common cases, a missing entry in this map is interpreted as synonymous of
+ * {@code (name, Collections.singletonList(name))} entry.</p>
*/
private final Map<String, List<String>> aliases;
@@ -185,27 +186,47 @@ public class FeatureNaming<E> {
ArgumentChecks.ensureNonNull("name", name);
ArgumentChecks.ensureNonNull("value", value);
final String key = name.toString();
- if (values.putIfAbsent(key, value) != null) {
- throw new IllegalNameException(locale(store), Resources.Keys.FeatureAlreadyPresent_2, name(store), key);
+ final E previous = values.put(key, value);
+ if (previous != null) {
+ final List<String> fullNames = aliases.get(key); // Null is synonymous of singletonList(key).
+ if (fullNames == null || fullNames.contains(key)) {
+ /*
+ * If we already had a value for the given name and if that value was associated to a user-specified
+ * name (not to an alias of that name), then we have a name collision. Restore the previous value.
+ */
+ if (values.put(key, previous) != value) {
+ throw new ConcurrentModificationException(name(store).toString()); // Paranoiac check.
+ }
+ throw new IllegalNameException(locale(store), Resources.Keys.FeatureAlreadyPresent_2, name(store), key);
+ }
+ CollectionsExt.addToMultiValuesMap(aliases, key, key);
}
+ /*
+ * At this point we associated the value to the given name. Now try to associate the same value to all aliases.
+ * In this process, we may discover that some slots are already filled by other values.
+ */
while (name instanceof ScopedName) {
name = ((ScopedName) name).tail();
final String alias = name.toString();
- if (CollectionsExt.addToMultiValuesMap(aliases, alias, key).size() > 1) {
+ final List<String> fullNames = CollectionsExt.addToMultiValuesMap(aliases, alias, key);
+ if (fullNames.size() > 1) {
/*
* If there is more than one GenericName for the same alias, we have an ambiguity.
- * Remove any value associated to that alias. The 'get' method in this class will
- * know that the value is missing because of ambiguous name.
+ * Remove any value associated to that alias, unless the value was associated to
+ * exactly the user-specified name (not an alias). The 'get' method in this class
+ * will know when the value is missing because of ambiguous name.
*/
- values.remove(alias);
- } else if (values.put(alias, value) != null) {
+ if (!fullNames.contains(alias)) {
+ values.remove(alias);
+ }
+ } else if (values.putIfAbsent(alias, value) != null) {
/*
- * If no previous GenericName existed for that alias, then no value should exist for that alias.
- * If a previous value existed, then (assuming that we do not have a bug in our algorithm) this
- * object changed in a concurrent thread during this method execution. We do not try to detect
- * all such errors, but this check is an easy one to perform opportunistically.
+ * If a value already existed for that alias but the alias was not declared in the 'aliases' map,
+ * this means that the list was implicitly Collections.singletonList(key). Since we now have an
+ * additional value, we need to add explicitly the previously implicit value.
*/
- throw new ConcurrentModificationException(name(store).toString());
+ assert !fullNames.contains(alias) : alias;
+ CollectionsExt.addToMultiValuesMap(aliases, alias, alias);
}
}
}
@@ -222,16 +243,28 @@ public class FeatureNaming<E> {
* representation, but for which {@link ScopedName#tail()} returns different values.
*/
public boolean remove(final DataStore store, GenericName name) throws IllegalNameException {
- ArgumentChecks.ensureNonNull("name", name);
+ ArgumentChecks.ensureNonNull("name", name);
final String key = name.toString();
if (values.remove(key) == null) {
return false;
}
+ List<String> remaining = CollectionsExt.removeFromMultiValuesMap(aliases, key, key);
+ if (remaining != null && remaining.size() == 1) {
+ /*
+ * If there is exactly one remaining element, that element is a non-ambiguous alias.
+ * So we can associate the value to that alias.
+ */
+ final String select = remaining.get(0);
+ assert !select.equals(key) : select; // Should have been removed by removeFromMultiValuesMap(\u2026).
+ if (values.put(key, values.get(select)) != null) {
+ throw new ConcurrentModificationException(name(store).toString()); // Paranoiac check.
+ }
+ }
boolean error = false;
while (name instanceof ScopedName) {
name = ((ScopedName) name).tail();
final String alias = name.toString();
- final List<String> remaining = CollectionsExt.removeFromMultiValuesMap(aliases, alias, key);
+ remaining = CollectionsExt.removeFromMultiValuesMap(aliases, alias, key);
/*
* The list of remaining GenericNames may be empty but should never be null unless the tail
* is inconsistent with the one found by the 'add(\u2026) method. Otherwise if there is exactly
Modified: sis/branches/JDK8/storage/sis-storage/src/test/java/org/apache/sis/storage/FeatureNamingTest.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/storage/sis-storage/src/test/java/org/apache/sis/storage/FeatureNamingTest.java?rev=1791248&r1=1791247&r2=1791248&view=diff
==============================================================================
--- sis/branches/JDK8/storage/sis-storage/src/test/java/org/apache/sis/storage/FeatureNamingTest.java [UTF-8] (original)
+++ sis/branches/JDK8/storage/sis-storage/src/test/java/org/apache/sis/storage/FeatureNamingTest.java [UTF-8] Thu Apr 13 13:11:41 2017
@@ -23,6 +23,7 @@ import org.apache.sis.util.iso.Names;
import org.junit.Test;
import static org.junit.Assert.*;
+import org.opengis.util.LocalName;
/**
@@ -125,6 +126,39 @@ public final strictfp class FeatureNamin
}
/**
+ * Tests two names having the same tip, but where only one of the two names have a namespace.
+ *
+ * @throws IllegalNameException if an unexpected error occurred while adding or getting an element.
+ */
+ @Test
+ @DependsOnMethod("testAmbiguity")
+ public void testQualifiedAndUnqualifiedName() throws IllegalNameException {
+ final DataStoreMock store = new DataStoreMock("testDataStore");
+ final LocalName local = Names.createLocalName(null, null, "A");
+ FeatureNaming<Integer> map = new FeatureNaming<>();
+ map.add(store, local, 3);
+ map.add(store, A, 2);
+ map.add(store, B, 5);
+ map.add(store, otherA, 7);
+ assertEquals("B", Integer.valueOf(5), map.get(store, "B"));
+ assertEquals("A", Integer.valueOf(3), map.get(store, "A"));
+ assertEquals("myNS:A", Integer.valueOf(2), map.get(store, "myNS:A"));
+ assertEquals("other:A", Integer.valueOf(7), map.get(store, "other:A"));
+ /*
+ * Same tests, but with elements added in different order.
+ */
+ map = new FeatureNaming<>();
+ map.add(store, otherA, 7);
+ map.add(store, B, 5);
+ map.add(store, A, 2);
+ map.add(store, local, 3);
+ assertEquals("B", Integer.valueOf(5), map.get(store, "B"));
+ assertEquals("A", Integer.valueOf(3), map.get(store, "A"));
+ assertEquals("myNS:A", Integer.valueOf(2), map.get(store, "myNS:A"));
+ assertEquals("other:A", Integer.valueOf(7), map.get(store, "other:A"));
+ }
+
+ /**
* Tests removing an entry. It should change the state about whether an alias is ambiguous or not.
*
* @throws IllegalNameException if an unexpected error occurred while adding or getting an element.