Revert media type change for Newtonsoft JsonPatch package#65559
Revert media type change for Newtonsoft JsonPatch package#65559
Conversation
There was a problem hiding this comment.
Pull request overview
Reverts the stricter AcceptsMetadata/endpoint metadata behavior added for Microsoft.AspNetCore.JsonPatch (Newtonsoft.Json) so PATCH requests can again use Content-Type: application/json, while keeping Microsoft.AspNetCore.JsonPatch.SystemTextJson’s stricter behavior unchanged.
Changes:
- Remove
IEndpointParameterMetadataProvider/AcceptsMetadatapopulation from NewtonsoftJsonPatchDocumenttypes and drop the extra framework reference that made the package unusable for Blazor WASM. - Add end-to-end tests validating which
Content-Typevalues are accepted for both Newtonsoft and System.Text.Json JsonPatch packages. - Update OpenAPI tests to verify
Accepts(...)/[Consumes]can fully override the inferred JsonPatch request media type.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.RequestBody.cs | Tightens assertions to ensure inferred JsonPatch media types are fully overridden by explicit Accepts/Consumes. |
| src/Features/JsonPatch/test/Microsoft.AspNetCore.JsonPatch.Tests.csproj | Adds required ASP.NET Core references for new end-to-end tests on .NET Core targets. |
| src/Features/JsonPatch/test/JsonPatchContentTypeEndToEndTest.cs | New end-to-end coverage for accepted/rejected PATCH content types (Newtonsoft package). |
| src/Features/JsonPatch/src/Microsoft.AspNetCore.JsonPatch.csproj | Removes framework reference that was introduced for endpoint metadata support (improves Blazor WASM compatibility). |
| src/Features/JsonPatch/src/JsonPatchDocumentOfT.cs | Removes endpoint-parameter metadata provider implementation that enforced application/json-patch+json. |
| src/Features/JsonPatch/src/JsonPatchDocument.cs | Removes endpoint-parameter metadata provider implementation that enforced application/json-patch+json. |
| src/Features/JsonPatch.SystemTextJson/test/Microsoft.AspNetCore.JsonPatch.SystemTextJson.Tests.csproj | Adds required references for new end-to-end tests. |
| src/Features/JsonPatch.SystemTextJson/test/JsonPatchContentTypeEndToEndTest.cs | New end-to-end coverage confirming stricter content-type behavior remains for SystemTextJson package. |
src/Features/JsonPatch/test/JsonPatchContentTypeEndToEndTest.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
BrennanConroy
left a comment
There was a problem hiding this comment.
I assume some sort of breaking change announcement is going to be made for this area?
Is there going to be a follow up discussion for blazor-wasm getting a non-newtonsoft json version?
Maybe. I'm not sure how necessary it is given that we're reverting to the pre-.NET 10 behavior. It might just create more confusion. #65325 and #64330 should serve as notice of the unintentional breaking change until the next patch.
As far as I know, you simply cannot reference Microsoft.AspNetCore.Http.Abstractions from blazor-wasm either directly or indirectly (you wouldn't want Blazor WASM code referencing |
|
/ba-g It appears only quarantined tests unrelated to the PR are failing. |
|
/backport to release/10.0 |
|
Started backporting to |
|
@halter73 backporting to git am output$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch
Applying: Revert media type change for Newtonsoft JsonPatch package
Using index info to reconstruct a base tree...
M src/Features/JsonPatch.SystemTextJson/test/Microsoft.AspNetCore.JsonPatch.SystemTextJson.Tests.csproj
M src/Features/JsonPatch/test/Microsoft.AspNetCore.JsonPatch.Tests.csproj
Falling back to patching base and 3-way merge...
Auto-merging src/Features/JsonPatch.SystemTextJson/test/Microsoft.AspNetCore.JsonPatch.SystemTextJson.Tests.csproj
CONFLICT (content): Merge conflict in src/Features/JsonPatch.SystemTextJson/test/Microsoft.AspNetCore.JsonPatch.SystemTextJson.Tests.csproj
Auto-merging src/Features/JsonPatch/test/Microsoft.AspNetCore.JsonPatch.Tests.csproj
CONFLICT (content): Merge conflict in src/Features/JsonPatch/test/Microsoft.AspNetCore.JsonPatch.Tests.csproj
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 Revert media type change for Newtonsoft JsonPatch package
Error: The process '/usr/bin/git' failed with exit code 128 |
…onPatchDocument types - Remove IEndpointParameterMetadataProvider implementation from JsonPatchDocument and JsonPatchDocument<T> - Remove Microsoft.AspNetCore.Http.Abstractions reference from JsonPatch.csproj - Add end-to-end content type tests for both Newtonsoft and SystemTextJson JsonPatch - Update test csproj files with required references (resolving merge conflicts) - Add DoesNotContain assertions to OpenApi tests for custom content types Co-authored-by: halter73 <54385+halter73@users.noreply.github.com>
…t JsonPatchDocument (#65674) * Initial plan * Backport PR #65559: Remove IEndpointParameterMetadataProvider from JsonPatchDocument types - Remove IEndpointParameterMetadataProvider implementation from JsonPatchDocument and JsonPatchDocument<T> - Remove Microsoft.AspNetCore.Http.Abstractions reference from JsonPatch.csproj - Add end-to-end content type tests for both Newtonsoft and SystemTextJson JsonPatch - Update test csproj files with required references (resolving merge conflicts) - Add DoesNotContain assertions to OpenApi tests for custom content types Co-authored-by: halter73 <54385+halter73@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: halter73 <54385+halter73@users.noreply.github.com> Co-authored-by: Stephen Halter <halter73@gmail.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This reverts the media type change from #62988 from the Newtonsoft.Json-based Microsoft.AspNetCore.JsonPatch package because in addition to enhancing the OpenAPI metadata, it had the unintended consequence of requiring all JSON Patch requests use exactly the normative
Content-Type: application/json-patch+jsonheader. This caused requests using the previously allowedContent-Type: application/jsonheader to fail with a415 Unsupported Media typeresponse. This regression was reported by multiple people.This leaves the Microsoft.AspNetCore.JsonPatch.SystemTextJson package unchanged, since it was a NuGet package introduce for .NET 10 and therefore has no regression making us more comfortable with the stricter behavior.
An additional benefit of reverting the Newtonsoft.Json-based Microsoft.AspNetCore.JsonPatch changes is that the package should once again become usable from Blazor WASM since we're removing the newly-added
<Reference Include="Microsoft.AspNetCore.Http.Abstractions" Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp'" />. The inability to reference the package in a Blazor WASM app was also a regression in .NET 10 reported by multiple people.This adds new tests to verify
PatchContentTypes_AreHandledAsExpected. The OpenApi tests were also updated to verify that the request media type inferred from theJsonPatchDocumentparameter can be completely overridden.Fixes #65325
Fixes #64330
I plan to backport this to .NET 10 for servicing once this is approved.