Skip to content

Commit

Permalink
feat: make POST requests cacheable (#24)
Browse files Browse the repository at this point in the history
  • Loading branch information
dwrBad authored Mar 19, 2022
1 parent 0ab63e8 commit b701148
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 7 deletions.
22 changes: 16 additions & 6 deletions src/http-data-source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,13 @@ export abstract class HTTPDataSource<TContext = any> extends DataSource {
request: Request,
response: Response<TResult>,
): boolean {
return statusCodeCacheableByDefault.has(response.statusCode) && request.method === 'GET'
return statusCodeCacheableByDefault.has(response.statusCode) && this.isRequestCacheable(request)
}

protected isRequestCacheable(request: Request): boolean {
// default behaviour is to cache only get requests
// If extending to non GET requests take care to provide an adequate onCacheKeyCalculation and isResponseCacheable
return request.method === 'GET'
}

/**
Expand Down Expand Up @@ -201,7 +207,7 @@ export abstract class HTTPDataSource<TContext = any> extends DataSource {
* Execute a HTTP GET request.
* Note that the **memoizedResults** and **cache** will be checked before request is made.
* By default the received response will be memoized.
*
*
* @param path the path to the resource
* @param requestOptions
*/
Expand Down Expand Up @@ -380,9 +386,11 @@ export abstract class HTTPDataSource<TContext = any> extends DataSource {

const cacheKey = this.onCacheKeyCalculation(request)

// check if we have any GET call in the cache to respond immediately
if (request.method === 'GET') {
// Memoize GET calls for the same data source instance
const isRequestMemoizable = this.isRequestMemoizable(request)

// check if we have a memoizable call in the cache to respond immediately
if (isRequestMemoizable) {
// Memoize calls for the same data source instance
// a single instance of the data sources is scoped to one graphql request
if (this.memoizedResults.has(cacheKey)) {
const response = await this.memoizedResults.get(cacheKey)!
Expand All @@ -403,7 +411,9 @@ export abstract class HTTPDataSource<TContext = any> extends DataSource {
headers,
}

if (options.method === 'GET') {
const requestIsCacheable = this.isRequestCacheable(request)

if (requestIsCacheable) {
// try to fetch from shared cache
if (request.requestCache) {
try {
Expand Down
72 changes: 71 additions & 1 deletion test/http-data-source.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1310,7 +1310,7 @@ test('Should respond with stale-if-error cache on origin error', async (t) => {
t.is(cacheMap.size, 1)
})

test('Should not cache POST requests', async (t) => {
test('Should not cache POST requests by default', async (t) => {
t.plan(6)

const path = '/'
Expand Down Expand Up @@ -1481,6 +1481,76 @@ test('Should only cache GET successful responses with the correct status code',
t.is(cacheMap.size, 0)
})

test('Should cache POST successful responses if isRequestCacheable allows to do so', async (t) => {
t.plan(7)

const path = '/custom/cacheable/post/route'

const wanted = { name: 'foo' }
const server = http.createServer((req, res) => {
t.is(req.method, 'POST')
res.writeHead(200, {
'content-type': 'application/json',
})
res.write(JSON.stringify(wanted))
res.end()
res.socket?.unref()
})

t.teardown(server.close.bind(server))

server.listen()

const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}`

const dataSource = new (class extends HTTPDataSource {
constructor() {
super(baseURL)
}
protected isRequestCacheable(request: Request): boolean {
return request.method === 'GET' || (request.method === 'POST' && request.path === path)
}
postFoo() {
return this.post(path, {
body: wanted,
requestCache: {
maxTtl: 10,
maxTtlIfError: 20,
},
})
}
})()

const cacheMap = new Map<string, string>()

dataSource.initialize({
context: {
a: 1,
},
cache: {
async delete(key: string) {
return cacheMap.delete(key)
},
async get(key: string) {
return cacheMap.get(key)
},
async set(key: string, value: string) {
cacheMap.set(key, value)
},
},
})

let response = await dataSource.postFoo()
t.deepEqual(response.body, wanted)
t.false(response.memoized)
t.false(response.isFromCache)

response = await dataSource.postFoo()
t.deepEqual(response.body, wanted)
t.false(response.memoized)
t.true(response.isFromCache)
})

test.serial('Global maxAge should be used when no maxAge was set or similar.', async (t) => {
const path = '/'

Expand Down

0 comments on commit b701148

Please sign in to comment.