feat: export RedisStorage.Client field

[dunglas]

I have a custom Caddy module where I would like to reuse the existing Redis connection to run custom queries.

It's currently possible using (unreliable) magic like this:

s := ctx.Storage().(*storageredis.RedisStorage)

rs := reflect.ValueOf(s).Elem()
rf := rs.Field(18)
client = reflect.NewAt(rf.Type(), unsafe.Pointer(rf.UnsafeAddr())).Elem().Interface().(redis.UniversalClient)

This works, but this isn't very reliable. Exporting the "client" field would make such usage more easy and reliable:

s := ctx.Storage().(*storageredis.RedisStorage)
client = s.Client

I also had to update the dependencies to make the tests green.

[pberkel]

Hi @dunglas, thanks for your contribution, I understand what you are trying to do but am hesitant to publicly expose internal structures of this storage module that might restrict the ability to make future modifications (i.e. upgrading the Redis client or swapping it with an entirely different library could introduce breaking changes). Is the module you are developing intended to be publicly sharable or private?

[dunglas]

Hi @pberkel, thanks for the quick review!

Indeed this is an issue, especially with this Redis library that has a long history of releasing major versions frequently. Maybe could we done one the following to mitigate this issue:

• mark the new exported field as "experimental" in the documentation block and explain in a comment that there is no BC promise for it (Caddy itself already does this for several symbols)
• keep the field unexported but add a getter with the signature getClient() any. The user will have to do a type assertion to cast the client to the actual type, so this is future-proof

I need this for a private project, but I think that the use case of reusing the existing Redis connection pool is common.

[francislavoie]

I think a getter with any is probably better, yeah. I would go with that.

I don't think the go.mod needs any changes here (aside from go-redis I guess, that's fine), as-is this would make the plugin no longer build with versions of Caddy older than 2.8.4 which is unfortunate. Some people have reasons to be using older Caddy versions in transition periods. Best to leave that as-is unless you actually need to use new API surface in Caddy. xcaddy will choose the correct Caddy version otherwise.

[pberkel]

Thanks for the advice @francislavoie and apologies for the slow response @dunglas. If you could refactor your solution based on @francislavoie's comments, I'll merge it.