Rename min rate APIs for consistency

[cesarblum]

We should do this before RTM:

• Rename KestrelServerLimits.RequestBodyMinimumDataRate to MinRequestBodyDataRate (consistent with MaxRequestBodySize, and other limits that all start with “Max”)
• Rename IHttpRequestBodyMinimumDataRateFeature to IHttpMinRequestBodyDataRateFeature (consistent with IHttpMaxRequestBodySizeFeature)
• Rename the rate parameter of the MinimumDataRate ctor to bytesPerSecond, so that the rate’s unit of measurement is clear

[cesarblum]

@muratg

[muratg]

@Eilon @DamianEdwards @shirhatti FYI -- proposed breaking change between preview2 and RTM to make the names more consistent.

[Eilon]

Can someone show the full list of before/after values? Hard to tell what's consistent etc.

[muratg]

@CesarBS could you make a table?

[cesarblum]

KestrelServerLimits

Before	After
MaxResponseBufferSize	MaxResponseBufferSize
MaxRequestBufferSize	MaxRequestBufferSize
MaxRequestLineSize	MaxRequestLineSize
MaxRequestHeadersTotalSize	MaxRequestHeadersTotalSize
MaxRequestHeaderCount	MaxRequestHeaderCount
MaxRequestBodySize	MaxRequestBodySize
RequestBodyMinimumDataRate	MinRequestBodyDataRate
MaxConcurrentConnections	MaxConcurrentConnections
MaxConcurrentUpgradedConnections	MaxConcurrentUpgradedConnections
KeepAliveTimeout	KeepAliveTimeout
RequestHeadersTimeout	RequestHeadersTimeout

Features

Before	After
IHttpMaxRequestBodySizeFeature	IHttpMaxRequestBodySizeFeature
IHttpRequestBodyMinimumDataRateFeature	IHttpMinRequestBodyDataRateFeature

MinimumDataRate

Before

public class MinimumDataRate
{
    public MinimumDataRate(double rate, TimeSpan gracePeriod)
    {
        ...
    }

    public double Rate { get; }

    public TimeSpan GracePeriod { get; }
}

After

public class MinimumDataRate
{
    public MinimumDataRate(double bytesPerSecond, TimeSpan gracePeriod)
    {
        ...
    }

    public double BytesPerSecond { get; }

    public TimeSpan GracePeriod { get; }
}

[dougbu]

I suggest we also look at MinimumDataRate's usability. Configuring rates requires newing up an instance and its Microsoft.AspNetCore.Server.Kestrel.Core.Features namespace is obscure, increasing the number of usings in Program and/or Startup classes.

Could either move MinimumDataRate' to a common namespace e.g. Microsoft.AspNetCore.Hosting or make the BytesPerSecond and GracePeriod properties settable and lazy-load MinRequestBodyDataRate in KestrelServerLimits and IHttpMinRequestBodyDataRateFeature. The settable / lazy-load option has the advantage of reducing the boilerplate users write.

E.g. current default setting:

using Microsoft.AspNetCore.Server.Kestrel.Core.Features;

    var dataRate = new MinimumDataRate(rate: 100, gracePeriod: TimeSpan.FromSeconds(2));
    limits.RequestBodyMinimumDataRate = dataRate;

Proposed new default setting, second alternative:

    limits.RequestBodyMinimumDataRate.BytesPerSecond = 100;
    // Perhaps optional. Would probably need a reasonable default for `GracePeriod` anyway.
    limits.RequestBodyMinimumDataRate.GracePeriod = TimeSpan.FromSeconds(2);

[cesarblum]

When we had the discussion about having a MinimumDataRate type, it was because there was an additional property in the feature and we wanted to make it clear that the two properties related to the rate should belong together.

But now that it's just the rate, we should remove MinimumDataRate from the feature and just put the properties in the feature itself, and change KestrelServerLimits like @dougbu is suggesting.

[cesarblum]

Should also make IHttpMinRequestBodyDataRate configurable only prior to any reads, for parity with how IHttpMinRequestBodySize works.

[Eilon]

I'm not sure we need to worry much about how many namespaces are imported for a very obscure server feature. The importance of this feature is that it exists and has smart defaults. For the few people in the world who would override the defaults, I'm not concerned how many namespace they import. VS will add the namespace for you anyway. Or, just make it a struct, and then you don't need to worry about it being null.

Having said all that, I think the new suggested names in general are fine.

I'd like @DamianEdwards / @davidfowl / @blowdart to share any thoughts they have on those before committing to anything.

[blowdart]

I'm good with the naming changes.