Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(project-access): check for files in srv folder treats directories as files and throws exception #2923

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
5 changes: 5 additions & 0 deletions .changeset/selfish-frogs-pump.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sap-ux/project-access': patch
---

fix: check files in srv folder of CAP project when srv folder contains subfolders
83 changes: 70 additions & 13 deletions packages/project-access/src/file/file-search.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { Editor, FileMap } from 'mem-fs-editor';
import { basename, dirname, extname, join } from 'path';
import { default as find } from 'findit2';
import { basename, dirname, extname, join, sep, posix } from 'path';
import { default as find, type FindError } from 'findit2';
import { fileExists } from './file-access';
import { promises as fs } from 'fs';

Expand All @@ -10,17 +10,23 @@ import { promises as fs } from 'fs';
* @param changes - memfs editor changes, usually retrieved by fs.dump()
* @param fileNames - array of file names to search for
* @param extensionNames - array of extensions names to search for
* @param root - path to root folder
* @returns - array of deleted and modified files filtered by query
*/
function getMemFsChanges(
changes: FileMap,
fileNames: string[],
extensionNames: string[]
extensionNames: string[],
root: string
): { deleted: string[]; modified: string[] } {
const deleted: string[] = [];
const modified: string[] = [];
const filteredChanges = Object.keys(changes).filter(
(f) => fileNames.includes(basename(f)) || extensionNames.includes(extname(f))
(f) =>
f.startsWith(root.replaceAll(sep, posix.sep)) &&
(fileNames.includes(basename(f)) ||
extensionNames.includes(extname(f)) ||
(fileNames.length === 0 && extensionNames.length === 0))
);
for (const file of filteredChanges) {
if (changes[file].state === 'deleted') {
Expand All @@ -33,8 +39,46 @@ function getMemFsChanges(
return { deleted, modified };
}

/**
* Returns the search results and fatal errors.
* This is required to include potential memfs changes in the search results.
*
* @param results - array of file paths that were found during the search.
* @param fileNames - array of file names that were searched for
* @param extensionNames - array of file extensions that were searched for
* @param root - root directory where the search was performed
* @param errors - array of errors that occurred during the search
* @param [memFs] - optional memfs editor instance
* @returns - object containing the search results and any fatal errors
*/
function getFindResultOnEnd(
results: string[],
fileNames: string[],
extensionNames: string[],
root: string,
errors: FindError[],
memFs?: Editor
): { searchResult: string[]; fatalErrors: FindError[] } {
let searchResult = results;
let fatalErrors = errors;

if (memFs) {
const { modified, deleted } = getMemFsChanges(memFs.dump(''), fileNames, extensionNames, root);
const merged = Array.from(new Set([...results, ...modified]));
searchResult = merged.filter((f) => !deleted.includes(f));
// Filter out errors that are of type ENOENT and path is part of memfs changes, which can happen for folders that contain memfs files only.
fatalErrors = errors.filter(
(e) =>
e.code !== 'ENOENT' ||
(typeof e.path === 'string' && !modified.some((f) => f.startsWith(e.path as string)))
);
}
return { searchResult, fatalErrors };
}

/**
* Find function to search for files by names or file extensions.
* Empty name and extension option returns all files in the given folder.
*
* @param options - find options
* @param [options.fileNames] - optional array of file names to search for
Expand All @@ -59,6 +103,7 @@ export function findBy(options: {
const extensionNames = Array.isArray(options.extensionNames) ? options.extensionNames : [];
const excludeFolders = Array.isArray(options.excludeFolders) ? options.excludeFolders : [];
const noTraversal = options.noTraversal ?? false;
const errors: FindError[] = [];

const finder = find(options.root);
finder.on('directory', (dir: string, _stat: unknown, stop: () => void) => {
Expand All @@ -68,21 +113,33 @@ export function findBy(options: {
}
});
finder.on('file', (file: string) => {
if (extensionNames.includes(extname(file)) || fileNames.includes(basename(file))) {
if (
extensionNames.includes(extname(file)) ||
fileNames.includes(basename(file)) ||
(fileNames.length === 0 && extensionNames.length === 0)
) {
results.push(file);
}
});
finder.on('end', () => {
let searchResult: string[] = results;
if (options.memFs) {
const { modified, deleted } = getMemFsChanges(options.memFs.dump(''), fileNames, extensionNames);
const merged = Array.from(new Set([...results, ...modified]));
searchResult = merged.filter((f) => !deleted.includes(f));
const { searchResult, fatalErrors } = getFindResultOnEnd(
results,
fileNames,
extensionNames,
options.root,
errors,
options.memFs
);

if (fatalErrors.length === 0) {
resolve(searchResult);
} else {
// eslint-disable-next-line prefer-promise-reject-errors
reject(fatalErrors);
}
resolve(searchResult);
});
finder.on('error', (error: Error) => {
reject(error);
finder.on('error', (error: FindError) => {
errors.push(error);
});
});
}
Expand Down
30 changes: 6 additions & 24 deletions packages/project-access/src/project/cap.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { spawn } from 'child_process';
import { basename, dirname, join, normalize, relative, sep, resolve } from 'path';
import { basename, dirname, join, normalize, relative, sep } from 'path';
import type { Logger } from '@sap-ux/logger';
import type { Editor } from 'mem-fs-editor';
import { FileName } from '../constants';
Expand All @@ -18,6 +18,7 @@ import {
deleteDirectory,
deleteFile,
fileExists,
findBy,
readDirectory,
readFile,
readJSON,
Expand Down Expand Up @@ -83,30 +84,11 @@ export async function isCapJavaProject(
* @returns {Promise<boolean>} - Resolves to `true` if files are found in the `srv` folder; otherwise, `false`.
*/
async function checkFilesInSrvFolder(srvFolderPath: string, memFs?: Editor): Promise<boolean> {
if (!memFs) {
return await fileExists(srvFolderPath);
}
// Load the srv folder and its files into mem-fs
// This is necessary as mem-fs operates in-memory and doesn't automatically include files from disk.
// By loading the files, we ensure they are available within mem-fs.
if (await fileExists(srvFolderPath)) {
const fileSystemFiles = await readDirectory(srvFolderPath);
for (const file of fileSystemFiles) {
const filePath = join(srvFolderPath, file);
if (await fileExists(filePath)) {
const fileContent = await readFile(filePath);
memFs.write(filePath, fileContent);
}
}
try {
return (await findBy({ root: srvFolderPath, memFs })).length > 0;
} catch (error) {
return false;
}
// Dump the mem-fs state
const memFsDump = memFs.dump() as { [key: string]: { contents: string; state: 'modified' | 'deleted' } };
const memFsFiles = Object.keys(memFsDump).filter((filePath) => {
const normalisedFilePath = resolve(filePath);
const normalisedSrvPath = resolve(srvFolderPath);
return normalisedFilePath.startsWith(normalisedSrvPath);
});
return memFsFiles.length > 0;
}

/**
Expand Down
3 changes: 2 additions & 1 deletion packages/project-access/src/types/findit2.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ declare module 'findit2' {
/**
* Fired whenever an error occurs.
*/
on(event: 'error', listener: (error: Error) => void): this;
on(event: 'error', listener: (error: FindError) => void): this;
};
export type FindError = Error & { code?: string; path?: string };
export = find;
}
26 changes: 25 additions & 1 deletion packages/project-access/test/file/file-search.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { join } from 'path';
import { getFilePaths } from '../../src';
import { findFiles, findFilesByExtension, findFileUp } from '../../src/file';
import { findBy, findFiles, findFilesByExtension, findFileUp } from '../../src/file';
import { create as createStorage } from 'mem-fs';
import { create } from 'mem-fs-editor';

Expand Down Expand Up @@ -117,4 +117,28 @@ describe('findFiles', () => {
expect(filePaths).toEqual(expect.arrayContaining(expectedPaths));
});
});

describe('findBy()', () => {
test('findBy with file name including memFs', async () => {
const path = join(root, 'childA');
const memFs = create(createStorage());
memFs.write(join(__dirname, 'child'), 'test');
memFs.write(join(path, 'memfs-path', 'child'), 'test');

const result = await findBy({ fileNames: ['child'], root: path, memFs });
expect(result.sort()).toEqual([join(path, 'child'), join(path, 'memfs-path/child')].sort());
});

test('findBy with extension name including memFs', async () => {
const path = join(root, 'childA');
const memFs = create(createStorage());
memFs.write(join(__dirname, 'child.extension'), 'test');
memFs.write(join(path, 'memfs-path', 'child.extension'), 'test');

const result = await findBy({ extensionNames: ['.extension'], root: path, memFs });
expect(result.sort()).toEqual(
[join(path, 'child.extension'), join(path, 'memfs-path/child.extension')].sort()
);
});
});
});
15 changes: 15 additions & 0 deletions packages/project-access/test/project/cap.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,21 @@ describe('Test getCapProjectType() & isCapProject()', () => {
const capPath = join(__dirname, '..', 'test-data', 'project', 'info', 'empty-project');
expect(await getCapProjectType(capPath, memFs)).toBe(undefined);
});

test('Test if getCapProjectType() considers deletions in memfs', async () => {
const capPath = join(__dirname, '..', 'test-data', 'project', 'cap-app');
const memFsWithDeletion = create(createStorage());
memFsWithDeletion.delete(join(capPath, 'srv', 'keep'));
expect(await getCapProjectType(capPath, memFsWithDeletion)).toBe(undefined);
});

test('Test if getCapProjectType() considers addition in memfs', async () => {
const capPath = join(__dirname, '..', 'test-data', 'project', 'cap-root', 'invalid-cap-root-no-srv');
const memFsWithAddition = create(createStorage());
memFsWithAddition.write(join(capPath, 'srv', 'keep'), '');
memFsWithAddition.write(join('/tmp/any/file/test'), 'test');
expect(await getCapProjectType(capPath, memFsWithAddition)).toBe('CAPNodejs');
});
});

describe('Test isCapNodeJsProject()', () => {
Expand Down
3 changes: 2 additions & 1 deletion packages/project-access/test/project/ui5-config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ describe('Test getAllUi5YamlFileNames()', () => {
test('Read list of Ui5 yaml files', async () => {
const memFs = create(createStorage());

expect(await getAllUi5YamlFileNames(join(samplesRoot, 'default-with-ui5-yaml'), memFs)).toMatchInlineSnapshot(`
expect((await getAllUi5YamlFileNames(join(samplesRoot, 'default-with-ui5-yaml'), memFs)).sort())
.toMatchInlineSnapshot(`
Array [
"ui5-custom-multi.yaml",
"ui5-custom.yaml",
Expand Down