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

Commit 105c99c

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 d178200 commit 105c99c

File tree

6 files changed

+481
-134
lines changed

6 files changed

+481
-134
lines changed

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

Lines changed: 85 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -9,104 +9,124 @@
99

1010
namespace Microsoft.AspNet.Mvc
1111
{
12+
/// <summary>
13+
/// An action result which formats the given object as JSON.
14+
/// </summary>
1215
public class JsonResult : ActionResult
1316
{
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)
17+
/// <summary>
18+
/// The list of content-types used for formatting when <see cref="ContentTypes"/> is null or empty.
19+
/// </summary>
20+
public static readonly IReadOnlyList<MediaTypeHeaderValue> DefaultContentTypes = new MediaTypeHeaderValue[]
2621
{
27-
}
22+
MediaTypeHeaderValue.Parse("application/json"),
23+
MediaTypeHeaderValue.Parse("text/json"),
24+
};
2825

2926
/// <summary>
30-
/// Creates an instance of <see cref="JsonResult"/> class.
27+
/// Creates a new <see cref="JsonResult"/> with the given <paramref name="data"/>.
3128
/// </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)
29+
/// <param name="value">The value to format as JSON.</param>
30+
public JsonResult(object value)
31+
: this(value, formatter: null)
4032
{
41-
_defaultFormatter = defaultFormatter;
42-
_objectResult = new ObjectResult(data);
4333
}
4434

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

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

48+
/// <summary>
49+
/// Gets or sets the list of supported Content-Types.
50+
/// </summary>
51+
public IList<MediaTypeHeaderValue> ContentTypes { get; set; }
52+
53+
/// <summary>
54+
/// Gets or sets the formatter.
55+
/// </summary>
56+
public IOutputFormatter Formatter { get; set; }
57+
58+
/// <summary>
59+
/// Gets or sets the value to be formatted.
60+
/// </summary>
61+
public object Value { get; set; }
62+
63+
/// <inheritdoc />
6964
public override async Task ExecuteResultAsync([NotNull] ActionContext context)
7065
{
66+
var objectResult = new ObjectResult(Value);
67+
7168
// Set the content type explicitly to application/json and text/json.
7269
// if the user has not already set it.
7370
if (ContentTypes == null || ContentTypes.Count == 0)
7471
{
75-
ContentTypes = _defaultSupportedContentTypes;
72+
foreach (var contentType in DefaultContentTypes)
73+
{
74+
objectResult.ContentTypes.Add(contentType);
75+
}
76+
}
77+
else
78+
{
79+
objectResult.ContentTypes = ContentTypes;
7680
}
7781

7882
var formatterContext = new OutputFormatterContext()
7983
{
80-
DeclaredType = _objectResult.DeclaredType,
8184
ActionContext = context,
85+
DeclaredType = objectResult.DeclaredType,
8286
Object = Value,
8387
};
8488

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);
89+
var formatter = SelectFormatter(objectResult, formatterContext);
8990
await formatter.WriteAsync(formatterContext);
9091
}
9192

92-
private IOutputFormatter SelectFormatter(OutputFormatterContext formatterContext)
93+
private IOutputFormatter SelectFormatter(ObjectResult objectResult, OutputFormatterContext formatterContext)
9394
{
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)
95+
if (Formatter == null)
10296
{
103-
formatter = _defaultFormatter ?? formatterContext.ActionContext
104-
.HttpContext
105-
.RequestServices
106-
.GetRequiredService<JsonOutputFormatter>();
107-
}
97+
// If no formatter was provided, then run Conneg with the formatters configured in options.
98+
var formatters = formatterContext
99+
.ActionContext
100+
.HttpContext
101+
.RequestServices
102+
.GetRequiredService<IOutputFormattersProvider>()
103+
.OutputFormatters
104+
.OfType<IJsonOutputFormatter>()
105+
.ToArray();
106+
107+
var formatter = objectResult.SelectFormatter(formatterContext, formatters);
108+
if (formatter == null)
109+
{
110+
// If the available user-configured formatters can't write this type, then fall back to the
111+
// 'global' one.
112+
formatter = formatterContext
113+
.ActionContext
114+
.HttpContext
115+
.RequestServices
116+
.GetRequiredService<JsonOutputFormatter>();
108117

109-
return formatter;
118+
// Run SelectFormatter again to try to choose a content type that this formatter can do.
119+
objectResult.SelectFormatter(formatterContext, new[] { formatter });
120+
}
121+
122+
return formatter;
123+
}
124+
else
125+
{
126+
// Run SelectFormatter to try to choose a content type that this formatter can do.
127+
objectResult.SelectFormatter(formatterContext, new[] { Formatter });
128+
return Formatter;
129+
}
110130
}
111131
}
112132
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
namespace Microsoft.AspNet.Mvc
5+
{
6+
/// <summary>
7+
/// An output formatter that specializes in writing JSON content.
8+
/// </summary>
9+
/// <remarks>
10+
/// The <see cref="JsonResult"/> class filter the collection of
11+
/// <see cref="IOutputFormattersProvider.OutputFormatters"/> and use only those which implement
12+
/// <see cref="IJsonOutputFormatter"/>.
13+
///
14+
/// To create a custom formatter that can be used by <see cref="JsonResult"/>, derive from
15+
/// <see cref="JsonOutputFormatter"/> or implement <see cref="IJsonOutputFormatter"/>.
16+
/// </remarks>
17+
public interface IJsonOutputFormatter : IOutputFormatter
18+
{
19+
}
20+
}

src/Microsoft.AspNet.Mvc.Core/Formatters/JsonOutputFormatter.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99

1010
namespace Microsoft.AspNet.Mvc
1111
{
12-
public class JsonOutputFormatter : OutputFormatter
12+
public class JsonOutputFormatter : OutputFormatter, IJsonOutputFormatter
1313
{
1414
private JsonSerializerSettings _serializerSettings;
1515

0 commit comments

Comments
 (0)