sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject svn commit: r1451957 - in /sis/branches/JDK7: sis-metadata/src/main/java/org/apache/sis/metadata/ sis-utility/src/main/java/org/apache/sis/io/ sis-utility/src/main/java/org/apache/sis/util/
Date Sat, 02 Mar 2013 23:11:14 GMT
Author: desruisseaux
Date: Sat Mar  2 23:11:13 2013
New Revision: 1451957

URL: http://svn.apache.org/r1451957
Log:
- Try to explain more in the javadoc what we are doing.
- Replace HashMap<Class,...> by IdentityHashMap and explain why in javadoc.
- Removed MetadataStandard.isModifiable(...) method (part of effort to simplify the code).

Modified:
    sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/AbstractMetadata.java
    sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataStandard.java
    sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/PropertyAccessor.java
    sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/StandardImplementation.java
    sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/io/CompoundFormat.java
    sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/util/Numbers.java

Modified: sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/AbstractMetadata.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/AbstractMetadata.java?rev=1451957&r1=1451956&r2=1451957&view=diff
==============================================================================
--- sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/AbstractMetadata.java
[UTF-8] (original)
+++ sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/AbstractMetadata.java
[UTF-8] Sat Mar  2 23:11:13 2013
@@ -18,7 +18,6 @@ package org.apache.sis.metadata;
 
 import java.util.logging.Logger;
 import java.lang.reflect.Modifier;
-import net.jcip.annotations.ThreadSafe;
 import org.apache.sis.util.ComparisonMode;
 import org.apache.sis.util.LenientComparable;
 import org.apache.sis.util.logging.Logging;
@@ -26,19 +25,32 @@ import org.apache.sis.util.logging.Loggi
 
 /**
  * Base class for metadata implementations, providing basic operations using Java reflection.
- * Available operations include the {@linkplain #AbstractMetadata(Object) copy constructor},
- * together with {@link #equals(Object)} and {@link #hashCode()} implementations.
+ * All {@code AbstractMetadata} instances shall be associated to a {@link MetadataStandard},
+ * which is provided by subclasses in the {@link #getStandard()} method. There is typically
+ * a large number of {@code AbstractMetadata} subclasses (not necessarily as direct children)
+ * for the same standard.
  *
- * {@section Requirements for subclasses}
- * Subclasses need to implement the interfaces of some {@linkplain MetadataStandard metadata
standard}
- * and return that standard in the {@link #getStandard()} method.
+ * <p>This base class reduces the effort required to implement metadata interface by
providing
+ * {@link #equals(Object)}, {@link #hashCode()} and {@link #toString()} implementations.
+ * Those methods are implemented using Java reflection for invoking the getter methods
+ * defined by the {@code MetadataStandard}.</p>
+ *
+ * <p>{@code AbstractMetadata} may be read-only or read/write, at implementation choice.
+ * The {@link ModifiableMetadata} subclass provides the basis of most SIS metadata classes
+ * having writing capabilities.</p>
+ *
+ * {@section Synchronization}
+ * The methods in this class are not synchronized. Synchronizations may be done by getter
and
+ * setter methods in subclasses, at implementation choice. We never synchronize the methods
that
+ * perform deep traversal of the metadata tree (like {@code equals(Object)}, {@code hashCode()}
+ * or {@code toString()}) because such synchronizations are deadlock prone. For example if
+ * subclasses synchronize their getter methods, then many locks may be acquired in various
orders.
  *
  * @author  Martin Desruisseaux (Geomatys)
  * @since   0.3 (derived from geotk-2.4)
  * @version 0.3
  * @module
  */
-@ThreadSafe
 public abstract class AbstractMetadata implements LenientComparable {
     /**
      * The logger for messages related to metadata implementations.
@@ -52,8 +64,36 @@ public abstract class AbstractMetadata i
     }
 
     /**
+     * Returns the metadata standard implemented by subclasses.
+     *
+     * @return The metadata standard implemented.
+     *
+     * @todo This method returns {@link MetadataStandard#ISO_19115} for now,
+     *       but will become an abstract method soon.
+     */
+    public MetadataStandard getStandard() {
+        return MetadataStandard.ISO_19115;
+    }
+
+    /**
+     * Returns the metadata interface implemented by this class. It should be one of the
+     * interfaces defined in the {@linkplain #getStandard() metadata standard} implemented
+     * by this class.
+     *
+     * @return The standard interface implemented by this implementation class.
+     *
+     * @see MetadataStandard#getInterface(Class)
+     */
+    public Class<?> getInterface() {
+        // No need to sychronize, since this method does not depend on property values.
+        return getStandard().getInterface(getClass());
+    }
+
+    /**
      * Returns the class of the given metadata, ignoring SIS private classes
      * like {@link org.apache.sis.metadata.iso.citation.CitationConstant}.
+     * This method does <strong>not</strong> ignores user's private classes,
+     * only the SIS ones.
      *
      * @see <a href="http://jira.geotoolkit.org/browse/GEOTK-48">GEOTK-48</a>
      */
@@ -86,12 +126,20 @@ public abstract class AbstractMetadata i
         if (object == this) {
             return true;
         }
+        if (object == null) {
+            return false;
+        }
         if (mode == ComparisonMode.STRICT) {
-            if (object == null || getPublicClass(object) != getPublicClass(this)) {
+            if (getPublicClass(object) != getPublicClass(this)) {
+                return false;
+            }
+        }
+        final MetadataStandard standard = getStandard();
+        if (mode != ComparisonMode.STRICT) {
+            if (!getInterface().isInstance(object)) {
                 return false;
             }
         }
-        // TODO: There is some code to port here.
         /*
          * DEADLOCK WARNING: A deadlock may occur if the same pair of objects is being compared
          * in an other thread (see http://jira.codehaus.org/browse/GEOT-1777). Ideally we
would
@@ -99,16 +147,28 @@ public abstract class AbstractMetadata i
          * a workaround is to always get the locks in the same order. Unfortunately we have
no
          * guarantee that the caller didn't looked the object himself. For now the safest
approach
          * is to not synchronize at all.
+         *
+         * Edit: actually, even if we could synchronize the two objects atomically, a deadlock
+         *       risk would still exists for the reason documented in this class's javadoc.
          */
-        // TODO: There is some code to port here.
-        throw new UnsupportedOperationException("Not yet implemented.");
+        return standard.shallowEquals(this, object, mode, false);
     }
 
     /**
      * Performs a {@linkplain ComparisonMode#STRICT strict} comparison of this metadata with
-     * the given object.
+     * the given object. This method is implemented as below:
+     *
+     * {@preformat java
+     *     public final boolean equals(final Object object) {
+     *         return equals(object, ComparisonMode.STRICT);
+     *     }
+     * }
+     *
+     * If a subclass needs to override the behavior of this method, then
+     * override {@link #equals(Object, ComparisonMode)} instead.
      *
-     * @param object The object to compare with this metadata for equality.
+     * @param  object The object to compare with this metadata for equality.
+     * @return {@code true} if the given object is strictly equals to this metadata.
      */
     @Override
     public final boolean equals(final Object object) {
@@ -120,10 +180,16 @@ public abstract class AbstractMetadata i
      * is defined as the sum of hash code values of all non-null properties. This is the
      * same contract than {@link java.util.Set#hashCode()} and ensure that the hash code
      * value is insensitive to the ordering of properties.
+     *
+     * {@section Performance note}
+     * This method does not cache the value because current implementation has no notification
+     * mechanism for tracking changes in children properties. If this metadata is known to
be
+     * immutable, then subclasses may consider caching the hash code value at their choice.
+     *
+     * @see MetadataStandard#hashCode(Object)
      */
     @Override
-    public synchronized int hashCode() {
-        // TODO: There is some code to port here.
-        throw new UnsupportedOperationException("Not yet implemented.");
+    public int hashCode() {
+        return getStandard().hashCode(this);
     }
 }

Modified: sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataStandard.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataStandard.java?rev=1451957&r1=1451956&r2=1451957&view=diff
==============================================================================
--- sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataStandard.java
[UTF-8] (original)
+++ sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataStandard.java
[UTF-8] Sat Mar  2 23:11:13 2013
@@ -41,21 +41,34 @@ import static org.apache.sis.util.Argume
  * {@linkplain java.lang.reflect Java reflection}. The following rules are assumed:</p>
  *
  * <ul>
- *   <li>Properties (or metadata attributes) are defined by the collection of {@code
get*()}
- *       methods with arbitrary return type, or {@code is*()} methods with boolean return
type,
- *       found in the <strong>interface</strong>. Getters declared only in the
implementation
- *       are ignored.</li>
- *   <li>Every properties are <cite>readable</cite>.</li>
- *   <li>A property is <cite>writable</cite> if a {@code set*(...)} method
is defined
- *       in the implementation class for the corresponding {@code get*()} method. The
- *       setter doesn't need to be defined in the interface.</li>
+ *   <li>Properties (or metadata attributes) are defined by the collection of
+ *       following getter methods found <strong>in the interface</strong>
+ *       (methods declared only in the implementation are ignored):
+ *       <ul>
+ *         <li>{@code get*()} methods with arbitrary return type;</li>
+ *         <li>or {@code is*()} methods with boolean return type.</li>
+ *       </ul></li>
+ *   <li>Every properties are <cite>readable</cite>.
+ *       But a property is also <cite>writable</cite> if a {@code set*(…)}
method is defined
+ *       <strong>in the implementation class</strong> for the corresponding getter
method.
+ *       The setter method doesn't need to be defined in the interface.</li>
  * </ul>
  *
  * An instance of {@code MetadataStandard} is associated to every {@link AbstractMetadata}
objects.
  * The {@code AbstractMetadata} base class usually form the basis of ISO 19115 implementations
but
- * can also be used for other standards. An instance of {@code MetadataStandard} is also
associated
- * with Image I/O {@link org.apache.sis.image.io.metadata.SpatialMetadataFormat} in order
to define
- * the tree of XML nodes to be associated with raster data.
+ * can also be used for other standards.
+ *
+ * {@section Defining new <code>MetadataStandard</code> instances}
+ * Users should use the pre-defined constants when applicable.
+ * However if new instances need to be defined, then there is a choice:
+ *
+ * <ul>
+ *   <li>For <em>read-only</em> metadata, {@code MetadataStandard} can
be instantiated directly.
+ *       only getter methods will be used and all operations that modify the metadata properties
+ *       will throw an {@link UnmodifiableMetadataException}.</li>
+ *   <li>For <em>read/write</em> metadata, the {@link #getImplementation(Class)}
+ *       method must be overridden in a {@code MetadataStandard} subclass.</li>
+ * </ul>
  *
  * @author  Martin Desruisseaux (Geomatys)
  * @since   0.3 (derived from geotk-2.4)
@@ -67,6 +80,8 @@ public class MetadataStandard {
     /**
      * The package of SIS implementation of ISO 19115.
      * This package name has a trailing dot.
+     *
+     * @see #interfacePackage
      */
     static final String SIS_PACKAGE = "org.apache.sis.metadata.iso.";
 
@@ -363,7 +378,7 @@ public class MetadataStandard {
      * @throws ClassCastException if the specified implementation class does
      *         not implement an interface of this standard.
      *
-     * @see AbstractMetadata#getInterface
+     * @see AbstractMetadata#getInterface()
      */
     public Class<?> getInterface(final Class<?> type) throws ClassCastException
{
         ensureNonNull("type", type);
@@ -386,19 +401,6 @@ public class MetadataStandard {
     }
 
     /**
-     * Returns {@code true} if this metadata is modifiable. This method is not public because
it
-     * uses heuristic rules. In case of doubt, this method conservatively returns {@code
true}.
-     *
-     * @throws ClassCastException if the specified implementation class do
-     *         not implements a metadata interface of the expected package.
-     *
-     * @see ModifiableMetadata#isModifiable()
-     */
-    final boolean isModifiable(final Class<?> implementation) throws ClassCastException
{
-        return getAccessor(implementation, true).isModifiable();
-    }
-
-    /**
      * Replaces every properties in the specified metadata by their
      * {@linkplain ModifiableMetadata#unmodifiable() unmodifiable variant}.
      *
@@ -442,18 +444,21 @@ public class MetadataStandard {
 
     /**
      * Compares the two specified metadata objects.
-     * The comparison is <cite>shallow</cite>, i.e. all metadata attributes are
compared using the
-     * {@link LenientComparable#equals(Object, ComparisonMode)} method if possible, or the
-     * {@link Object#equals(Object)} method otherwise, without explicit recursive call to
-     * this {@code shallowEquals(...)} method for child metadata.
+     * The two metadata arguments shall be implementations of a metadata interface defined
by
+     * this {@code MetadataStandard}, otherwise an exception will be thrown. However the
two
+     * arguments do not need to be the same implementation class.
      *
      * <p>This method can optionally excludes null values from the comparison. In metadata,
-     * null value often means "don't know", so in some occasion we want to consider two
+     * null value often means "don't know", so in some occasions we want to consider two
      * metadata as different only if a property value is know for sure to be different.</p>
      *
-     * <p>The first arguments must be an implementation of a metadata interface, otherwise
an
-     * exception will be thrown. The two arguments do not need to be the same implementation
-     * however.</p>
+     * {@section Shallow or deep comparisons}
+     * This method implements a <cite>shallow</cite> comparison in that properties
are compared by
+     * invoking their {@code properties.equals(…)} method without <em>explicit</em>
recursive call
+     * to this {@code shallowEquals(…)} method for children metadata. However the comparison
will
+     * do <em>implicit</em> recursive calls if the {@code properties.equals(…)}
implementations
+     * invoke this {@code shallowEquals(…)} method. In the later case, the final result
is a deep
+     * comparison.
      *
      * @param metadata1 The first metadata object to compare.
      * @param metadata2 The second metadata object to compare.

Modified: sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/PropertyAccessor.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/PropertyAccessor.java?rev=1451957&r1=1451956&r2=1451957&view=diff
==============================================================================
--- sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/PropertyAccessor.java
[UTF-8] (original)
+++ sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/PropertyAccessor.java
[UTF-8] Sat Mar  2 23:11:13 2013
@@ -96,6 +96,14 @@ final class PropertyAccessor {
     /**
      * Getters shared between many instances of this class. Two different implementations
      * may share the same getters but different setters.
+     *
+     * {@note In the particular case of <code>Class</code> keys, <code>IdentityHashMap</code>
and
+     *        <code>HashMap</code> have identical behavior since <code>Class</code>
is final and
+     *        does not override the <code>equals(Object)</code> and <code>hashCode()</code>
methods.
+     *        The <code>IdentityHashMap</code> Javadoc claims that it is faster
than the regular
+     *        <code>HashMap</code>. But maybe the most interesting property is
that it allocates
+     *        less objects since <code>IdentityHashMap</code> implementation
doesn't need the chain
+     *        of objects created by <code>HashMap</code>.}
      */
     private static final Map<Class<?>, Method[]> SHARED_GETTERS = new IdentityHashMap<>();
 
@@ -951,8 +959,8 @@ final class PropertyAccessor {
 
     /**
      * Compares the two specified metadata objects. The comparison is <cite>shallow</cite>,
-     * i.e. all metadata attributes are compared using the {@link Object#equals(Object)}
-     * method without recursive call to this {@code shallowEquals} method for other metadata.
+     * i.e. all metadata properties are compared using their {@code properties.equals(…)}
+     * method without explicit calls to this {@code shallowEquals} method for children.
      *
      * <p>This method can optionally excludes null values from the comparison. In metadata,
      * null value often means "don't know", so in some occasion we want to consider two
@@ -963,6 +971,8 @@ final class PropertyAccessor {
      * @param  mode      The strictness level of the comparison.
      * @param  skipNulls If {@code true}, only non-null values will be compared.
      * @throws BackingStoreException If the implementation threw a checked exception.
+     *
+     * @see MetadataStandard#shallowEquals(Object, Object, ComparisonMode, boolean)
      */
     public boolean shallowEquals(final Object metadata1, final Object metadata2,
             final ComparisonMode mode, final boolean skipNulls) throws BackingStoreException
@@ -1069,24 +1079,6 @@ final class PropertyAccessor {
     }
 
     /**
-     * Returns {@code true} if the metadata is modifiable. This method is not public because
it
-     * uses heuristic rules. In case of doubt, this method conservatively returns {@code
true}.
-     */
-    final boolean isModifiable() {
-        if (setters != null) {
-            return true;
-        }
-        for (int i=0; i<allCount; i++) {
-            // Immutable objects usually don't need to be cloned. So if
-            // an object is cloneable, it is probably not immutable.
-            if (Cloneable.class.isAssignableFrom(getters[i].getReturnType())) {
-                return true;
-            }
-        }
-        return false;
-    }
-
-    /**
      * Returns a hash code for the specified metadata. The hash code is defined as the
      * sum of hash code values of all non-null properties. This is the same contract than
      * {@link java.util.Set#hashCode} and ensure that the hash code value is insensitive

Modified: sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/StandardImplementation.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/StandardImplementation.java?rev=1451957&r1=1451956&r2=1451957&view=diff
==============================================================================
--- sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/StandardImplementation.java
[UTF-8] (original)
+++ sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/StandardImplementation.java
[UTF-8] Sat Mar  2 23:11:13 2013
@@ -55,6 +55,14 @@ final class StandardImplementation exten
 
     /**
      * Implementations for a given interface, computed when first needed then cached.
+     *
+     * {@note In the particular case of <code>Class</code> keys, <code>IdentityHashMap</code>
and
+     *        <code>HashMap</code> have identical behavior since <code>Class</code>
is final and
+     *        does not override the <code>equals(Object)</code> and <code>hashCode()</code>
methods.
+     *        The <code>IdentityHashMap</code> Javadoc claims that it is faster
than the regular
+     *        <code>HashMap</code>. But maybe the most interesting property is
that it allocates
+     *        less objects since <code>IdentityHashMap</code> implementation
doesn't need the chain
+     *        of objects created by <code>HashMap</code>.}
      */
     private final Map<Class<?>,Class<?>> implementations;
 

Modified: sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/io/CompoundFormat.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/io/CompoundFormat.java?rev=1451957&r1=1451956&r2=1451957&view=diff
==============================================================================
--- sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/io/CompoundFormat.java [UTF-8]
(original)
+++ sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/io/CompoundFormat.java [UTF-8]
Sat Mar  2 23:11:13 2013
@@ -17,7 +17,7 @@
 package org.apache.sis.io;
 
 import java.util.Map;
-import java.util.HashMap;
+import java.util.IdentityHashMap;
 import java.util.Locale;
 import java.util.TimeZone;
 import java.util.Date;
@@ -95,7 +95,7 @@ public abstract class CompoundFormat<T> 
      * The formats for smaller unit of information.
      * Will be created only when first needed.
      */
-    private transient Map<Class<?>,Format> formats;
+    private transient Map<Class<?>, Format> formats;
 
     /**
      * Creates a new format for the given locale. The given locale can be {@code null} or
@@ -328,7 +328,7 @@ public abstract class CompoundFormat<T> 
             format = createFormat(type);
             if (format != null) {
                 if (formats == null) {
-                    this.formats = formats = new HashMap<>(4);
+                    this.formats = formats = new IdentityHashMap<>(4);
                 }
                 formats.put(type, format);
                 break;
@@ -399,7 +399,7 @@ public abstract class CompoundFormat<T> 
         @SuppressWarnings("unchecked")
         final CompoundFormat<T> clone = (CompoundFormat<T>) super.clone();
         if (clone.formats != null) {
-            clone.formats = new HashMap<>(clone.formats);
+            clone.formats = new IdentityHashMap<>(clone.formats);
             for (final Map.Entry<Class<?>,Format> entry : clone.formats.entrySet())
{
                 entry.setValue((Format) entry.getValue().clone());
             }

Modified: sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/util/Numbers.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/util/Numbers.java?rev=1451957&r1=1451956&r2=1451957&view=diff
==============================================================================
--- sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/util/Numbers.java [UTF-8] (original)
+++ sis/branches/JDK7/sis-utility/src/main/java/org/apache/sis/util/Numbers.java [UTF-8] Sat
Mar  2 23:11:13 2013
@@ -17,7 +17,7 @@
 package org.apache.sis.util;
 
 import java.util.Map;
-import java.util.HashMap;
+import java.util.IdentityHashMap;
 import java.util.List;
 import java.util.Queue;
 import java.util.Set;
@@ -51,8 +51,16 @@ public final class Numbers extends Stati
 
     /**
      * Mapping between a primitive type and its wrapper, if any.
+     *
+     * {@note In the particular case of <code>Class</code> keys, <code>IdentityHashMap</code>
and
+     *        <code>HashMap</code> have identical behavior since <code>Class</code>
is final and
+     *        does not override the <code>equals(Object)</code> and <code>hashCode()</code>
methods.
+     *        The <code>IdentityHashMap</code> Javadoc claims that it is faster
than the regular
+     *        <code>HashMap</code>. But maybe the most interesting property is
that it allocates
+     *        less objects since <code>IdentityHashMap</code> implementation
doesn't need the chain
+     *        of objects created by <code>HashMap</code>.}
      */
-    private static final Map<Class<?>,Numbers> MAPPING = new HashMap<>(16);
+    private static final Map<Class<?>,Numbers> MAPPING = new IdentityHashMap<>(16);
     static {
         new Numbers(BigDecimal.class, true, false, (byte) (DOUBLE+2)); // Undocumented enum.
         new Numbers(BigInteger.class, false, true, (byte) (DOUBLE+1)); // Undocumented enum.



Mime
View raw message