Support the XAUTOCLAIM command. #2095
Conversation
There was a problem hiding this comment.
I tried to add the major points here which is a decent set of changes (2 methods instead of 1, to match all existing things with different return types) but since I'm away next week I was just slammed with work TODOs before out and didn't get a chance to tweak and push changes to PRs much today, sorry about that :(
|
@NickCraver just pushed an update with your suggested changes. Please let me know if there is anything else and I'll take care of it over the weekend. Thanks! |
Awesome! Eyes are super tired so I'll try and lay eyes on this fresh in the morning :) |
These are subtly needed but not obvious from the previous result processors because they always processed regardless of results. Need to ensure sanity checks in tests on how we handle a key that isn't there as well.
There was a problem hiding this comment.
@ttingen I think this is looking great - I made some tweaks based on aggregate PRs I'm seeing (wanna batch many classes up in the Results\ folder, etc.) but this was already great and I appreciate the thorough test suite added <3
@mgravell @slorello89 @Avital-Fine Could one of you please lay eyes on this as well since I'm touched and committed?
Matching this to #2095 and I'll move others here separately.
|
@NickCraver thanks, it's been fun to get back into this. Just going through your updates so I can mirror those practices in future PRs. |
|
@ttingen Definitely take some of them as stylistic and none as an issue, getting a feel for how I'd like a few things to shift as we go through a large influx of code here and it's a bit in flux and far from perfect. Your bits here were awesome, anything I can read through, understand, and tweak if wanted in 20 minutes is fantastic :) |
@NickCraver, working on `BITFIELD`/`BITFIELD_RO` and I realized that several of the new commands we've been adding are write-commands and therefore won't work on replicas, opening this to fix the command-map in Message.cs (pretty sure this is the right place but LMK if I'm offbase here) will need to push an update to #2095 #2094 to reflect this as well. Co-authored-by: Nick Craver <[email protected]>
|
@ttingen Sorry for the delay here, was trying to get some fundamentals revamped on docs and checking before merging additional commands. Thanks for all the work! |
|
@NickCraver No worries. Always glad to help. |
Added XAUTOCLAIM support as part of #2055
The XCLAIM command has two methods, StreamClaim & StreamClaimIdsOnly. Since the XAUTOCLAIM result is a bit more complex than the XCLAIM command, I opted to consolidate to a single method and single result type.
To return just the message IDs in the result, the
idsOnlyparameter should be set totrue. The caveat here is that theStreamEntryinstances will only haveIdpopulated whenidsOnlyistrue, theValuesarray will be empty. This behavior is called out in the XML docs for the method.I wasn't sure if this is the proper "feel" you want for this command or not. Let me know if you want to break it up like
StreamClaim.