CreatedResult should accept null location

[singh733]

Task #39454

CreatedResult should accept null location

Description
The primary resource created by the request is identified by either a Location header field in the response or, if no Location field is received, by the effective request URI.

[ghost]

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

[halter73]

Thanks @singh733! I'll try to get back to you soon with the API decision.

[halter73]

When approving the API in #39454 (comment), we did not consider that this property would need to be made nullable. I'll update the API proposal and see if everyone still thinks we should change MVC's CreatedResult or if it's too breaking.

We can work out the PublicAPI.Unshipped.txt changes to get the build to pass. It gets complicated when changing the nullability of shipped APIs. We might have to bypass the build check when changing the nullablity of a property setter like this.

[BrennanConroy]

To fix this you need to update the shipped.txt file for nullability changes instead of the unshipped.txt file.

[singh733]

@BrennanConroy When we update the shipped.txt file we got the following error in build.

[BrennanConroy]

That's expected, someone will be able to override that and merge the PR

[halter73]

I think you're good once you delete TypedResults.Created(string uri) and TypedResults.Created(Uri uri). We can deal with the PublicAPI.Unshipped.txt changes like I mentioned earlier.

[halter73]

One more thing. There were also a bunch of Created() overloads in the approved API that this should add.

[captainsafia]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[captainsafia]

Triage: Need to follow-up on the PublicAPI.txt issues that are blocking this. Let's see about merging with override if there are no other issues in this PR.

[singh733]

@captainsafia Do you need any change from my side ?

[halter73]

@singh733 Thanks for offering to continue updating this after all this time! I appreciate you sticking with it.

I think I managed to massage the PublicAPI.Unshipped.txt files to get the build passing. I thought we were going to have to resort to changing a PublicAPI.Shipped.txt to get the nullability change to the Location property through, but it seems it's alright with claiming the setter is unchanged. Sometimes the analyzer doesn't capture enough information to notice API changes. See dotnet/roslyn-analyzers#3047 and dotnet/roslyn-analyzers#3901. While I was at it, I rebased your changes on main.

Because this didn't make it for .NET 7, I removed the changes that removed Created(string uri) and Created(Uri uri) from TypedResults now that it's shipped API.

I also reverted the change that caused Results to always return Created<object> even when given a null value like TypedResults do. I like always returning the same runtime type from a given method in principle, but now I don't think it's worth breaking code that might depend on the Results returning Created instead of Created<TValue> in the null value case. Hopefully code that cares about the runtime types of these things will use TypedResults instead.

[halter73]

Thanks again @singh733!