Runtime: Add DecompressionMethods.All?

Created on 20 Apr 2018  路  10Comments  路  Source: dotnet/runtime

The addition of System.Net.DecompressionMethods.Brotli was recently approved. I have been writing DecompressionMethods.Deflate | DecompressionMethods.GZip in my source automatically; since I don't want to exclude the possibility of Brotli, I'm going to be writing DecompressionMethods.Deflate | DecompressionMethods.GZip | DecompressionMethods.Brotli.

I am very interested in an All member, since that sums up exactly what I care about.

namespace System.Net
{
    [Flags]
    public enum DecompressionMethods
    {
        None = 0,
        GZip = 1,
        Deflate = 2,
        Brotli = 4,
+       All = ~None
    }
}

Would you please review this possibility?

Usage

The enum is get/set via property HttpClientHandler.AutomaticDecompression. Default is None. All value will ease up opt-in into any decompression.

Default value is None is set in all handlers via internal HttpHandlerDefaults.DefaultAutomaticDecompression:
https://github.com/dotnet/corefx/blob/9cde05ff84d118a9cc5629b85d4a319dd32172ae/src/Common/src/System/Net/Http/HttpHandlerDefaults.cs#L19

[EDIT] Added details by @karelz

Hackathon api-approved area-System.Net up-for-grabs

Most helpful comment

@stephentoub Either it had not occurred to me at all, or I was hesitant that setting all bits would not be officially supported. (Potential future InvalidEnumArgumentException or new modifier flags which I wouldn't want to opt into.) With this API change, ~0 becomes safe and documented.

All 10 comments

Does it mean the implementation will try all decompression methods on received data? Something else?

@kasper3 No, the client sends Accept-Encoding and looks at the response's Content-Encoding.

Sounds reasonable to me. Marking as ready for review.
cc @dotnet/ncl (in case there are implications I missed)

I have been writing DecompressionMethods.Deflate | DecompressionMethods.GZip in my source automatically
since I don't want to exclude the possibility of Brotli, I'm going to be writing DecompressionMethods.Deflate | DecompressionMethods.GZip | DecompressionMethods.Brotli

Out of curiosity, why have you been writing that rather than ~DecompressionMethods.None?

@stephentoub Either it had not occurred to me at all, or I was hesitant that setting all bits would not be officially supported. (Potential future InvalidEnumArgumentException or new modifier flags which I wouldn't want to opt into.) With this API change, ~0 becomes safe and documented.

Video

Seems reasonable.

This should be simple to implement. See PR adding the Brotli enum value in PR dotnet/corefx#29729.

@karelz, isn't Brotli = 4 also in the same (2.2-only) boat?

Yes it is, so it will be even simpler than I expected. Check PR dotnet/corefx#29729.

@karelz I'd like to take this one for the Hackathon.

Was this page helpful?
0 / 5 - 0 ratings