ZiggyCreatures / FusionCache

FusionCache is an easy to use, fast and robust hybrid cache with advanced resiliency features.
MIT License
1.65k stars 87 forks source link

[FEATURE] Improve nullable for GetOrSet with non-null factory #202

Closed angularsen closed 6 months ago

angularsen commented 6 months ago

Problem

After upgrading from 0.13 to 0.26, we're getting nullable warnings on all GetOrSetAsync usages.

Converting null literal or possible null value into non-nullable type

It complains that the async factory argument must return a nullable TValue?, and that the resulting value of GetOrSetAsync is now also nullable.

Our pattern has been to let factories return non-null values and throw on error, so the caller of GetOrSetAsync also would assume non-null or exception. We now have to either suppress a lot of warnings or handle null values we don't expect to get.

I realize this changed a long time ago, but is this nullability intentional and necessary? Should I consume this in a different way to avoid this?

The nullability for GetOrSetAsync changed in v0.16: https://github.com/ZiggyCreatures/FusionCache/commit/988612d741c5003f9f436e4100b4b06c8168820b#diff-e43fd00541c067193a3e4713140acc31de4af7c49fcc3061b74e11da63d974c3R272

Solution

I don't know yet, just checking in here first.

Alternatives

Workaround is to append ! to all GetOrSetAsync() usages.

Additional context

Example:

    // Before
    MyObject cachedImages = await _cache.GetOrSetAsync("mykey",
        async ct2 => await GetValueAsync(ct2),
        token: ct);

    private async Task<MyObject> GetValueAsync(CancellationToken ct)
    {
        // Get the value and return it
    }
    // After
    MyObject cachedImages = (await _cache.GetOrSetAsync("mykey",
        async ct2 => await GetValueAsync(ct2),
        token: ct))!; // Suppress nullable warning

    // Return type MyObject also needs to be nullable here, but the above suppression also suppresses this.
    private async Task<MyObject?> GetValueAsync(CancellationToken ct)
    {
        // Get the value and return it
    }
jodydonetti commented 6 months ago

Hi @angularsen

I realize this changed a long time ago, but is this nullability intentional and necessary?

Eh, good question: this is linked to the evolution of nullable reference types and the related static analysis along the different c# versions. I tried to adjust the annotations to better handle it with each new version, because what the compiler allowed me to do and was able to infer changed at every release (the first version had problems with generics without where T : class/struct constraints, for example).

Now that you've mentioned it I just tried to update the signature of each GetOrSet[Async] overload in the codebase and... I have to say it seems to be working.

I think I'll be able to publish a v1.0.0-preview2 in a couple of days with the last juicy things I've cooked before going full v1.0: would you be so kind to then try it out and see if it works well?

Sounds good?

angularsen commented 6 months ago

Absolutely, will do. I am back on the code base where we use FusionCache so I can quickly give it a whirl.

jodydonetti commented 6 months ago

Hi @angularsen , I just released preview2.

Please give it a try, thanks!

angularsen commented 6 months ago

Happy to report that all nullability issues are now gone, and working as expected πŸŽ‰ Some initial testing of functionality in 1.0.0-preview2 seems good so far, I'll report back if I find anything.

Thanks for all your work, your response time and can-do attitude is simply impressive πŸ™‡

jodydonetti commented 6 months ago

Thanks to you @angularsen , happy it worked well. And you did it on a Sunday no less, really grateful πŸ™

jodydonetti commented 6 months ago

Hi all, I've finally released v1.0 πŸ₯³

Feels a little unreal tbh, but yep: v1.0 is out.

angularsen commented 6 months ago

That's awesome, congratulations! @jodydonetti

In my eyes, this has been so solid it's been a couple of major versions already by now πŸ˜„ We've had it in production for a long time without any hiccups that trace back to FusionCache really.

The 1.0 release changelog is a good one and we've run the pre-release also without any issues so far. I've tried several projects before and this is the best caching solution for .NET, period!

I'm happy to contribute by the way. I don't have as much spare time as I used to, but at least for stuff I run into or suggest myself I'm willing to put in some effort to help out. In my past few requests you've pretty much just nailed it immediately so there has been no need πŸ˜… and I'm grateful for that! Just know that the option is there and you don't have to do everything yourself, it can be quite healthy in open source I think.

Cheers!

jodydonetti commented 6 months ago

That's awesome, congratulations! @jodydonetti

Thanks!

In my eyes, this has been so solid it's been a couple of major versions already by now πŸ˜„ We've had it in production for a long time without any hiccups that trace back to FusionCache really.

Really happy about this, and thanks for letting me know: sometimes I wonder if no response from the community means it's all good or it means that it's not being used (even though download numbers are good). Here and there people told me that they had nothing to say since everything just worked, which is awesome and actually an objective from the get to, but still it can be hard for me to know for real without somebody telling me. So yeah, thanks for letting me know!

The 1.0 release changelog is a good one and we've run the pre-release also without any issues so far. I've tried several projects before and this is the best caching solution for .NET, period!

What can I say, I'm really flattered 😊

I'm happy to contribute by the way. I don't have as much spare time as I used to, but at least for stuff I run into or suggest myself I'm willing to put in some effort to help out. In my past few requests you've pretty much just nailed it immediately so there has been no need πŸ˜… and I'm grateful for that! Just know that the option is there and you don't have to do everything yourself, it can be quite healthy in open source I think.

Again, thanks: those are wonderful words, and your support is felt! I like what I'm doing with FusionCache, it's my passion project, so probably I run towards new features or fixes because I feel good doing them and it's not something I feel the pressure of but the other way around, it's a way for me to relax and try to do something good. But yeah, the help from the community is also wonderful and I know some of you out there are here to help, and I can assure you it's very much appreciated.

Cheers to v1.0 🍷