The RESTDataSource class does not take Vary headers into account by default, and overriding cacheKeyFor doesn't seem sufficient

[dchambers]

The documentation for the overridable cacheKeyFor method within RESTDataSource is as follows:

  // By default, we use the full request URL as the cache key.
  // You can override this to remove query parameters or compute a cache key in any way that makes sense.
  // For example, you could use this to take Vary header fields into account.
  // Although we do validate header fields and don't serve responses from cache when they don't match,
  // new reponses overwrite old ones with different vary header fields.
  protected cacheKeyFor(request: Request): string {
    return request.url;
  }

which suggests that implementors could get the Vary header fields to be taken into account by including them within the cache key. However, given that the cache is checked before the response is available, we clearly can't act based on the value of the Vary header at this point?

There is also a cacheOptionsFor method that does receive both the request and the response, and I could use this to store the vary fields for future reference, but at present the only metadata from this which are stored along with the cached entity is the overridden ttl, if one is provided:

await this.keyValueCache.set(cacheKey, entry, {
  ttl,
});

I decided to try and work around these issues by creating a VaryCachingRESTDataSource class that would maintain its own vary field for all cached entities, plus also maintain the original request headers used the first time an entity is cached so that we can still make use of that cached value in the future once we know what what the Vary headers are, if any.

Unfortunately, this still didn't work for me, and I've so far been unable to have caching from servers that return Vary headers. Is there something I'm overlooking here, or is complete support for the Vary header not currently possible as things stand?

[dchambers]

In case it's of interest, this is the code I have so far for the VaryCachingRESTDataSource class I wrote:

import { RESTDataSource } from 'apollo-datasource-rest'
import flatten from 'lodash/flatten'

const pluckHeaders = (headers, keyName) =>
  Array.from(headers).reduce(
    (acc, [key, value]) => (key === keyName ? [...acc, value] : acc),
    []
  )

const pluckHeader = (headers, keyName) => {
  const pluckedHeaders = pluckHeaders(headers, keyName)
  return pluckedHeaders[pluckedHeaders.length - 1]
}

const createCacheKey = (url, headers, varyFields = []) => {
  const varyFieldsStr = varyFields.map(h =>
    pluckHeader(headers, h.toLowerCase())
  )
  return varyFieldsStr.length === 0 ? url : url + '::' + varyFieldsStr
}

class VaryCachingRESTDataSource extends RESTDataSource {
  static varyFields = {}
  static origRequestHeaders = {}

  constructor() {
    super()
    console.log('>>> new VaryCachingRESTDataSource()')
  }

  cacheKeyFor(request) {
    console.log('>>> cacheKeyFor (BEGIN)')
    const varyFields = this.constructor.varyFields[request.url]
    const cacheKey = createCacheKey(request.url, request.headers, varyFields)

    if (varyFields) {
      const origRequestHeaders = this.constructor.origRequestHeaders[
        request.url
      ]
      const origCacheKey = createCacheKey(
        request.url,
        origRequestHeaders,
        varyFields
      )

      if (cacheKey === origCacheKey) {
        console.log('>>> not returning generated cacheKey', cacheKey)
        console.log(
          '>>> returning orig cacheKey',
          createCacheKey(request.url, origRequestHeaders)
        )
        return createCacheKey(request.url, origRequestHeaders)
      }
    } else {
      this.constructor.origRequestHeaders[request.url] = request.headers
    }
    console.log('>>> cacheKeyFor', cacheKey)

    return cacheKey
  }

  cacheOptionsFor(response, request) {
    console.log('>>> cacheOptionsFor (BEGIN)')
    const varyHeaders = pluckHeaders(response.headers, 'vary')
    console.log('>>> varyHeaders', varyHeaders)
    const varyFields = flatten(varyHeaders.map(h => h.split(/\s*,\s*/)))
    console.log('>>> varyFields', varyFields)

    this.constructor.varyFields[request.url] = varyFields

    return super.cacheOptionsFor
      ? super.cacheOptionsFor(response, request)
      : null
  }

  flushCache() {
    console.log('>>> flushCache (BEGIN)')
    this.httpCache.keyValueCache.store.reset()
    this.constructor.varyFields = {}
    this.constructor.origRequestHeaders = {}
    console.log('>>> flushCache (END)')
  }
}

export default VaryCachingRESTDataSource

[dchambers]

So I've just this minute got this working. It turns out that public has to be used if the client sends an Authorization header. I'll close this ticket shortly once I figure out whether I needed all of the crazy code I ended up writing...

[dchambers]

Closing as I've now verified that the VaryCachingRESTDataSource class is needed to get caching working properly (maybe can be simplified though), since without it does cache return cached data as long as the same user keeps returning, but each time the user changes it has to make a new request, which it then caches again.

[dchambers]

In case it helps anyone else, this is what I ended up with:

import { RESTDataSource } from 'apollo-datasource-rest'

const pluckHeaders = (headers, keyName) =>
  Array.from(headers).reduce(
    (acc, [key, value]) => (key === keyName ? [...acc, value] : acc),
    []
  )

const pluckHeader = (headers, keyName) => {
  const pluckedHeaders = pluckHeaders(headers, keyName)
  return pluckedHeaders[pluckedHeaders.length - 1]
}

const createCacheKey = (url, headers, varyFields = []) => {
  const varyFieldsStr = varyFields.map(h =>
    pluckHeader(headers, h.toLowerCase())
  )
  return varyFieldsStr.length === 0 ? url : url + '::' + varyFieldsStr
}

/**
 * The `RESTDataSource` class offers caching, but will not cache resource
 * variations as defined by the `Vary` header. Instead, it supports the caching
 * of a single resource per URL, and will then return this cached resource only
 * while clients continue to request the same variety, but will overwrite the
 * cache with a different resource variety as soon as that's requested.
 */
class VaryCachingRESTDataSource extends RESTDataSource {
  static varyFields = {}
  static origRequestHeaders = {}

  constructor() {
    super()
  }

  cacheKeyFor(request) {
    const varyFields = this.constructor.varyFields[request.url]
    const cacheKey = createCacheKey(request.url, request.headers, varyFields)

    if (varyFields) {
      const origRequestHeaders = this.constructor.origRequestHeaders[
        request.url
      ]
      const origCacheKey = createCacheKey(
        request.url,
        origRequestHeaders,
        varyFields
      )

      if (cacheKey === origCacheKey) {
        return createCacheKey(request.url, origRequestHeaders)
      }
    } else {
      this.constructor.origRequestHeaders[request.url] = request.headers
    }

    return cacheKey
  }

  cacheOptionsFor(response, request) {
    const varyHeaders = pluckHeaders(response.headers, 'vary')
    const varyFields = varyHeaders.map(h => h.split(/\s*,\s*/)).flat()

    this.constructor.varyFields[request.url] = varyFields

    return super.cacheOptionsFor
      ? super.cacheOptionsFor(response, request)
      : null
  }

  flushCache() {
    this.constructor.varyFields = {}
    this.constructor.origRequestHeaders = {}
    const keyValueCache = this.httpCache.keyValueCache

    if (keyValueCache.store) {
      keyValueCache.store.reset()
    } else if (keyValueCache.client) {
      keyValueCache.client.flushdb()
    } else {
      throw new Error(
        'The `flushCache` method only currently works with the built-in and redis caches'
      )
    }
  }
}

export default VaryCachingRESTDataSource