This is an automated email from the ASF dual-hosted git repository. angela pushed a commit to branch SLING-11468 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-resource.git
commit 60634ebc5fc92cdd4370d7390cb88b74f5c6cec5 Author: angela <[email protected]> AuthorDate: Wed Jul 20 09:26:10 2022 +0200 SLING-11468 : Simplify JcrPropertyMapCacheEntry.convertToType --- .../sling/jcr/resource/internal/JcrValueMap.java | 6 +- .../internal/helper/JcrPropertyMapCacheEntry.java | 141 +++++++-------- .../helper/JcrPropertyMapCacheEntryTest.java | 191 ++++++++++++++++++++- 3 files changed, 260 insertions(+), 78 deletions(-) diff --git a/src/main/java/org/apache/sling/jcr/resource/internal/JcrValueMap.java b/src/main/java/org/apache/sling/jcr/resource/internal/JcrValueMap.java index f435770..8955971 100644 --- a/src/main/java/org/apache/sling/jcr/resource/internal/JcrValueMap.java +++ b/src/main/java/org/apache/sling/jcr/resource/internal/JcrValueMap.java @@ -251,11 +251,7 @@ public class JcrValueMap implements ValueMap { if (entry == null) { entry = new JcrPropertyMapCacheEntry(prop); cache.put(key, entry); - - final Object defaultValue = entry.getPropertyValue(); - if (defaultValue != null) { - valueCache.put(key, entry.getPropertyValue()); - } + valueCache.put(key, entry.getPropertyValue()); } return entry; } catch (final RepositoryException re) { 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 d5f1efd..f9e2091 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 @@ -284,79 +284,84 @@ public class JcrPropertyMapCacheEntry { if (type.isInstance(initialValue)) { return (T) initialValue; } + + if (initialValue instanceof InputStream) { + return convertInputStream(index, (InputStream) initialValue, type, node, dynamicClassLoader); + } else { + return convert(initialValue, type, node); + } + } + + private @Nullable <T> T convertInputStream(int index, + final @NotNull InputStream value, + final @NotNull Class<T> type, + final @NotNull Node node, + final @Nullable ClassLoader dynamicClassLoader) throws RepositoryException { + // object input stream + if (ObjectInputStream.class.isAssignableFrom(type)) { + try { + return (T) new PropertyObjectInputStream(value, dynamicClassLoader); + } catch (final IOException ioe) { + // ignore and use fallback + } - Object value = initialValue; - - // special case input stream first - if (value instanceof InputStream) { - // object input stream - if (ObjectInputStream.class.isAssignableFrom(type)) { - try { - return (T) new PropertyObjectInputStream((InputStream) value, dynamicClassLoader); - } catch (final IOException ioe) { - // ignore and use fallback - } - - // 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 { - value = Long.valueOf(this.property.getLengths()[index]); - } - - // string: read binary - } else if (String.class == type) { - final InputStream in = (InputStream) value; - try { - final ByteArrayOutputStream baos = new ByteArrayOutputStream(); - final byte[] buffer = new byte[2048]; - int l; - while ((l = in.read(buffer)) >= 0) { - if (l > 0) { - baos.write(buffer, 0, l); - } - } - value = new String(baos.toByteArray(), StandardCharsets.UTF_8); - } catch (final IOException e) { - throw new IllegalArgumentException(e); - } finally { - try { - in.close(); - } catch (final IOException ignore) { - // ignore - } + // 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; + } + return convert(propertyToLength(property, index), type, node); + + // string: read binary + } else if (String.class == type) { + return (T) inputStreamToString(value); + + // any serializable + } else if (Serializable.class.isAssignableFrom(type)) { + try (ObjectInputStream ois = new PropertyObjectInputStream(value, dynamicClassLoader)) { + final Object obj = ois.readObject(); + if (type.isInstance(obj)) { + return (T) obj; } - - // any serializable - } else if (Serializable.class.isAssignableFrom(type)) { - ObjectInputStream ois = null; - try { - ois = new PropertyObjectInputStream((InputStream) value, dynamicClassLoader); - final Object obj = ois.readObject(); - if (type.isInstance(obj)) { - return (T) obj; - } - value = obj; - } catch (final ClassNotFoundException | IOException cnfe) { - // ignore and use fallback - } finally { - if (ois != null) { - try { - ois.close(); - } catch (final IOException ignore) { - // ignore - } - } + return convert(obj, type, node); + } catch (final ClassNotFoundException | IOException cnfe) { + // ignore and use fallback + } + // ignore + } + + // fallback + return convert(value, type, node); + } + + private static @NotNull Long propertyToLength(@NotNull Property property, int index) throws RepositoryException { + if (index == -1) { + return Long.valueOf(property.getLength()); + } else { + return Long.valueOf(property.getLengths()[index]); + } + } + + private static @NotNull String inputStreamToString(@NotNull InputStream value) { + try (InputStream in = value) { + final ByteArrayOutputStream baos = new ByteArrayOutputStream(); + final byte[] buffer = new byte[2048]; + int l; + while ((l = in.read(buffer)) >= 0) { + if (l > 0) { + baos.write(buffer, 0, l); } } + return new String(baos.toByteArray(), StandardCharsets.UTF_8); + } catch (final IOException e) { + throw new IllegalArgumentException(e); } + } + private @Nullable <T> T convert(final @NotNull Object value, + final @NotNull Class<T> type, + final @NotNull Node node) throws RepositoryException { if (String.class == type) { return (T) getConverter(value).toString(); @@ -403,7 +408,7 @@ public class JcrPropertyMapCacheEntry { // fallback in case of unsupported type return null; - } + } /** * Create a converter for an object. 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 d53c08d..0a64ce4 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,6 +18,7 @@ */ package org.apache.sling.jcr.resource.internal.helper; +import com.google.common.collect.Maps; import org.apache.jackrabbit.value.BooleanValue; import org.apache.jackrabbit.value.ValueFactoryImpl; import org.junit.Before; @@ -26,16 +27,23 @@ import org.junit.Test; import javax.jcr.Node; import javax.jcr.Property; import javax.jcr.PropertyType; +import javax.jcr.RepositoryException; import javax.jcr.Session; +import javax.jcr.Value; import javax.jcr.ValueFactory; import javax.jcr.ValueFormatException; import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.InputStream; import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; import java.util.Calendar; import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.Map; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; @@ -119,6 +127,39 @@ public class JcrPropertyMapCacheEntryTest { verifyZeroInteractions(node); } + @Test + public void testCannotStore() throws Exception { + Object value = new TestClass(); + try { + new JcrPropertyMapCacheEntry(value, node); + fail("IllegalArgumentException expected"); + } catch (IllegalArgumentException e) { + // success + } + + try { + new JcrPropertyMapCacheEntry(new Object[] {value}, node); + fail("IllegalArgumentException expected"); + } catch (IllegalArgumentException e) { + // success + } + } + + @Test + public void testGetPropertyValueOrNull() throws Exception { + JcrPropertyMapCacheEntry entry = new JcrPropertyMapCacheEntry(true, node); + assertEquals(Boolean.TRUE, entry.getPropertyValueOrNull()); + } + + @Test + public void testGetPropertyValueOrNullWithRepositoryException() throws Exception { + Property prop = mock(Property.class); + when(prop.getType()).thenReturn(PropertyType.BINARY); + when(prop.getValue()).thenThrow(new RepositoryException()); + JcrPropertyMapCacheEntry entry = new JcrPropertyMapCacheEntry(prop); + assertNull(entry.getPropertyValueOrNull()); + } + @Test public void testInputStreamToString() throws Exception { InputStream in = new ByteArrayInputStream("test".getBytes()); @@ -151,7 +192,7 @@ public class JcrPropertyMapCacheEntryTest { } @Test - public void testBinaryPropertyToLong() throws Exception { + public void testBinaryPropertyToInteger() throws Exception { Property prop = mock(Property.class); when(prop.getType()).thenReturn(PropertyType.BINARY); when(prop.getStream()).thenReturn(new ByteArrayInputStream("10".getBytes())); @@ -159,8 +200,8 @@ public class JcrPropertyMapCacheEntryTest { when(prop.getLength()).thenReturn(2L); JcrPropertyMapCacheEntry entry = new JcrPropertyMapCacheEntry(prop); - Long result = entry.convertToType(Long.class, node, null); - assertEquals(Long.valueOf(2), result); + Integer result = entry.convertToType(Integer.class, node, null); + assertEquals(Integer.valueOf(2), result); verify(prop, times(2)).isMultiple(); verify(prop).getValue(); @@ -193,6 +234,31 @@ public class JcrPropertyMapCacheEntryTest { verifyNoMoreInteractions(prop); verifyZeroInteractions(node); } + + @Test + public void testMvBinaryPropertyToFloatArray() throws Exception { + Property prop = mock(Property.class); + when(prop.getType()).thenReturn(PropertyType.BINARY); + when(prop.isMultiple()).thenReturn(true); + when(prop.getValue()).thenThrow(new ValueFormatException("multi-valued")); + Value[] vs = new Value[] {vf.createValue("10.7", PropertyType.BINARY), vf.createValue("10.7", PropertyType.BINARY)}; + when(prop.getValues()).thenReturn(vs); + when(prop.getLength()).thenThrow(new ValueFormatException("multi-valued")); + when(prop.getLengths()).thenReturn(new long[] {4L, 4L}); + + JcrPropertyMapCacheEntry entry = new JcrPropertyMapCacheEntry(prop); + Float[] result = entry.convertToType(Float[].class, node, null); + assertNotNull(result); + assertEquals(2, result.length); + assertEquals(Float.valueOf(4.0f), result[0]); + + verify(prop, times(2)).isMultiple(); + verify(prop).getValues(); + verify(prop).getType(); + verify(prop, times(2)).getLengths(); + verifyNoMoreInteractions(prop); + verifyZeroInteractions(node); + } @Test public void testInputStreamToObjectInputStream() throws Exception { @@ -203,11 +269,38 @@ public class JcrPropertyMapCacheEntryTest { assertNull(result); // TODO: is this the expected result? verifyZeroInteractions(node); } + + @Test + public void testInputStreamToSerializableSameType() throws Exception { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + try (ObjectOutputStream oos = new ObjectOutputStream(out)) { + oos.writeObject(Maps.newHashMap()); + } + + JcrPropertyMapCacheEntry entry = new JcrPropertyMapCacheEntry(new ByteArrayInputStream(out.toByteArray()), node); + // same type + Map<?,?> result = entry.convertToType(HashMap.class, node, null); + assertNotNull(result); + verifyZeroInteractions(node); + } + + @Test + public void testInputStreamToSerializable() throws Exception { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + try (ObjectOutputStream oos = new ObjectOutputStream(out)) { + oos.writeObject(new LinkedHashMap<>()); + } + + JcrPropertyMapCacheEntry entry = new JcrPropertyMapCacheEntry(new ByteArrayInputStream(out.toByteArray()), node); + // different type that cannot be converted + Calendar result = entry.convertToType(Calendar.class, node, LinkedHashMap.class.getClassLoader()); + assertNull(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())); @@ -224,12 +317,100 @@ public class JcrPropertyMapCacheEntryTest { verifyZeroInteractions(node); } + @Test + public void testMvPropertyToBoolean() throws Exception { + Property prop = mock(Property.class); + when(prop.getType()).thenReturn(PropertyType.STRING); + when(prop.isMultiple()).thenReturn(true); + when(prop.getValue()).thenThrow(new ValueFormatException("multi-valued")); + Value[] vs = new Value[] {vf.createValue("true"), vf.createValue("false")}; + when(prop.getValues()).thenReturn(vs); + + JcrPropertyMapCacheEntry entry = new JcrPropertyMapCacheEntry(prop); + assertTrue(entry.isArray()); + + Boolean result = entry.convertToType(Boolean.class, node, null); + assertNotNull(result); + assertTrue(result); + + verify(prop, times(2)).isMultiple(); + verify(prop).getValues(); + verify(prop).getType(); + verifyNoMoreInteractions(prop); + verifyZeroInteractions(node); + } + + @Test + public void testEmptyMvPropertyToString() throws Exception { + Property prop = mock(Property.class); + when(prop.getType()).thenReturn(PropertyType.STRING); + when(prop.isMultiple()).thenReturn(true); + when(prop.getValue()).thenThrow(new ValueFormatException("multi-valued")); + Value[] vs = new Value[0]; + when(prop.getValues()).thenReturn(vs); + + JcrPropertyMapCacheEntry entry = new JcrPropertyMapCacheEntry(prop); + assertTrue(entry.isArray()); + + String result = entry.convertToType(String.class, node, null); + assertNull(result); + + verify(prop, times(2)).isMultiple(); + verify(prop).getValues(); + verify(prop).getType(); + verifyNoMoreInteractions(prop); + verifyZeroInteractions(node); + } + + @Test + public void testConversionFails() throws RepositoryException { + Property prop = mock(Property.class); + when(prop.getType()).thenReturn(PropertyType.STRING); + when(prop.isMultiple()).thenReturn(false); + when(prop.getValue()).thenReturn(vf.createValue("string")); + + JcrPropertyMapCacheEntry entry = new JcrPropertyMapCacheEntry(prop); + assertFalse(entry.isArray()); + + Short result = entry.convertToType(Short.class, node, null); + assertNull(result); + + verify(prop, times(2)).isMultiple(); + verify(prop).getValue(); + verify(prop).getType(); + verifyNoMoreInteractions(prop); + verifyZeroInteractions(node); + } + + @Test + public void testStringToValue() throws RepositoryException { + Property prop = mock(Property.class); + when(prop.getType()).thenReturn(PropertyType.STRING); + when(prop.isMultiple()).thenReturn(false); + when(prop.getValue()).thenReturn(vf.createValue("string")); + + JcrPropertyMapCacheEntry entry = new JcrPropertyMapCacheEntry(prop); + assertFalse(entry.isArray()); + + Value result = entry.convertToType(Value.class, node, null); + assertNotNull(result); + assertEquals(PropertyType.STRING, result.getType()); + assertEquals("string", result.getString()); + + verify(prop, times(2)).isMultiple(); + verify(prop).getValue(); + verify(prop).getType(); + verify(node).getSession(); + verifyNoMoreInteractions(prop, 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);
