-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat(core): Add Apollo server cache configuration option #3775
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
base: minor
Are you sure you want to change the base?
Changes from 4 commits
cb28c51
7cdd86c
da49949
dcd8aee
8767e14
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,223 @@ | ||
| import { KeyValueCache } from '@apollo/utils.keyvaluecache'; | ||
| import { mergeConfig } from '@vendure/core'; | ||
| import { createTestEnvironment } from '@vendure/testing'; | ||
| import path from 'path'; | ||
| import { afterAll, beforeAll, describe, expect, it } from 'vitest'; | ||
|
|
||
| import { initialData } from '../../../e2e-common/e2e-initial-data'; | ||
| import { TEST_SETUP_TIMEOUT_MS, testConfig } from '../../../e2e-common/test-config'; | ||
|
|
||
| import * as Codegen from './graphql/generated-e2e-admin-types'; | ||
| import { GET_PRODUCT_LIST, GET_PRODUCT_SIMPLE } from './graphql/shared-definitions'; | ||
|
|
||
| class MockCache implements KeyValueCache<string> { | ||
| static getCalls: Array<{ key: string; result: string | undefined }> = []; | ||
| static setCalls: Array<{ key: string; value: string; options?: { ttl?: number } }> = []; | ||
| static deleteCalls: Array<{ key: string; result: boolean }> = []; | ||
|
|
||
| private store = new Map<string, string>(); | ||
|
|
||
| static reset() { | ||
| this.getCalls = []; | ||
| this.setCalls = []; | ||
| this.deleteCalls = []; | ||
| } | ||
|
|
||
| get(key: string): Promise<string | undefined> { | ||
| // eslint-disable-next-line | ||
| console.log(`MockCache get: ${key}`); | ||
| const result = this.store.get(key); | ||
| MockCache.getCalls.push({ key, result }); | ||
| return Promise.resolve(result); | ||
| } | ||
|
|
||
| set(key: string, value: string, options?: { ttl?: number }): Promise<void> { | ||
| // eslint-disable-next-line | ||
| console.log(`MockCache set: ${key}`, value); | ||
| this.store.set(key, value); | ||
| MockCache.setCalls.push({ key, value, options }); | ||
| return Promise.resolve(); | ||
| } | ||
|
|
||
| delete(key: string): Promise<boolean> { | ||
| const result = this.store.delete(key); | ||
| MockCache.deleteCalls.push({ key, result }); | ||
| return Promise.resolve(result); | ||
| } | ||
| } | ||
|
|
||
| describe('Apollo cache configuration', () => { | ||
| describe('with custom cache implementation', () => { | ||
| const mockCache = new MockCache(); | ||
| const { server, adminClient, shopClient } = createTestEnvironment( | ||
| mergeConfig(testConfig(), { | ||
| apiOptions: { | ||
| cache: mockCache, | ||
| }, | ||
| }), | ||
| ); | ||
|
|
||
| beforeAll(async () => { | ||
| await server.init({ | ||
| initialData, | ||
| productsCsvPath: path.join(__dirname, 'fixtures/e2e-products-minimal.csv'), | ||
| customerCount: 1, | ||
| }); | ||
| await adminClient.asSuperAdmin(); | ||
| }, TEST_SETUP_TIMEOUT_MS); | ||
|
|
||
| afterAll(async () => { | ||
| await server.destroy(); | ||
| }); | ||
|
|
||
| it('should configure Apollo Server with custom cache', async () => { | ||
| MockCache.reset(); | ||
|
|
||
| // Make a GraphQL query that could potentially be cached | ||
| const result = await shopClient.query< | ||
| Codegen.GetProductListQuery, | ||
| Codegen.GetProductListQueryVariables | ||
| >(GET_PRODUCT_LIST, { | ||
| options: { | ||
| filter: { id: { eq: 'T_1' } }, | ||
| take: 1, | ||
| }, | ||
| }); | ||
|
|
||
| expect(result.products.items.length).toBe(1); | ||
| expect(result.products.items[0].id).toBe('T_1'); | ||
| expect(result.products.items[0].name).toBe('Laptop'); | ||
| }); | ||
|
|
||
| it('should handle cache operations without errors', async () => { | ||
| MockCache.reset(); | ||
|
|
||
| // Test multiple queries to potentially trigger cache operations | ||
| const shopResponse = await shopClient.query< | ||
| Codegen.GetProductListQuery, | ||
| Codegen.GetProductListQueryVariables | ||
| >(GET_PRODUCT_LIST, { | ||
| options: { | ||
| take: 1, | ||
| }, | ||
| }); | ||
|
|
||
| expect(shopResponse.products.items.length).toBe(1); | ||
|
|
||
| const adminResponse = await adminClient.query< | ||
| Codegen.GetProductListQuery, | ||
| Codegen.GetProductListQueryVariables | ||
| >(GET_PRODUCT_LIST, { | ||
| options: { take: 1 }, | ||
| }); | ||
|
|
||
| expect(adminResponse.products.items.length).toBe(1); | ||
| }); | ||
|
|
||
| it('should work with both shop and admin APIs', async () => { | ||
| MockCache.reset(); | ||
|
|
||
| const [shopResult, adminResult] = await Promise.all([ | ||
| shopClient.query<Codegen.GetProductSimpleQuery, Codegen.GetProductSimpleQueryVariables>( | ||
| GET_PRODUCT_SIMPLE, | ||
| { | ||
| id: 'T_1', | ||
| }, | ||
| ), | ||
| adminClient.query<Codegen.GetProductSimpleQuery, Codegen.GetProductSimpleQueryVariables>( | ||
| GET_PRODUCT_SIMPLE, | ||
| { | ||
| id: 'T_1', | ||
| }, | ||
| ), | ||
| ]); | ||
|
|
||
| expect(shopResult.product).toBeDefined(); | ||
| expect(adminResult.product).toBeDefined(); | ||
| expect(shopResult.product?.id).toBe(adminResult.product?.id); | ||
| }); | ||
| }); | ||
|
|
||
| describe('with bounded cache', () => { | ||
| const { server, adminClient, shopClient } = createTestEnvironment( | ||
| mergeConfig(testConfig(), { | ||
| apiOptions: { | ||
| cache: 'bounded', | ||
| }, | ||
| }), | ||
| ); | ||
|
|
||
| beforeAll(async () => { | ||
| await server.init({ | ||
| initialData, | ||
| productsCsvPath: path.join(__dirname, 'fixtures/e2e-products-minimal.csv'), | ||
| customerCount: 1, | ||
| }); | ||
| await adminClient.asSuperAdmin(); | ||
| }, TEST_SETUP_TIMEOUT_MS); | ||
|
|
||
| afterAll(async () => { | ||
| await server.destroy(); | ||
| }); | ||
|
|
||
| it('should configure Apollo Server with bounded cache', async () => { | ||
| const result = await shopClient.query< | ||
| Codegen.GetProductSimpleQuery, | ||
| Codegen.GetProductSimpleQueryVariables | ||
| >(GET_PRODUCT_SIMPLE, { | ||
| id: 'T_1', | ||
| }); | ||
|
|
||
| expect(result.product).toBeDefined(); | ||
| expect(result.product?.id).toBe('T_1'); | ||
| }); | ||
|
|
||
| it('should handle concurrent requests with bounded cache', async () => { | ||
| const queries = Array.from({ length: 5 }, (_, i) => | ||
| shopClient.query<Codegen.GetProductSimpleQuery, Codegen.GetProductSimpleQueryVariables>( | ||
| GET_PRODUCT_SIMPLE, | ||
| { | ||
| id: `T_${i + 1}`, | ||
| }, | ||
| ), | ||
| ); | ||
|
|
||
| const results = await Promise.all(queries); | ||
|
|
||
| results.forEach((result, index) => { | ||
| if (result.product) { | ||
| expect(result.product.id).toBe(`T_${index + 1}`); | ||
| } | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| describe('without cache configuration', () => { | ||
| const { server, adminClient, shopClient } = createTestEnvironment(testConfig()); | ||
|
|
||
| beforeAll(async () => { | ||
| await server.init({ | ||
| initialData, | ||
| productsCsvPath: path.join(__dirname, 'fixtures/e2e-products-minimal.csv'), | ||
| customerCount: 1, | ||
| }); | ||
| await adminClient.asSuperAdmin(); | ||
| }, TEST_SETUP_TIMEOUT_MS); | ||
|
|
||
| afterAll(async () => { | ||
| await server.destroy(); | ||
| }); | ||
|
|
||
| it('should work without cache configuration', async () => { | ||
| const result = await shopClient.query< | ||
| Codegen.GetProductSimpleQuery, | ||
| Codegen.GetProductSimpleQueryVariables | ||
| >(GET_PRODUCT_SIMPLE, { | ||
| id: 'T_1', | ||
| }); | ||
|
|
||
| expect(result.product).toBeDefined(); | ||
| expect(result.product?.id).toBe('T_1'); | ||
| }); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| import { ApolloServerPlugin } from '@apollo/server'; | ||
| import type { KeyValueCache } from '@apollo/utils.keyvaluecache'; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainPublic API now references an external Apollo type; ensure dependency and consider decoupling ApiOptions.cache exports a type from '@apollo/utils.keyvaluecache'. This will surface in emitted d.ts and requires downstream consumers to resolve that package. Two follow-ups:
Run this to verify dependency presence and where the type appears in the emitted d.ts: 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Check dependency in packages/core/package.json"
jq -r '.dependencies["@apollo/utils.keyvaluecache"], .peerDependencies["@apollo/utils.keyvaluecache"], .devDependencies["@apollo/utils.keyvaluecache"]' packages/core/package.json || true
echo
echo "Find references to '@apollo/utils.keyvaluecache' in core source"
rg -n "@apollo/utils\.keyvaluecache" packages/core/src -C1
echo
echo "Optional: confirm the type import is emitted in d.ts (requires build artifacts)"
fd -a 'vendure-config.d.ts' packages/core/dist | xargs -I{} rg -n "@apollo/utils\.keyvaluecache" {}Length of output: 1247 Ensure explicit dependency on '@apollo/utils.keyvaluecache' and avoid leaking third-party types ApiOptions.cache currently imports and re-exports Apollo’s
In packages/core/src/config/vendure-config.ts around line 2, the file imports |
||
| import { RenderPageOptions } from '@apollographql/graphql-playground-html'; | ||
| import { DynamicModule, Type } from '@nestjs/common'; | ||
| import { CorsOptions } from '@nestjs/common/interfaces/external/cors-options.interface'; | ||
|
|
@@ -209,6 +210,17 @@ export interface ApiOptions { | |
| * @default [] | ||
| */ | ||
| apolloServerPlugins?: ApolloServerPlugin[]; | ||
| /** | ||
| * @description | ||
| * Pass a [custom Apollo cache](https://www.apollographql.com/docs/apollo-server/performance/caching) to the underlying Apollo Server. | ||
| * Note: this option only supplies the cache instance. To enable GraphQL response caching you must also add | ||
| * the `responseCachePlugin` (or another plugin that uses `server.cache`) to `apolloServerPlugins`, and set | ||
| * appropriate cache hints on fields. This option is unrelated to Vendure's {@link SystemOptions}.`cacheStrategy`. | ||
| * | ||
| * @default 'bounded' | ||
| * @since 3.5.0 | ||
| */ | ||
| cache?: KeyValueCache<string> | 'bounded'; | ||
| /** | ||
| * @description | ||
| * Controls whether introspection of the GraphQL APIs is enabled. For production, it is recommended to disable | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Tests don’t actually verify that Apollo uses the provided cache
Right now we only assert that queries succeed. Without a plugin calling server.cache (e.g. responseCachePlugin), the cache won’t be exercised. Either:
Minimal, self-contained approach (no external deps): add a tiny plugin that touches server.cache so we can assert calls. Apply these diffs:
If the underlying Apollo context in this environment doesn’t expose
requestContext.server.cache, switch to using the official@apollo/server-plugin-response-cacheand assert on MockCache calls after issuing duplicate requests. I can provide an alternate diff once you confirm the plugin you prefer.Also applies to: 73-90, 92-116, 117-139