go-pkgz / lcw

Loading Cache Wrapper
https://go-pkgz.umputun.dev/lcw/
MIT License
20 stars 5 forks source link

Modernize library, introduce generic interface #31

Closed umputun closed 2 years ago

umputun commented 2 years ago

With go 1.18 we can make use of generics to simplify the API. I mean the primary access methods

Get(key string, fn func() (interface{}, error)) (val interface{}, err error) // load or get from cache
Peek(key string) (interface{}, bool)                                        

this change will make usage much easier. In addition, we may want to update dependencies

note: this is a breaking change and should be released as v2. As of now, we don't even have v1, so we will need to tag the current one as v1.0.0 first

@paskal - let me know if you want to deal with this

paskal commented 2 years ago

Redis will work with generics only if keys would be restricted to something like fmt.Stringer or kept as strings: telegram-cloud-photo-size-2-5231252703647220141-y telegram-cloud-photo-size-2-5231252703647220142-y And then there are still problems. Value in Redis is a key which works only with interface{} but not V any. telegram-cloud-photo-size-2-5231252703647220167-y And keys are failing apart as well, as they are returned as a list of strings and it's impossible to put them into Invalidate function: telegram-cloud-photo-size-2-5231252703647220197-y

WDYT?

umputun commented 2 years ago

The first point makes sense but practically limits other caches significantly. Actually, it doesn't even need generics if all we want is stringrer as a key.

Regarding the second point, I don't really understand what the issue is because, as far as I recall, any is an alias for interface{} and is supposed to be interchangeable. Are you sure those errors you showed are real and not some bug on IDEA side?

paskal commented 2 years ago

It's Go itself:

~/lcw/v2 paskal/generics +5 !9 ❯ go test ./... -count 1
# github.com/go-pkgz/lcw/v2 [github.com/go-pkgz/lcw/v2.test]
./redis_cache.go:51:10: cannot use v (variable of type string) as type V in return statement
./redis_cache.go:61:10: cannot use v (variable of type string) as type V in return statement

interface{} means "anything", any means "put there anything but then stick to using that type". I don't think it's solvable for the Redis cache.

paskal commented 2 years ago

Also:

~/lcw/v2 paskal/generics +5 !9 ❯ go test ./... -count 1
# github.com/go-pkgz/lcw/v2 [github.com/go-pkgz/lcw/v2.test]
./redis_cache.go:143:14: invalid operation: cannot use type assertion on type parameter value data (variable of type V constrained by any)
image

The only way forward with generics is without Redis and with dropping the size check support. I think we shouldn't go in that direction.

umputun commented 2 years ago

interface{} means "anything", any means "put there anything but then stick to using that type". I don't think it's solvable for the Redis cache.

Well, this is surprising and even confusing as any is literally defined as an alias type for interface{}. This is from the source of go SDK, package builtin:

// any is an alias for interface{} and is equivalent to interface{} in all ways.
type any = interface{}` 
paskal commented 2 years ago

I can't make it work, unfortunately. The current state of generics doesn't allow this use case to work unless we'll have Redis library supporting generics.

umputun commented 2 years ago

ok, let's kill it for now

paskal commented 2 years ago

If anyone would be willing to try it, my latest attempt is stored in the paskal/generics branch.