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