fix: Remove destination cache when getting destinations from service binding (#5635)

ZhongpinWang · web-flow · commit 51cab2aec1bf · 2025-03-17T11:35:51.000+01:00
* fix: remove destination cache for getting from vcap

* lint and lock

* changeset

* try to trigger workflows

* fix: using jest.restoreAllMocks
diff --git a/.changeset/twenty-ideas-sleep.md b/.changeset/twenty-ideas-sleep.md
@@ -0,0 +1,5 @@
+---
+'@sap-cloud-sdk/connectivity': patch
+---
+
+[Fixed Issue] Remove destination cache in `getDestinationFromServiceBinding()` function to let cached destinations retrieved in `getDestinationFromDestinationService()` function be added with the `proxyConfiguration` property.
diff --git a/packages/connectivity/src/scp-cf/destination/destination-from-vcap.spec.ts b/packages/connectivity/src/scp-cf/destination/destination-from-vcap.spec.ts
@@ -1,41 +1,32 @@
 import {
   mockServiceToken,
-  providerUserPayload,
   providerUserToken,
-  signedJwt,
-  signedJwtForVerification,
-  testTenants
+  signedJwt
 } from '../../../../../test-resources/test/test-util';
-import * as tokenAccessor from '../token-accessor';
-import { decodeJwt } from '../jwt';
+import * as xsuaaService from '../xsuaa-service';
+import { clientCredentialsTokenCache } from '../client-credentials-token-cache';
 import { getDestination } from './destination-accessor';
 import { getDestinationFromServiceBinding } from './destination-from-vcap';
-import { destinationCache } from './destination-cache';
 import SpyInstance = jest.SpyInstance;
 import type { Service } from '../environment-accessor';
 
 describe('vcap-service-destination', () => {
-  const serviceTokenSpy = jest.spyOn(tokenAccessor, 'serviceToken');
-
-  beforeAll(() => {
-    mockServiceToken();
-  });
-
   beforeEach(() => {
     mockServiceBindings();
   });
 
-  afterEach(async () => {
+  afterEach(() => {
     delete process.env.VCAP_SERVICES;
-    jest.clearAllMocks();
-    await destinationCache.clear();
+    clientCredentialsTokenCache.clear();
+    jest.restoreAllMocks();
   });
 
   function getActualClientId(spyInstance: SpyInstance): string {
     return spyInstance.mock.calls[0][0]['credentials']['clientid'];
   }
 
   it('creates a destination for the aicore service', async () => {
+    const serviceTokenSpy = mockServiceToken();
     await expect(
       getDestinationFromServiceBinding({
         destinationName: 'my-aicore',
@@ -52,6 +43,7 @@ describe('vcap-service-destination', () => {
   });
 
   it('creates a destination for the business logging service', async () => {
+    const serviceTokenSpy = mockServiceToken();
     await expect(
       getDestinationFromServiceBinding({
         destinationName: 'my-business-logging',
@@ -68,6 +60,7 @@ describe('vcap-service-destination', () => {
   });
 
   it('creates ad destination for the xsuaa service', async () => {
+    const serviceTokenSpy = mockServiceToken();
     await expect(
       getDestinationFromServiceBinding({
         destinationName: 'my-xsuaa',
@@ -84,6 +77,7 @@ describe('vcap-service-destination', () => {
   });
 
   it('creates a destination for the service manager service', async () => {
+    const serviceTokenSpy = mockServiceToken();
     await expect(
       getDestinationFromServiceBinding({
         destinationName: 'my-service-manager',
@@ -100,6 +94,7 @@ describe('vcap-service-destination', () => {
   });
 
   it('creates a destination for the destination service', async () => {
+    const serviceTokenSpy = mockServiceToken();
     await expect(
       getDestinationFromServiceBinding({
         destinationName: 'my-destination-service',
@@ -115,6 +110,7 @@ describe('vcap-service-destination', () => {
   });
 
   it('creates a destination for the saas registry', async () => {
+    const serviceTokenSpy = mockServiceToken();
     await expect(
       getDestinationFromServiceBinding({
         destinationName: 'my-saas-registry',
@@ -130,6 +126,7 @@ describe('vcap-service-destination', () => {
   });
 
   it('creates a destination for the workflow', async () => {
+    const serviceTokenSpy = mockServiceToken();
     await expect(
       getDestinationFromServiceBinding({
         destinationName: 'my-workflow',
@@ -157,6 +154,16 @@ describe('vcap-service-destination', () => {
   });
 
   it('uses the cache if enabled', async () => {
+    const getClientCredentialsTokenSpy = jest
+      .spyOn(xsuaaService, 'getClientCredentialsToken')
+      .mockResolvedValue({
+        access_token: providerUserToken,
+        token_type: 'bearer',
+        expires_in: 1000,
+        scope: 'some scope',
+        jti: 'some jti'
+      });
+
     await getDestinationFromServiceBinding({
       destinationName: 'my-destination-service',
       useCache: true,
@@ -167,35 +174,7 @@ describe('vcap-service-destination', () => {
       useCache: true,
       jwt: providerUserToken
     });
-    expect(serviceTokenSpy).toBeCalledTimes(1);
-    expect(
-      destinationCache
-        .getCacheInstance()
-        .get(`${providerUserPayload.zid}::my-destination`)
-    ).toBeDefined();
-  });
-
-  it('returns undefined if cached destination JWT has expired', async () => {
-    const jwtPayload = {
-      iat: 1692273899,
-      exp: 1692317098,
-      zid: testTenants.provider,
-      user_id: 'user-prov',
-      ext_attr: { enhancer: 'XSUAA' }
-    };
-    const jwt = signedJwtForVerification(jwtPayload);
-    jest.useFakeTimers();
-    await getDestinationFromServiceBinding({
-      destinationName: 'my-destination-service',
-      useCache: true,
-      jwt
-    });
-    jest.advanceTimersByTime(720 * 60 * 1000 + 1);
-    await expect(
-      destinationCache
-        .getCacheInstance()
-        .get(`${jwtPayload.zid}::my-destination-service`)
-    ).resolves.toBeUndefined();
+    expect(getClientCredentialsTokenSpy).toHaveBeenCalledTimes(1);
   });
 
   it('creates a destination using a custom transformation function', async () => {
@@ -274,7 +253,7 @@ describe('vcap-service-destination', () => {
     expect(serviceBindingTransformFn).toBeCalledTimes(1);
   });
 
-  it('sets forwarded auth token if needed (uncached)', async () => {
+  it('sets forwarded auth token if needed', async () => {
     const jwt = signedJwt({});
 
     const destination = await getDestinationFromServiceBinding({
@@ -288,51 +267,6 @@ describe('vcap-service-destination', () => {
 
     expect(destination?.authTokens?.[0]).toMatchObject({ value: jwt });
   });
-
-  it('sets forwarded auth token if needed (cached)', async () => {
-    const jwt = signedJwt({ zid: 'tenant' });
-
-    destinationCache.cacheRetrievedDestination(
-      decodeJwt(jwt),
-      {
-        name: 'my-custom-service',
-        forwardAuthToken: true
-      },
-      'tenant'
-    );
-
-    const destination = await getDestinationFromServiceBinding({
-      destinationName: 'my-custom-service',
-      useCache: true,
-      jwt
-    });
-
-    expect(destination?.authTokens?.[0]).toMatchObject({ value: jwt });
-  });
-
-  // TODO: fix in #1123
-  it.skip('does not cache the forwarded token', async () => {
-    const jwt = signedJwt({ zid: 'tenant' });
-
-    await getDestinationFromServiceBinding({
-      destinationName: 'my-custom-service',
-      useCache: true,
-      jwt,
-      serviceBindingTransformFn: async ({ name }) => ({
-        forwardAuthToken: true,
-        name
-      })
-    });
-
-    const destination = await destinationCache.retrieveDestinationFromCache(
-      decodeJwt(jwt),
-      'my-custom-service',
-      'tenant'
-    );
-
-    expect(destination).toBeDefined();
-    expect(destination?.authTokens).toBeUndefined();
-  });
 });
 
 function mockServiceBindings() {
diff --git a/packages/connectivity/src/scp-cf/destination/destination-from-vcap.ts b/packages/connectivity/src/scp-cf/destination/destination-from-vcap.ts
@@ -1,12 +1,11 @@
 import { createLogger } from '@sap-cloud-sdk/util';
-import { decodeJwt, decodeOrMakeJwt } from '../jwt';
+import { decodeJwt } from '../jwt';
 import { getServiceBindingByInstanceName } from '../environment-accessor';
 import {
   addProxyConfigurationInternet,
   proxyStrategy
 } from './http-proxy-util';
 import { isHttpDestination } from './destination-service-types';
-import { destinationCache } from './destination-cache';
 import { serviceToDestinationTransformers } from './service-binding-to-destination';
 import { setForwardedAuthTokenIfNeeded } from './forward-auth-token';
 import type { DestinationFetchOptions } from './destination-accessor-types';
@@ -54,27 +53,8 @@ export async function getDestinationFromServiceBinding(
       : undefined;
 
   const retrievalOptions = { ...options, jwt: decodedJwt };
-  let destination;
-  if (options.useCache) {
-    destination = await destinationCache.retrieveDestinationFromCache(
-      decodeOrMakeJwt(retrievalOptions.jwt),
-      retrievalOptions.destinationName,
-      'tenant'
-    );
-  }
+  const destination = await retrieveDestination(retrievalOptions);
 
-  if (!destination) {
-    destination = await retrieveDestinationWithoutCache(retrievalOptions);
-
-    if (options.useCache) {
-      // As the grant type is clientCredential, isolation strategy is 'tenant'.
-      await destinationCache.cacheRetrievedDestination(
-        decodeOrMakeJwt(options.jwt),
-        destination,
-        'tenant'
-      );
-    }
-  }
   const destWithProxy =
     destination &&
     isHttpDestination(destination) &&
@@ -89,7 +69,7 @@ export async function getDestinationFromServiceBinding(
   return destWithProxy;
 }
 
-async function retrieveDestinationWithoutCache({
+async function retrieveDestination({
   useCache,
   jwt,
   destinationName,
@@ -120,19 +100,6 @@ export async function destinationForServiceBinding(
   options: DestinationForServiceBindingOptions &
     PartialDestinationFetchOptions = {}
 ): Promise<Destination> {
-  if (options.useCache) {
-    const destinationFromCache =
-      await destinationCache.retrieveDestinationFromCache(
-        decodeOrMakeJwt(options.jwt),
-        serviceInstanceName,
-        'tenant'
-      );
-
-    if (destinationFromCache) {
-      return destinationFromCache;
-    }
-  }
-
   const optionsForTransformation = {
     useCache: options.useCache,
     jwt: options.jwt
@@ -148,15 +115,6 @@ export async function destinationForServiceBinding(
       ? addProxyConfigurationInternet(destination)
       : destination;
 
-  if (options.useCache) {
-    // As the grant type is clientCredential, isolation strategy is 'tenant'.
-    await destinationCache.cacheRetrievedDestination(
-      decodeOrMakeJwt(options.jwt),
-      destWithProxy,
-      'tenant'
-    );
-  }
-
   return destWithProxy;
 }
 
diff --git a/yarn.lock b/yarn.lock
@@ -1677,12 +1677,12 @@
   dependencies:
     "@sinonjs/commons" "^3.0.0"
 
-"@stylistic/eslint-plugin@^4.1.0":
-  version "4.1.0"
-  resolved "https://registry.npmjs.org/@stylistic/eslint-plugin/-/eslint-plugin-4.1.0.tgz#8882a6fe307cc94022e4495720373e8a5652d60c"
-  integrity sha512-bytbL7qiici7yPyEiId0fGPK9kjQbzcPMj2aftPfzTCyJ/CRSKdtI+iVjM0LSGzGxfunflI+MDDU9vyIIeIpoQ==
+"@stylistic/eslint-plugin@^3.1.0":
+  version "3.1.0"
+  resolved "https://registry.npmjs.org/@stylistic/eslint-plugin/-/eslint-plugin-3.1.0.tgz#a9f655c518f76bfc5feb46b467d0f06e511b289d"
+  integrity sha512-pA6VOrOqk0+S8toJYhQGv2MWpQQR0QpeUo9AhNkC49Y26nxBQ/nH1rta9bUU1rPw2fJ1zZEMV5oCX5AazT7J2g==
   dependencies:
-    "@typescript-eslint/utils" "^8.23.0"
+    "@typescript-eslint/utils" "^8.13.0"
     eslint-visitor-keys "^4.2.0"
     espree "^10.3.0"
     estraverse "^5.3.0"
@@ -2128,7 +2128,7 @@
     semver "^7.6.0"
     ts-api-utils "^2.0.1"
 
-"@typescript-eslint/utils@8.26.0", "@typescript-eslint/utils@^8.13.0", "@typescript-eslint/utils@^8.23.0":
+"@typescript-eslint/utils@8.26.0", "@typescript-eslint/utils@^8.13.0":
   version "8.26.0"
   resolved "https://registry.npmjs.org/@typescript-eslint/utils/-/utils-8.26.0.tgz#845d20ed8378a5594e6445f54e53b972aee7b3e6"
   integrity sha512-2L2tU3FVwhvU14LndnQCA2frYC8JnPDVKyQtWFPf8IYFMt/ykEN1bPolNhNbCVgOmdzTlWdusCTKA/9nKrf8Ig==

-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +---
 +'@sap-cloud-sdk/connectivity': patch
 +---
++
 +[Fixed Issue] Remove destination cache in `getDestinationFromServiceBinding()` function to let cached destinations retrieved in `getDestinationFromDestinationService()` function be added with the `proxyConfiguration` property.