reusable xml id validation method (#4584)

* reusable xml id validation method

* removed async method and moved the source and test to project folder

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Klaus Keller <[email protected]>

Author: 3 people

.changeset/funny-socks-rescue.md

@@ -0,0 +1,7 @@
+---
+'@sap-ux/project-access': minor
+'@sap-ux/fe-fpm-writer': patch
+---
+
+Added isUI5IdUnique function to check UI5 control ID uniqueness in XML views/fragments.
+Exported findFilesByExtension from @sap-ux/project-access and removed deep imports.

packages/fe-fpm-writer/src/common/file.ts

@@ -1,8 +1,7 @@
 import type { CopyOptions, Editor } from 'mem-fs-editor';
 import type { TabInfo } from '../common/types';
 import { sep, normalize } from 'node:path';
-import { findFilesByExtension } from '@sap-ux/project-access/dist/file';
-import { DOMParser } from '@xmldom/xmldom';
+import { findFilesByExtension, isUI5IdUnique } from '@sap-ux/project-access';
 
 /**
  * Options for creating an ID generator with cached file contents.
@@ -168,25 +167,16 @@ export const CONFIG = {
 function generateUniqueElementId(baseId: string, filteredFilesContent: string[], validatedIds: string[] = []): string {
     const maxAttempts = 1000;
 
-    function checkElementIdAvailable(id: string, xmlContent: string): boolean {
-        const xmlDocument = new DOMParser({ errorHandler: (): void => {} }).parseFromString(xmlContent);
-        return xmlDocument.documentElement ? !xmlDocument.getElementById(id) : true;
-    }
-
-    if (
-        filteredFilesContent.every((content) => content === '' || checkElementIdAvailable(baseId, content)) &&
-        !validatedIds.includes(baseId)
-    ) {
+    // Check both in-memory validatedIds and filesystem files
+    if (!validatedIds.includes(baseId) && isUI5IdUnique(baseId, filteredFilesContent)) {
         return baseId;
     }
 
     for (let counter = 1; counter < maxAttempts; counter++) {
         const candidateId = `${baseId}${counter}`;
 
-        if (
-            filteredFilesContent.every((content) => content === '' || checkElementIdAvailable(candidateId, content)) &&
-            !validatedIds.includes(candidateId)
-        ) {
+        // Check both in-memory validatedIds and filesystem files
+        if (!validatedIds.includes(candidateId) && isUI5IdUnique(candidateId, filteredFilesContent)) {
             return candidateId;
         }
     }

packages/fiori-docs-embeddings/data_local/ux-ui5-tooling-README.md

@@ -77,6 +77,27 @@ Executing `npx fiori run` in your project with the configuration below in the `u
       url: https://my.backend.com:1234
 ```
 
+#### [Using connectPath for credential retrieval](#using-connectpath-for-credential-retrieval)
+
+When credentials are saved against a full service URL (for example, `https://my.backend.com:1234/sap/opu/odata/example/service`) rather than just the base URL, you need to specify the `connectPath` parameter to ensure the correct credentials are retrieved. The `connectPath` should match the path portion of the URL against which the credentials were saved.
+
+```yaml
+- name: fiori-tools-proxy
+  afterMiddleware: compression
+  configuration:
+    backend:
+    - path: /sap
+      url: https://my.backend.com:1234
+      connectPath: /sap/opu/odata/UI5/ABAP_REPOSITORY_SRV
+```
+
+**When to use `connectPath`:**
+- Your credentials are stored in the SAP System configuration with a full service URL including the path
+- You need to connect to a specific service endpoint for authentication
+- The stored system URL differs from the base server URL
+
+**Note:** If credentials are saved against the base URL only (e.g., `https://my.backend.com:1234`), the `connectPath` parameter is not needed.
+
 #### [Connecting to a back-end system with destination](#connecting-to-a-back-end-system-with-destination)
 
 If the back-end is hidden behind a destination then you can also provide the `destination` in the configuration.
@@ -170,6 +191,21 @@ Let's that you want to configure the proxy to send requests from a certain path
       destination: my_backend
 ```
 
+#### [Add Query Parameters](#add-query-parameters)
+Add query parameters to the proxied request by using the `params` configuration option, e.g.
+
+```
+- name: fiori-tools-proxy
+  afterMiddleware: compression
+  configuration:
+    backend:
+    - path: /sap
+      url: https://my.backend.com:1234
+      params:
+        saml2: 'disabled'
+```
+
+
 #### [Providing Proxy Configuration](#providing-proxy-configuration)
 By the default the `fiori-tools-proxy` will read the proxy configuration from the Node.js environment variables `proxy`, `https-proxy` and `noproxy`. If those variables are not set, then you can also provide the proxy configuration in the `ui5.yaml` file. **Please note: if you want to exclude any domains from the proxy then you will need to set the `noproxy` variable, e.g. `npm config set noproxy "sap.com"`**.
 
@@ -220,6 +256,7 @@ Here is the full list of the available configuration options for the backend pro
 - `ignorePath` (available with version 1.8.5): true/false, Default: false - specify whether you want to ignore the proxy path of the incoming request (note: you will have to append / manually if required)
 - `localAddress` (available with version 1.8.5): Local interface string to bind for outgoing connections
 - `changeOrigin` (available with version 1.8.5): true/false, Default: true - changes the origin of the host header to the target URL
+- `params` (available with version 1.23.0): object, adds query parameters to the proxied request
 - `preserveHeaderKeyCase` (available with version 1.8.5): true/false, Default: false - specify whether you want to keep letter case of response header key
 - `auth` (available with version 1.8.5): Basic authentication i.e. 'user:password' to compute an Authorization header
 - `hostRewrite` (available with version 1.8.5): rewrites the location hostname on (301/302/307/308) redirects

packages/project-access/src/index.ts

@@ -6,7 +6,7 @@ export {
     MinCdsVersion,
     fioriToolsDirectory
 } from './constants';
-export { getFilePaths } from './file';
+export { getFilePaths, findFilesByExtension } from './file';
 export { normalizePath } from './path';
 export {
     addPackageDevDependency,
@@ -67,10 +67,11 @@ export {
     readFlexChanges,
     processServices,
     getMainService,
-    getGlobalCdsHomePath
+    getGlobalCdsHomePath,
+    isUI5IdUnique
 } from './project';
 export { execNpmCommand } from './command/npm-command';
 export * from './types';
-export * from './library';
+export { checkDependencies, getReuseLibs } from './library';
 export { findRecursiveHierarchyKey, getTableCapabilitiesByEntitySet } from './odata';
 export { hasDependency } from './project';

packages/project-access/src/project/index.ts

@@ -61,3 +61,4 @@ export {
     refreshSpecificationDistTags
 } from './specification';
 export { readFlexChanges } from './flex-changes';
+export { isUI5IdUnique } from './ui5-xml-id-validator';

packages/project-access/src/project/ui5-xml-id-validator.ts

@@ -0,0 +1,87 @@
+import { XMLParser } from 'fast-xml-parser';
+
+/**
+ * Recursively checks if an object (parsed XML) contains an element with the specified id attribute.
+ *
+ * @param obj - parsed XML object to search in
+ * @param id - id to search for
+ * @param attrPrefix - attribute prefix used by the parser (default: '@_')
+ * @returns true if an element with the id is found
+ */
+function hasElementWithId(obj: unknown, id: string, attrPrefix = '@_'): boolean {
+    if (typeof obj !== 'object' || obj === null) {
+        return false;
+    }
+
+    const objRecord = obj as Record<string, unknown>;
+    const idAttr = `${attrPrefix}id`;
+
+    // Check if this element has the id attribute
+    if (objRecord[idAttr] === id) {
+        return true;
+    }
+
+    for (const key in objRecord) {
+        if (key.startsWith(attrPrefix)) {
+            continue; // Skip attributes
+        }
+
+        if (checkIdInValue(objRecord[key], id, attrPrefix)) {
+            return true;
+        }
+    }
+
+    return false;
+}
+
+/**
+ * Checks if a value (object or array) contains an element with the specified id.
+ *
+ * @param value - value to check (can be array or object)
+ * @param id - id to search for
+ * @param attrPrefix - attribute prefix used by the parser
+ * @returns true if id is found in the value
+ */
+function checkIdInValue(value: unknown, id: string, attrPrefix: string): boolean {
+    if (Array.isArray(value)) {
+        return value.some((item) => hasElementWithId(item, id, attrPrefix));
+    }
+    if (typeof value === 'object' && value !== null) {
+        return hasElementWithId(value, id, attrPrefix);
+    }
+    return false;
+}
+
+/**
+ * Checks if an element with the specified id is available (does not exist) in the XML content.
+ *
+ * @param id - id to check for availability
+ * @param xmlContent - XML content as string
+ * @returns true if the id is available (not found), false if it exists
+ */
+function checkElementIdAvailable(id: string, xmlContent: string): boolean {
+    const parser = new XMLParser({
+        ignoreAttributes: false,
+        attributeNamePrefix: '@_',
+        parseAttributeValue: false
+    });
+
+    try {
+        const xmlDocument: unknown = parser.parse(xmlContent);
+        return xmlDocument ? !hasElementWithId(xmlDocument, id) : true;
+    } catch {
+        // Parse error = no valid document = no element with id
+        return true;
+    }
+}
+
+/**
+ * Checks if a UI5 control ID is unique across XML files (fragments and views).
+ *
+ * @param id - ID to check
+ * @param files - Array of XML file contents to check
+ * @returns true if the id is unique (available), false if it already exists
+ */
+export function isUI5IdUnique(id: string, files: string[]): boolean {
+    return files.every((content) => content === '' || checkElementIdAvailable(id, content));
+}

packages/project-access/test/project/ui5-xml-id-validator.test.ts

@@ -0,0 +1,111 @@
+import { isUI5IdUnique } from '../../src/project/ui5-xml-id-validator';
+
+describe('isUI5IdUnique', () => {
+    const sampleView = `<mvc:View
+    xmlns:mvc="sap.ui.core.mvc"
+    xmlns="sap.m"
+    controllerName="my.app.controller.Main">
+    <Page id="mainPage" title="Main View">
+        <content>
+            <Button id = "submitButton" text="Submit" />
+            <Input id="nameInput" placeholder="Enter name" />
+            <Table id ="dataTable">
+                <columns>
+                    <Column>
+                        <Text text="Name" />
+                    </Column>
+                </columns>
+            </Table>
+        </content>
+    </Page>
+</mvc:View>`;
+
+    const sampleFragment = `<core:FragmentDefinition
+    xmlns="sap.m"
+    xmlns:core="sap.ui.core">
+    <Dialog id="confirmDialog" title="Confirm Action">
+        <content>
+            <Text id= "dialogText" text="Are you sure?" />
+        </content>
+        <beginButton>
+            <Button id="confirmButton" text="Confirm" press="onConfirm" />
+        </beginButton>
+        <endButton>
+            <Button id="cancelButton" text="Cancel" press="onCancel" />
+        </endButton>
+    </Dialog>
+</core:FragmentDefinition>`;
+
+    const sampleViewWithNamespace = `<mvc:View
+    xmlns:mvc="sap.ui.core.mvc"
+    xmlns="sap.m"
+    xmlns:f="sap.ui.layout.form">
+    <f:SimpleForm id="detailForm">
+        <f:content>
+            <Label text="Title" />
+            <Input id="titleInput" />
+        </f:content>
+    </f:SimpleForm>
+</mvc:View>`;
+
+    test('should return true when id does not exist in any files', () => {
+        const result = isUI5IdUnique('newButton', [sampleView, sampleFragment]);
+        expect(result).toBe(true);
+    });
+
+    test('should return false when id exists in view', () => {
+        const result = isUI5IdUnique('submitButton', [sampleView, sampleFragment]);
+        expect(result).toBe(false);
+    });
+
+    test('should return false when id exists in fragment', () => {
+        const result = isUI5IdUnique('confirmDialog', [sampleView, sampleFragment]);
+        expect(result).toBe(false);
+    });
+
+    test('should return true when id is unique across multiple files', () => {
+        const result = isUI5IdUnique('uniqueId', [sampleView, sampleFragment, sampleViewWithNamespace]);
+        expect(result).toBe(true);
+    });
+
+    test('should return false when id exists in nested elements', () => {
+        const result = isUI5IdUnique('dataTable', [sampleView]);
+        expect(result).toBe(false);
+    });
+
+    test('should return false when id exists in fragment dialog content', () => {
+        const result = isUI5IdUnique('dialogText', [sampleFragment]);
+        expect(result).toBe(false);
+    });
+
+    test('should return true for empty files array', () => {
+        const result = isUI5IdUnique('anyId', []);
+        expect(result).toBe(true);
+    });
+
+    test('should return true when XML parsing fails', () => {
+        // fast-xml-parser is lenient, but completely invalid content should fail
+        const invalidXml = '<<<>>><invalid';
+        const result = isUI5IdUnique('test', [invalidXml]);
+        expect(result).toBe(true);
+    });
+
+    test('should return true when files contain empty strings', () => {
+        const result = isUI5IdUnique('testId', ['', '', sampleView]);
+        expect(result).toBe(true);
+    });
+
+    test('should handle ids with special characters', () => {
+        const xmlWithSpecialId = `<?xml version="1.0" encoding="UTF-8"?>
+<mvc:View xmlns:mvc="sap.ui.core.mvc" xmlns="sap.m">
+    <Button id="button-with-dash" text="Test" />
+    <Button id="button_with_underscore" text="Test" />
+    <Button id="button.with.dot" text="Test" />
+</mvc:View>`;
+
+        expect(isUI5IdUnique('button-with-dash', [xmlWithSpecialId])).toBe(false);
+        expect(isUI5IdUnique('button_with_underscore', [xmlWithSpecialId])).toBe(false);
+        expect(isUI5IdUnique('button.with.dot', [xmlWithSpecialId])).toBe(false);
+        expect(isUI5IdUnique('button-not-exists', [xmlWithSpecialId])).toBe(true);
+    });
+});