sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject svn commit: r1633323 - in /sis/branches/JDK8/core/sis-feature/src: main/java/org/apache/sis/feature/DefaultAssociationRole.java main/java/org/apache/sis/feature/DefaultFeatureType.java test/java/org/apache/sis/feature/DefaultAssociationRoleTest.java
Date Tue, 21 Oct 2014 09:24:44 GMT
Author: desruisseaux
Date: Tue Oct 21 09:24:43 2014
New Revision: 1633323

URL: http://svn.apache.org/r1633323
Log:
Complete the support of cyclic FeatureType graphs.

Modified:
    sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultAssociationRole.java
    sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultFeatureType.java
    sis/branches/JDK8/core/sis-feature/src/test/java/org/apache/sis/feature/DefaultAssociationRoleTest.java

Modified: sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultAssociationRole.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultAssociationRole.java?rev=1633323&r1=1633322&r2=1633323&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultAssociationRole.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-feature/src/main/java/org/apache/sis/feature/DefaultAssociationRole.java
[UTF-8] Tue Oct 21 09:24:43 2014
@@ -17,6 +17,9 @@
 package org.apache.sis.feature;
 
 import java.util.Map;
+import java.util.List;
+import java.util.ArrayList;
+import java.util.IdentityHashMap;
 import org.opengis.util.GenericName;
 import org.opengis.util.InternationalString;
 import org.apache.sis.util.resources.Errors;
@@ -78,19 +81,6 @@ public class DefaultAssociationRole exte
     private volatile transient String titleProperty;
 
     /**
-     * {@code true} if we determined that the feature type name in this association has been
resolved.
-     * A value of {@code true} means that {@link #valueType} is a resolved feature type.
However a value
-     * of {@code false} only means that we are not sure, and that we should check again next
time.
-     *
-     * <div class="note"><b>Note:</b>
-     * Strictly speaking, this field should be declared {@code volatile} since the names
could be
-     * resolved late after construction, after the {@code DefaultAssociationRole} instance
became
-     * used by different threads. However this is not the intended usage of deferred associations.
-     * </div>
-     */
-    private transient boolean isResolved;
-
-    /**
      * Constructs an association to the given feature type. The properties map is given unchanged
to
      * the {@linkplain AbstractIdentifiedType#AbstractIdentifiedType(Map) super-class constructor}.
      * The following table is a reminder of main (not all) recognized map entries:
@@ -140,19 +130,33 @@ public class DefaultAssociationRole exte
 
     /**
      * Constructs an association to a feature type of the given name.
-     * This constructor shall be used only when creating a cyclic graph of {@link DefaultFeatureType}
instances.
+     * This constructor can be used when creating a cyclic graph of {@link DefaultFeatureType}
instances.
+     * In such cases, at least one association needs to be created while its {@code FeatureType}
is not yet available.
      *
      * <div class="note"><b>Example:</b>
-     * A code first creates {@code FeatureType} <var>A</var>, then {@code FeatureType}
<var>B</var>.
-     * It is easy to define an association from <var>B</var> to <var>A</var>
since the later exists
-     * at <var>B</var> creation time. But if one wants to define also the converse
association, i.e.
-     * from <var>A</var> to <var>B</var>, then it is not possible
to create <var>A</var> with the
-     * {@linkplain #DefaultAssociationRole(Map, FeatureType, int, int) above constructor}
since
-     * <var>B</var> does not yet exist. We can only give the <em>name</em>
of <var>B</var> and
-     * let {@link DefaultFeatureType} substitutes that name by the actual instance when the
later
-     * will be known.
+     * The following establishes a bidirectional association between feature types <var>A</var>
and <var>B</var>:
+     *
+     * {@preformat java
+     *   GenericName nameOfA = Names.createTypeName("Feature type A");
+     *   GenericName nameOfB = Names.createTypeName("Feature type B");
+     *   FeatureType typeA = new DefaultFeatureType(nameOfA, false, null,
+     *       new DefaultAssociationRole(Names.createLocalName("Association to B"), nameOfB),
+     *       // More properties if desired.
+     *   );
+     *   FeatureType typeB = new DefaultFeatureType(nameOfB, false, null,
+     *       new DefaultAssociationRole(Names.createLocalName("Association to A"), featureA),
+     *       // More properties if desired.
+     *   );
+     * }
+     *
+     * After the above code completed, the {@linkplain #getValueType() value type} of "<cite>association
to B</cite>"
+     * has been automatically set to the {@code typeB} instance.
      * </div>
      *
+     * Callers shall make sure that the feature types graph will not contain more than one
feature of the given name.
+     * If more than one {@code FeatureType} instance of the given name is found at resolution
time, the selected one
+     * is undetermined.
+     *
      * @param identification The name and other information to be given to this association
role.
      * @param valueType      The name of the type of feature values.
      * @param minimumOccurs  The minimum number of occurrences of the association within
its containing entity.
@@ -179,37 +183,50 @@ public class DefaultAssociationRole exte
      * @return {@code true} if this association references a resolved feature type after
this method call.
      */
     final boolean resolve(final DefaultFeatureType creating) {
-        boolean resolved = isResolved;
-        if (!resolved) {
-            FeatureType type = valueType;
-            if (type instanceof NamedFeatureType) {
-                type = search(creating, type.getName());
+        FeatureType type = valueType;
+        if (type instanceof NamedFeatureType) {
+            final GenericName name = type.getName();
+            if (name.equals(creating.getName())) {
+                type = creating; // This is the most common case.
+            } else {
+                /*
+                 * The feature that we need to resolve is not the one we just created. Maybe
we can find
+                 * this desired feature in an association of the 'creating' feature, instead
than beeing
+                 * the 'creating' feature itself. This is a little bit unusual, but not illegal.
+                 */
+                final List<FeatureType> deferred = new ArrayList<>();
+                type = search(creating, name, deferred);
                 if (type == null) {
-                    return false;
+                    /*
+                     * Did not found the desired FeatureType in the 'creating' instance.
+                     * Try harder, by searching recursively in associations of associations.
+                     */
+                    if (deferred.isEmpty() || (type = deepSearch(deferred, name)) == null)
{
+                        return false;
+                    }
                 }
-                valueType = type;
-            }
-            isResolved = true; // Necessary for avoiding never-ending loop in case of cycle.
-            try {
-                resolved = creating.resolve(type);
-            } finally {
-                isResolved = resolved;
             }
+            valueType = type;
         }
-        return resolved;
+        return true;
     }
 
     /**
-     * Searches in the given {@code feature} for a feature type of the given name.
+     * Searches in the given {@code feature} for an associated feature type of the given
name.
+     * This method does not search recursively in the associations of the associated features.
+     * Such recursive search will be performed by {@link #deepSearch(List, GenericName)}
only
+     * if we do not find the desired feature in the most direct way.
+     *
+     * <p>Current implementation does not check that there is no duplicated names.
+     * See {@link #deepSearch(List, GenericName)} for a rational.</p>
      *
      * @param  feature The feature in which to search.
      * @param  name The name of the feature to search.
+     * @param  deferred Where to store {@code FeatureType}s to be eventually used for a deep
search.
      * @return The feature of the given name, or {@code null} if none.
      */
-    private static FeatureType search(final FeatureType feature, final GenericName name)
{
-        if (name.equals(feature.getName())) {
-            return feature;
-        }
+    @SuppressWarnings("null")
+    private static FeatureType search(final FeatureType feature, final GenericName name,
final List<FeatureType> deferred) {
         /*
          * Search only in associations declared in the given feature, not in inherited associations.
          * The inherited associations will be checked in a separated loop below if we did
not found
@@ -229,6 +246,7 @@ public class DefaultAssociationRole exte
                 if (name.equals(valueType.getName())) {
                     return valueType;
                 }
+                deferred.add(valueType);
             }
         }
         /*
@@ -238,7 +256,10 @@ public class DefaultAssociationRole exte
          * "covariant return type" in the Java language.
          */
         for (FeatureType type : feature.getSuperTypes()) {
-            type = search(type, name);
+            if (name.equals(type.getName())) {
+                return type;
+            }
+            type = search(type, name, deferred);
             if (type != null) {
                 return type;
             }
@@ -247,6 +268,36 @@ public class DefaultAssociationRole exte
     }
 
     /**
+     * Potentially invoked after {@link #search(FeatureType, GenericName, List)} for searching
+     * in associations of associations.
+     *
+     * <p>Current implementation does not check that there is no duplicated names.
Even if we did so,
+     * a graph of feature types may have no duplicated names at this time but some duplicated
names
+     * later. We rather put a warning in {@link #DefaultAssociationRole(Map, GenericName,
int, int)}
+     * javadoc.</p>
+     *
+     * @param  feature The feature in which to search.
+     * @param  name The name of the feature to search.
+     * @param  done The feature types collected by {@link #search(FeatureType, GenericName,
List)}.
+     * @return The feature of the given name, or {@code null} if none.
+     */
+    private static FeatureType deepSearch(final List<FeatureType> deferred, final GenericName
name) {
+        final Map<FeatureType,Boolean> done = new IdentityHashMap<>(8);
+        for (int i=0; i<deferred.size();) {
+            FeatureType valueType = deferred.get(i++);
+            if (done.put(valueType, Boolean.TRUE) == null) {
+                deferred.subList(0, i).clear(); // Discard previous value for making more
room.
+                valueType = search(valueType, name, deferred);
+                if (valueType != null) {
+                    return valueType;
+                }
+                i = 0;
+            }
+        }
+        return null;
+    }
+
+    /**
      * Returns the type of feature values.
      *
      * @return The type of feature values.

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=1633323&r1=1633322&r2=1633323&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] Tue Oct 21 09:24:43 2014
@@ -23,6 +23,7 @@ import java.util.HashSet;
 import java.util.Map;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
+import java.util.IdentityHashMap;
 import java.util.Collection;
 import java.util.Collections;
 import java.io.IOException;
@@ -128,6 +129,8 @@ public class DefaultFeatureType extends 
      * Strictly speaking, this field should be declared {@code volatile} since the names
could
      * be resolved late after construction, after the {@code DefaultFeatureType} instance
became
      * used by different threads. However this is not the intended usage of deferred associations.
+     * Furthermore a wrong value ({@code false} when it should be {@code true}) should only
cause
+     * more computation than needed, without changing the result.
      * </div>
      */
     private transient boolean isResolved;
@@ -243,7 +246,7 @@ public class DefaultFeatureType extends 
             default: this.properties = UnmodifiableArrayList.wrap(Arrays.copyOf(properties,
properties.length, PropertyType[].class)); break;
         }
         computeTransientFields();
-        isResolved = resolve(this, isSimple);
+        isResolved = resolve(this, null, isSimple);
     }
 
     /**
@@ -419,10 +422,11 @@ public class DefaultFeatureType extends 
      * <p>{@code this} shall be the instance in process of being created, not other
instance
      * (i.e. recursive method invocations are performed on the same {@code this} instance).</p>
      *
-     * @param  feature The feature type for which to resolve the properties.
+     * @param  feature  The feature type for which to resolve the properties.
+     * @param  previous Previous results, for avoiding never ending loop.
      * @return {@code true} if all names have been resolved.
      */
-    final boolean resolve(final FeatureType feature) {
+    private boolean resolve(final FeatureType feature, final Map<FeatureType,Boolean>
previous) {
         /*
          * The isResolved field is used only as a cache for skipping completely the DefaultFeatureType
instance if
          * we have determined that there is no unresolved name.  If the given argument is
not a DefaultFeatureType
@@ -431,28 +435,52 @@ public class DefaultFeatureType extends 
          */
         if (feature instanceof DefaultFeatureType) {
             final DefaultFeatureType dt = (DefaultFeatureType) feature;
-            return dt.isResolved = resolve(feature, dt.isResolved);
+            return dt.isResolved = resolve(feature, previous, dt.isResolved);
         } else {
-            return resolve(feature, feature.isSimple());
+            return resolve(feature, previous, feature.isSimple());
         }
     }
 
     /**
-     * Implementation of {@link #resolve(FeatureType)}, also to be invoked from the constructor.
+     * Implementation of {@link #resolve(FeatureType, Map)}, also to be invoked from the
constructor.
      *
      * @param  feature  The feature type for which to resolve the properties.
+     * @param  previous Previous results, for avoiding never ending loop. Initially {@code
null}.
      * @param  resolved {@code true} if we already know that all names are resolved.
      * @return {@code true} if all names have been resolved.
      */
-    private boolean resolve(final FeatureType feature, boolean resolved) {
+    private boolean resolve(final FeatureType feature, Map<FeatureType,Boolean> previous,
boolean resolved) {
         if (!resolved) {
             resolved = true;
             for (final FeatureType type : feature.getSuperTypes()) {
-                resolved &= resolve(type);
+                resolved &= resolve(type, previous);
             }
             for (final PropertyType property : feature.getProperties(false)) {
-                if (property instanceof DefaultAssociationRole) {
-                    resolved &= ((DefaultAssociationRole) property).resolve(this);
+                if (property instanceof FeatureAssociationRole) {
+                    if (property instanceof DefaultAssociationRole) {
+                        if (!((DefaultAssociationRole) property).resolve(this)) {
+                            resolved = false;
+                            continue;
+                        }
+                    }
+                    /*
+                     * Resolve recursively the associated features, with a check against
infinite recursivity.
+                     * If we fall in a loop (for example A → B → C → A), conservatively
returns 'false'. This
+                     * may not be the most accurate answer, but will not cause any more hurt
than checking more
+                     * often than necessary.
+                     */
+                    final FeatureType valueType = ((FeatureAssociationRole) property).getValueType();
+                    if (valueType != this) {
+                        if (previous == null) {
+                            previous = new IdentityHashMap<>(8);
+                        }
+                        Boolean r = previous.put(valueType, Boolean.FALSE);
+                        if (r == null) {
+                            r = resolve(valueType, previous);
+                            previous.put(valueType, r);
+                        }
+                        resolved &= r;
+                    }
                 }
             }
         }

Modified: sis/branches/JDK8/core/sis-feature/src/test/java/org/apache/sis/feature/DefaultAssociationRoleTest.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-feature/src/test/java/org/apache/sis/feature/DefaultAssociationRoleTest.java?rev=1633323&r1=1633322&r2=1633323&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-feature/src/test/java/org/apache/sis/feature/DefaultAssociationRoleTest.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-feature/src/test/java/org/apache/sis/feature/DefaultAssociationRoleTest.java
[UTF-8] Tue Oct 21 09:24:43 2014
@@ -50,6 +50,7 @@ public final strictfp class DefaultAssoc
      *
      * @param cyclic {@code true} if in addition to the association from <var>A</var>
to <var>B</var>,
      *        we also want an association from <var>B</var> to <var>A</var>,
thus creating a cycle.
+     * @return The association to use for testing purpose.
      */
     static DefaultAssociationRole twinTown(final boolean cyclic) {
         final Map<String,?> properties = singletonMap(NAME_KEY, "twin town");
@@ -67,6 +68,7 @@ public final strictfp class DefaultAssoc
      *
      * @param cyclic {@code true} if in addition to the association from <var>A</var>
to <var>B</var>,
      *        we also want an association from <var>B</var> to <var>A</var>,
thus creating a cycle.
+     * @return The association to use for testing purpose.
      */
     static DefaultFeatureType twinTownCity(final boolean cyclic) {
         final DefaultAssociationRole twinTown = twinTown(cyclic);
@@ -80,6 +82,7 @@ public final strictfp class DefaultAssoc
      * @param name     The name as either a {@link String} or a {@link GenericName}.
      * @param parent   A feature type created by {@link DefaultFeatureTypeTest#city()}, or
{@code null}.
      * @param property The association to an other feature.
+     * @return The feature type to use for testing purpose.
      */
     private static DefaultFeatureType createType(final Object name,
             final FeatureType parent, final FeatureAssociationRole... property)
@@ -147,29 +150,50 @@ public final strictfp class DefaultAssoc
         final GenericName nameOfB = DefaultFactories.SIS_NAMES.createTypeName(null, "B");
         final GenericName nameOfC = DefaultFactories.SIS_NAMES.createTypeName(null, "C");
         final GenericName nameOfD = DefaultFactories.SIS_NAMES.createTypeName(null, "D");
-
+        /*
+         * Associations defined only by the FeatureType name.
+         */
         final DefaultAssociationRole toB = new DefaultAssociationRole(singletonMap(NAME_KEY,
"toB"), nameOfB, 1, 1);
         final DefaultAssociationRole toC = new DefaultAssociationRole(singletonMap(NAME_KEY,
"toC"), nameOfC, 1, 1);
         final DefaultAssociationRole toD = new DefaultAssociationRole(singletonMap(NAME_KEY,
"toD"), nameOfD, 1, 1);
-        final DefaultFeatureType   typeA = createType(nameOfA, null, toB);
-        final DefaultFeatureType   typeB = createType(nameOfB, null, toC);
-        final DefaultFeatureType   typeC = createType(nameOfC, null, toD);
-
-        final DefaultAssociationRole toA = new DefaultAssociationRole(singletonMap(NAME_KEY,
"toA"), typeA, 1, 1);
-        final DefaultFeatureType typeD = createType(nameOfD, null, toA, toB, toC, toD);
-
+        final DefaultFeatureType typeA = createType(nameOfA, null, toB);
+        final DefaultFeatureType typeB = createType(nameOfB, null, toC);
+        final DefaultFeatureType typeC = createType(nameOfC, null, toD);
+        /*
+         * Association defined with real FeatureType instance, except for an association
to itself.
+         * Construction of this FeatureType shall cause the resolution of all above FeatureTypes.
+         */
+        final DefaultAssociationRole toAr = new DefaultAssociationRole(singletonMap(NAME_KEY,
"toA"),         typeA, 1, 1);
+        final DefaultAssociationRole toBr = new DefaultAssociationRole(singletonMap(NAME_KEY,
toB.getName()), typeB, 1, 1);
+        final DefaultAssociationRole toCr = new DefaultAssociationRole(singletonMap(NAME_KEY,
toC.getName()), typeC, 1, 1);
+        final DefaultFeatureType typeD = createType(nameOfD, null, toAr, toBr, toCr, toD);
+        /*
+         * Verify the property given to the constructors. There is no reason for those properties
+         * to change as they are not the instances to be replaced by the name resolutions,
but we
+         * verify them as a paranoiac check.
+         */
         assertSame("A.properties", toB, getSingleton(typeA.getProperties(false)));
         assertSame("B.properties", toC, getSingleton(typeB.getProperties(false)));
         assertSame("C.properties", toD, getSingleton(typeC.getProperties(false)));
-        assertSame("D.properties", toA, typeD.getProperty("toA"));
-        assertSame("D.properties", toB, typeD.getProperty("toB"));
-        assertSame("D.properties", toC, typeD.getProperty("toC"));
-        assertSame("D.properties", toD, typeD.getProperty("toD"));
-        assertSame("toA", typeA, toA.getValueType());
-//      assertSame("toB", typeB, toB.getValueType());
-//      assertSame("toC", typeC, toC.getValueType());
-        assertSame("toD", typeD, toD.getValueType());
-
+        assertSame("D.properties", toAr, typeD.getProperty("toA"));
+        assertSame("D.properties", toBr, typeD.getProperty("toB"));
+        assertSame("D.properties", toCr, typeD.getProperty("toC"));
+        assertSame("D.properties", toD,  typeD.getProperty("toD"));
+        /*
+         * CORE OF THIS TEST: verify that the values of toB, toC and toD have been replaced
by the actual
+         * FeatureType instances. Also verify that as a result, toB.equals(toBr) and toC.equals(toCr).
+         */
+        assertSame("toA", typeA, toAr.getValueType());
+        assertSame("toB", typeB, toBr.getValueType());
+        assertSame("toB", typeB, toB .getValueType());
+        assertSame("toC", typeC, toCr.getValueType());
+        assertSame("toC", typeC, toC .getValueType());
+        assertSame("toD", typeD, toD .getValueType());
+        assertEquals("toB", toB, toBr);
+        assertEquals("toC", toC, toCr);
+        /*
+         * Other equality tests, mostly for verifying that we do not fall in an infinite
loop here.
+         */
         assertFalse("equals", typeA.equals(typeD));
         assertFalse("equals", typeD.equals(typeA));
         assertFalse("equals", typeB.equals(typeC));



Mime
View raw message