Improve cache auto-configuration for Redis #10944
Conversation
Expose key prefix, TTL and null value settings for spring-data-redis' RedisCacheConfiguration in Spring .properties/yml configuration files. Example: spring.cache.redis.ttl=PT15M spring.cache.redis.keyPrefix=foo spring.cache.redis.useKeyPrefix=false spring.cache.redis.cacheNullValues=false Fixes gs-10795
spring-projects-issues
added
the
status: waiting-for-triage
|
Addresses this |
snicoll
changed the title
| public static class Redis { | ||
|
|
||
| /** | ||
| * The TTL for entries. By default, entries do not expire, a value of zero |
There was a problem hiding this comment.
Configuration key descriptions do not start with The, A, etc. Please look at existing keys for inspiration
| * The TTL for entries. By default, entries do not expire, a value of zero | ||
| * indicates an eternal entry, and the given value will be converted to seconds.. | ||
| * | ||
| * @see Duration#parse(CharSequence) |
There was a problem hiding this comment.
We don't make such @see link here. We use the javadoc to infer the description. Can you please remove them all?
| private boolean cacheNullValues = true; | ||
|
|
||
| /** | ||
| * The non-null Redis key prefix to use instead of the default one. |
There was a problem hiding this comment.
I am not sure what you mean by "non-null" here.
| return this; | ||
| } | ||
|
|
||
| public Optional<String> getKeyPrefix() { |
There was a problem hiding this comment.
Configuration properties must be JavaBean properties (the type must match) and we don't support Optional here.
| @@ -57,13 +59,38 @@ | |||
|
|
|||
| @Bean | |||
| public RedisCacheManager cacheManager(RedisConnectionFactory redisConnectionFactory) { | |||
|
|
|||
There was a problem hiding this comment.
Please refrain from making unnecessary formatting changes.
| RedisCacheManagerBuilder builder = RedisCacheManager | ||
| .builder(redisConnectionFactory); | ||
| .builder(redisConnectionFactory).cacheDefaults(getConfiguration()); | ||
|
|
| List<String> cacheNames = this.cacheProperties.getCacheNames(); | ||
| if (!cacheNames.isEmpty()) { | ||
| builder.initialCacheNames(new LinkedHashSet<>(cacheNames)); | ||
| } | ||
|
|
snicoll
added
priority: low
status: waiting-for-feedback
* Remove unnecessary formatting changes * Discontinue use of Optional for Redis property configuration * Remove @see tags * Change verbiage for property descriptions Fixes gs-10795
|
Thanks so much for the detailed feedback, @snicoll; I hope that the follow-up commit has addressed your concerns! I'm happy to be able to give back in some small way to a project that I've used so much throughout my career. |
snicoll
removed
the
status: waiting-for-feedback
* pr/10944: Polish "Improve cache auto-configuration for Redis" Improve cache auto-configuration for Redis Respond to MR feedback Improve spring-boot-autoconfigure for Redis caches
|
@ryonday Thanks for the feedback. They did partially and I had an extra polish pass. It's now merged in |
|
Thanks so much for the polish pass, I have a few more small PR ideas and it helps to have a template for the preferred conventions and style to make it easier on reviewers! Cheers! |
Expose key prefix, TTL and null value settings for spring-data-redis' RedisCacheConfiguration in Spring .properties/yml configuration files.
Example:
Fixes gs-10795