sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject 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/
Date Thu, 13 Apr 2017 13:11:41 GMT
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(…).
+            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(…) 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.



Mime
View raw message