Refactor to have MethodSupplier abstract class, and make ResponderSupplier use it

onlyskin · onlyskin · commit dfbb70b19af0 · 2018-01-08T17:43:45.000+02:00
diff --git a/src/main/java/httpserver/ResponderSupplier.java b/src/main/java/httpserver/ResponderSupplier.java
@@ -1,30 +1,34 @@
 package httpserver;
 
-import httpserver.responder.Responder;
+import httpserver.request.Request;
+import httpserver.responder.MethodResponder;
 
-import java.util.HashMap;
-import java.util.Map;
+import java.util.ArrayList;
+import java.util.List;
 
 public class ResponderSupplier {
-    private final Responder invalidMethodResponder;
-    private final Map<Method, Responder> methodResponderMap;
+    private final MethodResponder invalidMethodResponder;
+    private final List<MethodResponder> methodResponders;
 
-    public ResponderSupplier(Responder invalidMethodResponder) {
+    public ResponderSupplier(MethodResponder invalidMethodResponder) {
         this.invalidMethodResponder = invalidMethodResponder;
-        this.methodResponderMap = new HashMap<>();
+        this.methodResponders = new ArrayList<>();
     }
 
-    public Responder responderForMethodString(String methodString) {
-        Method method;
-        try {
-            method = Method.valueOf(methodString);
-        } catch (IllegalArgumentException e) {
-            return invalidMethodResponder;
+    public MethodResponder supplyResponder(Request request) {
+        for (MethodResponder methodResponder: methodResponders) {
+            if (methodResponder.handles(request)) {
+                return methodResponder;
+            }
         }
-        return methodResponderMap.get(method);
+        return invalidMethodResponder;
     }
 
-    public void registerResponder(Method method, Responder responder) {
-        this.methodResponderMap.put(method, responder);
+    public void registerResponder(MethodResponder methodResponder) {
+        this.methodResponders.add(methodResponder);
+    }
+
+    public List<MethodResponder> allResponders() {
+        return methodResponders;
     }
 }
diff --git a/src/main/java/httpserver/ResponderSupplierFactory.java b/src/main/java/httpserver/ResponderSupplierFactory.java
@@ -18,30 +18,30 @@ public ResponderSupplier makeResponderSupplier() {
         RangeHeaderValueParser rangeHeaderValueParser = new RangeHeaderValueParser();
         Hasher hasher = new Hasher();
 
-        responderSupplier.registerResponder(Method.GET, new GetResponder(
+        responderSupplier.registerResponder(new GetResponder(
                 routeMap,
                 pathExaminer,
                 html,
                 rangeHeaderValueParser));
-        responderSupplier.registerResponder(Method.HEAD, new HeadResponder(
+        responderSupplier.registerResponder(new HeadResponder(
                 routeMap,
                 pathExaminer,
                 html,
                 rangeHeaderValueParser));
-        responderSupplier.registerResponder(Method.POST, new PostResponder(
+        responderSupplier.registerResponder(new PostResponder(
                 pathExaminer,
                 fileOperator));
-        responderSupplier.registerResponder(Method.PUT, new PutResponder(
+        responderSupplier.registerResponder(new PutResponder(
                 pathExaminer,
                 fileOperator));
-        responderSupplier.registerResponder(Method.PATCH, new PatchResponder(
+        responderSupplier.registerResponder(new PatchResponder(
                 pathExaminer,
                 fileOperator,
                 hasher));
-        responderSupplier.registerResponder(Method.DELETE, new DeleteResponder(
+        responderSupplier.registerResponder(new DeleteResponder(
                 pathExaminer,
                 fileOperator));
-        responderSupplier.registerResponder(Method.OPTIONS, new OptionsResponder(responderSupplier));
+        responderSupplier.registerResponder(new OptionsResponder(responderSupplier));
 
         return responderSupplier;
     }
diff --git a/src/main/java/httpserver/responder/DeleteResponder.java b/src/main/java/httpserver/responder/DeleteResponder.java
@@ -12,11 +12,12 @@
 import java.io.IOException;
 import java.nio.file.Path;
 
-public class DeleteResponder implements Responder {
+public class DeleteResponder extends MethodResponder {
     private final FileOperator fileOperator;
     private final PathExaminer pathExaminer;
 
     public DeleteResponder(PathExaminer pathExaminer, FileOperator fileOperator) {
+        super.methodString = "DELETE";
         this.pathExaminer = pathExaminer;
         this.fileOperator = fileOperator;
     }
diff --git a/src/main/java/httpserver/responder/GeneralResponder.java b/src/main/java/httpserver/responder/GeneralResponder.java
@@ -16,7 +16,7 @@ public GeneralResponder(ResponderSupplier responderSupplier) {
     }
 
     public Response respond(AppConfig appConfig, Request request) {
-        Responder responder = responderSupplier.responderForMethodString(request.getMethodString());
+        Responder responder = responderSupplier.supplyResponder(request);
         try {
             return responder.respond(appConfig, request);
         } catch (IOException e) {
diff --git a/src/main/java/httpserver/responder/GetResponder.java b/src/main/java/httpserver/responder/GetResponder.java
@@ -15,7 +15,7 @@
 import java.io.IOException;
 import java.nio.file.Path;
 
-public class GetResponder implements Responder {
+public class GetResponder extends MethodResponder {
 
     private final PathExaminer pathExaminer;
     private final RouteMap specialCaseRouteMap;
@@ -26,6 +26,7 @@ public GetResponder(RouteMap getRouteMap,
                         PathExaminer pathExaminer,
                         Html html,
                         RangeHeaderValueParser rangeHeaderValueParser) {
+        super.methodString = "GET";
         this.pathExaminer = pathExaminer;
         this.specialCaseRouteMap = getRouteMap;
         this.html = html;
diff --git a/src/main/java/httpserver/responder/HeadResponder.java b/src/main/java/httpserver/responder/HeadResponder.java
@@ -15,6 +15,7 @@ public HeadResponder(RouteMap getRouteMap,
                          Html html,
                          RangeHeaderValueParser rangeHeaderValueParser) {
         super(getRouteMap, pathExaminer, html, rangeHeaderValueParser);
+        super.methodString = "HEAD";
     }
 
     @Override
diff --git a/src/main/java/httpserver/responder/InvalidMethodResponder.java b/src/main/java/httpserver/responder/InvalidMethodResponder.java
@@ -5,7 +5,7 @@
 import httpserver.response.MethodNotAllowedResponse;
 import httpserver.response.Response;
 
-public class InvalidMethodResponder implements Responder {
+public class InvalidMethodResponder extends MethodResponder {
     @Override
     public Response respond(AppConfig appConfig, Request request) {
         return new MethodNotAllowedResponse();
@@ -15,4 +15,9 @@ public Response respond(AppConfig appConfig, Request request) {
     public boolean allows(String pathString) {
         return true;
     }
+
+    @Override
+    public boolean handles(Request request) {
+        return true;
+    }
 }
diff --git a/src/main/java/httpserver/responder/MethodResponder.java b/src/main/java/httpserver/responder/MethodResponder.java
@@ -0,0 +1,15 @@
+package httpserver.responder;
+
+import httpserver.request.Request;
+
+public abstract class MethodResponder implements Responder {
+    protected String methodString;
+
+    public String getMethodString() {
+        return methodString;
+    }
+
+    public boolean handles(Request request) {
+        return request.getMethodString().equals(this.methodString);
+    }
+}
diff --git a/src/main/java/httpserver/responder/OptionsResponder.java b/src/main/java/httpserver/responder/OptionsResponder.java
@@ -1,31 +1,31 @@
 package httpserver.responder;
 
 import httpserver.AppConfig;
-import httpserver.Method;
 import httpserver.request.Request;
 import httpserver.ResponderSupplier;
 import httpserver.header.Header;
 import httpserver.response.OkResponse;
 import httpserver.response.Response;
 
+import java.util.List;
 import java.util.StringJoiner;
 
-public class OptionsResponder implements Responder {
+public class OptionsResponder extends MethodResponder {
     private final ResponderSupplier responderSupplier;
 
     public OptionsResponder(ResponderSupplier responderSupplier) {
         this.responderSupplier = responderSupplier;
+        super.methodString = "OPTIONS";
     }
 
     @Override
     public Response respond(AppConfig appConfig, Request request) {
-        Method[] methods = Method.values();
+        List<MethodResponder> methodResponders = responderSupplier.allResponders();
         StringJoiner joiner = new StringJoiner(",");
 
-        for (Method method: methods) {
-            Responder responder = responderSupplier.responderForMethodString(method.toString());
-            if (responder.allows(request.getPathString())) {
-                joiner.add(method.toString());
+        for (MethodResponder methodResponder: methodResponders) {
+            if (methodResponder.allows(request.getPathString())) {
+                joiner.add(methodResponder.getMethodString());
             }
         }
 
diff --git a/src/main/java/httpserver/responder/PatchResponder.java b/src/main/java/httpserver/responder/PatchResponder.java
@@ -10,13 +10,14 @@
 import java.io.IOException;
 import java.nio.file.Path;
 
-public class PatchResponder implements Responder {
+public class PatchResponder extends MethodResponder {
     private PathExaminer pathExaminer;
     private final FileOperator fileOperator;
     private final Hasher hasher;
 
     public PatchResponder(PathExaminer pathExaminer,
                           FileOperator fileOperator, Hasher hasher) {
+        super.methodString = "PATCH";
         this.pathExaminer = pathExaminer;
         this.fileOperator = fileOperator;
         this.hasher = hasher;
diff --git a/src/main/java/httpserver/responder/PostResponder.java b/src/main/java/httpserver/responder/PostResponder.java
@@ -11,11 +11,12 @@
 import java.io.IOException;
 import java.nio.file.Path;
 
-public class PostResponder implements Responder {
+public class PostResponder extends MethodResponder {
     private final PathExaminer pathExaminer;
     private final FileOperator fileOperator;
 
     public PostResponder(PathExaminer pathExaminer, FileOperator fileOperator) {
+        super.methodString = "POST";
         this.pathExaminer = pathExaminer;
         this.fileOperator = fileOperator;
     }
diff --git a/src/main/java/httpserver/responder/PutResponder.java b/src/main/java/httpserver/responder/PutResponder.java
@@ -12,11 +12,12 @@
 import java.io.IOException;
 import java.nio.file.Path;
 
-public class PutResponder implements Responder {
+public class PutResponder extends MethodResponder {
     private final PathExaminer pathExaminer;
     private final FileOperator fileOperator;
 
     public PutResponder(PathExaminer pathExaminer, FileOperator fileOperator) {
+        super.methodString = "PUT";
         this.pathExaminer = pathExaminer;
         this.fileOperator = fileOperator;
     }
diff --git a/src/test/java/httpserver/ResponderSupplierTest.java b/src/test/java/httpserver/ResponderSupplierTest.java
@@ -1,10 +1,14 @@
 package httpserver;
 
+import httpserver.request.Request;
 import httpserver.responder.GetResponder;
 import httpserver.responder.InvalidMethodResponder;
-import httpserver.responder.Responder;
+import httpserver.responder.MethodResponder;
+import httpserver.responder.PostResponder;
 import org.junit.Test;
 
+import java.util.List;
+
 import static org.junit.Assert.*;
 import static org.mockito.Mockito.*;
 
@@ -13,25 +17,41 @@ public class ResponderSupplierTest {
     private final GetResponder getResponderMock;
     private final InvalidMethodResponder invalidMethodResponderMock;
     private final ResponderSupplier responderSupplier;
+    private final PostResponder postResponderMock;
 
     public ResponderSupplierTest() {
         getResponderMock = mock(GetResponder.class);
+        postResponderMock = mock(PostResponder.class);
         invalidMethodResponderMock = mock(InvalidMethodResponder.class);
         responderSupplier = new ResponderSupplier(invalidMethodResponderMock);
-        responderSupplier.registerResponder(Method.GET, getResponderMock);
+        responderSupplier.registerResponder(getResponderMock);
+        responderSupplier.registerResponder(postResponderMock);
     }
 
     @Test
     public void returnsResponderForMethod() throws Exception {
-        Responder result = responderSupplier.responderForMethodString("GET");
+        Request request = new Request("GET", null, null, null);
+        when(getResponderMock.handles(any())).thenReturn(true);
+
+        MethodResponder methodResponder = responderSupplier.supplyResponder(request);
 
-        assertEquals(getResponderMock, result);
+        assertEquals(getResponderMock, methodResponder);
     }
 
     @Test
     public void returnsInvalidMethodIfNotPresent() throws Exception {
-        Responder result = responderSupplier.responderForMethodString("AAA");
+        Request request = new Request("AAA", null, null, null);
+
+        MethodResponder methodResponder = responderSupplier.supplyResponder(request);
+
+        assertEquals(invalidMethodResponderMock, methodResponder);
+    }
+
+    @Test
+    public void returnsAllRespondersRegistered() throws Exception {
+        List<MethodResponder> actual = responderSupplier.allResponders();
 
-        assertEquals(invalidMethodResponderMock, result);
+        assertEquals(actual.get(0), getResponderMock);
+        assertEquals(actual.get(1), postResponderMock);
     }
 }
diff --git a/src/test/java/httpserver/responder/DeleteResponderTest.java b/src/test/java/httpserver/responder/DeleteResponderTest.java
@@ -57,7 +57,7 @@ public void returns404WhenAllowedButDoesntExist() throws Exception {
     }
 
     @Test
-    public void returns405WhenNotHandled() throws Exception {
+    public void returns405WhenNotAllowed() throws Exception {
         Request request = new Request("DELETE", "/not_allowed", new Header[0], "");
 
         Response response = deleteResponder.respond(appConfigMock, request);
@@ -70,4 +70,10 @@ public void allowsForm() throws Exception {
         assertTrue(deleteResponder.allows("/form"));
         assertFalse(deleteResponder.allows("/other"));
     }
+
+    @Test
+    public void handlesDELETE() throws Exception {
+        Request deleteRequest = new Request("DELETE", "", null, null);
+        assertTrue(deleteResponder.handles(deleteRequest));
+    }
 }
diff --git a/src/test/java/httpserver/responder/GeneralResponderTest.java b/src/test/java/httpserver/responder/GeneralResponderTest.java
@@ -16,32 +16,32 @@ public class GeneralResponderTest {
     private final AppConfig appConfigMock;
     private final Request requestMock;
     private final ResponderSupplier responderSupplierMock;
-    private final Responder responderMock;
+    private final MethodResponder methodResponderMock;
     private final GeneralResponder generalResponder;
 
     public GeneralResponderTest() {
         appConfigMock = mock(AppConfig.class);
         requestMock = mock(Request.class);
         when(requestMock.getMethodString()).thenReturn("methodString");
         responderSupplierMock = mock(ResponderSupplier.class);
-        responderMock = mock(Responder.class);
+        methodResponderMock = mock(MethodResponder.class);
         generalResponder = new GeneralResponder(responderSupplierMock);
     }
 
     @Test
-    public void callsResponderForMethodStringOnResponderSupplier() throws Exception {
-        when(responderSupplierMock.responderForMethodString(any())).thenReturn(responderMock);
+    public void callsSupplyResponderOnResponderSupplierWithRequest() throws Exception {
+        when(responderSupplierMock.supplyResponder(any())).thenReturn(methodResponderMock);
 
         generalResponder.respond(appConfigMock, requestMock);
 
-        verify(responderSupplierMock).responderForMethodString("methodString");
-        verify(responderMock).respond(appConfigMock, requestMock);
+        verify(responderSupplierMock).supplyResponder(requestMock);
+        verify(methodResponderMock).respond(appConfigMock, requestMock);
     }
 
     @Test
     public void returnsServerErrorResponseIfTheResponderCalledReturnsAnError() throws Exception {
-        when(responderMock.respond(any(), any())).thenThrow(new IOException());
-        when(responderSupplierMock.responderForMethodString(any())).thenReturn(responderMock);
+        when(methodResponderMock.respond(any(), any())).thenThrow(new IOException());
+        when(responderSupplierMock.supplyResponder(any())).thenReturn(methodResponderMock);
 
         Response response = generalResponder.respond(appConfigMock, requestMock);
 
diff --git a/src/test/java/httpserver/responder/GetResponderTest.java b/src/test/java/httpserver/responder/GetResponderTest.java
@@ -141,4 +141,10 @@ public void allowsForm() throws Exception {
         assertTrue(getResponder.allows("/form"));
         assertTrue(getResponder.allows("anything"));
     }
+
+    @Test
+    public void handlesGET() throws Exception {
+        Request getRequest = new Request("GET", "", null, null);
+        assertTrue(getResponder.handles(getRequest));
+    }
 }
diff --git a/src/test/java/httpserver/responder/HeadResponderTest.java b/src/test/java/httpserver/responder/HeadResponderTest.java
diff --git a/src/test/java/httpserver/responder/InvalidMethodResponderTest.java b/src/test/java/httpserver/responder/InvalidMethodResponderTest.java
diff --git a/src/test/java/httpserver/responder/OptionsResponderTest.java b/src/test/java/httpserver/responder/OptionsResponderTest.java
diff --git a/src/test/java/httpserver/responder/PatchResponderTest.java b/src/test/java/httpserver/responder/PatchResponderTest.java
diff --git a/src/test/java/httpserver/responder/PostResponderTest.java b/src/test/java/httpserver/responder/PostResponderTest.java
diff --git a/src/test/java/httpserver/responder/PutResponderTest.java b/src/test/java/httpserver/responder/PutResponderTest.java

Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,7 @@ public GeneralResponder(ResponderSupplier responderSupplier) {`
`16`	`16`	`}`
`17`	`17`
`18`	`18`	`public Response respond(AppConfig appConfig, Request request) {`
`19`		`- Responder responder = responderSupplier.responderForMethodString(request.getMethodString());`
	`19`	`+ Responder responder = responderSupplier.supplyResponder(request);`
`20`	`20`	`try {`
`21`	`21`	`return responder.respond(appConfig, request);`
`22`	`22`	`} catch (IOException e) {`
Original file line number	Diff line number	Diff line change
`@@ -15,6 +15,7 @@ public HeadResponder(RouteMap getRouteMap,`
`15`	`15`	`Html html,`
`16`	`16`	`RangeHeaderValueParser rangeHeaderValueParser) {`
`17`	`17`	`super(getRouteMap, pathExaminer, html, rangeHeaderValueParser);`
	`18`	`+ super.methodString = "HEAD";`
`18`	`19`	`}`
`19`	`20`
`20`	`21`	`@Override`