From a87857789558e954ad3f83421142a217406a4f33 Mon Sep 17 00:00:00 2001 From: denge Date: Mon, 1 Aug 2016 10:45:46 +0200 Subject: [PATCH] removes broken fragements from the cache with tests --- cache/cache.go | 20 +++++++++++- cache/cache_test.go | 12 +++++++ composition/composition_handler.go | 15 +++++++++ composition/composition_handler_test.go | 42 ++++++++++++++++++++++++- composition/content_merge.go | 13 ++++++++ composition/content_merge_test.go | 8 +++++ composition/interface_mocks_test.go | 19 ++++++++++- composition/interfaces.go | 4 +++ 8 files changed, 130 insertions(+), 3 deletions(-) diff --git a/cache/cache.go b/cache/cache.go index ffd9142..2329070 100644 --- a/cache/cache.go +++ b/cache/cache.go @@ -143,7 +143,6 @@ func (c *Cache) PurgeOldEntries() { c.lock.RLock() keys := c.lruBackend.Keys() c.lock.RUnlock() - purged := 0 for _, key := range keys { c.lock.RLock() @@ -163,7 +162,26 @@ func (c *Cache) PurgeOldEntries() { logging.Logger. WithFields(logrus.Fields(c.stats)). Infof("purged %v out of %v cache entries", purged, len(keys)) +} + +// Purge Entries with a specific hash +func (c *Cache) PurgeEntries(keys []string) { + purged := 0 + for _, key := range keys { + c.lock.RLock() + _, found := c.lruBackend.Peek(key) + c.lock.RUnlock() + if found { + c.lock.Lock() + c.lruBackend.Remove(key) + c.lock.Unlock() + purged++ + } + } + logging.Logger. + WithFields(logrus.Fields(c.stats)). + Infof("purged %v out of %v cache entries", purged, len(keys)) } func (c *Cache) Invalidate() { diff --git a/cache/cache_test.go b/cache/cache_test.go index ca0fd72..fab57cc 100644 --- a/cache/cache_test.go +++ b/cache/cache_test.go @@ -133,6 +133,18 @@ func Test_Cache_PurgeOldEntries(t *testing.T) { a.Equal(84, c.SizeByte()) } +func Test_Cache_PurgeEntries(t *testing.T) { + a := assert.New(t) + + c := NewCache("my-cache", 100, 100, time.Millisecond) + c.Set("hashString", "", 1, nil) + + c.PurgeEntries([]string{"hashString"}) + _, foundInCache := c.Get("hashString") + + a.False(foundInCache) +} + func Test_Cache_Invalidation(t *testing.T) { a := assert.New(t) diff --git a/composition/composition_handler.go b/composition/composition_handler.go index afefb14..54cff0a 100644 --- a/composition/composition_handler.go +++ b/composition/composition_handler.go @@ -15,6 +15,7 @@ type ContentFetcherFactory func(r *http.Request) FetchResultSupplier type CompositionHandler struct { contentFetcherFactory ContentFetcherFactory contentMergerFactory func(metaJSON map[string]interface{}) ContentMerger + cache Cache } // NewCompositionHandler creates a new Handler with the supplied defualtData, @@ -25,6 +26,17 @@ func NewCompositionHandler(contentFetcherFactory ContentFetcherFactory) *Composi contentMergerFactory: func(metaJSON map[string]interface{}) ContentMerger { return NewContentMerge(metaJSON) }, + cache: nil, + } +} + +func NewCompositionHandlerWithCache(contentFetcherFactory ContentFetcherFactory, cache Cache) *CompositionHandler { + return &CompositionHandler{ + contentFetcherFactory: contentFetcherFactory, + contentMergerFactory: func(metaJSON map[string]interface{}) ContentMerger { + return NewContentMerge(metaJSON) + }, + cache: cache, } } @@ -86,6 +98,9 @@ func (agg *CompositionHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) html, err := mergeContext.GetHtml() if err != nil { + if agg.cache != nil { + agg.cache.PurgeEntries(mergeContext.GetHashes()) + } logging.Application(r.Header).Error(err.Error()) http.Error(w, "Internal Server Error: "+err.Error(), 500) return diff --git a/composition/composition_handler_test.go b/composition/composition_handler_test.go index 136190f..434e2fd 100644 --- a/composition/composition_handler_test.go +++ b/composition/composition_handler_test.go @@ -5,12 +5,14 @@ import ( "errors" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" + "github.com/tarent/lib-compose/cache" "io/ioutil" "net/http" "net/http/httptest" "net/url" "strings" "testing" + "time" ) func Test_CompositionHandler_PositiveCase(t *testing.T) { @@ -159,12 +161,51 @@ func Test_CompositionHandler_ErrorInMerging(t *testing.T) { resp := httptest.NewRecorder() r, _ := http.NewRequest("GET", "http://example.com", nil) + aggregator.ServeHTTP(resp, r) a.Equal("Internal Server Error: an error\n", string(resp.Body.Bytes())) a.Equal(500, resp.Code) } +func Test_CompositionHandler_ErrorInMergingWithCache(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{}, + Err: nil, + Hash: "hashString", + }, + } + } + + aggregator := NewCompositionHandlerWithCache(ContentFetcherFactory(contentFetcherFactory), cache.NewCache("my-cache", 100, 100, time.Millisecond)) + aggregator.cache.Set("hashString", "", 1, nil) + aggregator.contentMergerFactory = func(jsonData map[string]interface{}) ContentMerger { + merger := NewMockContentMerger(ctrl) + merger.EXPECT().AddContent(gomock.Any()) + merger.EXPECT().GetHtml().Return(nil, errors.New("an error")) + merger.EXPECT().GetHashes().Return([]string{"hashString"}) + return merger + } + + resp := httptest.NewRecorder() + r, _ := http.NewRequest("GET", "http://example.com", nil) + + aggregator.ServeHTTP(resp, r) + + _, foundInCache := aggregator.cache.Get("hashString") + + a.False(foundInCache) + a.Equal("Internal Server Error: an error\n", string(resp.Body.Bytes())) + a.Equal(500, resp.Code) +} + func Test_CompositionHandler_ErrorInFetching(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() @@ -287,4 +328,3 @@ func (m MockFetchResultSupplier) MetaJSON() map[string]interface{} { func (m MockFetchResultSupplier) Empty() bool { return len([]*FetchResult(m)) == 0 } - diff --git a/composition/content_merge.go b/composition/content_merge.go index 0680778..6cdf066 100644 --- a/composition/content_merge.go +++ b/composition/content_merge.go @@ -20,6 +20,7 @@ type ContentMerge struct { Body map[string]Fragment Tail []Fragment Buffered bool + FdHashes []string } // NewContentMerge creates a new buffered ContentMerge @@ -31,6 +32,7 @@ func NewContentMerge(metaJSON map[string]interface{}) *ContentMerge { Body: make(map[string]Fragment), Tail: make([]Fragment, 0, 0), Buffered: true, + FdHashes: make([]string, 0, 0), } return cntx } @@ -86,6 +88,11 @@ func (cntx *ContentMerge) AddContent(fetchResult *FetchResult) { cntx.addBodyAttributes(fetchResult.Content.BodyAttributes()) cntx.addBody(fetchResult.Def.URL, fetchResult.Content.Body()) cntx.addTail(fetchResult.Content.Tail()) + cntx.addFdHash(fetchResult.Hash) +} + +func (cntx *ContentMerge) GetHashes() []string { + return cntx.FdHashes } func (cntx *ContentMerge) addHead(f Fragment) { @@ -114,6 +121,12 @@ func (cntx *ContentMerge) addTail(f Fragment) { } } +func (cntx *ContentMerge) addFdHash(hash string) { + if hash != "" { + cntx.FdHashes = append(cntx.FdHashes, hash) + } +} + // Returns a name from a url, which has template placeholders eliminated func urlToFragmentName(url string) string { url = strings.Replace(url, `§[`, `\§\[`, -1) diff --git a/composition/content_merge_test.go b/composition/content_merge_test.go index a5ed545..7c80cae 100644 --- a/composition/content_merge_test.go +++ b/composition/content_merge_test.go @@ -89,6 +89,14 @@ func Test_ContentMerge_MainFragmentDoesNotExist(t *testing.T) { a.Equal("Fragment does not exist: ", err.Error()) } +func Test_ContentMerge_FdHashes(t *testing.T) { + a := assert.New(t) + cm := NewContentMerge(nil) + + cm.addFdHash("testHash") + a.Equal(cm.GetHashes()[0], "testHash") +} + type closedWriterMock struct { } diff --git a/composition/interface_mocks_test.go b/composition/interface_mocks_test.go index ac241be..061b935 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" ) @@ -243,6 +242,16 @@ func (_mr *_MockContentMergerRecorder) AddContent(arg0 interface{}) *gomock.Call return _mr.mock.ctrl.RecordCall(_mr.mock, "AddContent", arg0) } +func (_m *MockContentMerger) GetHashes() []string { + ret := _m.ctrl.Call(_m, "GetHashes") + ret0, _ := ret[0].([]string) + return ret0 +} + +func (_mr *_MockContentMergerRecorder) GetHashes() *gomock.Call { + return _mr.mock.ctrl.RecordCall(_mr.mock, "GetHashes") +} + func (_m *MockContentMerger) GetHtml() ([]byte, error) { ret := _m.ctrl.Call(_m, "GetHtml") ret0, _ := ret[0].([]byte) @@ -356,6 +365,14 @@ func (_mr *_MockCacheRecorder) Invalidate() *gomock.Call { return _mr.mock.ctrl.RecordCall(_mr.mock, "Invalidate") } +func (_m *MockCache) PurgeEntries(_param0 []string) { + _m.ctrl.Call(_m, "PurgeEntries", _param0) +} + +func (_mr *_MockCacheRecorder) PurgeEntries(arg0 interface{}) *gomock.Call { + return _mr.mock.ctrl.RecordCall(_mr.mock, "PurgeEntries", arg0) +} + func (_m *MockCache) Set(_param0 string, _param1 string, _param2 int, _param3 interface{}) { _m.ctrl.Call(_m, "Set", _param0, _param1, _param2, _param3) } diff --git a/composition/interfaces.go b/composition/interfaces.go index 2c63948..07eff42 100644 --- a/composition/interfaces.go +++ b/composition/interfaces.go @@ -93,6 +93,9 @@ type ContentMerger interface { // Return the html as byte array GetHtml() ([]byte, error) + + // Return initial hashes related to the given contents + GetHashes() []string } type ResponseProcessor interface { @@ -110,4 +113,5 @@ type Cache interface { Get(hash string) (cacheObject interface{}, found bool) Set(hash string, label string, memorySize int, cacheObject interface{}) Invalidate() + PurgeEntries(keys []string) }