From f6525fb1532fc54ba504dcdf55123a06bd655756 Mon Sep 17 00:00:00 2001 From: Oliver Verlinden Date: Mon, 22 Jun 2015 11:09:56 +0200 Subject: [PATCH 1/8] Convert member variables to final --- .../java/net/gescobar/httpserver/HttpConnection.java | 10 +++++----- src/main/java/net/gescobar/httpserver/HttpServer.java | 2 +- src/main/java/net/gescobar/httpserver/Response.java | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/java/net/gescobar/httpserver/HttpConnection.java b/src/main/java/net/gescobar/httpserver/HttpConnection.java index 0b2aaa5..aa13ae8 100644 --- a/src/main/java/net/gescobar/httpserver/HttpConnection.java +++ b/src/main/java/net/gescobar/httpserver/HttpConnection.java @@ -20,17 +20,17 @@ */ public class HttpConnection implements Runnable { - private Logger log = LoggerFactory.getLogger(HttpConnection.class); + private final Logger log = LoggerFactory.getLogger(HttpConnection.class); /** * The socket with the underlying connection. */ - private Socket socket; + private final Socket socket; /** * The implementation that will handle requests. */ - private Handler handler; + private final Handler handler; /** * Constructor. @@ -87,12 +87,12 @@ private class RequestImpl implements Request { /** * The HTTP method */ - private String method; + private final String method; /** * The request path */ - private String path; + private final String path; /** * The request headers diff --git a/src/main/java/net/gescobar/httpserver/HttpServer.java b/src/main/java/net/gescobar/httpserver/HttpServer.java index f353bed..7e2154d 100644 --- a/src/main/java/net/gescobar/httpserver/HttpServer.java +++ b/src/main/java/net/gescobar/httpserver/HttpServer.java @@ -33,7 +33,7 @@ */ public class HttpServer { - private Logger log = LoggerFactory.getLogger(HttpServer.class); + private final Logger log = LoggerFactory.getLogger(HttpServer.class); /** * The default port to use unless other is specified. diff --git a/src/main/java/net/gescobar/httpserver/Response.java b/src/main/java/net/gescobar/httpserver/Response.java index f6a2aaf..d3b36d3 100644 --- a/src/main/java/net/gescobar/httpserver/Response.java +++ b/src/main/java/net/gescobar/httpserver/Response.java @@ -28,9 +28,9 @@ public enum HttpStatus { OVERLOADED(502, "Overloaded"), GATEWAY_TIMEOUT(503, "Gateway Timeout"); - private int code; + private final int code; - private String reason; + private final String reason; private HttpStatus(int code, String reason) { this.code = code; From 4f6270db2ef0b266408f12bb5da61ea5087f9787 Mon Sep 17 00:00:00 2001 From: Oliver Verlinden Date: Mon, 22 Jun 2015 11:28:10 +0200 Subject: [PATCH 2/8] Extract inner classes --- .../gescobar/httpserver/HttpConnection.java | 175 ------------------ .../net/gescobar/httpserver/RequestImpl.java | 136 ++++++++++++++ .../net/gescobar/httpserver/ResponseImpl.java | 49 +++++ 3 files changed, 185 insertions(+), 175 deletions(-) create mode 100644 src/main/java/net/gescobar/httpserver/RequestImpl.java create mode 100644 src/main/java/net/gescobar/httpserver/ResponseImpl.java diff --git a/src/main/java/net/gescobar/httpserver/HttpConnection.java b/src/main/java/net/gescobar/httpserver/HttpConnection.java index aa13ae8..04bbf99 100644 --- a/src/main/java/net/gescobar/httpserver/HttpConnection.java +++ b/src/main/java/net/gescobar/httpserver/HttpConnection.java @@ -72,179 +72,4 @@ public void run() { } - /** - * This is an internal implementation of the {@link Request} interface. - * - * @author German Escobar - */ - private class RequestImpl implements Request { - - /** - * The Host header. - */ - private String host; - - /** - * The HTTP method - */ - private final String method; - - /** - * The request path - */ - private final String path; - - /** - * The request headers - */ - private Map headers; - - /** - * Constructor. - * - * @param reader from which we are reading the headers. - * - * @throws IOException if an I/O error occurs in the underlying connection. - */ - public RequestImpl(BufferedReader reader) throws IOException { - - String request = reader.readLine(); - - // get the method and the path - method = request.split(" ")[0]; - path = request.split(" ")[1]; - - // get the headers - headers = retrieveHeaders(reader); - - } - - /** - * Helper method. Retrieves the headers of the request. - * - * @param reader the reader from which we are retrieving the request information. - * - * @return a Map object with the headers of the request. - * @throws IOException if an I/O error occurs in the underlying communication. - */ - private Map retrieveHeaders(BufferedReader reader) throws IOException { - - Map headers = new HashMap(); - - // iterate through the headers - String headerLine = reader.readLine(); - while( !headerLine.equals("") ) { - - // headers come in the form "name: value" - String name = headerLine.split(":")[0].trim(); - String value = headerLine.split(":")[1].trim(); - - // add to the headers only if there is no corresponding field (e.g. "Host" header is mapped to the - // *host* field of the request) - if ( !isKnownHeader(name, value) ) { - headers.put(name, value); - } - - // read next line - headerLine = reader.readLine(); - } - - return headers; - - } - - /** - * Checks if it is a known header and sets the corresponding field. - * - * @param name the name of the header to check. - * @param value the value of the header to check. - * - * @return true if it is a known header, false otherwise - */ - private boolean isKnownHeader(String name, String value) { - - boolean ret = false; - - if (name.equalsIgnoreCase("host")) { - host = value; - return true; - } - - return ret; - - } - - @Override - public String getMethod() { - return method; - } - - @Override - public String getPath() { - return path; - } - - @Override - public String getHost() { - return host; - } - - @Override - public Map getHeaders() { - return headers; - } - - @Override - public String getHeader(String name) { - return headers.get(name); - } - - } - - /** - * This is a private implementation of the {@link Response interface} - * - * @author German Escobar - */ - private class ResponseImpl implements Response { - - private HttpStatus status = HttpStatus.OK; - - private String contentType; - - @Override - public Response status(HttpStatus status) { - this.status = status; - - return this; - } - - @Override - public Response ok() { - return status(HttpStatus.OK); - } - - @Override - public Response notFound() { - return status(HttpStatus.NOT_FOUND); - } - - @Override - public Response contentType(String contentType) { - this.contentType = contentType; - - return this; - } - - public String toString() { - String ret = "HTTP/1.1 " + status.getCode() + " " + status.getReason() + "\r\n"; - - if (contentType != null) { - ret += "Content-Type: " + contentType + "\r\n"; - } - - return ret + "\r\n"; - } - - } } diff --git a/src/main/java/net/gescobar/httpserver/RequestImpl.java b/src/main/java/net/gescobar/httpserver/RequestImpl.java new file mode 100644 index 0000000..366b64c --- /dev/null +++ b/src/main/java/net/gescobar/httpserver/RequestImpl.java @@ -0,0 +1,136 @@ +package net.gescobar.httpserver; + +import java.io.BufferedReader; +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; + + +/** + * This is an internal implementation of the {@link Request} interface. + * + * @author German Escobar + */ +public class RequestImpl implements Request { + + /** + * The Host header. + */ + private String host; + + /** + * The HTTP method + */ + private final String method; + + /** + * The request path + */ + private final String path; + + /** + * The request headers + */ + private Map headers; + + /** + * Constructor. + * + * @param reader from which we are reading the headers. + * + * @throws IOException if an I/O error occurs in the underlying connection. + */ + public RequestImpl(BufferedReader reader) throws IOException { + + String request = reader.readLine(); + + // get the method and the path + method = request.split(" ")[0]; + path = request.split(" ")[1]; + + // get the headers + headers = retrieveHeaders(reader); + + } + + /** + * Helper method. Retrieves the headers of the request. + * + * @param reader the reader from which we are retrieving the request information. + * + * @return a Map object with the headers of the request. + * @throws IOException if an I/O error occurs in the underlying communication. + */ + private Map retrieveHeaders(BufferedReader reader) throws IOException { + + Map headers = new HashMap(); + + // iterate through the headers + String headerLine = reader.readLine(); + while( !headerLine.equals("") ) { + + // headers come in the form "name: value" + String name = headerLine.split(":")[0].trim(); + String value = headerLine.split(":")[1].trim(); + + // add to the headers only if there is no corresponding field (e.g. "Host" header is mapped to the + // *host* field of the request) + if ( !isKnownHeader(name, value) ) { + headers.put(name, value); + } + + // read next line + headerLine = reader.readLine(); + } + + return headers; + + } + + /** + * Checks if it is a known header and sets the corresponding field. + * + * @param name the name of the header to check. + * @param value the value of the header to check. + * + * @return true if it is a known header, false otherwise + */ + private boolean isKnownHeader(String name, String value) { + + boolean ret = false; + + if (name.equalsIgnoreCase("host")) { + host = value; + return true; + } + + return ret; + + } + + @Override + public String getMethod() { + return method; + } + + @Override + public String getPath() { + return path; + } + + @Override + public String getHost() { + return host; + } + + @Override + public Map getHeaders() { + return headers; + } + + @Override + public String getHeader(String name) { + return headers.get(name); + } + +} \ No newline at end of file diff --git a/src/main/java/net/gescobar/httpserver/ResponseImpl.java b/src/main/java/net/gescobar/httpserver/ResponseImpl.java new file mode 100644 index 0000000..e4dcc93 --- /dev/null +++ b/src/main/java/net/gescobar/httpserver/ResponseImpl.java @@ -0,0 +1,49 @@ + +package net.gescobar.httpserver; + +/** + * This is a private implementation of the {@link Response interface} + * + * @author German Escobar + */ +public class ResponseImpl implements Response { + + private Response.HttpStatus status = Response.HttpStatus.OK; + + private String contentType; + + @Override + public Response status(Response.HttpStatus status) { + this.status = status; + + return this; + } + + @Override + public Response ok() { + return status(Response.HttpStatus.OK); + } + + @Override + public Response notFound() { + return status(Response.HttpStatus.NOT_FOUND); + } + + @Override + public Response contentType(String contentType) { + this.contentType = contentType; + + return this; + } + + public String toString() { + String ret = "HTTP/1.1 " + status.getCode() + " " + status.getReason() + "\r\n"; + + if (contentType != null) { + ret += "Content-Type: " + contentType + "\r\n"; + } + + return ret + "\r\n"; + } + +} From d715cdeba1e156dff9272d1b94433c4a9ddfdb8b Mon Sep 17 00:00:00 2001 From: Oliver Verlinden Date: Mon, 22 Jun 2015 11:29:32 +0200 Subject: [PATCH 3/8] Use volatile running flag --- src/main/java/net/gescobar/httpserver/HttpServer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/net/gescobar/httpserver/HttpServer.java b/src/main/java/net/gescobar/httpserver/HttpServer.java index 7e2154d..60e8580 100644 --- a/src/main/java/net/gescobar/httpserver/HttpServer.java +++ b/src/main/java/net/gescobar/httpserver/HttpServer.java @@ -68,7 +68,7 @@ public class HttpServer { /** * Tells if the server is running or not. */ - private boolean running; + private volatile boolean running; /** * Constructor. Initializes the server with the default port and {@link Handler} implementation. From 623f59c40cbee65c100ebfcf10b473d043a89dff Mon Sep 17 00:00:00 2001 From: Oliver Verlinden Date: Mon, 22 Jun 2015 11:38:50 +0200 Subject: [PATCH 4/8] Ebable and use try with resources --- pom.xml | 4 ++-- .../java/net/gescobar/httpserver/HttpConnection.java | 10 ++++------ 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/pom.xml b/pom.xml index 12d48c2..6394361 100644 --- a/pom.xml +++ b/pom.xml @@ -60,8 +60,8 @@ 2.3.2 true - 1.6 - 1.6 + 1.7 + 1.7 false true true diff --git a/src/main/java/net/gescobar/httpserver/HttpConnection.java b/src/main/java/net/gescobar/httpserver/HttpConnection.java index 04bbf99..c5f3161 100644 --- a/src/main/java/net/gescobar/httpserver/HttpConnection.java +++ b/src/main/java/net/gescobar/httpserver/HttpConnection.java @@ -48,18 +48,16 @@ public void run() { log.trace("handling HTTP request ... "); - try { - - InputStream is = socket.getInputStream(); - BufferedReader reader = new BufferedReader( new InputStreamReader(is) ); - + try (InputStream is = socket.getInputStream(); + BufferedReader reader = new BufferedReader( new InputStreamReader(is) ); + OutputStream os = socket.getOutputStream();) { + // build the request and response object RequestImpl request = new RequestImpl(reader); ResponseImpl response = new ResponseImpl(); handler.handle(request, response); - OutputStream os = socket.getOutputStream(); os.write(response.toString().getBytes()); os.flush(); os.close(); From 96937922760062e65666711692cbbb9daf0f5f26 Mon Sep 17 00:00:00 2001 From: Oliver Verlinden Date: Mon, 22 Jun 2015 12:33:36 +0200 Subject: [PATCH 5/8] Remove unused imports --- src/main/java/net/gescobar/httpserver/HttpConnection.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/java/net/gescobar/httpserver/HttpConnection.java b/src/main/java/net/gescobar/httpserver/HttpConnection.java index c5f3161..47f1196 100644 --- a/src/main/java/net/gescobar/httpserver/HttpConnection.java +++ b/src/main/java/net/gescobar/httpserver/HttpConnection.java @@ -6,8 +6,6 @@ import java.io.InputStreamReader; import java.io.OutputStream; import java.net.Socket; -import java.util.HashMap; -import java.util.Map; import org.slf4j.Logger; import org.slf4j.LoggerFactory; From b05777c40bd82a014b61533b4b4741efd6be1818 Mon Sep 17 00:00:00 2001 From: Oliver Verlinden Date: Mon, 22 Jun 2015 12:49:28 +0200 Subject: [PATCH 6/8] Extract inner class --- .../gescobar/httpserver/HttpServerTest.java | 22 ------------------ .../net/gescobar/httpserver/TestHandler.java | 23 +++++++++++++++++++ 2 files changed, 23 insertions(+), 22 deletions(-) create mode 100644 src/test/java/net/gescobar/httpserver/TestHandler.java diff --git a/src/test/java/net/gescobar/httpserver/HttpServerTest.java b/src/test/java/net/gescobar/httpserver/HttpServerTest.java index 79141a6..6363450 100644 --- a/src/test/java/net/gescobar/httpserver/HttpServerTest.java +++ b/src/test/java/net/gescobar/httpserver/HttpServerTest.java @@ -123,26 +123,4 @@ public void doHandle(Request request, Response response) { } - - private class TestHandler implements Handler { - - private Request request; - - @Override - public final void handle(Request request, Response response) { - this.request = request; - - doHandle(request, response); - } - - public void doHandle(Request request, Response response) { - - } - - public Request getRequest() { - return request; - } - - } - } diff --git a/src/test/java/net/gescobar/httpserver/TestHandler.java b/src/test/java/net/gescobar/httpserver/TestHandler.java new file mode 100644 index 0000000..23f1a22 --- /dev/null +++ b/src/test/java/net/gescobar/httpserver/TestHandler.java @@ -0,0 +1,23 @@ + +package net.gescobar.httpserver; + +class TestHandler implements Handler { + + private Request request; + + @Override + public final void handle(Request request, Response response) { + this.request = request; + + doHandle(request, response); + } + + public void doHandle(Request request, Response response) { + + } + + public Request getRequest() { + return request; + } + +} From a9fe80d9384d6d3b80814631cfca795103f313a2 Mon Sep 17 00:00:00 2001 From: Oliver Verlinden Date: Mon, 22 Jun 2015 12:49:54 +0200 Subject: [PATCH 7/8] Restrict class visibility --- src/main/java/net/gescobar/httpserver/RequestImpl.java | 2 +- src/main/java/net/gescobar/httpserver/ResponseImpl.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/gescobar/httpserver/RequestImpl.java b/src/main/java/net/gescobar/httpserver/RequestImpl.java index 366b64c..e3741da 100644 --- a/src/main/java/net/gescobar/httpserver/RequestImpl.java +++ b/src/main/java/net/gescobar/httpserver/RequestImpl.java @@ -11,7 +11,7 @@ * * @author German Escobar */ -public class RequestImpl implements Request { +class RequestImpl implements Request { /** * The Host header. diff --git a/src/main/java/net/gescobar/httpserver/ResponseImpl.java b/src/main/java/net/gescobar/httpserver/ResponseImpl.java index e4dcc93..413d46d 100644 --- a/src/main/java/net/gescobar/httpserver/ResponseImpl.java +++ b/src/main/java/net/gescobar/httpserver/ResponseImpl.java @@ -6,7 +6,7 @@ * * @author German Escobar */ -public class ResponseImpl implements Response { +class ResponseImpl implements Response { private Response.HttpStatus status = Response.HttpStatus.OK; From 3f3fb597fca12467fbbaa93417f904aa4b946a27 Mon Sep 17 00:00:00 2001 From: Oliver Verlinden Date: Mon, 22 Jun 2015 13:07:45 +0200 Subject: [PATCH 8/8] Add @Override annotation, make member variable final --- src/main/java/net/gescobar/httpserver/RequestImpl.java | 2 +- src/main/java/net/gescobar/httpserver/ResponseImpl.java | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/gescobar/httpserver/RequestImpl.java b/src/main/java/net/gescobar/httpserver/RequestImpl.java index e3741da..bfb9c25 100644 --- a/src/main/java/net/gescobar/httpserver/RequestImpl.java +++ b/src/main/java/net/gescobar/httpserver/RequestImpl.java @@ -31,7 +31,7 @@ class RequestImpl implements Request { /** * The request headers */ - private Map headers; + private final Map headers; /** * Constructor. diff --git a/src/main/java/net/gescobar/httpserver/ResponseImpl.java b/src/main/java/net/gescobar/httpserver/ResponseImpl.java index 413d46d..fffc6cd 100644 --- a/src/main/java/net/gescobar/httpserver/ResponseImpl.java +++ b/src/main/java/net/gescobar/httpserver/ResponseImpl.java @@ -36,6 +36,7 @@ public Response contentType(String contentType) { return this; } + @Override public String toString() { String ret = "HTTP/1.1 " + status.getCode() + " " + status.getReason() + "\r\n";