dotnet / dotnet/extensions

[API Proposal]: Obsolete HybridCache compression options

Open
#6,594 5 comments 4 reactions 1 assignee Claimed by @svick View on GitHub
api-ready-for-review area-caching-hybrid
Dominant language
C#
Stars
3.2k
Forks
894
Avg merge
1d 12h
Merged PRs (30d)
23

Description

## Background and motivation (by @svick)

`HybridCacheOptions.DisableCompression` was introduced in anticipation of built-in compression support. It is not used by the default `HybridCache` implementation, and built-in compression is not planned for that implementation.

Its continued visibility implies that setting it changes cache behavior, which has caused user confusion.

The member should produce an obsoletion warning and be hidden from autocomplete. Applications requiring compression can implement it in their configured serializer.

## API proposal (by @svick)

```diff
namespace Microsoft.Extensions.Caching.Hybrid;

public class HybridCacheOptions
{
+ [Obsolete(
+ "HybridCache does not provide built-in compression, so this option doesn't do anything.",
+ DiagnosticId = "EXTOBS0003",
+ UrlFormat = "https://aka.ms/dotnet-extensions-warnings/{0}")]
+ [EditorBrowsable(EditorBrowsableState.Never)]
public bool DisableCompression { get; set; }
}
```

The proposed diagnostic identifier is the next currently available identifier and can be adjusted during implementation if necessary. The proposal is to make the obsoletion a warning, not an error, since the code works; the option is just misleading.

## API usage (by @svick)

Existing code produces an obsoletion warning:

```csharp
services.AddHybridCache(options =>
{
options.DisableCompression = true; // EXTOBS0003
});
```

Applications requiring compression should register a serializer that performs compression.

## Alternative designs (by @svick)

### Also obsolete `HybridCacheEntryFlags.DisableCompression`

```diff
namespace Microsoft.Extensions.Caching.Hybrid;

[Flags]
public enum HybridCacheEntryFlags
{
+ [Obsolete(
+ "HybridCache does not provide built-in compression, so this flag doesn't do anything.",
+ DiagnosticId = "SYSLIB0066",
+ UrlFormat = "https://aka.ms/dotnet-warnings/{0}")]
+ [EditorBrowsable(EditorBrowsableState.Never)]
DisableCompression
}
```

This would consistently hide both compression-related members. However, `HybridCacheEntryFlags` is part of the shared `HybridCache` abstraction, not the default implementation. Another implementation might already use `DisableCompression`, or might reasonably support it in the future. Obsoleting the flag would therefore constrain all implementations based only on a limitation of the default implementation, so this alternative is undesirable.

Though note that no known 3rd party implementation of `HybridCache` uses this flag.

## Risks (by @svick)

Adding `ObsoleteAttribute` is a source-breaking change for projects treating warnings as errors. This is intentional: `HybridCacheOptions.DisableCompression` has no effect in the default implementation.

Original issue

### Description

Currently in HybridCache there is an option [DisableCompression](https://learn.microsoft.com/en-us/dotnet/api/microsoft.extensions.caching.hybrid.hybridcacheoptions.disablecompression?view=net-9.0-pp#microsoft-extensions-caching-hybrid-hybridcacheoptions-disablecompression), which suggests that by default HybridCache might do compression (presumably before sending data to a distributed cache).

However searching through the implementation I can't see any evidence of this, is the setting a sort of aspiration flag for something that is on the roadmap?

### Reproduction Steps

Set `DisableCompression` to `true` or `false`

### Expected behavior

Some behaviour changes

### Actual behavior

Nothing changes

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.