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

Commit 817d602

Browse files
committed
Fix #1370 - Always use the provided formatter in JsonResult
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.
1 parent dc1aaf0 commit 817d602

File tree

4 files changed

+371
-134
lines changed

4 files changed

+371
-134
lines changed

src/Microsoft.AspNet.Mvc.Core/ActionResults/JsonResult.cs

Lines changed: 76 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -2,111 +2,121 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System.Collections.Generic;
5-
using System.Linq;
65
using System.Threading.Tasks;
76
using Microsoft.AspNet.Mvc.HeaderValueAbstractions;
87
using Microsoft.Framework.DependencyInjection;
98

109
namespace Microsoft.AspNet.Mvc
1110
{
11+
/// <summary>
12+
/// An action result which formats the given object as JSON.
13+
/// </summary>
1214
public class JsonResult : ActionResult
1315
{
14-
private static readonly IList<MediaTypeHeaderValue> _defaultSupportedContentTypes =
15-
new List<MediaTypeHeaderValue>()
16-
{
17-
MediaTypeHeaderValue.Parse("application/json"),
18-
MediaTypeHeaderValue.Parse("text/json"),
19-
};
20-
private IOutputFormatter _defaultFormatter;
21-
22-
private ObjectResult _objectResult;
23-
24-
public JsonResult(object data) :
25-
this(data, null)
16+
private static readonly MediaTypeHeaderValue[] _defaultSupportedContentTypes = new MediaTypeHeaderValue[]
2617
{
27-
}
18+
MediaTypeHeaderValue.Parse("application/json"),
19+
MediaTypeHeaderValue.Parse("text/json"),
20+
};
2821

2922
/// <summary>
30-
/// Creates an instance of <see cref="JsonResult"/> class.
23+
/// Creates a new <see cref="JsonResult"/> with the given <paramref name="data"/>.
3124
/// </summary>
32-
/// <param name="data"></param>
33-
/// <param name="defaultFormatter">If no matching formatter is found,
34-
/// the response is written to using defaultFormatter.</param>
35-
/// <remarks>
36-
/// The default formatter must be able to handle either application/json
37-
/// or text/json.
38-
/// </remarks>
39-
public JsonResult(object data, IOutputFormatter defaultFormatter)
25+
/// <param name="value">The value to format as JSON.</param>
26+
public JsonResult(object value)
27+
: this(value, null)
4028
{
41-
_defaultFormatter = defaultFormatter;
42-
_objectResult = new ObjectResult(data);
4329
}
4430

45-
public object Value
31+
/// <summary>
32+
/// Creates a new <see cref="JsonResult"/> with the given <paramref name="data"/>.
33+
/// </summary>
34+
/// <param name="value">The value to format as JSON.</param>
35+
/// <param name="formatter">The formatter to use, or <c>null</c> to choose a formatter dynamically.</param>
36+
public JsonResult(object value, IOutputFormatter formatter)
4637
{
47-
get
48-
{
49-
return _objectResult.Value;
50-
}
51-
set
52-
{
53-
_objectResult.Value = value;
54-
}
55-
}
38+
Value = value;
39+
Formatter = formatter;
5640

57-
public IList<MediaTypeHeaderValue> ContentTypes
58-
{
59-
get
60-
{
61-
return _objectResult.ContentTypes;
62-
}
63-
set
64-
{
65-
_objectResult.ContentTypes = value;
66-
}
41+
ContentTypes = new List<MediaTypeHeaderValue>();
6742
}
6843

44+
/// <summary>
45+
/// Gets or sets the list of supported Content-Types.
46+
/// </summary>
47+
public IList<MediaTypeHeaderValue> ContentTypes { get; set; }
48+
49+
/// <summary>
50+
/// Gets or sets the formatter.
51+
/// </summary>
52+
public IOutputFormatter Formatter { get; set; }
53+
54+
/// <summary>
55+
/// Gets or sets the value to be formatted.
56+
/// </summary>
57+
public object Value { get; set; }
58+
59+
/// <inheritdoc />
6960
public override async Task ExecuteResultAsync([NotNull] ActionContext context)
7061
{
62+
var objectResult = new ObjectResult(Value);
63+
7164
// Set the content type explicitly to application/json and text/json.
7265
// if the user has not already set it.
7366
if (ContentTypes == null || ContentTypes.Count == 0)
7467
{
75-
ContentTypes = _defaultSupportedContentTypes;
68+
objectResult.ContentTypes = _defaultSupportedContentTypes;
69+
}
70+
else
71+
{
72+
objectResult.ContentTypes = ContentTypes;
7673
}
7774

7875
var formatterContext = new OutputFormatterContext()
7976
{
80-
DeclaredType = _objectResult.DeclaredType,
8177
ActionContext = context,
8278
Object = Value,
8379
};
8480

85-
// Need to call this instead of directly calling _objectResult.ExecuteResultAsync
86-
// as that sets the status to 406 if a formatter is not found.
87-
// this can be cleaned up after https://github.com/aspnet/Mvc/issues/941 gets resolved.
88-
var formatter = SelectFormatter(formatterContext);
81+
var formatter = SelectFormatter(objectResult, formatterContext);
8982
await formatter.WriteAsync(formatterContext);
9083
}
9184

92-
private IOutputFormatter SelectFormatter(OutputFormatterContext formatterContext)
85+
private IOutputFormatter SelectFormatter(ObjectResult objectResult, OutputFormatterContext formatterContext)
9386
{
94-
var defaultFormatters = formatterContext.ActionContext
95-
.HttpContext
96-
.RequestServices
97-
.GetRequiredService<IOutputFormattersProvider>()
98-
.OutputFormatters;
99-
100-
var formatter = _objectResult.SelectFormatter(formatterContext, defaultFormatters);
101-
if (formatter == null)
87+
if (Formatter == null)
10288
{
103-
formatter = _defaultFormatter ?? formatterContext.ActionContext
104-
.HttpContext
105-
.RequestServices
106-
.GetRequiredService<JsonOutputFormatter>();
107-
}
89+
// If no formatter was provided, then run Conneg with the formatters configured in options.
90+
var formatters = formatterContext
91+
.ActionContext
92+
.HttpContext
93+
.RequestServices
94+
.GetRequiredService<IOutputFormattersProvider>()
95+
.OutputFormatters;
96+
97+
var formatter = objectResult.SelectFormatter(formatterContext, formatters);
98+
if (formatter == null)
99+
{
100+
// If the available user-configured formatters can't write this type, then fall back to the
101+
// 'global' one.
102+
formatter = formatterContext
103+
.ActionContext
104+
.HttpContext
105+
.RequestServices
106+
.GetRequiredService<JsonOutputFormatter>();
107+
108+
// Run SelectFormatter again to try to choose a content type that this formatter can do.
109+
objectResult.SelectFormatter(formatterContext, new[] { formatter });
110+
}
108111

109-
return formatter;
112+
return formatter;
113+
}
114+
else
115+
{
116+
// Run SelectFormatter to try to choose a content type that this formatter can do.
117+
objectResult.SelectFormatter(formatterContext, new[] { Formatter });
118+
return Formatter;
119+
}
110120
}
111121
}
112122
}

0 commit comments

Comments
 (0)