You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by en...@apache.org on 2020/09/28 02:40:57 UTC

[sling-org-apache-sling-api] branch master updated: SLING-9774 The ValueMapUtil#merge behavior is different from the deprecated CompositeValueMap

This is an automated email from the ASF dual-hosted git repository.

enorman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-api.git


The following commit(s) were added to refs/heads/master by this push:
     new ebe211f  SLING-9774 The ValueMapUtil#merge behavior is different from the deprecated CompositeValueMap
ebe211f is described below

commit ebe211f16cc8d7ca6f1c61c9338674d8fcc99ad6
Author: Eric Norman <en...@apache.org>
AuthorDate: Sun Sep 27 19:40:46 2020 -0700

    SLING-9774 The ValueMapUtil#merge behavior is different from the
    deprecated CompositeValueMap
---
 .../sling/api/wrappers/impl/MergingValueMap.java   | 41 +++++++++++++++++++---
 .../api/resource/impl/ValueMapUtilMergeTest.java   | 19 ++++++++++
 2 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/src/main/java/org/apache/sling/api/wrappers/impl/MergingValueMap.java b/src/main/java/org/apache/sling/api/wrappers/impl/MergingValueMap.java
index 9e2bef3..6cd2e5f 100644
--- a/src/main/java/org/apache/sling/api/wrappers/impl/MergingValueMap.java
+++ b/src/main/java/org/apache/sling/api/wrappers/impl/MergingValueMap.java
@@ -18,10 +18,6 @@
  */
 package org.apache.sling.api.wrappers.impl;
 
-import org.apache.sling.api.resource.ValueMap;
-import org.apache.sling.api.wrappers.ValueMapUtil;
-import org.jetbrains.annotations.NotNull;
-
 import java.util.AbstractMap;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -29,10 +25,15 @@ import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import org.apache.sling.api.resource.ValueMap;
+import org.apache.sling.api.wrappers.ValueMapUtil;
+import org.jetbrains.annotations.NotNull;
+
 /**
  * Merge provided {@code ValueMaps} into a single view {@code ValueMap} that aggregates
  * all key-value pairs of the given maps. The value for a key-value pair is taken from
@@ -108,12 +109,42 @@ public class MergingValueMap implements ValueMap {
     @Override
     public Object get(Object key) {
         return valueMaps.stream()
-                .map(vm -> vm.get(key))
+                .map(vm -> getOrEmpty(vm, key)) // SLING-9774
                 .filter(Objects::nonNull)
                 .findFirst()
+                .filter(Optional::isPresent)    // skip if Optional#empty
+                .map(Optional::get)             // dig the value out of the Optional
                 .orElse(null);
     }
 
+    /**
+     * SLING-9774 - Returns an {@code Optional} describing the value in the map value for
+     * the supplied key.  Special handling is supplied for an existing key that
+     * resolves to a null value.
+     *
+     * @param vm the map to get the value from
+     * @param key the key to get the value for
+     * @return an {@code Optional} with a present value if the specified value
+     * is non-null, otherwise if the map contains the key then an empty {@code Optional},
+     * otherwise null
+     */
+    private Optional<Object> getOrEmpty(ValueMap vm, Object key) {
+        Optional<Object> opt = null;
+        Object value = vm.get(key);
+        if (value == null) {
+            // check if the null value is for a real entry
+            if (vm.containsKey(key)) {
+                // key exists so return the empty Optional
+                //   instead of null
+                opt = Optional.empty();
+            }
+        } else {
+            // not null so just wrap the value
+            opt = Optional.of(value);
+        }
+        return opt;
+    }
+
     @NotNull
     @Override
     public Set<String> keySet() {
diff --git a/src/test/java/org/apache/sling/api/resource/impl/ValueMapUtilMergeTest.java b/src/test/java/org/apache/sling/api/resource/impl/ValueMapUtilMergeTest.java
index 470955a..c107cdb 100644
--- a/src/test/java/org/apache/sling/api/resource/impl/ValueMapUtilMergeTest.java
+++ b/src/test/java/org/apache/sling/api/resource/impl/ValueMapUtilMergeTest.java
@@ -157,6 +157,25 @@ public class ValueMapUtilMergeTest {
         typicalVM().clear();
     }
 
+    /**
+     * SLING-9774 - Test that a key=null value in the first map is used
+     * instead of the key=value entry in the second map
+     */
+    @Test
+    public void testNullValueForExistingKey() {
+        // value is null in the first map
+        ValueMap v1 = createValueMap("k1", null);
+
+        // value is not null in the second map
+        ValueMap v2 = createValueMap("k1", "11");
+
+        // expected to get null value since the first
+        //   map had a real key
+        ValueMap merge = merge(v1, v2);
+        assertTrue(merge.containsKey("k1"));
+        assertNull(merge.get("k1"));
+    }
+
     private static ValueMap createValueMap(Object... pairs) {
         ValueMap vm = new ValueMapDecorator(new HashMap<>());
         for (int i = 0; i < pairs.length; i += 2) {