Skip to content

Commit d2aae59

Browse files
kjose90github-actions[bot]devinealongieirl
authored
Resolve Transport Request Checks for Download Quick Fiori App Deploy (#3819)
* Resolve Transport request in adt app download logic * changeset * test * put transportService call after checking if package is local * sonar * clean up * Linting auto fix commit --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Austin Devine <[email protected]> Co-authored-by: J Long <[email protected]>
1 parent d338858 commit d2aae59

File tree

5 files changed

+199
-33
lines changed

5 files changed

+199
-33
lines changed

.changeset/short-pumas-cough.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@sap-ux/repo-app-import-sub-generator': patch
3+
---
4+
5+
Resolve Transport request in adt app download logic

packages/repo-app-import-sub-generator/src/app/app-config.ts

Lines changed: 58 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { TemplateType, type FioriElementsApp, type LROPSettings } from '@sap-ux/
22
import { OdataVersion } from '@sap-ux/odata-service-inquirer';
33
import type { AbapServiceProvider } from '@sap-ux/axios-extension';
44
import type { Editor } from 'mem-fs-editor';
5+
import { TransportChecksService } from '@sap-ux/axios-extension';
56
import { t } from '../utils/i18n';
67
import type { AppInfo, QfaJsonConfig } from '../app/types';
78
import { readManifest } from '../utils/file-helpers';
@@ -14,13 +15,61 @@ import { join } from 'node:path';
1415
import { getUI5Versions, type UI5Version } from '@sap-ux/ui5-info';
1516
import { type OdataServiceAnswers } from '@sap-ux/odata-service-inquirer';
1617

18+
/**
19+
* Shared context for downloading and deploying ABAP applications.
20+
*/
21+
export interface AppDownloadContext {
22+
serviceProvider?: AbapServiceProvider;
23+
qfaJson: QfaJsonConfig;
24+
}
25+
26+
/**
27+
* Resolve a transport request for the given app/package context.
28+
* This function performs defensive checks and logs clear, actionable messages.
29+
*
30+
* @param context - AppDownloadContext containing qfaJson and serviceProvider
31+
* @returns { Promise<string> } - Resolved transport request string
32+
* - '' when package is local ('$TMP')
33+
* - '<transport-request-id>' when transport request is found
34+
* - 'REPLACE_WITH_TRANSPORT' when no transport request is found
35+
* @throws Error when the transport check fails
36+
*/
37+
async function resolveTransportRequest(context: AppDownloadContext): Promise<string> {
38+
const { serviceProvider, qfaJson } = context;
39+
const packageName = qfaJson.metadata.package;
40+
const appName = qfaJson.deploymentDetails.repositoryName;
41+
42+
if (packageName === '$TMP') {
43+
return '';
44+
}
45+
46+
try {
47+
const transportService = await serviceProvider?.getAdtService<TransportChecksService>(TransportChecksService);
48+
const transportRequests = await transportService?.getTransportRequests(packageName, appName);
49+
if (transportRequests?.length === 1) {
50+
return transportRequests[0].transportNumber;
51+
}
52+
return 'REPLACE_WITH_TRANSPORT';
53+
} catch (error) {
54+
if (error.message === TransportChecksService.LocalPackageError) {
55+
return '';
56+
}
57+
const msg = t('error.transportCheckFailed', { error: error?.message });
58+
RepoAppDownloadLogger.logger?.error(msg);
59+
throw new Error(msg);
60+
}
61+
}
62+
1763
/**
1864
* Generates the deployment configuration for an ABAP application.
1965
*
20-
* @param {QfaJsonConfig} qfaJson - The QFA JSON configuration containing app details.
66+
* @param {AppDownloadContext} context - The download context with service provider and qfa info.
2167
* @returns {AbapDeployConfig} The deployment configuration containing `target` and `app` info.
2268
*/
23-
export const getAbapDeployConfig = (qfaJson: QfaJsonConfig): AbapDeployConfig => {
69+
export const getAbapDeployConfig = async (context: AppDownloadContext): Promise<AbapDeployConfig> => {
70+
const { qfaJson } = context;
71+
72+
const transportRequest = await resolveTransportRequest(context);
2473
return {
2574
target: {
2675
url: PromptState.baseURL,
@@ -31,7 +80,7 @@ export const getAbapDeployConfig = (qfaJson: QfaJsonConfig): AbapDeployConfig =>
3180
name: qfaJson.deploymentDetails.repositoryName,
3281
package: qfaJson.metadata.package,
3382
description: qfaJson.deploymentDetails.repositoryDescription,
34-
transport: 'REPLACE_WITH_TRANSPORT'
83+
transport: transportRequest
3584
}
3685
} as AbapDeployConfig; // NOSONAR
3786
};
@@ -45,9 +94,9 @@ export const getAbapDeployConfig = (qfaJson: QfaJsonConfig): AbapDeployConfig =>
4594
*/
4695
const fetchServiceMetadata = async (provider: AbapServiceProvider, serviceUrl: string): Promise<string | undefined> => {
4796
try {
48-
const metdata = await provider.service(serviceUrl).metadata();
97+
const metadata = await provider.service(serviceUrl).metadata();
4998
RepoAppDownloadLogger.logger?.debug('Metadata fetched successfully');
50-
return metdata as string | undefined;
99+
return metadata as string | undefined;
51100
} catch (err) {
52101
RepoAppDownloadLogger.logger?.error(t('error.metadataFetchError', { error: err.message }));
53102
}
@@ -59,7 +108,7 @@ const fetchServiceMetadata = async (provider: AbapServiceProvider, serviceUrl: s
59108
*
60109
* @param {AppInfo} app - Selected app information.
61110
* @param {string} extractedProjectPath - Path where the app files are extracted.
62-
* @param {QfaJsonConfig} qfaJson - The QFA JSON configuration containing app details.
111+
* @param {AppDownloadContext} context - The download context with service provider and qfa info.
63112
* @param {OdataServiceAnswers} systemSelection - User's selection of the OData service and system.
64113
* @param {Editor} fs - The file system editor to manipulate project files.
65114
* @returns {Promise<FioriElementsApp<LROPSettings>>} - A promise resolving to the generated app configuration.
@@ -68,13 +117,14 @@ const fetchServiceMetadata = async (provider: AbapServiceProvider, serviceUrl: s
68117
export async function getAppConfig(
69118
app: AppInfo,
70119
extractedProjectPath: string,
71-
qfaJson: QfaJsonConfig,
120+
context: AppDownloadContext,
72121
systemSelection: OdataServiceAnswers,
73122
fs: Editor
74123
): Promise<FioriElementsApp<LROPSettings>> {
75124
try {
76125
const manifest = readManifest(join(extractedProjectPath, FileName.Manifest), fs);
77126
const serviceProvider = PromptState.systemSelection?.connectedSystem?.serviceProvider as AbapServiceProvider;
127+
context.serviceProvider = serviceProvider;
78128
if (!manifest?.['sap.app']?.dataSources) {
79129
RepoAppDownloadLogger.logger?.error(t('error.dataSourcesNotFound'));
80130
}
@@ -116,7 +166,7 @@ export async function getAppConfig(
116166
type: TemplateType.ListReportObjectPage,
117167
settings: {
118168
entityConfig: {
119-
mainEntityName: qfaJson.serviceBindingDetails.mainEntityName
169+
mainEntityName: context.qfaJson.serviceBindingDetails.mainEntityName
120170
}
121171
}
122172
},

packages/repo-app-import-sub-generator/src/app/index.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import { writeApplicationInfoSettings } from '@sap-ux/fiori-tools-settings';
3131
import { generate as generateDeployConfig } from '@sap-ux/abap-deploy-config-writer';
3232
import { PromptState } from '../prompts/prompt-state';
3333
import { PromptNames } from './types';
34-
import { getAbapDeployConfig, getAppConfig } from './app-config';
34+
import { getAbapDeployConfig, getAppConfig, type AppDownloadContext } from './app-config';
3535
import type { AbapDeployConfig } from '@sap-ux/ui5-config';
3636
import { makeValidJson } from '../utils/file-helpers';
3737
import { replaceWebappFiles, validateAndUpdateManifestUI5Version } from '../utils/updates';
@@ -147,19 +147,22 @@ export default class extends Generator {
147147
const qfaJson: QfaJsonConfig = makeValidJson(qfaJsonFilePath, this.fs);
148148
// Generate project files
149149
validateQfaJsonFile(qfaJson);
150+
const context: AppDownloadContext = {
151+
qfaJson
152+
};
150153

151154
// Generate app config
152155
const config = await getAppConfig(
153156
this.answers.selectedApp,
154157
this.extractedProjectPath,
155-
qfaJson,
158+
context,
156159
this.answers.systemSelection,
157160
this.fs
158161
);
159162
await generate(this.projectPath, config, this.fs);
160163

161164
// Generate deploy config
162-
const deployConfig: AbapDeployConfig = getAbapDeployConfig(qfaJson);
165+
const deployConfig: AbapDeployConfig = await getAbapDeployConfig(context);
163166
await generateDeployConfig(this.projectPath, deployConfig, undefined, this.fs);
164167

165168
if (this.vscode) {

packages/repo-app-import-sub-generator/src/translations/repo-app-import-sub-generator.i18n.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@
3939
"invalidManifestStructureError": "Invalid `manifest.json file` structure: `sap.ui5` or `sap.app` are missing. Check they exist."
4040
},
4141
"quickDeployedAppDownloadErrors": {
42-
"noAppsFound": "No application with the ID {{ appId }} was found in the system. Please check if the application is deployed correctly or select another app."
42+
"noAppsFound": "No application with the ID {{ appId }} was found in the system. Please check if the application is deployed correctly or select another app.",
43+
"transportCheckFailed": "Transport request resolution failed with error: {error}."
4344
}
4445
},
4546
"warn": {

packages/repo-app-import-sub-generator/test/app-config.test.ts

Lines changed: 128 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,11 @@ describe('getAppConfig', () => {
175175
mainEntityName: mockQfaJson.serviceBindingDetails.mainEntityName
176176
}
177177
};
178-
const result = await getAppConfig(mockApp, '/path/to/project', mockQfaJsonWithoutNavEntity, mockSystem, mockFs);
178+
const context = {
179+
qfaJson: mockQfaJsonWithoutNavEntity,
180+
serviceProvider: mockServiceProvider
181+
};
182+
const result = await getAppConfig(mockApp, '/path/to/project', context, mockSystem, mockFs);
179183
expect(result).toEqual(expectedAppConfig);
180184
});
181185

@@ -207,13 +211,11 @@ describe('getAppConfig', () => {
207211
navigation_entity: mockQfaJson.serviceBindingDetails.navigationEntity
208212
}
209213
};
210-
const result = await getAppConfig(
211-
mockApp,
212-
'/path/to/project',
213-
mockQfaJsonJsonWithNavEntity,
214-
mockSystem,
215-
mockFs
216-
);
214+
const context = {
215+
qfaJson: mockQfaJsonJsonWithNavEntity,
216+
serviceProvider: mockSystem.connectedSystem?.serviceProvider as AbapServiceProvider
217+
};
218+
const result = await getAppConfig(mockApp, '/path/to/project', context, mockSystem, mockFs);
217219
expect(result).toEqual(expectedAppConfig);
218220
});
219221

@@ -249,13 +251,11 @@ describe('getAppConfig', () => {
249251
navigation_entity: mockQfaJson.serviceBindingDetails.navigationEntity
250252
}
251253
};
252-
const result = await getAppConfig(
253-
mockApp,
254-
'/path/to/project',
255-
mockQfaJsonJsonWithNavEntity,
256-
mockSystem,
257-
mockFs
258-
);
254+
const context = {
255+
qfaJson: mockQfaJsonJsonWithNavEntity,
256+
serviceProvider: mockSystem.connectedSystem?.serviceProvider as AbapServiceProvider
257+
};
258+
const result = await getAppConfig(mockApp, '/path/to/project', context, mockSystem, mockFs);
259259
expect(result).toEqual({
260260
...expectedAppConfig,
261261
ui5: {
@@ -270,7 +270,11 @@ describe('getAppConfig', () => {
270270
};
271271

272272
(readManifest as jest.Mock).mockReturnValue(mockManifest);
273-
const result = await getAppConfig(mockApp, '/path/to/project', mockQfaJson, mockSystem, mockFs);
273+
const context = {
274+
qfaJson: mockQfaJson,
275+
serviceProvider: mockSystem.connectedSystem?.serviceProvider as AbapServiceProvider
276+
};
277+
const result = await getAppConfig(mockApp, '/path/to/project', context, mockSystem, mockFs);
274278
expect(RepoAppDownloadLogger.logger.error).toHaveBeenCalledWith(t('error.dataSourcesNotFound'));
275279
});
276280

@@ -293,8 +297,11 @@ describe('getAppConfig', () => {
293297
};
294298

295299
(readManifest as jest.Mock).mockReturnValue(mockManifest);
296-
297-
await getAppConfig(mockApp, '/path/to/project', mockQfaJson, mockSystem, mockFs);
300+
const context = {
301+
qfaJson: mockQfaJson,
302+
serviceProvider: mockSystem.connectedSystem?.serviceProvider as AbapServiceProvider
303+
};
304+
await getAppConfig(mockApp, '/path/to/project', context, mockSystem, mockFs);
298305
expect(RepoAppDownloadLogger.logger?.error).toHaveBeenCalledWith(
299306
t('error.metadataFetchError', { error: errorMsg })
300307
);
@@ -343,13 +350,21 @@ describe('getAppConfig', () => {
343350
minimum_ui5_version: null
344351
}
345352
} as unknown as QfaJsonConfig;
346-
await getAppConfig(mockApp, '/path/to/project', mockQfaJsonJsonWithoutUi5Version, mockSystem, mockFs);
353+
const context = {
354+
qfaJson: mockQfaJsonJsonWithoutUi5Version,
355+
serviceProvider: mockSystem.connectedSystem?.serviceProvider as AbapServiceProvider
356+
};
357+
await getAppConfig(mockApp, '/path/to/project', context, mockSystem, mockFs);
347358
expect(RepoAppDownloadLogger.logger?.error).not.toHaveBeenCalled();
348359
});
349360
});
350361

351362
describe('getAbapDeployConfig', () => {
352-
it('should generate the correct deployment configuration', () => {
363+
beforeEach(() => {
364+
jest.clearAllMocks();
365+
});
366+
367+
it('should generate the correct deployment configuration', async () => {
353368
const expectedConfig = {
354369
target: {
355370
url: 'https://test-url.com',
@@ -363,7 +378,99 @@ describe('getAbapDeployConfig', () => {
363378
transport: 'REPLACE_WITH_TRANSPORT'
364379
}
365380
};
366-
const result = getAbapDeployConfig(mockQfaJson);
381+
const transportMock: { transportNumber: string }[] = [];
382+
const mockTransportService = {
383+
getTransportRequests: jest.fn().mockResolvedValue(transportMock)
384+
};
385+
const mockServiceProvider = {
386+
getAdtService: jest.fn().mockResolvedValue(mockTransportService)
387+
} as unknown as AbapServiceProvider;
388+
const context = {
389+
qfaJson: mockQfaJson,
390+
serviceProvider: mockServiceProvider
391+
};
392+
const result = await getAbapDeployConfig(context);
367393
expect(result).toEqual(expectedConfig);
368394
});
395+
396+
it('should return empty transport for local package ($TMP)', async () => {
397+
const localQfa = {
398+
...mockQfaJson,
399+
metadata: { ...mockQfaJson.metadata, package: '$TMP' }
400+
} as QfaJsonConfig;
401+
const mockServiceProvider = {
402+
getAdtService: jest.fn()
403+
} as unknown as AbapServiceProvider;
404+
const context = {
405+
qfaJson: localQfa,
406+
serviceProvider: mockServiceProvider
407+
};
408+
const result = await getAbapDeployConfig(context);
409+
expect(result.app.transport).toBe('');
410+
});
411+
412+
it('should use transport returned by transport service when transport request is available', async () => {
413+
const transportMock = [{ transportNumber: 'LT12345' }];
414+
const mockTransportService = {
415+
getTransportRequests: jest.fn().mockResolvedValue(transportMock)
416+
};
417+
const mockServiceProvider = {
418+
getAdtService: jest.fn().mockResolvedValue(mockTransportService)
419+
} as unknown as AbapServiceProvider;
420+
421+
const context = {
422+
qfaJson: mockQfaJson,
423+
serviceProvider: mockServiceProvider
424+
};
425+
const result = await getAbapDeployConfig(context);
426+
expect(mockServiceProvider.getAdtService).toHaveBeenCalled();
427+
expect(result.app.transport).toBe('LT12345');
428+
});
429+
430+
it('should return REPLACE_WITH_TRANSPORT when transport service returns no transports', async () => {
431+
const mockTransportService = {
432+
getTransportRequests: jest.fn().mockResolvedValue([])
433+
};
434+
const mockServiceProvider = {
435+
getAdtService: jest.fn().mockResolvedValue(mockTransportService)
436+
} as unknown as AbapServiceProvider;
437+
438+
const context = {
439+
qfaJson: mockQfaJson,
440+
serviceProvider: mockServiceProvider
441+
};
442+
const result = await getAbapDeployConfig(context);
443+
expect(result.app.transport).toBe('REPLACE_WITH_TRANSPORT');
444+
});
445+
446+
it('should generate new transport request when serviceProvider is not available in context', async () => {
447+
const context = {
448+
qfaJson: mockQfaJson
449+
} as unknown as { qfaJson: QfaJsonConfig };
450+
451+
const result = await getAbapDeployConfig(context);
452+
expect(result.app.transport).toBe('REPLACE_WITH_TRANSPORT');
453+
});
454+
455+
it('should log and throw error when transport service throws unexpected error', async () => {
456+
const errorMsg = 'Some backend error';
457+
const mockTransportService = {
458+
getTransportRequests: jest.fn().mockRejectedValue(new Error(errorMsg))
459+
};
460+
const mockServiceProvider = {
461+
getAdtService: jest.fn().mockResolvedValue(mockTransportService)
462+
} as unknown as AbapServiceProvider;
463+
464+
const context = {
465+
qfaJson: mockQfaJson,
466+
serviceProvider: mockServiceProvider
467+
};
468+
469+
await expect(getAbapDeployConfig(context)).rejects.toThrow(
470+
t('error.transportCheckFailed', { error: errorMsg })
471+
);
472+
expect(RepoAppDownloadLogger.logger.error).toHaveBeenCalledWith(
473+
t('error.transportCheckFailed', { error: errorMsg })
474+
);
475+
});
369476
});

0 commit comments

Comments
 (0)