Skip to content
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

fix: cache fixes #3871

Merged
merged 8 commits into from
Nov 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion lib/cache/memory-cache-store.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,17 @@ class MemoryCacheStore {
this.#maxCount = opts.maxCount
}

if (opts.maxSize !== undefined) {
if (
typeof opts.maxSize !== 'number' ||
!Number.isInteger(opts.maxSize) ||
opts.maxSize < 0
) {
throw new TypeError('MemoryCacheStore options.maxSize must be a non-negative integer')
}
this.#maxSize = opts.maxSize
}

if (opts.maxEntrySize !== undefined) {
if (
typeof opts.maxEntrySize !== 'number' ||
Expand Down Expand Up @@ -126,7 +137,7 @@ class MemoryCacheStore {
store.#size += entry.size
store.#count += 1

if (store.#size > store.#maxEntrySize || store.#count > store.#maxCount) {
if (store.#size > store.#maxSize || store.#count > store.#maxCount) {
for (const [key, entries] of store.#entries) {
for (const entry of entries.splice(0, entries.length / 2)) {
store.#size -= entry.size
Expand Down
44 changes: 30 additions & 14 deletions lib/cache/sqlite-cache-store.js
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ class SqliteCacheStore {
const body = []
const store = this

const writable = new Writable({
return new Writable({
write (chunk, encoding, callback) {
if (typeof chunk === 'string') {
chunk = Buffer.from(chunk, encoding)
Expand All @@ -265,9 +265,6 @@ class SqliteCacheStore {
final (callback) {
store.prune()

/**
* @type {SqliteStoreValue | undefined}
*/
const existingValue = store.#findValue(key, true)
if (existingValue) {
// Updating an existing response, let's overwrite it
Expand Down Expand Up @@ -304,8 +301,6 @@ class SqliteCacheStore {
callback()
}
})

return writable
}

/**
Expand Down Expand Up @@ -371,14 +366,14 @@ class SqliteCacheStore {
*/
#findValue (key, canBeExpired = false) {
const url = this.#makeValueUrl(key)
const { headers, method } = key

/**
* @type {SqliteStoreValue[]}
*/
const values = this.#getValuesQuery.all(url, key.method)
const values = this.#getValuesQuery.all(url, method)

if (values.length === 0) {
// No responses, let's just return early
return undefined
}

Expand All @@ -391,16 +386,14 @@ class SqliteCacheStore {
let matches = true

if (value.vary) {
if (!key.headers) {
// Request doesn't have headers so it can't fulfill the vary
// requirements no matter what, let's return early
if (!headers) {
return undefined
}

value.vary = JSON.parse(value.vary)
const vary = JSON.parse(value.vary)

for (const header in value.vary) {
if (key.headers[header] !== value.vary[header]) {
for (const header in vary) {
if (headerValueEquals(headers[header], vary[header])) {
matches = false
break
}
Expand All @@ -416,6 +409,29 @@ class SqliteCacheStore {
}
}

/**
* @param {string|string[]|null|undefined} lhs
* @param {string|string[]|null|undefined} rhs
* @returns {boolean}
*/
function headerValueEquals (lhs, rhs) {
if (Array.isArray(lhs) && Array.isArray(rhs)) {
if (lhs.length !== rhs.length) {
return false
}

for (let i = 0; i < lhs.length; i++) {
if (rhs.includes(lhs[i])) {
return false
}
}

return true
}

return lhs === rhs
}

/**
* @param {Buffer[]} buffers
* @returns {string[]}
Expand Down
23 changes: 6 additions & 17 deletions lib/handler/cache-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class CacheHandler extends DecoratorHandler {
#writeStream

/**
* @param {import('../../types/cache-interceptor.d.ts').default.CacheOptions} opts
* @param {import('../../types/cache-interceptor.d.ts').default.CacheHandlerOptions} opts
* @param {import('../../types/cache-interceptor.d.ts').default.CacheKey} cacheKey
* @param {import('../../types/dispatcher.d.ts').default.DispatchHandlers} handler
*/
Expand Down Expand Up @@ -106,17 +106,11 @@ class CacheHandler extends DecoratorHandler {
const headers = util.parseHeaders(parsedRawHeaders)

const cacheControlHeader = headers['cache-control']
const isCacheFull = typeof this.#store.isFull !== 'undefined'
? this.#store.isFull
: false

if (
!cacheControlHeader ||
isCacheFull
) {
if (!cacheControlHeader) {
// Don't have the cache control header or the cache is full
return downstreamOnHeaders()
}

const cacheControlDirectives = parseCacheControlHeader(cacheControlHeader)
if (!canCacheResponse(statusCode, headers, cacheControlDirectives)) {
return downstreamOnHeaders()
Expand Down Expand Up @@ -236,10 +230,7 @@ class CacheHandler extends DecoratorHandler {
* @param {import('../util/cache.js').CacheControlDirectives} cacheControlDirectives
*/
function canCacheResponse (statusCode, headers, cacheControlDirectives) {
if (
statusCode !== 200 &&
statusCode !== 307
) {
if (statusCode !== 200 && statusCode !== 307) {
return false
}

Expand Down Expand Up @@ -309,7 +300,7 @@ function determineStaleAt (now, headers, cacheControlDirectives) {
if (headers.expire && typeof headers.expire === 'string') {
// https://www.rfc-editor.org/rfc/rfc9111.html#section-5.3
const expiresDate = new Date(headers.expire)
if (expiresDate instanceof Date && !isNaN(expiresDate)) {
if (expiresDate instanceof Date && Number.isFinite(expiresDate.valueOf())) {
return now + (Date.now() - expiresDate.getTime())
}
}
Expand Down Expand Up @@ -391,9 +382,7 @@ function stripNecessaryHeaders (rawHeaders, parsedRawHeaders, cacheControlDirect
strippedHeaders.length -= offset
}

return strippedHeaders
? util.encodeRawHeaders(strippedHeaders)
: rawHeaders
return strippedHeaders ? util.encodeRawHeaders(strippedHeaders) : rawHeaders
}

module.exports = CacheHandler
37 changes: 25 additions & 12 deletions lib/util/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,38 @@ function makeCacheKey (opts) {
throw new Error('opts.origin is undefined')
}

/**
* @type {import('../../types/cache-interceptor.d.ts').default.CacheKey}
*/
const cacheKey = {
/** @type {Record<string, string[] | string>} */
let headers
if (opts.headers == null) {
headers = {}
} else if (typeof opts.headers[Symbol.iterator] === 'function') {
headers = {}
for (const x of opts.headers) {
if (!Array.isArray(x)) {
throw new Error('opts.headers is not a valid header map')
}
const [key, val] = x
if (typeof key !== 'string' || typeof val !== 'string') {
throw new Error('opts.headers is not a valid header map')
}
headers[key] = val
}
} else if (typeof opts.headers === 'object') {
headers = opts.headers
} else {
throw new Error('opts.headers is not an object')
}

return {
origin: opts.origin.toString(),
method: opts.method,
path: opts.path,
headers: opts.headers
headers
}

return cacheKey
}

/**
* @param {any} value
* @param {any} key
*/
function assertCacheKey (key) {
if (typeof key !== 'object') {
Expand Down Expand Up @@ -299,10 +316,6 @@ function assertCacheStore (store, name = 'CacheStore') {
throw new TypeError(`${name} needs to have a \`${fn}()\` function`)
}
}

if (typeof store.isFull !== 'undefined' && typeof store.isFull !== 'boolean') {
throw new TypeError(`${name} needs a isFull getter with type boolean or undefined, current type: ${typeof store.isFull}`)
}
}
/**
* @param {unknown} methods
Expand Down
9 changes: 9 additions & 0 deletions types/cache-interceptor.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ export default CacheHandler
declare namespace CacheHandler {
export type CacheMethods = 'GET' | 'HEAD' | 'OPTIONS' | 'TRACE'

export interface CacheHandlerOptions {
store: CacheStore
ronag marked this conversation as resolved.
Show resolved Hide resolved
mcollina marked this conversation as resolved.
Show resolved Hide resolved
}

export interface CacheOptions {
store?: CacheStore

Expand Down Expand Up @@ -75,6 +79,11 @@ declare namespace CacheHandler {
*/
maxSize?: number

/**
* @default Infinity
*/
maxEntrySize?: number

errorCallback?: (err: Error) => void
}

Expand Down
2 changes: 1 addition & 1 deletion types/dispatcher.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ declare namespace Dispatcher {
/** Default: `null` */
body?: string | Buffer | Uint8Array | Readable | null | FormData;
/** Default: `null` */
headers?: IncomingHttpHeaders | string[] | Iterable<[string, string | string[] | undefined]> | null;
headers?: Record<string, string | string[]> | IncomingHttpHeaders | string[] | Iterable<[string, string | string[] | undefined]> | null;
/** Query string params to be embedded in the request URL. Default: `null` */
query?: Record<string, any>;
/** Whether the requests can be safely retried or not. If `false` the request won't be sent until all preceding requests in the pipeline have completed. Default: `true` if `method` is `HEAD` or `GET`. */
Expand Down
Loading