fqc prefix is added and there's no way to disable it

[yuvalhvim]

https://github.com/apollographql/apollo-server/blob/3bfe4e5c505c9ba00de17eace9091645fc4022d0/packages/apollo-server-plugin-response-cache/src/ApolloServerPluginResponseCache.ts#L165

We want to use redis for caching, and we want different processes to have access to only their keys. We use redis permissions for prefixes, so process_1 could access keys process_1*.

I thought adding generateCacheKey was nice and could fix it all, but the fqc prefix is always there.

Can we just have total control over keys (in case one wants it, of course)?

[trevor-scheer]

Seems like a reasonable request and worth reconsidering as a part of the recent generateCacheKey work. I'd suppose a couple options would work for your use case then:

1. Don't apply a prefix when generateCacheKey is provided
2. Make prefix configurable or disable-able via an additional configuration option

I think I prefer 1 since it seems sensible in the case that you're overriding the cache key anyway. Unfortunately, it's a breaking change in the API which I'd rather avoid.

Interested in @glasser's opinion on this.

[glasser]

Well, the important part here is that this allows you to configure just one cache that is used for multiple features such as the response cache, REST DataSources, APQs, etc. It's important that these don't overlap with each other!

You could probably wrap your own cache implementation around the Redis implementation which adds the process key before the fqc key...

[yuvalhvim]

You could probably wrap your own cache implementation around the Redis implementation which adds the process key before the fqc key...

I can't tho. The plugin wraps whatever cache implementation it gets with PrefixingKeyValueCache which is initialized with fqc prefix. I have no way of adding a prefix before fqc.

And even if it was possible, I think it's a bit weird to have keys in the format of prefix_which_makes_sense:fqc:...

[glasser]

Sure, I'm not saying it's the greatest idea, but you could make yourself an "UnprefixingCache" that's similar to https://github.com/apollographql/apollo-utils/blob/main/packages/keyValueCache/src/PrefixingKeyValueCache.ts but removes a prefix from keys instead of adding it. Then you can do something like new ApolloServerPluginResponseCache({cache: new UnprefixingCache({prefixToRemove: 'fqc:', cache: myRedisCache})}).

That said, I think there's an argument to be made that the prefixing feature is specific to the concept of inheriting a shared cache from the overall ApolloServer. ie, one could argue that this (code from version-4):

      const cache = new PrefixingKeyValueCache(
        options.cache ?? outerRequestContext.server.cache,
        'fqc:',
      );

should instead be

      const cache = options.cache ?? new PrefixingKeyValueCache(
        outerRequestContext.server.cache,
        'fqc:',
      );

This would be a backwards-incompatible change that I wouldn't be excited about adding to AS3 but we could make the new @apollo/server-plugin-response-cache in AS4 have that change. We would do the same thing for APQs. @trevor-scheer thoughts?

[yuvalhvim]

I'm all for the change you suggested. But does that mean current versions won't get a fix? 🥺

Maybe add an optional parameter like shouldAddPrefix to Options which defaults to true?

[glasser]

Another approach that makes it harder to accidentally share a cache's namespace across multiple features (which could lead to serious security vulnerabilities if you're able to poison one feature's cache by using an unrelated feature) is to let users explicitly declare that their cache is unshared, and let features skip the prefixing in that case. This would also be fully backwards-compatible.

ApolloServerPluginResponseCache({
  cache: new UnsharedCache({ cache }),
})

IsolatedCache is in utils.keyvaluecache

new function in utils.keyvaluecache that's like prefixingUnlessUnsharedKeyValueCache(cache, prefix)

[glasser]

(and we could make ApolloServer throw if you pass an UnsharedCache to its top-level cache option)

[glasser]

Because of the risk of security issues if a cache is accidentally shared, I think I prefer my UnsharedCache suggestion over the "prefixing only happens when using the ApolloServer main cache" suggestion. That makes this not a backwards-incompatible change, so I'm going to remove this from the AS4 milestone. Happy to take patches to apollo-utils (@apollo/utils.keyvaluecache) and apollo-server implementing the above suggestion.

@trevor-scheer if you disagree and think the other solution is better we can implement this after the first AS4 RC.