fix: Maven Toolchains grows unexpectedly (#534)

* fix: Maven Toolchains grows unexpectedly

On self-hosted runners toolchains.xml may survive multiple runs and unexpectedly
grow as a result of the toolchains setup simply appending the JDK definition
even if one with the same `type` and `provides.id` already exists.

Restructuring the parsing step and filtering the potentially existing list of
toolchain definitions prevents this and also fixes toolchain.xml files that
already contain duplicates.

Fixes #530

* fix: guard toolchain dedup and preserve existing root attributes

Address reviewer feedback on the Maven toolchains dedup logic:

- Treat jdk toolchains without a string `provides.id` as non-deduplicatable
  and use optional access when comparing ids, so partially-formed or
  nonstandard toolchains.xml files no longer crash setup.
- Preserve the existing `<toolchains>` root attributes (xmlns,
  schemaLocation, …) when present, falling back to the 1.1.0 defaults only
  for attributes the existing file is missing. This avoids silently
  rewriting user-managed metadata or changing the effective namespace.

Adds tests covering custom root attributes and id-less jdk toolchains, and
rebuilds dist/.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Bruno Borges <brborges@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

src/toolchains.ts

@@ -83,47 +83,76 @@ export function generateToolchainDefinition(
   id: string,
   jdkHome: string
 ) {
-  let xmlObj;
-  if (original?.length) {
-    xmlObj = xmlCreate(original)
-      .root()
-      .ele({
-        toolchain: {
-          type: 'jdk',
-          provides: {
-            version: `${version}`,
-            vendor: `${vendor}`,
-            id: `${id}`
-          },
-          configuration: {
-            jdkHome: `${jdkHome}`
-          }
-        }
-      });
-  } else
-    xmlObj = xmlCreate({
-      toolchains: {
-        '@xmlns': 'http://maven.apache.org/TOOLCHAINS/1.1.0',
-        '@xmlns:xsi': 'http://www.w3.org/2001/XMLSchema-instance',
-        '@xsi:schemaLocation':
-          'http://maven.apache.org/TOOLCHAINS/1.1.0 https://maven.apache.org/xsd/toolchains-1.1.0.xsd',
-        toolchain: [
-          {
-            type: 'jdk',
-            provides: {
-              version: `${version}`,
-              vendor: `${vendor}`,
-              id: `${id}`
-            },
-            configuration: {
-              jdkHome: `${jdkHome}`
-            }
-          }
-        ]
+  let jsToolchains: Toolchain[] = [
+    {
+      type: 'jdk',
+      provides: {
+        version: `${version}`,
+        vendor: `${vendor}`,
+        id: `${id}`
+      },
+      configuration: {
+        jdkHome: `${jdkHome}`
       }
-    });
+    }
+  ];
+  // default root attributes, used when the existing file does not declare its own
+  let rootAttributes: Record<string, string> = {
+    '@xmlns': 'http://maven.apache.org/TOOLCHAINS/1.1.0',
+    '@xmlns:xsi': 'http://www.w3.org/2001/XMLSchema-instance',
+    '@xsi:schemaLocation':
+      'http://maven.apache.org/TOOLCHAINS/1.1.0 https://maven.apache.org/xsd/toolchains-1.1.0.xsd'
+  };
+  if (original?.length) {
+    // convert existing toolchains into TS native objects for better handling
+    // xmlbuilder2 will convert the document into a `{toolchains: { toolchain: [] | {} }}` structure
+    // instead of the desired `toolchains: [{}]` one or simply `[{}]`
+    const jsObj = xmlCreate(original)
+      .root()
+      .toObject() as unknown as ExtractedToolchains;
+    if (jsObj.toolchains) {
+      // preserve the existing root attributes (xmlns, schemaLocation, …) so we don't
+      // silently rewrite user-managed metadata or change the effective XML namespace;
+      // xmlbuilder2 exposes attributes as `@`-prefixed keys on the element object
+      const existingAttributes = Object.fromEntries(
+        Object.entries(jsObj.toolchains).filter(([key]) => key.startsWith('@'))
+      ) as Record<string, string>;
+      // fall back to the defaults only for attributes the existing file is missing
+      rootAttributes = {...rootAttributes, ...existingAttributes};
 
-  return xmlObj.end({
+      if (jsObj.toolchains.toolchain) {
+        // in case only a single child exists xmlbuilder2 will not create an array and using verbose = true equally doesn't work here
+        // See https://oozcitak.github.io/xmlbuilder2/serialization.html#js-object-and-map-serializers for details
+        if (Array.isArray(jsObj.toolchains.toolchain)) {
+          jsToolchains.push(...jsObj.toolchains.toolchain);
+        } else {
+          jsToolchains.push(jsObj.toolchains.toolchain);
+        }
+      }
+    }
+
+    // remove potential duplicates based on type & id (which should be a unique combination);
+    // self.findIndex will only return the first occurrence, ensuring duplicates are skipped
+    jsToolchains = jsToolchains.filter(
+      (value, index, self) =>
+        // ensure non-jdk toolchains are kept in the results, we must not touch them because they belong to the user
+        value.type !== 'jdk' ||
+        // keep toolchains that lack a usable string id (e.g. partially-formed user files);
+        // we cannot safely deduplicate them and must not crash while reading them
+        typeof value.provides?.id !== 'string' ||
+        index ===
+          self.findIndex(
+            t => t.type === value.type && t.provides?.id === value.provides?.id
+          )
+    );
+  }
+
+  return xmlCreate({
+    toolchains: {
+      ...rootAttributes,
+      toolchain: jsToolchains
+    }
+  }).end({
     format: 'xml',
     wellFormed: false,
     headless: false,
@@ -166,3 +195,24 @@ async function writeToolchainsFileToDisk(
     flag: 'w'
   });
 }
+
+interface ExtractedToolchains {
+  toolchains: {
+    // root attributes such as xmlns / schemaLocation are exposed as `@`-prefixed keys
+    [attribute: `@${string}`]: string;
+    toolchain?: Toolchain[] | Toolchain;
+  };
+}
+
+// Toolchain type definition according to Maven Toolchains XSD 1.1.0
+interface Toolchain {
+  type: string;
+  provides:
+    | {
+        version: string;
+        vendor: string;
+        id: string;
+      }
+    | any;
+  configuration: any;
+}