sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject [sis] 02/03: Change approach regarding MetadataCopier: throws IllegalArgumentException if the user specify an implementation class instead than interface. The previous approach was to fix automagically the user argument, but it would not work for some cases like implementations backed by java.lang.reflect.Proxy. We are better to let users know soon that they should specify an interface.
Date Mon, 09 Sep 2019 12:43:51 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

commit 59b2a4b18cca7567c8000f3c4ed25c1f36082bfb
Author: Martin Desruisseaux <martin.desruisseaux@geomatys.com>
AuthorDate: Mon Sep 9 11:39:25 2019 +0200

    Change approach regarding MetadataCopier: throws IllegalArgumentException if the user
specify an implementation class instead than interface.
    The previous approach was to fix automagically the user argument, but it would not work
for some cases like implementations backed by java.lang.reflect.Proxy.
    We are better to let users know soon that they should specify an interface.
---
 .../apache/sis/internal/metadata/Resources.java    | 22 +++++++++++++++
 .../sis/internal/metadata/Resources.properties     |  1 +
 .../sis/internal/metadata/Resources_fr.properties  |  1 +
 .../org/apache/sis/metadata/AbstractMetadata.java  |  1 -
 .../org/apache/sis/metadata/MetadataCopier.java    | 19 +++++++------
 .../apache/sis/metadata/MetadataCopierTest.java    | 33 +++++++++++++++++++++-
 6 files changed, 67 insertions(+), 10 deletions(-)

diff --git a/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/Resources.java
b/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/Resources.java
index 0a3d1b9..7323666 100644
--- a/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/Resources.java
+++ b/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/Resources.java
@@ -76,6 +76,11 @@ public final class Resources extends IndexedResourceBundle {
         public static final short ElementsOmitted_1 = 4;
 
         /**
+         * `{1}` is an implementation class. Specify the `{0}` interface instead.
+         */
+        public static final short ExpectedInterface_2 = 5;
+
+        /**
          * This metadata is not modifiable.
          */
         public static final short UnmodifiableMetadata = 1;
@@ -139,6 +144,23 @@ public final class Resources extends IndexedResourceBundle {
     }
 
     /**
+     * Gets a string for the given key and replaces all occurrences of "{0}",
+     * "{1}", with values of {@code arg0}, {@code arg1}, etc.
+     *
+     * @param  key   the key for the desired string.
+     * @param  arg0  value to substitute to "{0}".
+     * @param  arg1  value to substitute to "{1}".
+     * @return the formatted string for the given key.
+     * @throws MissingResourceException if no object for the given key can be found.
+     */
+    public static String format(final short  key,
+                                final Object arg0,
+                                final Object arg1) throws MissingResourceException
+    {
+        return forLocale(null).getString(key, arg0, arg1);
+    }
+
+    /**
      * The international string to be returned by {@link formatInternational}.
      */
     private static final class International extends ResourceInternationalString {
diff --git a/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/Resources.properties
b/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/Resources.properties
index 9f9540a..5ed3632 100644
--- a/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/Resources.properties
+++ b/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/Resources.properties
@@ -22,4 +22,5 @@
 BoxCrossesAntiMeridian            = Bounding box crosses the antimeridian.
 ElementAlreadyInitialized_1       = This metadata element is already initialized with value
\u201c{0}\u201d.
 ElementsOmitted_1                 = \u2026 {0} elements omitted \u2026
+ExpectedInterface_2               = `{1}` is an implementation class. Specify the `{0}` interface
instead.
 UnmodifiableMetadata              = This metadata is not modifiable.
diff --git a/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/Resources_fr.properties
b/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/Resources_fr.properties
index 48a6c58..ff8b409 100644
--- a/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/Resources_fr.properties
+++ b/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/Resources_fr.properties
@@ -27,4 +27,5 @@
 BoxCrossesAntiMeridian            = La bo\u00eete englobante traverse l\u2019antim\u00e9ridien.
 ElementAlreadyInitialized_1       = Cet \u00e9l\u00e9ment de m\u00e9ta-donn\u00e9e est d\u00e9j\u00e0
initialis\u00e9 avec la valeur \u00ab\u202f{0}\u202f\u00bb.
 ElementsOmitted_1                 = \u2026 {0} \u00e9l\u00e9ments omis \u2026
+ExpectedInterface_2               = `{1}` est une classe d\u2019impl\u00e9mentation. Sp\u00e9cifiez
l\u2019interface `{0}` \u00e0 la place.
 UnmodifiableMetadata              = Cette m\u00e9ta-donn\u00e9e n\u2019est pas modifiable.
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 d20bfcc..1431bda 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
@@ -106,7 +106,6 @@ public abstract class AbstractMetadata implements LenientComparable, Emptiable
{
      * @see MetadataStandard#getInterface(Class)
      */
     public Class<?> getInterface() {
-        // No need to sychronize, since this method does not depend on property values.
         return getStandard().getInterface(getClass());
     }
 
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 42f85a9..a99df1a 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
@@ -29,6 +29,7 @@ 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;
+import org.apache.sis.internal.metadata.Resources;
 
 
 /**
@@ -146,24 +147,26 @@ public class MetadataCopier extends MetadataVisitor<Object> {
      * @param  type      the interface of the metadata object to copy.
      * @param  metadata  the metadata object to copy, or {@code null}.
      * @return a copy of the given metadata object, or {@code null} if the given argument
is {@code null}.
+     * @throws IllegalArgumentException if {@code type} is an implementation class instead
than interface.
      * @throws UnsupportedOperationException if there is no implementation class for a metadata
to copy,
      *         or an implementation class does not provide a public default constructor.
      */
     public <T> T copy(final Class<T> type, final T metadata) {
         ArgumentChecks.ensureNonNull("type", type);
-        Class<?> interfaceType = type;
-        if (interfaceType != null) {
-            final MetadataStandard std = getStandard(metadata);
-            if (std != null) {
+        if (metadata instanceof AbstractMetadata) {
+            final Class<?> interfaceType = ((AbstractMetadata) metadata).getInterface();
+            if (type != interfaceType) {
                 /*
                  * In case the user specified an implementation despite the documentation
warning.
-                 * It will work most of the time, but may also result in a ClassCastException
thrown
-                 * at the end of this method. But the user has bee warned.
+                 * We could replace `type` by `interfaceType` and it would work most of the
time,
+                 * but we would still have some ClassCastExceptions for example if the given
type
+                 * is a java.lang.reflect.Proxy. It is probably better to let users know
soon that
+                 * they should specify an interface.
                  */
-                interfaceType = std.getInterface(type);
+                throw new IllegalArgumentException(Resources.format(Resources.Keys.ExpectedInterface_2,
interfaceType, type));
             }
         }
-        return type.cast(copyRecursively(interfaceType, metadata));
+        return type.cast(copyRecursively(type, metadata));
     }
 
     /**
diff --git a/core/sis-metadata/src/test/java/org/apache/sis/metadata/MetadataCopierTest.java
b/core/sis-metadata/src/test/java/org/apache/sis/metadata/MetadataCopierTest.java
index c92cd28..7765a19 100644
--- a/core/sis-metadata/src/test/java/org/apache/sis/metadata/MetadataCopierTest.java
+++ b/core/sis-metadata/src/test/java/org/apache/sis/metadata/MetadataCopierTest.java
@@ -16,6 +16,7 @@
  */
 package org.apache.sis.metadata;
 
+import org.opengis.metadata.citation.Citation;
 import org.apache.sis.metadata.iso.citation.DefaultCitation;
 import org.apache.sis.metadata.iso.citation.HardCodedCitations;
 import org.apache.sis.test.DependsOn;
@@ -31,7 +32,7 @@ import static org.apache.sis.test.TestUtilities.getSingleton;
  * Unless otherwise specified, all tests use the {@link MetadataStandard#ISO_19115} constant.
  *
  * @author  Martin Desruisseaux (Geomatys)
- * @version 0.8
+ * @version 1.0
  *
  * @see org.apache.sis.internal.metadata.MergerTest
  *
@@ -53,4 +54,34 @@ public final strictfp class MetadataCopierTest extends TestCase {
                       getSingleton(copy.getCitedResponsibleParties()));
         assertEquals(original, copy);
     }
+
+    /**
+     * Tests {@link MetadataCopier#copy(Class, Object)}.
+     */
+    @Test
+    public void testCopyWithType() {
+        final MetadataCopier copier = new MetadataCopier(MetadataStandard.ISO_19115);
+        final DefaultCitation original = HardCodedCitations.EPSG;
+        final Citation copy = copier.copy(Citation.class, original);
+        assertNotSame(original, copy);
+        assertNotSame(getSingleton(original.getCitedResponsibleParties()),
+                      getSingleton(copy.getCitedResponsibleParties()));
+        assertEquals(original, copy);
+    }
+
+    /**
+     * Tests {@link MetadataCopier#copy(Class, Object)} with an implementation class specified
instead of an interface.
+     */
+    @Test
+    public void testWrongArgument() {
+        final MetadataCopier copier = new MetadataCopier(MetadataStandard.ISO_19115);
+        final DefaultCitation original = HardCodedCitations.EPSG;
+        try {
+            copier.copy(DefaultCitation.class, original);
+            fail("Should not have accepted implementation class argument.");
+        } catch (IllegalArgumentException e) {
+            final String message = e.getMessage();
+            assertTrue(message, message.contains("DefaultCitation"));
+        }
+    }
 }


Mime
View raw message