Immutable Resource[Resolver|Transformer]Chains

Issue: SPR-16862

spring-webflux/src/main/java/org/springframework/web/reactive/resource/DefaultResourceResolverChain.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * 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.
@@ -17,7 +17,9 @@
 package org.springframework.web.reactive.resource;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
+import java.util.ListIterator;
 
 import reactor.core.publisher.Mono;
 
@@ -27,68 +29,63 @@ import org.springframework.util.Assert;
 import org.springframework.web.server.ServerWebExchange;
 
 /**
- * A default implementation of {@link ResourceResolverChain} for invoking a list
- * of {@link ResourceResolver}s.
+ * Default immutable implementation of {@link ResourceResolverChain}.
  *
  * @author Rossen Stoyanchev
  * @since 5.0
  */
 class DefaultResourceResolverChain implements ResourceResolverChain {
 
-	private final List<ResourceResolver> resolvers = new ArrayList<>();
+	@Nullable
+	private final ResourceResolver resolver;
 
-	private int index = -1;
+	@Nullable
+	private final ResourceResolverChain nextChain;
 
 
 	public DefaultResourceResolverChain(@Nullable List<? extends ResourceResolver> resolvers) {
-		if (resolvers != null) {
-			this.resolvers.addAll(resolvers);
+		resolvers = resolvers != null ? resolvers : Collections.emptyList();
+		DefaultResourceResolverChain chain = initChain(new ArrayList<>(resolvers));
+		this.resolver = chain.resolver;
+		this.nextChain = chain.nextChain;
+	}
+
+	private static DefaultResourceResolverChain initChain(ArrayList<? extends ResourceResolver> resolvers) {
+		DefaultResourceResolverChain chain = new DefaultResourceResolverChain(null, null);
+		ListIterator<? extends ResourceResolver> itr = resolvers.listIterator(resolvers.size());
+		while (itr.hasPrevious()) {
+			chain = new DefaultResourceResolverChain(itr.previous(), chain);
 		}
+		return chain;
+	}
+
+	private DefaultResourceResolverChain(@Nullable ResourceResolver resolver,
+			@Nullable ResourceResolverChain chain) {
+
+		Assert.isTrue((resolver == null && chain == null) || (resolver != null && chain != null),
+				"Both resolver and resolver chain must be null, or neither is");
+
+		this.resolver = resolver;
+		this.nextChain = chain;
 	}
 
 
 	@Override
+	@SuppressWarnings("ConstantConditions")
 	public Mono<Resource> resolveResource(@Nullable ServerWebExchange exchange, String requestPath,
 			List<? extends Resource> locations) {
 
-		ResourceResolver resolver = getNext();
-		if (resolver == null) {
-			return Mono.empty();
-		}
-
-		try {
-			return resolver.resolveResource(exchange, requestPath, locations, this);
-		}
-		finally {
-			this.index--;
-		}
+		return this.resolver != null ?
+				this.resolver.resolveResource(exchange, requestPath, locations, this.nextChain) :
+				Mono.empty();
 	}
 
 	@Override
+	@SuppressWarnings("ConstantConditions")
 	public Mono<String> resolveUrlPath(String resourcePath, List<? extends Resource> locations) {
-		ResourceResolver resolver = getNext();
-		if (resolver == null) {
-			return Mono.empty();
-		}
-
-		try {
-			return resolver.resolveUrlPath(resourcePath, locations, this);
-		}
-		finally {
-			this.index--;
-		}
-	}
-
-	@Nullable
-	private ResourceResolver getNext() {
-		Assert.state(this.index <= this.resolvers.size(),
-				"Current index exceeds the number of configured ResourceResolvers");
-
-		if (this.index == (this.resolvers.size() - 1)) {
-			return null;
-		}
-		this.index++;
-		return this.resolvers.get(this.index);
+		return this.resolver != null ?
+				this.resolver.resolveUrlPath(resourcePath, locations, this.nextChain) :
+				Mono.empty();
 	}
 
 }

spring-webflux/src/main/java/org/springframework/web/reactive/resource/DefaultResourceTransformerChain.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-201/ the original author or authors.
+ * 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.
@@ -17,7 +17,9 @@
 package org.springframework.web.reactive.resource;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
+import java.util.ListIterator;
 
 import reactor.core.publisher.Mono;
 
@@ -27,8 +29,7 @@ import org.springframework.util.Assert;
 import org.springframework.web.server.ServerWebExchange;
 
 /**
- * A default implementation of {@link ResourceTransformerChain} for invoking
- * a list of {@link ResourceTransformer}s.
+ * Default immutable implementation of {@link ResourceTransformerChain}.
  *
  * @author Rossen Stoyanchev
  * @since 5.0
@@ -37,9 +38,11 @@ class DefaultResourceTransformerChain implements ResourceTransformerChain {
 
 	private final ResourceResolverChain resolverChain;
 
-	private final List<ResourceTransformer> transformers = new ArrayList<>();
+	@Nullable
+	private final ResourceTransformer transformer;
 
-	private int index = -1;
+	@Nullable
+	private final ResourceTransformerChain nextChain;
 
 
 	public DefaultResourceTransformerChain(ResourceResolverChain resolverChain,
@@ -47,11 +50,34 @@ class DefaultResourceTransformerChain implements ResourceTransformerChain {
 
 		Assert.notNull(resolverChain, "ResourceResolverChain is required");
 		this.resolverChain = resolverChain;
-		if (transformers != null) {
-			this.transformers.addAll(transformers);
+
+		transformers = transformers != null ? transformers : Collections.emptyList();
+		DefaultResourceTransformerChain chain = initTransformerChain(resolverChain, new ArrayList<>(transformers));
+		this.transformer = chain.transformer;
+		this.nextChain = chain.nextChain;
+	}
+
+	private DefaultResourceTransformerChain initTransformerChain(ResourceResolverChain resolverChain,
+			ArrayList<ResourceTransformer> transformers) {
+
+		DefaultResourceTransformerChain chain = new DefaultResourceTransformerChain(resolverChain, null, null);
+		ListIterator<? extends ResourceTransformer> itr = transformers.listIterator(transformers.size());
+		while (itr.hasPrevious()) {
+			chain = new DefaultResourceTransformerChain(resolverChain, itr.previous(), chain);
 		}
+		return chain;
 	}
 
+	public DefaultResourceTransformerChain(ResourceResolverChain resolverChain,
+			@Nullable ResourceTransformer transformer, @Nullable ResourceTransformerChain chain) {
+
+		Assert.isTrue((transformer == null && chain == null) || (transformer != null && chain != null),
+				"Both transformer and transformer chain must be null, or neither is");
+
+		this.resolverChain = resolverChain;
+		this.transformer = transformer;
+		this.nextChain = chain;
+	}
 
 	public ResourceResolverChain getResolverChain() {
 		return this.resolverChain;
@@ -59,30 +85,11 @@ class DefaultResourceTransformerChain implements ResourceTransformerChain {
 
 
 	@Override
+	@SuppressWarnings("ConstantConditions")
 	public Mono<Resource> transform(ServerWebExchange exchange, Resource resource) {
-		ResourceTransformer transformer = getNext();
-		if (transformer == null) {
-			return Mono.just(resource);
-		}
-		try {
-			return transformer.transform(exchange, resource, this);
-		}
-		finally {
-			this.index--;
-		}
-	}
-
-	@Nullable
-	private ResourceTransformer getNext() {
-		Assert.state(this.index <= this.transformers.size(),
-				"Current index exceeds the number of configured ResourceTransformer's");
-
-		if (this.index == (this.transformers.size() - 1)) {
-			return null;
-		}
-
-		this.index++;
-		return this.transformers.get(this.index);
+		return this.transformer != null ?
+				this.transformer.transform(exchange, resource, this.nextChain) :
+				Mono.just(resource);
 	}
 
-}
+}

spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java

@@ -83,20 +83,25 @@ import org.springframework.web.server.WebHandler;
  */
 public class ResourceWebHandler implements WebHandler, InitializingBean {
 
-	/** Set of supported HTTP methods */
 	private static final Set<HttpMethod> SUPPORTED_METHODS = EnumSet.of(HttpMethod.GET, HttpMethod.HEAD);
 
-	private static final ResponseStatusException NOT_FOUND_EXCEPTION =
-			new ResponseStatusException(HttpStatus.NOT_FOUND);
+	private static final Exception NOT_FOUND_EXCEPTION = new ResponseStatusException(HttpStatus.NOT_FOUND);
 
 	private static final Log logger = LogFactory.getLog(ResourceWebHandler.class);
 
+
 	private final List<Resource> locations = new ArrayList<>(4);
 
 	private final List<ResourceResolver> resourceResolvers = new ArrayList<>(4);
 
 	private final List<ResourceTransformer> resourceTransformers = new ArrayList<>(4);
 
+	@Nullable
+	private ResourceResolverChain resolverChain;
+
+	@Nullable
+	private ResourceTransformerChain transformerChain;
+
 	@Nullable
 	private CacheControl cacheControl;
 
@@ -199,10 +204,16 @@ public class ResourceWebHandler implements WebHandler, InitializingBean {
 		if (this.resourceResolvers.isEmpty()) {
 			this.resourceResolvers.add(new PathResourceResolver());
 		}
+
 		initAllowedLocations();
+
 		if (getResourceHttpMessageWriter() == null) {
 			this.resourceHttpMessageWriter = new ResourceHttpMessageWriter();
 		}
+
+		// Initialize immutable resolver and transformer chains
+		this.resolverChain = new DefaultResourceResolverChain(this.resourceResolvers);
+		this.transformerChain = new DefaultResourceTransformerChain(this.resolverChain, this.resourceTransformers);
 	}
 
 	/**
@@ -330,12 +341,11 @@ public class ResourceWebHandler implements WebHandler, InitializingBean {
 			return Mono.empty();
 		}
 
-		ResourceResolverChain resolveChain = createResolverChain();
-		return resolveChain.resolveResource(exchange, path, getLocations())
-				.flatMap(resource -> {
-					ResourceTransformerChain transformerChain = createTransformerChain(resolveChain);
-					return transformerChain.transform(exchange, resource);
-				});
+		Assert.notNull(this.resolverChain, "ResourceResolverChain not initialized.");
+		Assert.notNull(this.transformerChain, "ResourceTransformerChain not initialized.");
+
+		return this.resolverChain.resolveResource(exchange, path, getLocations())
+				.flatMap(resource -> this.transformerChain.transform(exchange, resource));
 	}
 
 	/**
@@ -470,14 +480,6 @@ public class ResourceWebHandler implements WebHandler, InitializingBean {
 		return false;
 	}
 
-	private ResourceResolverChain createResolverChain() {
-		return new DefaultResourceResolverChain(getResourceResolvers());
-	}
-
-	private ResourceTransformerChain createTransformerChain(ResourceResolverChain resolverChain) {
-		return new DefaultResourceTransformerChain(resolverChain, getResourceTransformers());
-	}
-
 	/**
 	 * Set headers on the response. Called for both GET and HEAD requests.
 	 * @param exchange current exchange

spring-webflux/src/test/java/org/springframework/web/reactive/resource/EncodedResourceResolverTests.java

@@ -116,7 +116,6 @@ public class EncodedResourceResolverTests {
 	}
 
 	@Test
-	@Ignore // SPR-16862
 	public void resolveGzippedWithVersion() {
 
 		MockServerWebExchange exchange = MockServerWebExchange.from(

spring-webmvc/src/main/java/org/springframework/web/servlet/resource/DefaultResourceResolverChain.java

@@ -17,7 +17,9 @@
 package org.springframework.web.servlet.resource;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
+import java.util.ListIterator;
 import javax.servlet.http.HttpServletRequest;
 
 import org.springframework.core.io.Resource;
@@ -25,72 +27,63 @@ import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 
 /**
- * A default implementation of {@link ResourceResolverChain} for invoking a list
- * of {@link ResourceResolver}s.
+ * Default immutable implementation of {@link ResourceResolverChain}.
  *
- * @author Jeremy Grelle
  * @author Rossen Stoyanchev
- * @author Sam Brannen
  * @since 4.1
  */
 class DefaultResourceResolverChain implements ResourceResolverChain {
 
-	private final List<ResourceResolver> resolvers = new ArrayList<>();
+	@Nullable
+	private final ResourceResolver resolver;
 
-	private int index = -1;
+	@Nullable
+	private final ResourceResolverChain nextChain;
 
 
 	public DefaultResourceResolverChain(@Nullable List<? extends ResourceResolver> resolvers) {
-		if (resolvers != null) {
-			this.resolvers.addAll(resolvers);
-		}
+		resolvers = resolvers != null ? resolvers : Collections.emptyList();
+		DefaultResourceResolverChain chain = initChain(new ArrayList<>(resolvers));
+		this.resolver = chain.resolver;
+		this.nextChain = chain.nextChain;
 	}
 
+	private static DefaultResourceResolverChain initChain(ArrayList<? extends ResourceResolver> resolvers) {
+		DefaultResourceResolverChain chain = new DefaultResourceResolverChain(null, null);
+		ListIterator<? extends ResourceResolver> itr = resolvers.listIterator(resolvers.size());
+		while (itr.hasPrevious()) {
+			chain = new DefaultResourceResolverChain(itr.previous(), chain);
+		}
+		return chain;
+	}
 
-	@Override
-	@Nullable
-	public Resource resolveResource(
-			@Nullable HttpServletRequest request, String requestPath, List<? extends Resource> locations) {
+	private DefaultResourceResolverChain(@Nullable ResourceResolver resolver,
+			@Nullable ResourceResolverChain chain) {
 
-		ResourceResolver resolver = getNext();
-		if (resolver == null) {
-			return null;
-		}
+		Assert.isTrue((resolver == null && chain == null) || (resolver != null && chain != null),
+				"Both resolver and resolver chain must be null, or neither is");
 
-		try {
-			return resolver.resolveResource(request, requestPath, locations, this);
-		}
-		finally {
-			this.index--;
-		}
+		this.resolver = resolver;
+		this.nextChain = chain;
 	}
 
+
 	@Override
 	@Nullable
-	public String resolveUrlPath(String resourcePath, List<? extends Resource> locations) {
-		ResourceResolver resolver = getNext();
-		if (resolver == null) {
-			return null;
-		}
+	@SuppressWarnings("ConstantConditions")
+	public Resource resolveResource(@Nullable HttpServletRequest request, String requestPath,
+			List<? extends Resource> locations) {
 
-		try {
-			return resolver.resolveUrlPath(resourcePath, locations, this);
-		}
-		finally {
-			this.index--;
-		}
+		return this.resolver != null ?
+				this.resolver.resolveResource(request, requestPath, locations, this.nextChain) : null;
 	}
 
+	@Override
 	@Nullable
-	private ResourceResolver getNext() {
-		Assert.state(this.index <= this.resolvers.size(),
-				"Current index exceeds the number of configured ResourceResolvers");
-
-		if (this.index == (this.resolvers.size() - 1)) {
-			return null;
-		}
-		this.index++;
-		return this.resolvers.get(this.index);
+	@SuppressWarnings("ConstantConditions")
+	public String resolveUrlPath(String resourcePath, List<? extends Resource> locations) {
+		return this.resolver != null ?
+				this.resolver.resolveUrlPath(resourcePath, locations, this.nextChain) : null;
 	}
 
 }

spring-webmvc/src/main/java/org/springframework/web/servlet/resource/DefaultResourceTransformerChain.java

@@ -18,7 +18,9 @@ package org.springframework.web.servlet.resource;
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
+import java.util.ListIterator;
 import javax.servlet.http.HttpServletRequest;
 
 import org.springframework.core.io.Resource;
@@ -26,8 +28,7 @@ import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 
 /**
- * A default implementation of {@link ResourceTransformerChain} for invoking
- * a list of {@link ResourceTransformer}s.
+ * Default immutable implementation of {@link ResourceTransformerChain}.
  *
  * @author Rossen Stoyanchev
  * @since 4.1
@@ -36,9 +37,11 @@ class DefaultResourceTransformerChain implements ResourceTransformerChain {
 
 	private final ResourceResolverChain resolverChain;
 
-	private final List<ResourceTransformer> transformers = new ArrayList<>();
+	@Nullable
+	private final ResourceTransformer transformer;
 
-	private int index = -1;
+	@Nullable
+	private final ResourceTransformerChain nextChain;
 
 
 	public DefaultResourceTransformerChain(ResourceResolverChain resolverChain,
@@ -46,9 +49,33 @@ class DefaultResourceTransformerChain implements ResourceTransformerChain {
 
 		Assert.notNull(resolverChain, "ResourceResolverChain is required");
 		this.resolverChain = resolverChain;
-		if (transformers != null) {
-			this.transformers.addAll(transformers);
+
+		transformers = transformers != null ? transformers : Collections.emptyList();
+		DefaultResourceTransformerChain chain = initTransformerChain(resolverChain, new ArrayList<>(transformers));
+		this.transformer = chain.transformer;
+		this.nextChain = chain.nextChain;
+	}
+
+	private DefaultResourceTransformerChain initTransformerChain(ResourceResolverChain resolverChain,
+			ArrayList<ResourceTransformer> transformers) {
+
+		DefaultResourceTransformerChain chain = new DefaultResourceTransformerChain(resolverChain, null, null);
+		ListIterator<? extends ResourceTransformer> itr = transformers.listIterator(transformers.size());
+		while (itr.hasPrevious()) {
+			chain = new DefaultResourceTransformerChain(resolverChain, itr.previous(), chain);
 		}
+		return chain;
+	}
+
+	public DefaultResourceTransformerChain(ResourceResolverChain resolverChain,
+			@Nullable ResourceTransformer transformer, @Nullable ResourceTransformerChain chain) {
+
+		Assert.isTrue((transformer == null && chain == null) || (transformer != null && chain != null),
+				"Both transformer and transformer chain must be null, or neither is");
+
+		this.resolverChain = resolverChain;
+		this.transformer = transformer;
+		this.nextChain = chain;
 	}
 
 
@@ -58,30 +85,10 @@ class DefaultResourceTransformerChain implements ResourceTransformerChain {
 
 
 	@Override
+	@SuppressWarnings("ConstantConditions")
 	public Resource transform(HttpServletRequest request, Resource resource) throws IOException {
-		ResourceTransformer transformer = getNext();
-		if (transformer == null) {
-			return resource;
-		}
-
-		try {
-			return transformer.transform(request, resource, this);
-		}
-		finally {
-			this.index--;
-		}
-	}
-
-	@Nullable
-	private ResourceTransformer getNext() {
-		Assert.state(this.index <= this.transformers.size(),
-				"Current index exceeds the number of configured ResourceTransformers");
-
-		if (this.index == (this.transformers.size() - 1)) {
-			return null;
-		}
-		this.index++;
-		return this.transformers.get(this.index);
+		return transformer != null ?
+				this.transformer.transform(request, resource, this.nextChain) : resource;
 	}
 
 }

spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java

@@ -111,6 +111,12 @@ public class ResourceHttpRequestHandler extends WebContentGenerator
 
 	private final List<ResourceTransformer> resourceTransformers = new ArrayList<>(4);
 
+	@Nullable
+	private ResourceResolverChain resolverChain;
+
+	@Nullable
+	private ResourceTransformerChain transformerChain;
+
 	@Nullable
 	private ResourceHttpMessageConverter resourceHttpMessageConverter;
 
@@ -324,6 +330,10 @@ public class ResourceHttpRequestHandler extends WebContentGenerator
 
 		initAllowedLocations();
 
+		// Initialize immutable resolver and transformer chains
+		this.resolverChain = new DefaultResourceResolverChain(this.resourceResolvers);
+		this.transformerChain = new DefaultResourceTransformerChain(this.resolverChain, this.resourceTransformers);
+
 		if (this.resourceHttpMessageConverter == null) {
 			this.resourceHttpMessageConverter = new ResourceHttpMessageConverter();
 		}
@@ -525,15 +535,13 @@ public class ResourceHttpRequestHandler extends WebContentGenerator
 			return null;
 		}
 
-		ResourceResolverChain resolveChain = new DefaultResourceResolverChain(getResourceResolvers());
-		Resource resource = resolveChain.resolveResource(request, path, getLocations());
-		if (resource == null || getResourceTransformers().isEmpty()) {
-			return resource;
-		}
+		Assert.notNull(this.resolverChain, "ResourceResolverChain not initialized.");
+		Assert.notNull(this.transformerChain, "ResourceTransformerChain not initialized.");
 
-		ResourceTransformerChain transformChain =
-				new DefaultResourceTransformerChain(resolveChain, getResourceTransformers());
-		resource = transformChain.transform(request, resource);
+		Resource resource = this.resolverChain.resolveResource(request, path, getLocations());
+		if (resource != null) {
+			resource = this.transformerChain.transform(request, resource);
+		}
 		return resource;
 	}