From 03133630cb442fd2ee5b775002a1d995fded06df Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 20 Jun 2017 18:08:42 +0200 Subject: [PATCH] Missing @Nullable annotations in WebFlux, in particular around locale resolution Issue: SPR-15036 Issue: SPR-15540 --- .../context/i18n/LocaleContextHolder.java | 42 +++++++++++++++++-- .../reactive/DefaultPathSegmentContainer.java | 10 ++--- .../server/reactive/DefaultRequestPath.java | 3 +- .../AcceptHeaderLocaleContextResolver.java | 2 +- .../server/i18n/LocaleContextResolver.java | 5 +-- .../util/pattern/PathPatternComparator.java | 10 ++++- .../config/WebFluxConfigurationSupport.java | 20 +++++---- .../DefaultRenderingResponseBuilder.java | 3 +- .../web/reactive/handler/PathMatchResult.java | 14 ++++--- .../reactive/handler/PathPatternRegistry.java | 14 +++---- .../resource/ResourceUrlProvider.java | 7 +++- .../view/ViewResolutionResultHandler.java | 4 +- .../view/freemarker/FreeMarkerView.java | 4 +- 13 files changed, 94 insertions(+), 44 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/context/i18n/LocaleContextHolder.java b/spring-context/src/main/java/org/springframework/context/i18n/LocaleContextHolder.java index b6fa354bf81..8497f28b9c8 100644 --- a/spring-context/src/main/java/org/springframework/context/i18n/LocaleContextHolder.java +++ b/spring-context/src/main/java/org/springframework/context/i18n/LocaleContextHolder.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2017 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. @@ -190,12 +190,30 @@ public abstract class LocaleContextHolder { * {@link #getLocaleContext()} and call {@link LocaleContext#getLocale()} * @return the current Locale, or the system default Locale if no * specific Locale has been associated with the current thread + * @see #getLocaleContext() * @see LocaleContext#getLocale() * @see #setDefaultLocale(Locale) * @see java.util.Locale#getDefault() */ public static Locale getLocale() { - LocaleContext localeContext = getLocaleContext(); + return getLocale(getLocaleContext()); + } + + /** + * Return the Locale associated with the given user context, if any, + * or the system default Locale otherwise. This is effectively a + * replacement for {@link java.util.Locale#getDefault()}, + * able to optionally respect a user-level Locale setting. + * @param localeContext the user-level locale context to check + * @return the current Locale, or the system default Locale if no + * specific Locale has been associated with the current thread + * @since 5.0 + * @see #getLocale() + * @see LocaleContext#getLocale() + * @see #setDefaultLocale(Locale) + * @see java.util.Locale#getDefault() + */ + public static Locale getLocale(@Nullable LocaleContext localeContext) { if (localeContext != null) { Locale locale = localeContext.getLocale(); if (locale != null) { @@ -276,12 +294,30 @@ public abstract class LocaleContextHolder { * after downcasting to {@link TimeZoneAwareLocaleContext}. * @return the current TimeZone, or the system default TimeZone if no * specific TimeZone has been associated with the current thread + * @see #getLocaleContext() * @see TimeZoneAwareLocaleContext#getTimeZone() * @see #setDefaultTimeZone(TimeZone) * @see java.util.TimeZone#getDefault() */ public static TimeZone getTimeZone() { - LocaleContext localeContext = getLocaleContext(); + return getTimeZone(getLocaleContext()); + } + + /** + * Return the TimeZone associated with the given user context, if any, + * or the system default TimeZone otherwise. This is effectively a + * replacement for {@link java.util.TimeZone#getDefault()}, + * able to optionally respect a user-level TimeZone setting. + * @param localeContext the user-level locale context to check + * @return the current TimeZone, or the system default TimeZone if no + * specific TimeZone has been associated with the current thread + * @since 5.0 + * @see #getTimeZone() + * @see TimeZoneAwareLocaleContext#getTimeZone() + * @see #setDefaultTimeZone(TimeZone) + * @see java.util.TimeZone#getDefault() + */ + public static TimeZone getTimeZone(@Nullable LocaleContext localeContext) { if (localeContext instanceof TimeZoneAwareLocaleContext) { TimeZone timeZone = ((TimeZoneAwareLocaleContext) localeContext).getTimeZone(); if (timeZone != null) { diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultPathSegmentContainer.java b/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultPathSegmentContainer.java index b8c5c8400c6..00cf5174b7c 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultPathSegmentContainer.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultPathSegmentContainer.java @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package org.springframework.http.server.reactive; import java.nio.charset.Charset; @@ -21,6 +22,7 @@ import java.util.Collections; import java.util.List; import java.util.stream.Collectors; +import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; import org.springframework.util.LinkedMultiValueMap; @@ -93,7 +95,7 @@ class DefaultPathSegmentContainer implements PathSegmentContainer { @Override - public boolean equals(Object other) { + public boolean equals(@Nullable Object other) { if (this == other) { return true; } @@ -224,12 +226,10 @@ class DefaultPathSegmentContainer implements PathSegmentContainer { private final MultiValueMap parameters; - DefaultPathSegment(String value, String valueDecoded, String semicolonContent, MultiValueMap params) { Assert.isTrue(!value.contains("/"), "Invalid path segment value: " + value); - this.value = value; this.valueDecoded = valueDecoded; this.valueCharsDecoded = valueDecoded.toCharArray(); @@ -238,7 +238,6 @@ class DefaultPathSegmentContainer implements PathSegmentContainer { this.parameters = CollectionUtils.unmodifiableMultiValueMap(params); } - @Override public String value() { return this.value; @@ -269,9 +268,8 @@ class DefaultPathSegmentContainer implements PathSegmentContainer { return this.parameters; } - @Override - public boolean equals(Object other) { + public boolean equals(@Nullable Object other) { if (this == other) { return true; } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultRequestPath.java b/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultRequestPath.java index 60aec196488..8607442735f 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultRequestPath.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultRequestPath.java @@ -19,6 +19,7 @@ import java.net.URI; import java.nio.charset.Charset; import java.util.List; +import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.StringUtils; @@ -126,7 +127,7 @@ class DefaultRequestPath implements RequestPath { @Override - public boolean equals(Object other) { + public boolean equals(@Nullable Object other) { if (this == other) { return true; } diff --git a/spring-web/src/main/java/org/springframework/web/server/i18n/AcceptHeaderLocaleContextResolver.java b/spring-web/src/main/java/org/springframework/web/server/i18n/AcceptHeaderLocaleContextResolver.java index 4732a567f99..ef9a70f3cbb 100644 --- a/spring-web/src/main/java/org/springframework/web/server/i18n/AcceptHeaderLocaleContextResolver.java +++ b/spring-web/src/main/java/org/springframework/web/server/i18n/AcceptHeaderLocaleContextResolver.java @@ -50,7 +50,7 @@ public class AcceptHeaderLocaleContextResolver implements LocaleContextResolver * determined via {@link HttpHeaders#getAcceptLanguageAsLocales()}. * @param locales the supported locales */ - public void setSupportedLocales(List locales) { + public void setSupportedLocales(@Nullable List locales) { this.supportedLocales.clear(); if (locales != null) { this.supportedLocales.addAll(locales); diff --git a/spring-web/src/main/java/org/springframework/web/server/i18n/LocaleContextResolver.java b/spring-web/src/main/java/org/springframework/web/server/i18n/LocaleContextResolver.java index da5d28e3850..49961170183 100644 --- a/spring-web/src/main/java/org/springframework/web/server/i18n/LocaleContextResolver.java +++ b/spring-web/src/main/java/org/springframework/web/server/i18n/LocaleContextResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2017 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. @@ -36,7 +36,6 @@ public interface LocaleContextResolver { /** * Resolve the current locale context via the given exchange. - * *

The returned context may be a * {@link org.springframework.context.i18n.TimeZoneAwareLocaleContext}, * containing a locale with associated time zone information. @@ -44,7 +43,7 @@ public interface LocaleContextResolver { *

Custom resolver implementations may also return extra settings in * the returned context, which again can be accessed through downcasting. * @param exchange current server exchange - * @return the current locale context (never {@code null} + * @return the current locale context (never {@code null}) */ LocaleContext resolveLocaleContext(ServerWebExchange exchange); diff --git a/spring-web/src/main/java/org/springframework/web/util/pattern/PathPatternComparator.java b/spring-web/src/main/java/org/springframework/web/util/pattern/PathPatternComparator.java index b4a5dd8a66c..b589bd01638 100644 --- a/spring-web/src/main/java/org/springframework/web/util/pattern/PathPatternComparator.java +++ b/spring-web/src/main/java/org/springframework/web/util/pattern/PathPatternComparator.java @@ -18,6 +18,8 @@ package org.springframework.web.util.pattern; import java.util.Comparator; +import org.springframework.lang.Nullable; + /** * {@link PathPattern} comparator that takes account of a specified * path and sorts anything that exactly matches it to be first. @@ -34,12 +36,14 @@ public class PathPatternComparator implements Comparator { private final String path; + public PathPatternComparator(String path) { this.path = path; } + @Override - public int compare(PathPattern o1, PathPattern o2) { + public int compare(@Nullable PathPattern o1, @Nullable PathPattern o2) { // Nulls get sorted to the end if (o1 == null) { return (o2 == null ? 0 : +1); @@ -47,6 +51,7 @@ public class PathPatternComparator implements Comparator { else if (o2 == null) { return -1; } + // exact matches get sorted first if (o1.getPatternString().equals(path)) { return (o2.getPatternString().equals(path)) ? 0 : -1; @@ -54,6 +59,7 @@ public class PathPatternComparator implements Comparator { else if (o2.getPatternString().equals(path)) { return +1; } + // compare pattern specificity int result = o1.compareTo(o2); // if equal specificity, sort using pattern string @@ -63,4 +69,4 @@ public class PathPatternComparator implements Comparator { return result; } -} \ No newline at end of file +} diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurationSupport.java b/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurationSupport.java index f271b3c4a6f..8c0a510cac8 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurationSupport.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurationSupport.java @@ -112,11 +112,13 @@ public class WebFluxConfigurationSupport implements ApplicationContextAware { mapping.setCorsConfigurations(getCorsConfigurations()); PathMatchConfigurer configurer = getPathMatchConfigurer(); - if (configurer.isUseTrailingSlashMatch() != null) { - mapping.setUseTrailingSlashMatch(configurer.isUseTrailingSlashMatch()); + Boolean useTrailingSlashMatch = configurer.isUseTrailingSlashMatch(); + Boolean useCaseSensitiveMatch = configurer.isUseCaseSensitiveMatch(); + if (useTrailingSlashMatch != null) { + mapping.setUseTrailingSlashMatch(useTrailingSlashMatch); } - if (configurer.isUseCaseSensitiveMatch() != null) { - mapping.setUseCaseSensitiveMatch(configurer.isUseCaseSensitiveMatch()); + if (useCaseSensitiveMatch != null) { + mapping.setUseCaseSensitiveMatch(useCaseSensitiveMatch); } return mapping; } @@ -209,11 +211,13 @@ public class WebFluxConfigurationSupport implements ApplicationContextAware { AbstractHandlerMapping handlerMapping = registry.getHandlerMapping(); if (handlerMapping != null) { PathMatchConfigurer configurer = getPathMatchConfigurer(); - if (configurer.isUseTrailingSlashMatch() != null) { - handlerMapping.setUseTrailingSlashMatch(configurer.isUseTrailingSlashMatch()); + Boolean useTrailingSlashMatch = configurer.isUseTrailingSlashMatch(); + Boolean useCaseSensitiveMatch = configurer.isUseCaseSensitiveMatch(); + if (useTrailingSlashMatch != null) { + handlerMapping.setUseTrailingSlashMatch(useTrailingSlashMatch); } - if (configurer.isUseCaseSensitiveMatch() != null) { - handlerMapping.setUseCaseSensitiveMatch(configurer.isUseCaseSensitiveMatch()); + if (useCaseSensitiveMatch != null) { + handlerMapping.setUseCaseSensitiveMatch(useCaseSensitiveMatch); } } else { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultRenderingResponseBuilder.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultRenderingResponseBuilder.java index 14317bb3945..6633bc8c43b 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultRenderingResponseBuilder.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultRenderingResponseBuilder.java @@ -27,6 +27,7 @@ import java.util.stream.Stream; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; +import org.springframework.context.i18n.LocaleContextHolder; import org.springframework.core.Conventions; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; @@ -155,7 +156,7 @@ class DefaultRenderingResponseBuilder implements RenderingResponse.Builder { ServerHttpResponse response = exchange.getResponse(); writeStatusAndHeaders(response); MediaType contentType = exchange.getResponse().getHeaders().getContentType(); - Locale locale = exchange.getLocaleContext().getLocale(); + Locale locale = LocaleContextHolder.getLocale(exchange.getLocaleContext()); Stream viewResolverStream = context.viewResolvers().stream(); return Flux.fromStream(viewResolverStream) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/PathMatchResult.java b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/PathMatchResult.java index 1886482a193..ee07aa5a392 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/PathMatchResult.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/PathMatchResult.java @@ -22,8 +22,10 @@ import org.springframework.web.util.pattern.PathPattern; /** * Result of matching an request lookup path against {@link PathPattern} instances. - *

Each result optionnally associates the matching {@link PathPattern} + * + *

Each result optionally associates the matching {@link PathPattern} * with a request handler of type {@code T}. + * * @author Brian Clozel * @since 5.0 * @see PathPatternRegistry @@ -34,17 +36,19 @@ public class PathMatchResult { private final T handler; - public PathMatchResult(PathPattern pattern, T handler) { - Assert.notNull(pattern, "PathPattern should not be null"); + + public PathMatchResult(PathPattern pattern, @Nullable T handler) { + Assert.notNull(pattern, "PathPattern must not be null"); this.pattern = pattern; this.handler = handler; } + /** * Return the {@link PathPattern} that matched the incoming request. */ public PathPattern getPattern() { - return pattern; + return this.pattern; } /** @@ -52,7 +56,7 @@ public class PathMatchResult { */ @Nullable public T getHandler() { - return handler; + return this.handler; } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/PathPatternRegistry.java b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/PathPatternRegistry.java index 94f42afa67c..99be68ea19c 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/PathPatternRegistry.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/PathPatternRegistry.java @@ -25,6 +25,7 @@ import java.util.SortedSet; import java.util.TreeSet; import java.util.stream.Collectors; +import org.springframework.lang.Nullable; import org.springframework.util.StringUtils; import org.springframework.web.util.pattern.PathPattern; import org.springframework.web.util.pattern.PathPatternComparator; @@ -55,7 +56,6 @@ public class PathPatternRegistry { /** * Create a new {@code PathPatternRegistry} using * the provided instance of {@link PathPatternParser}. - * * @param patternParser the {@link PathPatternParser} to use */ public PathPatternRegistry(PathPatternParser patternParser) { @@ -66,7 +66,6 @@ public class PathPatternRegistry { * Create a new {@code PathPatternRegistry} using * the provided instance of {@link PathPatternParser} * and the given map of {@link PathPattern}. - * * @param patternParser the {@link PathPatternParser} to use * @param patternsMap the map of {@link PathPattern} to use */ @@ -75,6 +74,7 @@ public class PathPatternRegistry { this.patternsMap = new HashMap<>(patternsMap); } + /** * Return a (read-only) map of all patterns and associated values. */ @@ -84,7 +84,6 @@ public class PathPatternRegistry { /** * Return a {@code SortedSet} of {@code PathPattern}s matching the given {@code lookupPath}. - * *

The returned set sorted with the most specific * patterns first, according to the given {@code lookupPath}. * @param lookupPath the URL lookup path to be matched against @@ -99,7 +98,6 @@ public class PathPatternRegistry { /** * Return, if any, the most specific {@code PathPattern} matching the given {@code lookupPath}. - * * @param lookupPath the URL lookup path to be matched against */ public Optional> findFirstMatch(String lookupPath) { @@ -120,7 +118,7 @@ public class PathPatternRegistry { /** * Parse the given {@code rawPattern} and adds it to this registry. * @param rawPattern raw path pattern to parse and register - * @param handler + * @param handler the associated handler object */ public void register(String rawPattern, T handler) { String fixedPattern = prependLeadingSlash(rawPattern); @@ -137,6 +135,7 @@ public class PathPatternRegistry { } } + private class PathMatchResultComparator implements Comparator> { private final String path; @@ -146,7 +145,7 @@ public class PathPatternRegistry { } @Override - public int compare(PathMatchResult o1, PathMatchResult o2) { + public int compare(@Nullable PathMatchResult o1, @Nullable PathMatchResult o2) { // Nulls get sorted to the end if (o1 == null) { return (o2 == null ? 0 : +1); @@ -171,7 +170,6 @@ public class PathPatternRegistry { } return result; } - } -} \ No newline at end of file +} diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java index a6856081b77..d19b8012d29 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java @@ -32,12 +32,12 @@ import org.springframework.context.event.ContextRefreshedEvent; import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.lang.Nullable; +import org.springframework.web.reactive.handler.PathMatchResult; +import org.springframework.web.reactive.handler.PathPatternRegistry; import org.springframework.web.reactive.handler.SimpleUrlHandlerMapping; import org.springframework.web.server.ServerWebExchange; -import org.springframework.web.reactive.handler.PathMatchResult; import org.springframework.web.util.pattern.PathPattern; import org.springframework.web.util.pattern.PathPatternParser; -import org.springframework.web.reactive.handler.PathPatternRegistry; /** * A central component to use to obtain the public URL path that clients should @@ -205,6 +205,9 @@ public class ResourceUrlProvider implements ApplicationListener { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java index a606808ad7b..7841d5f6bf8 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java @@ -29,6 +29,7 @@ import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import org.springframework.beans.BeanUtils; +import org.springframework.context.i18n.LocaleContextHolder; import org.springframework.core.Conventions; import org.springframework.core.MethodParameter; import org.springframework.core.Ordered; @@ -200,8 +201,7 @@ public class ViewResolutionResultHandler extends HandlerResultHandlerSupport Mono> viewsMono; Model model = result.getModel(); MethodParameter parameter = result.getReturnTypeSource(); - - Locale locale = exchange.getLocaleContext().getLocale(); + Locale locale = LocaleContextHolder.getLocale(exchange.getLocaleContext()); Class clazz = valueType.getRawClass(); if (clazz == null) { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/freemarker/FreeMarkerView.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/freemarker/FreeMarkerView.java index 8f6eed61586..c1be5fe3953 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/freemarker/FreeMarkerView.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/freemarker/FreeMarkerView.java @@ -39,6 +39,7 @@ import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanFactoryUtils; import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.context.ApplicationContextException; +import org.springframework.context.i18n.LocaleContextHolder; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.http.MediaType; import org.springframework.lang.Nullable; @@ -188,8 +189,7 @@ public class FreeMarkerView extends AbstractUrlBasedView { logger.debug("Rendering FreeMarker template [" + getUrl() + "]."); } - Locale locale = exchange.getLocaleContext().getLocale(); - + Locale locale = LocaleContextHolder.getLocale(exchange.getLocaleContext()); DataBuffer dataBuffer = exchange.getResponse().bufferFactory().allocateBuffer(); try { Charset charset = getCharset(contentType);