eko / gocache

☔️ A complete Go cache library that brings you multiple ways of managing your caches
https://vincent.composieux.fr/article/i-wrote-gocache-a-complete-and-extensible-go-cache-library/
MIT License
2.42k stars 192 forks source link

bigcache GetWithTTL not supported #172

Closed melpomene closed 1 year ago

melpomene commented 1 year ago

The bigcache implementation of GetWithTTL is just a hard coded 0 duration but with no indication in API it will not work as intended.

I suggest instead returning an error that lets the user know that the method is in fact not supported and it should not be relied upon.

Current behavior:

// GetWithTTL returns data stored from a given key and its corresponding TTL
func (s *BigcacheStore) GetWithTTL(ctx context.Context, key any) (any, time.Duration, error) {
    item, err := s.Get(ctx, key)
    return item, 0, err
}

Proposed change:

ErrNotImplemented = errors.New("Method not implemented for codec")

// GetWithTTL returns data stored from a given key and its corresponding TTL
func (s *BigcacheStore) GetWithTTL(ctx context.Context, key any) (any, time.Duration, error) {
    return nil, 0, store.ErrNotImplemented
}
eko commented 1 year ago

Hi @melpomene,

Thank you for your feedback.

I agree that this API could be marked as not implemented and precise that the Get() method have to be use.

Please feel free to submit a pull request to change this behavior.

Thanks

melpomene commented 1 year ago

Thank you @eko. Submitted a pull request: #194

exherb commented 1 year ago

https://github.com/eko/gocache/blob/cb0dccd38eabab63faa1f01331aab0015bbf10a5/lib/cache/chain.go#L63C3-L63C3

BUT when chain bigcache with Redis, it broker our services.

eko commented 1 year ago

@melpomene I think @exherb is right.

I don't have this in mind but we should keep this method implemented, whereas throwing an error.