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

Issue with default Newtonsoft Serializer #1510

Closed
WajahatAliAbid opened this issue Aug 30, 2021 · 5 comments · Fixed by #1655
Closed

Issue with default Newtonsoft Serializer #1510

WajahatAliAbid opened this issue Aug 30, 2021 · 5 comments · Fixed by #1655
Assignees
Labels
merged Issue has been merged to dev and is waiting for the next release proposal Proposal for a new functionality in Ocelot

Comments

@WajahatAliAbid
Copy link

WajahatAliAbid commented Aug 30, 2021

Expected Behavior

With .net core 3.1 and above, asp.net core projects use System.Text.Json serializer by default. But when we add Ocelot in our DI, asp.net core projects start using Newtonsoft.json. I tried diagnosing this issue and found that Ocelot was adding NewtonsoftJson in the dependency tree which changed the default serializer to NewtonsoftJson instead of the expected System.Text.Json with .net core 3.1 and above projects. For reference: OcelotBuilder

I've an HTTP Client which uses System.Text.Json as the serializer and it is receiving a dynamic JSON object which I expect to be serialized properly when returning as ActionResult from Controller. Following is my expected response

{
    "DynamicObject": {
        "Field1": "Value1",
        "Field2": "Value2"
    }
}

Actual Behavior

I get the following response instead which is basically serialized by Newtonsoft.Json

{
    "DynamicObject": {
        "ValueKind": 1
    }
}

Steps to Reproduce the Problem

  1. Add Ocelot and register it in startup
  2. Create a json string and deserialize it into object type using System.Text.Json.JsonSerializer
  3. Return that object from controller action
class DynamicResultType
{
  public object DynamicObject { get; set; }
}

[ApiController]
[Route("/api")]
public class HomeController : ControllerBase
{
  public ActionResult<DynamicResultType> Get()
  {
      var text = "{\"DynamicObject\": {\"Field1\": \"Value1\",\"Field2\": \"Value2\"}}";
      var obj = System.Text.Json.JsonSerializer.Deserialize<DynamicResultType>();
      return obj;
  }
}

Specifications

  • Version: 17.0.0
  • Platform: Linux (Manjaro KDE)
@PavelFischerCoupa
Copy link

Run into the same issue during migration of project from net 3.1 to 6.0

@psavoldelli
Copy link

as a workaround
Adding newtonsoft (from ocelot service builder) removed the input and output formatter.

Can be revert after the AddOcelot()

services.AddOptions<MvcOptions>().PostConfigure<IOptions<JsonOptions>, ILoggerFactory>((opts, jsonOpts, logger) => {
      if (opts.InputFormatters.OfType<SystemTextJsonInputFormatter>().Count() == 0)
      {
          opts.InputFormatters.RemoveType<NewtonsoftJsonInputFormatter>();
          opts.InputFormatters.Add(new SystemTextJsonInputFormatter(jsonOpts.Value,
              logger.CreateLogger<SystemTextJsonInputFormatter>()
          ));
      }
      if (opts.OutputFormatters.OfType<SystemTextJsonOutputFormatter>().Count() == 0)
      {
          opts.OutputFormatters.RemoveType<NewtonsoftJsonOutputFormatter>();
          opts.OutputFormatters.Add(new SystemTextJsonOutputFormatter(jsonOpts.Value.JsonSerializerOptions));
      }
  });

@raman-m
Copy link
Member

raman-m commented Sep 21, 2023

Hi @WajahatAliAbid !
Thanks for your interest in Ocelot!

I hope your problem will be resolved by the linked PR in Development, right?
The linked PR has extensions, docs and samples including how to remove NewtonsoftJson lib from Ocelot core.
Enjoy! And watch for new release on October 1... Soon... ⏲️

@WajahatAliAbid
Copy link
Author

@raman-m Thank you. I went through the pull request and it certainly seems to solve the problem. Looking forward to the new release.

raman-m added a commit that referenced this issue Sep 23, 2023
…peline building via the IMvcCoreBuilder interface (#1655)

* AddOcelot support custom mvc builder.

* Fix SA1503 : The opening and closing braces for a C# statement have been omitted.

* Fix SA1135 : A using directive is not qualified.

* Add AddDefaultAspNetServices method. Remove anonymous delegate.

* Make method names shorter

* add unit test for AddOcelotUsingBuilder

* Sort Usings

* Refactor integration test for the case with AddOcelotUsingBuilder

* Update OcelotBuilder.cs

* Update gettingstarted.rst

Fix target framework as net7.0

* Add "The AddOcelot method" paragraph

* Use correct heading levels for subsections

* Update configuration.rst

Use correct heading levels for (sub)sections

* Update configuration.rst

Readability improvements

* Incapsulate all system services to the AddDefaultAspNetServices method

* Add developer's XML-docs for OcelotBuilder and related extensions

* Create dependencyinjection.rst

* Update dependencyinjection.rst

Overview paragraph

* Update dependencyinjection.rst

"IServiceCollection extensions" paragraph.
"The OcelotBuilder class" paragraph.

* Update dependencyinjection.rst: Custom Builder paragraph

* Add Dependency Injection feature to the index of docs

* Update gettingstarted.rst: Add footnotes

* Correction of typo in English grammar

* Unit tests for AddOcelotUsingBuilder extension

* Improve readability of BDDfy scenarios in logs

* Update requestaggregation.rst. Add footnotes 

Mark down corrections

* Update tracing.rst: Add footnotes. Markdown text corrections

* Update qualityofservice.rst: Add footnotes

* Update administration.rst: Add footnotes. Markdown text corrections

* Update configuration.rst: Add footnotes. Text corrections

* IDE1006 Naming rule violation: These words must begin with upper case characters: should_*

---------

Co-authored-by: Raman Maksimchuk <[email protected]>
@raman-m raman-m added the merged Issue has been merged to dev and is waiting for the next release label Sep 23, 2023
@raman-m
Copy link
Member

raman-m commented Oct 9, 2023

@WajahatAliAbid
Use 20.0.0 version officially

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged Issue has been merged to dev and is waiting for the next release proposal Proposal for a new functionality in Ocelot
Projects
None yet
4 participants