Merge pull request #2213 from adamretter/hotfix/map-constructor-duplicate-keys

Map Constructor should not allow duplicate keys

Author: dizzzz

src/org/exist/xquery/ErrorCodes.java

@@ -131,8 +131,9 @@ public class ErrorCodes {
             "Bind the prefix xmlns to any namespace URI.\n" +
             "Bind a prefix to the namespace URI http://www.w3.org/2000/xmlns/.\n" +
             "Bind any prefix (including the empty prefix) to a zero-length namespace URI.");
-    public static final ErrorCode XQDY0138 = new W3CErrorCode("XQDY0138", "Position n does not exist in this array");
     public static final ErrorCode XQDY0102 = new W3CErrorCode("XQDY0102", "If the name of an element in an element constructor is in no namespace, creating a default namespace for that element using a computed namespace constructor is an error.");
+    public static final ErrorCode XQDY0137 = new W3CErrorCode("XQDY0137", "No two keys in a map may have the same key value");
+    public static final ErrorCode XQDY0138 = new W3CErrorCode("XQDY0138", "Position n does not exist in this array");
 
     /* XQuery 1.0 and XPath 2.0 Functions and Operators http://www.w3.org/TR/xpath-functions/#error-summary */
     public static final ErrorCode FOER0000 = new W3CErrorCode("FOER0000", "Unidentified error.");

src/org/exist/xquery/functions/map/MapExpr.java

@@ -15,21 +15,22 @@
  */
 public class MapExpr extends AbstractExpression {
 
-    protected List<Mapping> mappings = new ArrayList(13);
+    private final List<Mapping> mappings = new ArrayList<>(13);
 
-    public MapExpr(XQueryContext context) {
+    public MapExpr(final XQueryContext context) {
         super(context);
     }
 
-    public void map(PathExpr key, PathExpr value) {
+    public void map(final PathExpr key, final PathExpr value) {
         final Mapping mapping = new Mapping(key.simplify(), value.simplify());
         this.mappings.add(mapping);
     }
 
-    public void analyze(AnalyzeContextInfo contextInfo) throws XPathException {
-        if(getContext().getXQueryVersion() < 30){
+    @Override
+    public void analyze(final AnalyzeContextInfo contextInfo) throws XPathException {
+        if (getContext().getXQueryVersion() < 30) {
             throw new XPathException(this, ErrorCodes.EXXQDY0003,
-                "Map is not available before XQuery 3.0");
+                    "Map is not available before XQuery 3.0");
         }
         contextInfo.setParent(this);
         for (final Mapping mapping : this.mappings) {
@@ -38,35 +39,43 @@ public void analyze(AnalyzeContextInfo contextInfo) throws XPathException {
         }
     }
 
-    public Sequence eval(Sequence contextSequence, Item contextItem) throws XPathException {
-        if (contextItem != null)
-            {contextSequence = contextItem.toSequence();}
+    @Override
+    public Sequence eval(Sequence contextSequence, final Item contextItem) throws XPathException {
+        if (contextItem != null) {
+            contextSequence = contextItem.toSequence();
+        }
         final MapType map = new MapType(this.context);
         for (final Mapping mapping : this.mappings) {
             final Sequence key = mapping.key.eval(contextSequence);
-            if (key.getItemCount() != 1)
-                {throw new XPathException(MapErrorCode.EXMPDY001, "Expected single value for key, got " + key.getItemCount());}
+            if (key.getItemCount() != 1) {
+                throw new XPathException(this, MapErrorCode.EXMPDY001, "Expected single value for key, got " + key.getItemCount());
+            }
             final AtomicValue atomic = key.itemAt(0).atomize();
             final Sequence value = mapping.value.eval(contextSequence);
+            if (map.contains(atomic)) {
+                throw new XPathException(this, ErrorCodes.XQDY0137, "Key \"" + atomic.getStringValue() + "\" already exists in map.");
+            }
             map.add(atomic, value);
         }
         return map;
     }
 
+    @Override
     public int returnsType() {
         return Type.MAP;
     }
 
     @Override
-    public void accept(ExpressionVisitor visitor) {
+    public void accept(final ExpressionVisitor visitor) {
         super.accept(visitor);
-        for (final Mapping mapping: this.mappings) {
+        for (final Mapping mapping : this.mappings) {
             mapping.key.accept(visitor);
             mapping.value.accept(visitor);
         }
     }
 
-    public void dump(ExpressionDumper dumper) {
+    @Override
+    public void dump(final ExpressionDumper dumper) {
         dumper.display("map {");
         for (final Mapping mapping : this.mappings) {
             mapping.key.dump(dumper);
@@ -77,7 +86,7 @@ public void dump(ExpressionDumper dumper) {
     }
 
     @Override
-    public void resetState(boolean postOptimization) {
+    public void resetState(final boolean postOptimization) {
         super.resetState(postOptimization);
         mappings.forEach(m -> m.resetState(postOptimization));
     }
@@ -86,12 +95,12 @@ private static class Mapping {
         final Expression key;
         final Expression value;
 
-        public Mapping(Expression key, Expression value) {
+        public Mapping(final Expression key, final Expression value) {
             this.key = key;
             this.value = value;
         }
 
-        private void resetState(boolean postOptimization) {
+        private void resetState(final boolean postOptimization) {
             key.resetState(postOptimization);
             value.resetState(postOptimization);
         }