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

Commit 9f4c671

Browse files
ryanbrandenburgryanbrandenburg
committed
* CR fixes
1 parent 9d43f3f commit 9f4c671

File tree

13 files changed

+158
-45
lines changed

13 files changed

+158
-45
lines changed

‎src/Microsoft.AspNet.Mvc.Formatters.Json/Internal/MvcJsonMvcOptionsSetup.cs‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4+
using Microsoft.Extensions.Logging;
45
using Microsoft.Extensions.OptionsModel;
56
using Microsoft.Net.Http.Headers;
67
using Newtonsoft.Json;
@@ -10,18 +11,23 @@ namespace Microsoft.AspNet.Mvc.Formatters.Json.Internal
1011
{
1112
public class MvcJsonMvcOptionsSetup : ConfigureOptions<MvcOptions>
1213
{
13-
public MvcJsonMvcOptionsSetup(IOptions<MvcJsonOptions> jsonOptions)
14-
: base((_) => ConfigureMvc(_, jsonOptions.Value.SerializerSettings))
14+
public MvcJsonMvcOptionsSetup(ILoggerFactory loggerFactory, IOptions<MvcJsonOptions> jsonOptions)
15+
: base((_) => ConfigureMvc(_, jsonOptions.Value.SerializerSettings, loggerFactory))
1516
{
1617
}
1718

18-
public static void ConfigureMvc(MvcOptions options, JsonSerializerSettings serializerSettings)
19+
public static void ConfigureMvc(
20+
MvcOptions options,
21+
JsonSerializerSettings serializerSettings,
22+
ILoggerFactory loggerFactory)
1923
{
20-
options.OutputFormatters.Add(new JsonOutputFormatter(serializerSettings));
21-
22-
options.InputFormatters.Add(new JsonInputFormatter(serializerSettings));
23-
options.InputFormatters.Add(new JsonPatchInputFormatter(serializerSettings));
24+
var inputLogger = loggerFactory.CreateLogger<JsonInputFormatter>();
25+
var inputPatchLogger = loggerFactory.CreateLogger<JsonPatchInputFormatter>();
2426

27+
options.OutputFormatters.Add(new JsonOutputFormatter(serializerSettings));
28+
options.InputFormatters.Add(new JsonInputFormatter(inputLogger, serializerSettings));
29+
options.InputFormatters.Add(new JsonPatchInputFormatter(inputPatchLogger, serializerSettings));
30+
2531
options.FormatterMappings.SetMediaTypeMappingForFormat("json", MediaTypeHeaderValue.Parse("application/json"));
2632

2733
options.ValidationExcludeFilters.Add(typeof(JToken));

‎src/Microsoft.AspNet.Mvc.Formatters.Json/JsonInputFormatter.cs‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
using Microsoft.AspNet.Mvc.Formatters.Json.Logging;
77
using Microsoft.AspNet.Mvc.Internal;
88
using Microsoft.AspNet.Mvc.ModelBinding;
9-
using Microsoft.Extensions.DependencyInjection;
109
using Microsoft.Extensions.Logging;
1110
using Newtonsoft.Json;
1211

@@ -15,19 +14,27 @@ namespace Microsoft.AspNet.Mvc.Formatters
1514
public class JsonInputFormatter : InputFormatter
1615
{
1716
private JsonSerializerSettings _serializerSettings;
17+
private ILogger _logger;
1818

19-
public JsonInputFormatter()
20-
: this(SerializerSettingsProvider.CreateSerializerSettings())
19+
20+
public JsonInputFormatter(ILogger logger)
21+
: this(logger, SerializerSettingsProvider.CreateSerializerSettings())
2122
{
2223
}
2324

24-
public JsonInputFormatter(JsonSerializerSettings serializerSettings)
25+
public JsonInputFormatter(ILogger logger, JsonSerializerSettings serializerSettings)
2526
{
2627
if (serializerSettings == null)
2728
{
2829
throw new ArgumentNullException(nameof(serializerSettings));
2930
}
3031

32+
if (logger == null)
33+
{
34+
throw new ArgumentNullException(nameof(logger));
35+
}
36+
37+
_logger = logger;
3138
_serializerSettings = serializerSettings;
3239

3340
SupportedEncodings.Add(UTF8EncodingWithoutBOM);
@@ -72,9 +79,6 @@ public override Task<InputFormatterResult> ReadRequestBodyAsync(InputFormatterCo
7279
return InputFormatterResult.FailureAsync();
7380
}
7481

75-
var loggerFactory = context.HttpContext.RequestServices.GetRequiredService<ILoggerFactory>();
76-
var logger = loggerFactory.CreateLogger<FileResult>();
77-
7882
var request = context.HttpContext.Request;
7983
using (var streamReader = context.ReaderFactory(request.Body, effectiveEncoding))
8084
{
@@ -110,7 +114,7 @@ public override Task<InputFormatterResult> ReadRequestBodyAsync(InputFormatterCo
110114
var metadata = GetPathMetadata(context.Metadata, eventArgs.ErrorContext.Path);
111115
context.ModelState.TryAddModelError(key, eventArgs.ErrorContext.Error, metadata);
112116

113-
logger.JsonInputException(eventArgs.ErrorContext.Error);
117+
_logger.JsonInputException(eventArgs.ErrorContext.Error);
114118

115119
// Error must always be marked as handled
116120
// Failure to do so can cause the exception to be rethrown at every recursive level and

‎src/Microsoft.AspNet.Mvc.Formatters.Json/JsonPatchInputFormatter.cs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,20 @@
66
using System.Threading.Tasks;
77
using Microsoft.AspNet.JsonPatch;
88
using Microsoft.AspNet.Mvc.Internal;
9+
using Microsoft.Extensions.Logging;
910
using Newtonsoft.Json;
1011

1112
namespace Microsoft.AspNet.Mvc.Formatters
1213
{
1314
public class JsonPatchInputFormatter : JsonInputFormatter
1415
{
15-
public JsonPatchInputFormatter()
16-
: this(SerializerSettingsProvider.CreateSerializerSettings())
16+
public JsonPatchInputFormatter(ILogger logger)
17+
: this(logger, SerializerSettingsProvider.CreateSerializerSettings())
1718
{
1819
}
1920

20-
public JsonPatchInputFormatter(JsonSerializerSettings serializerSettings)
21-
: base(serializerSettings)
21+
public JsonPatchInputFormatter(ILogger logger, JsonSerializerSettings serializerSettings)
22+
: base(logger, serializerSettings)
2223
{
2324
// Clear all values and only include json-patch+json value.
2425
SupportedMediaTypes.Clear();

‎src/Microsoft.AspNet.Mvc.Formatters.Json/Logging/JsonInputFormatterLoggerExtensions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ internal static class JsonInputFormatterLoggerExtensions
1010
static JsonInputFormatterLoggerExtensions()
1111
{
1212
_jsonInputFormatterCrashed = LoggerMessage.Define<string>(
13-
LogLevel.Debug,
13+
LogLevel.Verbose,
1414
1,
1515
"Json input formatting failed, exception was '{0}'.");
1616
}

‎test/Microsoft.AspNet.Mvc.Formatters.Json.Test/JsonInputFormatterTest.cs‎

Lines changed: 63 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
using System.Threading.Tasks;
1111
using Microsoft.AspNet.Http;
1212
using Microsoft.AspNet.Mvc.ModelBinding;
13+
using Microsoft.Extensions.Logging;
1314
using Moq;
1415
using Newtonsoft.Json;
1516
using Newtonsoft.Json.Serialization;
@@ -34,7 +35,9 @@ public class JsonInputFormatterTest
3435
public void CanRead_ReturnsTrueForAnySupportedContentType(string requestContentType, bool expectedCanRead)
3536
{
3637
// Arrange
37-
var formatter = new JsonInputFormatter();
38+
var loggerMock = GetLogger();
39+
40+
var formatter = new JsonInputFormatter(loggerMock);
3841
var contentBytes = Encoding.UTF8.GetBytes("content");
3942

4043
var httpContext = GetHttpContext(contentBytes, contentType: requestContentType);
@@ -58,7 +61,8 @@ public void CanRead_ReturnsTrueForAnySupportedContentType(string requestContentT
5861
public void DefaultMediaType_ReturnsApplicationJson()
5962
{
6063
// Arrange
61-
var formatter = new JsonInputFormatter();
64+
var loggerMock = GetLogger();
65+
var formatter = new JsonInputFormatter(loggerMock);
6266

6367
// Act
6468
var mediaType = formatter.SupportedMediaTypes[0];
@@ -83,7 +87,8 @@ public static IEnumerable<object[]> JsonFormatterReadSimpleTypesData
8387
public async Task JsonFormatterReadsSimpleTypes(string content, Type type, object expected)
8488
{
8589
// Arrange
86-
var formatter = new JsonInputFormatter();
90+
var logger = GetLogger();
91+
var formatter = new JsonInputFormatter(logger);
8792
var contentBytes = Encoding.UTF8.GetBytes(content);
8893

8994
var httpContext = GetHttpContext(contentBytes);
@@ -109,7 +114,8 @@ public async Task JsonFormatterReadsComplexTypes()
109114
{
110115
// Arrange
111116
var content = "{name: 'Person Name', Age: '30'}";
112-
var formatter = new JsonInputFormatter();
117+
var logger = GetLogger();
118+
var formatter = new JsonInputFormatter(logger);
113119
var contentBytes = Encoding.UTF8.GetBytes(content);
114120

115121
var httpContext = GetHttpContext(contentBytes);
@@ -137,7 +143,8 @@ public async Task ReadAsync_AddsModelValidationErrorsToModelState()
137143
{
138144
// Arrange
139145
var content = "{name: 'Person Name', Age: 'not-an-age'}";
140-
var formatter = new JsonInputFormatter();
146+
var logger = GetLogger();
147+
var formatter = new JsonInputFormatter(logger);
141148
var contentBytes = Encoding.UTF8.GetBytes(content);
142149

143150
var modelState = new ModelStateDictionary();
@@ -166,7 +173,8 @@ public async Task ReadAsync_InvalidArray_AddsOverflowErrorsToModelState()
166173
{
167174
// Arrange
168175
var content = "[0, 23, 300]";
169-
var formatter = new JsonInputFormatter();
176+
var logger = GetLogger();
177+
var formatter = new JsonInputFormatter(logger);
170178
var contentBytes = Encoding.UTF8.GetBytes(content);
171179

172180
var modelState = new ModelStateDictionary();
@@ -187,14 +195,22 @@ public async Task ReadAsync_InvalidArray_AddsOverflowErrorsToModelState()
187195
Assert.True(result.HasError);
188196
Assert.Equal("The supplied value is invalid for Byte.", modelState["[2]"].Errors[0].ErrorMessage);
189197
Assert.Null(modelState["[2]"].Errors[0].Exception);
198+
Mock.Get<ILogger>(logger).Verify(l => l.Log(
199+
LogLevel.Verbose,
200+
It.IsAny<int>(),
201+
It.IsAny<object>(),
202+
It.IsAny<Exception>(),
203+
It.IsAny<Func<object, Exception, string>>()),
204+
Times.Once);
190205
}
191206

192207
[Fact]
193208
public async Task ReadAsync_InvalidComplexArray_AddsOverflowErrorsToModelState()
194209
{
195210
// Arrange
196211
var content = "[{name: 'Name One', Age: 30}, {name: 'Name Two', Small: 300}]";
197-
var formatter = new JsonInputFormatter();
212+
var logger = GetLogger();
213+
var formatter = new JsonInputFormatter(logger);
198214
var contentBytes = Encoding.UTF8.GetBytes(content);
199215

200216
var modelState = new ModelStateDictionary();
@@ -216,14 +232,22 @@ public async Task ReadAsync_InvalidComplexArray_AddsOverflowErrorsToModelState()
216232
Assert.Equal(
217233
"Error converting value 300 to type 'System.Byte'. Path '[1].Small', line 1, position 59.",
218234
modelState["names[1].Small"].Errors[0].Exception.Message);
235+
Mock.Get<ILogger>(logger).Verify(l => l.Log(
236+
LogLevel.Verbose,
237+
It.IsAny<int>(),
238+
It.IsAny<object>(),
239+
It.IsAny<Exception>(),
240+
It.IsAny<Func<object, Exception, string>>()),
241+
Times.Once);
219242
}
220243

221244
[Fact]
222245
public async Task ReadAsync_UsesTryAddModelValidationErrorsToModelState()
223246
{
224247
// Arrange
225248
var content = "{name: 'Person Name', Age: 'not-an-age'}";
226-
var formatter = new JsonInputFormatter();
249+
var logger = GetLogger();
250+
var formatter = new JsonInputFormatter(logger);
227251
var contentBytes = Encoding.UTF8.GetBytes(content);
228252

229253
var modelState = new ModelStateDictionary();
@@ -249,14 +273,23 @@ public async Task ReadAsync_UsesTryAddModelValidationErrorsToModelState()
249273
Assert.False(modelState.ContainsKey("age"));
250274
var error = Assert.Single(modelState[""].Errors);
251275
Assert.IsType<TooManyModelErrorsException>(error.Exception);
276+
Mock.Get<ILogger>(logger).Verify(l => l.Log(
277+
It.IsAny<LogLevel>(),
278+
It.IsAny<int>(),
279+
It.IsAny<object>(),
280+
It.IsAny<Exception>(),
281+
It.IsAny<Func<object, Exception, string>>()),
282+
Times.Once);
252283
}
253284

254285
[Fact]
255286
public void Creates_SerializerSettings_ByDefault()
256287
{
257288
// Arrange
289+
var logger = GetLogger();
290+
258291
// Act
259-
var jsonFormatter = new JsonInputFormatter();
292+
var jsonFormatter = new JsonInputFormatter(logger);
260293

261294
// Assert
262295
Assert.NotNull(jsonFormatter.SerializerSettings);
@@ -266,9 +299,11 @@ public void Creates_SerializerSettings_ByDefault()
266299
public void Constructor_UsesSerializerSettings()
267300
{
268301
// Arrange
302+
var logger = GetLogger();
303+
269304
// Act
270305
var serializerSettings = new JsonSerializerSettings();
271-
var jsonFormatter = new JsonInputFormatter(serializerSettings);
306+
var jsonFormatter = new JsonInputFormatter(logger, serializerSettings);
272307

273308
// Assert
274309
Assert.Same(serializerSettings, jsonFormatter.SerializerSettings);
@@ -280,8 +315,8 @@ public async Task ChangesTo_DefaultSerializerSettings_TakesEffect()
280315
// Arrange
281316
// missing password property here
282317
var contentBytes = Encoding.UTF8.GetBytes("{ \"UserName\" : \"John\"}");
283-
284-
var jsonFormatter = new JsonInputFormatter();
318+
var logger = GetLogger();
319+
var jsonFormatter = new JsonInputFormatter(logger);
285320
// by default we ignore missing members, so here explicitly changing it
286321
jsonFormatter.SerializerSettings.MissingMemberHandling = MissingMemberHandling.Error;
287322

@@ -313,8 +348,8 @@ public async Task CustomSerializerSettingsObject_TakesEffect()
313348
// Arrange
314349
// missing password property here
315350
var contentBytes = Encoding.UTF8.GetBytes("{ \"UserName\" : \"John\"}");
316-
317-
var jsonFormatter = new JsonInputFormatter();
351+
var logger = GetLogger();
352+
var jsonFormatter = new JsonInputFormatter(logger);
318353
// by default we ignore missing members, so here explicitly changing it
319354
jsonFormatter.SerializerSettings = new JsonSerializerSettings()
320355
{
@@ -343,6 +378,20 @@ public async Task CustomSerializerSettingsObject_TakesEffect()
343378
Assert.Contains("Required property 'Password' not found in JSON", modelErrorMessage);
344379
}
345380

381+
private static ILogger GetLogger()
382+
{
383+
var logger = new Mock<ILogger>();
384+
logger.Setup(l => l.Log(
385+
LogLevel.Verbose,
386+
It.IsAny<int>(),
387+
It.IsAny<object>(),
388+
It.IsAny<Exception>(),
389+
It.IsAny<Func<object, Exception, string>>()));
390+
logger.Setup(l => l.IsEnabled(LogLevel.Verbose)).Returns(true);
391+
392+
return logger.Object;
393+
}
394+
346395
private static HttpContext GetHttpContext(
347396
byte[] contentBytes,
348397
string contentType = "application/json")

‎test/Microsoft.AspNet.Mvc.Formatters.Json.Test/JsonOutputFormatterTests.cs‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
using Microsoft.AspNet.Mvc.Internal;
1313
using Microsoft.AspNet.Routing;
1414
using Microsoft.AspNet.Testing;
15+
using Microsoft.Extensions.Logging;
1516
using Microsoft.Net.Http.Headers;
1617
using Moq;
1718
using Newtonsoft.Json;
@@ -40,7 +41,8 @@ public void Constructor_UsesSerializerSettings()
4041
// Arrange
4142
// Act
4243
var serializerSettings = new JsonSerializerSettings();
43-
var jsonFormatter = new JsonInputFormatter(serializerSettings);
44+
var logger = GetLogger();
45+
var jsonFormatter = new JsonInputFormatter(logger, serializerSettings);
4446

4547
// Assert
4648
Assert.Same(serializerSettings, jsonFormatter.SerializerSettings);
@@ -284,6 +286,13 @@ private static Encoding CreateOrGetSupportedEncoding(
284286
return encoding;
285287
}
286288

289+
private static ILogger GetLogger()
290+
{
291+
var logger = new Mock<ILogger>();
292+
293+
return logger.Object;
294+
}
295+
287296
private static OutputFormatterWriteContext GetOutputFormatterContext(
288297
object outputValue,
289298
Type outputType,

0 commit comments

Comments
 (0)