Add spring.data.redis.lettuce.read-from property

[nosan]

#42576

[wilkinsona]

Thanks, @nosan. Unfortunately, when I looked at this yesterday, I mistakenly thought that ReadFrom was an enum. The changes proposed here demonstrate that's not the case. Sorry for any wasted effort here but I'm not sure that I like having three properties (read-from.type, read-from.cidr-notations, and read-from.pattern). A lot of the time, only type will be needed and the other two are only needed when type has a particular value.

I wonder if we could still use a single property. The values that map directly to a ReadFrom constant are straightforward but we'd need some special handling for subnet and regex. Perhaps we could support something like subnet:notationOne,notationTwo,notationThree and regex:pattern? This support could perhaps be implemented as a @ConfigurationPropertiesBinding converter and the property could be typed as a ReadFrom instance.

I don't want to waste any more of your time (apologies again for the time possibly already wasted) so please don't do anything about the above until we've had a chance to discuss this as a team.

[nosan]

Thanks, @wilkinsona.
No problem at all.

[nosan]

@Bean
@ConfigurationPropertiesBinding
StringToReadFromConverter stringToReadFromConverter() {
   return new StringToReadFromConverter();
}
	

static class StringToReadFromConverter implements GenericConverter {

		private static final Set<ConvertiblePair> CONVERTIBLE_TYPES;

		static {
			CONVERTIBLE_TYPES = Set.of(new ConvertiblePair(String.class, ReadFrom.class));
		}

		@Override
		public Set<ConvertiblePair> getConvertibleTypes() {
			return CONVERTIBLE_TYPES;
		}

		@Override
		public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
			if (source == null) {
				return null;
			}
			String value = source.toString();
			int index = value.indexOf(':');
			if (index != -1) {
				String type = value.substring(0, index);
				if (type.equalsIgnoreCase("subnet")) {
					return ReadFrom.subnet(StringUtils.commaDelimitedListToStringArray(value.substring(index + 1)));
				}
				if (type.equalsIgnoreCase("regex")) {
					return ReadFrom.regex(Pattern.compile(value.substring(index + 1)));
				}
			}
			return ReadFrom.valueOf(value);
		}

	}
	

in that case, it will be possible to use the following syntax:

spring.data.redis.lettuce.read-from=regex:192.*
spring.data.redis.lettuce.read-from=subnet:192.12.128.0/32,192.168.255.255/32
spring.data.redis.lettuce.read-from=any

[nosan]

Is it ok to use ReadFrom field type from io.lettuce.core in RedisProperties.Lettuce properties?

/**
* Defines from which Redis nodes data is read.
*/
private ReadFrom readFrom;

[nosan]

perhaps it would be beneficial to reach out to someone from the Lettuce team to explore the possibility of adding support for the regex and subnet types in the ReadFrom.valueOf(...) method

At the moment:

 //ReadFrom valueOf(String name)
 
        if (name.equalsIgnoreCase("subnet")) {
            throw new IllegalArgumentException("subnet must be created via ReadFrom#subnet");
        }

        if (name.equalsIgnoreCase("regex")) {
            throw new IllegalArgumentException("regex must be created via ReadFrom#regex");
        }
        

[wilkinsona]

perhaps it would be beneficial to reach out to someone from the Lettuce team

I think that's a good idea. WDYT, @mp911de, about supporting subnet:… and regex:… or similar syntax with ReadFrom.valueOf to ease property-based configuration?

[mp911de]

I like the idea a lot, especially flexibility introduced with regex: and subnet: variants is pretty exciting.

[wilkinsona]

Thanks, @mp911de. I've opened redis/lettuce#3013 for further consideration.

[nosan]

@wilkinsona
redis/lettuce#3016 It's been merged and will be available in Lettuce 6.5.0.RELEASE.
Meanwhile, I've updated the PR. Could I kindly ask you to review it once again, please?

I have the following questions:

• How can I correctly map kebab-case values? ReadFrom.valueOf supports case-insensitive names (e.g., upstreampreferred). I used readFrom.replaceAll("-", "")
• What values should I use for regex and subnet in additional-spring-configuration-metadata.json?

[philwebb]

How can I correctly map kebab-case values? ReadFrom.valueOf supports case-insensitive names (e.g., upstreampreferred). I used readFrom.replaceAll("-", "")

I think we can use the same logic as in LenientObjectToEnumConverterFactory. I've done that in bea7704

What values should I use for regex and subnet in additional-spring-configuration-metadata.json?

I think the ones you've added are correct.

[nosan]

Thank you, @philwebb
I cherry-pick your commit 👍

[wilkinsona]

I wonder if we should hold off on documenting regex: and subnet: in the metadata until we're using Lettuce 6.5 by default. The feature's still useful before then and if someone using Lettuce 6.4 wants to configure a regex or subnet read from, they can always use a customizer till Lettuce 6.5 is out and they've overridden the version or we've upgraded.

[nosan]

I wonder if we should hold off on documenting regex: and subnet: in the metadata until we're using Lettuce 6.5 by default. The feature's still useful before then and if someone using Lettuce 6.4 wants to configure a regex or subnet read from, they can always use a customizer till Lettuce 6.5 is out and they've overridden the version or we've upgraded.

It depends on when it gets merged :) If you think it's ready to be merged, I can remove subnet: and regex: from the metadata.

[nosan]

Another approach could be to temporarily add support for regex: and subnet: in Spring Boot, and then remove it once version 6.5.0 is released.

[wilkinsona]

We discussed this today and decided that we're going to err on the side of caution and wait till Lettuce 6.5 is out before merging anything. Hopefully that'll be in the Boot 3.5 timeframe.

[nosan]

Thank you for your feedback, @wilkinsona.
Feel free to reach out if any further work is required on this.

[nosan]

6.5.0.RELEASE has been released. I bumped the version to 6.5.0 and uncommented tests for regex and subnet.

[philwebb]

Thanks @nosan. We're going to wait for 3.5 to merge this one. The version bump is a bit risky post RC

[nosan]

Thanks, @philwebb.
Should I revert bump version commit, meanwhile?

I know that you have a separate workflow for dependencies upgrade. So maybe, this version bump shouldn’t be included here.

[philwebb]

No it's fine. We'll just deal with that nearer the time.