Hybrid Cache silently fails to correctly handle cached tuples

[martincostello]

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

It might be the case it's a feature not a bug, but it seems like a foot-gun, so logging an issue.

I just migrated an existing app that just uses IMemoryCache to HybridCache to just have a play around with it (and because new stuff is fun), and discovered there was an issue with some existing piece of code that caches a collection of tuples (code). A tuple was used solely out of convenience/laziness to avoid defining a custom type when condensing a large object down to just the three values needed from it.

The code caching the tuple silently corrupts the value returned by the item factory before it is returned to the caller.

Here's the value in the debugger immediately before returning in the delegate that created the cached item (code):

Here's the value in the debugger in the calling method returned by GetOrCreateAsync() (code):

The cache has persisted the fact the collection contains 4 items, but all of the items have the default value.

I can imagine that there's probably something about Tuple<,,> that makes it difficult/dangerous for JSON serialization, but the silent corruption of the data is worrying, particularly as it works with the in-memory cache so might be something people miss if migrating (e.g. in anticipation of adding a secondary cache later).

I caught the problem via my existing tests as stubbed HTTP calls wouldn't match due to the cached values causing the request parameters to be different, so instead the tests fail due to an exception. Yay tests!

In adopting the cache I had to change some objects from being cached directly as objects to a string instead as the object didn't have a parameterless constructor, but I found that out pretty easily as the cache threw an exception telling me I couldn't do that so was much more obvious.

It feels like either this should Just Work™, or throw an exception as to being an unsupported use case.

It was pretty easy to avoid the problem once identified by defining a custom record and caching that instead.

Expected Behavior

Either:

• An exception is thrown that this isn't a supported use case, or:
• The tuple values are correctly (de)serialized, or:
• Something in the docs calling this out? I did read HybridCache library in ASP.NET Core first, but that only refers to serialization for the secondary cache case, which I'm not using.

Steps To Reproduce

1. Clone martincostello/costellobot@fb71d73
2. Run build.ps1 in the root of the repository (or open the solution in Visual Studio and run the Other_Pull_Requests_Are_Approved test).

Exceptions (if any)

None.

.NET Version

9.0.201

Anything else?

No response

[BrennanConroy]

Move to https://github.com/dotnet/extensions?

[martincostello]

I wasn't sure where to put it tbh, but here seemed to still have more activity for bugs than there for Hybrid Cache so I went with this repo (unless I was subconsciously searching for Distributed Cache and thus didn't find activity).

[BrennanConroy]

@mgravell

[mgravell]

Yes, we should move to extensions.

On Monday I'll post a workaround to configure the serializer to handle this case. Note that value-tuples should already work (with or without names), so replacing Tuple.Create(a,b,c) with simply (a,b,c) may also be a quick workaround. I'll aim to have the library detect this, however.

The short version here being: "default serializer is STJ, and default STJ doesn't love this type".

[mgravell]

I've checked, and we do have an existing passing test for Tuple.Create; I will check what has happened in the specific example on Monday. Edit, oh it is root vs child. Hmmm, that's vexing. I'll need to think of the best approach here.

[martincostello]

It is already using (a, b, c) rather than Tuple.Create(a, b, c) (ref):

.Select((p) => (p.Owner.Login, p.Name, p.Id))

Maybe it's something to do with the fact it's in a collection?

[mgravell]

Yeah, I realised that - see edit; we check the root type - I'll need to think whether we should do a deep search for tuples as inner members. I guess we must.

[mgravell]

Proposed fix ^^^; is a bit more complicated, but we only run that code once per type, and it should avoid a lot of head-scratching.