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: CPE loading changes from backend and not from workspace #2429

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
943265f
fix: CPE loading changes from backend and not from workspace
mmilko01 Oct 1, 2024
30bec72
fix: set workspacePath as URL parameter
mmilko01 Oct 2, 2024
11119c3
Merge branch 'main' into fix/cpe-loading-backend-changes
mmilko01 Oct 3, 2024
ca0327f
refactor: move version validation in client
mmilko01 Oct 7, 2024
b1b9f9d
Merge branch 'main' into fix/cpe-loading-backend-changes
mmilko01 Oct 7, 2024
40ccd8b
chore: update changeset
mmilko01 Oct 7, 2024
c1178f9
test: fix existing tests
mmilko01 Oct 7, 2024
f36b1a2
Merge branch 'fix/cpe-loading-backend-changes' of https://github.com/…
mmilko01 Oct 7, 2024
7c81e2a
test: enhance tests
mmilko01 Oct 7, 2024
cd4dedd
fix: pass URL parameter using params parameter
mmilko01 Oct 7, 2024
e31fc3e
chore: add comment
mmilko01 Oct 7, 2024
dbeb08c
fix: wrong assignment
mmilko01 Oct 7, 2024
130d7a2
fix: decode params
mmilko01 Oct 7, 2024
c66bee2
Merge branch 'main' into fix/cpe-loading-backend-changes
mmilko01 Oct 8, 2024
57805ce
refacor: enhance set nested property algorithm
mmilko01 Oct 9, 2024
33efc0f
test: add init test with descriptor
mmilko01 Oct 9, 2024
dd8a125
Merge branch 'main' into fix/cpe-loading-backend-changes
mmilko01 Oct 10, 2024
aa96419
Merge branch 'main' into fix/cpe-loading-backend-changes
mmilko01 Oct 15, 2024
db72b4a
Merge branch 'main' into fix/cpe-loading-backend-changes
mmilko01 Oct 16, 2024
819600b
refactor: move nested function
mmilko01 Oct 16, 2024
d78eef9
Merge branch 'fix/cpe-loading-backend-changes' of https://github.com/…
mmilko01 Oct 16, 2024
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
8 changes: 8 additions & 0 deletions .changeset/plenty-chairs-thank.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"@sap-ux/adp-tooling": patch
"@sap-ux/axios-extension": patch
"@sap-ux-private/preview-middleware-client": patch
"@sap-ux/preview-middleware": patch
---

CPE loading changes from backend and not from workspace
2 changes: 1 addition & 1 deletion packages/adp-tooling/src/preview/adp-preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ export class AdpPreview {
}
const buffer = zip.toBuffer();

this.mergedDescriptor = (await this.lrep.mergeAppDescriptorVariant(buffer))[this.descriptorVariantId];
this.mergedDescriptor = (await this.lrep.mergeAppDescriptorVariant(buffer, '//'))[this.descriptorVariantId];
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ describe('AdaptationProject', () => {
.reply(200)
.persist(true);
nock(backend)
.put('/sap/bc/lrep/appdescr_variant_preview/')
.put('/sap/bc/lrep/appdescr_variant_preview/?workspacePath=//')
.reply(200, {
'my.adaptation': mockMergedDescriptor
})
Expand Down
15 changes: 13 additions & 2 deletions packages/axios-extension/src/abap/lrep-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,13 +166,24 @@ export class LayeredRepositoryService extends Axios implements Service {
* Merge a given app descriptor variant with the stord app descriptor.
*
* @param appDescriptorVariant zip file containing an app descriptor variant
* @param workspacePath value for workspacePath URL parameter
* @returns a promise with an object containing merged app descriptors with their id as keys.
*/
public async mergeAppDescriptorVariant(
appDescriptorVariant: Buffer
appDescriptorVariant: Buffer,
workspacePath?: string
): Promise<{ [key: string]: MergedAppDescriptor }> {
const path = '/appdescr_variant_preview/';
const params = new URLSearchParams(this.defaults?.params);

if (workspacePath) {
params.append('workspacePath', workspacePath);
}

try {
const response = await this.put('/appdescr_variant_preview/', appDescriptorVariant, {
const response = await this.put(path, appDescriptorVariant, {
paramsSerializer: (params) => decodeURIComponent(params.toString()),
nikmace marked this conversation as resolved.
Show resolved Hide resolved
params,
headers: {
'Content-Type': 'application/zip'
}
Expand Down
13 changes: 11 additions & 2 deletions packages/axios-extension/test/abap/lrep-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { LayeredRepositoryService, createForAbap } from '../../src';
import type { AxiosError } from '../../src';
import type { ToolsLogger } from '@sap-ux/logger';
import * as Logger from '@sap-ux/logger';
import { describe } from 'node:test';

const loggerMock: ToolsLogger = {
debug: jest.fn(),
Expand Down Expand Up @@ -264,14 +263,24 @@ describe('LayeredRepositoryService', () => {
});

describe('mergeAppDescriptorVariant', () => {
const mockResult = { hello: 'world' };
test('merge valid app variant', async () => {
const mockResult = { hello: 'world', components: [{ url: '/webapp' }] };
nock(server).put(`${LayeredRepositoryService.PATH}/appdescr_variant_preview/`).reply(200, mockResult);

const mergedDescriptor = await service.mergeAppDescriptorVariant(Buffer.from('~test'));
expect(mergedDescriptor).toEqual(mockResult);
});

test('merge valid app variant with adjusted workspace path', async () => {
const mockResult = { hello: 'world', components: [{ url: '/' }] };
nock(server)
.put(`${LayeredRepositoryService.PATH}/appdescr_variant_preview/?workspacePath=//`)
.reply(200, mockResult);

const mergedDescriptor = await service.mergeAppDescriptorVariant(Buffer.from('~test'), '//');
expect(mergedDescriptor).toEqual(mockResult);
});

test('error is thrown', async () => {
nock(server).put(`${LayeredRepositoryService.PATH}/appdescr_variant_preview/`).reply(500);
try {
Expand Down
40 changes: 39 additions & 1 deletion packages/preview-middleware-client/src/flp/bootstrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,16 @@ async function ushellBootstrap(fnCallback) {
const response = await fetch(`${basePath}/resources/sap-ui-version.json`);
const json = await response.json();
const version = json?.libraries?.find((lib) => lib.name === 'sap.ui.core')?.version ?? '1.121.0';
const majorUi5Version = parseInt(version.split('.')[0], 10);
const [major, minor] = version.split('.');
const majorUi5Version = parseInt(major, 10);
const minorUi5Version = parseInt(minor, 10);
if (majorUi5Version >= 2) {
src = `${basePath}/resources/sap/ushell/bootstrap/sandbox2.js`;
}

if (majorUi5Version === 1 && minorUi5Version < 72) {
tobiasqueck marked this conversation as resolved.
Show resolved Hide resolved
removeAsyncHintsRequests();
}
} catch (error) {
console.warn('Failed to fetch sap-ui-version.json. Assuming it is a 1.x version.');
}
Expand All @@ -33,6 +39,38 @@ async function ushellBootstrap(fnCallback) {
}
}

/**
* For UI5 version 1.71 and below, we need to remove the asyncHints.requests
* to load the changes in an Adaptation project.
* This logic needs to be executed here to have a reliable check for
* UI5 version and remove the asyncHints.requests before the sandbox is loaded.
* The sandbox shell modifies the `window['sap-ushell-config']`.
*/
function removeAsyncHintsRequests() {
const obj = window['sap-ushell-config']['applications'];

if (!obj || typeof obj !== 'object') return;

const stack = [obj];

while (stack.length > 0) {
const current = stack.pop();

if (current.asyncHints) {
if (current.asyncHints.requests) {
current.asyncHints.requests = [];
}
return;
}

for (const key in current) {
if (typeof current[key] === 'object' && current[key] !== null) {
stack.push(current[key]);
}
}
}
}

// eslint-disable-next-line fiori-custom/sap-no-global-define
window['sap-ui-config'] = {
'xx-bootTask': ushellBootstrap
Expand Down
23 changes: 18 additions & 5 deletions packages/preview-middleware/src/base/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { readFileSync } from 'fs';
import { mergeTestConfigDefaults } from './test';
import { type Editor, create } from 'mem-fs-editor';
import { create as createStorage } from 'mem-fs';
import type { MergedAppDescriptor } from '@sap-ux/axios-extension';

export interface CustomConnector {
applyConnector: string;
Expand All @@ -33,7 +34,7 @@ export interface TemplateConfig {
additionalInformation: string;
applicationType: 'URL';
url: string;
applicationDependencies?: { manifest: boolean };
applicationDependencies?: MergedAppDescriptor;
}
>;
ui5: {
Expand Down Expand Up @@ -204,22 +205,34 @@ function getFlexSettings(): TemplateConfig['ui5']['flex'] {
* @param manifest manifest of the additional target app
* @param app configuration for the preview
* @param logger logger instance
* @param descriptor descriptor of the additional target app
*/
export async function addApp(templateConfig: TemplateConfig, manifest: Partial<Manifest>, app: App, logger: Logger) {
export async function addApp(
templateConfig: TemplateConfig,
manifest: Partial<Manifest>,
app: App,
logger: Logger,
descriptor?: MergedAppDescriptor
) {
const id = manifest['sap.app']?.id ?? '';

app.intent ??= {
object: id.replace(/\./g, ''),
action: 'preview'
};

const appName = `${app.intent?.object}-${app.intent?.action}`;
templateConfig.ui5.resources[id] = app.target;
templateConfig.apps[`${app.intent?.object}-${app.intent?.action}`] = {
templateConfig.apps[appName] = {
title: (await getI18nTextFromProperty(app.local, manifest['sap.app']?.title, logger)) ?? id,
description: (await getI18nTextFromProperty(app.local, manifest['sap.app']?.description, logger)) ?? '',
additionalInformation: `SAPUI5.Component=${app.componentId ?? id}`,
applicationType: 'URL',
url: app.target,
applicationDependencies: { manifest: true }
tobiasqueck marked this conversation as resolved.
Show resolved Hide resolved
url: app.target
};
if (descriptor) {
templateConfig.apps[appName].applicationDependencies = descriptor;
}
}

/**
Expand Down
16 changes: 11 additions & 5 deletions packages/preview-middleware/src/base/flp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
type OperationType
} from '@sap-ux/adp-tooling';
import { isAppStudio, exposePort } from '@sap-ux/btp-utils';
import type { MergedAppDescriptor } from '@sap-ux/axios-extension';

import { deleteChange, readChanges, writeChange } from './flex';
import { generateImportList, mergeTestConfigDefaults } from './test';
Expand Down Expand Up @@ -104,8 +105,14 @@ export class FlpSandbox {
* @param manifest application manifest
* @param componentId optional componentId e.g. for adaptation projects
* @param resources optional additional resource mappings
* @param descriptor optional additional descriptor mappings
*/
async init(manifest: Manifest, componentId?: string, resources: Record<string, string> = {}): Promise<void> {
async init(
manifest: Manifest,
componentId?: string,
resources: Record<string, string> = {},
descriptor?: MergedAppDescriptor
): Promise<void> {
this.createFlexHandler();
this.config.libs ??= await this.hasLocateReuseLibsScript();
const id = manifest['sap.app'].id;
Expand All @@ -120,7 +127,8 @@ export class FlpSandbox {
local: '.',
intent: this.config.intent
},
this.logger
this.logger,
descriptor
);
this.addStandardRoutes();
if (this.rta) {
Expand Down Expand Up @@ -531,10 +539,8 @@ export async function initAdp(
}

const descriptor = adp.descriptor;
descriptor.asyncHints.requests = [];
const { name, manifest } = descriptor;

await flp.init(manifest, name, adp.resources);
await flp.init(manifest, name, adp.resources, descriptor);
flp.router.use(adp.descriptor.url, adp.proxy.bind(adp) as RequestHandler);
flp.addOnChangeRequestHandler(adp.onChangeRequest.bind(adp));
flp.router.use(json());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ exports[`config generatePreviewFiles minimum settings 1`] = `
}
}
},
applications: {\\"app-preview\\":{\\"title\\":\\"My Simple App\\",\\"description\\":\\"This is a very simple application.\\",\\"additionalInformation\\":\\"SAPUI5.Component=test.fe.v2.app\\",\\"applicationType\\":\\"URL\\",\\"url\\":\\"..\\",\\"applicationDependencies\\":{\\"manifest\\":true}}}
applications: {\\"app-preview\\":{\\"title\\":\\"My Simple App\\",\\"description\\":\\"This is a very simple application.\\",\\"additionalInformation\\":\\"SAPUI5.Component=test.fe.v2.app\\",\\"applicationType\\":\\"URL\\",\\"url\\":\\"..\\"}}
};
</script>

Expand Down Expand Up @@ -190,7 +190,7 @@ Object {
}
}
},
applications: {\\"simpleApp-preview\\":{\\"title\\":\\"My Simple App\\",\\"description\\":\\"This is a very simple application.\\",\\"additionalInformation\\":\\"SAPUI5.Component=test.fe.v2.app\\",\\"applicationType\\":\\"URL\\",\\"url\\":\\"/apps/simple-app\\",\\"applicationDependencies\\":{\\"manifest\\":true}},\\"testfev2other-preview\\":{\\"title\\":\\"My Other App\\",\\"description\\":\\"This is a very simple application.\\",\\"additionalInformation\\":\\"SAPUI5.Component=test.fe.v2.other\\",\\"applicationType\\":\\"URL\\",\\"url\\":\\"/apps/other-app\\",\\"applicationDependencies\\":{\\"manifest\\":true}}}
applications: {\\"simpleApp-preview\\":{\\"title\\":\\"My Simple App\\",\\"description\\":\\"This is a very simple application.\\",\\"additionalInformation\\":\\"SAPUI5.Component=test.fe.v2.app\\",\\"applicationType\\":\\"URL\\",\\"url\\":\\"/apps/simple-app\\"},\\"testfev2other-preview\\":{\\"title\\":\\"My Other App\\",\\"description\\":\\"This is a very simple application.\\",\\"additionalInformation\\":\\"SAPUI5.Component=test.fe.v2.other\\",\\"applicationType\\":\\"URL\\",\\"url\\":\\"/apps/other-app\\"}}
};
</script>

Expand Down Expand Up @@ -271,7 +271,7 @@ Object {
}
}
},
applications: {\\"myapp-myaction\\":{\\"title\\":\\"My Simple App\\",\\"description\\":\\"This is a very simple application.\\",\\"additionalInformation\\":\\"SAPUI5.Component=test.fe.v2.app\\",\\"applicationType\\":\\"URL\\",\\"url\\":\\"..\\",\\"applicationDependencies\\":{\\"manifest\\":true}}}
applications: {\\"myapp-myaction\\":{\\"title\\":\\"My Simple App\\",\\"description\\":\\"This is a very simple application.\\",\\"additionalInformation\\":\\"SAPUI5.Component=test.fe.v2.app\\",\\"applicationType\\":\\"URL\\",\\"url\\":\\"..\\"}}
};
</script>

Expand Down
Loading
Loading