From 34477b4f03f02e8b3eed6a5c52612fe9ce7f4b87 Mon Sep 17 00:00:00 2001 From: Arjen Poutsma Date: Thu, 17 Aug 2023 16:42:44 +0200 Subject: [PATCH] Schedule FreeMarker template lookup on bounded elastic scheduler This commit makes sure that FreeMarker template lookups, which potentially block, are scheduled on the bounded elastic scheduler. Closes gh-30903 --- .../result/view/AbstractUrlBasedView.java | 16 +++- .../result/view/UrlBasedViewResolver.java | 10 +-- .../view/freemarker/FreeMarkerView.java | 74 ++++++++++++++---- .../view/freemarker/FreeMarkerMacroTests.java | 78 ++++++++++--------- .../view/freemarker/FreeMarkerViewTests.java | 34 +++++--- 5 files changed, 140 insertions(+), 72 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/AbstractUrlBasedView.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/AbstractUrlBasedView.java index d6b833da4c..3fcc1c04f8 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/AbstractUrlBasedView.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/AbstractUrlBasedView.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2023 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. @@ -18,6 +18,8 @@ package org.springframework.web.reactive.result.view; import java.util.Locale; +import reactor.core.publisher.Mono; + import org.springframework.beans.factory.InitializingBean; import org.springframework.lang.Nullable; @@ -81,6 +83,18 @@ public abstract class AbstractUrlBasedView extends AbstractView implements Initi */ public abstract boolean checkResourceExists(Locale locale) throws Exception; + /** + * Deferred check whether the resource for the configured URL actually exists. + *

The default implementation calls {@link #checkResourceExists(Locale)}. + * @param locale the desired Locale that we're looking for + * @return {@code false} if the resource exists + * {@code false} if we know that it does not exist + * @since 6.1 + */ + public Mono resourceExists(Locale locale) { + return Mono.fromCallable(() -> checkResourceExists(locale)); + } + @Override public String toString() { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/UrlBasedViewResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/UrlBasedViewResolver.java index 2102271034..6247d03d01 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/UrlBasedViewResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/UrlBasedViewResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 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. @@ -238,12 +238,8 @@ public class UrlBasedViewResolver extends ViewResolverSupport } View view = applyLifecycleMethods(viewName, urlBasedView); - try { - return (urlBasedView.checkResourceExists(locale) ? Mono.just(view) : Mono.empty()); - } - catch (Exception ex) { - return Mono.error(ex); - } + return urlBasedView.resourceExists(locale) + .flatMap(exists -> exists ? Mono.just(view) : Mono.empty()); } /** 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 f399b719e8..38d5894436 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 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. @@ -32,8 +32,10 @@ import freemarker.template.DefaultObjectWrapperBuilder; import freemarker.template.ObjectWrapper; import freemarker.template.SimpleHash; import freemarker.template.Template; +import freemarker.template.TemplateException; import freemarker.template.Version; import reactor.core.publisher.Mono; +import reactor.core.scheduler.Schedulers; import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanFactoryUtils; @@ -210,6 +212,24 @@ public class FreeMarkerView extends AbstractUrlBasedView { } } + /** + * Check that the FreeMarker template used for this view exists and is valid. + *

Can be overridden to customize the behavior, for example in case of + * multiple templates to be rendered into a single view. + * @since 6.1 + */ + @Override + public Mono resourceExists(Locale locale) { + return lookupTemplate(locale) + .map(template -> Boolean.TRUE) + .switchIfEmpty(Mono.just(Boolean.FALSE)) + .onErrorResume(FileNotFoundException.class, t -> Mono.just(Boolean.FALSE)) + .onErrorMap(ParseException.class, ex -> new ApplicationContextException( + "Failed to parse FreeMarker template for URL [" + getUrl() + "]", ex)) + .onErrorMap(IOException.class, ex -> new ApplicationContextException( + "Could not load FreeMarker template for URL [" + getUrl() + "]", ex)); + } + /** * Prepare the model to use for rendering by potentially exposing a * {@link RequestContext} for use in Spring FreeMarker macros and then @@ -243,7 +263,7 @@ public class FreeMarkerView extends AbstractUrlBasedView { @Nullable MediaType contentType, ServerWebExchange exchange) { return exchange.getResponse().writeWith(Mono - .fromCallable(() -> { + .defer(() -> { // Expose all standard FreeMarker hash models. SimpleHash freeMarkerModel = getTemplateModel(renderAttributes, exchange); @@ -252,19 +272,22 @@ public class FreeMarkerView extends AbstractUrlBasedView { } Locale locale = LocaleContextHolder.getLocale(exchange.getLocaleContext()); - FastByteArrayOutputStream bos = new FastByteArrayOutputStream(); - try { - Charset charset = getCharset(contentType); - Writer writer = new OutputStreamWriter(bos, charset); - getTemplate(locale).process(freeMarkerModel, writer); - - byte[] bytes = bos.toByteArrayUnsafe(); - return exchange.getResponse().bufferFactory().wrap(bytes); - } - catch (IOException ex) { - String message = "Could not load FreeMarker template for URL [" + getUrl() + "]"; - throw new IllegalStateException(message, ex); - } + return lookupTemplate(locale) + .flatMap(template -> { + try { + FastByteArrayOutputStream bos = new FastByteArrayOutputStream(); + Charset charset = getCharset(contentType); + Writer writer = new OutputStreamWriter(bos, charset); + template.process(freeMarkerModel, writer); + byte[] bytes = bos.toByteArrayUnsafe(); + DataBuffer buffer = exchange.getResponse().bufferFactory().wrap(bytes); + return Mono.just(buffer); + } + catch (TemplateException | IOException ex ) { + String message = "Could not load FreeMarker template for URL [" + getUrl() + "]"; + return Mono.error(new IllegalStateException(message, ex)); + } + }); }) .doOnDiscard(DataBuffer.class, DataBufferUtils::release)); } @@ -302,11 +325,32 @@ public class FreeMarkerView extends AbstractUrlBasedView { *

By default, the template specified by the "url" bean property will be retrieved. * @param locale the current locale * @return the FreeMarker template to render + * @deprecated since 6.1, in favor of {@link #lookupTemplate(Locale)}, to be + * removed in 6.2 */ + @Deprecated(since = "6.1", forRemoval = true) protected Template getTemplate(Locale locale) throws IOException { return (getEncoding() != null ? obtainConfiguration().getTemplate(getUrl(), locale, getEncoding()) : obtainConfiguration().getTemplate(getUrl(), locale)); } + /** + * Retrieve the FreeMarker template for the given locale, to be rendered by this view. + *

By default, the template specified by the "url" bean property will be retrieved, + * and the returned mono will subscribe on the + * {@linkplain Schedulers#boundedElastic() bounded elastic scheduler} as template + * lookups can be blocking operations. + * @param locale the current locale + * @return the FreeMarker template to render + * @since 6.1 + */ + protected Mono