sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject svn commit: r1556920 - in /sis/branches/JDK7/core: sis-referencing/src/main/java/org/apache/sis/referencing/AbstractIdentifiedObject.java sis-utility/src/main/java/org/apache/sis/internal/util/UnmodifiableArrayList.java
Date Thu, 09 Jan 2014 18:55:40 GMT
Author: desruisseaux
Date: Thu Jan  9 18:55:40 2014
New Revision: 1556920

URL: http://svn.apache.org/r1556920
Log:
Removed the AbstractIdentifiedObject.setNames(Collection<ReferenceIdentifier>) because
whether that method
was invoked or not was JAXB-implementation dependent. Instead design AbstractIdentifiedObject
in a way that
should work the same on all supported JDK versions.

Modified:
    sis/branches/JDK7/core/sis-referencing/src/main/java/org/apache/sis/referencing/AbstractIdentifiedObject.java
    sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/internal/util/UnmodifiableArrayList.java

Modified: sis/branches/JDK7/core/sis-referencing/src/main/java/org/apache/sis/referencing/AbstractIdentifiedObject.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/core/sis-referencing/src/main/java/org/apache/sis/referencing/AbstractIdentifiedObject.java?rev=1556920&r1=1556919&r2=1556920&view=diff
==============================================================================
--- sis/branches/JDK7/core/sis-referencing/src/main/java/org/apache/sis/referencing/AbstractIdentifiedObject.java
[UTF-8] (original)
+++ sis/branches/JDK7/core/sis-referencing/src/main/java/org/apache/sis/referencing/AbstractIdentifiedObject.java
[UTF-8] Thu Jan  9 18:55:40 2014
@@ -18,7 +18,6 @@ package org.apache.sis.referencing;
 
 import java.util.Map;
 import java.util.Set;
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.AbstractCollection;
@@ -42,6 +41,7 @@ import org.opengis.referencing.Reference
 import org.apache.sis.internal.referencing.ReferencingUtilities;
 import org.apache.sis.internal.jaxb.referencing.Code;
 import org.apache.sis.internal.util.Numerics;
+import org.apache.sis.internal.util.UnmodifiableArrayList;
 import org.apache.sis.io.wkt.FormattableObject;
 import org.apache.sis.xml.Namespaces;
 import org.apache.sis.util.Deprecable;
@@ -490,7 +490,7 @@ public class AbstractIdentifiedObject ex
          * Invoked by JAXB at unmarshalling time for each identifier. The first identifier
will be taken
          * as the name and all other identifiers (if any) as aliases.
          *
-         * <p>Some JAXB implementations never invoke {@link AbstractIdentifiedObject#setNames(Collection)}.
+         * <p>Some (but not all) JAXB implementations never invoke setter method for
collections.
          * Instead they invoke {@link AbstractIdentifiedObject#getNames()} and add directly
the identifiers
          * in the returned collection. Consequently this method must writes directly in the
enclosing object.
          * See <a href="https://java.net/jira/browse/JAXB-488">JAXB-488</a> for
more information.</p>
@@ -499,57 +499,59 @@ public class AbstractIdentifiedObject ex
         public boolean add(final ReferenceIdentifier id) {
             if (name == null) {
                 name = id;
-                return true;
-            }
-            if (alias == null) {
-                alias = new ArrayList<>(4);
+            } else {
+                /*
+                 * Our Code and RS_Identifier implementations should always create NamedIdentifier
instance,
+                 * so the 'instanceof' check should not be necessary. But we do a paranoiac
check anyway.
+                 */
+                final GenericName n = id instanceof GenericName ? (GenericName) id : new
NamedIdentifier(id);
+                if (alias == null) {
+                    alias = Collections.singleton(n);
+                } else {
+                    /*
+                     * This implementation is inefficient since each addition copies the
array, but we rarely
+                     * have more than two aliases.  This implementation is okay for a small
number of aliases
+                     * and ensures that the enclosing AbstractIdentifiedObject is unmodifiable
except by this
+                     * add(…) method.
+                     *
+                     * Note about alternative approaches
+                     * ---------------------------------
+                     * An alternative approach could be to use an ArrayList and replace it
by an unmodifiable
+                     * list only after unmarshalling (using an afterUnmarshal(Unmarshaller,
Object) method),
+                     * but we want to avoid Unmarshaller dependency (for reducing classes
loading for users
+                     * who are not interrested in XML) and it may actually be less efficient
for the vast
+                     * majority of cases where there is less than 3 aliases.
+                     */
+                    final int size = alias.size();
+                    final GenericName[] names = alias.toArray(new GenericName[size + 1]);
+                    names[size] = n;
+                    alias = UnmodifiableArrayList.wrap(names);
+                }
             }
-            /*
-             * Our Code and RS_Identifier implementations should always create NamedIdentifier
instance,
-             * so the 'instanceof' check should not be necessary. But do a paranoiac check
anyway.
-             */
-            return alias.add(id instanceof GenericName ? (GenericName) id : new NamedIdentifier(id));
+            return true;
         }
     }
 
     /**
      * Returns the {@link #name} and all aliases which are also instance of {@lik ReferenceIdentifier}.
      * The later happen often in SIS implementation since many aliases are instance of {@link
NamedIdentifier}.
-     */
-    @XmlElement(name = "name", required = true)
-    final Collection<ReferenceIdentifier> getNames() {
-        return new Names();
-    }
-
-    /**
-     * Sets the first element as the {@link #name} and all remaining elements as {@link #alias}.
-     * This method is invoked by some implementations of JAXB (not all of them) at unmarshalling
time.
-     * It should not be invoked anymore after the object has been made available to the user.
      *
-     * <p>Some JAXB implementations never invoke this setter method. Instead they invoke
{@link #getNames()}
-     * and add directly the identifiers in the returned collection. Whether JAXB will perform
a final call to
+     * <p>The returned collection is <cite>live</cite>: adding elements
in that collection will modify this
+     * {@code AbstractIdentifiedObject} instance. This is needed for unmarshalling with JAXB
and should not
+     * be used in other context.</p>
+     *
+     * {@section Why there is no <code>setNames(…)</code> method}
+     * Some JAXB implementations never invoke setter method for collections. Instead they
invoke the getter and
+     * add directly the identifiers in the returned collection. Whether JAXB will perform
or not a final call to
      * {@code setNames(…)} is JAXB-implementation dependent (JDK7 does but JDK6 and JDK8
early access do not).
-     * Consequently we can not rely on this method to be invoked. It is better if this method
is invoked, but
-     * we will not lost data if it is not.</p>
+     * It seems a more portable approach (at least for JAXB reference implementations) to
design our class
+     * without setter method, in order to have the same behavior on all supported JDK versions.
      *
      * @see <a href="https://java.net/jira/browse/JAXB-488">JAXB-488</a>
      */
-    private void setNames(final Collection<ReferenceIdentifier> names) {
-        /*
-         * If the collection is an instance of Names, then assume that the collection is
the instance obtained
-         * by getNames(), in which case this IdentifiedObject already contains the content
of that collection.
-         * This behavior is necessary for working around the JAXB-488 issue.
-         */
-        if (!(names instanceof Names)) {
-            getNames().addAll(names);
-        }
-        /*
-         * Froze aliases in an unmodifiable set. In JAXB implementations that do not invoke
the setter method,
-         * the aliases list is left modifiable. This is a hole in our object immutability.
-         */
-        if (alias != null) {
-            alias = immutableSet(true, alias.toArray(new GenericName[alias.size()]));
-        }
+    @XmlElement(name = "name", required = true)
+    final Collection<ReferenceIdentifier> getNames() {
+        return new Names();
     }
 
     /**

Modified: sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/internal/util/UnmodifiableArrayList.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/internal/util/UnmodifiableArrayList.java?rev=1556920&r1=1556919&r2=1556920&view=diff
==============================================================================
--- sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/internal/util/UnmodifiableArrayList.java
[UTF-8] (original)
+++ sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/internal/util/UnmodifiableArrayList.java
[UTF-8] Thu Jan  9 18:55:40 2014
@@ -18,6 +18,8 @@ package org.apache.sis.internal.util;
 
 import java.io.Serializable;
 import java.util.AbstractList;
+import java.util.Arrays;
+import java.lang.reflect.Array;
 import org.apache.sis.util.ArgumentChecks;
 import org.apache.sis.util.collection.CheckedContainer;
 
@@ -69,7 +71,7 @@ public class UnmodifiableArrayList<E> ex
     /**
      * The wrapped array.
      */
-    private final E[] array;
+    final E[] array;
 
     /**
      * Creates a new instance wrapping the given array. A direct reference to the given array
is
@@ -167,6 +169,8 @@ public class UnmodifiableArrayList<E> ex
 
     /**
      * Returns the list size.
+     *
+     * @return The size of this list.
      */
     @Override
     public int size() {
@@ -192,6 +196,9 @@ public class UnmodifiableArrayList<E> ex
 
     /**
      * Returns the element at the specified index.
+     *
+     * @param  index The index of the element to get.
+     * @return The element at the given index.
      */
     @Override
     public E get(final int index) {
@@ -355,11 +362,21 @@ public class UnmodifiableArrayList<E> ex
             ArgumentChecks.ensureValidIndex(size, index);
             return super.get(index + lower);
         }
+
+        /**
+         * Returns a copy of the backing array section viewed by this sublist.
+         */
+        @Override
+        public E[] toArray() {
+            return Arrays.copyOfRange(array, lower, lower + size);
+        }
     }
 
     /**
      * Returns a copy of the backing array. Note that the array type is {@code E[]} rather
than {@code Object[]}.
      * This is not what {@code ArrayList} does, but is not forbidden by {@link List#toArray()}
javadoc neither.
+     *
+     * @return A copy of the wrapped array.
      */
     @Override
     public E[] toArray() {
@@ -367,6 +384,35 @@ public class UnmodifiableArrayList<E> ex
     }
 
     /**
+     * Copies the backing array in the given one if the list fits in the given array.
+     * If the list does not fit in the given array, returns the collection in a new array.
+     *
+     * @param  <T>   The type of array element.
+     * @param  dest  The array where to copy the elements if the list can fits in the array.
+     * @return The given array, or a newly created array if this list is larger than the
given array.
+     */
+    @Override
+    @SuppressWarnings("unchecked")
+    public <T> T[] toArray(T[] dest) {
+        final int size = size();
+        if (dest.length != size) {
+            if (dest.length < size) {
+                /*
+                 * We are cheating here since the array component may not be assignable to
T.
+                 * But if this assumption is wrong, then the call to System.arraycopy(…)
later
+                 * will thrown an ArrayStoreException, which is the exception type required
by
+                 * the Collection.toArray(T[]) javadoc.
+                 */
+                dest = (T[]) Array.newInstance(dest.getClass().getComponentType(), size);
+            } else {
+                dest[size] = null; // Required by Collection.toArray(T[]) javadoc.
+            }
+        }
+        System.arraycopy(array, lower(), dest, 0, size);
+        return dest;
+    }
+
+    /**
      * Compares this list with the given object for equality.
      *
      * @param  object The object to compare with this list.



Mime
View raw message