From commits-return-13838-apmail-sis-commits-archive=sis.apache.org@sis.apache.org Wed Aug 19 10:36:17 2020 Return-Path: X-Original-To: apmail-sis-commits-archive@www.apache.org Delivered-To: apmail-sis-commits-archive@www.apache.org Received: from mailroute1-lw-us.apache.org (mailroute1-lw-us.apache.org [207.244.88.153]) by minotaur.apache.org (Postfix) with ESMTP id 7A1FF1A0D8 for ; Wed, 19 Aug 2020 10:36:17 +0000 (UTC) Received: from mail.apache.org (localhost [127.0.0.1]) by mailroute1-lw-us.apache.org (ASF Mail Server at mailroute1-lw-us.apache.org) with SMTP id 08CB11250E0 for ; Wed, 19 Aug 2020 10:36:17 +0000 (UTC) Received: (qmail 30323 invoked by uid 500); 19 Aug 2020 10:36:16 -0000 Delivered-To: apmail-sis-commits-archive@sis.apache.org Received: (qmail 30284 invoked by uid 500); 19 Aug 2020 10:36:16 -0000 Mailing-List: contact commits-help@sis.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: sis-dev@sis.apache.org Delivered-To: mailing list commits@sis.apache.org Received: (qmail 30199 invoked by uid 99); 19 Aug 2020 10:36:16 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 19 Aug 2020 10:36:16 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id 73D29890BB; Wed, 19 Aug 2020 10:36:16 +0000 (UTC) Date: Wed, 19 Aug 2020 10:36:19 +0000 To: "commits@sis.apache.org" Subject: =?utf-8?q?=5Bsis=5D_03/03=3A_Add_a_ModifiableMetadata=2EdeepCopy?= =?utf-8?b?KOKApikgbWV0aG9kLg==?= MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit From: desruisseaux@apache.org In-Reply-To: <159783337632.5116.15046673587150724110@gitbox.apache.org> References: <159783337632.5116.15046673587150724110@gitbox.apache.org> X-Git-Host: gitbox.apache.org X-Git-Repo: sis X-Git-Refname: refs/heads/geoapi-4.0 X-Git-Reftype: branch X-Git-Rev: e6cffdba3b1c545d0fb3fccf7b6ff3831ebe238f X-Git-NotificationType: diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated Message-Id: <20200819103616.73D29890BB@gitbox.apache.org> 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 e6cffdba3b1c545d0fb3fccf7b6ff3831ebe238f Author: Martin Desruisseaux AuthorDate: Tue Aug 18 16:05:22 2020 +0200 Add a ModifiableMetadata.deepCopy(…) method. --- .../org/apache/sis/metadata/MetadataCopier.java | 2 +- .../org/apache/sis/metadata/MetadataVisitor.java | 35 ++++++++--- .../apache/sis/metadata/ModifiableMetadata.java | 68 +++++++++++++++++++++- .../java/org/apache/sis/metadata/StateChanger.java | 27 +++++---- .../sis/metadata/ModifiableMetadataTest.java | 42 ++++++++++++- 5 files changed, 153 insertions(+), 21 deletions(-) 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 1e34396..33cda77 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 @@ -114,7 +114,7 @@ public class MetadataCopier extends MetadataVisitor { @Override protected Object copyRecursively(final Class type, final Object metadata) { if (metadata instanceof ModifiableMetadata) { final ModifiableMetadata.State state = ((ModifiableMetadata) metadata).state(); - if (state == ModifiableMetadata.State.FINAL) { + if (state != null && state.isUnmodifiable()) { return metadata; } } 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 index 80859d2..d69b274 100644 --- 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 @@ -51,8 +51,8 @@ abstract class MetadataVisitor { /** * 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. + * Keys are visited metadata instances and values are computed value. + * The value may be null if the computation is in progress. * *

The problem

* Cyclic associations can exist in ISO 19115 metadata. For example {@code Instrument} has a reference @@ -76,6 +76,8 @@ abstract class MetadataVisitor { /** * 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. + * + * @see #creator() */ private int nestedCount; @@ -101,6 +103,9 @@ abstract class MetadataVisitor { * The thread-local variable that created this {@code MetadataVisitor} instance. * This is usually a static final {@code VISITORS} constant defined in the subclass. * May be {@code null} if this visitor does not use thread-local instances. + * + *

If this method returns a non-null value, then {@link ThreadLocal#remove()} will be invoked + * after {@link MetadataVisitor} finished to walk through a metadata and all its children.

*/ ThreadLocal> creator() { return null; @@ -133,8 +138,7 @@ abstract class MetadataVisitor { * already been visited. The computation result is returned (may be the result of a previous computation). * *

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.

+ * It maintains a map of visited nodes for preventing the same node to be visited twice.

* * @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. @@ -148,9 +152,9 @@ abstract class MetadataVisitor { 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) { - final Filter filter = preVisit(accessor); - final boolean preconstructed; - final R sentinel; + final Filter filter = preVisit(accessor); // NONE, NON_EMPTY, WRITABLE or WRITABLE_RESULT. + final boolean preconstructed; // Whether to write in instance provided by `result()`. + final R sentinel; // Value in the map for telling that visit started. switch (filter) { case NONE: return null; case WRITABLE_RESULT: preconstructed = true; sentinel = result(); break; @@ -160,6 +164,11 @@ abstract class MetadataVisitor { // Should never happen, unless this method is invoked concurrently in another thread. throw new ConcurrentModificationException(); } + /* + * Name of properties walked from root node to the node being visited by current method invocation. + * The path is provided by `propertyPath` and the number of valid elements is given by `nestedCount`, + * which is 1 during the visit of first element. + */ if (nestedCount >= propertyPath.length) { propertyPath = Arrays.copyOf(propertyPath, nestedCount * 2); } @@ -173,6 +182,10 @@ abstract class MetadataVisitor { */ allowNull = Semaphores.queryAndSet(Semaphores.NULL_COLLECTION); } + /* + * Actual visiting. The `accessor.walk(this, metadata)` method calls below will callback the abstract + * `visit(type, value)` method declared in this class. + */ try { switch (filter) { case NON_EMPTY: accessor.walkReadable(this, metadata); break; @@ -186,6 +199,10 @@ abstract class MetadataVisitor { throw new MetadataVisitorException(Arrays.copyOf(propertyPath, nestedCount), accessor.type, e); } finally { if (--nestedCount == 0) { + /* + * We are back to the root metadata (i.e. we finished walking through all children). + * Clear thread local variables, which should restore them to their initial value. + */ if (!allowNull) { Semaphores.clear(Semaphores.NULL_COLLECTION); } @@ -193,6 +210,10 @@ abstract class MetadataVisitor { if (creator != null) creator.remove(); } } + /* + * Cache the result in case this node is visited again (e.g. if the metadata graph contains + * cycles or if the same child node is referenced from many places). + */ final R result = preconstructed ? sentinel : result(); if (visited.put(metadata, result) != sentinel) { throw new ConcurrentModificationException(); diff --git a/core/sis-metadata/src/main/java/org/apache/sis/metadata/ModifiableMetadata.java b/core/sis-metadata/src/main/java/org/apache/sis/metadata/ModifiableMetadata.java index 1c06835..40a0add 100644 --- a/core/sis-metadata/src/main/java/org/apache/sis/metadata/ModifiableMetadata.java +++ b/core/sis-metadata/src/main/java/org/apache/sis/metadata/ModifiableMetadata.java @@ -86,7 +86,7 @@ import static org.apache.sis.internal.metadata.MetadataUtilities.valueIfDefined; * } * * @author Martin Desruisseaux (Geomatys) - * @version 1.0 + * @version 1.1 * @since 0.3 * @module */ @@ -203,6 +203,13 @@ public abstract class ModifiableMetadata extends AbstractMetadata { private State(final byte code) { this.code = code; } + + /** + * Whether this enumeration represents a state where data can not be modified anymore. + */ + final boolean isUnmodifiable() { + return code >= ModifiableMetadata.FINAL; + } } /** @@ -261,7 +268,7 @@ public abstract class ModifiableMetadata extends AbstractMetadata { * The effect of invoking this method may be recursive. For example transitioning to {@link State#FINAL} * implies transitioning all children {@code ModifiableMetadata} instances to the final state too. * - * @param target the desired new state. + * @param target the desired new state (editable, completable or final). * @return {@code true} if the state of this {@code ModifiableMetadata} changed as a result of this method call. * @throws UnmodifiableMetadataException if a transition to a less restrictive state * (e.g. from {@link State#FINAL} to {@link State#EDITABLE}) was attempted. @@ -287,6 +294,63 @@ public abstract class ModifiableMetadata extends AbstractMetadata { } /** + * Copies (if necessary) this metadata and all its children. + * Changes in the returned metadata will not affect this {@code ModifiableMetadata} instance, and conversely. + * The returned metadata will be in the {@linkplain #state() state} specified by the {@code target} argument. + * The state of this {@code ModifiableMetadata} instance stay unchanged. + * + *

As a special case, this method returns {@code this} if and only if the specified target is {@link State#FINAL} + * and this {@code ModifiableMetadata} instance is already in final state. In that particular case, copies are not + * needed for protecting metadata against changes because neither {@code this} or the returned value can be modified.

+ * + *

This method is typically invoked for getting a modifiable metadata from an unmodifiable one:

+ * + * {@preformat java + * Metadata source = ...; // Any implementation. + * DefaultMetadata md = DefaultMetadata.castOrCopy(source); + * md = (DefaultMetadata) md.deepCopy(DefaultMetadata.State.EDITABLE); + * } + * + *

Alternative

+ * If unconditional copy is desired, or if the metadata to copy may be arbitrary implementations + * of GeoAPI interfaces (i.e. not necessarily a {@code ModifiableMetadata} subclass), + * then the following code can be used instead: + * + * {@preformat java + * MetadataCopier copier = new MetadataCopier(MetadataStandard.ISO_19115); + * Metadata source = ...; // Any implementation. + * Metadata copy = copier.copy(Metadata.class, source); + * } + * + * The {@code Metadata} type in above example can be replaced by any other ISO 19115 type. + * Types from other standards can also be used if the {@link MetadataStandard#ISO_19115} constant + * is replaced accordingly. + * + * @param target the desired state (editable, completable or final). + * @return a copy (except in above-cited special case) of this metadata in the specified state. + * + * @see MetadataCopier + * + * @since 1.1 + */ + public ModifiableMetadata deepCopy(final State target) { + final MetadataCopier copier; + if (target.isUnmodifiable()) { + if (state >= FINAL) { + return this; + } + copier = MetadataCopier.forModifiable(getStandard()); + } else { + copier = new MetadataCopier(getStandard()); + } + final ModifiableMetadata md = (ModifiableMetadata) copier.copyRecursively(getInterface(), this); + if (target.code > EDITABLE) { + md.transitionTo(target); + } + return md; + } + + /** * Checks if changes in the metadata are allowed. All {@code setFoo(…)} methods in subclasses * shall invoke this method (directly or indirectly) before to apply any change. * The current property value should be specified in argument. diff --git a/core/sis-metadata/src/main/java/org/apache/sis/metadata/StateChanger.java b/core/sis-metadata/src/main/java/org/apache/sis/metadata/StateChanger.java index f9305cf..15b462c 100644 --- a/core/sis-metadata/src/main/java/org/apache/sis/metadata/StateChanger.java +++ b/core/sis-metadata/src/main/java/org/apache/sis/metadata/StateChanger.java @@ -32,9 +32,6 @@ import org.apache.sis.metadata.iso.identification.DefaultRepresentativeFraction; /** * Invokes {@link ModifiableMetadata#transitionTo(ModifiableMetadata.State)} recursively on metadata elements. * - * As of Apache SIS 1.0, this class is used only for {@link ModifiableMetadata.State#FINAL}. - * But a future version may use this object for other states too. - * * @author Martin Desruisseaux (Geomatys) * @version 1.0 * @since 0.3 @@ -43,9 +40,9 @@ import org.apache.sis.metadata.iso.identification.DefaultRepresentativeFraction; final class StateChanger extends MetadataVisitor { /** * The {@code StateChanger} instance in current use. The clean way would have been to pass - * the instance in argument to all {@code apply(State.FINAL)} methods in metadata packages. - * But above-cited methods are public, and we do not want to expose {@code StateChanger} - * in public API. This thread-local is a workaround for that situation. + * the instance in argument to {@link ModifiableMetadata#transitionTo(ModifiableMetadata.State)}. + * But above-cited method ix public and we do not want to expose {@code StateChanger} in public API. + * This thread-local is a workaround for that situation. */ private static final ThreadLocal VISITORS = ThreadLocal.withInitial(StateChanger::new); @@ -66,7 +63,13 @@ final class StateChanger extends MetadataVisitor { } /** - * Applies a state change on the given metadata object. + * Applies a state change on the given metadata object. This is the implementation + * {@link ModifiableMetadata#transitionTo(ModifiableMetadata.State)} public method. + * + *

This is conceptually an instance (non-static) method. But the {@code this} value is not known + * by the caller, because doing otherwise would force us to give public visibility to classes that we + * want to keep package-private. The {@link #VISITORS} thread local variable is used as a workaround + * for providing {@code this} instance without making {@code StateChanger} public.

*/ static void applyTo(final ModifiableMetadata.State target, final ModifiableMetadata metadata) { final StateChanger changer = VISITORS.get(); @@ -78,6 +81,8 @@ final class StateChanger extends MetadataVisitor { /** * Returns the thread-local variable that created this {@code StateChanger} instance. + * {@link ThreadLocal#remove()} will be invoked after {@link MetadataVisitor} finished + * to walk through the given metadata and all its children. */ @Override final ThreadLocal creator() { @@ -108,7 +113,7 @@ final class StateChanger extends MetadataVisitor { } /** - * Recursively change the state of all elements in the given array. + * Recursively changes the state of all elements in the given array. */ private void applyToAll(final Object[] array) throws CloneNotSupportedException { for (int i=0; i < array.length; i++) { @@ -144,8 +149,10 @@ final class StateChanger extends MetadataVisitor { return object; } if (object instanceof DefaultRepresentativeFraction) { - ((DefaultRepresentativeFraction) object).freeze(); - return object; + if (target.isUnmodifiable()) { + ((DefaultRepresentativeFraction) object).freeze(); + return object; + } } /* * CASE 2 - The object is a collection. All elements are replaced by their diff --git a/core/sis-metadata/src/test/java/org/apache/sis/metadata/ModifiableMetadataTest.java b/core/sis-metadata/src/test/java/org/apache/sis/metadata/ModifiableMetadataTest.java index f40095b..864fad2 100644 --- a/core/sis-metadata/src/test/java/org/apache/sis/metadata/ModifiableMetadataTest.java +++ b/core/sis-metadata/src/test/java/org/apache/sis/metadata/ModifiableMetadataTest.java @@ -35,7 +35,7 @@ import static org.apache.sis.test.Assert.*; * This class uses {@link DefaultMedium} as an arbitrary metadata implementation for running the tests. * * @author Martin Desruisseaux (Geomatys) - * @version 1.0 + * @version 1.1 * @since 1.0 * @module */ @@ -185,4 +185,44 @@ public final strictfp class ModifiableMetadataTest extends TestCase { } assertPropertiesEqual(null, "The original note."); } + + /** + * Tests {@link ModifiableMetadata#deepCopy(ModifiableMetadata.State)}. + */ + @Test + public void testDeepCopy() { + /* + * Make one of `DefaultMedium` property unmodifiable + * for testing `deepCopy(…)` decision to copy or not. + */ + assertTrue(((DefaultIdentifier) md.getIdentifier()).transitionTo(ModifiableMetadata.State.FINAL)); + /* + * Test the request for an editable copy. All children + * should be copied, including the unmodifiable child. + */ + DefaultMedium copy = (DefaultMedium) md.deepCopy(ModifiableMetadata.State.EDITABLE); + assertEquals (md, copy); + assertNotSame(md, copy); + assertNotSame(md.getIdentifier(), copy.getIdentifier()); + assertSame (md.getMediumNote(), copy.getMediumNote()); // Not a cloneable property. + assertEquals(ModifiableMetadata.State.FINAL, identifierState()); + assertEquals(ModifiableMetadata.State.EDITABLE, ((DefaultIdentifier) copy.getIdentifier()).state()); + /* + * Test the request for an unmodifiable copy. This time, + * the unmodifiable children should not be copied. + */ + copy = (DefaultMedium) md.deepCopy(ModifiableMetadata.State.FINAL); + assertEquals (md, copy); + assertNotSame(md, copy); + assertSame (md.getIdentifier(), copy.getIdentifier()); // Not copied because unmodifiable. + assertSame (md.getMediumNote(), copy.getMediumNote()); // Not a cloneable property. + assertEquals(ModifiableMetadata.State.FINAL, identifierState()); + assertEquals(ModifiableMetadata.State.FINAL, ((DefaultIdentifier) copy.getIdentifier()).state()); + /* + * Special case when all metadata at play are unmodifiable. + */ + md.transitionTo(ModifiableMetadata.State.FINAL); + copy = (DefaultMedium) md.deepCopy(ModifiableMetadata.State.FINAL); + assertSame(md, copy); + } }