Skip to content
Permalink
Browse files

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.


git-svn-id: https://svn.apache.org/repos/asf/ofbiz/ofbiz-framework/trunk@1857818 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
mthl committed Apr 19, 2019
1 parent 60a6ac1 commit d7127ce9ead5a45763eb79cdd3313e8a47074ed4
@@ -28,6 +28,7 @@
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 void clear() {
@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
@@ -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"));
}
}

0 comments on commit d7127ce

Please sign in to comment.
You can’t perform that action at this time.