Redis Client: fix NPE when constructing XPendingSummary #45687
Conversation
Certain fields that were deemed to be mandatory can in fact be `null`, as demonstrated in newly added tests. Also, this commit fixes translating `XTrimArgs` to the actual argument array in case `MAXLEN` is set to `0`. That is, in fact, a valid value, as demonstrated by the newly added tests and also by inspecting Redis source code [1]. [1] https://github.com/redis/redis/blob/7.2.4/src/t_stream.c#L941
Status for workflow
|
There was a problem hiding this comment.
I added a small question but in general LGTM.
I will let @cescoffier approve and merge though.
| consumers.put(nested.get(0).toString(), nested.get(1).toLong()); | ||
| if (r.get(3) != null) { | ||
| for (Response nested : r.get(3)) { | ||
| consumers.put(nested.get(0).toString(), nested.get(1).toLong()); |
There was a problem hiding this comment.
In there we are sure the values are not null? If so, LGTM!
There was a problem hiding this comment.
I'm pretty sure that both the keys and the values are never null, but not completely, to be honest.
There was a problem hiding this comment.
me neither - it's not really said on the command documentation. I think it's good to go.
| consumers.put(nested.get(0).toString(), nested.get(1).toLong()); | ||
| if (r.get(3) != null) { | ||
| for (Response nested : r.get(3)) { | ||
| consumers.put(nested.get(0).toString(), nested.get(1).toLong()); |
There was a problem hiding this comment.
me neither - it's not really said on the command documentation. I think it's good to go.
Certain fields that were deemed to be mandatory can in fact be
null, as demonstrated in newly added tests.Also, this commit fixes translating
XTrimArgsto the actual argument array in caseMAXLENis set to0. That is, in fact, a valid value, as demonstrated by the newly added tests and also by inspecting Redis source code [1].[1] https://github.com/redis/redis/blob/7.2.4/src/t_stream.c#L941
Fixes #45682