Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore cache operations if generated key is an empty Optional #34258

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -48,6 +48,7 @@
* @author Chris Beams
* @author Phillip Webb
* @author Stephane Nicoll
* @author Yanming Zhou
*/
public abstract class AbstractCacheAnnotationTests {

Expand Down Expand Up @@ -281,6 +282,19 @@ protected void testEvictAllEarly(CacheableService<?> service) {
assertThat(r4).isNotSameAs(r2);
}

protected void testEvictWithOptionalKey(CacheableService<?> service) {
Object o1 = new Object();
Object r1 = service.cache(o1);

service.evictWithOptionalKey(null);
Object r2 = service.cache(o1);
assertThat(r2).isSameAs(r1);

service.evictWithOptionalKey(o1);
Object r3 = service.cache(o1);
assertThat(r3).isNotSameAs(r1);
}

protected void testConditionalExpression(CacheableService<?> service) {
Object r1 = service.conditional(4);
Object r2 = service.conditional(4);
Expand All @@ -305,6 +319,31 @@ protected void testConditionalExpressionSync(CacheableService<?> service) {
assertThat(r4).isSameAs(r3);
}


protected void testOptionalKey(CacheableService<?> service) {
Object r1 = service.optional(4);
Object r2 = service.optional(4);

assertThat(r2).isNotSameAs(r1);

Object r3 = service.optional(3);
Object r4 = service.optional(3);

assertThat(r4).isSameAs(r3);
}

protected void testOptionalKeySync(CacheableService<?> service) {
Object r1 = service.optionalSync(4);
Object r2 = service.optionalSync(4);

assertThat(r2).isNotSameAs(r1);

Object r3 = service.optionalSync(3);
Object r4 = service.optionalSync(3);

assertThat(r4).isSameAs(r3);
}

protected void testUnlessExpression(CacheableService<?> service) {
Cache cache = this.cm.getCache("testCache");
cache.clear();
Expand Down Expand Up @@ -423,6 +462,18 @@ protected void testConditionalCacheUpdate(CacheableService<?> service) {
assertThat(Integer.parseInt(cache.get(three).get().toString())).isEqualTo(three);
}

protected void testOptionalCacheUpdate(CacheableService<?> service) {
int one = 1;
int three = 3;

Cache cache = this.cm.getCache("testCache");
assertThat(Integer.parseInt(service.optionalUpdate(one).toString())).isEqualTo(one);
assertThat(cache.get(one)).isNull();

assertThat(Integer.parseInt(service.optionalUpdate(three).toString())).isEqualTo(three);
assertThat(Integer.parseInt(cache.get(three).get().toString())).isEqualTo(three);
}

protected void testMultiCache(CacheableService<?> service) {
Object o1 = new Object();
Object o2 = new Object();
Expand Down Expand Up @@ -610,11 +661,21 @@ void testEvictWithKeyEarly() {
testEvictWithKeyEarly(this.cs);
}

@Test
void testEvictWithOptionalKey() {
testEvictWithOptionalKey(this.cs);
}

@Test
void testConditionalExpression() {
testConditionalExpression(this.cs);
}

@Test
void testOptionalKey() {
testOptionalKey(this.cs);
}

@Test
void testConditionalExpressionSync() {
testConditionalExpressionSync(this.cs);
Expand Down Expand Up @@ -819,6 +880,16 @@ void testClassConditionalUpdate() {
testConditionalCacheUpdate(this.ccs);
}

@Test
void testOptionalUpdate() {
testOptionalCacheUpdate(this.cs);
}

@Test
void testClassOptionalUpdate() {
testOptionalCacheUpdate(this.ccs);
}

@Test
void testMultiCache() {
testMultiCache(this.cs);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2023 the original author or authors.
* Copyright 2002-2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -19,6 +19,8 @@
import java.io.IOException;
import java.util.concurrent.atomic.AtomicLong;

import org.jspecify.annotations.Nullable;

import org.springframework.cache.annotation.CacheEvict;
import org.springframework.cache.annotation.CachePut;
import org.springframework.cache.annotation.Cacheable;
Expand All @@ -32,6 +34,7 @@
* @author Costin Leau
* @author Phillip Webb
* @author Stephane Nicoll
* @author Yanming Zhou
*/
@Cacheable("testCache")
public class AnnotatedClassCacheableService implements CacheableService<Object> {
Expand Down Expand Up @@ -73,6 +76,16 @@ public Object conditionalSync(int field) {
return null;
}

@Override
public Object optional(int field) {
return null;
}

@Override
public Object optionalSync(int field) {
return null;
}

@Override
@Cacheable(cacheNames = "testCache", unless = "#result > 10")
public Object unless(int arg) {
Expand Down Expand Up @@ -107,6 +120,11 @@ public void evictAllEarly(Object arg1) {
throw new RuntimeException("exception thrown - evict should still occur");
}

@Override
@CacheEvict(cacheNames = "testCache", key = "T(java.util.Optional).ofNullable(#name != null ? #p0 : null)")
public void evictWithOptionalKey(@Nullable Object arg1) {
}

@Override
@Cacheable(cacheNames = "testCache", key = "#p0")
public Object key(Object arg1, Object arg2) {
Expand Down Expand Up @@ -167,6 +185,12 @@ public Object conditionalUpdate(Object arg) {
return arg;
}

@Override
@CachePut(cacheNames = "testCache", key = "T(java.util.Optional).ofNullable(#p0 == 3 ? #arg : null)")
public Object optionalUpdate(Object arg) {
return arg;
}

@Override
public Object nullValue(Object arg1) {
nullInvocations.incrementAndGet();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -16,6 +16,8 @@

package org.springframework.cache.config;

import org.jspecify.annotations.Nullable;

/**
* Copy of the shared {@code CacheableService}: necessary
* due to issues with Gradle test fixtures and AspectJ configuration
Expand All @@ -26,6 +28,7 @@
* @author Costin Leau
* @author Phillip Webb
* @author Stephane Nicoll
* @author Yanming Zhou
*/
public interface CacheableService<T> {

Expand All @@ -47,10 +50,16 @@ public interface CacheableService<T> {

void evictAllEarly(Object arg1);

void evictWithOptionalKey(@Nullable Object arg1);

T conditional(int field);

T conditionalSync(int field);

T optional(int field);

T optionalSync(int field);

T unless(int arg);

T key(Object arg1, Object arg2);
Expand All @@ -63,7 +72,9 @@ public interface CacheableService<T> {

T update(Object arg1);

T conditionalUpdate(Object arg2);
T conditionalUpdate(Object arg);

T optionalUpdate(Object arg);

Number nullInvocations();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -19,6 +19,8 @@
import java.io.IOException;
import java.util.concurrent.atomic.AtomicLong;

import org.jspecify.annotations.Nullable;

import org.springframework.cache.annotation.CacheEvict;
import org.springframework.cache.annotation.CachePut;
import org.springframework.cache.annotation.Cacheable;
Expand All @@ -34,6 +36,7 @@
* @author Costin Leau
* @author Phillip Webb
* @author Stephane Nicoll
* @author Yanming Zhou
*/
public class DefaultCacheableService implements CacheableService<Long> {

Expand Down Expand Up @@ -94,6 +97,11 @@ public void evictAllEarly(Object arg1) {
throw new RuntimeException("exception thrown - evict should still occur");
}

@Override
@CacheEvict(cacheNames = "testCache", key = "T(java.util.Optional).ofNullable(#p0 != null ? #p0 : null)")
public void evictWithOptionalKey(@Nullable Object arg1) {
}

@Override
@Cacheable(cacheNames = "testCache", condition = "#p0 == 3")
public Long conditional(int classField) {
Expand All @@ -106,6 +114,18 @@ public Long conditionalSync(int classField) {
return this.counter.getAndIncrement();
}

@Override
@Cacheable(cacheNames = "testCache", key = "T(java.util.Optional).ofNullable(#p0 == 3 ? #p0 : null)")
public Long optional(int classField) {
return this.counter.getAndIncrement();
}

@Override
@Cacheable(cacheNames = "testCache", sync = true, key = "T(java.util.Optional).ofNullable(#p0 == 3 ? #p0 : null)")
public Long optionalSync(int classField) {
return this.counter.getAndIncrement();
}

@Override
@Cacheable(cacheNames = "testCache", unless = "#result > 10")
public Long unless(int arg) {
Expand Down Expand Up @@ -172,6 +192,12 @@ public Long conditionalUpdate(Object arg) {
return Long.valueOf(arg.toString());
}

@Override
@CachePut(cacheNames = "testCache", key = "T(java.util.Optional).ofNullable(#p0 == 3 ? #arg : null)")
public Long optionalUpdate(Object arg) {
return Long.valueOf(arg.toString());
}

@Override
@Cacheable("testCache")
public Long nullValue(Object arg1) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 the original author or authors.
* Copyright 2002-2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -36,6 +36,7 @@
* @author Costin Leau
* @author Stephane Nicoll
* @author Sam Brannen
* @author Yanming Zhou
* @since 3.1
* @see CacheConfig
*/
Expand Down Expand Up @@ -67,6 +68,7 @@
* Spring Expression Language (SpEL) expression for computing the key dynamically.
* <p>Default is {@code ""}, meaning all method parameters are considered as a key,
* unless a custom {@link #keyGenerator} has been set.
* <p>Cache is not involved if evaluated result is an empty {@link java.util.Optional}.
* <p>The SpEL expression evaluates against a dedicated context that provides the
* following meta-data:
* <ul>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 the original author or authors.
* Copyright 2002-2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -44,6 +44,7 @@
* @author Phillip Webb
* @author Stephane Nicoll
* @author Sam Brannen
* @author Yanming Zhou
* @since 3.1
* @see CacheConfig
*/
Expand Down Expand Up @@ -75,6 +76,7 @@
* Spring Expression Language (SpEL) expression for computing the key dynamically.
* <p>Default is {@code ""}, meaning all method parameters are considered as a key,
* unless a custom {@link #keyGenerator} has been set.
* Cache is not involved if evaluated result is an empty {@link java.util.Optional}.
* <p>The SpEL expression evaluates against a dedicated context that provides the
* following meta-data:
* <ul>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2023 the original author or authors.
* Copyright 2002-2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -52,6 +52,7 @@
* @author Phillip Webb
* @author Stephane Nicoll
* @author Sam Brannen
* @author Yanming Zhou
* @since 3.1
* @see CacheConfig
*/
Expand Down Expand Up @@ -90,6 +91,7 @@

/**
* Spring Expression Language (SpEL) expression for computing the key dynamically.
* <p>Cache is not involved if evaluated result is an empty {@link java.util.Optional}.
* <p>Default is {@code ""}, meaning all method parameters are considered as a key,
* unless a custom {@link #keyGenerator} has been configured.
* <p>The SpEL expression evaluates against a dedicated context that provides the
Expand Down
Loading
Loading