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

Commit b871516

Browse files
kichallakichalla
committed
Addressed more feedback
1 parent d3055f7 commit b871516

File tree

11 files changed

+80
-82
lines changed

11 files changed

+80
-82
lines changed

‎src/Microsoft.AspNet.Mvc.Core/ActionResults/ContentResult.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public override async Task ExecuteResultAsync([NotNull] ActionContext context)
6060

6161
if (Content != null)
6262
{
63-
await response.WriteAsync(Content, new ResponseEncodingWrapper(encoding));
63+
await response.WriteAsync(Content, encoding);
6464
}
6565
}
6666
}

‎src/Microsoft.AspNet.Mvc.Core/ActionResults/FileResult.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public abstract class FileResult : ActionResult
2424
/// the provided <paramref name="contentType"/>.
2525
/// </summary>
2626
/// <param name="contentType">The Content-Type header of the response.</param>
27-
protected FileResult(string contentType)
27+
protected FileResult([NotNull] string contentType)
2828
: this(new MediaTypeHeaderValue(contentType))
2929
{
3030
}
@@ -42,7 +42,7 @@ protected FileResult([NotNull] MediaTypeHeaderValue contentType)
4242
/// <summary>
4343
/// Gets or sets the Content-Type header value that will be written to the response.
4444
/// </summary>
45-
public MediaTypeHeaderValue ContentType { get; set; }
45+
public MediaTypeHeaderValue ContentType { get; }
4646

4747
/// <summary>
4848
/// Gets the file name that will be used in the Content-Disposition header of the response.

‎src/Microsoft.AspNet.Mvc.Core/ActionResults/ViewExecutor.cs

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,5 +158,53 @@ public override void Write(byte[] buffer, int offset, int count)
158158
}
159159
}
160160
}
161+
162+
/// <summary>
163+
/// Encoding wrapper which makes preamble to be not required.
164+
/// </summary>
165+
private class ResponseEncodingWrapper : Encoding
166+
{
167+
private readonly Encoding _encoding;
168+
169+
public ResponseEncodingWrapper(Encoding innerEncoding)
170+
{
171+
_encoding = innerEncoding;
172+
}
173+
174+
public override int GetByteCount(char[] chars, int index, int count)
175+
{
176+
return _encoding.GetByteCount(chars, index, count);
177+
}
178+
179+
public override int GetBytes(char[] chars, int charIndex, int charCount, byte[] bytes, int byteIndex)
180+
{
181+
return _encoding.GetBytes(chars, charIndex, charCount, bytes, byteIndex);
182+
}
183+
184+
public override int GetCharCount(byte[] bytes, int index, int count)
185+
{
186+
return _encoding.GetCharCount(bytes, index, count);
187+
}
188+
189+
public override int GetChars(byte[] bytes, int byteIndex, int byteCount, char[] chars, int charIndex)
190+
{
191+
return _encoding.GetChars(bytes, byteIndex, byteCount, chars, charIndex);
192+
}
193+
194+
public override int GetMaxByteCount(int charCount)
195+
{
196+
return _encoding.GetMaxByteCount(charCount);
197+
}
198+
199+
public override int GetMaxCharCount(int byteCount)
200+
{
201+
return _encoding.GetMaxByteCount(byteCount);
202+
}
203+
204+
public override byte[] GetPreamble()
205+
{
206+
return new byte[] { };
207+
}
208+
}
161209
}
162210
}

‎src/Microsoft.AspNet.Mvc.Core/ResponseEncodingWrapper.cs

Lines changed: 0 additions & 55 deletions
This file was deleted.

‎test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/ContentResultTest.cs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,29 +79,33 @@ public async Task ContentResult_Response_NullContent_SetsContentTypeAndEncoding(
7979
Assert.Equal("text/plain; charset=utf-7", httpContext.Response.ContentType);
8080
}
8181

82-
public static TheoryData<MediaTypeHeaderValue, string, byte[]> ContentResultContentTypeData
82+
public static TheoryData<MediaTypeHeaderValue, string, string, byte[]> ContentResultContentTypeData
8383
{
8484
get
8585
{
86-
return new TheoryData<MediaTypeHeaderValue, string, byte[]>
86+
return new TheoryData<MediaTypeHeaderValue, string, string, byte[]>
8787
{
8888
{
8989
null,
90+
"κόσμε",
9091
"text/plain; charset=utf-8",
91-
new byte[] { 97, 98, 99, 100 } //utf-8 without BOM
92+
new byte[] { 206, 186, 225, 189, 185, 207, 131, 206, 188, 206, 181 } //utf-8 without BOM
9293
},
9394
{
9495
new MediaTypeHeaderValue("text/foo"),
96+
"κόσμε",
9597
"text/foo; charset=utf-8",
96-
new byte[] { 97, 98, 99, 100 } //utf-8 without BOM
98+
new byte[] { 206, 186, 225, 189, 185, 207, 131, 206, 188, 206, 181 } //utf-8 without BOM
9799
},
98100
{
99101
MediaTypeHeaderValue.Parse("text/foo;p1=p1-value"),
102+
"κόσμε",
100103
"text/foo; p1=p1-value; charset=utf-8",
101-
new byte[] { 97, 98, 99, 100 } //utf-8 without BOM
104+
new byte[] { 206, 186, 225, 189, 185, 207, 131, 206, 188, 206, 181 } //utf-8 without BOM
102105
},
103106
{
104107
new MediaTypeHeaderValue("text/foo") { Encoding = Encoding.ASCII },
108+
"abcd",
105109
"text/foo; charset=us-ascii",
106110
new byte[] { 97, 98, 99, 100 }
107111
}
@@ -113,13 +117,14 @@ public static TheoryData<MediaTypeHeaderValue, string, byte[]> ContentResultCont
113117
[MemberData(nameof(ContentResultContentTypeData))]
114118
public async Task ContentResult_SetContentTypeAndEncoding(
115119
MediaTypeHeaderValue contentType,
120+
string content,
116121
string expectedContentType,
117122
byte[] expectedContentData)
118123
{
119124
// Arrange
120125
var contentResult = new ContentResult
121126
{
122-
Content = "abcd",
127+
Content = content,
123128
ContentType = contentType
124129
};
125130
var httpContext = GetHttpContext();

‎test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/FileContentResultTest.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public async Task WriteFileAsync_CopiesBuffer_ToOutputStream()
4848
}
4949

5050
[Fact]
51-
public async Task SetsSuppliedContentTypeAndEncoding()
51+
public async Task ExecuteResultAsync_SetsSuppliedContentTypeAndEncoding()
5252
{
5353
// Arrange
5454
var expectedContentType = "text/foo; charset=us-ascii";

‎test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/FilePathResultTest.cs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ public async Task ExecuteResultAsync_SetsSuppliedContentTypeAndEncoding()
118118
// Arrange
119119
var expectedContentType = "text/foo; charset=us-ascii";
120120
// path will be C:/.../TestFiles/FilePathResultTestFile.txt
121-
var path = Path.GetFullPath(Path.Combine(".", "TestFiles", "FilePathResultTestFile.txt"));
121+
var path = Path.GetFullPath(Path.Combine(".", "TestFiles", "FilePathResultTestFile_ASCII.txt"));
122122
path = path.Replace(@"\", "/");
123123

124124
// Point the FileProviderRoot to a subfolder
@@ -128,18 +128,17 @@ public async Task ExecuteResultAsync_SetsSuppliedContentTypeAndEncoding()
128128
};
129129

130130
var httpContext = new DefaultHttpContext();
131-
httpContext.Response.Body = new MemoryStream();
131+
var memoryStream = new MemoryStream();
132+
httpContext.Response.Body = memoryStream;
132133

133134
var context = new ActionContext(httpContext, new RouteData(), new ActionDescriptor());
134135

135136
// Act
136137
await result.ExecuteResultAsync(context);
137-
httpContext.Response.Body.Position = 0;
138138

139139
// Assert
140-
Assert.NotNull(httpContext.Response.Body);
141-
var contents = await new StreamReader(httpContext.Response.Body).ReadToEndAsync();
142-
Assert.Equal("FilePathResultTestFile contents", contents);
140+
var contents = Encoding.ASCII.GetString(memoryStream.ToArray());
141+
Assert.Equal("FilePathResultTestFile contents ASCII encoded", contents);
143142
Assert.Equal(expectedContentType, httpContext.Response.ContentType);
144143
}
145144

‎test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/FileResultTest.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public void Constructor_SetsContentType()
2222
var result = new EmptyFileResult("text/plain");
2323

2424
// Assert
25-
Assert.Equal("text/plain", result.ContentType?.ToString());
25+
Assert.Equal("text/plain", result.ContentType.ToString());
2626
}
2727

2828
[Fact]

‎test/Microsoft.AspNet.Mvc.Core.Test/ControllerTests.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -632,7 +632,7 @@ public void File_WithContents()
632632
// Assert
633633
Assert.NotNull(result);
634634
Assert.Same(fileContents, result.FileContents);
635-
Assert.Equal("application/pdf", result.ContentType?.ToString());
635+
Assert.Equal("application/pdf", result.ContentType.ToString());
636636
Assert.Equal(string.Empty, result.FileDownloadName);
637637
}
638638

@@ -649,7 +649,7 @@ public void File_WithContentsAndFileDownloadName()
649649
// Assert
650650
Assert.NotNull(result);
651651
Assert.Same(fileContents, result.FileContents);
652-
Assert.Equal("application/pdf", result.ContentType?.ToString());
652+
Assert.Equal("application/pdf", result.ContentType.ToString());
653653
Assert.Equal("someDownloadName", result.FileDownloadName);
654654
}
655655

@@ -666,7 +666,7 @@ public void File_WithPath()
666666
// Assert
667667
Assert.NotNull(result);
668668
Assert.Equal(path, result.FileName);
669-
Assert.Equal("application/pdf", result.ContentType?.ToString());
669+
Assert.Equal("application/pdf", result.ContentType.ToString());
670670
Assert.Equal(string.Empty, result.FileDownloadName);
671671
}
672672

@@ -683,7 +683,7 @@ public void File_WithPathAndFileDownloadName()
683683
// Assert
684684
Assert.NotNull(result);
685685
Assert.Equal(path, result.FileName);
686-
Assert.Equal("application/pdf", result.ContentType?.ToString());
686+
Assert.Equal("application/pdf", result.ContentType.ToString());
687687
Assert.Equal("someDownloadName", result.FileDownloadName);
688688
}
689689

@@ -705,7 +705,7 @@ public void File_WithStream()
705705
// Assert
706706
Assert.NotNull(result);
707707
Assert.Same(fileStream, result.FileStream);
708-
Assert.Equal("application/pdf", result.ContentType?.ToString());
708+
Assert.Equal("application/pdf", result.ContentType.ToString());
709709
Assert.Equal(string.Empty, result.FileDownloadName);
710710
}
711711

@@ -728,7 +728,7 @@ public void File_WithStreamAndFileDownloadName()
728728
// Assert
729729
Assert.NotNull(result);
730730
Assert.Same(fileStream, result.FileStream);
731-
Assert.Equal("application/pdf", result.ContentType?.ToString());
731+
Assert.Equal("application/pdf", result.ContentType.ToString());
732732
Assert.Equal("someDownloadName", result.FileDownloadName);
733733
mockHttpContext.Verify(
734734
x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()),
@@ -996,7 +996,7 @@ public void Controller_Content_WithParameterContentStringAndContentType_SetsResu
996996
Assert.IsType<ContentResult>(actualContentResult);
997997
Assert.Equal("TestContent", actualContentResult.Content);
998998
Assert.Null(actualContentResult.ContentType.Encoding);
999-
Assert.Equal("text/plain", actualContentResult.ContentType?.ToString());
999+
Assert.Equal("text/plain", actualContentResult.ContentType.ToString());
10001000
}
10011001

10021002
[Fact]
@@ -1012,7 +1012,7 @@ public void Controller_Content_WithParameterContentAndTypeAndEncoding_SetsResult
10121012
Assert.IsType<ContentResult>(actualContentResult);
10131013
Assert.Equal("TestContent", actualContentResult.Content);
10141014
Assert.Same(Encoding.UTF8, actualContentResult.ContentType.Encoding);
1015-
Assert.Equal("text/plain; charset=utf-8", actualContentResult.ContentType?.ToString());
1015+
Assert.Equal("text/plain; charset=utf-8", actualContentResult.ContentType.ToString());
10161016
}
10171017

10181018
[Fact]

‎test/Microsoft.AspNet.Mvc.Core.Test/ControllerUnitTestabilityTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ public void ControllerFileContent_InvokedInUnitTests()
128128
Assert.NotNull(result);
129129

130130
var fileContentResult = Assert.IsType<FileContentResult>(result);
131-
Assert.Equal(contentType, fileContentResult.ContentType?.ToString());
131+
Assert.Equal(contentType, fileContentResult.ContentType.ToString());
132132
Assert.Equal(fileName ?? string.Empty, fileContentResult.FileDownloadName);
133133

134134
if (content == null)
@@ -162,7 +162,7 @@ public void ControllerFileStream_InvokedInUnitTests()
162162
Assert.NotNull(result);
163163

164164
var fileStreamResult = Assert.IsType<FileStreamResult>(result);
165-
Assert.Equal(contentType, fileStreamResult.ContentType?.ToString());
165+
Assert.Equal(contentType, fileStreamResult.ContentType.ToString());
166166
Assert.Equal(fileName ?? string.Empty, fileStreamResult.FileDownloadName);
167167

168168
if (content == null)

‎test/Microsoft.AspNet.Mvc.Core.Test/TestFiles/FilePathResultTestFile_ASCII.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
FilePathResultTestFile contents ASCII encoded

0 commit comments

Comments
 (0)