Skip to content

Commit 7e0637f

Browse files
JohnAlbinJohnAlbin
committed
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.
1 parent 5581933 commit 7e0637f

File tree

4 files changed

+93
-54
lines changed

4 files changed

+93
-54
lines changed

‎packages/next-drupal/src/client.ts‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,10 @@ export class DrupalClient {
233233
this.tokenExpiresOn = Date.now() + token.expires_in * 1000
234234
}
235235

236-
async fetch(input: RequestInfo, init?: FetchOptions): Promise<Response> {
236+
async fetch(
237+
input: RequestInfo,
238+
{ withAuth, ...init }: FetchOptions = {}
239+
): Promise<Response> {
237240
init = {
238241
...init,
239242
credentials: "include",
@@ -245,10 +248,10 @@ export class DrupalClient {
245248

246249
// Using the auth set on the client.
247250
// TODO: Abstract this to a re-usable.
248-
if (init?.withAuth) {
251+
if (withAuth) {
249252
this.debug(`Using authenticated request.`)
250253

251-
if (init.withAuth === true) {
254+
if (withAuth === true) {
252255
if (typeof this._auth === "undefined") {
253256
throw new Error(
254257
"auth is not configured. See https://next-drupal.org/docs/client/auth"
@@ -289,32 +292,32 @@ export class DrupalClient {
289292
`${this._auth.token_type} ${this._auth.access_token}`
290293
}
291294
}
292-
} else if (typeof init.withAuth === "string") {
295+
} else if (typeof withAuth === "string") {
293296
this.debug(`Using custom authorization header.`)
294297

295-
init["headers"]["Authorization"] = init.withAuth
296-
} /* c8 ignore next 4 */ else if (typeof init.withAuth === "function") {
298+
init["headers"]["Authorization"] = withAuth
299+
} /* c8 ignore next 4 */ else if (typeof withAuth === "function") {
297300
this.debug(`Using custom authorization callback.`)
298301

299-
init["headers"]["Authorization"] = init.withAuth()
300-
} else if (isBasicAuth(init.withAuth)) {
302+
init["headers"]["Authorization"] = withAuth()
303+
} else if (isBasicAuth(withAuth)) {
301304
this.debug(`Using basic authorization header.`)
302305

303306
const basic = Buffer.from(
304-
`${init.withAuth.username}:${init.withAuth.password}`
307+
`${withAuth.username}:${withAuth.password}`
305308
).toString("base64")
306309

307310
init["headers"]["Authorization"] = `Basic ${basic}`
308-
} else if (isClientIdSecretAuth(init.withAuth)) {
311+
} else if (isClientIdSecretAuth(withAuth)) {
309312
// Fetch an access token and add it to the request.
310313
// Access token can be fetched from cache or using a custom auth method.
311-
const token = await this.getAccessToken(init.withAuth)
314+
const token = await this.getAccessToken(withAuth)
312315
if (token) {
313316
init["headers"]["Authorization"] = `Bearer ${token.access_token}`
314317
}
315-
} /* c8 ignore next 4 */ else if (isAccessTokenAuth(init.withAuth)) {
318+
} /* c8 ignore next 4 */ else if (isAccessTokenAuth(withAuth)) {
316319
init["headers"]["Authorization"] =
317-
`${init.withAuth.token_type} ${init.withAuth.access_token}`
320+
`${withAuth.token_type} ${withAuth.access_token}`
318321
}
319322
}
320323

‎packages/next-drupal/tests/DrupalClient/fetch-related-methods.test.ts‎

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,6 @@ describe("fetch()", () => {
105105
Accept: "application/vnd.api+json",
106106
Authorization: "Basic YWRtaW46cGFzc3dvcmQ=",
107107
},
108-
withAuth: true,
109108
})
110109
)
111110
})
@@ -133,7 +132,6 @@ describe("fetch()", () => {
133132
Accept: "application/vnd.api+json",
134133
Authorization: "Basic YXJzaGFkQG5leHQtZHJ1cGFsLm9yZzphYmMxMjM=",
135134
},
136-
withAuth: true,
137135
})
138136
)
139137
})
@@ -227,7 +225,6 @@ describe("fetch()", () => {
227225
foo: "bar",
228226
Authorization: "Basic YXJzaGFkQG5leHQtZHJ1cGFsLm9yZzphYmMxMjM=",
229227
},
230-
withAuth: true,
231228
})
232229
)
233230
})
@@ -464,7 +461,9 @@ describe("getMenu()", () => {
464461
expect(fetchSpy).toHaveBeenCalledWith(
465462
expect.anything(),
466463
expect.objectContaining({
467-
withAuth: true,
464+
headers: expect.objectContaining({
465+
Authorization: "Bearer sample-token",
466+
}),
468467
})
469468
)
470469
})
@@ -653,7 +652,9 @@ describe("getResource()", () => {
653652
expect(fetchSpy).toHaveBeenCalledWith(
654653
expect.anything(),
655654
expect.objectContaining({
656-
withAuth: true,
655+
headers: expect.objectContaining({
656+
Authorization: "Bearer sample-token",
657+
}),
657658
})
658659
)
659660
})
@@ -803,7 +804,9 @@ describe("getResourceByPath()", () => {
803804
expect(fetchSpy).toHaveBeenCalledWith(
804805
expect.anything(),
805806
expect.not.objectContaining({
806-
withAuth: true,
807+
headers: expect.objectContaining({
808+
Authorization: expect.anything(),
809+
}),
807810
})
808811
)
809812
expect(getAccessTokenSpy).not.toHaveBeenCalled()
@@ -814,7 +817,9 @@ describe("getResourceByPath()", () => {
814817
auth: mocks.auth.clientIdSecret,
815818
})
816819
const fetchSpy = spyOnFetch()
817-
const getAccessTokenSpy = jest.spyOn(client, "getAccessToken")
820+
const getAccessTokenSpy = jest
821+
.spyOn(client, "getAccessToken")
822+
.mockImplementation(async () => mocks.auth.accessToken)
818823

819824
await client.getResourceByPath<DrupalNode>(
820825
"/recipes/deep-mediterranean-quiche",
@@ -826,7 +831,9 @@ describe("getResourceByPath()", () => {
826831
expect(fetchSpy).toHaveBeenCalledWith(
827832
expect.anything(),
828833
expect.objectContaining({
829-
withAuth: true,
834+
headers: expect.objectContaining({
835+
Authorization: `${mocks.auth.accessToken.token_type} ${mocks.auth.accessToken.access_token}`,
836+
}),
830837
})
831838
)
832839
expect(getAccessTokenSpy).toHaveBeenCalled()
@@ -930,7 +937,9 @@ describe("getResourceCollection()", () => {
930937
expect(fetchSpy).toHaveBeenCalledWith(
931938
expect.anything(),
932939
expect.objectContaining({
933-
withAuth: true,
940+
headers: expect.objectContaining({
941+
Authorization: "Bearer sample-token",
942+
}),
934943
})
935944
)
936945
})
@@ -1029,7 +1038,9 @@ describe("getSearchIndex()", () => {
10291038
expect(fetchSpy).toHaveBeenCalledWith(
10301039
expect.anything(),
10311040
expect.objectContaining({
1032-
withAuth: true,
1041+
headers: expect.objectContaining({
1042+
Authorization: "Bearer sample-token",
1043+
}),
10331044
})
10341045
)
10351046
})
@@ -1114,7 +1125,9 @@ describe("getView()", () => {
11141125
expect(fetchSpy).toHaveBeenCalledWith(
11151126
expect.anything(),
11161127
expect.objectContaining({
1117-
withAuth: true,
1128+
headers: expect.objectContaining({
1129+
Authorization: "Bearer sample-token",
1130+
}),
11181131
})
11191132
)
11201133
})
@@ -1179,7 +1192,9 @@ describe("translatePath()", () => {
11791192
expect(fetchSpy).toHaveBeenCalledWith(
11801193
expect.anything(),
11811194
expect.objectContaining({
1182-
withAuth: true,
1195+
headers: expect.objectContaining({
1196+
Authorization: "Bearer sample-token",
1197+
}),
11831198
})
11841199
)
11851200
})

‎packages/next-drupal/tests/DrupalClient/pages-router-methods.test.ts‎

Lines changed: 46 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -232,13 +232,16 @@ describe("buildStaticPathsParamsFromPaths()", () => {
232232

233233
describe("getAuthFromContextAndOptions()", () => {
234234
const clientIdSecret = mocks.auth.clientIdSecret
235+
const accessToken = mocks.auth.accessToken
235236

236237
test("should use the withAuth option if provided and NOT in preview", async () => {
237238
const client = new DrupalClient(BASE_URL, {
238239
auth: clientIdSecret,
239240
})
240241
const fetchSpy = spyOnFetch()
241-
jest.spyOn(client, "getAccessToken")
242+
jest
243+
.spyOn(client, "getAccessToken")
244+
.mockImplementation(async () => accessToken)
242245

243246
await client.getResourceFromContext(
244247
"node--article",
@@ -253,7 +256,9 @@ describe("getAuthFromContextAndOptions()", () => {
253256
expect(fetchSpy).toHaveBeenCalledWith(
254257
expect.anything(),
255258
expect.objectContaining({
256-
withAuth: true,
259+
headers: expect.objectContaining({
260+
Authorization: `${accessToken.token_type} ${accessToken.access_token}`,
261+
}),
257262
})
258263
)
259264

@@ -274,11 +279,9 @@ describe("getAuthFromContextAndOptions()", () => {
274279
expect(fetchSpy).toHaveBeenLastCalledWith(
275280
expect.anything(),
276281
expect.objectContaining({
277-
withAuth: {
278-
clientId: "foo",
279-
clientSecret: "bar",
280-
scope: "baz",
281-
},
282+
headers: expect.objectContaining({
283+
Authorization: `${accessToken.token_type} ${accessToken.access_token}`,
284+
}),
282285
})
283286
)
284287
})
@@ -296,19 +299,19 @@ describe("getAuthFromContextAndOptions()", () => {
296299
expect(fetchSpy).toHaveBeenCalledWith(
297300
expect.anything(),
298301
expect.objectContaining({
299-
withAuth: false,
302+
headers: expect.not.objectContaining({
303+
Authorization: expect.anything(),
304+
}),
300305
})
301306
)
302307

303308
const client2 = new DrupalClient(BASE_URL, {
304309
auth: clientIdSecret,
305310
withAuth: true,
306311
})
307-
jest.spyOn(client2, "getAccessToken").mockImplementation(async () => ({
308-
token_type: "",
309-
expires_in: 0,
310-
access_token: "",
311-
}))
312+
jest
313+
.spyOn(client2, "getAccessToken")
314+
.mockImplementation(async () => accessToken)
312315

313316
await client2.getResourceFromContext("node--article", {
314317
preview: false,
@@ -317,7 +320,9 @@ describe("getAuthFromContextAndOptions()", () => {
317320
expect(fetchSpy).toHaveBeenLastCalledWith(
318321
expect.anything(),
319322
expect.objectContaining({
320-
withAuth: true,
323+
headers: expect.objectContaining({
324+
Authorization: `${accessToken.token_type} ${accessToken.access_token}`,
325+
}),
321326
})
322327
)
323328
})
@@ -327,7 +332,8 @@ describe("getAuthFromContextAndOptions()", () => {
327332
auth: clientIdSecret,
328333
withAuth: true,
329334
})
330-
const fetchSpy = spyOnFetch()
335+
const fetchSpy = jest.spyOn(client, "fetch")
336+
spyOnFetch()
331337

332338
await client.getResourceFromContext("node--article", {
333339
preview: true,
@@ -345,7 +351,8 @@ describe("getAuthFromContextAndOptions()", () => {
345351
const client = new DrupalClient(BASE_URL, {
346352
auth: clientIdSecret,
347353
})
348-
const fetchSpy = spyOnFetch()
354+
const fetchSpy = jest.spyOn(client, "fetch")
355+
spyOnFetch()
349356

350357
await client.getResourceFromContext("node--article", {
351358
preview: true,
@@ -375,7 +382,8 @@ describe("getAuthFromContextAndOptions()", () => {
375382
},
376383
withAuth: true,
377384
})
378-
const fetchSpy = spyOnFetch()
385+
const fetchSpy = jest.spyOn(client, "fetch")
386+
spyOnFetch()
379387

380388
await client.getResourceFromContext("node--article", {
381389
preview: true,
@@ -414,7 +422,9 @@ describe("getAuthFromContextAndOptions()", () => {
414422
expect(fetchSpy).toHaveBeenCalledWith(
415423
expect.anything(),
416424
expect.objectContaining({
417-
withAuth: `Bearer example-token`,
425+
headers: expect.objectContaining({
426+
Authorization: `Bearer example-token`,
427+
}),
418428
})
419429
)
420430
})
@@ -440,7 +450,9 @@ describe("getAuthFromContextAndOptions()", () => {
440450
expect(fetchSpy).toHaveBeenCalledWith(
441451
expect.anything(),
442452
expect.objectContaining({
443-
withAuth: `Bearer example-token`,
453+
headers: expect.objectContaining({
454+
Authorization: `Bearer example-token`,
455+
}),
444456
})
445457
)
446458
})
@@ -775,7 +787,9 @@ describe("getResourceCollectionFromContext()", () => {
775787
expect(fetchSpy).toHaveBeenCalledWith(
776788
expect.anything(),
777789
expect.objectContaining({
778-
withAuth: true,
790+
headers: expect.objectContaining({
791+
Authorization: "Bearer sample-token",
792+
}),
779793
})
780794
)
781795
})
@@ -981,7 +995,9 @@ describe("getResourceFromContext()", () => {
981995
expect(fetchSpy).toHaveBeenCalledWith(
982996
expect.anything(),
983997
expect.objectContaining({
984-
withAuth: true,
998+
headers: expect.objectContaining({
999+
Authorization: "Bearer sample-token",
1000+
}),
9851001
})
9861002
)
9871003
})
@@ -1010,7 +1026,9 @@ describe("getResourceFromContext()", () => {
10101026
expect(fetchSpy).toHaveBeenCalledWith(
10111027
expect.anything(),
10121028
expect.objectContaining({
1013-
withAuth: `Bearer sample-token`,
1029+
headers: expect.objectContaining({
1030+
Authorization: `Bearer sample-token`,
1031+
}),
10141032
})
10151033
)
10161034
})
@@ -1148,7 +1166,9 @@ describe("getStaticPathsFromContext()", () => {
11481166
expect(fetchSpy).toHaveBeenCalledWith(
11491167
expect.anything(),
11501168
expect.objectContaining({
1151-
withAuth: true,
1169+
headers: expect.objectContaining({
1170+
Authorization: "Bearer sample-token",
1171+
}),
11521172
})
11531173
)
11541174
})
@@ -1245,7 +1265,9 @@ describe("translatePathFromContext()", () => {
12451265
expect(fetchSpy).toHaveBeenCalledWith(
12461266
expect.anything(),
12471267
expect.objectContaining({
1248-
withAuth: true,
1268+
headers: expect.objectContaining({
1269+
Authorization: "Bearer sample-token",
1270+
}),
12491271
})
12501272
)
12511273
})

0 commit comments

Comments
 (0)