-
-
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 1 commit
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,239 @@ | ||
| import { KeyValueCache } from '@apollo/utils.keyvaluecache'; | ||
| import { mergeConfig } from '@vendure/core'; | ||
| import { createTestEnvironment } from '@vendure/testing'; | ||
| import gql from 'graphql-tag'; | ||
| 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'; | ||
|
|
||
| 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 = []; | ||
| } | ||
|
|
||
| async 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 result; | ||
| } | ||
|
|
||
| async 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 }); | ||
| } | ||
|
|
||
| async delete(key: string): Promise<boolean> { | ||
| const result = this.store.delete(key); | ||
| MockCache.deleteCalls.push({ key, result }); | ||
| return 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(gql` | ||
| query GetProduct { | ||
| product(id: "T_1") { | ||
| id | ||
| name | ||
| slug | ||
| } | ||
| } | ||
| `); | ||
|
|
||
| expect(result.product).toBeDefined(); | ||
| expect(result.product.id).toBe('T_1'); | ||
| expect(result.product.name).toBe('Laptop'); | ||
| }); | ||
|
|
||
| it('should handle cache operations without errors', async () => { | ||
| MockCache.reset(); | ||
|
|
||
| // Test multiple queries to potentially trigger cache operations | ||
| await shopClient.query(gql` | ||
| query GetProducts { | ||
| products(options: { take: 5 }) { | ||
| items { | ||
| id | ||
| name | ||
| } | ||
| } | ||
| } | ||
| `); | ||
|
|
||
| await adminClient.query(gql` | ||
| query GetProducts { | ||
| products(options: { take: 3 }) { | ||
| items { | ||
| id | ||
| name | ||
| slug | ||
| } | ||
| } | ||
| } | ||
| `); | ||
|
|
||
| // The cache instance should be properly configured and accessible | ||
| // We don't verify specific cache calls as Apollo Server's internal | ||
| // caching behavior may vary, but we ensure no errors occur | ||
| expect(true).toBe(true); // Test passes if no errors thrown | ||
| }); | ||
|
|
||
| it('should work with both shop and admin APIs', async () => { | ||
| MockCache.reset(); | ||
|
|
||
| const [shopResult, adminResult] = await Promise.all([ | ||
| shopClient.query(gql` | ||
| query ShopQuery { | ||
| product(id: "T_1") { | ||
| id | ||
| name | ||
| } | ||
| } | ||
| `), | ||
| adminClient.query(gql` | ||
| query AdminQuery { | ||
| product(id: "T_1") { | ||
| id | ||
| name | ||
| slug | ||
| } | ||
| } | ||
| `), | ||
| ]); | ||
|
|
||
| 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(gql` | ||
| query GetProduct { | ||
| product(id: "T_1") { | ||
| id | ||
| name | ||
| } | ||
| } | ||
| `); | ||
|
|
||
| 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(gql` | ||
| query GetProduct${i} { | ||
| product(id: "T_${i + 1}") { | ||
| id | ||
| name | ||
| } | ||
| } | ||
| `), | ||
| ); | ||
|
|
||
| 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(gql` | ||
| query GetProduct { | ||
| product(id: "T_1") { | ||
| id | ||
| name | ||
| } | ||
| } | ||
| `); | ||
|
|
||
| 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,15 @@ export interface ApiOptions { | |
| * @default [] | ||
| */ | ||
| apolloServerPlugins?: ApolloServerPlugin[]; | ||
| /** | ||
| * @description | ||
| * Pass a [custom cache](https://www.apollographql.com/docs/apollo-server/performance/caching) for server-side caching to the Apollo server, | ||
| * which is the underlying GraphQL server used by Vendure. | ||
| * | ||
| * @default undefined | ||
| * @since 3.5.0 | ||
| */ | ||
| cache?: KeyValueCache<string> | 'bounded'; | ||
dlhck marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| /** | ||
| * @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