diff options
author | Bjørn Christian Seime <bjorncs@yahoo-inc.com> | 2016-06-21 12:58:36 +0200 |
---|---|---|
committer | Bjørn Christian Seime <bjorncs@yahoo-inc.com> | 2016-06-21 12:58:36 +0200 |
commit | 646c1ba0de9846d99aa8318c8183459fc6fcd388 (patch) | |
tree | 107e39d4eb1d4ea85d12521a3870ce9fc582b460 | |
parent | f6773e61e61b7147226b999d2a904ce179030603 (diff) |
Added some TODOs to the code.
2 files changed, 12 insertions, 0 deletions
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletOutputStreamWriter.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletOutputStreamWriter.java index 271805765c2..7a15107a3eb 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletOutputStreamWriter.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletOutputStreamWriter.java @@ -41,6 +41,10 @@ public class ServletOutputStreamWriter { private static final Logger log = Logger.getLogger(ServletOutputStreamWriter.class.getName()); + // TODO: This reference is not guaranteed to be unique; ByteBuffer.allocate(0) MAY in principle return a singleton! + // If so, application code could fake a close by writing such a byte buffer. + // The problem can be solved by filtering out zero-length byte buffers from application code. + // Other ways to express this are also possible, e.g. with a 'closed' state checked when queue goes empty. private static final ByteBuffer CLOSE_STREAM_BUFFER = ByteBuffer.allocate(0); private final Object monitor = new Object(); @@ -74,6 +78,7 @@ public class ServletOutputStreamWriter { public void setSendingError() { synchronized (monitor) { + // TODO: This assert seems fishy. Investigate. assertStateIs(state, State.NOT_STARTED); state = State.FINISHED_OR_ERROR; } @@ -109,6 +114,7 @@ public class ServletOutputStreamWriter { } if (thisThreadShouldWrite) { + // TODO: Consider refactoring to avoid multiple monitor entry-exit. writeBuffersInQueueToOutputStream(); } } diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java index b0781c402d5..126e4fee9e6 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java @@ -115,6 +115,8 @@ public class ServletResponseController { } try { + // TODO: sendError() is a synchronous call. Refactor. (Also, we should control the response content - + // this method generates a response body based on Jetty's own response templates ("Powered by Jetty"). servletResponse.sendError( statusCode, reasonPhrase); @@ -166,10 +168,14 @@ public class ServletResponseController { private static void setStatus_holdingLock(Response jdiscResponse, HttpServletResponse servletResponse) { if (jdiscResponse instanceof HttpResponse) { + // TODO: Figure out what this does to the response (with Jetty), and move to non-deprecated APIs. + // Deprecate our own code as necessary. servletResponse.setStatus(jdiscResponse.getStatus(), ((HttpResponse) jdiscResponse).getMessage()); } else { Optional<String> errorMessage = getErrorMessage(jdiscResponse); if (errorMessage.isPresent()) { + // TODO: Figure out what this does to the response (with Jetty), and move to non-deprecated APIs. + // Deprecate our own code as necessary. servletResponse.setStatus(jdiscResponse.getStatus(), errorMessage.get()); } else { servletResponse.setStatus(jdiscResponse.getStatus()); |