From 59aee923e45e91ae336354f30d20478b8fc3f8ad Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Sun, 17 Mar 2019 13:37:12 +0300 Subject: [PATCH 1/2] Add filter to AbstractCachingViewResolver This commit introduces a filter that specifies whether a View should be cached by the AbstractCachingViewResolver or not. Closes gh-22391 --- .../view/AbstractCachingViewResolver.java | 20 ++++- .../web/servlet/view/ViewCacheFilter.java | 32 +++++++ .../view/ViewResolverCacheFilterTest.java | 86 +++++++++++++++++++ 3 files changed, 137 insertions(+), 1 deletion(-) create mode 100644 spring-webmvc/src/main/java/org/springframework/web/servlet/view/ViewCacheFilter.java create mode 100644 spring-webmvc/src/test/java/org/springframework/web/servlet/view/ViewResolverCacheFilterTest.java diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/view/AbstractCachingViewResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/view/AbstractCachingViewResolver.java index dc9bfc7689..9af1f12674 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/view/AbstractCachingViewResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/view/AbstractCachingViewResolver.java @@ -68,6 +68,9 @@ public abstract class AbstractCachingViewResolver extends WebApplicationObjectSu /** Fast access cache for Views, returning already cached instances without a global lock. */ private final Map viewAccessCache = new ConcurrentHashMap<>(DEFAULT_CACHE_LIMIT); + /** Filter function which determines if view should be cached. */ + private ViewCacheFilter viewCacheFilter = (viewName, view, locale) -> true; + /** Map from view key to View instance, synchronized for View creation. */ @SuppressWarnings("serial") private final Map viewCreationCache = @@ -134,6 +137,21 @@ public abstract class AbstractCachingViewResolver extends WebApplicationObjectSu this.cacheUnresolved = cacheUnresolved; } + /** + * Filter function which determines if view should be cached. + * Default behaviour is to cache all views. + */ + public void setViewCacheFilter(ViewCacheFilter cacheFilter) { + this.viewCacheFilter = cacheFilter; + } + + /** + * Return filter function which determines if view should be cached. + */ + public ViewCacheFilter getViewCacheFilter() { + return this.viewCacheFilter; + } + /** * Return if caching of unresolved views is enabled. */ @@ -160,7 +178,7 @@ public abstract class AbstractCachingViewResolver extends WebApplicationObjectSu if (view == null && this.cacheUnresolved) { view = UNRESOLVED_VIEW; } - if (view != null) { + if (view != null && this.viewCacheFilter.filter(viewName, view, locale)) { this.viewAccessCache.put(cacheKey, view); this.viewCreationCache.put(cacheKey, view); } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/view/ViewCacheFilter.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/view/ViewCacheFilter.java new file mode 100644 index 0000000000..ec543ef360 --- /dev/null +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/view/ViewCacheFilter.java @@ -0,0 +1,32 @@ +/* + * Copyright 2002-2018 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.web.servlet.view; + +import java.util.Locale; + +import org.springframework.web.servlet.View; + +/** + * Filter which determines if view should be cached in {@link AbstractCachingViewResolver}. + * + * @author Sergey Galkin + */ +@FunctionalInterface +public interface ViewCacheFilter { + + boolean filter(String viewName, View view, Locale locale); +} diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/view/ViewResolverCacheFilterTest.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/view/ViewResolverCacheFilterTest.java new file mode 100644 index 0000000000..321ff6353f --- /dev/null +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/view/ViewResolverCacheFilterTest.java @@ -0,0 +1,86 @@ +/* + * Copyright 2002-2018 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.web.servlet.view; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.Locale; +import org.junit.Test; +import org.springframework.web.servlet.View; + +public class ViewResolverCacheFilterTest { + + private interface ViewLoader { + View load(String viewName); + } + + private static class TestViewResolver extends AbstractCachingViewResolver { + + private final ViewLoader viewLoader; + + private TestViewResolver(ViewLoader viewLoader) { + this.viewLoader = viewLoader; + } + + @Override + protected View loadView(String viewName, Locale locale) { + return viewLoader.load(viewName); + } + } + + private final static String VIEW_NAME = "name"; + private final ViewLoader viewLoader = mock(ViewLoader.class); + private final TestViewResolver resolver = new TestViewResolver(viewLoader); + + @Test + public void viewWillBePlacedInCache() throws Exception { + resolver.setViewCacheFilter((n, v, l) -> true); + + resolver.resolveViewName(VIEW_NAME, Locale.ENGLISH); + resolver.resolveViewName(VIEW_NAME, Locale.ENGLISH); + + verify(viewLoader, times(1)).load(any()); + } + + @Test + public void viewWillNotBePlacedInCached() throws Exception { + resolver.setViewCacheFilter((n, v, l) -> false); + + resolver.resolveViewName(VIEW_NAME, Locale.ENGLISH); + resolver.resolveViewName(VIEW_NAME, Locale.ENGLISH); + + verify(viewLoader, times(2)).load(any()); + } + + @Test + public void verifyPassedParamsToFilter() throws Exception { + View view = mock(View.class); + when(viewLoader.load(any())).thenReturn(view); + + ViewCacheFilter filter = mock(ViewCacheFilter.class); + resolver.setViewCacheFilter(filter); + + resolver.resolveViewName(VIEW_NAME, Locale.ENGLISH); + + verify(filter, times(1)).filter(eq(VIEW_NAME), eq(view), eq(Locale.ENGLISH)); + } +} From 132fa702b75e46a2de01d2dceaf89ef4e25295fd Mon Sep 17 00:00:00 2001 From: Arjen Poutsma Date: Thu, 1 Aug 2019 11:33:16 +0200 Subject: [PATCH 2/2] Polish contribution See gh-22606 --- .../view/AbstractCachingViewResolver.java | 58 +++++++++---- .../web/servlet/view/ViewCacheFilter.java | 32 ------- .../view/ViewResolverCacheFilterTest.java | 86 ------------------- .../web/servlet/view/ViewResolverTests.java | 41 +++++++++ 4 files changed, 83 insertions(+), 134 deletions(-) delete mode 100644 spring-webmvc/src/main/java/org/springframework/web/servlet/view/ViewCacheFilter.java delete mode 100644 spring-webmvc/src/test/java/org/springframework/web/servlet/view/ViewResolverCacheFilterTest.java diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/view/AbstractCachingViewResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/view/AbstractCachingViewResolver.java index 9af1f12674..f54f612e61 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/view/AbstractCachingViewResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/view/AbstractCachingViewResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -24,6 +24,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.springframework.lang.Nullable; +import org.springframework.util.Assert; import org.springframework.web.context.support.WebApplicationObjectSupport; import org.springframework.web.servlet.View; import org.springframework.web.servlet.ViewResolver; @@ -58,6 +59,9 @@ public abstract class AbstractCachingViewResolver extends WebApplicationObjectSu } }; + /** Default cache filter that always caches. */ + private static final CacheFilter DEFAULT_CACHE_FILTER = (view, viewName, locale) -> true; + /** The maximum number of entries in the cache. */ private volatile int cacheLimit = DEFAULT_CACHE_LIMIT; @@ -65,12 +69,12 @@ public abstract class AbstractCachingViewResolver extends WebApplicationObjectSu /** Whether we should refrain from resolving views again if unresolved once. */ private boolean cacheUnresolved = true; + /** Filter function that determines if view should be cached. */ + private CacheFilter cacheFilter = DEFAULT_CACHE_FILTER; + /** Fast access cache for Views, returning already cached instances without a global lock. */ private final Map viewAccessCache = new ConcurrentHashMap<>(DEFAULT_CACHE_LIMIT); - /** Filter function which determines if view should be cached. */ - private ViewCacheFilter viewCacheFilter = (viewName, view, locale) -> true; - /** Map from view key to View instance, synchronized for View creation. */ @SuppressWarnings("serial") private final Map viewCreationCache = @@ -138,28 +142,28 @@ public abstract class AbstractCachingViewResolver extends WebApplicationObjectSu } /** - * Filter function which determines if view should be cached. - * Default behaviour is to cache all views. + * Return if caching of unresolved views is enabled. */ - public void setViewCacheFilter(ViewCacheFilter cacheFilter) { - this.viewCacheFilter = cacheFilter; + public boolean isCacheUnresolved() { + return this.cacheUnresolved; } /** - * Return filter function which determines if view should be cached. + * Sets the filter that determines if view should be cached. + * Default behaviour is to cache all views. */ - public ViewCacheFilter getViewCacheFilter() { - return this.viewCacheFilter; + public void setCacheFilter(CacheFilter cacheFilter) { + Assert.notNull(cacheFilter, "CacheFilter must not be null"); + this.cacheFilter = cacheFilter; } /** - * Return if caching of unresolved views is enabled. + * Return filter function that determines if view should be cached. */ - public boolean isCacheUnresolved() { - return this.cacheUnresolved; + public CacheFilter getCacheFilter() { + return this.cacheFilter; } - @Override @Nullable public View resolveViewName(String viewName, Locale locale) throws Exception { @@ -178,7 +182,7 @@ public abstract class AbstractCachingViewResolver extends WebApplicationObjectSu if (view == null && this.cacheUnresolved) { view = UNRESOLVED_VIEW; } - if (view != null && this.viewCacheFilter.filter(viewName, view, locale)) { + if (view != null && this.cacheFilter.filter(view, viewName, locale)) { this.viewAccessCache.put(cacheKey, view); this.viewCreationCache.put(cacheKey, view); } @@ -284,4 +288,26 @@ public abstract class AbstractCachingViewResolver extends WebApplicationObjectSu @Nullable protected abstract View loadView(String viewName, Locale locale) throws Exception; + + /** + * Filter that determines if view should be cached. + * + * @author Sergey Galkin + * @author Arjen Poutsma + * @since 5.2 + */ + @FunctionalInterface + public interface CacheFilter { + + /** + * Indicates whether the given view should be cached. The name and + * locale used to resolve the view are also provided. + * @param view the view + * @param viewName the name used to resolve {@code view} + * @param locale the locale used to resolve {@code view} + * @return {@code true} if the view should be cached; {@code false} + * otherwise + */ + boolean filter(View view, String viewName, Locale locale); + } } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/view/ViewCacheFilter.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/view/ViewCacheFilter.java deleted file mode 100644 index ec543ef360..0000000000 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/view/ViewCacheFilter.java +++ /dev/null @@ -1,32 +0,0 @@ -/* - * Copyright 2002-2018 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. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.web.servlet.view; - -import java.util.Locale; - -import org.springframework.web.servlet.View; - -/** - * Filter which determines if view should be cached in {@link AbstractCachingViewResolver}. - * - * @author Sergey Galkin - */ -@FunctionalInterface -public interface ViewCacheFilter { - - boolean filter(String viewName, View view, Locale locale); -} diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/view/ViewResolverCacheFilterTest.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/view/ViewResolverCacheFilterTest.java deleted file mode 100644 index 321ff6353f..0000000000 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/view/ViewResolverCacheFilterTest.java +++ /dev/null @@ -1,86 +0,0 @@ -/* - * Copyright 2002-2018 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. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.web.servlet.view; - -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -import java.util.Locale; -import org.junit.Test; -import org.springframework.web.servlet.View; - -public class ViewResolverCacheFilterTest { - - private interface ViewLoader { - View load(String viewName); - } - - private static class TestViewResolver extends AbstractCachingViewResolver { - - private final ViewLoader viewLoader; - - private TestViewResolver(ViewLoader viewLoader) { - this.viewLoader = viewLoader; - } - - @Override - protected View loadView(String viewName, Locale locale) { - return viewLoader.load(viewName); - } - } - - private final static String VIEW_NAME = "name"; - private final ViewLoader viewLoader = mock(ViewLoader.class); - private final TestViewResolver resolver = new TestViewResolver(viewLoader); - - @Test - public void viewWillBePlacedInCache() throws Exception { - resolver.setViewCacheFilter((n, v, l) -> true); - - resolver.resolveViewName(VIEW_NAME, Locale.ENGLISH); - resolver.resolveViewName(VIEW_NAME, Locale.ENGLISH); - - verify(viewLoader, times(1)).load(any()); - } - - @Test - public void viewWillNotBePlacedInCached() throws Exception { - resolver.setViewCacheFilter((n, v, l) -> false); - - resolver.resolveViewName(VIEW_NAME, Locale.ENGLISH); - resolver.resolveViewName(VIEW_NAME, Locale.ENGLISH); - - verify(viewLoader, times(2)).load(any()); - } - - @Test - public void verifyPassedParamsToFilter() throws Exception { - View view = mock(View.class); - when(viewLoader.load(any())).thenReturn(view); - - ViewCacheFilter filter = mock(ViewCacheFilter.class); - resolver.setViewCacheFilter(filter); - - resolver.resolveViewName(VIEW_NAME, Locale.ENGLISH); - - verify(filter, times(1)).filter(eq(VIEW_NAME), eq(view), eq(Locale.ENGLISH)); - } -} diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/view/ViewResolverTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/view/ViewResolverTests.java index 018fd92612..7a353f5f2e 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/view/ViewResolverTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/view/ViewResolverTests.java @@ -516,6 +516,47 @@ public class ViewResolverTests { assertThat(count.intValue()).isEqualTo(3); } + @Test + public void cacheFilterEnabled() throws Exception { + AtomicInteger count = new AtomicInteger(); + + // filter is enabled by default + AbstractCachingViewResolver viewResolver = new AbstractCachingViewResolver() { + @Override + protected View loadView(String viewName, Locale locale) { + assertThat(viewName).isEqualTo("view"); + assertThat(locale).isEqualTo(Locale.getDefault()); + count.incrementAndGet(); + return new TestView(); + } + }; + + viewResolver.resolveViewName("view", Locale.getDefault()); + viewResolver.resolveViewName("view", Locale.getDefault()); + + assertThat(count.intValue()).isEqualTo(1); + } + + @Test + public void cacheFilterDisabled() throws Exception { + AtomicInteger count = new AtomicInteger(); + + AbstractCachingViewResolver viewResolver = new AbstractCachingViewResolver() { + @Override + protected View loadView(String viewName, Locale locale) { + count.incrementAndGet(); + return new TestView(); + } + }; + + viewResolver.setCacheFilter((view, viewName, locale) -> false); + + viewResolver.resolveViewName("view", Locale.getDefault()); + viewResolver.resolveViewName("view", Locale.getDefault()); + + assertThat(count.intValue()).isEqualTo(2); + } + public static class TestView extends InternalResourceView {