Update IDistributedCache to account for nullability

[JunTaoLuo]

Background and Motivation

The Get and GetAsync methods of the IDistributedCache interface do not match the intended nullability in terms of return type. As per the API documentation:

/// <returns>The located value or null.</returns>

I noticed this when we were working on adding community contributed APIs, see: dotnet/aspnetcore#32266 (comment).

The request here is to update the API in the IDistributedCache interface.

Proposed API

namespace Microsoft.Extensions.Caching.Distributed
{
  public interface IDistributedCache
  {
-    byte[] Get(string key);
+    byte[]? Get(string key);
-    Task<byte[]> GetAsync(string key, CancellationToken token = default(CancellationToken));
+    Task<byte[]?> GetAsync(string key, CancellationToken token = default(CancellationToken));
  }

Usage Examples

The return type will be update:

- byte[] data = cache.Get(key);
+ byte[]? data = cache.Get(key);

Alternative Designs

The alternative is to keep the current API. This presents a difficulty when adding nullability checks in implementations of IDistributedCache see dotnet/aspnetcore#32266.

Risks

Implementations of the IDistributedCache interface will need to be updated to account for the nullability changes proposed here.

[ghost]

Tagging subscribers to this area: @eerhardt, @maryamariyan, @michaelgsharp
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and Motivation

The Get and GetAsync methods of the IDistributedCache interface do not match the intended nullability in terms of return type. As per the API documentation:

/// <returns>The located value or null.</returns>

I noticed this when we were working on adding community contributed APIs, see: dotnet/aspnetcore#32266 (comment).

The request here is to update the API in the IDistributedCache interface.

Proposed API

namespace Microsoft.Extensions.Caching.Distributed
{
  public interface IDistributedCache
  {
-    byte[] Get(string key);
+    byte[]? Get(string key);
-    Task<byte[]> GetAsync(string key, CancellationToken token = default(CancellationToken));
+    Task<byte[]?> GetAsync(string key, CancellationToken token = default(CancellationToken));
  }

Usage Examples

The return type will be update:

- byte[] data = cache.Get(key);
+ byte[]? data = cache.Get(key);

Alternative Designs

The alternative is to keep the current API. This presents a difficulty when adding nullability checks in implementations of IDistributedCache see dotnet/aspnetcore#32266.

Risks

Implementations of the IDistributedCache interface will need to be updated to account for the nullability changes proposed here.

Author:	JunTaoLuo
Assignees:	-
Labels:	api-suggestion, area-Extensions-Caching
Milestone:	6.0.0

[JunTaoLuo]

FYI @ericstj @maryamariyan, it seems like nullability is not enabled for Microsoft.Extensions.Caching.Abstractions package. Although only the IDistributedCache API is blocking the merging of the PR in aspnetcore, maybe it's a good opportunity to review the other APIs in the package and enable nullability for the package?

[eerhardt]

See #43605. Almost none of the Microsoft.Extensions.* libraries have nullability enabled.

[JunTaoLuo]

Is there an estimate of which preview when Caching.Abstractions will be annotated? We can't enable nullability in our package until it's enabled here first.

[JunTaoLuo]

FYI, we don't need all of Caching.Abstractions to be updated with nullability enabled. To unblock us, we just need to make the tactical API changes I proposed.

[ericstj]

cc @jeffhandley
Annotating Extensions (as well as other dotnet/runtime packages) isn't committed for 6.0 but we would like to see progress on it, it doesn't look like you're blocked on it, so I'm not seeing a need to raise priority or track as an independent issue. Let me know if I'm misunderstanding.

[maryamariyan]

Is there an estimate of which preview when Caching.Abstractions will be annotated?

For Caching.Abstractions, looks like it depends on primitives. So it would be only a matter of enabling nullables for these two libraries

[JunTaoLuo]

it doesn't look like you're blocked on it

It is blocking us from enabling nullability in Caching.SqlServer and Caching.StackExchangeRedis as part of dotnet/aspnetcore#5680.

[ericstj]

What's the nullable goal for ASP.NET in 6.0? Are you planning to enable it for all apps? You'll need more than just these assemblies annotated. Aren't there other assemblies that ship in the ASP.NET ref-pack which aren't annotated yet?

[eerhardt]

We've reached feature complete for .NET 6. Moving to 7.

[maryamariyan]

Looking again at #43605, all dependencies to Microsoft.Extensions.Caching.Abstractions are nullable annotated. So would be good to fix this next.