Merge 1b69be0 into 6e3b190

qvest-digital · Jun 19, 2016 · 807b6b1 · 807b6b1
2 parents 6e3b190 + 1b69be0
commit 807b6b1
Show file tree

Hide file tree

Showing 7 changed files with 38 additions and 85 deletions.
diff --git 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
@@ -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
@@ -35,20 +35,9 @@ 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) {
+	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]
@@ -58,48 +47,38 @@ func (cntx *ContentMerge) WriteHtmlUnbuffered(w io.Writer) error {
 		return f.Execute(w, cntx.MetaJSON, executeFragment)
 	}
 
-	if _, err := io.WriteString(w, "<html>\n  <head>\n    "); err != nil {
-		return err
-	}
+	io.WriteString(w, "<html>\n  <head>\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  </head>\n  <body"); err != nil {
-		return err
-	}
+	io.WriteString(w, "\n  </head>\n  <body")
 
 	for _, f := range cntx.BodyAttrs {
-		if _, err := io.WriteString(w, " "); err != nil {
-			return err
-		}
+		io.WriteString(w, " ")
 
 		if err := f.Execute(w, cntx.MetaJSON, executeFragment); err != nil {
-			return err
+			return nil, err
 		}
 	}
 
-	if _, err := io.WriteString(w, ">\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  </body>\n</html>\n"); err != nil {
-		return err
-	}
+	io.WriteString(w, "\n  </body>\n</html>\n")
 
-	return nil
+	return w.Bytes(), nil
 }
 
 func (cntx *ContentMerge) AddContent(fetchResult *FetchResult) {

diff --git 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
@@ -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
@@ -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 {

diff --git 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