From 9e6b2c04ce02849c6d6ef981901dc4775ddb3f1d Mon Sep 17 00:00:00 2001 From: ryu <114303361+ryuapp@users.noreply.github.com> Date: Sat, 31 Aug 2024 14:28:15 +0900 Subject: [PATCH 1/7] feat(cache): add `duration` option --- src/middleware/cache/index.ts | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/src/middleware/cache/index.ts b/src/middleware/cache/index.ts index f7eecf6e4..07a3844f2 100644 --- a/src/middleware/cache/index.ts +++ b/src/middleware/cache/index.ts @@ -16,6 +16,7 @@ import type { MiddlewareHandler } from '../../types' * @param {boolean} [options.wait=false] - A boolean indicating if Hono should wait for the Promise of the `cache.put` function to resolve before continuing with the request. Required to be true for the Deno environment. * @param {string} [options.cacheControl] - A string of directives for the `Cache-Control` header. * @param {string | string[]} [options.vary] - Sets the `Vary` header in the response. If the original response header already contains a `Vary` header, the values are merged, removing any duplicates. + * @param {number} [options.duration] - A number of seconds to cache the response. * @param {Function} [options.keyGenerator] - Generates keys for every request in the `cacheName` store. This can be used to cache data based on request parameters or context parameters. * @returns {MiddlewareHandler} The middleware handler function. * @throws {Error} If the `vary` option includes "*". @@ -36,6 +37,7 @@ export const cache = (options: { wait?: boolean cacheControl?: string vary?: string | string[] + duration?: number keyGenerator?: (c: Context) => Promise | string }): MiddlewareHandler => { if (!globalThis.caches) { @@ -72,7 +74,9 @@ export const cache = (options: { let [name, value] = directive.trim().split('=', 2) name = name.toLowerCase() if (!existingDirectives.includes(name)) { - c.header('Cache-Control', `${name}${value ? `=${value}` : ''}`, { append: true }) + c.header('Cache-Control', `${name}${value ? `=${value}` : ''}`, { + append: true, + }) } } } @@ -98,6 +102,8 @@ export const cache = (options: { } } + const expirationHeader = 'Hono-Cache-Expiration' + return async function cache(c, next) { let key = c.req.url if (options.keyGenerator) { @@ -109,7 +115,18 @@ export const cache = (options: { const cache = await caches.open(cacheName) const response = await cache.match(key) if (response) { - return new Response(response.body, response) + if (options.duration) { + const duration = Number(response.headers.get(expirationHeader)) + if (duration) { + const now = Date.now() + response.headers.delete(expirationHeader) + if (duration > now) { + return new Response(response.body, response) + } + } + } else { + return new Response(response.body, response) + } } await next() @@ -118,6 +135,10 @@ export const cache = (options: { } addHeader(c) const res = c.res.clone() + if (options.duration) { + const expiration = Date.now() + options.duration * 1000 + res.headers.set(expirationHeader, String(expiration)) + } if (options.wait) { await cache.put(key, res) } else { From 21fd78587b6c61189bccbe5e72ef61b350437da9 Mon Sep 17 00:00:00 2001 From: ryu <114303361+ryuapp@users.noreply.github.com> Date: Sat, 7 Sep 2024 20:47:32 +0900 Subject: [PATCH 2/7] fix: not working with Cloudflare Co-Authored-By: Taku Amano --- src/middleware/cache/index.ts | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/middleware/cache/index.ts b/src/middleware/cache/index.ts index 07a3844f2..f646d7878 100644 --- a/src/middleware/cache/index.ts +++ b/src/middleware/cache/index.ts @@ -102,7 +102,7 @@ export const cache = (options: { } } - const expirationHeader = 'Hono-Cache-Expiration' + const durationHeader = 'Hono-Cache-Duration' return async function cache(c, next) { let key = c.req.url @@ -116,13 +116,11 @@ export const cache = (options: { const response = await cache.match(key) if (response) { if (options.duration) { - const duration = Number(response.headers.get(expirationHeader)) - if (duration) { - const now = Date.now() - response.headers.delete(expirationHeader) - if (duration > now) { - return new Response(response.body, response) - } + const duration = Number(response.headers.get(durationHeader)) + if (duration && duration > Date.now()) { + const newResponse = new Response(response.body, response) + newResponse.headers.delete(durationHeader) + return newResponse } } else { return new Response(response.body, response) @@ -136,8 +134,8 @@ export const cache = (options: { addHeader(c) const res = c.res.clone() if (options.duration) { - const expiration = Date.now() + options.duration * 1000 - res.headers.set(expirationHeader, String(expiration)) + const duration = String(Date.now() + options.duration * 1000) + res.headers.set(durationHeader, duration) } if (options.wait) { await cache.put(key, res) From a819fd0c8f8773b5928a4e1507d1edcb7e496c6f Mon Sep 17 00:00:00 2001 From: ryu <114303361+ryuapp@users.noreply.github.com> Date: Sat, 7 Sep 2024 20:49:13 +0900 Subject: [PATCH 3/7] test: add simple tests for Deno --- runtime_tests/deno/cache.test.ts | 67 ++++++++++++++++++++++++++++++++ runtime_tests/deno/deno.json | 1 + runtime_tests/deno/deno.lock | 42 ++++++++++++++------ 3 files changed, 99 insertions(+), 11 deletions(-) create mode 100644 runtime_tests/deno/cache.test.ts diff --git a/runtime_tests/deno/cache.test.ts b/runtime_tests/deno/cache.test.ts new file mode 100644 index 000000000..ae00f575a --- /dev/null +++ b/runtime_tests/deno/cache.test.ts @@ -0,0 +1,67 @@ +import { Hono } from '../../src/hono.ts' +import { cache } from '../../src/middleware/cache/index.ts' +import { expect } from '@std/expect' +import { FakeTime } from '@std/testing/time' + +Deno.test('Get cached text', async () => { + const c = await caches.open('my-app') + await c.delete('http://localhost') + + let oneTimeWord = 'Hello Hono' + + const app = new Hono() + app.use( + '/', + cache({ + cacheName: 'my-app', + wait: true, + }) + ) + app.get('/', (c) => { + const result = oneTimeWord + oneTimeWord = 'Not Found' + return c.text(result) + }) + + await app.request('http://localhost') + const res = await app.request('http://localhost') + expect(await res.text()).toBe('Hello Hono') + expect(res).not.toBeNull() + expect(res.status).toBe(200) +}) + +Deno.test( + { + name: 'Do not get cached text over duration', + sanitizeResources: false, + }, + async () => { + const time = new FakeTime() + const c = await caches.open('my-app') + await c.delete('http://localhost') + + let oneTimeWord = 'Hello Hono' + + const app = new Hono() + app.use( + '/', + cache({ + cacheName: 'my-app', + wait: true, + duration: 60, + }) + ) + app.get('/', (c) => { + const result = oneTimeWord + oneTimeWord = 'Not Found' + return c.text(result) + }) + + await app.request('http://localhost') + await time.tickAsync(600_100) // 10min + const res = await app.request('http://localhost') + expect(await res.text()).toBe('Not Found') + expect(res).not.toBeNull() + expect(res.status).toBe(200) + } +) diff --git a/runtime_tests/deno/deno.json b/runtime_tests/deno/deno.json index 240c5d2e4..012e7b7ee 100644 --- a/runtime_tests/deno/deno.json +++ b/runtime_tests/deno/deno.json @@ -13,6 +13,7 @@ ], "imports": { "@std/assert": "jsr:@std/assert@^1.0.3", + "@std/expect": "jsr:@std/expect@^1.0.2", "@std/testing": "jsr:@std/testing@^1.0.1", "hono/jsx/jsx-runtime": "../../src/jsx/jsx-runtime.ts" } diff --git a/runtime_tests/deno/deno.lock b/runtime_tests/deno/deno.lock index 49d0b042e..537fe7fe1 100644 --- a/runtime_tests/deno/deno.lock +++ b/runtime_tests/deno/deno.lock @@ -2,24 +2,43 @@ "version": "3", "packages": { "specifiers": { - "jsr:@std/assert@^1.0.3": "jsr:@std/assert@1.0.3", - "jsr:@std/internal@^1.0.2": "jsr:@std/internal@1.0.2", - "jsr:@std/testing@^1.0.1": "jsr:@std/testing@1.0.1" + "jsr:@std/assert@^1.0.3": "jsr:@std/assert@1.0.4", + "jsr:@std/assert@^1.0.4": "jsr:@std/assert@1.0.4", + "jsr:@std/async@^1.0.5": "jsr:@std/async@1.0.5", + "jsr:@std/data-structures@^1.0.2": "jsr:@std/data-structures@1.0.2", + "jsr:@std/expect@^1.0.2": "jsr:@std/expect@1.0.2", + "jsr:@std/internal@^1.0.3": "jsr:@std/internal@1.0.3", + "jsr:@std/testing@^1.0.1": "jsr:@std/testing@1.0.2" }, "jsr": { - "@std/assert@1.0.3": { - "integrity": "b0d03ce1ced880df67132eea140623010d415848df66f6aa5df76507ca7c26d8", + "@std/assert@1.0.4": { + "integrity": "d4c767ea578e5bc09c15b6e503376003e5b2d1f4c0cdf08524a92101ff4d7b96", "dependencies": [ - "jsr:@std/internal@^1.0.2" + "jsr:@std/internal@^1.0.3" ] }, - "@std/internal@1.0.2": { - "integrity": "f4cabe2021352e8bfc24e6569700df87bf070914fc38d4b23eddd20108ac4495" + "@std/async@1.0.5": { + "integrity": "31d68214bfbb31bd4c6022401d484e3964147c76c9220098baa703a39b6c2da6" }, - "@std/testing@1.0.1": { - "integrity": "9c25841137ee818933e1722091bb9ed5fdc251c35e84c97979a52196bdb6c5c3", + "@std/data-structures@1.0.2": { + "integrity": "d586efced05ed91923514a192996d23673f32f695f7ebc37f0cc1e17928be600" + }, + "@std/expect@1.0.2": { + "integrity": "37d5162eb1d30505e09f77f3fabbf6cde1e81ed0e72d2371cddbce1173eb86b4", + "dependencies": [ + "jsr:@std/assert@^1.0.4", + "jsr:@std/internal@^1.0.3" + ] + }, + "@std/internal@1.0.3": { + "integrity": "208e9b94a3d5649bd880e9ca38b885ab7651ab5b5303a56ed25de4755fb7b11e" + }, + "@std/testing@1.0.2": { + "integrity": "9e8a7f4e26c219addabe7942d09c3450fa0a74e9662341961bc0ef502274dec3", "dependencies": [ - "jsr:@std/assert@^1.0.3" + "jsr:@std/assert@^1.0.4", + "jsr:@std/async@^1.0.5", + "jsr:@std/data-structures@^1.0.2" ] } } @@ -28,6 +47,7 @@ "workspace": { "dependencies": [ "jsr:@std/assert@^1.0.3", + "jsr:@std/expect@^1.0.2", "jsr:@std/testing@^1.0.1" ] } From da804cb1a82e1d6136228d0176bbf1a16e338583 Mon Sep 17 00:00:00 2001 From: ryu <114303361+ryuapp@users.noreply.github.com> Date: Wed, 11 Sep 2024 02:03:39 +0900 Subject: [PATCH 4/7] chore --- runtime_tests/deno/cache.test.ts | 8 +++++--- src/middleware/cache/index.ts | 11 ++++------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/runtime_tests/deno/cache.test.ts b/runtime_tests/deno/cache.test.ts index ae00f575a..94f963c43 100644 --- a/runtime_tests/deno/cache.test.ts +++ b/runtime_tests/deno/cache.test.ts @@ -3,7 +3,7 @@ import { cache } from '../../src/middleware/cache/index.ts' import { expect } from '@std/expect' import { FakeTime } from '@std/testing/time' -Deno.test('Get cached text', async () => { +Deno.test('Should return cached response', async () => { const c = await caches.open('my-app') await c.delete('http://localhost') @@ -26,13 +26,14 @@ Deno.test('Get cached text', async () => { await app.request('http://localhost') const res = await app.request('http://localhost') expect(await res.text()).toBe('Hello Hono') + expect(res.headers.get('Hono-Cached-Time')).toBeNull() expect(res).not.toBeNull() expect(res.status).toBe(200) }) Deno.test( { - name: 'Do not get cached text over duration', + name: 'Should not return cached response over duration', sanitizeResources: false, }, async () => { @@ -58,9 +59,10 @@ Deno.test( }) await app.request('http://localhost') - await time.tickAsync(600_100) // 10min + await time.tickAsync(60000) const res = await app.request('http://localhost') expect(await res.text()).toBe('Not Found') + expect(res.headers.get('Hono-Cached-Time')).toBeNull() expect(res).not.toBeNull() expect(res.status).toBe(200) } diff --git a/src/middleware/cache/index.ts b/src/middleware/cache/index.ts index f646d7878..6c04207d2 100644 --- a/src/middleware/cache/index.ts +++ b/src/middleware/cache/index.ts @@ -102,7 +102,7 @@ export const cache = (options: { } } - const durationHeader = 'Hono-Cache-Duration' + const cachedTime = 'Hono-Cached-Time' return async function cache(c, next) { let key = c.req.url @@ -116,11 +116,9 @@ export const cache = (options: { const response = await cache.match(key) if (response) { if (options.duration) { - const duration = Number(response.headers.get(durationHeader)) + const duration = Number(response.headers.get(cachedTime)) + options.duration * 1000 if (duration && duration > Date.now()) { - const newResponse = new Response(response.body, response) - newResponse.headers.delete(durationHeader) - return newResponse + return new Response(response.body, response).headers.delete(cachedTime) } } else { return new Response(response.body, response) @@ -134,8 +132,7 @@ export const cache = (options: { addHeader(c) const res = c.res.clone() if (options.duration) { - const duration = String(Date.now() + options.duration * 1000) - res.headers.set(durationHeader, duration) + res.headers.set(cachedTime, String(Date.now())) } if (options.wait) { await cache.put(key, res) From a988479cac22d99a20207deef7e1116e6f7cd891 Mon Sep 17 00:00:00 2001 From: ryu <114303361+ryuapp@users.noreply.github.com> Date: Tue, 17 Sep 2024 01:00:50 +0900 Subject: [PATCH 5/7] fix: avoid immutable responses Co-Authored-By: Taku Amano --- runtime_tests/deno/cache.test.ts | 71 ++++++++++++++++++++++++++++++-- src/middleware/cache/index.ts | 40 ++++++++++++++---- 2 files changed, 98 insertions(+), 13 deletions(-) diff --git a/runtime_tests/deno/cache.test.ts b/runtime_tests/deno/cache.test.ts index 94f963c43..ae1c0f3a9 100644 --- a/runtime_tests/deno/cache.test.ts +++ b/runtime_tests/deno/cache.test.ts @@ -26,9 +26,10 @@ Deno.test('Should return cached response', async () => { await app.request('http://localhost') const res = await app.request('http://localhost') expect(await res.text()).toBe('Hello Hono') - expect(res.headers.get('Hono-Cached-Time')).toBeNull() + expect(res.headers.get('Hono-Cache-Expires')).toBeNull() expect(res).not.toBeNull() expect(res.status).toBe(200) + await c.delete('http://localhost') }) Deno.test( @@ -59,11 +60,73 @@ Deno.test( }) await app.request('http://localhost') - await time.tickAsync(60000) - const res = await app.request('http://localhost') + let res = await app.request('http://localhost') + + await time.tickAsync(59999) + + expect(await res.text()).toBe('Hello Hono') + expect(res.headers.get('Hono-Cache-Expires')).toBeNull() + expect(res).not.toBeNull() + expect(res.status).toBe(200) + + await time.tickAsync(1) + + res = await app.request('http://localhost') expect(await res.text()).toBe('Not Found') - expect(res.headers.get('Hono-Cached-Time')).toBeNull() + expect(res.headers.get('Hono-Cache-Expires')).toBeNull() + expect(res).not.toBeNull() + expect(res.status).toBe(200) + await c.delete('http://localhost') + } +) + +Deno.test( + { + name: 'Should return cached response if the response is immutable', + sanitizeResources: false, + }, + async () => { + const c = await caches.open('my-app') + await c.delete('http://localhost') + + let oneTime = true + let oneTimeWord = 'Hello Hono' + + const example = new Hono() + example.get('/', (c) => { + const result = oneTimeWord + oneTimeWord = 'Not Found' + return c.text(result) + }) + + const app = new Hono() + app.use( + '/', + cache({ + cacheName: 'my-app', + wait: true, + }) + ) + app.get('/', async (c) => { + const result = oneTime ? await fetch(`http://localhost:${server.addr.port}`) : 'Not Found' + oneTime = false + if (result instanceof Response) { + return result + } else { + return c.text('Not Found') + } + }) + + const server = Deno.serve({ port: 0 }, example.fetch) + + await app.request('http://localhost') + const res = await app.request('http://localhost') + expect(await res.text()).toBe('Hello Hono') + expect(res.headers.get('Hono-Cache-Expires')).toBeNull() expect(res).not.toBeNull() expect(res.status).toBe(200) + + await server.shutdown() + await c.delete('http://localhost') } ) diff --git a/src/middleware/cache/index.ts b/src/middleware/cache/index.ts index 6c04207d2..e1ec87cdb 100644 --- a/src/middleware/cache/index.ts +++ b/src/middleware/cache/index.ts @@ -102,7 +102,26 @@ export const cache = (options: { } } - const cachedTime = 'Hono-Cached-Time' + const internalCacheExpires = 'Hono-Cache-Expires' + + // Create a new response for avoiding the immutable response. + const createNewResponse = (res: Response, callbackFn: (res: Response) => void): Response => { + try { + callbackFn(res) + } catch (e) { + if (e instanceof TypeError && e.message.includes('immutable')) { + // `res` is immutable (probably a response from a fetch API), so retry with a new response. + res = new Response(res.body, { + headers: res.headers, + status: res.status, + }) + callbackFn(res) + } else { + throw e + } + } + return res + } return async function cache(c, next) { let key = c.req.url @@ -115,13 +134,14 @@ export const cache = (options: { const cache = await caches.open(cacheName) const response = await cache.match(key) if (response) { - if (options.duration) { - const duration = Number(response.headers.get(cachedTime)) + options.duration * 1000 - if (duration && duration > Date.now()) { - return new Response(response.body, response).headers.delete(cachedTime) + if (typeof options.duration === 'number'){ + const expires = Number(response.headers.get(internalCacheExpires)) + if (expires > Date.now()) { + const res = createNewResponse(response, (res) => res.headers.delete(internalCacheExpires)) + return res } } else { - return new Response(response.body, response) + return response } } @@ -130,9 +150,11 @@ export const cache = (options: { return } addHeader(c) - const res = c.res.clone() - if (options.duration) { - res.headers.set(cachedTime, String(Date.now())) + let res = c.res.clone() + + if (options.duration && options.duration > 0) { + const expiresTime = String(Date.now() + options.duration * 1000) + res = createNewResponse(res, (res) => res.headers.set(internalCacheExpires, expiresTime)) } if (options.wait) { await cache.put(key, res) From e4e61c0145da4d799e15f3fca6a6c9d2eaf231e1 Mon Sep 17 00:00:00 2001 From: ryu <114303361+ryuapp@users.noreply.github.com> Date: Tue, 17 Sep 2024 02:09:01 +0900 Subject: [PATCH 6/7] fmt --- src/middleware/cache/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/middleware/cache/index.ts b/src/middleware/cache/index.ts index e1ec87cdb..749dfe463 100644 --- a/src/middleware/cache/index.ts +++ b/src/middleware/cache/index.ts @@ -134,7 +134,7 @@ export const cache = (options: { const cache = await caches.open(cacheName) const response = await cache.match(key) if (response) { - if (typeof options.duration === 'number'){ + if (typeof options.duration === 'number') { const expires = Number(response.headers.get(internalCacheExpires)) if (expires > Date.now()) { const res = createNewResponse(response, (res) => res.headers.delete(internalCacheExpires)) From 99027e388186d7d3a53e94ae3b07a4912278fb64 Mon Sep 17 00:00:00 2001 From: ryu <114303361+ryuapp@users.noreply.github.com> Date: Tue, 17 Sep 2024 02:12:38 +0900 Subject: [PATCH 7/7] fix: import order --- runtime_tests/deno/cache.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/runtime_tests/deno/cache.test.ts b/runtime_tests/deno/cache.test.ts index ae1c0f3a9..583a90cd8 100644 --- a/runtime_tests/deno/cache.test.ts +++ b/runtime_tests/deno/cache.test.ts @@ -1,7 +1,7 @@ -import { Hono } from '../../src/hono.ts' -import { cache } from '../../src/middleware/cache/index.ts' import { expect } from '@std/expect' import { FakeTime } from '@std/testing/time' +import { Hono } from '../../src/hono.ts' +import { cache } from '../../src/middleware/cache/index.ts' Deno.test('Should return cached response', async () => { const c = await caches.open('my-app')