sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject svn commit: r1634819 - in /sis/branches/JDK8/core/sis-metadata/src: main/java/org/apache/sis/metadata/ test/java/org/apache/sis/metadata/
Date Tue, 28 Oct 2014 08:59:15 GMT
Author: desruisseaux
Date: Tue Oct 28 08:59:14 2014
New Revision: 1634819

URL: http://svn.apache.org/r1634819
Log:
Allow implementation to alter the API defined by interfaces.

Modified:
    sis/branches/JDK8/core/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataStandard.java
    sis/branches/JDK8/core/sis-metadata/src/main/java/org/apache/sis/metadata/PropertyAccessor.java
    sis/branches/JDK8/core/sis-metadata/src/main/java/org/apache/sis/metadata/PropertyComparator.java
    sis/branches/JDK8/core/sis-metadata/src/main/java/org/apache/sis/metadata/SpecialCases.java
    sis/branches/JDK8/core/sis-metadata/src/test/java/org/apache/sis/metadata/MetadataTestCase.java
    sis/branches/JDK8/core/sis-metadata/src/test/java/org/apache/sis/metadata/PropertyAccessorTest.java

Modified: sis/branches/JDK8/core/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataStandard.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataStandard.java?rev=1634819&r1=1634818&r2=1634819&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataStandard.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataStandard.java
[UTF-8] Tue Oct 28 08:59:14 2014
@@ -102,6 +102,18 @@ public class MetadataStandard implements
     private static final long serialVersionUID = 7549790450195184843L;
 
     /**
+     * {@code true} if implementations can alter the API defined in the interfaces by
+     * adding or removing properties. If {@code true}, then {@link PropertyAccessor}
+     * will check for {@link Deprecated} and {@link org.opengis.annotation.UML}
+     * annotations in the implementation classes in addition to the interfaces.
+     *
+     * <p>A value of {@code true} is useful when Apache SIS implements a newer standard
+     * than GeoAPI, but have a slight performance cost at construction time. Performance
+     * after construction should be the same.</p>
+     */
+    static final boolean IMPLEMENTATION_CAN_ALTER_API = false;
+
+    /**
      * Metadata instances defined in this class. The current implementation does not yet
      * contains the user-defined instances. However this may be something we will need to
      * do in the future.
@@ -175,7 +187,7 @@ public class MetadataStandard implements
     private final MetadataStandard[] dependencies;
 
     /**
-     * Accessors for the specified implementations.
+     * Accessors for the specified implementation classes.
      * The only legal value types are:
      *
      * <ul>
@@ -311,7 +323,7 @@ public class MetadataStandard implements
      */
     final PropertyAccessor getAccessor(final Class<?> implementation, final boolean
mandatory) {
         /*
-         * Check for accessors created by previous call to this method.
+         * Check for accessors created by previous calls to this method.
          * Values are added to this cache but never cleared.
          */
         final Object value = accessors.get(implementation);
@@ -362,14 +374,7 @@ public class MetadataStandard implements
             if (SpecialCases.isSpecialCase(type)) {
                 accessor = new SpecialCases(citation, type, implementation);
             } else {
-                /*
-                 * If "multi-value returns" was allowed in the Java language, the 'onlyUML'
boolean would
-                 * be returned by 'findInterface(Class)' method when it falls in the special
case for the
-                 * UML annotation on implementation class. But since we do not have multi-values,
we have
-                 * to infer it from our knownledge of how 'findInterface(Class)' is implemented.
-                 */
-                final boolean onlyUML = (type == implementation && !type.isInterface());
-                accessor = new PropertyAccessor(citation, type, implementation, onlyUML);
+                accessor = new PropertyAccessor(citation, type, implementation);
             }
             return accessor;
         });
@@ -469,7 +474,7 @@ public class MetadataStandard implements
                     }
                     // Found more than one interface; we don't know which one to pick.
                     // Returns 'null' for now; the caller will thrown an exception.
-                } else if (isPendingAPI(type)) {
+                } else if (IMPLEMENTATION_CAN_ALTER_API && isPendingAPI(type)) {
                     /*
                      * Found no interface. According to our method contract we should return
null.
                      * However we make an exception if the implementation class has a UML
annotation.

Modified: sis/branches/JDK8/core/sis-metadata/src/main/java/org/apache/sis/metadata/PropertyAccessor.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-metadata/src/main/java/org/apache/sis/metadata/PropertyAccessor.java?rev=1634819&r1=1634818&r2=1634819&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-metadata/src/main/java/org/apache/sis/metadata/PropertyAccessor.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-metadata/src/main/java/org/apache/sis/metadata/PropertyAccessor.java
[UTF-8] Tue Oct 28 08:59:14 2014
@@ -21,7 +21,6 @@ import java.util.List;
 import java.util.Arrays;
 import java.util.Locale;
 import java.util.HashMap;
-import java.util.IdentityHashMap;
 import java.util.Iterator;
 import java.util.Collection;
 import java.lang.reflect.Method;
@@ -87,19 +86,6 @@ import static org.apache.sis.util.collec
  */
 class PropertyAccessor {
     /**
-     * Getters shared between many instances of this class. Two different implementations
-     * may share the same getters but different setters.
-     *
-     * <div class="note"><b>Implementation note:</b>
-     * In the particular case of {@code Class} keys, {@code IdentityHashMap} and {@code HashMap}
have identical
-     * behavior since {@code Class} is final and does not override the {@code equals(Object)}
and {@code hashCode()}
-     * methods. The {@code IdentityHashMap} Javadoc claims that it is faster than the regular
{@code HashMap}.
-     * But maybe the most interesting property is that it allocates less objects since {@code
IdentityHashMap}
-     * implementation doesn't need the chain of objects created by {@code HashMap}.</div>
-     */
-    private static final Map<Class<?>, Method[]> SHARED_GETTERS = new IdentityHashMap<>();
-
-    /**
      * Enumeration constants for the {@code mode} argument in the
      * {@link #count(Object, ValueExistencePolicy, int)} method.
      */
@@ -253,14 +239,13 @@ class PropertyAccessor {
      * @param  standard       The standard which define the {@code type} interface.
      * @param  type           The interface implemented by the metadata class.
      * @param  implementation The class of metadata implementations, or {@code type} if none.
-     * @param  onlyUML        {@code true} for taking only the getter methods having a {@link
UML} annotation.
      */
-    PropertyAccessor(final Citation standard, final Class<?> type, final Class<?>
implementation, final boolean onlyUML) {
+    PropertyAccessor(final Citation standard, final Class<?> type, final Class<?>
implementation) {
         assert type.isAssignableFrom(implementation) : implementation;
         this.standard       = standard;
         this.type           = type;
         this.implementation = implementation;
-        this.getters        = getGetters(type, implementation, onlyUML);
+        this.getters        = getGetters(type, implementation);
         int allCount = getters.length;
         int standardCount = allCount;
         if (allCount != 0 && getters[allCount-1] == EXTRA_GETTER) {
@@ -420,70 +405,82 @@ class PropertyAccessor {
      * since it may be shared among many instances of {@code PropertyAccessor}.
      *
      * @param  type The metadata interface.
-     * @param  implementation The class of metadata implementations.
-     * @param  onlyUML {@code true} for taking only the getter methods having a {@link UML}
annotation.
+     * @param  implementation The class of metadata implementations, or {@code type} if none.
      * @return The getters declared in the given interface (never {@code null}).
      */
-    private static Method[] getGetters(final Class<?> type, final Class<?> implementation,
final boolean onlyUML) {
-        synchronized (SHARED_GETTERS) {
-            Method[] getters = SHARED_GETTERS.get(type);
-            if (getters == null) {
-                getters = type.getMethods();
-                // Following is similar in purpose to the PropertyAccessor.mapping field,
-                // but index values are different because of the call to Arrays.sort(...).
-                final Map<String,Integer> mapping = new HashMap<>(hashMapCapacity(getters.length));
-                boolean hasExtraGetter = false;
-                int count = 0;
-                for (final Method candidate : getters) {
-                    if (Classes.isPossibleGetter(candidate) && (!onlyUML || candidate.isAnnotationPresent(UML.class)))
{
-                        final String name = candidate.getName();
-                        if (name.startsWith(SET)) { // Paranoiac check.
-                            continue;
-                        }
-                        /*
-                         * At this point, we are ready to accept the method. Before doing
so,
-                         * check if the method override an other method defined in a parent
-                         * class with a covariant return type. The JVM considers such cases
-                         * as two different methods, while from a Java developer point of
-                         * view this is the same method (GEOTK-205).
-                         */
-                        final Integer pi = mapping.put(name, count);
-                        if (pi != null) {
-                            final Class<?> pt = getters[pi].getReturnType();
-                            final Class<?> ct = candidate  .getReturnType();
-                            if (ct.isAssignableFrom(pt)) {
-                                continue; // Previous type was more accurate.
-                            }
-                            if (pt.isAssignableFrom(ct)) {
-                                getters[pi] = candidate;
-                                continue;
-                            }
-                            throw new ClassCastException(Errors.format(Errors.Keys.IllegalArgumentClass_3,
-                                    Classes.getShortName(type) + '.' + name, ct, pt));
+    private static Method[] getGetters(final Class<?> type, final Class<?> implementation)
{
+        /*
+         * Indices map is used for choosing what to do in case of name collision.
+         */
+        Method[] getters = (MetadataStandard.IMPLEMENTATION_CAN_ALTER_API ? implementation
: type).getMethods();
+        final Map<String,Integer> indices = new HashMap<>(hashMapCapacity(getters.length));
+        boolean hasExtraGetter = false;
+        int count = 0;
+        for (Method candidate : getters) {
+            if (Classes.isPossibleGetter(candidate)) {
+                final String name = candidate.getName();
+                if (name.startsWith(SET)) { // Paranoiac check.
+                    continue;
+                }
+                /*
+                 * The candidate method should be declared in the interface. If not, then
we require it to have
+                 * a @UML annotation. The later case happen when the Apache SIS implementation
contains methods
+                 * for a new international standard not yet reflected in the GeoAPI interfaces.
+                 */
+                if (MetadataStandard.IMPLEMENTATION_CAN_ALTER_API) {
+                    if (type == implementation) {
+                        if (!type.isInterface() && !candidate.isAnnotationPresent(UML.class))
{
+                            continue; // @UML considered optional only for interfaces.
                         }
-                        getters[count++] = candidate;
-                        if (!hasExtraGetter) {
-                            hasExtraGetter = name.equals(EXTRA_GETTER.getName());
+                    } else try {
+                        candidate = type.getMethod(name, (Class[]) null);
+                    } catch (NoSuchMethodException e) {
+                        if (!candidate.isAnnotationPresent(UML.class)) {
+                            continue; // Not a method from an interface, and no @UML in implementation.
                         }
                     }
                 }
                 /*
-                 * Sort the standard methods before to add the extra methods (if any) in
order to
-                 * keep the extra methods last. The code checking for the extra methods require
-                 * them to be last.
+                 * At this point, we are ready to accept the method. Before doing so,
+                 * check if the method override an other method defined in a parent
+                 * class with a covariant return type. The JVM considers such cases
+                 * as two different methods, while from a Java developer point of
+                 * view this is the same method (GEOTK-205).
                  */
-                Arrays.sort(getters, 0, count, new PropertyComparator(implementation));
-                if (!hasExtraGetter) {
-                    if (getters.length == count) {
-                        getters = Arrays.copyOf(getters, count+1);
+                final Integer pi = indices.put(name, count);
+                if (pi != null) {
+                    final Class<?> pt = getters[pi].getReturnType();
+                    final Class<?> ct = candidate  .getReturnType();
+                    if (ct.isAssignableFrom(pt)) {
+                        continue; // Previous type was more accurate.
+                    }
+                    if (pt.isAssignableFrom(ct)) {
+                        getters[pi] = candidate;
+                        continue;
                     }
-                    getters[count++] = EXTRA_GETTER;
+                    throw new ClassCastException(Errors.format(Errors.Keys.IllegalArgumentClass_3,
+                            Classes.getShortName(type) + '.' + name, ct, pt));
                 }
-                getters = ArraysExt.resize(getters, count);
-                SHARED_GETTERS.put(type, getters);
+                getters[count++] = candidate;
+                if (!hasExtraGetter) {
+                    hasExtraGetter = name.equals(EXTRA_GETTER.getName());
+                }
+            }
+        }
+        /*
+         * Sort the standard methods before to add the extra methods (if any) in order to
+         * keep the extra methods last. The code checking for the extra methods require
+         * them to be last.
+         */
+        Arrays.sort(getters, 0, count, new PropertyComparator(implementation));
+        if (!hasExtraGetter) {
+            if (getters.length == count) {
+                getters = Arrays.copyOf(getters, count+1);
             }
-            return getters;
+            getters[count++] = EXTRA_GETTER;
         }
+        getters = ArraysExt.resize(getters, count);
+        return getters;
     }
 
     /**
@@ -619,10 +616,13 @@ class PropertyAccessor {
     }
 
     /**
-     * Returns {@code true} if the property at the given index is deprecated.
+     * Returns {@code true} if the property at the given index is deprecated, either in the
interface that
+     * declare the method or in the implementation class. A method may be deprecated in the
implementation
+     * but not in the interface when the implementation has been updated for a new standard
+     * while the interface is still reflecting the old standard.
      */
     private boolean isDeprecated(final int index) {
-        return getters[index].isAnnotationPresent(Deprecated.class);
+        return PropertyComparator.isDeprecated(implementation, getters[index]);
     }
 
     /**

Modified: sis/branches/JDK8/core/sis-metadata/src/main/java/org/apache/sis/metadata/PropertyComparator.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-metadata/src/main/java/org/apache/sis/metadata/PropertyComparator.java?rev=1634819&r1=1634818&r2=1634819&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-metadata/src/main/java/org/apache/sis/metadata/PropertyComparator.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-metadata/src/main/java/org/apache/sis/metadata/PropertyComparator.java
[UTF-8] Tue Oct 28 08:59:14 2014
@@ -84,13 +84,19 @@ final class PropertyComparator implement
     private final Map<Object,Integer> order;
 
     /**
+     * The implementation class, or the interface is the implementation class is unknown.
+     */
+    private final Class<?> implementation;
+
+    /**
      * Creates a new comparator for the given implementation class.
      *
-     * @param implementation The implementation class, or {@code null} if unknown.
+     * @param implementation The implementation class, or the interface if the implementation
class is unknown.
      */
     PropertyComparator(Class<?> implementation) {
+        this.implementation = implementation;
         order = new HashMap<>();
-        while (implementation != null) {
+        do {
             final XmlType xml = implementation.getAnnotation(XmlType.class);
             if (xml != null) {
                 final String[] propOrder = xml.propOrder();
@@ -108,7 +114,37 @@ final class PropertyComparator implement
                 }
             }
             implementation = implementation.getSuperclass();
+        } while (implementation != null);
+    }
+
+    /**
+     * Returns {@code true} if the given method is deprecated, either in the interface that
declare the method
+     * or in the implementation class. A method may be deprecated in the implementation but
not in the interface
+     * when the implementation has been updated for a new standard, while the interface is
still reflecting the
+     * old standard.
+     *
+     * @param  implementation The implementation class, or the interface is the implementation
class is unknown.
+     * @param  method The method to check for deprecation.
+     * @return {@code true} if the method is deprecated.
+     */
+    static boolean isDeprecated(final Class<?> implementation, Method method) {
+        if (!MetadataStandard.IMPLEMENTATION_CAN_ALTER_API) {
+            return method.isAnnotationPresent(Deprecated.class);
+        }
+        if (method.isAnnotationPresent(Deprecated.class)) {
+            return true;
+        }
+        if (method.getDeclaringClass() == implementation) {
+            return false;
+        }
+        try {
+            method = implementation.getMethod(method.getName(), (Class[]) null);
+        } catch (NoSuchMethodException e) {
+            // Should never happen since the implementation is supposed to implement
+            // the interface that declare the method given in argument.
+            throw new AssertionError(e);
         }
+        return method.isAnnotationPresent(Deprecated.class);
     }
 
     /**
@@ -116,8 +152,8 @@ final class PropertyComparator implement
      */
     @Override
     public int compare(final Method m1, final Method m2) {
-        final boolean deprecated = m1.isAnnotationPresent(Deprecated.class);
-        if (deprecated != m2.isAnnotationPresent(Deprecated.class)) {
+        final boolean deprecated = isDeprecated(implementation, m1);
+        if (deprecated != isDeprecated(implementation, m2)) {
             return deprecated ? +1 : -1;
         }
         int c = indexOf(m2) - indexOf(m1); // indexOf(…) are sorted in descending order.

Modified: sis/branches/JDK8/core/sis-metadata/src/main/java/org/apache/sis/metadata/SpecialCases.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-metadata/src/main/java/org/apache/sis/metadata/SpecialCases.java?rev=1634819&r1=1634818&r2=1634819&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-metadata/src/main/java/org/apache/sis/metadata/SpecialCases.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-metadata/src/main/java/org/apache/sis/metadata/SpecialCases.java
[UTF-8] Tue Oct 28 08:59:14 2014
@@ -51,7 +51,7 @@ final class SpecialCases extends Propert
      * @param  implementation The class of metadata implementations, or {@code type} if none.
      */
     SpecialCases(final Citation standard, final Class<?> type, final Class<?>
implementation) {
-        super(standard, type, implementation, false);
+        super(standard, type, implementation);
         assert isSpecialCase(type) : type;
         westBoundLongitude = indexOf("westBoundLongitude", true);
         eastBoundLongitude = indexOf("eastBoundLongitude", true);

Modified: sis/branches/JDK8/core/sis-metadata/src/test/java/org/apache/sis/metadata/MetadataTestCase.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-metadata/src/test/java/org/apache/sis/metadata/MetadataTestCase.java?rev=1634819&r1=1634818&r2=1634819&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-metadata/src/test/java/org/apache/sis/metadata/MetadataTestCase.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-metadata/src/test/java/org/apache/sis/metadata/MetadataTestCase.java
[UTF-8] Tue Oct 28 08:59:14 2014
@@ -185,7 +185,7 @@ public abstract strictfp class MetadataT
                 final Class<?> impl = getImplementation(type);
                 if (impl != null) {
                     assertTrue(type.isAssignableFrom(impl));
-                    testPropertyValues(new PropertyAccessor(standard.getCitation(), type,
impl, false));
+                    testPropertyValues(new PropertyAccessor(standard.getCitation(), type,
impl));
                 }
             }
         }

Modified: sis/branches/JDK8/core/sis-metadata/src/test/java/org/apache/sis/metadata/PropertyAccessorTest.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-metadata/src/test/java/org/apache/sis/metadata/PropertyAccessorTest.java?rev=1634819&r1=1634818&r2=1634819&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-metadata/src/test/java/org/apache/sis/metadata/PropertyAccessorTest.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-metadata/src/test/java/org/apache/sis/metadata/PropertyAccessorTest.java
[UTF-8] Tue Oct 28 08:59:14 2014
@@ -91,7 +91,7 @@ public final strictfp class PropertyAcce
      * Creates a new property accessor for the {@link DefaultCitation} class.
      */
     private static PropertyAccessor createPropertyAccessor() {
-        return new PropertyAccessor(HardCodedCitations.ISO_19115, Citation.class, DefaultCitation.class,
false);
+        return new PropertyAccessor(HardCodedCitations.ISO_19115, Citation.class, DefaultCitation.class);
     }
 
     /**
@@ -199,7 +199,7 @@ public final strictfp class PropertyAcce
     @Test
     @DependsOnMethod("testConstructor")
     public void testConstructorWithInheritance() {
-        assertMappingEquals(new PropertyAccessor(HardCodedCitations.ISO_19115, DataIdentification.class,
DefaultDataIdentification.class, false),
+        assertMappingEquals(new PropertyAccessor(HardCodedCitations.ISO_19115, DataIdentification.class,
DefaultDataIdentification.class),
         //……Declaring type………………………Method………………………………………………………………………JavaBeans………………………………………………………UML
identifier………………………………………Sentence……………………………………………………………Type………………………………………………………………
             Identification.class, "getCitation",                   "citation",          
        "citation",                  "Citation",                     Citation.class,
             Identification.class, "getAbstract",                   "abstract",          
        "abstract",                  "Abstract",                     InternationalString.class,
@@ -238,7 +238,7 @@ public final strictfp class PropertyAcce
     @DependsOnMethod("testConstructorWithInheritance")
     public void testConstructorWithCovariantReturnType() {
         final Class<?> type = GeographicCRS.class;
-        assertMappingEquals(new PropertyAccessor(HardCodedCitations.ISO, type, type, false),
+        assertMappingEquals(new PropertyAccessor(HardCodedCitations.ISO, type, type),
         //……Declaring type……………………………Method……………………………………………JavaBeans……………………………UML
identifier………………Sentence…………………………………Type…………………………………………………………
             GeographicCRS.class,    "getCoordinateSystem", "coordinateSystem", "coordinateSystem",
"Coordinate system",  EllipsoidalCS.class,       // Covariant return type
             GeodeticCRS.class,      "getDatum",            "datum",            "datum", 
          "Datum",              GeodeticDatum.class,       // Covariant return type
@@ -368,7 +368,7 @@ public final strictfp class PropertyAcce
     @DependsOnMethod("testSet")
     public void testSetDeprecated() {
         final PropertyAccessor accessor = new PropertyAccessor(HardCodedCitations.ISO_19115,
-                CoverageDescription.class, DefaultCoverageDescription.class, false);
+                CoverageDescription.class, DefaultCoverageDescription.class);
         final int indexOfDeprecated  = accessor.indexOf("contentType", true);
         final int indexOfReplacement = accessor.indexOf("attributeGroup", true);
         assertTrue("Deprecated elements shall be sorted after non-deprecated ones.",



Mime
View raw message