sidewinder icon indicating copy to clipboard operation
sidewinder copied to clipboard

Add support for Redis SortedSets

Open waikikamoukow opened this issue 2 years ago • 4 comments

This PR adds basic support for SortedSets, implemented using the SortedSets feature of Redis.

Currently the main thing missing is an equivalent implementation for MemoryStore. I don't believe there is a clear equivalent, so I think the two options are to either implement something custom for this, or to add a dependency that provides something we could use. In particular, the js-sdsl library provides OrderedSet that I think would be suitable if adding this dependency is reasonable.

@sinclairzx81 do you have a preference here? Let me know if you have a preferred direction and/or other suggestions for this feature.

waikikamoukow avatar Jun 21 '23 20:06 waikikamoukow

@sinclairzx81 I looked into using ioredis-mock for the MemoryStore but realised that actually MemoryStore isn't required when using ioredis-mock because you can plug it straight into the regular RedisStore. With this in mind, is MemoryStore needed?

waikikamoukow avatar Jul 03 '23 14:07 waikikamoukow

@waikikamoukow Hi!

I looked into using ioredis-mock for the MemoryStore but realised that actually MemoryStore isn't required when using ioredis-mock because you can plug it straight into the regular RedisStore. With this in mind, is MemoryStore needed?

Yeah, let's keep the MemoryStore for now, this just to provide a bit of flexibility to be able to run a different mock library under the MemoryStore if we need to (such as https://www.npmjs.com/package/redis-mock). It's possible other mocking libraries may not be compatible with the Store interface, so we can iron out any disparities by hiding any differences behind the MemoryStore implementation (this with some consideration to ioredis-mock's ability to run multiple / disposable instances)

import { RedisMock } from 'ioredis-mock' // example import

export class MemoryStore implements Store {
  readonly #store: RedisMock  // private (and potentially swappable at this level)
  constructor() {
     #store = new RedisMock() // new instance please !! :)
  }
  //
  // Store implementation here
  //
  public disconnect() {
     #store.dispose() // free any data / gc collect
  }
}

sinclairzx81 avatar Jul 03 '23 17:07 sinclairzx81

@sinclairzx81 I've update MemoryStore to use ioredis-mock. This has made it basically a clone of RedisStore since it implements the same interface as ioredis.

It doesn't appear to have a dispose function or provide a RedisMock type definition (at least as far as I could see). I'm not sure if your example was just meant to be indicative or if I'm missing something with ioredis-mock.

waikikamoukow avatar Jul 06 '23 13:07 waikikamoukow

@waikikamoukow Hi,

I've update MemoryStore to use ioredis-mock. This has made it basically a clone of RedisStore since it implements the same interface as ioredis.

Yup, that's fine :)

Have pushed a small update to support multiple instances of the RedisMock (I remembered that if you specify a different connectionName and port, the library will create different pools of data, which is interesting). Have written a couple of tests around that (for Instances and SetOptions here), just need a couple tests asserting the SortedSetOptions behaviours on zrange and this functionality should be good go.

Once those tests are in, can publish on 0.12.8 :) Cheers!

sinclairzx81 avatar Jul 08 '23 03:07 sinclairzx81