sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject svn commit: r1726601 - in /sis/branches/JDK8/core: sis-metadata/src/main/java/org/apache/sis/internal/metadata/ sis-referencing/src/main/java/org/apache/sis/referencing/factory/ sis-utility/src/main/java/org/apache/sis/internal/util/ sis-utility/src/te...
Date Mon, 25 Jan 2016 09:51:33 GMT
Author: desruisseaux
Date: Mon Jan 25 09:51:33 2016
New Revision: 1726601

URL: http://svn.apache.org/viewvc?rev=1726601&view=rev
Log:
Prepare for implementation of MultiAuthoritiesFactory.getAuthorityCodes():
reduce the amount of code during which a synchronization lock is hold,
and delegate part of this synchronization mechanism to a custom iterator.

Added:
    sis/branches/JDK8/core/sis-utility/src/main/java/org/apache/sis/internal/util/LazySynchronizedSetIterator.java
  (with props)
Modified:
    sis/branches/JDK8/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/NameMeaning.java
    sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/AuthorityFactoryIdentifier.java
    sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/MultiAuthoritiesFactory.java
    sis/branches/JDK8/core/sis-utility/src/test/java/org/apache/sis/internal/util/DefinitionURITest.java

Modified: sis/branches/JDK8/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/NameMeaning.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/NameMeaning.java?rev=1726601&r1=1726600&r2=1726601&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/NameMeaning.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/NameMeaning.java
[UTF-8] Mon Jan 25 09:51:33 2016
@@ -19,6 +19,7 @@ package org.apache.sis.internal.metadata
 import java.util.Map;
 import java.util.HashMap;
 import java.util.Locale;
+import javax.measure.unit.Unit;
 import org.opengis.parameter.*;
 import org.opengis.referencing.*;
 import org.opengis.referencing.cs.*;
@@ -63,12 +64,21 @@ public final class NameMeaning extends S
         CoordinateOperation.class,
         OperationMethod.class,
         ParameterDescriptor.class,
-        ReferenceSystem.class
+        ReferenceSystem.class,
+        Unit.class
     };
 
     /**
      * The object types for instances of {@link #CLASSES}.
      * See {@link org.apache.sis.internal.util.DefinitionURI} javadoc for a list of object
types in URN.
+     *
+     * <p>Types not yet listed (waiting to see if there is a use for them):</p>
+     *
+     * "group"              for  ParameterValueGroup.class;
+     * "verticalDatumType"  for  VerticalDatumType.class;
+     * "pixelInCell"        for  PixelInCell.class;
+     * "rangeMeaning"       for  RangeMeaning.class;
+     * "axisDirection"      for  AxisDirection.class;
      */
     private static final String[] TYPES = {
         "crs",
@@ -80,7 +90,8 @@ public final class NameMeaning extends S
         "coordinateOperation",
         "method",
         "parameter",
-        "referenceSystem"
+        "referenceSystem",
+        "uom"
     };
 
     /**

Modified: sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/AuthorityFactoryIdentifier.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/AuthorityFactoryIdentifier.java?rev=1726601&r1=1726600&r2=1726601&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/AuthorityFactoryIdentifier.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/AuthorityFactoryIdentifier.java
[UTF-8] Mon Jan 25 09:51:33 2016
@@ -130,10 +130,10 @@ final class AuthorityFactoryIdentifier {
      * for avoiding to log the same message twice.
      *
      * <p>This field does not need to be declared {@code volatile} because {@code MultiAuthoritiesFactory}
-     * will read and write this field in a {@code synchronized} block using the same lock
(at least for the
-     * same instance of {@code AuthorityFactoryIdentifier}; lock may vary for other instances).</p>
+     * will read and write this field (indirectly, though a call to {@code logConflictWarning(…)}
only in
+     * a {@code synchronized} block.</p>
      *
-     * @see #conflict(AuthorityFactory)
+     * @see #logConflictWarning(AuthorityFactory)
      */
     private boolean hasLoggedWarning;
 
@@ -270,7 +270,7 @@ final class AuthorityFactoryIdentifier {
     /**
      * Logs a message reporting a conflict between the factory identified by this {@code
AuthorityFactoryIdentifier}
      * and another factory, if this instance has not already logged a warning. This method
assumes that it is invoked
-     * by the {@code MultiAuthoritiesFactory.getAuthorityFactory(…)} method.
+     * by the {@code MultiAuthoritiesFactory.getAuthorityFactory(…)} method in a synchronized
block.
      *
      * @param used The factory which will be used.
      */

Modified: sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/MultiAuthoritiesFactory.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/MultiAuthoritiesFactory.java?rev=1726601&r1=1726600&r2=1726601&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/MultiAuthoritiesFactory.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/MultiAuthoritiesFactory.java
[UTF-8] Mon Jan 25 09:51:33 2016
@@ -38,6 +38,7 @@ import javax.measure.unit.Unit;
 import org.apache.sis.internal.util.Citations;
 import org.apache.sis.internal.util.DefinitionURI;
 import org.apache.sis.internal.util.CollectionsExt;
+import org.apache.sis.internal.util.LazySynchronizedSetIterator;
 import org.apache.sis.util.ArraysExt;
 import org.apache.sis.util.CharSequences;
 import org.apache.sis.util.ArgumentChecks;
@@ -207,6 +208,24 @@ public class MultiAuthoritiesFactory ext
         return null;
     }
 
+    /**
+     * Returns the set of authority codes for objects of the given type.
+     * This method returns the union of codes returned by all factories specified at construction
time.
+     *
+     * <p><b>Warnings:</b>
+     * <ul>
+     *   <li>Callers should not retain a reference to the returned collection for a
long time,
+     *       since it may be backed by database connections (depending on the factory implementations).</li>
+     *   <li>The returned set is not thread-safe. Each thread should ask its own instance
and let
+     *       the garbage collector disposes it as soon as the collection is not needed anymore.</li>
+     *   <li>Call to the {@link Set#size()} method on the returned collection should
be avoided
+     *       since it may be costly.</li>
+     * </ul>
+     *
+     * @param  type The spatial reference objects type.
+     * @return The set of authority codes for spatial reference objects of the given type.
+     * @throws FactoryException if access to an underlying factory failed.
+     */
     @Override
     public Set<String> getAuthorityCodes(Class<? extends IdentifiedObject> type)
throws FactoryException {
         throw new UnsupportedOperationException("Not supported yet.");      // TODO
@@ -214,7 +233,10 @@ public class MultiAuthoritiesFactory ext
 
     /**
      * Returns the code spaces of all factories given to the constructor.
-     * This method may be relatively costly since it implies instantiation of all factories.
+     *
+     * <div class="note"><b>Implementation note:</b>
+     * the current implementation may be relatively costly since it implies instantiation
of all factories.
+     * </div>
      *
      * @return The code spaces of all factories.
      */
@@ -224,12 +246,8 @@ public class MultiAuthoritiesFactory ext
         Set<String> union = codeSpaces;
         if (union == null) {
             union = new LinkedHashSet<>();
-            for (final Iterable<? extends AuthorityFactory> provider : providers) {
-                if (provider != null) synchronized (provider) {
-                    for (final AuthorityFactory factory : provider) {
-                        union.addAll(getCodeSpaces(factory));
-                    }
-                }
+            for (final Iterator<AuthorityFactory> it = getAllFactories(); it.hasNext();)
{
+                union.addAll(getCodeSpaces(it.next()));
             }
             codeSpaces = union = CollectionsExt.unmodifiableOrCopy(union);
         }
@@ -264,6 +282,19 @@ public class MultiAuthoritiesFactory ext
     }
 
     /**
+     * Returns an iterator over all factories in this {@link MultiAuthoritiesFactory}.
+     * This iterator takes care of synchronization on the {@code Iterable<AuthorityFactory>}
instances
+     * and ensures that the same factory is not returned twice.
+     *
+     * <p>Note that despite the above-cited synchronization, the returned iterator
is <strong>not</strong>
+     * thread-safe: each thread needs to use its own iterator instance. However provided
that the above
+     * condition is meet, threads can safely use their iterators concurrently.</p>
+     */
+    final Iterator<AuthorityFactory> getAllFactories() {
+        return new LazySynchronizedSetIterator<>(providers);
+    }
+
+    /**
      * Returns the factory identified by the given type, authority and version.
      *
      * @param  <T>        The compile-time value of {@code type}.
@@ -312,9 +343,12 @@ public class MultiAuthoritiesFactory ext
          */
         if (request.hasVersion()) {
             factory = factories.get(request.versionOf(null));
-            if (factory != null && request.versionOf(factory.getAuthority()) == request)
{
-                // Default factory is for the version that user requested. Cache that finding.
-                return cache(request, factory);
+            if (factory != null) {
+                if (request.versionOf(factory.getAuthority()) == request) {
+                    // Default factory is for the version that user requested. Cache that
finding.
+                    return cache(request, factory);
+                }
+                factory = null;
             }
         }
         /*
@@ -328,58 +362,72 @@ public class MultiAuthoritiesFactory ext
         if ((doneMask & (1 << type)) == 0) {
             if (type >= 0 && type < providers.length) {
                 final Iterable<? extends AuthorityFactory> provider = providers[type];
-                synchronized (provider) {   // Should never be null because of the 'doneMask'
check.
-                    /*
-                     * Search for a factory for the given authority. Caches all factories
that we find
-                     * during the iteration process. Some factories may already be cached
as a result
-                     * of a partial iteration in a previous call to getAuthorityFactory(…).
-                     */
-                    for (final Iterator<? extends AuthorityFactory> it = provider.iterator();
it.hasNext();) {
+                final Iterator<? extends AuthorityFactory> it;
+                synchronized (provider) {               // Should never be null because of
the 'doneMask' check.
+                    it = provider.iterator();
+                    while (it.hasNext()) {
                         factory = it.next();
-                        for (final String namespace : getCodeSpaces(factory)) {
-                            final AuthorityFactoryIdentifier unversioned = request.unversioned(namespace);
-                            AuthorityFactory cached = cache(unversioned, factory);
-                            final AuthorityFactory found = request.equals(unversioned) ?
cached : null;
-                            /*
-                             * Only if we have no choice, ask to the factory what is its
version number.
-                             * We have no choice when ignoring the version number causes
a conflict, or
-                             * when the user asked for a specific version.
-                             */
-                            if (factory != cached || (request.hasVersion() && request.isSameAuthority(unversioned)))
{
-                                final AuthorityFactoryIdentifier versioned = unversioned.versionOf(factory.getAuthority());
-                                if (versioned != unversioned) {
-                                    /*
-                                     * Before to cache the factory with a key containing
the factory version, make sure
-                                     * that we took in account the version of the default
factory. This will prevent the
-                                     * call to 'cache(versioned, factory)' to overwrite the
default factory.
-                                     */
-                                    if (factory != cached) {
-                                        cache(unversioned.versionOf(cached.getAuthority()),
cached);
-                                    }
-                                    cached = cache(versioned, factory);
-                                }
+                        if (factory != null) break;     // Paranoiac check against null factories.
+                    }
+                }
+                /*
+                 * Search for a factory for the given authority. Caches all factories that
we find
+                 * during the iteration process. Some factories may already be cached as
a result
+                 * of a partial iteration in a previous call to getAuthorityFactory(…).
+                 */
+                while (factory != null) {
+                    for (final String namespace : getCodeSpaces(factory)) {
+                        final AuthorityFactoryIdentifier unversioned = request.unversioned(namespace);
+                        AuthorityFactory cached = cache(unversioned, factory);
+                        final AuthorityFactory found = request.equals(unversioned) ? cached
: null;
+                        /*
+                         * Only if we have no choice, ask to the factory what is its version
number.
+                         * We have no choice when ignoring the version number causes a conflict,
or
+                         * when the user asked for a specific version.
+                         */
+                        if (factory != cached || (request.hasVersion() && request.isSameAuthority(unversioned)))
{
+                            final AuthorityFactoryIdentifier versioned = unversioned.versionOf(factory.getAuthority());
+                            if (versioned != unversioned) {
                                 /*
-                                 * If there is a conflict, log a warning provided that we
did not already reported
-                                 * that conflict. The flag telling us if we already logged
a warning is in the key,
-                                 * so we have to find that key in the loop below. This is
inefficient, but conflict
-                                 * should not happen in a sane environment.
+                                 * Before to cache the factory with a key containing the
factory version, make sure
+                                 * that we took in account the version of the default factory.
This will prevent the
+                                 * call to 'cache(versioned, factory)' to overwrite the default
factory.
                                  */
                                 if (factory != cached) {
-                                    for (final AuthorityFactoryIdentifier identifier : factories.keySet())
{
-                                        if (identifier.equals(versioned)) {
+                                    cache(unversioned.versionOf(cached.getAuthority()), cached);
+                                }
+                                cached = cache(versioned, factory);
+                            }
+                            /*
+                             * If there is a conflict, log a warning provided that we did
not already reported
+                             * that conflict. The flag telling us if we already logged a
warning is in the key,
+                             * so we have to find that key in the loop below. This is inefficient,
but conflict
+                             * should not happen in a sane environment.
+                             */
+                            if (factory != cached) {
+                                for (final AuthorityFactoryIdentifier identifier : factories.keySet())
{
+                                    if (identifier.equals(versioned)) {
+                                        synchronized (provider) {
                                             identifier.logConflictWarning(cached);
-                                            break;
                                         }
+                                        break;
                                     }
                                 }
-                                if (request.equals(versioned)) {
-                                    return cached;
-                                }
                             }
-                            if (found != null) {
-                                return found;
+                            if (request.equals(versioned)) {
+                                return cached;
                             }
                         }
+                        if (found != null) {
+                            return found;
+                        }
+                    }
+                    factory = null;
+                    synchronized (provider) {
+                        while (it.hasNext()) {
+                            factory = it.next();
+                            if (factory != null) break;         // Paranoiac check against
null factories.
+                        }
                     }
                 }
             } else if (type >= AuthorityFactoryIdentifier.GEODETIC) {

Added: sis/branches/JDK8/core/sis-utility/src/main/java/org/apache/sis/internal/util/LazySynchronizedSetIterator.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-utility/src/main/java/org/apache/sis/internal/util/LazySynchronizedSetIterator.java?rev=1726601&view=auto
==============================================================================
--- sis/branches/JDK8/core/sis-utility/src/main/java/org/apache/sis/internal/util/LazySynchronizedSetIterator.java
(added)
+++ sis/branches/JDK8/core/sis-utility/src/main/java/org/apache/sis/internal/util/LazySynchronizedSetIterator.java
[UTF-8] Mon Jan 25 09:51:33 2016
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sis.internal.util;
+
+import java.util.Iterator;
+import java.util.IdentityHashMap;
+import java.util.NoSuchElementException;
+
+
+/**
+ * An iterator over all elements given by an array of {@code Iterable<E>}, skipping
null elements.
+ * All uses of an {@code Iterable<E>} (including its iterator) is synchronized on that
{@code Iterable} instance.
+ *
+ * <p>Note that despite the above-cited synchronization, this iterator is <strong>not</strong>
thread-safe:
+ * each thread needs to use its own iterator instance. However provided that the above condition
is meet,
+ * different threads can safely use their iterators concurrently even if the underlying {@code
Iterable}s
+ * were not thread-safe, because of the synchronization on {@code Iterable<E>} instances.</p>
+ *
+ * <p>This class extends {@code IdentityHashMap} for making sure that we do not return
the same instance twice
+ * This particular hierarchy is an implementation details that users of this iterator should
ignore.</p>
+ *
+ * @param <E> The type of elements to be returned by the iterator.
+ *
+ * @author  Martin Desruisseaux (Geomatys)
+ * @since   0.7
+ * @version 0.7
+ * @module
+ */
+@SuppressWarnings("serial")
+public final class LazySynchronizedSetIterator<E> extends IdentityHashMap<E,Boolean>
implements Iterator<E> {
+    /**
+     * The providers of iterators. This array shall not be modified by {@code LazySynchronizedSetIterator},
+     * since this is a direct reference to the array given to the constructor (not a copy).
+     */
+    private final Iterable<? extends E>[] providers;
+
+    /**
+     * Index of the {@code Iterable<E>} instance that provides the {@link #it} value.
+     * This is also the instance to use as a synchronization lock.
+     */
+    private int providerIndex;
+
+    /**
+     * The iterator on which to delegate calls to {@link #hasNext()} and {@link #next()}.
+     * This iterator is provided by {@code providers[providerIndex].iterator()}.
+     */
+    private Iterator<? extends E> it;
+
+    /**
+     * The next value to be returned by {@link #next()}, or {@code null} if not yet determined.
+     */
+    private E next;
+
+    /**
+     * Creates a new iterator over all elements returned by the given providers.
+     * Null elements in the given array will be ignored.
+     *
+     * @param providers The providers of iterators. This array is <strong>not</strong>
cloned.
+     */
+    public LazySynchronizedSetIterator(final Iterable<? extends E>[] providers) {
+        this.providers = providers;
+    }
+
+    /**
+     * Returns {@code true} if {@link #next()} can return a non-null element.
+     * This method delegates to the iterators of all {@linkplain #providers}
+     * until one is found that return a non-null element.
+     *
+     * @return {@code true} if there is more elements to return.
+     */
+    @Override
+    public boolean hasNext() {
+        if (next != null) {
+            return true;
+        }
+        while (providerIndex < providers.length) {
+            final Iterable<? extends E> provider = providers[providerIndex];
+            if (provider != null) {
+                synchronized (provider) {
+                    if (it == null) {
+                        it = provider.iterator();
+                    }
+                    while (it.hasNext()) {
+                        next = it.next();
+                        if (next != null && put(next, Boolean.TRUE) == null) {
+                            return true;
+                        }
+                    }
+                    it = null;
+                }
+            }
+            providerIndex++;
+        }
+        return false;
+    }
+
+    /**
+     * Returns the next element in this iteration.
+     *
+     * @return The next element.
+     */
+    @Override
+    public E next() {
+        E value = next;
+        if (value == null) {
+            if (!hasNext()) {
+                throw new NoSuchElementException();
+            }
+            value = next;
+        }
+        next = null;
+        return value;
+    }
+}

Propchange: sis/branches/JDK8/core/sis-utility/src/main/java/org/apache/sis/internal/util/LazySynchronizedSetIterator.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: sis/branches/JDK8/core/sis-utility/src/main/java/org/apache/sis/internal/util/LazySynchronizedSetIterator.java
------------------------------------------------------------------------------
    svn:mime-type = text/plain;charset=UTF-8

Modified: sis/branches/JDK8/core/sis-utility/src/test/java/org/apache/sis/internal/util/DefinitionURITest.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-utility/src/test/java/org/apache/sis/internal/util/DefinitionURITest.java?rev=1726601&r1=1726600&r2=1726601&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-utility/src/test/java/org/apache/sis/internal/util/DefinitionURITest.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-utility/src/test/java/org/apache/sis/internal/util/DefinitionURITest.java
[UTF-8] Mon Jan 25 09:51:33 2016
@@ -34,14 +34,23 @@ import static org.junit.Assert.*;
  */
 public final strictfp class DefinitionURITest extends TestCase {
     /**
-     * Tests {@link DefinitionURI#parse(String)} on {@code "urn:ogc:def:crs:EPSG:8.2:4326"}.
-     * This is a URN without parameters defined by EPSG.
+     * Tests {@link DefinitionURI#parse(String)} on strings that should not be recognized
as URN.
      */
     @Test
-    public void testParse() {
+    public void testParseInvalid() {
         assertNull(DefinitionURI.parse("EPSG:4326"));
+        assertNull(DefinitionURI.parse("EPSG::4326"));
+        assertNull(DefinitionURI.parse("urn:ogcx:def:CRS:EPSG:8.2:4326"));
+    }
 
-        DefinitionURI parsed = DefinitionURI.parse(" urn:ogc:def: crs : EPSG: 8.2 :4326 ");
+    /**
+     * Tests {@link DefinitionURI#parse(String)} on {@code "urn:ogc:def:crs:EPSG:8.2:4326"}.
+     * This is a URN without parameters defined by EPSG. This test also puts some spaces
for
+     * testing the parser capability to ignore them.
+     */
+    @Test
+    public void testParse() {
+        final DefinitionURI parsed = DefinitionURI.parse(" urn:ogc:def: crs : EPSG: 8.2 :4326
");
         assertNotNull("DefinitionURI", parsed);
         assertEquals ("isHTTP",    false,   parsed.isHTTP);
         assertEquals ("isGML",     false,   parsed.isGML);
@@ -51,8 +60,15 @@ public final strictfp class DefinitionUR
         assertEquals ("code",      "4326",  parsed.code);
         assertNull   ("parameters",         parsed.parameters);
         assertEquals ("toString()", "urn:ogc:def:crs:EPSG:8.2:4326", parsed.toString());
+    }
 
-        parsed = DefinitionURI.parse("URN :X-OGC: Def:crs:EPSG::4326");
+    /**
+     * Tests {@link DefinitionURI#parse(String)} on {@code "urn:ogc:def:crs:EPSG::4326"}.
+     * This is a URN without version. This test also mixes lower and upper cases.
+     */
+    @Test
+    public void testParseWithoutVersion() {
+        final DefinitionURI parsed = DefinitionURI.parse("URN :X-OGC: Def:crs:EPSG::4326");
         assertNotNull("DefinitionURI", parsed);
         assertEquals ("isHTTP",    false,   parsed.isHTTP);
         assertEquals ("isGML",     false,   parsed.isGML);



Mime
View raw message