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

services.AddControllers().AddNewtonsoftJson() doesn't override System.Text.Json #13564

Closed
scalablecory opened this issue Aug 29, 2019 · 10 comments
Assignees
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates investigate Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue.

Comments

@scalablecory
Copy link
Contributor

From @Yannick183 on Wednesday, August 28, 2019 8:16:27 AM

Issue Title

services.AddControllers().AddNewtonsoftJson() doesn't seem to override System.Text.Json

General

I tried to upgrade a WebAPI project from dotnetcore 2.2 to version 3 but I'm facing some issues with Json serialization.

Using version 3.0.100-preview8-013656, I tried to override System.Text.Json by adding this to ConfigureServices

services.AddControllers().AddNewtonsoftJson(options =>
{
    options.SerializerSettings.Converters.Add(new StringEnumConverter(new CamelCaseNamingStrategy()));
    options.SerializerSettings.ReferenceLoopHandling = ReferenceLoopHandling.Ignore;
});

Some of my controller endpoints fail with a HTTP500 error because of some circular references that should have been prevented by the [JsonIgnore] attribute. The error is the following :
System.Text.Json.JsonException: CurrentDepth (32) is equal to or larger than the maximum allowed depth of 32.

As you can see, the error comes from System.Text.Json.JsonException which I suspect means that Newtonsoft.Json isn't used by the controller to serialize the response.

I use ModelMetadataType to add [JsonIgnore] to some attributes like this. I even tried adding System.Text.Json.Serialization.JsonIgnore to the property but without any result.

    [ModelMetadataType(typeof(ISomeEntityClassMetadata))]
    public partial class SomeEntityClass : ISomeEntityClassMetadata
    {
    }

    public interface ISomeEntityClassMetadata
    {
        [System.Text.Json.Serialization.JsonIgnore]
        [JsonIgnore]
        SomeOtherEntityClass SomeOtherEntityNavigation { get; set; }
    }

Copied from original issue: dotnet/core#3269

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

Thanks for contacting us, @scalablecory.
@pranavkm can you please look into this? Thanks.

@pranavkm
Copy link
Contributor

@scalablecory do you have a sample application that reproduces the error?

@pranavkm pranavkm added the Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. label Aug 30, 2019
@scalablecory
Copy link
Contributor Author

@Yannick183 is the author of the issue, I just moved it :). @Yannick183 do you have a repro?

@remyjette
Copy link

@Yannick183 did you make sure you're using the most recent preview versions of all NuGet packages?

I was experiencing the same issue when I tried to upgrade my ASP.NET Core 3.0 preview 3 app to preview 8. But when trying to make a sample application repro to assist with this issue, I realized that my old app was still using the preview 3 version of Microsoft.AspNetCore.Mvc.NewtonsoftJson. After upgrading everything works again. Give that a try if you haven't?

@Yannick183
Copy link

OK now I feel stupid. @remyjette I realized I didn't have Microsoft.AspNetCore.Mvc.NewtonsoftJson I only had Newtonsoft.Json installed but since I had the .AddNewtonsoftJson() extension method available I thought it was enough. I was unable to find it in NuGet GUI Microsoft.AspNetCore.Mvc.NewtonsoftJson so my guess was that package was used for previous versions only.
I then tried to install it from command line Install-Package Microsoft.AspNetCore.Mvc.NewtonsoftJson -Version 3.0.0-preview8.19405.7 and it worked. Now System.Text.Json is overriden as expected and the circular reference problem is gone.

I don't know why I couldn't find the package from NuGet GUI though. You can see below that the search term doesn't provide any result for Microsoft.AspNetCore.Mvc.NewtonsoftJson but the command line worked.

Using Visual Studio 2019 16.3.0 Preview 2.0

image

@scalablecory
Copy link
Contributor Author

Thanks for following up @Yannick183. I'm glad you found the solution; this is great feedback for ways we might improve documentation.

@anaximander23
Copy link

Just ran into this myself; I have to say, it's very strange that the .AddNewtonsoftJson() method behaves in this way - that is, that it's available, and doesn't throw an error, when the relevant NuGet package isn't even installed.

  • If it's doing something useful (ie. something other than replacing the deserialiser in use during model binding) then maybe the method name should be made more explicit to clarify what it actually does?
  • If it's not doing something useful (ie. it's intended solely to change the deserialiser in use during model binding, and without the NuGet package it just silently does nothing) then perhaps it should be declared inside of that NuGet package so that it's not accessible when it's not going to have the desired effect?

Either way, while you're right, it would be nice if the documentation properly explained how this should work, it would be even nicer if the code didn't let developers get into this baffling situation.

@sudoman281
Copy link

Same problem here. Installing the Microsoft.AspNetCore.Mvc.NewtonsoftJson fixed the issue. I couldn't find the package in the GUI either.

@Vake93
Copy link

Vake93 commented Nov 26, 2019

Why not move the method AddNewtonsoftJson to Microsoft.AspNetCore.Mvc.NewtonsoftJson? Since this is a extension method anyway? Spend a good amount of time figuring out why the upgraded project from 2.2 to 3 isn't working with JsonConverter Attribute. Finally saw this issue and installing Microsoft.AspNetCore.Mvc.NewtonsoftJson fixed it.

@xirami
Copy link

xirami commented Dec 8, 2019

See https://stackoverflow.com/a/59237265/1162622

Obviously PropertyNamingPolicy should have PascalCase

@ghost ghost locked as resolved and limited conversation to collaborators Jan 7, 2020
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 investigate Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue.
Projects
None yet
Development

No branches or pull requests

9 participants