sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject [sis] branch master updated: Fix `NoSuchMethodException` when cloning an array and `NullPointerException` when writing legacy metadata properties. This is a side-effect of work done in ASF-OGC-OSGeo join code sprint.
Date Sun, 21 Feb 2021 21:50:45 GMT
This is an automated email from the ASF dual-hosted git repository.

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


The following commit(s) were added to refs/heads/master by this push:
     new 5dbfe58  Fix `NoSuchMethodException` when cloning an array and `NullPointerException`
when writing legacy metadata properties. This is a side-effect of work done in ASF-OGC-OSGeo
join code sprint.
5dbfe58 is described below

commit 5dbfe58a0fb7a804354db6c410280eb0a84288a1
Author: Martin Desruisseaux <martin.desruisseaux@geomatys.com>
AuthorDate: Sun Feb 21 22:50:14 2021 +0100

    Fix `NoSuchMethodException` when cloning an array and `NullPointerException` when writing
legacy metadata properties.
    This is a side-effect of work done in ASF-OGC-OSGeo join code sprint.
---
 .../apache/sis/metadata/iso/DefaultMetadata.java   | 29 +++++++--
 .../java/org/apache/sis/internal/util/Cloner.java  | 76 +++++++++++++++++++---
 2 files changed, 91 insertions(+), 14 deletions(-)

diff --git a/core/sis-metadata/src/main/java/org/apache/sis/metadata/iso/DefaultMetadata.java
b/core/sis-metadata/src/main/java/org/apache/sis/metadata/iso/DefaultMetadata.java
index d37f1ea..db4f24b 100644
--- a/core/sis-metadata/src/main/java/org/apache/sis/metadata/iso/DefaultMetadata.java
+++ b/core/sis-metadata/src/main/java/org/apache/sis/metadata/iso/DefaultMetadata.java
@@ -21,6 +21,7 @@ import java.util.Locale;
 import java.util.Set;
 import java.util.EnumSet;
 import java.util.Map;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -648,7 +649,16 @@ public class DefaultMetadata extends ISOMetadata implements Metadata
{
      */
     @Deprecated
     public void setLocales(final Collection<? extends Locale> newValues) {
-        getLocales().addAll(newValues);
+        final Collection<Locale> legacy = getLocales();
+        if (legacy != null) {
+            legacy.addAll(newValues);
+        } else if (!newValues.isEmpty()) {
+            final Map<Locale,Charset> locales = new LinkedHashMap<>();
+            for (final Locale locale : newValues) {
+                locales.put(locale, null);
+            }
+            setLocalesAndCharsets(locales);
+        }
     }
 
     /**
@@ -798,11 +808,15 @@ public class DefaultMetadata extends ISOMetadata implements Metadata
{
         checkWritePermission(parentMetadata);
         // See "Note about deprecated methods implementation"
         DefaultCitation parent = DefaultCitation.castOrCopy(parentMetadata);
-        if (parent == null) {
-            parent = new DefaultCitation();
+        if (newValue != null) {
+            if (parent == null) {
+                parent = new DefaultCitation();
+            }
+            parent.setTitle(new SimpleInternationalString(newValue));
+            setParentMetadata(parent);
+        } else if (parent != null) {
+            parent.setTitle(null);
         }
-        parent.setTitle(new SimpleInternationalString(newValue));
-        setParentMetadata(parent);
     }
 
     /**
@@ -1310,20 +1324,23 @@ public class DefaultMetadata extends ISOMetadata implements Metadata
{
      */
     @Deprecated
     public void setDataSetUri(final String newValue) throws URISyntaxException {
-        final URI uri = new URI(newValue);
+        final URI uri = (newValue != null) ? new URI(newValue) : null;
         Collection<Identification> info = identificationInfo;   // See "Note about
deprecated methods implementation"
         checkWritePermission(MetadataUtilities.valueIfDefined(info));
         AbstractIdentification firstId = AbstractIdentification.castOrCopy(CollectionsExt.first(info));
         if (firstId == null) {
+            if (uri == null) return;
             firstId = new DefaultDataIdentification();
         }
         DefaultCitation citation = DefaultCitation.castOrCopy(firstId.getCitation());
         if (citation == null) {
+            if (uri == null) return;
             citation = new DefaultCitation();
         }
         Collection<OnlineResource> onlineResources = citation.getOnlineResources();
         DefaultOnlineResource firstOnline = DefaultOnlineResource.castOrCopy(CollectionsExt.first(onlineResources));
         if (firstOnline == null) {
+            if (uri == null) return;
             firstOnline = new DefaultOnlineResource();
         }
         firstOnline.setLinkage(uri);
diff --git a/core/sis-utility/src/main/java/org/apache/sis/internal/util/Cloner.java b/core/sis-utility/src/main/java/org/apache/sis/internal/util/Cloner.java
index 878d2ec..df2836b 100644
--- a/core/sis-utility/src/main/java/org/apache/sis/internal/util/Cloner.java
+++ b/core/sis-utility/src/main/java/org/apache/sis/internal/util/Cloner.java
@@ -16,6 +16,9 @@
  */
 package org.apache.sis.internal.util;
 
+import java.util.IdentityHashMap;
+import java.util.ConcurrentModificationException;
+import java.lang.reflect.Array;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.lang.reflect.InvocationTargetException;
@@ -28,7 +31,7 @@ import org.apache.sis.util.resources.Errors;
  * for the lack of public {@code clone()} method in the {@link Cloneable} interface.
  *
  * @author  Martin Desruisseaux (Geomatys)
- * @version 1.0
+ * @version 1.1
  * @since   0.3
  * @module
  */
@@ -54,10 +57,17 @@ public final class Cloner {
     private final boolean isCloneRequired;
 
     /**
+     * Results of cloning instances previously meet during this {@code Cloner} lifetime.
+     * This is used for preserving reference graph, and also as a safety against infinite
recursivity.
+     * Keys must be compared using identity comparison, not {@link Object#equals(Object)}.
+     */
+    private final IdentityHashMap<Object,Object> cloneResults;
+
+    /**
      * Creates a new {@code Cloner} instance which requires public {@code clone()} method
to be present.
      */
     public Cloner() {
-        isCloneRequired = true;
+        this(true);
     }
 
     /**
@@ -68,6 +78,32 @@ public final class Cloner {
      */
     public Cloner(final boolean isCloneRequired) {
         this.isCloneRequired = isCloneRequired;
+        cloneResults = new IdentityHashMap<>();
+    }
+
+    /**
+     * Clones the given array, then clone all array elements recursively.
+     *
+     * @param  array          the array to clone.
+     * @param  componentType  value of {@code array.getClass().getComponentType()}.
+     * @return the cloned array, potentially with recursively cloned elements.
+     * @throws CloneNotSupportedException if an array element can not be cloned.
+     */
+    @SuppressWarnings("SuspiciousSystemArraycopy")
+    private Object cloneArray(final Object array, final Class<?> componentType) throws
CloneNotSupportedException {
+        final int length = Array.getLength(array);
+        final Object copy = Array.newInstance(componentType, length);
+        if (cloneResults.put(array, copy) != null) {                    // Must be done before
to clone recursively.
+            throw new ConcurrentModificationException();                // Should never happen
unless we have a bug.
+        }
+        if (componentType.isPrimitive()) {
+            System.arraycopy(array, 0, copy, 0, length);
+        } else {
+            for (int i=0; i<length; i++) {
+                Array.set(copy, i, clone(Array.get(array, i)));
+            }
+        }
+        return copy;
     }
 
     /**
@@ -88,14 +124,23 @@ public final class Cloner {
         if (object == null) {
             return null;
         }
-        SecurityException security = null;
+        Object result = cloneResults.get(object);
+        if (result != null) {
+            return result;
+        }
         final Class<?> valueType = object.getClass();
+        final Class<?> componentType = valueType.getComponentType();
+        if (componentType != null) {
+            return cloneArray(object, componentType);
+        }
+        SecurityException security = null;
+        result = object;
         try {
             if (valueType != type) {
                 method = valueType.getMethod("clone", (Class<?>[]) null);
                 type = valueType;                                           // Set only if
the above line succeed.
                 /*
-                 * If the class implementing the 'clone()' method is not public, we may not
be able to access that
+                 * If the class implementing the `clone()` method is not public, we may not
be able to access that
                  * method even if it is public. Try to make the method accessible. If we
fail for security reason,
                  * we will still attempt to clone (maybe a parent class is public), but we
remember the exception
                  * in order to report it in case of failure.
@@ -108,11 +153,11 @@ public final class Cloner {
                 }
             }
             /*
-             * 'method' may be null if a previous call to this clone(Object) method threw
NoSuchMethodException
-             * (see the first 'catch' block below). In this context, 'null' means "no public
clone() method".
+             * `method` may be null if a previous call to this clone(Object) method threw
NoSuchMethodException
+             * (see the first `catch` block below). In this context, `null` means "no public
clone() method".
              */
             if (method != null) {
-                return method.invoke(object, (Object[]) null);
+                result = method.invoke(object, (Object[]) null);
             }
         } catch (NoSuchMethodException e) {
             if (isCloneRequired) {
@@ -131,7 +176,11 @@ public final class Cloner {
         } catch (SecurityException e) {
             throw fail(e, valueType);
         }
-        return object;
+        if (cloneResults.put(object, result) != null) {
+            // Should never happen unless we have a bug.
+            throw new ConcurrentModificationException();
+        }
+        return result;
     }
 
     /**
@@ -177,9 +226,20 @@ public final class Cloner {
      *
      * @since 0.6
      */
+    @SuppressWarnings("SuspiciousSystemArraycopy")
     public static Object cloneIfPublic(final Object object) throws CloneNotSupportedException
{
         if (object != null) {
             final Class<?> type = object.getClass();
+            final Class<?> componentType = type.getComponentType();
+            if (componentType != null) {
+                if (componentType.isPrimitive()) {
+                    final int length = Array.getLength(object);
+                    final Object copy = Array.newInstance(componentType, length);
+                    System.arraycopy(object, 0, copy, 0, length);
+                    return copy;
+                }
+                return new Cloner().cloneArray(object, componentType);
+            }
             try {
                 final Method m = type.getMethod("clone", (Class[]) null);
                 if (Modifier.isPublic(m.getModifiers())) {


Mime
View raw message