Added IConnectionSocketFeature for exposing underlying Socket!

[ShreyasJejurkar]

As part of #28401

[ShreyasJejurkar]

Am yet to write tests, but am not able to find a file where it should be written! Waiting for some help on the same!

[ShreyasJejurkar]

Should I merge the following two lines into one line?

aspnetcore/src/Servers/Kestrel/Transport.Sockets/src/Internal/SocketConnection.cs

Line 47 in ea59c5b

_socket = socket;

aspnetcore/src/Servers/Kestrel/Transport.Sockets/src/Internal/SocketConnection.cs

Line 51 in ea59c5b

Socket = socket;

[halter73]

Am yet to write tests, but am not able to find a file where it should be written! Waiting for some help on the same!

There aren't any Socket-transport-specific functional tests yet since all functional tests live in the shared https://github.com/dotnet/aspnetcore/tree/main/src/Servers/Kestrel/test/FunctionalTests directory so they can be run against both the Socket and libuv transports. A shared test won't work for this since it relies on the connection having an underlying System.Net.Socket.

I think copying ConnectionClosedTokenFiresOnServerAbort into a new test class in https://github.com/dotnet/aspnetcore/tree/main/src/Servers/Kestrel/test/Sockets.FunctionalTests, but using context.Features.Get<IConnectionSocketFeature>().Socket!.Dispose() instead of context.Abort() might work.

[ghost]

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[davidfowl]

@MCCshreyas do you need anything to help finish this?

[ShreyasJejurkar]

Hi, @davidfowl I have put this comment! Can you please check what needs to be done! @halter73 asked to remove virtual, but then I am not able to override it derive class!

#30803 (comment)

[ShreyasJejurkar]

Only that thing is remaining from my point of view!

[halter73]

I responded to the comment about the virtual.

We should add a test before merging, but I can take it from here since there's no similar test to really use as an example.

While I'm at it, I'll clean up the shared code generation and get this though API review before merging. Thanks for the PR!

[ShreyasJejurkar]

Hi @halter73 sorry for later response. I will get that comment addressed. Last time I tried to follow your instructions about tests, but last time the code wasn't compiling because of that virtual error. If possible I will write test as well with upcoming commit.

[ShreyasJejurkar]

There are some recent changes made in main branch, on how the code gets generated in TransportConnection.Generated.cs, am facing some issues while pulling the latest SDK, so I edited that file manually. I hope it's ok to do so!

[davidfowl]

Thanks for your hard work @MCCshreyas! There are a few changes we need to make here since we decided to make the Socket field non-nullable. I'd like to take over to make some changes to the code generator.

[davidfowl]

Closed in favor of #31588. Kept your commits so you get credit! I'll likely squash them though.