@tishun Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

hi @tishun !

Is it any updates about your PR? i guess it`s wating for you to take action?

hi @tishun !

Is it any updates about your PR? i guess it`s wating for you to take action?

Hey @leonovdmitry ,
Happy to see there is some interest in the change.

The current agreement with the Pivotal team is for them to try and review and push this change with the 3.5 release. Mid-January was a tentative and nonbinding ETA that I hope we will be able to meet to start work on approving this.

In the mean time if you feel there are use cases that we've missed please let me know.

I have a follow-up commit for the RedisMap that I hope to push today or tomorrow.

hi @tishun !
Is it any updates about your PR? i guess it`s wating for you to take action?

Hey @leonovdmitry , Happy to see there is some interest in the change.

The current agreement with the Pivotal team is for them to try and review and push this change with the 3.5 release. Mid-January was a tentative and nonbinding ETA that I hope we will be able to meet to start work on approving this.

In the mean time if you feel there are use cases that we've missed please let me know.

I have a follow-up commit for the RedisMap that I hope to push today or tomorrow.

@tishun thank you for feedback, yep, i was a bit surprised not to find this function in the current library version, but happy to see PR, so thanks for your work.

About use cases, I don't know if this could be part of the core, but it would be convenient to have a method for get/set ttl for only one field with the result in the form of a response code(Long) and not a list of codes(List)

Good luck and have a nice day!

Thanks for your pull request. There are some minor things to be updated. Can you please squash your commits and force-push these along with a DCO sign-off in the commit message?

Addressed the review comments and added the signoff.

Also added changes to the RedisMap, please have a look at them so we do not miss them

Thank you again for the PR @tishun.

From what I've seen so far things look pretty good, still I think there are a couple of things we need to take care of before we can actually bring the change in.

It would be nice if:

• all hash command (imperative/reactive/string) interfaces share an equivalent feature set and implement httl, hpttl, ...
• hpttl would be issued when requesting ttl with msec precision
• we use either a long, TimeUnit or Duration method signature - maybe both make sense and we put in some default methods delegating Duration to long, TimeUnit
• XX, NX, GT, LT options could be passed on. Maybe Expiration can help with that to avoid overloaded methods
• Reactive-/HashOperations interfaces have an equivalent featureset for ttl operations
• the return type of commands keeps reference to its input, so that users invoking eg. httl are not forced to memorize the input fields to map them back to the actual values returned by the command
• reactive methods which require output to be processed in order would, by declaration, lift the burden of paying close attention to enforced sequential mapping from the consuming side.

Thank you again for the PR @tishun.

Hey @christophstrobl , thanks for spending time and looking at this change!

From what I've seen so far things look pretty good, still I think there are a couple of things we need to take care of before we can actually bring the change in.

Sure! Could you please confirm my understanding of your comments (below)?

It would be nice if:

• all hash command (imperative/reactive/string) interfaces share an equivalent feature set and implement httl, hpttl, ...

Apologies, I may need some elaboration on share an equivalent feature set. I see I've missed the hpttl command in some of the interfaces, I will add it, is there something else you meant?

If there are some differences in the naming of the API methods it is because I was aiming at consistency with the existing codebase, for example BoundHashOperations.getExpire() is similar to BoundKeyOperations.getExpire(), while RedisHashCommands.hTtl() is similar to RedisKeyCommands.ttl(). All of these end up calling the HTTL command.

• hpttl would be issued when requesting ttl with msec precision

It is, actually we always are always executing HPTTL even for seconds precision. I was thinking that the HPTTL command could be used in both cases for simplification, but having spent some more time thinking on that it is not correct behavior so I will address that.

• we use either a long, TimeUnit or Duration method signature - maybe both make sense and we put in some default methods delegating Duration to long, TimeUnit

@mp911de mentioned (weird, I cant see this comment now, is it because I force pushed?) that the long, TimeUnit was an old way of doing Duration and asked that I change all usages to Duration, as this is a new API and we need not maintain two different ways to handle that. Same comment was made about using Instant instead of Date.

I seem to use TimeUnit only for httl (since I combined them both with HPTTL). When I make the change from the previous point I will completely remove the use of TimeUnit and only stick with Duration.

Does that make sense?

• XX, NX, GT, LT options could be passed on. Maybe Expiration can help with that to avoid overloaded methods

I omitted that because it was missing for key expiration too. If you think it is important I will add it to the existing APIs.
It would end up with some inconsistency, but I don't think it is a big problem in this case, as long as you are okay with it.

• Reactive-/HashOperations interfaces have an equivalent featureset for ttl operations

Oh, I seem to have missed the entire ReactiveHashOperations interface. So many of them. Will address this too.

• the return type of commands keeps reference to its input, so that users invoking eg. httl are not forced to memorize the input fields to map them back to the actual values returned by the command

Well ... this would be a rather huge inconsistency if I am getting this right.
None of the existing methods that accept multiple fields actually return the fields (or key) as a result, e.g.

class HashOperations {
 ...
 Long delete(H key, Object... hashKeys);
 List<HV> multiGet(H key, Collection<HK> hashKeys);
 ... 

Did you mean we should, instead of returning a List of results, rather return a Map of <Field, Result>?

• reactive methods which require output to be processed in order would, by declaration, lift the burden of paying close attention to enforced sequential mapping from the consuming side.

... and not do the above for Reactive methods?

@tishun thanks for getting back to us. No worries about the reactive part, yes there are a lot of places changes need to go in. We already started working on this one based on your PR to have it on main in the near future. Some of the proposed changes are already done. So no need to take action right now - I'll drop a message here once we have something for you to take a look at/comment on.

I agree that for the *commands we should stay close to the Redis API. The *operations should offer a higher level view on the response. If you want to have the raw thing, it's always possible to fall back to the commands interface.

Sorry, I did not get the delete reference or yours. There's no other value than the number of affected entries, so...
multiget has been there for 10+ years but we'd likely not do it this way if it was to be added in 2025.

@tishun thanks for getting back to us. No worries about the reactive part, yes there are a lot of places changes need to go in. We already started working on this one based on your PR to have it on main in the near future. Some of the proposed changes are already done. So no need to take action right now - I'll drop a message here once we have something for you to take a look at/comment on.

Oh, awesome, so your team will handle the changes you suggested? Let me know if I can help in any way!

Sorry, I did not get the delete reference or yours. There's no other value than the number of affected entries, so... multiget has been there for 10+ years but we'd likely not do it this way if it was to be added in 2025.

.delete() was a poor example, sorry about that. I get your point on multiget() - makes sense that we improve the API even if it is a bit inconsistent with the existing one.

Looking at expiration control over multiple keys, I wonder whether it made sense to introduce a fluent-API like interface (e.g. BoundExpirations) to channel interaction with expirations.
Bildschirmfoto 2025年02月18日 um 09 13 48

The screenshot captures IDE experience, therefore I decided for a screenshot instead of code.

Thoughts?

FWIW, we should include type hints in LettuceConnection.TypeHints to cover HEXPIRE and friends return types.

I've added some last changes to wrap this change up (documentation, polishing, tests) at https://github.com/spring-projects/spring-data-redis/tree/issue/3054. We plan to merge this PR in the next days for inclusion in the next milestone.

Thank you for your contribution. That's merged and polished now.

Oh @mp911de , FWIW I think we need to change the @since JavaDoc annotations, because I speculated we are targeting 3.4, but it seems HFE support is scheduled for 3.5?

This has already happened in 5228bc7. I'm currently catching up with conditional key expiry, see #3114. I suggest you having a look at the commits referenced in this PR (or directly at main) as we've extensively refined data structures and the expiration API.