diff options
author | Jaime Martinez <jmartinez@gitlab.com> | 2021-02-19 06:44:02 +0300 |
---|---|---|
committer | Jaime Martinez <jmartinez@gitlab.com> | 2021-03-18 03:24:10 +0300 |
commit | f104ed91e6c9400cad4576f826e40dffe0ede3f4 (patch) | |
tree | 843c80a0209615ca87c23c1fcc82f92d4ef46771 /internal | |
parent | 4f597a45e15283cdefa66792575a6da602376ba2 (diff) |
Use better naming
Diffstat (limited to 'internal')
-rw-r--r-- | internal/source/gitlab/cache/entry.go | 74 | ||||
-rw-r--r-- | internal/source/gitlab/cache/entry_test.go | 16 | ||||
-rw-r--r-- | internal/source/gitlab/cache/memstore.go | 22 |
3 files changed, 56 insertions, 56 deletions
diff --git a/internal/source/gitlab/cache/entry.go b/internal/source/gitlab/cache/entry.go index bf6866bd..07f45ea8 100644 --- a/internal/source/gitlab/cache/entry.go +++ b/internal/source/gitlab/cache/entry.go @@ -14,29 +14,29 @@ import ( // holds a pointer to *api.Lookup when the domain lookup has been retrieved // successfully type Entry struct { - domain string - created time.Time - retrieve *sync.Once - refresh *sync.Once - mux *sync.RWMutex - retrieved chan struct{} - response *api.Lookup - refreshTimeout time.Duration - maxDuration time.Duration - retriever *Retriever + domain string + created time.Time + retrieve *sync.Once + refresh *sync.Once + mux *sync.RWMutex + retrieved chan struct{} + response *api.Lookup + refreshTimeout time.Duration + expirationTimeout time.Duration + retriever *Retriever } -func newCacheEntry(domain string, refreshTimeout, maxDuration time.Duration, retriever *Retriever) *Entry { +func newCacheEntry(domain string, refreshTimeout, entryExpirationTimeout time.Duration, retriever *Retriever) *Entry { return &Entry{ - domain: domain, - created: time.Now(), - retrieve: &sync.Once{}, - refresh: &sync.Once{}, - mux: &sync.RWMutex{}, - retrieved: make(chan struct{}), - refreshTimeout: refreshTimeout, - maxDuration: maxDuration, - retriever: retriever, + domain: domain, + created: time.Now(), + retrieve: &sync.Once{}, + refresh: &sync.Once{}, + mux: &sync.RWMutex{}, + retrieved: make(chan struct{}), + refreshTimeout: refreshTimeout, + expirationTimeout: entryExpirationTimeout, + retriever: retriever, } } @@ -46,7 +46,7 @@ func (e *Entry) IsUpToDate() bool { e.mux.RLock() defer e.mux.RUnlock() - return e.isResolved() && !e.isExpired() + return e.isResolved() && !e.isOutdated() } // NeedsRefresh return true if the entry has been resolved correctly but it has @@ -55,7 +55,7 @@ func (e *Entry) NeedsRefresh() bool { e.mux.RLock() defer e.mux.RUnlock() - return e.isResolved() && e.isExpired() + return e.isResolved() && e.isOutdated() } // Lookup returns a retriever Lookup response. @@ -90,10 +90,13 @@ func (e *Entry) Refresh(store Store) { } func (e *Entry) refreshFunc(store Store) { - entry := newCacheEntry(e.domain, e.refreshTimeout, e.maxDuration, e.retriever) + entry := newCacheEntry(e.domain, e.refreshTimeout, e.expirationTimeout, e.retriever) entry.Retrieve(context.Background()) - if e.reuseEntry(entry) { + + // do not replace existing Entry `e` when `entry` has an error + // and `e` has not expired. See https://gitlab.com/gitlab-org/gitlab-pages/-/issues/281. + if entry.hasTemporaryError() && !e.isExpired() { entry.response = e.response entry.created = e.created } @@ -109,7 +112,7 @@ func (e *Entry) setResponse(lookup api.Lookup) { close(e.retrieved) } -func (e *Entry) isExpired() bool { +func (e *Entry) isOutdated() bool { return time.Since(e.created) > e.refreshTimeout } @@ -117,18 +120,15 @@ func (e *Entry) isResolved() bool { return e.response != nil } -func (e *Entry) maxTimeInCache() bool { - return time.Since(e.created) > e.maxDuration +func (e *Entry) isExpired() bool { + return time.Since(e.created) > e.expirationTimeout } -// reuseEntry as the refreshed entry when there is an error after resolving the lookup again -// and is different to domain.ErrDomainDoesNotExist. This is an edge case to prevent serving -// a page right after being deleted. -// It should only be refreshed as long as it hasn't passed e.maxDuration. -// See https://gitlab.com/gitlab-org/gitlab-pages/-/issues/281. -func (e *Entry) reuseEntry(entry *Entry) bool { - return entry.response != nil && - entry.response.Error != nil && - !errors.Is(entry.response.Error, domain.ErrDomainDoesNotExist) && - !e.maxTimeInCache() +// hasTemporaryError checks currently refreshed entry for errors after resolving the lookup again +// and is different to domain.ErrDomainDoesNotExist (this is an edge case to prevent serving +// a page right after being deleted). +func (e *Entry) hasTemporaryError() bool { + return e.response != nil && + e.response.Error != nil && + !errors.Is(e.response.Error, domain.ErrDomainDoesNotExist) } diff --git a/internal/source/gitlab/cache/entry_test.go b/internal/source/gitlab/cache/entry_test.go index 528f2a6f..919c89c6 100644 --- a/internal/source/gitlab/cache/entry_test.go +++ b/internal/source/gitlab/cache/entry_test.go @@ -158,22 +158,22 @@ type lookupMock struct { responses map[string]api.Lookup } -func (mm *lookupMock) GetLookup(ctx context.Context, domainName string) api.Lookup { +func (lm *lookupMock) GetLookup(ctx context.Context, domainName string) api.Lookup { lookup := api.Lookup{ Name: domainName, } - lookup, ok := mm.responses[domainName] + lookup, ok := lm.responses[domainName] if !ok { lookup.Error = domain.ErrDomainDoesNotExist return lookup } - // return error after mm.successCount - mm.currentCount++ - if mm.currentCount > mm.successCount { - mm.currentCount = 0 - mm.failed = true + // return error after lm.successCount + lm.currentCount++ + if lm.currentCount > lm.successCount { + lm.currentCount = 0 + lm.failed = true lookup.Error = http.ErrServerClosed } @@ -181,6 +181,6 @@ func (mm *lookupMock) GetLookup(ctx context.Context, domainName string) api.Look return lookup } -func (mm *lookupMock) Status() error { +func (lm *lookupMock) Status() error { return nil } diff --git a/internal/source/gitlab/cache/memstore.go b/internal/source/gitlab/cache/memstore.go index 4565590e..8ba069e4 100644 --- a/internal/source/gitlab/cache/memstore.go +++ b/internal/source/gitlab/cache/memstore.go @@ -11,22 +11,22 @@ import ( ) type memstore struct { - store *cache.Cache - mux *sync.RWMutex - retriever *Retriever - entryRefreshTimeout time.Duration - entryMaxDuration time.Duration + store *cache.Cache + mux *sync.RWMutex + retriever *Retriever + entryRefreshTimeout time.Duration + entryExpirationTimeout time.Duration } func newMemStore(client api.Client, cc *config.Cache) Store { retriever := NewRetriever(client, cc.RetrievalTimeout, cc.MaxRetrievalInterval, cc.MaxRetrievalRetries) return &memstore{ - store: cache.New(cc.CacheExpiry, cc.CacheCleanupInterval), - mux: &sync.RWMutex{}, - retriever: retriever, - entryRefreshTimeout: cc.EntryRefreshTimeout, - entryMaxDuration: cc.CacheExpiry, + store: cache.New(cc.CacheExpiry, cc.CacheCleanupInterval), + mux: &sync.RWMutex{}, + retriever: retriever, + entryRefreshTimeout: cc.EntryRefreshTimeout, + entryExpirationTimeout: cc.CacheExpiry, } } @@ -48,7 +48,7 @@ func (m *memstore) LoadOrCreate(domain string) *Entry { return entry.(*Entry) } - newEntry := newCacheEntry(domain, m.entryRefreshTimeout, m.entryMaxDuration, m.retriever) + newEntry := newCacheEntry(domain, m.entryRefreshTimeout, m.entryExpirationTimeout, m.retriever) m.store.SetDefault(domain, newEntry) return newEntry |