svn commit: r1837462 - in /ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections: MapContext.java MapStack.java

[Taher Alkhateeb]

Author: taher
Date: Sun Aug  5 12:36:09 2018
New Revision: 1837462

URL: http://svn.apache.org/viewvc?rev=1837462&view=rev
Log:
Improved: Further refactor MapContext and MapStack
(OFBIZ-10484)

This commit applies multiple new refactoring enhancements as follows:
- rename stackList to contexts (the data structure holding the context Deque)
- refactor the size function to utilize streams to sum all keys
- Implement a function "entryStream()" that returns a stream of all keys in the
  correct sequential order and utilize this function in multiple functions for
  iterating over the keys including "containsValue", "values" and "entrySet".
  This implementation is necessary to avoid code repetition and ensure correct
  order with no duplicates
- Re-design the get functions of the context map to use a generic function with
  a functional interface "withMapContainingKey". The purpose of it is to avoid
  checking for nulls and instead use the "containsKey" function followed by the
  injected lambda expressions in both signatures of the get function

Thanks: Mathieu Lirzin for the patch and implementing suggested changes.

Modified:
    ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapContext.java
    ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapStack.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=1837462&r1=1837461&r2=1837462&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 Sun Aug  5 12:36:09 2018
@@ -18,34 +18,41 @@
  *******************************************************************************/
 package org.apache.ofbiz.base.util.collections;
 
+import static java.util.stream.Collectors.collectingAndThen;
+import static java.util.stream.Collectors.toCollection;
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toSet;
+
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Deque;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedList;
-import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
-import java.util.stream.Collectors;
+import java.util.function.Function;
+import java.util.stream.Stream;
 
 import org.apache.ofbiz.base.util.UtilGenerics;
 
-
 /**
- * Map Stack
+ * Map Context
  *
+ * Provide a combined view for a collection of maps which are organized in a deque.
+ * All write operations affect only the head of the deque.
  */
 public class MapContext<K, V> implements Map<K, V>, LocalizedMap<V> {
 
     public static final String module = MapContext.class.getName();
 
-    protected Deque<Map<K, V>> stackList = new LinkedList<>();
+    protected Deque<Map<K, V>> contexts = new LinkedList<>();
 
     /** Puts a new Map on the top of the stack */
     public void push() {
-        stackList.addFirst(new HashMap<K, V>());
+        contexts.addFirst(new HashMap<K, V>());
     }
 
     /** Puts an existing Map on the top of the stack (top meaning will override lower layers on the stack) */
@@ -53,7 +60,7 @@ public class MapContext<K, V> implements
         if (existingMap == null) {
             throw new IllegalArgumentException("Error: cannot push null existing Map onto a MapContext");
         }
-        stackList.addFirst(existingMap);
+        contexts.addFirst(existingMap);
     }
 
     /** Puts an existing Map on the BOTTOM of the stack (bottom meaning will be overriden by lower layers on the stack, ie everything else already there) */
@@ -61,13 +68,13 @@ public class MapContext<K, V> implements
         if (existingMap == null) {
             throw new IllegalArgumentException("Error: cannot add null existing Map to bottom of a MapContext");
         }
-        stackList.addLast(existingMap);
+        contexts.addLast(existingMap);
     }
 
     /** Remove and returns the Map from the top of the stack; if there is only one Map on the stack it returns null and does not remove it */
     public Map<K, V> pop() {
         // always leave at least one Map in the List, ie never pop off the last Map
-        return stackList.size() > 1 ? stackList.removeFirst() : null;
+        return contexts.size() > 1 ? contexts.removeFirst() : null;
     }
 
     /* (non-Javadoc)
@@ -75,10 +82,11 @@ public class MapContext<K, V> implements
      */
     @Override
     public int size() {
-        // a little bit tricky; to represent the apparent size we need to aggregate all keys and get a count of unique keys
-        // this is a bit of a slow way, but gets the best number possible
-        Set<K> keys = this.keySet();
-        return keys.size();
+        return contexts.stream()
+                .flatMap(ctx -> ctx.keySet().stream())
+                .distinct()
+                .mapToInt(k -> 1)
+                .sum();
     }
 
     /* (non-Javadoc)
@@ -86,7 +94,16 @@ public class MapContext<K, V> implements
      */
     @Override
     public boolean isEmpty() {
-        return stackList.stream().allMatch(Map::isEmpty);
+        return contexts.stream().allMatch(Map::isEmpty);
+    }
+
+    // Return a sequential stream of actual entries.
+    private Stream<Map.Entry<K, V>> entryStream() {
+        Set<K> seenKeys = new HashSet<>();
+        return contexts.stream()
+                .flatMap(ctx -> ctx.entrySet().stream())
+                .sequential()
+                .filter(e -> seenKeys.add(e.getKey()));
     }
 
     /* (non-Javadoc)
@@ -94,7 +111,7 @@ public class MapContext<K, V> implements
      */
     @Override
     public boolean containsKey(Object key) {
-        return stackList.stream().anyMatch(m -> m.containsKey(key));
+        return contexts.stream().anyMatch(ctx -> ctx.containsKey(key));
     }
 
     /* (non-Javadoc)
@@ -102,25 +119,18 @@ public class MapContext<K, V> implements
      */
     @Override
     public boolean containsValue(Object value) {
-        // walk the stackList and the entries for each Map and if nothing is in for the current key, consider it an option, otherwise ignore
-        Set<K> resultKeySet = new HashSet<>();
-        for (Map<K, V> curMap: this.stackList) {
-            for (Map.Entry<K, V> curEntry: curMap.entrySet()) {
-                if (!resultKeySet.contains(curEntry.getKey())) {
-                    resultKeySet.add(curEntry.getKey());
-                    if (value == null) {
-                        if (curEntry.getValue() == null) {
-                            return true;
-                        }
-                    } else {
-                        if (value.equals(curEntry.getValue())) {
-                            return true;
-                        }
-                    }
-                }
+        return entryStream().anyMatch(e -> Objects.equals(value, e.getValue()));
+    }
+
+    private V withContextContainingKey(Object key, Function<Map<K, V>, V> f) {
+        for (Map<K, V> ctx: contexts) {
+            /* Use `containsKey` rather than checking for null.
+               This allows a null value at the head of the deque to override the followings. */
+            if (ctx.containsKey(key)) {
+                return f.apply(ctx);
             }
         }
-        return false;
+        return null;
     }
 
     /* (non-Javadoc)
@@ -128,14 +138,7 @@ public class MapContext<K, V> implements
      */
     @Override
     public V get(Object key) {
-        // walk the stackList and for the first place it is found return true; otherwise refurn false
-        for (Map<K, V> curMap: this.stackList) {
-            // only return if the curMap contains the key, rather than checking for null; this allows a null at a lower level to override a value at a higher level
-            if (curMap.containsKey(key)) {
-                return curMap.get(key);
-            }
-        }
-        return null;
+        return withContextContainingKey(key, ctx -> ctx.get(key));
     }
 
     /* (non-Javadoc)
@@ -143,18 +146,13 @@ public class MapContext<K, V> implements
      */
     @Override
     public V get(String name, Locale locale) {
-        // walk the stackList and for the first place it is found return true; otherwise refurn false
-        for (Map<K, V> curMap: this.stackList) {
-            // only return if the curMap contains the key, rather than checking for null; this allows a null at a lower level to override a value at a higher level
-            if (curMap.containsKey(name)) {
-                if (curMap instanceof LocalizedMap<?>) {
-                    LocalizedMap<V> lmap = UtilGenerics.cast(curMap);
-                    return lmap.get(name, locale);
-                }
-                return curMap.get(name);
+        return withContextContainingKey(name, ctx -> {
+            if (ctx instanceof LocalizedMap<?>) {
+                LocalizedMap<V> lmap = UtilGenerics.cast(ctx);
+                return lmap.get(name, locale);
             }
-        }
-        return null;
+            return ctx.get(name);
+        });
     }
 
     /* (non-Javadoc)
@@ -162,8 +160,7 @@ public class MapContext<K, V> implements
      */
     @Override
     public V put(K key, V value) {
-        // all write operations are local: only put in the Map on the top of the stack
-        return stackList.getFirst().put(key, value);
+        return contexts.getFirst().put(key, value);
     }
 
     /* (non-Javadoc)
@@ -171,8 +168,7 @@ public class MapContext<K, V> implements
      */
     @Override
     public V remove(Object key) {
-        // all write operations are local: only remove from the Map on the top of the stack
-        return stackList.getFirst().remove(key);
+        return contexts.getFirst().remove(key);
     }
 
     /* (non-Javadoc)
@@ -180,8 +176,7 @@ public class MapContext<K, V> implements
      */
     @Override
     public void putAll(Map<? extends K, ? extends V> arg0) {
-        // all write operations are local: only put in the Map on the top of the stack
-        stackList.getFirst().putAll(arg0);
+        contexts.getFirst().putAll(arg0);
     }
 
     /* (non-Javadoc)
@@ -189,8 +184,7 @@ public class MapContext<K, V> implements
      */
     @Override
     public void clear() {
-        // all write operations are local: only clear the Map on the top of the stack
-        stackList.getFirst().clear();
+        contexts.getFirst().clear();
     }
 
     /* (non-Javadoc)
@@ -198,10 +192,9 @@ public class MapContext<K, V> implements
      */
     @Override
     public Set<K> keySet() {
-        Set<K> keys = stackList.stream()
-                .flatMap(m -> m.keySet().stream())
-                .collect(Collectors.toSet());
-        return Collections.unmodifiableSet(keys);
+        return contexts.stream()
+                .flatMap(ctx -> ctx.keySet().stream())
+                .collect(collectingAndThen(toSet(), Collections::unmodifiableSet));
     }
 
     /* (non-Javadoc)
@@ -209,18 +202,9 @@ public class MapContext<K, V> implements
      */
     @Override
     public Collection<V> values() {
-        // walk the stackList and the entries for each Map and if nothing is in for the current key, put it in
-        Set<K> resultKeySet = new HashSet<>();
-        List<V> resultValues = new LinkedList<>();
-        for (Map<K, V> curMap: this.stackList) {
-            for (Map.Entry<K, V> curEntry: curMap.entrySet()) {
-                if (!resultKeySet.contains(curEntry.getKey())) {
-                    resultKeySet.add(curEntry.getKey());
-                    resultValues.add(curEntry.getValue());
-                }
-            }
-        }
-        return Collections.unmodifiableCollection(resultValues);
+        return entryStream()
+                .map(Map.Entry::getValue)
+                .collect(collectingAndThen(toList(), Collections::unmodifiableCollection));
     }
 
     /* (non-Javadoc)
@@ -228,27 +212,18 @@ public class MapContext<K, V> implements
      */
     @Override
     public Set<Map.Entry<K, V>> entrySet() {
-        // walk the stackList and the entries for each Map and if nothing is in for the current key, put it in
-        Set<K> resultKeySet = new HashSet<>();
-        Set<Map.Entry<K, V>> resultEntrySet = new HashSet<>();
-        for (Map<K, V> curMap: this.stackList) {
-            for (Map.Entry<K, V> curEntry: curMap.entrySet()) {
-                if (!resultKeySet.contains(curEntry.getKey())) {
-                    resultKeySet.add(curEntry.getKey());
-                    resultEntrySet.add(curEntry);
-                }
-            }
-        }
-        return Collections.unmodifiableSet(resultEntrySet);
+        return entryStream()
+                // Don't use Collectors#toSet() which does not use encounter order.
+                .collect(collectingAndThen(toCollection(HashSet::new), Collections::unmodifiableSet));
     }
 
     @Override
     public String toString() {
         StringBuilder fullMapString = new StringBuilder();
         int curLevel = 0;
-        for (Map<K, V> curMap: this.stackList) {
+        for (Map<K, V> ctx: contexts) {
             fullMapString.append("============================== Start stack level " + curLevel + "\n");
-            for (Map.Entry<K, V> curEntry: curMap.entrySet()) {
+            for (Map.Entry<K, V> curEntry: ctx.entrySet()) {
 
                 fullMapString.append("==>[");
                 fullMapString.append(curEntry.getKey());

Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapStack.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapStack.java?rev=1837462&r1=1837461&r2=1837462&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapStack.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapStack.java Sun Aug  5 12:36:09 2018
@@ -41,9 +41,9 @@ public class MapStack<K> extends MapCont
     public static <K> MapStack<K> create(Map<K, Object> baseMap) {
         MapStack<K> newValue = new MapStack<>();
         if (baseMap instanceof MapStack) {
-            newValue.stackList.addAll(((MapStack<K>) baseMap).stackList);
+            newValue.contexts.addAll(((MapStack<K>) baseMap).contexts);
         } else {
-            newValue.stackList.addFirst(baseMap);
+            newValue.contexts.addFirst(baseMap);
         }
         return newValue;
     }
@@ -51,7 +51,7 @@ public class MapStack<K> extends MapCont
     /** Does a shallow copy of the internal stack of the passed MapStack; enables simultaneous stacks that share common parent Maps */
     public static <K> MapStack<K> create(MapStack<K> source) {
         MapStack<K> newValue = new MapStack<>();
-        newValue.stackList.addAll(source.stackList);
+        newValue.contexts.addAll(source.contexts);
         return newValue;
     }
 

[Jacques Le Roux]

This is rather OFBIZ-10485

Jacques


Le 05/08/2018 à 14:36, [hidden email] a écrit :

> Author: taher
> Date: Sun Aug  5 12:36:09 2018
> New Revision: 1837462
>
> URL: http://svn.apache.org/viewvc?rev=1837462&view=rev
> Log:
> Improved: Further refactor MapContext and MapStack
> (OFBIZ-10484)
>
> This commit applies multiple new refactoring enhancements as follows:
> - rename stackList to contexts (the data structure holding the context Deque)
> - refactor the size function to utilize streams to sum all keys
> - Implement a function "entryStream()" that returns a stream of all keys in the
>    correct sequential order and utilize this function in multiple functions for
>    iterating over the keys including "containsValue", "values" and "entrySet".
>    This implementation is necessary to avoid code repetition and ensure correct
>    order with no duplicates
> - Re-design the get functions of the context map to use a generic function with
>    a functional interface "withMapContainingKey". The purpose of it is to avoid
>    checking for nulls and instead use the "containsKey" function followed by the
>    injected lambda expressions in both signatures of the get function
>
> Thanks: Mathieu Lirzin for the patch and implementing suggested changes.
>
> Modified:
>      ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapContext.java
>      ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapStack.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=1837462&r1=1837461&r2=1837462&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 Sun Aug  5 12:36:09 2018
> @@ -18,34 +18,41 @@
>    *******************************************************************************/
>   package org.apache.ofbiz.base.util.collections;
>  
> +import static java.util.stream.Collectors.collectingAndThen;
> +import static java.util.stream.Collectors.toCollection;
> +import static java.util.stream.Collectors.toList;
> +import static java.util.stream.Collectors.toSet;
> +
>   import java.util.Collection;
>   import java.util.Collections;
>   import java.util.Deque;
>   import java.util.HashMap;
>   import java.util.HashSet;
>   import java.util.LinkedList;
> -import java.util.List;
>   import java.util.Locale;
>   import java.util.Map;
> +import java.util.Objects;
>   import java.util.Set;
> -import java.util.stream.Collectors;
> +import java.util.function.Function;
> +import java.util.stream.Stream;
>  
>   import org.apache.ofbiz.base.util.UtilGenerics;
>  
> -
>   /**
> - * Map Stack
> + * Map Context
>    *
> + * Provide a combined view for a collection of maps which are organized in a deque.
> + * All write operations affect only the head of the deque.
>    */
>   public class MapContext<K, V> implements Map<K, V>, LocalizedMap<V> {
>  
>       public static final String module = MapContext.class.getName();
>  
> -    protected Deque<Map<K, V>> stackList = new LinkedList<>();
> +    protected Deque<Map<K, V>> contexts = new LinkedList<>();
>  
>       /** Puts a new Map on the top of the stack */
>       public void push() {
> -        stackList.addFirst(new HashMap<K, V>());
> +        contexts.addFirst(new HashMap<K, V>());
>       }
>  
>       /** Puts an existing Map on the top of the stack (top meaning will override lower layers on the stack) */
> @@ -53,7 +60,7 @@ public class MapContext<K, V> implements
>           if (existingMap == null) {
>               throw new IllegalArgumentException("Error: cannot push null existing Map onto a MapContext");
>           }
> -        stackList.addFirst(existingMap);
> +        contexts.addFirst(existingMap);
>       }
>  
>       /** Puts an existing Map on the BOTTOM of the stack (bottom meaning will be overriden by lower layers on the stack, ie everything else already there) */
> @@ -61,13 +68,13 @@ public class MapContext<K, V> implements
>           if (existingMap == null) {
>               throw new IllegalArgumentException("Error: cannot add null existing Map to bottom of a MapContext");
>           }
> -        stackList.addLast(existingMap);
> +        contexts.addLast(existingMap);
>       }
>  
>       /** Remove and returns the Map from the top of the stack; if there is only one Map on the stack it returns null and does not remove it */
>       public Map<K, V> pop() {
>           // always leave at least one Map in the List, ie never pop off the last Map
> -        return stackList.size() > 1 ? stackList.removeFirst() : null;
> +        return contexts.size() > 1 ? contexts.removeFirst() : null;
>       }
>  
>       /* (non-Javadoc)
> @@ -75,10 +82,11 @@ public class MapContext<K, V> implements
>        */
>       @Override
>       public int size() {
> -        // a little bit tricky; to represent the apparent size we need to aggregate all keys and get a count of unique keys
> -        // this is a bit of a slow way, but gets the best number possible
> -        Set<K> keys = this.keySet();
> -        return keys.size();
> +        return contexts.stream()
> +                .flatMap(ctx -> ctx.keySet().stream())
> +                .distinct()
> +                .mapToInt(k -> 1)
> +                .sum();
>       }
>  
>       /* (non-Javadoc)
> @@ -86,7 +94,16 @@ public class MapContext<K, V> implements
>        */
>       @Override
>       public boolean isEmpty() {
> -        return stackList.stream().allMatch(Map::isEmpty);
> +        return contexts.stream().allMatch(Map::isEmpty);
> +    }
> +
> +    // Return a sequential stream of actual entries.
> +    private Stream<Map.Entry<K, V>> entryStream() {
> +        Set<K> seenKeys = new HashSet<>();
> +        return contexts.stream()
> +                .flatMap(ctx -> ctx.entrySet().stream())
> +                .sequential()
> +                .filter(e -> seenKeys.add(e.getKey()));
>       }
>  
>       /* (non-Javadoc)
> @@ -94,7 +111,7 @@ public class MapContext<K, V> implements
>        */
>       @Override
>       public boolean containsKey(Object key) {
> -        return stackList.stream().anyMatch(m -> m.containsKey(key));
> +        return contexts.stream().anyMatch(ctx -> ctx.containsKey(key));
>       }
>  
>       /* (non-Javadoc)
> @@ -102,25 +119,18 @@ public class MapContext<K, V> implements
>        */
>       @Override
>       public boolean containsValue(Object value) {
> -        // walk the stackList and the entries for each Map and if nothing is in for the current key, consider it an option, otherwise ignore
> -        Set<K> resultKeySet = new HashSet<>();
> -        for (Map<K, V> curMap: this.stackList) {
> -            for (Map.Entry<K, V> curEntry: curMap.entrySet()) {
> -                if (!resultKeySet.contains(curEntry.getKey())) {
> -                    resultKeySet.add(curEntry.getKey());
> -                    if (value == null) {
> -                        if (curEntry.getValue() == null) {
> -                            return true;
> -                        }
> -                    } else {
> -                        if (value.equals(curEntry.getValue())) {
> -                            return true;
> -                        }
> -                    }
> -                }
> +        return entryStream().anyMatch(e -> Objects.equals(value, e.getValue()));
> +    }
> +
> +    private V withContextContainingKey(Object key, Function<Map<K, V>, V> f) {
> +        for (Map<K, V> ctx: contexts) {
> +            /* Use `containsKey` rather than checking for null.
> +               This allows a null value at the head of the deque to override the followings. */
> +            if (ctx.containsKey(key)) {
> +                return f.apply(ctx);
>               }
>           }
> -        return false;
> +        return null;
>       }
>  
>       /* (non-Javadoc)
> @@ -128,14 +138,7 @@ public class MapContext<K, V> implements
>        */
>       @Override
>       public V get(Object key) {
> -        // walk the stackList and for the first place it is found return true; otherwise refurn false
> -        for (Map<K, V> curMap: this.stackList) {
> -            // only return if the curMap contains the key, rather than checking for null; this allows a null at a lower level to override a value at a higher level
> -            if (curMap.containsKey(key)) {
> -                return curMap.get(key);
> -            }
> -        }
> -        return null;
> +        return withContextContainingKey(key, ctx -> ctx.get(key));
>       }
>  
>       /* (non-Javadoc)
> @@ -143,18 +146,13 @@ public class MapContext<K, V> implements
>        */
>       @Override
>       public V get(String name, Locale locale) {
> -        // walk the stackList and for the first place it is found return true; otherwise refurn false
> -        for (Map<K, V> curMap: this.stackList) {
> -            // only return if the curMap contains the key, rather than checking for null; this allows a null at a lower level to override a value at a higher level
> -            if (curMap.containsKey(name)) {
> -                if (curMap instanceof LocalizedMap<?>) {
> -                    LocalizedMap<V> lmap = UtilGenerics.cast(curMap);
> -                    return lmap.get(name, locale);
> -                }
> -                return curMap.get(name);
> +        return withContextContainingKey(name, ctx -> {
> +            if (ctx instanceof LocalizedMap<?>) {
> +                LocalizedMap<V> lmap = UtilGenerics.cast(ctx);
> +                return lmap.get(name, locale);
>               }
> -        }
> -        return null;
> +            return ctx.get(name);
> +        });
>       }
>  
>       /* (non-Javadoc)
> @@ -162,8 +160,7 @@ public class MapContext<K, V> implements
>        */
>       @Override
>       public V put(K key, V value) {
> -        // all write operations are local: only put in the Map on the top of the stack
> -        return stackList.getFirst().put(key, value);
> +        return contexts.getFirst().put(key, value);
>       }
>  
>       /* (non-Javadoc)
> @@ -171,8 +168,7 @@ public class MapContext<K, V> implements
>        */
>       @Override
>       public V remove(Object key) {
> -        // all write operations are local: only remove from the Map on the top of the stack
> -        return stackList.getFirst().remove(key);
> +        return contexts.getFirst().remove(key);
>       }
>  
>       /* (non-Javadoc)
> @@ -180,8 +176,7 @@ public class MapContext<K, V> implements
>        */
>       @Override
>       public void putAll(Map<? extends K, ? extends V> arg0) {
> -        // all write operations are local: only put in the Map on the top of the stack
> -        stackList.getFirst().putAll(arg0);
> +        contexts.getFirst().putAll(arg0);
>       }
>  
>       /* (non-Javadoc)
> @@ -189,8 +184,7 @@ public class MapContext<K, V> implements
>        */
>       @Override
>       public void clear() {
> -        // all write operations are local: only clear the Map on the top of the stack
> -        stackList.getFirst().clear();
> +        contexts.getFirst().clear();
>       }
>  
>       /* (non-Javadoc)
> @@ -198,10 +192,9 @@ public class MapContext<K, V> implements
>        */
>       @Override
>       public Set<K> keySet() {
> -        Set<K> keys = stackList.stream()
> -                .flatMap(m -> m.keySet().stream())
> -                .collect(Collectors.toSet());
> -        return Collections.unmodifiableSet(keys);
> +        return contexts.stream()
> +                .flatMap(ctx -> ctx.keySet().stream())
> +                .collect(collectingAndThen(toSet(), Collections::unmodifiableSet));
>       }
>  
>       /* (non-Javadoc)
> @@ -209,18 +202,9 @@ public class MapContext<K, V> implements
>        */
>       @Override
>       public Collection<V> values() {
> -        // walk the stackList and the entries for each Map and if nothing is in for the current key, put it in
> -        Set<K> resultKeySet = new HashSet<>();
> -        List<V> resultValues = new LinkedList<>();
> -        for (Map<K, V> curMap: this.stackList) {
> -            for (Map.Entry<K, V> curEntry: curMap.entrySet()) {
> -                if (!resultKeySet.contains(curEntry.getKey())) {
> -                    resultKeySet.add(curEntry.getKey());
> -                    resultValues.add(curEntry.getValue());
> -                }
> -            }
> -        }
> -        return Collections.unmodifiableCollection(resultValues);
> +        return entryStream()
> +                .map(Map.Entry::getValue)
> +                .collect(collectingAndThen(toList(), Collections::unmodifiableCollection));
>       }
>  
>       /* (non-Javadoc)
> @@ -228,27 +212,18 @@ public class MapContext<K, V> implements
>        */
>       @Override
>       public Set<Map.Entry<K, V>> entrySet() {
> -        // walk the stackList and the entries for each Map and if nothing is in for the current key, put it in
> -        Set<K> resultKeySet = new HashSet<>();
> -        Set<Map.Entry<K, V>> resultEntrySet = new HashSet<>();
> -        for (Map<K, V> curMap: this.stackList) {
> -            for (Map.Entry<K, V> curEntry: curMap.entrySet()) {
> -                if (!resultKeySet.contains(curEntry.getKey())) {
> -                    resultKeySet.add(curEntry.getKey());
> -                    resultEntrySet.add(curEntry);
> -                }
> -            }
> -        }
> -        return Collections.unmodifiableSet(resultEntrySet);
> +        return entryStream()
> +                // Don't use Collectors#toSet() which does not use encounter order.
> +                .collect(collectingAndThen(toCollection(HashSet::new), Collections::unmodifiableSet));
>       }
>  
>       @Override
>       public String toString() {
>           StringBuilder fullMapString = new StringBuilder();
>           int curLevel = 0;
> -        for (Map<K, V> curMap: this.stackList) {
> +        for (Map<K, V> ctx: contexts) {
>               fullMapString.append("============================== Start stack level " + curLevel + "\n");
> -            for (Map.Entry<K, V> curEntry: curMap.entrySet()) {
> +            for (Map.Entry<K, V> curEntry: ctx.entrySet()) {
>  
>                   fullMapString.append("==>[");
>                   fullMapString.append(curEntry.getKey());
>
> Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapStack.java
> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapStack.java?rev=1837462&r1=1837461&r2=1837462&view=diff
> ==============================================================================
> --- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapStack.java (original)
> +++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapStack.java Sun Aug  5 12:36:09 2018
> @@ -41,9 +41,9 @@ public class MapStack<K> extends MapCont
>       public static <K> MapStack<K> create(Map<K, Object> baseMap) {
>           MapStack<K> newValue = new MapStack<>();
>           if (baseMap instanceof MapStack) {
> -            newValue.stackList.addAll(((MapStack<K>) baseMap).stackList);
> +            newValue.contexts.addAll(((MapStack<K>) baseMap).contexts);
>           } else {
> -            newValue.stackList.addFirst(baseMap);
> +            newValue.contexts.addFirst(baseMap);
>           }
>           return newValue;
>       }
> @@ -51,7 +51,7 @@ public class MapStack<K> extends MapCont
>       /** Does a shallow copy of the internal stack of the passed MapStack; enables simultaneous stacks that share common parent Maps */
>       public static <K> MapStack<K> create(MapStack<K> source) {
>           MapStack<K> newValue = new MapStack<>();
> -        newValue.stackList.addAll(source.stackList);
> +        newValue.contexts.addAll(source.contexts);
>           return newValue;
>       }
>  
>
>
>