-
-
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 all 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,120 @@ | ||
| import type { KeyValueCache } from '@apollo/utils.keyvaluecache'; | ||
| import { mergeConfig } from '@vendure/core'; | ||
| import { createTestEnvironment } from '@vendure/testing'; | ||
| import path from 'path'; | ||
| import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } 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'; | ||
|
|
||
| describe('Apollo cache configuration', () => { | ||
| describe('with custom cache implementation', () => { | ||
| const mockCache: KeyValueCache<string> = { | ||
| get: vi.fn().mockResolvedValue(undefined), | ||
| set: vi.fn().mockResolvedValue(undefined), | ||
| delete: vi.fn().mockResolvedValue(true), | ||
| }; | ||
|
|
||
| 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); | ||
|
|
||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| afterAll(async () => { | ||
| await server.destroy(); | ||
| }); | ||
|
|
||
| it('should configure Apollo Server with custom cache', async () => { | ||
| // Make a GraphQL query | ||
| 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'); | ||
|
|
||
| // The custom cache is configured and available to Apollo Server | ||
| // Apollo Server may use the cache for internal operations | ||
| // The fact that queries execute without errors confirms proper configuration | ||
| expect(mockCache).toBeDefined(); | ||
| expect(typeof mockCache.get).toBe('function'); | ||
|
Member
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. Here I would expect to see something like expect(mockCache.set).toHaveBeenCalledWith(...)and so on. |
||
| expect(typeof mockCache.set).toBe('function'); | ||
| expect(typeof mockCache.delete).toBe('function'); | ||
| }); | ||
|
|
||
| it('should handle multiple queries without errors', async () => { | ||
| // Test multiple queries | ||
| 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); | ||
|
|
||
| // Both queries execute successfully with the custom cache configured | ||
| expect(shopResponse).toBeDefined(); | ||
|
Member
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. Here I would expect the use of
|
||
| expect(adminResponse).toBeDefined(); | ||
| }); | ||
|
|
||
| it('should work with both shop and admin APIs', async () => { | ||
| 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); | ||
| expect(shopResult.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