Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WithKeepTTL #57

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 26 additions & 7 deletions cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ type ItemOption func(*itemOptions)
type itemOptions struct {
expiration time.Time // default none
referenceCount int
keepTTL bool
}

// WithExpiration is an option to set expiration time for any items.
Expand All @@ -82,6 +83,17 @@ func WithExpiration(exp time.Duration) ItemOption {
}
}

// WithKeepTTL is the expiration time to keep existing keys when calling Set. By default, it is replaced by the new time or never expires.
func WithKeepTTL(_keep ...bool) ItemOption {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this better?

Suggested change
func WithKeepTTL(_keep ...bool) ItemOption {
func WithKeepTTL(keep bool) ItemOption {

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both are feasible, but I prefer to input less code because function names already have a certain intention

return func(o *itemOptions) {
if len(_keep) > 0 {
o.keepTTL = _keep[0]
} else {
o.keepTTL = true
}
}
}

// WithReferenceCount is an option to set reference count for any items.
// This option is only applicable to cache policies that have a reference count (e.g., Clock, LFU).
// referenceCount specifies the reference count value to set for the cache item.
Expand All @@ -94,7 +106,7 @@ func WithReferenceCount(referenceCount int) ItemOption {
}

// newItem creates a new item with specified any options.
func newItem[K comparable, V any](key K, val V, opts ...ItemOption) *Item[K, V] {
func newItem[K comparable, V any](key K, val V, opts ...ItemOption) (*Item[K, V], *itemOptions) {
o := new(itemOptions)
for _, optFunc := range opts {
optFunc(o)
Expand All @@ -104,7 +116,7 @@ func newItem[K comparable, V any](key K, val V, opts ...ItemOption) *Item[K, V]
Value: val,
Expiration: o.expiration,
InitialReferenceCount: o.referenceCount,
}
}, o
}

// Cache is a thread safe cache.
Expand Down Expand Up @@ -231,8 +243,11 @@ func (c *Cache[K, V]) GetOrSet(key K, val V, opts ...ItemOption) (actual V, load
item, ok := c.cache.Get(key)

if !ok || item.Expired() {
item := newItem(key, val, opts...)
c.cache.Set(key, item)
replaceItem, o := newItem(key, val, opts...)
if o.keepTTL && ok && !replaceItem.hasExpiration() {
replaceItem.Expiration = item.Expiration
}
c.cache.Set(key, replaceItem)
return val, false
}

Expand Down Expand Up @@ -273,9 +288,13 @@ func (c *Cache[K, V]) DeleteExpired() {
func (c *Cache[K, V]) Set(key K, val V, opts ...ItemOption) {
c.mu.Lock()
defer c.mu.Unlock()
item := newItem(key, val, opts...)
item, o := newItem(key, val, opts...)
if item.hasExpiration() {
c.expManager.update(key, item.Expiration)
} else if o.keepTTL {
if old, has := c.cache.Get(key); has {
item.Expiration = old.Expiration
}
}
c.cache.Set(key, item)
}
Expand Down Expand Up @@ -334,7 +353,7 @@ func (nc *NumberCache[K, V]) Increment(key K, n V) V {
defer nc.nmu.Unlock()
got, _ := nc.Cache.Get(key)
nv := got + n
nc.Cache.Set(key, nv)
nc.Cache.Set(key, nv, WithKeepTTL())
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not intend to make destructive changes to this library to ensure backward compatibility unless there is a good reason to do so. Why is this change necessary? Please provide a clear reason.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default rules for redis/memcached and more cache libraries... We do not recommend using different designs to avoid thinking burdens

return nv
}

Expand All @@ -345,6 +364,6 @@ func (nc *NumberCache[K, V]) Decrement(key K, n V) V {
defer nc.nmu.Unlock()
got, _ := nc.Cache.Get(key)
nv := got - n
nc.Cache.Set(key, nv)
nc.Cache.Set(key, nv, WithKeepTTL())
return nv
}
42 changes: 42 additions & 0 deletions cache_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,48 @@ func TestDeleteExpired(t *testing.T) {
t.Errorf("want %d items but got %d", want, got)
}
})

t.Run("keepTTL", func(t *testing.T) {

t.Run("incr must keep ttl", func(t *testing.T) {
defer restore()
c := NewNumber[string, int]()

c.Set("1", 10, WithExpiration(10*time.Millisecond))
c.Increment("1", 20)
nowFunc = func() time.Time {
return now.Add(30 * time.Millisecond).Add(time.Millisecond)
}
c.DeleteExpired()
if c.Len() != 0 {
t.Fail()
}
})

testCase := func(t *testing.T, wantN int, opts ...ItemOption) {
defer restore()
c := NewNumber[string, int]()

c.Set("1", 10, WithExpiration(10*time.Millisecond))
c.Set("1", 20, opts...)
nowFunc = func() time.Time {
return now.Add(300 * time.Millisecond).Add(time.Millisecond)
}
c.DeleteExpired()
if c.Len() != wantN {
t.Fail()
}
}
t.Run("must forever when default set", func(t *testing.T) {
testCase(t, 1)
})
t.Run("must expired when KeepTTL=true", func(t *testing.T) {
testCase(t, 0, WithKeepTTL())
})
t.Run("must forever when KeepTTL=false", func(t *testing.T) {
testCase(t, 1, WithKeepTTL(false))
})
})
}

func max(x, y int) int {
Expand Down