refactor(cf-deploy-config-writer): scope BTP destinations cache to call site (#4458) (#4461)

longieirl · github-actions[bot] · web-flow · commit 6231731af0c3 · 2026-04-09T10:50:24.000+01:00
* refactor(cf-deploy-config-writer): scope BTP destinations cache to call site

Replace module-level cachedDestinationsList with an optional cache parameter
on getBTPDestinations() and getDestinationProperties(). Each caller controls
cache lifetime — per-run caching is opt-in via a shared {} object, while the
default empty {} gives a fresh fetch every call. Eliminates cross-test
contamination and stale-cache issues within long-running processes.

* Linting auto fix commit

---------

Co-authored-by: github-actions[bot] &lt;github-actions[bot]@users.noreply.github.com&gt;
diff --git a/.changeset/cf-deploy-config-btp-cache-scope.md b/.changeset/cf-deploy-config-btp-cache-scope.md
@@ -0,0 +1,11 @@
+---
+'@sap-ux/cf-deploy-config-writer': patch
+---
+
+refactor: scope BTP destinations cache to call site instead of module level
+
+`getBTPDestinations` and `getDestinationProperties` now accept an optional
+`cache` parameter (`{ list?: Destinations }`). Each generator run passes its own
+cache object for deduplication within a single invocation; independent calls use
+independent caches by default. This removes the module-level mutable variable
+that caused cross-test contamination and prevented stale-cache detection.
diff --git a/packages/cf-deploy-config-writer/src/utils.ts b/packages/cf-deploy-config-writer/src/utils.ts
@@ -35,8 +35,6 @@ import {
 } from './constants';
 import { type MTABaseConfig, type CFBaseConfig, type CFAppConfig } from './types';
 
-let cachedDestinationsList: Destinations = {};
-
 /**
  * Read manifest file for processing.
  *
@@ -91,17 +89,20 @@ export function toPosixPath(dirPath: string): string {
  * Retrieves destination configuration from SAP BTP when running in Business Application Studio.
  *
  * @param destination The destination name to look up in BTP destination service
+ * @param cache Mutable object that holds the cached destinations list; pass a new `{}` per generator run
+ * @param cache.list
  * @returns Object containing destination URL format flag and authentication type
  * @returns {boolean} destinationIsFullUrl - True if destination uses full URL format
  * @returns {Authentication | undefined} destinationAuthentication - Authentication type configured for the destination
  */
 export async function getDestinationProperties(
-    destination: string | undefined
+    destination: string | undefined,
+    cache: { list?: Destinations } = {}
 ): Promise<{ destinationIsFullUrl: boolean; destinationAuthentication: Authentication | undefined }> {
     let destinationIsFullUrl = false;
     let destinationAuthentication;
     if (isAppStudio() && destination) {
-        const destinations = await getBTPDestinations();
+        const destinations = await getBTPDestinations(cache);
         if (destinations[destination]) {
             destinationIsFullUrl = isFullUrlDestination(destinations[destination]);
             destinationAuthentication = destinations[destination].Authentication as Authentication;
@@ -112,14 +113,15 @@ export async function getDestinationProperties(
 
 /**
  * Retrieve the list of destinations from SAP BTP.
+ * Caching is scoped to the provided cache object; pass a new `{}` per generator run.
  *
+ * @param cache Mutable object that holds the cached list across multiple calls within one run
+ * @param cache.list
  * @returns Destinations list
  */
-export async function getBTPDestinations(): Promise<Destinations> {
-    if (Object.keys(cachedDestinationsList).length === 0) {
-        cachedDestinationsList = await listDestinations({ stripS4HCApiHosts: true });
-    }
-    return cachedDestinationsList;
+export async function getBTPDestinations(cache: { list?: Destinations } = {}): Promise<Destinations> {
+    cache.list ??= await listDestinations({ stripS4HCApiHosts: true });
+    return cache.list;
 }
 
 /**
diff --git a/packages/cf-deploy-config-writer/test/unit/utils.test.ts b/packages/cf-deploy-config-writer/test/unit/utils.test.ts
@@ -1,6 +1,19 @@
-import { validateVersion, toMtaModuleName, runCommand } from '../../src/utils';
+import {
+    validateVersion,
+    toMtaModuleName,
+    runCommand,
+    getBTPDestinations,
+    getDestinationProperties
+} from '../../src/utils';
 import { MTAVersion } from '../../src/constants';
 import { CommandRunner } from '@sap-ux/nodejs-utils';
+import * as btp from '@sap-ux/btp-utils';
+
+jest.mock('@sap-ux/btp-utils', () => ({
+    ...jest.requireActual('@sap-ux/btp-utils'),
+    isAppStudio: jest.fn(),
+    listDestinations: jest.fn()
+}));
 
 describe('CF utils', () => {
     beforeAll(async () => {
@@ -52,4 +65,152 @@ describe('CF utils', () => {
             jest.restoreAllMocks();
         });
     });
+
+    describe('getBTPDestinations', () => {
+        let listDestinationsMock: jest.SpyInstance;
+
+        beforeEach(() => {
+            listDestinationsMock = jest.spyOn(btp, 'listDestinations');
+            listDestinationsMock.mockReset();
+        });
+
+        afterEach(() => {
+            jest.restoreAllMocks();
+        });
+
+        test('fetches destinations and populates cache', async () => {
+            const mockDestinations = { dest1: { Name: 'dest1' } } as any;
+            listDestinationsMock.mockResolvedValue(mockDestinations);
+
+            const cache: { list?: any } = {};
+            const result = await getBTPDestinations(cache);
+
+            expect(result).toBe(mockDestinations);
+            expect(listDestinationsMock).toHaveBeenCalledTimes(1);
+            expect(cache.list).toBe(mockDestinations);
+        });
+
+        test('returns cached result without calling listDestinations again', async () => {
+            const mockDestinations = { dest1: { Name: 'dest1' } } as any;
+            listDestinationsMock.mockResolvedValue(mockDestinations);
+
+            const cache: { list?: any } = {};
+            await getBTPDestinations(cache);
+            const result = await getBTPDestinations(cache);
+
+            expect(result).toBe(mockDestinations);
+            expect(listDestinationsMock).toHaveBeenCalledTimes(1);
+        });
+
+        test('each independent cache object triggers a fresh fetch', async () => {
+            const first = { dest1: { Name: 'dest1' } } as any;
+            const second = { dest2: { Name: 'dest2' } } as any;
+            listDestinationsMock.mockResolvedValueOnce(first).mockResolvedValueOnce(second);
+
+            const result1 = await getBTPDestinations({});
+            const result2 = await getBTPDestinations({});
+
+            expect(result1).toBe(first);
+            expect(result2).toBe(second);
+            expect(listDestinationsMock).toHaveBeenCalledTimes(2);
+        });
+
+        test('uses empty cache by default (no module-level state)', async () => {
+            const mockDestinations = {} as any;
+            listDestinationsMock.mockResolvedValue(mockDestinations);
+
+            await getBTPDestinations();
+            await getBTPDestinations();
+
+            // Without a shared cache object, each call with default `{}` fetches fresh
+            expect(listDestinationsMock).toHaveBeenCalledTimes(2);
+        });
+    });
+
+    describe('getDestinationProperties', () => {
+        let isAppStudioMock: jest.SpyInstance;
+        let listDestinationsMock: jest.SpyInstance;
+
+        const mockDestinations = {
+            fullUrlDest: {
+                Name: 'fullUrlDest',
+                Authentication: 'OAuth2SAMLBearerAssertion',
+                WebIDEAdditionalData: btp.WebIDEAdditionalData.FULL_URL,
+                WebIDEUsage: btp.WebIDEUsage.ODATA_GENERIC
+            },
+            normalDest: {
+                Name: 'normalDest',
+                Authentication: 'NoAuthentication',
+                WebIDEUsage: btp.WebIDEUsage.ODATA_GENERIC
+            }
+        } as any;
+
+        beforeEach(() => {
+            isAppStudioMock = jest.spyOn(btp, 'isAppStudio');
+            listDestinationsMock = jest.spyOn(btp, 'listDestinations').mockResolvedValue(mockDestinations);
+            listDestinationsMock.mockReset();
+            listDestinationsMock.mockResolvedValue(mockDestinations);
+        });
+
+        afterEach(() => {
+            jest.restoreAllMocks();
+        });
+
+        test('returns defaults when not in AppStudio', async () => {
+            isAppStudioMock.mockReturnValue(false);
+
+            const result = await getDestinationProperties('fullUrlDest');
+
+            expect(result.destinationIsFullUrl).toBe(false);
+            expect(result.destinationAuthentication).toBeUndefined();
+            expect(listDestinationsMock).not.toHaveBeenCalled();
+        });
+
+        test('returns defaults when destination is undefined', async () => {
+            isAppStudioMock.mockReturnValue(true);
+
+            const result = await getDestinationProperties(undefined);
+
+            expect(result.destinationIsFullUrl).toBe(false);
+            expect(result.destinationAuthentication).toBeUndefined();
+            expect(listDestinationsMock).not.toHaveBeenCalled();
+        });
+
+        test('resolves full-URL destination in AppStudio', async () => {
+            isAppStudioMock.mockReturnValue(true);
+
+            const result = await getDestinationProperties('fullUrlDest');
+
+            expect(result.destinationIsFullUrl).toBe(true);
+            expect(result.destinationAuthentication).toBe('OAuth2SAMLBearerAssertion');
+        });
+
+        test('resolves non-full-URL destination in AppStudio', async () => {
+            isAppStudioMock.mockReturnValue(true);
+
+            const result = await getDestinationProperties('normalDest');
+
+            expect(result.destinationIsFullUrl).toBe(false);
+            expect(result.destinationAuthentication).toBe('NoAuthentication');
+        });
+
+        test('returns defaults when destination name not found in list', async () => {
+            isAppStudioMock.mockReturnValue(true);
+
+            const result = await getDestinationProperties('unknownDest');
+
+            expect(result.destinationIsFullUrl).toBe(false);
+            expect(result.destinationAuthentication).toBeUndefined();
+        });
+
+        test('shared cache object avoids duplicate listDestinations calls', async () => {
+            isAppStudioMock.mockReturnValue(true);
+
+            const cache: { list?: any } = {};
+            await getDestinationProperties('fullUrlDest', cache);
+            await getDestinationProperties('normalDest', cache);
+
+            expect(listDestinationsMock).toHaveBeenCalledTimes(1);
+        });
+    });
 });
