Avoid closure in IConnectionBuilder.Use

[divyeshio]

Avoid closure in IConnectionBuilder.Use

• You've read the Contributor Guide and Code of Conduct.
• You've included unit or integration tests for your change, where applicable.
• You've included inline docs for your change, where applicable.
• There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Add new Use connectionbuilder extension method

Description

Add a new IConnectionBuilder.Use overload that requires users to pass the ConnectionContext to the next function which will save allocations over the previous extension method.

Fixes #46533

[ghost]

Thanks for your PR, @divyeshio. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

[divyeshio]

@divyeshio please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@dotnet-policy-service agree [company="{your company}"]

Options:

• (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.

@dotnet-policy-service agree

• (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.

@dotnet-policy-service agree company="Microsoft"

Contributor License Agreement

@dotnet-policy-service agree

[JamesNK]

Could this change create ambiguous method errors when building?

These two Use overloads take a delegate with two arguments:

public static IConnectionBuilder Use(
    this IConnectionBuilder connectionBuilder,
    Func<ConnectionContext, Func<Task>, Task> middleware)

And

public static IConnectionBuilder Use(
    this IConnectionBuilder connectionBuilder,
    Func<ConnectionContext, ConnectionDelegate, Task> middleware)

It looks like implicit lambda args won't work anymore:

builder.Use((context, next) =>
{
    return Task.CompletedTask;
});

(I haven't tested this)

[divyeshio]

Similar changes for middleware have been already merged. This has been discussed at #31463

[JamesNK]

Ok! Are there any places the existing overload is used in aspnetcore that can be changed to the new method?

[divyeshio]

I tried to look for any references for previous method but couldn't find in anywhere including samples, tests.

Note: This is my first ever contribution, I might be missing something.

[amcasey]

Ok! Are there any places the existing overload is used in aspnetcore that can be changed to the new method?

Maybe here?

[amcasey]

This does look pretty analogous to #31784, so @BrennanConroy might have thoughts.

[Tratcher]

There should at least be a test you can use this in.
https://github.com/dotnet/aspnetcore/blob/main/src/Servers/Kestrel/test/InMemory.FunctionalTests/ConnectionMiddlewareTests.cs

[divyeshio]

There should at least be a test you can use this in. https://github.com/dotnet/aspnetcore/blob/main/src/Servers/Kestrel/test/InMemory.FunctionalTests/ConnectionMiddlewareTests.cs

None of the tests use this extension method

[Tratcher]

There should at least be a test you can use this in. https://github.com/dotnet/aspnetcore/blob/main/src/Servers/Kestrel/test/InMemory.FunctionalTests/ConnectionMiddlewareTests.cs

None of the tests use this extension method

I meant that you can copy an existing test and adapt it to use these extensions.

[JamesNK]

We want a test to use the new method to ensure it works correctly.

[amcasey]

Seems sensible to me. Good to go @Tratcher @JamesNK ?

[Tratcher]

Thanks