Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Consider changing JsonResult's behavior when you provide a formatter #1370

Closed
rynowak opened this issue Oct 16, 2014 · 2 comments
Closed

Consider changing JsonResult's behavior when you provide a formatter #1370

rynowak opened this issue Oct 16, 2014 · 2 comments
Assignees
Milestone

Comments

@rynowak
Copy link
Member

rynowak commented Oct 16, 2014

This code is a bit of a trap.

            var formatter = new JsonOutputFormatter()
            {
                SerializerSettings = serializerSettings,
            };

            formatter.SupportedEncodings.Clear();
            formatter.SupportedEncodings.Add(encoding);

            return new JsonResult(content, formatter);

I new up a formatter, and give it the settings and and encoding a I want for this specific action, and then provide it to JsonResult. It will be ignored if I have a formatter registered in options that handles json (the default).

It turns out I have no way to specify a settings/formatter/encoding for a result ad-hoc unless I drop down to use objectresult.

@danroth27 danroth27 added this to the 6.0.0-rc1 milestone Oct 16, 2014
@danroth27 danroth27 modified the milestones: 6.0.0-beta2, 6.0.0-rc1 Oct 29, 2014
@danroth27
Copy link
Member

The explicitly specified formatter should always be used. The current behavior of using the specified formatter as a fallback from content negotiation is unexpected and incorrect.
If content negotiation fails and a formatter was not explicitly provided then the default JsonFormatter should be used.

rynowak added a commit that referenced this issue Oct 29, 2014
The change here is to always use the provided formatter, instead of using
it as a fallback. This is much less surprising for users.

There are some other subtle changes here and cleanup of the tests, as well
as documentation additions.

The primary change is that we still want to run 'select' on a formatter
even if it's the only one. This allows us to choose a content type based
on the accept header.

In the case of a user-provided formatter, we'll try to honor the best
possible combination of Accept and specified ContentTypes (specified
ContentTypes win if there's a conflict). If nothing works, we'll still run
the user-provided formatter and let it decide what to do.

In the case of the default (formatters from options) we do conneg, and if
there's a conflict, fall back to a global (from services)
JsonOutputFormatter - we let it decide what to do.

This should leave us with a defined and tested behavior for all cases.
rynowak added a commit that referenced this issue Oct 30, 2014
The change here is to always use the provided formatter, instead of using
it as a fallback. This is much less surprising for users.

There are some other subtle changes here and cleanup of the tests, as well
as documentation additions.

The primary change is that we still want to run 'select' on a formatter
even if it's the only one. This allows us to choose a content type based
on the accept header.

In the case of a user-provided formatter, we'll try to honor the best
possible combination of Accept and specified ContentTypes (specified
ContentTypes win if there's a conflict). If nothing works, we'll still run
the user-provided formatter and let it decide what to do.

In the case of the default (formatters from options) we do conneg, and if
there's a conflict, fall back to a global (from services)
JsonOutputFormatter - we let it decide what to do.

This should leave us with a defined and tested behavior for all cases.
@rynowak
Copy link
Member Author

rynowak commented Nov 4, 2014

105c99c

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants