Polish contribution

See gh-22606

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<Object, View> 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<Object, View> viewCreationCache =
@@ -137,21 +141,6 @@ 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.
 	 */
@@ -159,6 +148,21 @@ public abstract class AbstractCachingViewResolver extends WebApplicationObjectSu
 		return this.cacheUnresolved;
 	}
 
+	/**
+	 * Sets the filter that determines if view should be cached.
+	 * Default behaviour is to cache all views.
+	 */
+	public void setCacheFilter(CacheFilter cacheFilter) {
+		Assert.notNull(cacheFilter, "CacheFilter must not be null");
+		this.cacheFilter = cacheFilter;
+	}
+
+	/**
+	 * Return filter function that determines if view should be cached.
+	 */
+	public CacheFilter getCacheFilter() {
+		return this.cacheFilter;
+	}
 
 	@Override
 	@Nullable
@@ -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);
+	}
 }

spring-webmvc/src/main/java/org/springframework/web/servlet/view/ViewCacheFilter.java

@@ -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);
-}

spring-webmvc/src/test/java/org/springframework/web/servlet/view/ViewResolverCacheFilterTest.java

@@ -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));
-	}
-}

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 {