Skip to content

Dot not run json:api registered json serialization when content-type header is null. #596

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

Closed
theandrewcosta opened this issue Oct 23, 2019 · 6 comments

Comments

@theandrewcosta
Copy link

Description

I have a situation where I would like to serialize json responses for NON json:api endpoints in a different way than for that of json:api endpoints.

As it stands now, the json:api library registers an IOutputFormatter that is called for json serialization when the content type is null or it equals the "application/vnd.api+json" content type.

return string.IsNullOrEmpty(contentTypeString) || contentTypeString == Constants.ContentType;

This might be ok for most situations, but in our situation, we don't use json:api for all endpoints that return json payloads.

Would it not make most sense to run the json serialization code registered by the json:api library only when requests come in with content type or accept header with the value "application/vnd.api+json"?

We've had to fork the library and make modifications to JsonApiOutputFormatter.cs so that the json serialization registered by the json:api library is not run (and therefore are default serialization code is run) when the content-type header is null.

Environment

  • JsonApiDotNetCore Version: 3.1.0.0
@theandrewcosta
Copy link
Author

Although the following changes to JsonApiOutputFormatter.cs may not be the perfect solution, it worked for us:

using System;
using System.Threading.Tasks;
using JsonApiDotNetCore.Internal;
using Microsoft.AspNetCore.Mvc.Formatters;
using Microsoft.Extensions.DependencyInjection;

namespace JsonApiDotNetCore.Formatters
{
    public class JsonApiOutputFormatter : IOutputFormatter
    {
        public bool CanWriteResult(OutputFormatterCanWriteContext context)
        {
            if (context == null)
                throw new ArgumentNullException(nameof(context));

            var acceptString = context.HttpContext.Request.Headers[Constants.AcceptHeader];

            return acceptString == Constants.ContentType;
        }

        public async Task WriteAsync(OutputFormatterWriteContext context)
        {
            var writer = context.HttpContext.RequestServices.GetService<IJsonApiWriter>();
            await writer.WriteAsync(context);
        }
    }
}

Thoughts?

@maurei
Copy link
Member

maurei commented Oct 23, 2019

Would it not make most sense to run the json serialization code registered by the json:api library only when requests come in with content type or accept header with the value "application/vnd.api+json"?

Absolutely. I'm not sure why we're also returning json:api responses when the content type is null rather than only application/vnd.api+json.

Apart from that, if you're hitting your non-json:api endpoints with a different content type like eg the standard application/json, the CanWriteResult should return false which should be enough to achieve the effect you desire -- no need to fork the library for that. Am I misunderstanding?

[edit]
Nevermind, I realise now that this will trigger a HTTP 406. How exactly does your edit solve your problem? Did you register an additional IOutputFormatter which prevents you from getting a 406?

@theandrewcosta
Copy link
Author

Nevermind, I realise now that this will trigger a HTTP 406. How exactly does your edit solve your problem? Did you register an additional IOutputFormatter which prevents you from getting a 406?

Yea exactly, we have another IOutputFormatter registered that handles default cases.

Apart from that, if you're hitting your non-json:api endpoints with a different content type like eg the standard application/json, the CanWriteResult should return false which should be enough to achieve the effect you desire -- no need to fork the library for that. Am I misunderstanding?

The thing is, we have several different front end applications consuming this back end service. They do not always send the "application/json" content type. All these apps, at the moment, use the endpoints that should NOT return the serialization as "application/vnd.api+json". So for these cases we would like to use our default serialization. It would be much simpler and safer in our specific case to offer the json:api serialization only to requests that specify it in the content-type or accept header.

@maurei
Copy link
Member

maurei commented Oct 23, 2019

Alright, that make sense. Still, I think you can configure v3.1 to do with you want without having to fork the library. That would work as follows: call the AddJsonApi() overload that allows you to pass along a mvcBuilder:

public static IServiceCollection AddJsonApi<TContext>(this IServiceCollection services, Action<JsonApiOptions> options, IMvcCoreBuilder mvcBuilder) { ... }

Configure JADNC like so:

// in Startup.cs

var builder = services.AddMvcCore();
services.AddJsonApi(options => { ... },  mvcBuilder: builder);
// the line below here is the important one
builder.AddMvcOptions(options => options.OutputFormatters.Insert(0, new YourCustomOverrideFormatter()));


// YourCustomOverrideFormatter.cs
public class YourCustomOverrideFormatter : IOutputFormatter
{
    public bool CanWriteResult(OutputFormatterCanWriteContext context)
    {
        if (context == null)
            throw new ArgumentNullException(nameof(context));

        return string.IsNullOrEmpty(context.HttpContext.Request.ContentType);
    }

    public async Task WriteAsync(OutputFormatterWriteContext context)
    {
        // your default handler
    }
}

If you register YourCustomOverrideFormatter after you call AddJsonApi(), your formatter will be executed before the default JADNC one. This way you should be able to make your usecase work without having to fork.

@maurei
Copy link
Member

maurei commented Oct 23, 2019

Let me know if the above work around works for you.

That's just treating the symptoms of the issue, though. Regarding the cause: I'm not sure why json:api content is returned for null content type header values. I would have to investigate. Probably this can be changed to just application/vnd.api+json.

Looking at the bigger picture: it would be good if a user can register a IOutputFormatter and IInputFormatter to be used by JADNC internally. This is currently not possible, but in v4.0 the necessary steps have been made to easily implement this. The usage of this would look something like

// Startup.cs
services.AddSingleton<IJsonApiOutputFormatter>(CustomOutputFormatter);
services.AddSingleton<IJsonApiInputFormatter>(CustomInputFormatter);
services.AddJsonApi(options => { ... }); // Internally a intermediate service provide is built to resolve a developers overrides like these above

@maurei
Copy link
Member

maurei commented Nov 7, 2019

Removing the string.IsNullOrEmpty(contentTypeString) would not be conform spec: GET requests do not need to have that content type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants