sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject [sis] 03/03: Replace AbstractMetadata.hashCode() and AbstractMetadata.prune() implementation by a MetadataVisitor. The intent is to share more common implementation, in particular the non-obvious parts about cycles and the use of Semaphores. For now we use MetadataVisitor only for hashCode(), isEmpty() and prune(), but we should migrate more functionalities in the future.
Date Mon, 25 Jun 2018 16:39:44 GMT
This is an automated email from the ASF dual-hosted git repository.

desruisseaux pushed a commit to branch geoapi-4.0
in repository https://gitbox.apache.org/repos/asf/sis.git

commit ccb3bafbdd599b90d4484c0715ce777d878ac149
Author: Martin Desruisseaux <martin.desruisseaux@geomatys.com>
AuthorDate: Mon Jun 25 18:37:30 2018 +0200

    Replace AbstractMetadata.hashCode() and AbstractMetadata.prune() implementation by a MetadataVisitor.
    The intent is to share more common implementation, in particular the non-obvious parts about cycles and the use of Semaphores.
    For now we use MetadataVisitor only for hashCode(), isEmpty() and prune(), but we should migrate more functionalities in the future.
---
 .../org/apache/sis/metadata/AbstractMetadata.java  |  29 +--
 .../java/org/apache/sis/metadata/HashCode.java     | 103 ++++++++
 .../org/apache/sis/metadata/MetadataCopier.java    |  16 +-
 .../org/apache/sis/metadata/MetadataStandard.java  |  26 +-
 .../org/apache/sis/metadata/MetadataVisitor.java   | 197 +++++++++++++++
 .../org/apache/sis/metadata/PropertyAccessor.java  |  66 ++---
 .../main/java/org/apache/sis/metadata/Pruner.java  | 276 +++++++++------------
 .../org/apache/sis/metadata/RecursivityGuard.java  |  69 ------
 .../java/org/apache/sis/metadata/HashCodeTest.java | 154 ++++++++++++
 .../apache/sis/metadata/PropertyAccessorTest.java  |  23 --
 .../java/org/apache/sis/metadata/PrunerTest.java   |   4 +-
 .../apache/sis/test/suite/MetadataTestSuite.java   |   1 +
 12 files changed, 624 insertions(+), 340 deletions(-)

diff --git a/core/sis-metadata/src/main/java/org/apache/sis/metadata/AbstractMetadata.java b/core/sis-metadata/src/main/java/org/apache/sis/metadata/AbstractMetadata.java
index f4818b6..2f1c7b3 100644
--- a/core/sis-metadata/src/main/java/org/apache/sis/metadata/AbstractMetadata.java
+++ b/core/sis-metadata/src/main/java/org/apache/sis/metadata/AbstractMetadata.java
@@ -18,7 +18,6 @@ package org.apache.sis.metadata;
 
 import java.util.Map;
 import javax.xml.bind.annotation.XmlTransient;
-import org.apache.sis.internal.system.Semaphores;
 import org.apache.sis.util.Emptiable;
 import org.apache.sis.util.ComparisonMode;
 import org.apache.sis.util.LenientComparable;
@@ -70,7 +69,7 @@ import org.apache.sis.util.collection.TreeTable;
  * use a single lock for the whole metadata tree (including children).
  *
  * @author  Martin Desruisseaux (Geomatys)
- * @version 0.8
+ * @version 1.0
  *
  * @see MetadataStandard
  *
@@ -138,21 +137,7 @@ public abstract class AbstractMetadata implements LenientComparable, Emptiable {
      */
     @Override
     public boolean isEmpty() {
-        /*
-         * The NULL_COLLECTION semaphore prevents creation of new empty collections by getter methods
-         * (a consequence of lazy instantiation). The intent is to avoid creation of unnecessary objects
-         * for all unused properties. Users should not see behavioral difference, except if they override
-         * some getters with an implementation invoking other getters. However in such cases, users would
-         * have been exposed to null values at XML marshalling time anyway.
-         */
-        final boolean allowNull = Semaphores.queryAndSet(Semaphores.NULL_COLLECTION);
-        try {
-            return Pruner.isEmpty(this, getInterface(), true, false);
-        } finally {
-            if (!allowNull) {
-                Semaphores.clear(Semaphores.NULL_COLLECTION);
-            }
-        }
+        return Pruner.isEmpty(this, false);
     }
 
     /**
@@ -163,15 +148,7 @@ public abstract class AbstractMetadata implements LenientComparable, Emptiable {
      * @throws UnmodifiableMetadataException if this metadata is not modifiable.
      */
     public void prune() {
-        // See comment in 'isEmpty()' about NULL_COLLECTION semaphore purpose.
-        final boolean allowNull = Semaphores.queryAndSet(Semaphores.NULL_COLLECTION);
-        try {
-            Pruner.isEmpty(this, getInterface(), true, true);
-        } finally {
-            if (!allowNull) {
-                Semaphores.clear(Semaphores.NULL_COLLECTION);
-            }
-        }
+        Pruner.isEmpty(this, true);
     }
 
     /**
diff --git a/core/sis-metadata/src/main/java/org/apache/sis/metadata/HashCode.java b/core/sis-metadata/src/main/java/org/apache/sis/metadata/HashCode.java
new file mode 100644
index 0000000..3b7bb3a
--- /dev/null
+++ b/core/sis-metadata/src/main/java/org/apache/sis/metadata/HashCode.java
@@ -0,0 +1,103 @@
+/*
+ * 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.metadata;
+
+
+/**
+ * Computes a hash code for the specified metadata. The hash code is defined as the sum of hash codes
+ * of all non-empty properties, plus the hash code of the interface. This is a similar contract than
+ * {@link java.util.Set#hashCode()} (except for the interface) and ensures that the hash code value
+ * is insensitive to the ordering of properties.
+ *
+ * @author  Martin Desruisseaux (Geomatys)
+ * @version 1.0
+ * @since   1.0
+ * @module
+ */
+final class HashCode extends MetadataVisitor<Integer> {
+    /**
+     * Provider of visitor instances used by {@link MetadataStandard#hashCode(Object)}.
+     */
+    private static final ThreadLocal<HashCode> VISITORS = ThreadLocal.withInitial(HashCode::new);
+
+    /**
+     * The hash code value, returned by {@link MetadataStandard#hashCode(Object)} after calculation.
+     */
+    private int code;
+
+    /**
+     * Instantiated by {@link #VISITORS} only.
+     */
+    private HashCode() {
+    }
+
+    /**
+     * Returns the visitor for the current thread if it already exists, or creates a new one otherwise.
+     */
+    static HashCode getOrCreate() {
+        return VISITORS.get();
+    }
+
+    /**
+     * Returns the thread-local variable that created this {@code HashCode} instance.
+     */
+    @Override
+    final ThreadLocal<? extends MetadataVisitor<?>> creator() {
+        return VISITORS;
+    }
+
+    /**
+     * Resets the hash code to an initial value for a new metadata instance.
+     * If another hash code computation was in progress, that code shall be saved before this method is invoked.
+     *
+     * @param  type  the standard interface of the metadata for which a hash code value will be computed.
+     */
+    @Override
+    void preVisit(final Class<?> type) {
+        code = type.hashCode();
+    }
+
+    /**
+     * Adds the hash code of the given metadata property value. Invoking this method may cause recursive calls
+     * to {@code HashCode} methods on this visitor instance if the given value is itself another metadata object.
+     *
+     * @param  value  the metadata property value for which to add hash code.
+     * @return {@code null}, meaning to not modify the metadata property value.
+     */
+    @Override
+    Object visit(final Class<?> type, final Object value) {
+        if (!ValueExistencePolicy.isNullOrEmpty(value)) {
+            /*
+             * We make a local copy of the 'code' field in the 'c' local variable
+             * because the call to 'value.hashCode()' may cause a recursive call
+             * to the methods of this visitor, which would overwrite 'code'.
+             */
+            int c = code;
+            c += value.hashCode();
+            code = c;
+        }
+        return null;
+    }
+
+    /**
+     * Returns the hash code result after visiting all elements in a metadata instance.
+     */
+    @Override
+    Integer result() {
+        return code;
+    }
+}
diff --git a/core/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataCopier.java b/core/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataCopier.java
index ec71c19..a7e8a28 100644
--- a/core/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataCopier.java
+++ b/core/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataCopier.java
@@ -25,6 +25,7 @@ import java.util.IdentityHashMap;
 import java.util.Arrays;
 import java.util.Collection;
 import org.apache.sis.util.ArgumentChecks;
+import org.apache.sis.util.Exceptions;
 import org.apache.sis.util.resources.Errors;
 import org.apache.sis.util.collection.CodeListSet;
 
@@ -80,7 +81,7 @@ import org.apache.sis.util.collection.CodeListSet;
  * </div>
  *
  * @author  Martin Desruisseaux (Geomatys)
- * @version 0.8
+ * @version 1.0
  *
  * @see ModifiableMetadata#unmodifiable()
  *
@@ -172,16 +173,9 @@ public class MetadataCopier {
                 final PropertyAccessor accessor = std.getAccessor(new CacheKey(metadata.getClass(), type), false);
                 if (accessor != null) try {
                     return accessor.copy(metadata, this);
-                } catch (RuntimeException e) {
-                    throw e;
-                } catch (Exception e) {
-                    /*
-                     * In our PropertyAccessor.copy(…) implementation, checked exceptions can only be thrown
-                     * by the constructor.   Note that Class.newInstance() may throw more checked exceptions
-                     * than the ones declared in its method signature,  so we really need to catch Exception
-                     * (ReflectiveOperationException is not sufficient).
-                     */
-                    throw new UnsupportedOperationException(Errors.format(Errors.Keys.CanNotCopy_1, accessor.type), e);
+                } catch (ReflectiveOperationException e) {
+                    throw new UnsupportedOperationException(Errors.format(Errors.Keys.CanNotCopy_1, accessor.type),
+                            Exceptions.unwrap(e));
                 }
             }
         }
diff --git a/core/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataStandard.java b/core/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataStandard.java
index 21173ab..4517504 100644
--- a/core/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataStandard.java
+++ b/core/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataStandard.java
@@ -90,7 +90,7 @@ import static org.apache.sis.util.ArgumentChecks.ensureNonNullElement;
  * by a large amount of {@link ModifiableMetadata}.
  *
  * @author  Martin Desruisseaux (Geomatys)
- * @version 0.8
+ * @version 1.0
  *
  * @see AbstractMetadata
  *
@@ -1024,24 +1024,14 @@ public class MetadataStandard implements Serializable {
      */
     public int hashCode(final Object metadata) throws ClassCastException {
         if (metadata != null) {
-            final Map<Object,Object> inProgress = RecursivityGuard.HASH_CODES.get();
-            if (inProgress.put(metadata, Boolean.TRUE) == null) {
-                // See comment in 'equals(…) about NULL_COLLECTION semaphore purpose.
-                final boolean allowNull = Semaphores.queryAndSet(Semaphores.NULL_COLLECTION);
-                try {
-                    return getAccessor(new CacheKey(metadata.getClass()), true).hashCode(metadata);
-                } finally {
-                    inProgress.remove(metadata);
-                    if (!allowNull) {
-                        Semaphores.clear(Semaphores.NULL_COLLECTION);
-                    }
-                }
-            }
+            final Integer hash = HashCode.getOrCreate().walk(this, null, metadata, true);
+            if (hash != null) return hash;
             /*
-             * If we get there, a cycle has been found. We can not compute a hash code value for that metadata.
-             * However it should not be a problem since this metadata is part of a bigger metadata object, and
-             * that enclosing object has other properties for computing its hash code. We just need the result
-             * to be consistent, wich should be the case if properties ordering is always the same.
+             * 'hash' may be null if a cycle has been found. Example: A depends on B which depends on A,
+             * in which case the null value is returned for the second occurrence of A (not the first one).
+             * We can not compute a hash code value here, but it should be okay since that metadata is part
+             * of a bigger metadata object, and that enclosing object should have other properties for computing
+             * its hash code.
              */
         }
         return 0;
diff --git a/core/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataVisitor.java b/core/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataVisitor.java
new file mode 100644
index 0000000..4b9c414
--- /dev/null
+++ b/core/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataVisitor.java
@@ -0,0 +1,197 @@
+/*
+ * 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.metadata;
+
+import java.util.Map;
+import java.util.IdentityHashMap;
+import java.util.ConcurrentModificationException;
+import org.apache.sis.internal.system.Semaphores;
+
+
+/**
+ * A visitor of metadata properties with a safety against infinite recursivity.
+ * The visitor may compute a result, for example a hash code value or a boolean
+ * testing whether the metadata is empty. Each {@code MetadataVisitor} instance
+ * is used by one thread; this class does not need to be thread-safe.
+ *
+ * @author  Martin Desruisseaux (Geomatys)
+ * @version 1.0
+ *
+ * @param  <R>  the type of result of walking in the metadata.
+ *
+ * @since 1.0
+ * @module
+ */
+abstract class MetadataVisitor<R> {
+    /**
+     * Sentinel value that may be returned by {@link #visit(Class, Object)} for meaning that a property value
+     * should be set to {@code null}. If the property type is a collection, then "null" value is interpreted
+     * as an instruction to {@linkplain java.util.Collection#clear() clear} the collection.
+     *
+     * <div class="note"><b>Note:</b> a sentinel value is required because {@code visit(…)} already uses
+     * the {@code null} return value for meaning that the property value shall not be modified.</div>
+     */
+    static final Object CLEAR = Void.TYPE;                              // The choice of this type is arbitrary.
+
+    /**
+     * Sentinel value that may be returned by {@link #visit(Class, Object)} for notifying the walker to stop.
+     * This value causes {@link #walk walk(…)} to stop its iteration, but does not stop iteration by the parent
+     * if {@code walk(…)} has been invoked recursively. The {@link #result()} method shall return a valid result
+     * even if the iteration has been terminated.
+     */
+    static final Object SKIP_SIBLINGS = InterruptedException.class;     // The choice of this type is arbitrary.
+
+    /**
+     * A guard against infinite recursivity in {@link #walk(MetadataStandard, Class, Object, boolean)}.
+     * Keys are visited metadata instances and values are computed value. The value may be null if
+     * the computation is in progress.
+     *
+     * <div class="section">The problem</div>
+     * Cyclic associations can exist in ISO 19115 metadata. For example {@code Instrument} has a reference
+     * to the platform it is mounted on, and the {@code Platform} has a list of instruments mounted on it.
+     * Consequently walking down the metadata tree can cause infinite recursivity, unless we keep trace of
+     * previously visited metadata objects in order to avoid visiting them again.
+     *
+     * We use an {@link IdentityHashMap} for that purpose, since the recursivity problem exists only when revisiting
+     * the exact same instance. Furthermore, {@code HashMap} would not suit since it invokes {@code equals(Object)}
+     * and {@code hashCode()}, which are among the methods that we want to avoid invoking twice.
+     */
+    private final Map<Object,R> visited;
+
+    /**
+     * Count of nested calls to {@link #walk(MetadataStandard, Class, Object, boolean)} method.
+     * When this count reach zero, the visitor should be removed from the thread local variable.
+     */
+    private int nestedCount;
+
+    /**
+     * Value of the {@link Semaphores#NULL_COLLECTION} flag when we started the walk.
+     * The {@code NULL_COLLECTION} flag prevents creation of new empty collections by getter methods
+     * (a consequence of lazy instantiation). The intent is to avoid creation of unnecessary objects
+     * for all unused properties. Users should not see behavioral difference, except if they override
+     * some getters with an implementation invoking other getters. However in such cases, users would
+     * have been exposed to null values at XML marshalling time anyway.
+     */
+    private boolean allowNull;
+
+    /**
+     * Creates a new visitor.
+     */
+    protected MetadataVisitor() {
+        visited = new IdentityHashMap<>();
+    }
+
+    /**
+     * The thread-local variable that created this {@code MetadataVisitor} instance.
+     * This is usually a static final {@code VISITORS} constant defined in the subclass.
+     */
+    abstract ThreadLocal<? extends MetadataVisitor<?>> creator();
+
+    /**
+     * Invokes {@link #visit(Class, Object)} for all elements of the given metadata if that metadata has not
+     * already been visited. The computation result is returned (may be the result of a previous computation).
+     *
+     * <p>This method is typically invoked recursively while we iterate down the metadata tree.
+     * It creates a map of visited nodes when the iteration begin, and deletes that map when the
+     * iteration ends.</p>
+     *
+     * @param  standard   the metadata standard implemented by the object to visit.
+     * @param  type       the standard interface implemented by the metadata object, or {@code null} if unknown.
+     * @param  metadata   the metadata to visit.
+     * @param  mandatory  {@code true} for throwing an exception for unsupported metadata type, or {@code false} for ignoring.
+     * @return the value of {@link #result()} after all elements of the given metadata have been visited.
+     *         If the given metadata instance has already been visited, then this is the previously computed value.
+     *         If the computation is in progress (e.g. a cyclic graph), then this method returns {@code null}.
+     */
+    final R walk(final MetadataStandard standard, final Class<?> type, final Object metadata, final boolean mandatory) {
+        if (!visited.containsKey(metadata)) {               // Reminder: the associated value may be null.
+            final PropertyAccessor accessor = standard.getAccessor(new CacheKey(metadata.getClass(), type), mandatory);
+            if (accessor != null) {
+                preVisit(accessor.type);
+                if (visited.put(metadata, null) != null) {
+                    // Should never happen, unless this method is invoked concurrently in another thread.
+                    throw new ConcurrentModificationException();
+                }
+                if (nestedCount++ == 0) {
+                    /*
+                     * The NULL_COLLECTION semaphore prevents creation of new empty collections by getter methods
+                     * (a consequence of lazy instantiation). The intent is to avoid creation of unnecessary objects
+                     * for all unused properties. Users should not see behavioral difference, except if they override
+                     * some getters with an implementation invoking other getters. However in such cases, users would
+                     * have been exposed to null values at XML marshalling time anyway.
+                     */
+                    allowNull = Semaphores.queryAndSet(Semaphores.NULL_COLLECTION);
+                }
+                try {
+                    accessor.walk(this, metadata);
+                } finally {
+                    if (--nestedCount == 0) {
+                        if (!allowNull) {
+                            Semaphores.clear(Semaphores.NULL_COLLECTION);
+                        }
+                        creator().remove();
+                    }
+                }
+                final R result = result();
+                if (visited.put(metadata, result) != null) {
+                    throw new ConcurrentModificationException();
+                }
+                return result;
+            }
+        }
+        return visited.get(metadata);
+    }
+
+    /**
+     * Invoked when a new metadata is about to be visited. After this method has been invoked,
+     * {@link #visit(Class, Object)} will be invoked for each property in the metadata object.
+     *
+     * @param  type  the standard interface implemented by the metadata instance being visited.
+     */
+    void preVisit(Class<?> type) {
+    }
+
+    /**
+     * Invoked when a new metadata property is being visited. The current property value is given in argument.
+     * The return value is interpreted as below:
+     *
+     * <ul>
+     *   <li>{@link #SKIP_SIBLINGS}: do not iterate over other elements of current metadata,
+     *       but continue iteration over elements of the parent metadata.</li>
+     *   <li>{@link #CLEAR}: clear the property value (e.g. by setting it to {@code null}),
+     *       then continue with next sibling property.</li>
+     *   <li>Any other non-null value: set the property value to the given value,
+     *       then continue with next sibling property.</li>
+     *   <li>{@code null}: continue with next sibling property without setting any value.</li>
+     * </ul>
+     *
+     * @param  type   the type of elements. Note that this is not necessarily the type
+     *                of given {@code value} argument if the later is a collection.
+     * @param  value  value of the metadata property being visited.
+     * @return one of the sentinel values ({@link #CLEAR} or {@link #SKIP_SIBLINGS}),
+     *         or the new property value to set, or {@code null} for leaving the property value unchanged.
+     */
+    abstract Object visit(Class<?> type, Object value);
+
+    /**
+     * Returns the result of visiting all elements in a metadata instance.
+     * This method is invoked after all metadata properties have been visited,
+     * or after a {@link #visit(Class, Object)} method call returned {@link #SKIP_SIBLINGS}.
+     * The value returned by this method will be cached in case the same metadata instance is revisited again.
+     */
+    abstract R result();
+}
diff --git a/core/sis-metadata/src/main/java/org/apache/sis/metadata/PropertyAccessor.java b/core/sis-metadata/src/main/java/org/apache/sis/metadata/PropertyAccessor.java
index 1f5ae01..07da234 100644
--- a/core/sis-metadata/src/main/java/org/apache/sis/metadata/PropertyAccessor.java
+++ b/core/sis-metadata/src/main/java/org/apache/sis/metadata/PropertyAccessor.java
@@ -94,7 +94,7 @@ class PropertyAccessor {
      * Enumeration constants for the {@code mode} argument
      * in the {@link #set(int, Object, Object, int)} method.
      */
-    static final int RETURN_NULL=0, RETURN_PREVIOUS=1, APPEND=2;
+    static final int RETURN_NULL=0, RETURN_PREVIOUS=1, APPEND=2, IGNORE_READ_ONLY=3;
 
     /**
      * Additional getter to declare in every list of getter methods that do not already provide
@@ -744,15 +744,17 @@ class PropertyAccessor {
      * {@link #RETURN_PREVIOUS}. The {@code mode} argument can be one of the following:
      *
      * <ul>
-     *   <li>RETURN_NULL:     Set the value and returns {@code null}.</li>
-     *   <li>RETURN_PREVIOUS: Set the value and returns the previous value. If the previous value was a
-     *                        collection or a map, then that value is copied in a new collection or map
-     *                        before the new value is set because the setter methods typically copy the
-     *                        new collection in their existing instance.</li>
-     *   <li>APPEND:          Set the value only if it doesn't overwrite an existing value, then returns
-     *                        {@link Boolean#TRUE} if the metadata changed as a result of this method call,
-     *                        {@link Boolean#FALSE} if the metadata didn't changed or {@code null} if the
-     *                        value can not be set because an other value already exists.</li>
+     *   <li>RETURN_NULL:      Set the value and returns {@code null}.</li>
+     *   <li>RETURN_PREVIOUS:  Set the value and returns the previous value. If the previous value was a
+     *                         collection or a map, then that value is copied in a new collection or map
+     *                         before the new value is set because the setter methods typically copy the
+     *                         new collection in their existing instance.</li>
+     *   <li>APPEND:           Set the value only if it does not overwrite an existing value, then returns
+     *                         {@link Boolean#TRUE} if the metadata changed as a result of this method call,
+     *                         {@link Boolean#FALSE} if the metadata didn't changed or {@code null} if the
+     *                         value can not be set because an other value already exists.</li>
+     *   <li>IGNORE_READ_ONLY: Set the value and returns {@code null} on success. If the property is read-only,
+     *                         do not throw an exception; returns exception class instead.</li>
      * </ul>
      *
      * <p>The {@code APPEND} mode has an additional side effect: it sets the {@code append} argument to
@@ -769,8 +771,8 @@ class PropertyAccessor {
      * @param  metadata  the metadata object on which to set the value.
      * @param  value     the new value.
      * @param  mode      whether this method should first fetches the old value,
-     *                   as one of the {@code RETURN_*} constants.
-     * @return the old value, or {@code null} if {@code returnValue} was {@code RETURN_NULL}.
+     *                   as one of the constants listed in this method javadoc.
+     * @return the old value, or {@code null} if {@code mode} was {@code RETURN_NULL} or {@code IGNORE_READ_ONLY}.
      * @throws UnmodifiableMetadataException if the property for the given key is read-only.
      * @throws ClassCastException if the given value is not of the expected type.
      * @throws BackingStoreException if the implementation threw a checked exception.
@@ -788,6 +790,7 @@ class PropertyAccessor {
                 final Object oldValue;
                 final Object snapshot;                      // Copy of oldValue before modification.
                 switch (mode) {
+                    case IGNORE_READ_ONLY:
                     case RETURN_NULL: {
                         oldValue = null;
                         snapshot = null;
@@ -826,8 +829,8 @@ class PropertyAccessor {
                 final Object[] newValues = new Object[] {value};
                 Boolean changed = convert(getter, metadata, oldValue, newValues, elementTypes[index], mode == APPEND);
                 if (changed == null) {
-                    changed = (mode == RETURN_NULL) || (newValues[0] != oldValue);
-                    if (changed && mode == APPEND && !ValueExistencePolicy.isNullOrEmpty(oldValue)) {
+                    changed = (mode == RETURN_NULL) || (mode == IGNORE_READ_ONLY) || (newValues[0] != oldValue);
+                    if (changed && mode == APPEND && !isNullOrEmpty(oldValue)) {
                         /*
                          * If 'convert' did not added the value in a collection and if a value already
                          * exists, do not modify the existing value. Exit now with "no change" status.
@@ -841,7 +844,11 @@ class PropertyAccessor {
                 return (mode == APPEND) ? changed : snapshot;
             }
         }
-        throw new UnmodifiableMetadataException(Errors.format(Errors.Keys.CanNotSetPropertyValue_1, names[index]));
+        if (mode == IGNORE_READ_ONLY) {
+            return UnmodifiableMetadataException.class;
+        }
+        throw new UnmodifiableMetadataException(Errors.format(
+                Errors.Keys.CanNotSetPropertyValue_1, type.getSimpleName() + '.' + names[index]));
     }
 
     /**
@@ -1239,10 +1246,9 @@ class PropertyAccessor {
      * @param  copier     contains a map of metadata objects already copied.
      * @return a copy of the given metadata object, or {@code metadata} itself if there is
      *         no known implementation class or that implementation has no setter method.
-     * @throws Exception if an error occurred while creating the copy. This include any
-     *         checked checked exception that the no-argument constructor may throw.
+     * @throws ReflectiveOperationException if an error occurred while creating the copy.
      */
-    final Object copy(final Object metadata, final MetadataCopier copier) throws Exception {
+    final Object copy(final Object metadata, final MetadataCopier copier) throws ReflectiveOperationException {
         if (setters == null) {
             return metadata;
         }
@@ -1269,23 +1275,25 @@ class PropertyAccessor {
     }
 
     /**
-     * Computes a hash code for the specified metadata. The hash code is defined as the sum
-     * of hash code values of all non-empty properties, plus the hash code of the interface.
-     * This is a similar contract than {@link java.util.Set#hashCode()} (except for the interface)
-     * and ensures that the hash code value is insensitive to the ordering of properties.
+     * Invokes {@link MetadataVisitor#visit(Class, Object)} for all non-null properties in the given metadata.
+     * This method is not recursive, i.e. it does not traverse the children of the elements in the given metadata.
      *
-     * @throws BackingStoreException if the implementation threw a checked exception.
+     * @param  visitor   the object on which to invoke {@link MetadataVisitor#visit(Class, Object)}.
+     * @param  metadata  the metadata instance for which to visit the non-null properties.
      */
-    public int hashCode(final Object metadata) throws BackingStoreException {
+    final void walk(final MetadataVisitor<?> visitor, final Object metadata) {
         assert type.isInstance(metadata) : metadata;
-        int code = type.hashCode();
         for (int i=0; i<standardCount; i++) {
-            final Object value = get(getters[i], metadata);
-            if (!isNullOrEmpty(value)) {
-                code += value.hashCode();
+            final Object element = get(getters[i], metadata);
+            if (element != null) {
+                Object r = visitor.visit(elementTypes[i], element);
+                if (r != null) {
+                    if (r == MetadataVisitor.SKIP_SIBLINGS) break;
+                    if (r == MetadataVisitor.CLEAR) r = null;
+                    set(i, metadata, r, IGNORE_READ_ONLY);
+                }
             }
         }
-        return code;
     }
 
     /**
diff --git a/core/sis-metadata/src/main/java/org/apache/sis/metadata/Pruner.java b/core/sis-metadata/src/main/java/org/apache/sis/metadata/Pruner.java
index 5864fe7..d61b546 100644
--- a/core/sis-metadata/src/main/java/org/apache/sis/metadata/Pruner.java
+++ b/core/sis-metadata/src/main/java/org/apache/sis/metadata/Pruner.java
@@ -16,7 +16,6 @@
  */
 package org.apache.sis.metadata;
 
-import java.util.Map;
 import java.util.Iterator;
 import java.util.Collection;
 import org.opengis.util.ControlledVocabulary;
@@ -29,44 +28,46 @@ import static org.apache.sis.metadata.ValueExistencePolicy.*;
 /**
  * Implementation of {@link AbstractMetadata#isEmpty()} and {@link ModifiableMetadata#prune()} methods.
  *
+ * The {@link #visited} map inherited by this class is the thread-local map of metadata objects already tested.
+ * Keys are metadata instances, and values are the results of the {@code metadata.isEmpty()} operation.
+ * If the final operation requested by the user is {@code isEmpty()}, then this map will contain one of
+ * few {@code false} values since the walk in the tree will stop at the first {@code false} value found.
+ * If the final operation requested by the user is {@code prune()}, then this map will contain a mix of
+ * {@code false} and {@code true} values since the operation will unconditionally walk through the entire tree.
+ *
  * @author  Martin Desruisseaux (Geomatys)
- * @version 0.8
+ * @version 1.0
  * @since   0.3
  * @module
  */
-final class Pruner {
+final class Pruner extends MetadataVisitor<Boolean> {
     /**
-     * The thread-local map of metadata objects already tested. The keys are metadata instances, and values
-     * are the results of the {@code metadata.isEmpty()} operation.
-     *
-     * If the final operation requested by the user is {@code isEmpty()}, then this map will contain at most
-     * one {@code false} value since the walk in the tree will stop at the first {@code false} value found.
-     *
-     * If the final operation requested by the user is {@code prune()}, then this map will contain a mix of
-     * {@code false} and {@code true} values since the operation will unconditionally walk through the entire tree.
+     * Provider of visitor instances.
      */
-    private static final RecursivityGuard<Boolean> MAPS = new RecursivityGuard<>();
+    private static final ThreadLocal<Pruner> VISITORS = ThreadLocal.withInitial(Pruner::new);
 
     /**
-     * For internal usage only.
+     * {@code true} for removing empty properties.
      */
-    private Pruner() {
-    }
+    private boolean prune;
+
+    /**
+     * Whether the metadata is empty.
+     */
+    private boolean isEmpty;
 
     /**
-     * Returns the metadata properties. When used for pruning empty values, the map needs to
-     * include empty (but non-null) values in order to allow us to set them to {@code null}.
+     * Creates a new object which will test or prune metadata properties.
      */
-    private static Map<String, Object> asMap(final Object metadata, final PropertyAccessor accessor, final boolean prune) {
-        return new ValueMap(metadata, accessor, KeyNamePolicy.JAVABEANS_PROPERTY, prune ? NON_NULL : NON_EMPTY);
+    private Pruner() {
     }
 
     /**
-     * Returns {@code true} if the value for the given entry is a primitive type.
+     * Returns the thread-local variable that created this {@code Pruner} instance.
      */
-    private static boolean isPrimitive(final Map.Entry<String,Object> entry) {
-        return (entry instanceof ValueMap.Property) &&
-                ((ValueMap.Property) entry).getValueType().isPrimitive();
+    @Override
+    final ThreadLocal<? extends MetadataVisitor<?>> creator() {
+        return VISITORS;
     }
 
     /**
@@ -74,164 +75,115 @@ final class Pruner {
      * This method is the entry point for the {@link AbstractMetadata#isEmpty()} and
      * {@link ModifiableMetadata#prune()} public methods.
      *
-     * <p>This method is typically invoked recursively while we iterate down the metadata tree.
-     * It creates a map of visited nodes when the iteration begin, and deletes that map when the
-     * iteration ends.</p>
-     *
-     * @param  metadata      the metadata object.
-     * @param  mandatory     {@code true} if we shall throw an exception if {@code metadata} is not of the expected class.
-     * @param  propertyType  if the given metadata is the return value of a property, the declared type of that property.
-     * @param  prune         {@code true} for deleting empty entries.
+     * @param  metadata  the metadata object.
+     * @param  prune     {@code true} for deleting empty entries.
      * @return {@code true} if all metadata properties are null or empty.
      */
-    static boolean isEmpty(final AbstractMetadata metadata, final Class<?> propertyType,
-            final boolean mandatory, final boolean prune)
-    {
-        final PropertyAccessor accessor = metadata.getStandard().getAccessor(
-                new CacheKey(metadata.getClass(), propertyType), mandatory);
-        if (accessor == null) {
-            return false;                       // For metadata of unknown class, conservatively assume non-empty.
-        }
-        final Map<String,Object> properties = asMap(metadata, accessor, prune);
-        final Map<Object,Boolean> tested = MAPS.get();
-        if (!tested.isEmpty()) {
-            return isEmpty(accessor, properties, tested, prune);
-        } else try {
-            tested.put(metadata, Boolean.FALSE);
-            return isEmpty(accessor, properties, tested, prune);
-        } finally {
-            MAPS.remove();
-            /*
-             * Note: we could invoke 'tested.clear()' instead in order to recycle the existing
-             *       IdentityHashMap instance, but we presume that usage of this class will be
-             *       rare enough for not being worth to keep those objects around.
-             */
-        }
+    static boolean isEmpty(final AbstractMetadata metadata, final boolean prune) {
+        final Pruner visitor = VISITORS.get();
+        final boolean p = visitor.prune;
+        visitor.prune = prune;
+        final Boolean r = visitor.walk(metadata.getStandard(), metadata.getInterface(), metadata, false);
+        visitor.prune = p;
+        return (r != null) && r;        // If there is a cycle (r == null), then the metadata is non-empty.
+    }
+
+    /**
+     * Marks a metadata instance as empty before we start visiting its non-null properties.
+     * If the metadata does not contain any property, then this field will stay {@code true}.
+     */
+    @Override
+    void preVisit(Class<?> type) {
+        isEmpty = true;
     }
 
     /**
-     * {@link #isEmpty(AbstractMetadata, Class, boolean, boolean)} implementation, potentially
-     * invoked recursively for inspecting child metadata and optionally removing empty ones.
-     * The map given in argument is a safety guard against infinite recursivity.
+     * Invoked for each element in the metadata to test or prune. This method is invoked only for new elements
+     * not yet processed by {@code Pruner}. The element may be a value object or a collection. For convenience
+     * we will proceed as if we had only collections, wrapping value object in a singleton collection.
      *
-     * @param  accessor    the accessor that provided the metadata {@code properties}.
-     * @param  properties  the metadata properties.
-     * @param  tested      an initially singleton map, to be filled with tested metadata.
-     * @param  prune       {@code true} for removing empty properties.
-     * @return {@code true} if all metadata properties are null or empty.
+     * @param  type   the type of elements. Note that this is not necessarily the type
+     *                of given {@code element} argument if the later is a collection.
+     * @param  value  value of the metadata element being visited.
      */
-    private static boolean isEmpty(final PropertyAccessor accessor, final Map<String,Object> properties,
-            final Map<Object,Boolean> tested, final boolean prune)
-    {
-        boolean isEmpty = true;
-        for (final Map.Entry<String,Object> entry : properties.entrySet()) {
-            final Object value = entry.getValue();
-            /*
-             * No need to check for null values, because the ValueExistencePolicy argument
-             * given to asMap(…) asked for non-null values. If nevertheless a value is null,
-             * following code should be robust to that.
-             *
-             * We use the 'tested' map in order to avoid computing the same value twice, but
-             * also as a check against infinite recursivity - which is why a value needs to be
-             * set before to iterate over children. The default value is 'false' because if we
-             * test the same object through a "A → B → A" dependency chain, this means that A
-             * was not empty (since it contains B).
-             */
-            final Boolean isEntryEmpty = tested.put(value, Boolean.FALSE);
-            if (isEntryEmpty != null) {
-                if (isEntryEmpty) {                     // If a value was already set, restore the original value.
-                    tested.put(value, Boolean.TRUE);
-                } else {
-                    isEmpty = false;
-                    if (!prune) break;                  // No need to continue if we are not pruning the metadata.
-                }
-            } else {
+    @Override
+    Object visit(final Class<?> type, final Object value) {
+        final boolean isEmptyMetadata = isEmpty;    // Save the value in case it is overwritten by recursive invocations.
+        boolean isEmptyValue = true;
+        final Collection<?> values = CollectionsExt.toCollection(value);
+        for (final Iterator<?> it = values.iterator(); it.hasNext();) {
+            final Object element = it.next();
+            if (!isNullOrEmpty(element)) {
                 /*
-                 * At this point, 'value' is a new instance not yet processed by Pruner. The value may
-                 * be a data object or a collection. For convenience we will proceed as if we had only
-                 * collections, wrapping data object in a singleton collection if necessary.
+                 * At this point, 'element' is not an empty CharSequence, Collection or array.
+                 * It may be an other metadata, a Java primitive type or user-defined object.
+                 *
+                 *  - For AbstractMetadata, delegate to the public API in case it has been overriden.
+                 *  - For user-defined Emptiable, delegate to the user's isEmpty() method. Note that
+                 *    we test at different times depending if 'prune' is true of false.
                  */
-                Class<?> elementType = null;                    // To be computed when first needed.
-                boolean allElementsAreEmpty = true;
-                final Collection<?> values = CollectionsExt.toCollection(value);
-                for (final Iterator<?> it = values.iterator(); it.hasNext();) {
-                    final Object element = it.next();
-                    if (!isNullOrEmpty(element)) {
+                boolean isEmptyElement = false;
+                if (element instanceof AbstractMetadata) {
+                    final AbstractMetadata md = (AbstractMetadata) element;
+                    if (prune) md.prune();
+                    isEmptyElement = md.isEmpty();
+                } else if (!prune && element instanceof Emptiable) {
+                    isEmptyElement = ((Emptiable) element).isEmpty();
+                    // If 'prune' is true, we will rather test for Emptiable after our pruning attempt.
+                } else if (!(element instanceof ControlledVocabulary)) {
+                    final MetadataStandard standard = MetadataStandard.forClass(element.getClass());
+                    if (standard != null) {
                         /*
-                         * At this point, 'element' is not an empty CharSequence, Collection or array.
-                         * It may be an other metadata, a Java primitive type or user-defined object.
-                         *
-                         *  - For AbstractMetadata, delegate to the public API in case it has been overriden.
-                         *  - For user-defined Emptiable, delegate to the user's isEmpty() method. Note that
-                         *    we test at different times depending if 'prune' is true of false.
+                         * For implementation that are not subtype of AbstractMetadata but nevertheless
+                         * implement some metadata interfaces, we will invoke recursively this method.
                          */
-                        boolean isEmptyElement = false;
-                        if (element instanceof AbstractMetadata) {
-                            final AbstractMetadata md = (AbstractMetadata) element;
-                            if (prune) md.prune();
-                            isEmptyElement = md.isEmpty();
-                        } else if (!prune && element instanceof Emptiable) {
-                            isEmptyElement = ((Emptiable) element).isEmpty();
-                            // If 'prune' is true, we will rather test for Emptiable after our pruning attempt.
-                        } else if (!(element instanceof ControlledVocabulary)) {
-                            final MetadataStandard standard = MetadataStandard.forClass(element.getClass());
-                            if (standard != null) {
-                                /*
-                                 * For implementation that are not subtype of AbstractMetadata but nevertheless
-                                 * implement some metadata interface, we will invoke recursively this method.
-                                 * But since a class may implement more than one interface, we need to get the
-                                 * type of the value returned by the getter method in order to take in account
-                                 * only that type.
-                                 */
-                                if (elementType == null) {
-                                    elementType = accessor.type(accessor.indexOf(entry.getKey(), false),
-                                                                TypeValuePolicy.ELEMENT_TYPE);
-                                }
-                                final PropertyAccessor elementAccessor = standard.getAccessor(
-                                        new CacheKey(element.getClass(), elementType), false);
-                                if (elementAccessor != null) {
-                                    isEmptyElement = isEmpty(elementAccessor, asMap(element, elementAccessor, prune), tested, prune);
-                                    if (!isEmptyElement && element instanceof Emptiable) {
-                                        isEmptyElement = ((Emptiable) element).isEmpty();
-                                    }
-                                }
-                            } else if (isPrimitive(entry)) {
-                                if (value instanceof Number) {
-                                    isEmptyElement = Double.isNaN(((Number) value).doubleValue());
-                                } else {
-                                    // Typically methods of the kind 'isFooAvailable()'.
-                                    isEmptyElement = Boolean.FALSE.equals(value);
-                                }
-                            }
-                        }
-                        if (!isEmptyElement) {
-                            // At this point, we have determined that the property is not empty.
-                            // If we are not removing empty nodes, there is no need to continue.
-                            if (!prune) {
-                                return false;
+                        final Boolean r = walk(standard, type, value, false);
+                        if (r != null) {
+                            isEmptyElement = r;
+                            if (!isEmptyElement && element instanceof Emptiable) {
+                                isEmptyElement = ((Emptiable) element).isEmpty();
                             }
-                            allElementsAreEmpty = false;
-                            continue;
                         }
-                    }
-                    // Found an empty element. Remove it if we are
-                    // allowed to do so, then check next elements.
-                    if (prune && values == value) {
-                        it.remove();
+                    } else if (value instanceof Number) {
+                        isEmptyElement = Double.isNaN(((Number) value).doubleValue());
+                    } else if (value instanceof Boolean) {
+                        // Typically methods of the kind 'isFooAvailable()'.
+                        isEmptyElement = !((Boolean) value);
                     }
                 }
-                // If all elements were empty, set the whole property to 'null'.
-                isEmpty &= allElementsAreEmpty;
-                if (allElementsAreEmpty) {
-                    tested.put(value, Boolean.TRUE);
-                    if (prune) try {
-                        entry.setValue(null);
-                    } catch (UnsupportedOperationException e) {
-                        // Entry is read only - ignore.
+                if (!isEmptyElement) {
+                    /*
+                     * At this point, we have determined that the property is not empty.
+                     * If we are not removing empty nodes, there is no need to continue.
+                     */
+                    if (!prune) {
+                        isEmpty = false;
+                        return SKIP_SIBLINGS;
                     }
+                    isEmptyValue = false;
+                    continue;
                 }
             }
+            /*
+             * Found an empty element. Remove it if the element is part of a collection,
+             * then move to the next element in the collection (not yet the next property).
+             */
+            if (prune && values == value) {
+                it.remove();
+            }
         }
+        /*
+         * If all elements were empty, set the whole property to 'null'.
+         */
+        isEmpty = isEmptyMetadata & isEmptyValue;
+        return isEmptyValue & prune ? CLEAR : null;
+    }
+
+    /**
+     * Returns the result of visiting all elements in the metadata.
+     */
+    @Override
+    Boolean result() {
         return isEmpty;
     }
 }
diff --git a/core/sis-metadata/src/main/java/org/apache/sis/metadata/RecursivityGuard.java b/core/sis-metadata/src/main/java/org/apache/sis/metadata/RecursivityGuard.java
deleted file mode 100644
index ad122cf..0000000
--- a/core/sis-metadata/src/main/java/org/apache/sis/metadata/RecursivityGuard.java
+++ /dev/null
@@ -1,69 +0,0 @@
-/*
- * 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.metadata;
-
-import java.util.Map;
-import java.util.IdentityHashMap;
-
-
-/**
- * A guard against infinite recursivity in {@link AbstractMetadata#hashCode()},
- * {@link AbstractMetadata#isEmpty()} and {@link ModifiableMetadata#prune()} methods.
- * {@link AbstractMetadata#equals(Object)} uses an other implementation in {@link ObjectPair}.
- *
- * <div class="section">The problem</div>
- * Cyclic associations can exist in ISO 19115 metadata. For example {@code Instrument} know the platform
- * it is mounted on, and the {@code Platform} know its list of instrument. Consequently walking down the
- * metadata tree can cause infinite recursivity, unless we keep trace of previously visited metadata objects
- * in order to avoid visiting them again. We use an {@link IdentityHashMap} for that purpose, since the
- * recursivity problem exists only when revisiting the exact same instance. Furthermore, {@code HashMap}
- * would not suit since it invokes {@code equals(Object)} and {@code hashCode()}, which are precisely
- * the methods that we want to avoid invoking twice.
- *
- * @author  Martin Desruisseaux (Geomatys)
- * @version 0.3
- *
- * @param <V>  the kind of values to store in the maps.
- *
- * @see #HASH_CODES
- * @see ObjectPair#CURRENT
- * @see Pruner#MAPS
- *
- * @since 0.3
- * @module
- */
-final class RecursivityGuard<V> extends ThreadLocal<Map<Object,V>> {
-    /**
-     * The recursivity guard to use during {@code hashCode()} computations.
-     * The values have no meaning for this map; only the keys matter.
-     */
-    static final RecursivityGuard<Object> HASH_CODES = new RecursivityGuard<>();
-
-    /**
-     * Creates a new thread-local map.
-     */
-    RecursivityGuard() {
-    }
-
-    /**
-     * Creates an initially empty hash map when first needed, before any recursive invocation.
-     */
-    @Override
-    protected Map<Object,V> initialValue() {
-        return new IdentityHashMap<>();
-    }
-}
diff --git a/core/sis-metadata/src/test/java/org/apache/sis/metadata/HashCodeTest.java b/core/sis-metadata/src/test/java/org/apache/sis/metadata/HashCodeTest.java
new file mode 100644
index 0000000..bf2f284
--- /dev/null
+++ b/core/sis-metadata/src/test/java/org/apache/sis/metadata/HashCodeTest.java
@@ -0,0 +1,154 @@
+/*
+ * 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.metadata;
+
+import java.util.Arrays;
+import org.opengis.util.InternationalString;
+import org.opengis.metadata.Identifier;
+import org.opengis.metadata.citation.Role;
+import org.opengis.metadata.citation.Citation;
+import org.opengis.metadata.citation.Individual;
+import org.opengis.metadata.citation.Responsibility;
+import org.opengis.metadata.acquisition.Instrument;
+import org.opengis.metadata.acquisition.Platform;
+import org.apache.sis.util.iso.SimpleInternationalString;
+import org.apache.sis.metadata.iso.DefaultIdentifier;
+import org.apache.sis.metadata.iso.citation.DefaultCitation;
+import org.apache.sis.metadata.iso.citation.DefaultIndividual;
+import org.apache.sis.metadata.iso.citation.DefaultResponsibility;
+import org.apache.sis.metadata.iso.acquisition.DefaultInstrument;
+import org.apache.sis.metadata.iso.acquisition.DefaultPlatform;
+import org.apache.sis.test.DependsOnMethod;
+import org.apache.sis.test.DependsOn;
+import org.apache.sis.test.TestCase;
+import org.junit.Test;
+
+import static java.util.Collections.singleton;
+import static org.junit.Assert.*;
+
+
+/**
+ * Tests the {@link HashCode} class. This is also used as a relatively simple {@link MetadataVisitor} test.
+ * The entry point is the {@link HashCode#walk(MetadataStandard, Class, Object, boolean)} method.
+ *
+ *
+ * @author  Martin Desruisseaux (Geomatys)
+ * @version 1.0
+ * @since   1.0
+ * @module
+ */
+@DependsOn(PropertyAccessorTest.class)
+public final strictfp class HashCodeTest extends TestCase {
+    /**
+     * Computes the hash code value of the given object.
+     */
+    private static Integer hash(final Object metadata) {
+        return HashCode.getOrCreate().walk(MetadataStandard.ISO_19115, null, metadata, true);
+    }
+
+    /**
+     * Tests hash code computation of an object that do not contain other metadata.
+     */
+    @Test
+    public void testSimple() {
+        final DefaultCitation instance = new DefaultCitation();
+        final int baseCode = Citation.class.hashCode();
+        assertEquals("Empty metadata.", Integer.valueOf(baseCode), hash(instance));
+
+        final InternationalString title = new SimpleInternationalString("Some title");
+        instance.setTitle(title);
+        assertEquals("Metadata with a single value.", Integer.valueOf(baseCode + title.hashCode()), hash(instance));
+
+        final InternationalString alternateTitle = new SimpleInternationalString("An other title");
+        instance.setAlternateTitles(singleton(alternateTitle));
+        assertEquals("Metadata with two values.",
+                     Integer.valueOf(baseCode + title.hashCode() + Arrays.asList(alternateTitle).hashCode()),
+                     hash(instance));
+    }
+
+    /**
+     * Tests hash code computation of an object containing another metadata object.
+     */
+    @Test
+    @DependsOnMethod("testSimple")
+    public void testNested() {
+        final InternationalString   title    = new SimpleInternationalString("Some title");
+        final InternationalString   person   = new SimpleInternationalString("Illustre inconnu");
+        final DefaultIndividual     party    = new DefaultIndividual(person, null, null);
+        final DefaultResponsibility resp     = new DefaultResponsibility(Role.AUTHOR, null, party);
+        final DefaultCitation       instance = new DefaultCitation(title);
+        instance.getCitedResponsibleParties().add(resp);
+        /*
+         * Individual hash code is the sum of all its properties, none of them being a collection.
+         */
+        int expected = Individual.class.hashCode() + person.hashCode();
+        assertEquals("Individual", Integer.valueOf(expected), hash(party));
+        /*
+         * The +31 below come from java.util.List contract, since above Individual is a list member.
+         */
+        expected += Responsibility.class.hashCode() + Role.AUTHOR.hashCode() + 31;
+        assertEquals("Responsibility", Integer.valueOf(expected), hash(resp));
+        /*
+         * The +31 below come from java.util.List contract, since above Responsibility is a list member.
+         */
+        expected += Citation.class.hashCode() + title.hashCode() + 31;
+        assertEquals("Citation", Integer.valueOf(expected), hash(instance));
+    }
+
+    /**
+     * Tests hash code computation of an object graph containing a cycle.
+     */
+    @Test
+    @DependsOnMethod("testNested")
+    public void testCycle() {
+        /*
+         * We will create a Platform and an Instrument, both of them with no other property than an identifier.
+         * The assertions verifying Identifier hash codes are not the main purpose of this test, but we perform
+         * those verifications for making sure that the assertion done at the end of this method has good premises.
+         */
+        final DefaultIdentifier platformID   = new DefaultIdentifier("P1");
+        final DefaultIdentifier instrumentID = new DefaultIdentifier("I1");
+        int platformHash   = Identifier.class.hashCode() +   platformID.getCode().hashCode();
+        int instrumentHash = Identifier.class.hashCode() + instrumentID.getCode().hashCode();
+        assertEquals("platformID",   Integer.valueOf(platformHash),   hash(platformID));
+        assertEquals("instrumentID", Integer.valueOf(instrumentHash), hash(instrumentID));
+        /*
+         * Verify Platform and Instrument hash codes before we link them together.
+         */
+        final DefaultPlatform   platform   = new DefaultPlatform();
+        final DefaultInstrument instrument = new DefaultInstrument();
+        platform  .setIdentifier(platformID);
+        instrument.setIdentifier(instrumentID);
+        platformHash   +=   Platform.class.hashCode();
+        instrumentHash += Instrument.class.hashCode();
+        assertEquals("Platform",   Integer.valueOf(platformHash),   hash(platform));
+        assertEquals("Instrument", Integer.valueOf(instrumentHash), hash(instrument));
+        /*
+         * Add the instrument to the platform. The +31 below come from java.util.List contract,
+         * since the Instrument is contained in a list.
+         */
+        platform.getInstruments().add(instrument);
+        platformHash += instrumentHash + 31;
+        assertEquals("Platform", Integer.valueOf(platformHash), hash(platform));
+        /*
+         * Add a reference from the instrument back to the platform. This is where the graph become cyclic.
+         * The hash code computation is expected to behave as if the platform was not specified.
+         */
+        instrument.setMountedOn(platform);
+        assertEquals("Platform", Integer.valueOf(platformHash), hash(platform));
+    }
+}
diff --git a/core/sis-metadata/src/test/java/org/apache/sis/metadata/PropertyAccessorTest.java b/core/sis-metadata/src/test/java/org/apache/sis/metadata/PropertyAccessorTest.java
index 7198c2e..f2373bc 100644
--- a/core/sis-metadata/src/test/java/org/apache/sis/metadata/PropertyAccessorTest.java
+++ b/core/sis-metadata/src/test/java/org/apache/sis/metadata/PropertyAccessorTest.java
@@ -62,7 +62,6 @@ import org.apache.sis.test.DependsOn;
 import org.apache.sis.test.TestCase;
 import org.junit.Test;
 
-import static java.util.Collections.singleton;
 import static org.apache.sis.test.MetadataAssert.*;
 import static org.apache.sis.test.TestUtilities.getSingleton;
 import static org.apache.sis.metadata.PropertyAccessor.APPEND;
@@ -618,28 +617,6 @@ public final strictfp class PropertyAccessorTest extends TestCase {
     }
 
     /**
-     * Tests {@link PropertyAccessor#hashCode(Object)}.
-     */
-    @Test
-    public void testHashCode() {
-        final DefaultCitation  instance = new DefaultCitation();
-        final PropertyAccessor accessor = createPropertyAccessor();
-        final int              baseCode = Citation.class.hashCode();
-        int hashCode = accessor.hashCode(instance);
-        assertEquals("Empty metadata.", baseCode, hashCode);
-
-        final InternationalString title = new SimpleInternationalString("Some title");
-        instance.setTitle(title);
-        hashCode = accessor.hashCode(instance);
-        assertEquals("Metadata with a single value.", baseCode + title.hashCode(), hashCode);
-
-        final InternationalString alternateTitle = new SimpleInternationalString("An other title");
-        instance.setAlternateTitles(singleton(alternateTitle));
-        hashCode = accessor.hashCode(instance);
-        assertEquals("Metadata with two values.", baseCode + title.hashCode() + Arrays.asList(alternateTitle).hashCode(), hashCode);
-    }
-
-    /**
      * Tests {@link PropertyAccessor#toString()}. The {@code toString()}
      * method is only for debugging purpose, but we test it anyway.
      */
diff --git a/core/sis-metadata/src/test/java/org/apache/sis/metadata/PrunerTest.java b/core/sis-metadata/src/test/java/org/apache/sis/metadata/PrunerTest.java
index 001de86..8c34d86 100644
--- a/core/sis-metadata/src/test/java/org/apache/sis/metadata/PrunerTest.java
+++ b/core/sis-metadata/src/test/java/org/apache/sis/metadata/PrunerTest.java
@@ -38,11 +38,11 @@ import static org.apache.sis.metadata.ValueExistencePolicy.isNullOrEmpty;
  * Tests the {@link AbstractMetadata#isEmpty()} and {@link ModifiableMetadata#prune()} methods.
  *
  * @author  Martin Desruisseaux (Geomatys)
- * @version 0.8
+ * @version 1.0
  * @since   0.3
  * @module
  */
-@DependsOn(ValueMapTest.class)
+@DependsOn(HashCodeTest.class)
 public final strictfp class PrunerTest extends TestCase {
     /**
      * The root metadata object being tested.
diff --git a/core/sis-metadata/src/test/java/org/apache/sis/test/suite/MetadataTestSuite.java b/core/sis-metadata/src/test/java/org/apache/sis/test/suite/MetadataTestSuite.java
index 3d66fb1..766fcf5 100644
--- a/core/sis-metadata/src/test/java/org/apache/sis/test/suite/MetadataTestSuite.java
+++ b/core/sis-metadata/src/test/java/org/apache/sis/test/suite/MetadataTestSuite.java
@@ -50,6 +50,7 @@ import org.junit.BeforeClass;
     org.apache.sis.metadata.TreeTableViewTest.class,
     org.apache.sis.metadata.TreeTableFormatTest.class,
     org.apache.sis.metadata.MetadataStandardTest.class,
+    org.apache.sis.metadata.HashCodeTest.class,
     org.apache.sis.metadata.PrunerTest.class,
     org.apache.sis.metadata.AbstractMetadataTest.class,
     org.apache.sis.metadata.MetadataCopierTest.class,


Mime
View raw message