Skip to content

Commit

Permalink
Nullability fine-tuning and related polishing
Browse files Browse the repository at this point in the history 
Issue: SPR-17250
  • Loading branch information
@jhoeller
jhoeller committed Oct 14, 2018
1 parent 5f2d47a commit 58b3af9
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 59 deletions.
7 changes: 2 additions & 5 deletions spring-web/src/main/java/org/springframework/http/ReadOnlyHttpHeaders.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
* {@code HttpHeaders} object that can only be read, not written to.
*
* @author Brian Clozel
* @since 5.1
* @since 5.1.1
*/
class ReadOnlyHttpHeaders extends HttpHeaders {

Expand Down Expand Up @@ -59,10 +59,7 @@ public MediaType getContentType() {
@Override
public List<String> get(Object key) {
List<String> values = this.headers.get(key);
if (values != null) {
return Collections.unmodifiableList(values);
}
return values;
return (values != null ? Collections.unmodifiableList(values) : null);
}

@Override
Expand Down
27 changes: 13 additions & 14 deletions spring-web/src/main/java/org/springframework/http/server/reactive/JettyHeadersAdapter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,18 @@
* {@code MultiValueMap} implementation for wrapping Jetty HTTP headers.
*
* @author Brian Clozel
* @since 5.1
* @since 5.1.1
*/
class JettyHeadersAdapter implements MultiValueMap<String, String> {

private final HttpFields headers;


JettyHeadersAdapter(HttpFields headers) {
this.headers = headers;
}


@Override
public String getFirst(String key) {
return this.headers.get(key);
Expand Down Expand Up @@ -95,24 +97,18 @@ public int size() {

@Override
public boolean isEmpty() {
return this.headers.size() == 0;
return (this.headers.size() == 0);
}

@Override
public boolean containsKey(Object key) {
if (key instanceof String) {
return this.headers.containsKey((String) key);
}
return false;
return (key instanceof String && this.headers.containsKey((String) key));
}

@Override
public boolean containsValue(Object value) {
if (value instanceof String) {
return this.headers.stream()
.anyMatch(field -> field.contains((String) value));
}
return false;
return (value instanceof String &&
this.headers.stream().anyMatch(field -> field.contains((String) value)));
}

@Nullable
Expand Down Expand Up @@ -144,8 +140,8 @@ public List<String> remove(Object key) {
}

@Override
public void putAll(Map<? extends String, ? extends List<String>> m) {
m.forEach(this::put);
public void putAll(Map<? extends String, ? extends List<String>> map) {
map.forEach(this::put);
}

@Override
Expand Down Expand Up @@ -179,6 +175,7 @@ public int size() {
};
}


private class EntryIterator implements Iterator<Entry<String, List<String>>> {

private Enumeration<String> names = headers.getFieldNames();
Expand All @@ -194,6 +191,7 @@ public Entry<String, List<String>> next() {
}
}


private class HeaderEntry implements Entry<String, List<String>> {

private final String key;
Expand All @@ -204,7 +202,7 @@ private class HeaderEntry implements Entry<String, List<String>> {

@Override
public String getKey() {
return this.key.toString();
return this.key;
}

@Override
Expand All @@ -219,4 +217,5 @@ public List<String> setValue(List<String> value) {
return previousValues;
}
}

}
5 changes: 5 additions & 0 deletions ...g-web/src/main/java/org/springframework/http/server/reactive/JettyHttpHandlerAdapter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,14 @@
import org.springframework.core.io.buffer.DataBufferFactory;
import org.springframework.http.HttpHeaders;
import org.springframework.http.MediaType;
import org.springframework.util.Assert;

/**
* {@link ServletHttpHandlerAdapter} extension that uses Jetty APIs for writing
* to the response with {@link ByteBuffer}.
*
* @author Violeta Georgieva
* @author Brian Clozel
* @since 5.0
* @see org.springframework.web.server.adapter.AbstractReactiveWebInitializer
*/
Expand All @@ -53,6 +55,8 @@ public JettyHttpHandlerAdapter(HttpHandler httpHandler) {
@Override
protected ServletServerHttpRequest createRequest(HttpServletRequest request, AsyncContext context)
throws IOException, URISyntaxException {

Assert.notNull(getServletPath(), "Servlet path is not initialized");
return new JettyServerHttpRequest(request, context, getServletPath(), getDataBufferFactory(), getBufferSize());
}

Expand All @@ -64,6 +68,7 @@ protected ServletServerHttpResponse createResponse(HttpServletResponse response,
response, context, getDataBufferFactory(), getBufferSize(), request);
}


private static final class JettyServerHttpRequest extends ServletServerHttpRequest {

JettyServerHttpRequest(HttpServletRequest request, AsyncContext asyncContext,
Expand Down
16 changes: 10 additions & 6 deletions spring-web/src/main/java/org/springframework/http/server/reactive/NettyHeadersAdapter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,18 @@
* {@code MultiValueMap} implementation for wrapping Netty HTTP headers.
*
* @author Brian Clozel
* @since 5.1
* @since 5.1.1
*/
class NettyHeadersAdapter implements MultiValueMap<String, String> {

private final HttpHeaders headers;


NettyHeadersAdapter(HttpHeaders headers) {
this.headers = headers;
}


@Override
@Nullable
public String getFirst(String key) {
Expand Down Expand Up @@ -99,14 +101,14 @@ public boolean isEmpty() {

@Override
public boolean containsKey(Object key) {
return (key instanceof String) && this.headers.contains((String) key);
return (key instanceof String && this.headers.contains((String) key));
}

@Override
public boolean containsValue(Object value) {
return (value instanceof String) &&
return (value instanceof String &&
this.headers.entries().stream()
.anyMatch(entry -> value != null && value.equals(entry.getValue()));
.anyMatch(entry -> value.equals(entry.getValue())));
}

@Override
Expand Down Expand Up @@ -138,8 +140,8 @@ public List<String> remove(Object key) {
}

@Override
public void putAll(Map<? extends String, ? extends List<String>> m) {
m.forEach(this.headers::add);
public void putAll(Map<? extends String, ? extends List<String>> map) {
map.forEach(this.headers::add);
}

@Override
Expand Down Expand Up @@ -173,6 +175,7 @@ public int size() {
};
}


private class EntryIterator implements Iterator<Entry<String, List<String>>> {

private Iterator<String> names = headers.names().iterator();
Expand All @@ -188,6 +191,7 @@ public Entry<String, List<String>> next() {
}
}


private class HeaderEntry implements Entry<String, List<String>> {

private final String key;
Expand Down
21 changes: 13 additions & 8 deletions spring-web/src/main/java/org/springframework/http/server/reactive/TomcatHeadersAdapter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,23 +38,25 @@
* {@code MultiValueMap} implementation for wrapping Tomcat HTTP headers.
*
* @author Brian Clozel
* @since 5.1
* @since 5.1.1
*/
class TomcatHeadersAdapter implements MultiValueMap<String, String> {

private final MimeHeaders headers;


TomcatHeadersAdapter(MimeHeaders headers) {
this.headers = headers;
}


@Override
public String getFirst(String key) {
return this.headers.getHeader(key);
}

@Override
public void add(String key, String value) {
public void add(String key, @Nullable String value) {
this.headers.addValue(key).setString(value);
}

Expand All @@ -69,7 +71,7 @@ public void addAll(MultiValueMap<String, String> values) {
}

@Override
public void set(String key, String value) {
public void set(String key, @Nullable String value) {
this.headers.setValue(key).setString(value);
}

Expand Down Expand Up @@ -98,13 +100,13 @@ public int size() {

@Override
public boolean isEmpty() {
return this.headers.size() == 0;
return (this.headers.size() == 0);
}

@Override
public boolean containsKey(Object key) {
if (key instanceof String) {
return this.headers.findHeader((String) key, 0) != -1;
return (this.headers.findHeader((String) key, 0) != -1);
}
return false;
}
Expand Down Expand Up @@ -152,8 +154,8 @@ public List<String> remove(Object key) {
}

@Override
public void putAll(Map<? extends String, ? extends List<String>> m) {
m.forEach(this::put);
public void putAll(Map<? extends String, ? extends List<String>> map) {
map.forEach(this::put);
}

@Override
Expand Down Expand Up @@ -191,6 +193,7 @@ public int size() {
};
}


private class EntryIterator implements Iterator<Entry<String, List<String>>> {

private Enumeration<String> names = headers.names();
Expand All @@ -206,11 +209,12 @@ public Entry<String, List<String>> next() {
}
}


private final class HeaderEntry implements Entry<String, List<String>> {

private final String key;

private HeaderEntry(String key) {
HeaderEntry(String key) {
this.key = key;
}

Expand All @@ -234,4 +238,5 @@ public List<String> setValue(List<String> value) {
return previous;
}
}

}
32 changes: 20 additions & 12 deletions ...-web/src/main/java/org/springframework/http/server/reactive/TomcatHttpHandlerAdapter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
*
* @author Violeta Georgieva
* @author Brian Clozel
* @author Brian Clozel
* @since 5.0
* @see org.springframework.web.server.adapter.AbstractReactiveWebInitializer
*/
Expand All @@ -64,7 +63,7 @@ public TomcatHttpHandlerAdapter(HttpHandler httpHandler) {
protected ServletServerHttpRequest createRequest(HttpServletRequest request, AsyncContext asyncContext)
throws IOException, URISyntaxException {

Assert.notNull(getServletPath(), "servletPath is not initialized.");
Assert.notNull(getServletPath(), "Servlet path is not initialized");
return new TomcatServerHttpRequest(
request, asyncContext, getServletPath(), getDataBufferFactory(), getBufferSize());
}
Expand All @@ -77,16 +76,20 @@ protected ServletServerHttpResponse createResponse(HttpServletResponse response,
response, asyncContext, getDataBufferFactory(), getBufferSize(), request);
}


private static final class TomcatServerHttpRequest extends ServletServerHttpRequest {

private static final Field COYOTE_REQUEST_FIELD = ReflectionUtils.findField(RequestFacade.class, "request");
private static final Field COYOTE_REQUEST_FIELD;

private final int bufferSize;

private final DataBufferFactory factory;

static {
ReflectionUtils.makeAccessible(COYOTE_REQUEST_FIELD);
Field field = ReflectionUtils.findField(RequestFacade.class, "request");
Assert.state(field != null, "Incompatible Tomcat implementation");
ReflectionUtils.makeAccessible(field);
COYOTE_REQUEST_FIELD = field;
}

TomcatServerHttpRequest(HttpServletRequest request, AsyncContext context,
Expand All @@ -99,8 +102,10 @@ private static final class TomcatServerHttpRequest extends ServletServerHttpRequ
}

private static HttpHeaders createTomcatHttpHeaders(HttpServletRequest request) {
Request tomcatRequest = ((org.apache.catalina.connector.Request) ReflectionUtils
.getField(COYOTE_REQUEST_FIELD, request)).getCoyoteRequest();
org.apache.catalina.connector.Request connectorRequest = (org.apache.catalina.connector.Request)
ReflectionUtils.getField(COYOTE_REQUEST_FIELD, request);
Assert.state(connectorRequest != null, "No Tomcat connector request");
Request tomcatRequest = connectorRequest.getCoyoteRequest();
TomcatHeadersAdapter headers = new TomcatHeadersAdapter(tomcatRequest.getMimeHeaders());
return new HttpHeaders(headers);
}
Expand All @@ -112,11 +117,9 @@ protected DataBuffer readFromInputStream() throws IOException {
DataBuffer dataBuffer = this.factory.allocateBuffer(capacity);
try {
ByteBuffer byteBuffer = dataBuffer.asByteBuffer(0, capacity);

ServletRequest request = getNativeRequest();
int read = ((CoyoteInputStream) request.getInputStream()).read(byteBuffer);
logBytesRead(read);

if (read > 0) {
dataBuffer.writePosition(read);
release = false;
Expand All @@ -140,10 +143,13 @@ else if (read == -1) {

private static final class TomcatServerHttpResponse extends ServletServerHttpResponse {

private static final Field COYOTE_RESPONSE_FIELD = ReflectionUtils.findField(ResponseFacade.class, "response");
private static final Field COYOTE_RESPONSE_FIELD;

static {
ReflectionUtils.makeAccessible(COYOTE_RESPONSE_FIELD);
Field field = ReflectionUtils.findField(ResponseFacade.class, "response");
Assert.state(field != null, "Incompatible Tomcat implementation");
ReflectionUtils.makeAccessible(field);
COYOTE_RESPONSE_FIELD = field;
}

TomcatServerHttpResponse(HttpServletResponse response, AsyncContext context,
Expand All @@ -153,8 +159,10 @@ private static final class TomcatServerHttpResponse extends ServletServerHttpRes
}

private static HttpHeaders createTomcatHttpHeaders(HttpServletResponse response) {
Response tomcatResponse = ((org.apache.catalina.connector.Response) ReflectionUtils
.getField(COYOTE_RESPONSE_FIELD, response)).getCoyoteResponse();
org.apache.catalina.connector.Response connectorResponse = (org.apache.catalina.connector.Response)
ReflectionUtils.getField(COYOTE_RESPONSE_FIELD, response);
Assert.state(connectorResponse != null, "No Tomcat connector response");
Response tomcatResponse = connectorResponse.getCoyoteResponse();
TomcatHeadersAdapter headers = new TomcatHeadersAdapter(tomcatResponse.getMimeHeaders());
return new HttpHeaders(headers);
}
Expand Down
Loading

0 comments on commit 58b3af9

Please sign in to comment.