jaredwray / keyv

Simple key-value storage with support for multiple backends
https://keyv.org
MIT License
2.61k stars 143 forks source link

Upgrading keyv to 3.x lead to strange and confusing key prefixing behaviour #1174

Closed klippx closed 6 days ago

klippx commented 1 week ago

Describe the bug

Trying to upgrade to @keyv/redis to 3.0.1 but now all my keys are being set with a weird prefix that I do not want. Why this breaking change, and no way to get out of it?

My usage:

import { KeyvAdapter } from '@apollo/utils.keyvadapter'; # 4.0.0 - not doing any prefixing, just adds a dataloader
import KeyvRedis from '@keyv/redis'; # 3.0.1
import Keyv from 'keyv'; # 5.1.2

const createClient = () => {
  const keyvRedis = new KeyvRedis(redis, { useRedisSets: false });
  const keyv = new Keyv({
    store: keyvRedis,
    namespace: 'myNamespace',
  })
  const adapter = new KeyvAdapter(keyv);
  return adapter
}
const client = createClient();
await client.set('fqc:uuid1', 'world');

Before: { "myNamespace:fqc:uuid1": "world" }

After: { "sets:namespace:myNamespace:myNamespace:fqc:uuid1": "world" }

This code seems to be the problem:

  _getKeyName = (key) => {
    if (!this.opts.useRedisSets) {
      return `sets:${this._getNamespace()}:${key}`;
    }
    return key;
  };

This is so confusing, shouldn't it be the other way around? If I am opting in to useRedisSets THEN prefix...?

And you also add the prefix twice! A) once from this._getNamespace() in @keyv/redis and B) it's already in the key from keyv itself because it is also prefixing.

On the third hand, if I pass useRedisSets: true then I the keys become correct but then I opt in to using redis sets which we DO NOT WANT because we have proven memory problems with it with the way we use it, and I start to get keys like namespace:myNamespace which I do not expect or need.

So: 1) We need to have option useRedisSets: false - no set behaviour 2) We want to keep the old key names or the key names will just be long for no reason 3) No double key prefixing

jaredwray commented 6 days ago

@klippx - we are building a new redis adapter now and hope to have it out in the next 2-3 weeks that will do a couple of things:

  1. It will not default to using redisSets
  2. It will handle prefix Key much better
  3. It will be based on node-redis

Going to close this and reference it in the main thread. Thanks @klippx!