Skip to content

Commit 1dcfb6f

Browse files
committed
Share implementation between executor and IResult
1 parent bdc812c commit 1dcfb6f

File tree

5 files changed

+92
-83
lines changed

5 files changed

+92
-83
lines changed

src/Mvc/Mvc.Core/src/FileStreamResult.cs

+21-25
Original file line numberDiff line numberDiff line change
@@ -84,39 +84,35 @@ public override Task ExecuteResultAsync(ActionContext context)
8484

8585
async Task IResult.ExecuteAsync(HttpContext httpContext)
8686
{
87-
if (httpContext == null)
88-
{
89-
throw new ArgumentNullException(nameof(httpContext));
90-
}
91-
92-
using (FileStream)
93-
{
94-
var loggerFactory = httpContext.RequestServices.GetRequiredService<ILoggerFactory>();
95-
var logger = loggerFactory.CreateLogger<RedirectResult>();
96-
logger.ExecutingFileResult(this);
97-
98-
long? fileLength = null;
99-
if (FileStream.CanSeek)
100-
{
101-
fileLength = FileStream.Length;
102-
}
103-
104-
var (range, rangeLength, serveBody) = FileResultExecutorBase.SetHeadersAndLog(
87+
var loggerFactory = httpContext.RequestServices.GetRequiredService<ILoggerFactory>();
88+
var logger = loggerFactory.CreateLogger<RedirectResult>();
89+
90+
Task writeFileAsync(HttpContext httpContext, FileStreamResult result, RangeItemHeaderValue? range, long rangeLength)
91+
=> FileStreamResultExecutor.WriteFileAsyncInternal(httpContext, this, range, rangeLength, logger!);
92+
93+
(RangeItemHeaderValue? range, long rangeLength, bool serveBody) setHeadersAndLog(
94+
HttpContext httpContext,
95+
FileResult result,
96+
long? fileLength,
97+
bool enableRangeProcessing,
98+
DateTimeOffset? lastModified,
99+
EntityTagHeaderValue? etag)
100+
=> FileResultExecutorBase.SetHeadersAndLog(
105101
httpContext,
106102
this,
107103
fileLength,
108104
EnableRangeProcessing,
109105
LastModified,
110106
EntityTag,
111-
logger);
107+
logger!);
112108

113-
if (!serveBody)
114-
{
115-
return;
116-
}
117109

118-
await FileStreamResultExecutor.WriteFileAsyncInternal(httpContext, this, range, rangeLength, logger);
119-
}
110+
await FileStreamResultExecutor.ExecuteAsyncInternal(
111+
httpContext,
112+
this,
113+
setHeadersAndLog,
114+
writeFileAsync,
115+
logger);
120116
}
121117
}
122118
}

src/Mvc/Mvc.Core/src/Infrastructure/FileStreamResultExecutor.cs

+32-17
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,37 @@ public FileStreamResultExecutor(ILoggerFactory loggerFactory)
2727

2828
/// <inheritdoc />
2929
public virtual async Task ExecuteAsync(ActionContext context, FileStreamResult result)
30+
{
31+
await ExecuteAsyncInternal(
32+
context,
33+
result,
34+
SetHeadersAndLog,
35+
WriteFileAsync,
36+
Logger);
37+
}
38+
39+
/// <summary>
40+
/// Write the contents of the FileStreamResult to the response body.
41+
/// </summary>
42+
/// <param name="context">The <see cref="ActionContext"/>.</param>
43+
/// <param name="result">The FileStreamResult to write.</param>
44+
/// <param name="range">The <see cref="RangeItemHeaderValue"/>.</param>
45+
/// <param name="rangeLength">The range length.</param>
46+
protected virtual Task WriteFileAsync(
47+
ActionContext context,
48+
FileStreamResult result,
49+
RangeItemHeaderValue? range,
50+
long rangeLength)
51+
{
52+
return WriteFileAsyncInternal(context.HttpContext, result, range, rangeLength, Logger);
53+
}
54+
55+
internal static async Task ExecuteAsyncInternal<TContext>(
56+
TContext context,
57+
FileStreamResult result,
58+
Func<TContext, FileStreamResult, long?, bool, DateTimeOffset?, EntityTagHeaderValue?, (RangeItemHeaderValue?, long, bool)> SetHeadersAndLog,
59+
Func<TContext, FileStreamResult, RangeItemHeaderValue?, long, Task> WriteFileAsync,
60+
ILogger logger)
3061
{
3162
if (context == null)
3263
{
@@ -40,7 +71,7 @@ public virtual async Task ExecuteAsync(ActionContext context, FileStreamResult r
4071

4172
using (result.FileStream)
4273
{
43-
Logger.ExecutingFileResult(result);
74+
logger.ExecutingFileResult(result);
4475

4576
long? fileLength = null;
4677
if (result.FileStream.CanSeek)
@@ -65,22 +96,6 @@ public virtual async Task ExecuteAsync(ActionContext context, FileStreamResult r
6596
}
6697
}
6798

68-
/// <summary>
69-
/// Write the contents of the FileStreamResult to the response body.
70-
/// </summary>
71-
/// <param name="context">The <see cref="ActionContext"/>.</param>
72-
/// <param name="result">The FileStreamResult to write.</param>
73-
/// <param name="range">The <see cref="RangeItemHeaderValue"/>.</param>
74-
/// <param name="rangeLength">The range length.</param>
75-
protected virtual Task WriteFileAsync(
76-
ActionContext context,
77-
FileStreamResult result,
78-
RangeItemHeaderValue? range,
79-
long rangeLength)
80-
{
81-
return WriteFileAsyncInternal(context.HttpContext, result, range, rangeLength, Logger);
82-
}
83-
8499
internal static Task WriteFileAsyncInternal(HttpContext httpContext, FileStreamResult result, RangeItemHeaderValue? range, long rangeLength, ILogger logger)
85100
{
86101
if (httpContext == null)

src/Mvc/Mvc.Core/src/Infrastructure/LocalRedirectResultExecutor.cs

+26-11
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
using Microsoft.AspNetCore.Mvc.Core;
1010
using Microsoft.AspNetCore.Mvc.Routing;
1111
using Microsoft.Extensions.Logging;
12-
using Microsoft.Net.Http.Headers;
1312

1413
namespace Microsoft.AspNetCore.Mvc.Infrastructure
1514
{
@@ -45,36 +44,52 @@ public LocalRedirectResultExecutor(ILoggerFactory loggerFactory, IUrlHelperFacto
4544
/// <inheritdoc />
4645
public virtual Task ExecuteAsync(ActionContext context, LocalRedirectResult result)
4746
{
48-
if (context == null)
47+
var urlHelper = result.UrlHelper ?? _urlHelperFactory.GetUrlHelper(context);
48+
49+
return ExecuteAsyncInternal(
50+
context.HttpContext,
51+
result,
52+
urlHelper.IsLocalUrl,
53+
urlHelper.Content,
54+
_logger);
55+
}
56+
57+
internal static Task ExecuteAsyncInternal(
58+
HttpContext httpContext,
59+
LocalRedirectResult result,
60+
Func<string, bool> isLocalUrl,
61+
Func<string, string> getContent,
62+
ILogger logger
63+
)
64+
{
65+
if (httpContext == null)
4966
{
50-
throw new ArgumentNullException(nameof(context));
67+
throw new ArgumentNullException(nameof(httpContext));
5168
}
5269

5370
if (result == null)
5471
{
5572
throw new ArgumentNullException(nameof(result));
5673
}
5774

58-
var urlHelper = result.UrlHelper ?? _urlHelperFactory.GetUrlHelper(context);
59-
6075
// IsLocalUrl is called to handle Urls starting with '~/'.
61-
if (!urlHelper.IsLocalUrl(result.Url))
76+
if (!isLocalUrl(result.Url))
6277
{
6378
throw new InvalidOperationException(Resources.UrlNotLocal);
6479
}
6580

66-
var destinationUrl = urlHelper.Content(result.Url);
67-
_logger.LocalRedirectResultExecuting(destinationUrl);
81+
var destinationUrl = getContent(result.Url);
82+
logger.LocalRedirectResultExecuting(destinationUrl);
6883

6984
if (result.PreserveMethod)
7085
{
71-
context.HttpContext.Response.StatusCode = result.Permanent ?
86+
httpContext.Response.StatusCode = result.Permanent ?
7287
StatusCodes.Status308PermanentRedirect : StatusCodes.Status307TemporaryRedirect;
73-
context.HttpContext.Response.Headers.Location = destinationUrl;
88+
httpContext.Response.Headers.Location = destinationUrl;
7489
}
7590
else
7691
{
77-
context.HttpContext.Response.Redirect(destinationUrl, result.Permanent);
92+
httpContext.Response.Redirect(destinationUrl, result.Permanent);
7893
}
7994

8095
return Task.CompletedTask;

src/Mvc/Mvc.Core/src/LocalRedirectResult.cs

+6-27
Original file line numberDiff line numberDiff line change
@@ -109,36 +109,15 @@ public override Task ExecuteResultAsync(ActionContext context)
109109

110110
Task IResult.ExecuteAsync(HttpContext httpContext)
111111
{
112-
if (httpContext == null)
113-
{
114-
throw new ArgumentNullException(nameof(httpContext));
115-
}
116-
117-
// IsLocalUrl is called to handle Urls starting with '~/'.
118-
if (!UrlHelperBase.CheckIsLocalUrl(Url))
119-
{
120-
throw new InvalidOperationException(Resources.UrlNotLocal);
121-
}
122-
123-
var destinationUrl = UrlHelperBase.Content(httpContext, Url);
124-
125112
var loggerFactory = httpContext.RequestServices.GetRequiredService<ILoggerFactory>();
126113
var logger = loggerFactory.CreateLogger<RedirectResult>();
127-
logger.LocalRedirectResultExecuting(destinationUrl);
128-
129-
if (PreserveMethod)
130-
{
131-
httpContext.Response.StatusCode = Permanent
132-
? StatusCodes.Status308PermanentRedirect
133-
: StatusCodes.Status307TemporaryRedirect;
134-
httpContext.Response.Headers.Location = destinationUrl;
135-
}
136-
else
137-
{
138-
httpContext.Response.Redirect(destinationUrl, Permanent);
139-
}
140114

141-
return Task.CompletedTask;
115+
return LocalRedirectResultExecutor.ExecuteAsyncInternal(
116+
httpContext,
117+
this,
118+
UrlHelperBase.CheckIsLocalUrl,
119+
url => UrlHelperBase.Content(httpContext, Url),
120+
logger);
142121
}
143122
}
144123
}

src/Mvc/Mvc.Core/test/BaseLocalRedirectResultTest.cs

+7-3
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@ public static async Task Execute_ReturnsExpectedValues<TContext>(
3030
var result = new LocalRedirectResult(contentPath);
3131

3232
// Act
33-
await result.ExecuteResultAsync(actionContext);
33+
object context = typeof(TContext) == typeof(HttpContext) ? httpContext : actionContext;
34+
await function(result, (TContext)context);
3435

3536
// Assert
3637
Assert.Equal(expectedPath, httpContext.Response.Headers.Location.ToString());
@@ -48,7 +49,8 @@ public static async Task Execute_Throws_ForNonLocalUrl<TContext>(
4849
var result = new LocalRedirectResult(contentPath);
4950

5051
// Act & Assert
51-
var exception = await Assert.ThrowsAsync<InvalidOperationException>(() => result.ExecuteResultAsync(actionContext));
52+
object context = typeof(TContext) == typeof(HttpContext) ? httpContext : actionContext;
53+
var exception = await Assert.ThrowsAsync<InvalidOperationException>(() => function(result, (TContext)context));
5254
Assert.Equal(
5355
"The supplied URL is not local. A URL with an absolute path is considered local if it does not " +
5456
"have a host/authority part. URLs using virtual paths ('~/') are also local.",
@@ -66,7 +68,9 @@ public static async Task Execute_Throws_ForNonLocalUrlTilde<TContext>(
6668
var result = new LocalRedirectResult(contentPath);
6769

6870
// Act & Assert
69-
var exception = await Assert.ThrowsAsync<InvalidOperationException>(() => result.ExecuteResultAsync(actionContext));
71+
object context = typeof(TContext) == typeof(HttpContext) ? httpContext : actionContext;
72+
73+
var exception = await Assert.ThrowsAsync<InvalidOperationException>(() => function(result, (TContext)context));
7074
Assert.Equal(
7175
"The supplied URL is not local. A URL with an absolute path is considered local if it does not " +
7276
"have a host/authority part. URLs using virtual paths ('~/') are also local.",

0 commit comments

Comments
 (0)