-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Gateway: introduce make-fetch-happen
#3783
Merged
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
6dfb96f
Initial implementation of make-fetch-happen
trevor-scheer 59a5697
Implement in memory cache manager for make-fetch-happen
trevor-scheer 517966c
Cleanup types
trevor-scheer 1c1ce29
Cleanup
trevor-scheer d5c52bc
Improve provided typings for `make-fetch-happen`
trevor-scheer efeae34
Add @ts-ignore with explanation
trevor-scheer bbe3dee
Add changelog entry
trevor-scheer a537651
Skip node v6 for federation and gateway packages
trevor-scheer 7ac184c
Rename Cache -> HttpRequestCache
trevor-scheer 40acac8
Simplify cache key generation
trevor-scheer 535fb10
Cache native objects rather than stringified versions of them
trevor-scheer 5b3a333
Use the FetcherOptions type
trevor-scheer 2b3f234
Fixes to handwritten make-fetch-happen types
trevor-scheer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,19 @@ | ||
const config = require('../../jest.config.base'); | ||
|
||
module.exports = Object.assign(Object.create(null), config, { | ||
const NODE_MAJOR_VERSION = parseInt( | ||
process.versions.node.split('.', 1)[0], | ||
10 | ||
); | ||
|
||
const additionalConfig = { | ||
setupFiles: [ | ||
'core-js/features/array/flat', | ||
'core-js/features/array/flat-map', | ||
], | ||
}); | ||
testPathIgnorePatterns: [ | ||
...config.testPathIgnorePatterns, | ||
...NODE_MAJOR_VERSION === 6 ? ["<rootDir>"] : [] | ||
] | ||
}; | ||
|
||
module.exports = Object.assign(Object.create(null), config, additionalConfig); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,17 @@ | ||
const path = require('path'); | ||
const config = require('../../jest.config.base'); | ||
|
||
const NODE_MAJOR_VERSION = parseInt( | ||
process.versions.node.split('.', 1)[0], | ||
10 | ||
); | ||
|
||
const additionalConfig = { | ||
setupFilesAfterEnv: [path.resolve(__dirname, './src/__tests__/testSetup.ts')], | ||
testPathIgnorePatterns: [ | ||
...config.testPathIgnorePatterns, | ||
...NODE_MAJOR_VERSION === 6 ? ["<rootDir>"] : [] | ||
] | ||
}; | ||
module.exports = Object.assign(Object.create(null), additionalConfig, config); | ||
|
||
module.exports = Object.assign(Object.create(null), config, additionalConfig); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
import { CacheManager } from 'make-fetch-happen'; | ||
import { Request, Response, Headers } from 'apollo-server-env'; | ||
import { InMemoryLRUCache } from 'apollo-server-caching'; | ||
|
||
const MAX_SIZE = 5 * 1024 * 1024; // 5MB | ||
|
||
function cacheKey(request: Request) { | ||
return `gateway:request-cache:${request.method}:${request.url}`; | ||
} | ||
|
||
interface CachedRequest { | ||
body: string; | ||
status: number; | ||
statusText: string; | ||
headers: Headers; | ||
} | ||
|
||
export class HttpRequestCache implements CacheManager { | ||
constructor( | ||
public cache: InMemoryLRUCache<CachedRequest> = new InMemoryLRUCache({ | ||
maxSize: MAX_SIZE, | ||
}), | ||
) {} | ||
|
||
// Return true if entry exists, else false | ||
async delete(request: Request) { | ||
const key = cacheKey(request); | ||
const entry = await this.cache.get(key); | ||
await this.cache.delete(key); | ||
return Boolean(entry); | ||
} | ||
|
||
async put(request: Request, response: Response) { | ||
let body = await response.text(); | ||
|
||
this.cache.set(cacheKey(request), { | ||
body, | ||
status: response.status, | ||
statusText: response.statusText, | ||
headers: response.headers, | ||
}); | ||
|
||
return new Response(body, response); | ||
} | ||
|
||
async match(request: Request) { | ||
return this.cache.get(cacheKey(request)).then(response => { | ||
if (response) { | ||
const { body, ...requestInit } = response; | ||
return new Response(body, requestInit); | ||
} | ||
return; | ||
}); | ||
} | ||
} |
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
declare module 'make-fetch-happen' { | ||
import { | ||
Response, | ||
Request, | ||
RequestInfo, | ||
RequestInit, | ||
} from 'apollo-server-env'; | ||
|
||
// If adding to these options, they should mirror those from `make-fetch-happen` | ||
// @see: https://github.com/npm/make-fetch-happen/#extra-options | ||
export interface FetcherOptions { | ||
abernix marked this conversation as resolved.
Show resolved
Hide resolved
trevor-scheer marked this conversation as resolved.
Show resolved
Hide resolved
|
||
cacheManager?: string | CacheManager; | ||
// @see: https://www.npmjs.com/package/retry#retrytimeoutsoptions | ||
retry?: | ||
| boolean | ||
| number | ||
| { | ||
// The maximum amount of times to retry the operation. Default is 10. Seting this to 1 means do it once, then retry it once | ||
retries?: number; | ||
// The exponential factor to use. Default is 2. | ||
factor?: number; | ||
// The number of milliseconds before starting the first retry. Default is 1000. | ||
minTimeout?: number; | ||
// The maximum number of milliseconds between two retries. Default is Infinity. | ||
maxTimeout?: number; | ||
// Randomizes the timeouts by multiplying with a factor between 1 to 2. Default is false. | ||
randomize?: boolean; | ||
}; | ||
onRetry?(): void; | ||
} | ||
|
||
export interface CacheManager { | ||
delete(req: Request): Promise<Boolean>; | ||
put(req: Request, res: Response): Promise<Response>; | ||
match(req: Request): Promise<Response | undefined>; | ||
} | ||
|
||
/** | ||
* This is an augmentation of the fetch function types provided by `apollo-server-env` | ||
* @see: https://git.io/JvBwX | ||
*/ | ||
export interface Fetcher { | ||
(input?: RequestInfo, init?: RequestInit & FetcherOptions): Promise< | ||
Response | ||
>; | ||
} | ||
|
||
let fetch: Fetcher & { | ||
defaults(opts?: FetcherOptions): Fetcher; | ||
}; | ||
|
||
export default fetch; | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Do we actually need to implement a
delete
method? When does it get called?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.
Ours is never actually called (at least within our tests), however I do want our exported interface to comply with
make-fetch-happen
's requirements listed here: https://github.com/npm/make-fetch-happen#--optscachemanagerOn that note, I'm unsure if I should export the
CacheManager
interface from the gateway package or if I can leave it in themake-fetch-happen
definition file. Thoughts?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.
@trevor-scheer I think we should leave it in
make-fetch-happen
, and avoid exposing this directly to the user, for now.