Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions packages/fe-fpm-writer/src/common/file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,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 { validateId } from '@sap-ux/project-access';

/**
* Options for creating an ID generator with cached file contents.
Expand Down Expand Up @@ -168,9 +168,16 @@ export const CONFIG = {
function generateUniqueElementId(baseId: string, filteredFilesContent: string[], validatedIds: string[] = []): string {
const maxAttempts = 1000;

/**
* 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 xmlDocument = new DOMParser({ errorHandler: (): void => {} }).parseFromString(xmlContent);
return xmlDocument.documentElement ? !xmlDocument.getElementById(id) : true;
// Use validateId from @sap-ux/project-access (synchronous overload)
return validateId(id, undefined, { files: [xmlContent] });
}
Comment thread
Jimmy-Joseph19 marked this conversation as resolved.
Outdated

if (
Expand Down
174 changes: 173 additions & 1 deletion packages/project-access/src/library/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,13 @@ import {
type ReuseLib,
type LibraryXml
} from '../types';
import { findFiles, readJSON } from '../file';
import { findFiles, findFilesByExtension, readJSON } from '../file';
import { FileName } from '../constants';
import { existsSync, promises as fs } from 'node:fs';
import { XMLParser } from 'fast-xml-parser';
import { getI18nPropertiesPaths } from '../project/i18n';
import { getPropertiesI18nBundle } from '@sap-ux/i18n';
import type { Editor } from 'mem-fs-editor';

/**
* Reads the manifest file and returns the reuse library.
Expand Down Expand Up @@ -307,3 +308,174 @@ export function getManifestDependencies(manifest: Manifest): string[] {

return result;
}

/**
* Validates if an id is unique across XML files (fragments and views) in the project.
* Synchronous overload - when files are provided directly.
*
* @param baseId - id to validate
* @param validatedIds - array of ids that are already validated/used
* @param options - validation options with files array
* @param options.files - array of XML file contents to check
* @param options.appPath - must be undefined for synchronous overload
* @param options.memFs - must be undefined for synchronous overload
* @returns true if the id is unique (available), false if it already exists
*/
export function validateId(
baseId: string,
validatedIds: string[] | undefined,
options: { files: string[]; appPath?: never; memFs?: never }
): boolean;

/**
* Validates if an id is unique across XML files (fragments and views) in the project.
* Asynchronous overload - when appPath is provided (requires file system access).
*
* @param baseId - id to validate
* @param validatedIds - array of ids that are already validated/used
* @param options - validation options with appPath
* @param options.files - must be undefined for asynchronous overload
* @param options.appPath - path to search for XML files
* @param options.memFs - optional mem-fs-editor instance for reading files
* @returns Promise that resolves to true if the id is unique (available), false if it already exists
*/
export function validateId(
baseId: string,
validatedIds: string[] | undefined,
options: { files?: never; appPath: string; memFs?: Editor }
): Promise<boolean>;

/**
* Validates if an id is unique across XML files (fragments and views) in the project.
* Asynchronous overload - when no options are provided.
*
* @param baseId - id to validate
* @param validatedIds - array of ids that are already validated/used
* @param options - undefined (no validation options)
* @returns Promise that resolves to true (always valid when no files to check)
*/
export function validateId(baseId: string, validatedIds?: string[], options?: undefined): Promise<boolean>;

// Implementation
export function validateId(
baseId: string,
validatedIds?: string[],
options?: { files?: string[]; appPath?: string; memFs?: Editor }
): boolean | Promise<boolean> {
const { memFs, appPath, files: fileContents } = options ?? {};

/**
* 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 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;
}

/**
* Recursively searches for an element with the specified id attribute in a parsed XML object.
*
* @param obj - parsed XML object to search in
* @param id - id attribute value to search for
* @param attrPrefix - attribute prefix used by the parser (default: '@_')
* @returns true if an element with the specified id is found
*/
function hasElementWithId(obj: unknown, id: string, attrPrefix: string = '@_'): boolean {
if (!obj || typeof obj !== 'object') {
return false;
}

const objRecord = obj as Record<string, unknown>;

// Check if current object has the id attribute
if (objRecord[`${attrPrefix}id`] === id) {
return true;
}

// Recursively search in all properties
for (const key in objRecord) {
if (key.startsWith(attrPrefix)) {
continue; // Skip attributes
}

if (checkIdInValue(objRecord[key], id, attrPrefix)) {
return true;
}
}

return false;
}

/**
* Validates the ID against the provided files.
*
* @param files - array of XML file contents to validate against
* @returns true if the id is unique (available), false if it already exists
*/
function validateAgainstFiles(files: string[]): boolean {
return (
files.every((content) => content === '' || checkElementIdAvailable(baseId, content)) &&
!validatedIds?.includes(baseId)
);
}

// Synchronous path: when files are provided directly
if (fileContents !== undefined) {
return validateAgainstFiles(fileContents);
}

// Asynchronous path: when appPath is provided or no options
return (async (): Promise<boolean> => {
let files: string[] | undefined = fileContents;
Comment thread
Jimmy-Joseph19 marked this conversation as resolved.
Outdated
if (!files && appPath) {
const xmlFiles = await findFilesByExtension(
'.xml',
appPath,
['.git', 'node_modules', 'dist', 'annotations', 'localService'],
memFs
);
const lookupFiles = ['.fragment.xml', '.view.xml'];
files = xmlFiles.filter((fileName: string) =>
lookupFiles.some((lookupFile) => fileName.endsWith(lookupFile))
);
}

if (files) {
return validateAgainstFiles(files);
Comment thread
Jimmy-Joseph19 marked this conversation as resolved.
Outdated
}
return true;
})();
}
2 changes: 1 addition & 1 deletion packages/project-access/src/library/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export { getReuseLibs, checkDependencies } from './helpers';
export { getReuseLibs, checkDependencies, validateId } from './helpers';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Jimmy-Joseph19, I know it is not a part that is touched by this PR, but could you please also change from export * to named exports (list all exports) in

export * from './library';

to

export { getReuseLibs, checkDependencies, validateId } from './library';

This would restore consistency in public interface and avoids mistaken public exports from subfolders.

163 changes: 162 additions & 1 deletion packages/project-access/test/library/helpers.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import { join } from 'node:path';
import { checkDependencies, getReuseLibs, getLibraryDesc, getManifestDesc } from '../../src/library/helpers';
import {
checkDependencies,
getReuseLibs,
getLibraryDesc,
getManifestDesc,
validateId
} from '../../src/library/helpers';
import * as manifestJson from '../test-data/libs/sap.reuse.ex.test.lib.attachmentservice/src/sap/reuse/ex/test/lib/attachmentservice/manifest.json';
import type { LibraryXml, Manifest, ReuseLib } from '../../src';
import * as fileUtils from '../../src/file';
Expand Down Expand Up @@ -101,3 +107,158 @@ describe('library utils', () => {
expect(description).toEqual('test description');
});
});

describe('validateId', () => {
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', async () => {
const result = await validateId('newButton', undefined, {
files: [sampleView, sampleFragment]
});
expect(result).toBe(true);
Comment thread
Jimmy-Joseph19 marked this conversation as resolved.
Outdated
});

test('should return false when id exists in view', async () => {
const result = await validateId('submitButton', undefined, {
files: [sampleView, sampleFragment]
});
expect(result).toBe(false);
});

test('should return false when id exists in fragment', async () => {
const result = await validateId('confirmDialog', undefined, {
files: [sampleView, sampleFragment]
});
expect(result).toBe(false);
});

test('should return false when id is in validatedIds array', async () => {
const result = await validateId('newButton', ['newButton', 'anotherButton'], {
files: [sampleView, sampleFragment]
});
expect(result).toBe(false);
});

test('should return true when id is unique across multiple files', async () => {
const result = await validateId('uniqueId', undefined, {
files: [sampleView, sampleFragment, sampleViewWithNamespace]
});
expect(result).toBe(true);
});

test('should return false when id exists in nested elements', async () => {
const result = await validateId('dataTable', undefined, {
files: [sampleView]
});
expect(result).toBe(false);
});

test('should return false when id exists in fragment dialog content', async () => {
const result = await validateId('dialogText', undefined, {
files: [sampleFragment]
});
expect(result).toBe(false);
});

test('should return true for empty files array', async () => {
const result = await validateId('anyId', undefined, {
files: []
});
expect(result).toBe(true);
});

test('should return true when XML parsing fails', async () => {
// fast-xml-parser is lenient, but completely invalid content should fail
const invalidXml = '<<<>>><invalid';
const result = await validateId('test', undefined, {
files: [invalidXml]
});
expect(result).toBe(true);
});

test('should return true when files contain empty strings', async () => {
const result = await validateId('testId', undefined, {
files: ['', '', sampleView]
});
expect(result).toBe(true);
});

test('should handle ids with special characters', async () => {
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>`;

const result1 = await validateId('button-with-dash', undefined, {
files: [xmlWithSpecialId]
});
expect(result1).toBe(false);

const result2 = await validateId('button_with_underscore', undefined, {
files: [xmlWithSpecialId]
});
expect(result2).toBe(false);

const result3 = await validateId('button.with.dot', undefined, {
files: [xmlWithSpecialId]
});
expect(result3).toBe(false);

const result4 = await validateId('button-not-exists', undefined, {
files: [xmlWithSpecialId]
});
expect(result4).toBe(true);
});

test('should return true when no options provided', async () => {
const result = await validateId('anyId');
expect(result).toBe(true);
});
});
Loading