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: static webapp path #2931

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
11 changes: 11 additions & 0 deletions .changeset/orange-donkeys-type.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
'@sap-ux/adp-flp-config-sub-generator': patch
'@sap-ux/mockserver-config-writer': patch
'@sap-ux/odata-service-writer': patch
'@sap-ux/adp-tooling': patch
'@sap-ux/telemetry': patch
'@sap-ux/create': patch
'@sap-ux/i18n': patch
---

fix: usage of static webapp path
2 changes: 1 addition & 1 deletion packages/adp-flp-config-sub-generator/src/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ export default class extends Generator {
requestOptions['auth'] = { username: this.credentials.username, password: this.credentials.password };
}
const provider = await createAbapServiceProvider(target, requestOptions, false, this.toolsLogger);
const variant = getVariant(this.projectRootPath);
const variant = await getVariant(this.projectRootPath);
const manifestService = await ManifestService.initMergedManifest(
provider,
this.projectRootPath,
Expand Down
2 changes: 1 addition & 1 deletion packages/adp-tooling/src/base/abap/manifest-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ export class ManifestService {
*/
private async fetchMergedManifest(basePath: string, descriptorVariantId: string): Promise<void> {
const zip = new ZipFile();
const files = getWebappFiles(basePath);
const files = await getWebappFiles(basePath);
for (const file of files) {
zip.addFile(file.relativePath, Buffer.from(file.content, 'utf-8'));
}
Expand Down
44 changes: 25 additions & 19 deletions packages/adp-tooling/src/base/helper.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import type { Editor } from 'mem-fs-editor';
import { readdirSync, readFileSync } from 'fs';
import { join, isAbsolute, relative } from 'path';

import { UI5Config } from '@sap-ux/ui5-config';

import { join, isAbsolute, relative, basename, dirname } from 'path';
import { getWebappPath, FileName, readUi5Yaml } from '@sap-ux/project-access';
import type { UI5Config } from '@sap-ux/ui5-config';
import type { DescriptorVariant, AdpPreviewConfig } from '../types';

/**
Expand All @@ -13,11 +12,12 @@ import type { DescriptorVariant, AdpPreviewConfig } from '../types';
* @param {Editor} fs - The mem-fs editor instance.
* @returns {DescriptorVariant} The app descriptor variant.
*/
export function getVariant(basePath: string, fs?: Editor): DescriptorVariant {
export async function getVariant(basePath: string, fs?: Editor): Promise<DescriptorVariant> {
const webappPath = await getWebappPath(basePath);
if (fs) {
return fs.readJSON(join(basePath, 'webapp', 'manifest.appdescr_variant')) as unknown as DescriptorVariant;
return fs.readJSON(join(webappPath, FileName.ManifestAppDescrVar)) as unknown as DescriptorVariant;
}
return JSON.parse(readFileSync(join(basePath, 'webapp', 'manifest.appdescr_variant'), 'utf-8'));
return JSON.parse(readFileSync(join(webappPath, FileName.ManifestAppDescrVar), 'utf-8'));
}

/**
Expand All @@ -27,8 +27,8 @@ export function getVariant(basePath: string, fs?: Editor): DescriptorVariant {
* @param {DescriptorVariant} variant - The descriptor variant object.
* @param {Editor} fs - The mem-fs editor instance.
*/
export function updateVariant(basePath: string, variant: DescriptorVariant, fs: Editor) {
fs.writeJSON(join(basePath, 'webapp', 'manifest.appdescr_variant'), variant);
export async function updateVariant(basePath: string, variant: DescriptorVariant, fs: Editor): Promise<void> {
fs.writeJSON(join(await getWebappPath(basePath), FileName.ManifestAppDescrVar), variant);
}

/**
Expand All @@ -41,9 +41,9 @@ export function updateVariant(basePath: string, variant: DescriptorVariant, fs:
* @returns {boolean} Returns `true` if FLP configuration changes exist, otherwise `false`.
* @throws {Error} Throws an error if the variant could not be retrieved.
*/
export function flpConfigurationExists(basePath: string): boolean {
export async function flpConfigurationExists(basePath: string): Promise<boolean> {
try {
const variant = getVariant(basePath);
const variant = await getVariant(basePath);
return variant.content?.some(
({ changeType }) =>
changeType === 'appdescr_app_changeInbound' || changeType === 'appdescr_app_addNewInbound'
Expand All @@ -62,13 +62,19 @@ export function flpConfigurationExists(basePath: string): boolean {
*/
export async function getAdpConfig(basePath: string, yamlPath: string): Promise<AdpPreviewConfig> {
const ui5ConfigPath = isAbsolute(yamlPath) ? yamlPath : join(basePath, yamlPath);
const ui5Conf = await UI5Config.newInstance(readFileSync(ui5ConfigPath, 'utf-8'));
const customMiddlerware =
ui5Conf.findCustomMiddleware<{ adp: AdpPreviewConfig }>('fiori-tools-preview') ??
ui5Conf.findCustomMiddleware<{ adp: AdpPreviewConfig }>('preview-middleware');
const adp = customMiddlerware?.configuration?.adp;
let ui5Conf: UI5Config;
let adp: AdpPreviewConfig | undefined;
try {
ui5Conf = await readUi5Yaml(dirname(ui5ConfigPath), basename(ui5ConfigPath));
const customMiddleware =
ui5Conf.findCustomMiddleware<{ adp: AdpPreviewConfig }>('fiori-tools-preview') ??
ui5Conf.findCustomMiddleware<{ adp: AdpPreviewConfig }>('preview-middleware');
adp = customMiddleware?.configuration?.adp;
} catch (error) {
// do nothing here
}
if (!adp) {
throw new Error('No system configuration found in ui5.yaml');
throw new Error(`No system configuration found in ${basename(ui5ConfigPath)}`);
}
return adp;
}
Expand All @@ -79,8 +85,8 @@ export async function getAdpConfig(basePath: string, yamlPath: string): Promise<
* @param {string} basePath - The path to the adaptation project.
* @returns {Array<{ relativePath: string; content: string }>} The files in the webapp folder.
*/
export function getWebappFiles(basePath: string): { relativePath: string; content: string }[] {
const dir = join(basePath, 'webapp');
export async function getWebappFiles(basePath: string): Promise<{ relativePath: string; content: string }[]> {
const dir = await getWebappPath(basePath);
const files: { relativePath: string; content: string }[] = [];

const getFilesRecursivelySync = (directory: string): void => {
Expand Down
4 changes: 2 additions & 2 deletions packages/adp-tooling/src/preview/change-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ export async function addAnnotationFile(
serviceUrl: datasoruces[dataSourceId].uri,
fileName: basename(dataSource[annotationDataSourceKey].uri)
},
variant: getVariant(projectRoot),
variant: await getVariant(projectRoot),
isCommand: false
},
fs
Expand All @@ -272,7 +272,7 @@ export async function addAnnotationFile(
* @returns Promise<ManifestService>
*/
async function getManifestService(basePath: string, logger: Logger): Promise<ManifestService> {
const variant = getVariant(basePath);
const variant = await getVariant(basePath);
const { target, ignoreCertErrors = false } = await getAdpConfig(basePath, join(basePath, FileName.Ui5Yaml));
const provider = await createAbapServiceProvider(target, { ignoreCertErrors }, true, logger);
return await ManifestService.initMergedManifest(provider, basePath, variant, logger as unknown as ToolsLogger);
Expand Down
2 changes: 1 addition & 1 deletion packages/adp-tooling/src/preview/routes-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ export default class RoutesHandler {
private async getManifestService(): Promise<ManifestService> {
const project = this.util.getProject();
const basePath = project.getRootPath();
const variant = getVariant(basePath);
const variant = await getVariant(basePath);
const { target, ignoreCertErrors = false } = await getAdpConfig(
basePath,
path.join(basePath, FileName.Ui5Yaml)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ import type { ListQuestion, FileBrowserQuestion, YUIQuestion } from '@sap-ux/inq
import type { ManifestNamespace } from '@sap-ux/project-access';
import { AnnotationFileSelectType, type AddAnnotationsAnswers } from '../../types';
import { t } from '../../i18n';
import { filterDataSourcesByType } from '@sap-ux/project-access';
import { filterDataSourcesByType, getWebappPath, DirName } from '@sap-ux/project-access';
import { existsSync } from 'fs';
import { validateEmptyString } from '@sap-ux/project-input-validator';
import { join, isAbsolute, sep } from 'path';
import { join, isAbsolute, basename } from 'path';

/**
* Gets the prompts for adding annotations to OData service.
Expand Down Expand Up @@ -60,7 +60,7 @@ export function getPrompts(
default: '',
when: (answers: AddAnnotationsAnswers) =>
answers.id !== '' && answers.fileSelectOption === AnnotationFileSelectType.ExistingFile,
validate: (value) => {
validate: async (value: string) => {
const validationResult = validateEmptyString(value);
if (typeof validationResult === 'string') {
return validationResult;
Expand All @@ -71,8 +71,11 @@ export function getPrompts(
return t('validators.fileDoesNotExist');
}

const fileName = filePath.split(sep).pop();
if (existsSync(join(basePath, 'webapp', 'changes', 'annotations', fileName))) {
if (
existsSync(
join(await getWebappPath(basePath), DirName.Changes, DirName.Annotations, basename(filePath))
)
) {
return t('validators.annotationFileAlreadyExists');
}

Expand Down
4 changes: 2 additions & 2 deletions packages/adp-tooling/src/writer/inbound-navigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export async function generateInboundConfig(
fs = create(createStorage());
}

const variant = getVariant(basePath, fs);
const variant = await getVariant(basePath, fs);

if (!config?.inboundId) {
config.addInboundId = true;
Expand All @@ -34,7 +34,7 @@ export async function generateInboundConfig(

enhanceInboundConfig(config, variant.id, variant.content as Content[]);

updateVariant(basePath, variant, fs);
await updateVariant(basePath, variant, fs);
await updateI18n(basePath, variant.id, config, fs);

return fs;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,9 @@ describe('ManifestService', () => {
describe('initMergedManifest', () => {
it('should initialize and fetch the merged manifest', async () => {
const variant = { id: 'descriptorVariantId', reference: 'referenceAppId' };
(getWebappFiles as jest.MockedFunction<typeof getWebappFiles>).mockReturnValue([
{ relativePath: 'path', content: 'content' }
]);
(getWebappFiles as jest.MockedFunction<typeof getWebappFiles>).mockReturnValue(
Promise.resolve([{ relativePath: 'path', content: 'content' }])
);
manifestService = await ManifestService.initMergedManifest(
provider,
'basePath',
Expand Down
40 changes: 26 additions & 14 deletions packages/adp-tooling/test/unit/base/helper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,18 +40,18 @@ describe('helper', () => {
jest.clearAllMocks();
});

test('should return variant', () => {
test('should return variant', async () => {
readFileSyncMock.mockImplementation(() => mockVariant);

expect(getVariant(basePath)).toStrictEqual(JSON.parse(mockVariant));
expect(await getVariant(basePath)).toStrictEqual(JSON.parse(mockVariant));
});

test('should return variant using fs editor', () => {
test('should return variant using fs editor', async () => {
const fs = {
readJSON: jest.fn().mockReturnValue(JSON.parse(mockVariant))
} as unknown as Editor;

const result = getVariant(basePath, fs);
const result = await getVariant(basePath, fs);

expect(fs.readJSON).toHaveBeenCalledWith(join(basePath, 'webapp', 'manifest.appdescr_variant'));
expect(result).toStrictEqual(JSON.parse(mockVariant));
Expand All @@ -68,8 +68,8 @@ describe('helper', () => {
jest.clearAllMocks();
});

it('should write the updated variant content to the manifest file', () => {
updateVariant(basePath, mockVariant, fs);
it('should write the updated variant content to the manifest file', async () => {
await updateVariant(basePath, mockVariant, fs);

expect(fs.writeJSON).toHaveBeenCalledWith(
join(basePath, 'webapp', 'manifest.appdescr_variant'),
Expand All @@ -85,7 +85,7 @@ describe('helper', () => {
jest.clearAllMocks();
});

it('should return true if valid FLP configuration exists', () => {
it('should return true if valid FLP configuration exists', async () => {
readFileSyncMock.mockReturnValue(
JSON.stringify({
content: [
Expand All @@ -95,13 +95,13 @@ describe('helper', () => {
})
);

const result = flpConfigurationExists(basePath);
const result = await flpConfigurationExists(basePath);

expect(result).toBe(true);
expect(readFileSyncMock).toHaveBeenCalledWith(appDescrPath, 'utf-8');
});

it('should return false if no valid FLP configuration exists', () => {
it('should return false if no valid FLP configuration exists', async () => {
readFileSyncMock.mockReturnValue(
JSON.stringify({
content: [
Expand All @@ -111,18 +111,18 @@ describe('helper', () => {
})
);

const result = flpConfigurationExists(basePath);
const result = await flpConfigurationExists(basePath);

expect(result).toBe(false);
expect(readFileSyncMock).toHaveBeenCalledWith(appDescrPath, 'utf-8');
});

it('should throw an error if getVariant fails', () => {
it('should throw an error if getVariant fails', async () => {
readFileSyncMock.mockImplementation(() => {
throw new Error('Failed to retrieve variant');
});

expect(() => flpConfigurationExists(basePath)).toThrow(
await expect(flpConfigurationExists(basePath)).rejects.toThrow(
'Failed to check if FLP configuration exists: Failed to retrieve variant'
);
expect(readFileSyncMock).toHaveBeenCalledWith(appDescrPath, 'utf-8');
Expand Down Expand Up @@ -163,8 +163,20 @@ describe('helper', () => {
jest.clearAllMocks();
});

test('should return webapp files', () => {
expect(getWebappFiles(basePath)).toEqual([
test('should return webapp files', async () => {
jest.spyOn(UI5Config, 'newInstance').mockResolvedValue({
findCustomMiddleware: jest.fn().mockReturnValue({
configuration: {
adp: mockAdp
}
} as Partial<CustomMiddleware> as CustomMiddleware<object>),
getConfiguration: jest.fn().mockReturnValue({
paths: {
webapp: 'webapp'
}
})
} as Partial<UI5Config> as UI5Config);
expect(await getWebappFiles(basePath)).toEqual([
{
relativePath: join('i18n', 'i18n.properties'),
content: expect.any(String)
Expand Down
4 changes: 2 additions & 2 deletions packages/adp-tooling/test/unit/preview/adp-preview.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import type { ReaderCollection } from '@ui5/fs';
import type { SuperTest, Test } from 'supertest';
import * as fs from 'fs';

import { AdpPreview } from '../../../src/preview/adp-preview';
import { AdpPreview } from '../../../src';
import type { AdpPreviewConfig, CommonChangeProperties } from '../../../src';
import * as helper from '../../../src/base/helper';
import * as editors from '../../../src/writer/editors';
Expand Down Expand Up @@ -476,7 +476,7 @@ describe('AdaptationProject', () => {
middlewareUtil,
logger
);
jest.spyOn(helper, 'getVariant').mockReturnValue({
jest.spyOn(helper, 'getVariant').mockResolvedValue({
content: [],
id: 'adp/project',
layer: 'VENDOR',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ id=\\"btn-30303030\\""
}
})
} as any);
jest.spyOn(helper, 'getVariant').mockReturnValue({
jest.spyOn(helper, 'getVariant').mockResolvedValue({
content: [],
id: 'adp/project',
layer: 'VENDOR',
Expand Down Expand Up @@ -578,7 +578,6 @@ id=\\"btn-30303030\\""
error: jest.fn()
};

const fragmentName = 'Share';
const change = {
changeType: 'appdescr_app_addAnnotationsToOData',
content: {
Expand Down
Loading