From 38f0310a62204aec7b0d2af450b5424173f52709 Mon Sep 17 00:00:00 2001 From: Sebastian Mancke Date: Sun, 19 Jun 2016 13:18:03 +0200 Subject: [PATCH 01/17] best speed for gzip handler --- util/gzip_handler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/gzip_handler.go b/util/gzip_handler.go index ad5da4d..b855065 100644 --- a/util/gzip_handler.go +++ b/util/gzip_handler.go @@ -28,7 +28,7 @@ var GzipCompressableTypes = []string{ } var gzipWriterPool = sync.Pool{ - New: func() interface{} { return gzip.NewWriter(nil) }, + New: func() interface{} { w, _ := gzip.NewWriterLevel(nil, gzip.BestSpeed); return w }, } // Transparently gzip the response body if From 827c9558fca0fb4a413d8d29031da73b52bc6117 Mon Sep 17 00:00:00 2001 From: Sebastian Mancke Date: Sun, 19 Jun 2016 13:21:55 +0200 Subject: [PATCH 02/17] Set Content-Length header to enable http keep alive --- composition/composition_handler.go | 16 ++++++----- composition/composition_handler_test.go | 5 ++-- composition/content_merge.go | 15 +++-------- composition/content_merge_test.go | 36 +++---------------------- composition/interface_mocks_test.go | 15 ++++++----- composition/interfaces.go | 4 +-- 6 files changed, 29 insertions(+), 62 deletions(-) diff --git a/composition/composition_handler.go b/composition/composition_handler.go index 704bb57..89622ff 100644 --- a/composition/composition_handler.go +++ b/composition/composition_handler.go @@ -4,6 +4,7 @@ import ( log "github.com/Sirupsen/logrus" "io" "net/http" + "strconv" "strings" ) @@ -50,7 +51,7 @@ func (agg *CompositionHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) } else if res.Def.Required { log.WithField("fetchResult", res).Errorf("error loading content from: %v", res.Def.URL) - res.Def.ErrHandler.Handle(res.Err, w, r) + res.Def.ErrHandler.Handle(res.Err, w, r) return } else { log.WithField("fetchResult", res).Warnf("optional content not loaded: %v", res.Def.URL) @@ -69,15 +70,16 @@ func (agg *CompositionHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) // Overwrite Content-Type to ensure, that the encoding is correct w.Header().Set("Content-Type", "text/html; charset=utf-8") - if status != 200 { - w.WriteHeader(status) - } - err := mergeContext.WriteHtml(w) + html, err := mergeContext.GetHtml() if err != nil { - log.Error(err.Error()) - http.Error(w, "Internal Server Error: " + err.Error(), 500) + log.Error(err.Error()) + http.Error(w, "Internal Server Error: "+err.Error(), 500) return } + + w.Header().Set("Content-Length", strconv.Itoa(len(html))) + w.WriteHeader(status) + w.Write(html) } func MetadataForRequest(r *http.Request) map[string]interface{} { diff --git a/composition/composition_handler_test.go b/composition/composition_handler_test.go index c1e2d31..606a397 100644 --- a/composition/composition_handler_test.go +++ b/composition/composition_handler_test.go @@ -87,9 +87,10 @@ func Test_CompositionHandler_CorrectHeaderAndStatusCodeReturned(t *testing.T) { ch.ServeHTTP(resp, r) a.Equal(302, resp.Code) - a.Equal(3, len(resp.Header())) + a.Equal(4, len(resp.Header())) a.Equal("/look/somewhere", resp.Header().Get("Location")) a.Equal("", resp.Header().Get("Transfer-Encoding")) + a.NotEqual("", resp.Header().Get("Content-Length")) a.Contains(resp.Header()["Set-Cookie"], "cookie-content 1") a.Contains(resp.Header()["Set-Cookie"], "cookie-content 2") } @@ -151,7 +152,7 @@ func Test_CompositionHandler_ErrorInMerging(t *testing.T) { aggregator.contentMergerFactory = func(jsonData map[string]interface{}) ContentMerger { merger := NewMockContentMerger(ctrl) merger.EXPECT().AddContent(gomock.Any()) - merger.EXPECT().WriteHtml(gomock.Any()).Return(errors.New("an error")) + merger.EXPECT().GetHtml().Return(nil, errors.New("an error")) return merger } diff --git a/composition/content_merge.go b/composition/content_merge.go index e540990..4ffe38d 100644 --- a/composition/content_merge.go +++ b/composition/content_merge.go @@ -35,17 +35,10 @@ func NewContentMerge(metaJSON map[string]interface{}) *ContentMerge { return cntx } -func (cntx *ContentMerge) WriteHtml(w io.Writer) error { - if cntx.Buffered { - buff := bytes.NewBuffer(make([]byte, 0, DefaultBufferSize)) - if err := cntx.WriteHtmlUnbuffered(buff); err != nil { - return err - } - _, err := buff.WriteTo(w) - return err - } else { - return cntx.WriteHtmlUnbuffered(w) - } +func (cntx *ContentMerge) GetHtml() ([]byte, error) { + buff := bytes.NewBuffer(make([]byte, 0, DefaultBufferSize)) + err := cntx.WriteHtmlUnbuffered(buff) + return buff.Bytes(), err } func (cntx *ContentMerge) WriteHtmlUnbuffered(w io.Writer) error { diff --git a/composition/content_merge_test.go b/composition/content_merge_test.go index 291116f..9360576 100644 --- a/composition/content_merge_test.go +++ b/composition/content_merge_test.go @@ -1,7 +1,6 @@ package composition import ( - "bytes" "errors" "github.com/stretchr/testify/assert" "io" @@ -54,11 +53,9 @@ func Test_ContentMerge_PositiveCase(t *testing.T) { cm.AddContent(asFetchResult(page2)) cm.AddContent(asFetchResult(page3)) - buff := bytes.NewBuffer(nil) - err := cm.WriteHtml(buff) - + html, err := cm.GetHtml() a.NoError(err) - a.Equal(expected, buff.String()) + a.Equal(expected, string(html)) } type MockPage1BodyFragment struct { @@ -82,36 +79,9 @@ func (f MockPage1BodyFragment) Execute(w io.Writer, data map[string]interface{}, func Test_ContentMerge_MainFragmentDoesNotExist(t *testing.T) { a := assert.New(t) cm := NewContentMerge(nil) - buff := bytes.NewBuffer(nil) - err := cm.WriteHtml(buff) + _, err := cm.GetHtml() a.Error(err) a.Equal("Fragment does not exist: ", err.Error()) - // the buffered merger should not write if errors occur - a.Equal(0, len(buff.Bytes())) -} - -func Test_ContentMerge_ErrorOnWrite(t *testing.T) { - a := assert.New(t) - - page := NewMemoryContent() - page.body[""] = StringFragment("Hello World\n") - - cm := NewContentMerge(nil) - cm.AddContent(asFetchResult(page)) - - err := cm.WriteHtml(closedWriterMock{}) - a.Error(err) - a.Equal("writer closed", err.Error()) -} - -func Test_ContentMerge_ErrorOnWriteUnbuffered(t *testing.T) { - a := assert.New(t) - - cm := NewContentMerge(nil) - cm.Buffered = false - err := cm.WriteHtml(closedWriterMock{}) - a.Error(err) - a.Equal("writer closed", err.Error()) } type closedWriterMock struct { diff --git a/composition/interface_mocks_test.go b/composition/interface_mocks_test.go index de2a534..5a08735 100644 --- a/composition/interface_mocks_test.go +++ b/composition/interface_mocks_test.go @@ -5,9 +5,9 @@ package composition import ( gomock "github.com/golang/mock/gomock" + io "io" http "net/http" - ) // Mock of Fragment interface @@ -223,14 +223,15 @@ func (_mr *_MockContentMergerRecorder) AddContent(arg0 interface{}) *gomock.Call return _mr.mock.ctrl.RecordCall(_mr.mock, "AddContent", arg0) } -func (_m *MockContentMerger) WriteHtml(_param0 io.Writer) error { - ret := _m.ctrl.Call(_m, "WriteHtml", _param0) - ret0, _ := ret[0].(error) - return ret0 +func (_m *MockContentMerger) GetHtml() ([]byte, error) { + ret := _m.ctrl.Call(_m, "GetHtml") + ret0, _ := ret[0].([]byte) + ret1, _ := ret[1].(error) + return ret0, ret1 } -func (_mr *_MockContentMergerRecorder) WriteHtml(arg0 interface{}) *gomock.Call { - return _mr.mock.ctrl.RecordCall(_mr.mock, "WriteHtml", arg0) +func (_mr *_MockContentMergerRecorder) GetHtml() *gomock.Call { + return _mr.mock.ctrl.RecordCall(_mr.mock, "GetHtml") } // Mock of ContentParser interface diff --git a/composition/interfaces.go b/composition/interfaces.go index 81fbeca..f6091ca 100644 --- a/composition/interfaces.go +++ b/composition/interfaces.go @@ -75,8 +75,8 @@ type ContentMerger interface { // Add content to the meger AddContent(fetchResult *FetchResult) - // Merge and write all content supplied writer - WriteHtml(w io.Writer) error + // Return the html as byte array + GetHtml() ([]byte, error) } type ResponseProcessor interface { From 1b69be0abf7ce141d2236cb73e0b3fd86a092b2b Mon Sep 17 00:00:00 2001 From: Sebastian Mancke Date: Sun, 19 Jun 2016 13:33:52 +0200 Subject: [PATCH 03/17] simplified method and error handling --- composition/content_merge.go | 36 +++++++++++------------------------- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/composition/content_merge.go b/composition/content_merge.go index 4ffe38d..0c27911 100644 --- a/composition/content_merge.go +++ b/composition/content_merge.go @@ -36,12 +36,8 @@ func NewContentMerge(metaJSON map[string]interface{}) *ContentMerge { } func (cntx *ContentMerge) GetHtml() ([]byte, error) { - buff := bytes.NewBuffer(make([]byte, 0, DefaultBufferSize)) - err := cntx.WriteHtmlUnbuffered(buff) - return buff.Bytes(), err -} + w := bytes.NewBuffer(make([]byte, 0, DefaultBufferSize)) -func (cntx *ContentMerge) WriteHtmlUnbuffered(w io.Writer) error { var executeFragment func(fragmentName string) error executeFragment = func(fragmentName string) error { f, exist := cntx.Body[fragmentName] @@ -51,48 +47,38 @@ func (cntx *ContentMerge) WriteHtmlUnbuffered(w io.Writer) error { return f.Execute(w, cntx.MetaJSON, executeFragment) } - if _, err := io.WriteString(w, "\n \n "); err != nil { - return err - } + io.WriteString(w, "\n \n ") for _, f := range cntx.Head { if err := f.Execute(w, cntx.MetaJSON, executeFragment); err != nil { - return err + return nil, err } } - if _, err := io.WriteString(w, "\n \n \n \n "); err != nil { - return err - } + io.WriteString(w, ">\n ") if err := executeFragment(""); err != nil { - return err + return nil, err } for _, f := range cntx.Tail { if err := f.Execute(w, cntx.MetaJSON, executeFragment); err != nil { - return err + return nil, err } } - if _, err := io.WriteString(w, "\n \n\n"); err != nil { - return err - } + io.WriteString(w, "\n \n\n") - return nil + return w.Bytes(), nil } func (cntx *ContentMerge) AddContent(fetchResult *FetchResult) { From ba8ca96ae2bcd752a749ba6a85d35a175806889d Mon Sep 17 00:00:00 2001 From: Sebastian Mancke Date: Mon, 20 Jun 2016 14:04:51 +0200 Subject: [PATCH 04/17] change constant name --- composition/file_content_loader.go | 4 ++-- composition/file_content_loader_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/composition/file_content_loader.go b/composition/file_content_loader.go index 948a4ef..6751ca7 100644 --- a/composition/file_content_loader.go +++ b/composition/file_content_loader.go @@ -9,7 +9,7 @@ import ( "time" ) -var RequestProcessorsNotApplicable = errors.New("request processors are not apliable on file content") +var ResponseProcessorsNotApplicable = errors.New("request processors are not apliable on file content") type FileContentLoader struct { parser ContentParser @@ -23,7 +23,7 @@ func NewFileContentLoader() *FileContentLoader { func (loader *FileContentLoader) Load(fd *FetchDefinition) (Content, error) { if fd.RespProc != nil { - return nil, RequestProcessorsNotApplicable + return nil, ResponseProcessorsNotApplicable } filename := strings.TrimPrefix(fd.URL, FileURLPrefix) f, err := os.Open(filename) diff --git a/composition/file_content_loader_test.go b/composition/file_content_loader_test.go index ea63652..67c7824 100644 --- a/composition/file_content_loader_test.go +++ b/composition/file_content_loader_test.go @@ -67,7 +67,7 @@ func Test_FileContentLoader_RequestProcessor(t *testing.T) { fd.RespProc = NewMockResponseProcessor(ctrl) _, err := NewFileContentLoader().Load(fd) - a.Equal(RequestProcessorsNotApplicable, err) + a.Equal(ResponseProcessorsNotApplicable, err) } const letterBytes = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" From 9adbe65c8719942ccb88f807b28e78a354003d18 Mon Sep 17 00:00:00 2001 From: Sebastian Mancke Date: Mon, 20 Jun 2016 14:20:19 +0200 Subject: [PATCH 05/17] Update README.md changed coveralls link --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 9b4d16b..1567bd7 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ # lib-compose -[![Build Status](https://api.travis-ci.org/tarent/lib-compose.svg)](https://travis-ci.org/tarent/lib-compose) [![Coverage Status](https://coveralls.io/repos/tarent/lib-compose/badge.svg?branch=master&service=github)](https://coveralls.io/github/smancke/guble?branch=master) [![Go Report Card](https://goreportcard.com/badge/github.com/tarent/lib-compose)](https://goreportcard.com/report/github.com/tarent/lib-compose) +[![Build Status](https://api.travis-ci.org/tarent/lib-compose.svg)](https://travis-ci.org/tarent/lib-compose) [![Coverage Status](https://coveralls.io/repos/tarent/lib-compose/badge.svg?branch=master&service=github)](https://coveralls.io/github/tarent/lib-compose?branch=master) [![Go Report Card](https://goreportcard.com/badge/github.com/tarent/lib-compose)](https://goreportcard.com/report/github.com/tarent/lib-compose) lib-ui-service is a golang library for common code used by UI-Services. UI-Services are those services, which integrate a set of HTML page to one combined page. From f55239908c8e80a6c523449486a3e0ad6dd35f82 Mon Sep 17 00:00:00 2001 From: Dino Omanovic Date: Mon, 20 Jun 2016 14:36:53 +0200 Subject: [PATCH 06/17] Initial commit, changed method signatures and tests for http status pass through --- composition/content_fetcher.go | 5 +++-- composition/http_content_loader.go | 24 +++++++++++++++--------- composition/http_content_loader_test.go | 17 +++++++++-------- composition/interface_mocks_test.go | 5 +++-- composition/interfaces.go | 2 +- 5 files changed, 31 insertions(+), 22 deletions(-) diff --git a/composition/content_fetcher.go b/composition/content_fetcher.go index a0fcf80..5a2915a 100644 --- a/composition/content_fetcher.go +++ b/composition/content_fetcher.go @@ -17,6 +17,7 @@ type FetchResult struct { Def *FetchDefinition Err error Content Content + HttpStatus int Hash string // the hash of the FetchDefinition } @@ -91,7 +92,7 @@ func (fetcher *ContentFetcher) AddFetchJob(d *FetchDefinition) { // want to override the original URL with expanded values definitionCopy := *d definitionCopy.URL = url - fetchResult.Content, fetchResult.Err = fetcher.fetch(&definitionCopy) + fetchResult.Content, fetchResult.Err, fetchResult.HttpStatus = fetcher.fetch(&definitionCopy) if fetchResult.Err == nil { fetcher.addMeta(fetchResult.Content.Meta()) @@ -109,7 +110,7 @@ func (fetcher *ContentFetcher) AddFetchJob(d *FetchDefinition) { }() } -func (fetcher *ContentFetcher) fetch(fd *FetchDefinition) (Content, error) { +func (fetcher *ContentFetcher) fetch(fd *FetchDefinition) (Content, error, int) { return fetcher.contentLoader.Load(fd) } diff --git a/composition/http_content_loader.go b/composition/http_content_loader.go index 692bd3b..96804e5 100644 --- a/composition/http_content_loader.go +++ b/composition/http_content_loader.go @@ -23,13 +23,14 @@ func NewHttpContentLoader() *HttpContentLoader { } // TODO: Should we filter the headers, which we forward here, or is it correct to copy all of them? -func (loader *HttpContentLoader) Load(fd *FetchDefinition) (Content, error) { +func (loader *HttpContentLoader) Load(fd *FetchDefinition) (Content, error, int) { client := &http.Client{Timeout: fd.Timeout} + statusCode:=502 var err error request, err := http.NewRequest(fd.Method, fd.URL, fd.Body) if err != nil { - return nil, err + return nil, err, 0 } request.Header = fd.Header if request.Header == nil { @@ -41,26 +42,31 @@ func (loader *HttpContentLoader) Load(fd *FetchDefinition) (Content, error) { resp, err := client.Do(request) + if resp != nil { + statusCode = resp.StatusCode + } + + logging.Call(request, resp, start, err) if err != nil { - return nil, err + return nil, err, statusCode } if fd.RespProc != nil { err = fd.RespProc.Process(resp, fd.URL) } if err != nil { - return nil, err + return nil, err, statusCode } - if resp.StatusCode < 200 || resp.StatusCode > 399 { - return nil, fmt.Errorf("(http %v) on loading url %q", resp.StatusCode, fd.URL) + if statusCode < 200 || statusCode > 399 { + return nil, fmt.Errorf("(http %v) on loading url %q", statusCode, fd.URL), statusCode } c := NewMemoryContent() c.url = fd.URL c.httpHeader = resp.Header - c.httpStatusCode = resp.StatusCode + c.httpStatusCode = statusCode // take the first parser for the content type // direct access to the map does not work, because the @@ -81,11 +87,11 @@ func (loader *HttpContentLoader) Load(fd *FetchDefinition) (Content, error) { WithField("full_url", c.URL()). WithField("duration", time.Since(parsingStart)). Debug("content parsing") - return c, err + return c, err, statusCode } } } c.reader = resp.Body - return c, nil + return c, nil, statusCode } diff --git a/composition/http_content_loader_test.go b/composition/http_content_loader_test.go index a719539..03859a1 100644 --- a/composition/http_content_loader_test.go +++ b/composition/http_content_loader_test.go @@ -19,7 +19,7 @@ func XTest_HttpContentLoader_Load(t *testing.T) { defer server.Close() loader := &HttpContentLoader{} - c, err := loader.Load(NewFetchDefinition(server.URL)) + c, err, _ := loader.Load(NewFetchDefinition(server.URL)) a.NoError(err) a.NotNil(c) a.Nil(c.Reader()) @@ -68,7 +68,7 @@ func Test_HttpContentLoader_Load(t *testing.T) { loader.parser["text/html"] = mockParser - c, err := loader.Load(NewFetchDefinition(server.URL)) + c, err, _ := loader.Load(NewFetchDefinition(server.URL)) a.NoError(err) a.NotNil(c) a.Nil(c.Reader()) @@ -100,7 +100,7 @@ func Test_HttpContentLoader_Load_ResponseProcessor(t *testing.T) { mockResponseProcessor := NewMockResponseProcessor(ctrl) mockResponseProcessor.EXPECT().Process(gomock.Any(), gomock.Any()) - c, err := loader.Load(NewFetchDefinitionWithResponseProcessor(server.URL, mockResponseProcessor)) + c, err, _ := loader.Load(NewFetchDefinitionWithResponseProcessor(server.URL, mockResponseProcessor)) a.NoError(err) a.NotNil(c) a.Nil(c.Reader()) @@ -141,7 +141,7 @@ func Test_HttpContentLoader_Load_POST(t *testing.T) { fd.Method = "POST" fd.Body = strings.NewReader("post content") - c, err := loader.Load(fd) + c, err, _ := loader.Load(fd) a.NoError(err) a.NotNil(c) a.Nil(c.Reader()) @@ -160,7 +160,7 @@ func Test_HttpContentLoader_LoadStream(t *testing.T) { defer server.Close() loader := &HttpContentLoader{} - c, err := loader.Load(NewFetchDefinition(server.URL)) + c, err, _ := loader.Load(NewFetchDefinition(server.URL)) a.NoError(err) a.NotNil(c.Reader()) body, err := ioutil.ReadAll(c.Reader()) @@ -179,7 +179,7 @@ func Test_HttpContentLoader_LoadStream_No_Composition_Header(t *testing.T) { defer server.Close() loader := &HttpContentLoader{} - c, err := loader.Load(NewFetchDefinition(server.URL)) + c, err, _ := loader.Load(NewFetchDefinition(server.URL)) a.NoError(err) a.NotNil(c.Reader()) body, err := ioutil.ReadAll(c.Reader()) @@ -196,17 +196,18 @@ func Test_HttpContentLoader_LoadError500(t *testing.T) { defer server.Close() loader := &HttpContentLoader{} - c, err := loader.Load(NewFetchDefinition(server.URL)) + c, err, statusCode := loader.Load(NewFetchDefinition(server.URL)) a.Error(err) a.Nil(c) a.Contains(err.Error(), "http 500") + assert.True(t, statusCode==500) } func Test_HttpContentLoader_LoadErrorNetwork(t *testing.T) { a := assert.New(t) loader := &HttpContentLoader{} - c, err := loader.Load(NewFetchDefinition("...")) + c, err, _ := loader.Load(NewFetchDefinition("...")) a.Error(err) a.Nil(c) a.Contains(err.Error(), "unsupported protocol scheme") diff --git a/composition/interface_mocks_test.go b/composition/interface_mocks_test.go index de2a534..d2cad75 100644 --- a/composition/interface_mocks_test.go +++ b/composition/interface_mocks_test.go @@ -62,11 +62,12 @@ func (_m *MockContentLoader) EXPECT() *_MockContentLoaderRecorder { return _m.recorder } -func (_m *MockContentLoader) Load(_param0 *FetchDefinition) (Content, error) { +func (_m *MockContentLoader) Load(_param0 *FetchDefinition) (Content, error, int) { ret := _m.ctrl.Call(_m, "Load", _param0) ret0, _ := ret[0].(Content) ret1, _ := ret[1].(error) - return ret0, ret1 + ret2, _ := ret[2].(int) + return ret0, ret1, ret2 } func (_mr *_MockContentLoaderRecorder) Load(arg0 interface{}) *gomock.Call { diff --git a/composition/interfaces.go b/composition/interfaces.go index 81fbeca..3aee994 100644 --- a/composition/interfaces.go +++ b/composition/interfaces.go @@ -15,7 +15,7 @@ type Fragment interface { type ContentLoader interface { // Load synchronously loads a content. // The loader has to ensure to return the call withing the supplied timeout. - Load(fd *FetchDefinition) (Content, error) + Load(fd *FetchDefinition) (Content, error, int) } type ContentParser interface { From f4bf7424870a6b568ffe3fff6683d7bc934b24c1 Mon Sep 17 00:00:00 2001 From: Dino Omanovic Date: Mon, 20 Jun 2016 16:15:27 +0200 Subject: [PATCH 07/17] Http status pass through: finished the passing through of status codes, missing some tests --- composition/composition_handler.go | 3 ++- composition/composition_handler_test.go | 3 ++- composition/content_fetcher.go | 3 ++- composition/content_fetcher_test.go | 10 +++++----- composition/fetch_definition.go | 4 ++-- composition/http_content_loader_test.go | 20 ++++++++++++++++++++ composition/interfaces.go | 4 ++-- 7 files changed, 35 insertions(+), 12 deletions(-) diff --git a/composition/composition_handler.go b/composition/composition_handler.go index 704bb57..71ee3dd 100644 --- a/composition/composition_handler.go +++ b/composition/composition_handler.go @@ -50,7 +50,8 @@ func (agg *CompositionHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) } else if res.Def.Required { log.WithField("fetchResult", res).Errorf("error loading content from: %v", res.Def.URL) - res.Def.ErrHandler.Handle(res.Err, w, r) + w.WriteHeader(res.HttpStatus) + res.Def.ErrHandler.Handle(res.Err, res.HttpStatus, w, r) return } else { log.WithField("fetchResult", res).Warnf("optional content not loaded: %v", res.Def.URL) diff --git a/composition/composition_handler_test.go b/composition/composition_handler_test.go index c1e2d31..f9affa7 100644 --- a/composition/composition_handler_test.go +++ b/composition/composition_handler_test.go @@ -175,6 +175,7 @@ func Test_CompositionHandler_ErrorInFetching(t *testing.T) { Def: NewFetchDefinition("/foo"), Content: nil, Err: errors.New(errorString), + HttpStatus: 502, }, } } @@ -184,7 +185,7 @@ func Test_CompositionHandler_ErrorInFetching(t *testing.T) { r, _ := http.NewRequest("GET", "http://example.com", nil) aggregator.ServeHTTP(resp, r) - a.Equal("Bad Gateway: "+errorString+"\n", string(resp.Body.Bytes())) + a.Equal("Error: "+errorString+"\n", string(resp.Body.Bytes())) a.Equal(502, resp.Code) } diff --git a/composition/content_fetcher.go b/composition/content_fetcher.go index 5a2915a..686d042 100644 --- a/composition/content_fetcher.go +++ b/composition/content_fetcher.go @@ -5,6 +5,7 @@ import ( "sync" "github.com/tarent/lib-compose/logging" + "net/http" ) // IsFetchable returns, whether the fetch definition refers to a fetchable resource @@ -73,7 +74,7 @@ func (fetcher *ContentFetcher) AddFetchJob(d *FetchDefinition) { fetcher.activeJobs.Add(1) - fetchResult := &FetchResult{Def: d, Hash: hash, Err: errors.New("not fetched")} + fetchResult := &FetchResult{Def: d, Hash: hash, Err: errors.New("not fetched"), HttpStatus:http.StatusBadGateway} fetcher.r.results = append(fetcher.r.results, fetchResult) go func() { diff --git a/composition/content_fetcher_test.go b/composition/content_fetcher_test.go index 0b58ed5..fffbc47 100644 --- a/composition/content_fetcher_test.go +++ b/composition/content_fetcher_test.go @@ -68,9 +68,9 @@ func Test_ContentFetcher_FetchingWithDependency(t *testing.T) { a := assert.New(t) loader := NewMockContentLoader(ctrl) - barFd := getFetchDefinitionMock(ctrl, loader, "/bar", nil, time.Millisecond*2, map[string]interface{}{"foo": "bar"}) - fooFd := getFetchDefinitionMock(ctrl, loader, "/foo", []*FetchDefinition{barFd}, time.Millisecond*2, map[string]interface{}{"bli": "bla"}) - bazzFd := getFetchDefinitionMock(ctrl, loader, "/bazz", []*FetchDefinition{barFd}, time.Millisecond, map[string]interface{}{}) + barFd := getFetchDefinitionMock(ctrl, loader, "/bar", nil, http.StatusOK, time.Millisecond*2, map[string]interface{}{"foo": "bar"}) + fooFd := getFetchDefinitionMock(ctrl, loader, "/foo", []*FetchDefinition{barFd}, http.StatusOK, time.Millisecond*2, map[string]interface{}{"bli": "bla"}) + bazzFd := getFetchDefinitionMock(ctrl, loader, "/bazz", []*FetchDefinition{barFd}, http.StatusOK, time.Millisecond, map[string]interface{}{}) fetcher := NewContentFetcher(nil) fetcher.contentLoader = loader @@ -91,7 +91,7 @@ func Test_ContentFetcher_FetchingWithDependency(t *testing.T) { a.Equal("bla", meta["bli"]) } -func getFetchDefinitionMock(ctrl *gomock.Controller, loaderMock *MockContentLoader, url string, requiredContent []*FetchDefinition, loaderBlocking time.Duration, metaJSON map[string]interface{}) *FetchDefinition { +func getFetchDefinitionMock(ctrl *gomock.Controller, loaderMock *MockContentLoader, url string, requiredContent []*FetchDefinition, requiredStatusCode int, loaderBlocking time.Duration, metaJSON map[string]interface{}) *FetchDefinition { fd := NewFetchDefinition(url) fd.Timeout = time.Second * 42 @@ -110,7 +110,7 @@ func getFetchDefinitionMock(ctrl *gomock.Controller, loaderMock *MockContentLoad func(fetchDefinition *FetchDefinition) { time.Sleep(loaderBlocking) }). - Return(content, nil) + Return(content, nil, requiredStatusCode) return fd } diff --git a/composition/fetch_definition.go b/composition/fetch_definition.go index d190eae..437d536 100644 --- a/composition/fetch_definition.go +++ b/composition/fetch_definition.go @@ -165,7 +165,7 @@ func NewDefaultErrorHandler() *DefaultErrorHandler { return deh } -func (der *DefaultErrorHandler) Handle(err error, w http.ResponseWriter, r *http.Request) { - http.Error(w, "Bad Gateway: " + err.Error(), 502) +func (der *DefaultErrorHandler) Handle(err error, status int, w http.ResponseWriter, r *http.Request) { + http.Error(w, "Error: " + err.Error(), status) } diff --git a/composition/http_content_loader_test.go b/composition/http_content_loader_test.go index 03859a1..1f20b5d 100644 --- a/composition/http_content_loader_test.go +++ b/composition/http_content_loader_test.go @@ -187,6 +187,26 @@ func Test_HttpContentLoader_LoadStream_No_Composition_Header(t *testing.T) { a.Equal("{}", string(body)) } +func Test_HttpContentLoader_Pass_404(t *testing.T) { + a := assert.New(t) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(404) + w.Write([]byte("{}")) + })) + + + defer server.Close() + + loader := &HttpContentLoader{} + c, err, status := loader.Load(NewFetchDefinition(server.URL)) + a.Error(err) + a.Nil(c) + a.Equal(404, status) +} + + + func Test_HttpContentLoader_LoadError500(t *testing.T) { a := assert.New(t) diff --git a/composition/interfaces.go b/composition/interfaces.go index 3aee994..2b3af51 100644 --- a/composition/interfaces.go +++ b/composition/interfaces.go @@ -15,7 +15,7 @@ type Fragment interface { type ContentLoader interface { // Load synchronously loads a content. // The loader has to ensure to return the call withing the supplied timeout. - Load(fd *FetchDefinition) (Content, error, int) + Load(fd *FetchDefinition) (content Content, err error, status int) } type ContentParser interface { @@ -87,5 +87,5 @@ type ResponseProcessor interface { type ErrorHandler interface { // handle http request errors - Handle(err error, w http.ResponseWriter, r *http.Request) + Handle(err error, status int, w http.ResponseWriter, r *http.Request) } From 6fd3c315ae53c1efd5e59c080252918a89c05e99 Mon Sep 17 00:00:00 2001 From: Dino Omanovic Date: Mon, 20 Jun 2016 18:32:25 +0200 Subject: [PATCH 08/17] Fixed merge problems --- composition/file_content_loader.go | 10 +++++----- composition/file_content_loader_test.go | 8 ++++---- composition/interfaces.go | 4 ++-- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/composition/file_content_loader.go b/composition/file_content_loader.go index 6751ca7..21b01d6 100644 --- a/composition/file_content_loader.go +++ b/composition/file_content_loader.go @@ -21,14 +21,14 @@ func NewFileContentLoader() *FileContentLoader { } } -func (loader *FileContentLoader) Load(fd *FetchDefinition) (Content, error) { +func (loader *FileContentLoader) Load(fd *FetchDefinition) (Content, error, int) { if fd.RespProc != nil { - return nil, ResponseProcessorsNotApplicable + return nil, ResponseProcessorsNotApplicable, 502 } filename := strings.TrimPrefix(fd.URL, FileURLPrefix) f, err := os.Open(filename) if err != nil { - return nil, fmt.Errorf("error opening file %v: %v", fd.URL, err) + return nil, fmt.Errorf("error opening file %v: %v", fd.URL, err), 502 } c := NewMemoryContent() @@ -43,9 +43,9 @@ func (loader *FileContentLoader) Load(fd *FetchDefinition) (Content, error) { WithField("duration", time.Since(parsingStart)). Debug("content parsing") f.Close() - return c, err + return c, err,c.httpStatusCode } c.reader = f - return c, nil + return c, nil, c.httpStatusCode } diff --git a/composition/file_content_loader_test.go b/composition/file_content_loader_test.go index 67c7824..e73284f 100644 --- a/composition/file_content_loader_test.go +++ b/composition/file_content_loader_test.go @@ -24,7 +24,7 @@ func Test_FileContentLoader_LoadHTML(t *testing.T) { a.NoError(err) loader := NewFileContentLoader() - c, err := loader.Load(NewFetchDefinition(FileURLPrefix + fileName)) + c, err, _ := loader.Load(NewFetchDefinition(FileURLPrefix + fileName)) a.NoError(err) a.NotNil(c) a.Nil(c.Reader()) @@ -42,7 +42,7 @@ func Test_FileContentLoader_LoadStream(t *testing.T) { a.NoError(err) loader := NewFileContentLoader() - c, err := loader.Load(NewFetchDefinition(FileURLPrefix + fileName)) + c, err, _ := loader.Load(NewFetchDefinition(FileURLPrefix + fileName)) a.NoError(err) a.NotNil(c) body, err := ioutil.ReadAll(c.Reader()) @@ -54,7 +54,7 @@ func Test_FileContentLoader_LoadError(t *testing.T) { a := assert.New(t) loader := NewFileContentLoader() - _, err := loader.Load(NewFetchDefinition("/tmp/some/non/existing/path")) + _, err, _ := loader.Load(NewFetchDefinition("/tmp/some/non/existing/path")) a.Error(err) } @@ -66,7 +66,7 @@ func Test_FileContentLoader_RequestProcessor(t *testing.T) { fd := NewFetchDefinition("/tmp/some/non/existing/path") fd.RespProc = NewMockResponseProcessor(ctrl) - _, err := NewFileContentLoader().Load(fd) + _, err, _ := NewFileContentLoader().Load(fd) a.Equal(ResponseProcessorsNotApplicable, err) } diff --git a/composition/interfaces.go b/composition/interfaces.go index 2be420f..c63f9db 100644 --- a/composition/interfaces.go +++ b/composition/interfaces.go @@ -1,8 +1,8 @@ package composition //go:generate go get github.com/golang/mock/mockgen -//go:generate mockgen -self_package composition -package composition -destination interface_mocks_test.go github.com/tarent/lib-compose/composition Fragment,ContentLoader,Content,ContentMerger,ContentParser,ResponseProcessor -//go:generate sed -ie "s/composition .github.com\\/tarent\\/lib-compose\\/composition.//g;s/composition\\.//g" interface_mocks_test.go +//go:generate mockgen -self_package composition -package composition -destination interface_mocks_test.go lib-compose/composition Fragment,ContentLoader,Content,ContentMerger,ContentParser,ResponseProcessor +//go:generate sed -ie "s/composition .lib-compose\\/composition.//g;s/composition\\.//g" interface_mocks_test.go import ( "io" "net/http" From ee1aad4113fe5f67d12973ed9e11e80889c38b37 Mon Sep 17 00:00:00 2001 From: Dino Omanovic Date: Mon, 20 Jun 2016 18:40:41 +0200 Subject: [PATCH 09/17] go fmt --- composition/composition_handler.go | 2 +- composition/composition_handler_test.go | 6 +++--- composition/content_fetcher.go | 12 ++++++------ composition/fetch_definition.go | 2 +- composition/fetch_definition_test.go | 2 +- composition/file_content_loader.go | 2 +- composition/http_content_loader.go | 3 +-- composition/http_content_loader_test.go | 5 +---- composition/interface_mocks_test.go | 2 +- composition_example/example_ui_service.go | 2 +- 10 files changed, 17 insertions(+), 21 deletions(-) diff --git a/composition/composition_handler.go b/composition/composition_handler.go index 55116f3..58ae1f8 100644 --- a/composition/composition_handler.go +++ b/composition/composition_handler.go @@ -51,7 +51,7 @@ func (agg *CompositionHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) } else if res.Def.Required { log.WithField("fetchResult", res).Errorf("error loading content from: %v", res.Def.URL) - res.Def.ErrHandler.Handle(res.Err, res.HttpStatus, w, r) + res.Def.ErrHandler.Handle(res.Err, res.HttpStatus, w, r) return } else { log.WithField("fetchResult", res).Warnf("optional content not loaded: %v", res.Def.URL) diff --git a/composition/composition_handler_test.go b/composition/composition_handler_test.go index 1cdd61d..1199f8b 100644 --- a/composition/composition_handler_test.go +++ b/composition/composition_handler_test.go @@ -173,9 +173,9 @@ func Test_CompositionHandler_ErrorInFetching(t *testing.T) { contentFetcherFactory := func(r *http.Request) FetchResultSupplier { return MockFetchResultSupplier{ &FetchResult{ - Def: NewFetchDefinition("/foo"), - Content: nil, - Err: errors.New(errorString), + Def: NewFetchDefinition("/foo"), + Content: nil, + Err: errors.New(errorString), HttpStatus: 502, }, } diff --git a/composition/content_fetcher.go b/composition/content_fetcher.go index c5e2339..39c7188 100644 --- a/composition/content_fetcher.go +++ b/composition/content_fetcher.go @@ -5,8 +5,8 @@ import ( "sync" "github.com/tarent/lib-compose/logging" - "strings" "net/http" + "strings" ) // IsFetchable returns, whether the fetch definition refers to a fetchable resource @@ -16,11 +16,11 @@ func (def *FetchDefinition) IsFetchable() bool { } type FetchResult struct { - Def *FetchDefinition - Err error - Content Content + Def *FetchDefinition + Err error + Content Content HttpStatus int - Hash string // the hash of the FetchDefinition + Hash string // the hash of the FetchDefinition } // ContentFetcher is a type, which can fetch a set of Content pages in parallel. @@ -77,7 +77,7 @@ func (fetcher *ContentFetcher) AddFetchJob(d *FetchDefinition) { fetcher.activeJobs.Add(1) - fetchResult := &FetchResult{Def: d, Hash: hash, Err: errors.New("not fetched"), HttpStatus:http.StatusBadGateway} + fetchResult := &FetchResult{Def: d, Hash: hash, Err: errors.New("not fetched"), HttpStatus: http.StatusBadGateway} fetcher.r.results = append(fetcher.r.results, fetchResult) go func() { diff --git a/composition/fetch_definition.go b/composition/fetch_definition.go index cfea68c..20efefd 100644 --- a/composition/fetch_definition.go +++ b/composition/fetch_definition.go @@ -168,5 +168,5 @@ func NewDefaultErrorHandler() *DefaultErrorHandler { } func (der *DefaultErrorHandler) Handle(err error, status int, w http.ResponseWriter, r *http.Request) { - http.Error(w, "Error: " + err.Error(), status) + http.Error(w, "Error: "+err.Error(), status) } diff --git a/composition/fetch_definition_test.go b/composition/fetch_definition_test.go index 954d003..ad4d6ac 100644 --- a/composition/fetch_definition_test.go +++ b/composition/fetch_definition_test.go @@ -41,5 +41,5 @@ func Test_FetchDefinition_use_DefaultErrorHandler_if_not_set(t *testing.T) { a := assert.New(t) fd := NewFetchDefinitionWithErrorHandler("http://upstream:8080/", nil) - a.Equal(NewDefaultErrorHandler(), fd.ErrHandler) + a.Equal(NewDefaultErrorHandler(), fd.ErrHandler) } diff --git a/composition/file_content_loader.go b/composition/file_content_loader.go index 21b01d6..230e280 100644 --- a/composition/file_content_loader.go +++ b/composition/file_content_loader.go @@ -43,7 +43,7 @@ func (loader *FileContentLoader) Load(fd *FetchDefinition) (Content, error, int) WithField("duration", time.Since(parsingStart)). Debug("content parsing") f.Close() - return c, err,c.httpStatusCode + return c, err, c.httpStatusCode } c.reader = f diff --git a/composition/http_content_loader.go b/composition/http_content_loader.go index 96804e5..4376e45 100644 --- a/composition/http_content_loader.go +++ b/composition/http_content_loader.go @@ -25,7 +25,7 @@ func NewHttpContentLoader() *HttpContentLoader { // TODO: Should we filter the headers, which we forward here, or is it correct to copy all of them? func (loader *HttpContentLoader) Load(fd *FetchDefinition) (Content, error, int) { client := &http.Client{Timeout: fd.Timeout} - statusCode:=502 + statusCode := 502 var err error request, err := http.NewRequest(fd.Method, fd.URL, fd.Body) @@ -46,7 +46,6 @@ func (loader *HttpContentLoader) Load(fd *FetchDefinition) (Content, error, int) statusCode = resp.StatusCode } - logging.Call(request, resp, start, err) if err != nil { return nil, err, statusCode diff --git a/composition/http_content_loader_test.go b/composition/http_content_loader_test.go index 552f497..2fed114 100644 --- a/composition/http_content_loader_test.go +++ b/composition/http_content_loader_test.go @@ -159,7 +159,6 @@ func Test_HttpContentLoader_Pass_404(t *testing.T) { w.Write([]byte("{}")) })) - defer server.Close() loader := &HttpContentLoader{} @@ -169,8 +168,6 @@ func Test_HttpContentLoader_Pass_404(t *testing.T) { a.Equal(404, status) } - - func Test_HttpContentLoader_LoadError500(t *testing.T) { a := assert.New(t) @@ -184,7 +181,7 @@ func Test_HttpContentLoader_LoadError500(t *testing.T) { a.Error(err) a.Nil(c) a.Contains(err.Error(), "http 500") - assert.True(t, statusCode==500) + assert.True(t, statusCode == 500) } func Test_HttpContentLoader_LoadErrorNetwork(t *testing.T) { diff --git a/composition/interface_mocks_test.go b/composition/interface_mocks_test.go index dc6ae68..4df027a 100644 --- a/composition/interface_mocks_test.go +++ b/composition/interface_mocks_test.go @@ -5,7 +5,7 @@ package composition import ( gomock "github.com/golang/mock/gomock" - + io "io" http "net/http" ) diff --git a/composition_example/example_ui_service.go b/composition_example/example_ui_service.go index 869a369..3520b5a 100644 --- a/composition_example/example_ui_service.go +++ b/composition_example/example_ui_service.go @@ -2,9 +2,9 @@ package main import ( gorilla "github.com/gorilla/handlers" + "github.com/tarent/lib-compose/composition" "net/http" "os" - "github.com/tarent/lib-compose/composition" ) func main() { From 7084fd180e6b630228a9881d96c0c03f6542c734 Mon Sep 17 00:00:00 2001 From: Sebastian Mancke Date: Mon, 20 Jun 2016 20:18:52 +0200 Subject: [PATCH 10/17] added http protocol to log entry --- logging/logger.go | 1 + logging/logger_test.go | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/logging/logger.go b/logging/logger.go index ad53504..baa2fc6 100644 --- a/logging/logger.go +++ b/logging/logger.go @@ -76,6 +76,7 @@ func access(r *http.Request, start time.Time, statusCode int, err error) *logrus "host": r.Host, "url": url, "method": r.Method, + "proto": r.Proto, "duration": time.Since(start).Nanoseconds() / 1000000, } diff --git a/logging/logger_test.go b/logging/logger_test.go index 292598b..43237a5 100644 --- a/logging/logger_test.go +++ b/logging/logger_test.go @@ -20,6 +20,7 @@ type logReccord struct { Host string `json:"host"` URL string `json:"url"` Method string `json:"method"` + Proto string `json:"proto"` Duration int `json:"duration"` ResponseStatus int `json:"response_status"` UserCorrelationId string `json:"user_correlation_id"` @@ -48,7 +49,7 @@ func Test_Logger_Set(t *testing.T) { a.Regexp(`^time.* level\=error msg\=oops foo\=bar.*`, b.String()) } -func Test_Logger_Access(t *testing.T) { +func Test_Logger_Call(t *testing.T) { a := assert.New(t) // given a logger @@ -112,7 +113,7 @@ func Test_Logger_Access(t *testing.T) { a.Equal("/foo?q=bar", data.URL) } -func Test_Logger_Call(t *testing.T) { +func Test_Logger_Access(t *testing.T) { a := assert.New(t) // given a logger @@ -145,6 +146,7 @@ func Test_Logger_Call(t *testing.T) { a.Equal("", data.Error) a.Equal("www.example.org", data.Host) a.Equal("GET", data.Method) + a.Equal("HTTP/1.1", data.Proto) a.Equal("201 GET /foo?...", data.Message) a.Equal("127.0.0.1", data.RemoteIp) a.Equal(201, data.ResponseStatus) From 87f66fba73a997af3f09c6ebbc5b801f46e30b2c Mon Sep 17 00:00:00 2001 From: Dino Omanovic Date: Tue, 21 Jun 2016 11:05:11 +0200 Subject: [PATCH 11/17] Implemented review changes (pass-http-status-codes PR) --- composition/composition_handler.go | 2 +- composition/composition_handler_test.go | 7 +++---- composition/content_fetcher.go | 16 +++++++-------- composition/content_fetcher_test.go | 10 +++++----- composition/file_content_loader.go | 10 +++++----- composition/file_content_loader_test.go | 8 ++++---- composition/http_content_loader.go | 26 ++++++++++++------------- composition/http_content_loader_test.go | 23 ++++++++++------------ composition/interface_mocks_test.go | 9 ++++----- composition/interfaces.go | 2 +- 10 files changed, 53 insertions(+), 60 deletions(-) diff --git a/composition/composition_handler.go b/composition/composition_handler.go index 58ae1f8..6dd444d 100644 --- a/composition/composition_handler.go +++ b/composition/composition_handler.go @@ -51,7 +51,7 @@ func (agg *CompositionHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) } else if res.Def.Required { log.WithField("fetchResult", res).Errorf("error loading content from: %v", res.Def.URL) - res.Def.ErrHandler.Handle(res.Err, res.HttpStatus, w, r) + res.Def.ErrHandler.Handle(res.Err, res.Content.HttpStatusCode(), w, r) return } else { log.WithField("fetchResult", res).Warnf("optional content not loaded: %v", res.Def.URL) diff --git a/composition/composition_handler_test.go b/composition/composition_handler_test.go index 1199f8b..5e43d50 100644 --- a/composition/composition_handler_test.go +++ b/composition/composition_handler_test.go @@ -173,10 +173,9 @@ func Test_CompositionHandler_ErrorInFetching(t *testing.T) { contentFetcherFactory := func(r *http.Request) FetchResultSupplier { return MockFetchResultSupplier{ &FetchResult{ - Def: NewFetchDefinition("/foo"), - Content: nil, - Err: errors.New(errorString), - HttpStatus: 502, + Def: NewFetchDefinition("/foo"), + Content: &MemoryContent{httpStatusCode: 502}, + Err: errors.New(errorString), }, } } diff --git a/composition/content_fetcher.go b/composition/content_fetcher.go index 39c7188..5a0bc09 100644 --- a/composition/content_fetcher.go +++ b/composition/content_fetcher.go @@ -5,7 +5,6 @@ import ( "sync" "github.com/tarent/lib-compose/logging" - "net/http" "strings" ) @@ -16,11 +15,10 @@ func (def *FetchDefinition) IsFetchable() bool { } type FetchResult struct { - Def *FetchDefinition - Err error - Content Content - HttpStatus int - Hash string // the hash of the FetchDefinition + Def *FetchDefinition + Err error + Content Content + Hash string // the hash of the FetchDefinition } // ContentFetcher is a type, which can fetch a set of Content pages in parallel. @@ -77,7 +75,7 @@ func (fetcher *ContentFetcher) AddFetchJob(d *FetchDefinition) { fetcher.activeJobs.Add(1) - fetchResult := &FetchResult{Def: d, Hash: hash, Err: errors.New("not fetched"), HttpStatus: http.StatusBadGateway} + fetchResult := &FetchResult{Def: d, Hash: hash, Err: errors.New("not fetched")} fetcher.r.results = append(fetcher.r.results, fetchResult) go func() { @@ -96,7 +94,7 @@ func (fetcher *ContentFetcher) AddFetchJob(d *FetchDefinition) { // want to override the original URL with expanded values definitionCopy := *d definitionCopy.URL = url - fetchResult.Content, fetchResult.Err, fetchResult.HttpStatus = fetcher.fetch(&definitionCopy) + fetchResult.Content, fetchResult.Err = fetcher.fetch(&definitionCopy) if fetchResult.Err == nil { fetcher.addMeta(fetchResult.Content.Meta()) @@ -114,7 +112,7 @@ func (fetcher *ContentFetcher) AddFetchJob(d *FetchDefinition) { }() } -func (fetcher *ContentFetcher) fetch(fd *FetchDefinition) (Content, error, int) { +func (fetcher *ContentFetcher) fetch(fd *FetchDefinition) (Content, error) { if strings.HasPrefix(fd.URL, FileURLPrefix) { return fetcher.fileContentLoader.Load(fd) } diff --git a/composition/content_fetcher_test.go b/composition/content_fetcher_test.go index 58596a6..e7c8043 100644 --- a/composition/content_fetcher_test.go +++ b/composition/content_fetcher_test.go @@ -68,9 +68,9 @@ func Test_ContentFetcher_FetchingWithDependency(t *testing.T) { a := assert.New(t) loader := NewMockContentLoader(ctrl) - barFd := getFetchDefinitionMock(ctrl, loader, "/bar", nil, http.StatusOK, time.Millisecond*2, map[string]interface{}{"foo": "bar"}) - fooFd := getFetchDefinitionMock(ctrl, loader, "/foo", []*FetchDefinition{barFd}, http.StatusOK, time.Millisecond*2, map[string]interface{}{"bli": "bla"}) - bazzFd := getFetchDefinitionMock(ctrl, loader, "/bazz", []*FetchDefinition{barFd}, http.StatusOK, time.Millisecond, map[string]interface{}{}) + barFd := getFetchDefinitionMock(ctrl, loader, "/bar", nil, time.Millisecond*2, map[string]interface{}{"foo": "bar"}) + fooFd := getFetchDefinitionMock(ctrl, loader, "/foo", []*FetchDefinition{barFd}, time.Millisecond*2, map[string]interface{}{"bli": "bla"}) + bazzFd := getFetchDefinitionMock(ctrl, loader, "/bazz", []*FetchDefinition{barFd}, time.Millisecond, map[string]interface{}{}) fetcher := NewContentFetcher(nil) fetcher.httpContentLoader = loader @@ -91,7 +91,7 @@ func Test_ContentFetcher_FetchingWithDependency(t *testing.T) { a.Equal("bla", meta["bli"]) } -func getFetchDefinitionMock(ctrl *gomock.Controller, loaderMock *MockContentLoader, url string, requiredContent []*FetchDefinition, requiredStatusCode int, loaderBlocking time.Duration, metaJSON map[string]interface{}) *FetchDefinition { +func getFetchDefinitionMock(ctrl *gomock.Controller, loaderMock *MockContentLoader, url string, requiredContent []*FetchDefinition, loaderBlocking time.Duration, metaJSON map[string]interface{}) *FetchDefinition { fd := NewFetchDefinition(url) fd.Timeout = time.Second * 42 @@ -110,7 +110,7 @@ func getFetchDefinitionMock(ctrl *gomock.Controller, loaderMock *MockContentLoad func(fetchDefinition *FetchDefinition) { time.Sleep(loaderBlocking) }). - Return(content, nil, requiredStatusCode) + Return(content, nil) return fd } diff --git a/composition/file_content_loader.go b/composition/file_content_loader.go index 230e280..6751ca7 100644 --- a/composition/file_content_loader.go +++ b/composition/file_content_loader.go @@ -21,14 +21,14 @@ func NewFileContentLoader() *FileContentLoader { } } -func (loader *FileContentLoader) Load(fd *FetchDefinition) (Content, error, int) { +func (loader *FileContentLoader) Load(fd *FetchDefinition) (Content, error) { if fd.RespProc != nil { - return nil, ResponseProcessorsNotApplicable, 502 + return nil, ResponseProcessorsNotApplicable } filename := strings.TrimPrefix(fd.URL, FileURLPrefix) f, err := os.Open(filename) if err != nil { - return nil, fmt.Errorf("error opening file %v: %v", fd.URL, err), 502 + return nil, fmt.Errorf("error opening file %v: %v", fd.URL, err) } c := NewMemoryContent() @@ -43,9 +43,9 @@ func (loader *FileContentLoader) Load(fd *FetchDefinition) (Content, error, int) WithField("duration", time.Since(parsingStart)). Debug("content parsing") f.Close() - return c, err, c.httpStatusCode + return c, err } c.reader = f - return c, nil, c.httpStatusCode + return c, nil } diff --git a/composition/file_content_loader_test.go b/composition/file_content_loader_test.go index e73284f..67c7824 100644 --- a/composition/file_content_loader_test.go +++ b/composition/file_content_loader_test.go @@ -24,7 +24,7 @@ func Test_FileContentLoader_LoadHTML(t *testing.T) { a.NoError(err) loader := NewFileContentLoader() - c, err, _ := loader.Load(NewFetchDefinition(FileURLPrefix + fileName)) + c, err := loader.Load(NewFetchDefinition(FileURLPrefix + fileName)) a.NoError(err) a.NotNil(c) a.Nil(c.Reader()) @@ -42,7 +42,7 @@ func Test_FileContentLoader_LoadStream(t *testing.T) { a.NoError(err) loader := NewFileContentLoader() - c, err, _ := loader.Load(NewFetchDefinition(FileURLPrefix + fileName)) + c, err := loader.Load(NewFetchDefinition(FileURLPrefix + fileName)) a.NoError(err) a.NotNil(c) body, err := ioutil.ReadAll(c.Reader()) @@ -54,7 +54,7 @@ func Test_FileContentLoader_LoadError(t *testing.T) { a := assert.New(t) loader := NewFileContentLoader() - _, err, _ := loader.Load(NewFetchDefinition("/tmp/some/non/existing/path")) + _, err := loader.Load(NewFetchDefinition("/tmp/some/non/existing/path")) a.Error(err) } @@ -66,7 +66,7 @@ func Test_FileContentLoader_RequestProcessor(t *testing.T) { fd := NewFetchDefinition("/tmp/some/non/existing/path") fd.RespProc = NewMockResponseProcessor(ctrl) - _, err, _ := NewFileContentLoader().Load(fd) + _, err := NewFileContentLoader().Load(fd) a.Equal(ResponseProcessorsNotApplicable, err) } diff --git a/composition/http_content_loader.go b/composition/http_content_loader.go index 4376e45..eca8ecc 100644 --- a/composition/http_content_loader.go +++ b/composition/http_content_loader.go @@ -23,14 +23,17 @@ func NewHttpContentLoader() *HttpContentLoader { } // TODO: Should we filter the headers, which we forward here, or is it correct to copy all of them? -func (loader *HttpContentLoader) Load(fd *FetchDefinition) (Content, error, int) { +func (loader *HttpContentLoader) Load(fd *FetchDefinition) (Content, error) { client := &http.Client{Timeout: fd.Timeout} - statusCode := 502 + + c := NewMemoryContent() + c.url = fd.URL + c.httpStatusCode = 502 var err error request, err := http.NewRequest(fd.Method, fd.URL, fd.Body) if err != nil { - return nil, err, 0 + return c, err } request.Header = fd.Header if request.Header == nil { @@ -43,29 +46,26 @@ func (loader *HttpContentLoader) Load(fd *FetchDefinition) (Content, error, int) resp, err := client.Do(request) if resp != nil { - statusCode = resp.StatusCode + c.httpStatusCode = resp.StatusCode } logging.Call(request, resp, start, err) if err != nil { - return nil, err, statusCode + return c, err } if fd.RespProc != nil { err = fd.RespProc.Process(resp, fd.URL) } if err != nil { - return nil, err, statusCode + return c, err } - if statusCode < 200 || statusCode > 399 { - return nil, fmt.Errorf("(http %v) on loading url %q", statusCode, fd.URL), statusCode + if c.httpStatusCode < 200 || c.httpStatusCode > 399 { + return c, fmt.Errorf("(http %v) on loading url %q", c.httpStatusCode, fd.URL) } - c := NewMemoryContent() - c.url = fd.URL c.httpHeader = resp.Header - c.httpStatusCode = statusCode // take the first parser for the content type // direct access to the map does not work, because the @@ -86,11 +86,11 @@ func (loader *HttpContentLoader) Load(fd *FetchDefinition) (Content, error, int) WithField("full_url", c.URL()). WithField("duration", time.Since(parsingStart)). Debug("content parsing") - return c, err, statusCode + return c, err } } } c.reader = resp.Body - return c, nil, statusCode + return c, nil } diff --git a/composition/http_content_loader_test.go b/composition/http_content_loader_test.go index 2fed114..aaa60b7 100644 --- a/composition/http_content_loader_test.go +++ b/composition/http_content_loader_test.go @@ -32,7 +32,7 @@ func Test_HttpContentLoader_Load(t *testing.T) { loader.parser["text/html"] = mockParser - c, err, _ := loader.Load(NewFetchDefinition(server.URL)) + c, err := loader.Load(NewFetchDefinition(server.URL)) a.NoError(err) a.NotNil(c) a.Nil(c.Reader()) @@ -64,7 +64,7 @@ func Test_HttpContentLoader_Load_ResponseProcessor(t *testing.T) { mockResponseProcessor := NewMockResponseProcessor(ctrl) mockResponseProcessor.EXPECT().Process(gomock.Any(), gomock.Any()) - c, err, _ := loader.Load(NewFetchDefinitionWithResponseProcessor(server.URL, mockResponseProcessor)) + c, err := loader.Load(NewFetchDefinitionWithResponseProcessor(server.URL, mockResponseProcessor)) a.NoError(err) a.NotNil(c) a.Nil(c.Reader()) @@ -105,7 +105,7 @@ func Test_HttpContentLoader_Load_POST(t *testing.T) { fd.Method = "POST" fd.Body = strings.NewReader("post content") - c, err, _ := loader.Load(fd) + c, err := loader.Load(fd) a.NoError(err) a.NotNil(c) a.Nil(c.Reader()) @@ -124,7 +124,7 @@ func Test_HttpContentLoader_LoadStream(t *testing.T) { defer server.Close() loader := &HttpContentLoader{} - c, err, _ := loader.Load(NewFetchDefinition(server.URL)) + c, err := loader.Load(NewFetchDefinition(server.URL)) a.NoError(err) a.NotNil(c.Reader()) body, err := ioutil.ReadAll(c.Reader()) @@ -143,7 +143,7 @@ func Test_HttpContentLoader_LoadStream_No_Composition_Header(t *testing.T) { defer server.Close() loader := &HttpContentLoader{} - c, err, _ := loader.Load(NewFetchDefinition(server.URL)) + c, err := loader.Load(NewFetchDefinition(server.URL)) a.NoError(err) a.NotNil(c.Reader()) body, err := ioutil.ReadAll(c.Reader()) @@ -162,10 +162,9 @@ func Test_HttpContentLoader_Pass_404(t *testing.T) { defer server.Close() loader := &HttpContentLoader{} - c, err, status := loader.Load(NewFetchDefinition(server.URL)) + c, err := loader.Load(NewFetchDefinition(server.URL)) a.Error(err) - a.Nil(c) - a.Equal(404, status) + a.Equal(404, c.HttpStatusCode()) } func Test_HttpContentLoader_LoadError500(t *testing.T) { @@ -177,20 +176,18 @@ func Test_HttpContentLoader_LoadError500(t *testing.T) { defer server.Close() loader := &HttpContentLoader{} - c, err, statusCode := loader.Load(NewFetchDefinition(server.URL)) + c, err := loader.Load(NewFetchDefinition(server.URL)) a.Error(err) - a.Nil(c) a.Contains(err.Error(), "http 500") - assert.True(t, statusCode == 500) + assert.True(t, c.HttpStatusCode() == 500) } func Test_HttpContentLoader_LoadErrorNetwork(t *testing.T) { a := assert.New(t) loader := &HttpContentLoader{} - c, err, _ := loader.Load(NewFetchDefinition("...")) + _, err := loader.Load(NewFetchDefinition("...")) a.Error(err) - a.Nil(c) a.Contains(err.Error(), "unsupported protocol scheme") } diff --git a/composition/interface_mocks_test.go b/composition/interface_mocks_test.go index 4df027a..17daf00 100644 --- a/composition/interface_mocks_test.go +++ b/composition/interface_mocks_test.go @@ -1,12 +1,12 @@ // Automatically generated by MockGen. DO NOT EDIT! -// Source: github.com/tarent/lib-compose/composition (interfaces: Fragment,ContentLoader,Content,ContentMerger,ContentParser,ResponseProcessor) +// Source: lib-compose/composition (interfaces: Fragment,ContentLoader,Content,ContentMerger,ContentParser,ResponseProcessor) package composition import ( gomock "github.com/golang/mock/gomock" - io "io" + http "net/http" ) @@ -62,12 +62,11 @@ func (_m *MockContentLoader) EXPECT() *_MockContentLoaderRecorder { return _m.recorder } -func (_m *MockContentLoader) Load(_param0 *FetchDefinition) (Content, error, int) { +func (_m *MockContentLoader) Load(_param0 *FetchDefinition) (Content, error) { ret := _m.ctrl.Call(_m, "Load", _param0) ret0, _ := ret[0].(Content) ret1, _ := ret[1].(error) - ret2, _ := ret[2].(int) - return ret0, ret1, ret2 + return ret0, ret1 } func (_mr *_MockContentLoaderRecorder) Load(arg0 interface{}) *gomock.Call { diff --git a/composition/interfaces.go b/composition/interfaces.go index c63f9db..252eab9 100644 --- a/composition/interfaces.go +++ b/composition/interfaces.go @@ -15,7 +15,7 @@ type Fragment interface { type ContentLoader interface { // Load synchronously loads a content. // The loader has to ensure to return the call withing the supplied timeout. - Load(fd *FetchDefinition) (content Content, err error, status int) + Load(fd *FetchDefinition) (content Content, err error) } type ContentParser interface { From 5b58477c7964ebaa8e8c08ccfff73d77bafe14a7 Mon Sep 17 00:00:00 2001 From: Dino Omanovic Date: Tue, 21 Jun 2016 17:06:11 +0200 Subject: [PATCH 12/17] Added X-Forwarded-Host header --- composition/fetch_definition.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/composition/fetch_definition.go b/composition/fetch_definition.go index 20efefd..1c46af8 100644 --- a/composition/fetch_definition.go +++ b/composition/fetch_definition.go @@ -25,7 +25,8 @@ var ForwardRequestHeaders = []string{ "If-Unmodified-Since", "Pragma", "Referer", - "Transfer-Encoding"} + "Transfer-Encoding", + "X-Forwarded-Host"} // ForwardResponseHeaders are those headers, // which are incuded from the servers backend response to the client. From 36b9ffd8606a55cf2d86fdb4a3e599e94b63fe75 Mon Sep 17 00:00:00 2001 From: Dino Omanovic Date: Tue, 21 Jun 2016 17:55:28 +0200 Subject: [PATCH 13/17] Move ForwardedHostHandler to lib-compose --- util/forwardedhosthandler.go | 23 ++++++++++++ util/forwardedhosthandler_test.go | 61 +++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+) create mode 100644 util/forwardedhosthandler.go create mode 100644 util/forwardedhosthandler_test.go diff --git a/util/forwardedhosthandler.go b/util/forwardedhosthandler.go new file mode 100644 index 0000000..4281305 --- /dev/null +++ b/util/forwardedhosthandler.go @@ -0,0 +1,23 @@ +package util + +import ( + "net/http" +) + +const X_FORWARDED_HOST_HEADER_KEY = "X-Forwarded-Host" + +type ForwardedHostHandler struct { + Next http.Handler +} + +func NewForwardedHostHandler(next http.Handler) *ForwardedHostHandler { + return &ForwardedHostHandler{Next: next} +} + +func (p *ForwardedHostHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + r.Header.Set(X_FORWARDED_HOST_HEADER_KEY, r.Host) + + if p.Next != nil { + p.Next.ServeHTTP(w, r) + } +} diff --git a/util/forwardedhosthandler_test.go b/util/forwardedhosthandler_test.go new file mode 100644 index 0000000..4962817 --- /dev/null +++ b/util/forwardedhosthandler_test.go @@ -0,0 +1,61 @@ +package util + +import ( + mockHttp "content-ui-service/mocks/net/http" + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" + "net/http" + "net/http/httptest" + "testing" +) + +func Test_DelegateIsCalled(t *testing.T) { + ctl := gomock.NewController(t) + defer ctl.Finish() + + mockDelegator := mockHttp.NewMockHandler(ctl) + fhh := NewForwardedHostHandler(mockDelegator) + + req, _ := http.NewRequest("GET", "", nil) + resp := httptest.NewRecorder() + + //expected the delegate should not been called + mockDelegator.EXPECT().ServeHTTP(gomock.Any(), gomock.Any()).Times(1) + + // when + fhh.ServeHTTP(resp, req) +} + +func Test_XForwardeHostHeaderIsSet(t *testing.T) { + ctl := gomock.NewController(t) + defer ctl.Finish() + + fhh := ForwardedHostHandler{} + + req, _ := http.NewRequest("GET", "", nil) + + expected := "testitest" + req.Host = expected + resp := httptest.NewRecorder() + + //when + fhh.ServeHTTP(resp, req) + assert.Contains(t, req.Header.Get(X_FORWARDED_HOST_HEADER_KEY), expected) +} + +func Test_OldXForwardedHostHeaderIsDropped(t *testing.T) { + ctl := gomock.NewController(t) + defer ctl.Finish() + + fhh := ForwardedHostHandler{} + + req, _ := http.NewRequest("GET", "", nil) + + notExpected := "nottestitest" + req.Header.Set(X_FORWARDED_HOST_HEADER_KEY, notExpected) + resp := httptest.NewRecorder() + + //when + fhh.ServeHTTP(resp, req) + assert.Equal(t, req.Header.Get(X_FORWARDED_HOST_HEADER_KEY), "") +} From 9af4254157fce2ed2819788fd5386b17b7671d3d Mon Sep 17 00:00:00 2001 From: Sebastian Mancke Date: Tue, 21 Jun 2016 18:05:10 +0200 Subject: [PATCH 14/17] distinguish in the log message between calls and access logs --- logging/log_middleware_test.go | 6 +++--- logging/logger.go | 8 ++++---- logging/logger_test.go | 8 ++++---- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/logging/log_middleware_test.go b/logging/log_middleware_test.go index f446b00..005192a 100644 --- a/logging/log_middleware_test.go +++ b/logging/log_middleware_test.go @@ -28,7 +28,7 @@ func Test_LogMiddleware_Panic(t *testing.T) { data := logRecordFromBuffer(b) a.Contains(data.Error, "logging.Test_LogMiddleware_Panic.func1") a.Contains(data.Error, "runtime error: index out of range") - a.Contains(data.Message, "ERROR GET /foo") + a.Contains(data.Message, "ERROR ->GET /foo") a.Equal(data.Level, "error") } @@ -50,7 +50,7 @@ func Test_LogMiddleware_Log_implicit200(t *testing.T) { data := logRecordFromBuffer(b) a.Equal("", data.Error) - a.Equal("200 GET /foo", data.Message) + a.Equal("200 ->GET /foo", data.Message) a.Equal(200, data.ResponseStatus) a.Equal("info", data.Level) } @@ -73,7 +73,7 @@ func Test_LogMiddleware_Log_404(t *testing.T) { data := logRecordFromBuffer(b) a.Equal("", data.Error) - a.Equal("404 GET /foo", data.Message) + a.Equal("404 ->GET /foo", data.Message) a.Equal(404, data.ResponseStatus) a.Equal("warning", data.Level) } diff --git a/logging/logger.go b/logging/logger.go index baa2fc6..191007c 100644 --- a/logging/logger.go +++ b/logging/logger.go @@ -44,9 +44,9 @@ func Access(r *http.Request, start time.Time, statusCode int) { var msg string if len(r.URL.RawQuery) == 0 { - msg = fmt.Sprintf("%v %v %v", statusCode, r.Method, r.URL.Path) + msg = fmt.Sprintf("%v ->%v %v", statusCode, r.Method, r.URL.Path) } else { - msg = fmt.Sprintf("%v %v %v?...", statusCode, r.Method, r.URL.Path) + msg = fmt.Sprintf("%v ->%v %v?...", statusCode, r.Method, r.URL.Path) } if statusCode >= 200 && statusCode <= 399 { @@ -61,7 +61,7 @@ func Access(r *http.Request, start time.Time, statusCode int) { // AccessError logs an error while accessing func AccessError(r *http.Request, start time.Time, err error) { e := access(r, start, 0, err) - e.Errorf("ERROR %v %v", r.Method, r.URL.Path) + e.Errorf("ERROR ->%v %v", r.Method, r.URL.Path) } func access(r *http.Request, start time.Time, statusCode int, err error) *logrus.Entry { @@ -131,7 +131,7 @@ func Call(r *http.Request, resp *http.Response, start time.Time, err error) { fields["response_status"] = resp.StatusCode fields["content_type"] = resp.Header.Get("Content-Type") e := Logger.WithFields(fields) - msg := fmt.Sprintf("%v %v %v", resp.StatusCode, r.Method, r.URL.String()) + msg := fmt.Sprintf("%v %v-> %v", resp.StatusCode, r.Method, r.URL.String()) if resp.StatusCode >= 200 && resp.StatusCode <= 399 { e.Info(msg) diff --git a/logging/logger_test.go b/logging/logger_test.go index 43237a5..c970166 100644 --- a/logging/logger_test.go +++ b/logging/logger_test.go @@ -86,7 +86,7 @@ func Test_Logger_Call(t *testing.T) { a.Equal("", data.Error) a.Equal("www.example.org", data.Host) a.Equal("GET", data.Method) - a.Equal("404 GET http://www.example.org/foo?q=bar", data.Message) + a.Equal("404 GET-> http://www.example.org/foo?q=bar", data.Message) a.Equal(404, data.ResponseStatus) a.Equal("call", data.Type) a.Equal("/foo?q=bar", data.URL) @@ -147,7 +147,7 @@ func Test_Logger_Access(t *testing.T) { a.Equal("www.example.org", data.Host) a.Equal("GET", data.Method) a.Equal("HTTP/1.1", data.Proto) - a.Equal("201 GET /foo?...", data.Message) + a.Equal("201 ->GET /foo?...", data.Message) a.Equal("127.0.0.1", data.RemoteIp) a.Equal(201, data.ResponseStatus) a.Equal("access", data.Type) @@ -170,7 +170,7 @@ func Test_Logger_Access_ErrorCases(t *testing.T) { // then: all fields match data := logRecordFromBuffer(b) a.Equal("warning", data.Level) - a.Equal("404 GET /foo", data.Message) + a.Equal("404 ->GET /foo", data.Message) // when a status 500 is logged b.Reset() @@ -186,7 +186,7 @@ func Test_Logger_Access_ErrorCases(t *testing.T) { data = logRecordFromBuffer(b) a.Equal("error", data.Level) a.Equal("oops", data.Error) - a.Equal("ERROR GET /foo", data.Message) + a.Equal("ERROR ->GET /foo", data.Message) } func Test_Logger_Application(t *testing.T) { From c1a4012585b2866a166ff7b8db92bf91fa3f7718 Mon Sep 17 00:00:00 2001 From: Dino Omanovic Date: Tue, 21 Jun 2016 18:10:58 +0200 Subject: [PATCH 15/17] Fixed failing forwardedhandler tests --- composition/interface_mocks_test.go | 1 - composition/interfaces.go | 2 ++ composition/mocks/net/http/mock_http.go | 38 +++++++++++++++++++++++++ scripts/mockgen.sh | 30 +++++++++++++++++++ util/forwardedhosthandler_test.go | 2 +- 5 files changed, 71 insertions(+), 2 deletions(-) create mode 100644 composition/mocks/net/http/mock_http.go create mode 100644 scripts/mockgen.sh diff --git a/composition/interface_mocks_test.go b/composition/interface_mocks_test.go index 17daf00..7a658f2 100644 --- a/composition/interface_mocks_test.go +++ b/composition/interface_mocks_test.go @@ -6,7 +6,6 @@ package composition import ( gomock "github.com/golang/mock/gomock" io "io" - http "net/http" ) diff --git a/composition/interfaces.go b/composition/interfaces.go index 252eab9..347941d 100644 --- a/composition/interfaces.go +++ b/composition/interfaces.go @@ -3,6 +3,8 @@ package composition //go:generate go get github.com/golang/mock/mockgen //go:generate mockgen -self_package composition -package composition -destination interface_mocks_test.go lib-compose/composition Fragment,ContentLoader,Content,ContentMerger,ContentParser,ResponseProcessor //go:generate sed -ie "s/composition .lib-compose\\/composition.//g;s/composition\\.//g" interface_mocks_test.go +//go:generate sh ../scripts/mockgen.sh + import ( "io" "net/http" diff --git a/composition/mocks/net/http/mock_http.go b/composition/mocks/net/http/mock_http.go new file mode 100644 index 0000000..baa0111 --- /dev/null +++ b/composition/mocks/net/http/mock_http.go @@ -0,0 +1,38 @@ +// Automatically generated by MockGen. DO NOT EDIT! +// Source: net/http (interfaces: Handler) + +package http + +import ( + gomock "github.com/golang/mock/gomock" + http "net/http" +) + +// Mock of Handler interface +type MockHandler struct { + ctrl *gomock.Controller + recorder *_MockHandlerRecorder +} + +// Recorder for MockHandler (not exported) +type _MockHandlerRecorder struct { + mock *MockHandler +} + +func NewMockHandler(ctrl *gomock.Controller) *MockHandler { + mock := &MockHandler{ctrl: ctrl} + mock.recorder = &_MockHandlerRecorder{mock} + return mock +} + +func (_m *MockHandler) EXPECT() *_MockHandlerRecorder { + return _m.recorder +} + +func (_m *MockHandler) ServeHTTP(_param0 http.ResponseWriter, _param1 *http.Request) { + _m.ctrl.Call(_m, "ServeHTTP", _param0, _param1) +} + +func (_mr *_MockHandlerRecorder) ServeHTTP(arg0, arg1 interface{}) *gomock.Call { + return _mr.mock.ctrl.RecordCall(_mr.mock, "ServeHTTP", arg0, arg1) +} diff --git a/scripts/mockgen.sh b/scripts/mockgen.sh new file mode 100644 index 0000000..a6d5603 --- /dev/null +++ b/scripts/mockgen.sh @@ -0,0 +1,30 @@ +#!/bin/bash + +# should be in your path +MOCKGEN=mockgen +SED=sed +GOFMT=gofmt +MKDIR=mkdir + +generate_mock() { + SRC=$1 + PKG=$(dirname $SRC) + DST=$PKG/mock_$(basename $SRC) + + $MKDIR -p $(dirname $DST) + $MOCKGEN -source ./$SRC -destination ./$DST -package $(basename $PKG) + $GOFMT -w ./$DST +} + +generate_vendor_mock() { + PKG=$1 + INTERFACES=$2 + DST=mocks/$PKG/mock_$(basename $PKG).go + + $MKDIR -p $(dirname $DST) + $MOCKGEN -destination ./$DST -package $(basename $PKG) $PKG $INTERFACES + $GOFMT -w ./$DST +} + +# generate vendor mocks +generate_vendor_mock net/http Handler \ No newline at end of file diff --git a/util/forwardedhosthandler_test.go b/util/forwardedhosthandler_test.go index 4962817..1ffec21 100644 --- a/util/forwardedhosthandler_test.go +++ b/util/forwardedhosthandler_test.go @@ -1,9 +1,9 @@ package util import ( - mockHttp "content-ui-service/mocks/net/http" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" + mockHttp "lib-compose/composition/mocks/net/http" "net/http" "net/http/httptest" "testing" From efeac40b0cda0c16cb0100f760da1aeddf957b2a Mon Sep 17 00:00:00 2001 From: Sebastian Mancke Date: Tue, 21 Jun 2016 18:20:10 +0200 Subject: [PATCH 16/17] distinguish in the log message between calls and access logs --- composition/composition_handler_test.go | 3 ++- composition/content_merge.go | 2 +- composition/content_merge_test.go | 3 ++- composition_example/expected_test_result.html | 1 + 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/composition/composition_handler_test.go b/composition/composition_handler_test.go index 5e43d50..f01d18e 100644 --- a/composition/composition_handler_test.go +++ b/composition/composition_handler_test.go @@ -36,7 +36,8 @@ func Test_CompositionHandler_PositiveCase(t *testing.T) { r, _ := http.NewRequest("GET", "http://example.com", nil) ch.ServeHTTP(resp, r) - expected := ` + expected := ` + diff --git a/composition/content_merge.go b/composition/content_merge.go index 0c27911..0680778 100644 --- a/composition/content_merge.go +++ b/composition/content_merge.go @@ -47,7 +47,7 @@ func (cntx *ContentMerge) GetHtml() ([]byte, error) { return f.Execute(w, cntx.MetaJSON, executeFragment) } - io.WriteString(w, "\n \n ") + io.WriteString(w, "\n\n \n ") for _, f := range cntx.Head { if err := f.Execute(w, cntx.MetaJSON, executeFragment); err != nil { diff --git a/composition/content_merge_test.go b/composition/content_merge_test.go index 9360576..a8146ae 100644 --- a/composition/content_merge_test.go +++ b/composition/content_merge_test.go @@ -10,7 +10,8 @@ import ( func Test_ContentMerge_PositiveCase(t *testing.T) { a := assert.New(t) - expected := ` + expected := ` + diff --git a/composition_example/expected_test_result.html b/composition_example/expected_test_result.html index 84cdd12..8efd0c1 100644 --- a/composition_example/expected_test_result.html +++ b/composition_example/expected_test_result.html @@ -1,3 +1,4 @@ + From 492afb260a441c65ba8c3d1ee75ec591c7e81ea7 Mon Sep 17 00:00:00 2001 From: Dino Omanovic Date: Tue, 21 Jun 2016 18:21:23 +0200 Subject: [PATCH 17/17] Fixed import path for httphandler mock --- util/forwardedhosthandler_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/forwardedhosthandler_test.go b/util/forwardedhosthandler_test.go index 1ffec21..e1a45b4 100644 --- a/util/forwardedhosthandler_test.go +++ b/util/forwardedhosthandler_test.go @@ -3,7 +3,7 @@ package util import ( "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" - mockHttp "lib-compose/composition/mocks/net/http" + mockHttp "github.com/tarent/lib-compose/composition/mocks/net/http" "net/http" "net/http/httptest" "testing"