SLING-11465 : NPE in JcrPropertyMapCacheEntry when converting from InputStream value to Number (#30)
* SLING-11465 : NPE in JcrPropertyMapCacheEntry when converting from InputStream value to Number,
SLING-11466 : JcrPropertyMapCacheEntry: ValueFormatException when converting value InputStream to number-array
* SLING-11466 : JcrPropertyMapCacheEntry: ValueFormatException when converting value InputStream to number-array
* SLING-11465 : NPE in JcrPropertyMapCacheEntry when converting from InputStream value to Number
diff --git a/src/main/java/org/apache/sling/jcr/resource/internal/helper/JcrPropertyMapCacheEntry.java b/src/main/java/org/apache/sling/jcr/resource/internal/helper/JcrPropertyMapCacheEntry.java
index d1bcdf6..d5f1efd 100644
--- a/src/main/java/org/apache/sling/jcr/resource/internal/helper/JcrPropertyMapCacheEntry.java
+++ b/src/main/java/org/apache/sling/jcr/resource/internal/helper/JcrPropertyMapCacheEntry.java
@@ -83,7 +83,7 @@
* @param node the node
* @throws RepositoryException if the provided value cannot be stored
*/
- public JcrPropertyMapCacheEntry(final Object value, final Node node) throws RepositoryException {
+ public JcrPropertyMapCacheEntry(final @NotNull Object value, final @NotNull Node node) throws RepositoryException {
this.property = null;
this.propertyValue = value;
this.isArray = value.getClass().isArray();
@@ -98,7 +98,7 @@
}
}
- private static void failIfCannotStore(final Object value, final Node node) throws RepositoryException {
+ private static void failIfCannotStore(final @NotNull Object value, final @NotNull Node node) throws RepositoryException {
if (value instanceof InputStream) {
// InputStream is storable and calling createValue for nothing
// eats its contents
@@ -120,7 +120,7 @@
* @param node the node
* @return the converted value
*/
- private static Value createValue(final Object obj, final Node node) throws RepositoryException {
+ private static @Nullable Value createValue(final @NotNull Object obj, final @NotNull Node node) throws RepositoryException {
final Session session = node.getSession();
Value value = JcrResourceUtil.createValue(obj, session);
if (value == null && obj instanceof Serializable) {
@@ -143,7 +143,7 @@
* @param value The array
* @return an object array
*/
- private static Object[] convertToObjectArray(final Object value) {
+ private static @NotNull Object[] convertToObjectArray(final @NotNull Object value) {
final Object[] values;
if (value instanceof long[]) {
values = ArrayUtils.toObject((long[]) value);
@@ -180,7 +180,7 @@
* @return The current value
* @throws RepositoryException If something goes wrong
*/
- public Object getPropertyValue() throws RepositoryException {
+ public @NotNull Object getPropertyValue() throws RepositoryException {
return this.propertyValue != null ? this.propertyValue : JcrResourceUtil.toJavaObject(property);
}
@@ -188,7 +188,7 @@
* Get the current property value.
* @return The current value or {@code null} if not possible.
*/
- public Object getPropertyValueOrNull() {
+ public @Nullable Object getPropertyValueOrNull() {
try {
return getPropertyValue();
} catch (final RepositoryException e) {
@@ -205,9 +205,9 @@
* @return The converted object
*/
@SuppressWarnings("unchecked")
- public <T> T convertToType(final @NotNull Class<T> type,
- final @NotNull Node node,
- final @Nullable ClassLoader dynamicClassLoader) {
+ public @Nullable<T> T convertToType(final @NotNull Class<T> type,
+ final @NotNull Node node,
+ final @Nullable ClassLoader dynamicClassLoader) {
T result = null;
try {
@@ -223,10 +223,10 @@
}
} else {
-
+ // source is not multivalued
final Object sourceObject = this.getPropertyValue();
if (targetIsArray) {
- result = (T) convertToArray(new Object[]{sourceObject}, type.getComponentType(), node, dynamicClassLoader);
+ result = (T) convertToArray(sourceObject, type.getComponentType(), node, dynamicClassLoader);
} else {
result = convertToType(-1, sourceObject, type, node, dynamicClassLoader);
}
@@ -242,10 +242,25 @@
return result;
}
- private <T> T[] convertToArray(final @NotNull Object[] sourceArray,
- final @NotNull Class<T> type,
- final @NotNull Node node,
- final @Nullable ClassLoader dynamicClassLoader) throws RepositoryException {
+ private @NotNull<T> T[] convertToArray(final @NotNull Object source,
+ final @NotNull Class<T> type,
+ final @NotNull Node node,
+ final @Nullable ClassLoader dynamicClassLoader) throws RepositoryException {
+ List<T> values = new ArrayList<>();
+ T value = convertToType(-1, source, type, node, dynamicClassLoader);
+ if (value != null) {
+ values.add(value);
+ }
+
+ @SuppressWarnings("unchecked")
+ T[] result = (T[]) Array.newInstance(type, values.size());
+ return values.toArray(result);
+ }
+
+ private @NotNull<T> T[] convertToArray(final @NotNull Object[] sourceArray,
+ final @NotNull Class<T> type,
+ final @NotNull Node node,
+ final @Nullable ClassLoader dynamicClassLoader) throws RepositoryException {
List<T> values = new ArrayList<>();
for (int i = 0; i < sourceArray.length; i++) {
T value = convertToType(i, sourceArray[i], type, node, dynamicClassLoader);
@@ -261,11 +276,11 @@
}
@SuppressWarnings("unchecked")
- private <T> T convertToType(final int index,
- final @NotNull Object initialValue,
- final @NotNull Class<T> type,
- final @NotNull Node node,
- final @Nullable ClassLoader dynamicClassLoader) throws RepositoryException {
+ private @Nullable<T> T convertToType(final int index,
+ final @NotNull Object initialValue,
+ final @NotNull Class<T> type,
+ final @NotNull Node node,
+ final @Nullable ClassLoader dynamicClassLoader) throws RepositoryException {
if (type.isInstance(initialValue)) {
return (T) initialValue;
}
@@ -284,6 +299,11 @@
// any number: length of binary
} else if (Number.class.isAssignableFrom(type)) {
+ // avoid NPE if this instance has not been created from a property (see SLING-11465)
+ if (property == null) {
+ return null;
+ }
+
if (index == -1) {
value = Long.valueOf(this.property.getLength());
} else {
@@ -391,7 +411,7 @@
* @param value The object to convert
* @return A converter for {@code value}
*/
- private static Converter getConverter(final Object value) {
+ private static @NotNull Converter getConverter(final @NotNull Object value) {
if (value instanceof Number) {
// byte, short, int, long, double, float, BigDecimal
return new NumberConverter((Number) value);
diff --git a/src/test/java/org/apache/sling/jcr/resource/internal/helper/JcrPropertyMapCacheEntryTest.java b/src/test/java/org/apache/sling/jcr/resource/internal/helper/JcrPropertyMapCacheEntryTest.java
index 145a4fa..d53c08d 100644
--- a/src/test/java/org/apache/sling/jcr/resource/internal/helper/JcrPropertyMapCacheEntryTest.java
+++ b/src/test/java/org/apache/sling/jcr/resource/internal/helper/JcrPropertyMapCacheEntryTest.java
@@ -18,19 +18,50 @@
*/
package org.apache.sling.jcr.resource.internal.helper;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verifyZeroInteractions;
-
+import org.apache.jackrabbit.value.BooleanValue;
+import org.apache.jackrabbit.value.ValueFactoryImpl;
+import org.junit.Before;
import org.junit.Test;
import javax.jcr.Node;
+import javax.jcr.Property;
+import javax.jcr.PropertyType;
+import javax.jcr.Session;
+import javax.jcr.ValueFactory;
+import javax.jcr.ValueFormatException;
+import java.io.ByteArrayInputStream;
+import java.io.InputStream;
+import java.io.ObjectInputStream;
+import java.util.Calendar;
+import java.util.HashMap;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.verifyZeroInteractions;
+import static org.mockito.Mockito.when;
/**
* Testcase for {@link JcrPropertyMapCacheEntry}
*/
public class JcrPropertyMapCacheEntryTest {
+ private final ValueFactory vf = ValueFactoryImpl.getInstance();
+ private final Session session = mock(Session.class);
private final Node node = mock(Node.class);
+
+ @Before
+ public void before() throws Exception {
+ when(session.getValueFactory()).thenReturn(vf);
+ when(node.getSession()).thenReturn(session);
+ }
@Test
public void testByteArray() throws Exception {
@@ -87,4 +118,169 @@
new JcrPropertyMapCacheEntry(new char[0], node);
verifyZeroInteractions(node);
}
+
+ @Test
+ public void testInputStreamToString() throws Exception {
+ InputStream in = new ByteArrayInputStream("test".getBytes());
+ JcrPropertyMapCacheEntry entry = new JcrPropertyMapCacheEntry(in, node);
+
+ String result = entry.convertToType(String.class, node, null);
+ assertEquals("test", result);
+ verifyZeroInteractions(node);
+ }
+
+ @Test
+ public void testInputStreamToLong() throws Exception {
+ InputStream in = new ByteArrayInputStream("10".getBytes());
+ JcrPropertyMapCacheEntry entry = new JcrPropertyMapCacheEntry(in, node);
+
+ Long result = entry.convertToType(Long.class, node, null);
+ assertNull(result);
+ verifyZeroInteractions(node);
+ }
+
+ @Test
+ public void testInputStreamToIntegerArray() throws Exception {
+ InputStream in = new ByteArrayInputStream("10".getBytes());
+ JcrPropertyMapCacheEntry entry = new JcrPropertyMapCacheEntry(in, node);
+
+ Integer[] result = entry.convertToType(Integer[].class, node, null);
+ assertNotNull(result);
+ assertEquals(0, result.length);
+ verifyZeroInteractions(node);
+ }
+
+ @Test
+ public void testBinaryPropertyToLong() throws Exception {
+ Property prop = mock(Property.class);
+ when(prop.getType()).thenReturn(PropertyType.BINARY);
+ when(prop.getStream()).thenReturn(new ByteArrayInputStream("10".getBytes()));
+ when(prop.getValue()).thenReturn(vf.createValue(new ByteArrayInputStream("10".getBytes())));
+ when(prop.getLength()).thenReturn(2L);
+
+ JcrPropertyMapCacheEntry entry = new JcrPropertyMapCacheEntry(prop);
+ Long result = entry.convertToType(Long.class, node, null);
+ assertEquals(Long.valueOf(2), result);
+
+ verify(prop, times(2)).isMultiple();
+ verify(prop).getValue();
+ verify(prop).getType();
+ verify(prop).getLength();
+ verifyNoMoreInteractions(prop);
+
+ verifyZeroInteractions(node);
+ }
+
+ @Test
+ public void testBinaryPropertyToDoubleArray() throws Exception {
+ Property prop = mock(Property.class);
+ when(prop.getType()).thenReturn(PropertyType.BINARY);
+ when(prop.getStream()).thenReturn(new ByteArrayInputStream("10.7".getBytes()));
+ when(prop.getValue()).thenReturn(vf.createValue(new ByteArrayInputStream("10.7".getBytes())));
+ when(prop.getLength()).thenReturn(4L);
+ when(prop.getLengths()).thenThrow(new ValueFormatException("single-valued"));
+
+ JcrPropertyMapCacheEntry entry = new JcrPropertyMapCacheEntry(prop);
+ Double[] result = entry.convertToType(Double[].class, node, null);
+ assertNotNull(result);
+ assertEquals(1, result.length);
+ assertEquals(Double.valueOf(4.0), result[0]);
+
+ verify(prop, times(2)).isMultiple();
+ verify(prop).getValue();
+ verify(prop).getType();
+ verify(prop).getLength();
+ verifyNoMoreInteractions(prop);
+ verifyZeroInteractions(node);
+ }
+
+ @Test
+ public void testInputStreamToObjectInputStream() throws Exception {
+ InputStream in = new ByteArrayInputStream("value".getBytes());
+
+ JcrPropertyMapCacheEntry entry = new JcrPropertyMapCacheEntry(in, node);
+ ObjectInputStream result = entry.convertToType(ObjectInputStream.class, node, null);
+ assertNull(result); // TODO: is this the expected result?
+ verifyZeroInteractions(node);
+ }
+
+ @Test
+ public void testBinaryPropertyToObjectInputStream() throws Exception {
+ InputStream in = new ByteArrayInputStream("value".getBytes());
+
+ Property prop = mock(Property.class);
+ when(prop.getType()).thenReturn(PropertyType.BINARY);
+ when(prop.getStream()).thenReturn(new ByteArrayInputStream("value".getBytes()));
+ when(prop.getValue()).thenReturn(vf.createValue(new ByteArrayInputStream("value".getBytes())));
+
+ JcrPropertyMapCacheEntry entry = new JcrPropertyMapCacheEntry(prop);
+ ObjectInputStream result = entry.convertToType(ObjectInputStream.class, node, null);
+ assertNull(result); // TODO: is this the expected result?
+
+ verify(prop, times(2)).isMultiple();
+ verify(prop).getValue();
+ verify(prop).getType();
+ verifyNoMoreInteractions(prop);
+ verifyZeroInteractions(node);
+ }
+
+ @Test
+ public void testConvertToSameType() throws Exception {
+ Calendar cal = Calendar.getInstance();
+
+ JcrPropertyMapCacheEntry entry = new JcrPropertyMapCacheEntry(cal, node);
+ Calendar result = entry.convertToType(Calendar.class, node, null);
+ assertSame(cal, result);
+ verify(node).getSession();
+ verifyNoMoreInteractions(node);
+ }
+
+ @Test
+ public void testStringToProperty() throws Exception {
+ JcrPropertyMapCacheEntry entry = new JcrPropertyMapCacheEntry("value", node);
+ Property result = entry.convertToType(Property.class, node, null);
+ assertNull(result); // TODO is this expected?
+
+ verify(node).getSession();
+ verify(session).getValueFactory();
+ verifyNoMoreInteractions(node, session);
+ }
+
+ @Test
+ public void testPropertyToProperty() throws Exception {
+ Property prop = mock(Property.class);
+ when(prop.getType()).thenReturn(PropertyType.BOOLEAN);
+ when(prop.getValue()).thenReturn(BooleanValue.valueOf("true"));
+
+ JcrPropertyMapCacheEntry entry = new JcrPropertyMapCacheEntry(prop);
+ Property result = entry.convertToType(Property.class, node, null);
+ assertSame(prop, result);
+
+ verifyZeroInteractions(node);
+ verify(prop).getType();
+ verify(prop).getValue();
+ verify(prop, times(2)).isMultiple();
+ verifyNoMoreInteractions(prop);
+ }
+
+ @Test
+ public void testCreateFromSerializable() throws Exception {
+ Object value = new HashMap<>();
+ JcrPropertyMapCacheEntry entry = new JcrPropertyMapCacheEntry(value, node);
+ Object propValue = entry.getPropertyValue();
+ assertTrue(propValue instanceof HashMap);
+ }
+
+ @Test
+ public void testCreateFromUnstorableValue() throws Exception {
+ try {
+ Object value = new TestClass();
+ new JcrPropertyMapCacheEntry(value, node);
+ fail("IllegalArgumentException expected");
+ } catch (IllegalArgumentException e) {
+ // success
+ }
+ }
+
+ private static final class TestClass {};
}
\ No newline at end of file
