Merge branch 'master' into overwork-content-merge

Resolved conflicts in test files.

.gitignore

@@ -1 +1,2 @@
-cover.out
+cover.out
+/.idea

cache/cache.go

@@ -49,7 +49,7 @@ func NewCache(name string, maxEntries int, maxSizeMB int, maxAge time.Duration)
 	return c
 }
 
-// LogEvery Start a Goroutine, which logs statisitcs periodically.
+// LogEvery Start a Goroutine, which logs statistics periodically.
 func (c *Cache) LogEvery(d time.Duration) {
 	go c.logEvery(d)
 }

cache/cache_strategy.go

@@ -47,7 +47,7 @@ const (
 	ReasonResponseUncachableByDefault = cacheobject.ReasonResponseUncachableByDefault
 )
 
-var DefaultIncludeHeaders = []string{"Authorization", "Accept-Encoding"}
+var DefaultIncludeHeaders = []string{"Authorization", "Accept-Encoding", "Host"}
 
 var DefaultCacheStrategy = NewCacheStrategyWithDefault()
 
@@ -73,12 +73,12 @@ func NewCacheStrategy(includeHeaders []string, includeCookies []string, ignoreRe
 	}
 }
 
-// Hash computes a hash value based in the url, the method and selected header and cookien attributes,
+// Hash computes a hash value based on the url, the method and selected header and cookie attributes.
 func (tcs *CacheStrategy) Hash(method string, url string, requestHeader http.Header) string {
 	return tcs.HashWithParameters(method, url, requestHeader, tcs.includeHeaders, tcs.includeCookies)
 }
 
-// Hash computes a hash value based in the url, the method and selected header and cookien attributes,
+// Hash computes a hash value based on the url, the method and selected header and cookie attributes.
 func (tcs *CacheStrategy) HashWithParameters(method string, url string, requestHeader http.Header, includeHeaders []string, includeCookies []string) string {
 	hasher := md5.New()
 
@@ -99,7 +99,8 @@ func (tcs *CacheStrategy) HashWithParameters(method string, url string, requestH
 		}
 	}
 
-	return hex.EncodeToString(hasher.Sum(nil))
+	hash := hex.EncodeToString(hasher.Sum(nil))
+	return hash
 }
 
 func (tcs *CacheStrategy) IsCacheable(method string, url string, statusCode int, requestHeader http.Header, responseHeader http.Header) bool {

composition/composition_handler.go

@@ -41,6 +41,11 @@ func NewCompositionHandlerWithCache(contentFetcherFactory ContentFetcherFactory,
 }
 
 func (agg *CompositionHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	// If we know the host but don't have the Host header [any more] then we
+	// set [or restore] the header, because why would You just remove it!?:
+	if (r.Host != "") && (r.Header.Get("Host") == "") {
+		r.Header.Set("Host", r.Host)
+	}
 
 	fetcher := agg.contentFetcherFactory(r)
 

composition/composition_handler_test.go

@@ -126,7 +126,7 @@ func Test_CompositionHandler_CorrectHeaderAndStatusCodeReturned(t *testing.T) {
 	ch.ServeHTTP(resp, r)
 
 	a.Equal(200, resp.Code)
-	a.Equal(3, len(resp.Header())) // Set-Cookie + Content-Type + Content-Lenth
+	a.Equal(3, len(resp.Header())) // Set-Cookie + Content-Type + Content-Length
 	a.Equal("", resp.Header().Get("Transfer-Encoding"))
 	a.Contains(resp.Header()["Set-Cookie"], "cookie-content 1")
 	a.Contains(resp.Header()["Set-Cookie"], "cookie-content 2")
@@ -479,6 +479,42 @@ func Test_getBaseUrlFromRequest(t *testing.T) {
 	}
 }
 
+
+// Jira 3946: go deletes the "Host" header from the request (for whatever reasons):
+// https://golang.org/src/net/http/request.go:
+//   123		// For incoming requests, the Host header is promoted to the
+//   124		// Request.Host field and removed from the Header map.
+// But our cache strategies might want this header in order to generate different
+// cache-IDs (hashes) for different host names (e.g. preview-mode-whatever.de vs. production-whatever.de).
+func Test_CompositionHandler_RestoreHostHeader(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+	a := assert.New(t)
+
+	contentFetcherFactory := func(r *http.Request) FetchResultSupplier {
+		return MockFetchResultSupplier{
+			&FetchResult{
+				Def: NewFetchDefinition("/foo"),
+				Content: &MemoryContent{
+					body: map[string]Fragment{
+						"": StringFragment("Hello World\n"),
+					},
+				},
+			},
+		}
+	}
+	ch := NewCompositionHandler(ContentFetcherFactory(contentFetcherFactory))
+
+	resp := httptest.NewRecorder()
+	r, _ := http.NewRequest("GET", "/", nil)
+	r.Host = "MyHost"
+	ch.ServeHTTP(resp, r)
+
+	a.Equal(200, resp.Code)
+	a.Equal("MyHost", r.Header.Get("Host"))
+}
+
+
 type MockFetchResultSupplier []*FetchResult
 
 func (m MockFetchResultSupplier) WaitForResults() []*FetchResult {

composition/content_fetcher.go

@@ -62,7 +62,7 @@ func NewContentFetcher(defaultMetaJSON map[string]interface{}) *ContentFetcher {
 }
 
 // Wait blocks until all jobs are done,
-// eighter sucessful or with an error result and returns the content and errors.
+// either successful or with an error result and returns the content and errors.
 // Do we need to return the Results in a special order????
 func (fetcher *ContentFetcher) WaitForResults() []*FetchResult {
 	fetcher.activeJobs.Wait()
@@ -72,21 +72,21 @@ func (fetcher *ContentFetcher) WaitForResults() []*FetchResult {
 
 	results := fetcher.r.results
 
-	//to keep initial order if no priority settings are given, do a check before for sorting
+	// To keep initial order if no priority settings are given, do a check before for sorting.
 	if hasPrioritySetting(results) {
 		sort.Sort(FetchResults(results))
 	}
 
 	return results
 }
 
-// AddFetchJob addes one job to the fetcher and recursively adds the dependencies also.
+// AddFetchJob adds one job to the fetcher and recursively adds the dependencies also.
 func (fetcher *ContentFetcher) AddFetchJob(d *FetchDefinition) {
 	fetcher.r.mutex.Lock()
 	defer fetcher.r.mutex.Unlock()
 
 	hash := d.Hash()
-	if fetcher.isAlreadySheduled(hash) {
+	if fetcher.isAlreadyScheduled(hash) {
 		return
 	}
 
@@ -107,8 +107,8 @@ func (fetcher *ContentFetcher) AddFetchJob(d *FetchDefinition) {
 			return
 		}
 
-		// create a copy of the fetch definition, to because we do not
-		// want to override the original URL with expanded values
+		// Create a copy of the fetch definition, to because we do not
+		// want to override the original URL with expanded values.
 		definitionCopy := *d
 		definitionCopy.URL = url
 		fetchResult.Content, fetchResult.Err = fetcher.Loader.Load(&definitionCopy)
@@ -128,7 +128,6 @@ func (fetcher *ContentFetcher) AddFetchJob(d *FetchDefinition) {
 					WithField("correlation_id", logging.GetCorrelationId(definitionCopy.Header)).
 					Errorf("failed fetching %v", d.URL)
 			}
-
 		}
 	}()
 }
@@ -139,9 +138,9 @@ func (fetcher *ContentFetcher) Empty() bool {
 	return len(fetcher.r.results) == 0
 }
 
-// isAlreadySheduled checks, if there is already a job for a FetchDefinition, or it is already fetched.
+// isAlreadyScheduled checks if there is already a job for a FetchDefinition, or it is already fetched.
 // The method has to be called in a locked mutex block.
-func (fetcher *ContentFetcher) isAlreadySheduled(fetchDefinitionHash string) bool {
+func (fetcher *ContentFetcher) isAlreadyScheduled(fetchDefinitionHash string) bool {
 	for _, fetchResult := range fetcher.r.results {
 		if fetchDefinitionHash == fetchResult.Hash {
 			return true

composition/content_fetcher_test.go

@@ -80,8 +80,8 @@ func Test_ContentFetchResultPrioritySort(t *testing.T) {
         a := assert.New(t)
 
         barFd := NewFetchDefinitionWithPriority("/bar", 30)
-        fooFd := NewFetchDefinitionWithPriority("/foo", 10)
-        bazzFd := NewFetchDefinitionWithPriority("/bazz", 5)
+        fooFd := NewFetchDefinitionWithPriority("/foo",  10)
+        bazzFd := NewFetchDefinitionWithPriority("/bazz",  5)
 
         results := []*FetchResult{{Def: barFd}, {Def: fooFd}, {Def: bazzFd}}
 

composition/content_merge_test.go

@@ -61,7 +61,7 @@ func Test_ContentMerge_PositiveCase(t *testing.T) {
 		head: StringFragment("    <page3-head/>"),
 		body: map[string]Fragment{
 			"": StringFragment("<page3-body-a/>"),
-		}}, 0)
+		}}, MAX_PRIORITY) // just to trigger the priority-parsing and see that it doesn't crash..
 
 	html, err := cm.GetHtml()
 	a.NoError(err)

composition/fetch_definition.go

@@ -12,7 +12,7 @@ import (
 const MAX_PRIORITY int = 4294967295
 
 // ForwardRequestHeaders are those headers,
-// which are incuded from the original client request to the backend request.
+// which are included from the original client request to the backend request.
 // TODO: Add Host header to an XFF header
 var ForwardRequestHeaders = []string{
 	"Authorization",
@@ -31,10 +31,11 @@ var ForwardRequestHeaders = []string{
 	"X-Forwarded-Host",
 	"X-Correlation-Id",
 	"X-Feature-Toggle",
+	"Host",
 }
 
 // ForwardResponseHeaders are those headers,
-// which are incuded from the servers backend response to the client.
+// which are included from the servers backend response to the client.
 var ForwardResponseHeaders = []string{
 	"Age",
 	"Allow",
@@ -77,50 +78,42 @@ type FetchDefinition struct {
 	Priority               int
 }
 
-// Creates a fetch definition
-func NewFetchDefinition(url string) *FetchDefinition {
-	return NewFetchDefinitionWithResponseProcessorAndPriority(url, nil, DefaultPriority)
-}
-
-func NewFetchDefinitionWithPriority(url string, priority int) *FetchDefinition {
-	return NewFetchDefinitionWithResponseProcessorAndPriority(url, nil, priority)
-}
+// All the FetchDefinitions, that get passed a request, will forward the ForwardRequestHeaders. But only
+// those that are *FromRequest will also append the original requests other information, like method, body, URL.
 
-func NewFetchDefinitionWithErrorHandler(url string, errHandler ErrorHandler) *FetchDefinition {
-	return NewFetchDefinitionWithErrorHandlerAndPriority(url, errHandler, DefaultPriority)
-}
 
-func NewFetchDefinitionWithErrorHandlerAndPriority(url string, errHandler ErrorHandler, priority int) *FetchDefinition {
-	if errHandler == nil {
-		errHandler = NewDefaultErrorHandler()
-	}
+// Creates a fetch definition (warning: this one will not forward any request headers).
+func NewFetchDefinition(url string) *FetchDefinition {
 	return &FetchDefinition{
 		Name:            urlToName(url), // the name defauls to the url
 		URL:             url,
 		Timeout:         DefaultTimeout,
 		FollowRedirects: false,
 		Required:        true,
 		Method:          "GET",
-		ErrHandler:      errHandler,
+		ErrHandler:      NewDefaultErrorHandler(),
 		CacheStrategy:   cache.DefaultCacheStrategy,
-		Priority:        priority,
+		Priority:        DefaultPriority,
 	}
 }
 
-// If a ResponseProcessor-Implementation is given it can be used to change the response before composition
-func NewFetchDefinitionWithResponseProcessor(url string, rp ResponseProcessor) *FetchDefinition {
-	return NewFetchDefinitionWithResponseProcessorAndPriority(url, rp, DefaultPriority)
+func NewFetchDefinitionWithPriority(url string, priority int) *FetchDefinition {
+	fd := NewFetchDefinition(url)
+	fd.Priority = priority
+	return fd
 }
 
+
 // If a ResponseProcessor-Implementation is given it can be used to change the response before composition
 // Priority is used to determine which property from which head has to be taken by collision of multiple fetches
-func NewFetchDefinitionWithResponseProcessorAndPriority(url string, rp ResponseProcessor, priority int) *FetchDefinition {
+func NewFetchDefinitionWithResponseProcessorAndPriority(url string, rp ResponseProcessor, r *http.Request, priority int) *FetchDefinition {
 	return &FetchDefinition{
 		Name:            urlToName(url), // the name defauls to the url
 		URL:             url,
 		Timeout:         DefaultTimeout,
 		FollowRedirects: false,
 		Required:        true,
+		Header:          copyHeaders(r.Header, nil, ForwardRequestHeaders),
 		Method:          "GET",
 		RespProc:        rp,
 		ErrHandler:      NewDefaultErrorHandler(),
@@ -220,6 +213,7 @@ func copyHeaders(src, dest http.Header, whitelist []string) http.Header {
 			dest.Add(k, v)
 		}
 	}
+
 	return dest
 }
 

composition/fetch_definition_test.go

@@ -7,6 +7,7 @@ import (
 	"strings"
 	"testing"
 	"time"
+	"net/url"
 )
 
 func Test_FetchDefinition_NewFetchDefinitionFromRequest(t *testing.T) {
@@ -21,6 +22,7 @@ func Test_FetchDefinition_NewFetchDefinitionFromRequest(t *testing.T) {
 		"Cookie":          {"aa=bb;"},
 		"X-Feature-Toggle": {"true"},
 		"Accept-Encoding": {"gzip"}, // should not be copied
+		"X-Correlation-Id": {"foobar123"},
 	}
 
 	fd := NewFetchDefinitionFromRequest("http://upstream:8080/", r)
@@ -32,6 +34,7 @@ func Test_FetchDefinition_NewFetchDefinitionFromRequest(t *testing.T) {
 	a.Equal("aa=bb;", fd.Header.Get("Cookie"))
 	a.Equal("true", fd.Header.Get("X-Feature-Toggle"))
 	a.Equal("", fd.Header.Get("Accept-Encoding"))
+	a.Equal("foobar123", fd.Header.Get("X-Correlation-Id"))
 
 	a.Equal("POST", fd.Method)
 	b, err := ioutil.ReadAll(fd.Body)
@@ -42,16 +45,19 @@ func Test_FetchDefinition_NewFetchDefinitionFromRequest(t *testing.T) {
 func Test_FetchDefinition_use_DefaultErrorHandler_if_not_set(t *testing.T) {
 	a := assert.New(t)
 
-	fd := NewFetchDefinitionWithErrorHandler("http://upstream:8080/", nil)
+	fd := NewFetchDefinition("http://upstream:8080/")
 	a.Equal(NewDefaultErrorHandler(), fd.ErrHandler)
 }
 
 
 func Test_FetchDefinition_NewFunctions_have_default_priority(t *testing.T) {
 	a := assert.New(t)
+	request := &http.Request{}
+	request.URL = &url.URL{}
+
 	fd1 := NewFetchDefinition("foo")
-	fd2 := NewFetchDefinitionWithErrorHandler("baa", nil)
-	fd3 := NewFetchDefinitionWithResponseProcessor("blub", nil)
+	fd2 := NewFetchDefinitionFromRequest("baa", request)
+	fd3 := NewFetchDefinitionWithResponseProcessorFromRequest("blub", request, nil)
 
 	r, err := http.NewRequest("POST", "https://example.com/content?foo=bar", nil)
 	a.NoError(err)
@@ -67,9 +73,12 @@ func Test_FetchDefinition_NewFunctions_have_default_priority(t *testing.T) {
 
 func Test_FetchDefinition_NewFunctions_have_parameter_priority(t *testing.T) {
 	a := assert.New(t)
+	request := &http.Request{}
+	request.URL = &url.URL{}
+
 	fd1 := NewFetchDefinitionWithPriority("foo", 42)
-	fd2 := NewFetchDefinitionWithErrorHandlerAndPriority("baa", nil, 54)
-	fd3 := NewFetchDefinitionWithResponseProcessorAndPriority("blub", nil, 74)
+	fd2 := NewFetchDefinitionWithPriorityFromRequest("baa", request, 54)
+	fd3 := NewFetchDefinitionWithResponseProcessorAndPriorityFromRequest("blub", request, nil, 74)
 
 
 	r, err := http.NewRequest("POST", "https://example.com/content?foo=bar", nil)

composition/http_content_loader_test.go

@@ -9,6 +9,7 @@ import (
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"strings"
 	"testing"
 	"time"
@@ -50,6 +51,8 @@ func Test_HttpContentLoader_Load_ResponseProcessor(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
 	a := assert.New(t)
+	request := &http.Request{}
+	request.URL = &url.URL{}
 
 	server := testServer("the body", time.Millisecond*0)
 	defer server.Close()
@@ -68,11 +71,10 @@ 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(NewFetchDefinitionWithResponseProcessorFromRequest(server.URL, request, mockResponseProcessor))
 	a.NoError(err)
 	a.NotNil(c)
 	a.Nil(c.Reader())
-	a.Equal(server.URL, c.Name())
 	eqFragment(t, "some head content", c.Head())
 	a.Equal(0, len(c.Body()))
 }