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,