Skip to content

Commit

Permalink
address CR
Browse files Browse the repository at this point in the history
  • Loading branch information
@graemerocher
graemerocher committed Nov 22, 2024
1 parent 50599a8 commit cfc6889
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 41 deletions.
16 changes: 11 additions & 5 deletions ...jetty/src/test/groovy/io/micronaut/servlet/jetty/JettyStaticResourceResolutionSpec.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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><head></head><body>HTML Page from static file</body></html>"
Expand All @@ -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"

Expand Down Expand Up @@ -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:
Expand All @@ -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><head></head><body>HTML Page from resources</body></html>"
Expand Down Expand Up @@ -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><head></head><body>HTML Page from resources</body></html>"
Expand Down Expand Up @@ -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><head></head><body>HTML Page from resources</body></html>"

Expand Down Expand Up @@ -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><head></head><body>HTML Page from resources/foo</body></html>"

and: 'the cache control header is not set'
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down
56 changes: 20 additions & 36 deletions servlet-engine/src/main/java/io/micronaut/servlet/engine/DefaultServletHttpRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ public final class DefaultServletHttpRequest<B> implements
ServerHttpRequest<B>,
ParsedBodyHolder<B> {

private static final Logger LOG = LoggerFactory.getLogger(DefaultServletHttpRequest.class);
private static final String NULL_KEY = "Attribute key cannot be null";

private final ConversionService conversionService;
Expand Down Expand Up @@ -170,11 +169,7 @@ public <T> Optional<T> get(CharSequence name, ArgumentConversionContext<T> 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
}
Expand All @@ -185,7 +180,7 @@ public <T> Optional<T> get(CharSequence name, ArgumentConversionContext<T> conve
@Override
public Set<String> names() {
try {
Enumeration<String> attributeNames = asyncContext != null ? asyncContext.getRequest().getAttributeNames() : delegate.getAttributeNames();
Enumeration<String> attributeNames = delegate().getAttributeNames();
return CollectionUtils.enumerationToSet(attributeNames);
} catch (IllegalStateException e) {
// ignore, request no longer active
Expand All @@ -196,12 +191,8 @@ public Set<String> names() {
@Override
public Collection<Object> 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();
Expand All @@ -211,32 +202,21 @@ public Collection<Object> values() {
@Override
public MutableConvertibleValues<Object> 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<Object> 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<Object> 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;
}
};
Expand Down Expand Up @@ -300,56 +280,60 @@ public Optional<Principal> getUserPrincipal() {

@Override
public boolean isSecure() {
return asyncContext != null ? asyncContext.getRequest().isSecure() : delegate.isSecure();
return delegate().isSecure();
}

@NonNull
@Override
public Optional<MediaType> 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<Locale> 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);
Expand Down

0 comments on commit cfc6889

Please sign in to comment.