fix: refresh descriptor after manifest changes

.changeset/light-moles-search.md

@@ -0,0 +1,6 @@
+---
+"@sap-ux/adp-tooling": patch
+"@sap-ux/preview-middleware": patch
+---
+
+fix: refresh descriptor after manifest changes

packages/adp-tooling/src/preview/adp-preview.ts

@@ -12,7 +12,6 @@ import RoutesHandler from './routes-handler';
 import type { AdpPreviewConfig, CommonChangeProperties, DescriptorVariant, OperationType } from '../types';
 import type { Editor } from 'mem-fs-editor';
 import { addXmlFragment, isAddXMLChange, moduleNameContentMap, tryFixChange } from './change-handler';
-
 declare global {
     // false positive, const can't be used here https://github.com/eslint/eslint/issues/15896
     // eslint-disable-next-line no-var
@@ -119,9 +118,12 @@ export class AdpPreview {
 
     /**
      * Synchronize local changes with the backend.
-     *
+     * The descriptor is refreshed only if the global flag is set to true.
      */
     async sync(): Promise<void> {
+        if (!global.__SAP_UX_MANIFEST_SYNC_REQUIRED__ && this.mergedDescriptor) {
+            return;
+        }
         if (!this.lrep || !this.descriptorVariantId) {
             throw new Error('Not initialized');
         }
@@ -133,6 +135,7 @@ export class AdpPreview {
         const buffer = zip.toBuffer();
 
         this.mergedDescriptor = (await this.lrep.mergeAppDescriptorVariant(buffer, '//'))[this.descriptorVariantId];
+        global.__SAP_UX_MANIFEST_SYNC_REQUIRED__ = false;
     }
 
     /**
@@ -144,10 +147,7 @@ export class AdpPreview {
      */
     async proxy(req: Request, res: Response, next: NextFunction): Promise<void> {
         if (req.path === '/manifest.json') {
-            if (global.__SAP_UX_MANIFEST_SYNC_REQUIRED__) {
-                await this.sync();
-                global.__SAP_UX_MANIFEST_SYNC_REQUIRED__ = false;
-            }
+            await this.sync();
             res.status(200);
             res.send(JSON.stringify(this.descriptor.manifest, undefined, 2));
         } else if (req.path === '/Component-preload.js') {

packages/adp-tooling/test/unit/preview/adp-preview.test.ts

@@ -120,21 +120,23 @@ describe('AdaptationProject', () => {
         }
     };
 
-    beforeAll(() => {
-        nock(backend)
-            .get((path) => path.startsWith('/sap/bc/lrep/actions/getcsrftoken/'))
-            .reply(200)
-            .persist(true);
-        nock(backend)
-            .put('/sap/bc/lrep/appdescr_variant_preview/?workspacePath=//')
-            .reply(200, {
-                'my.adaptation': mockMergedDescriptor
-            })
-            .persist(true);
-    });
-
     const logger = new ToolsLogger();
     describe('init', () => {
+        beforeAll(() => {
+            nock(backend)
+                .get((path) => path.startsWith('/sap/bc/lrep/actions/getcsrftoken/'))
+                .reply(200)
+                .persist(true);
+            nock(backend)
+                .put('/sap/bc/lrep/appdescr_variant_preview/?workspacePath=//')
+                .reply(200, {
+                    'my.adaptation': mockMergedDescriptor
+                })
+                .persist(true);
+        });
+        afterAll(() => {
+            nock.cleanAll();
+        });
         test('default (no) config', async () => {
             const adp = new AdpPreview(
                 {
@@ -180,7 +182,37 @@ describe('AdaptationProject', () => {
         });
     });
     describe('sync', () => {
+        let secondCall: boolean = false;
+        beforeAll(() => {
+            nock(backend)
+                .get((path) => path.startsWith('/sap/bc/lrep/actions/getcsrftoken/'))
+                .reply(200)
+                .persist(true);
+            nock(backend)
+                .put('/sap/bc/lrep/appdescr_variant_preview/?workspacePath=//')
+                .reply(200, () => {
+                    if (secondCall) {
+                        return {
+                            'my.adaptation': 'testDescriptor'
+                        };
+                    }
+                    return {
+                        'my.adaptation': mockMergedDescriptor
+                    };
+                })
+                .persist(true);
+        });
+
+        afterAll(() => {
+            nock.cleanAll();
+        });
+
+        afterEach(() => {
+            global.__SAP_UX_MANIFEST_SYNC_REQUIRED__ = false;
+        });
+
         test('updates merged descriptor', async () => {
+            global.__SAP_UX_MANIFEST_SYNC_REQUIRED__ = true;
             const adp = new AdpPreview(
                 {
                     target: {
@@ -203,11 +235,102 @@ describe('AdaptationProject', () => {
             await adp.sync();
             expect(adp.descriptor).toBeDefined();
         });
+
+        test('skip updating the merge descriptor if no manifest changes and descriptor was already fetched', async () => {
+            const adp = new AdpPreview(
+                {
+                    target: {
+                        url: backend
+                    }
+                },
+                mockProject as unknown as ReaderCollection,
+                middlewareUtil,
+                logger
+            );
+
+            mockProject.byGlob.mockResolvedValueOnce([
+                {
+                    getPath: () => '/manifest.appdescr_variant',
+                    getBuffer: () => Buffer.from(descriptorVariant)
+                }
+            ]);
+            await adp.init(JSON.parse(descriptorVariant));
+            (adp as any).mergedDescriptor = undefined;
+            await adp.sync();
+            expect(adp.descriptor).toEqual(mockMergedDescriptor);
+            secondCall = true;
+            await adp.sync();
+            secondCall = false;
+            expect(adp.descriptor).not.toEqual('testDescriptor');
+        });
+
+        test('update descriptor if no manifest changes, but this is first descriptor fetch', async () => {
+            const adp = new AdpPreview(
+                {
+                    target: {
+                        url: backend
+                    }
+                },
+                mockProject as unknown as ReaderCollection,
+                middlewareUtil,
+                logger
+            );
+
+            mockProject.byGlob.mockResolvedValueOnce([
+                {
+                    getPath: () => '/manifest.appdescr_variant',
+                    getBuffer: () => Buffer.from(descriptorVariant)
+                }
+            ]);
+            await adp.init(JSON.parse(descriptorVariant));
+            (adp as any).mergedDescriptor = undefined;
+            await adp.sync();
+            expect(adp.descriptor).toEqual(mockMergedDescriptor);
+        });
+
+        test('update descriptor if descriptor was already fetched, but there are manifest changes', async () => {
+            const adp = new AdpPreview(
+                {
+                    target: {
+                        url: backend
+                    }
+                },
+                mockProject as unknown as ReaderCollection,
+                middlewareUtil,
+                logger
+            );
+
+            mockProject.byGlob.mockResolvedValueOnce([
+                {
+                    getPath: () => '/manifest.appdescr_variant',
+                    getBuffer: () => Buffer.from(descriptorVariant)
+                }
+            ]);
+            await adp.init(JSON.parse(descriptorVariant));
+            (adp as any).mergedDescriptor = undefined;
+            await adp.sync();
+            expect(adp.descriptor).toEqual(mockMergedDescriptor);
+            secondCall = true;
+            global.__SAP_UX_MANIFEST_SYNC_REQUIRED__ = true;
+            await adp.sync();
+            secondCall = false;
+            expect(adp.descriptor).toEqual('testDescriptor');
+        });
     });
     describe('proxy', () => {
         let server: SuperTest<Test>;
         const next = jest.fn().mockImplementation((_req, res) => res.status(200).send());
         beforeAll(async () => {
+            nock(backend)
+                .get((path) => path.startsWith('/sap/bc/lrep/actions/getcsrftoken/'))
+                .reply(200)
+                .persist(true);
+            nock(backend)
+                .put('/sap/bc/lrep/appdescr_variant_preview/?workspacePath=//')
+                .reply(200, {
+                    'my.adaptation': mockMergedDescriptor
+                })
+                .persist(true);
             const adp = new AdpPreview(
                 {
                     target: {
@@ -235,6 +358,10 @@ describe('AdaptationProject', () => {
             server = supertest(app);
         });
 
+        afterAll(() => {
+            nock.cleanAll();
+        });
+
         afterEach(() => {
             global.__SAP_UX_MANIFEST_SYNC_REQUIRED__ = false;
         });
@@ -321,6 +448,16 @@ describe('AdaptationProject', () => {
     describe('addApis', () => {
         let server: SuperTest<Test>;
         beforeAll(async () => {
+            nock(backend)
+                .get((path) => path.startsWith('/sap/bc/lrep/actions/getcsrftoken/'))
+                .reply(200)
+                .persist(true);
+            nock(backend)
+                .put('/sap/bc/lrep/appdescr_variant_preview/?workspacePath=//')
+                .reply(200, {
+                    'my.adaptation': mockMergedDescriptor
+                })
+                .persist(true);
             const adp = new AdpPreview(
                 {
                     target: {

packages/preview-middleware/src/base/config.ts

@@ -206,23 +206,11 @@ 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,
-    descriptor?: MergedAppDescriptor
-) {
+export async function addApp(templateConfig: TemplateConfig, manifest: Partial<Manifest>, app: App, logger: Logger) {
     const id = manifest['sap.app']?.id ?? '';
 
-    app.intent ??= {
-        object: id.replace(/\./g, ''),
-        action: 'preview'
-    };
-
-    const appName = `${app.intent?.object}-${app.intent?.action}`;
+    const appName = getAppName(manifest, app.intent);
     templateConfig.ui5.resources[id] = app.target;
     templateConfig.apps[appName] = {
         title: (await getI18nTextFromProperty(app.local, manifest['sap.app']?.title, logger)) ?? id,
@@ -231,9 +219,24 @@ export async function addApp(
         applicationType: 'URL',
         url: app.target
     };
-    if (descriptor) {
-        templateConfig.apps[appName].applicationDependencies = descriptor;
-    }
+}
+
+/**
+ * Get the application name based on the manifest and app configuration.
+ *
+ * @param manifest - The application manifest.
+ * @param intent - The app configuration.
+ * @returns The application name.
+ */
+export function getAppName(manifest: Partial<Manifest>, intent?: Intent): string {
+    const id = manifest['sap.app']?.id ?? '';
+
+    intent ??= {
+        object: id.replace(/\./g, ''),
+        action: 'preview'
+    };
+
+    return `${intent?.object}-${intent?.action}`;
 }
 
 /**