refactor(drupal-next): do not pass withAuth to init param of fetcher

The `withAuth` option belongs to the DrupalClient.fetch() method but was mistakenly being passed to the init param of the global fetch and custom fetcher functions.
chapter-three · Feb 27, 2024 · 7e0637f · 7e0637f
1 parent 5581933
commit 7e0637f
Show file tree

Hide file tree

Showing 4 changed files with 93 additions and 54 deletions.
diff --git a/packages/next-drupal/src/client.ts b/packages/next-drupal/src/client.ts
@@ -233,7 +233,10 @@ export class DrupalClient {
  this.tokenExpiresOn = Date.now() + token.expires_in * 1000
  }
 
- async fetch(input: RequestInfo, init?: FetchOptions): Promise<Response> {
+ async fetch(
+ input: RequestInfo,
+ { withAuth, ...init }: FetchOptions = {}
+ ): Promise<Response> {
  init = {
  ...init,
  credentials: "include",
@@ -245,10 +248,10 @@ export class DrupalClient {
 
  // Using the auth set on the client.
  // TODO: Abstract this to a re-usable.
- if (init?.withAuth) {
+ if (withAuth) {
  this.debug(`Using authenticated request.`)
 
- if (init.withAuth === true) {
+ if (withAuth === true) {
  if (typeof this._auth === "undefined") {
  throw new Error(
  "auth is not configured. See https://next-drupal.org/docs/client/auth"
@@ -289,32 +292,32 @@ export class DrupalClient {
  `${this._auth.token_type} ${this._auth.access_token}`
  }
  }
- } else if (typeof init.withAuth === "string") {
+ } else if (typeof withAuth === "string") {
  this.debug(`Using custom authorization header.`)
 
- init["headers"]["Authorization"] = init.withAuth
- } /* c8 ignore next 4 */ else if (typeof init.withAuth === "function") {
+ init["headers"]["Authorization"] = withAuth
+ } /* c8 ignore next 4 */ else if (typeof withAuth === "function") {
  this.debug(`Using custom authorization callback.`)
 
- init["headers"]["Authorization"] = init.withAuth()
- } else if (isBasicAuth(init.withAuth)) {
+ init["headers"]["Authorization"] = withAuth()
+ } else if (isBasicAuth(withAuth)) {
  this.debug(`Using basic authorization header.`)
 
  const basic = Buffer.from(
- `${init.withAuth.username}:${init.withAuth.password}`
+ `${withAuth.username}:${withAuth.password}`
  ).toString("base64")
 
  init["headers"]["Authorization"] = `Basic ${basic}`
- } else if (isClientIdSecretAuth(init.withAuth)) {
+ } else if (isClientIdSecretAuth(withAuth)) {
  // Fetch an access token and add it to the request.
  // Access token can be fetched from cache or using a custom auth method.
- const token = await this.getAccessToken(init.withAuth)
+ const token = await this.getAccessToken(withAuth)
  if (token) {
  init["headers"]["Authorization"] = `Bearer ${token.access_token}`
  }
- } /* c8 ignore next 4 */ else if (isAccessTokenAuth(init.withAuth)) {
+ } /* c8 ignore next 4 */ else if (isAccessTokenAuth(withAuth)) {
  init["headers"]["Authorization"] =
- `${init.withAuth.token_type} ${init.withAuth.access_token}`
+ `${withAuth.token_type} ${withAuth.access_token}`
  }
  }
 

diff --git a/packages/next-drupal/tests/DrupalClient/fetch-related-methods.test.ts b/packages/next-drupal/tests/DrupalClient/fetch-related-methods.test.ts
@@ -105,7 +105,6 @@ describe("fetch()", () => {
  Accept: "application/vnd.api+json",
  Authorization: "Basic YWRtaW46cGFzc3dvcmQ=",
  },
- withAuth: true,
  })
  )
  })
@@ -133,7 +132,6 @@ describe("fetch()", () => {
  Accept: "application/vnd.api+json",
  Authorization: "Basic YXJzaGFkQG5leHQtZHJ1cGFsLm9yZzphYmMxMjM=",
  },
- withAuth: true,
  })
  )
  })
@@ -227,7 +225,6 @@ describe("fetch()", () => {
  foo: "bar",
  Authorization: "Basic YXJzaGFkQG5leHQtZHJ1cGFsLm9yZzphYmMxMjM=",
  },
- withAuth: true,
  })
  )
  })
@@ -464,7 +461,9 @@ describe("getMenu()", () => {
  expect(fetchSpy).toHaveBeenCalledWith(
  expect.anything(),
  expect.objectContaining({
- withAuth: true,
+ headers: expect.objectContaining({
+ Authorization: "Bearer sample-token",
+ }),
  })
  )
  })
@@ -653,7 +652,9 @@ describe("getResource()", () => {
  expect(fetchSpy).toHaveBeenCalledWith(
  expect.anything(),
  expect.objectContaining({
- withAuth: true,
+ headers: expect.objectContaining({
+ Authorization: "Bearer sample-token",
+ }),
  })
  )
  })
@@ -803,7 +804,9 @@ describe("getResourceByPath()", () => {
  expect(fetchSpy).toHaveBeenCalledWith(
  expect.anything(),
  expect.not.objectContaining({
- withAuth: true,
+ headers: expect.objectContaining({
+ Authorization: expect.anything(),
+ }),
  })
  )
  expect(getAccessTokenSpy).not.toHaveBeenCalled()
@@ -814,7 +817,9 @@ describe("getResourceByPath()", () => {
  auth: mocks.auth.clientIdSecret,
  })
  const fetchSpy = spyOnFetch()
- const getAccessTokenSpy = jest.spyOn(client, "getAccessToken")
+ const getAccessTokenSpy = jest
+ .spyOn(client, "getAccessToken")
+ .mockImplementation(async () => mocks.auth.accessToken)
 
  await client.getResourceByPath<DrupalNode>(
  "/recipes/deep-mediterranean-quiche",
@@ -826,7 +831,9 @@ describe("getResourceByPath()", () => {
  expect(fetchSpy).toHaveBeenCalledWith(
  expect.anything(),
  expect.objectContaining({
- withAuth: true,
+ headers: expect.objectContaining({
+ Authorization: `${mocks.auth.accessToken.token_type} ${mocks.auth.accessToken.access_token}`,
+ }),
  })
  )
  expect(getAccessTokenSpy).toHaveBeenCalled()
@@ -930,7 +937,9 @@ describe("getResourceCollection()", () => {
  expect(fetchSpy).toHaveBeenCalledWith(
  expect.anything(),
  expect.objectContaining({
- withAuth: true,
+ headers: expect.objectContaining({
+ Authorization: "Bearer sample-token",
+ }),
  })
  )
  })
@@ -1029,7 +1038,9 @@ describe("getSearchIndex()", () => {
  expect(fetchSpy).toHaveBeenCalledWith(
  expect.anything(),
  expect.objectContaining({
- withAuth: true,
+ headers: expect.objectContaining({
+ Authorization: "Bearer sample-token",
+ }),
  })
  )
  })
@@ -1114,7 +1125,9 @@ describe("getView()", () => {
  expect(fetchSpy).toHaveBeenCalledWith(
  expect.anything(),
  expect.objectContaining({
- withAuth: true,
+ headers: expect.objectContaining({
+ Authorization: "Bearer sample-token",
+ }),
  })
  )
  })
@@ -1179,7 +1192,9 @@ describe("translatePath()", () => {
  expect(fetchSpy).toHaveBeenCalledWith(
  expect.anything(),
  expect.objectContaining({
- withAuth: true,
+ headers: expect.objectContaining({
+ Authorization: "Bearer sample-token",
+ }),
  })
  )
  })

diff --git a/packages/next-drupal/tests/DrupalClient/pages-router-methods.test.ts b/packages/next-drupal/tests/DrupalClient/pages-router-methods.test.ts
@@ -232,13 +232,16 @@ describe("buildStaticPathsParamsFromPaths()", () => {
 
 describe("getAuthFromContextAndOptions()", () => {
  const clientIdSecret = mocks.auth.clientIdSecret
+ const accessToken = mocks.auth.accessToken
 
  test("should use the withAuth option if provided and NOT in preview", async () => {
  const client = new DrupalClient(BASE_URL, {
  auth: clientIdSecret,
  })
  const fetchSpy = spyOnFetch()
- jest.spyOn(client, "getAccessToken")
+ jest
+ .spyOn(client, "getAccessToken")
+ .mockImplementation(async () => accessToken)
 
  await client.getResourceFromContext(
  "node--article",
@@ -253,7 +256,9 @@ describe("getAuthFromContextAndOptions()", () => {
  expect(fetchSpy).toHaveBeenCalledWith(
  expect.anything(),
  expect.objectContaining({
- withAuth: true,
+ headers: expect.objectContaining({
+ Authorization: `${accessToken.token_type} ${accessToken.access_token}`,
+ }),
  })
  )
 
@@ -274,11 +279,9 @@ describe("getAuthFromContextAndOptions()", () => {
  expect(fetchSpy).toHaveBeenLastCalledWith(
  expect.anything(),
  expect.objectContaining({
- withAuth: {
- clientId: "foo",
- clientSecret: "bar",
- scope: "baz",
- },
+ headers: expect.objectContaining({
+ Authorization: `${accessToken.token_type} ${accessToken.access_token}`,
+ }),
  })
  )
  })
@@ -296,19 +299,19 @@ describe("getAuthFromContextAndOptions()", () => {
  expect(fetchSpy).toHaveBeenCalledWith(
  expect.anything(),
  expect.objectContaining({
- withAuth: false,
+ headers: expect.not.objectContaining({
+ Authorization: expect.anything(),
+ }),
  })
  )
 
  const client2 = new DrupalClient(BASE_URL, {
  auth: clientIdSecret,
  withAuth: true,
  })
- jest.spyOn(client2, "getAccessToken").mockImplementation(async () => ({
- token_type: "",
- expires_in: 0,
- access_token: "",
- }))
+ jest
+ .spyOn(client2, "getAccessToken")
+ .mockImplementation(async () => accessToken)
 
  await client2.getResourceFromContext("node--article", {
  preview: false,
@@ -317,7 +320,9 @@ describe("getAuthFromContextAndOptions()", () => {
  expect(fetchSpy).toHaveBeenLastCalledWith(
  expect.anything(),
  expect.objectContaining({
- withAuth: true,
+ headers: expect.objectContaining({
+ Authorization: `${accessToken.token_type} ${accessToken.access_token}`,
+ }),
  })
  )
  })
@@ -327,7 +332,8 @@ describe("getAuthFromContextAndOptions()", () => {
  auth: clientIdSecret,
  withAuth: true,
  })
- const fetchSpy = spyOnFetch()
+ const fetchSpy = jest.spyOn(client, "fetch")
+ spyOnFetch()
 
  await client.getResourceFromContext("node--article", {
  preview: true,
@@ -345,7 +351,8 @@ describe("getAuthFromContextAndOptions()", () => {
  const client = new DrupalClient(BASE_URL, {
  auth: clientIdSecret,
  })
- const fetchSpy = spyOnFetch()
+ const fetchSpy = jest.spyOn(client, "fetch")
+ spyOnFetch()
 
  await client.getResourceFromContext("node--article", {
  preview: true,
@@ -375,7 +382,8 @@ describe("getAuthFromContextAndOptions()", () => {
  },
  withAuth: true,
  })
- const fetchSpy = spyOnFetch()
+ const fetchSpy = jest.spyOn(client, "fetch")
+ spyOnFetch()
 
  await client.getResourceFromContext("node--article", {
  preview: true,
@@ -414,7 +422,9 @@ describe("getAuthFromContextAndOptions()", () => {
  expect(fetchSpy).toHaveBeenCalledWith(
  expect.anything(),
  expect.objectContaining({
- withAuth: `Bearer example-token`,
+ headers: expect.objectContaining({
+ Authorization: `Bearer example-token`,
+ }),
  })
  )
  })
@@ -440,7 +450,9 @@ describe("getAuthFromContextAndOptions()", () => {
  expect(fetchSpy).toHaveBeenCalledWith(
  expect.anything(),
  expect.objectContaining({
- withAuth: `Bearer example-token`,
+ headers: expect.objectContaining({
+ Authorization: `Bearer example-token`,
+ }),
  })
  )
  })
@@ -775,7 +787,9 @@ describe("getResourceCollectionFromContext()", () => {
  expect(fetchSpy).toHaveBeenCalledWith(
  expect.anything(),
  expect.objectContaining({
- withAuth: true,
+ headers: expect.objectContaining({
+ Authorization: "Bearer sample-token",
+ }),
  })
  )
  })
@@ -981,7 +995,9 @@ describe("getResourceFromContext()", () => {
  expect(fetchSpy).toHaveBeenCalledWith(
  expect.anything(),
  expect.objectContaining({
- withAuth: true,
+ headers: expect.objectContaining({
+ Authorization: "Bearer sample-token",
+ }),
  })
  )
  })
@@ -1010,7 +1026,9 @@ describe("getResourceFromContext()", () => {
  expect(fetchSpy).toHaveBeenCalledWith(
  expect.anything(),
  expect.objectContaining({
- withAuth: `Bearer sample-token`,
+ headers: expect.objectContaining({
+ Authorization: `Bearer sample-token`,
+ }),
  })
  )
  })
@@ -1148,7 +1166,9 @@ describe("getStaticPathsFromContext()", () => {
  expect(fetchSpy).toHaveBeenCalledWith(
  expect.anything(),
  expect.objectContaining({
- withAuth: true,
+ headers: expect.objectContaining({
+ Authorization: "Bearer sample-token",
+ }),
  })
  )
  })
@@ -1245,7 +1265,9 @@ describe("translatePathFromContext()", () => {
  expect(fetchSpy).toHaveBeenCalledWith(
  expect.anything(),
  expect.objectContaining({
- withAuth: true,
+ headers: expect.objectContaining({
+ Authorization: "Bearer sample-token",
+ }),
  })
  )
  })