sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject [sis] branch geoapi-4.0 updated: Remove (for now) the attempt to use unique metadata instances on invocation of ModifiableMetadata.apply(State.FINAL). The current attempt was incomplete. The MetadataVisitor now available should make easier to make a new attempt later. This may happen in the context of the GeoTIFF and netCDF readers development (consolidation of GeoTIFF and netCDF metadata was the trigger for this MetadataVisitor effort).
Date Mon, 02 Jul 2018 17:19:06 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


The following commit(s) were added to refs/heads/geoapi-4.0 by this push:
     new 420564d  Remove (for now) the attempt to use unique metadata instances on invocation
of ModifiableMetadata.apply(State.FINAL). The current attempt was incomplete. The MetadataVisitor
now available should make easier to make a new attempt later. This may happen in the context
of the GeoTIFF and netCDF readers development (consolidation of GeoTIFF and netCDF metadata
was the trigger for this MetadataVisitor effort).
420564d is described below

commit 420564d81320b746053a67f352b276bd1bbec31e
Author: Martin Desruisseaux <martin.desruisseaux@geomatys.com>
AuthorDate: Mon Jul 2 19:16:16 2018 +0200

    Remove (for now) the attempt to use unique metadata instances on invocation of ModifiableMetadata.apply(State.FINAL).
    The current attempt was incomplete. The MetadataVisitor now available should make easier
to make a new attempt later.
    This may happen in the context of the GeoTIFF and netCDF readers development
    (consolidation of GeoTIFF and netCDF metadata was the trigger for this MetadataVisitor
effort).
---
 .../org/apache/sis/metadata/MetadataCopier.java    |  3 +-
 .../org/apache/sis/metadata/MetadataVisitor.java   | 21 +++++--
 .../java/org/apache/sis/metadata/StateChanger.java | 66 ++++++++--------------
 3 files changed, 40 insertions(+), 50 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 329a6e9..b0ecffc 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
@@ -197,7 +197,8 @@ public class MetadataCopier extends MetadataVisitor<Object> {
 
     /**
      * Returns the metadata instance resulting from the copy. This method is invoked <strong>before</strong>
-     * metadata properties are visited.
+     * metadata properties are visited. The returned value is a new, initially empty, metadata
instance
+     * created by {@link #preVisit(PropertyAccessor)}.
      */
     @Override
     final Object result() {
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 b64e160..d3423a7 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
@@ -266,12 +266,21 @@ abstract class MetadataVisitor<R> {
 
     /**
      * Returns the result of visiting all elements in a metadata instance.
-     * This method is invoked exactly once per metadata instance.
-     * It is usually invoked after all metadata properties have been visited
-     * (or after a {@link #visit(Class, Object)} method call returned {@link #SKIP_SIBLINGS}),
-     * unless {@link #preVisit(PropertyAccessor)} returned {@link Filter#WRITABLE_RESULT}
-     * in which case this method is invoked <strong>before</strong> metadata
properties are visited.
+     * This method is invoked zero or one time per metadata instance.
+     * The invocation time depends on the value returned by {@link #preVisit(PropertyAccessor)}:
+     *
+     * <ul>
+     *   <li>If {@link Filter#NONE}, then this method is never invoked for the current
metadata instance.</li>
+     *   <li>If {@link Filter#NON_EMPTY} or {@link Filter#WRITABLE}, then this method
is invoked after all properties
+     *       have been visited or after {@link #visit(Class, Object)} returned {@link #SKIP_SIBLINGS}.</li>
+     *   <li>If {@link Filter#WRITABLE_RESULT}, then this method is invoked <strong>before</strong>
metadata
+     *       properties are visited. In such case, this method should return an initially
empty instance.</li>
+     * </ul>
+     *
      * The value returned by this method will be cached in case the same metadata instance
is revisited again.
+     * This value can be {@code null}.
      */
-    abstract R result();
+    R result() {
+        return null;
+    }
 }
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 8c25e3a..da08937 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
@@ -21,7 +21,6 @@ import java.util.Set;
 import java.util.EnumSet;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.LinkedHashMap;
 import org.apache.sis.internal.util.Cloner;
 import org.apache.sis.util.collection.CodeListSet;
@@ -56,12 +55,6 @@ final class StateChanger extends MetadataVisitor<Boolean> {
     private ModifiableMetadata.State target;
 
     /**
-     * All objects made immutable during iteration over children properties.
-     * Keys and values are the same instances. This is used for sharing unique instances
when possible.
-     */
-    private final Map<Object,Object> existings;
-
-    /**
      * The cloner, created when first needed.
      */
     private Cloner cloner;
@@ -70,7 +63,6 @@ final class StateChanger extends MetadataVisitor<Boolean> {
      * Creates a new {@code StateChanger} instance.
      */
     private StateChanger() {
-        existings = new HashMap<>(32);
     }
 
     /**
@@ -104,29 +96,28 @@ final class StateChanger extends MetadataVisitor<Boolean> {
     }
 
     /**
-     * Returns a unique instance of the given object (metadata or value).
+     * Invoked for metadata instances on which to apply a change of state.
+     *
+     * @param  type    ignored (can be {@code null}).
+     * @param  object  the object to transition to a different state.
+     * @return the given object or a copy of the given object with its state changed.
      */
-    private Object unique(final Object object) {
-        if (object != null) {
-            final Object c = existings.putIfAbsent(object, object);
-            if (c != null) {
-                return c;
-            }
-        }
-        return object;
+    @Override
+    final Object visit(final Class<?> type, final Object object) throws CloneNotSupportedException
{
+        return applyTo(object);
     }
 
     /**
      * Recursively change the state of all elements in the given array.
      */
-    private void applyTo(final Object[] array) throws CloneNotSupportedException {
+    private void applyToAll(final Object[] array) throws CloneNotSupportedException {
         for (int i=0; i < array.length; i++) {
-            array[i] = visit(null, array[i]);
+            array[i] = applyTo(array[i]);
         }
     }
 
     /**
-     * Returns an unmodifiable copy of the specified object.
+     * Returns the given object, or a copy of the given object, with its state changed.
      * This method performs the following heuristic tests:
      *
      * <ul>
@@ -140,26 +131,24 @@ final class StateChanger extends MetadataVisitor<Boolean> {
      *   <li>Otherwise, the object is assumed immutable and returned unchanged.</li>
      * </ul>
      *
-     * @param  type    ignored (can be {@code null}).
-     * @param  object  the object to convert in an immutable one.
-     * @return a presumed immutable view of the specified object.
+     * @param  object  the object to transition to a different state.
+     * @return the given object or a copy of the given object with its state changed.
      */
-    @Override
-    final Object visit(final Class<?> type, final Object object) throws CloneNotSupportedException
{
+    private Object applyTo(final Object object) throws CloneNotSupportedException {
         /*
          * CASE 1 - The object is an org.apache.sis.metadata.* implementation.
-         *          It may have its own algorithm for freezing itself.
+         *          It may have its own algorithm for changing its state.
          */
         if (object instanceof ModifiableMetadata) {
             ((ModifiableMetadata) object).apply(target);
-            return unique(object);
+            return object;
         }
         if (target != ModifiableMetadata.State.FINAL) {
             return object;
         }
         if (object instanceof DefaultRepresentativeFraction) {
             ((DefaultRepresentativeFraction) object).freeze();
-            return unique(object);
+            return object;
         }
         /*
          * CASE 2 - The object is a collection. All elements are replaced by their
@@ -177,7 +166,7 @@ final class StateChanger extends MetadataVisitor<Boolean> {
                     break;
                 }
                 case 1: {
-                    final Object value = visit(null, array[0]);
+                    final Object value = applyTo(array[0]);
                     collection = isSet ? Collections.singleton(value)
                                        : Collections.singletonList(value);
                     break;
@@ -189,7 +178,7 @@ final class StateChanger extends MetadataVisitor<Boolean> {
                         } else if (collection instanceof CodeListSet<?>) {
                             collection = Collections.unmodifiableSet(((CodeListSet<?>)
collection).clone());
                         } else {
-                            applyTo(array);
+                            applyToAll(array);
                             collection = CollectionsExt.immutableSet(false, array);
                         }
                     } else {
@@ -198,7 +187,7 @@ final class StateChanger extends MetadataVisitor<Boolean> {
                          * Conservatively assumes a List if we are not sure to have a Set
since the list
                          * is less destructive (no removal of duplicated values).
                          */
-                        applyTo(array);
+                        applyToAll(array);
                         collection = UnmodifiableArrayList.wrap(array);
                     }
                     break;
@@ -213,7 +202,7 @@ final class StateChanger extends MetadataVisitor<Boolean> {
         if (object instanceof Map<?,?>) {
             final Map<Object,Object> map = new LinkedHashMap<>((Map<?,?>)
object);
             for (final Map.Entry<Object,Object> entry : map.entrySet()) {
-                entry.setValue(visit(null, entry.getValue()));
+                entry.setValue(applyTo(entry.getValue()));
             }
             return CollectionsExt.unmodifiableOrCopy(map);
         }
@@ -224,20 +213,11 @@ final class StateChanger extends MetadataVisitor<Boolean> {
             if (cloner == null) {
                 cloner = new Cloner(false);
             }
-            return unique(cloner.clone(object));
+            return cloner.clone(object);
         }
         /*
          * CASE 5 - Any other case. The object is assumed immutable and returned unchanged.
          */
-        return unique(object);
-    }
-
-    /**
-     * Returns an arbitrary value used by {@link MetadataVisitor} for remembering that
-     * a metadata instance has been processed.
-     */
-    @Override
-    Boolean result() {
-        return Boolean.TRUE;
+        return object;
     }
 }


Mime
View raw message