Allow access to pattern and channel, not only payload

[skabbes]

I would like to use redis, and graphql-redis-subscriptions to track page viewers for an internal app. The details of that are probably a bit much for a Github Issue, so let me try to condense it down a similar problem that exhibits he same issues.

Context
Users publish strings to the key users:{userId}:auth when they perform auth events - login, logout, change password etc.

redis:6379> publish users:1:auth resetpassword
redis:6379> publish users:2:auth login
redis:6379> publish users:2:auth logout
redis:6379> publish users:1:auth newpassword
... etc

I would like to build a graphql subscription for admins that shows auth events in realtime, so I would like to psubscribe to users:*:auth. I can do that easily with

asyncIterator('users:*:auth', { pattern: true });

But then I have the issue with building the graphQL Response, using Payload Manipulation. RedisPubSub discards the channel that redis sends in the pmessage callback and therefore it is impossible to extract the userId out of that channel to build the graphQL response.

I know there are probably infinite ways to work around this in the redis data model, but it feels a little wrong to fix this in that layer, especially since redis and ioredis support this out of the box. In addition, my original problem relies on Keyspace Notificaitons in which I have no control over the message payload at all.

Suggested Solution
I can think of a few solutions that might work, and I'll provide them here in case that's helpful - but ultimately the maintainers will know best. If there's one that you think works best, let me know and I'd be happy to help write the code too.

1. Add RedisPubSub.asyncIteratorRaw, which iterates values that all have the form { message, channel, pattern } I think this is probably the best and easiest option for maintaining backwards compatibility

2. Add an extra context arg to deserializer.

parsedMessage = this.deserializer ? this.deserializer(message, { channel, pattern }) : JSON.parse(message, this.reviver);

3.. Add a new callback to RedisPubSub that allows changing that asyncIterator's value.

parsedMessage = this.deserializer ? this.deserializer(message) : JSON.parse(message, this.reviver);
const iterValue = this.transformIteratorValue({ message: parsedMessage, channel, pattern });
//...
for (const subId of subscribers) {
  const [, listener] = this.subscriptionMap[subId];
  listener(iterValue);
}

[davidyaha]

Hey @skabbes
Thanks for this issue and sorry I failed to maintain this package for quite a while.

So if I understand correctly, your use case can be generalized as "need to psubscribe on non graphQL mutation events".
If this is the case I think that solution number 2 that you've suggested would work best as that message might also be protobuf message or anything else that relies on the channel to marshal a JSON message.

Does that sound right?
If so, I'd gladly merge a PR with that solution.
Thanks again!

[skabbes]

need to psubscribe on non graphQL mutation events

This might be true. Since the code base I work on isn't GraphQL everywhere yet, so I don't know enough about "graphQL mutation events" and how they work with subscriptions. But maybe to clarify a bit further...

I'd say it as "I need to psubscribe to something where the channel it was delivered to is holds important context, and this library hides that from me"

I'd still say that 1 is the better solution, since you can pick per subscription whether you care about the channel or not. 95% of the time, I could care less about the channel, since its already in the subscription args, or the data is in the message payload.

But the code is relatively trivial either way, so maybe I'll put up 2 options for you - remove any ambiguity of understanding between these 2 solutions.

[davidyaha]

The reason I don't particularly love option 1 is because asyncIterator is more of a data structure rather than a function.
I do agree with your take where you might not want the channel for most subscriptions but that is probably the case for the deserializer function.

You mainly the deserializer when your message could not be parsed with simple JSON.parse so if you deal with many different subscriptions where some of them have JSON messages and some of them use a different syntax (e.g. base64 strings of binary constructs) you might need the context regardless.

So in a way, I think solution 2 was supposed to be in the original implementation.
Also, your code for the deserializer would be much simpler and will hold the logic which is "if this is a message from channel A, create this JSON object, else use JSON.parse"

[skabbes]

Ya, fair enough on the asyncIterator comment - I guess I sort of read it more as createAsyncIterator (the function that creates the datastructure). I tried to do a solution that used the options parameter to asyncIterator to possibly solve your concerns there. Maybe take a look here to see what you think.

The reason that I don't like the deserializer solution is that it is global to the entire pubsub system, you'll end up with a ball of spaghetti trying to sometimes use it and sometimes not. I can imagine that you could have 2 different graphQL subscriptions that use the same pattern, and receives message to the same channel - its still not enough context to know how to create the graphQL object (since it would be dependent on the subscription, not only pattern or channel).

I do agree that maybe the "deserializer with context" is needed regardless, for cases of people (not me) using protobuf or binary data. And in fact it is existed before, I probably wouldn't have even opened this discussion :)

Either way, either one (or both) of these solutions unblock my issue. So I'll defer to you.