sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject svn commit: r1456274 - in /sis/branches/JDK7/sis-utility/src: main/java/org/apache/sis/internal/converter/ main/java/org/apache/sis/util/ test/java/org/apache/sis/internal/converter/
Date Wed, 13 Mar 2013 23:26:09 GMT
Author: desruisseaux
Date: Wed Mar 13 23:26:08 2013
New Revision: 1456274

URL: http://svn.apache.org/r1456274
Log:
Final version of FallbackConverter. The class size is a little bit smaller but not that much.
However the internal working and the test case are a little bit more systematic.

Modified:
    sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/ClassPair.java
    sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/CollectionConverter.java
    sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/Column.java
    sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/FallbackConverter.java
    sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/SystemConverter.java
    sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/util/Classes.java
    sis/branches/JDK7/sis-utility/src/test/java/org/apache/sis/internal/converter/FallbackConverterTest.java

Modified: sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/ClassPair.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/ClassPair.java?rev=1456274&r1=1456273&r2=1456274&view=diff
==============================================================================
--- sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/ClassPair.java [UTF-8] (original)
+++ sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/ClassPair.java [UTF-8] Wed Mar 13 23:26:08 2013
@@ -114,7 +114,19 @@ class ClassPair<S,T> implements Serializ
             assert converter.getSourceClass().isAssignableFrom(sourceClass) : sourceClass;
             assert targetClass.isAssignableFrom(converter.getTargetClass()) : targetClass;
         }
-        return (ObjectConverter<S,T>) converter;
+        return (ObjectConverter<? super S, ? extends T>) converter;
+    }
+
+    /**
+     * Returns {@code true} if the source and target classes of the given converter
+     * are strictly equal to the source and target classes of this {@code ClassPair}.
+     *
+     * @param  The converter to check.
+     * @return {@code true} if the given converter is for the same source and target classes.
+     */
+    final boolean isExactlyFor(final ObjectConverter<? super S, ? extends T> converter) {
+        return converter.getSourceClass() == sourceClass &&
+               converter.getTargetClass() == targetClass;
     }
 
     /**

Modified: sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/CollectionConverter.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/CollectionConverter.java?rev=1456274&r1=1456273&r2=1456274&view=diff
==============================================================================
--- sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/CollectionConverter.java [UTF-8] (original)
+++ sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/CollectionConverter.java [UTF-8] Wed Mar 13 23:26:08 2013
@@ -18,10 +18,10 @@ package org.apache.sis.internal.converte
 
 import java.util.Collection;
 import java.util.ArrayList;
+import java.util.EnumSet;
 import java.util.LinkedHashSet;
-import java.io.Serializable;
-import java.io.ObjectStreamException;
 import net.jcip.annotations.Immutable;
+import org.apache.sis.math.FunctionProperty;
 
 
 /**
@@ -29,45 +29,44 @@ import net.jcip.annotations.Immutable;
  * The source class is fixed to {@code Collection}. The target class is determined
  * by the inner class which extends this {@code CollectionConverter} class.
  *
- * <p>All subclasses will have a unique instance. For this reason, it is not necessary to
- * override the {@code hashCode()} and {@code equals(Object)} methods, since identity
- * comparisons will work just well.</p>
- *
  * @author  Martin Desruisseaux (Geomatys)
  * @since   0.3 (derived from geotk-3.02)
  * @version 0.3
  * @module
  */
 @Immutable
-abstract class CollectionConverter<T> extends SurjectiveConverter<Collection<?>,T> implements Serializable {
+abstract class CollectionConverter<T> extends SystemConverter<Collection<?>,T> {
     /**
      * For cross-version compatibility.
      */
     private static final long serialVersionUID = -4515250904953131514L;
 
     /**
-     * Returns the source class, which is always {@link Collection}.
+     * For inner classes only.
      */
-    @Override
-    @SuppressWarnings({"unchecked","rawtypes"})
-    public final Class<Collection<?>> getSourceClass() {
-        return (Class) Collection.class;
+    @SuppressWarnings("unchecked")
+    CollectionConverter(final Class<T> targetClass) {
+        super((Class) Collection.class, targetClass);
     }
 
+    /**
+     * Returns {@link FunctionProperty#SURJECTIVE} by default.
+     */
+    @Override
+    public java.util.Set<FunctionProperty> properties() {
+        return EnumSet.of(FunctionProperty.SURJECTIVE);
+    }
 
     /**
      * Converter from {@link Collection} to {@link java.util.List}.
      */
     @Immutable
     static final class List extends CollectionConverter<java.util.List<?>> {
-        /** Cross-version compatibility. */ static final long serialVersionUID = 5492247760609833586L;
-        /** The unique, shared instance. */ static final List INSTANCE = new List();
-        /** For {@link #INSTANCE} only.  */ private List() {}
+        private static final long serialVersionUID = 5492247760609833586L;
 
-        @Override
-        @SuppressWarnings({"unchecked","rawtypes"})
-        public Class<java.util.List<?>> getTargetClass() {
-            return (Class) java.util.List.class;
+        @SuppressWarnings("unchecked")
+        List() {
+            super((Class) java.util.List.class);
         }
 
         @Override
@@ -80,11 +79,6 @@ abstract class CollectionConverter<T> ex
             }
             return new ArrayList<>(source);
         }
-
-        /** Returns the singleton instance on deserialization. */
-        Object readResolve() throws ObjectStreamException {
-            return INSTANCE;
-        }
     }
 
 
@@ -93,14 +87,11 @@ abstract class CollectionConverter<T> ex
      */
     @Immutable
     static final class Set extends CollectionConverter<java.util.Set<?>> {
-        /** Cross-version compatibility. */ static final long serialVersionUID = -4200659837453206164L;
-        /** The unique, shared instance. */ static final Set INSTANCE = new Set();
-        /** For {@link #INSTANCE} only.  */ private Set() {}
+        private static final long serialVersionUID = -4200659837453206164L;
 
-        @Override
-        @SuppressWarnings({"unchecked","rawtypes"})
-        public Class<java.util.Set<?>> getTargetClass() {
-            return (Class) java.util.Set.class;
+        @SuppressWarnings("unchecked")
+        Set() {
+            super((Class) java.util.Set.class);
         }
 
         @Override
@@ -113,10 +104,5 @@ abstract class CollectionConverter<T> ex
             }
             return new LinkedHashSet<>(source);
         }
-
-        /** Returns the singleton instance on deserialization. */
-        Object readResolve() throws ObjectStreamException {
-            return INSTANCE;
-        }
     }
 }

Modified: sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/Column.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/Column.java?rev=1456274&r1=1456273&r2=1456274&view=diff
==============================================================================
--- sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/Column.java [UTF-8] (original)
+++ sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/Column.java [UTF-8] Wed Mar 13 23:26:08 2013
@@ -20,6 +20,7 @@ import java.io.Serializable;
 import java.io.InvalidObjectException;
 import org.opengis.util.InternationalString;
 import org.apache.sis.util.Debug;
+import org.apache.sis.util.ObjectConverter;
 import org.apache.sis.util.resources.Vocabulary;
 import org.apache.sis.util.collection.TreeTable;
 import org.apache.sis.util.collection.TableColumn;
@@ -81,6 +82,22 @@ final class Column extends TableColumn<C
     }
 
     /**
+     * Creates a node for the given converter and adds it to the given tree.
+     * Used by {@link FallbackConverter} and {@link ConverterRegistry} for
+     * implementing their {@code toString()} method.
+     *
+     * @param  converter The converter for which to create a tree.
+     * @param  addTo     The node in which to add the converter.
+     * @return The child node created by this method.
+     */
+    static TreeTable.Node toTree(final ObjectConverter<?,?> converter, final TreeTable.Node addTo) {
+        final TreeTable.Node node = addTo.newChild();
+        node.setValue(SOURCE, converter.getSourceClass());
+        node.setValue(TARGET, converter.getTargetClass());
+        return node;
+    }
+
+    /**
      * Formats the given tree table. This method is used for the implementation of
      * {@link FallbackConverter#toString()} and {@link ConverterRegistry#toString()}
      * methods. Since they are mostly for debugging purpose, we do not bother to cache

Modified: sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/FallbackConverter.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/FallbackConverter.java?rev=1456274&r1=1456273&r2=1456274&view=diff
==============================================================================
--- sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/FallbackConverter.java [UTF-8] (original)
+++ sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/FallbackConverter.java [UTF-8] Wed Mar 13 23:26:08 2013
@@ -20,14 +20,13 @@ import java.util.Arrays;
 import java.util.Set;
 import java.util.EnumSet;
 import java.util.Iterator;
-import java.io.Serializable;
+import net.jcip.annotations.Immutable;
 import org.apache.sis.util.Classes;
 import org.apache.sis.util.ObjectConverter;
 import org.apache.sis.math.FunctionProperty;
 import org.apache.sis.util.UnconvertibleObjectException;
 import org.apache.sis.util.collection.DefaultTreeTable;
 import org.apache.sis.util.collection.TreeTable;
-import org.apache.sis.util.resources.Errors;
 import org.apache.sis.util.Debug;
 
 
@@ -40,6 +39,11 @@ import org.apache.sis.util.Debug;
  * asked explicitly for it. Trying the generic converter first is both closer to what the user
  * asked and less likely to throw many exceptions before we found a successful conversion.</p>
  *
+ * <p>All converters in a {@code FallbackConverter} tree have the same source class {@code <S>},
+ * and different target classes {@code <? extends T>} <strong>not</strong> equal to {@code <T>}.
+ * The tree should never have two classes {@code <T1>} and {@code <T2>} such as one is assignable
+ * from the other.</p>
+ *
  * <p>Instances are created by the {@link #merge(ObjectConverter, ObjectConverter)} method.
  * It is invoked when a new converter is {@linkplain ConverterRegistry#register(ObjectConverter)
  * registered} for the same source and target class than an existing converter.</p>
@@ -52,7 +56,8 @@ import org.apache.sis.util.Debug;
  * @version 0.3
  * @module
  */
-final class FallbackConverter<S,T> extends ClassPair<S,T> implements ObjectConverter<S,T>, Serializable {
+@Immutable
+final class FallbackConverter<S,T> extends SystemConverter<S,T> {
     /**
      * For cross-version compatibility.
      */
@@ -61,7 +66,7 @@ final class FallbackConverter<S,T> exten
     /**
      * The primary converter, to be tried first.
      */
-    private ObjectConverter<S, ? extends T> primary;
+    final ObjectConverter<S, ? extends T> primary;
 
     /**
      * The fallback converter. Its target type should not be assignable from the primary target
@@ -70,7 +75,7 @@ final class FallbackConverter<S,T> exten
      * type he would have asked explicitly for it. In addition this layout reduces the amount of
      * exceptions to be thrown and caught before we found a successful conversion.
      */
-    private ObjectConverter<S, ? extends T> fallback;
+    final ObjectConverter<S, ? extends T> fallback;
 
     /**
      * Creates a converter using the given primary and fallback converters. This method may
@@ -88,36 +93,32 @@ final class FallbackConverter<S,T> exten
                               final ObjectConverter<S, ? extends T> fallback)
     {
         super(sourceClass, targetClass);
-        if (swap(primary, fallback)) {
+        if (swap(primary, fallback.getClass())) {
             this.primary  = fallback;
             this.fallback = primary;
         } else {
             this.primary  = primary;
             this.fallback = fallback;
         }
-        assert sourceClass.equals          (primary .getSourceClass()) : primary;
-        assert sourceClass.equals          (fallback.getSourceClass()) : fallback;
-        assert targetClass.isAssignableFrom(primary .getTargetClass()) : primary;
-        assert targetClass.isAssignableFrom(fallback.getTargetClass()) : fallback;
     }
 
     /**
      * Returns {@code true} if the given primary and fallback converters should be interchanged.
      * This method may invoke itself recursively.
      *
-     * @param  primary  The primary converter to test.
-     * @param  fallback The fallback converter to test.
+     * @param  primary The primary converter to test.
+     * @param  fallbackClass The target class of the fallback converter to test.
      * @return {@code true} if the given primary and fallback converters should be interchanged.
      */
-    private static <S> boolean swap(final ObjectConverter<S,?> primary, final ObjectConverter<S,?> fallback) {
-        assert !primary.equals(fallback) : primary;
+    private static <S> boolean swap(final ObjectConverter<S,?> primary, final Class<?> fallbackClass) {
         if (primary instanceof FallbackConverter<?,?>) {
             final FallbackConverter<S,?> candidate = (FallbackConverter<S,?>) primary;
-            return swap(candidate.primary, fallback) && swap(candidate.fallback, fallback);
+            return swap(candidate.primary,  fallbackClass) &&
+                   swap(candidate.fallback, fallbackClass);
         } else {
-            final Class<?> t1 = primary .getTargetClass();
-            final Class<?> t2 = fallback.getTargetClass();
-            return !t1.isAssignableFrom(t2) && t2.isAssignableFrom(t1);
+            final Class<?> targetClass = primary.getTargetClass();
+            return fallbackClass.isAssignableFrom(targetClass) && // This condition is more likely to fail first.
+                    !targetClass.isAssignableFrom(fallbackClass);
         }
     }
 
@@ -136,58 +137,33 @@ final class FallbackConverter<S,T> exten
      *     assert targetClass.isAssignableFrom(converter.getTargetClass()) : converter;
      * }
      *
+     * In the current implementation, the {@code primary} converter can be either an arbitrary
+     * {@code ObjectConverter}, or a previously created {@code FallbackConverter}. However the
+     * {@code fallback} converter shall <strong>not</strong> be a {@code FallbackConverter}.
+     * This restriction exists because the tree built in such case would probably not be the
+     * desired one. It should be okay if only SIS code deal with {@code FallbackConverter}.
+     *
      * @param  <S> The base type of source objects.
      * @param  <T> The base type of converted objects.
-     * @param  existing The existing tree of converters, or {@code null} if none.
-     * @param  converter A new fallback to insert in the converters tree, or {@code null}.
+     * @param  primary The first converter, which may be a {@code Fallback} tree.
+     * @param  fallback A new fallback to insert in the converters tree.
      * @return A tree of converters which contains the given {@code converter}. May be either
      *         {@code existing}, {@code converter} or a new {@code FallbackConverter} instance.
      */
     public static <S,T> ObjectConverter<S, ? extends T> merge(
-                  final ObjectConverter<S, ? extends T> existing,
-                  final ObjectConverter<S, ? extends T> converter)
+                  final ObjectConverter<S, ? extends T> primary,
+                  final ObjectConverter<S, ? extends T> fallback)
     {
-        if (converter == null) return existing;
-        if (existing  == null) return converter;
-        if (existing instanceof FallbackConverter<?,?>) {
-            /*
-             * If we can merge into the existing tree of converters, return that tree
-             * after the merge. Otherwise we will create a new FallbackConverter instance.
-             */
-            if (((FallbackConverter<S, ? extends T>) existing).tryMerge(converter)) {
-                return existing;
-            }
+        assert !(fallback instanceof FallbackConverter<?,?>) : fallback; // See javadoc
+        final ObjectConverter<S, ? extends T> candidate = mergeIfSubtype(primary, fallback, true);
+        if (candidate != null) {
+            return candidate;
         }
-        return create(existing, converter);
-    }
-
-    /**
-     * Creates a converter using the given primary and fallback converters. This method may
-     * interchange the two converters in order to meet the {@linkplain #fallback} contract.
-     *
-     * <p>This method has no information about {@code <T>} type because of parameterized types
-     * erasure, and should not need that information if we didn't made a mistake in this class.
-     * Nevertheless for safety, direct or indirect callers are encouraged to verify themselves
-     * as below:</p>
-     *
-     * {@preformat java
-     *     Class<T> targetClass = ...;
-     *     FallbackConverter<S, ? extends T> converter = create(...);
-     *     assert targetClass.isAssignableFrom(converter.getTargetClass()) : converter;
-     * }
-     *
-     * @param primary  The primary converter.
-     * @param fallback The fallback converter.
-     */
-    private static <S,T> FallbackConverter<S, ? extends T> create(
-            final ObjectConverter<S, ? extends T> primary,
-            final ObjectConverter<S, ? extends T> fallback)
-    {
         final Class<S>           source  = primary .getSourceClass();
         final Class<? extends T> target1 = primary .getTargetClass();
         final Class<? extends T> target2 = fallback.getTargetClass();
         Class<?> target = Classes.findCommonClass(target1, target2);
-        if (target.equals(Object.class)) {
+        if (target == Object.class) {
             /*
              * If there is no common parent class other than Object, looks for a common interface.
              * We perform this special processing for Object.class because this class is handled
@@ -213,6 +189,8 @@ final class FallbackConverter<S,T> exten
          * erasure. If there is no logical error in our algorithm, the cast should be ok.
          * Nevertheless callers are encouraged to verify as documented in the Javadoc.
          */
+        assert target.isAssignableFrom(target1) : target1;
+        assert target.isAssignableFrom(target2) : target2;
         @SuppressWarnings({"unchecked","rawtypes"})
         final FallbackConverter<S, ? extends T> converter =
                 new FallbackConverter(source, target, primary, fallback);
@@ -220,141 +198,92 @@ final class FallbackConverter<S,T> exten
     }
 
     /**
-     * Tries to insert the given converter in this tree of converters. This is possible
-     * only if the target class of the given converter is equals or more specialized
-     * than the target class of this converter.
+     * Merges if the {@code converter} target class of is a subtype of the {@code branch}
+     * target class. Otherwise returns {@code null}.
      *
-     * @param  converter The converter to try to insert in this tree of converters.
-     * @return {@code true} if the insertion has been done, or {@code false} otherwise.
-     */
-    private boolean tryMerge(final ObjectConverter<S,?> converter) { // Do NOT synchronize here.
-        if (!targetClass.isAssignableFrom(converter.getTargetClass())) {
-            return false; // Can not merge because of incompatible type.
+     * <p>The {@code branch} can be either an arbitrary {@code ObjectConverter}, or a previously
+     * created {@code FallbackConverter}. However the {@code converter} shall be a new instance,
+     * <strong>not</strong> a {@code FallbackConverter} instance.
+     * See {@link #merge(ObjectConverter, ObjectConverter)} javadoc for more information.</p>
+     *
+     * @param  <S> The source class of the {@code branch} converter.
+     * @param  <T> The target class of the {@code branch} converter
+     * @param  branch The converter to eventually merge with {@code converter}.
+     * @param  converter The converter to eventually merge with {@code branch}.
+     * @param  isCreateAllowed To be given verbatim to {@link #merge(ObjectConverter, boolean)}.
+     * @return The merged converter, or {@code null} if the {@code converter}
+     *         target class is not a subtype of the {@code branch} target class.
+     */
+    private static <S,T> ObjectConverter<S, ? extends T> mergeIfSubtype(
+            final ObjectConverter<S,T> branch,
+            final ObjectConverter<S,?> converter,
+            final boolean isCreateAllowed)
+    {
+        if (branch.equals(converter)) {
+            return branch;
         }
-        @SuppressWarnings("unchecked")
-        final FallbackConverter<S, ? extends T> child =
-                merge((ObjectConverter<S, ? extends T>) converter);
-        if (child != null) {
-            // Didn't merged in this tree, but found a child
-            // which looks like a better insertion point.
-            return child.tryMerge(converter);
+        final Class<T> targetClass = branch.getTargetClass();
+        if (!targetClass.isAssignableFrom(converter.getTargetClass())) {
+            return null;
         }
-        return true;
-    }
-
-    /**
-     * Inserts the given converter in this tree of fallback converters. If this method detects
-     * that the insertion should be done in a child of this tree, then this method returns that
-     * child. It is caller responsibility to invoke this method again on the child. We proceed
-     * that way in order to release the synchronization lock before to acquire the child lock,
-     * in order to reduce the risk of dead-lock.
-     *
-     * @param  converter The converter to insert in this tree of converters.
-     * @return {@code null} if the insertion has been done, or a non-null value
-     *         if the insertion should be done in the returned converter instead.
-     */
-    private synchronized FallbackConverter<S, ? extends T> merge(final ObjectConverter<S, ? extends T> converter) {
-        final Class<? extends T> childClass = converter.getTargetClass();
         /*
-         * First searches on the fallback side of the tree since they are expected
-         * to contain the most specialized classes. Go down the tree until we find
-         * the last node capable to accept the converter. Only after that point we
-         * may switch the search to the primary side of the tree.
+         * At this point we know that 'converter.targetClass' is <T> or a subtype of <T>,
+         * so the cast below is safe. If the branch is an instance of FallbackConverter,
+         * continue to follow that branch.
          */
-        Class<? extends T> candidateClass = fallback.getTargetClass();
-        if (candidateClass.isAssignableFrom(childClass)) {
+        @SuppressWarnings("unchecked")
+        final ObjectConverter<S, ? extends T> checked = (ObjectConverter<S, ? extends T>) converter;
+        if (branch instanceof FallbackConverter<?,?>) {
             /*
-             * The new converter could be inserted at this point. Checks if we can
-             * continue to walk down the tree, looking for a more specialized node.
+             * Will follow either 'branch.fallback' or 'branch.primary', depending which one
+             * is the most appropriate. If none can be followed, then the result will be the
+             * same than in the 'else' block.
              */
-            if (fallback instanceof FallbackConverter<?,?>) {
-                /*
-                 * If (candidateClass != childClass), we could have a situation like below:
-                 *
-                 * Adding:  String ⇨ Number
-                 * to:      String ⇨ Number            : FallbackConverter
-                 *            ├─String ⇨ Short
-                 *            └─String ⇨ Number        : FallbackConverter
-                 *                ├─String ⇨ Integer
-                 *                └─String ⇨ Long
-                 *
-                 * We don't want to insert the generic Number converter between specialized
-                 * ones (Integer and Long). So rather than going down the tree in this case,
-                 * we will stop the search as if the above "isAssignableFrom" check failed.
-                 * Otherwise return the insertion point, which is 'fallback', for recursive
-                 * invocation by the caller.
-                 */
-                if (candidateClass != childClass) {
-                    return (FallbackConverter<S, ? extends T>) fallback;
-                }
-            } else {
-                /*
-                 * Splits at this point the node in two branches. The previous converter
-                 * will be the primary branch and the new converter will be the fallback
-                 * branch. The "primary vs fallback" contract is respected since we know
-                 * at this point that the new converter is more specialized,  because of
-                 * the isAssignableFrom(...) check performed above.
-                 */
-                fallback = create(fallback, converter);
-                return null;
-            }
-        }
-        /*
-         * We were looking in the fallback branch. Now look in the primary branch
-         * of the same node. The same comments than above apply.
-         */
-        candidateClass = primary.getTargetClass();
-        if (candidateClass.isAssignableFrom(childClass)) {
-            if (primary instanceof FallbackConverter<?,?>) {
-                if (candidateClass != childClass) {
-                    return (FallbackConverter<S, ? extends T>) primary;
-                }
-            } else {
-                primary = create(primary, converter);
-                return null;
-            }
-        }
-        /*
-         * The branch can not hold the converter. If we can't go down anymore in any
-         * of the two branches, insert the converter at the point we have reached so
-         * far. If the converter is more generic, inserts it as the primary branch in
-         * order to respect the "more generic first" contract.
-         */
-        if (childClass.isAssignableFrom(primary .getTargetClass()) &&
-           !childClass.isAssignableFrom(fallback.getTargetClass()))
-        {
-            primary = create(primary, converter);
+            return ((FallbackConverter<S,T>) branch).merge(checked, isCreateAllowed);
         } else {
-            fallback = create(fallback, converter);
+            /*
+             * Both 'branch' and 'checked' are ordinary converters (not FallbackConverter).
+             */
+            return new FallbackConverter<>(branch.getSourceClass(), targetClass, branch, checked);
         }
-        return null;
     }
 
     /**
-     * Returns the primary or fallback converter.
+     * Merge {@code this} with an other converter whose target class is a subtype of
+     * this {@link #targetClass}. If either {@link #fallback} or {@link #primary} are
+     * other {@code FallbackConverter} instances, then this method will follow those
+     * branches.
      *
-     * @param asPrimary {@code true} for the primary branch, or {@code false} for the fallback branch.
-     * @return the requested converter.
-     */
-    final ObjectConverter<S,? extends T> getConverter(final boolean asPrimary) {
-        assert Thread.holdsLock(this);
-        return asPrimary ? primary : fallback;
-    }
-
-    /**
-     * Returns the base type of source objects.
-     */
-    @Override
-    public final Class<S> getSourceClass() {
-        return sourceClass;
-    }
-
-    /**
-     * Returns the base type of target objects.
+     * @param  converter The converter to merge with {@code this}.
+     * @param  isCreateAllowed Initially {@code true}, then set to {@code false} if this method
+     *         is invoking itself recursively for the same branch. This information is used for
+     *         making sure that new converters are appended at the end of existing ones
+     *         (otherwise, insertion order is not preserved).
+     * @return The merged converter.
      */
-    @Override
-    public final Class<T> getTargetClass() {
-        return targetClass;
+    private ObjectConverter<S, ? extends T> merge(
+            final ObjectConverter<S, ? extends T> converter,
+            final boolean isCreateAllowed)
+    {
+        ObjectConverter<S, ? extends T> candidate;
+        final ObjectConverter<S, ? extends T> newPrimary, newFallback;
+        candidate = mergeIfSubtype(fallback, converter, fallback.getTargetClass() != targetClass);
+        if (candidate != null) {
+            newPrimary  = primary;
+            newFallback = candidate;
+        } else {
+            candidate = mergeIfSubtype(primary, converter, primary.getTargetClass() != targetClass);
+            if (candidate != null) {
+                newPrimary  = candidate;
+                newFallback = fallback;
+            } else if (isCreateAllowed) {
+                newPrimary  = this;
+                newFallback = converter;
+            } else {
+                return null;
+            }
+        }
+        return new FallbackConverter<>(sourceClass, targetClass, newPrimary, newFallback);
     }
 
     /**
@@ -363,11 +292,6 @@ final class FallbackConverter<S,T> exten
      */
     @Override
     public final Set<FunctionProperty> properties() {
-        final ObjectConverter<S, ? extends T> primary, fallback;
-        synchronized (this) {
-            primary  = this.primary;
-            fallback = this.fallback;
-        }
         Set<FunctionProperty> properties = primary.properties();
         if (!(primary instanceof FallbackConverter<?,?>)) {
             properties = EnumSet.copyOf(properties);
@@ -382,11 +306,6 @@ final class FallbackConverter<S,T> exten
      */
     @Override
     public T convert(final S source) throws UnconvertibleObjectException {
-        final ObjectConverter<S, ? extends T> primary, fallback;
-        synchronized (this) {
-            primary  = this.primary;
-            fallback = this.fallback;
-        }
         try {
             return primary.convert(source);
         } catch (UnconvertibleObjectException exception) {
@@ -400,42 +319,20 @@ final class FallbackConverter<S,T> exten
     }
 
     /**
-     * {@code FallbackConverter} are not convertible. This is because the parameterized
-     * types are defined as {@code <S, ? extends T>}. The inverse of those types would
-     * be {@code <? extends S, T>}, which is not compatible with the design of this class.
-     */
-    @Override
-    public ObjectConverter<T, S> inverse() throws UnsupportedOperationException {
-        throw new UnsupportedOperationException(Errors.format(Errors.Keys.NonInvertibleConversion));
-    }
-
-    /**
      * Creates a node for the given converter and adds it to the given tree.
      * This method invokes itself recursively for scanning through fallbacks.
      *
-     * <p>This method creates a simplified tree, in that the cascading of fallbacks converter
-     * of same {@link #targetClass} are hidden: only their leaves are created. The purpose is
-     * to help the developer to focus more on the important elements (the leaf converters)
-     * and be less distracted by the amount of {@code FallbackConverter}s traversed in order
-     * to reach those leaves.</p>
-     *
      * @param converter The converter for which to create a tree.
      * @param addTo The node in which to add the converter.
      */
-    private void toTree(final ObjectConverter<?,?> converter, final TreeTable.Node addTo) {
-        FallbackConverter<?,?> more = null;
+    private void toTree(final ObjectConverter<?,?> converter, TreeTable.Node addTo) {
         if (converter instanceof FallbackConverter<?,?>) {
-            more = (FallbackConverter<?,?>) converter;
-            if (more.targetClass == targetClass) { // Simplification case (omit the node).
-                more.toTree(addTo);
-                return;
+            if (converter.getTargetClass() != targetClass) {
+                addTo = Column.toTree(converter, addTo);
             }
-        }
-        final TreeTable.Node node = addTo.newChild();
-        node.setValue(Column.SOURCE, converter.getSourceClass());
-        node.setValue(Column.TARGET, converter.getTargetClass());
-        if (more != null) {
-            more.toTree(node);
+            ((FallbackConverter<?,?>) converter).toTree(addTo);
+        } else {
+            Column.toTree(converter, addTo);
         }
     }
 
@@ -446,11 +343,6 @@ final class FallbackConverter<S,T> exten
      * @param addTo The node in which to add the converter.
      */
     final void toTree(final TreeTable.Node addTo) {
-        final ObjectConverter<S,? extends T> primary, fallback;
-        synchronized (this) {
-            primary  = this.primary;
-            fallback = this.fallback;
-        }
         toTree(primary,  addTo);
         toTree(fallback, addTo);
     }

Modified: sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/SystemConverter.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/SystemConverter.java?rev=1456274&r1=1456273&r2=1456274&view=diff
==============================================================================
--- sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/SystemConverter.java [UTF-8] (original)
+++ sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/SystemConverter.java [UTF-8] Wed Mar 13 23:26:08 2013
@@ -71,8 +71,7 @@ abstract class SystemConverter<S,T> exte
      */
     @Override
     public ObjectConverter<T, S> inverse() throws UnsupportedOperationException {
-        throw new UnsupportedOperationException(Errors.format(
-                Errors.Keys.UnsupportedOperation_1, "inverse"));
+        throw new UnsupportedOperationException(Errors.format(Errors.Keys.NonInvertibleConversion));
     }
 
     /**

Modified: sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/util/Classes.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/util/Classes.java?rev=1456274&r1=1456273&r2=1456274&view=diff
==============================================================================
--- sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/util/Classes.java [UTF-8] (original)
+++ sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/util/Classes.java [UTF-8] Wed Mar 13 23:26:08 2013
@@ -275,14 +275,14 @@ public final class Classes extends Stati
      * @param  <T>  The compile-time type of the {@code Class} argument.
      * @param  type The class or interface for which to get all implemented interfaces.
      * @return All implemented interfaces (not including the given {@code type} if it was an
-     *         interface), or an empty set if none. Callers can freely modify the returned set.
+     *         interface), or an empty array if none.
      *
      * @see Class#getInterfaces()
      */
     @SuppressWarnings({"unchecked","rawtypes"}) // Generic array creation.
     public static <T> Class<? super T>[] getAllInterfaces(final Class<T> type) {
         final Set<Class<?>> interfaces = getInterfaceSet(type);
-        return interfaces.toArray(new Class[interfaces.size()]);
+        return (interfaces != null) ? interfaces.toArray(new Class[interfaces.size()]) : EMPTY_ARRAY;
     }
 
     /**
@@ -297,6 +297,10 @@ public final class Classes extends Stati
      *       Consequently callers can cast {@code Class<? super T>[]} to {@code Class<?>[]}
      *       while they can not cast {@code Set<Class<? super T>>} to {@code Set<Class<?>>}.</li>
      * </ul>
+     *
+     * @param  type The class or interface for which to get all implemented interfaces.
+     * @return All implemented interfaces (not including the given {@code type} if it was an
+     *         interface), or {@code null} if none. Callers can freely modify the returned set.
      */
     static Set<Class<?>> getInterfaceSet(Class<?> type) {
         Set<Class<?>> interfaces = null;
@@ -304,7 +308,7 @@ public final class Classes extends Stati
             interfaces = getInterfaceSet(type, interfaces);
             type = type.getSuperclass();
         }
-        return (interfaces != null) ? interfaces : Collections.<Class<?>>emptySet();
+        return interfaces;
     }
 
     /**
@@ -493,6 +497,9 @@ next:       for (final Class<?> candidat
     public static Set<Class<?>> findCommonInterfaces(final Class<?> c1, final Class<?> c2) {
         final Set<Class<?>> interfaces = getInterfaceSet(c1);
         final Set<Class<?>> buffer     = getInterfaceSet(c2); // To be recycled.
+        if (interfaces == null || buffer == null) {
+            return Collections.emptySet();
+        }
         interfaces.retainAll(buffer);
         for (Iterator<Class<?>> it=interfaces.iterator(); it.hasNext();) {
             final Class<?> candidate = it.next();

Modified: sis/branches/JDK7/sis-utility/src/test/java/org/apache/sis/internal/converter/FallbackConverterTest.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/sis-utility/src/test/java/org/apache/sis/internal/converter/FallbackConverterTest.java?rev=1456274&r1=1456273&r2=1456274&view=diff
==============================================================================
--- sis/branches/JDK7/sis-utility/src/test/java/org/apache/sis/internal/converter/FallbackConverterTest.java [UTF-8] (original)
+++ sis/branches/JDK7/sis-utility/src/test/java/org/apache/sis/internal/converter/FallbackConverterTest.java [UTF-8] Wed Mar 13 23:26:08 2013
@@ -36,102 +36,87 @@ import static org.apache.sis.test.Assert
  */
 public final strictfp class FallbackConverterTest extends TestCase {
     /**
+     * Conversions that are expected to be supported.
+     * Greater values imply all conversions identified by lower values.
+     */
+    private static final int SHORT=0, LONG=1, FLOAT=2, BOOLEAN=3;
+
+    /**
      * Tests a chain of fallback converters. The initial fallback will understand {@link Short}
-     * and {@link Long} types. Later we will add other types like {@link Boolean} and {@link Float}.
+     * and {@link Long} types. Then we progressively add more types.
+     *
+     * <p>This test compares the string representations for convenience.  In theory those string
+     * representations are not committed API, so if the {@code FallbackConverter} implementation
+     * change, it is okay to update this test accordingly.</p>
      */
     @Test
     public void testChain() {
-        @SuppressWarnings({"unchecked","rawtypes"}) // Generic array creation.
-        final ObjectConverter<String,?>[] converters = new ObjectConverter[] {
-            StringConverter.Short  .INSTANCE,
-            StringConverter.Long   .INSTANCE,
-            StringConverter.Float  .INSTANCE,
-            StringConverter.Integer.INSTANCE,
-            StringConverter.Boolean.INSTANCE,
-            StringConverter.Double .INSTANCE
-        };
-        // Index at which some kind of values are expected.
-        final int LONG    = 1;
-        final int FLOAT   = 2;
-        final int DOUBLE  = 5;
-        final int BOOLEAN = 4;
-        assertEquals(Long.class,    converters[LONG]   .getTargetClass());
-        assertEquals(Float.class,   converters[FLOAT]  .getTargetClass());
-        assertEquals(Double.class,  converters[DOUBLE] .getTargetClass());
-        assertEquals(Boolean.class, converters[BOOLEAN].getTargetClass());
-        /*
-         * Create the fallback chain and check at every steps. The source class should never
-         * change. But the target type will be relaxed more and more as we add new fallbacks.
-         * The initial target will be Short, then Number, and finally Object.
-         */
-        ObjectConverter<String,?> c = null;
-        for (int i=0; i<converters.length; i++) {
-            c = FallbackConverter.merge(c, converters[i]);
-            final ObjectConverter<String,?>[] expected;
-            final Class<?> targetClass;
-            if (i == 0) {
-                targetClass = Short.class;
-                expected    = converters;
-            } else if (i < BOOLEAN) {
-                targetClass = Number.class;
-                expected    = converters;
-            } else if (i < DOUBLE) {
-                targetClass = Object.class;
-                expected    = converters;
-            } else {
-                targetClass = Object.class;
-                expected    = converters.clone();
-                expected[BOOLEAN] = converters[DOUBLE];
-                expected[DOUBLE]  = converters[BOOLEAN];
-                // Boolean is expected to be last.
-            }
-            assertEquals(targetClass, c.getTargetClass());
-            assertEquals(String.class, c.getSourceClass());
-            assertEquals(i+1, count(c, expected, 0));
-            /*
-             * Try conversions. Parsing numbers should returns a Short, Long or Float because
-             * they were the first converters declared. Parsing a boolean should succeed only
-             * after we registered the boolean converter.
-             */
-            assertConvert(c, "5",                      Short.valueOf((short) 5));
-            assertConvert(c, "80000", (i >= LONG)    ? Long.valueOf(80000) : null);
-            assertConvert(c, "5.0",   (i >= FLOAT)   ? Float.valueOf(5f) : null);
-            assertConvert(c, "2.5",   (i >= FLOAT)   ? Float.valueOf(2.5f) : null);
-            assertConvert(c, "1E+20", (i >= FLOAT)   ? Float.valueOf(1E+20f) : null);
-            assertConvert(c, "1E+80", (i >= FLOAT)   ? Float.POSITIVE_INFINITY : null);
-            assertConvert(c, "true",  (i >= BOOLEAN) ? Boolean.TRUE : null);
-        }
-        /*
-         * Adds a generic converter for number. It is going to change everything.
-         * This converter is expected to be registered before any other number
-         * converter. Same tests than above will return different objects.
-         */
-        @SuppressWarnings({"unchecked","rawtypes"}) // Generic array creation.
-        final ObjectConverter<String,?>[] expected = new ObjectConverter[converters.length + 1];
-        System.arraycopy(converters, 0, expected, 1, converters.length);
-        expected[BOOLEAN + 1] = converters[DOUBLE];
-        expected[DOUBLE  + 1] = converters[BOOLEAN];
-        c = FallbackConverter.merge(c, expected[0] = StringConverter.Number.INSTANCE);
+        final EnumSet<FunctionProperty> SURJECTIVE = EnumSet.of(FunctionProperty.SURJECTIVE);
+        final EnumSet<FunctionProperty> INVERTIBLE = EnumSet.of(FunctionProperty.SURJECTIVE, FunctionProperty.INVERTIBLE);
+
+        ObjectConverter<String,?> c = StringConverter.Short.INSTANCE;
+        assertEquals(String.class, c.getSourceClass());
+        assertEquals(Short.class,  c.getTargetClass());
+        assertEquals(INVERTIBLE,   c.properties());
+        tryConversions(c, SHORT);
+        assertMultilinesEquals(
+                "StringConverter.Short[String ⇨ Short]", c.toString());
+
+        c = FallbackConverter.merge(c, StringConverter.Long.INSTANCE);
+        assertEquals(String.class, c.getSourceClass());
+        assertEquals(Number.class, c.getTargetClass());
+        assertEquals(SURJECTIVE,   c.properties());
+        tryConversions(c, LONG);
+        assertMultilinesEquals(
+                "String     ⇨ Number\n" +
+                "  ├─String ⇨ Short\n" +
+                "  └─String ⇨ Long\n", c.toString());
+
+        c = FallbackConverter.merge(c, StringConverter.Float.INSTANCE);
+        assertEquals(String.class, c.getSourceClass());
+        assertEquals(Number.class, c.getTargetClass());
+        assertEquals(SURJECTIVE,   c.properties());
+        tryConversions(c, FLOAT);
+        assertMultilinesEquals(
+                "String     ⇨ Number\n" +
+                "  ├─String ⇨ Short\n" +
+                "  ├─String ⇨ Long\n" +
+                "  └─String ⇨ Float\n", c.toString());
+
+        c = FallbackConverter.merge(c, StringConverter.Integer.INSTANCE);
+        assertEquals(String.class, c.getSourceClass());
+        assertEquals(Number.class, c.getTargetClass());
+        assertEquals(SURJECTIVE,   c.properties());
+        tryConversions(c, FLOAT);
+        assertMultilinesEquals(
+                "String     ⇨ Number\n" +
+                "  ├─String ⇨ Short\n" +
+                "  ├─String ⇨ Long\n" +
+                "  ├─String ⇨ Float\n" +
+                "  └─String ⇨ Integer\n", c.toString());
+
+        c = FallbackConverter.merge(c, StringConverter.Boolean.INSTANCE);
         assertEquals(String.class, c.getSourceClass());
         assertEquals(Object.class, c.getTargetClass());
-        assertEquals(expected.length, count(c, expected, 0));
-        assertConvert(c, "5",     Byte.valueOf((byte) 5));
-        assertConvert(c, "80000", Integer.valueOf(80000));
-        assertConvert(c, "5.0",   Byte.valueOf((byte) 5));
-        assertConvert(c, "2.5",   Float.valueOf(2.5f));
-        assertConvert(c, "1E+20", Double.valueOf(1E+20));
-        assertConvert(c, "1E+80", Double.valueOf(1E+80));
-        assertConvert(c, "true",  Boolean.TRUE);
-        assertEquals(EnumSet.of(FunctionProperty.SURJECTIVE), c.properties());
-        /*
-         * Compares the string representations. In theory this is not needed, since the
-         * above tests performs the same check in a more programmatic way. However this
-         * is a convenient visual check and a useful debugging tool.
-         */
+        assertEquals(SURJECTIVE,   c.properties());
+        tryConversions(c, BOOLEAN);
+        assertMultilinesEquals(
+                "String         ⇨ Object\n" +
+                "  ├─String     ⇨ Number\n" +
+                "  │   ├─String ⇨ Short\n" +
+                "  │   ├─String ⇨ Long\n" +
+                "  │   ├─String ⇨ Float\n" +
+                "  │   └─String ⇨ Integer\n" +
+                "  └─String     ⇨ Boolean\n", c.toString());
+
+        c = FallbackConverter.merge(c, StringConverter.Double.INSTANCE);
+        assertEquals(String.class, c.getSourceClass());
+        assertEquals(Object.class, c.getTargetClass());
+        assertEquals(SURJECTIVE,   c.properties());
+        tryConversions(c, BOOLEAN);
         assertMultilinesEquals(
                 "String         ⇨ Object\n" +
                 "  ├─String     ⇨ Number\n" +
-                "  │   ├─String ⇨ Number\n" +
                 "  │   ├─String ⇨ Short\n" +
                 "  │   ├─String ⇨ Long\n" +
                 "  │   ├─String ⇨ Float\n" +
@@ -141,17 +126,30 @@ public final strictfp class FallbackConv
     }
 
     /**
+     * Tries conversions. Parsing numbers should returns a Short, Long or Float because
+     * they were the first converters declared. Parsing a boolean should succeed only
+     * after we registered the boolean converter.
+     */
+    private static void tryConversions(final ObjectConverter<String,?> c, final int supported) {
+        assertConvertedEquals(c, (supported >= SHORT)   ? Short.valueOf((short) 5) : null, "5");
+        assertConvertedEquals(c, (supported >= LONG )   ? Long .valueOf(    80000) : null, "80000");
+        assertConvertedEquals(c, (supported >= FLOAT)   ? Float.valueOf(       5f) : null, "5.0");
+        assertConvertedEquals(c, (supported >= FLOAT)   ? Float.valueOf(     2.5f) : null, "2.5");
+        assertConvertedEquals(c, (supported >= FLOAT)   ? Float.valueOf(   1E+20f) : null, "1E+20");
+        assertConvertedEquals(c, (supported >= FLOAT)   ? Float.POSITIVE_INFINITY  : null, "1E+80");
+        assertConvertedEquals(c, (supported >= BOOLEAN) ? Boolean.TRUE             : null, "true");
+    }
+
+    /**
      * Converts the given value and compares the result with the expected one.
      *
-     * @param converter The converter to use.
-     * @param value     The value to convert.
-     * @param expected  The expected result, or {@code null} if the conversion is expected to
-     *                  thrown an exception.
-     *
+     * @param  converter The converter to use.
+     * @param  expected  The expected result, or {@code null} if the conversion is expected to fail.
+     * @param  value     The value to convert.
      * @throws UnconvertibleObjectException if an exception was not expected but occurred.
      */
-    private static void assertConvert(final ObjectConverter<String,?> converter,
-            final String value, final Object expected) throws UnconvertibleObjectException
+    private static void assertConvertedEquals(final ObjectConverter<String,?> converter,
+            final Object expected, final String value) throws UnconvertibleObjectException
     {
         final Object result;
         try {
@@ -168,27 +166,4 @@ public final strictfp class FallbackConv
         }
         assertEquals(expected, result);
     }
-
-    /**
-     * Counts the number of converters, descending down in the tree if necessary.
-     *
-     * @param converter The converter for which to count underlying converters.
-     * @param expected  Expected converters while we go down the tree, or {@code null}.
-     * @param offset    The index of the first element to consider in the array.
-     */
-    private static int count(final ObjectConverter<?,?> converter,
-                             final ObjectConverter<?,?>[] expected, final int offset)
-    {
-        if (converter instanceof FallbackConverter<?,?>) {
-            final FallbackConverter<?,?> fallback = (FallbackConverter<?,?>) converter;
-            synchronized (fallback) {
-                final int c = count(fallback.getConverter(true), expected, offset);
-                return c + count(fallback.getConverter(false), expected, offset + c);
-            }
-        }
-        if (expected != null) {
-            assertEquals(String.valueOf(offset), expected[offset], converter);
-        }
-        return 1;
-    }
 }



Mime
View raw message