From a71bd7c03f76237e486a184d0361b461f8b295a8 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 23 May 2018 09:53:26 -0400 Subject: [PATCH] Immutable Resource[Resolver|Transformer]Chains Backport of #f121aa5e31, applied to spring-webflux only. Issue: SPR-16862 --- .../DefaultResourceResolverChain.java | 79 ++++++++-------- .../DefaultResourceTransformerChain.java | 67 +++++++------- .../reactive/resource/ResourceWebHandler.java | 32 ++++--- .../resource/GzipResourceResolverTests.java | 90 ++++++++----------- .../foo-e36d2e05253c6c7085a91522ce43a0b4.css | 1 - 5 files changed, 129 insertions(+), 140 deletions(-) delete mode 100644 spring-webflux/src/test/resources/org/springframework/web/reactive/resource/test/foo-e36d2e05253c6c7085a91522ce43a0b4.css diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/DefaultResourceResolverChain.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/DefaultResourceResolverChain.java index 2098ad5f9c9..e2b56f2eba5 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/DefaultResourceResolverChain.java +++ b/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 resolvers = new ArrayList<>(); + @Nullable + private final ResourceResolver resolver; - private int index = -1; + @Nullable + private final ResourceResolverChain nextChain; public DefaultResourceResolverChain(@Nullable List 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 resolvers) { + DefaultResourceResolverChain chain = new DefaultResourceResolverChain(null, null); + ListIterator 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 resolveResource(@Nullable ServerWebExchange exchange, String requestPath, List 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 resolveUrlPath(String resourcePath, List 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(); } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/DefaultResourceTransformerChain.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/DefaultResourceTransformerChain.java index 5e35e7c1b6f..79f36350216 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/DefaultResourceTransformerChain.java +++ b/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 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 transformers) { + + DefaultResourceTransformerChain chain = new DefaultResourceTransformerChain(resolverChain, null, null); + ListIterator 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 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); } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java index 1d1782a392c..aff662bb8f6 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java @@ -97,6 +97,12 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { private final List resourceTransformers = new ArrayList<>(4); + @Nullable + private ResourceResolverChain resolverChain; + + @Nullable + private ResourceTransformerChain transformerChain; + @Nullable private CacheControl cacheControl; @@ -199,10 +205,17 @@ 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 +343,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 +482,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 diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/GzipResourceResolverTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/GzipResourceResolverTests.java index 85cb2a351d4..bc340550adb 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/GzipResourceResolverTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/GzipResourceResolverTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 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. @@ -38,13 +38,11 @@ import org.springframework.cache.concurrent.ConcurrentMapCache; import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.FileSystemResource; import org.springframework.core.io.Resource; -import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; import org.springframework.mock.web.test.server.MockServerWebExchange; import org.springframework.util.FileCopyUtils; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; +import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.*; /** @@ -65,7 +63,7 @@ public class GzipResourceResolverTests { @BeforeClass public static void createGzippedResources() throws IOException { createGzFile("/js/foo.js"); - createGzFile("foo-e36d2e05253c6c7085a91522ce43a0b4.css"); + createGzFile("foo.css"); } private static void createGzFile(String filePath) throws IOException { @@ -103,75 +101,59 @@ public class GzipResourceResolverTests { @Test - public void resolveGzippedFile() throws IOException { - MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("") - .header("Accept-Encoding", "gzip")); + public void resolveGzippedFile() { - String file = "js/foo.js"; - Resource resolved = this.resolver.resolveResource(exchange, file, this.locations).block(TIMEOUT); + MockServerWebExchange exchange = MockServerWebExchange.from(get("").header("Accept-Encoding", "gzip")); + Resource resolved = this.resolver.resolveResource(exchange, "js/foo.js", this.locations).block(TIMEOUT); - String gzFile = file+".gz"; - Resource resource = new ClassPathResource("test/" + gzFile, getClass()); - assertEquals(resource.getDescription(), resolved.getDescription()); - assertEquals(new ClassPathResource("test/" + file).getFilename(), resolved.getFilename()); - assertTrue("Expected " + resolved + " to be of type " + HttpResource.class, - resolved instanceof HttpResource); + assertEquals(getResource("js/foo.js.gz").getDescription(), resolved.getDescription()); + assertEquals(getResource("js/foo.js").getFilename(), resolved.getFilename()); + assertTrue(resolved instanceof HttpResource); } @Test - public void resolveFingerprintedGzippedFile() throws IOException { - MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("") - .header("Accept-Encoding", "gzip")); + public void resolveFingerprintedGzippedFile() { String file = "foo-e36d2e05253c6c7085a91522ce43a0b4.css"; + MockServerWebExchange exchange = MockServerWebExchange.from(get("").header("Accept-Encoding", "gzip")); Resource resolved = this.resolver.resolveResource(exchange, file, this.locations).block(TIMEOUT); - String gzFile = file + ".gz"; - Resource resource = new ClassPathResource("test/" + gzFile, getClass()); - assertEquals(resource.getDescription(), resolved.getDescription()); - assertEquals(new ClassPathResource("test/"+file).getFilename(), resolved.getFilename()); - assertTrue("Expected " + resolved + " to be of type " + HttpResource.class, - resolved instanceof HttpResource); + assertEquals(getResource("foo.css.gz").getDescription(), resolved.getDescription()); + assertEquals(getResource("foo.css").getFilename(), resolved.getFilename()); + assertTrue(resolved instanceof HttpResource); } @Test - public void resolveFromCacheWithEncodingVariants() throws IOException { - MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("") - .header("Accept-Encoding", "gzip")); + public void resolveFromCacheWithEncodingVariants() { - String file = "js/foo.js"; - Resource resolved = this.resolver.resolveResource(exchange, file, this.locations).block(TIMEOUT); + MockServerWebExchange exchange = MockServerWebExchange.from(get("").header("Accept-Encoding", "gzip")); + Resource resolved = this.resolver.resolveResource(exchange, "js/foo.js", this.locations).block(TIMEOUT); - String gzFile = file+".gz"; - Resource gzResource = new ClassPathResource("test/"+gzFile, getClass()); - assertEquals(gzResource.getDescription(), resolved.getDescription()); - assertEquals(new ClassPathResource("test/" + file).getFilename(), resolved.getFilename()); - assertTrue("Expected " + resolved + " to be of type " + HttpResource.class, - resolved instanceof HttpResource); + assertEquals(getResource("js/foo.js.gz").getDescription(), resolved.getDescription()); + assertEquals(getResource("js/foo.js").getFilename(), resolved.getFilename()); + assertTrue(resolved instanceof HttpResource); // resolved resource is now cached in CachingResourceResolver - exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/js/foo.js")); - resolved = this.resolver.resolveResource(exchange, file, this.locations).block(TIMEOUT); + exchange = MockServerWebExchange.from(get("/js/foo.js")); + resolved = this.resolver.resolveResource(exchange, "js/foo.js", this.locations).block(TIMEOUT); - Resource resource = new ClassPathResource("test/"+file, getClass()); - assertEquals(resource.getDescription(), resolved.getDescription()); - assertEquals(new ClassPathResource("test/" + file).getFilename(), resolved.getFilename()); - assertFalse("Expected " + resolved + " to *not* be of type " + HttpResource.class, - resolved instanceof HttpResource); + assertEquals(getResource("js/foo.js").getDescription(), resolved.getDescription()); + assertEquals(getResource("js/foo.js").getFilename(), resolved.getFilename()); + assertFalse(resolved instanceof HttpResource); } @Test // SPR-13149 - public void resolveWithNullRequest() throws IOException { - String file = "js/foo.js"; - Resource resolved = this.resolver.resolveResource(null, file, this.locations).block(TIMEOUT); - - String gzFile = file+".gz"; - Resource gzResource = new ClassPathResource("test/" + gzFile, getClass()); - assertEquals(gzResource.getDescription(), resolved.getDescription()); - assertEquals(new ClassPathResource("test/" + file).getFilename(), resolved.getFilename()); - assertTrue("Expected " + resolved + " to be of type " + HttpResource.class, - resolved instanceof HttpResource); + public void resolveWithNullRequest() { + Resource resolved = this.resolver.resolveResource(null, "js/foo.js", this.locations).block(TIMEOUT); + + assertEquals(getResource("js/foo.js.gz").getDescription(), resolved.getDescription()); + assertEquals(getResource("js/foo.js").getFilename(), resolved.getFilename()); + assertTrue(resolved instanceof HttpResource); + } + + private Resource getResource(String filePath) { + return new ClassPathResource("test/" + filePath, getClass()); } } diff --git a/spring-webflux/src/test/resources/org/springframework/web/reactive/resource/test/foo-e36d2e05253c6c7085a91522ce43a0b4.css b/spring-webflux/src/test/resources/org/springframework/web/reactive/resource/test/foo-e36d2e05253c6c7085a91522ce43a0b4.css deleted file mode 100644 index e2f0b1c742a..00000000000 --- a/spring-webflux/src/test/resources/org/springframework/web/reactive/resource/test/foo-e36d2e05253c6c7085a91522ce43a0b4.css +++ /dev/null @@ -1 +0,0 @@ -h1 { color:red; } \ No newline at end of file