svn commit: r1857818 - in /ofbiz/ofbiz-framework/trunk/framework/base/src: main/java/org/apache/ofbiz/base/util/collections/ test/java/org/apache/ofbiz/base/util/collections/

[mthl]

Author: mthl
Date: Fri Apr 19 15:02:25 2019
New Revision: 1857818

URL: http://svn.apache.org/viewvc?rev=1857818&view=rev
Log:
Fixed: Ensure that ‘MapContext’ preserves insertion order properties
(OFBIZ-10933)

This fixes a regression introduced in revision 1837462.

When pushing a ‘LinkedHashMap’ inside a ‘MapContext’, the iteration
order of the ‘MapContext’ values was not corresponding to the
insertion order of the embedded ‘LinkedHashMap’ which is important in
the ‘ControllerConfig’ case where configuration elements are stored in
‘LinkedHashMap’ objects and the ‘include’ mechanism relies on
‘MapContext’.

To avoid such regression in the future, a test reproducing the bug has
been added.

Thanks: Gil Portenseigne for investigating.

Added:
    ofbiz/ofbiz-framework/trunk/framework/base/src/test/java/org/apache/ofbiz/base/util/collections/
    ofbiz/ofbiz-framework/trunk/framework/base/src/test/java/org/apache/ofbiz/base/util/collections/MapContextTest.java   (with props)
Modified:
    ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapContext.java

Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapContext.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapContext.java?rev=1857818&r1=1857817&r2=1857818&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapContext.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapContext.java Fri Apr 19 15:02:25 2019
@@ -28,6 +28,7 @@ import java.util.Collections;
 import java.util.Deque;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.LinkedList;
 import java.util.Locale;
 import java.util.Map;
@@ -213,8 +214,8 @@ public class MapContext<K, V> implements
     @Override
     public Set<Map.Entry<K, V>> entrySet() {
         return entryStream()
-                // Don't use Collectors#toSet() which does not use encounter order.
-                .collect(collectingAndThen(toCollection(HashSet::new), Collections::unmodifiableSet));
+                // Use LinkedHashSet for users relying on the insertion order of the inner maps.
+                .collect(collectingAndThen(toCollection(LinkedHashSet::new), Collections::unmodifiableSet));
     }
 
     @Override

Added: ofbiz/ofbiz-framework/trunk/framework/base/src/test/java/org/apache/ofbiz/base/util/collections/MapContextTest.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/test/java/org/apache/ofbiz/base/util/collections/MapContextTest.java?rev=1857818&view=auto
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/base/src/test/java/org/apache/ofbiz/base/util/collections/MapContextTest.java (added)
+++ ofbiz/ofbiz-framework/trunk/framework/base/src/test/java/org/apache/ofbiz/base/util/collections/MapContextTest.java Fri Apr 19 15:02:25 2019
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.ofbiz.base.util.collections;
+
+import static org.hamcrest.Matchers.contains;
+import static org.junit.Assert.assertThat;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.ofbiz.base.util.UtilMisc;
+import org.apache.ofbiz.webapp.control.ConfigXMLReader.ControllerConfig;
+import org.junit.Test;
+
+public class MapContextTest {
+
+    /**
+     * A node containing properties and including other nodes.
+     *
+     * This class is simplification of the Controller configuration objects
+     * useful to test {@code MapContext} objects.
+     *
+     * @see ControllerConfig
+     */
+    static class PNode<K, V> {
+        /** The properties of the node. */
+        public Map<K, V> props;
+        /** The included identifier of nodes. */
+        private List<PNode<K, V>> includes;
+
+        /**
+         * Constructs a node without properties.
+         *
+         * @param includes  the included nodes
+         */
+        @SafeVarargs
+        public PNode(PNode<K, V>... includes) {
+            this(Collections.emptyMap(), includes);
+        }
+
+        /**
+         * Constructs a node with some properties.
+         *
+         * @param props  the properties of the node
+         * @param includes  the included nodes
+         */
+        @SafeVarargs
+        public PNode(Map<K, V> props, PNode<K, V>... includes) {
+            this.props = props;
+            this.includes = Arrays.asList(includes);
+        }
+
+        /**
+         * Combines the properties of included nodes.
+         *
+         * @return a map context containing the properties of the tree.
+         */
+        public MapContext<K, V> allProps() {
+            MapContext<K, V> res = new MapContext<>();
+            includes.forEach(inc -> res.push(inc.allProps()));
+            res.push(props);
+            return res;
+        }
+    }
+
+    // Checks that the order warranty of LinkedHashMap objects are preserved
+    // when pushing them in a MapContext.
+    @Test
+    public void ControllerConfigLikeContext() {
+        Map<String, String> propsA =
+                UtilMisc.toMap(LinkedHashMap::new, "aa", "1", "ab", "1");
+        Map<String, String> propsB =
+                UtilMisc.toMap(LinkedHashMap::new, "ba", "3", "bb", "8", "bc", "1", "bd", "14");
+        PNode<String, String> pn = new PNode<>(propsA,
+                new PNode<>(propsB, new PNode<>(), new PNode<>()),
+                new PNode<>(new PNode<>()),
+                new PNode<>());
+
+        MapContext<String, String> mc = pn.allProps();
+        assertThat("insertion order of LinkedHashMap is preserved by the 'values' method",
+                mc.values(), contains("1", "1", "3", "8", "1", "14"));
+    }
+}

Propchange: ofbiz/ofbiz-framework/trunk/framework/base/src/test/java/org/apache/ofbiz/base/util/collections/MapContextTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: ofbiz/ofbiz-framework/trunk/framework/base/src/test/java/org/apache/ofbiz/base/util/collections/MapContextTest.java
------------------------------------------------------------------------------
    svn:keywords = Date Rev Author URL Id

Propchange: ofbiz/ofbiz-framework/trunk/framework/base/src/test/java/org/apache/ofbiz/base/util/collections/MapContextTest.java
------------------------------------------------------------------------------
    svn:mime-type = text/plain