Skip to content

Commit 9b6967e

Browse files
Fix staticcheck issues in sherdlock fetcher
Signed-off-by: Shashank <yshashank959@gmail.com>
1 parent 659aff6 commit 9b6967e

File tree

7 files changed

+37
-46
lines changed

7 files changed

+37
-46
lines changed

test_output.txt

-42.2 KB
Binary file not shown.

token/services/selector/sherdlock/fetcher.go

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ func (f *mixedFetcher) UnspentTokensIteratorBy(ctx context.Context, walletID str
105105
logger.DebugfContext(ctx, "call unspent tokens iterator")
106106
it, err := f.eagerFetcher.UnspentTokensIteratorBy(ctx, walletID, currency)
107107
logger.DebugfContext(ctx, "fetched eager iterator")
108-
if err == nil && it.(enhancedIterator[*token2.UnspentTokenInWallet]).HasNext() {
108+
if err == nil && it.(interface{ HasNext() bool }).HasNext() {
109109
logger.DebugfContext(ctx, "eager iterator had tokens. Returning iterator")
110110
f.m.UnspentTokensInvocations.With(fetcherTypeLabel, eager).Add(1)
111111

@@ -118,12 +118,6 @@ func (f *mixedFetcher) UnspentTokensIteratorBy(ctx context.Context, walletID str
118118
return f.lazyFetcher.UnspentTokensIteratorBy(ctx, walletID, currency)
119119
}
120120

121-
// newCachedFetcher is an internal alias for NewCachedFetcher to maintain compatibility within the package if needed,
122-
// though we usually just use the exported version now.
123-
func newCachedFetcher(tokenDB TokenDB, cacheSize int64, freshnessInterval time.Duration, maxQueriesBeforeRefresh int) *cachedFetcher {
124-
return NewCachedFetcher(tokenDB, cacheSize, freshnessInterval, maxQueriesBeforeRefresh)
125-
}
126-
127121
// newMixedFetcher is an internal alias for NewMixedFetcher.
128122
func newMixedFetcher(tokenDB TokenDB, m *Metrics, cacheSize int64, freshnessInterval time.Duration, maxQueries int) *mixedFetcher {
129123
return NewMixedFetcher(tokenDB, m, cacheSize, freshnessInterval, maxQueries)
@@ -150,11 +144,6 @@ func (f *lazyFetcher) UnspentTokensIteratorBy(ctx context.Context, walletID stri
150144
return collections.NewPermutatedIterator[token2.UnspentTokenInWallet](it)
151145
}
152146

153-
type enhancedIterator[T any] interface {
154-
iterators.Iterator[T]
155-
HasNext() bool
156-
}
157-
158147
type permutatableIterator[T any] interface {
159148
iterators.Iterator[T]
160149
NewPermutation() iterators.Iterator[T]
@@ -177,7 +166,7 @@ type cachedFetcher struct {
177166
maxQueriesBeforeRefresh uint32
178167

179168
// TODO: A better strategy is to keep following variables per cache key (type/owner combination) and lock/fetch only the 'expired' entry
180-
lastFetched time.Time
169+
lastFetched int64
181170
queriesResponded uint32
182171
// prevKeys tracks cache keys from the previous update cycle to identify stale entries that need removal.
183172
prevKeys map[string]struct{}
@@ -218,7 +207,6 @@ func NewCachedFetcher(tokenDB TokenDB, cacheSize int64, freshnessInterval time.D
218207
}
219208
}
220209

221-
// update refreshes the token cache from the database, adding new entries before removing stale ones to prevent race conditions.
222210
func (f *cachedFetcher) update(ctx context.Context) {
223211
f.mu.Lock()
224212
defer f.mu.Unlock()
@@ -238,7 +226,7 @@ func (f *cachedFetcher) update(ctx context.Context) {
238226

239227
m := f.groupTokensByKey(ctx, it)
240228
f.updateCache(ctx, m)
241-
f.lastFetched = time.Now()
229+
atomic.StoreInt64(&f.lastFetched, time.Now().UnixNano())
242230
atomic.StoreUint32(&f.queriesResponded, 0)
243231
}
244232

@@ -312,5 +300,10 @@ func (f *cachedFetcher) isCacheOverused() bool {
312300

313301
// isCacheStale checks if the cache has exceeded its freshness interval.
314302
func (f *cachedFetcher) isCacheStale() bool {
315-
return time.Since(f.lastFetched) > f.freshnessInterval
303+
lastFetched := atomic.LoadInt64(&f.lastFetched)
304+
if lastFetched == 0 {
305+
return true
306+
}
307+
308+
return time.Since(time.Unix(0, lastFetched)) > f.freshnessInterval
316309
}

token/services/selector/sherdlock/fetcher_test.go

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func TestNewCachedFetcher_WithDefaults(t *testing.T) {
4747
mockDB := new(mockTokenDB)
4848

4949
// Test with zero values (should use defaults)
50-
fetcher := newCachedFetcher(mockDB, 0, 0, 0)
50+
fetcher := NewCachedFetcher(mockDB, 0, 0, 0)
5151

5252
assert.NotNil(t, fetcher)
5353
assert.Equal(t, defaultCacheFreshnessInterval, fetcher.freshnessInterval)
@@ -62,7 +62,7 @@ func TestNewCachedFetcher_WithCustomValues(t *testing.T) {
6262
customFreshness := 60 * time.Second
6363
customMaxQueries := 200
6464

65-
fetcher := newCachedFetcher(mockDB, customSize, customFreshness, customMaxQueries)
65+
fetcher := NewCachedFetcher(mockDB, customSize, customFreshness, customMaxQueries)
6666

6767
assert.NotNil(t, fetcher)
6868
assert.Equal(t, customFreshness, fetcher.freshnessInterval)
@@ -72,13 +72,13 @@ func TestNewCachedFetcher_WithCustomValues(t *testing.T) {
7272

7373
func TestCachedFetcher_IsCacheStale(t *testing.T) {
7474
mockDB := new(mockTokenDB)
75-
fetcher := newCachedFetcher(mockDB, 0, 100*time.Millisecond, 0)
75+
fetcher := NewCachedFetcher(mockDB, 0, 100*time.Millisecond, 0)
7676

7777
// Initially cache should be stale (lastFetched is zero time)
7878
assert.True(t, fetcher.isCacheStale())
7979

8080
// Update lastFetched to now
81-
fetcher.lastFetched = time.Now()
81+
atomic.StoreInt64(&fetcher.lastFetched, time.Now().UnixNano())
8282
assert.False(t, fetcher.isCacheStale())
8383

8484
// Wait for cache to become stale
@@ -89,7 +89,7 @@ func TestCachedFetcher_IsCacheStale(t *testing.T) {
8989
func TestCachedFetcher_IsCacheOverused(t *testing.T) {
9090
mockDB := new(mockTokenDB)
9191
maxQueries := 5
92-
fetcher := newCachedFetcher(mockDB, 0, 0, maxQueries)
92+
fetcher := NewCachedFetcher(mockDB, 0, 0, maxQueries)
9393

9494
// Initially not overused
9595
assert.False(t, fetcher.isCacheOverused())
@@ -107,7 +107,7 @@ func TestCachedFetcher_IsCacheOverused(t *testing.T) {
107107

108108
func TestCachedFetcher_Update(t *testing.T) {
109109
mockDB := new(mockTokenDB)
110-
fetcher := newCachedFetcher(mockDB, 0, 1*time.Second, 100)
110+
fetcher := NewCachedFetcher(mockDB, 0, 1*time.Second, 100)
111111

112112
// Create test tokens
113113
tokens := []*token2.UnspentTokenInWallet{
@@ -158,7 +158,7 @@ func TestCachedFetcher_Update(t *testing.T) {
158158

159159
func TestCachedFetcher_UnspentTokensIteratorBy_CacheHit(t *testing.T) {
160160
mockDB := new(mockTokenDB)
161-
fetcher := newCachedFetcher(mockDB, 0, 10*time.Second, 100)
161+
fetcher := NewCachedFetcher(mockDB, 0, 10*time.Second, 100)
162162

163163
// Populate cache
164164
tokens := []*token2.UnspentTokenInWallet{
@@ -179,7 +179,7 @@ func TestCachedFetcher_UnspentTokensIteratorBy_CacheHit(t *testing.T) {
179179

180180
require.NoError(t, err)
181181
assert.NotNil(t, it)
182-
assert.True(t, it.(enhancedIterator[*token2.UnspentTokenInWallet]).HasNext())
182+
assert.True(t, it.(interface{ HasNext() bool }).HasNext())
183183

184184
// Verify query counter incremented
185185
assert.Equal(t, uint32(1), atomic.LoadUint32(&fetcher.queriesResponded))
@@ -189,7 +189,7 @@ func TestCachedFetcher_UnspentTokensIteratorBy_CacheHit(t *testing.T) {
189189

190190
func TestCachedFetcher_UnspentTokensIteratorBy_CacheMiss(t *testing.T) {
191191
mockDB := new(mockTokenDB)
192-
fetcher := newCachedFetcher(mockDB, 0, 10*time.Second, 100)
192+
fetcher := NewCachedFetcher(mockDB, 0, 10*time.Second, 100)
193193

194194
// Populate cache with different key
195195
tokens := []*token2.UnspentTokenInWallet{
@@ -211,15 +211,15 @@ func TestCachedFetcher_UnspentTokensIteratorBy_CacheMiss(t *testing.T) {
211211
require.NoError(t, err)
212212
assert.NotNil(t, it)
213213
// Should return empty iterator
214-
assert.False(t, it.(enhancedIterator[*token2.UnspentTokenInWallet]).HasNext())
214+
assert.False(t, it.(interface{ HasNext() bool }).HasNext())
215215

216216
mockDB.AssertExpectations(t)
217217
}
218218

219219
func TestCachedFetcher_UnspentTokensIteratorBy_StaleCache(t *testing.T) {
220220
mockDB := new(mockTokenDB)
221221
// Very short freshness interval
222-
fetcher := newCachedFetcher(mockDB, 0, 50*time.Millisecond, 100)
222+
fetcher := NewCachedFetcher(mockDB, 0, 50*time.Millisecond, 100)
223223

224224
// Initial population
225225
tokens1 := []*token2.UnspentTokenInWallet{
@@ -260,7 +260,7 @@ func TestCachedFetcher_UnspentTokensIteratorBy_StaleCache(t *testing.T) {
260260

261261
func TestCachedFetcher_CacheClear(t *testing.T) {
262262
mockDB := new(mockTokenDB)
263-
fetcher := newCachedFetcher(mockDB, 0, 10*time.Second, 100)
263+
fetcher := NewCachedFetcher(mockDB, 0, 10*time.Second, 100)
264264

265265
// First update with tokens
266266
tokens1 := []*token2.UnspentTokenInWallet{
@@ -299,7 +299,7 @@ func TestCachedFetcher_CacheClear(t *testing.T) {
299299
mockDB.On("SpendableTokensIteratorBy", mock.Anything, "", token2.Type("")).Return(mockIterator2, nil).Once()
300300

301301
// Force cache to be stale so update will actually run
302-
fetcher.lastFetched = time.Now().Add(-20 * time.Second)
302+
atomic.StoreInt64(&fetcher.lastFetched, time.Now().Add(-20 * time.Second).UnixNano())
303303

304304
fetcher.update(ctx)
305305

@@ -317,7 +317,7 @@ func TestCachedFetcher_CacheClear(t *testing.T) {
317317
func TestNewMixedFetcher(t *testing.T) {
318318
mockDB := new(mockTokenDB)
319319

320-
fetcher := newMixedFetcher(mockDB, nil, 100, 30*time.Second, 100)
320+
fetcher := NewMixedFetcher(mockDB, nil, 100, 30*time.Second, 100)
321321

322322
assert.NotNil(t, fetcher)
323323
assert.NotNil(t, fetcher.lazyFetcher)
@@ -486,7 +486,7 @@ func TestLazyFetcher_UnspentTokensIteratorBy_ErrorHandling(t *testing.T) {
486486

487487
func TestMixedFetcher_FallbackBehavior(t *testing.T) {
488488
mockDB := new(mockTokenDB)
489-
fetcher := newMixedFetcher(mockDB, NewMetrics(&disabled.Provider{}), 0, 10*time.Second, 100)
489+
fetcher := NewMixedFetcher(mockDB, NewMetrics(&disabled.Provider{}), 0, 10*time.Second, 100)
490490

491491
t.Run("uses lazy fetcher when eager returns error", func(t *testing.T) {
492492
// Setup: eager fetcher will fail to update
@@ -553,7 +553,7 @@ func TestMixedFetcher_FallbackBehavior(t *testing.T) {
553553

554554
func TestCachedFetcher_ConcurrentAccess(t *testing.T) {
555555
mockDB := new(mockTokenDB)
556-
fetcher := newCachedFetcher(mockDB, 0, 1*time.Second, 100)
556+
fetcher := NewCachedFetcher(mockDB, 0, 1*time.Second, 100)
557557

558558
t.Run("handles concurrent reads during update", func(t *testing.T) {
559559
// Populate cache
@@ -596,7 +596,7 @@ func TestCachedFetcher_ConcurrentAccess(t *testing.T) {
596596

597597
func TestCachedFetcher_GroupTokensByKey(t *testing.T) {
598598
mockDB := new(mockTokenDB)
599-
fetcher := newCachedFetcher(mockDB, 0, 1*time.Second, 100)
599+
fetcher := NewCachedFetcher(mockDB, 0, 1*time.Second, 100)
600600

601601
t.Run("groups tokens correctly", func(t *testing.T) {
602602
tokens := []*token2.UnspentTokenInWallet{
@@ -637,7 +637,7 @@ func TestCachedFetcher_GroupTokensByKey(t *testing.T) {
637637
// TestCachedFetcher_UpdateCache verifies cache updates without race conditions (add before remove).
638638
func TestCachedFetcher_UpdateCache(t *testing.T) {
639639
mockDB := new(mockTokenDB)
640-
fetcher := newCachedFetcher(mockDB, 0, 1*time.Second, 100)
640+
fetcher := NewCachedFetcher(mockDB, 0, 1*time.Second, 100)
641641

642642
t.Run("removes stale keys", func(t *testing.T) {
643643
ctx := t.Context()
@@ -751,7 +751,7 @@ func TestCachedFetcher_UpdateCache(t *testing.T) {
751751
func TestCachedFetcher_SoftRefresh(t *testing.T) {
752752
mockDB := new(mockTokenDB)
753753
maxQueries := 3
754-
fetcher := newCachedFetcher(mockDB, 0, 10*time.Second, maxQueries)
754+
fetcher := NewCachedFetcher(mockDB, 0, 10*time.Second, maxQueries)
755755

756756
t.Run("triggers soft refresh when overused", func(t *testing.T) {
757757
// Initial population
@@ -886,7 +886,7 @@ func TestFetcherProvider_GetFetcher(t *testing.T) {
886886
// TestCachedFetcher_UpdateWithDatabaseError verifies cache stays stale when DB update fails.
887887
func TestCachedFetcher_UpdateWithDatabaseError(t *testing.T) {
888888
mockDB := new(mockTokenDB)
889-
fetcher := newCachedFetcher(mockDB, 0, 1*time.Second, 100)
889+
fetcher := NewCachedFetcher(mockDB, 0, 1*time.Second, 100)
890890

891891
t.Run("handles database error gracefully", func(t *testing.T) {
892892
expectedErr := errors.New("database connection failed")
@@ -929,7 +929,7 @@ func TestTokenKey_EdgeCases(t *testing.T) {
929929
func TestMixedFetcher_MetricsTracking(t *testing.T) {
930930
mockDB := new(mockTokenDB)
931931
metrics := NewMetrics(&disabled.Provider{})
932-
fetcher := newMixedFetcher(mockDB, metrics, 0, 10*time.Second, 100)
932+
fetcher := NewMixedFetcher(mockDB, metrics, 0, 10*time.Second, 100)
933933

934934
t.Run("tracks eager fetcher usage", func(t *testing.T) {
935935
// Populate cache

token/services/selector/sherdlock/manager_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,7 @@ func startContainer(t *testing.T) (func(), string) {
116116
t.Helper()
117117
cfg := postgres2.DefaultConfig(postgres2.WithDBName(t.Name()))
118118
terminate, _, err := postgres2.StartPostgres(t.Context(), cfg, nil)
119-
if err != nil {
120-
t.Skipf("skipping test: failed to start postgres container: %v", err)
121-
}
119+
require.NoError(t, err)
122120

123121
return terminate, cfg.DataSource()
124122
}

token/services/selector/sherdlock/selector_shutdown_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ func TestSelector_UnlockAllOnQuantityParseError(t *testing.T) {
7575
unlockAllCalled := false
7676

7777
mockFetcher := &mockTokenFetcher{
78-
unspentTokensIteratorByFunc: func(_ context.Context, _ string, _ token2.Type) (iterator[*token2.UnspentTokenInWallet], error) {
78+
unspentTokensIteratorByFunc: func(_ context.Context, _ string, _ token2.Type) (Iterator[*token2.UnspentTokenInWallet], error) {
7979
// quantity "not-a-number" will fail token2.ToQuantity for any precision
8080
tok := &token2.UnspentTokenInWallet{
8181
Id: token2.ID{TxId: "tx1", Index: 0},

token/services/selector/sherdlock/service.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import (
1919
)
2020

2121
type SelectorService struct {
22-
managerLazyCache lazy2.Provider[token.ManagementService, token.SelectorManager]
22+
managerLazyCache lazy2.Provider[*token.ManagementService, token.SelectorManager]
2323
mu sync.Mutex
2424
managers []*Manager
2525
}
@@ -56,7 +56,7 @@ func (s *SelectorService) SelectorManager(tms *token.ManagementService) (token.S
5656
return nil, errors.Errorf("invalid tms, nil reference")
5757
}
5858

59-
return s.managerLazyCache.Get(*tms)
59+
return s.managerLazyCache.Get(tms)
6060
}
6161

6262
// Shutdown stops all background goroutines for every manager created by this service.
@@ -95,7 +95,7 @@ type loader struct {
9595
onCreate func(*Manager)
9696
}
9797

98-
func (s *loader) load(tms token.ManagementService) (token.SelectorManager, error) {
98+
func (s *loader) load(tms *token.ManagementService) (token.SelectorManager, error) {
9999
pp := tms.PublicParametersManager().PublicParameters()
100100
if pp == nil {
101101
return nil, errors.Errorf("public parameters not set yet for TMS [%s]", tms.ID())
@@ -126,6 +126,6 @@ func (s *loader) load(tms token.ManagementService) (token.SelectorManager, error
126126
return mgr, nil
127127
}
128128

129-
func key(tms token.ManagementService) string {
129+
func key(tms *token.ManagementService) string {
130130
return tms.ID().String()
131131
}

token/services/utils/retry_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,7 @@ func TestNextDelay_FixedBackoff(t *testing.T) {
380380
// All subsequent intervals should be approximately equal (fixed delay)
381381
// Use generous tolerance for scheduling jitter
382382
for i := 1; i < len(intervals); i++ {
383-
assert.InDelta(t, 10*time.Millisecond, intervals[i], float64(10*time.Millisecond),
383+
assert.InDelta(t, 10*time.Millisecond, intervals[i], float64(50*time.Millisecond),
384384
"interval %d should be ~10ms for fixed backoff, got %v", i, intervals[i])
385385
}
386386
}

0 commit comments

Comments
 (0)