This is an automated email from the ASF dual-hosted git repository. angela pushed a commit to branch SLING-11465_SLING-11466 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-resource.git
commit d0c86d5cbbd81137a075484720d160e1d9971176 Author: angela <[email protected]> AuthorDate: Tue Jul 19 15:02:48 2022 +0200 SLING-11465 : NPE in JcrPropertyMapCacheEntry when converting from InputStream value to Number, SLING-11466 : JcrPropertyMapCacheEntry: ValueFormatException when converting value InputStream to number-array --- .../internal/helper/JcrPropertyMapCacheEntry.java | 38 ++-- .../helper/JcrPropertyMapCacheEntryTest.java | 202 ++++++++++++++++++++- 2 files changed, 218 insertions(+), 22 deletions(-) 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..6aac379 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 @@ public class JcrPropertyMapCacheEntry { * @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 @@ public class JcrPropertyMapCacheEntry { } } - 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 @@ public class JcrPropertyMapCacheEntry { * @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 @@ public class JcrPropertyMapCacheEntry { * @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 @@ public class JcrPropertyMapCacheEntry { * @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 @@ public class JcrPropertyMapCacheEntry { * 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 @@ public class JcrPropertyMapCacheEntry { * @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 { @@ -242,10 +242,10 @@ public class JcrPropertyMapCacheEntry { 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[] 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 +261,11 @@ public class JcrPropertyMapCacheEntry { } @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; } @@ -391,7 +391,7 @@ public class JcrPropertyMapCacheEntry { * @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..90ff8d8 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 @@ public class JcrPropertyMapCacheEntryTest { 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); + assertEquals(Long.valueOf(2), 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(1, result.length); + assertEquals(Integer.valueOf(2), result[0]); + 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(10.7), result[0]); + + verify(prop, times(2)).isMultiple(); + verify(prop).getValue(); + verify(prop).getType(); + 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
