Skip to content

Commit 09a4bc1

Browse files
klippxEmrysMyrddin
klippx
and
EmrysMyrddin
authored
fix: [@envelop/response-cache] ttl yielded as NaN (#2090)
Co-authored-by: Valentin Cocaud <[email protected]>
1 parent cafc43f commit 09a4bc1

File tree

3 files changed

+241
-21
lines changed

3 files changed

+241
-21
lines changed

‎.changeset/odd-nails-refuse.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@envelop/response-cache': patch
3+
---
4+
5+
Fix TTL being NaN when using `@cacheControl` without `maxAge` argument.

‎packages/plugins/response-cache/src/plugin.ts‎

Lines changed: 38 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
DocumentNode,
55
ExecutionArgs,
66
getOperationAST,
7+
GraphQLDirective,
78
Kind,
89
print,
910
TypeInfo,
@@ -237,10 +238,8 @@ const getDocumentWithMetadataAndTTL = memoize4(function addTypeNameToDocument(
237238
const parentType = typeInfo.getParentType();
238239
if (parentType) {
239240
const schemaCoordinate = `${parentType.name}.${fieldNode.name.value}`;
240-
const maybeTtl = ttlPerSchemaCoordinate[schemaCoordinate];
241-
if (maybeTtl !== undefined) {
242-
ttl = calculateTtl(maybeTtl, ttl);
243-
}
241+
const maybeTtl = ttlPerSchemaCoordinate[schemaCoordinate] as unknown;
242+
ttl = calculateTtl(maybeTtl, ttl);
244243
}
245244
},
246245
}),
@@ -280,6 +279,11 @@ const getDocumentWithMetadataAndTTL = memoize4(function addTypeNameToDocument(
280279
return [visit(document, visitWithTypeInfo(typeInfo, visitor)), ttl];
281280
});
282281

282+
type CacheControlDirective = {
283+
maxAge?: number;
284+
scope?: 'PUBLIC' | 'PRIVATE';
285+
};
286+
283287
export function useResponseCache<PluginContext extends Record<string, any> = {}>({
284288
cache = createInMemoryCache(),
285289
ttl: globalTtl = Infinity,
@@ -328,18 +332,24 @@ export function useResponseCache<PluginContext extends Record<string, any> = {}>
328332
}
329333
schema = newSchema;
330334

331-
const cacheControlDirective = schema.getDirective('cacheControl');
335+
const directive = schema.getDirective('cacheControl') as unknown as
336+
| GraphQLDirective
337+
| undefined;
338+
332339
mapSchema(schema, {
333-
...(cacheControlDirective && {
340+
...(directive && {
334341
[MapperKind.COMPOSITE_TYPE]: type => {
335-
const cacheControlAnnotations = getDirective(schema, type, 'cacheControl');
342+
const cacheControlAnnotations = getDirective(
343+
schema,
344+
type,
345+
'cacheControl',
346+
) as unknown as CacheControlDirective[] | undefined;
336347
cacheControlAnnotations?.forEach(cacheControl => {
337-
const ttl = cacheControl.maxAge * 1000;
338-
if (ttl != null) {
339-
ttlPerType[type.name] = ttl;
348+
if (cacheControl.maxAge) {
349+
ttlPerType[type.name] = cacheControl.maxAge * 1000;
340350
}
341351
if (cacheControl.scope) {
342-
scopePerSchemaCoordinate[`${type.name}`] = cacheControl.scope;
352+
scopePerSchemaCoordinate[type.name] = cacheControl.scope;
343353
}
344354
});
345355
return type;
@@ -354,12 +364,15 @@ export function useResponseCache<PluginContext extends Record<string, any> = {}>
354364
idFieldByTypeName.set(typeName, fieldName);
355365
}
356366

357-
if (cacheControlDirective) {
358-
const cacheControlAnnotations = getDirective(schema, fieldConfig, 'cacheControl');
367+
if (directive) {
368+
const cacheControlAnnotations = getDirective(
369+
schema,
370+
fieldConfig,
371+
'cacheControl',
372+
) as unknown as CacheControlDirective[] | undefined;
359373
cacheControlAnnotations?.forEach(cacheControl => {
360-
const ttl = cacheControl.maxAge * 1000;
361-
if (ttl != null) {
362-
ttlPerSchemaCoordinate[schemaCoordinates] = ttl;
374+
if (cacheControl.maxAge) {
375+
ttlPerSchemaCoordinate[schemaCoordinates] = cacheControl.maxAge * 1000;
363376
}
364377
if (cacheControl.scope) {
365378
scopePerSchemaCoordinate[schemaCoordinates] = cacheControl.scope;
@@ -437,7 +450,8 @@ export function useResponseCache<PluginContext extends Record<string, any> = {}>
437450

438451
types.add(typename);
439452
if (typename in ttlPerType) {
440-
currentTtl = calculateTtl(ttlPerType[typename], currentTtl);
453+
const maybeTtl = ttlPerType[typename] as unknown;
454+
currentTtl = calculateTtl(maybeTtl, currentTtl);
441455
}
442456
if (entityId != null) {
443457
identifier.set(`${typename}:${entityId}`, { typename, id: entityId });
@@ -617,11 +631,14 @@ export function resultWithMetadata(
617631
};
618632
}
619633

620-
function calculateTtl(typeTtl: number, currentTtl: number | undefined): number {
621-
if (typeof currentTtl === 'number') {
622-
return Math.min(currentTtl, typeTtl);
634+
function calculateTtl(typeTtl: unknown, currentTtl: number | undefined): number | undefined {
635+
if (typeof typeTtl === 'number' && !Number.isNaN(typeTtl)) {
636+
if (typeof currentTtl === 'number') {
637+
return Math.min(currentTtl, typeTtl);
638+
}
639+
return typeTtl;
623640
}
624-
return typeTtl;
641+
return currentTtl;
625642
}
626643

627644
function unwrapTypenames(type: any): string[] {

‎packages/plugins/response-cache/test/response-cache.spec.ts‎

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3292,6 +3292,204 @@ describe('useResponseCache', () => {
32923292
await testInstance.execute(query);
32933293
expect(spy).toHaveBeenCalledTimes(2);
32943294
});
3295+
3296+
it('should cache correctly for session with ttl being a valid number', async () => {
3297+
jest.useFakeTimers();
3298+
const spy = jest.fn(() => [
3299+
{
3300+
id: 1,
3301+
name: 'User 1',
3302+
comments: [
3303+
{
3304+
id: 1,
3305+
text: 'Comment 1 of User 1',
3306+
},
3307+
],
3308+
},
3309+
{
3310+
id: 2,
3311+
name: 'User 2',
3312+
comments: [
3313+
{
3314+
id: 2,
3315+
text: 'Comment 2 of User 2',
3316+
},
3317+
],
3318+
},
3319+
]);
3320+
3321+
const schema = makeExecutableSchema({
3322+
typeDefs: /* GraphQL */ `
3323+
${cacheControlDirective}
3324+
type Query {
3325+
users: [User!]!
3326+
}
3327+
3328+
type User {
3329+
id: ID!
3330+
name: String! @cacheControl(scope: PRIVATE)
3331+
comments: [Comment!]!
3332+
recentComment: Comment
3333+
}
3334+
3335+
type Comment {
3336+
id: ID!
3337+
text: String!
3338+
}
3339+
`,
3340+
resolvers: {
3341+
Query: {
3342+
users: spy,
3343+
},
3344+
},
3345+
});
3346+
3347+
const cache: Cache = {
3348+
get: jest.fn(),
3349+
set: jest.fn(),
3350+
invalidate: () => {},
3351+
};
3352+
const testInstance = createTestkit(
3353+
[
3354+
useResponseCache({
3355+
session: () => 'PHP BOSS',
3356+
cache,
3357+
ttl: 200,
3358+
}),
3359+
],
3360+
schema,
3361+
);
3362+
3363+
const query = /* GraphQL */ `
3364+
query test {
3365+
users {
3366+
id
3367+
name
3368+
comments {
3369+
id
3370+
text
3371+
}
3372+
}
3373+
}
3374+
`;
3375+
3376+
await testInstance.execute(query);
3377+
expect(cache.get).toHaveBeenCalledWith(
3378+
'c3b653bbea8797070b0072c7d9b7f69ad28f24f4cf0fae91fcaadd205e87880d',
3379+
);
3380+
expect(cache.set).toHaveBeenCalledWith(
3381+
'c3b653bbea8797070b0072c7d9b7f69ad28f24f4cf0fae91fcaadd205e87880d',
3382+
{
3383+
data: {
3384+
users: [
3385+
{ comments: [{ id: '1', text: 'Comment 1 of User 1' }], id: '1', name: 'User 1' },
3386+
{ comments: [{ id: '2', text: 'Comment 2 of User 2' }], id: '2', name: 'User 2' },
3387+
],
3388+
},
3389+
},
3390+
expect.any(Object),
3391+
200,
3392+
);
3393+
});
3394+
3395+
it('should cache correctly for session with ttl being Infinity', async () => {
3396+
jest.useFakeTimers();
3397+
const spy = jest.fn(() => [
3398+
{
3399+
id: 1,
3400+
name: 'User 1',
3401+
comments: [
3402+
{
3403+
id: 1,
3404+
text: 'Comment 1 of User 1',
3405+
},
3406+
],
3407+
},
3408+
{
3409+
id: 2,
3410+
name: 'User 2',
3411+
comments: [
3412+
{
3413+
id: 2,
3414+
text: 'Comment 2 of User 2',
3415+
},
3416+
],
3417+
},
3418+
]);
3419+
3420+
const schema = makeExecutableSchema({
3421+
typeDefs: /* GraphQL */ `
3422+
${cacheControlDirective}
3423+
type Query {
3424+
users: [User!]!
3425+
}
3426+
3427+
type User {
3428+
id: ID!
3429+
name: String! @cacheControl(scope: PRIVATE)
3430+
comments: [Comment!]!
3431+
recentComment: Comment
3432+
}
3433+
3434+
type Comment {
3435+
id: ID!
3436+
text: String!
3437+
}
3438+
`,
3439+
resolvers: {
3440+
Query: {
3441+
users: spy,
3442+
},
3443+
},
3444+
});
3445+
3446+
const cache: Cache = {
3447+
get: jest.fn(),
3448+
set: jest.fn(),
3449+
invalidate: () => {},
3450+
};
3451+
const testInstance = createTestkit(
3452+
[
3453+
useResponseCache({
3454+
session: () => 'PHP BOSS',
3455+
cache,
3456+
ttl: Infinity,
3457+
}),
3458+
],
3459+
schema,
3460+
);
3461+
3462+
const query = /* GraphQL */ `
3463+
query test {
3464+
users {
3465+
id
3466+
name
3467+
comments {
3468+
id
3469+
text
3470+
}
3471+
}
3472+
}
3473+
`;
3474+
3475+
await testInstance.execute(query);
3476+
expect(cache.get).toHaveBeenCalledWith(
3477+
'c3b653bbea8797070b0072c7d9b7f69ad28f24f4cf0fae91fcaadd205e87880d',
3478+
);
3479+
expect(cache.set).toHaveBeenCalledWith(
3480+
'c3b653bbea8797070b0072c7d9b7f69ad28f24f4cf0fae91fcaadd205e87880d',
3481+
{
3482+
data: {
3483+
users: [
3484+
{ comments: [{ id: '1', text: 'Comment 1 of User 1' }], id: '1', name: 'User 1' },
3485+
{ comments: [{ id: '2', text: 'Comment 2 of User 2' }], id: '2', name: 'User 2' },
3486+
],
3487+
},
3488+
},
3489+
expect.any(Object),
3490+
Infinity,
3491+
);
3492+
});
32953493
});
32963494

32973495
it('should cache queries using @stream', async () => {

0 commit comments

Comments
 (0)