SUPPORT-27819 fix concurrent lock acquiring when concurrently fetchin…

…g token (#777) * SUPPORT-27819 fix concurrent lock acquiring when concurrently fetching token * chore(tokenCache): re-introduce token cache - re-introduce store as a default value for tokenCache to fix tokenCache.get error. * chore(auth-request-executor): skip test - skip test as implementation was replaced * chore(SUPPORT-27819): remove unnecessary comments - remove unnecessary and unused code blocks * chore(release-changeset): add release changeset - add release changeset for @commercetools/ts-client --------- Co-authored-by: Chukwuemeka Ajima <[email protected]> Co-authored-by: Ajima Chukwuemeka <[email protected]>
commercetools · Aug 9, 2024 · 38c7035 · 38c7035
1 parent e76b5cf
commit 38c7035
Show file tree

Hide file tree

Showing 10 changed files with 84 additions and 57 deletions.
diff --git a/.changeset/seven-eels-glow.md b/.changeset/seven-eels-glow.md
@@ -0,0 +1,5 @@
+---
+'@commercetools/ts-client': patch
+---
+
+Bug fixes for @commercetools/ts-client
diff --git a/packages/platform-sdk/test/integration-tests/category/category-fixture.ts b/packages/platform-sdk/test/integration-tests/category/category-fixture.ts
@@ -2,13 +2,12 @@ import { apiRoot } from '../test-utils'
 import { CategoryDraft } from '../../../src'
 import { randomUUID } from 'crypto'
 
-const categoryDraft: CategoryDraft = {
-  key: 'test-key-category-' + randomUUID(),
-  name: { en: 'test-name-category' + randomUUID() },
-  slug: { en: 'test-slug-category' + randomUUID() },
-}
-
 export const createCategory = async () => {
+  const categoryDraft: CategoryDraft = {
+    key: 'test-key-category-' + randomUUID(),
+    name: { en: 'test-name-category' + randomUUID() },
+    slug: { en: 'test-slug-category' + randomUUID() },
+  }
   return await apiRoot.categories().post({ body: categoryDraft }).execute()
 }
 

diff --git a/packages/platform-sdk/test/integration-tests/sdk-v3/concurrent-processing.test.ts b/packages/platform-sdk/test/integration-tests/sdk-v3/concurrent-processing.test.ts
@@ -0,0 +1,41 @@
+import { createCategory } from '../category/category-fixture'
+import {
+  authMiddlewareOptionsV3,
+  createTokenCache,
+  httpMiddlewareOptionsV3,
+  projectKey,
+} from '../test-utils'
+import { ClientBuilder as ClientBuilderV3 } from '@commercetools/ts-client/src'
+import { createApiBuilderFromCtpClient } from '../../../src'
+
+describe('Concurrent processing in sdk v3', () => {
+  it(`should fetch 2 categories concurrently`, async () => {
+    const category1 = await createCategory()
+    const category2 = await createCategory()
+
+    const tokenCache = createTokenCache()
+
+    authMiddlewareOptionsV3.tokenCache = tokenCache
+
+    const client = new ClientBuilderV3()
+      .withClientCredentialsFlow(authMiddlewareOptionsV3)
+      .withHttpMiddleware(httpMiddlewareOptionsV3)
+      .withConcurrentModificationMiddleware()
+      .build()
+
+    const apiRoot = createApiBuilderFromCtpClient(client).withProjectKey({
+      projectKey,
+    })
+
+    const key = category1.body.key
+    const key2 = category2.body.key
+
+    const [response1, response2] = await Promise.all([
+      apiRoot.categories().withKey({ key }).get().execute(),
+      apiRoot.categories().withKey({ key: key2 }).get().execute(),
+    ])
+
+    expect(response1.statusCode).toBe(200)
+    expect(response2.statusCode).toBe(200)
+  })
+})
diff --git a/packages/platform-sdk/test/integration-tests/test-utils.ts b/packages/platform-sdk/test/integration-tests/test-utils.ts
@@ -68,6 +68,13 @@ export const authMiddlewareOptionsV3 = {
   httpClient: fetch,
 }
 
+export function createTokenCache() {
+  return _tokenCache<TokenStore, TokenCache>({
+    token: '',
+    expirationTime: -1,
+  })
+}
+
 const ctpClient = new ClientBuilder()
   .withProjectKey(projectKey)
   .withClientCredentialsFlow(authMiddlewareOptions)

diff --git a/packages/sdk-client-v3/package.json b/packages/sdk-client-v3/package.json
@@ -34,6 +34,7 @@
   },
   "dependencies": {
     "abort-controller": "3.0.0",
+    "async-mutex": "^0.5.0",
     "buffer": "^6.0.3",
     "node-fetch": "^2.6.1",
     "uuid": "10.0.0"

diff --git a/packages/sdk-client-v3/src/middleware/auth-middleware/auth-request-executor.ts b/packages/sdk-client-v3/src/middleware/auth-middleware/auth-request-executor.ts
@@ -12,15 +12,8 @@ import { buildRequestForRefreshTokenFlow } from './auth-request-builder'
 export async function executeRequest(
   options: executeRequestOptions
 ): Promise<ClientRequest> {
-  const {
-    request,
-    httpClient,
-    tokenCache,
-    tokenCacheKey,
-    requestState,
-    userOption,
-    next,
-  } = options
+  const { request, httpClient, tokenCache, tokenCacheKey, userOption, next } =
+    options
 
   let url = options.url
   let body = options.body
@@ -59,12 +52,6 @@ export async function executeRequest(
    */
   pendingTasks.push({ request, next })
 
-  // if a token is currently being fetched, then wait
-  if (requestState.get()) return
-
-  // signal that a token is being fetched
-  requestState.set(true)
-
   /**
    * use refreshToken flow if there is refresh-token
    * and there's either no token or the token is expired
@@ -76,7 +63,6 @@ export async function executeRequest(
       (tokenCacheObject.token && Date.now() > tokenCacheObject.expirationTime))
   ) {
     if (!userOption) {
-      requestState.set(false)
       throw new Error('Missing required options.')
     }
 
@@ -121,9 +107,6 @@ export async function executeRequest(
       // cache new generated token, refreshToken and expiration time
       tokenCache.set({ token, expirationTime, refreshToken })
 
-      // signal that a token fetch is complete
-      requestState.set(false)
-
       /**
        * Freeze and copy pending queue, reset
        * original one for accepting new pending tasks
@@ -156,10 +139,8 @@ export async function executeRequest(
     )
     /**
      * reject the error immediately
-     * and free up the middleware chain and
-     * release the requestState by setting it to false
+     * and free up the middleware chain
      */
-    requestState.set(false)
     request.reject({
       ...request,
       headers: { ...request.headers },
@@ -169,10 +150,6 @@ export async function executeRequest(
       },
     })
   } catch (error) {
-    /**
-     * on error release the state by setting it to false
-     */
-    requestState.set(false)
     return {
       ...request,
       headers: { ...request.headers },

diff --git a/packages/sdk-client-v3/src/middleware/auth-middleware/client-credentials-flow.ts b/packages/sdk-client-v3/src/middleware/auth-middleware/client-credentials-flow.ts
@@ -1,4 +1,5 @@
 import fetch from 'node-fetch'
+import { Mutex } from 'async-mutex'
 import {
   AuthMiddlewareOptions,
   Middleware,
@@ -18,7 +19,7 @@ import { executeRequest } from './auth-request-executor'
 export default function createAuthMiddlewareForClientCredentialsFlow(
   options: AuthMiddlewareOptions
 ): Middleware {
-  const requestState = store<RequestState, RequestStateStore>(false)
+  const requestState = new Mutex()
   const pendingTasks: Array<Task> = []
   const tokenCache =
     options.tokenCache ||
@@ -52,7 +53,13 @@ export default function createAuthMiddlewareForClientCredentialsFlow(
       }
 
       // make request to coco
-      const requestWithAuth = await executeRequest(requestOptions)
+      let requestWithAuth
+      try {
+        await requestState.acquire()
+        requestWithAuth = await executeRequest(requestOptions)
+      } finally {
+        requestState.release()
+      }
       if (requestWithAuth) {
         // make the request and inject the token into the header
         return next(requestWithAuth)

diff --git a/packages/sdk-client-v3/src/types/types.d.ts b/packages/sdk-client-v3/src/types/types.d.ts
@@ -142,10 +142,7 @@ export type RefreshAuthMiddlewareOptions = {
   httpClient?: Function
 }
 
-export type RequestStateStore = {
-  get: () => RequestState
-  set: (requestState: RequestState) => void
-}
+export type RequestStateStore = any
 
 /* Request */
 type requestBaseOptions = {

diff --git a/packages/sdk-client-v3/tests/auth/auth-request-executor.test.ts b/packages/sdk-client-v3/tests/auth/auth-request-executor.test.ts
@@ -56,25 +56,6 @@ describe('Auth request executor', () => {
       )
     })
 
-    test('should return if a token is already being fetched', async () => {
-      const options = createTestExecutorOptions({
-        requestState: { get: jest.fn(() => true), set: jest.fn() },
-        tokenCache: {
-          set: jest.fn(),
-          get: jest.fn(() => ({
-            token: 'test-cached-token',
-            expirationTime: Date.now() - 999,
-          })),
-        },
-        httpClient: jest.fn(() => ({
-          statusCode: 200,
-          data: { statusCode: 200, access_token: 'test-access-token' },
-        })),
-      })
-
-      expect(await executeRequest(options)).toEqual(undefined)
-    })
-
     test('should throw if userOptions are not provided for token refresh', () => {
       const options = createTestExecutorOptions({
         userOption: null,

diff --git a/yarn.lock b/yarn.lock
@@ -3922,6 +3922,13 @@ assert@^2.1.0:
     object.assign "^4.1.4"
     util "^0.12.5"
 
+async-mutex@^0.5.0:
+  version "0.5.0"
+  resolved "https://registry.yarnpkg.com/async-mutex/-/async-mutex-0.5.0.tgz#353c69a0b9e75250971a64ac203b0ebfddd75482"
+  integrity sha512-1A94B18jkJ3DYq284ohPxoXbfTA5HsQ7/Mf4DEhcyLx3Bz27Rh59iScbB6EPiP+B+joue6YCxcMXSbFC1tZKwA==
+  dependencies:
+    tslib "^2.4.0"
+
 at-least-node@^1.0.0:
   version "1.0.0"
   resolved "https://registry.npmjs.org/at-least-node/-/at-least-node-1.0.0.tgz"
@@ -8766,6 +8773,11 @@ tsconfig@^7.0.0:
     strip-bom "^3.0.0"
     strip-json-comments "^2.0.0"
 
+tslib@^2.4.0:
+  version "2.6.3"
+  resolved "https://registry.yarnpkg.com/tslib/-/tslib-2.6.3.tgz#0438f810ad7a9edcde7a241c3d80db693c8cbfe0"
+  integrity sha512-xNvxJEOUiWPGhUuUdQgAJPKOOJfGnIyKySOc09XkKsgdUV/3E2zvwZYdejjmRgPCgcym1juLH3226yA7sEFJKQ==
+
 tty-browserify@^0.0.1:
   version "0.0.1"
   resolved "https://registry.npmjs.org/tty-browserify/-/tty-browserify-0.0.1.tgz"