diff --git a/lru/lru.go b/lru/lru.go index ab6028c..6cf4177 100644 --- a/lru/lru.go +++ b/lru/lru.go @@ -41,7 +41,7 @@ func (c *Cache[K, V]) Size() int { return c.n } -// Len returns the length of the cache. +// Len returns the length of the values stored in the cache. func (c *Cache[K, V]) Len() int { return c.cache.Len() } @@ -74,7 +74,7 @@ func (c *Cache[K, V]) Get(ctx context.Context, key K) (V, error) { //nolint:iret c.mu.Unlock() if err != nil { - return zeroValue[V](), fmt.Errorf("evict expired value for key: %w", err) + return zeroValue[V](), fmt.Errorf("evict expired value: %w", err) } return c.populateByGetter(ctx, key) @@ -85,7 +85,7 @@ func (c *Cache[K, V]) populateByGetter(ctx context.Context, key K) (V, error) { return zeroValue[V](), fmt.Errorf("value not found for key: %v: %w", key, ErrNotFound) } - c.mu.Lock() // TODO: not sure about position of this lock + c.mu.Lock() ch := make(chan getterResult[V], 1) defer close(ch) @@ -155,10 +155,10 @@ func (c *Cache[K, V]) Set(ctx context.Context, key K, value V) error { exp = time.Now().Add(c.ttl) } - // If the key already exists, update the value and move the element to the front of the list. + // If the key already exists, update the value and exp, and move the element to the front of the list. if el, ok := c.lookup[key]; ok { el.Value.val = value - el.Value.exp = exp // TODO: what should happen with exp when updating value? + el.Value.exp = exp c.cache.MoveToFront(el) @@ -220,12 +220,12 @@ func (c *Cache[K, V]) evictExpired(ctx context.Context) error { el := c.cache.Front() for el != nil { - if el.Value.exp.Before(now) { - c.cache.Remove(el) + if el.Value.exp.After(now) { + continue + } - if err := c.evict(ctx, el); err != nil { - return err - } + if err := c.evict(ctx, el); err != nil { + return err } el = el.Next() diff --git a/lru/lru_test.go b/lru/lru_test.go index fa20edc..a122438 100644 --- a/lru/lru_test.go +++ b/lru/lru_test.go @@ -143,7 +143,7 @@ func Test_GetterPanics(t *testing.T) { wg.Wait() } -func Test_WithOnEvict(t *testing.T) { +func Test_OnEvictPanics(t *testing.T) { t.Parallel() c := New( @@ -153,16 +153,62 @@ func Test_WithOnEvict(t *testing.T) { }), ) + ctx := context.Background() + + err := c.Set(ctx, 1, "one") + assert.NoError(t, err) + time.Sleep(time.Millisecond) + v, err := c.Get(ctx, 1) + + assert.EqualError(t, err, "evict expired value: evict value for key: 1: panic") + assert.Empty(t, v) +} + +func Test_OnEvictReturnsError(t *testing.T) { + t.Parallel() + + c := New( + WithTTL[int, string](time.Nanosecond), + WithOnEvict[int, string](func(ctx context.Context, v string) error { + return errors.New("oops") //nolint:goerr113 + }), + ) + ctx := context.Background() err := c.Set(ctx, 1, "one") assert.NoError(t, err) + time.Sleep(time.Millisecond) + + v, err := c.Get(ctx, 1) + + assert.EqualError(t, err, "evict expired value: evict value for key: 1: oops") + assert.Empty(t, v) +} + +func Test_OnEvictOK(t *testing.T) { + t.Parallel() + + c := New( + WithTTL[int, string](time.Nanosecond), + WithOnEvict[int, string](func(ctx context.Context, v string) error { + return nil + }), + ) + + ctx := context.Background() + + err := c.Set(ctx, 1, "one") + assert.NoError(t, err) + + time.Sleep(time.Millisecond) + v, err := c.Get(ctx, 1) - assert.EqualError(t, err, "evict expired value for key: evict value for key: 1: panic") + assert.ErrorIs(t, err, ErrNotFound) assert.Empty(t, v) }