From cfc68895d4e92c44a2258262b1460d3cfe9cdb65 Mon Sep 17 00:00:00 2001 From: Graeme Rocher Date: Fri, 22 Nov 2024 10:52:14 -0400 Subject: [PATCH] address CR --- .../JettyStaticResourceResolutionSpec.groovy | 16 ++++-- .../engine/DefaultServletHttpRequest.java | 56 +++++++------------ 2 files changed, 31 insertions(+), 41 deletions(-) diff --git a/http-server-jetty/src/test/groovy/io/micronaut/servlet/jetty/JettyStaticResourceResolutionSpec.groovy b/http-server-jetty/src/test/groovy/io/micronaut/servlet/jetty/JettyStaticResourceResolutionSpec.groovy index d3cf99249..b72d250a5 100644 --- a/http-server-jetty/src/test/groovy/io/micronaut/servlet/jetty/JettyStaticResourceResolutionSpec.groovy +++ b/http-server-jetty/src/test/groovy/io/micronaut/servlet/jetty/JettyStaticResourceResolutionSpec.groovy @@ -35,7 +35,6 @@ import static io.micronaut.http.HttpHeaders.CONTENT_LENGTH import static io.micronaut.http.HttpHeaders.CONTENT_TYPE @MicronautTest -@Property(name = "micronaut.servlet.async-supported", value = "false") class JettyStaticResourceResolutionSpec extends Specification implements TestPropertyProvider { private static Path tempDir @@ -86,6 +85,7 @@ class JettyStaticResourceResolutionSpec extends Specification implements TestPro then: response.status == HttpStatus.OK response.header(CONTENT_TYPE) == "text/html" + Integer.parseInt(response.header(CONTENT_LENGTH)) > 0 response.headers.contains(CACHE_CONTROL) response.header(CACHE_CONTROL) == "private,max-age=60" response.body() == "HTML Page from static file" @@ -103,6 +103,7 @@ class JettyStaticResourceResolutionSpec extends Specification implements TestPro file.exists() response.status == HttpStatus.OK response.header(CONTENT_TYPE) == "text/html" + Integer.parseInt(response.header(CONTENT_LENGTH)) > 0 response.headers.contains(CACHE_CONTROL) response.header(CACHE_CONTROL) == "private,max-age=60" @@ -133,10 +134,7 @@ class JettyStaticResourceResolutionSpec extends Specification implements TestPro EmbeddedServer embeddedServer = ApplicationContext.run(EmbeddedServer, [ 'micronaut.router.static-resources.default.paths': ['classpath:public', 'file:' + tempFile.parent], 'micronaut.router.static-resources.default.mapping': '/static/**']) - - def url = embeddedServer.getURL() - println("URL IS $url") - HttpClient rxClient = embeddedServer.applicationContext.createBean(HttpClient, url) + HttpClient rxClient = embeddedServer.applicationContext.createBean(HttpClient, embeddedServer.getURL()) when: @@ -149,6 +147,7 @@ class JettyStaticResourceResolutionSpec extends Specification implements TestPro file.exists() response.code() == HttpStatus.OK.code response.header(CONTENT_TYPE) == "text/html" + Integer.parseInt(response.header(CONTENT_LENGTH)) > 0 response.headers.contains(CACHE_CONTROL) response.body() == "HTML Page from resources" @@ -181,6 +180,7 @@ class JettyStaticResourceResolutionSpec extends Specification implements TestPro file.exists() response.code() == HttpStatus.OK.code response.header(CONTENT_TYPE) == "text/html" + Integer.parseInt(response.header(CONTENT_LENGTH)) > 0 response.headers.contains(CACHE_CONTROL) response.body() == "HTML Page from resources" @@ -214,6 +214,7 @@ class JettyStaticResourceResolutionSpec extends Specification implements TestPro file.exists() response.code() == HttpStatus.OK.code response.header(CONTENT_TYPE) == "text/html" + Integer.parseInt(response.header(CONTENT_LENGTH)) > 0 response.headers.contains(CACHE_CONTROL) response.body() == "HTML Page from resources" @@ -242,6 +243,7 @@ class JettyStaticResourceResolutionSpec extends Specification implements TestPro file.exists() response.code() == HttpStatus.OK.code response.header(CONTENT_TYPE) == "text/html" + Integer.parseInt(response.header(CONTENT_LENGTH)) > 0 response.body() == "HTML Page from resources/foo" and: 'the cache control header is not set' @@ -289,11 +291,13 @@ class JettyStaticResourceResolutionSpec extends Specification implements TestPro with(nestResponse) { code() == HttpStatus.OK.code header(CONTENT_TYPE) == "text/html" + Integer.parseInt(header(CONTENT_LENGTH)) > 0 body() == nestText } with(nestTestResponse) { code() == HttpStatus.OK.code + Integer.parseInt(header(CONTENT_LENGTH)) > 0 body() == nestTestText } @@ -331,12 +335,14 @@ class JettyStaticResourceResolutionSpec extends Specification implements TestPro with(nestResponse) { code() == HttpStatus.OK.code header(CONTENT_TYPE) == "text/html" + Integer.parseInt(header(CONTENT_LENGTH)) > 0 body() == nestText } with(publicResponse) { code() == HttpStatus.OK.code header(CONTENT_TYPE) == "text/html" + Integer.parseInt(header(CONTENT_LENGTH)) > 0 body() == publicText } diff --git a/servlet-engine/src/main/java/io/micronaut/servlet/engine/DefaultServletHttpRequest.java b/servlet-engine/src/main/java/io/micronaut/servlet/engine/DefaultServletHttpRequest.java index 239ac3386..50578496a 100644 --- a/servlet-engine/src/main/java/io/micronaut/servlet/engine/DefaultServletHttpRequest.java +++ b/servlet-engine/src/main/java/io/micronaut/servlet/engine/DefaultServletHttpRequest.java @@ -97,7 +97,6 @@ public final class DefaultServletHttpRequest implements ServerHttpRequest, ParsedBodyHolder { - private static final Logger LOG = LoggerFactory.getLogger(DefaultServletHttpRequest.class); private static final String NULL_KEY = "Attribute key cannot be null"; private final ConversionService conversionService; @@ -170,11 +169,7 @@ public Optional get(CharSequence name, ArgumentConversionContext conve Objects.requireNonNull(name, NULL_KEY); Object attribute = null; try { - if (asyncContext != null) { - attribute = asyncContext.getRequest().getAttribute(name.toString()); - } else { - attribute = delegate.getAttribute(name.toString()); - } + attribute = delegate().getAttribute(name.toString()); } catch (IllegalStateException e) { // ignore, request not longer active } @@ -185,7 +180,7 @@ public Optional get(CharSequence name, ArgumentConversionContext conve @Override public Set names() { try { - Enumeration attributeNames = asyncContext != null ? asyncContext.getRequest().getAttributeNames() : delegate.getAttributeNames(); + Enumeration attributeNames = delegate().getAttributeNames(); return CollectionUtils.enumerationToSet(attributeNames); } catch (IllegalStateException e) { // ignore, request no longer active @@ -196,12 +191,8 @@ public Set names() { @Override public Collection values() { try { - if (asyncContext != null) { - return names().stream() - .map(name -> asyncContext.getRequest().getAttribute(name)).toList(); - } else { - return names().stream().map(delegate::getAttribute).toList(); - } + ServletRequest request = delegate(); + return names().stream().map(request::getAttribute).toList(); } catch (IllegalStateException e) { // ignore, request no longer active return Collections.emptyList(); @@ -211,32 +202,21 @@ public Collection values() { @Override public MutableConvertibleValues put(CharSequence key, @Nullable Object value) { Objects.requireNonNull(key, NULL_KEY); - if (asyncContext != null) { - asyncContext.getRequest().setAttribute(key.toString(), value); - } else { - delegate.setAttribute(key.toString(), value); - } + delegate().setAttribute(key.toString(), value); return this; } @Override public MutableConvertibleValues remove(CharSequence key) { Objects.requireNonNull(key, NULL_KEY); - if (asyncContext != null) { - asyncContext.getRequest().removeAttribute(key.toString()); - } else { - delegate.removeAttribute(key.toString()); - } + delegate().removeAttribute(key.toString()); return this; } @Override public MutableConvertibleValues clear() { - if (asyncContext != null) { - names().forEach(name -> asyncContext.getRequest().removeAttribute(name)); - } else { - names().forEach(delegate::removeAttribute); - } + ServletRequest request = delegate(); + names().forEach(request::removeAttribute); return this; } }; @@ -300,56 +280,60 @@ public Optional getUserPrincipal() { @Override public boolean isSecure() { - return asyncContext != null ? asyncContext.getRequest().isSecure() : delegate.isSecure(); + return delegate().isSecure(); } @NonNull @Override public Optional getContentType() { - String contentType = asyncContext != null ? asyncContext.getRequest().getContentType() : delegate.getContentType(); + String contentType = delegate().getContentType(); return Optional.ofNullable(contentType) .map(MediaType::new); } @Override public long getContentLength() { - return asyncContext != null ? asyncContext.getRequest().getContentLength() : delegate.getContentLength(); + return delegate().getContentLength(); } @NonNull @Override public InetSocketAddress getRemoteAddress() { - ServletRequest servletRequest = asyncContext != null ? asyncContext.getRequest() : delegate; + ServletRequest servletRequest = delegate(); return new InetSocketAddress( servletRequest.getRemoteHost(), servletRequest.getRemotePort() ); } + private ServletRequest delegate() { + return asyncContext != null ? asyncContext.getRequest() : delegate; + } + @NonNull @Override public InetSocketAddress getServerAddress() { return new InetSocketAddress( - asyncContext != null ? asyncContext.getRequest().getServerPort() : delegate.getServerPort() + delegate().getServerPort() ); } @Nullable @Override public String getServerName() { - return asyncContext != null ? asyncContext.getRequest().getServerName() : delegate.getServerName(); + return delegate().getServerName(); } @Override @NonNull public Optional getLocale() { - return Optional.ofNullable(asyncContext != null ? asyncContext.getRequest().getLocale() : delegate.getLocale()); + return Optional.ofNullable(delegate().getLocale()); } @NonNull @Override public Charset getCharacterEncoding() { - String characterEncoding = asyncContext != null ? asyncContext.getRequest().getCharacterEncoding() : delegate.getCharacterEncoding(); + String characterEncoding = delegate().getCharacterEncoding(); return Optional.ofNullable(characterEncoding) .map(Charset::forName) .orElse(StandardCharsets.UTF_8);