Some FetchDefinitions should not be removed from cache if there was a…

…n error in a context they were a part of. (For example a site navigation if some random page threw a 404)

composition/composition_handler.go

@@ -89,9 +89,17 @@ func (agg *CompositionHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 	w.Header().Set("Content-Type", "text/html; charset=utf-8")
 
 	html, err := agg.processHtml(mergeContext, w, r)
-	// Return if an error occured within the html aggregation
+	// Return if an error occured within the html aggregation and
+	// remove potentially erroneous results from cache
 	if err != nil {
-		agg.purgeCacheEntries(results)
+		//Filter without allocating
+		resultsToPurge := results[:0]
+		for _, res := range results {
+			if !res.Def.NoPurgeOnMergeError {
+				resultsToPurge = append(resultsToPurge, res)
+			}
+		}
+		agg.purgeCacheEntries(resultsToPurge)
 		return
 	}
 

composition/composition_handler_test.go

@@ -346,14 +346,21 @@ func Test_CompositionHandler_ErrorInMergingWithCache(t *testing.T) {
 				Err:     nil,
 				Hash:    "hashString",
 			},
+			&FetchResult{
+				Def:     NewFetchDefinition("/bar").ShouldSurviveContextPurge(true),
+				Content: &MemoryContent{},
+				Err:     nil,
+				Hash:    "survivor",
+			},
 		}
 	}
 
 	aggregator := NewCompositionHandlerWithCache(ContentFetcherFactory(contentFetcherFactory), cache.NewCache("my-cache", 100, 100, time.Millisecond))
 	aggregator.cache.Set("hashString", "", 1, nil)
+	aggregator.cache.Set("survivor", "", 1, nil)
 	aggregator.contentMergerFactory = func(jsonData map[string]interface{}) ContentMerger {
 		merger := NewMockContentMerger(ctrl)
-		merger.EXPECT().AddContent(gomock.Any(), 0)
+		merger.EXPECT().AddContent(gomock.Any(), 0).Times(2)
 		merger.EXPECT().GetHtml().Return(nil, errors.New("an error"))
 		return merger
 	}
@@ -364,8 +371,10 @@ func Test_CompositionHandler_ErrorInMergingWithCache(t *testing.T) {
 	aggregator.ServeHTTP(resp, r)
 
 	_, foundInCache := aggregator.cache.Get("hashString")
+	_, foundSurvivorInCache := aggregator.cache.Get("survivor")
 
 	a.False(foundInCache)
+	a.True(foundSurvivorInCache)
 	a.Equal("Internal Server Error: an error\n", string(resp.Body.Bytes()))
 	a.Equal(500, resp.Code)
 }

composition/fetch_definition.go

@@ -76,20 +76,25 @@ type FetchDefinition struct {
 	ServiceDiscoveryActive bool
 	ServiceDiscovery       servicediscovery.ServiceDiscovery
 	Priority               int
+	// Defines whether the result of this FD should remain cached
+	// even if all cached entries of an erroneous context are removed
+	// (e.g. the FD relates to a central component which is used everywhere)
+	NoPurgeOnMergeError bool
 }
 
 // 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:      NewDefaultErrorHandler(),
-		CacheStrategy:   cache.DefaultCacheStrategy,
-		Priority:        DefaultPriority,
+		Name:                urlToName(url), // the name defauls to the url
+		URL:                 url,
+		Timeout:             DefaultTimeout,
+		FollowRedirects:     false,
+		Required:            true,
+		Method:              "GET",
+		ErrHandler:          NewDefaultErrorHandler(),
+		CacheStrategy:       cache.DefaultCacheStrategy,
+		Priority:            DefaultPriority,
+		NoPurgeOnMergeError: false,
 	}
 }
 
@@ -139,6 +144,11 @@ func (fd *FetchDefinition) WithName(name string) *FetchDefinition {
 	return fd
 }
 
+func (fd *FetchDefinition) ShouldSurviveContextPurge(shouldSurvive bool) *FetchDefinition {
+	fd.NoPurgeOnMergeError = shouldSurvive
+	return fd
+}
+
 // Hash returns a unique hash for the fetch request.
 // If two hashes of fetch resources are equal, they refer the same resource
 // and can e.g. be taken as replacement for each other. E.g. in case of caching.