[refactor] Always sort internal-module FunctionDef[]; drop functionsOrdered flag (#6378)

exist-core/src/main/java/org/exist/backup/xquery/BackupModule.java

@@ -49,7 +49,7 @@ public class BackupModule extends AbstractInternalModule
 
     public BackupModule(final Map<String, List<?>> parameters)
     {
-        super( functions, parameters, true );
+        super(functions, parameters);
     }
 
     public String getNamespaceURI()

exist-core/src/main/java/org/exist/xquery/AbstractInternalModule.java

@@ -36,33 +36,59 @@
  * in name or the number of expected arguments. It is thus possible to implement
  * similar XQuery functions in one single class.
  *
+ * <p>The {@code FunctionDef[]} passed in does not need to be sorted; this constructor
+ * defensive-copies and sorts it by {@link FunctionId} order so {@link #getFunctionDef(QName, int)}
+ * can always use binary search. The original array reference is not mutated.</p>
+ *
  * @author <a href="mailto:wolfgang@exist-db.org">Wolfgang Meier</a>
  * @author ljo
  */
 public abstract class AbstractInternalModule implements InternalModule {
 
+    protected final FunctionDef[] mFunctions;
+    protected final Map<QName, Variable> mGlobalVariables = new HashMap<>();
+    private final Map<String, List<?>> parameters;
+
     public static class FunctionComparator implements Comparator<FunctionDef> {
         @Override
         public int compare(final FunctionDef o1, final FunctionDef o2) {
             return o1.getSignature().getFunctionId().compareTo(o2.getSignature().getFunctionId());
         }
     }
 
-    protected final FunctionDef[] mFunctions;
-    protected final boolean ordered;
-    private final Map<String, List<?>> parameters;
-
-    protected final Map<QName, Variable> mGlobalVariables = new HashMap<>();
-
     public AbstractInternalModule(final FunctionDef[] functions, final Map<String, List<?>> parameters) {
-        this(functions, parameters, false);
+        // Defensive-copy + sort so the caller's static final array is left intact
+        // and getFunctionDef() can binary-search regardless of declaration order.
+        // See https://github.com/eXist-db/exist/issues/6378 (and #6376 which surfaced
+        // the latent bug).
+        if (functions != null && functions.length > 1) {
+            final FunctionDef[] sorted = functions.clone();
+            Arrays.sort(sorted, new FunctionComparator());
+            this.mFunctions = sorted;
+        } else {
+            this.mFunctions = functions;
+        }
+        this.parameters = parameters;
     }
 
+    /**
+     * Pre-#6378 constructor that took a {@code functionsOrdered} flag. The flag is now
+     * ignored — the function table is always sorted and {@link #getFunctionDef(QName, int)}
+     * always uses binary search. Retained so external modules continue to compile against
+     * eXist 7 without source changes; call sites should migrate to
+     * {@link #AbstractInternalModule(FunctionDef[], Map)}.
+     *
+     * @param functions         the array of functions
+     * @param parameters        configuration parameters
+     * @param functionsOrdered  ignored as of #6378
+     *
+     * @deprecated since 7.0.0; the {@code functionsOrdered} parameter has no effect.
+     *             Use {@link #AbstractInternalModule(FunctionDef[], Map)}.
+     */
+    @Deprecated(since = "7.0.0", forRemoval = true)
     public AbstractInternalModule(final FunctionDef[] functions, final Map<String, List<?>> parameters,
-                                  final boolean functionsOrdered) {
-        this.mFunctions = functions;
-        this.ordered = functionsOrdered;
-        this.parameters = parameters;
+                                  @SuppressWarnings("unused") final boolean functionsOrdered) {
+        this(functions, parameters);
     }
 
     @Override
@@ -113,17 +139,7 @@ public Iterator<FunctionSignature> getSignaturesForFunction(final QName qname) {
 
     @Override
     public FunctionDef getFunctionDef(QName qname, int arity) {
-        final FunctionId id = new FunctionId(qname, arity);
-        if (ordered) {
-            return binarySearch(id);
-        } else {
-            for (FunctionDef mFunction : mFunctions) {
-                if (id.compareTo(mFunction.getSignature().getFunctionId()) == 0) {
-                    return mFunction;
-                }
-            }
-        }
-        return null;
+        return binarySearch(new FunctionId(qname, arity));
     }
 
     private FunctionDef binarySearch(final FunctionId id) {

exist-core/src/main/java/org/exist/xquery/functions/array/ArrayModule.java

@@ -68,7 +68,7 @@ public class ArrayModule extends AbstractInternalModule {
     );
 
     public ArrayModule(Map<String, List<?>> parameters) {
-        super(functions, parameters, false);
+        super(functions, parameters);
     }
 
     static FunctionSignature functionSignature(final String name, final String description, final FunctionReturnSequenceType returnType, final FunctionParameterSequenceType... paramTypes) {

exist-core/src/main/java/org/exist/xquery/functions/fn/FnModule.java

@@ -21,7 +21,6 @@
  */
 package org.exist.xquery.functions.fn;
 
-import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 
@@ -277,17 +276,12 @@ public class FnModule extends AbstractInternalModule {
         new FunctionDef(FunContainsToken.FS_CONTAINS_TOKEN[0], FunContainsToken.class),
         new FunctionDef(FunContainsToken.FS_CONTAINS_TOKEN[1], FunContainsToken.class)
     };
-
-    static {
-        Arrays.sort(functions, new FunctionComparator());
-    }
-
     public final static ErrorCodes.ErrorCode SENR0001 = new ErrorCodes.ErrorCode("SENR0001", "serialization error in fn:serialize");
     public final static ErrorCodes.ErrorCode SEPM0019 = new ErrorCodes.ErrorCode("SEPM0019", "It is an error if an instance of the data model " +
             "used to specify the settings of serialization parameters specifies the value of the same parameter more than once.");
 
     public FnModule(Map<String, List<?>> parameters) {
-        super(functions, parameters, true);
+        super(functions, parameters);
     }
 
     @Override

exist-core/src/main/java/org/exist/xquery/functions/inspect/InspectionModule.java

@@ -56,7 +56,7 @@ public class InspectionModule extends AbstractInternalModule {
     );
 
     public InspectionModule(final Map<String, List<?>> parameters) {
-        super(functions, parameters, true);
+        super(functions, parameters);
     }
 
     @Override

exist-core/src/main/java/org/exist/xquery/functions/map/MapModule.java

@@ -59,7 +59,7 @@ public class MapModule extends AbstractInternalModule {
     );
 
     public MapModule(Map<String, List<?>> parameters) {
-        super(functions, parameters, false);
+        super(functions, parameters);
     }
 
     public String getNamespaceURI() {

exist-core/src/main/java/org/exist/xquery/functions/request/RequestModule.java

@@ -21,7 +21,6 @@
  */
 package org.exist.xquery.functions.request;
 
-import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 
@@ -77,13 +76,8 @@ public class RequestModule extends AbstractInternalModule {
             new FunctionDef(GetPathInfo.signature, GetPathInfo.class),
             new FunctionDef(IsMultiPartContent.signature, IsMultiPartContent.class)
     };
-
-    static {
-        Arrays.sort(functions, new FunctionComparator());
-    }
-
     public RequestModule(final Map<String, List<?>> parameters) {
-        super(functions, parameters, true);
+        super(functions, parameters);
     }
 
     @Override

exist-core/src/main/java/org/exist/xquery/functions/util/UtilModule.java

@@ -21,7 +21,6 @@
  */
 package org.exist.xquery.functions.util;
 
-import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 
@@ -154,19 +153,14 @@ public class UtilModule extends AbstractInternalModule {
             new FunctionDef(BaseConversionFunctions.FNS_OCTAL_TO_INT, BaseConversionFunctions.class),
             new FunctionDef(LineNumber.signature, LineNumber.class)
     };
-
-    static {
-        Arrays.sort(functions, new FunctionComparator());
-    }
-
     public final static QName EXCEPTION_QNAME = new QName("exception", UtilModule.NAMESPACE_URI, UtilModule.PREFIX);
 
     public final static QName EXCEPTION_MESSAGE_QNAME = new QName("exception-message", UtilModule.NAMESPACE_URI, UtilModule.PREFIX);
 
     public final static QName ERROR_CODE_QNAME = new QName("error-code", UtilModule.NAMESPACE_URI, UtilModule.PREFIX);
 
     public UtilModule(final Map<String, List<? extends Object>> parameters) throws XPathException {
-        super(functions, parameters, true);
+        super(functions, parameters);
 
         final List<String> evalDisabledParamList = (List<String>) getParameter("evalDisabled");
         if (evalDisabledParamList != null && !evalDisabledParamList.isEmpty()) {

exist-core/src/main/java/org/exist/xquery/functions/websocket/ConsoleCompatModule.java

@@ -49,7 +49,7 @@ public class ConsoleCompatModule extends AbstractInternalModule {
     };
 
     public ConsoleCompatModule(final Map<String, List<? extends Object>> parameters) {
-        super(functions, parameters, false);
+        super(functions, parameters);
     }
 
     @Override

exist-core/src/main/java/org/exist/xquery/functions/websocket/WebSocketModule.java

@@ -53,7 +53,7 @@ public class WebSocketModule extends AbstractInternalModule {
     private static volatile ConsoleAdapter adapter = null;
 
     public WebSocketModule(final Map<String, List<? extends Object>> parameters) {
-        super(functions, parameters, false);
+        super(functions, parameters);
     }
 
     public static void log(final String channel, final String message) {

exist-core/src/main/java/org/exist/xquery/functions/xmldb/XMLDBModule.java

@@ -21,7 +21,6 @@
  */
 package org.exist.xquery.functions.xmldb;
 
-import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 
@@ -110,13 +109,8 @@ public class XMLDBModule extends AbstractInternalModule {
     };
 
     private boolean allowAnyUri = false;
-
-    static {
-        Arrays.sort(functions, new FunctionComparator());
-    }
-
     public XMLDBModule(final Map<String, List<?>> parameters) {
-        super(functions, parameters, true);
+        super(functions, parameters);
 
         final List<String> allowAnyUriParameterList = (List<String>) getParameter("allowAnyUri");
         if (allowAnyUriParameterList != null && !allowAnyUriParameterList.isEmpty()) {