sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject svn commit: r1456484 - in /sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter: ClassPair.java ConverterRegistry.java SystemConverter.java
Date Thu, 14 Mar 2013 15:37:02 GMT
Author: desruisseaux
Date: Thu Mar 14 15:37:01 2013
New Revision: 1456484

URL: http://svn.apache.org/r1456484
Log:
Completed the port of ConverterRegistry, excepts toString() and tests.

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/ConverterRegistry.java
    sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/SystemConverter.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=1456484&r1=1456483&r2=1456484&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] Thu Mar 14 15:37:01 2013
@@ -19,13 +19,16 @@ package org.apache.sis.internal.converte
 import java.io.Serializable;
 import net.jcip.annotations.Immutable;
 import org.apache.sis.util.ObjectConverter;
+import org.apache.sis.util.Debug;
 
 
 /**
  * Holds explicit {@link #sourceClass} and {@link #targetClass} values. Used as key in a
hash
- * map of converters. Also used as the base class for subclasses working on explicit source
and
- * target class. We allows this opportunist leveraging of implementation because those classes
- * are not public (otherwise a separated hierarchy may have been preferable).
+ * map of converters. Also used as the base class for converters defined in this package.
+ *
+ * <p>The only direct subtype allowed is {@link SystemConverter}.
+ * <strong>No other direct subtype shall exist</strong>.
+ * See {@link #equals(Object)} for an explanation.</p>
  *
  * @param <S> The base type of source objects.
  * @param <T> The base type of converted objects.
@@ -45,12 +48,12 @@ class ClassPair<S,T> implements Serializ
     /**
      * The source class.
      */
-    protected final Class<S> sourceClass;
+    final Class<S> sourceClass;
 
     /**
      * The target class.
      */
-    protected final Class<T> targetClass;
+    final Class<T> targetClass;
 
     /**
      * Creates an entry for the given source and target classes.
@@ -80,7 +83,7 @@ class ClassPair<S,T> implements Serializ
      *
      * @return A key for the parent source, or {@code null}.
      */
-    public final ClassPair<? super S, T> parentSource() {
+    final ClassPair<? super S, T> parentSource() {
         final Class<? super S> source;
         if (sourceClass.isInterface()) {
             @SuppressWarnings({"unchecked","rawtypes"})
@@ -118,27 +121,21 @@ class ClassPair<S,T> implements Serializ
     }
 
     /**
-     * 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;
-    }
-
-    /**
-     * Compares the given object with this entry for equality. Two entries are considered
-     * equals if they have the same source and target classes. This is required for use
-     * as {@link java.util.HashMap} keys in {@link ConverterRegistry}.
-     *
-     * @param  other The object to compare with this entry.
-     * @return {@code true} if the given object is a entry having the same source and target
classes.
+     * Compares the given object with this {@code ClassPair} for equality. Two {@code ClassPair}
+     * instances are considered equal if they have the same source and target classes, ignoring
+     * all other properties eventually defined in subclasses.
+     *
+     * <p>This method is designed for use by {@link ConverterRegistry} as {@link java.util.HashMap}
+     * keys. Its primary purpose is <strong>not</strong> to determine if two
objects are converters
+     * doing the same conversions. However the {@link SystemConverter} subclass overrides
this
+     * method with an additional safety check.</p>
+     *
+     * @param  other The object to compare with this {@code ClassPair}.
+     * @return {@code true} if the given object is a {@code ClassPair}
+     *         having the same source and target classes.
      */
     @Override
-    public final boolean equals(final Object other) {
+    public boolean equals(final Object other) {
         if (other instanceof ClassPair<?,?>) {
             final ClassPair<?,?> that = (ClassPair<?,?>) other;
             return sourceClass == that.sourceClass &&
@@ -148,7 +145,8 @@ class ClassPair<S,T> implements Serializ
     }
 
     /**
-     * Returns a hash code value for this entry.
+     * Returns a hash code value for this {@code ClassPair}.
+     * See {@link #equals(Object)} javadoc for information on the scope of this method.
      */
     @Override
     public final int hashCode() {
@@ -159,6 +157,7 @@ class ClassPair<S,T> implements Serializ
      * Returns a string representation for this entry.
      * Used for formatting error messages.
      */
+    @Debug
     @Override
     public String toString() {
         return sourceClass.getSimpleName() + " ⇨ " + targetClass.getSimpleName();

Modified: sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/ConverterRegistry.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/ConverterRegistry.java?rev=1456484&r1=1456483&r2=1456484&view=diff
==============================================================================
--- sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/ConverterRegistry.java
[UTF-8] (original)
+++ sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/internal/converter/ConverterRegistry.java
[UTF-8] Thu Mar 14 15:37:01 2013
@@ -18,24 +18,25 @@ package org.apache.sis.internal.converte
 
 import java.util.Map;
 import java.util.LinkedHashMap;
-import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReadWriteLock;
-import java.util.concurrent.locks.ReentrantReadWriteLock;
 import net.jcip.annotations.ThreadSafe;
 import org.apache.sis.internal.util.SystemListener;
+import org.apache.sis.util.Classes;
+import org.apache.sis.util.ArgumentChecks;
 import org.apache.sis.util.ObjectConverter;
+import org.apache.sis.util.UnconvertibleObjectException;
+import org.apache.sis.util.resources.Errors;
 
 
 /**
  * A collection of {@link ObjectConverter} instances.
  * A converter from the given <var>source type</var> to the given <var>target
type</var> can be
- * obtained by a call to {@link #converter(Class, Class)}. If no converter exists for the
given
+ * obtained by a call to {@link #find(Class, Class)}. If no converter exists for the given
  * source and target types, then this registry searches for a suitable converter accepting
a
  * parent class of the given source type, or returning a sub-class of the given target type.
  *
  * <p>New instances of {@code ConverterRegistry} are initially empty. Custom converters
must be
- * explicitly {@linkplain #register registered}. However a system-wide registry initialized
- * with default converters is provided by the {@link #SYSTEM} constant.</p>
+ * explicitly {@linkplain #register(ObjectConverter) registered}. However a system-wide registry
+ * initialized with default converters is provided by the {@link #SYSTEM} constant.</p>
  *
  * {@section Note about conversions from interfaces}
  * {@code ConverterRegistry} is primarily designed for handling converters from classes to
@@ -78,78 +79,373 @@ public class ConverterRegistry {
     }
 
     /**
-     * The map of converters of any kind. We use a single read/write lock for the whole map
-     * because write operations will be rare (so {@code ConcurrentHashMap} may be an overkill).
-     */
-    private final Map<ClassPair<?,?>, SystemConverter<?,?>> converters;
-
-    /**
-     * The locks for the {@link #converters} map.
+     * The map of converters of any kind. For any key of type {@code ClassPair<S,T>},
+     * the value shall be of type {@code ObjectConverter<? super S, ? extends T>}.
+     * To ensure this constraint, values should be read and written using only the
+     * type-safe {@link #get(ClassPair)} and {@link #put(ClassPair, ObjectConverter)}
+     * methods.
+     *
+     * <p>In the special case where the value is actually {@code SystemConverter<S,T>},
+     * then the key and the value may be the same instance (in order to save object
+     * allocations).</p>
+     *
+     * {@section Synchronization note}
+     * Synchronization if performed by {@code synchronized(converters)} statements. We tried
+     * {@code ReadWriteLock}, but this is not very convenient because read operations may
be
+     * followed by write operations at any time if the requested converter is not in the
cache.
+     * Furthermore profiling has not identified this class as a noticeable contention point.
      */
-    private final ReadWriteLock locks;
+    private final Map<ClassPair<?,?>, ObjectConverter<?,?>> converters;
 
     /**
      * Creates an initially empty set of object converters.
      */
     public ConverterRegistry() {
         converters = new LinkedHashMap<>();
-        locks = new ReentrantReadWriteLock();
     }
 
     /**
      * Removes all converters from this registry.
      */
     public void clear() {
-        final Lock lock = locks.writeLock();
-        lock.lock();
-        try {
+        synchronized (converters) {
             converters.clear();
-        } finally {
-            lock.unlock();
         }
     }
 
     /**
+     * Gets the value from the {@linkplain #converters} map for the given key.
+     */
+    private <S,T> ObjectConverter<? super S, ? extends T> get(final ClassPair<S,T>
key) {
+        assert Thread.holdsLock(converters);
+        return key.cast(converters.get(key));
+    }
+
+    /**
+     * Puts the given value in the {@linkplain #converters} map for the given key.
+     */
+    @SuppressWarnings("unchecked")
+    private <S,T> void put(ClassPair<S,T> key, final ObjectConverter<? super
S, ? extends T> converter) {
+        assert key.getClass() == ClassPair.class; // See SystemConverter.equals(Object)
+        assert Thread.holdsLock(converters);
+        if (converter instanceof SystemConverter<?,?> &&
+            converter.getSourceClass() == key.sourceClass &&
+            converter.getTargetClass() == key.targetClass)
+        {
+            /*
+             * Opportunistically share the same instance for the keys and the values, in
order
+             * to reduce a little bit the amount of objects in the JVM. However we must remove
+             * any old value from the map using the old key, otherwise put operation may
fail.
+             * See SystemConverter.equals(Object) for more explanation.
+             */
+            converters.remove(key);
+            key = (SystemConverter<S,T>) converter;
+        }
+        converters.put(key, converter);
+    }
+
+    /**
+     * If {@code existing} or one of its children is equals to the given {@code converter},
+     * returns it. Otherwise returns {@code null}.
+     *
+     * @param  <S> The {@code converter} source class.
+     * @param  <T> The {@code converter} target class.
+     * @param  converter The converter to replace by an existing converter, if possible.
+     * @param  existing Existing converter to test.
+     * @return A converter equals to {@code converter}, or {@code null} if none.
+     */
+    @SuppressWarnings("unchecked")
+    private static <S,T> ObjectConverter<S,T> findEquals(ObjectConverter<S,T>
converter,
+            final ObjectConverter<S, ? extends T> existing)
+    {
+        if (converter instanceof FallbackConverter<?,?>) {
+            final FallbackConverter<S,T> fc = (FallbackConverter<S,T>) converter;
+            converter = findEquals(fc, fc.primary);
+            if (converter == null) {
+                converter = findEquals(fc, fc.fallback);
+            }
+        } else if (converter.equals(existing)) {
+            converter = (ObjectConverter<S,T>) existing;
+        } else {
+            converter = null;
+        }
+        return converter;
+    }
+
+    /**
+     * Returns a converter equals to the given {@code converter}, or {@code null} if none.
+     *
+     * @param  <S> The {@code converter} source class.
+     * @param  <T> The {@code converter} target class.
+     * @param  converter The converter to replace by an existing converter, if possible.
+     * @return A converter equals to {@code converter}, or {@code null} if none.
+     */
+    @SuppressWarnings("unchecked")
+    final <S,T> ObjectConverter<S,T> findEquals(final SystemConverter<S,T>
converter) {
+        ObjectConverter<? super S, ? extends T> existing;
+        synchronized (converters) {
+            existing = get(converter);
+        }
+        if (existing != null && existing.getSourceClass() == converter.getSourceClass())
{
+            return findEquals(converter, (ObjectConverter<S, ? extends T>) existing);
+        }
+        return null;
+    }
+
+    /**
      * Returns an unique instance of the given converter. If a converter already exists for
the
      * same source an target classes, then that converter is returned. Otherwise that converter
-     * is cached if {@code cache} is {@code true} and returned.
+     * is cached and returned.
      *
      * @param  converter The converter to look for a unique instance.
-     * @param  cache Whether to cache the given converter if there is no existing instance.
      * @return A previously existing instance if one exists, or the given converter otherwise.
      */
     @SuppressWarnings("unchecked")
-    final <S,T> ObjectConverter<S,T> unique(final SystemConverter<S,T>
converter, final boolean cache) {
-        SystemConverter<S,T> existing;
-        Lock lock = locks.readLock();
-        lock.lock();
-        try {
-            existing = (SystemConverter<S,T>) converters.get(converter);
-        } finally {
-            lock.unlock();
+    final <S,T> ObjectConverter<S,T> unique(final SystemConverter<S,T>
converter) {
+        ObjectConverter<S,T> existing = findEquals(converter);
+        if (existing == null) {
+            register(converter);
+            existing = findEquals(converter);
+            if (existing == null) {
+                return converter;
+            }
+        }
+        return existing;
+    }
+
+    /**
+     * Registers a new converter. This method should be invoked only once for a given converter,
+     * for example in class static initializer. For example if a {@code Angle} class is defined,
+     * the static initializer of that class could register a converter from {@code Angle}
to
+     * {@code Double}.
+     *
+     * <p>This method registers the converter for the {@linkplain ObjectConverter#getTargetClass()
+     * target class}, some parents of the target class (see below) and every interfaces except
+     * {@link Cloneable} which are implemented by the target class and not by the source
class.
+     * For example a converter producing {@link Double} can be used for clients that just
ask
+     * for a {@link Number}.</p>
+     *
+     * {@section Which super-classes of the target class are registered}
+     * Consider a converter from class {@code S} to class {@code T} where the two classes
+     * are related in a hierarchy as below:
+     *
+     * {@preformat text
+     *   C1
+     *   └───C2
+     *       ├───C3
+     *       │   └───S
+     *       └───C4
+     *           └───T
+     * }
+     *
+     * Invoking this method will register the given converter for all the following cases:
+     *
+     * <ul>
+     *   <li>{@code S} → {@code T}</li>
+     *   <li>{@code S} → {@code C4}</li>
+     * </ul>
+     *
+     * No {@code S} → {@code C2} or {@code S} → {@code C1} converter will be registered,
+     * because an identity converter would be sufficient for those cases.
+     *
+     * {@section Which sub-classes of the source class are registered}
+     * Sub-classes of the source class will be registered on a case-by-case basis when the
+     * {@link #find(Class, Class)} is invoked, because we can not know the set of all
+     * sub-classes in advance (and would not necessarily want to register all of them anyway).
+     *
+     * @param <S> The class of source value.
+     * @param <T> The class of target (converted) values.
+     * @param converter The converter to register.
+     */
+    public <S,T> void register(final ObjectConverter<S,T> converter) {
+        ArgumentChecks.ensureNonNull("converter", converter);
+        /*
+         * If the given converter is a FallbackConverter (maybe obtained from an other
+         * ConverterRegistry), unwraps it and registers its component individually.
+         */
+        if (converter instanceof FallbackConverter<?,?>) {
+            final FallbackConverter<S,T> fc = (FallbackConverter<S,T>) converter;
+            register(fc.primary);
+            register(fc.fallback);
+            return;
         }
         /*
-         * If no instance existed before for the source and target classes, stores this
-         * instance in the pool. However we will need to check again during the write
-         * operation in case an other thread had the time to add an instance in the pool.
+         * Registers an individual converter.
          */
-        if (existing == null) {
-            if (!cache) {
+        final Class<S> sourceClass = converter.getSourceClass();
+        final Class<T> targetClass = converter.getTargetClass();
+        final Class<?> stopAt = Classes.findCommonClass(sourceClass, targetClass);
+        ArgumentChecks.ensureNonNull("sourceClass", sourceClass);
+        ArgumentChecks.ensureNonNull("targetClass", targetClass);
+        synchronized (converters) {
+            for (Class<? super T> i=targetClass; i!=null && i!=stopAt; i=i.getSuperclass())
{
+                register(new ClassPair<>(sourceClass, i), converter);
+            }
+            /*
+             * At this point, the given class and parent classes
+             * have been registered. Now registers interfaces.
+             */
+            for (final Class<? super T> i : Classes.getAllInterfaces(targetClass))
{
+                if (i.isAssignableFrom(sourceClass)) {
+                    /*
+                     * Target interface is already implemented by the source, so
+                     * there is no reason to convert the source to that interface.
+                     */
+                    continue;
+                }
+                if (Cloneable.class.isAssignableFrom(i)) {
+                    /*
+                     * Exclude this special case. If we were accepting it, we would basically
+                     * provide converters from immutable to mutable objects (e.g. from String
+                     * to Locale), which is not something we would like to encourage. Even
if
+                     * the user really wanted a mutable object, in order to modify it he
needs
+                     * to known the exact type, so asking for a conversion to Cloneable is
too
+                     * vague.
+                     */
+                    continue;
+                }
+                if (sourceClass == Number.class && Comparable.class.isAssignableFrom(i))
{
+                    /*
+                     * Exclude this special case. java.lang.Number does not implement Comparable,
+                     * but its subclasses do. Accepting this case would lead ConverterRegistry
to
+                     * offer converters from Number to String, which is not the best move
if the
+                     * user want to compare numbers.
+                     */
+                    continue;
+                }
+                if (!i.isAssignableFrom(sourceClass)) {
+                    register(new ClassPair<>(sourceClass, i), converter);
+                }
+            }
+        }
+    }
+
+    /**
+     * Registers the given converter under the given key. If a previous converter is already
+     * registered for the given key, then there is a choice:
+     *
+     * <ul>
+     *   <li>If one converter is defined exactly for the {@code <S,T>} classes
while the
+     *       other converter is not, then the most accurate converter will have precedence.</li>
+     *   <li>Otherwise the new converter is registered in addition of the old one in
a
+     *       chain of fallbacks.</li>
+     * </ul>
+     *
+     * @param key The key under which to register the converter.
+     * @param converter The converter to register.
+     */
+    @SuppressWarnings("unchecked")
+    private <S,T> void register(final ClassPair<S,T> key, ObjectConverter<S,
? extends T> converter) {
+        final ObjectConverter<? super S, ? extends T> existing = get(key);
+        if (existing != null) {
+            /*
+             * An other converter already exists for the given key. If the converters are
+             * equal (i.e. the user registered the same converter twice), do nothing.
+             */
+            if (existing.equals(converter)) {
+                return;
+            }
+            /*
+             * FallbackConverters are created only for converters having the same source
class.
+             * If this is not the case, the new converter will replace the existing one because
+             * its source is more specific:  the source of 'converter' is of type <S>
while the
+             * source of 'existing' is of type <? super S>.
+             */
+            assert converter.getSourceClass() == key.sourceClass; // Enforced by parameterized
type.
+            if (existing.getSourceClass() == key.sourceClass) {
+                final boolean oldIsExact = existing .getTargetClass() == key.targetClass;
+                final boolean newIsExact = converter.getTargetClass() == key.targetClass;
+                if (oldIsExact & !newIsExact) {
+                    /*
+                     * The existing converter was defined exactly for the <S,T> classes,
while the
+                     * new one was defined for an other target. Do not touch the old converter
and
+                     * discard the new one. The new converter is not really lost since it
should
+                     * have been registered in a previous iteration for its own <S,T>
classes.
+                     */
+                    return;
+                }
+                if (newIsExact == oldIsExact) {
+                    /*
+                     * If no converter is considered more accurate than the other, keep both
of
+                     * them in a fallback chain. Note that the cast to <S,…> is safe
because we
+                     * checked the source class in the above 'if' statement.
+                     */
+                    converter = FallbackConverter.merge((ObjectConverter<S, ? extends
T>) existing, converter);
+                    assert key.targetClass.isAssignableFrom(converter.getTargetClass()) :
converter; // See FallbackConverter.merge javadoc.
+                }
+            }
+        }
+        put(key, converter);
+    }
+
+    /**
+     * Returns a converter for the specified source and target classes.
+     *
+     * @param  <S> The source class.
+     * @param  <T> The target class.
+     * @param  source The source class.
+     * @param  target The target class, or {@code Object.class} for any.
+     * @return The converter from the specified source class to the target class.
+     * @throws UnconvertibleObjectException if no converter is found.
+     */
+    public <S,T> ObjectConverter<? super S, ? extends T> find(final Class<S>
source, final Class<T> target)
+            throws UnconvertibleObjectException
+    {
+        final ClassPair<S,T> key = new ClassPair<>(source, target);
+        synchronized (converters) {
+            ObjectConverter<? super S, ? extends T> converter = get(key);
+            if (converter != null) {
                 return converter;
             }
-            lock = locks.writeLock();
-            lock.lock();
-            try {
-                existing = (SystemConverter<S,T>) converters.put(converter, converter);
-                if (existing != null) {
-                    converters.put(existing, existing);
-                } else {
-                    existing = converter;
+            /*
+             * At this point, no converter were found explicitly for the given key. Searches
a
+             * converter accepting some super-class of S, and if we find any cache the result.
+             * This is the complement of the search performed in the register(ObjectConverter)
+             * method, which looked for the parents of the target class. Here we process
the
+             * case of the source class.
+             */
+            ClassPair<? super S, T> candidate = key;
+            while ((candidate = candidate.parentSource()) != null) {
+                converter = get(candidate);
+                if (converter != null) {
+                    put(key, converter);
+                    return converter;
                 }
-            } finally {
-                lock.unlock();
+            }
+            /*
+             * No converter found. Gives a chance to subclasses to provide dynamically-generated
+             * converter. The default implementation does not provide any.
+             */
+            converter = createConverter(source, target);
+            if (converter != null) {
+                put(key, converter);
+                return converter;
             }
         }
-        return existing;
+        /*
+         * No explicit converter were found. Checks for the trivial case where an identity
+         * converter would fit. We perform this operation last in order to give a chance
to
+         * register an explicit converter if we need to.
+         */
+        if (target.isAssignableFrom(source)) {
+            return key.cast(IdentityConverter.create(source));
+        }
+        throw new UnconvertibleObjectException(Errors.format(Errors.Keys.CanNotConvertFromType_2,
source, target));
+    }
+
+    /**
+     * Creates a new converter for the given source and target types, or {@code null} if
none.
+     * This method is invoked by <code>{@linkplain #find find}(source, target)</code>
when no
+     * registered converter were found for the given types.
+     *
+     * @param  <S> The source class.
+     * @param  <T> The target class.
+     * @param  source The source class.
+     * @param  target The target class, or {@code Object.class} for any.
+     * @return A newly generated converter from the specified source class to the target
class,
+     *         or {@code null} if none.
+     */
+    protected <S,T> ObjectConverter<S,T> createConverter(final Class<S>
source, final Class<T> target) {
+        return null;
     }
 }

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=1456484&r1=1456483&r2=1456484&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] Thu Mar 14 15:37:01 2013
@@ -75,12 +75,47 @@ abstract class SystemConverter<S,T> exte
     }
 
     /**
+     * Performs the comparisons documented in {@link ClassPair#equals(Object)} with an additional
+     * check: if <strong>both</strong> objects to compare are {@code SystemConverter},
then also
+     * requires the two objects to be of the same class. We do that in order to differentiate
the
+     * "ordinary" converters from the {@link FallbackConverter}.
+     *
+     * {@section Implementation note}
+     * This is admittedly a little bit convolved. A cleaner approach would have been to not
allow
+     * the {@code ConverterRegister} hash map to contain anything else than {@code ClassPair}
keys,
+     * but the current strategy of using the same instance for keys and values reduces a
little bit
+     * the number of objects to create in the JVM. An other cleaner approach would have been
to
+     * compare {@code ObjectConverter}s in a separated method, but users invoking {@code
equals}
+     * on our system converters could be surprised.
+     *
+     * <p>Our {@code equals(Object)} definition have the following implications regarding
+     * the way to use the {@link ConverterRegistry#converters} map:</p>
+     * <ul>
+     *   <li>When searching for a converter of the same class than the key (as in the
+     *       {@link ConverterRegistry#findEquals(SystemConverter)} method), then there
+     *       is no restriction on the key that can be given to the {@code Map.get(K)}
+     *       method. The {@code Map} is "normal".</li>
+     *   <li>When searching for a converter for a pair of source and target classes
+     *       (as in {@link ConverterRegistry#find(Class, Class)}), the key shall be
+     *       an instance of {@code ClassPair} instance (not a subclass).</li>
+     * </ul>
+     */
+    @Override
+    public final boolean equals(final Object other) {
+        if (super.equals(other)) {
+            final Class<?> type = other.getClass();
+            return type == ClassPair.class || type == getClass();
+        }
+        return false;
+    }
+
+    /**
      * Returns an unique instance of this converter. If a converter already exists for the
same
      * source an target classes, then this converter is returned. Otherwise this converter
is
      * cached and returned.
      */
     final ObjectConverter<S,T> unique() {
-        return ConverterRegistry.SYSTEM.unique(this, true);
+        return ConverterRegistry.SYSTEM.unique(this);
     }
 
     /**
@@ -88,7 +123,8 @@ abstract class SystemConverter<S,T> exte
      * in the virtual machine, we do not cache the instance (for now) for security reasons.
      */
     protected final Object readResolve() throws ObjectStreamException {
-        return ConverterRegistry.SYSTEM.unique(this, false);
+        final ObjectConverter<S,T> existing = ConverterRegistry.SYSTEM.findEquals(this);
+        return (existing != null) ? existing : this;
     }
 
     /**



Mime
View raw message