From 1687d90a8cbf9af530ce75d4a06abf128844e9ab Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 8 Apr 2026 13:41:32 +0200 Subject: [PATCH] Polishing --- .../util/ConcurrentLruCache.java | 14 +++++-- .../springframework/web/ErrorResponse.java | 8 ++-- .../web/bind/WebDataBinder.java | 4 +- .../web/method/HandlerMethod.java | 16 ++++---- .../resource/ResourceHttpRequestHandler.java | 39 +++++++++++-------- 5 files changed, 50 insertions(+), 31 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/util/ConcurrentLruCache.java b/spring-core/src/main/java/org/springframework/util/ConcurrentLruCache.java index f5506f387db..ee11b0ca8f7 100644 --- a/spring-core/src/main/java/org/springframework/util/ConcurrentLruCache.java +++ b/spring-core/src/main/java/org/springframework/util/ConcurrentLruCache.java @@ -32,9 +32,11 @@ import org.jspecify.annotations.Nullable; /** * Simple LRU (Least Recently Used) cache, bounded by a specified cache capacity. + * *

This is a simplified, opinionated implementation of an LRU cache for internal * use in Spring Framework. It is inspired from * ConcurrentLinkedHashMap. + * *

Read and write operations are internally recorded in dedicated buffers, * then drained at chosen times to avoid contention. * @@ -70,6 +72,7 @@ public final class ConcurrentLruCache { private final AtomicReference drainStatus = new AtomicReference<>(DrainStatus.IDLE); + /** * Create a new cache instance with the given capacity and generator function. * @param capacity the maximum number of entries in the cache @@ -89,6 +92,7 @@ public final class ConcurrentLruCache { this.writeOperations = new WriteOperations(); } + /** * Retrieve an entry from the cache, potentially triggering generation of the value. * @param key the key to retrieve the entry for @@ -344,10 +348,13 @@ public final class ConcurrentLruCache { abstract boolean shouldDrainBuffers(boolean delayable); } + private enum CacheEntryState { + ACTIVE, PENDING_REMOVAL, REMOVED } + private record CacheEntry(V value, CacheEntryState state) { boolean isActive() { @@ -355,6 +362,7 @@ public final class ConcurrentLruCache { } } + private static final class ReadOperations { private static final int BUFFER_COUNT = detectNumberOfBuffers(); @@ -452,6 +460,7 @@ public final class ConcurrentLruCache { } } + private static final class WriteOperations { private static final int DRAIN_THRESHOLD = 16; @@ -478,11 +487,12 @@ public final class ConcurrentLruCache { task.run(); } } - } + @SuppressWarnings("serial") private static final class Node extends AtomicReference> { + final K key; @Nullable Node prev; @@ -522,7 +532,6 @@ public final class ConcurrentLruCache { @Nullable Node last; - @Nullable Node poll() { if (this.first == null) { return null; @@ -596,7 +605,6 @@ public final class ConcurrentLruCache { unlink(e); } } - } } diff --git a/spring-web/src/main/java/org/springframework/web/ErrorResponse.java b/spring-web/src/main/java/org/springframework/web/ErrorResponse.java index 6da004fe746..4908762524a 100644 --- a/spring-web/src/main/java/org/springframework/web/ErrorResponse.java +++ b/spring-web/src/main/java/org/springframework/web/ErrorResponse.java @@ -29,15 +29,15 @@ import org.springframework.http.ProblemDetail; /** * Representation of a complete RFC 9457 error response including status, - * headers, and an RFC 9457 formatted {@link ProblemDetail} body. Allows any - * exception to expose HTTP error response information. + * headers, and an RFC 9457 formatted {@link ProblemDetail} body. + * Allows any exception to expose HTTP error response information. * *

{@link ErrorResponseException} is a default implementation of this * interface and a convenient base class for other exceptions to use. * *

{@code ErrorResponse} is supported as a return value from - * {@code @ExceptionHandler} methods that render directly to the response, for example, - * by being marked {@code @ResponseBody}, or declared in an + * {@code @ExceptionHandler} methods that render directly to the response, + * for example, by being marked {@code @ResponseBody}, or declared in an * {@code @RestController} or {@code RestControllerAdvice} class. * * @author Rossen Stoyanchev diff --git a/spring-web/src/main/java/org/springframework/web/bind/WebDataBinder.java b/spring-web/src/main/java/org/springframework/web/bind/WebDataBinder.java index d659aad4f46..7f1710cfe87 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/WebDataBinder.java +++ b/spring-web/src/main/java/org/springframework/web/bind/WebDataBinder.java @@ -200,7 +200,9 @@ public class WebDataBinder extends DataBinder { * @return the resolved value, or {@code null} * @since 6.1 */ - protected @Nullable Object resolvePrefixValue(String name, Class type, BiFunction, Object> resolver) { + protected @Nullable Object resolvePrefixValue( + String name, Class type, BiFunction, @Nullable Object> resolver) { + Object value = resolver.apply(name, type); if (value == null) { String prefix = getFieldDefaultPrefix(); diff --git a/spring-web/src/main/java/org/springframework/web/method/HandlerMethod.java b/spring-web/src/main/java/org/springframework/web/method/HandlerMethod.java index 6e098307ad8..7114c2a300b 100644 --- a/spring-web/src/main/java/org/springframework/web/method/HandlerMethod.java +++ b/spring-web/src/main/java/org/springframework/web/method/HandlerMethod.java @@ -96,7 +96,7 @@ public class HandlerMethod extends AnnotatedMethod { private @Nullable HandlerMethod resolvedFromHandlerMethod; - private @Nullable HandlerMethod resolvedBeanHandlerMethod; + private volatile @Nullable HandlerMethod resolvedBeanHandlerMethod; private final String description; @@ -233,7 +233,10 @@ public class HandlerMethod extends AnnotatedMethod { this.responseStatus = annotation.code(); this.responseStatusReason = resolvedReason; if (StringUtils.hasText(this.responseStatusReason) && getMethod().getReturnType() != void.class) { - logger.warn("Return value of [" + getMethod() + "] will be ignored since @ResponseStatus 'reason' attribute is set."); + if (logger.isWarnEnabled()) { + logger.warn("Return value of [" + getMethod() + + "] will be ignored since @ResponseStatus 'reason' attribute is set."); + } } } } @@ -352,23 +355,22 @@ public class HandlerMethod extends AnnotatedMethod { *

If the {@link #getBean() handler} is not String, return the same instance. */ public HandlerMethod createWithResolvedBean() { - if (this.resolvedBeanHandlerMethod != null) { - return this.resolvedBeanHandlerMethod; + HandlerMethod resolvedBeanHandlerMethod = this.resolvedBeanHandlerMethod; + if (resolvedBeanHandlerMethod != null) { + return resolvedBeanHandlerMethod; } + // We need to resolve a bean name reference. if (!(this.bean instanceof String beanName)) { return this; } - Assert.state(this.beanFactory != null, "Cannot resolve bean name without BeanFactory"); Object handler = this.beanFactory.getBean(beanName); - Assert.notNull(handler, "No handler instance"); HandlerMethod handlerMethod = new HandlerMethod(this, handler, false); if (this.beanFactory.isSingleton(beanName)) { this.resolvedBeanHandlerMethod = handlerMethod; } - return handlerMethod; } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java index 8949b91d413..72e04b077d7 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java @@ -199,7 +199,7 @@ public class ResourceHttpRequestHandler extends WebContentGenerator /** * Configure the list of {@link ResourceResolver ResourceResolvers} to use. - *

By default {@link PathResourceResolver} is configured. If using this property, + *

By default, {@link PathResourceResolver} is configured. If using this property, * it is recommended to add {@link PathResourceResolver} as the last resolver. */ public void setResourceResolvers(@Nullable List resourceResolvers) { @@ -218,7 +218,7 @@ public class ResourceHttpRequestHandler extends WebContentGenerator /** * Configure the list of {@link ResourceTransformer ResourceTransformers} to use. - *

By default no transformers are configured for use. + *

By default, no transformers are configured for use. */ public void setResourceTransformers(@Nullable List resourceTransformers) { this.resourceTransformers.clear(); @@ -236,7 +236,7 @@ public class ResourceHttpRequestHandler extends WebContentGenerator /** * Configure the {@link ResourceHttpMessageConverter} to use. - *

By default a {@link ResourceHttpMessageConverter} will be configured. + *

By default, a {@link ResourceHttpMessageConverter} will be configured. * @since 4.3 */ public void setResourceHttpMessageConverter(@Nullable ResourceHttpMessageConverter messageConverter) { @@ -253,7 +253,7 @@ public class ResourceHttpRequestHandler extends WebContentGenerator /** * Configure the {@link ResourceRegionHttpMessageConverter} to use. - *

By default a {@link ResourceRegionHttpMessageConverter} will be configured. + *

By default, a {@link ResourceRegionHttpMessageConverter} will be configured. * @since 4.3 */ public void setResourceRegionHttpMessageConverter(@Nullable ResourceRegionHttpMessageConverter messageConverter) { @@ -294,7 +294,7 @@ public class ResourceHttpRequestHandler extends WebContentGenerator /** * Specify the CORS configuration for resources served by this handler. - *

By default this is not set in which allows cross-origin requests. + *

By default, this is not set in which allows cross-origin requests. */ public void setCorsConfiguration(CorsConfiguration corsConfiguration) { this.corsConfiguration = corsConfiguration; @@ -523,7 +523,6 @@ public class ResourceHttpRequestHandler extends WebContentGenerator // For very general mappings (for example, "/") we need to check 404 first Resource resource = getResource(request); if (resource == null) { - logger.debug("Resource not found"); throw new NoResourceFoundException(HttpMethod.valueOf(request.getMethod()), request.getRequestURI(), getPath(request)); } @@ -539,10 +538,12 @@ public class ResourceHttpRequestHandler extends WebContentGenerator prepareResponse(response); // Header phase - String eTagValue = (getEtagGenerator() != null ? getEtagGenerator().apply(resource) : null); + String etagValue = (getEtagGenerator() != null ? getEtagGenerator().apply(resource) : null); long lastModified = (isUseLastModified() ? resource.lastModified() : -1); - if (new ServletWebRequest(request, response).checkNotModified(eTagValue, lastModified)) { - logger.trace("Resource not modified"); + if (new ServletWebRequest(request, response).checkNotModified(etagValue, lastModified)) { + if (logger.isTraceEnabled()) { + logger.trace("Resource not modified: " + resource); + } return; } @@ -553,8 +554,7 @@ public class ResourceHttpRequestHandler extends WebContentGenerator // Content phase ServletServerHttpResponse outputMessage = new ServletServerHttpResponse(response); if (request.getHeader(HttpHeaders.RANGE) == null) { - Assert.state(this.resourceHttpMessageConverter != null, "Not initialized"); - + Assert.state(this.resourceHttpMessageConverter != null, "Converter not initialized"); if (HttpMethod.HEAD.matches(request.getMethod())) { this.resourceHttpMessageConverter.addDefaultHeaders(outputMessage, resource, mediaType); outputMessage.flush(); @@ -564,7 +564,7 @@ public class ResourceHttpRequestHandler extends WebContentGenerator } } else { - Assert.state(this.resourceRegionHttpMessageConverter != null, "Not initialized"); + Assert.state(this.resourceRegionHttpMessageConverter != null, "Converter not initialized"); ServletServerHttpRequest inputMessage = new ServletServerHttpRequest(request); try { List httpRanges = inputMessage.getHeaders().getRange(); @@ -584,14 +584,21 @@ public class ResourceHttpRequestHandler extends WebContentGenerator String path = getPath(request); path = processPath(path); if (ResourceHandlerUtils.shouldIgnoreInputPath(path) || isInvalidPath(path)) { + if (logger.isDebugEnabled()) { + logger.debug("Ignoring invalid resource path [" + path + "]"); + } return null; } - Assert.state(this.resolverChain != null, "ResourceResolverChain not initialized."); - Assert.state(this.transformerChain != null, "ResourceTransformerChain not initialized."); - + Assert.state(this.resolverChain != null, "ResourceResolverChain not initialized"); + Assert.state(this.transformerChain != null, "ResourceTransformerChain not initialized"); Resource resource = this.resolverChain.resolveResource(request, path, getLocations()); - if (resource != null) { + if (resource == null) { + if (logger.isDebugEnabled()) { + logger.debug("Resource not found for path [" + path + "]"); + } + } + else { resource = this.transformerChain.transform(request, resource); } return resource;