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 <[email protected]> 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,
