Update C# code for #29448
Update C# code for #29448
- [x] You've read the Contributor Guide and Code of Conduct.
- [x] You've included unit or integration tests for your change, where applicable.
- [x] You've included inline docs for your change, where applicable.
- [x] 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.
Summary of the changes (Less than 80 chars)
Description
When #29448 was merged, there were no static lambdas in C# and AOT was not a thing.
A new struct UriComponents has been introduced to encapsulate the components of a URI: Scheme, Host, PathBase, Path, Query, and Fragment to avoid a dependency on ValueTuple{6}. The string.Create method call has been updated to use a new lambda function that leverages the UriComponents struct to copy each component of the URI into the buffer, adjusting the buffer slice as needed. This function also handles the case where both PathBase and Path components have a slash, removing the trailing slash from PathBase to avoid duplication.
Finally, the BuildAbsolute method has been updated to use the new UriComponents struct and the updated string.Create call.
Related to #29448 and #28905
CI failures look legit. Probably need some #ifs.
A new struct UriComponents has been introduced to encapsulate the components of a URI: Scheme, Host, PathBase, Path, Query, and Fragment to avoid a dependency on ValueTuple{6}.
I'm not an AOT expert, but isn't creating a new struct going to do basically the same thing in AOT as depending on ValueTuple{6}?
@BrennanConroy,
A new struct UriComponents has been introduced to encapsulate the components of a URI: Scheme, Host, PathBase, Path, Query, and Fragment to avoid a dependency on ValueTuple{6}.
I'm not an AOT expert, but isn't creating a new struct going to do basically the same thing in AOT as depending on
ValueTuple{6}?
From @stephentoub on https://github.com/dotnet/aspnetcore/pull/55611#issuecomment-2143672330:
And it's going to contribute to a larger NativeAOT footprint, due to the use of the ValueTuple`5, which may not be otherwise used in the app and which brings with it a non-trivial footprint.
ValueTuple implements multiple interfaces and as a result contains a lot of code that generally can't be trimmed. If it's not otherwise used in the app, it's unnecessary bloat. At the app level, not a big deal, but at the runtime level, we pay attention to such things.
@amcasey,
CI failures look legit. Probably need some
#ifs.
There was a name collision between System.UriComponent and Microsoft.AspNetCore.Http.Extensions.UriHelper+UriComponent (introduced in this PR) that I have solved.
The other errors look to me as environmental and not related to this change. Am I missing anything?
@amcasey,
CI failures look legit. Probably need some
#ifs.There was a name collision between
System.UriComponentandMicrosoft.AspNetCore.Http.Extensions.UriHelper+UriComponent(introduced in this PR) that I have solved.The other errors look to me as environmental and not related to this change. Am I missing anything?
Nope, I was referring to the name collision. The new failure is probably unrelated, though it's not one I've seen before. A nuget restore failure could have been a transient network issue?
@amcasey, all builds seem to have succeeded.
All good, @BrennanConroy?
Hi @adityamandaleeka,
Anything blocking this PR?