Skip to content

Commit

Permalink
Polishing
Browse files Browse the repository at this point in the history
  • Loading branch information
jhoeller committed Mar 4, 2024
1 parent 24759a7 commit e9110c0
Show file tree
Hide file tree
Showing 21 changed files with 97 additions and 124 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2021 the original author or authors.
* Copyright 2002-2024 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 @@ -29,12 +29,10 @@
import org.springframework.lang.Nullable;

/**
* Abstract implementation of {@link JCacheOperationSource} that caches attributes
* Abstract implementation of {@link JCacheOperationSource} that caches operations
* for methods and implements a fallback policy: 1. specific target method;
* 2. declaring method.
*
* <p>This implementation caches attributes by method after they are first used.
*
* @author Stephane Nicoll
* @author Juergen Hoeller
* @since 4.1
Expand All @@ -43,35 +41,36 @@
public abstract class AbstractFallbackJCacheOperationSource implements JCacheOperationSource {

/**
* Canonical value held in cache to indicate no caching attribute was
* found for this method and we don't need to look again.
* Canonical value held in cache to indicate no cache operation was
* found for this method, and we don't need to look again.
*/
private static final Object NULL_CACHING_ATTRIBUTE = new Object();
private static final Object NULL_CACHING_MARKER = new Object();


protected final Log logger = LogFactory.getLog(getClass());

private final Map<MethodClassKey, Object> cache = new ConcurrentHashMap<>(1024);
private final Map<MethodClassKey, Object> operationCache = new ConcurrentHashMap<>(1024);


@Override
@Nullable
public JCacheOperation<?> getCacheOperation(Method method, @Nullable Class<?> targetClass) {
MethodClassKey cacheKey = new MethodClassKey(method, targetClass);
Object cached = this.cache.get(cacheKey);
Object cached = this.operationCache.get(cacheKey);

if (cached != null) {
return (cached != NULL_CACHING_ATTRIBUTE ? (JCacheOperation<?>) cached : null);
return (cached != NULL_CACHING_MARKER ? (JCacheOperation<?>) cached : null);
}
else {
JCacheOperation<?> operation = computeCacheOperation(method, targetClass);
if (operation != null) {
if (logger.isDebugEnabled()) {
logger.debug("Adding cacheable method '" + method.getName() + "' with operation: " + operation);
}
this.cache.put(cacheKey, operation);
this.operationCache.put(cacheKey, operation);
}
else {
this.cache.put(cacheKey, NULL_CACHING_ATTRIBUTE);
this.operationCache.put(cacheKey, NULL_CACHING_MARKER);
}
return operation;
}
Expand All @@ -84,7 +83,7 @@ private JCacheOperation<?> computeCacheOperation(Method method, @Nullable Class<
return null;
}

// The method may be on an interface, but we need attributes from the target class.
// The method may be on an interface, but we need metadata from the target class.
// If the target class is null, the method will be unchanged.
Method specificMethod = AopUtils.getMostSpecificMethod(method, targetClass);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2024 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 @@ -212,10 +212,8 @@ protected String generateDefaultCacheName(Method method) {
for (Class<?> parameterType : parameterTypes) {
parameters.add(parameterType.getName());
}

return method.getDeclaringClass().getName()
+ '.' + method.getName()
+ '(' + StringUtils.collectionToCommaDelimitedString(parameters) + ')';
return method.getDeclaringClass().getName() + '.' + method.getName() +
'(' + StringUtils.collectionToCommaDelimitedString(parameters) + ')';
}

private int countNonNull(Object... instances) {
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-2024 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 @@ -46,6 +46,7 @@ public class BeanFactoryJCacheOperationSourceAdvisor extends AbstractBeanFactory
* Set the cache operation attribute source which is used to find cache
* attributes. This should usually be identical to the source reference
* set on the cache interceptor itself.
* @see JCacheInterceptor#setCacheOperationSource
*/
public void setCacheOperationSource(JCacheOperationSource cacheOperationSource) {
this.pointcut.setCacheOperationSource(cacheOperationSource);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2024 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 @@ -34,7 +34,7 @@ public interface JCacheOperationSource {
* Return the cache operations for this method, or {@code null}
* if the method contains no <em>JSR-107</em> related metadata.
* @param method the method to introspect
* @param targetClass the target class (may be {@code null}, in which case
* @param targetClass the target class (can be {@code null}, in which case
* the declaring class of the method must be used)
* @return the cache operation for this method, or {@code null} if none found
*/
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-2024 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 @@ -24,7 +24,7 @@
import org.springframework.util.ObjectUtils;

/**
* A Pointcut that matches if the underlying {@link JCacheOperationSource}
* A {@code Pointcut} that matches if the underlying {@link JCacheOperationSource}
* has an operation for a given method.
*
* @author Stephane Nicoll
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2021 the original author or authors.
* Copyright 2002-2024 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 @@ -32,31 +32,27 @@
import org.springframework.util.ClassUtils;

/**
* Abstract implementation of {@link CacheOperation} that caches attributes
* Abstract implementation of {@link CacheOperationSource} that caches operations
* for methods and implements a fallback policy: 1. specific target method;
* 2. target class; 3. declaring method; 4. declaring class/interface.
*
* <p>Defaults to using the target class's caching attribute if none is
* associated with the target method. Any caching attribute associated with
* the target method completely overrides a class caching attribute.
* <p>Defaults to using the target class's declared cache operations if none are
* associated with the target method. Any cache operations associated with
* the target method completely override any class-level declarations.
* If none found on the target class, the interface that the invoked method
* has been called through (in case of a JDK proxy) will be checked.
*
* <p>This implementation caches attributes by method after they are first
* used. If it is ever desirable to allow dynamic changing of cacheable
* attributes (which is very unlikely), caching could be made configurable.
*
* @author Costin Leau
* @author Juergen Hoeller
* @since 3.1
*/
public abstract class AbstractFallbackCacheOperationSource implements CacheOperationSource {

/**
* Canonical value held in cache to indicate no caching attribute was
* found for this method and we don't need to look again.
* Canonical value held in cache to indicate no cache operation was
* found for this method, and we don't need to look again.
*/
private static final Collection<CacheOperation> NULL_CACHING_ATTRIBUTE = Collections.emptyList();
private static final Collection<CacheOperation> NULL_CACHING_MARKER = Collections.emptyList();


/**
Expand All @@ -71,14 +67,14 @@ public abstract class AbstractFallbackCacheOperationSource implements CacheOpera
* <p>As this base class is not marked Serializable, the cache will be recreated
* after serialization - provided that the concrete subclass is Serializable.
*/
private final Map<Object, Collection<CacheOperation>> attributeCache = new ConcurrentHashMap<>(1024);
private final Map<Object, Collection<CacheOperation>> operationCache = new ConcurrentHashMap<>(1024);


/**
* Determine the caching attribute for this method invocation.
* <p>Defaults to the class's caching attribute if no method attribute is found.
* Determine the cache operations for this method invocation.
* <p>Defaults to class-declared metadata if no method-level metadata is found.
* @param method the method for the current invocation (never {@code null})
* @param targetClass the target class for this invocation (may be {@code null})
* @param targetClass the target class for this invocation (can be {@code null})
* @return {@link CacheOperation} for this method, or {@code null} if the method
* is not cacheable
*/
Expand All @@ -90,21 +86,21 @@ public Collection<CacheOperation> getCacheOperations(Method method, @Nullable Cl
}

Object cacheKey = getCacheKey(method, targetClass);
Collection<CacheOperation> cached = this.attributeCache.get(cacheKey);
Collection<CacheOperation> cached = this.operationCache.get(cacheKey);

if (cached != null) {
return (cached != NULL_CACHING_ATTRIBUTE ? cached : null);
return (cached != NULL_CACHING_MARKER ? cached : null);
}
else {
Collection<CacheOperation> cacheOps = computeCacheOperations(method, targetClass);
if (cacheOps != null) {
if (logger.isTraceEnabled()) {
logger.trace("Adding cacheable method '" + method.getName() + "' with attribute: " + cacheOps);
logger.trace("Adding cacheable method '" + method.getName() + "' with operations: " + cacheOps);
}
this.attributeCache.put(cacheKey, cacheOps);
this.operationCache.put(cacheKey, cacheOps);
}
else {
this.attributeCache.put(cacheKey, NULL_CACHING_ATTRIBUTE);
this.operationCache.put(cacheKey, NULL_CACHING_MARKER);
}
return cacheOps;
}
Expand All @@ -129,7 +125,7 @@ private Collection<CacheOperation> computeCacheOperations(Method method, @Nullab
return null;
}

// The method may be on an interface, but we need attributes from the target class.
// The method may be on an interface, but we need metadata from the target class.
// If the target class is null, the method will be unchanged.
Method specificMethod = AopUtils.getMostSpecificMethod(method, targetClass);

Expand Down Expand Up @@ -163,19 +159,19 @@ private Collection<CacheOperation> computeCacheOperations(Method method, @Nullab


/**
* Subclasses need to implement this to return the caching attribute for the
* Subclasses need to implement this to return the cache operations for the
* given class, if any.
* @param clazz the class to retrieve the attribute for
* @return all caching attribute associated with this class, or {@code null} if none
* @param clazz the class to retrieve the cache operations for
* @return all cache operations associated with this class, or {@code null} if none
*/
@Nullable
protected abstract Collection<CacheOperation> findCacheOperations(Class<?> clazz);

/**
* Subclasses need to implement this to return the caching attribute for the
* Subclasses need to implement this to return the cache operations for the
* given method, if any.
* @param method the method to retrieve the attribute for
* @return all caching attribute associated with this method, or {@code null} if none
* @param method the method to retrieve the cache operations for
* @return all cache operations associated with this method, or {@code null} if none
*/
@Nullable
protected abstract Collection<CacheOperation> findCacheOperations(Method method);
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-2024 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 @@ -54,7 +54,7 @@ default boolean isCandidateClass(Class<?> targetClass) {
* Return the collection of cache operations for this method,
* or {@code null} if the method contains no <em>cacheable</em> annotations.
* @param method the method to introspect
* @param targetClass the target class (may be {@code null}, in which case
* @param targetClass the target class (can be {@code null}, in which case
* the declaring class of the method must be used)
* @return all cache operations for this method, or {@code null} if none found
*/
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-2024 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 @@ -28,15 +28,15 @@

/**
* A {@code Pointcut} that matches if the underlying {@link CacheOperationSource}
* has an attribute for a given method.
* has an operation for a given method.
*
* @author Costin Leau
* @author Juergen Hoeller
* @author Sam Brannen
* @since 3.1
*/
@SuppressWarnings("serial")
class CacheOperationSourcePointcut extends StaticMethodMatcherPointcut implements Serializable {
final class CacheOperationSourcePointcut extends StaticMethodMatcherPointcut implements Serializable {

@Nullable
private CacheOperationSource cacheOperationSource;
Expand Down Expand Up @@ -78,7 +78,7 @@ public String toString() {
* {@link ClassFilter} that delegates to {@link CacheOperationSource#isCandidateClass}
* for filtering classes whose methods are not worth searching to begin with.
*/
private class CacheOperationSourceClassFilter implements ClassFilter {
private final class CacheOperationSourceClassFilter implements ClassFilter {

@Override
public boolean matches(Class<?> clazz) {
Expand All @@ -88,14 +88,15 @@ public boolean matches(Class<?> clazz) {
return (cacheOperationSource == null || cacheOperationSource.isCandidateClass(clazz));
}

@Nullable
private CacheOperationSource getCacheOperationSource() {
return cacheOperationSource;
}

@Override
public boolean equals(@Nullable Object other) {
return (this == other || (other instanceof CacheOperationSourceClassFilter that &&
ObjectUtils.nullSafeEquals(cacheOperationSource, that.getCacheOperationSource())));
ObjectUtils.nullSafeEquals(getCacheOperationSource(), that.getCacheOperationSource())));
}

@Override
Expand All @@ -105,9 +106,8 @@ public int hashCode() {

@Override
public String toString() {
return CacheOperationSourceClassFilter.class.getName() + ": " + cacheOperationSource;
return CacheOperationSourceClassFilter.class.getName() + ": " + getCacheOperationSource();
}

}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 the original author or authors.
* Copyright 2002-2024 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 @@ -98,8 +98,8 @@ public AsyncAnnotationBeanPostProcessor() {
* applying the corresponding default if a supplier is not resolvable.
* @since 5.1
*/
public void configure(
@Nullable Supplier<Executor> executor, @Nullable Supplier<AsyncUncaughtExceptionHandler> exceptionHandler) {
public void configure(@Nullable Supplier<Executor> executor,
@Nullable Supplier<AsyncUncaughtExceptionHandler> exceptionHandler) {

this.executor = executor;
this.exceptionHandler = exceptionHandler;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
*/
class DataBinderConstructTests {


@Test
void dataClassBinding() {
MapValueResolver valueResolver = new MapValueResolver(Map.of("param1", "value1", "param2", "true"));
Expand Down Expand Up @@ -78,7 +77,7 @@ void dataClassBindingWithMissingParameter() {
assertThat(bindingResult.getFieldValue("param3")).isNull();
}

@Test // gh-31821
@Test // gh-31821
void dataClassBindingWithNestedOptionalParameterWithMissingParameter() {
MapValueResolver valueResolver = new MapValueResolver(Map.of("param1", "value1"));
DataBinder binder = initDataBinder(NestedDataClass.class);
Expand Down
Loading

0 comments on commit e9110c0

Please sign in to comment.