sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject svn commit: r1723418 - in /sis/branches/JDK8/core/sis-referencing/src: main/java/org/apache/sis/referencing/factory/ main/java/org/apache/sis/referencing/factory/sql/ test/java/org/apache/sis/referencing/factory/ test/java/org/apache/sis/referencing/fa...
Date Wed, 06 Jan 2016 22:09:56 GMT
Author: desruisseaux
Date: Wed Jan  6 22:09:56 2016
New Revision: 1723418

URL: http://svn.apache.org/viewvc?rev=1723418&view=rev
Log:
Test and debug IdentifiedObjectFinder for EPSG dataset.

Modified:
    sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/ConcurrentAuthorityFactory.java
    sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/GeodeticAuthorityFactory.java
    sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/IdentifiedObjectFinder.java
    sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/sql/EPSGDataAccess.java
    sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/factory/AuthorityFactoryProxyTest.java
    sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/factory/IdentifiedObjectFinderTest.java
    sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/factory/sql/EPSGFactoryTest.java

Modified: sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/ConcurrentAuthorityFactory.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/ConcurrentAuthorityFactory.java?rev=1723418&r1=1723417&r2=1723418&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/ConcurrentAuthorityFactory.java [UTF-8] (original)
+++ sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/ConcurrentAuthorityFactory.java [UTF-8] Wed Jan  6 22:09:56 2016
@@ -51,12 +51,12 @@ import org.apache.sis.util.Disposable;
 import org.apache.sis.util.ArgumentChecks;
 import org.apache.sis.util.logging.Logging;
 import org.apache.sis.util.collection.Cache;
-import org.apache.sis.internal.referencing.NilReferencingObject;
 import org.apache.sis.internal.system.ReferenceQueueConsumer;
 import org.apache.sis.internal.system.DelayedExecutor;
 import org.apache.sis.internal.system.DelayedRunnable;
 import org.apache.sis.internal.system.Shutdown;
 import org.apache.sis.internal.system.Loggers;
+import org.apache.sis.internal.util.CollectionsExt;
 import org.apache.sis.util.logging.PerformanceLevel;
 import org.apache.sis.util.resources.Errors;
 import org.apache.sis.util.resources.Messages;
@@ -129,11 +129,11 @@ public abstract class ConcurrentAuthorit
 
     /**
      * The pool of objects identified by {@link Finder#find(IdentifiedObject)}.
-     * Values may be {@link NilReferencingObject} if an object has been searched but has not been found.
+     * Values may be an empty set if an object has been searched but has not been found.
      *
      * <p>Every access to this pool must be synchronized on {@code findPool}.</p>
      */
-    private final Map<IdentifiedObject, IdentifiedObject> findPool = new WeakHashMap<>();
+    private final Map<IdentifiedObject, Set<IdentifiedObject>> findPool = new WeakHashMap<>();
 
     /**
      * Holds the reference to a Data Access Object used by {@link ConcurrentAuthorityFactory}, together with
@@ -1673,21 +1673,14 @@ public abstract class ConcurrentAuthorit
      * @throws FactoryException if the finder can not be created.
      */
     @Override
-    public IdentifiedObjectFinder createIdentifiedObjectFinder(final Class<? extends IdentifiedObject> type)
-            throws FactoryException
-    {
-        return new Finder(this, type);
+    public IdentifiedObjectFinder newIdentifiedObjectFinder() throws FactoryException {
+        return new Finder(this);
     }
 
     /**
      * An implementation of {@link IdentifiedObjectFinder} which delegates
      * the work to the underlying Data Access Object and caches the result.
      *
-     * <div class="section">Implementation note</div>
-     * we will create objects using directly the underlying Data Access Object, not using the cache.
-     * This is because hundred of objects may be created during a scan while only one will be typically retained.
-     * We do not want to flood the cache with every false candidates that we encounter during the scan.
-     *
      * <div class="section">Synchronization note</div>
      * our public API claims that {@link IdentifiedObjectFinder}s are not thread-safe.
      * Nevertheless we synchronize this particular implementation for safety, because the consequence of misuse
@@ -1715,8 +1708,8 @@ public abstract class ConcurrentAuthorit
         /**
          * Creates a finder for the given type of objects.
          */
-        Finder(final ConcurrentAuthorityFactory<?> factory, final Class<? extends IdentifiedObject> type) {
-            super(factory, type);
+        Finder(final ConcurrentAuthorityFactory<?> factory) {
+            super(factory);
         }
 
         /**
@@ -1744,7 +1737,7 @@ public abstract class ConcurrentAuthorit
                  * method succeed, no matter what happen after this point.
                  */
                 acquireCount = 1;
-                finder = delegate.createIdentifiedObjectFinder(getObjectType());
+                finder = delegate.newIdentifiedObjectFinder();
                 finder.setWrapper(this);
             } else {
                 acquireCount++;
@@ -1798,19 +1791,19 @@ public abstract class ConcurrentAuthorit
          * The default implementation performs the same lookup than the Data Access Object and caches the result.
          */
         @Override
-        public IdentifiedObject find(final IdentifiedObject object) throws FactoryException {
-            final Map<IdentifiedObject, IdentifiedObject> findPool = ((ConcurrentAuthorityFactory<?>) factory).findPool;
+        public Set<IdentifiedObject> find(final IdentifiedObject object) throws FactoryException {
+            final Map<IdentifiedObject, Set<IdentifiedObject>> findPool = ((ConcurrentAuthorityFactory<?>) factory).findPool;
+            Set<IdentifiedObject> candidate;
             synchronized (findPool) {
-                final IdentifiedObject candidate = findPool.get(object);
-                if (candidate != null) {
-                    return (candidate == NilReferencingObject.INSTANCE) ? null : candidate;
-                }
+                candidate = findPool.get(object);
+            }
+            if (candidate != null) {
+                return candidate;
             }
             /*
              * Nothing has been found in the cache. Delegates the search to the Data Access Object.
              * We must delegate to 'finder' (not to 'super') in order to take advantage of overridden methods.
              */
-            final IdentifiedObject candidate;
             synchronized (this) {
                 try {
                     acquire();
@@ -1820,12 +1813,17 @@ public abstract class ConcurrentAuthorit
                 }
             }
             /*
-             * If the full scan was allowed, then stores the result even if null so
+             * If the full scan was allowed, then stores the result even if empty so
              * we can remember that no object has been found for the given argument.
              */
-            if (candidate != null || isFullScanAllowed()) {
+            if (isFullScanAllowed()) {
+                candidate = CollectionsExt.unmodifiableOrCopy(candidate);
+                Set<IdentifiedObject> concurrent;
                 synchronized (findPool) {
-                    findPool.put(object, (candidate == null) ? NilReferencingObject.INSTANCE : candidate);
+                    concurrent = findPool.putIfAbsent(object, candidate);
+                }
+                if (concurrent != null) {
+                    return concurrent;      // May happen if the same set has been computed in another thread.
                 }
             }
             return candidate;

Modified: sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/GeodeticAuthorityFactory.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/GeodeticAuthorityFactory.java?rev=1723418&r1=1723417&r2=1723418&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/GeodeticAuthorityFactory.java [UTF-8] (original)
+++ sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/GeodeticAuthorityFactory.java [UTF-8] Wed Jan  6 22:09:56 2016
@@ -1136,19 +1136,11 @@ public abstract class GeodeticAuthorityF
      * from an incomplete one, for example from an object without "{@code ID[…]}" or
      * "{@code AUTHORITY[…]}" element in <cite>Well Known Text</cite>.
      *
-     * <p>The {@code type} argument is a hint for optimizing the searches.
-     * The specified type should be a GeoAPI interface like {@code GeographicCRS.class},
-     * but this method accepts also implementation classes.
-     * If the type is unknown, one can use {@code IdentifiedObject.class}.
-     * However a more accurate type may help to speed up the search since it reduces the amount
-     * of tables to scan in some implementations (for example the factories backed by EPSG databases).</p>
-     *
-     * @param  type The type of objects to look for.
      * @return A finder to use for looking up unidentified objects.
      * @throws FactoryException if the finder can not be created.
      */
-    public IdentifiedObjectFinder createIdentifiedObjectFinder(Class<? extends IdentifiedObject> type) throws FactoryException {
-        return new IdentifiedObjectFinder(this, type);
+    public IdentifiedObjectFinder newIdentifiedObjectFinder() throws FactoryException {
+        return new IdentifiedObjectFinder(this);
     }
 
     /**

Modified: sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/IdentifiedObjectFinder.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/IdentifiedObjectFinder.java?rev=1723418&r1=1723417&r2=1723418&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/IdentifiedObjectFinder.java [UTF-8] (original)
+++ sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/IdentifiedObjectFinder.java [UTF-8] Wed Jan  6 22:09:56 2016
@@ -17,8 +17,9 @@
 package org.apache.sis.referencing.factory;
 
 import java.util.Set;
-import java.util.logging.Level;
-import java.util.logging.Logger;
+import java.util.Iterator;
+import java.util.Collections;
+import java.util.LinkedHashSet;
 import org.opengis.util.GenericName;
 import org.opengis.util.FactoryException;
 import org.opengis.metadata.Identifier;
@@ -29,11 +30,10 @@ import org.opengis.referencing.NoSuchAut
 import org.apache.sis.referencing.AbstractIdentifiedObject;
 import org.apache.sis.referencing.IdentifiedObjects;
 import org.apache.sis.internal.util.Citations;
-import org.apache.sis.internal.system.Loggers;
+import org.apache.sis.util.collection.BackingStoreException;
 import org.apache.sis.util.ArgumentChecks;
 import org.apache.sis.util.ComparisonMode;
 import org.apache.sis.util.Utilities;
-import org.apache.sis.util.logging.Logging;
 import org.apache.sis.util.resources.Errors;
 
 
@@ -46,10 +46,9 @@ import org.apache.sis.util.resources.Err
  * <p>The steps for using {@code IdentifiedObjectFinder} are:</p>
  * <ol>
  *   <li>Get a new instance by calling
- *       {@link GeodeticAuthorityFactory#createIdentifiedObjectFinder(Class)}.</li>
+ *       {@link GeodeticAuthorityFactory#newIdentifiedObjectFinder()}.</li>
  *   <li>Optionally configure that instance by calling its setter methods.</li>
- *   <li>Perform a search by invoking the {@link #find(IdentifiedObject)} or
- *       {@link #findIdentifier(IdentifiedObject)} methods.</li>
+ *   <li>Perform a search by invoking the {@link #find(IdentifiedObject)} method.</li>
  *   <li>The same {@code IdentifiedObjectFinder} instance can be reused for consecutive searches.</li>
  * </ol>
  *
@@ -62,7 +61,7 @@ import org.apache.sis.util.resources.Err
  * @version 0.7
  * @module
  *
- * @see GeodeticAuthorityFactory#createIdentifiedObjectFinder(Class)
+ * @see GeodeticAuthorityFactory#newIdentifiedObjectFinder()
  * @see IdentifiedObjects#lookupIdentifier(IdentifiedObject, boolean)
  */
 public class IdentifiedObjectFinder {
@@ -78,9 +77,9 @@ public class IdentifiedObjectFinder {
     protected final AuthorityFactory factory;
 
     /**
-     * The proxy for objects creation. This is usually set at construction time.
+     * The proxy for objects creation. This is updated before every object to search.
      */
-    private AuthorityFactoryProxy<?> proxy;
+    private transient AuthorityFactoryProxy<?> proxy;
 
     /**
      * The cache or the adapter which is wrapping this finder, or {@code null} if none.
@@ -97,25 +96,47 @@ public class IdentifiedObjectFinder {
     private transient IdentifiedObject searching;
 
     /**
+     * {@code true} if the {@link #find(IdentifiedObject)} method is invoked from the {@link #wrapper}.
+     * This is used for controlling the interactions with {@link #wrapper} during recursive calls.
+     * Example with {@code EPSGDataAccess}:
+     *
+     * <ol>
+     *   <li>When searching for a {@code ProjectedCRS}, first search for its base {@code GeodeticCRS}
+     *       in order to restrict the search scope.</li>
+     *   <li>The search for {@code GeodeticCRS} will in turn searches for {@code GeodeticDatum} in order
+     *       to restrict the search scope.</li>
+     *   <li>The search for {@code GeodeticDatum} will in turn searches for {@code Ellipsoid} in order
+     *       to restrict the search scope.</li>
+     * </ol>
+     *
+     * As seen from the above examples, there is usually no more than 4 recursive calls.
+     */
+    private transient boolean isInvokedFromWrapper;
+
+    /**
      * {@code true} for performing full scans, or {@code false} otherwise.
      */
     private boolean fullScan = true;
 
     /**
+     * {@code true} if the search should include deprecated objects.
+     */
+    private boolean includeDeprecated;
+
+    /**
      * Creates a finder using the specified factory.
      *
      * <div class="note"><b>Design note:</b>
      * this constructor is protected because instances of this class should not be created directly.
-     * Use {@link GeodeticAuthorityFactory#createIdentifiedObjectFinder(Class)} instead.</div>
+     * Use {@link GeodeticAuthorityFactory#newIdentifiedObjectFinder()} instead.</div>
      *
      * @param factory The factory to scan for the identified objects.
-     * @param type    The type of objects to lookup.
      *
-     * @see GeodeticAuthorityFactory#createIdentifiedObjectFinder(Class)
+     * @see GeodeticAuthorityFactory#newIdentifiedObjectFinder()
      */
-    protected IdentifiedObjectFinder(final AuthorityFactory factory, final Class<? extends IdentifiedObject> type) {
+    protected IdentifiedObjectFinder(final AuthorityFactory factory) {
+        ArgumentChecks.ensureNonNull("factory", factory);
         this.factory = factory;
-        proxy = AuthorityFactoryProxy.getInstance(type);
     }
 
     /**
@@ -126,25 +147,12 @@ public class IdentifiedObjectFinder {
      * <p>This method also copies the configuration of the given finder, thus providing a central place
      * where to add calls to setters methods if such methods are added in a future SIS version.</p>
      *
-     * <div class="section">Maintenance note</div>
-     * Adding properties to this method is not sufficient. See also the classes that override
-     * {@link #setFullScanAllowed(boolean)} — some of them may be defined in other packages.
-     *
      * @param other The cache or the adapter wrapping this finder.
      */
     final void setWrapper(final IdentifiedObjectFinder other) {
         wrapper = other;
         setFullScanAllowed(other.isFullScanAllowed());
-    }
-
-    /**
-     * Returns the type of the objects to be created by the proxy instance.
-     * This method needs to be overridden by the sub-classes that do not define a proxy.
-     *
-     * @return The type of object to be created, as a GeoAPI interface.
-     */
-    Class<? extends IdentifiedObject> getObjectType() {
-        return proxy.type.asSubclass(IdentifiedObject.class);
+        setIncludeDeprecated(other.getIncludeDeprecated());
     }
 
     /**
@@ -170,18 +178,38 @@ public class IdentifiedObjectFinder {
     }
 
     /**
-     * Sets whatever an exhaustive scan against all registered objects is allowed.
+     * Sets whether an exhaustive scan against all registered objects is allowed.
      * The default value is {@code true}.
      *
-     * @param fullScan {@code true} for allowing exhaustive scans,
+     * @param allowed {@code true} for allowing exhaustive scans,
      *        or {@code false} for faster but less complete searches.
      */
-    public void setFullScanAllowed(final boolean fullScan) {
-        this.fullScan = fullScan;
+    public void setFullScanAllowed(final boolean allowed) {
+        fullScan = allowed;
+    }
+
+    /**
+     * If {@code true} and full scans are allowed, then the search will include deprecated objects.
+     * The default value is {@code false}.
+     *
+     * @return {@code true} if the search should include deprecated objects.
+     */
+    public boolean getIncludeDeprecated() {
+        return includeDeprecated;
+    }
+
+    /**
+     * Sets whether the search should include deprecated objects.
+     * The default value is {@code false}.
+     *
+     * @param include {@code true} for including deprecated objects in the search.
+     */
+    public void setIncludeDeprecated(final boolean include) {
+        includeDeprecated = include;
     }
 
     /**
-     * Lookups an object which is approximatively equal to the specified object.
+     * Lookups objects which are approximatively equal to the specified object.
      * The default implementation tries to instantiate some {@linkplain AbstractIdentifiedObject identified objects}
      * from the authority factory specified at construction time, in the following order:
      *
@@ -193,98 +221,104 @@ public class IdentifiedObjectFinder {
      *       in addition of identifiers, then the name and {@linkplain AbstractIdentifiedObject#getAlias() aliases} are
      *       used for creating objects to be tested.</li>
      *   <li>If {@linkplain #isFullScanAllowed() full scan is allowed}, then full {@linkplain #getCodeCandidates
-     *       set of authority codes} are used for creating objects to be tested.</li>
+     *       set of candidate codes} is used for creating objects to be tested.</li>
      * </ul>
      *
-     * The first of the above created objects which is equal to the specified object in the
-     * the sense of {@link ComparisonMode#APPROXIMATIVE} is returned.
+     * The created objects which are equal to the specified object in the
+     * the sense of {@link ComparisonMode#APPROXIMATIVE} are returned.
      *
      * @param  object The object looked up.
-     * @return The identified object, or {@code null} if not found.
+     * @return The identified objects, or an empty set if not found.
      * @throws FactoryException if an error occurred while creating an object.
      */
-    public IdentifiedObject find(final IdentifiedObject object) throws FactoryException {
+    public Set<IdentifiedObject> find(final IdentifiedObject object) throws FactoryException {
         ArgumentChecks.ensureNonNull("object", object);
         final IdentifiedObject state = searching;
+        if (state == object) {
+            throw new IllegalArgumentException(Errors.format(Errors.Keys.RecursiveCreateCallForKey_1, object.getName()));
+        }
+        if (state != null) {
+            /*
+             * This 'find' method may be invoked recursively by some implementations that need to resolve
+             * the dependencies of the given object. For example if the given object is a geodetic CRS,
+             * then the finder backed by the EPSG database will first search for a matching ellipsoid in
+             * order to reduce the scope of the search.
+             *
+             * When invoked recursively, we delegate to the wrapper if it exists. The wrapper is typically
+             * a ConcurrentAuthorityFactory.Finder, in which case we want to leverage its cache capability.
+             */
+            if (wrapper != null && !isInvokedFromWrapper) try {
+                isInvokedFromWrapper = true;
+                return wrapper.find(object);
+            } finally {
+                isInvokedFromWrapper = false;
+            }
+        }
+        final boolean ivf = isInvokedFromWrapper;
+        final AuthorityFactoryProxy<?> p = proxy;
+        proxy = AuthorityFactoryProxy.getInstance(object.getClass());
+        isInvokedFromWrapper = false;
+        searching = object;
         try {
-            searching = object;
-            if (state == null) {
-                /*
-                 * First check if one of the identifiers can be used to find directly an identified object.
-                 * Verify that the object that we found is actually equal to given one; we do not blindly
-                 * trust the identifiers in the user object.
-                 */
-                IdentifiedObject candidate = createFromIdentifiers(object);
-                if (candidate != null) {
-                    return candidate;
-                }
-                /*
-                 * We are unable to find the object from its identifiers. Try a quick name lookup.
-                 * Some implementations like the one backed by the EPSG database are capable to find
-                 * an object from its name.
-                 */
-                candidate = createFromNames(object);
-                if (candidate != null) {
-                    return candidate;
-                }
-                /*
-                 * Here we exhausted the quick paths. Perform a full scan (costly) if we are allowed to,
-                 * otherwise abandon.
-                 */
-                return fullScan ? createFromCodes(object) : null;
-            } else if (state != object) {
-                /*
-                 * This 'find' method may be invoked recursively by some implementations that need to resolve
-                 * the dependencies of the given object. For example if the given object is a geodetic CRS,
-                 * the finder backed by the EPSG database will first search for a matching ellipsoid in order
-                 * to reduce the scope of the search. Note that since dependencies can be of any type, we
-                 * have to temporarily change the proxy.
-                 *
-                 * When invoked recursively, we delegate to the wrapper if it exists. The wrapper is typically
-                 * a ConcurrentAuthorityFactory.Finder, in which case we want to leverage its cache capability.
-                 */
-                IdentifiedObjectFinder finder = wrapper;
-                if (finder == null) {
-                    finder = this;
-                }
-                final AuthorityFactoryProxy<?> old = finder.proxy;
-                finder.proxy = AuthorityFactoryProxy.getInstance(object.getClass());
-                try {
-                    return finder.find(object);
-                } finally {
-                    finder.proxy = old;
-                }
-            } else {
-                throw new IllegalArgumentException(Errors.format(Errors.Keys.RecursiveCreateCallForKey_1, object.getName()));
+            /*
+             * First check if one of the identifiers can be used to find directly an identified object.
+             * Verify that the object that we found is actually equal to given one; we do not blindly
+             * trust the identifiers in the user object.
+             */
+            IdentifiedObject candidate = createFromIdentifiers(object);
+            if (candidate != null) {
+                return Collections.singleton(candidate);
+            }
+            /*
+             * We are unable to find the object from its identifiers. Try a quick name lookup.
+             * Some implementations like the one backed by the EPSG database are capable to find
+             * an object from its name.
+             */
+            candidate = createFromNames(object);
+            if (candidate != null) {
+                return Collections.singleton(candidate);
             }
+            /*
+             * Here we exhausted the quick paths. Perform a full scan (costly) if we are allowed to,
+             * otherwise abandon.
+             */
+            return fullScan ? createFromCodes(object) : Collections.emptySet();
         } finally {
+            isInvokedFromWrapper = ivf;
             searching = state;
+            proxy = p;
         }
     }
 
     /**
-     * Returns the identifier of the specified object, or {@code null} if none.
-     * On some implementations, invoking this method is more efficient than invoking
-     * {@link #find(IdentifiedObject)} when the full object is not needed.
+     * Lookups only one object which is approximatively equal to the specified object.
+     * If the set returned by {@link #find(IdentifiedObject)} contains exactly one element,
+     * then that element is returned. Otherwise this method returns {@code null}.
      *
-     * <p>The default implementation invokes <code>{@linkplain #find find}(object)</code> and extracts
-     * the identifier code from the returned {@linkplain AbstractIdentifiedObject identified object}.</p>
+     * <p>Note that this method returns {@code null} even if there is more than one element,
+     * because in such case we consider that there is an ambiguity.</p>
      *
      * @param  object The object looked up.
-     * @return The identifier of the given object, or {@code null} if none were found.
+     * @return The identified object, or {@code null} if none or ambiguous.
      * @throws FactoryException if an error occurred while creating an object.
      */
-    public String findIdentifier(final IdentifiedObject object) throws FactoryException {
-        final IdentifiedObject candidate = find(object);
-        if (candidate == null) {
-            return null;
-        }
-        final Citation authority = getAuthority();
-        Identifier identifier = IdentifiedObjects.getIdentifier(candidate, authority);
-        if (identifier == null) {
-            identifier = candidate.getName();
+    public IdentifiedObject findSingleton(final IdentifiedObject object) throws FactoryException {
+        /*
+         * Do not invoke Set.size() because it may be a costly operation if the subclass
+         * implements a mechanism that create IdentifiedObject instances only on demand.
+         */
+        try {
+            final Iterator<IdentifiedObject> it = find(object).iterator();
+            if (it.hasNext()) {
+                final IdentifiedObject candidate = it.next();
+                if (!it.hasNext()) {
+                    return candidate;
+                }
+            }
+        } catch (BackingStoreException e) {
+            throw e.unwrapOrRethrow(FactoryException.class);
         }
-        return IdentifiedObjects.toString(identifier);
+        return null;
     }
 
     /**
@@ -301,7 +335,7 @@ public class IdentifiedObjectFinder {
      * @see #createFromCodes(IdentifiedObject)
      * @see #createFromNames(IdentifiedObject)
      */
-    final IdentifiedObject createFromIdentifiers(final IdentifiedObject object) throws FactoryException {
+    private IdentifiedObject createFromIdentifiers(final IdentifiedObject object) throws FactoryException {
         final Citation authority = getAuthority();
         for (final Identifier id : object.getIdentifiers()) {
             if (Citations.identifierMatches(authority, id.getAuthority())) {
@@ -337,7 +371,7 @@ public class IdentifiedObjectFinder {
      * @see #createFromCodes(IdentifiedObject)
      * @see #createFromIdentifiers(IdentifiedObject)
      */
-    final IdentifiedObject createFromNames(final IdentifiedObject object) throws FactoryException {
+    private IdentifiedObject createFromNames(final IdentifiedObject object) throws FactoryException {
         String code = object.getName().getCode();
         IdentifiedObject candidate;
         try {
@@ -390,32 +424,34 @@ public class IdentifiedObjectFinder {
      * @see #createFromIdentifiers(IdentifiedObject)
      * @see #createFromNames(IdentifiedObject)
      */
-    final IdentifiedObject createFromCodes(final IdentifiedObject object) throws FactoryException {
-        final Set<String> codes = getCodeCandidates(object);
-        if (!codes.isEmpty()) {
-            for (final String code : codes) {
-                final IdentifiedObject candidate;
-                try {
-                    candidate = create(code);
-                } catch (FactoryException e) {
-                    continue;
-                }
-                if (Utilities.deepEquals(candidate, object, COMPARISON_MODE)) {
-                    return candidate;
+    private Set<IdentifiedObject> createFromCodes(final IdentifiedObject object) throws FactoryException {
+        boolean strict = false;
+        final Set<IdentifiedObject> result = new LinkedHashSet<>();     // We need to preserve order.
+        for (final String code : getCodeCandidates(object)) {
+            final IdentifiedObject candidate;
+            try {
+                candidate = create(code);
+            } catch (FactoryException e) {
+                continue;
+            }
+            if (Utilities.deepEquals(candidate, object, strict ? ComparisonMode.IGNORE_METADATA : COMPARISON_MODE)) {
+                if (!strict && Utilities.deepEquals(candidate, object, ComparisonMode.IGNORE_METADATA)) {
+                    /*
+                     * If we find at least one object without rounding error, do not accept rounding error for anyone.
+                     */
+                    result.clear();
+                    strict = true;
                 }
+                result.add(candidate);
             }
-            final Logger logger = Logging.getLogger(Loggers.CRS_FACTORY);
-            logger.log(Level.FINEST, "No match found for “{0}” among {1}",
-                    new Object[] {object.getName(), codes});
         }
-        return null;
+        return result;
     }
 
     /**
-     * Creates an object for the given code. This method is invoked by the default implementation of
-     * {@link #find(IdentifiedObject)} and {@link #findIdentifier(IdentifiedObject)} methods for each
-     * code returned by the {@link #getCodeCandidates(IdentifiedObject)} method, in iteration order,
-     * until an object approximatively equals to the requested object is found.
+     * Creates an object for the given code. This method is invoked by the default {@link #find(IdentifiedObject)}
+     * method implementation of for each code returned by the {@link #getCodeCandidates(IdentifiedObject)} method,
+     * in iteration order.
      *
      * @param  code The authority code for which to create an object.
      * @return The identified object for the given code, or {@code null} to stop attempts.
@@ -448,6 +484,11 @@ public class IdentifiedObjectFinder {
      * @throws FactoryException if an error occurred while fetching the set of code candidates.
      */
     protected Set<String> getCodeCandidates(final IdentifiedObject object) throws FactoryException {
-        return factory.getAuthorityCodes(getObjectType());
+        AuthorityFactoryProxy<?> p = proxy;
+        if (object != searching) {
+            // Should not happen, unless the user invokes this method with unusual objects.
+            p = AuthorityFactoryProxy.getInstance(object.getClass());
+        }
+        return factory.getAuthorityCodes(p.type.asSubclass(IdentifiedObject.class));
     }
 }

Modified: sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/sql/EPSGDataAccess.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/sql/EPSGDataAccess.java?rev=1723418&r1=1723417&r2=1723418&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/sql/EPSGDataAccess.java [UTF-8] (original)
+++ sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/sql/EPSGDataAccess.java [UTF-8] Wed Jan  6 22:09:56 2016
@@ -2730,19 +2730,12 @@ addURIs:    for (int i=0; ; i++) {
      * The finder tries to fetch a fully {@linkplain AbstractIdentifiedObject identified object} from an incomplete one,
      * for example from an object without "{@code ID[…]}" or "{@code AUTHORITY[…]}" element in <cite>Well Known Text</cite>.
      *
-     * <p>The {@code type} argument is a hint for optimizing the searches.
-     * If the type is unknown, one can use {@code IdentifiedObject.class}.
-     * However a more accurate type help to speed up the search since it reduces the amount of tables to scan.</p>
-     *
-     * @param  type The type of objects to look for.
      * @return A finder to use for looking up unidentified objects.
      * @throws FactoryException if the finder can not be created.
      */
     @Override
-    public IdentifiedObjectFinder createIdentifiedObjectFinder(final Class<? extends IdentifiedObject> type)
-            throws FactoryException
-    {
-        return new Finder(type);
+    public IdentifiedObjectFinder newIdentifiedObjectFinder() throws FactoryException {
+        return new Finder();
     }
 
     /**
@@ -2752,17 +2745,8 @@ addURIs:    for (int i=0; ; i++) {
         /**
          * Creates a new finder.
          */
-        Finder(final Class<? extends IdentifiedObject> type) {
-            super(parent, type);
-        }
-
-        /**
-         * Returns the authority of the codes to search.
-         * Overridden for efficiency.
-         */
-        @Override
-        public Citation getAuthority() {
-            return Citations.EPSG;
+        Finder() {
+            super(parent);
         }
 
         /**
@@ -2775,14 +2759,14 @@ addURIs:    for (int i=0; ; i++) {
             String select = "COORD_REF_SYS_CODE";
             String from   = "Coordinate Reference System";
             final String where;
-            final Comparable<?> code;
+            final Set<Comparable<?>> codes;
             if (object instanceof Ellipsoid) {
                 select = "ELLIPSOID_CODE";
                 from   = "Ellipsoid";
                 where  = "SEMI_MAJOR_AXIS";
-                code   = ((Ellipsoid) object).getSemiMajorAxis();
+                codes  = Collections.singleton(((Ellipsoid) object).getSemiMajorAxis());
             } else {
-                IdentifiedObject dependency;
+                final IdentifiedObject dependency;
                 if (object instanceof GeneralDerivedCRS) {
                     dependency = ((GeneralDerivedCRS) object).getBaseCRS();
                     where      = "SOURCE_GEOGCRS_CODE";
@@ -2801,30 +2785,26 @@ addURIs:    for (int i=0; ; i++) {
                 /*
                  * Search for the dependency.  The super.find(…) method performs a check (not documented in public API)
                  * for detecting when it is invoked recursively, which is the case here. Consequently the super.find(…)
-                 * behavior below is slightly different than usual:
-                 *
-                 * 1) Since invoked recursively, super.find(…) accepts an argument of different type than the type
-                 *    given at construction time.
-                 *
-                 * 2) Since invoked recursively, super.find(…) checks the cache of the ConcurrentAuthorityFactory
-                 *    wrapper (if any). If found, the dependency will also be stored in the cache. This is desirable
-                 *    since this method may be invoked (indirectly) in a loop for many CRS objects sharing the same
-                 *    CoordinateSystem or Datum dependencies.
+                 * behavior below is slightly different than usual: since invoked recursively, super.find(…) checks the
+                 * cache of the ConcurrentAuthorityFactory wrapper. If found, the dependency will also be stored in the
+                 * cache. This is desirable since this method may be invoked (indirectly) in a loop for many CRS objects
+                 * sharing the same CoordinateSystem or Datum dependencies.
                  */
-                dependency = find(dependency);
-                if (dependency == null) {
+                codes = new LinkedHashSet<>();
+                for (final IdentifiedObject dep : find(dependency)) {
+                    Identifier id = IdentifiedObjects.getIdentifier(dep, Citations.EPSG);
+                    if (id != null) {               // Should never be null, but let be safe.
+                        codes.add(id.getCode());
+                    }
+                }
+                codes.remove(null);                 // Paranoiac safety.
+                if (codes.isEmpty()) {
                     // Dependency not found.
                     return Collections.emptySet();
                 }
-                Identifier id = IdentifiedObjects.getIdentifier(dependency, Citations.EPSG);
-                if (id == null || (code = id.getCode()) == null) {
-                    // Identifier not found (malformed CRS object?).
-                    // Conservatively scans all objects.
-                    return super.getCodeCandidates(object);
-                }
             }
             /*
-             * Build the SQL statement. The code can be any of the following type:
+             * Build the SQL statement. The code can be any of the following types:
              *
              * - A String, which represent a foreigner key as an integer value.
              *   The search will require an exact match.
@@ -2832,36 +2812,43 @@ addURIs:    for (int i=0; ; i++) {
              * - A floating point number, in which case the search will be performed
              *   with a tolerance threshold of 1 cm for a planet of the size of Earth.
              */
+            final Set<String> result = new LinkedHashSet<>();       // We need to preserve order in this set.
             final StringBuilder buffer = new StringBuilder(60);
             buffer.append("SELECT ").append(select).append(" FROM [").append(from).append("] WHERE ").append(where);
-            if (code instanceof Number) {
-                final double value = ((Number) code).doubleValue();
-                final double tolerance = Math.abs(value * (Formulas.LINEAR_TOLERANCE / ReferencingServices.AUTHALIC_RADIUS));
-                buffer.append(">=").append(value - tolerance).append(" AND ").append(where)
-                      .append("<=").append(value + tolerance);
-            } else {
-                buffer.append('=').append(code);
-            }
-            buffer.append(" ORDER BY ABS(DEPRECATED), ");
-            if (code instanceof Number) {
-                buffer.append("ABS(").append(select).append('-').append(code).append(')');
-            } else {
-                buffer.append(select);          // Only for making order determinist.
-            }
-            final Set<String> result = new LinkedHashSet<>();
-            try {
-                final String sql = translator.apply(buffer.toString());
-                try (Statement s = connection.createStatement();
-                     ResultSet r = s.executeQuery(sql))
-                {
-                    while (r.next()) {
-                        result.add(r.getString(1));
+            final int start = buffer.length();
+            for (final Comparable<?> code : codes) {
+                buffer.setLength(start);
+                if (code instanceof Number) {
+                    final double value = ((Number) code).doubleValue();
+                    final double tolerance = Math.abs(value * (Formulas.LINEAR_TOLERANCE / ReferencingServices.AUTHALIC_RADIUS));
+                    buffer.append(">=").append(value - tolerance).append(" AND ").append(where)
+                          .append("<=").append(value + tolerance);
+                } else {
+                    buffer.append('=').append(code);
+                }
+                if (!getIncludeDeprecated()) {
+                    buffer.append(" AND DEPRECATED=0");
+                }
+                buffer.append(" ORDER BY ABS(DEPRECATED), ");
+                if (code instanceof Number) {
+                    buffer.append("ABS(").append(select).append('-').append(code).append(')');
+                } else {
+                    buffer.append(select);          // Only for making order determinist.
+                }
+                try {
+                    final String sql = translator.apply(buffer.toString());
+                    try (Statement s = connection.createStatement();
+                         ResultSet r = s.executeQuery(sql))
+                    {
+                        while (r.next()) {
+                            result.add(r.getString(1));
+                        }
                     }
+                } catch (SQLException exception) {
+                    throw databaseFailure(Identifier.class, String.valueOf(code), exception);
                 }
-            } catch (SQLException exception) {
-                throw databaseFailure(Identifier.class, String.valueOf(code), exception);
+                result.remove(null);    // Should not have null element, but let be safe.
             }
-            result.remove(null);    // Should not have null element, but let be safe.
             return result;
         }
     }

Modified: sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/factory/AuthorityFactoryProxyTest.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/factory/AuthorityFactoryProxyTest.java?rev=1723418&r1=1723417&r2=1723418&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/factory/AuthorityFactoryProxyTest.java [UTF-8] (original)
+++ sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/factory/AuthorityFactoryProxyTest.java [UTF-8] Wed Jan  6 22:09:56 2016
@@ -27,7 +27,6 @@ import org.apache.sis.referencing.datum.
 import org.apache.sis.referencing.crs.DefaultGeographicCRS;
 import org.apache.sis.referencing.crs.DefaultProjectedCRS;
 import org.apache.sis.referencing.crs.DefaultDerivedCRS;
-import org.apache.sis.referencing.CommonCRS;
 
 import org.apache.sis.test.DependsOn;
 import org.apache.sis.test.TestCase;
@@ -74,30 +73,6 @@ public final strictfp class AuthorityFac
     }
 
     /**
-     * Tests {@link IdentifiedObjectFinder#createFromCodes(IdentifiedObject)}.
-     * We use the {@link CommonAuthorityFactory} for testing purpose.
-     *
-     * @throws FactoryException if an error occurred while creating a CRS.
-     */
-    @Test
-    public void testCreateFromCodes() throws FactoryException {
-        final CRSAuthorityFactory factory = new CommonAuthorityFactory(DefaultFactories.forBuildin(NameFactory.class));
-        final IdentifiedObjectFinder proxy = new IdentifiedObjectFinder(factory, GeographicCRS.class);
-        CoordinateReferenceSystem expected = factory.createCoordinateReferenceSystem("84");
-        assertSame(expected, CommonCRS.WGS84.normalizedGeographic());
-        assertSame   (expected, proxy.createFromCodes      (expected));
-        assertSame   (expected, proxy.createFromIdentifiers(expected));
-        assertNull   (          proxy.createFromNames      (expected));
-        assertSame   (expected, proxy.createFromCodes      (CommonCRS.WGS84.normalizedGeographic()));
-        assertNull   (          proxy.createFromNames      (CommonCRS.WGS84.normalizedGeographic()));
-
-        expected = factory.createCoordinateReferenceSystem("83");
-        assertSame   (expected, proxy.createFromCodes      (expected));
-        assertSame   (expected, proxy.createFromIdentifiers(expected));
-        assertNull   (          proxy.createFromNames      (expected));
-    }
-
-    /**
      * Tests {@link AuthorityFactoryProxy#createFromAPI(AuthorityFactory, String)}.
      * We use the {@link CommonAuthorityFactory} for testing purpose.
      *

Modified: sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/factory/IdentifiedObjectFinderTest.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/factory/IdentifiedObjectFinderTest.java?rev=1723418&r1=1723417&r2=1723418&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/factory/IdentifiedObjectFinderTest.java [UTF-8] (original)
+++ sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/factory/IdentifiedObjectFinderTest.java [UTF-8] Wed Jan  6 22:09:56 2016
@@ -59,23 +59,23 @@ public final strictfp class IdentifiedOb
     }
 
     /**
-     * Tests the {@link IdentifiedObjectFinder#find(IdentifiedObject)} method.
+     * Tests the {@link IdentifiedObjectFinder#findSingleton(IdentifiedObject)} method.
      *
      * @throws FactoryException if the creation of a CRS failed.
      */
     @Test
-    public void testFind() throws FactoryException {
+    public void testFindSingleton() throws FactoryException {
         final GeographicCRS CRS84 = factory.createGeographicCRS("CRS:84");
-        final IdentifiedObjectFinder finder = factory.createIdentifiedObjectFinder(CoordinateReferenceSystem.class);
+        final IdentifiedObjectFinder finder = factory.newIdentifiedObjectFinder();
         assertTrue("Newly created finder should default to full scan.", finder.isFullScanAllowed());
 
         finder.setFullScanAllowed(false);
         assertSame("Should find without the need for scan, since we can use the CRS:84 identifier.",
-                   CRS84, finder.find(CRS84));
+                   CRS84, finder.findSingleton(CRS84));
 
         finder.setFullScanAllowed(true);
         assertSame("Allowing scanning should not make any difference for this CRS84 instance.",
-                   CRS84, finder.find(CRS84));
+                   CRS84, finder.findSingleton(CRS84));
         /*
          * Same test than above, using a CRS without identifier.
          * The intend is to force a full scan.
@@ -87,26 +87,24 @@ public final strictfp class IdentifiedOb
 
         finder.setFullScanAllowed(false);
         assertNull("Should not find WGS84 without a full scan, since it does not contains the CRS:84 identifier.",
-                   finder.find(search));
+                   finder.findSingleton(search));
 
         finder.setFullScanAllowed(true);
         assertSame("A full scan should allow us to find WGS84, since it is equals ignoring metadata to CRS:84.",
-                   CRS84, finder.find(search));
-
-        assertEquals("CRS:84", finder.findIdentifier(search));
+                   CRS84, finder.findSingleton(search));
     }
 
     /**
-     * Tests the {@link IdentifiedObjectFinder#find(IdentifiedObject)} method through the finder provided by
-     * {@link ConcurrentAuthorityFactory}. The objects found are expected to be cached.
+     * Tests the {@link IdentifiedObjectFinder#findSingleton(IdentifiedObject)} method through the finder
+     * provided by {@link ConcurrentAuthorityFactory}. The objects found are expected to be cached.
      *
      * @throws FactoryException if the creation of a CRS failed.
      */
     @Test
-    @DependsOnMethod("testFind")
-    public void testFindOnCachedInstance() throws FactoryException {
+    @DependsOnMethod("testFindSingleton")
+    public void testFindOnCachingInstance() throws FactoryException {
         factory = new Cached(factory);
-        testFind();
+        testFindSingleton();
     }
 
     /**

Modified: sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/factory/sql/EPSGFactoryTest.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/factory/sql/EPSGFactoryTest.java?rev=1723418&r1=1723417&r2=1723418&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/factory/sql/EPSGFactoryTest.java [UTF-8] (original)
+++ sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/factory/sql/EPSGFactoryTest.java [UTF-8] Wed Jan  6 22:09:56 2016
@@ -21,6 +21,7 @@ import java.util.List;
 import java.util.Locale;
 import org.opengis.metadata.extent.GeographicBoundingBox;
 import org.opengis.parameter.ParameterValueGroup;
+import org.opengis.referencing.IdentifiedObject;
 import org.opengis.referencing.NoSuchAuthorityCodeException;
 import org.opengis.referencing.crs.*;
 import org.opengis.referencing.cs.AxisDirection;
@@ -34,11 +35,14 @@ import org.opengis.referencing.operation
 import org.opengis.util.FactoryException;
 import org.apache.sis.util.logging.Logging;
 import org.apache.sis.internal.system.Loggers;
+import org.apache.sis.referencing.CRS;
+import org.apache.sis.referencing.cs.AxesConvention;
+import org.apache.sis.referencing.crs.DefaultGeographicCRS;
 import org.apache.sis.referencing.datum.BursaWolfParameters;
 import org.apache.sis.referencing.datum.DefaultGeodeticDatum;
 import org.apache.sis.referencing.operation.AbstractCoordinateOperation;
+import org.apache.sis.referencing.factory.IdentifiedObjectFinder;
 import org.apache.sis.referencing.factory.UnavailableFactoryException;
-import org.apache.sis.referencing.CRS;
 
 // Test imports
 import org.junit.Rule;
@@ -716,4 +720,50 @@ public final strictfp class EPSGFactoryT
         }
         assertEquals(count, all.size());        // Size may have been modified after above loop.
     }
+
+    /**
+     * Tests {@link EPSGFactory#newIdentifiedObjectFinder()} method with a geographic CRS.
+     *
+     * @throws FactoryException if an error occurred while querying the factory.
+     */
+    @Test
+    @DependsOnMethod("testWGS84")
+    public void testFindGeographic() throws FactoryException {
+        assumeNotNull(factory);
+        final IdentifiedObjectFinder finder = factory.newIdentifiedObjectFinder();
+        final DefaultGeographicCRS crs = (DefaultGeographicCRS) CRS.fromWKT(
+                "GEOGCS[“WGS 84”,\n" +
+                "  DATUM[“WGS 84”,\n" +     // Use the alias instead than primary name for forcing a deeper search.
+                "    SPHEROID[“WGS 1984”, 6378137.0, 298.257223563]],\n" +  // Different name for forcing a deeper search.
+                "  PRIMEM[“Greenwich”, 0.0],\n" +
+                "  UNIT[“degree”, 0.017453292519943295],\n" +
+                "  AXIS[“Geodetic latitude”, NORTH],\n" +
+                "  AXIS[“Geodetic longitude”, EAST]]");
+        /*
+         * First, search for a CRS with axis order that does not match the ones in the EPSG database.
+         * IdentifiedObjectFinder should not accept EPSG::4326 as a match for the given CRS.
+         */
+        assertTrue("Full scan should be enabled by default.", finder.isFullScanAllowed());
+        assertTrue("Should not find WGS84 because the axis order is not the same.",
+                   finder.find(crs.forConvention(AxesConvention.NORMALIZED)).isEmpty());
+        /*
+         * Ensure that the cache is empty.
+         */
+        finder.setFullScanAllowed(false);
+        assertTrue("Should not find without a full scan, because the WKT contains no identifier " +
+                   "and the CRS name is ambiguous (more than one EPSG object have this name).",
+                   finder.find(crs).isEmpty());
+        /*
+         * Scan the database for searching the CRS.
+         */
+        finder.setFullScanAllowed(true);
+        final IdentifiedObject found = finder.findSingleton(crs);
+        assertNotNull("With full scan allowed, the CRS should be found.", found);
+        assertEpsgNameAndIdentifierEqual("WGS 84", 4326, found);
+        /*
+         * Should find the CRS without the need of a full scan, because of the cache.
+         */
+        finder.setFullScanAllowed(false);
+        assertSame("The CRS should still in the cache.", found, finder.findSingleton(crs));
+    }
 }



Mime
View raw message