sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject svn commit: r1528101 - in /sis/branches/JDK7/core: sis-metadata/src/test/java/org/apache/sis/xml/ sis-utility/src/main/java/org/apache/sis/internal/util/ sis-utility/src/test/java/org/apache/sis/internal/util/ sis-utility/src/test/java/org/apache/sis/t...
Date Tue, 01 Oct 2013 14:26:51 GMT
Author: desruisseaux
Date: Tue Oct  1 14:26:51 2013
New Revision: 1528101

URL: http://svn.apache.org/r1528101
Log:
Fixed a NullPointerException during unmarshalling of an empty collection (SIS-139).

Added:
    sis/branches/JDK7/core/sis-utility/src/test/java/org/apache/sis/internal/util/CheckedArrayListTest.java
  (with props)
Modified:
    sis/branches/JDK7/core/sis-metadata/src/test/java/org/apache/sis/xml/MetadataMarshallingTest.java
    sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/internal/util/CheckedArrayList.java
    sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/internal/util/CheckedHashSet.java
    sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/internal/util/UnmodifiableArrayList.java
    sis/branches/JDK7/core/sis-utility/src/test/java/org/apache/sis/test/suite/UtilityTestSuite.java

Modified: sis/branches/JDK7/core/sis-metadata/src/test/java/org/apache/sis/xml/MetadataMarshallingTest.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/core/sis-metadata/src/test/java/org/apache/sis/xml/MetadataMarshallingTest.java?rev=1528101&r1=1528100&r2=1528101&view=diff
==============================================================================
--- sis/branches/JDK7/core/sis-metadata/src/test/java/org/apache/sis/xml/MetadataMarshallingTest.java
[UTF-8] (original)
+++ sis/branches/JDK7/core/sis-metadata/src/test/java/org/apache/sis/xml/MetadataMarshallingTest.java
[UTF-8] Tue Oct  1 14:26:51 2013
@@ -26,6 +26,7 @@ import javax.xml.bind.Unmarshaller;
 import javax.xml.bind.JAXBException;
 import org.opengis.util.InternationalString;
 import org.apache.sis.util.iso.SimpleInternationalString;
+import org.apache.sis.metadata.iso.DefaultMetadata;
 import org.apache.sis.metadata.iso.extent.DefaultExtent;
 import org.apache.sis.metadata.iso.extent.DefaultTemporalExtent;
 import org.apache.sis.metadata.iso.extent.DefaultGeographicBoundingBox;
@@ -100,6 +101,26 @@ public final strictfp class MetadataMars
     }
 
     /**
+     * Tests a collection that contains no element.
+     * This was used to cause a {@code NullPointerException} prior SIS-139 fix.
+     *
+     * @throws JAXBException If an error occurred during the during unmarshalling processes.
+     *
+     * @since 0.4
+     *
+     * @see <a href="https://issues.apache.org/jira/browse/SIS-139">SIS-139</a>
+     */
+    @Test
+    public void testEmptyCollection() throws JAXBException {
+        final String xml =
+                "<gmd:MD_Metadata xmlns:gmd=\"" + Namespaces.GMD + "\">\n" +
+                "  <gmd:contact/>\n" +
+                "</gmd:MD_Metadata>";
+        final DefaultMetadata metadata = (DefaultMetadata) XML.unmarshal(xml);
+        assertTrue(metadata.getContacts().isEmpty());
+    }
+
+    /**
      * Tests the (un)marshalling of a text group with a default {@code <gco:CharacterString>}
element.
      * This test is somewhat a duplicate of {@link FreeTextMarshallingTest}, but the context
is more
      * elaborated.

Modified: sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/internal/util/CheckedArrayList.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/internal/util/CheckedArrayList.java?rev=1528101&r1=1528100&r2=1528101&view=diff
==============================================================================
--- sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/internal/util/CheckedArrayList.java
[UTF-8] (original)
+++ sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/internal/util/CheckedArrayList.java
[UTF-8] Tue Oct  1 14:26:51 2013
@@ -17,9 +17,14 @@
 package org.apache.sis.internal.util;
 
 import java.util.List;
+import java.util.AbstractList;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+import org.apache.sis.internal.jaxb.Context;
+import org.apache.sis.util.ArraysExt;
 import org.apache.sis.util.resources.Errors;
 import org.apache.sis.util.collection.CheckedContainer;
 
@@ -42,7 +47,7 @@ import static org.apache.sis.util.Argume
  *
  * @author  Martin Desruisseaux (Geomatys)
  * @since   0.3 (derived from geotk-2.1)
- * @version 0.3
+ * @version 0.4
  * @module
  *
  * @see Collections#checkedList(List, Class)
@@ -90,30 +95,86 @@ public final class CheckedArrayList<E> e
     }
 
     /**
+     * Returns {@code true} if a unmarshalling process is under way.
+     * In the later case, logs a warning for non-null element of the wrong type.
+     *
+     * @see <a href="https://issues.apache.org/jira/browse/SIS-139">SIS-139</a>
+     */
+    static boolean warning(final Collection<?> source, final Object element, final
Class<?> type) {
+        final Context context = Context.current();
+        if (context == null) {
+            return false;
+        }
+        if (element != null) {
+            final LogRecord record = Errors.getResources(context.getLocale()).getLogRecord(Level.WARNING,
+                    Errors.Keys.IllegalArgumentClass_3, "element", type, element.getClass());
+            record.setSourceClassName(source.getClass().getName());
+            record.setSourceMethodName("add");
+            Context.warningOccured(context, source, record);
+        }
+        return true;
+    }
+
+    /**
      * Ensures that the given element is non-null and assignable to the type
      * specified at construction time.
      *
      * @param  element the object to check, or {@code null}.
+     * @return {@code true} if the instance is valid, {@code false} if it shall be ignored.
      * @throws IllegalArgumentException if the specified element can not be added to this
list.
      */
-    private void ensureValid(final E element) throws IllegalArgumentException {
-        if (!type.isInstance(element)) {
-            ensureNonNull("element", element);
-            throw new IllegalArgumentException(Errors.format(
-                    Errors.Keys.IllegalArgumentClass_3, "element", type, element.getClass()));
+    private boolean ensureValid(final E element) throws IllegalArgumentException {
+        if (type.isInstance(element)) {
+            return true;
         }
+        if (warning(this, element, type)) {
+            /*
+             * If a unmarshalling process is under way, silently discard null element.
+             * This case happen when a XML element for a collection contains no child.
+             * See https://issues.apache.org/jira/browse/SIS-139
+             */
+            return false;
+        }
+        ensureNonNull("element", element);
+        throw new IllegalArgumentException(Errors.format(
+                Errors.Keys.IllegalArgumentClass_3, "element", type, element.getClass()));
     }
 
     /**
      * Ensures that all elements of the given collection can be added to this list.
      *
      * @param  collection the collection to check, or {@code null}.
+     * @return The potentially filtered collection of elements to add.
      * @throws IllegalArgumentException if at least one element can not be added to this
list.
      */
-    private void ensureValidCollection(final Collection<? extends E> collection) throws
IllegalArgumentException {
-        for (final E element : collection) {
-            ensureValid(element);
+    @SuppressWarnings("unchecked")
+    private List<E> ensureValidCollection(final Collection<? extends E> collection)
throws IllegalArgumentException {
+        int count = 0;
+        final Object[] array = collection.toArray();
+        for (int i=0; i<array.length; i++) {
+            final Object element = array[i];
+            if (ensureValid((E) element)) {
+                array[count++] = element;
+            }
         }
+        // Not-so-unsafe cast: we verified in the above loop that all elements are instance
of E.
+        // The array itself may not be an instance of E[], but this is not important for
Mediator.
+        return new Mediator<>(ArraysExt.resize((E[]) array, count));
+    }
+
+    /**
+     * A wrapper around the given array for use by {@link #addAll(Collection)} only.  This
wrapper violates
+     * some {@link List} method contracts, so it must really be used only as a temporary
object for passing
+     * the array to {@code AbstractList.addAll(…)} implementation. In particular {@link
#toArray()} returns
+     * directly the internal array, because this is the method to be invoked by {@code addAll(…)}
 (this is
+     * actually the only important method for this wrapper).
+     */
+    private static final class Mediator<E> extends AbstractList<E> {
+        private final E[] array;
+        Mediator(final E[] array)           {this.array = array;}
+        @Override public int size()         {return array.length;}
+        @Override public E   get(int index) {return array[index];}
+        @Override public E[] toArray()      {return array;} // See class javadoc.
     }
 
     /**
@@ -127,8 +188,10 @@ public final class CheckedArrayList<E> e
      */
     @Override
     public E set(final int index, final E element) throws IllegalArgumentException {
-        ensureValid(element);
-        return super.set(index, element);
+        if (ensureValid(element)) {
+            return super.set(index, element);
+        }
+        return get(index);
     }
 
     /**
@@ -140,8 +203,10 @@ public final class CheckedArrayList<E> e
      */
     @Override
     public boolean add(final E element) throws IllegalArgumentException {
-        ensureValid(element);
-        return super.add(element);
+        if (ensureValid(element)) {
+            return super.add(element);
+        }
+        return false;
     }
 
     /**
@@ -154,8 +219,9 @@ public final class CheckedArrayList<E> e
      */
     @Override
     public void add(final int index, final E element) throws IllegalArgumentException {
-        ensureValid(element);
-        super.add(index, element);
+        if (ensureValid(element)) {
+            super.add(index, element);
+        }
     }
 
     /**
@@ -168,8 +234,7 @@ public final class CheckedArrayList<E> e
      */
     @Override
     public boolean addAll(final Collection<? extends E> collection) throws IllegalArgumentException
{
-        ensureValidCollection(collection);
-        return super.addAll(collection);
+        return super.addAll(ensureValidCollection(collection));
     }
 
     /**
@@ -183,7 +248,6 @@ public final class CheckedArrayList<E> e
      */
     @Override
     public boolean addAll(final int index, final Collection<? extends E> collection)
throws IllegalArgumentException {
-        ensureValidCollection(collection);
-        return super.addAll(index, collection);
+        return super.addAll(index, ensureValidCollection(collection));
     }
 }

Modified: sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/internal/util/CheckedHashSet.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/internal/util/CheckedHashSet.java?rev=1528101&r1=1528100&r2=1528101&view=diff
==============================================================================
--- sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/internal/util/CheckedHashSet.java
[UTF-8] (original)
+++ sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/internal/util/CheckedHashSet.java
[UTF-8] Tue Oct  1 14:26:51 2013
@@ -41,7 +41,7 @@ import static org.apache.sis.util.Argume
  *
  * @author  Martin Desruisseaux (Geomatys)
  * @since   0.3 (derived from geotk-2.1)
- * @version 0.3
+ * @version 0.4
  * @module
  *
  * @see Collections#checkedSet(Set, Class)
@@ -98,6 +98,14 @@ public final class CheckedHashSet<E> ext
     @Override
     public boolean add(final E element) throws IllegalArgumentException {
         if (!type.isInstance(element)) {
+            if (CheckedArrayList.warning(this, element, type)) {
+                /*
+                 * If a unmarshalling process is under way, silently discard null element.
+                 * This case happen when a XML element for a collection contains no child.
+                 * See https://issues.apache.org/jira/browse/SIS-139
+                 */
+                return false;
+            }
             ensureNonNull("element", element);
             throw new IllegalArgumentException(Errors.format(
                     Errors.Keys.IllegalArgumentClass_3, "element", type, element.getClass()));

Modified: sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/internal/util/UnmodifiableArrayList.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/internal/util/UnmodifiableArrayList.java?rev=1528101&r1=1528100&r2=1528101&view=diff
==============================================================================
--- sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/internal/util/UnmodifiableArrayList.java
[UTF-8] (original)
+++ sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/internal/util/UnmodifiableArrayList.java
[UTF-8] Tue Oct  1 14:26:51 2013
@@ -358,6 +358,15 @@ public class UnmodifiableArrayList<E> ex
     }
 
     /**
+     * Returns a copy of the backing array. Note that the array type is {@code E[]} rather
than {@code Object[]}.
+     * This is not what {@code ArrayList} does, but is not forbidden by {@link List#toArray()}
javadoc neither.
+     */
+    @Override
+    public E[] toArray() {
+        return array.clone();
+    }
+
+    /**
      * Compares this list with the given object for equality.
      *
      * @param  object The object to compare with this list.

Added: sis/branches/JDK7/core/sis-utility/src/test/java/org/apache/sis/internal/util/CheckedArrayListTest.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/core/sis-utility/src/test/java/org/apache/sis/internal/util/CheckedArrayListTest.java?rev=1528101&view=auto
==============================================================================
--- sis/branches/JDK7/core/sis-utility/src/test/java/org/apache/sis/internal/util/CheckedArrayListTest.java
(added)
+++ sis/branches/JDK7/core/sis-utility/src/test/java/org/apache/sis/internal/util/CheckedArrayListTest.java
[UTF-8] Tue Oct  1 14:26:51 2013
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sis.internal.util;
+
+import java.util.Arrays;
+import java.util.Collection;
+import org.apache.sis.util.NullArgumentException;
+import org.apache.sis.test.TestCase;
+import org.junit.Test;
+
+import static org.apache.sis.test.Assert.*;
+
+
+/**
+ * Tests the {@link CheckedArrayList} class.
+ *
+ * @author  Martin Desruisseaux (Geomatys)
+ * @since   0.4
+ * @version 0.4
+ * @module
+ */
+public final strictfp class CheckedArrayListTest extends TestCase {
+    /**
+     * Tests {@link CheckedArrayList#add(Object)}.
+     */
+    @Test
+    public void testAdd() {
+        final CheckedArrayList<String> list = new CheckedArrayList<>(String.class);
+        assertTrue(list.add("One"));
+        assertTrue(list.add("Two"));
+        assertTrue(list.add("Three"));
+        assertEquals(Arrays.asList("One", "Two", "Three"), list);
+    }
+
+    /**
+     * Tests {@link CheckedArrayList#addAll(Collection)}.
+     */
+    @Test
+    public void testAddAll() {
+        final CheckedArrayList<String> list = new CheckedArrayList<>(String.class);
+        assertTrue(list.add("One"));
+        assertTrue(list.addAll(Arrays.asList("Two", "Three")));
+        assertEquals(Arrays.asList("One", "Two", "Three"), list);
+    }
+
+    /**
+     * Ensures that we can not add null elements.
+     */
+    @Test
+    public void testAddNull() {
+        final CheckedArrayList<String> list = new CheckedArrayList<>(String.class);
+        try {
+            list.add(null);
+        } catch (NullArgumentException e) {
+            final String message = e.getMessage();
+            assertTrue("element", message.contains("element"));
+        }
+    }
+
+    /**
+     * Ensures that we can not add null elements.
+     */
+    @Test
+    public void testAddAllNull() {
+        final CheckedArrayList<String> list = new CheckedArrayList<>(String.class);
+        final Collection<String> toAdd = Arrays.asList("One", null, "Three");
+        try {
+            list.addAll(toAdd);
+        } catch (NullArgumentException e) {
+            final String message = e.getMessage();
+            assertTrue("element", message.contains("element"));
+        }
+    }
+
+    /**
+     * Ensures that we can not element of the wrong type.
+     */
+    @Test
+    @SuppressWarnings({"unchecked","rawtypes"})
+    public void testAddWrongType() {
+        final CheckedArrayList list = new CheckedArrayList<>(String.class);
+        try {
+            list.add(Integer.valueOf(4));
+        } catch (IllegalArgumentException e) {
+            final String message = e.getMessage();
+            assertTrue("element", message.contains("element"));
+            assertTrue("Integer", message.contains("Integer"));
+            assertTrue("String",  message.contains("String"));
+        }
+    }
+}

Propchange: sis/branches/JDK7/core/sis-utility/src/test/java/org/apache/sis/internal/util/CheckedArrayListTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: sis/branches/JDK7/core/sis-utility/src/test/java/org/apache/sis/internal/util/CheckedArrayListTest.java
------------------------------------------------------------------------------
    svn:mime-type = text/plain;charset=UTF-8

Modified: sis/branches/JDK7/core/sis-utility/src/test/java/org/apache/sis/test/suite/UtilityTestSuite.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/core/sis-utility/src/test/java/org/apache/sis/test/suite/UtilityTestSuite.java?rev=1528101&r1=1528100&r2=1528101&view=diff
==============================================================================
--- sis/branches/JDK7/core/sis-utility/src/test/java/org/apache/sis/test/suite/UtilityTestSuite.java
[UTF-8] (original)
+++ sis/branches/JDK7/core/sis-utility/src/test/java/org/apache/sis/test/suite/UtilityTestSuite.java
[UTF-8] Tue Oct  1 14:26:51 2013
@@ -62,6 +62,7 @@ import org.junit.BeforeClass;
     org.apache.sis.internal.jdk8.JDK8Test.class,
 
     // Collections.
+    org.apache.sis.internal.util.CheckedArrayListTest.class,
     org.apache.sis.internal.system.ReferenceQueueConsumerTest.class,
     org.apache.sis.util.collection.WeakHashSetTest.class,
     org.apache.sis.util.collection.WeakValueHashMapTest.class,



Mime
View raw message