Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use JsonPatchDocument without Newtonsoft.Json #12994

Closed
rmja opened this issue Aug 9, 2019 · 5 comments
Closed

Use JsonPatchDocument without Newtonsoft.Json #12994

rmja opened this issue Aug 9, 2019 · 5 comments
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates question

Comments

@rmja
Copy link
Contributor

rmja commented Aug 9, 2019

Describe the bug

The JsonPatchDocument does not specify a converter for the new System.Text.Json serializer and so any request to an action using JsonPatchDocument<T> returns a 400 model valutation error.

To Reproduce

Steps to reproduce the behavior:

  1. Using this version of ASP.NET Core '3.0.0-preview7'
  2. Use the System.Text.Json serializer, not Json.Net.
  3. Make an action with parameter JsonPatchDocument<SomeModel>
  4. Call the action with a simple patch, e.g. [{"op":"replace","path":"/number","value":115}].
  5. See error 400 being returned

Expected behavior

Excpect the parameter to be correctly bound to the patch and a status 200.

Additional context

Include the output of dotnet --info

.NET Core SDK (reflecting any global.json):
 Version:   3.0.100-preview7-012821
 Commit:    6348f1068a

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.18362
 OS Platform: Windows
 RID:         win10-x86
 Base Path:   C:\Program Files (x86)\dotnet\sdk\3.0.100-preview7-012821\

Host (useful for support):
  Version: 3.0.0-preview7-27912-14
  Commit:  4da6ee6450

.NET Core SDKs installed:
  3.0.100-preview7-012821 [C:\Program Files (x86)\dotnet\sdk]

.NET Core runtimes installed:
  Microsoft.AspNetCore.App 3.0.0-preview7.19365.7 [C:\Program Files (x86)\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 3.0.0-preview7-27912-14 [C:\Program Files (x86)\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.0.0-preview7-27912-14 [C:\Program Files (x86)\dotnet\shared\Microsoft.WindowsDesktop.App]
@rmja
Copy link
Contributor Author

rmja commented Aug 9, 2019

The following hack seems to work:
In Startup.cs:

public void ConfigureServices(IServiceCollection services)
{
            var fakeServiceProvider = new ServiceCollection();
            fakeServiceProvider
                .AddLogging()
                .AddMvc()
                .AddNewtonsoftJson();

            var fakeMvcOptions = fakeServiceProvider.BuildServiceProvider().GetRequiredService<IOptions<MvcOptions>>().Value;

            services.AddMvc(options =>
                {
                    var jsonPatchInputFormatter = fakeMvcOptions.InputFormatters.OfType<Microsoft.AspNetCore.Mvc.Formatters.NewtonsoftJsonPatchInputFormatter>().Single();
                    options.InputFormatters.Insert(0, jsonPatchInputFormatter);
                });
}

Is there any more elegang way to accomplish JsonPatch support?

@mkArtakMSFT mkArtakMSFT added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Aug 9, 2019
@mkArtakMSFT
Copy link
Member

Thanks for contacting us, @rmja.
We don't support JSONPatch without Newtonsoft.Json.
You can simply call .AddNewtonsoftJson() on the service collection instance.

@rmja
Copy link
Contributor Author

rmja commented Aug 14, 2019

@mkArtakMSFT year, but that replaces the System.Text.Json formatter for all json which is really unfortunate. How do I use Json.Net only for application/json-patch+json requests in a more clean way than my example above?

@dhhunter
Copy link

A PATCH request to WebAPI always returns an UnsupportedMediaType Response. Taking a dependence on Newtonsoft.Json (and calling AddNetwtonsoftJson()) does not change that. I have tried decorating the method with a Consumes attribute, writing my own custom InputFormatter (which appear to be broken in general), even changing the ContentType Header to a media type that should be supported (IE: application\json), nothing seems to work.

@mkArtakMSFT mkArtakMSFT reopened this Aug 15, 2019
@mkArtakMSFT
Copy link
Member

@rmja your original code is actually the way to go in that case.

@dhhunter your concerns are unrelated to this topic. Please file a separate issue if you want, and we will look into that separately.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates question
Projects
None yet
Development

No branches or pull requests

3 participants