This is an automated email from the ASF dual-hosted git repository. kwin pushed a commit to branch feature/collections-as-mulltivalue-props in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-resource.git
commit 528a5f205cf8397129a74fe8786a212a1bf8b100 Author: Konrad Windszus <[email protected]> AuthorDate: Mon Sep 22 18:54:12 2025 +0200 SLING-12811 Support collection types for reading from/writing to multivalue properties --- .../internal/helper/JcrPropertyMapCacheEntry.java | 187 +++++++++++---------- .../internal/JcrModifiableValueMapTest.java | 32 +++- .../helper/JcrPropertyMapCacheEntryTest.java | 59 ++++--- 3 files changed, 168 insertions(+), 110 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 33ee26d..29f23a4 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 @@ -30,6 +30,7 @@ import java.nio.charset.StandardCharsets; import java.time.ZonedDateTime; import java.util.ArrayList; import java.util.Calendar; +import java.util.Collection; import java.util.Date; import java.util.List; @@ -61,12 +62,10 @@ public class JcrPropertyMapCacheEntry { /** The value of the object. */ private final Object propertyValue; - /** - * Create a new cache entry from a property. + /** Create a new cache entry from a property. * * @param prop the property - * @throws RepositoryException if the provided property cannot be converted to a Java Object - */ + * @throws RepositoryException if the provided property cannot be converted to a Java Object */ public JcrPropertyMapCacheEntry(final @NotNull Property prop) throws RepositoryException { this.property = prop; this.isArray = prop.isMultiple(); @@ -77,19 +76,22 @@ public class JcrPropertyMapCacheEntry { } } - /** - * Create a new cache entry from a value. + /** Create a new cache entry from a value. + * * @param value the value * @param node the node - * @throws RepositoryException if the provided value cannot be stored - */ - public JcrPropertyMapCacheEntry(final @NotNull Object value, final @NotNull Node node) throws RepositoryException { + * @throws RepositoryException if the provided value cannot be stored */ + public JcrPropertyMapCacheEntry(@NotNull Object value, final @NotNull Node node) throws RepositoryException { this.property = null; - this.propertyValue = value; - this.isArray = value.getClass().isArray(); + if (value instanceof Collection) { + propertyValue = ((Collection<?>) value).toArray(); + } else { + propertyValue = value; + } + this.isArray = propertyValue.getClass().isArray(); // check if values can be stored in JCR if (isArray) { - final Object[] values = convertToObjectArray(value); + final Object[] values = convertToObjectArray(propertyValue); for (Object o : values) { failIfCannotStore(o, node); } @@ -110,16 +112,12 @@ public class JcrPropertyMapCacheEntry { } } - /** - * Create a value for the object. - * If the value type is supported directly through a jcr property type, - * the corresponding value is created. If the value is serializable, - * it is serialized through an object stream. Otherwise null is returned. + /** Create a value for the object. If the value type is supported directly through a jcr property type, the corresponding value is + * created. If the value is serializable, it is serialized through an object stream. Otherwise null is returned. * * @param obj the object - * @param node the node - * @return the converted value - */ + * @param node the node + * @return the converted value */ 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); @@ -138,11 +136,10 @@ public class JcrPropertyMapCacheEntry { return value; } - /** - * Convert the object to an array + /** Convert the object to an array + * * @param value The array - * @return an object array - */ + * @return an object array */ private static @NotNull Object[] convertToObjectArray(final @NotNull Object value) { final Object[] values; if (value instanceof long[]) { @@ -167,27 +164,24 @@ public class JcrPropertyMapCacheEntry { return values; } - /** - * Whether this value is an array or not - * @return {@code true} if an array. - */ + /** Whether this value is an array or not + * + * @return {@code true} if an array. */ public boolean isArray() { return this.isArray; } - /** - * Get the current property value. + /** Get the current property value. + * * @return The current value - * @throws RepositoryException If something goes wrong - */ + * @throws RepositoryException If something goes wrong */ public @NotNull Object getPropertyValue() throws RepositoryException { return this.propertyValue != null ? this.propertyValue : JcrResourceUtil.toJavaObject(property); } - /** - * Get the current property value. - * @return The current value or {@code null} if not possible. - */ + /** Get the current property value. + * + * @return The current value or {@code null} if not possible. */ public @Nullable Object getPropertyValueOrNull() { try { return getPropertyValue(); @@ -196,18 +190,17 @@ public class JcrPropertyMapCacheEntry { } } - /** - * Convert the default value to the given type + /** Convert the default value to the given type + * * @param type The type class * @param node The node * @param dynamicClassLoader The classloader * @param <T> The type - * @return The converted object - */ + * @return The converted object */ @SuppressWarnings("unchecked") - public @Nullable<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 { @@ -220,6 +213,8 @@ public class JcrPropertyMapCacheEntry { result = (T) convertToArray(sourceArray, type.getComponentType(), node, dynamicClassLoader); } else if (sourceArray.length > 0) { result = convertToType(-1, sourceArray[0], type, node, dynamicClassLoader); + } else if (sourceArray.length > 0) { + result = convertToType(-1, sourceArray[0], type, node, dynamicClassLoader); } } else { @@ -242,10 +237,10 @@ public class JcrPropertyMapCacheEntry { return result; } - private @NotNull<T> T[] convertToArray(final @NotNull Object source, - 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) { @@ -256,11 +251,11 @@ public class JcrPropertyMapCacheEntry { 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 { + + 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); @@ -275,28 +270,57 @@ public class JcrPropertyMapCacheEntry { return values.toArray(result); } + private @NotNull <T> T[] convertToCollection(final @NotNull Object[] sourceArray, + final @NotNull Class<? extends Collection> type, + final @NotNull Node node, + final @Nullable ClassLoader dynamicClassLoader) throws RepositoryException { + @SuppressWarnings("unchecked") + if (type.isAssignableFrom(List.class)) { + List<T> values = new ArrayList<>(); + for (int i = 0; i < sourceArray.length; i++) { + T value = convertToType(i, sourceArray[i], type.getComponentType(), node, dynamicClassLoader); + if (value != null) { + values.add(value); + } + } + return values.toArray((T[]) Array.newInstance(type.getComponentType(), values.size())); + } else if (type.isAssignableFrom(Set.class)) { + Set<T> values = new HashSet<>(); + for (int i = 0; i < sourceArray.length; i++) { + T value = convertToType(i, sourceArray[i], type.getComponentType(), node, dynamicClassLoader); + if (value != null) { + values.add(value); + } + } + return values.toArray((T[]) Array.newInstance(type.getComponentType(), values.size())); + } + + T[] result = (T[]) Array.newInstance(sourceCollection.getClass().getTypeParameters()[0].getClass(), sourceCollection.size()); + return sourceCollection.toArray(result); + } + @SuppressWarnings("unchecked") - 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 { + 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; } - + 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 { + 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 { @@ -305,19 +329,19 @@ public class JcrPropertyMapCacheEntry { // ignore and use fallback } - // any number: length of binary + // 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 + + // string: read binary } else if (String.class == type) { return (T) inputStreamToString(value); - - // any serializable + + // any serializable } else if (Serializable.class.isAssignableFrom(type)) { try (ObjectInputStream ois = new PropertyObjectInputStream(value, dynamicClassLoader)) { final Object obj = ois.readObject(); @@ -329,12 +353,12 @@ public class JcrPropertyMapCacheEntry { // 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()); @@ -342,7 +366,7 @@ public class JcrPropertyMapCacheEntry { return Long.valueOf(property.getLengths()[index]); } } - + private static @NotNull String inputStreamToString(@NotNull InputStream value) { try (InputStream in = value) { final ByteArrayOutputStream baos = new ByteArrayOutputStream(); @@ -360,8 +384,8 @@ public class JcrPropertyMapCacheEntry { } private @Nullable <T> T convert(final @NotNull Object value, - final @NotNull Class<T> type, - final @NotNull Node node) throws RepositoryException { + final @NotNull Class<T> type, + final @NotNull Node node) throws RepositoryException { if (String.class == type) { return (T) getConverter(value).toString(); @@ -410,12 +434,10 @@ public class JcrPropertyMapCacheEntry { return null; } - /** - * Create a converter for an object. + /** Create a converter for an object. * * @param value The object to convert - * @return A converter for {@code value} - */ + * @return A converter for {@code value} */ private static @NotNull Converter getConverter(final @NotNull Object value) { if (value instanceof Number) { // byte, short, int, long, double, float, BigDecimal @@ -433,10 +455,7 @@ public class JcrPropertyMapCacheEntry { return new StringConverter(value); } - /** - * This is an extended version of the object input stream which uses the - * thread context class loader. - */ + /** This is an extended version of the object input stream which uses the thread context class loader. */ private static class PropertyObjectInputStream extends ObjectInputStream { private final ClassLoader classloader; @@ -446,9 +465,7 @@ public class JcrPropertyMapCacheEntry { this.classloader = classLoader; } - /** - * @see java.io.ObjectInputStream#resolveClass(java.io.ObjectStreamClass) - */ + /** @see java.io.ObjectInputStream#resolveClass(java.io.ObjectStreamClass) */ @Override protected Class<?> resolveClass(final ObjectStreamClass classDesc) throws IOException, ClassNotFoundException { diff --git a/src/test/java/org/apache/sling/jcr/resource/internal/JcrModifiableValueMapTest.java b/src/test/java/org/apache/sling/jcr/resource/internal/JcrModifiableValueMapTest.java index b335add..413b708 100644 --- a/src/test/java/org/apache/sling/jcr/resource/internal/JcrModifiableValueMapTest.java +++ b/src/test/java/org/apache/sling/jcr/resource/internal/JcrModifiableValueMapTest.java @@ -19,6 +19,7 @@ package org.apache.sling.jcr.resource.internal; import static org.apache.sling.jcr.resource.internal.AssertCalendar.assertEqualsCalendar; +import static org.junit.Assert.assertArrayEquals; import java.io.ByteArrayInputStream; import java.io.InputStream; @@ -174,6 +175,31 @@ public class JcrModifiableValueMapTest extends SlingRepositoryTestBase { assertContains(pvm2, currentlyStored); } + public void testMultivaluePut() + throws Exception { + getSession().refresh(false); + final ModifiableValueMap pvm = new JcrModifiableValueMap(this.rootNode, getHelperData()); + assertContains(pvm, initialSet()); + assertNull(pvm.get("something")); + + // now put a multi-value property + final String[] values = {"a", "b", "c"}; + pvm.put("something", values); + pvm.put("something2", List.of("a", "b", "c")); + + final Map<String, Object> currentlyStored = this.initialSet(); + currentlyStored.put("something", values); + currentlyStored.put("something2", values); // Set is converted to array, original type is lost + assertContains(pvm, currentlyStored); + + getSession().save(); + assertContains(pvm, currentlyStored); + + final ValueMap pvm2 = new JcrValueMap(this.rootNode, getHelperData()); + assertContains(pvm2, currentlyStored); + assertEquals(Set.of("a", "b", "c"), pvm2.get("something2", Set.class)); + } + public void testRemove() throws Exception { getSession().refresh(false); @@ -344,7 +370,11 @@ public class JcrModifiableValueMapTest extends SlingRepositoryTestBase { private void assertContains(ValueMap map, Map<String, Object> values) { for (Map.Entry<String, Object> entry : values.entrySet()) { final Object stored = map.get(entry.getKey()); - assertEquals(values.get(entry.getKey()), stored); + if (stored.getClass().isArray()) { + assertArrayEquals((Object[])values.get(entry.getKey()), (Object[])stored); + } else { + assertEquals(values.get(entry.getKey()), stored); + } } } 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 dfebca3..f52d3cc 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,30 +18,6 @@ */ 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; -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; @@ -54,6 +30,33 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; +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.List; +import java.util.Map; + +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 org.apache.jackrabbit.value.BooleanValue; +import org.apache.jackrabbit.value.ValueFactoryImpl; +import org.junit.Before; +import org.junit.Test; + +import com.google.common.collect.Maps; + /** * Testcase for {@link JcrPropertyMapCacheEntry} */ @@ -124,6 +127,14 @@ public class JcrPropertyMapCacheEntryTest { new JcrPropertyMapCacheEntry(new char[0], node); verifyNoMoreInteractions(node); } + + @Test + public void testCollections() throws Exception { + JcrPropertyMapCacheEntry entry = new JcrPropertyMapCacheEntry(List.of("a", "b"), node); + assertTrue(entry.isArray()); + new JcrPropertyMapCacheEntry(List.of(1, 2), node); + assertTrue(entry.isArray()); + } @Test(expected = IllegalArgumentException.class) public void testCannotStore() throws Exception {
