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

Restructure TS Client V3 #814

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/short-moles-rhyme.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@commercetools/ts-client': minor
---

Restructure middleware logic
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,16 @@ describe('Concurrent processing in sdk v3', () => {
const key2 = category2.body.key

const [response1, response2] = await Promise.all([
apiRoot.categories().withKey({ key }).get().execute(),
apiRoot.categories().withKey({ key: key2 }).get().execute(),
apiRoot
.categories()
.withKey({ key: key as string })
.get()
.execute(),
apiRoot
.categories()
.withKey({ key: key2 as string })
.get()
.execute(),
])

expect(response1.statusCode).toBe(200)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,25 +37,24 @@ describe('Concurrent Modification Middleware', () => {
category = await createCategory()
taxCategory = await ensureTaxCategory()
productType = await ensureProductType(productTypeDraftForProduct)
})

beforeEach(async () => {
//Published product
// Published product
const productDraft = await createProductDraft(
category,
taxCategory,
productType,
true
)

const productCreateResponse = await createProduct(productDraft)
product = productCreateResponse.body
})

afterEach(async () => {
afterAll(async () => {
await fetchAndDeleteProduct(product.id)
})

it('should retry the request with correct version when the first time the version is wrong', async () => {
it('should not retry if the first attempt returned a `200` status code.', async () => {
const ctpClientV3 = new ClientBuilderV3()
.withHttpMiddleware(httpMiddlewareOptionsV3)
.withConcurrentModificationMiddleware()
Expand All @@ -73,7 +72,53 @@ describe('Concurrent Modification Middleware', () => {
.withId({ ID: product.id })
.post({
body: {
version: product.version + 1,
version: product.version,
actions: [
{
action: 'changeName',
name: { en: 'test-name' + new Date().getTime() },
},
],
},
})
.execute()
expect(productUpdateResponse.statusCode).toBe(200)
})

it('should trigger and fix a concurrent error', async () => {
async function fn(version: number, request: MiddlewareRequest, response) {
expect(response.statusCode).toEqual(409)
expect(response.error.name).toEqual('ConcurrentModification')

// update version
request.body = {
...(request.body as object),
version,
}

return JSON.stringify(request.body)
}

const ctpClientV3 = new ClientBuilderV3()
.withHttpMiddleware(httpMiddlewareOptionsV3)
.withConcurrentModificationMiddleware({
concurrentModificationHandlerFn: fn,
})
.withClientCredentialsFlow(authMiddlewareOptionsV3)
.build()

const apiRootV3 = createApiBuilderFromCtpClient(ctpClientV3).withProjectKey(
{
projectKey,
}
)

const productUpdateResponse = await apiRootV3
.products()
.withId({ ID: product.id })
.post({
body: {
version: product.version - 2,
actions: [
{
action: 'changeName',
Expand All @@ -83,15 +128,28 @@ describe('Concurrent Modification Middleware', () => {
},
})
.execute()

expect(productUpdateResponse.statusCode).toBe(200)
})

it(`should retry the request with the custom logic provided`, async () => {
let isHandleCalled = false
const handleConcurrentModification = async (version, request) => {
if (request.uri.includes('/products/')) {
isHandleCalled = true
const concurrentModificationHandlerFn = async (
version: number,
request: MiddlewareRequest,
response
) => {
if ((request.uri as string).includes('/products/')) {
;(request.body as { [key: string]: any }).version = version

expect(request).toEqual(
expect.objectContaining({
method: 'POST',
uriTemplate: '/{projectKey}/products/{ID}',
baseUri: 'https://api.europe-west1.gcp.commercetools.com',
})
)

expect(response.statusCode).toEqual(409)
return JSON.stringify(request.body)
} else {
throw new Error()
Expand All @@ -100,9 +158,7 @@ describe('Concurrent Modification Middleware', () => {

const ctpClientV3 = new ClientBuilderV3()
.withHttpMiddleware(httpMiddlewareOptionsV3)
.withConcurrentModificationMiddleware({
concurrentModificationHandlerFn: handleConcurrentModification,
})
.withConcurrentModificationMiddleware({ concurrentModificationHandlerFn })
.withClientCredentialsFlow(authMiddlewareOptions)
.build()

Expand All @@ -111,12 +167,13 @@ describe('Concurrent Modification Middleware', () => {
projectKey,
}
)
const productUpdateResponse = await apiRootV3

await apiRootV3
.products()
.withId({ ID: product.id })
.post({
body: {
version: product.version + 1,
version: +product.version + 1,
actions: [
{
action: 'changeName',
Expand All @@ -126,8 +183,7 @@ describe('Concurrent Modification Middleware', () => {
},
})
.execute()
expect(productUpdateResponse.statusCode).toBe(200)
expect(isHandleCalled).toBe(true)
.catch((e) => e)
})
})

Expand Down Expand Up @@ -199,13 +255,12 @@ describe('Correlation ID and user agent middlewares', () => {

const response = await apiRootV3.get().execute()
expect(response.headers?.['x-correlation-id'][0]).toEqual(correlationId)
expect(response?.originalRequest.headers['User-Agent']).toContain(
expect((response?.originalRequest as any).headers['User-Agent']).toContain(
'test-app/commercetools-sdk-javascript-v3'
)
})

it('should include request in the logs', async () => {
let responseFromLoggerMiddleware
const httpMiddlewareOptions = {
...httpMiddlewareOptionsV3,
includeOriginalRequest: true,
Expand All @@ -215,7 +270,13 @@ describe('Correlation ID and user agent middlewares', () => {
.withHttpMiddleware(httpMiddlewareOptions)
.withLoggerMiddleware({
loggerFn: (response) => {
responseFromLoggerMiddleware = response
expect(response.statusCode).toEqual(200)
expect(response).toHaveProperty('originalRequest')
expect(response.originalRequest).toBeInstanceOf(Object)
expect(response).toBeDefined()
expect(response.originalRequest?.headers?.['Authorization']).toMatch(
/^Bearer [A-Za-z0-9\-_]+$/
)
},
})
.withClientCredentialsFlow(authMiddlewareOptionsV3)
Expand All @@ -228,12 +289,6 @@ describe('Correlation ID and user agent middlewares', () => {
)

await apiRootV3.get().execute()

expect(responseFromLoggerMiddleware).toBeDefined()
expect(responseFromLoggerMiddleware.originalRequest).toBeInstanceOf(Object)
expect(
responseFromLoggerMiddleware.originalRequest.headers['Authorization']
).toMatch(/^Bearer [A-Za-z0-9\-_]+$/)
})
})

Expand Down
5 changes: 1 addition & 4 deletions packages/sdk-client-v3/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +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"
"node-fetch": "^2.6.1"
},
"files": ["dist", "CHANGELOG.md"],
"author": "Chukwuemeka Ajima <[email protected]>",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,32 +1,29 @@
import { Mutex } from 'async-mutex'
import fetch from 'node-fetch'
import {
AuthMiddlewareOptions,
ClientRequest,
Middleware,
MiddlewareRequest,
MiddlewareResponse,
Next,
Task,
TokenCache,
TokenStore,
} from '../../types/types'
import { buildTokenCacheKey, store } from '../../utils'
import { buildTokenCacheKey, mergeAuthHeader, store } from '../../utils'
import { buildRequestForAnonymousSessionFlow } from './auth-request-builder'
import { executeRequest } from './auth-request-executor'

export default function createAuthMiddlewareForAnonymousSessionFlow(
options: AuthMiddlewareOptions
): Middleware {
const pendingTasks: Array<Task> = []
const requestState = new Mutex()
const tokenCache =
options.tokenCache ||
store<TokenStore, TokenCache>({
token: '',
expirationTime: -1,
})

let tokenCacheObject: TokenStore
let tokenFetchPromise: Promise<boolean> | null = null
const tokenCacheKey = buildTokenCacheKey(options)

return (next: Next) => {
Expand All @@ -40,32 +37,43 @@ export default function createAuthMiddlewareForAnonymousSessionFlow(
return next(request)
}

/**
* If there is a token in the tokenCache, and it's not
* expired, append the token in the `Authorization` header.
*/
tokenCacheObject = tokenCache.get(tokenCacheKey)
if (
tokenCacheObject &&
tokenCacheObject.token &&
Date.now() < tokenCacheObject.expirationTime
) {
return next(mergeAuthHeader(tokenCacheObject.token, request))
}

// prepare request options
const requestOptions = {
request,
requestState,
tokenCache,
pendingTasks,
tokenCacheKey,
httpClient: options.httpClient || fetch,
...buildRequestForAnonymousSessionFlow(options),
userOption: options,
next,
}

// make request to coco
let requestWithAuth: ClientRequest

try {
await requestState.acquire()
requestWithAuth = await executeRequest(requestOptions)
} finally {
requestState.release()
// If a token is already being fetched, wait for it to finish
if (tokenFetchPromise) {
await tokenFetchPromise
} else {
// Otherwise, fetch the token and let others wait for this process to complete
tokenFetchPromise = executeRequest(requestOptions)
await tokenFetchPromise
lojzatran marked this conversation as resolved.
Show resolved Hide resolved
tokenFetchPromise = null
}

if (requestWithAuth) {
return next(requestWithAuth)
}
// Now the token is present in the tokenCache and can be accessed
tokenCacheObject = tokenCache.get(tokenCacheKey)
return next(mergeAuthHeader(tokenCacheObject.token, request))
}
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { Buffer } from 'buffer/'
import {
AuthMiddlewareOptions,
IBuiltRequestParams,
Expand Down Expand Up @@ -27,9 +26,8 @@ export function buildRequestForClientCredentialsFlow(
throw new Error('Missing required credentials (clientId, clientSecret)')

const scope = options.scopes ? options.scopes.join(' ') : undefined
const basicAuth = Buffer.from(`${clientId}:${clientSecret}`).toString(
'base64'
)
const basicAuth = btoa(`${clientId}:${clientSecret}`)
lojzatran marked this conversation as resolved.
Show resolved Hide resolved

// This is mostly useful for internal testing purposes to be able to check
// other oauth endpoints.
const oauthUri = options.oauthUri || '/oauth/token'
Expand Down Expand Up @@ -93,11 +91,10 @@ export function buildRequestForRefreshTokenFlow(
if (!(clientId && clientSecret))
throw new Error('Missing required credentials (clientId, clientSecret)')

const basicAuth = Buffer.from(`${clientId}:${clientSecret}`).toString(
'base64'
)
// This is mostly useful for internal testing purposes to be able to check
// other oauth endpoints.
const basicAuth = btoa(`${clientId}:${clientSecret}`)

// This is mostly useful for internal testing
// purposes to be able to check other oauth endpoints.
const oauthUri = options.oauthUri || '/oauth/token'
const url = options.host.replace(/\/$/, '') + oauthUri
const body = `grant_type=refresh_token&refresh_token=${encodeURIComponent(
Expand Down Expand Up @@ -137,10 +134,7 @@ export function buildRequestForPasswordFlow(

const scope = (options.scopes || []).join(' ')
const scopeStr = scope ? `&scope=${scope}` : ''

const basicAuth = Buffer.from(`${clientId}:${clientSecret}`).toString(
'base64'
)
const basicAuth = btoa(`${clientId}:${clientSecret}`)

/**
* This is mostly useful for internal testing purposes to be able to check
Expand Down
Loading