From 784360cf249dd0ddd4e27928ad6b7d52ce8f2b69 Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Thu, 26 Apr 2018 15:48:51 -0500 Subject: [PATCH 01/43] Added missing test --- http/parser_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/http/parser_test.go b/http/parser_test.go index 0be030f..7948c54 100644 --- a/http/parser_test.go +++ b/http/parser_test.go @@ -74,6 +74,11 @@ var _ = Describe("LineRequestParser", func() { request, err = parser.Parse(reader) }) + It("returns the parsed request", func() { + Expect(request.Method).To(Equal("GET")) + Expect(request.Target).To(Equal("/foo")) + }) + It("returns no error", func() { Expect(err).To(BeNil()) }) From 1e6c62222aa85ed20ff71aaeb1e4cc5d165df318 Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Thu, 26 Apr 2018 16:00:08 -0500 Subject: [PATCH 02/43] Moved MakeResourceRequest over to a method on RequestLine --- fs/route.go | 2 +- http/method_dispatcher.go | 37 +++++++++++++++++++++++------------ http/router.go | 12 ------------ playground/parameter_route.go | 2 +- playground/readonly_route.go | 2 +- playground/writable_route.go | 2 +- teapot/route.go | 2 +- 7 files changed, 29 insertions(+), 30 deletions(-) diff --git a/fs/route.go b/fs/route.go index fa40ef3..f2d9606 100644 --- a/fs/route.go +++ b/fs/route.go @@ -19,7 +19,7 @@ type FileSystemRoute struct { } func (route FileSystemRoute) Route(requested *http.RequestLine) http.Request { - return http.MakeResourceRequest(requested, route.Resource) + return requested.MakeResourceRequest(route.Resource) } // Represents files and directories on the file system diff --git a/http/method_dispatcher.go b/http/method_dispatcher.go index 615fd84..f2bcd5b 100644 --- a/http/method_dispatcher.go +++ b/http/method_dispatcher.go @@ -4,40 +4,51 @@ import ( "sort" "github.com/kkrull/gohttp/msg/clienterror" + "github.com/kkrull/gohttp/msg/servererror" ) -func MakeResourceRequest(requested *RequestLine, resource Resource) Request { - if requested.Method == "OPTIONS" { +type RequestLine struct { + Method string + Target string + QueryParameters map[string]string +} + +func (requestLine *RequestLine) NotImplemented() Response { + return &servererror.NotImplemented{Method: requestLine.Method} +} + +func (requestLine *RequestLine) MakeResourceRequest(resource Resource) Request { + if requestLine.Method == "OPTIONS" { return &optionsRequest{ - SupportedMethods: supportedMethods(requested.Target, resource), + SupportedMethods: requestLine.supportedMethods(resource), } } - method := knownMethods[requested.Method] + method := knownMethods[requestLine.Method] if method == nil { - return unknownHttpMethod(requested, resource) + return requestLine.unknownHttpMethod(resource) } - request := method.MakeRequest(requested, resource) + request := method.MakeRequest(requestLine, resource) if request == nil { - return unsupportedMethod(requested, resource) + return requestLine.unsupportedMethod(resource) } return request } -func unknownHttpMethod(requested *RequestLine, resource Resource) Request { - return clienterror.MethodNotAllowed(supportedMethods(requested.Target, resource)...) +func (requestLine *RequestLine) unknownHttpMethod(resource Resource) Request { + return clienterror.MethodNotAllowed(requestLine.supportedMethods(resource)...) } -func unsupportedMethod(requested *RequestLine, resource Resource) Request { - return clienterror.MethodNotAllowed(supportedMethods(requested.Target, resource)...) +func (requestLine *RequestLine) unsupportedMethod(resource Resource) Request { + return clienterror.MethodNotAllowed(requestLine.supportedMethods(resource)...) } -func supportedMethods(target string, resource Resource) []string { +func (requestLine *RequestLine) supportedMethods(resource Resource) []string { supported := []string{"OPTIONS"} for name, method := range knownMethods { - imaginaryRequest := &RequestLine{Method: name, Target: target} + imaginaryRequest := &RequestLine{Method: name, Target: requestLine.Target} request := method.MakeRequest(imaginaryRequest, resource) if request != nil { supported = append(supported, name) diff --git a/http/router.go b/http/router.go index fec186e..7e53cc2 100644 --- a/http/router.go +++ b/http/router.go @@ -2,8 +2,6 @@ package http import ( "bufio" - - "github.com/kkrull/gohttp/msg/servererror" ) func NewRouter() *RequestLineRouter { @@ -55,13 +53,3 @@ type RequestParser interface { type Route interface { Route(requested *RequestLine) Request } - -type RequestLine struct { - Method string - Target string - QueryParameters map[string]string -} - -func (requestLine *RequestLine) NotImplemented() Response { - return &servererror.NotImplemented{Method: requestLine.Method} -} diff --git a/playground/parameter_route.go b/playground/parameter_route.go index 6d29cd9..4ad3ac2 100644 --- a/playground/parameter_route.go +++ b/playground/parameter_route.go @@ -19,7 +19,7 @@ func (route *ParameterRoute) Route(requested *http.RequestLine) http.Request { return nil } - return http.MakeResourceRequest(requested, route.Decoder) + return requested.MakeResourceRequest(route.Decoder) } type ParameterDecoder interface { diff --git a/playground/readonly_route.go b/playground/readonly_route.go index 524267a..3990b51 100644 --- a/playground/readonly_route.go +++ b/playground/readonly_route.go @@ -19,7 +19,7 @@ func (route *ReadOnlyRoute) Route(requested *http.RequestLine) http.Request { return nil } - return http.MakeResourceRequest(requested, route.Resource) + return requested.MakeResourceRequest(route.Resource) } type ReadOnlyResource interface { diff --git a/playground/writable_route.go b/playground/writable_route.go index 4dbae84..b1bccf8 100644 --- a/playground/writable_route.go +++ b/playground/writable_route.go @@ -22,7 +22,7 @@ func (route *ReadWriteRoute) Route(requested *http.RequestLine) http.Request { return nil } - return http.MakeResourceRequest(requested, route.Resource) + return requested.MakeResourceRequest(route.Resource) } type ReadWriteResource interface { diff --git a/teapot/route.go b/teapot/route.go index 6f368e2..c4e912b 100644 --- a/teapot/route.go +++ b/teapot/route.go @@ -20,7 +20,7 @@ func (route *Route) Route(requested *http.RequestLine) http.Request { return nil } - return http.MakeResourceRequest(requested, route.Teapot) + return requested.MakeResourceRequest(route.Teapot) } type Teapot interface { From a01312983700d419004019085ac35e65e2f053d6 Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Thu, 26 Apr 2018 16:46:22 -0500 Subject: [PATCH 03/43] Making room for RequestMessage interface --- capability/route.go | 4 ++-- capability/route_test.go | 6 +++--- fs/route_test.go | 6 +++--- http/method_dispatcher.go | 14 +++++++------- http/methods.go | 8 ++++---- http/parser.go | 4 ++-- http/parser_test.go | 4 ++-- mock/http_mocks.go | 6 +++--- playground/parameter_route.go | 2 +- playground/parameter_route_test.go | 12 ++++++------ playground/playground_suite_test.go | 2 +- playground/readonly_route.go | 2 +- playground/readonly_route_test.go | 6 +++--- playground/writable_route.go | 2 +- playground/writable_route_test.go | 6 +++--- teapot/route.go | 2 +- teapot/route_test.go | 6 +++--- 17 files changed, 46 insertions(+), 46 deletions(-) diff --git a/capability/route.go b/capability/route.go index 2fe977d..df08eef 100644 --- a/capability/route.go +++ b/capability/route.go @@ -20,9 +20,9 @@ type ServerCapabilityRoute struct { } func (route *ServerCapabilityRoute) Route(requested *http.RequestLine) http.Request { - if requested.Target != "*" { + if requested.TheTarget != "*" { return nil - } else if requested.Method != "OPTIONS" { + } else if requested.TheMethod != "OPTIONS" { return clienterror.MethodNotAllowed("OPTIONS") } diff --git a/capability/route_test.go b/capability/route_test.go index 51f1e34..eba698f 100644 --- a/capability/route_test.go +++ b/capability/route_test.go @@ -42,21 +42,21 @@ var _ = Describe("ServerCapabilityRoute", func() { Context("when the target is *", func() { It("routes OPTIONS to ServerResource", func() { - requested = &http.RequestLine{Method: "OPTIONS", Target: "*"} + requested = &http.RequestLine{TheMethod: "OPTIONS", TheTarget: "*"} routedRequest = router.Route(requested) routedRequest.Handle(&bufio.Writer{}) controller.OptionsShouldHaveBeenCalled() }) It("returns MethodNotAllowed for any other method", func() { - requested = &http.RequestLine{Method: "GET", Target: "*"} + requested = &http.RequestLine{TheMethod: "GET", TheTarget: "*"} routedRequest = router.Route(requested) Expect(routedRequest).To(BeEquivalentTo(clienterror.MethodNotAllowed("OPTIONS"))) }) }) It("returns nil to pass on any other target", func() { - requested = &http.RequestLine{Method: "OPTIONS", Target: "/"} + requested = &http.RequestLine{TheMethod: "OPTIONS", TheTarget: "/"} routedRequest = router.Route(requested) Expect(routedRequest).To(BeNil()) }) diff --git a/fs/route_test.go b/fs/route_test.go index 3886e11..235c5d2 100644 --- a/fs/route_test.go +++ b/fs/route_test.go @@ -38,21 +38,21 @@ var _ = Describe("FileSystemRoute", func() { Describe("#Route", func() { It("routes GET requests to GetRequest", func() { - requested := &http.RequestLine{Method: "GET", Target: "/foo"} + requested := &http.RequestLine{TheMethod: "GET", TheTarget: "/foo"} routedRequest := route.Route(requested) routedRequest.Handle(response) resource.GetShouldHaveReceived("/foo") }) It("routes HEAD requests to HeadRequest", func() { - requested := &http.RequestLine{Method: "HEAD", Target: "/foo"} + requested := &http.RequestLine{TheMethod: "HEAD", TheTarget: "/foo"} routedRequest := route.Route(requested) routedRequest.Handle(response) resource.HeadShouldHaveReceived("/foo") }) It("routes any other method to MethodNotAllowed", func() { - requested := &http.RequestLine{Method: "TRACE", Target: "/"} + requested := &http.RequestLine{TheMethod: "TRACE", TheTarget: "/"} routedRequest := route.Route(requested) Expect(routedRequest).To(BeEquivalentTo(clienterror.MethodNotAllowed("GET", "HEAD", "OPTIONS"))) }) diff --git a/http/method_dispatcher.go b/http/method_dispatcher.go index f2bcd5b..849fe98 100644 --- a/http/method_dispatcher.go +++ b/http/method_dispatcher.go @@ -8,23 +8,23 @@ import ( ) type RequestLine struct { - Method string - Target string - QueryParameters map[string]string + TheMethod string + TheTarget string + TheQueryParameters map[string]string } func (requestLine *RequestLine) NotImplemented() Response { - return &servererror.NotImplemented{Method: requestLine.Method} + return &servererror.NotImplemented{Method: requestLine.TheMethod} } func (requestLine *RequestLine) MakeResourceRequest(resource Resource) Request { - if requestLine.Method == "OPTIONS" { + if requestLine.TheMethod == "OPTIONS" { return &optionsRequest{ SupportedMethods: requestLine.supportedMethods(resource), } } - method := knownMethods[requestLine.Method] + method := knownMethods[requestLine.TheMethod] if method == nil { return requestLine.unknownHttpMethod(resource) } @@ -48,7 +48,7 @@ func (requestLine *RequestLine) unsupportedMethod(resource Resource) Request { func (requestLine *RequestLine) supportedMethods(resource Resource) []string { supported := []string{"OPTIONS"} for name, method := range knownMethods { - imaginaryRequest := &RequestLine{Method: name, Target: requestLine.Target} + imaginaryRequest := &RequestLine{TheMethod: name, TheTarget: requestLine.TheTarget} request := method.MakeRequest(imaginaryRequest, resource) if request != nil { supported = append(supported, name) diff --git a/http/methods.go b/http/methods.go index 27013df..4e20166 100644 --- a/http/methods.go +++ b/http/methods.go @@ -14,7 +14,7 @@ type getMethod struct{} func (method *getMethod) MakeRequest(requested *RequestLine, resource Resource) Request { supportedResource, ok := resource.(GetResource) if ok { - return &getRequest{Resource: supportedResource, Target: requested.Target} + return &getRequest{Resource: supportedResource, Target: requested.TheTarget} } return nil @@ -41,7 +41,7 @@ type headMethod struct{} func (*headMethod) MakeRequest(requested *RequestLine, resource Resource) Request { supportedResource, ok := resource.(HeadResource) if ok { - return &headRequest{Resource: supportedResource, Target: requested.Target} + return &headRequest{Resource: supportedResource, Target: requested.TheTarget} } return nil @@ -82,7 +82,7 @@ type postMethod struct{} func (*postMethod) MakeRequest(requested *RequestLine, resource Resource) Request { supportedResource, ok := resource.(PostResource) if ok { - return &postRequest{Resource: supportedResource, Target: requested.Target} + return &postRequest{Resource: supportedResource, Target: requested.TheTarget} } return nil @@ -109,7 +109,7 @@ type putMethod struct{} func (*putMethod) MakeRequest(requested *RequestLine, resource Resource) Request { supportedResource, ok := resource.(PutResource) if ok { - return &putRequest{Resource: supportedResource, Target: requested.Target} + return &putRequest{Resource: supportedResource, Target: requested.TheTarget} } return nil diff --git a/http/parser.go b/http/parser.go index e621b02..e84a6be 100644 --- a/http/parser.go +++ b/http/parser.go @@ -53,8 +53,8 @@ func (parser *parseMethodObject) parseRequestLine(text string) (ok *RequestLine, } return &RequestLine{ - Method: fields[0], - Target: fields[1], + TheMethod: fields[0], + TheTarget: fields[1], }, nil } diff --git a/http/parser_test.go b/http/parser_test.go index 7948c54..dbfb1ae 100644 --- a/http/parser_test.go +++ b/http/parser_test.go @@ -75,8 +75,8 @@ var _ = Describe("LineRequestParser", func() { }) It("returns the parsed request", func() { - Expect(request.Method).To(Equal("GET")) - Expect(request.Target).To(Equal("/foo")) + Expect(request.TheMethod).To(Equal("GET")) + Expect(request.TheTarget).To(Equal("/foo")) }) It("returns no error", func() { diff --git a/mock/http_mocks.go b/mock/http_mocks.go index e27cbb1..d65a892 100644 --- a/mock/http_mocks.go +++ b/mock/http_mocks.go @@ -88,13 +88,13 @@ func (mock *Route) Route(requested *http.RequestLine) http.Request { func (mock *Route) ShouldHaveReceived(method string, target string) { ExpectWithOffset(1, mock.routeRequested).To(BeEquivalentTo(&http.RequestLine{ - Method: method, - Target: target, + TheMethod: method, + TheTarget: target, })) } func (mock *Route) ShouldHaveReceivedParameters(parameters map[string]string) { - ExpectWithOffset(1, mock.routeRequested.QueryParameters).To(Equal(parameters)) + ExpectWithOffset(1, mock.routeRequested.TheQueryParameters).To(Equal(parameters)) } type Response struct { diff --git a/playground/parameter_route.go b/playground/parameter_route.go index 4ad3ac2..64118f1 100644 --- a/playground/parameter_route.go +++ b/playground/parameter_route.go @@ -15,7 +15,7 @@ type ParameterRoute struct { } func (route *ParameterRoute) Route(requested *http.RequestLine) http.Request { - if requested.Target != "/parameters" { + if requested.TheTarget != "/parameters" { return nil } diff --git a/playground/parameter_route_test.go b/playground/parameter_route_test.go index 3284572..76b7ec2 100644 --- a/playground/parameter_route_test.go +++ b/playground/parameter_route_test.go @@ -42,9 +42,9 @@ var _ = Describe("ParameterRoute", func() { "two": "2", } requested := &http.RequestLine{ - Method: "GET", - Target: "/parameters", - QueryParameters: parameters, + TheMethod: "GET", + TheTarget: "/parameters", + TheQueryParameters: parameters, } routedRequest := router.Route(requested) @@ -55,7 +55,7 @@ var _ = Describe("ParameterRoute", func() { Context("when the method is OPTIONS", func() { BeforeEach(func() { - requested := &http.RequestLine{Method: "OPTIONS", Target: "/parameters"} + requested := &http.RequestLine{TheMethod: "OPTIONS", TheTarget: "/parameters"} routedRequest := router.Route(requested) Expect(routedRequest).NotTo(BeNil()) routedRequest.Handle(response) @@ -67,14 +67,14 @@ var _ = Describe("ParameterRoute", func() { }) It("replies Method Not Allowed on any other method", func() { - requested := &http.RequestLine{Method: "TRACE", Target: "/parameters"} + requested := &http.RequestLine{TheMethod: "TRACE", TheTarget: "/parameters"} routedRequest := router.Route(requested) Expect(routedRequest).To(BeAssignableToTypeOf(clienterror.MethodNotAllowed())) }) }) It("returns nil for any other target", func() { - requested := &http.RequestLine{Method: "GET", Target: "/"} + requested := &http.RequestLine{TheMethod: "GET", TheTarget: "/"} Expect(router.Route(requested)).To(BeNil()) }) }) diff --git a/playground/playground_suite_test.go b/playground/playground_suite_test.go index ec3df2a..39b6c45 100644 --- a/playground/playground_suite_test.go +++ b/playground/playground_suite_test.go @@ -112,7 +112,7 @@ func (mock *ReadWriteResourceMock) PutShouldHaveBeenCalled() { /* Helpers */ func handleRequest(router http.Route, method, target string) { - requested := &http.RequestLine{Method: method, Target: target} + requested := &http.RequestLine{TheMethod: method, TheTarget: target} routedRequest := router.Route(requested) ExpectWithOffset(1, routedRequest).NotTo(BeNil()) diff --git a/playground/readonly_route.go b/playground/readonly_route.go index 3990b51..6a521f7 100644 --- a/playground/readonly_route.go +++ b/playground/readonly_route.go @@ -15,7 +15,7 @@ type ReadOnlyRoute struct { } func (route *ReadOnlyRoute) Route(requested *http.RequestLine) http.Request { - if requested.Target != "/method_options2" { + if requested.TheTarget != "/method_options2" { return nil } diff --git a/playground/readonly_route_test.go b/playground/readonly_route_test.go index 7799868..db9f61c 100644 --- a/playground/readonly_route_test.go +++ b/playground/readonly_route_test.go @@ -46,7 +46,7 @@ var _ = Describe("ReadOnlyRoute", func() { var response = &bytes.Buffer{} BeforeEach(func() { - requested := &http.RequestLine{Method: "OPTIONS", Target: "/method_options2"} + requested := &http.RequestLine{TheMethod: "OPTIONS", TheTarget: "/method_options2"} routedRequest := router.Route(requested) Expect(routedRequest).NotTo(BeNil()) @@ -60,14 +60,14 @@ var _ = Describe("ReadOnlyRoute", func() { }) It("returns MethodNotAllowed for any other method", func() { - requested := &http.RequestLine{Method: "PUT", Target: "/method_options2"} + requested := &http.RequestLine{TheMethod: "PUT", TheTarget: "/method_options2"} routedRequest := router.Route(requested) Expect(routedRequest).To(BeEquivalentTo(clienterror.MethodNotAllowed("GET", "HEAD", "OPTIONS"))) }) }) It("returns nil on any other target", func() { - requested := &http.RequestLine{Method: "GET", Target: "/"} + requested := &http.RequestLine{TheMethod: "GET", TheTarget: "/"} routedRequest := router.Route(requested) Expect(routedRequest).To(BeNil()) }) diff --git a/playground/writable_route.go b/playground/writable_route.go index b1bccf8..976a08b 100644 --- a/playground/writable_route.go +++ b/playground/writable_route.go @@ -18,7 +18,7 @@ type ReadWriteRoute struct { } func (route *ReadWriteRoute) Route(requested *http.RequestLine) http.Request { - if requested.Target != "/method_options" { + if requested.TheTarget != "/method_options" { return nil } diff --git a/playground/writable_route_test.go b/playground/writable_route_test.go index 24c3a0c..feea785 100644 --- a/playground/writable_route_test.go +++ b/playground/writable_route_test.go @@ -46,7 +46,7 @@ var _ = Describe("ReadWriteRoute", func() { var response = &bytes.Buffer{} BeforeEach(func() { - requested := &http.RequestLine{Method: "OPTIONS", Target: "/method_options"} + requested := &http.RequestLine{TheMethod: "OPTIONS", TheTarget: "/method_options"} routedRequest := router.Route(requested) Expect(routedRequest).NotTo(BeNil()) @@ -70,14 +70,14 @@ var _ = Describe("ReadWriteRoute", func() { }) It("returns MethodNotAllowed for any other method", func() { - requested := &http.RequestLine{Method: "TRACE", Target: "/method_options"} + requested := &http.RequestLine{TheMethod: "TRACE", TheTarget: "/method_options"} routedRequest := router.Route(requested) Expect(routedRequest).To(BeEquivalentTo(clienterror.MethodNotAllowed("GET", "HEAD", "OPTIONS", "POST", "PUT"))) }) }) It("returns nil on any other target", func() { - requested := &http.RequestLine{Method: "GET", Target: "/"} + requested := &http.RequestLine{TheMethod: "GET", TheTarget: "/"} routedRequest := router.Route(requested) Expect(routedRequest).To(BeNil()) }) diff --git a/teapot/route.go b/teapot/route.go index c4e912b..387cb7e 100644 --- a/teapot/route.go +++ b/teapot/route.go @@ -16,7 +16,7 @@ type Route struct { } func (route *Route) Route(requested *http.RequestLine) http.Request { - if !route.Teapot.RespondsTo(requested.Target) { + if !route.Teapot.RespondsTo(requested.TheTarget) { return nil } diff --git a/teapot/route_test.go b/teapot/route_test.go index 6e94933..0bec4da 100644 --- a/teapot/route_test.go +++ b/teapot/route_test.go @@ -26,14 +26,14 @@ var _ = Describe("teapotRoute", func() { }) It("routes GET requests to that target to the teapot", func() { - requested = &http.RequestLine{Method: "GET", Target: "/caffeine"} + requested = &http.RequestLine{TheMethod: "GET", TheTarget: "/caffeine"} routedRequest = router.Route(requested) routedRequest.Handle(&bufio.Writer{}) teapotMock.GetShouldHaveReceived("/caffeine") }) It("returns MethodNotAllowed for any other method", func() { - requested := &http.RequestLine{Method: "TRACE", Target: "/caffeine"} + requested := &http.RequestLine{TheMethod: "TRACE", TheTarget: "/caffeine"} routedRequest := router.Route(requested) Expect(routedRequest).To(BeEquivalentTo(clienterror.MethodNotAllowed("GET", "OPTIONS"))) }) @@ -43,7 +43,7 @@ var _ = Describe("teapotRoute", func() { teapotMock = &TeapotMock{} router = &teapot.Route{Teapot: teapotMock} - requested = &http.RequestLine{Method: "GET", Target: "/file.txt"} + requested = &http.RequestLine{TheMethod: "GET", TheTarget: "/file.txt"} routedRequest = router.Route(requested) Expect(routedRequest).To(BeNil()) }) From ecf0bce415e0a204cbe456cb0ab873af2208fbbd Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Thu, 26 Apr 2018 16:53:18 -0500 Subject: [PATCH 04/43] Introducing RequestMessage interface --- http/method_dispatcher.go | 12 ++++++++++++ http/router.go | 7 +++++++ 2 files changed, 19 insertions(+) diff --git a/http/method_dispatcher.go b/http/method_dispatcher.go index 849fe98..727cf93 100644 --- a/http/method_dispatcher.go +++ b/http/method_dispatcher.go @@ -13,6 +13,18 @@ type RequestLine struct { TheQueryParameters map[string]string } +func (requestLine *RequestLine) Method() string { + return requestLine.TheMethod +} + +func (requestLine *RequestLine) Target() string { + return requestLine.TheTarget +} + +func (requestLine *RequestLine) QueryParameters() map[string]string { + return requestLine.TheQueryParameters +} + func (requestLine *RequestLine) NotImplemented() Response { return &servererror.NotImplemented{Method: requestLine.TheMethod} } diff --git a/http/router.go b/http/router.go index 7e53cc2..df41fb4 100644 --- a/http/router.go +++ b/http/router.go @@ -50,6 +50,13 @@ type RequestParser interface { Parse(reader *bufio.Reader) (ok *RequestLine, err Response) } +type RequestMessage interface { + MakeResourceRequest(resource Resource) Request + Method() string + Target() string + QueryParameters() map[string]string +} + type Route interface { Route(requested *RequestLine) Request } From 7134734e9a2b19823ed85c3866c54714e3175146 Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Thu, 26 Apr 2018 16:57:43 -0500 Subject: [PATCH 05/43] Routes now depend upon the http.RequestMessage interface --- capability/route.go | 6 +++--- capability/route_test.go | 2 +- fs/route.go | 2 +- http/router.go | 2 +- mock/http_mocks.go | 6 +++--- playground/parameter_route.go | 4 ++-- playground/readonly_route.go | 4 ++-- playground/writable_route.go | 4 ++-- teapot/route.go | 4 ++-- 9 files changed, 17 insertions(+), 17 deletions(-) diff --git a/capability/route.go b/capability/route.go index df08eef..c736c12 100644 --- a/capability/route.go +++ b/capability/route.go @@ -19,10 +19,10 @@ type ServerCapabilityRoute struct { Controller ServerResource } -func (route *ServerCapabilityRoute) Route(requested *http.RequestLine) http.Request { - if requested.TheTarget != "*" { +func (route *ServerCapabilityRoute) Route(requested http.RequestMessage) http.Request { + if requested.Target() != "*" { return nil - } else if requested.TheMethod != "OPTIONS" { + } else if requested.Method() != "OPTIONS" { return clienterror.MethodNotAllowed("OPTIONS") } diff --git a/capability/route_test.go b/capability/route_test.go index eba698f..56539c5 100644 --- a/capability/route_test.go +++ b/capability/route_test.go @@ -31,7 +31,7 @@ var _ = Describe("ServerCapabilityRoute", func() { var ( router http.Route controller *ServerCapabilityServerMock - requested *http.RequestLine + requested http.RequestMessage routedRequest http.Request ) diff --git a/fs/route.go b/fs/route.go index f2d9606..3c4cce5 100644 --- a/fs/route.go +++ b/fs/route.go @@ -18,7 +18,7 @@ type FileSystemRoute struct { Resource FileSystemResource } -func (route FileSystemRoute) Route(requested *http.RequestLine) http.Request { +func (route FileSystemRoute) Route(requested http.RequestMessage) http.Request { return requested.MakeResourceRequest(route.Resource) } diff --git a/http/router.go b/http/router.go index df41fb4..2b6807e 100644 --- a/http/router.go +++ b/http/router.go @@ -58,5 +58,5 @@ type RequestMessage interface { } type Route interface { - Route(requested *RequestLine) Request + Route(requested RequestMessage) Request } diff --git a/mock/http_mocks.go b/mock/http_mocks.go index d65a892..06bd60a 100644 --- a/mock/http_mocks.go +++ b/mock/http_mocks.go @@ -78,10 +78,10 @@ func (mock *Request) VerifyHandle(writer *bufio.Writer) { type Route struct { RouteReturns http.Request - routeRequested *http.RequestLine + routeRequested http.RequestMessage } -func (mock *Route) Route(requested *http.RequestLine) http.Request { +func (mock *Route) Route(requested http.RequestMessage) http.Request { mock.routeRequested = requested return mock.RouteReturns } @@ -94,7 +94,7 @@ func (mock *Route) ShouldHaveReceived(method string, target string) { } func (mock *Route) ShouldHaveReceivedParameters(parameters map[string]string) { - ExpectWithOffset(1, mock.routeRequested.TheQueryParameters).To(Equal(parameters)) + ExpectWithOffset(1, mock.routeRequested.QueryParameters()).To(Equal(parameters)) } type Response struct { diff --git a/playground/parameter_route.go b/playground/parameter_route.go index 64118f1..1cddbfd 100644 --- a/playground/parameter_route.go +++ b/playground/parameter_route.go @@ -14,8 +14,8 @@ type ParameterRoute struct { Decoder ParameterDecoder } -func (route *ParameterRoute) Route(requested *http.RequestLine) http.Request { - if requested.TheTarget != "/parameters" { +func (route *ParameterRoute) Route(requested http.RequestMessage) http.Request { + if requested.Target() != "/parameters" { return nil } diff --git a/playground/readonly_route.go b/playground/readonly_route.go index 6a521f7..e66147a 100644 --- a/playground/readonly_route.go +++ b/playground/readonly_route.go @@ -14,8 +14,8 @@ type ReadOnlyRoute struct { Resource ReadOnlyResource } -func (route *ReadOnlyRoute) Route(requested *http.RequestLine) http.Request { - if requested.TheTarget != "/method_options2" { +func (route *ReadOnlyRoute) Route(requested http.RequestMessage) http.Request { + if requested.Target() != "/method_options2" { return nil } diff --git a/playground/writable_route.go b/playground/writable_route.go index 976a08b..9f3a995 100644 --- a/playground/writable_route.go +++ b/playground/writable_route.go @@ -17,8 +17,8 @@ type ReadWriteRoute struct { Resource ReadWriteResource } -func (route *ReadWriteRoute) Route(requested *http.RequestLine) http.Request { - if requested.TheTarget != "/method_options" { +func (route *ReadWriteRoute) Route(requested http.RequestMessage) http.Request { + if requested.Target() != "/method_options" { return nil } diff --git a/teapot/route.go b/teapot/route.go index 387cb7e..a335e06 100644 --- a/teapot/route.go +++ b/teapot/route.go @@ -15,8 +15,8 @@ type Route struct { Teapot Teapot } -func (route *Route) Route(requested *http.RequestLine) http.Request { - if !route.Teapot.RespondsTo(requested.TheTarget) { +func (route *Route) Route(requested http.RequestMessage) http.Request { + if !route.Teapot.RespondsTo(requested.Target()) { return nil } From ccbe9e6837af3266f676ad3cf4185a59aba2b036 Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Thu, 26 Apr 2018 17:20:35 -0500 Subject: [PATCH 06/43] Using a constructor to make RequestMessages --- capability/route_test.go | 6 +++--- fs/route_test.go | 6 +++--- http/method_dispatcher.go | 7 +++++++ http/parser_test.go | 6 +++--- mock/http_mocks.go | 6 ++---- playground/parameter_route_test.go | 12 ++++-------- playground/playground_suite_test.go | 2 +- playground/readonly_route_test.go | 6 +++--- playground/writable_route_test.go | 6 +++--- teapot/route_test.go | 8 ++++---- 10 files changed, 33 insertions(+), 32 deletions(-) diff --git a/capability/route_test.go b/capability/route_test.go index 56539c5..5e1231b 100644 --- a/capability/route_test.go +++ b/capability/route_test.go @@ -42,21 +42,21 @@ var _ = Describe("ServerCapabilityRoute", func() { Context("when the target is *", func() { It("routes OPTIONS to ServerResource", func() { - requested = &http.RequestLine{TheMethod: "OPTIONS", TheTarget: "*"} + requested = http.NewRequestMessage("OPTIONS", "*") routedRequest = router.Route(requested) routedRequest.Handle(&bufio.Writer{}) controller.OptionsShouldHaveBeenCalled() }) It("returns MethodNotAllowed for any other method", func() { - requested = &http.RequestLine{TheMethod: "GET", TheTarget: "*"} + requested = http.NewRequestMessage("GET", "*") routedRequest = router.Route(requested) Expect(routedRequest).To(BeEquivalentTo(clienterror.MethodNotAllowed("OPTIONS"))) }) }) It("returns nil to pass on any other target", func() { - requested = &http.RequestLine{TheMethod: "OPTIONS", TheTarget: "/"} + requested = http.NewRequestMessage("OPTIONS", "/") routedRequest = router.Route(requested) Expect(routedRequest).To(BeNil()) }) diff --git a/fs/route_test.go b/fs/route_test.go index 235c5d2..3ec67f8 100644 --- a/fs/route_test.go +++ b/fs/route_test.go @@ -38,21 +38,21 @@ var _ = Describe("FileSystemRoute", func() { Describe("#Route", func() { It("routes GET requests to GetRequest", func() { - requested := &http.RequestLine{TheMethod: "GET", TheTarget: "/foo"} + requested := http.NewRequestMessage("GET", "/foo") routedRequest := route.Route(requested) routedRequest.Handle(response) resource.GetShouldHaveReceived("/foo") }) It("routes HEAD requests to HeadRequest", func() { - requested := &http.RequestLine{TheMethod: "HEAD", TheTarget: "/foo"} + requested := http.NewRequestMessage("HEAD", "/foo") routedRequest := route.Route(requested) routedRequest.Handle(response) resource.HeadShouldHaveReceived("/foo") }) It("routes any other method to MethodNotAllowed", func() { - requested := &http.RequestLine{TheMethod: "TRACE", TheTarget: "/"} + requested := http.NewRequestMessage("TRACE", "/") routedRequest := route.Route(requested) Expect(routedRequest).To(BeEquivalentTo(clienterror.MethodNotAllowed("GET", "HEAD", "OPTIONS"))) }) diff --git a/http/method_dispatcher.go b/http/method_dispatcher.go index 727cf93..8ee5ca0 100644 --- a/http/method_dispatcher.go +++ b/http/method_dispatcher.go @@ -7,6 +7,13 @@ import ( "github.com/kkrull/gohttp/msg/servererror" ) +func NewRequestMessage(method, target string) RequestMessage { + return &RequestLine{ + TheMethod: method, + TheTarget: target, + } +} + type RequestLine struct { TheMethod string TheTarget string diff --git a/http/parser_test.go b/http/parser_test.go index dbfb1ae..1e8fff7 100644 --- a/http/parser_test.go +++ b/http/parser_test.go @@ -13,7 +13,7 @@ var _ = Describe("LineRequestParser", func() { Describe("#Parse", func() { var ( parser http.RequestParser - request *http.RequestLine + request http.RequestMessage err http.Response ) @@ -75,8 +75,8 @@ var _ = Describe("LineRequestParser", func() { }) It("returns the parsed request", func() { - Expect(request.TheMethod).To(Equal("GET")) - Expect(request.TheTarget).To(Equal("/foo")) + Expect(request.Method()).To(Equal("GET")) + Expect(request.Target()).To(Equal("/foo")) }) It("returns no error", func() { diff --git a/mock/http_mocks.go b/mock/http_mocks.go index 06bd60a..55c9eb7 100644 --- a/mock/http_mocks.go +++ b/mock/http_mocks.go @@ -87,10 +87,8 @@ func (mock *Route) Route(requested http.RequestMessage) http.Request { } func (mock *Route) ShouldHaveReceived(method string, target string) { - ExpectWithOffset(1, mock.routeRequested).To(BeEquivalentTo(&http.RequestLine{ - TheMethod: method, - TheTarget: target, - })) + ExpectWithOffset(1, mock.routeRequested.Method()).To(Equal(method)) + ExpectWithOffset(1, mock.routeRequested.Target()).To(Equal(target)) } func (mock *Route) ShouldHaveReceivedParameters(parameters map[string]string) { diff --git a/playground/parameter_route_test.go b/playground/parameter_route_test.go index 76b7ec2..46e9696 100644 --- a/playground/parameter_route_test.go +++ b/playground/parameter_route_test.go @@ -41,11 +41,7 @@ var _ = Describe("ParameterRoute", func() { "one": "1", "two": "2", } - requested := &http.RequestLine{ - TheMethod: "GET", - TheTarget: "/parameters", - TheQueryParameters: parameters, - } + requested := http.NewRequestMessage("GET", "/parameters") //TODO KDK: Add query parameters routedRequest := router.Route(requested) Expect(routedRequest).NotTo(BeNil()) @@ -55,7 +51,7 @@ var _ = Describe("ParameterRoute", func() { Context("when the method is OPTIONS", func() { BeforeEach(func() { - requested := &http.RequestLine{TheMethod: "OPTIONS", TheTarget: "/parameters"} + requested := http.NewRequestMessage("OPTIONS", "/parameters") routedRequest := router.Route(requested) Expect(routedRequest).NotTo(BeNil()) routedRequest.Handle(response) @@ -67,14 +63,14 @@ var _ = Describe("ParameterRoute", func() { }) It("replies Method Not Allowed on any other method", func() { - requested := &http.RequestLine{TheMethod: "TRACE", TheTarget: "/parameters"} + requested := http.NewRequestMessage("TRACE", "/parameters") routedRequest := router.Route(requested) Expect(routedRequest).To(BeAssignableToTypeOf(clienterror.MethodNotAllowed())) }) }) It("returns nil for any other target", func() { - requested := &http.RequestLine{TheMethod: "GET", TheTarget: "/"} + requested := http.NewRequestMessage("GET", "/") Expect(router.Route(requested)).To(BeNil()) }) }) diff --git a/playground/playground_suite_test.go b/playground/playground_suite_test.go index 39b6c45..e4429fb 100644 --- a/playground/playground_suite_test.go +++ b/playground/playground_suite_test.go @@ -112,7 +112,7 @@ func (mock *ReadWriteResourceMock) PutShouldHaveBeenCalled() { /* Helpers */ func handleRequest(router http.Route, method, target string) { - requested := &http.RequestLine{TheMethod: method, TheTarget: target} + requested := http.NewRequestMessage(method, target) routedRequest := router.Route(requested) ExpectWithOffset(1, routedRequest).NotTo(BeNil()) diff --git a/playground/readonly_route_test.go b/playground/readonly_route_test.go index db9f61c..a5428cf 100644 --- a/playground/readonly_route_test.go +++ b/playground/readonly_route_test.go @@ -46,7 +46,7 @@ var _ = Describe("ReadOnlyRoute", func() { var response = &bytes.Buffer{} BeforeEach(func() { - requested := &http.RequestLine{TheMethod: "OPTIONS", TheTarget: "/method_options2"} + requested := http.NewRequestMessage("OPTIONS", "/method_options2") routedRequest := router.Route(requested) Expect(routedRequest).NotTo(BeNil()) @@ -60,14 +60,14 @@ var _ = Describe("ReadOnlyRoute", func() { }) It("returns MethodNotAllowed for any other method", func() { - requested := &http.RequestLine{TheMethod: "PUT", TheTarget: "/method_options2"} + requested := http.NewRequestMessage("PUT", "/method_options2") routedRequest := router.Route(requested) Expect(routedRequest).To(BeEquivalentTo(clienterror.MethodNotAllowed("GET", "HEAD", "OPTIONS"))) }) }) It("returns nil on any other target", func() { - requested := &http.RequestLine{TheMethod: "GET", TheTarget: "/"} + requested := http.NewRequestMessage("GET", "/") routedRequest := router.Route(requested) Expect(routedRequest).To(BeNil()) }) diff --git a/playground/writable_route_test.go b/playground/writable_route_test.go index feea785..14790cf 100644 --- a/playground/writable_route_test.go +++ b/playground/writable_route_test.go @@ -46,7 +46,7 @@ var _ = Describe("ReadWriteRoute", func() { var response = &bytes.Buffer{} BeforeEach(func() { - requested := &http.RequestLine{TheMethod: "OPTIONS", TheTarget: "/method_options"} + requested := http.NewRequestMessage("OPTIONS", "/method_options") routedRequest := router.Route(requested) Expect(routedRequest).NotTo(BeNil()) @@ -70,14 +70,14 @@ var _ = Describe("ReadWriteRoute", func() { }) It("returns MethodNotAllowed for any other method", func() { - requested := &http.RequestLine{TheMethod: "TRACE", TheTarget: "/method_options"} + requested := http.NewRequestMessage("TRACE", "/method_options") routedRequest := router.Route(requested) Expect(routedRequest).To(BeEquivalentTo(clienterror.MethodNotAllowed("GET", "HEAD", "OPTIONS", "POST", "PUT"))) }) }) It("returns nil on any other target", func() { - requested := &http.RequestLine{TheMethod: "GET", TheTarget: "/"} + requested := http.NewRequestMessage("GET", "/") routedRequest := router.Route(requested) Expect(routedRequest).To(BeNil()) }) diff --git a/teapot/route_test.go b/teapot/route_test.go index 0bec4da..57f0255 100644 --- a/teapot/route_test.go +++ b/teapot/route_test.go @@ -14,7 +14,7 @@ var _ = Describe("teapotRoute", func() { var ( router http.Route teapotMock *TeapotMock - requested *http.RequestLine + requested http.RequestMessage routedRequest http.Request ) @@ -26,14 +26,14 @@ var _ = Describe("teapotRoute", func() { }) It("routes GET requests to that target to the teapot", func() { - requested = &http.RequestLine{TheMethod: "GET", TheTarget: "/caffeine"} + requested = http.NewRequestMessage("GET", "/caffeine") routedRequest = router.Route(requested) routedRequest.Handle(&bufio.Writer{}) teapotMock.GetShouldHaveReceived("/caffeine") }) It("returns MethodNotAllowed for any other method", func() { - requested := &http.RequestLine{TheMethod: "TRACE", TheTarget: "/caffeine"} + requested := http.NewRequestMessage("TRACE", "/caffeine") routedRequest := router.Route(requested) Expect(routedRequest).To(BeEquivalentTo(clienterror.MethodNotAllowed("GET", "OPTIONS"))) }) @@ -43,7 +43,7 @@ var _ = Describe("teapotRoute", func() { teapotMock = &TeapotMock{} router = &teapot.Route{Teapot: teapotMock} - requested = &http.RequestLine{TheMethod: "GET", TheTarget: "/file.txt"} + requested = http.NewRequestMessage("GET", "/file.txt") routedRequest = router.Route(requested) Expect(routedRequest).To(BeNil()) }) From 1d815ba0cdf21273a72014fe46c558c9e2d8eed9 Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Thu, 26 Apr 2018 17:22:20 -0500 Subject: [PATCH 07/43] Now only the http package is using the concrete type --- http/method_dispatcher.go | 24 ++++++++++++------------ http/methods.go | 8 ++++---- http/parser.go | 12 ++++++------ http/parser_test.go | 2 +- http/router.go | 4 ++-- 5 files changed, 25 insertions(+), 25 deletions(-) diff --git a/http/method_dispatcher.go b/http/method_dispatcher.go index 8ee5ca0..0466736 100644 --- a/http/method_dispatcher.go +++ b/http/method_dispatcher.go @@ -8,35 +8,35 @@ import ( ) func NewRequestMessage(method, target string) RequestMessage { - return &RequestLine{ + return &requestMessage{ TheMethod: method, TheTarget: target, } } -type RequestLine struct { +type requestMessage struct { TheMethod string TheTarget string TheQueryParameters map[string]string } -func (requestLine *RequestLine) Method() string { +func (requestLine *requestMessage) Method() string { return requestLine.TheMethod } -func (requestLine *RequestLine) Target() string { +func (requestLine *requestMessage) Target() string { return requestLine.TheTarget } -func (requestLine *RequestLine) QueryParameters() map[string]string { +func (requestLine *requestMessage) QueryParameters() map[string]string { return requestLine.TheQueryParameters } -func (requestLine *RequestLine) NotImplemented() Response { +func (requestLine *requestMessage) NotImplemented() Response { return &servererror.NotImplemented{Method: requestLine.TheMethod} } -func (requestLine *RequestLine) MakeResourceRequest(resource Resource) Request { +func (requestLine *requestMessage) MakeResourceRequest(resource Resource) Request { if requestLine.TheMethod == "OPTIONS" { return &optionsRequest{ SupportedMethods: requestLine.supportedMethods(resource), @@ -56,18 +56,18 @@ func (requestLine *RequestLine) MakeResourceRequest(resource Resource) Request { return request } -func (requestLine *RequestLine) unknownHttpMethod(resource Resource) Request { +func (requestLine *requestMessage) unknownHttpMethod(resource Resource) Request { return clienterror.MethodNotAllowed(requestLine.supportedMethods(resource)...) } -func (requestLine *RequestLine) unsupportedMethod(resource Resource) Request { +func (requestLine *requestMessage) unsupportedMethod(resource Resource) Request { return clienterror.MethodNotAllowed(requestLine.supportedMethods(resource)...) } -func (requestLine *RequestLine) supportedMethods(resource Resource) []string { +func (requestLine *requestMessage) supportedMethods(resource Resource) []string { supported := []string{"OPTIONS"} for name, method := range knownMethods { - imaginaryRequest := &RequestLine{TheMethod: name, TheTarget: requestLine.TheTarget} + imaginaryRequest := &requestMessage{TheMethod: name, TheTarget: requestLine.TheTarget} request := method.MakeRequest(imaginaryRequest, resource) if request != nil { supported = append(supported, name) @@ -86,7 +86,7 @@ var knownMethods = map[string]Method{ } type Method interface { - MakeRequest(requested *RequestLine, resource Resource) Request + MakeRequest(requested *requestMessage, resource Resource) Request } // Handles requests of supported HTTP methods for a resource diff --git a/http/methods.go b/http/methods.go index 4e20166..897db32 100644 --- a/http/methods.go +++ b/http/methods.go @@ -11,7 +11,7 @@ import ( type getMethod struct{} -func (method *getMethod) MakeRequest(requested *RequestLine, resource Resource) Request { +func (method *getMethod) MakeRequest(requested *requestMessage, resource Resource) Request { supportedResource, ok := resource.(GetResource) if ok { return &getRequest{Resource: supportedResource, Target: requested.TheTarget} @@ -38,7 +38,7 @@ type GetResource interface { type headMethod struct{} -func (*headMethod) MakeRequest(requested *RequestLine, resource Resource) Request { +func (*headMethod) MakeRequest(requested *requestMessage, resource Resource) Request { supportedResource, ok := resource.(HeadResource) if ok { return &headRequest{Resource: supportedResource, Target: requested.TheTarget} @@ -79,7 +79,7 @@ func (request *optionsRequest) Handle(client io.Writer) error { type postMethod struct{} -func (*postMethod) MakeRequest(requested *RequestLine, resource Resource) Request { +func (*postMethod) MakeRequest(requested *requestMessage, resource Resource) Request { supportedResource, ok := resource.(PostResource) if ok { return &postRequest{Resource: supportedResource, Target: requested.TheTarget} @@ -106,7 +106,7 @@ type PostResource interface { type putMethod struct{} -func (*putMethod) MakeRequest(requested *RequestLine, resource Resource) Request { +func (*putMethod) MakeRequest(requested *requestMessage, resource Resource) Request { supportedResource, ok := resource.(PutResource) if ok { return &putRequest{Resource: supportedResource, Target: requested.TheTarget} diff --git a/http/parser.go b/http/parser.go index e84a6be..646c3a1 100644 --- a/http/parser.go +++ b/http/parser.go @@ -10,7 +10,7 @@ import ( // Parses an HTTP request message one line at a time. type LineRequestParser struct{} -func (parser *LineRequestParser) Parse(reader *bufio.Reader) (ok *RequestLine, err Response) { +func (parser *LineRequestParser) Parse(reader *bufio.Reader) (ok *requestMessage, err Response) { methodObject := &parseMethodObject{reader: reader} return methodObject.Parse() } @@ -19,7 +19,7 @@ type parseMethodObject struct { reader *bufio.Reader } -func (parser *parseMethodObject) Parse() (ok *RequestLine, err Response) { +func (parser *parseMethodObject) Parse() (ok *requestMessage, err Response) { requestLine, err := parser.readCRLFLine() if err != nil { return nil, err @@ -28,7 +28,7 @@ func (parser *parseMethodObject) Parse() (ok *RequestLine, err Response) { return parser.doParseRequestLine(requestLine) } -func (parser *parseMethodObject) doParseRequestLine(requestLine string) (ok *RequestLine, err Response) { +func (parser *parseMethodObject) doParseRequestLine(requestLine string) (ok *requestMessage, err Response) { requested, err := parser.parseRequestLine(requestLine) if err != nil { return nil, err @@ -37,7 +37,7 @@ func (parser *parseMethodObject) doParseRequestLine(requestLine string) (ok *Req return parser.doParseHeaders(requested) } -func (parser *parseMethodObject) doParseHeaders(requested *RequestLine) (ok *RequestLine, err Response) { +func (parser *parseMethodObject) doParseHeaders(requested *requestMessage) (ok *requestMessage, err Response) { err = parser.parseHeaders() if err != nil { return nil, err @@ -46,13 +46,13 @@ func (parser *parseMethodObject) doParseHeaders(requested *RequestLine) (ok *Req return requested, nil } -func (parser *parseMethodObject) parseRequestLine(text string) (ok *RequestLine, badRequest Response) { +func (parser *parseMethodObject) parseRequestLine(text string) (ok *requestMessage, badRequest Response) { fields := strings.Split(text, " ") if len(fields) != 3 { return nil, &clienterror.BadRequest{DisplayText: "incorrectly formatted or missing request-line"} } - return &RequestLine{ + return &requestMessage{ TheMethod: fields[0], TheTarget: fields[1], }, nil diff --git a/http/parser_test.go b/http/parser_test.go index 1e8fff7..68846cc 100644 --- a/http/parser_test.go +++ b/http/parser_test.go @@ -74,7 +74,7 @@ var _ = Describe("LineRequestParser", func() { request, err = parser.Parse(reader) }) - It("returns the parsed request", func() { + It("returns the parsed request message", func() { Expect(request.Method()).To(Equal("GET")) Expect(request.Target()).To(Equal("/foo")) }) diff --git a/http/router.go b/http/router.go index 2b6807e..c769f01 100644 --- a/http/router.go +++ b/http/router.go @@ -35,7 +35,7 @@ func (router RequestLineRouter) RouteRequest(reader *bufio.Reader) (ok Request, return router.routeRequest(requested) } -func (router RequestLineRouter) routeRequest(requested *RequestLine) (ok Request, notImplemented Response) { +func (router RequestLineRouter) routeRequest(requested *requestMessage) (ok Request, notImplemented Response) { for _, route := range router.routes { request := route.Route(requested) if request != nil { @@ -47,7 +47,7 @@ func (router RequestLineRouter) routeRequest(requested *RequestLine) (ok Request } type RequestParser interface { - Parse(reader *bufio.Reader) (ok *RequestLine, err Response) + Parse(reader *bufio.Reader) (ok *requestMessage, err Response) } type RequestMessage interface { From d9841cdd2062366d76a640e52f5cc46bef574983 Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Thu, 26 Apr 2018 20:38:10 -0500 Subject: [PATCH 08/43] Re-enabled the /parameters route --- main/cmd/factory.go | 2 +- main/cmd/factory_test.go | 2 +- playground/parameter_route.go | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/main/cmd/factory.go b/main/cmd/factory.go index b192bfb..e9cbd2d 100644 --- a/main/cmd/factory.go +++ b/main/cmd/factory.go @@ -47,7 +47,7 @@ func (factory *InterruptFactory) TCPServer(contentRootPath string, host string, func (factory *InterruptFactory) routerWithAllRoutes(contentRootPath string) http.Router { router := http.NewRouter() router.AddRoute(capability.NewRoute()) - //router.AddRoute(playground.NewParameterRoute()) + router.AddRoute(playground.NewParameterRoute()) router.AddRoute(playground.NewReadOnlyRoute()) router.AddRoute(playground.NewReadWriteRoute()) router.AddRoute(teapot.NewRoute()) diff --git a/main/cmd/factory_test.go b/main/cmd/factory_test.go index 11b1d6c..2561d66 100644 --- a/main/cmd/factory_test.go +++ b/main/cmd/factory_test.go @@ -91,7 +91,7 @@ var _ = Describe("InterruptFactory", func() { Expect(typedServer.Routes()).To(ContainElement(BeAssignableToTypeOf(capability.NewRoute()))) }) - XIt("has a playground route for parameter decoding", func() { + It("has a playground route for parameter decoding", func() { Expect(typedServer.Routes()).To(ContainElement(BeAssignableToTypeOf(playground.NewParameterRoute()))) }) diff --git a/playground/parameter_route.go b/playground/parameter_route.go index 1cddbfd..44b7099 100644 --- a/playground/parameter_route.go +++ b/playground/parameter_route.go @@ -29,10 +29,10 @@ type ParameterDecoder interface { type TheDecoder struct{} -func (decoder *TheDecoder) Get(client io.Writer, target string) { - panic("implement me") +func (decoder *TheDecoder) Name() string { + return "Parameters" } -func (decoder *TheDecoder) Name() string { +func (decoder *TheDecoder) Get(client io.Writer, target string) { panic("implement me") } From bfa2e42b7abe204c1cb70780b01f4b365987db97 Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Thu, 26 Apr 2018 21:14:31 -0500 Subject: [PATCH 09/43] Passing RequestMessage to Resource#Get so it has access to query parameters, etc... --- fs/fs_suite_test.go | 5 +++-- fs/requests.go | 4 ++-- fs/requests_test.go | 13 +++++++------ fs/route.go | 2 +- http/methods.go | 11 +++++++---- playground/parameter_route.go | 4 ++-- playground/playground_suite_test.go | 6 +++--- playground/readonly_route.go | 6 +++--- playground/readonly_route_test.go | 2 +- playground/writable_route.go | 6 +++--- playground/writable_route_test.go | 2 +- teapot/requests.go | 5 +++-- teapot/requests_test.go | 5 +++-- teapot/route.go | 2 +- teapot/teapot_suite_test.go | 5 +++-- 15 files changed, 43 insertions(+), 35 deletions(-) diff --git a/fs/fs_suite_test.go b/fs/fs_suite_test.go index 4dcbcd9..96cee6c 100644 --- a/fs/fs_suite_test.go +++ b/fs/fs_suite_test.go @@ -4,6 +4,7 @@ import ( "io" "testing" + "github.com/kkrull/gohttp/http" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) @@ -22,8 +23,8 @@ func (mock *FileSystemResourceMock) Name() string { return "File system mock" } -func (mock *FileSystemResourceMock) Get(client io.Writer, target string) { - mock.getTarget = target +func (mock *FileSystemResourceMock) Get(client io.Writer, req http.RequestMessage) { + mock.getTarget = req.Target() } func (mock *FileSystemResourceMock) GetShouldHaveReceived(target string) { diff --git a/fs/requests.go b/fs/requests.go index 4cca831..f7eebb2 100644 --- a/fs/requests.go +++ b/fs/requests.go @@ -18,8 +18,8 @@ func (controller *ReadOnlyFileSystem) Name() string { return "Readonly file system" } -func (controller *ReadOnlyFileSystem) Get(client io.Writer, target string) { - response := controller.determineResponse(target) +func (controller *ReadOnlyFileSystem) Get(client io.Writer, req http.RequestMessage) { + response := controller.determineResponse(req.Target()) response.WriteTo(client) } diff --git a/fs/requests_test.go b/fs/requests_test.go index 80beb97..d0b0c94 100644 --- a/fs/requests_test.go +++ b/fs/requests_test.go @@ -7,6 +7,7 @@ import ( "path" "github.com/kkrull/gohttp/fs" + "github.com/kkrull/gohttp/http" "github.com/kkrull/gohttp/httptest" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -30,7 +31,7 @@ var _ = Describe("ReadOnlyFileSystem", func() { Describe("#Get", func() { Context("when the resolved target does not exist", func() { BeforeEach(func() { - controller.Get(responseBuffer, "/missing.txt") + controller.Get(responseBuffer, http.NewRequestMessage("GET", "/missing.txt")) response = httptest.ParseResponse(responseBuffer) }) @@ -52,7 +53,7 @@ var _ = Describe("ReadOnlyFileSystem", func() { BeforeEach(func() { existingFile := path.Join(basePath, "readable.txt") Expect(createTextFile(existingFile, "A")).To(Succeed()) - controller.Get(responseBuffer, "/readable.txt") + controller.Get(responseBuffer, http.NewRequestMessage("GET", "/readable.txt")) response = httptest.ParseResponse(responseBuffer) }) @@ -77,7 +78,7 @@ var _ = Describe("ReadOnlyFileSystem", func() { }) It("sets Content-Type to the MIME type registered for that extension", func() { - controller.Get(responseBuffer, "/image.jpeg") + controller.Get(responseBuffer, http.NewRequestMessage("GET", "/image.jpeg")) response = httptest.ParseResponse(responseBuffer) response.HeaderShould("Content-Type", Equal("image/jpeg")) }) @@ -90,7 +91,7 @@ var _ = Describe("ReadOnlyFileSystem", func() { }) It("sets Content-Type to text/plain", func() { - controller.Get(responseBuffer, "/assumed-text") + controller.Get(responseBuffer, http.NewRequestMessage("GET", "/assumed-text")) response = httptest.ParseResponse(responseBuffer) response.HeaderShould("Content-Type", Equal("text/plain")) }) @@ -103,7 +104,7 @@ var _ = Describe("ReadOnlyFileSystem", func() { }) It("responds with 200 OK", func() { - controller.Get(responseBuffer, "/") + controller.Get(responseBuffer, http.NewRequestMessage("GET", "/")) response = httptest.ParseResponse(responseBuffer) response.StatusShouldBe(200, "OK") }) @@ -117,7 +118,7 @@ var _ = Describe("ReadOnlyFileSystem", func() { ) BeforeEach(func() { - controller.Get(getResponseBuffer, "/missing.txt") + controller.Get(getResponseBuffer, http.NewRequestMessage("GET", "/missing.txt")) getResponse = httptest.ParseResponse(getResponseBuffer) controller.Head(responseBuffer, "/missing.txt") diff --git a/fs/route.go b/fs/route.go index 3c4cce5..524e837 100644 --- a/fs/route.go +++ b/fs/route.go @@ -25,6 +25,6 @@ func (route FileSystemRoute) Route(requested http.RequestMessage) http.Request { // Represents files and directories on the file system type FileSystemResource interface { Name() string - Get(client io.Writer, target string) + Get(client io.Writer, req http.RequestMessage) Head(client io.Writer, target string) } diff --git a/http/methods.go b/http/methods.go index 897db32..c3fbfaa 100644 --- a/http/methods.go +++ b/http/methods.go @@ -14,24 +14,27 @@ type getMethod struct{} func (method *getMethod) MakeRequest(requested *requestMessage, resource Resource) Request { supportedResource, ok := resource.(GetResource) if ok { - return &getRequest{Resource: supportedResource, Target: requested.TheTarget} + return &getRequest{ + Message: requested, + Resource: supportedResource, + } } return nil } type getRequest struct { + Message RequestMessage Resource GetResource - Target string } func (request *getRequest) Handle(client io.Writer) error { - request.Resource.Get(client, request.Target) + request.Resource.Get(client, request.Message) return nil } type GetResource interface { - Get(client io.Writer, target string) + Get(client io.Writer, req RequestMessage) } /* HEAD */ diff --git a/playground/parameter_route.go b/playground/parameter_route.go index 44b7099..78c215d 100644 --- a/playground/parameter_route.go +++ b/playground/parameter_route.go @@ -24,7 +24,7 @@ func (route *ParameterRoute) Route(requested http.RequestMessage) http.Request { type ParameterDecoder interface { Name() string - Get(client io.Writer, target string) + Get(client io.Writer, req http.RequestMessage) } type TheDecoder struct{} @@ -33,6 +33,6 @@ func (decoder *TheDecoder) Name() string { return "Parameters" } -func (decoder *TheDecoder) Get(client io.Writer, target string) { +func (decoder *TheDecoder) Get(client io.Writer, req http.RequestMessage) { panic("implement me") } diff --git a/playground/playground_suite_test.go b/playground/playground_suite_test.go index e4429fb..9d0edcc 100644 --- a/playground/playground_suite_test.go +++ b/playground/playground_suite_test.go @@ -25,7 +25,7 @@ func (mock *ParameterDecoderMock) Name() string { return "Parameter Decoder Mock" } -func (mock *ParameterDecoderMock) Get(client io.Writer, target string) { +func (mock *ParameterDecoderMock) Get(client io.Writer, req http.RequestMessage) { //TODO KDK: Work here to get the parameters and expand the interface //mock.getParameters = map[string]string { // "two": "2", @@ -48,7 +48,7 @@ func (mock *ReadOnlyResourceMock) Name() string { return "Readonly Mock" } -func (mock *ReadOnlyResourceMock) Get(client io.Writer, target string) { +func (mock *ReadOnlyResourceMock) Get(client io.Writer, req http.RequestMessage) { mock.getCalled = true } @@ -77,7 +77,7 @@ func (mock *ReadWriteResourceMock) Name() string { return "Read/Write Mock" } -func (mock *ReadWriteResourceMock) Get(client io.Writer, target string) { +func (mock *ReadWriteResourceMock) Get(client io.Writer, req http.RequestMessage) { mock.getCalled = true } diff --git a/playground/readonly_route.go b/playground/readonly_route.go index e66147a..947a4b9 100644 --- a/playground/readonly_route.go +++ b/playground/readonly_route.go @@ -24,7 +24,7 @@ func (route *ReadOnlyRoute) Route(requested http.RequestMessage) http.Request { type ReadOnlyResource interface { Name() string - Get(client io.Writer, target string) + Get(client io.Writer, req http.RequestMessage) Head(client io.Writer, target string) } @@ -35,8 +35,8 @@ func (controller *ReadableNopResource) Name() string { return "Readonly NOP" } -func (controller *ReadableNopResource) Get(client io.Writer, target string) { - controller.Head(client, target) +func (controller *ReadableNopResource) Get(client io.Writer, req http.RequestMessage) { + controller.Head(client, req.Target()) } func (controller *ReadableNopResource) Head(client io.Writer, target string) { diff --git a/playground/readonly_route_test.go b/playground/readonly_route_test.go index a5428cf..360fa09 100644 --- a/playground/readonly_route_test.go +++ b/playground/readonly_route_test.go @@ -87,7 +87,7 @@ var _ = Describe("ReadableNopResource", func() { Describe("#Get", func() { BeforeEach(func() { - controller.Get(response, "/") + controller.Get(response, http.NewRequestMessage("GET", "/")) }) It("responds 200 OK with no body", httptest.ShouldHaveNoBody(response, 200, "OK")) diff --git a/playground/writable_route.go b/playground/writable_route.go index 9f3a995..4f33029 100644 --- a/playground/writable_route.go +++ b/playground/writable_route.go @@ -27,7 +27,7 @@ func (route *ReadWriteRoute) Route(requested http.RequestMessage) http.Request { type ReadWriteResource interface { Name() string - Get(client io.Writer, target string) + Get(client io.Writer, req http.RequestMessage) Head(client io.Writer, target string) Post(client io.Writer, target string) Put(client io.Writer, target string) @@ -40,8 +40,8 @@ func (controller *ReadWriteNopResource) Name() string { return "Read/Write NOP" } -func (controller *ReadWriteNopResource) Get(client io.Writer, target string) { - controller.Head(client, target) +func (controller *ReadWriteNopResource) Get(client io.Writer, req http.RequestMessage) { + controller.Head(client, req.Target()) } func (controller *ReadWriteNopResource) Head(client io.Writer, target string) { diff --git a/playground/writable_route_test.go b/playground/writable_route_test.go index 14790cf..a586df3 100644 --- a/playground/writable_route_test.go +++ b/playground/writable_route_test.go @@ -97,7 +97,7 @@ var _ = Describe("ReadWriteNopResource", func() { Describe("#Get", func() { BeforeEach(func() { - controller.Get(response, "/") + controller.Get(response, http.NewRequestMessage("GET", "/")) }) It("responds 200 OK with no body", httptest.ShouldHaveNoBody(response, 200, "OK")) diff --git a/teapot/requests.go b/teapot/requests.go index 4bd6614..48091a0 100644 --- a/teapot/requests.go +++ b/teapot/requests.go @@ -3,6 +3,7 @@ package teapot import ( "io" + "github.com/kkrull/gohttp/http" "github.com/kkrull/gohttp/msg" ) @@ -22,13 +23,13 @@ func (teapot *IdentityTeapot) RespondsTo(target string) bool { } } -func (teapot *IdentityTeapot) Get(client io.Writer, target string) { +func (teapot *IdentityTeapot) Get(client io.Writer, req http.RequestMessage) { var beverageRequestHandlers = map[string]func(writer io.Writer){ "/coffee": teapot.getCoffee, "/tea": teapot.getTea, } - handler := beverageRequestHandlers[target] + handler := beverageRequestHandlers[req.Target()] handler(client) } diff --git a/teapot/requests_test.go b/teapot/requests_test.go index a023501..403fe82 100644 --- a/teapot/requests_test.go +++ b/teapot/requests_test.go @@ -3,6 +3,7 @@ package teapot_test import ( "bytes" + "github.com/kkrull/gohttp/http" "github.com/kkrull/gohttp/httptest" "github.com/kkrull/gohttp/teapot" . "github.com/onsi/ginkgo" @@ -34,7 +35,7 @@ var _ = Describe("IdentityTeapot", func() { responseBuffer = &bytes.Buffer{} theTeapot = &teapot.IdentityTeapot{} - theTeapot.Get(responseBuffer, "/coffee") + theTeapot.Get(responseBuffer, http.NewRequestMessage("GET", "/coffee")) response = httptest.ParseResponse(responseBuffer) }) @@ -60,7 +61,7 @@ var _ = Describe("IdentityTeapot", func() { responseBuffer = &bytes.Buffer{} theTeapot = &teapot.IdentityTeapot{} - theTeapot.Get(responseBuffer, "/tea") + theTeapot.Get(responseBuffer, http.NewRequestMessage("GET", "/tea")) response = httptest.ParseResponse(responseBuffer) }) diff --git a/teapot/route.go b/teapot/route.go index a335e06..6241e92 100644 --- a/teapot/route.go +++ b/teapot/route.go @@ -25,6 +25,6 @@ func (route *Route) Route(requested http.RequestMessage) http.Request { type Teapot interface { Name() string - Get(client io.Writer, target string) + Get(client io.Writer, req http.RequestMessage) RespondsTo(target string) bool } diff --git a/teapot/teapot_suite_test.go b/teapot/teapot_suite_test.go index bdae47e..d01ca58 100644 --- a/teapot/teapot_suite_test.go +++ b/teapot/teapot_suite_test.go @@ -4,6 +4,7 @@ import ( "io" "testing" + "github.com/kkrull/gohttp/http" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) @@ -26,8 +27,8 @@ func (mock *TeapotMock) RespondsTo(target string) bool { return mock.RespondsToTarget == target } -func (mock *TeapotMock) Get(client io.Writer, target string) { - mock.getTarget = target +func (mock *TeapotMock) Get(client io.Writer, req http.RequestMessage) { + mock.getTarget = req.Target() } func (mock *TeapotMock) GetShouldHaveReceived(target string) { From 314070dc169621e953b375889629f8d7db28e12b Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Fri, 27 Apr 2018 08:17:42 -0500 Subject: [PATCH 10/43] Extracted method-specific RequestMessage constructors --- capability/route_test.go | 6 ++--- fs/requests.go | 2 +- fs/requests_test.go | 12 +++++----- fs/route_test.go | 6 ++--- http/method_dispatcher.go | 35 ++++++++++++++++++++++++++++++ playground/parameter_route_test.go | 8 +++---- playground/readonly_route_test.go | 8 +++---- playground/writable_route_test.go | 8 +++---- teapot/requests_test.go | 4 ++-- teapot/route_test.go | 6 ++--- 10 files changed, 65 insertions(+), 30 deletions(-) diff --git a/capability/route_test.go b/capability/route_test.go index 5e1231b..8450f36 100644 --- a/capability/route_test.go +++ b/capability/route_test.go @@ -42,21 +42,21 @@ var _ = Describe("ServerCapabilityRoute", func() { Context("when the target is *", func() { It("routes OPTIONS to ServerResource", func() { - requested = http.NewRequestMessage("OPTIONS", "*") + requested = http.NewOptionsMessage("*") routedRequest = router.Route(requested) routedRequest.Handle(&bufio.Writer{}) controller.OptionsShouldHaveBeenCalled() }) It("returns MethodNotAllowed for any other method", func() { - requested = http.NewRequestMessage("GET", "*") + requested = http.NewGetMessage("*") routedRequest = router.Route(requested) Expect(routedRequest).To(BeEquivalentTo(clienterror.MethodNotAllowed("OPTIONS"))) }) }) It("returns nil to pass on any other target", func() { - requested = http.NewRequestMessage("OPTIONS", "/") + requested = http.NewOptionsMessage("/") routedRequest = router.Route(requested) Expect(routedRequest).To(BeNil()) }) diff --git a/fs/requests.go b/fs/requests.go index f7eebb2..9c6966d 100644 --- a/fs/requests.go +++ b/fs/requests.go @@ -18,7 +18,7 @@ func (controller *ReadOnlyFileSystem) Name() string { return "Readonly file system" } -func (controller *ReadOnlyFileSystem) Get(client io.Writer, req http.RequestMessage) { +func (controller *ReadOnlyFileSystem) Get(client io.Writer, req http.RequestMessage) { //TODO KDK: Should req be called something else? response := controller.determineResponse(req.Target()) response.WriteTo(client) } diff --git a/fs/requests_test.go b/fs/requests_test.go index d0b0c94..17926e2 100644 --- a/fs/requests_test.go +++ b/fs/requests_test.go @@ -31,7 +31,7 @@ var _ = Describe("ReadOnlyFileSystem", func() { Describe("#Get", func() { Context("when the resolved target does not exist", func() { BeforeEach(func() { - controller.Get(responseBuffer, http.NewRequestMessage("GET", "/missing.txt")) + controller.Get(responseBuffer, http.NewGetMessage("/missing.txt")) response = httptest.ParseResponse(responseBuffer) }) @@ -53,7 +53,7 @@ var _ = Describe("ReadOnlyFileSystem", func() { BeforeEach(func() { existingFile := path.Join(basePath, "readable.txt") Expect(createTextFile(existingFile, "A")).To(Succeed()) - controller.Get(responseBuffer, http.NewRequestMessage("GET", "/readable.txt")) + controller.Get(responseBuffer, http.NewGetMessage("/readable.txt")) response = httptest.ParseResponse(responseBuffer) }) @@ -78,7 +78,7 @@ var _ = Describe("ReadOnlyFileSystem", func() { }) It("sets Content-Type to the MIME type registered for that extension", func() { - controller.Get(responseBuffer, http.NewRequestMessage("GET", "/image.jpeg")) + controller.Get(responseBuffer, http.NewGetMessage("/image.jpeg")) response = httptest.ParseResponse(responseBuffer) response.HeaderShould("Content-Type", Equal("image/jpeg")) }) @@ -91,7 +91,7 @@ var _ = Describe("ReadOnlyFileSystem", func() { }) It("sets Content-Type to text/plain", func() { - controller.Get(responseBuffer, http.NewRequestMessage("GET", "/assumed-text")) + controller.Get(responseBuffer, http.NewGetMessage("/assumed-text")) response = httptest.ParseResponse(responseBuffer) response.HeaderShould("Content-Type", Equal("text/plain")) }) @@ -104,7 +104,7 @@ var _ = Describe("ReadOnlyFileSystem", func() { }) It("responds with 200 OK", func() { - controller.Get(responseBuffer, http.NewRequestMessage("GET", "/")) + controller.Get(responseBuffer, http.NewGetMessage("/")) response = httptest.ParseResponse(responseBuffer) response.StatusShouldBe(200, "OK") }) @@ -118,7 +118,7 @@ var _ = Describe("ReadOnlyFileSystem", func() { ) BeforeEach(func() { - controller.Get(getResponseBuffer, http.NewRequestMessage("GET", "/missing.txt")) + controller.Get(getResponseBuffer, http.NewGetMessage("/missing.txt")) getResponse = httptest.ParseResponse(getResponseBuffer) controller.Head(responseBuffer, "/missing.txt") diff --git a/fs/route_test.go b/fs/route_test.go index 3ec67f8..9986b0a 100644 --- a/fs/route_test.go +++ b/fs/route_test.go @@ -38,21 +38,21 @@ var _ = Describe("FileSystemRoute", func() { Describe("#Route", func() { It("routes GET requests to GetRequest", func() { - requested := http.NewRequestMessage("GET", "/foo") + requested := http.NewGetMessage("/foo") routedRequest := route.Route(requested) routedRequest.Handle(response) resource.GetShouldHaveReceived("/foo") }) It("routes HEAD requests to HeadRequest", func() { - requested := http.NewRequestMessage("HEAD", "/foo") + requested := http.NewHeadMessage("/foo") routedRequest := route.Route(requested) routedRequest.Handle(response) resource.HeadShouldHaveReceived("/foo") }) It("routes any other method to MethodNotAllowed", func() { - requested := http.NewRequestMessage("TRACE", "/") + requested := http.NewTraceMessage("/") routedRequest := route.Route(requested) Expect(routedRequest).To(BeEquivalentTo(clienterror.MethodNotAllowed("GET", "HEAD", "OPTIONS"))) }) diff --git a/http/method_dispatcher.go b/http/method_dispatcher.go index 0466736..e2efd86 100644 --- a/http/method_dispatcher.go +++ b/http/method_dispatcher.go @@ -7,6 +7,41 @@ import ( "github.com/kkrull/gohttp/msg/servererror" ) +func NewGetMessage(target string) RequestMessage { + return &requestMessage{ + TheMethod: "GET", + TheTarget: target, + } +} + +func NewHeadMessage(target string) RequestMessage { + return &requestMessage{ + TheMethod: "HEAD", + TheTarget: target, + } +} + +func NewOptionsMessage(target string) RequestMessage { + return &requestMessage{ + TheMethod: "OPTIONS", + TheTarget: target, + } +} + +func NewPutMessage(target string) RequestMessage { + return &requestMessage{ + TheMethod: "PUT", + TheTarget: target, + } +} + +func NewTraceMessage(target string) RequestMessage { + return &requestMessage{ + TheMethod: "TRACE", + TheTarget: target, + } +} + func NewRequestMessage(method, target string) RequestMessage { return &requestMessage{ TheMethod: method, diff --git a/playground/parameter_route_test.go b/playground/parameter_route_test.go index 46e9696..f52fd20 100644 --- a/playground/parameter_route_test.go +++ b/playground/parameter_route_test.go @@ -41,7 +41,7 @@ var _ = Describe("ParameterRoute", func() { "one": "1", "two": "2", } - requested := http.NewRequestMessage("GET", "/parameters") //TODO KDK: Add query parameters + requested := http.NewGetMessage("/parameters") //TODO KDK: Add query parameters routedRequest := router.Route(requested) Expect(routedRequest).NotTo(BeNil()) @@ -51,7 +51,7 @@ var _ = Describe("ParameterRoute", func() { Context("when the method is OPTIONS", func() { BeforeEach(func() { - requested := http.NewRequestMessage("OPTIONS", "/parameters") + requested := http.NewOptionsMessage("/parameters") routedRequest := router.Route(requested) Expect(routedRequest).NotTo(BeNil()) routedRequest.Handle(response) @@ -63,14 +63,14 @@ var _ = Describe("ParameterRoute", func() { }) It("replies Method Not Allowed on any other method", func() { - requested := http.NewRequestMessage("TRACE", "/parameters") + requested := http.NewTraceMessage("/parameters") routedRequest := router.Route(requested) Expect(routedRequest).To(BeAssignableToTypeOf(clienterror.MethodNotAllowed())) }) }) It("returns nil for any other target", func() { - requested := http.NewRequestMessage("GET", "/") + requested := http.NewGetMessage("/") Expect(router.Route(requested)).To(BeNil()) }) }) diff --git a/playground/readonly_route_test.go b/playground/readonly_route_test.go index 360fa09..d9038cb 100644 --- a/playground/readonly_route_test.go +++ b/playground/readonly_route_test.go @@ -46,7 +46,7 @@ var _ = Describe("ReadOnlyRoute", func() { var response = &bytes.Buffer{} BeforeEach(func() { - requested := http.NewRequestMessage("OPTIONS", "/method_options2") + requested := http.NewOptionsMessage("/method_options2") routedRequest := router.Route(requested) Expect(routedRequest).NotTo(BeNil()) @@ -60,14 +60,14 @@ var _ = Describe("ReadOnlyRoute", func() { }) It("returns MethodNotAllowed for any other method", func() { - requested := http.NewRequestMessage("PUT", "/method_options2") + requested := http.NewPutMessage("/method_options2") routedRequest := router.Route(requested) Expect(routedRequest).To(BeEquivalentTo(clienterror.MethodNotAllowed("GET", "HEAD", "OPTIONS"))) }) }) It("returns nil on any other target", func() { - requested := http.NewRequestMessage("GET", "/") + requested := http.NewGetMessage("/") routedRequest := router.Route(requested) Expect(routedRequest).To(BeNil()) }) @@ -87,7 +87,7 @@ var _ = Describe("ReadableNopResource", func() { Describe("#Get", func() { BeforeEach(func() { - controller.Get(response, http.NewRequestMessage("GET", "/")) + controller.Get(response, http.NewGetMessage("/")) }) It("responds 200 OK with no body", httptest.ShouldHaveNoBody(response, 200, "OK")) diff --git a/playground/writable_route_test.go b/playground/writable_route_test.go index a586df3..94590db 100644 --- a/playground/writable_route_test.go +++ b/playground/writable_route_test.go @@ -46,7 +46,7 @@ var _ = Describe("ReadWriteRoute", func() { var response = &bytes.Buffer{} BeforeEach(func() { - requested := http.NewRequestMessage("OPTIONS", "/method_options") + requested := http.NewOptionsMessage("/method_options") routedRequest := router.Route(requested) Expect(routedRequest).NotTo(BeNil()) @@ -70,14 +70,14 @@ var _ = Describe("ReadWriteRoute", func() { }) It("returns MethodNotAllowed for any other method", func() { - requested := http.NewRequestMessage("TRACE", "/method_options") + requested := http.NewTraceMessage("/method_options") routedRequest := router.Route(requested) Expect(routedRequest).To(BeEquivalentTo(clienterror.MethodNotAllowed("GET", "HEAD", "OPTIONS", "POST", "PUT"))) }) }) It("returns nil on any other target", func() { - requested := http.NewRequestMessage("GET", "/") + requested := http.NewGetMessage("/") routedRequest := router.Route(requested) Expect(routedRequest).To(BeNil()) }) @@ -97,7 +97,7 @@ var _ = Describe("ReadWriteNopResource", func() { Describe("#Get", func() { BeforeEach(func() { - controller.Get(response, http.NewRequestMessage("GET", "/")) + controller.Get(response, http.NewGetMessage("/")) }) It("responds 200 OK with no body", httptest.ShouldHaveNoBody(response, 200, "OK")) diff --git a/teapot/requests_test.go b/teapot/requests_test.go index 403fe82..a12c712 100644 --- a/teapot/requests_test.go +++ b/teapot/requests_test.go @@ -35,7 +35,7 @@ var _ = Describe("IdentityTeapot", func() { responseBuffer = &bytes.Buffer{} theTeapot = &teapot.IdentityTeapot{} - theTeapot.Get(responseBuffer, http.NewRequestMessage("GET", "/coffee")) + theTeapot.Get(responseBuffer, http.NewGetMessage("/coffee")) response = httptest.ParseResponse(responseBuffer) }) @@ -61,7 +61,7 @@ var _ = Describe("IdentityTeapot", func() { responseBuffer = &bytes.Buffer{} theTeapot = &teapot.IdentityTeapot{} - theTeapot.Get(responseBuffer, http.NewRequestMessage("GET", "/tea")) + theTeapot.Get(responseBuffer, http.NewGetMessage("/tea")) response = httptest.ParseResponse(responseBuffer) }) diff --git a/teapot/route_test.go b/teapot/route_test.go index 57f0255..4b7907f 100644 --- a/teapot/route_test.go +++ b/teapot/route_test.go @@ -26,14 +26,14 @@ var _ = Describe("teapotRoute", func() { }) It("routes GET requests to that target to the teapot", func() { - requested = http.NewRequestMessage("GET", "/caffeine") + requested = http.NewGetMessage("/caffeine") routedRequest = router.Route(requested) routedRequest.Handle(&bufio.Writer{}) teapotMock.GetShouldHaveReceived("/caffeine") }) It("returns MethodNotAllowed for any other method", func() { - requested := http.NewRequestMessage("TRACE", "/caffeine") + requested := http.NewTraceMessage("/caffeine") routedRequest := router.Route(requested) Expect(routedRequest).To(BeEquivalentTo(clienterror.MethodNotAllowed("GET", "OPTIONS"))) }) @@ -43,7 +43,7 @@ var _ = Describe("teapotRoute", func() { teapotMock = &TeapotMock{} router = &teapot.Route{Teapot: teapotMock} - requested = http.NewRequestMessage("GET", "/file.txt") + requested = http.NewGetMessage("/file.txt") routedRequest = router.Route(requested) Expect(routedRequest).To(BeNil()) }) From 52ff3a9bcdf6af64e7c7dc8d43b9de1fb4f7e209 Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Fri, 27 Apr 2018 11:23:45 -0500 Subject: [PATCH 11/43] Notes --- http/parser.go | 2 +- playground/parameter_route_test.go | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/http/parser.go b/http/parser.go index 646c3a1..d7fbc76 100644 --- a/http/parser.go +++ b/http/parser.go @@ -52,7 +52,7 @@ func (parser *parseMethodObject) parseRequestLine(text string) (ok *requestMessa return nil, &clienterror.BadRequest{DisplayText: "incorrectly formatted or missing request-line"} } - return &requestMessage{ + return &requestMessage{ //TODO KDK: RFC 3986 Section 3.4 -- the query is between ? and # or the end of the URI, non-inclusive TheMethod: fields[0], TheTarget: fields[1], }, nil diff --git a/playground/parameter_route_test.go b/playground/parameter_route_test.go index f52fd20..7debb01 100644 --- a/playground/parameter_route_test.go +++ b/playground/parameter_route_test.go @@ -41,7 +41,10 @@ var _ = Describe("ParameterRoute", func() { "one": "1", "two": "2", } - requested := http.NewGetMessage("/parameters") //TODO KDK: Add query parameters + requested := http.NewGetMessage( + "/parameters", + Query{Name: "one", Value: "1"}, + ) //TODO KDK: Add query parameters routedRequest := router.Route(requested) Expect(routedRequest).NotTo(BeNil()) From db3852bf08454afae5864f56c390d721e564bdc7 Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Fri, 27 Apr 2018 11:39:36 -0500 Subject: [PATCH 12/43] Stubbing in a query for query parameters --- http/method_dispatcher.go | 5 ++--- http/router.go | 6 +++++- httptest/request_message.go | 30 ++++++++++++++++++++++++++++++ playground/parameter_route_test.go | 20 +++++++++++--------- 4 files changed, 48 insertions(+), 13 deletions(-) create mode 100644 httptest/request_message.go diff --git a/http/method_dispatcher.go b/http/method_dispatcher.go index e2efd86..a7f689a 100644 --- a/http/method_dispatcher.go +++ b/http/method_dispatcher.go @@ -52,7 +52,6 @@ func NewRequestMessage(method, target string) RequestMessage { type requestMessage struct { TheMethod string TheTarget string - TheQueryParameters map[string]string } func (requestLine *requestMessage) Method() string { @@ -63,8 +62,8 @@ func (requestLine *requestMessage) Target() string { return requestLine.TheTarget } -func (requestLine *requestMessage) QueryParameters() map[string]string { - return requestLine.TheQueryParameters +func (requestLine *requestMessage) QueryParameters() []QueryParameter { + panic("implement me") } func (requestLine *requestMessage) NotImplemented() Response { diff --git a/http/router.go b/http/router.go index c769f01..94bec55 100644 --- a/http/router.go +++ b/http/router.go @@ -54,9 +54,13 @@ type RequestMessage interface { MakeResourceRequest(resource Resource) Request Method() string Target() string - QueryParameters() map[string]string + QueryParameters() []QueryParameter } type Route interface { Route(requested RequestMessage) Request } + +type QueryParameter struct { + Name, Value string +} \ No newline at end of file diff --git a/httptest/request_message.go b/httptest/request_message.go new file mode 100644 index 0000000..a8e45a2 --- /dev/null +++ b/httptest/request_message.go @@ -0,0 +1,30 @@ +package httptest + +import "github.com/kkrull/gohttp/http" + +type RequestMessage struct { + ItsMethod string + ItsPath string + ItsTarget string + queryParameters []http.QueryParameter +} + +func (*RequestMessage) MakeResourceRequest(resource http.Resource) http.Request { + panic("implement me") +} + +func (message *RequestMessage) Method() string { + return message.ItsMethod +} + +func (message *RequestMessage) Target() string { + return message.ItsTarget +} + +func (message *RequestMessage) AddQueryParameter(name string, value string) { + message.queryParameters = append(message.queryParameters, http.QueryParameter{Name: name, Value: value}) +} + +func (message *RequestMessage) QueryParameters() []http.QueryParameter { + return message.queryParameters +} diff --git a/playground/parameter_route_test.go b/playground/parameter_route_test.go index 7debb01..e5136ee 100644 --- a/playground/parameter_route_test.go +++ b/playground/parameter_route_test.go @@ -36,20 +36,22 @@ var _ = Describe("ParameterRoute", func() { }) Context("when the target is /parameters", func() { - XIt("routes GET to ParameterDecoder#Get with the decoded query parameters", func() { - parameters := map[string]string{ - "one": "1", - "two": "2", + It("routes GET to ParameterDecoder#Get with the decoded query parameters", func() { + requested := &httptest.RequestMessage{ + ItsMethod: "GET", + ItsTarget: "/parameters?one=1;two=2", + ItsPath: "/parameters", } - requested := http.NewGetMessage( - "/parameters", - Query{Name: "one", Value: "1"}, - ) //TODO KDK: Add query parameters + requested.AddQueryParameter("one", "1") + requested.AddQueryParameter("two", "2") routedRequest := router.Route(requested) Expect(routedRequest).NotTo(BeNil()) routedRequest.Handle(response) - decoder.GetShouldHaveReceived(parameters) + decoder.GetShouldHaveReceived(map[string]string{ + "one": "1", + "two": "2", + }) }) Context("when the method is OPTIONS", func() { From 04bfb675cae5ba8db4ef31fc39e56e781ae69394 Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Fri, 27 Apr 2018 18:33:34 -0500 Subject: [PATCH 13/43] Looking at the path instead of the whole target --- http/method_dispatcher.go | 6 +++++ http/method_dispatcher_test.go | 22 +++++++++++++++++++ http/router.go | 4 +++- httptest/request_message.go | 35 ++++++++++++++++++++++-------- playground/parameter_route.go | 2 +- playground/parameter_route_test.go | 18 +++++---------- 6 files changed, 64 insertions(+), 23 deletions(-) create mode 100644 http/method_dispatcher_test.go diff --git a/http/method_dispatcher.go b/http/method_dispatcher.go index a7f689a..72de1a7 100644 --- a/http/method_dispatcher.go +++ b/http/method_dispatcher.go @@ -2,6 +2,7 @@ package http import ( "sort" + "strings" "github.com/kkrull/gohttp/msg/clienterror" "github.com/kkrull/gohttp/msg/servererror" @@ -58,6 +59,11 @@ func (requestLine *requestMessage) Method() string { return requestLine.TheMethod } +func (requestLine *requestMessage) Path() string { + fields := strings.Split(requestLine.TheTarget, "?") + return fields[0] +} + func (requestLine *requestMessage) Target() string { return requestLine.TheTarget } diff --git a/http/method_dispatcher_test.go b/http/method_dispatcher_test.go new file mode 100644 index 0000000..54ed005 --- /dev/null +++ b/http/method_dispatcher_test.go @@ -0,0 +1,22 @@ +package http_test + +import ( + "github.com/kkrull/gohttp/http" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = Describe("RequestMessage", func() { + var message http.RequestMessage + + Describe("#Path", func() { + It("returns the full target, for a target without a query or fragment", func() { + message = http.NewGetMessage("/widget") + Expect(message.Path()).To(Equal("/widget")) + }) + It("returns the part of the target before any ?", func() { + message = http.NewGetMessage("/widget?field=value") + Expect(message.Path()).To(Equal("/widget")) + }) + }) +}) diff --git a/http/router.go b/http/router.go index 94bec55..78b8994 100644 --- a/http/router.go +++ b/http/router.go @@ -51,10 +51,12 @@ type RequestParser interface { } type RequestMessage interface { - MakeResourceRequest(resource Resource) Request Method() string Target() string + Path() string QueryParameters() []QueryParameter + + MakeResourceRequest(resource Resource) Request } type Route interface { diff --git a/httptest/request_message.go b/httptest/request_message.go index a8e45a2..328bbfc 100644 --- a/httptest/request_message.go +++ b/httptest/request_message.go @@ -1,24 +1,32 @@ package httptest -import "github.com/kkrull/gohttp/http" +import ( + "github.com/kkrull/gohttp/http" + + . "github.com/onsi/gomega" +) type RequestMessage struct { - ItsMethod string - ItsPath string - ItsTarget string + MethodReturns string + PathReturns string + TargetReturns string + + MakeResourceRequestReturns http.Request + makeResourceRequestReceived http.Resource + queryParameters []http.QueryParameter } -func (*RequestMessage) MakeResourceRequest(resource http.Resource) http.Request { - panic("implement me") +func (message *RequestMessage) Method() string { + return message.MethodReturns } -func (message *RequestMessage) Method() string { - return message.ItsMethod +func (message *RequestMessage) Path() string { + return message.PathReturns } func (message *RequestMessage) Target() string { - return message.ItsTarget + return message.TargetReturns } func (message *RequestMessage) AddQueryParameter(name string, value string) { @@ -28,3 +36,12 @@ func (message *RequestMessage) AddQueryParameter(name string, value string) { func (message *RequestMessage) QueryParameters() []http.QueryParameter { return message.queryParameters } + +func (message *RequestMessage) MakeResourceRequest(resource http.Resource) http.Request { + message.makeResourceRequestReceived = resource + return message.MakeResourceRequestReturns +} + +func (message *RequestMessage) MakeResourceRequestShouldHaveReceived(resource http.Resource) { + ExpectWithOffset(1, message.makeResourceRequestReceived).To(BeIdenticalTo(resource)) +} diff --git a/playground/parameter_route.go b/playground/parameter_route.go index 78c215d..56fade0 100644 --- a/playground/parameter_route.go +++ b/playground/parameter_route.go @@ -15,7 +15,7 @@ type ParameterRoute struct { } func (route *ParameterRoute) Route(requested http.RequestMessage) http.Request { - if requested.Target() != "/parameters" { + if requested.Path() != "/parameters" { return nil } diff --git a/playground/parameter_route_test.go b/playground/parameter_route_test.go index e5136ee..d88f3d0 100644 --- a/playground/parameter_route_test.go +++ b/playground/parameter_route_test.go @@ -5,6 +5,7 @@ import ( "github.com/kkrull/gohttp/http" "github.com/kkrull/gohttp/httptest" + "github.com/kkrull/gohttp/mock" "github.com/kkrull/gohttp/msg/clienterror" "github.com/kkrull/gohttp/playground" . "github.com/onsi/ginkgo" @@ -37,21 +38,14 @@ var _ = Describe("ParameterRoute", func() { Context("when the target is /parameters", func() { It("routes GET to ParameterDecoder#Get with the decoded query parameters", func() { + request := &mock.Request{} requested := &httptest.RequestMessage{ - ItsMethod: "GET", - ItsTarget: "/parameters?one=1;two=2", - ItsPath: "/parameters", + PathReturns: "/parameters", + MakeResourceRequestReturns: request, } - requested.AddQueryParameter("one", "1") - requested.AddQueryParameter("two", "2") - routedRequest := router.Route(requested) - Expect(routedRequest).NotTo(BeNil()) - routedRequest.Handle(response) - decoder.GetShouldHaveReceived(map[string]string{ - "one": "1", - "two": "2", - }) + Expect(router.Route(requested)).To(BeIdenticalTo(request)) + requested.MakeResourceRequestShouldHaveReceived(decoder) }) Context("when the method is OPTIONS", func() { From dc57d348bc00a5cf6ab58c7063cfb0d01cb8cd82 Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Fri, 27 Apr 2018 18:35:22 -0500 Subject: [PATCH 14/43] Renamed fields in requestMessage --- http/method_dispatcher.go | 72 +++++++++++++++++++-------------------- http/methods.go | 6 ++-- http/parser.go | 4 +-- 3 files changed, 41 insertions(+), 41 deletions(-) diff --git a/http/method_dispatcher.go b/http/method_dispatcher.go index 72de1a7..233772d 100644 --- a/http/method_dispatcher.go +++ b/http/method_dispatcher.go @@ -10,104 +10,104 @@ import ( func NewGetMessage(target string) RequestMessage { return &requestMessage{ - TheMethod: "GET", - TheTarget: target, + method: "GET", + target: target, } } func NewHeadMessage(target string) RequestMessage { return &requestMessage{ - TheMethod: "HEAD", - TheTarget: target, + method: "HEAD", + target: target, } } func NewOptionsMessage(target string) RequestMessage { return &requestMessage{ - TheMethod: "OPTIONS", - TheTarget: target, + method: "OPTIONS", + target: target, } } func NewPutMessage(target string) RequestMessage { return &requestMessage{ - TheMethod: "PUT", - TheTarget: target, + method: "PUT", + target: target, } } func NewTraceMessage(target string) RequestMessage { return &requestMessage{ - TheMethod: "TRACE", - TheTarget: target, + method: "TRACE", + target: target, } } func NewRequestMessage(method, target string) RequestMessage { return &requestMessage{ - TheMethod: method, - TheTarget: target, + method: method, + target: target, } } type requestMessage struct { - TheMethod string - TheTarget string + method string + target string } -func (requestLine *requestMessage) Method() string { - return requestLine.TheMethod +func (message *requestMessage) Method() string { + return message.method } -func (requestLine *requestMessage) Path() string { - fields := strings.Split(requestLine.TheTarget, "?") +func (message *requestMessage) Path() string { + fields := strings.Split(message.target, "?") return fields[0] } -func (requestLine *requestMessage) Target() string { - return requestLine.TheTarget +func (message *requestMessage) Target() string { + return message.target } -func (requestLine *requestMessage) QueryParameters() []QueryParameter { +func (message *requestMessage) QueryParameters() []QueryParameter { panic("implement me") } -func (requestLine *requestMessage) NotImplemented() Response { - return &servererror.NotImplemented{Method: requestLine.TheMethod} +func (message *requestMessage) NotImplemented() Response { + return &servererror.NotImplemented{Method: message.method} } -func (requestLine *requestMessage) MakeResourceRequest(resource Resource) Request { - if requestLine.TheMethod == "OPTIONS" { +func (message *requestMessage) MakeResourceRequest(resource Resource) Request { + if message.method == "OPTIONS" { return &optionsRequest{ - SupportedMethods: requestLine.supportedMethods(resource), + SupportedMethods: message.supportedMethods(resource), } } - method := knownMethods[requestLine.TheMethod] + method := knownMethods[message.method] if method == nil { - return requestLine.unknownHttpMethod(resource) + return message.unknownHttpMethod(resource) } - request := method.MakeRequest(requestLine, resource) + request := method.MakeRequest(message, resource) if request == nil { - return requestLine.unsupportedMethod(resource) + return message.unsupportedMethod(resource) } return request } -func (requestLine *requestMessage) unknownHttpMethod(resource Resource) Request { - return clienterror.MethodNotAllowed(requestLine.supportedMethods(resource)...) +func (message *requestMessage) unknownHttpMethod(resource Resource) Request { + return clienterror.MethodNotAllowed(message.supportedMethods(resource)...) } -func (requestLine *requestMessage) unsupportedMethod(resource Resource) Request { - return clienterror.MethodNotAllowed(requestLine.supportedMethods(resource)...) +func (message *requestMessage) unsupportedMethod(resource Resource) Request { + return clienterror.MethodNotAllowed(message.supportedMethods(resource)...) } -func (requestLine *requestMessage) supportedMethods(resource Resource) []string { +func (message *requestMessage) supportedMethods(resource Resource) []string { supported := []string{"OPTIONS"} for name, method := range knownMethods { - imaginaryRequest := &requestMessage{TheMethod: name, TheTarget: requestLine.TheTarget} + imaginaryRequest := &requestMessage{method: name, target: message.target} request := method.MakeRequest(imaginaryRequest, resource) if request != nil { supported = append(supported, name) diff --git a/http/methods.go b/http/methods.go index c3fbfaa..157795f 100644 --- a/http/methods.go +++ b/http/methods.go @@ -44,7 +44,7 @@ type headMethod struct{} func (*headMethod) MakeRequest(requested *requestMessage, resource Resource) Request { supportedResource, ok := resource.(HeadResource) if ok { - return &headRequest{Resource: supportedResource, Target: requested.TheTarget} + return &headRequest{Resource: supportedResource, Target: requested.target} } return nil @@ -85,7 +85,7 @@ type postMethod struct{} func (*postMethod) MakeRequest(requested *requestMessage, resource Resource) Request { supportedResource, ok := resource.(PostResource) if ok { - return &postRequest{Resource: supportedResource, Target: requested.TheTarget} + return &postRequest{Resource: supportedResource, Target: requested.target} } return nil @@ -112,7 +112,7 @@ type putMethod struct{} func (*putMethod) MakeRequest(requested *requestMessage, resource Resource) Request { supportedResource, ok := resource.(PutResource) if ok { - return &putRequest{Resource: supportedResource, Target: requested.TheTarget} + return &putRequest{Resource: supportedResource, Target: requested.target} } return nil diff --git a/http/parser.go b/http/parser.go index d7fbc76..3c4dbd8 100644 --- a/http/parser.go +++ b/http/parser.go @@ -53,8 +53,8 @@ func (parser *parseMethodObject) parseRequestLine(text string) (ok *requestMessa } return &requestMessage{ //TODO KDK: RFC 3986 Section 3.4 -- the query is between ? and # or the end of the URI, non-inclusive - TheMethod: fields[0], - TheTarget: fields[1], + method: fields[0], + target: fields[1], }, nil } From 6e43c479df0ff45bda9bb1e9332f9c360d22c95d Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Fri, 27 Apr 2018 18:48:55 -0500 Subject: [PATCH 15/43] Using RequestMessage#Path instead of #Target --- capability/route.go | 2 +- fs/requests.go | 18 +++++++++--------- playground/readonly_route.go | 4 ++-- playground/writable_route.go | 4 ++-- teapot/requests.go | 6 +++--- teapot/route.go | 4 ++-- teapot/route_test.go | 2 +- teapot/teapot_suite_test.go | 8 ++++---- 8 files changed, 24 insertions(+), 24 deletions(-) diff --git a/capability/route.go b/capability/route.go index c736c12..d67df32 100644 --- a/capability/route.go +++ b/capability/route.go @@ -20,7 +20,7 @@ type ServerCapabilityRoute struct { } func (route *ServerCapabilityRoute) Route(requested http.RequestMessage) http.Request { - if requested.Target() != "*" { + if requested.Path() != "*" { return nil } else if requested.Method() != "OPTIONS" { return clienterror.MethodNotAllowed("OPTIONS") diff --git a/fs/requests.go b/fs/requests.go index 9c6966d..2929d0e 100644 --- a/fs/requests.go +++ b/fs/requests.go @@ -18,8 +18,8 @@ func (controller *ReadOnlyFileSystem) Name() string { return "Readonly file system" } -func (controller *ReadOnlyFileSystem) Get(client io.Writer, req http.RequestMessage) { //TODO KDK: Should req be called something else? - response := controller.determineResponse(req.Target()) +func (controller *ReadOnlyFileSystem) Get(client io.Writer, req http.RequestMessage) { + response := controller.determineResponse(req.Path()) response.WriteTo(client) } @@ -28,18 +28,18 @@ func (controller *ReadOnlyFileSystem) Head(client io.Writer, target string) { response.WriteHeader(client) } -func (controller *ReadOnlyFileSystem) determineResponse(requestedTarget string) http.Response { - resolvedTarget := path.Join(controller.BaseDirectory, requestedTarget) - info, err := os.Stat(resolvedTarget) +func (controller *ReadOnlyFileSystem) determineResponse(requestedPath string) http.Response { + resolvedPath := path.Join(controller.BaseDirectory, requestedPath) + info, err := os.Stat(resolvedPath) if err != nil { - return &clienterror.NotFound{Target: requestedTarget} + return &clienterror.NotFound{Target: requestedPath} } else if info.IsDir() { - files, _ := ioutil.ReadDir(resolvedTarget) + files, _ := ioutil.ReadDir(resolvedPath) return &DirectoryListing{ Files: readFileNames(files), - HrefPrefix: requestedTarget} + HrefPrefix: requestedPath} } else { - return &FileContents{Filename: resolvedTarget} + return &FileContents{Filename: resolvedPath} } } diff --git a/playground/readonly_route.go b/playground/readonly_route.go index 947a4b9..2d47b32 100644 --- a/playground/readonly_route.go +++ b/playground/readonly_route.go @@ -15,7 +15,7 @@ type ReadOnlyRoute struct { } func (route *ReadOnlyRoute) Route(requested http.RequestMessage) http.Request { - if requested.Target() != "/method_options2" { + if requested.Path() != "/method_options2" { return nil } @@ -36,7 +36,7 @@ func (controller *ReadableNopResource) Name() string { } func (controller *ReadableNopResource) Get(client io.Writer, req http.RequestMessage) { - controller.Head(client, req.Target()) + controller.Head(client, req.Path()) } func (controller *ReadableNopResource) Head(client io.Writer, target string) { diff --git a/playground/writable_route.go b/playground/writable_route.go index 4f33029..df6c0df 100644 --- a/playground/writable_route.go +++ b/playground/writable_route.go @@ -18,7 +18,7 @@ type ReadWriteRoute struct { } func (route *ReadWriteRoute) Route(requested http.RequestMessage) http.Request { - if requested.Target() != "/method_options" { + if requested.Path() != "/method_options" { return nil } @@ -41,7 +41,7 @@ func (controller *ReadWriteNopResource) Name() string { } func (controller *ReadWriteNopResource) Get(client io.Writer, req http.RequestMessage) { - controller.Head(client, req.Target()) + controller.Head(client, req.Path()) } func (controller *ReadWriteNopResource) Head(client io.Writer, target string) { diff --git a/teapot/requests.go b/teapot/requests.go index 48091a0..d787b26 100644 --- a/teapot/requests.go +++ b/teapot/requests.go @@ -14,8 +14,8 @@ func (teapot *IdentityTeapot) Name() string { return "teapot" } -func (teapot *IdentityTeapot) RespondsTo(target string) bool { - switch target { +func (teapot *IdentityTeapot) RespondsTo(path string) bool { + switch path { case "/coffee", "/tea": return true default: @@ -29,7 +29,7 @@ func (teapot *IdentityTeapot) Get(client io.Writer, req http.RequestMessage) { "/tea": teapot.getTea, } - handler := beverageRequestHandlers[req.Target()] + handler := beverageRequestHandlers[req.Path()] handler(client) } diff --git a/teapot/route.go b/teapot/route.go index 6241e92..8d80d3b 100644 --- a/teapot/route.go +++ b/teapot/route.go @@ -16,7 +16,7 @@ type Route struct { } func (route *Route) Route(requested http.RequestMessage) http.Request { - if !route.Teapot.RespondsTo(requested.Target()) { + if !route.Teapot.RespondsTo(requested.Path()) { return nil } @@ -26,5 +26,5 @@ func (route *Route) Route(requested http.RequestMessage) http.Request { type Teapot interface { Name() string Get(client io.Writer, req http.RequestMessage) - RespondsTo(target string) bool + RespondsTo(path string) bool } diff --git a/teapot/route_test.go b/teapot/route_test.go index 4b7907f..057dcdb 100644 --- a/teapot/route_test.go +++ b/teapot/route_test.go @@ -21,7 +21,7 @@ var _ = Describe("teapotRoute", func() { Describe("#Route", func() { Context("when the target is a resource that the teapot can respond to", func() { BeforeEach(func() { - teapotMock = &TeapotMock{RespondsToTarget: "/caffeine"} + teapotMock = &TeapotMock{RespondsToPath: "/caffeine"} router = &teapot.Route{Teapot: teapotMock} }) diff --git a/teapot/teapot_suite_test.go b/teapot/teapot_suite_test.go index d01ca58..f706d32 100644 --- a/teapot/teapot_suite_test.go +++ b/teapot/teapot_suite_test.go @@ -15,16 +15,16 @@ func TestTeapot(t *testing.T) { } type TeapotMock struct { - RespondsToTarget string - getTarget string + RespondsToPath string + getTarget string } func (mock *TeapotMock) Name() string { return "teapot mock" } -func (mock *TeapotMock) RespondsTo(target string) bool { - return mock.RespondsToTarget == target +func (mock *TeapotMock) RespondsTo(path string) bool { + return mock.RespondsToPath == path } func (mock *TeapotMock) Get(client io.Writer, req http.RequestMessage) { From 1b42015ef63b5a53d0476162cc0687cbbe2b3dc5 Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Fri, 27 Apr 2018 18:50:33 -0500 Subject: [PATCH 16/43] Using path instead of target for `capability` --- capability/route_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/capability/route_test.go b/capability/route_test.go index 8450f36..1f548c6 100644 --- a/capability/route_test.go +++ b/capability/route_test.go @@ -40,7 +40,7 @@ var _ = Describe("ServerCapabilityRoute", func() { router = &capability.ServerCapabilityRoute{Controller: controller} }) - Context("when the target is *", func() { + Context("when the path is *", func() { It("routes OPTIONS to ServerResource", func() { requested = http.NewOptionsMessage("*") routedRequest = router.Route(requested) @@ -55,7 +55,7 @@ var _ = Describe("ServerCapabilityRoute", func() { }) }) - It("returns nil to pass on any other target", func() { + It("returns nil to pass on any other path", func() { requested = http.NewOptionsMessage("/") routedRequest = router.Route(requested) Expect(routedRequest).To(BeNil()) From efebe64d8887b427a93897dac1517e583b1aeebf Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Fri, 27 Apr 2018 18:52:42 -0500 Subject: [PATCH 17/43] Path instead of target for `fs` --- fs/fs_suite_test.go | 8 ++++---- fs/requests.go | 2 +- fs/requests_test.go | 10 +++++----- msg/clienterror/client_errors.go | 6 +++--- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/fs/fs_suite_test.go b/fs/fs_suite_test.go index 96cee6c..8473895 100644 --- a/fs/fs_suite_test.go +++ b/fs/fs_suite_test.go @@ -15,7 +15,7 @@ func TestFs(t *testing.T) { } type FileSystemResourceMock struct { - getTarget string + getPath string headTarget string } @@ -24,11 +24,11 @@ func (mock *FileSystemResourceMock) Name() string { } func (mock *FileSystemResourceMock) Get(client io.Writer, req http.RequestMessage) { - mock.getTarget = req.Target() + mock.getPath = req.Path() } -func (mock *FileSystemResourceMock) GetShouldHaveReceived(target string) { - ExpectWithOffset(1, mock.getTarget).To(Equal(target)) +func (mock *FileSystemResourceMock) GetShouldHaveReceived(path string) { + ExpectWithOffset(1, mock.getPath).To(Equal(path)) } func (mock *FileSystemResourceMock) Head(client io.Writer, target string) { diff --git a/fs/requests.go b/fs/requests.go index 2929d0e..ea6a02e 100644 --- a/fs/requests.go +++ b/fs/requests.go @@ -32,7 +32,7 @@ func (controller *ReadOnlyFileSystem) determineResponse(requestedPath string) ht resolvedPath := path.Join(controller.BaseDirectory, requestedPath) info, err := os.Stat(resolvedPath) if err != nil { - return &clienterror.NotFound{Target: requestedPath} + return &clienterror.NotFound{Path: requestedPath} } else if info.IsDir() { files, _ := ioutil.ReadDir(resolvedPath) return &DirectoryListing{ diff --git a/fs/requests_test.go b/fs/requests_test.go index 17926e2..9eda539 100644 --- a/fs/requests_test.go +++ b/fs/requests_test.go @@ -29,7 +29,7 @@ var _ = Describe("ReadOnlyFileSystem", func() { }) Describe("#Get", func() { - Context("when the resolved target does not exist", func() { + Context("when the resolved path does not exist", func() { BeforeEach(func() { controller.Get(responseBuffer, http.NewGetMessage("/missing.txt")) response = httptest.ParseResponse(responseBuffer) @@ -49,7 +49,7 @@ var _ = Describe("ReadOnlyFileSystem", func() { }) }) - Context("when the target is a readable text file in the base path", func() { + Context("when the path is a readable text file in the base path", func() { BeforeEach(func() { existingFile := path.Join(basePath, "readable.txt") Expect(createTextFile(existingFile, "A")).To(Succeed()) @@ -71,7 +71,7 @@ var _ = Describe("ReadOnlyFileSystem", func() { }) }) - Context("when the target is a readable file named with a registered extension", func() { + Context("when the path is a readable file named with a registered extension", func() { BeforeEach(func() { existingFile := path.Join(basePath, "image.jpeg") Expect(createTextFile(existingFile, "A")).To(Succeed()) @@ -84,7 +84,7 @@ var _ = Describe("ReadOnlyFileSystem", func() { }) }) - Context("when the target is a readable file without an extension", func() { + Context("when the path is a readable file without an extension", func() { BeforeEach(func() { existingFile := path.Join(basePath, "assumed-text") Expect(createTextFile(existingFile, "A")).To(Succeed()) @@ -97,7 +97,7 @@ var _ = Describe("ReadOnlyFileSystem", func() { }) }) - Context("when the target is /", func() { + Context("when the path is /", func() { BeforeEach(func() { existingFile := path.Join(basePath, "one") Expect(createTextFile(existingFile, "1")).To(Succeed()) diff --git a/msg/clienterror/client_errors.go b/msg/clienterror/client_errors.go index cea0a5f..e27b7c2 100644 --- a/msg/clienterror/client_errors.go +++ b/msg/clienterror/client_errors.go @@ -23,8 +23,8 @@ func (badRequest *BadRequest) WriteHeader(client io.Writer) error { } type NotFound struct { - Target string - body string + Path string + body string } func (notFound *NotFound) WriteTo(client io.Writer) error { @@ -37,7 +37,7 @@ func (notFound *NotFound) WriteHeader(client io.Writer) error { msg.WriteStatusLine(client, 404, "Not Found") msg.WriteContentTypeHeader(client, "text/plain") - notFound.body = fmt.Sprintf("Not found: %s", notFound.Target) + notFound.body = fmt.Sprintf("Not found: %s", notFound.Path) msg.WriteContentLengthHeader(client, len(notFound.body)) msg.WriteEndOfMessageHeader(client) return nil From 5794a5a74a3e282d4fdabd95a4fd9fd9e8141c6d Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Fri, 27 Apr 2018 18:54:52 -0500 Subject: [PATCH 18/43] Path instead of target for `http` --- http/router_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/http/router_test.go b/http/router_test.go index f744c9b..4a8be75 100644 --- a/http/router_test.go +++ b/http/router_test.go @@ -49,7 +49,7 @@ var _ = Describe("RequestLineRouter", func() { request, err = router.RouteRequest(makeReader("HEAD /foo HTTP/1.1\r\nAccept: */*\r\n\r\n")) }) - It("tries routing the method and target from the request until it finds a match", func() { + It("tries routing the method and path from the request until it finds a match", func() { unrelatedRoute.ShouldHaveReceived("HEAD", "/foo") matchingRoute.ShouldHaveReceived("HEAD", "/foo") }) From 9c2b731ebe553673db39ad2b7f86d791b9c72a2a Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Fri, 27 Apr 2018 18:57:09 -0500 Subject: [PATCH 19/43] Path instead of target for `playground` --- playground/parameter_route_test.go | 4 ++-- playground/readonly_route_test.go | 4 ++-- playground/writable_route_test.go | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/playground/parameter_route_test.go b/playground/parameter_route_test.go index d88f3d0..cb259ca 100644 --- a/playground/parameter_route_test.go +++ b/playground/parameter_route_test.go @@ -36,7 +36,7 @@ var _ = Describe("ParameterRoute", func() { response.Reset() }) - Context("when the target is /parameters", func() { + Context("when the path is /parameters", func() { It("routes GET to ParameterDecoder#Get with the decoded query parameters", func() { request := &mock.Request{} requested := &httptest.RequestMessage{ @@ -68,7 +68,7 @@ var _ = Describe("ParameterRoute", func() { }) }) - It("returns nil for any other target", func() { + It("returns nil for any other path", func() { requested := http.NewGetMessage("/") Expect(router.Route(requested)).To(BeNil()) }) diff --git a/playground/readonly_route_test.go b/playground/readonly_route_test.go index d9038cb..31711a1 100644 --- a/playground/readonly_route_test.go +++ b/playground/readonly_route_test.go @@ -31,7 +31,7 @@ var _ = Describe("ReadOnlyRoute", func() { router = &playground.ReadOnlyRoute{Resource: resource} }) - Context("when the target is /method_options2", func() { + Context("when the path is /method_options2", func() { It("routes GET to Teapot#Get", func() { handleRequest(router, "GET", "/method_options2") resource.GetShouldHaveBeenCalled() @@ -66,7 +66,7 @@ var _ = Describe("ReadOnlyRoute", func() { }) }) - It("returns nil on any other target", func() { + It("returns nil on any other path", func() { requested := http.NewGetMessage("/") routedRequest := router.Route(requested) Expect(routedRequest).To(BeNil()) diff --git a/playground/writable_route_test.go b/playground/writable_route_test.go index 94590db..b99b242 100644 --- a/playground/writable_route_test.go +++ b/playground/writable_route_test.go @@ -31,7 +31,7 @@ var _ = Describe("ReadWriteRoute", func() { router = &playground.ReadWriteRoute{Resource: resource} }) - Context("when the target is /method_options", func() { + Context("when the path is /method_options", func() { It("routes GET to Teapot#Get", func() { handleRequest(router, "GET", "/method_options") resource.GetShouldHaveBeenCalled() @@ -76,7 +76,7 @@ var _ = Describe("ReadWriteRoute", func() { }) }) - It("returns nil on any other target", func() { + It("returns nil on any other path", func() { requested := http.NewGetMessage("/") routedRequest := router.Route(requested) Expect(routedRequest).To(BeNil()) From 3912ece91d9b5accb5042917c22334596e22c673 Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Fri, 27 Apr 2018 18:58:16 -0500 Subject: [PATCH 20/43] Path instead of target for `teapot` --- http/methods.go | 2 +- http/router.go | 4 ++-- teapot/requests_test.go | 4 ++-- teapot/route_test.go | 6 +++--- teapot/teapot_suite_test.go | 8 ++++---- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/http/methods.go b/http/methods.go index 157795f..b44da3d 100644 --- a/http/methods.go +++ b/http/methods.go @@ -15,7 +15,7 @@ func (method *getMethod) MakeRequest(requested *requestMessage, resource Resourc supportedResource, ok := resource.(GetResource) if ok { return &getRequest{ - Message: requested, + Message: requested, Resource: supportedResource, } } diff --git a/http/router.go b/http/router.go index 78b8994..721de86 100644 --- a/http/router.go +++ b/http/router.go @@ -55,7 +55,7 @@ type RequestMessage interface { Target() string Path() string QueryParameters() []QueryParameter - + MakeResourceRequest(resource Resource) Request } @@ -65,4 +65,4 @@ type Route interface { type QueryParameter struct { Name, Value string -} \ No newline at end of file +} diff --git a/teapot/requests_test.go b/teapot/requests_test.go index a12c712..601fb48 100644 --- a/teapot/requests_test.go +++ b/teapot/requests_test.go @@ -30,7 +30,7 @@ var _ = Describe("IdentityTeapot", func() { }) Describe("#Get", func() { - Context("when the target is /coffee", func() { + Context("when the path is /coffee", func() { BeforeEach(func() { responseBuffer = &bytes.Buffer{} theTeapot = &teapot.IdentityTeapot{} @@ -56,7 +56,7 @@ var _ = Describe("IdentityTeapot", func() { }) }) - Context("when the target is /tea", func() { + Context("when the path is /tea", func() { BeforeEach(func() { responseBuffer = &bytes.Buffer{} theTeapot = &teapot.IdentityTeapot{} diff --git a/teapot/route_test.go b/teapot/route_test.go index 057dcdb..d5c52e2 100644 --- a/teapot/route_test.go +++ b/teapot/route_test.go @@ -19,13 +19,13 @@ var _ = Describe("teapotRoute", func() { ) Describe("#Route", func() { - Context("when the target is a resource that the teapot can respond to", func() { + Context("when the path is a resource that the teapot can respond to", func() { BeforeEach(func() { teapotMock = &TeapotMock{RespondsToPath: "/caffeine"} router = &teapot.Route{Teapot: teapotMock} }) - It("routes GET requests to that target to the teapot", func() { + It("routes GET requests to that path to the teapot", func() { requested = http.NewGetMessage("/caffeine") routedRequest = router.Route(requested) routedRequest.Handle(&bufio.Writer{}) @@ -39,7 +39,7 @@ var _ = Describe("teapotRoute", func() { }) }) - It("passes on any other target", func() { + It("passes on any other path", func() { teapotMock = &TeapotMock{} router = &teapot.Route{Teapot: teapotMock} diff --git a/teapot/teapot_suite_test.go b/teapot/teapot_suite_test.go index f706d32..3fe06d8 100644 --- a/teapot/teapot_suite_test.go +++ b/teapot/teapot_suite_test.go @@ -16,7 +16,7 @@ func TestTeapot(t *testing.T) { type TeapotMock struct { RespondsToPath string - getTarget string + getPath string } func (mock *TeapotMock) Name() string { @@ -28,9 +28,9 @@ func (mock *TeapotMock) RespondsTo(path string) bool { } func (mock *TeapotMock) Get(client io.Writer, req http.RequestMessage) { - mock.getTarget = req.Target() + mock.getPath = req.Path() } -func (mock *TeapotMock) GetShouldHaveReceived(target string) { - ExpectWithOffset(1, mock.getTarget).To(Equal(target)) +func (mock *TeapotMock) GetShouldHaveReceived(path string) { + ExpectWithOffset(1, mock.getPath).To(Equal(path)) } From 508eff03776176d207bf144b8b62f0527bd66f29 Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Mon, 30 Apr 2018 13:15:42 -0500 Subject: [PATCH 21/43] WIP parsing query strings --- http/method_dispatcher.go | 75 +++++++++++++++++++++++++++++++--- http/method_dispatcher_test.go | 69 +++++++++++++++++++++++++++++-- http/parser.go | 2 +- 3 files changed, 136 insertions(+), 10 deletions(-) diff --git a/http/method_dispatcher.go b/http/method_dispatcher.go index 233772d..cb6829d 100644 --- a/http/method_dispatcher.go +++ b/http/method_dispatcher.go @@ -60,16 +60,79 @@ func (message *requestMessage) Method() string { } func (message *requestMessage) Path() string { - fields := strings.Split(message.target, "?") - return fields[0] + path, _, _ := message.splitTarget() + return path } -func (message *requestMessage) Target() string { - return message.target +func (message *requestMessage) QueryParameters() []QueryParameter { + _, query, _ := message.splitTarget() + if query == "" { + return nil + } + + return parseQueryString(query) } -func (message *requestMessage) QueryParameters() []QueryParameter { - panic("implement me") +func (message *requestMessage) splitTarget() (path, query, fragment string) { + //there is a path + + splitOnQuery := strings.Split(message.target, "?") + if len(splitOnQuery) == 1 { + //there is a path + //there is no query string + //there may be a fragment + splitOnFragment := strings.Split(splitOnQuery[0], "#") + if len(splitOnFragment) == 1 { + //there is a path + //there is no query string + //there is no fragment + return message.target, "", "" + } + + //there is a path + //there is no query string + //there is a fragment + return splitOnFragment[0], "", splitOnFragment[1] + } + + //there is a path + //there is a query string + //there may be a fragment + splitOnFragment := strings.Split(splitOnQuery[1], "#") + if len(splitOnFragment) == 1 { + //there is a path + //there is a query string + //there is no fragment + return splitOnQuery[0], splitOnQuery[1], "" + } + + //there is a path + //there is a query string + //there is a fragment + return splitOnQuery[0], splitOnFragment[0], splitOnFragment[1] +} + +func parseQueryString(rawQuery string) []QueryParameter { + stringParameters := strings.Split(rawQuery, "&") + parsedParameters := make([]QueryParameter, len(stringParameters)) + for i, stringParameter := range stringParameters { + parsedParameters[i] = parseQueryParameter(stringParameter) + } + + return parsedParameters +} + +func parseQueryParameter(stringParameter string) QueryParameter { + nameValueFields := strings.Split(stringParameter, "=") + if len(nameValueFields) == 1 { + return QueryParameter{Name: nameValueFields[0]} + } else { + return QueryParameter{Name: nameValueFields[0], Value: nameValueFields[1]} + } +} + +func (message *requestMessage) Target() string { + return message.target } func (message *requestMessage) NotImplemented() Response { diff --git a/http/method_dispatcher_test.go b/http/method_dispatcher_test.go index 54ed005..101485a 100644 --- a/http/method_dispatcher_test.go +++ b/http/method_dispatcher_test.go @@ -9,14 +9,77 @@ import ( var _ = Describe("RequestMessage", func() { var message http.RequestMessage - Describe("#Path", func() { - It("returns the full target, for a target without a query or fragment", func() { + Context("given a target with no query or fragment", func() { + BeforeEach(func() { message = http.NewGetMessage("/widget") + }) + + It("the path is the full target", func() { Expect(message.Path()).To(Equal("/widget")) }) - It("returns the part of the target before any ?", func() { + It("there are no query parameters", func() { + Expect(message.QueryParameters()).To(BeEmpty()) + }) + }) + + Context("given a target with a query", func() { + It("the path is the part before the ?", func() { message = http.NewGetMessage("/widget?field=value") Expect(message.Path()).To(Equal("/widget")) }) + + It("parses the part after the ? into query parameters", func() { + message = http.NewGetMessage("/widget?field=value") + Expect(message.QueryParameters()).To( + ContainElement(http.QueryParameter{Name: "field", Value: "value"})) + }) + + It("parses parameters without a value into QueryParameter#Name", func() { + message = http.NewGetMessage("/widget?flag") + Expect(message.QueryParameters()).To( + ContainElement(http.QueryParameter{Name: "flag", Value: ""})) + }) + + It("uses '=' to split a parameter's name and value", func() { + message = http.NewGetMessage("/widget?field=value") + Expect(message.QueryParameters()).To( + ContainElement(http.QueryParameter{Name: "field", Value: "value"})) + }) + + It("uses '&' to split among multiple parameters", func() { + message = http.NewGetMessage("/widget?one=1&two=2") + Expect(message.QueryParameters()).To(Equal([]http.QueryParameter{ + {Name: "one", Value: "1"}, + {Name: "two", Value: "2"}, + })) + }) + }) + + Context("given a target with a fragment", func() { + BeforeEach(func() { + message = http.NewGetMessage("/widget#section") + }) + + It("the path is the part before the '#'", func() { + Expect(message.Path()).To(Equal("/widget")) + }) + It("there are no query parameters", func() { + Expect(message.QueryParameters()).To(BeEmpty()) + }) + }) + + Context("given a target with a query and a fragment", func() { + BeforeEach(func() { + message = http.NewGetMessage("/widget?field=value#section") + }) + + It("the path is the part before the ?", func() { + Expect(message.Path()).To(Equal("/widget")) + }) + It("query parameters are parsed from the part between the ? and the #", func() { + Expect(message.QueryParameters()).To(Equal([]http.QueryParameter{ + {Name: "field", Value: "value"}, + })) + }) }) }) diff --git a/http/parser.go b/http/parser.go index 3c4dbd8..456577b 100644 --- a/http/parser.go +++ b/http/parser.go @@ -52,7 +52,7 @@ func (parser *parseMethodObject) parseRequestLine(text string) (ok *requestMessa return nil, &clienterror.BadRequest{DisplayText: "incorrectly formatted or missing request-line"} } - return &requestMessage{ //TODO KDK: RFC 3986 Section 3.4 -- the query is between ? and # or the end of the URI, non-inclusive + return &requestMessage{ method: fields[0], target: fields[1], }, nil From 7a7e793669f384b27aaeceb965a47f5a0ddac98b Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Mon, 30 Apr 2018 16:50:59 -0500 Subject: [PATCH 22/43] Refactoring --- http/method_dispatcher.go | 45 ++++++++++++--------------------------- 1 file changed, 14 insertions(+), 31 deletions(-) diff --git a/http/method_dispatcher.go b/http/method_dispatcher.go index cb6829d..1e03961 100644 --- a/http/method_dispatcher.go +++ b/http/method_dispatcher.go @@ -74,42 +74,25 @@ func (message *requestMessage) QueryParameters() []QueryParameter { } func (message *requestMessage) splitTarget() (path, query, fragment string) { - //there is a path - splitOnQuery := strings.Split(message.target, "?") if len(splitOnQuery) == 1 { - //there is a path - //there is no query string - //there may be a fragment - splitOnFragment := strings.Split(splitOnQuery[0], "#") - if len(splitOnFragment) == 1 { - //there is a path - //there is no query string - //there is no fragment - return message.target, "", "" - } - - //there is a path - //there is no query string - //there is a fragment - return splitOnFragment[0], "", splitOnFragment[1] + query = "" + path, fragment = extractFragment(splitOnQuery[0]) + return } - //there is a path - //there is a query string - //there may be a fragment - splitOnFragment := strings.Split(splitOnQuery[1], "#") - if len(splitOnFragment) == 1 { - //there is a path - //there is a query string - //there is no fragment - return splitOnQuery[0], splitOnQuery[1], "" - } + path = splitOnQuery[0] + query, fragment = extractFragment(splitOnQuery[1]) + return +} - //there is a path - //there is a query string - //there is a fragment - return splitOnQuery[0], splitOnFragment[0], splitOnFragment[1] +func extractFragment(maybeHasFragment string) (prefix string, fragment string) { + fields := strings.Split(maybeHasFragment, "#") + if len(fields) == 1 { + return fields[0], "" + } else { + return fields[0], fields[1] + } } func parseQueryString(rawQuery string) []QueryParameter { From d2a97c641bcf0c7fd764f0409b78ec0ca1d79aed Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Mon, 30 Apr 2018 20:07:42 -0500 Subject: [PATCH 23/43] Moving tests over to RequestParser to consolidate parsing responsibility --- http/method_dispatcher_test.go | 85 -------------------------------- http/parser_test.go | 88 +++++++++++++++++++++++++++++----- http/router_test.go | 4 +- 3 files changed, 79 insertions(+), 98 deletions(-) delete mode 100644 http/method_dispatcher_test.go diff --git a/http/method_dispatcher_test.go b/http/method_dispatcher_test.go deleted file mode 100644 index 101485a..0000000 --- a/http/method_dispatcher_test.go +++ /dev/null @@ -1,85 +0,0 @@ -package http_test - -import ( - "github.com/kkrull/gohttp/http" - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" -) - -var _ = Describe("RequestMessage", func() { - var message http.RequestMessage - - Context("given a target with no query or fragment", func() { - BeforeEach(func() { - message = http.NewGetMessage("/widget") - }) - - It("the path is the full target", func() { - Expect(message.Path()).To(Equal("/widget")) - }) - It("there are no query parameters", func() { - Expect(message.QueryParameters()).To(BeEmpty()) - }) - }) - - Context("given a target with a query", func() { - It("the path is the part before the ?", func() { - message = http.NewGetMessage("/widget?field=value") - Expect(message.Path()).To(Equal("/widget")) - }) - - It("parses the part after the ? into query parameters", func() { - message = http.NewGetMessage("/widget?field=value") - Expect(message.QueryParameters()).To( - ContainElement(http.QueryParameter{Name: "field", Value: "value"})) - }) - - It("parses parameters without a value into QueryParameter#Name", func() { - message = http.NewGetMessage("/widget?flag") - Expect(message.QueryParameters()).To( - ContainElement(http.QueryParameter{Name: "flag", Value: ""})) - }) - - It("uses '=' to split a parameter's name and value", func() { - message = http.NewGetMessage("/widget?field=value") - Expect(message.QueryParameters()).To( - ContainElement(http.QueryParameter{Name: "field", Value: "value"})) - }) - - It("uses '&' to split among multiple parameters", func() { - message = http.NewGetMessage("/widget?one=1&two=2") - Expect(message.QueryParameters()).To(Equal([]http.QueryParameter{ - {Name: "one", Value: "1"}, - {Name: "two", Value: "2"}, - })) - }) - }) - - Context("given a target with a fragment", func() { - BeforeEach(func() { - message = http.NewGetMessage("/widget#section") - }) - - It("the path is the part before the '#'", func() { - Expect(message.Path()).To(Equal("/widget")) - }) - It("there are no query parameters", func() { - Expect(message.QueryParameters()).To(BeEmpty()) - }) - }) - - Context("given a target with a query and a fragment", func() { - BeforeEach(func() { - message = http.NewGetMessage("/widget?field=value#section") - }) - - It("the path is the part before the ?", func() { - Expect(message.Path()).To(Equal("/widget")) - }) - It("query parameters are parsed from the part between the ? and the #", func() { - Expect(message.QueryParameters()).To(Equal([]http.QueryParameter{ - {Name: "field", Value: "value"}, - })) - }) - }) -}) diff --git a/http/parser_test.go b/http/parser_test.go index 68846cc..9ac9c0c 100644 --- a/http/parser_test.go +++ b/http/parser_test.go @@ -24,7 +24,7 @@ var _ = Describe("LineRequestParser", func() { Describe("it returns 400 Bad Request", func() { It("for a completely blank request", func() { request, err = parser.Parse(makeReader("")) - Expect(err).To(beABadRequestResponse("line in request header not ending in CRLF")) + Expect(err).To(beABadRequestResponse("end of input before terminating CRLF")) }) It("for any line missing CR", func() { @@ -44,7 +44,7 @@ var _ = Describe("LineRequestParser", func() { It("for a request missing an ending CRLF", func() { request, err = parser.Parse(makeReader("GET / HTTP/1.1\r\n")) - Expect(err).To(beABadRequestResponse("line in request header not ending in CRLF")) + Expect(err).To(beABadRequestResponse("end of input before terminating CRLF")) }) It("when multiple spaces are separating fields in request-line", func() { @@ -64,9 +64,7 @@ var _ = Describe("LineRequestParser", func() { }) Context("given a well-formed request", func() { - var ( - reader *bufio.Reader - ) + var reader *bufio.Reader BeforeEach(func() { buffer := bytes.NewBufferString("GET /foo HTTP/1.1\r\nAccept: */*\r\n\r\n") @@ -74,7 +72,7 @@ var _ = Describe("LineRequestParser", func() { request, err = parser.Parse(reader) }) - It("returns the parsed request message", func() { + It("parses the request-line", func() { Expect(request.Method()).To(Equal("GET")) Expect(request.Target()).To(Equal("/foo")) }) @@ -88,14 +86,82 @@ var _ = Describe("LineRequestParser", func() { }) }) - Context("given a well-formed request with query parameters", func() { + Context("given a target with no query or fragment", func() { + BeforeEach(func() { + request, _ = parser.Parse(requestWithTarget("/widget")) + }) + + It("the path is the full target", func() { + Expect(request.Path()).To(Equal("/widget")) + }) + It("there are no query parameters", func() { + Expect(request.QueryParameters()).To(BeEmpty()) + }) + }) + + Context("given a target with a query", func() { + It("the path is the part before the ?", func() { + request, _ = parser.Parse(requestWithTarget("/widget?field=value")) + Expect(request.Path()).To(Equal("/widget")) + }) + + It("parses the part after the ? into query parameters", func() { + request, _ = parser.Parse(requestWithTarget("/widget?field=value")) + Expect(request.QueryParameters()).To( + ContainElement(http.QueryParameter{Name: "field", Value: "value"})) + }) + + It("parses parameters without a value into QueryParameter#Name", func() { + request, _ = parser.Parse(requestWithTarget("/widget?flag")) + Expect(request.QueryParameters()).To( + ContainElement(http.QueryParameter{Name: "flag", Value: ""})) + }) + + It("uses '=' to split a parameter's name and value", func() { + request, _ = parser.Parse(requestWithTarget("/widget?field=value")) + Expect(request.QueryParameters()).To( + ContainElement(http.QueryParameter{Name: "field", Value: "value"})) + }) + + It("uses '&' to split among multiple parameters", func() { + request, _ = parser.Parse(requestWithTarget("/widget?one=1&two=2")) + Expect(request.QueryParameters()).To(Equal([]http.QueryParameter{ + {Name: "one", Value: "1"}, + {Name: "two", Value: "2"}, + })) + }) + }) + + Context("given a target with a fragment", func() { BeforeEach(func() { - request, err = parser.Parse(makeReader("GET /foo?one=1 HTTP/1.1\r\n\r\n")) - Expect(err).NotTo(HaveOccurred()) + request, _ = parser.Parse(requestWithTarget("/widget#section")) }) - XIt("removes the query string from the target") - XIt("passes decoded parameters to the routes") + It("the path is the part before the '#'", func() { + Expect(request.Path()).To(Equal("/widget")) + }) + It("there are no query parameters", func() { + Expect(request.QueryParameters()).To(BeEmpty()) + }) + }) + + Context("given a target with a query and a fragment", func() { + BeforeEach(func() { + request, _ = parser.Parse(requestWithTarget("/widget?field=value#section")) + }) + + It("the path is the part before the ?", func() { + Expect(request.Path()).To(Equal("/widget")) + }) + It("query parameters are parsed from the part between the ? and the #", func() { + Expect(request.QueryParameters()).To(Equal([]http.QueryParameter{ + {Name: "field", Value: "value"}, + })) + }) }) }) }) + +func requestWithTarget(target string) *bufio.Reader { + return makeReader("GET %s HTTP/1.1\r\n\r\n", target) +} diff --git a/http/router_test.go b/http/router_test.go index 4a8be75..7d13cc0 100644 --- a/http/router_test.go +++ b/http/router_test.go @@ -62,8 +62,8 @@ var _ = Describe("RequestLineRouter", func() { }) }) -func makeReader(template string, values ...string) *bufio.Reader { - text := fmt.Sprintf(template, values) +func makeReader(template string, values ...interface{}) *bufio.Reader { + text := fmt.Sprintf(template, values...) return bufio.NewReader(bytes.NewBufferString(text)) } From 6b391adc7d8bdcca1a24920c5252e9bbaf82acc3 Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Mon, 30 Apr 2018 20:27:25 -0500 Subject: [PATCH 24/43] Clarified state machine in parseMethodObject --- http/parser.go | 41 +++++++++++++---------------------------- 1 file changed, 13 insertions(+), 28 deletions(-) diff --git a/http/parser.go b/http/parser.go index 456577b..2727148 100644 --- a/http/parser.go +++ b/http/parser.go @@ -12,61 +12,46 @@ type LineRequestParser struct{} func (parser *LineRequestParser) Parse(reader *bufio.Reader) (ok *requestMessage, err Response) { methodObject := &parseMethodObject{reader: reader} - return methodObject.Parse() + return methodObject.ReadingRequestLine() } +//A state machine that parses an HTTP request during the process of reading the request from input type parseMethodObject struct { reader *bufio.Reader } -func (parser *parseMethodObject) Parse() (ok *requestMessage, err Response) { +func (parser *parseMethodObject) ReadingRequestLine() (ok *requestMessage, badRequest Response) { requestLine, err := parser.readCRLFLine() if err != nil { return nil, err } - return parser.doParseRequestLine(requestLine) + return parser.parsingRequestLine(requestLine) } -func (parser *parseMethodObject) doParseRequestLine(requestLine string) (ok *requestMessage, err Response) { - requested, err := parser.parseRequestLine(requestLine) - if err != nil { - return nil, err - } - - return parser.doParseHeaders(requested) -} - -func (parser *parseMethodObject) doParseHeaders(requested *requestMessage) (ok *requestMessage, err Response) { - err = parser.parseHeaders() - if err != nil { - return nil, err - } - - return requested, nil -} - -func (parser *parseMethodObject) parseRequestLine(text string) (ok *requestMessage, badRequest Response) { - fields := strings.Split(text, " ") +func (parser *parseMethodObject) parsingRequestLine(requestLine string) (ok *requestMessage, badRequest Response) { + fields := strings.Split(requestLine, " ") if len(fields) != 3 { return nil, &clienterror.BadRequest{DisplayText: "incorrectly formatted or missing request-line"} } - return &requestMessage{ + requested := &requestMessage{ method: fields[0], target: fields[1], - }, nil + } + + return parser.readingHeaders(requested) } -func (parser *parseMethodObject) parseHeaders() (badRequest Response) { +func (parser *parseMethodObject) readingHeaders(requested *requestMessage) (ok *requestMessage, badRequest Response) { isBlankLineBetweenHeadersAndBody := func(line string) bool { return line == "" } for { line, err := parser.readCRLFLine() if err != nil { - return err + return nil, err } else if isBlankLineBetweenHeadersAndBody(line) { - return nil + return requested, nil } } } From 3ce0dc4e9c3e7f20ba702a4532dabaf058813321 Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Mon, 30 Apr 2018 21:08:11 -0500 Subject: [PATCH 25/43] Moved query string parsing over to LineRequestParser --- capability/route.go | 2 +- http/method_dispatcher.go | 86 +++++++++-------------------- http/parser.go | 50 ++++++++++++++++- playground/playground_suite_test.go | 4 +- 4 files changed, 78 insertions(+), 64 deletions(-) diff --git a/capability/route.go b/capability/route.go index d67df32..c736c12 100644 --- a/capability/route.go +++ b/capability/route.go @@ -20,7 +20,7 @@ type ServerCapabilityRoute struct { } func (route *ServerCapabilityRoute) Route(requested http.RequestMessage) http.Request { - if requested.Path() != "*" { + if requested.Target() != "*" { return nil } else if requested.Method() != "OPTIONS" { return clienterror.MethodNotAllowed("OPTIONS") diff --git a/http/method_dispatcher.go b/http/method_dispatcher.go index 1e03961..73f39c6 100644 --- a/http/method_dispatcher.go +++ b/http/method_dispatcher.go @@ -2,23 +2,24 @@ package http import ( "sort" - "strings" "github.com/kkrull/gohttp/msg/clienterror" "github.com/kkrull/gohttp/msg/servererror" ) -func NewGetMessage(target string) RequestMessage { +func NewGetMessage(path string) RequestMessage { return &requestMessage{ method: "GET", - target: target, + target: path, + path: path, } } -func NewHeadMessage(target string) RequestMessage { +func NewHeadMessage(path string) RequestMessage { return &requestMessage{ method: "HEAD", - target: target, + target: path, + path: path, } } @@ -26,33 +27,39 @@ func NewOptionsMessage(target string) RequestMessage { return &requestMessage{ method: "OPTIONS", target: target, + path: target, } } -func NewPutMessage(target string) RequestMessage { +func NewPutMessage(path string) RequestMessage { return &requestMessage{ method: "PUT", - target: target, + target: path, + path: path, } } -func NewTraceMessage(target string) RequestMessage { +func NewTraceMessage(path string) RequestMessage { return &requestMessage{ method: "TRACE", - target: target, + target: path, + path: path, } } -func NewRequestMessage(method, target string) RequestMessage { +func NewRequestMessage(method, path string) RequestMessage { return &requestMessage{ method: method, - target: target, + target: path, + path: path, } } type requestMessage struct { - method string - target string + method string + path string + target string + queryParameters []QueryParameter } func (message *requestMessage) Method() string { @@ -60,58 +67,19 @@ func (message *requestMessage) Method() string { } func (message *requestMessage) Path() string { - path, _, _ := message.splitTarget() - return path + return message.path } -func (message *requestMessage) QueryParameters() []QueryParameter { - _, query, _ := message.splitTarget() - if query == "" { - return nil - } - - return parseQueryString(query) +func (message *requestMessage) AddQueryFlag(name string) { + message.queryParameters = append(message.queryParameters, QueryParameter{Name: name}) } -func (message *requestMessage) splitTarget() (path, query, fragment string) { - splitOnQuery := strings.Split(message.target, "?") - if len(splitOnQuery) == 1 { - query = "" - path, fragment = extractFragment(splitOnQuery[0]) - return - } - - path = splitOnQuery[0] - query, fragment = extractFragment(splitOnQuery[1]) - return +func (message *requestMessage) AddQueryParameter(name, value string) { + message.queryParameters = append(message.queryParameters, QueryParameter{Name: name, Value: value}) } -func extractFragment(maybeHasFragment string) (prefix string, fragment string) { - fields := strings.Split(maybeHasFragment, "#") - if len(fields) == 1 { - return fields[0], "" - } else { - return fields[0], fields[1] - } -} - -func parseQueryString(rawQuery string) []QueryParameter { - stringParameters := strings.Split(rawQuery, "&") - parsedParameters := make([]QueryParameter, len(stringParameters)) - for i, stringParameter := range stringParameters { - parsedParameters[i] = parseQueryParameter(stringParameter) - } - - return parsedParameters -} - -func parseQueryParameter(stringParameter string) QueryParameter { - nameValueFields := strings.Split(stringParameter, "=") - if len(nameValueFields) == 1 { - return QueryParameter{Name: nameValueFields[0]} - } else { - return QueryParameter{Name: nameValueFields[0], Value: nameValueFields[1]} - } +func (message *requestMessage) QueryParameters() []QueryParameter { + return message.queryParameters } func (message *requestMessage) Target() string { diff --git a/http/parser.go b/http/parser.go index 2727148..1bcf9aa 100644 --- a/http/parser.go +++ b/http/parser.go @@ -35,9 +35,33 @@ func (parser *parseMethodObject) parsingRequestLine(requestLine string) (ok *req return nil, &clienterror.BadRequest{DisplayText: "incorrectly formatted or missing request-line"} } + return parser.parsingTarget(fields[0], fields[1]) +} + +func (parser *parseMethodObject) parsingTarget(method, target string) (ok *requestMessage, badRequest Response) { + path, query, _ := splitTarget(target) requested := &requestMessage{ - method: fields[0], - target: fields[1], + method: method, + target: target, + path: path, + } + + return parser.parsingQueryString(requested, query) +} + +func (parser *parseMethodObject) parsingQueryString(requested *requestMessage, rawQuery string) (ok *requestMessage, badRequest Response) { + if len(rawQuery) == 0 { + return parser.readingHeaders(requested) + } + + stringParameters := strings.Split(rawQuery, "&") + for _, stringParameter := range stringParameters { + nameValueFields := strings.Split(stringParameter, "=") + if len(nameValueFields) == 1 { + requested.AddQueryFlag(nameValueFields[0]) + } else { + requested.AddQueryParameter(nameValueFields[0], nameValueFields[1]) + } } return parser.readingHeaders(requested) @@ -72,3 +96,25 @@ func (parser *parseMethodObject) readCRLFLine() (line string, badRequest Respons trimmed := strings.TrimSuffix(maybeEndsInCR, "\r") return trimmed, nil } + +func splitTarget(target string) (path, query, fragment string) { + splitOnQuery := strings.Split(target, "?") + if len(splitOnQuery) == 1 { + query = "" + path, fragment = extractFragment(splitOnQuery[0]) + return + } + + path = splitOnQuery[0] + query, fragment = extractFragment(splitOnQuery[1]) + return +} + +func extractFragment(target string) (prefix string, fragment string) { + fields := strings.Split(target, "#") + if len(fields) == 1 { + return fields[0], "" + } else { + return fields[0], fields[1] + } +} diff --git a/playground/playground_suite_test.go b/playground/playground_suite_test.go index 9d0edcc..2cc5e02 100644 --- a/playground/playground_suite_test.go +++ b/playground/playground_suite_test.go @@ -111,8 +111,8 @@ func (mock *ReadWriteResourceMock) PutShouldHaveBeenCalled() { /* Helpers */ -func handleRequest(router http.Route, method, target string) { - requested := http.NewRequestMessage(method, target) +func handleRequest(router http.Route, method, path string) { + requested := http.NewRequestMessage(method, path) routedRequest := router.Route(requested) ExpectWithOffset(1, routedRequest).NotTo(BeNil()) From 4bd6368a1b53ebb2ddbdb840ad9ff45d9bca37dc Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Mon, 30 Apr 2018 21:16:34 -0500 Subject: [PATCH 26/43] Renamed file --- http/{method_dispatcher.go => messages.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename http/{method_dispatcher.go => messages.go} (100%) diff --git a/http/method_dispatcher.go b/http/messages.go similarity index 100% rename from http/method_dispatcher.go rename to http/messages.go From 2e60a6da410b93c02156c1009b3cc7224010d37b Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Mon, 30 Apr 2018 21:19:57 -0500 Subject: [PATCH 27/43] Placeholder tests --- http/parser_test.go | 4 ++++ playground/parameter_route_test.go | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/http/parser_test.go b/http/parser_test.go index 9ac9c0c..8120e74 100644 --- a/http/parser_test.go +++ b/http/parser_test.go @@ -159,6 +159,10 @@ var _ = Describe("LineRequestParser", func() { })) }) }) + + Context("given a target with percent-encoded query parameters", func() { + XIt("decodes each percent triplet to its corresponding ASCII character") + }) }) }) diff --git a/playground/parameter_route_test.go b/playground/parameter_route_test.go index cb259ca..61f7e82 100644 --- a/playground/parameter_route_test.go +++ b/playground/parameter_route_test.go @@ -74,3 +74,9 @@ var _ = Describe("ParameterRoute", func() { }) }) }) + +var _ = Describe("ParameterDecoder", func() { + Describe("#Get", func() { + XIt("works") + }) +}) \ No newline at end of file From bf3ee18c7122ff97a4f2379fb95c43695598ec89 Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Mon, 30 Apr 2018 21:24:01 -0500 Subject: [PATCH 28/43] Decoder -> Reporter --- playground/parameter_route.go | 15 ++++++++------- playground/parameter_route_test.go | 16 ++++++++-------- playground/playground_suite_test.go | 12 ++++++------ 3 files changed, 22 insertions(+), 21 deletions(-) diff --git a/playground/parameter_route.go b/playground/parameter_route.go index 56fade0..279f5d9 100644 --- a/playground/parameter_route.go +++ b/playground/parameter_route.go @@ -7,11 +7,11 @@ import ( ) func NewParameterRoute() *ParameterRoute { - return &ParameterRoute{Decoder: &TheDecoder{}} + return &ParameterRoute{Reporter: &AssignmentReporter{}} } type ParameterRoute struct { - Decoder ParameterDecoder + Reporter ParameterReporter } func (route *ParameterRoute) Route(requested http.RequestMessage) http.Request { @@ -19,20 +19,21 @@ func (route *ParameterRoute) Route(requested http.RequestMessage) http.Request { return nil } - return requested.MakeResourceRequest(route.Decoder) + return requested.MakeResourceRequest(route.Reporter) } -type ParameterDecoder interface { +type ParameterReporter interface { Name() string Get(client io.Writer, req http.RequestMessage) } -type TheDecoder struct{} +// Lists parameters as simple assignment statements +type AssignmentReporter struct{} -func (decoder *TheDecoder) Name() string { +func (decoder *AssignmentReporter) Name() string { return "Parameters" } -func (decoder *TheDecoder) Get(client io.Writer, req http.RequestMessage) { +func (decoder *AssignmentReporter) Get(client io.Writer, req http.RequestMessage) { panic("implement me") } diff --git a/playground/parameter_route_test.go b/playground/parameter_route_test.go index 61f7e82..936e6d1 100644 --- a/playground/parameter_route_test.go +++ b/playground/parameter_route_test.go @@ -17,7 +17,7 @@ var _ = Describe("::NewParameterRoute", func() { route := playground.NewParameterRoute() Expect(route).NotTo(BeNil()) Expect(route).To(BeEquivalentTo(&playground.ParameterRoute{ - Decoder: &playground.TheDecoder{}, + Reporter: &playground.AssignmentReporter{}, })) }) }) @@ -26,18 +26,18 @@ var _ = Describe("ParameterRoute", func() { Describe("#Route", func() { var ( router http.Route - decoder *ParameterDecoderMock + reporter *ParameterReporterMock response = &bytes.Buffer{} ) BeforeEach(func() { - decoder = &ParameterDecoderMock{} - router = &playground.ParameterRoute{Decoder: decoder} + reporter = &ParameterReporterMock{} + router = &playground.ParameterRoute{Reporter: reporter} response.Reset() }) Context("when the path is /parameters", func() { - It("routes GET to ParameterDecoder#Get with the decoded query parameters", func() { + It("routes GET to ParameterReporter#Get with the decoded query parameters", func() { request := &mock.Request{} requested := &httptest.RequestMessage{ PathReturns: "/parameters", @@ -45,7 +45,7 @@ var _ = Describe("ParameterRoute", func() { } Expect(router.Route(requested)).To(BeIdenticalTo(request)) - requested.MakeResourceRequestShouldHaveReceived(decoder) + requested.MakeResourceRequestShouldHaveReceived(reporter) }) Context("when the method is OPTIONS", func() { @@ -75,8 +75,8 @@ var _ = Describe("ParameterRoute", func() { }) }) -var _ = Describe("ParameterDecoder", func() { +var _ = Describe("AssignmentReporter", func() { Describe("#Get", func() { XIt("works") }) -}) \ No newline at end of file +}) diff --git a/playground/playground_suite_test.go b/playground/playground_suite_test.go index 2cc5e02..ba0cd2d 100644 --- a/playground/playground_suite_test.go +++ b/playground/playground_suite_test.go @@ -15,17 +15,17 @@ func TestPlayground(t *testing.T) { RunSpecs(t, "playground") } -/* ParameterDecoderMock */ +/* ParameterReporterMock */ -type ParameterDecoderMock struct { +type ParameterReporterMock struct { getParameters map[string]string } -func (mock *ParameterDecoderMock) Name() string { - return "Parameter Decoder Mock" +func (mock *ParameterReporterMock) Name() string { + return "Parameter Reporter Mock" } -func (mock *ParameterDecoderMock) Get(client io.Writer, req http.RequestMessage) { +func (mock *ParameterReporterMock) Get(client io.Writer, req http.RequestMessage) { //TODO KDK: Work here to get the parameters and expand the interface //mock.getParameters = map[string]string { // "two": "2", @@ -33,7 +33,7 @@ func (mock *ParameterDecoderMock) Get(client io.Writer, req http.RequestMessage) //} } -func (mock *ParameterDecoderMock) GetShouldHaveReceived(parameters map[string]string) { +func (mock *ParameterReporterMock) GetShouldHaveReceived(parameters map[string]string) { ExpectWithOffset(1, mock.getParameters).To(Equal(parameters)) } From 328eabe4150314f23ff174bd3ee7f30cd1191c86 Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Mon, 30 Apr 2018 21:57:31 -0500 Subject: [PATCH 29/43] Listing query parameters --- playground/parameter_route.go | 29 ++++++++++-- playground/parameter_route_test.go | 74 +++++++++++++++++++++++++++++- 2 files changed, 97 insertions(+), 6 deletions(-) diff --git a/playground/parameter_route.go b/playground/parameter_route.go index 279f5d9..1d73bdf 100644 --- a/playground/parameter_route.go +++ b/playground/parameter_route.go @@ -1,9 +1,12 @@ package playground import ( + "bytes" + "fmt" "io" "github.com/kkrull/gohttp/http" + "github.com/kkrull/gohttp/msg" ) func NewParameterRoute() *ParameterRoute { @@ -27,13 +30,29 @@ type ParameterReporter interface { Get(client io.Writer, req http.RequestMessage) } -// Lists parameters as simple assignment statements +// Lists query parameters as simple assignment statements type AssignmentReporter struct{} -func (decoder *AssignmentReporter) Name() string { - return "Parameters" +func (reporter *AssignmentReporter) Name() string { + return "Parameter Report" } -func (decoder *AssignmentReporter) Get(client io.Writer, req http.RequestMessage) { - panic("implement me") +func (reporter *AssignmentReporter) Get(client io.Writer, req http.RequestMessage) { + msg.WriteStatusLine(client, 200, "OK") + msg.WriteContentTypeHeader(client, "text/plain") + + body := reporter.makeBody(req) + msg.WriteContentLengthHeader(client, body.Len()) + msg.WriteEndOfMessageHeader(client) + + msg.WriteBody(client, body.String()) +} + +func (reporter *AssignmentReporter) makeBody(requestMessage http.RequestMessage) *bytes.Buffer { + body := &bytes.Buffer{} + for _, parameter := range requestMessage.QueryParameters() { + fmt.Fprintf(body, "%s = %s\n", parameter.Name, parameter.Value) + } + + return body } diff --git a/playground/parameter_route_test.go b/playground/parameter_route_test.go index 936e6d1..7dfdbdf 100644 --- a/playground/parameter_route_test.go +++ b/playground/parameter_route_test.go @@ -77,6 +77,78 @@ var _ = Describe("ParameterRoute", func() { var _ = Describe("AssignmentReporter", func() { Describe("#Get", func() { - XIt("works") + var ( + controller *playground.AssignmentReporter + request *httptest.RequestMessage + responseMessage *httptest.ResponseMessage + + response = &bytes.Buffer{} + ) + + BeforeEach(func() { + response.Reset() + }) + + Context("given any number of query parameters", func() { + BeforeEach(func() { + controller = &playground.AssignmentReporter{} + request = &httptest.RequestMessage{ + MethodReturns: "GET", + PathReturns: "/parameters", + } + + controller.Get(response, request) + responseMessage = httptest.ParseResponse(response) + }) + + It("responds 200 OK", func() { + responseMessage.StatusShouldBe(200, "OK") + responseMessage.ShouldBeWellFormed() + }) + It("sets Content-Type to text/plain", func() { + responseMessage.HeaderShould("Content-Type", Equal("text/plain")) + }) + }) + + Context("given a request with no query parameters", func() { + BeforeEach(func() { + controller = &playground.AssignmentReporter{} + request = &httptest.RequestMessage{ + MethodReturns: "GET", + PathReturns: "/parameters", + } + + controller.Get(response, request) + responseMessage = httptest.ParseResponse(response) + }) + + It("sets Content-Length to 0", func() { + responseMessage.HeaderShould("Content-Length", Equal("0")) + }) + It("writes no body", func() { + responseMessage.BodyShould(BeEmpty()) + }) + }) + + Context("given a request with 1 or more query parameters", func() { + BeforeEach(func() { + controller = &playground.AssignmentReporter{} + request = &httptest.RequestMessage{ + MethodReturns: "GET", + PathReturns: "/parameters", + } + request.AddQueryParameter("foo", "bar") + + controller.Get(response, request) + responseMessage = httptest.ParseResponse(response) + }) + + It("sets Content-Length", func() { + responseMessage.HeaderShould("Content-Length", Not(Equal("0"))) + }) + It("lists each parameter and its value in the body", func() { + responseMessage.BodyShould(ContainSubstring("foo = bar")) + }) + }) }) }) From e92a624ed7d30017e3ac13935136ec0343e0944d Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Mon, 30 Apr 2018 21:58:48 -0500 Subject: [PATCH 30/43] format --- http/messages.go | 12 ++++++------ http/parser.go | 2 +- playground/parameter_route_test.go | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/http/messages.go b/http/messages.go index 73f39c6..c6ae093 100644 --- a/http/messages.go +++ b/http/messages.go @@ -11,7 +11,7 @@ func NewGetMessage(path string) RequestMessage { return &requestMessage{ method: "GET", target: path, - path: path, + path: path, } } @@ -19,7 +19,7 @@ func NewHeadMessage(path string) RequestMessage { return &requestMessage{ method: "HEAD", target: path, - path: path, + path: path, } } @@ -27,7 +27,7 @@ func NewOptionsMessage(target string) RequestMessage { return &requestMessage{ method: "OPTIONS", target: target, - path: target, + path: target, } } @@ -35,7 +35,7 @@ func NewPutMessage(path string) RequestMessage { return &requestMessage{ method: "PUT", target: path, - path: path, + path: path, } } @@ -43,7 +43,7 @@ func NewTraceMessage(path string) RequestMessage { return &requestMessage{ method: "TRACE", target: path, - path: path, + path: path, } } @@ -51,7 +51,7 @@ func NewRequestMessage(method, path string) RequestMessage { return &requestMessage{ method: method, target: path, - path: path, + path: path, } } diff --git a/http/parser.go b/http/parser.go index 1bcf9aa..46aa7b5 100644 --- a/http/parser.go +++ b/http/parser.go @@ -43,7 +43,7 @@ func (parser *parseMethodObject) parsingTarget(method, target string) (ok *reque requested := &requestMessage{ method: method, target: target, - path: path, + path: path, } return parser.parsingQueryString(requested, query) diff --git a/playground/parameter_route_test.go b/playground/parameter_route_test.go index 7dfdbdf..5cac2fc 100644 --- a/playground/parameter_route_test.go +++ b/playground/parameter_route_test.go @@ -94,7 +94,7 @@ var _ = Describe("AssignmentReporter", func() { controller = &playground.AssignmentReporter{} request = &httptest.RequestMessage{ MethodReturns: "GET", - PathReturns: "/parameters", + PathReturns: "/parameters", } controller.Get(response, request) @@ -115,7 +115,7 @@ var _ = Describe("AssignmentReporter", func() { controller = &playground.AssignmentReporter{} request = &httptest.RequestMessage{ MethodReturns: "GET", - PathReturns: "/parameters", + PathReturns: "/parameters", } controller.Get(response, request) @@ -135,7 +135,7 @@ var _ = Describe("AssignmentReporter", func() { controller = &playground.AssignmentReporter{} request = &httptest.RequestMessage{ MethodReturns: "GET", - PathReturns: "/parameters", + PathReturns: "/parameters", } request.AddQueryParameter("foo", "bar") From 7fa876b6cebc1cc47d5d7a902719e68062a1bfbd Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Mon, 30 Apr 2018 22:02:34 -0500 Subject: [PATCH 31/43] Cleaned up unused mock --- playground/playground_suite_test.go | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/playground/playground_suite_test.go b/playground/playground_suite_test.go index ba0cd2d..7156846 100644 --- a/playground/playground_suite_test.go +++ b/playground/playground_suite_test.go @@ -17,25 +17,13 @@ func TestPlayground(t *testing.T) { /* ParameterReporterMock */ -type ParameterReporterMock struct { - getParameters map[string]string -} +type ParameterReporterMock struct{} func (mock *ParameterReporterMock) Name() string { return "Parameter Reporter Mock" } -func (mock *ParameterReporterMock) Get(client io.Writer, req http.RequestMessage) { - //TODO KDK: Work here to get the parameters and expand the interface - //mock.getParameters = map[string]string { - // "two": "2", - // "one": "1", - //} -} - -func (mock *ParameterReporterMock) GetShouldHaveReceived(parameters map[string]string) { - ExpectWithOffset(1, mock.getParameters).To(Equal(parameters)) -} +func (mock *ParameterReporterMock) Get(client io.Writer, req http.RequestMessage) {} /* ReadOnlyResourceMock */ From 50cb4c50b78e2ffc4cf86f6457e9a0cce8d13bc6 Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Tue, 1 May 2018 18:28:50 -0500 Subject: [PATCH 32/43] percent decoding --- http/parser.go | 2 +- http/parser_test.go | 7 ++++++- http/percent.go | 44 ++++++++++++++++++++++++++++++++++++++++++++ http/percent_test.go | 17 +++++++++++++++++ 4 files changed, 68 insertions(+), 2 deletions(-) create mode 100644 http/percent.go create mode 100644 http/percent_test.go diff --git a/http/parser.go b/http/parser.go index 46aa7b5..d04368f 100644 --- a/http/parser.go +++ b/http/parser.go @@ -60,7 +60,7 @@ func (parser *parseMethodObject) parsingQueryString(requested *requestMessage, r if len(nameValueFields) == 1 { requested.AddQueryFlag(nameValueFields[0]) } else { - requested.AddQueryParameter(nameValueFields[0], nameValueFields[1]) + requested.AddQueryParameter(nameValueFields[0], PercentDecode(nameValueFields[1])) } } diff --git a/http/parser_test.go b/http/parser_test.go index 8120e74..551befb 100644 --- a/http/parser_test.go +++ b/http/parser_test.go @@ -161,7 +161,12 @@ var _ = Describe("LineRequestParser", func() { }) Context("given a target with percent-encoded query parameters", func() { - XIt("decodes each percent triplet to its corresponding ASCII character") + It("decodes percent-encoded values", func() { + request, _ = parser.Parse(requestWithTarget("/widget?less=%3C")) + Expect(request.QueryParameters()).To(Equal([]http.QueryParameter{ + {Name: "less", Value: "<"}, + })) + }) }) }) }) diff --git a/http/percent.go b/http/percent.go new file mode 100644 index 0000000..6dbddf6 --- /dev/null +++ b/http/percent.go @@ -0,0 +1,44 @@ +package http + +import ( + "bytes" + "fmt" + "strconv" +) + +func PercentDecode(field string) string { + inputBuffer := bytes.NewBufferString(field) + inputBytes := make([]byte, len(field)) + inputSize, _ := inputBuffer.Read(inputBytes) + + outputBuffer := &bytes.Buffer{} + for i := 0; i < inputSize; { + input := inputBytes[i] + if input != '%' { + fmt.Printf("- Writing %x %s\n", input, string(input)) + outputBuffer.WriteByte(input) + i += 1 + continue + } + + var asciiCode int = 0 + asciiCode += 16 * base10Value(inputBytes[i+1]) + asciiCode += base10Value(inputBytes[i+2]) + fmt.Printf("- Writing %x %s\n", asciiCode, string(asciiCode)) + outputBuffer.WriteByte(byte(asciiCode)) + i += 3 + } + + fmt.Printf("field=<%s>, inputBytes_10[%d]=<%v>, inputBytes_16[%d]=<0x%x>, output[%d]=<%s>\n", + field, + inputSize, inputBytes, + inputSize, inputBytes, + outputBuffer.Len(), outputBuffer.String(), + ) + return outputBuffer.String() +} + +func base10Value(character byte) int { + value, _ := strconv.ParseInt(string(character), 16, 8) + return int(value) +} diff --git a/http/percent_test.go b/http/percent_test.go new file mode 100644 index 0000000..95fe812 --- /dev/null +++ b/http/percent_test.go @@ -0,0 +1,17 @@ +package http_test + +import ( + "github.com/kkrull/gohttp/http" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = Describe("PercentDecode", func() { + It("returns a string unchanged that has no percent triplets in it", func() { + Expect(http.PercentDecode("abcd")).To(Equal("abcd")) + }) + + It("decodes a % triplet into the ASCII character for its hexadecimal code", func() { + Expect(http.PercentDecode("%3C")).To(Equal("<")) + }) +}) From 54d575d8d401fbdaed82097b617ccbbfca1396e6 Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Wed, 2 May 2018 08:13:49 -0500 Subject: [PATCH 33/43] No need to convert to []byte for percent decoding --- http/percent.go | 48 ++++++++++++++++---------------------------- http/percent_test.go | 8 ++++++++ 2 files changed, 25 insertions(+), 31 deletions(-) diff --git a/http/percent.go b/http/percent.go index 6dbddf6..bc27406 100644 --- a/http/percent.go +++ b/http/percent.go @@ -2,43 +2,29 @@ package http import ( "bytes" - "fmt" "strconv" + "strings" ) func PercentDecode(field string) string { - inputBuffer := bytes.NewBufferString(field) - inputBytes := make([]byte, len(field)) - inputSize, _ := inputBuffer.Read(inputBytes) - outputBuffer := &bytes.Buffer{} - for i := 0; i < inputSize; { - input := inputBytes[i] - if input != '%' { - fmt.Printf("- Writing %x %s\n", input, string(input)) - outputBuffer.WriteByte(input) - i += 1 - continue - } - - var asciiCode int = 0 - asciiCode += 16 * base10Value(inputBytes[i+1]) - asciiCode += base10Value(inputBytes[i+2]) - fmt.Printf("- Writing %x %s\n", asciiCode, string(asciiCode)) - outputBuffer.WriteByte(byte(asciiCode)) - i += 3 - } - - fmt.Printf("field=<%s>, inputBytes_10[%d]=<%v>, inputBytes_16[%d]=<0x%x>, output[%d]=<%s>\n", - field, - inputSize, inputBytes, - inputSize, inputBytes, - outputBuffer.Len(), outputBuffer.String(), - ) + doDecoding2(field, outputBuffer) return outputBuffer.String() } -func base10Value(character byte) int { - value, _ := strconv.ParseInt(string(character), 16, 8) - return int(value) +func doDecoding2(field string, outputBuffer *bytes.Buffer) { + splits := strings.Split(field, "%") + inputBeforeFirstTriplet := splits[0] + outputBuffer.WriteString(inputBeforeFirstTriplet) + + inputContainingTriplets := splits[1:] + for _, hexCodePlusUnencoded := range inputContainingTriplets { + hexCode := hexCodePlusUnencoded[0:2] + asciiCode, _ := strconv.ParseInt(hexCode, 16, 8) + decodedCharacter := byte(asciiCode) + outputBuffer.WriteByte(decodedCharacter) + + unencodedInput := hexCodePlusUnencoded[2:] + outputBuffer.WriteString(unencodedInput) + } } diff --git a/http/percent_test.go b/http/percent_test.go index 95fe812..c9ef1a8 100644 --- a/http/percent_test.go +++ b/http/percent_test.go @@ -14,4 +14,12 @@ var _ = Describe("PercentDecode", func() { It("decodes a % triplet into the ASCII character for its hexadecimal code", func() { Expect(http.PercentDecode("%3C")).To(Equal("<")) }) + + It("retains characters after a percent triplet", func() { + Expect(http.PercentDecode("%3Cabc")).To(Equal(" Date: Wed, 2 May 2018 13:55:28 -0500 Subject: [PATCH 34/43] Making sure decoding handles errors --- http/parser.go | 3 ++- http/percent.go | 38 ++++++++++++++++++++++---------------- http/percent_test.go | 10 ++++++++++ 3 files changed, 34 insertions(+), 17 deletions(-) diff --git a/http/parser.go b/http/parser.go index d04368f..a01c2d7 100644 --- a/http/parser.go +++ b/http/parser.go @@ -60,7 +60,8 @@ func (parser *parseMethodObject) parsingQueryString(requested *requestMessage, r if len(nameValueFields) == 1 { requested.AddQueryFlag(nameValueFields[0]) } else { - requested.AddQueryParameter(nameValueFields[0], PercentDecode(nameValueFields[1])) + decodedValue, _ := PercentDecode(nameValueFields[1]) + requested.AddQueryParameter(nameValueFields[0], decodedValue) } } diff --git a/http/percent.go b/http/percent.go index bc27406..03e41b8 100644 --- a/http/percent.go +++ b/http/percent.go @@ -2,29 +2,35 @@ package http import ( "bytes" + "fmt" "strconv" "strings" ) -func PercentDecode(field string) string { +func PercentDecode(field string) (decoded string, malformed error) { outputBuffer := &bytes.Buffer{} - doDecoding2(field, outputBuffer) - return outputBuffer.String() -} - -func doDecoding2(field string, outputBuffer *bytes.Buffer) { splits := strings.Split(field, "%") - inputBeforeFirstTriplet := splits[0] - outputBuffer.WriteString(inputBeforeFirstTriplet) + unencodedPrefix, hexCodePrefixedSubstrings := splits[0], splits[1:] - inputContainingTriplets := splits[1:] - for _, hexCodePlusUnencoded := range inputContainingTriplets { - hexCode := hexCodePlusUnencoded[0:2] - asciiCode, _ := strconv.ParseInt(hexCode, 16, 8) - decodedCharacter := byte(asciiCode) - outputBuffer.WriteByte(decodedCharacter) + outputBuffer.WriteString(unencodedPrefix) + for _, hexCodePlusUnencoded := range hexCodePrefixedSubstrings { + if len(hexCodePlusUnencoded) < 2 { + return "", fmt.Errorf("%% followed by fewer than 2 characters: %s", field) + } - unencodedInput := hexCodePlusUnencoded[2:] - outputBuffer.WriteString(unencodedInput) + hexCodeCharacters, unencodedRemainder := splitAfterHexCode(hexCodePlusUnencoded) + outputBuffer.WriteByte(decode(hexCodeCharacters)) + outputBuffer.WriteString(unencodedRemainder) } + + return outputBuffer.String(), nil +} + +func splitAfterHexCode(hexCodePlusUnencoded string) (hexCode string, unencoded string) { + return hexCodePlusUnencoded[0:2], hexCodePlusUnencoded[2:] +} + +func decode(octetCharacters string) byte { + asciiCode, _ := strconv.ParseInt(octetCharacters, 16, 8) + return byte(asciiCode) } diff --git a/http/percent_test.go b/http/percent_test.go index c9ef1a8..8aa042d 100644 --- a/http/percent_test.go +++ b/http/percent_test.go @@ -22,4 +22,14 @@ var _ = Describe("PercentDecode", func() { It("decodes multiple percent triplets", func() { Expect(http.PercentDecode("%3Ca%3Cb")).To(Equal(" Date: Wed, 2 May 2018 16:52:36 -0500 Subject: [PATCH 35/43] Added a route to /redirect --- main/cmd/factory.go | 1 + main/cmd/factory_test.go | 4 ++ playground/redirect_route.go | 39 ++++++++++++++++ playground/redirect_route_test.go | 74 +++++++++++++++++++++++++++++++ 4 files changed, 118 insertions(+) create mode 100644 playground/redirect_route.go create mode 100644 playground/redirect_route_test.go diff --git a/main/cmd/factory.go b/main/cmd/factory.go index e9cbd2d..c7b529f 100644 --- a/main/cmd/factory.go +++ b/main/cmd/factory.go @@ -50,6 +50,7 @@ func (factory *InterruptFactory) routerWithAllRoutes(contentRootPath string) htt router.AddRoute(playground.NewParameterRoute()) router.AddRoute(playground.NewReadOnlyRoute()) router.AddRoute(playground.NewReadWriteRoute()) + router.AddRoute(playground.NewRedirectRoute()) router.AddRoute(teapot.NewRoute()) router.AddRoute(fs.NewRoute(contentRootPath)) return router diff --git a/main/cmd/factory_test.go b/main/cmd/factory_test.go index 2561d66..5961b96 100644 --- a/main/cmd/factory_test.go +++ b/main/cmd/factory_test.go @@ -103,6 +103,10 @@ var _ = Describe("InterruptFactory", func() { Expect(typedServer.Routes()).To(ContainElement(BeAssignableToTypeOf(playground.NewReadWriteRoute()))) }) + It("has a playground route for redirects", func() { + Expect(typedServer.Routes()).To(ContainElement(BeAssignableToTypeOf(playground.NewRedirectRoute()))) + }) + It("has a teapot route", func() { Expect(typedServer.Routes()).To(ContainElement(BeAssignableToTypeOf(teapot.NewRoute()))) }) diff --git a/playground/redirect_route.go b/playground/redirect_route.go new file mode 100644 index 0000000..93c5ba0 --- /dev/null +++ b/playground/redirect_route.go @@ -0,0 +1,39 @@ +package playground + +import ( + "io" + + "github.com/kkrull/gohttp/http" +) + +func NewRedirectRoute() http.Route { + return &RedirectRoute{ + Resource: &GoBackHomeResource{}, + } +} + +type RedirectRoute struct { + Resource RelocatedResource +} + +func (route *RedirectRoute) Route(requested http.RequestMessage) http.Request { + if requested.Path() != "/redirect" { + return nil + } + + return requested.MakeResourceRequest(route.Resource) +} + +type RelocatedResource interface { + Name() string +} + +type GoBackHomeResource struct{} + +func (*GoBackHomeResource) Name() string { + return "Relocated Resource" +} + +func (*GoBackHomeResource) Get(client io.Writer, req http.RequestMessage) { + panic("implement me") +} diff --git a/playground/redirect_route_test.go b/playground/redirect_route_test.go new file mode 100644 index 0000000..4a6f8e7 --- /dev/null +++ b/playground/redirect_route_test.go @@ -0,0 +1,74 @@ +package playground_test + +import ( + "bytes" + + "github.com/kkrull/gohttp/http" + "github.com/kkrull/gohttp/httptest" + "github.com/kkrull/gohttp/mock" + "github.com/kkrull/gohttp/msg/clienterror" + "github.com/kkrull/gohttp/playground" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = Describe("::NewRedirectRoute", func() { + It("returns a fully configured RedirectRoute", func() { + route := playground.NewRedirectRoute() + Expect(route).To(BeAssignableToTypeOf(&playground.RedirectRoute{})) + Expect(route).To(BeEquivalentTo(&playground.RedirectRoute{ + Resource: &playground.GoBackHomeResource{}, + })) + }) +}) + +var _ = Describe("RedirectRoute", func() { + Describe("#Route", func() { + var ( + router http.Route + resource *playground.GoBackHomeResource + response = &bytes.Buffer{} + ) + + BeforeEach(func() { + resource = &playground.GoBackHomeResource{} + router = &playground.RedirectRoute{Resource: resource} + response.Reset() + }) + + Context("when the path is /redirect", func() { + It("routes GET to RelocatedResource#Get", func() { + requestMessage := &httptest.RequestMessage{ + PathReturns: "/redirect", + MakeResourceRequestReturns: &mock.Request{}, + } + + Expect(router.Route(requestMessage)).To(BeIdenticalTo(requestMessage.MakeResourceRequestReturns)) + }) + + Context("when the method is OPTIONS", func() { + BeforeEach(func() { + requested := http.NewOptionsMessage("/redirect") + routedRequest := router.Route(requested) + Expect(routedRequest).NotTo(BeNil()) + routedRequest.Handle(response) + }) + + It("responds 200 OK with no body", httptest.ShouldHaveNoBody(response, 200, "OK")) + It("sets Allow to the methods implemented by this type", + httptest.ShouldAllowMethods(response, "GET", "OPTIONS")) + }) + + It("replies Method Not Allowed on any other method", func() { + requested := http.NewTraceMessage("/redirect") + routedRequest := router.Route(requested) + Expect(routedRequest).To(BeAssignableToTypeOf(clienterror.MethodNotAllowed())) + }) + }) + + It("returns nil for any other path", func() { + requestMessage := http.NewGetMessage("/no-route-for-you") + Expect(router.Route(requestMessage)).To(BeNil()) + }) + }) +}) From 1a73779db8abe6579f4e5e3e5f77ff2af25e218c Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Wed, 2 May 2018 17:06:50 -0500 Subject: [PATCH 36/43] GoHomeResource for redirect --- playground/redirect_route.go | 6 ++++- playground/redirect_route_test.go | 38 +++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/playground/redirect_route.go b/playground/redirect_route.go index 93c5ba0..bce5fc6 100644 --- a/playground/redirect_route.go +++ b/playground/redirect_route.go @@ -4,6 +4,7 @@ import ( "io" "github.com/kkrull/gohttp/http" + "github.com/kkrull/gohttp/msg" ) func NewRedirectRoute() http.Route { @@ -35,5 +36,8 @@ func (*GoBackHomeResource) Name() string { } func (*GoBackHomeResource) Get(client io.Writer, req http.RequestMessage) { - panic("implement me") + msg.WriteStatusLine(client, 302, "Found") + msg.WriteContentLengthHeader(client, 0) + msg.WriteHeader(client, "Location", "/") + msg.WriteEndOfMessageHeader(client) } diff --git a/playground/redirect_route_test.go b/playground/redirect_route_test.go index 4a6f8e7..6d0e9b3 100644 --- a/playground/redirect_route_test.go +++ b/playground/redirect_route_test.go @@ -72,3 +72,41 @@ var _ = Describe("RedirectRoute", func() { }) }) }) + +var _ = Describe("GoBackHomeResource", func() { + Describe("#Get", func() { + var ( + resource *playground.GoBackHomeResource + request *httptest.RequestMessage + responseMessage *httptest.ResponseMessage + + response = &bytes.Buffer{} + ) + + BeforeEach(func() { + response.Reset() + resource = &playground.GoBackHomeResource{} + request = &httptest.RequestMessage{ + MethodReturns: "GET", + PathReturns: "/redirect", + } + + resource.Get(response, request) + responseMessage = httptest.ParseResponse(response) + }) + + It("responds 302 Found", func() { + responseMessage.StatusShouldBe(302, "Found") + responseMessage.ShouldBeWellFormed() + }) + It("Sets Location to /", func() { + responseMessage.HeaderShould("Location", Equal("/")) + }) + It("sets Content-Length to 0", func() { + responseMessage.HeaderShould("Content-Length", Equal("0")) + }) + It("writes no body", func() { + responseMessage.BodyShould(BeEmpty()) + }) + }) +}) From 86168d8250c2558359f764f731d8ba88cb8dc502 Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Thu, 3 May 2018 15:59:08 -0500 Subject: [PATCH 37/43] Addressed review findings --- http/messages.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/http/messages.go b/http/messages.go index c6ae093..c27b5db 100644 --- a/http/messages.go +++ b/http/messages.go @@ -23,11 +23,13 @@ func NewHeadMessage(path string) RequestMessage { } } -func NewOptionsMessage(target string) RequestMessage { +// Creates an OPTIONS request to the specified target, which can either be a path starting with / +// or an asterisk-form query of the server as a whole (https://tools.ietf.org/html/rfc7230#section-5.3.4). +func NewOptionsMessage(targetAsteriskOrPath string) RequestMessage { return &requestMessage{ method: "OPTIONS", - target: target, - path: target, + target: targetAsteriskOrPath, + path: targetAsteriskOrPath, } } From 9a9f6d8f66817e7a2b95350e83fbf8d209b718de Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Fri, 4 May 2018 08:32:19 -0500 Subject: [PATCH 38/43] Introduced constants to DRY HTTP methods... ...and strip their names of magical powers --- capability/route.go | 6 ++--- capability/route_test.go | 4 ++-- capability/server_capability_test.go | 9 +++---- fs/route_test.go | 2 +- http/messages.go | 31 ++++++++++++++++--------- http/parser_test.go | 2 +- http/router_test.go | 4 ++-- msg/clienterror/client_requests_test.go | 4 ++-- playground/parameter_route_test.go | 8 +++---- playground/readonly_route_test.go | 8 +++---- playground/redirect_route_test.go | 4 ++-- playground/writable_route_test.go | 12 +++++----- teapot/route_test.go | 2 +- 13 files changed, 53 insertions(+), 43 deletions(-) diff --git a/capability/route.go b/capability/route.go index c736c12..f2ecb31 100644 --- a/capability/route.go +++ b/capability/route.go @@ -9,7 +9,7 @@ import ( func NewRoute() *ServerCapabilityRoute { controller := &StaticCapabilityServer{ - AvailableMethods: []string{"GET", "HEAD"}, + AvailableMethods: []string{http.GET, http.HEAD}, } return &ServerCapabilityRoute{Controller: controller} @@ -22,8 +22,8 @@ type ServerCapabilityRoute struct { func (route *ServerCapabilityRoute) Route(requested http.RequestMessage) http.Request { if requested.Target() != "*" { return nil - } else if requested.Method() != "OPTIONS" { - return clienterror.MethodNotAllowed("OPTIONS") + } else if requested.Method() != http.OPTIONS { + return clienterror.MethodNotAllowed(http.OPTIONS) } return &optionsRequest{Resource: route.Controller} diff --git a/capability/route_test.go b/capability/route_test.go index 1f548c6..02c746f 100644 --- a/capability/route_test.go +++ b/capability/route_test.go @@ -20,7 +20,7 @@ var _ = Describe("::NewRoute", func() { route := capability.NewRoute() Expect(route.Controller).To(BeEquivalentTo( &capability.StaticCapabilityServer{ - AvailableMethods: []string{"GET", "HEAD"}, + AvailableMethods: []string{http.GET, http.HEAD}, }, )) }) @@ -51,7 +51,7 @@ var _ = Describe("ServerCapabilityRoute", func() { It("returns MethodNotAllowed for any other method", func() { requested = http.NewGetMessage("*") routedRequest = router.Route(requested) - Expect(routedRequest).To(BeEquivalentTo(clienterror.MethodNotAllowed("OPTIONS"))) + Expect(routedRequest).To(BeEquivalentTo(clienterror.MethodNotAllowed(http.OPTIONS))) }) }) diff --git a/capability/server_capability_test.go b/capability/server_capability_test.go index 76dd885..1a585bf 100644 --- a/capability/server_capability_test.go +++ b/capability/server_capability_test.go @@ -4,6 +4,7 @@ import ( "bytes" "github.com/kkrull/gohttp/capability" + "github.com/kkrull/gohttp/http" "github.com/kkrull/gohttp/httptest" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -20,7 +21,7 @@ var _ = Describe("StaticCapabilityServer", func() { BeforeEach(func() { responseBuffer = &bytes.Buffer{} controller = &capability.StaticCapabilityServer{ - AvailableMethods: []string{"CONNECT", "TRACE"}, + AvailableMethods: []string{"CONNECT", http.TRACE}, } controller.Options(responseBuffer) response = httptest.ParseResponse(responseBuffer) @@ -40,14 +41,14 @@ var _ = Describe("StaticCapabilityServer", func() { BeforeEach(func() { responseBuffer = &bytes.Buffer{} controller = &capability.StaticCapabilityServer{ - AvailableMethods: []string{"OPTIONS"}, + AvailableMethods: []string{http.OPTIONS}, } controller.Options(responseBuffer) response = httptest.ParseResponse(responseBuffer) }) It("sets Allow to that one method", func() { - response.HeaderShould("Allow", Equal("OPTIONS")) + response.HeaderShould("Allow", Equal(http.OPTIONS)) }) }) @@ -55,7 +56,7 @@ var _ = Describe("StaticCapabilityServer", func() { BeforeEach(func() { responseBuffer = &bytes.Buffer{} controller = &capability.StaticCapabilityServer{ - AvailableMethods: []string{"CONNECT", "TRACE"}, + AvailableMethods: []string{"CONNECT", http.TRACE}, } controller.Options(responseBuffer) response = httptest.ParseResponse(responseBuffer) diff --git a/fs/route_test.go b/fs/route_test.go index 9986b0a..213068f 100644 --- a/fs/route_test.go +++ b/fs/route_test.go @@ -54,7 +54,7 @@ var _ = Describe("FileSystemRoute", func() { It("routes any other method to MethodNotAllowed", func() { requested := http.NewTraceMessage("/") routedRequest := route.Route(requested) - Expect(routedRequest).To(BeEquivalentTo(clienterror.MethodNotAllowed("GET", "HEAD", "OPTIONS"))) + Expect(routedRequest).To(BeEquivalentTo(clienterror.MethodNotAllowed(http.GET, http.HEAD, http.OPTIONS))) }) }) }) diff --git a/http/messages.go b/http/messages.go index c27b5db..668c8d3 100644 --- a/http/messages.go +++ b/http/messages.go @@ -7,9 +7,18 @@ import ( "github.com/kkrull/gohttp/msg/servererror" ) +const ( + GET string = "GET" + HEAD string = "HEAD" + OPTIONS string = "OPTIONS" + POST string = "POST" + PUT string = "PUT" + TRACE string = "TRACE" +) + func NewGetMessage(path string) RequestMessage { return &requestMessage{ - method: "GET", + method: GET, target: path, path: path, } @@ -17,7 +26,7 @@ func NewGetMessage(path string) RequestMessage { func NewHeadMessage(path string) RequestMessage { return &requestMessage{ - method: "HEAD", + method: HEAD, target: path, path: path, } @@ -27,7 +36,7 @@ func NewHeadMessage(path string) RequestMessage { // or an asterisk-form query of the server as a whole (https://tools.ietf.org/html/rfc7230#section-5.3.4). func NewOptionsMessage(targetAsteriskOrPath string) RequestMessage { return &requestMessage{ - method: "OPTIONS", + method: OPTIONS, target: targetAsteriskOrPath, path: targetAsteriskOrPath, } @@ -35,7 +44,7 @@ func NewOptionsMessage(targetAsteriskOrPath string) RequestMessage { func NewPutMessage(path string) RequestMessage { return &requestMessage{ - method: "PUT", + method: PUT, target: path, path: path, } @@ -43,7 +52,7 @@ func NewPutMessage(path string) RequestMessage { func NewTraceMessage(path string) RequestMessage { return &requestMessage{ - method: "TRACE", + method: TRACE, target: path, path: path, } @@ -93,7 +102,7 @@ func (message *requestMessage) NotImplemented() Response { } func (message *requestMessage) MakeResourceRequest(resource Resource) Request { - if message.method == "OPTIONS" { + if message.method == OPTIONS { return &optionsRequest{ SupportedMethods: message.supportedMethods(resource), } @@ -121,7 +130,7 @@ func (message *requestMessage) unsupportedMethod(resource Resource) Request { } func (message *requestMessage) supportedMethods(resource Resource) []string { - supported := []string{"OPTIONS"} + supported := []string{OPTIONS} for name, method := range knownMethods { imaginaryRequest := &requestMessage{method: name, target: message.target} request := method.MakeRequest(imaginaryRequest, resource) @@ -135,10 +144,10 @@ func (message *requestMessage) supportedMethods(resource Resource) []string { } var knownMethods = map[string]Method{ - "GET": &getMethod{}, - "HEAD": &headMethod{}, - "POST": &postMethod{}, - "PUT": &putMethod{}, + GET: &getMethod{}, + HEAD: &headMethod{}, + POST: &postMethod{}, + PUT: &putMethod{}, } type Method interface { diff --git a/http/parser_test.go b/http/parser_test.go index 551befb..8771956 100644 --- a/http/parser_test.go +++ b/http/parser_test.go @@ -73,7 +73,7 @@ var _ = Describe("LineRequestParser", func() { }) It("parses the request-line", func() { - Expect(request.Method()).To(Equal("GET")) + Expect(request.Method()).To(Equal(http.GET)) Expect(request.Target()).To(Equal("/foo")) }) diff --git a/http/router_test.go b/http/router_test.go index 7d13cc0..c193f93 100644 --- a/http/router_test.go +++ b/http/router_test.go @@ -50,8 +50,8 @@ var _ = Describe("RequestLineRouter", func() { }) It("tries routing the method and path from the request until it finds a match", func() { - unrelatedRoute.ShouldHaveReceived("HEAD", "/foo") - matchingRoute.ShouldHaveReceived("HEAD", "/foo") + unrelatedRoute.ShouldHaveReceived(http.HEAD, "/foo") + matchingRoute.ShouldHaveReceived(http.HEAD, "/foo") }) It("returns the request from the first matching Route", func() { diff --git a/msg/clienterror/client_requests_test.go b/msg/clienterror/client_requests_test.go index 6cdb6c8..6905ddb 100644 --- a/msg/clienterror/client_requests_test.go +++ b/msg/clienterror/client_requests_test.go @@ -18,11 +18,11 @@ var _ = Describe("MethodNotAllowed", func() { Describe("#Handle", func() { BeforeEach(func() { response.Reset() - request = clienterror.MethodNotAllowed("GET", "HEAD") + request = clienterror.MethodNotAllowed(http.GET, http.HEAD) request.Handle(response) }) It("responds 405 Method Not Allowed", httptest.ShouldHaveNoBody(response, 405, "Method Not Allowed")) - It("sets Allow to GET and HEAD", httptest.ShouldAllowMethods(response, "GET", "HEAD")) + It("sets Allow to GET and HEAD", httptest.ShouldAllowMethods(response, http.GET, http.HEAD)) }) }) diff --git a/playground/parameter_route_test.go b/playground/parameter_route_test.go index 5cac2fc..9d296f1 100644 --- a/playground/parameter_route_test.go +++ b/playground/parameter_route_test.go @@ -58,7 +58,7 @@ var _ = Describe("ParameterRoute", func() { It("responds 200 OK with no body", httptest.ShouldHaveNoBody(response, 200, "OK")) It("sets Allow to the methods implemented by this type", - httptest.ShouldAllowMethods(response, "GET", "OPTIONS")) + httptest.ShouldAllowMethods(response, http.GET, http.OPTIONS)) }) It("replies Method Not Allowed on any other method", func() { @@ -93,7 +93,7 @@ var _ = Describe("AssignmentReporter", func() { BeforeEach(func() { controller = &playground.AssignmentReporter{} request = &httptest.RequestMessage{ - MethodReturns: "GET", + MethodReturns: http.GET, PathReturns: "/parameters", } @@ -114,7 +114,7 @@ var _ = Describe("AssignmentReporter", func() { BeforeEach(func() { controller = &playground.AssignmentReporter{} request = &httptest.RequestMessage{ - MethodReturns: "GET", + MethodReturns: http.GET, PathReturns: "/parameters", } @@ -134,7 +134,7 @@ var _ = Describe("AssignmentReporter", func() { BeforeEach(func() { controller = &playground.AssignmentReporter{} request = &httptest.RequestMessage{ - MethodReturns: "GET", + MethodReturns: http.GET, PathReturns: "/parameters", } request.AddQueryParameter("foo", "bar") diff --git a/playground/readonly_route_test.go b/playground/readonly_route_test.go index 31711a1..01d73e5 100644 --- a/playground/readonly_route_test.go +++ b/playground/readonly_route_test.go @@ -33,12 +33,12 @@ var _ = Describe("ReadOnlyRoute", func() { Context("when the path is /method_options2", func() { It("routes GET to Teapot#Get", func() { - handleRequest(router, "GET", "/method_options2") + handleRequest(router, http.GET, "/method_options2") resource.GetShouldHaveBeenCalled() }) It("routes HEAD to Teapot#Options", func() { - handleRequest(router, "HEAD", "/method_options2") + handleRequest(router, http.HEAD, "/method_options2") resource.HeadShouldHaveBeenCalled() }) @@ -56,13 +56,13 @@ var _ = Describe("ReadOnlyRoute", func() { It("responds 200 OK with no body", httptest.ShouldHaveNoBody(response, 200, "OK")) It("sets Allow to the methods implemented by this type", - httptest.ShouldAllowMethods(response, "GET", "HEAD", "OPTIONS")) + httptest.ShouldAllowMethods(response, http.GET, http.HEAD, http.OPTIONS)) }) It("returns MethodNotAllowed for any other method", func() { requested := http.NewPutMessage("/method_options2") routedRequest := router.Route(requested) - Expect(routedRequest).To(BeEquivalentTo(clienterror.MethodNotAllowed("GET", "HEAD", "OPTIONS"))) + Expect(routedRequest).To(BeEquivalentTo(clienterror.MethodNotAllowed(http.GET, http.HEAD, http.OPTIONS))) }) }) diff --git a/playground/redirect_route_test.go b/playground/redirect_route_test.go index 6d0e9b3..154fb69 100644 --- a/playground/redirect_route_test.go +++ b/playground/redirect_route_test.go @@ -56,7 +56,7 @@ var _ = Describe("RedirectRoute", func() { It("responds 200 OK with no body", httptest.ShouldHaveNoBody(response, 200, "OK")) It("sets Allow to the methods implemented by this type", - httptest.ShouldAllowMethods(response, "GET", "OPTIONS")) + httptest.ShouldAllowMethods(response, http.GET, http.OPTIONS)) }) It("replies Method Not Allowed on any other method", func() { @@ -87,7 +87,7 @@ var _ = Describe("GoBackHomeResource", func() { response.Reset() resource = &playground.GoBackHomeResource{} request = &httptest.RequestMessage{ - MethodReturns: "GET", + MethodReturns: http.GET, PathReturns: "/redirect", } diff --git a/playground/writable_route_test.go b/playground/writable_route_test.go index b99b242..4bff57b 100644 --- a/playground/writable_route_test.go +++ b/playground/writable_route_test.go @@ -33,12 +33,12 @@ var _ = Describe("ReadWriteRoute", func() { Context("when the path is /method_options", func() { It("routes GET to Teapot#Get", func() { - handleRequest(router, "GET", "/method_options") + handleRequest(router, http.GET, "/method_options") resource.GetShouldHaveBeenCalled() }) It("routes HEAD to Teapot#Head", func() { - handleRequest(router, "HEAD", "/method_options") + handleRequest(router, http.HEAD, "/method_options") resource.HeadShouldHaveBeenCalled() }) @@ -56,23 +56,23 @@ var _ = Describe("ReadWriteRoute", func() { It("responds 200 OK with no body", httptest.ShouldHaveNoBody(response, 200, "OK")) It("sets Allow to the methods implemented by this type", - httptest.ShouldAllowMethods(response, "GET", "HEAD", "OPTIONS", "POST", "PUT")) + httptest.ShouldAllowMethods(response, http.GET, http.HEAD, http.OPTIONS, http.POST, http.PUT)) }) It("routes POST to Teapot#Post", func() { - handleRequest(router, "POST", "/method_options") + handleRequest(router, http.POST, "/method_options") resource.PostShouldHaveBeenCalled() }) It("routes PUT to Teapot#Put", func() { - handleRequest(router, "PUT", "/method_options") + handleRequest(router, http.PUT, "/method_options") resource.PutShouldHaveBeenCalled() }) It("returns MethodNotAllowed for any other method", func() { requested := http.NewTraceMessage("/method_options") routedRequest := router.Route(requested) - Expect(routedRequest).To(BeEquivalentTo(clienterror.MethodNotAllowed("GET", "HEAD", "OPTIONS", "POST", "PUT"))) + Expect(routedRequest).To(BeEquivalentTo(clienterror.MethodNotAllowed(http.GET, http.HEAD, http.OPTIONS, http.POST, http.PUT))) }) }) diff --git a/teapot/route_test.go b/teapot/route_test.go index d5c52e2..46c4e41 100644 --- a/teapot/route_test.go +++ b/teapot/route_test.go @@ -35,7 +35,7 @@ var _ = Describe("teapotRoute", func() { It("returns MethodNotAllowed for any other method", func() { requested := http.NewTraceMessage("/caffeine") routedRequest := router.Route(requested) - Expect(routedRequest).To(BeEquivalentTo(clienterror.MethodNotAllowed("GET", "OPTIONS"))) + Expect(routedRequest).To(BeEquivalentTo(clienterror.MethodNotAllowed(http.GET, http.OPTIONS))) }) }) From f3ccc2c07afbc72d5ce3443cbd1f4b5e90c29644 Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Fri, 4 May 2018 10:03:07 -0500 Subject: [PATCH 39/43] Extracted constants for some magic numbers --- http/parser.go | 3 ++- http/percent.go | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/http/parser.go b/http/parser.go index a01c2d7..23341e2 100644 --- a/http/parser.go +++ b/http/parser.go @@ -30,8 +30,9 @@ func (parser *parseMethodObject) ReadingRequestLine() (ok *requestMessage, badRe } func (parser *parseMethodObject) parsingRequestLine(requestLine string) (ok *requestMessage, badRequest Response) { + const numFieldsInRequestLine = 3 fields := strings.Split(requestLine, " ") - if len(fields) != 3 { + if len(fields) != numFieldsInRequestLine { return nil, &clienterror.BadRequest{DisplayText: "incorrectly formatted or missing request-line"} } diff --git a/http/percent.go b/http/percent.go index 03e41b8..82710b2 100644 --- a/http/percent.go +++ b/http/percent.go @@ -27,10 +27,12 @@ func PercentDecode(field string) (decoded string, malformed error) { } func splitAfterHexCode(hexCodePlusUnencoded string) (hexCode string, unencoded string) { - return hexCodePlusUnencoded[0:2], hexCodePlusUnencoded[2:] + return hexCodePlusUnencoded[:2], hexCodePlusUnencoded[2:] } func decode(octetCharacters string) byte { - asciiCode, _ := strconv.ParseInt(octetCharacters, 16, 8) + const base16 = 16 + const uintSizeInBits = 8 + asciiCode, _ := strconv.ParseInt(octetCharacters, base16, uintSizeInBits) return byte(asciiCode) } From 71748e149d7971a642f7087b54f2932866c0c22f Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Fri, 4 May 2018 15:45:32 -0500 Subject: [PATCH 40/43] Multiple returns to guard against missing nil checks --- http/messages.go | 10 +++++----- http/methods.go | 33 +++++++++++++++++++++------------ 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/http/messages.go b/http/messages.go index 668c8d3..7ce2758 100644 --- a/http/messages.go +++ b/http/messages.go @@ -113,8 +113,8 @@ func (message *requestMessage) MakeResourceRequest(resource Resource) Request { return message.unknownHttpMethod(resource) } - request := method.MakeRequest(message, resource) - if request == nil { + request, isSupported := method.MakeRequest(message, resource) + if !isSupported { return message.unsupportedMethod(resource) } @@ -133,8 +133,8 @@ func (message *requestMessage) supportedMethods(resource Resource) []string { supported := []string{OPTIONS} for name, method := range knownMethods { imaginaryRequest := &requestMessage{method: name, target: message.target} - request := method.MakeRequest(imaginaryRequest, resource) - if request != nil { + _, isSupported := method.MakeRequest(imaginaryRequest, resource) + if isSupported { supported = append(supported, name) } } @@ -151,7 +151,7 @@ var knownMethods = map[string]Method{ } type Method interface { - MakeRequest(requested *requestMessage, resource Resource) Request + MakeRequest(requested *requestMessage, resource Resource) (request Request, isSupported bool) } // Handles requests of supported HTTP methods for a resource diff --git a/http/methods.go b/http/methods.go index b44da3d..3d11027 100644 --- a/http/methods.go +++ b/http/methods.go @@ -11,16 +11,16 @@ import ( type getMethod struct{} -func (method *getMethod) MakeRequest(requested *requestMessage, resource Resource) Request { +func (method *getMethod) MakeRequest(requested *requestMessage, resource Resource) (request Request, isSupported bool) { supportedResource, ok := resource.(GetResource) if ok { return &getRequest{ Message: requested, Resource: supportedResource, - } + }, true } - return nil + return nil, false } type getRequest struct { @@ -41,13 +41,16 @@ type GetResource interface { type headMethod struct{} -func (*headMethod) MakeRequest(requested *requestMessage, resource Resource) Request { +func (*headMethod) MakeRequest(requested *requestMessage, resource Resource) (request Request, isSupported bool) { supportedResource, ok := resource.(HeadResource) if ok { - return &headRequest{Resource: supportedResource, Target: requested.target} + return &headRequest{ + Resource: supportedResource, + Target: requested.target, + }, true } - return nil + return nil, false } type headRequest struct { @@ -82,13 +85,16 @@ func (request *optionsRequest) Handle(client io.Writer) error { type postMethod struct{} -func (*postMethod) MakeRequest(requested *requestMessage, resource Resource) Request { +func (*postMethod) MakeRequest(requested *requestMessage, resource Resource) (request Request, isSupported bool) { supportedResource, ok := resource.(PostResource) if ok { - return &postRequest{Resource: supportedResource, Target: requested.target} + return &postRequest{ + Resource: supportedResource, + Target: requested.target, + }, true } - return nil + return nil, false } type postRequest struct { @@ -109,13 +115,16 @@ type PostResource interface { type putMethod struct{} -func (*putMethod) MakeRequest(requested *requestMessage, resource Resource) Request { +func (*putMethod) MakeRequest(requested *requestMessage, resource Resource) (request Request, isSupported bool) { supportedResource, ok := resource.(PutResource) if ok { - return &putRequest{Resource: supportedResource, Target: requested.target} + return &putRequest{ + Resource: supportedResource, + Target: requested.target, + }, true } - return nil + return nil, false } type putRequest struct { From 74220e1b63d8d72a6c19ff96dda3f8491d2a37d8 Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Sun, 6 May 2018 13:50:41 -0500 Subject: [PATCH 41/43] Extracted a custom error type for malformed octets --- http/percent.go | 10 +++++++++- http/percent_test.go | 11 +++++++++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/http/percent.go b/http/percent.go index 82710b2..77d2ec4 100644 --- a/http/percent.go +++ b/http/percent.go @@ -15,7 +15,7 @@ func PercentDecode(field string) (decoded string, malformed error) { outputBuffer.WriteString(unencodedPrefix) for _, hexCodePlusUnencoded := range hexCodePrefixedSubstrings { if len(hexCodePlusUnencoded) < 2 { - return "", fmt.Errorf("%% followed by fewer than 2 characters: %s", field) + return "", UnfinishedPercentEncoding{EnclosingField: field} } hexCodeCharacters, unencodedRemainder := splitAfterHexCode(hexCodePlusUnencoded) @@ -36,3 +36,11 @@ func decode(octetCharacters string) byte { asciiCode, _ := strconv.ParseInt(octetCharacters, base16, uintSizeInBits) return byte(asciiCode) } + +type UnfinishedPercentEncoding struct { + EnclosingField string +} + +func (invalid UnfinishedPercentEncoding) Error() string { + return fmt.Sprintf("%% followed by fewer than 2 characters: %s", invalid.EnclosingField) +} diff --git a/http/percent_test.go b/http/percent_test.go index 8aa042d..90dc081 100644 --- a/http/percent_test.go +++ b/http/percent_test.go @@ -25,11 +25,18 @@ var _ = Describe("PercentDecode", func() { It("returns an error when % is followed by no characters", func() { _, err := http.PercentDecode("abc%") - Expect(err).To(MatchError("% followed by fewer than 2 characters: abc%")) + Expect(err).To(BeEquivalentTo(http.UnfinishedPercentEncoding{EnclosingField: "abc%"})) }) It("returns an error when % is followed by 1 character", func() { _, err := http.PercentDecode("abc%1") - Expect(err).To(MatchError("% followed by fewer than 2 characters: abc%1")) + Expect(err).To(BeEquivalentTo(http.UnfinishedPercentEncoding{EnclosingField: "abc%1"})) + }) +}) + +var _ = Describe("UnfinishedPercentEncoding", func() { + It("describes the error and the offending text", func() { + err := &http.UnfinishedPercentEncoding{EnclosingField: "abc%"} + Expect(err).To(MatchError("% followed by fewer than 2 characters: abc%")) }) }) From a3268e77a2dd26022a9ec3282e5a2f5c4db24c25 Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Sun, 6 May 2018 20:31:52 -0500 Subject: [PATCH 42/43] DRY HTTP status codes/reasons --- capability/server_capability.go | 3 ++- fs/directory_listing.go | 3 ++- fs/file_contents.go | 3 ++- http/methods.go | 3 ++- msg/clienterror/client_errors.go | 5 ++--- msg/clienterror/client_requests.go | 2 +- msg/clienterror/doc.go | 2 ++ msg/clienterror/status.go | 9 +++++++++ msg/message.go | 9 +++++++-- msg/redirect/doc.go | 3 +++ msg/redirect/status.go | 7 +++++++ msg/servererror/doc.go | 2 ++ msg/servererror/server_errors.go | 5 ++--- msg/servererror/status.go | 8 ++++++++ msg/success/doc.go | 2 ++ msg/success/status.go | 5 +++++ playground/parameter_route.go | 3 ++- playground/redirect_route.go | 3 ++- playground/writable_route.go | 3 ++- teapot/requests.go | 6 ++++-- 20 files changed, 68 insertions(+), 18 deletions(-) create mode 100644 msg/clienterror/doc.go create mode 100644 msg/clienterror/status.go create mode 100644 msg/redirect/doc.go create mode 100644 msg/redirect/status.go create mode 100644 msg/servererror/doc.go create mode 100644 msg/servererror/status.go create mode 100644 msg/success/doc.go create mode 100644 msg/success/status.go diff --git a/capability/server_capability.go b/capability/server_capability.go index 87cd5e5..7efe2aa 100644 --- a/capability/server_capability.go +++ b/capability/server_capability.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/kkrull/gohttp/msg" + "github.com/kkrull/gohttp/msg/success" ) // Reports on server capabilities that are defined during startup and do not change after that @@ -13,7 +14,7 @@ type StaticCapabilityServer struct { } func (controller *StaticCapabilityServer) Options(client io.Writer) { - msg.WriteStatusLine(client, 200, "OK") + msg.WriteStatus(client, success.OKStatus) msg.WriteHeader(client, "Allow", strings.Join(controller.AvailableMethods, ",")) msg.WriteContentLengthHeader(client, 0) msg.WriteEndOfMessageHeader(client) diff --git a/fs/directory_listing.go b/fs/directory_listing.go index 0b77d4d..a0804a2 100644 --- a/fs/directory_listing.go +++ b/fs/directory_listing.go @@ -7,6 +7,7 @@ import ( "path" "github.com/kkrull/gohttp/msg" + "github.com/kkrull/gohttp/msg/success" ) type DirectoryListing struct { @@ -22,7 +23,7 @@ func (listing *DirectoryListing) WriteTo(client io.Writer) error { } func (listing *DirectoryListing) WriteHeader(client io.Writer) error { - msg.WriteStatusLine(client, 200, "OK") + msg.WriteStatus(client, success.OKStatus) msg.WriteContentTypeHeader(client, "text/html") listing.body = listing.messageListingFiles() diff --git a/fs/file_contents.go b/fs/file_contents.go index ce010ce..0659c12 100644 --- a/fs/file_contents.go +++ b/fs/file_contents.go @@ -8,6 +8,7 @@ import ( "strconv" "github.com/kkrull/gohttp/msg" + "github.com/kkrull/gohttp/msg/success" ) type FileContents struct { @@ -21,7 +22,7 @@ func (contents *FileContents) WriteTo(client io.Writer) error { } func (contents *FileContents) WriteHeader(client io.Writer) error { - msg.WriteStatusLine(client, 200, "OK") + msg.WriteStatus(client, success.OKStatus) contents.writeHeadersDescribingFile(client) msg.WriteEndOfMessageHeader(client) return nil diff --git a/http/methods.go b/http/methods.go index 3d11027..2457599 100644 --- a/http/methods.go +++ b/http/methods.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/kkrull/gohttp/msg" + "github.com/kkrull/gohttp/msg/success" ) /* GET */ @@ -74,7 +75,7 @@ type optionsRequest struct { } func (request *optionsRequest) Handle(client io.Writer) error { - msg.WriteStatusLine(client, 200, "OK") + msg.WriteStatus(client, success.OKStatus) msg.WriteContentLengthHeader(client, 0) msg.WriteHeader(client, "Allow", strings.Join(request.SupportedMethods, ",")) msg.WriteEndOfMessageHeader(client) diff --git a/msg/clienterror/client_errors.go b/msg/clienterror/client_errors.go index e27b7c2..618b559 100644 --- a/msg/clienterror/client_errors.go +++ b/msg/clienterror/client_errors.go @@ -1,4 +1,3 @@ -// HTTP 4xx Client Error responses from RFC 7231, Section 6.5 package clienterror import ( @@ -17,7 +16,7 @@ func (badRequest *BadRequest) WriteTo(client io.Writer) error { } func (badRequest *BadRequest) WriteHeader(client io.Writer) error { - msg.WriteStatusLine(client, 400, "Bad Request") + msg.WriteStatus(client, BadRequestStatus) //msg.WriteEndOfMessageHeader(client) return nil } @@ -34,7 +33,7 @@ func (notFound *NotFound) WriteTo(client io.Writer) error { } func (notFound *NotFound) WriteHeader(client io.Writer) error { - msg.WriteStatusLine(client, 404, "Not Found") + msg.WriteStatus(client, NotFoundStatus) msg.WriteContentTypeHeader(client, "text/plain") notFound.body = fmt.Sprintf("Not found: %s", notFound.Path) diff --git a/msg/clienterror/client_requests.go b/msg/clienterror/client_requests.go index de74b69..fc055b4 100644 --- a/msg/clienterror/client_requests.go +++ b/msg/clienterror/client_requests.go @@ -16,7 +16,7 @@ type methodNotAllowed struct { } func (notAllowed *methodNotAllowed) Handle(client io.Writer) error { - msg.WriteStatusLine(client, 405, "Method Not Allowed") + msg.WriteStatus(client, MethodNotAllowedStatus) msg.WriteContentLengthHeader(client, 0) msg.WriteHeader(client, "Allow", strings.Join(notAllowed.SupportedMethods, ",")) msg.WriteEndOfMessageHeader(client) diff --git a/msg/clienterror/doc.go b/msg/clienterror/doc.go new file mode 100644 index 0000000..410eda8 --- /dev/null +++ b/msg/clienterror/doc.go @@ -0,0 +1,2 @@ +// HTTP 4xx Client Error responses from RFC 7231, Section 6.5 +package clienterror diff --git a/msg/clienterror/status.go b/msg/clienterror/status.go new file mode 100644 index 0000000..f903783 --- /dev/null +++ b/msg/clienterror/status.go @@ -0,0 +1,9 @@ +package clienterror + +import "github.com/kkrull/gohttp/msg" + +var ( + BadRequestStatus = msg.Status{400, "Bad Request"} + NotFoundStatus = msg.Status{404, "Not Found"} + MethodNotAllowedStatus = msg.Status{405, "Method Not Allowed"} +) diff --git a/msg/message.go b/msg/message.go index b364712..2c67165 100644 --- a/msg/message.go +++ b/msg/message.go @@ -6,8 +6,13 @@ import ( "strconv" ) -func WriteStatusLine(client io.Writer, status int, reason string) { - fmt.Fprintf(client, "HTTP/1.1 %d %s\r\n", status, reason) +func WriteStatus(client io.Writer, status Status) { + fmt.Fprintf(client, "HTTP/1.1 %d %s\r\n", status.Code, status.Reason) +} + +type Status struct { + Code uint + Reason string } func WriteContentLengthHeader(client io.Writer, numBytes int) { diff --git a/msg/redirect/doc.go b/msg/redirect/doc.go new file mode 100644 index 0000000..abe4ac8 --- /dev/null +++ b/msg/redirect/doc.go @@ -0,0 +1,3 @@ +// Redirection 3xx responses from https://tools.ietf.org/html/rfc7231#section-6.4 + +package redirect diff --git a/msg/redirect/status.go b/msg/redirect/status.go new file mode 100644 index 0000000..e1dbd38 --- /dev/null +++ b/msg/redirect/status.go @@ -0,0 +1,7 @@ +package redirect + +import "github.com/kkrull/gohttp/msg" + +var ( + FoundStatus = msg.Status{Code: 302, Reason: "Found"} +) diff --git a/msg/servererror/doc.go b/msg/servererror/doc.go new file mode 100644 index 0000000..804c7fe --- /dev/null +++ b/msg/servererror/doc.go @@ -0,0 +1,2 @@ +// HTTP 5xx Server Error responses from RFC 7231, Section 6.6 +package servererror diff --git a/msg/servererror/server_errors.go b/msg/servererror/server_errors.go index b1dc587..26ac6e1 100644 --- a/msg/servererror/server_errors.go +++ b/msg/servererror/server_errors.go @@ -1,4 +1,3 @@ -// HTTP 5xx Server Error responses from RFC 7231, Section 6.6 package servererror import ( @@ -14,7 +13,7 @@ func (internalError *InternalServerError) WriteTo(client io.Writer) error { } func (internalError *InternalServerError) WriteHeader(client io.Writer) error { - msg.WriteStatusLine(client, 500, "Internal Server Error") + msg.WriteStatus(client, InternalServerErrorStatus) //msg.WriteEndOfMessageHeader(client) return nil } @@ -28,7 +27,7 @@ func (notImplemented *NotImplemented) WriteTo(client io.Writer) error { } func (notImplemented *NotImplemented) WriteHeader(client io.Writer) error { - msg.WriteStatusLine(client, 501, "Not Implemented") + msg.WriteStatus(client, NotImplementedStatus) //msg.WriteEndOfMessageHeader(client) return nil } diff --git a/msg/servererror/status.go b/msg/servererror/status.go new file mode 100644 index 0000000..a4997e0 --- /dev/null +++ b/msg/servererror/status.go @@ -0,0 +1,8 @@ +package servererror + +import "github.com/kkrull/gohttp/msg" + +var ( + InternalServerErrorStatus = msg.Status{500, "Internal Server Error"} + NotImplementedStatus = msg.Status{501, "Not Implemented"} +) diff --git a/msg/success/doc.go b/msg/success/doc.go new file mode 100644 index 0000000..00cd0f2 --- /dev/null +++ b/msg/success/doc.go @@ -0,0 +1,2 @@ +// HTTP 2xx Successful responses from RFC 7231, Section 6.3 +package success diff --git a/msg/success/status.go b/msg/success/status.go new file mode 100644 index 0000000..aae517b --- /dev/null +++ b/msg/success/status.go @@ -0,0 +1,5 @@ +package success + +import "github.com/kkrull/gohttp/msg" + +var OKStatus = msg.Status{200, "OK"} diff --git a/playground/parameter_route.go b/playground/parameter_route.go index 1d73bdf..c7bf420 100644 --- a/playground/parameter_route.go +++ b/playground/parameter_route.go @@ -7,6 +7,7 @@ import ( "github.com/kkrull/gohttp/http" "github.com/kkrull/gohttp/msg" + "github.com/kkrull/gohttp/msg/success" ) func NewParameterRoute() *ParameterRoute { @@ -38,7 +39,7 @@ func (reporter *AssignmentReporter) Name() string { } func (reporter *AssignmentReporter) Get(client io.Writer, req http.RequestMessage) { - msg.WriteStatusLine(client, 200, "OK") + msg.WriteStatus(client, success.OKStatus) msg.WriteContentTypeHeader(client, "text/plain") body := reporter.makeBody(req) diff --git a/playground/redirect_route.go b/playground/redirect_route.go index bce5fc6..4481119 100644 --- a/playground/redirect_route.go +++ b/playground/redirect_route.go @@ -5,6 +5,7 @@ import ( "github.com/kkrull/gohttp/http" "github.com/kkrull/gohttp/msg" + "github.com/kkrull/gohttp/msg/redirect" ) func NewRedirectRoute() http.Route { @@ -36,7 +37,7 @@ func (*GoBackHomeResource) Name() string { } func (*GoBackHomeResource) Get(client io.Writer, req http.RequestMessage) { - msg.WriteStatusLine(client, 302, "Found") + msg.WriteStatus(client, redirect.FoundStatus) msg.WriteContentLengthHeader(client, 0) msg.WriteHeader(client, "Location", "/") msg.WriteEndOfMessageHeader(client) diff --git a/playground/writable_route.go b/playground/writable_route.go index df6c0df..98a7808 100644 --- a/playground/writable_route.go +++ b/playground/writable_route.go @@ -5,6 +5,7 @@ import ( "github.com/kkrull/gohttp/http" "github.com/kkrull/gohttp/msg" + "github.com/kkrull/gohttp/msg/success" ) func NewReadWriteRoute() *ReadWriteRoute { @@ -57,7 +58,7 @@ func (controller *ReadWriteNopResource) Put(client io.Writer, target string) { } func writeOKWithNoBody(client io.Writer) { - msg.WriteStatusLine(client, 200, "OK") + msg.WriteStatus(client, success.OKStatus) msg.WriteContentLengthHeader(client, 0) msg.WriteEndOfMessageHeader(client) } diff --git a/teapot/requests.go b/teapot/requests.go index d787b26..1300941 100644 --- a/teapot/requests.go +++ b/teapot/requests.go @@ -5,6 +5,7 @@ import ( "github.com/kkrull/gohttp/http" "github.com/kkrull/gohttp/msg" + "github.com/kkrull/gohttp/msg/success" ) // Responds as a teapot that is aware of its own identity @@ -40,14 +41,15 @@ func (teapot *IdentityTeapot) getCoffee(client io.Writer) { } func writeHeaders(client io.Writer, body string) { - msg.WriteStatusLine(client, 418, "I'm a teapot") + teapotStatus := msg.Status{Code: 418, Reason: "I'm a teapot"} + msg.WriteStatus(client, teapotStatus) msg.WriteContentTypeHeader(client, "text/plain") msg.WriteContentLengthHeader(client, len(body)) msg.WriteEndOfMessageHeader(client) } func (teapot *IdentityTeapot) getTea(client io.Writer) { - msg.WriteStatusLine(client, 200, "OK") + msg.WriteStatus(client, success.OKStatus) msg.WriteContentLengthHeader(client, 0) msg.WriteEndOfMessageHeader(client) } From a51b9081614817ee78e6daf12405e53654fda872 Mon Sep 17 00:00:00 2001 From: Kyle Krull Date: Sun, 6 May 2018 20:49:37 -0500 Subject: [PATCH 43/43] Standardized one more IANA HTTP Method --- capability/server_capability_test.go | 4 ++-- http/messages.go | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/capability/server_capability_test.go b/capability/server_capability_test.go index 1a585bf..7f1bd9b 100644 --- a/capability/server_capability_test.go +++ b/capability/server_capability_test.go @@ -21,7 +21,7 @@ var _ = Describe("StaticCapabilityServer", func() { BeforeEach(func() { responseBuffer = &bytes.Buffer{} controller = &capability.StaticCapabilityServer{ - AvailableMethods: []string{"CONNECT", http.TRACE}, + AvailableMethods: []string{http.CONNECT, http.TRACE}, } controller.Options(responseBuffer) response = httptest.ParseResponse(responseBuffer) @@ -56,7 +56,7 @@ var _ = Describe("StaticCapabilityServer", func() { BeforeEach(func() { responseBuffer = &bytes.Buffer{} controller = &capability.StaticCapabilityServer{ - AvailableMethods: []string{"CONNECT", http.TRACE}, + AvailableMethods: []string{http.CONNECT, http.TRACE}, } controller.Options(responseBuffer) response = httptest.ParseResponse(responseBuffer) diff --git a/http/messages.go b/http/messages.go index 7ce2758..77e91e0 100644 --- a/http/messages.go +++ b/http/messages.go @@ -8,6 +8,7 @@ import ( ) const ( + CONNECT string = "CONNECT" GET string = "GET" HEAD string = "HEAD" OPTIONS string = "OPTIONS"