sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject svn commit: r1594011 - in /sis/branches/JDK8/core/sis-feature/src: main/java/org/apache/sis/feature/ test/java/org/apache/sis/feature/
Date Mon, 12 May 2014 15:43:57 GMT
Author: desruisseaux
Date: Mon May 12 15:43:56 2014
New Revision: 1594011

URL: http://svn.apache.org/r1594011
Log:
FeatureType.isAssignableFrom(FeatureType) needs to be safe. While in theory checking the name
should be sufficient,
in practice we may have FeatureType of the same name (even if the name is scoped) but different
properties. So we
will also check the property names and types.

Modified:
    sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultAssociation.java
    sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultAttribute.java
    sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultFeature.java
    sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultFeatureType.java
    sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/Property.java
    sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/Validator.java
    sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/package-info.java
    sis/branches/JDK8/core/sis-feature/src/test/java/org/apache/sis/feature/DefaultFeatureTypeTest.java

Modified: sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultAssociation.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultAssociation.java?rev=1594011&r1=1594010&r2=1594011&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultAssociation.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultAssociation.java
[UTF-8] Mon May 12 15:43:56 2014
@@ -107,6 +107,11 @@ public class DefaultAssociation extends 
      * <div class="warning"><b>Warning:</b> In a future SIS version, the
argument type may be changed
      * to {@code org.opengis.feature.Feature}. This change is pending GeoAPI revision.</div>
      *
+     * {@section Validation}
+     * The amount of validation performed by this method is implementation dependent.
+     * The current {@code DefaultAssociation} implementation performs only very cheap (if
any) validations.
+     * A more exhaustive verification can be performed by invoking the {@link #validate()}
method.
+     *
      * @param  value The new value, or {@code null}.
      * @throws RuntimeException If this method performs validation and the given value does
not meet the conditions.
      *         <span style="color:firebrick">This exception may be changed to {@code
IllegalPropertyException} in a
@@ -118,7 +123,7 @@ public class DefaultAssociation extends 
         if (value != null) {
             final DefaultFeatureType base = role.getValueType();
             final DefaultFeatureType type = value.getType();
-            if (base != type && !base.isAssignableFrom(type)) {
+            if (base != type && !base.maybeAssignableFrom(type)) {
                 throw new RuntimeException( // TODO: IllegalPropertyException, pending GeoAPI
revision.
                         Errors.format(Errors.Keys.IllegalArgumentClass_3, role.getName(),
base.getName(), type.getName()));
             }
@@ -127,6 +132,22 @@ public class DefaultAssociation extends 
     }
 
     /**
+     * Ensures that the current association value complies with the constraints defined by
the association role.
+     * This method can be invoked explicitly on a single association, or may be invoked implicitly
by a call to
+     * {@link DefaultFeature#validate()}.
+     *
+     * @throws RuntimeException If the current association value violates a constraint.
+     *         <span style="color:firebrick">This exception will be changed to {@code
IllegalPropertyException}
+     *         in a future SIS version.</span>
+     *
+     * @see DefaultFeature#validate()
+     */
+    @Override
+    public void validate() {
+        Validator.ensureValidValue(role, value);
+    }
+
+    /**
      * Returns a shallow copy of this association.
      * The association {@linkplain #getValue() value} is <strong>not</strong>
cloned.
      *

Modified: sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultAttribute.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultAttribute.java?rev=1594011&r1=1594010&r2=1594011&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultAttribute.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultAttribute.java
[UTF-8] Mon May 12 15:43:56 2014
@@ -141,6 +141,7 @@ public class DefaultAttribute<T> extends
      *
      * @see DefaultFeature#validate()
      */
+    @Override
     public void validate() {
         Validator.ensureValidValue(type, value);
     }

Modified: sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultFeature.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultFeature.java?rev=1594011&r1=1594010&r2=1594011&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultFeature.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultFeature.java
[UTF-8] Mon May 12 15:43:56 2014
@@ -318,9 +318,17 @@ public class DefaultFeature implements S
      *         in a future SIS version.</span>
      *
      * @see DefaultAttribute#validate()
+     * @see DefaultAssociation#validate()
      */
     public void validate() {
-        // TODO
+        for (final Map.Entry<String, Object> entry : properties.entrySet()) {
+            final Object value = entry.getValue();
+            if (value instanceof Property) {
+                ((Property) value).validate();
+            } else {
+                Validator.ensureValid(getPropertyType(entry.getKey()), value);
+            }
+        }
     }
 
     /**

Modified: sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultFeatureType.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultFeatureType.java?rev=1594011&r1=1594010&r2=1594011&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultFeatureType.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultFeatureType.java
[UTF-8] Mon May 12 15:43:56 2014
@@ -20,6 +20,8 @@ import java.util.Map;
 import java.util.Set;
 import java.util.List;
 import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.IdentityHashMap;
 import java.util.HashSet;
 import java.util.Arrays;
 import java.util.Collections;
@@ -48,7 +50,7 @@ import org.apache.sis.internal.util.Unmo
  * will be replaced by references to the {@code FeatureType} interface.</div>
  *
  * {@section Naming}
- * The feature type {@linkplain #getName() name} is mandatory and shall be unique. Those
names are the only
+ * The feature type {@linkplain #getName() name} is mandatory and should be unique. Those
names are the main
  * criterion used for deciding if a feature type {@linkplain #isAssignableFrom is assignable
from} another type.
  * Names can be {@linkplain org.apache.sis.util.iso.DefaultScopedName scoped} for avoiding
name collision.
  *
@@ -120,7 +122,7 @@ public class DefaultFeatureType extends 
     private final List<PropertyType> characteristics;
 
     /**
-     * A lookup table for fetching properties by name.
+     * A lookup table for fetching properties by name, including the properties from super-types.
      * This map shall not be modified after construction.
      *
      * @see #getProperty(String)
@@ -129,7 +131,7 @@ public class DefaultFeatureType extends 
 
     /**
      * Indices of properties in an array of properties similar to {@link #characteristics},
-     * but excluding operations.
+     * but excluding operations. This map includes the properties from the super-types.
      *
      * The size of this map may be smaller than the {@link #byName} size.
      * This map shall not be modified after construction.
@@ -205,14 +207,14 @@ public class DefaultFeatureType extends 
     private void computeTransientFields() {
         final int capacity = Containers.hashMapCapacity(characteristics.size());
         isSimple     = true;
-        byName       = new HashMap<>(capacity);
+        byName       = new LinkedHashMap<>(capacity);
         indices      = new HashMap<>(capacity);
         assignableTo = new HashSet<>(4);
         assignableTo.add(getName());
         scanCharacteristicsFrom(this);
-        assignableTo = CollectionsExt.unmodifiableOrCopy(assignableTo);
         byName       = CollectionsExt.unmodifiableOrCopy(byName);
         indices      = CollectionsExt.unmodifiableOrCopy(indices);
+        assignableTo = CollectionsExt.unmodifiableOrCopy(assignableTo);
     }
 
     /**
@@ -303,34 +305,72 @@ public class DefaultFeatureType extends 
     }
 
     /**
+     * Returns {@code true} if this type may be the same or a super-type of the given type,
using only
+     * the name as a criterion. This is a faster check than {@link #isAssignableFrom(DefaultFeatureType)}
+     */
+    final boolean maybeAssignableFrom(final DefaultFeatureType type) {
+        return type.assignableTo.contains(getName());
+    }
+
+    /**
      * Returns {@code true} if this type is same or a super-type of the given type.
-     * The check is based only on the feature type {@linkplain #getName() name}, which shall
be unique.
+     * The check is based mainly on the feature type {@linkplain #getName() name}, which
should be unique.
+     * However as a safety, this method also checks that all properties in this feature type
is assignable
+     * from a property of the same name in the given type.
      *
      * @param  type The type to be checked.
      * @return {@code true} if instances of the given type can be assigned to association
of this type.
      */
     public boolean isAssignableFrom(final DefaultFeatureType type) {
+        if (type == this) {
+            return true; // Optimization for a common case.
+        }
         ArgumentChecks.ensureNonNull("type", type);
-        return type.assignableTo.contains(getName());
+        return maybeAssignableFrom(type) && isSubsetOf(type, new IdentityHashMap<>(4));
     }
 
-    /*
-     * Returns {@code true} if the given {@code base} is assignable from any of the given
types.
-     *
-     * TODO - to be used as a fallback when argument is not an instance of DefaultFeatureType.
+    /**
+     * Return {@code true} if all properties in this type are also properties in the given
type.
      *
-    private static boolean isAssignableFrom(final DefaultFeatureType base, final Iterable<DefaultFeatureType>
types) {
-        for (final DefaultFeatureType type : types) {
-            if (base.getName().equals(type.getName())) {
-                return true;
-            }
-            if (isAssignableFrom(base, type.superTypes())) {
-                return true;
+     * @param type The type to check.
+     * @param done An initially empty map to be used for avoiding infinite recursivity.
+     */
+    private boolean isSubsetOf(final DefaultFeatureType type, final Map<DefaultFeatureType,Boolean>
done) {
+        if (done.put(this, Boolean.TRUE) == null) {
+            /*
+             * Ensures that all properties defined in this feature type is also defined
+             * in the given property, and that the former is assignable from the later.
+             */
+            for (final Map.Entry<String, PropertyType> entry : byName.entrySet()) {
+                final PropertyType property = entry.getValue();
+                final PropertyType other = type.getProperty(entry.getKey());
+                if (property != other) {
+                    if (other == null) {
+                        return false;
+                    }
+                    boolean isAssignable = true;
+                    /*
+                     * TODO: DefaultAttributeType and DefaultAssociationRole to be replaced
by GeoAPI interfaces
+                     *       (pending GeoAPI review).
+                     */
+                    if (property instanceof DefaultAttributeType<?>) {
+                        isAssignable &= (other instanceof DefaultAttributeType<?>)
&&
+                                        ((DefaultAttributeType<?>) property).getValueClass().isAssignableFrom(
+                                        ((DefaultAttributeType<?>) other   ).getValueClass());
+                    }
+                    if (property instanceof DefaultAssociationRole) {
+                        isAssignable &= (other instanceof DefaultAssociationRole) &&
+                                        ((DefaultAssociationRole) property).getValueType().isSubsetOf(
+                                        ((DefaultAssociationRole) other   ).getValueType(),
done);
+                    }
+                    if (!isAssignable) {
+                        return false;
+                    }
+                }
             }
         }
-        return false;
+        return true;
     }
-    */
 
     /**
      * Returns the direct parents of this feature type.
@@ -346,14 +386,16 @@ public class DefaultFeatureType extends 
     }
 
     /**
-     * Returns any feature operation, any feature attribute type and any feature association
role
-     * that carries characteristics of a feature type.
+     * Returns any feature operation, any feature attribute type and any feature association
role that
+     * carries characteristics of a feature type. The returned list does not include the
characteristics
+     * inherited from the {@linkplain #superTypes() super types}.
      *
      * <div class="warning"><b>Warning:</b>
      * The type of list elements will be changed to {@code PropertyType} if and when such
interface
      * will be defined in GeoAPI.</div>
      *
-     * @return Feature operation, attribute type and association role that carries characteristics
of a feature type.
+     * @return Feature operation, attribute type and association role that carries characteristics
of this
+     *         feature type (not including parent types).
      */
     public List<AbstractIdentifiedType> characteristics() {
         return (List) characteristics; // Cast is safe because the list is read-only.

Modified: sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/Property.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/Property.java?rev=1594011&r1=1594010&r2=1594011&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/Property.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/Property.java
[UTF-8] Mon May 12 15:43:56 2014
@@ -30,4 +30,5 @@ package org.apache.sis.feature;
  * @module
  */
 abstract class Property {
+    public abstract void validate();
 }

Modified: sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/Validator.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/Validator.java?rev=1594011&r1=1594010&r2=1594011&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/Validator.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/Validator.java
[UTF-8] Mon May 12 15:43:56 2014
@@ -47,6 +47,19 @@ final class Validator {
 
     /**
      * Ensures that the given value is valid for the given attribute type.
+     * This method delegates to one of the {@code ensureValidValue(…)} methods depending
of the value type.
+     */
+    static void ensureValid(final PropertyType type, final Object value) {
+        if (type instanceof DefaultAttributeType<?>) {
+            ensureValidValue((DefaultAttributeType<?>) type, value);
+        }
+        if (type instanceof DefaultAssociationRole) {
+            ensureValidValue((DefaultAssociationRole) type, (DefaultFeature) value);
+        }
+    }
+
+    /**
+     * Ensures that the given value is valid for the given attribute type.
      */
     static void ensureValidValue(final DefaultAttributeType<?> type, final Object value)
{
         if (value == null) {
@@ -62,4 +75,18 @@ final class Validator {
                     Errors.format(Errors.Keys.IllegalPropertyClass_2, type.getName(), value.getClass()));
         }
     }
+
+    /**
+     * Ensures that the given value is valid for the given association role.
+     */
+    static void ensureValidValue(final DefaultAssociationRole role, final DefaultFeature
value) {
+        if (value == null) {
+            return;
+        }
+        final DefaultFeatureType type = value.getType();
+        if (!role.getValueType().isAssignableFrom(type)) {
+            throw new RuntimeException( // TODO: IllegalAttributeException, pending GeoAPI
revision.
+                    Errors.format(Errors.Keys.IllegalPropertyClass_2, role.getName(), type.getName()));
+        }
+    }
 }

Modified: sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/package-info.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/package-info.java?rev=1594011&r1=1594010&r2=1594011&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/package-info.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/package-info.java
[UTF-8] Mon May 12 15:43:56 2014
@@ -40,8 +40,8 @@
  *       are constrained to the [1 … 1] cardinality. Such simple features are very common.</p></li>
  * </ul>
  *
- * Each feature type has a {@linkplain org.apache.sis.feature.DefaultFeatureType#getName()
name}, which shall be unique.
- * Those names are the only criterion used for checking if a feature type
+ * Each feature type has a {@linkplain org.apache.sis.feature.DefaultFeatureType#getName()
name},
+ * which should be unique. Those names are the main criterion used for checking if a feature
type
  * {@linkplain org.apache.sis.feature.DefaultFeatureType#isAssignableFrom is assignable from}
another type.
  * Names can be {@linkplain org.apache.sis.util.iso.DefaultScopedName scoped} for avoiding
name collision.
  *

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=1594011&r1=1594010&r2=1594011&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] Mon May 12 15:43:56 2014
@@ -52,6 +52,19 @@ public final strictfp class DefaultFeatu
     }
 
     /**
+     * Creates a sub-type of the "city" type with only one additional property,
+     * a string giving the date since the city is a capital.
+     *
+     * <p>We do not specify the country, since this will be the purpose of an other
test class.</p>
+     */
+    static DefaultFeatureType capital() {
+        return new DefaultFeatureType(singletonMap(DefaultFeatureType.NAME_KEY, "capital"),
false,
+                new DefaultFeatureType[] {cityPopulation()},
+                new DefaultAttributeType<>(singletonMap(DefaultAttributeType.NAME_KEY,
"since"),
+                        String.class, 1, 1, null));
+    }
+
+    /**
      * Tests the construction of a simple feature without super-types.
      * A feature is said "simple" if the cardinality of all attributes is [1 … 1].
      */
@@ -74,7 +87,7 @@ public final strictfp class DefaultFeatu
          */
         assertSame(characteristics.get(0), simple.getProperty("city"));
         assertSame(characteristics.get(1), simple.getProperty("population"));
-        assertNull(simple.getProperty("apple"));
+        assertNull(                        simple.getProperty("apple"));
     }
 
     /**
@@ -146,4 +159,40 @@ public final strictfp class DefaultFeatu
             assertTrue(message, message.contains("city"));
         }
     }
+
+    /**
+     * Tests a feature type which inherit from an other feature type.
+     */
+    @Test
+    @DependsOnMethod("testSimple")
+    public void testInheritance() {
+        final DefaultFeatureType capital = capital();
+        final DefaultFeatureType city = cityPopulation();
+
+        // Check based only on name.
+        assertTrue ("maybeAssignableFrom", city.maybeAssignableFrom(capital));
+        assertFalse("maybeAssignableFrom", capital.maybeAssignableFrom(city));
+
+        // Public API.
+        assertTrue ("isAssignableFrom", city.isAssignableFrom(capital));
+        assertFalse("isAssignableFrom", capital.isAssignableFrom(city));
+        /*
+         * Verify content.
+         */
+        List<AbstractIdentifiedType> characteristics = city.characteristics();
+        assertEquals("characteristics.size", 2,            characteristics.size());
+        assertEquals("characteristics[0]",   "city",       characteristics.get(0).getName().toString());
+        assertEquals("characteristics[1]",   "population", characteristics.get(1).getName().toString());
+
+        characteristics = capital.characteristics();
+        assertEquals("characteristics.size", 1,            characteristics.size());
+        assertEquals("characteristics[0]",   "since",      characteristics.get(0).getName().toString());
+        /*
+         * Verify search by name.
+         */
+        assertEquals("city",       capital.getProperty("city")      .getName().toString());
+        assertEquals("population", capital.getProperty("population").getName().toString());
+        assertEquals("since",      capital.getProperty("since")     .getName().toString());
+        assertNull  (              capital.getProperty("apple"));
+    }
 }



Mime
View raw message