feat(microservices): enable wildcards in redis microservice patterns #10359
Conversation
Pull Request Test Coverage Report for Build 8941bc69-2bf3-49ef-bed3-cadb50eb8c86
💛 - Coveralls |
| @@ -64,11 +62,11 @@ export class ServerRedis extends Server implements CustomTransportStrategy { | |||
| } | |||
|
|
|||
| public bindEvents(subClient: Redis, pubClient: Redis) { | |||
| subClient.on(MESSAGE_EVENT, this.getMessageHandler(pubClient).bind(this)); | |||
| subClient.on('pmessage', this.getMessageHandler(pubClient).bind(this)); | |||
There was a problem hiding this comment.
Correct me if I'm wrong but I think this would introduce a breaking change.
Can we use "pmessage" and psubscribe if explicitly enabled by the user?
There was a problem hiding this comment.
If psubscribe is being used, we must use 'pmessage' as well.
psubscribe is an extended version of subscribe, as far as I understand. The differences between subscribe and psubscribe are the wildcards and the third argument given back to the 'message'/'pmessage' event.
I do not believe there to be any breaking changes, where do you see one?
There was a problem hiding this comment.
If someone used project:* as a pattern, then it was interpreted as project:* not project:{wildcard - put anything here}. This is a breaking change.
Likewise, if someone inherited from the built-in ServerRedis class and had tests spying on "on('message')", then these tests would fail now.
There was a problem hiding this comment.
Yes, that is true. Would you rather like to ask the user explicitly whether they want to enable this feature than to make it a breaking change?
I don't know the usual way to go about this. Nor do I know how big the impact would be of a breaking change.
There was a problem hiding this comment.
I'd say exposing a flag (adding a new property to the redis transporter options) would be ideal. Someone could just explicitly turn wildcards on (e.g., { wildcards: true })
There was a problem hiding this comment.
I changed the PR and tested the system now so it uses an explicit option.
BTW, I found a bug in the test file of the redis server, where it changes the options by calling (server as any).options.options instead of (server as any).options.
The redis microservice now makes use of `psubscribe` and `pmessage` when the `wildcards` option is enabled in the options of the microservice, which makes it possible to use wildcards as specified by the Redis documentation. Closes nestjs#10344
tijsmoree
force-pushed
the
redis-microservice-psubscribe
branch
from
fad4577 to
39d6e8e
Compare
|
Any updates on this PR? Having wildcard support would be really useful. |
The redis microservice now makes use of
psubscribeandpmessageif thewildcardsoption is enabled in the options of the microservice, which makes it possible to use wildcards as specified by the Redis documentation.Closes #10344
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
The current setup uses
subscribeandmessagewhen listening for patterns in a Redis microservice, while usingpsubscribeandpmessagewould make it possible to use wildcards in those patterns as prescribed by the Redis documentation.Issue Number: #10344
What is the new behavior?
The change makes it possible to use wildcards in the Redis microservice patterns by enabling the
wildcardsoption of the microservice.Does this PR introduce a breaking change?
Other information