From 692a07240c34ee12c317eddb7cc28d744818b02d Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Fri, 16 Jan 2015 01:10:09 -0800 Subject: [PATCH] Some cleanup of ActionResults - #657 In general all properties are get/set so filters can change them. - some validate for not-null - where we use services it's get/set also Services are resolved in the Execute method if not provided. A few more ActionResults that return a body have the ability to set a status code now (optional). --- .../ActionResults/ContentResult.cs | 10 +++ .../ActionResults/CreatedAtActionResult.cs | 12 ++-- .../ActionResults/CreatedAtRouteResult.cs | 8 +-- .../ActionResults/CreatedResult.cs | 21 +++++- .../ActionResults/FileContentResult.cs | 33 ++++++++- .../ActionResults/FilePathResult.cs | 40 +++++++---- .../ActionResults/FileResult.cs | 12 ++-- .../ActionResults/FileStreamResult.cs | 33 ++++++++- .../ActionResults/JsonResult.cs | 15 +++++ .../ActionResults/NoContentResult.cs | 16 +---- .../ActionResults/ObjectResult.cs | 14 ++-- .../ActionResults/PartialViewResult.cs | 10 +++ .../ActionResults/RedirectResult.cs | 38 ++++++++--- .../ActionResults/RedirectToActionResult.cs | 35 ++++++---- .../ActionResults/RedirectToRouteResult.cs | 39 ++++++----- .../ActionResults/ViewResult.cs | 10 +++ src/Microsoft.AspNet.Mvc.Core/Controller.cs | 26 +++++-- .../ApiController.cs | 5 +- .../ActionResults/FilePathResultTest.cs | 67 ++++++++++++++----- .../RedirectToActionResultTest.cs | 14 ++-- .../RedirectToRouteResultTest.cs | 19 +++--- .../ControllerUnitTestabilityTests.cs | 11 ++- 22 files changed, 356 insertions(+), 132 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/ContentResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/ContentResult.cs index 5a3dd9c21a..9da4c0c7b1 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResults/ContentResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/ContentResult.cs @@ -16,6 +16,11 @@ namespace Microsoft.AspNet.Mvc public string ContentType { get; set; } + /// + /// Gets or sets the HTTP status code. + /// + public int? StatusCode { get; set; } + public override async Task ExecuteResultAsync([NotNull] ActionContext context) { var response = context.HttpContext.Response; @@ -33,6 +38,11 @@ namespace Microsoft.AspNet.Mvc contentTypeHeader.Encoding = ContentEncoding ?? Encodings.UTF8EncodingWithoutBOM; response.ContentType = contentTypeHeader.ToString(); + if (StatusCode != null) + { + response.StatusCode = StatusCode.Value; + } + if (Content != null) { await response.WriteAsync(Content, contentTypeHeader.Encoding); diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/CreatedAtActionResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/CreatedAtActionResult.cs index df74e54a34..279b890ad0 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResults/CreatedAtActionResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/CreatedAtActionResult.cs @@ -39,19 +39,19 @@ namespace Microsoft.AspNet.Mvc public IUrlHelper UrlHelper { get; set; } /// - /// Gets the name of the action to use for generating the URL. + /// Gets or sets the name of the action to use for generating the URL. /// - public string ActionName { get; private set; } + public string ActionName { get; set; } /// - /// Gets the name of the controller to use for generating the URL. + /// Gets or sets the name of the controller to use for generating the URL. /// - public string ControllerName { get; private set; } + public string ControllerName { get; set; } /// - /// Gets the route data to use for generating the URL. + /// Gets or sets the route data to use for generating the URL. /// - public IDictionary RouteValues { get; private set; } + public IDictionary RouteValues { get; set; } /// protected override void OnFormatting([NotNull] ActionContext context) diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/CreatedAtRouteResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/CreatedAtRouteResult.cs index d1e1989239..ba0ea40f50 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResults/CreatedAtRouteResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/CreatedAtRouteResult.cs @@ -47,14 +47,14 @@ namespace Microsoft.AspNet.Mvc public IUrlHelper UrlHelper { get; set; } /// - /// Gets the name of the route to use for generating the URL. + /// Gets or sets the name of the route to use for generating the URL. /// - public string RouteName { get; private set; } + public string RouteName { get; set; } /// - /// Gets the route data to use for generating the URL. + /// Gets or sets the route data to use for generating the URL. /// - public IDictionary RouteValues { get; private set; } + public IDictionary RouteValues { get; set; } /// protected override void OnFormatting([NotNull] ActionContext context) diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/CreatedResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/CreatedResult.cs index 6061f97d9d..006e0d1b77 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResults/CreatedResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/CreatedResult.cs @@ -10,6 +10,8 @@ namespace Microsoft.AspNet.Mvc /// public class CreatedResult : ObjectResult { + private string _location; + /// /// Initializes a new instance of the class with the values /// provided. @@ -24,9 +26,24 @@ namespace Microsoft.AspNet.Mvc } /// - /// Gets the location at which the content has been created. + /// Gets or sets the location at which the content has been created. /// - public string Location { get; private set; } + public string Location + { + get + { + return _location; + } + set + { + if (value == null) + { + throw new ArgumentNullException("value"); + } + + _location = value; + } + } /// protected override void OnFormatting([NotNull] ActionContext context) diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/FileContentResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/FileContentResult.cs index 9fe50228ee..6e3bde543d 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResults/FileContentResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/FileContentResult.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNet.Http; @@ -13,6 +14,19 @@ namespace Microsoft.AspNet.Mvc /// public class FileContentResult : FileResult { + private byte[] _fileContents; + + /// + /// Creates a new instance with + /// the provided . + /// + /// The bytes that represent the file contents. + public FileContentResult([NotNull] byte[] fileContents) + : base(contentType: null) + { + FileContents = fileContents; + } + /// /// Creates a new instance with /// the provided and the @@ -27,9 +41,24 @@ namespace Microsoft.AspNet.Mvc } /// - /// Gets the file contents. + /// Gets or sets the file contents. /// - public byte[] FileContents { get; private set; } + public byte[] FileContents + { + get + { + return _fileContents; + } + set + { + if (value == null) + { + throw new ArgumentNullException("value"); + } + + _fileContents = value; + } + } /// protected override Task WriteFileAsync(HttpResponse response, CancellationToken cancellation) diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/FilePathResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/FilePathResult.cs index 9eb2b9610f..56fbba95ab 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResults/FilePathResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/FilePathResult.cs @@ -15,7 +15,7 @@ using Microsoft.Framework.DependencyInjection; namespace Microsoft.AspNet.Mvc { /// - /// Represents an that when executed will + /// An that when executed will /// write a file from disk to the response using mechanisms provided /// by the host. /// @@ -23,16 +23,17 @@ namespace Microsoft.AspNet.Mvc { private const int DefaultBufferSize = 0x1000; + private string _fileName; + /// /// Creates a new instance with - /// the provided and the - /// provided . + /// the provided /// /// The path to the file. The path must be an absolute /// path. Relative and virtual paths are not supported. /// The Content-Type header of the response. - public FilePathResult([NotNull] string fileName, [NotNull] string contentType) - : base(contentType) + public FilePathResult([NotNull] string fileName) + : base(contentType: null) { FileName = fileName; } @@ -45,25 +46,36 @@ namespace Microsoft.AspNet.Mvc /// The path to the file. The path must be an absolute /// path. Relative and virtual paths are not supported. /// The Content-Type header of the response. - public FilePathResult( - [NotNull] string fileName, - [NotNull] string contentType, - [NotNull] IFileSystem fileSystem) + public FilePathResult([NotNull] string fileName, string contentType) : base(contentType) { FileName = fileName; - FileSystem = fileSystem; } /// - /// Gets the path to the file that will be sent back as the response. + /// Gets or sets the path to the file that will be sent back as the response. /// - public string FileName { get; private set; } + public string FileName + { + get + { + return _fileName; + } + set + { + if (value == null) + { + throw new ArgumentNullException("value"); + } + + _fileName = value; + } + } /// - /// Gets the used to resolve paths. + /// Gets or sets the used to resolve paths. /// - public IFileSystem FileSystem { get; private set; } + public IFileSystem FileSystem { get; set; } /// protected override Task WriteFileAsync(HttpResponse response, CancellationToken cancellation) diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/FileResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/FileResult.cs index 15a6af9fb2..a8f3b3ed24 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResults/FileResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/FileResult.cs @@ -23,15 +23,15 @@ namespace Microsoft.AspNet.Mvc /// the provided . /// /// The Content-Type header of the response. - protected FileResult([NotNull] string contentType) + protected FileResult(string contentType) { ContentType = contentType; } /// - /// Gets the Content-Type header value that will be written to the response. + /// Gets or sets the Content-Type header value that will be written to the response. /// - public string ContentType { get; private set; } + public string ContentType { get; set; } /// /// Gets the file name that will be used in the Content-Disposition header of the response. @@ -46,7 +46,11 @@ namespace Microsoft.AspNet.Mvc public override Task ExecuteResultAsync([NotNull] ActionContext context) { var response = context.HttpContext.Response; - response.ContentType = ContentType; + + if (ContentType != null) + { + response.ContentType = ContentType; + } if (!string.IsNullOrEmpty(FileDownloadName)) { diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/FileStreamResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/FileStreamResult.cs index d7e4337790..c9792f7a4b 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResults/FileStreamResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/FileStreamResult.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.IO; using System.Threading; using System.Threading.Tasks; @@ -17,6 +18,19 @@ namespace Microsoft.AspNet.Mvc // default buffer size as defined in BufferedStream type private const int BufferSize = 0x1000; + private Stream _fileStream; + + /// + /// Creates a new instance with + /// the provided . + /// + /// The stream with the file. + public FileStreamResult([NotNull] Stream fileStream) + : base(contentType: null) + { + FileStream = fileStream; + } + /// /// Creates a new instance with /// the provided and the @@ -31,9 +45,24 @@ namespace Microsoft.AspNet.Mvc } /// - /// Gets the stream with the file that will be sent back as the response. + /// Gets or sets the stream with the file that will be sent back as the response. /// - public Stream FileStream { get; private set; } + public Stream FileStream + { + get + { + return _fileStream; + } + set + { + if (value == null) + { + throw new ArgumentNullException("value"); + } + + _fileStream = value; + } + } /// protected async override Task WriteFileAsync(HttpResponse response, CancellationToken cancellation) diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/JsonResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/JsonResult.cs index 2fb77722b4..3816e2332c 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResults/JsonResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/JsonResult.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using System.Threading.Tasks; using Microsoft.Framework.DependencyInjection; @@ -55,6 +56,11 @@ namespace Microsoft.AspNet.Mvc /// public IOutputFormatter Formatter { get; set; } + /// + /// Gets or sets the HTTP status code. + /// + public int? StatusCode { get; set; } + /// /// Gets or sets the value to be formatted. /// @@ -86,7 +92,16 @@ namespace Microsoft.AspNet.Mvc Object = Value, }; + // JsonResult expects to always find a formatter, in contrast with ObjectResult, which might return + // a 406. var formatter = SelectFormatter(objectResult, formatterContext); + Debug.Assert(formatter != null); + + if (StatusCode != null) + { + context.HttpContext.Response.StatusCode = StatusCode.Value; + } + await formatter.WriteAsync(formatterContext); } diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/NoContentResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/NoContentResult.cs index 59ee0f6edd..2efda64bfe 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResults/NoContentResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/NoContentResult.cs @@ -1,23 +1,13 @@ // Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -#if ASPNET50 -using System.Net; -#endif - namespace Microsoft.AspNet.Mvc { - public class NoContentResult : ActionResult + public class NoContentResult : HttpStatusCodeResult { - public override void ExecuteResult([NotNull] ActionContext context) + public NoContentResult() + : base(204) { - var response = context.HttpContext.Response; - -#if ASPNET50 - response.StatusCode = (int)HttpStatusCode.NoContent; -#else - response.StatusCode = 204; -#endif } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/ObjectResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/ObjectResult.cs index 1af7787ca6..71dc28ffd4 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResults/ObjectResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/ObjectResult.cs @@ -14,6 +14,13 @@ namespace Microsoft.AspNet.Mvc { public class ObjectResult : ActionResult { + public ObjectResult(object value) + { + Value = value; + Formatters = new List(); + ContentTypes = new List(); + } + public object Value { get; set; } public IList Formatters { get; set; } @@ -27,13 +34,6 @@ namespace Microsoft.AspNet.Mvc /// public int? StatusCode { get; set; } - public ObjectResult(object value) - { - Value = value; - Formatters = new List(); - ContentTypes = new List(); - } - public override async Task ExecuteResultAsync(ActionContext context) { var formatters = GetDefaultFormatters(context); diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/PartialViewResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/PartialViewResult.cs index efcac52297..b6b9d345f2 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResults/PartialViewResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/PartialViewResult.cs @@ -13,6 +13,11 @@ namespace Microsoft.AspNet.Mvc /// public class PartialViewResult : ActionResult { + /// + /// Gets or sets the HTTP status code. + /// + public int? StatusCode { get; set; } + /// /// Gets or sets the name of the partial view to render. /// @@ -44,6 +49,11 @@ namespace Microsoft.AspNet.Mvc .EnsureSuccessful() .View; + if (StatusCode != null) + { + context.HttpContext.Response.StatusCode = StatusCode.Value; + } + using (view as IDisposable) { await ViewExecutor.ExecuteAsync(view, context, ViewData, contentType: null); diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/RedirectResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/RedirectResult.cs index b957da1af8..11c6616599 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResults/RedirectResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/RedirectResult.cs @@ -9,12 +9,14 @@ namespace Microsoft.AspNet.Mvc { public class RedirectResult : ActionResult { - public RedirectResult(string url) + private string _url; + + public RedirectResult([NotNull] string url) : this(url, permanent: false) { } - public RedirectResult(string url, bool permanent) + public RedirectResult([NotNull] string url, bool permanent) { if (string.IsNullOrEmpty(url)) { @@ -25,18 +27,33 @@ namespace Microsoft.AspNet.Mvc Url = url; } - public bool Permanent { get; private set; } + public bool Permanent { get; set; } - public string Url { get; private set; } + public string Url + { + get + { + return _url; + } + set + { + if (string.IsNullOrEmpty(value)) + { + throw new ArgumentException(Resources.ArgumentCannotBeNullOrEmpty, "value"); + } + + _url = value; + } + } + + public IUrlHelper UrlHelper { get; set; } public override void ExecuteResult([NotNull] ActionContext context) { - var destinationUrl = Url; - var urlHelper = context.HttpContext - .RequestServices - .GetRequiredService(); + var urlHelper = GetUrlHelper(context); // IsLocalUrl is called to handle Urls starting with '~/'. + var destinationUrl = Url; if (urlHelper.IsLocalUrl(destinationUrl)) { destinationUrl = urlHelper.Content(Url); @@ -44,5 +61,10 @@ namespace Microsoft.AspNet.Mvc context.HttpContext.Response.Redirect(destinationUrl, Permanent); } + + private IUrlHelper GetUrlHelper(ActionContext context) + { + return UrlHelper ?? context.HttpContext.RequestServices.GetRequiredService(); + } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/RedirectToActionResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/RedirectToActionResult.cs index 7798a220bd..8dd00ef26e 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResults/RedirectToActionResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/RedirectToActionResult.cs @@ -4,41 +4,47 @@ using System; using System.Collections.Generic; using Microsoft.AspNet.Mvc.Core; +using Microsoft.Framework.DependencyInjection; namespace Microsoft.AspNet.Mvc { public class RedirectToActionResult : ActionResult { - public RedirectToActionResult([NotNull] IUrlHelper urlHelper, string actionName, - string controllerName, IDictionary routeValues) - : this(urlHelper, actionName, controllerName, routeValues, permanent: false) + public RedirectToActionResult( + string actionName, + string controllerName, + IDictionary routeValues) + : this(actionName, controllerName, routeValues, permanent: false) { } - public RedirectToActionResult([NotNull] IUrlHelper urlHelper, string actionName, - string controllerName, IDictionary routeValues, bool permanent) + public RedirectToActionResult( + string actionName, + string controllerName, + IDictionary routeValues, + bool permanent) { - UrlHelper = urlHelper; ActionName = actionName; ControllerName = controllerName; RouteValues = routeValues; Permanent = permanent; } - public IUrlHelper UrlHelper { get; private set; } + public IUrlHelper UrlHelper { get; set; } - public string ActionName { get; private set; } + public string ActionName { get; set; } - public string ControllerName { get; private set; } + public string ControllerName { get; set; } - public IDictionary RouteValues { get; private set; } + public IDictionary RouteValues { get; set; } - public bool Permanent { get; private set; } + public bool Permanent { get; set; } public override void ExecuteResult([NotNull] ActionContext context) { - var destinationUrl = UrlHelper.Action(ActionName, ControllerName, RouteValues); + var urlHelper = GetUrlHelper(context); + var destinationUrl = urlHelper.Action(ActionName, ControllerName, RouteValues); if (string.IsNullOrEmpty(destinationUrl)) { throw new InvalidOperationException(Resources.NoRoutesMatched); @@ -46,5 +52,10 @@ namespace Microsoft.AspNet.Mvc context.HttpContext.Response.Redirect(destinationUrl, Permanent); } + + private IUrlHelper GetUrlHelper(ActionContext context) + { + return UrlHelper ?? context.HttpContext.RequestServices.GetRequiredService(); + } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/RedirectToRouteResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/RedirectToRouteResult.cs index 1186e5a0ac..3d571bf3a0 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResults/RedirectToRouteResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/RedirectToRouteResult.cs @@ -4,47 +4,47 @@ using System; using System.Collections.Generic; using Microsoft.AspNet.Mvc.Core; +using Microsoft.Framework.DependencyInjection; namespace Microsoft.AspNet.Mvc { public class RedirectToRouteResult : ActionResult { - public RedirectToRouteResult([NotNull] IUrlHelper urlHelper, - object routeValues) - : this(urlHelper, routeName: null, routeValues: routeValues) + public RedirectToRouteResult(object routeValues) + : this(routeName: null, routeValues: routeValues) { } - public RedirectToRouteResult([NotNull] IUrlHelper urlHelper, - string routeName, - object routeValues) - : this(urlHelper, routeName, routeValues, permanent: false) + public RedirectToRouteResult( + string routeName, + object routeValues) + : this(routeName, routeValues, permanent: false) { } - public RedirectToRouteResult([NotNull] IUrlHelper urlHelper, - string routeName, - object routeValues, - bool permanent) + public RedirectToRouteResult( + string routeName, + object routeValues, + bool permanent) { - UrlHelper = urlHelper; RouteName = routeName; RouteValues = TypeHelper.ObjectToDictionary(routeValues); Permanent = permanent; } - public IUrlHelper UrlHelper { get; private set; } + public IUrlHelper UrlHelper { get; set; } - public string RouteName { get; private set; } + public string RouteName { get; set; } - public IDictionary RouteValues { get; private set; } + public IDictionary RouteValues { get; set; } - public bool Permanent { get; private set; } + public bool Permanent { get; set; } public override void ExecuteResult([NotNull] ActionContext context) { - var destinationUrl = UrlHelper.RouteUrl(RouteValues); + var urlHelper = GetUrlHelper(context); + var destinationUrl = urlHelper.RouteUrl(RouteValues); if (string.IsNullOrEmpty(destinationUrl)) { throw new InvalidOperationException(Resources.NoRoutesMatched); @@ -52,5 +52,10 @@ namespace Microsoft.AspNet.Mvc context.HttpContext.Response.Redirect(destinationUrl, Permanent); } + + private IUrlHelper GetUrlHelper(ActionContext context) + { + return UrlHelper ?? context.HttpContext.RequestServices.GetRequiredService(); + } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/ViewResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/ViewResult.cs index 45454630f8..314fd6495e 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResults/ViewResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/ViewResult.cs @@ -13,6 +13,11 @@ namespace Microsoft.AspNet.Mvc /// public class ViewResult : ActionResult { + /// + /// Gets or sets the HTTP status code. + /// + public int? StatusCode { get; set; } + /// /// Gets or sets the name of the view to render. /// @@ -44,6 +49,11 @@ namespace Microsoft.AspNet.Mvc .EnsureSuccessful() .View; + if (StatusCode != null) + { + context.HttpContext.Response.StatusCode = StatusCode.Value; + } + using (view as IDisposable) { await ViewExecutor.ExecuteAsync(view, context, ViewData, contentType: null); diff --git a/src/Microsoft.AspNet.Mvc.Core/Controller.cs b/src/Microsoft.AspNet.Mvc.Core/Controller.cs index 72661425a0..e4e4b51226 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Controller.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Controller.cs @@ -398,8 +398,10 @@ namespace Microsoft.AspNet.Mvc public virtual RedirectToActionResult RedirectToAction(string actionName, string controllerName, object routeValues) { - return new RedirectToActionResult(Url, actionName, controllerName, - TypeHelper.ObjectToDictionary(routeValues)); + return new RedirectToActionResult(actionName, controllerName, TypeHelper.ObjectToDictionary(routeValues)) + { + UrlHelper = Url, + }; } /// @@ -453,8 +455,14 @@ namespace Microsoft.AspNet.Mvc public virtual RedirectToActionResult RedirectToActionPermanent(string actionName, string controllerName, object routeValues) { - return new RedirectToActionResult(Url, actionName, controllerName, - TypeHelper.ObjectToDictionary(routeValues), permanent: true); + return new RedirectToActionResult( + actionName, + controllerName, + TypeHelper.ObjectToDictionary(routeValues), + permanent: true) + { + UrlHelper = Url, + }; } /// @@ -489,7 +497,10 @@ namespace Microsoft.AspNet.Mvc [NonAction] public virtual RedirectToRouteResult RedirectToRoute(string routeName, object routeValues) { - return new RedirectToRouteResult(Url, routeName, routeValues); + return new RedirectToRouteResult(routeName, routeValues) + { + UrlHelper = Url, + }; } /// @@ -526,7 +537,10 @@ namespace Microsoft.AspNet.Mvc [NonAction] public virtual RedirectToRouteResult RedirectToRoutePermanent(string routeName, object routeValues) { - return new RedirectToRouteResult(Url, routeName, routeValues, permanent: true); + return new RedirectToRouteResult(routeName, routeValues, permanent: true) + { + UrlHelper = Url, + }; } /// diff --git a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/ApiController.cs b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/ApiController.cs index 1e3e594e0a..ac9db2ea00 100644 --- a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/ApiController.cs +++ b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/ApiController.cs @@ -359,7 +359,10 @@ namespace System.Web.Http [NonAction] public virtual RedirectToRouteResult RedirectToRoute([NotNull] string routeName, [NotNull] object routeValues) { - return new RedirectToRouteResult(Url, routeName, routeValues); + return new RedirectToRouteResult(routeName, routeValues) + { + UrlHelper = Url, + }; } /// diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/FilePathResultTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/FilePathResultTest.cs index cd2e131781..187d236012 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/FilePathResultTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/FilePathResultTest.cs @@ -34,8 +34,11 @@ namespace Microsoft.AspNet.Mvc { // Arrange var path = Path.GetFullPath(Path.Combine("TestFiles", "FilePathResultTestFile.txt")); - var fileSystem = new PhysicalFileSystem(Path.GetFullPath(".")); - var result = new FilePathResult(path, "text/plain", fileSystem); + + var result = new FilePathResult(path, "text/plain") + { + FileSystem = new PhysicalFileSystem(Path.GetFullPath(".")), + }; var httpContext = new DefaultHttpContext(); httpContext.Response.Body = new MemoryStream(); @@ -86,8 +89,11 @@ namespace Microsoft.AspNet.Mvc { // Arrange var path = Path.GetFullPath(Path.Combine("TestFiles", "FilePathResultTestFile.txt")); - var fileSystem = new PhysicalFileSystem(Path.GetFullPath(".")); - var result = new FilePathResult(path, "text/plain", fileSystem); + + var result = new FilePathResult(path, "text/plain") + { + FileSystem = new PhysicalFileSystem(Path.GetFullPath(".")), + }; var sendFileMock = new Mock(); sendFileMock @@ -117,8 +123,10 @@ namespace Microsoft.AspNet.Mvc path = path.Replace('/', '\\'); // Point the FileSystemRoot to a subfolder - var fileSystem = new PhysicalFileSystem(Path.GetFullPath("Utils")); - var result = new FilePathResult(path, "text/plain", fileSystem); + var result = new FilePathResult(path, "text/plain") + { + FileSystem = new PhysicalFileSystem(Path.GetFullPath("Utils")), + }; var httpContext = new DefaultHttpContext(); httpContext.Response.Body = new MemoryStream(); @@ -144,8 +152,10 @@ namespace Microsoft.AspNet.Mvc path = path.Replace(@"\", "/"); // Point the FileSystemRoot to a subfolder - var fileSystem = new PhysicalFileSystem(Path.GetFullPath("Utils")); - var result = new FilePathResult(path, "text/plain", fileSystem); + var result = new FilePathResult(path, "text/plain") + { + FileSystem = new PhysicalFileSystem(Path.GetFullPath("Utils")), + }; var httpContext = new DefaultHttpContext(); httpContext.Response.Body = new MemoryStream(); @@ -195,7 +205,10 @@ namespace Microsoft.AspNet.Mvc // Arrange var fileSystem = new PhysicalFileSystem(Path.GetFullPath("./TestFiles")); var expectedPath = Path.GetFullPath(Path.Combine(Directory.GetCurrentDirectory(), relativePathToFile)); - var filePathResult = new FilePathResult(path, "text/plain", fileSystem); + var filePathResult = new FilePathResult(path, "text/plain") + { + FileSystem = fileSystem, + }; // Act var result = filePathResult.ResolveFilePath(fileSystem); @@ -213,7 +226,10 @@ namespace Microsoft.AspNet.Mvc // Arrange var fileSystem = new PhysicalFileSystem(Path.GetFullPath("./TestFiles")); var expectedPath = Path.GetFullPath(Path.Combine(Directory.GetCurrentDirectory(), relativePathToFile)); - var filePathResult = new FilePathResult(path, "text/plain", fileSystem); + var filePathResult = new FilePathResult(path, "text/plain") + { + FileSystem = fileSystem, + }; // Act var ex = Assert.Throws(() => filePathResult.ResolveFilePath(fileSystem)); @@ -257,7 +273,11 @@ namespace Microsoft.AspNet.Mvc // Point the IFileSystem root to a different subfolder var fileSystem = new PhysicalFileSystem(Path.GetFullPath("./Utils")); - var filePathResult = new FilePathResult(path, "text/plain", fileSystem); + var filePathResult = new FilePathResult(path, "text/plain") + { + FileSystem = fileSystem, + }; + var expectedFileName = path.TrimStart('~').Replace('\\', '/'); var expectedMessage = "Could not find file: " + expectedFileName; @@ -300,7 +320,10 @@ namespace Microsoft.AspNet.Mvc public void NormalizePath_ConvertsBackSlashes_IntoForwardSlashes(string path, string expectedPath) { // Arrange - var fileResult = new FilePathResult(path, "text/plain", Mock.Of()); + var fileResult = new FilePathResult(path, "text/plain") + { + FileSystem = Mock.Of(), + }; // Act var normalizedPath = fileResult.NormalizePath(path); @@ -317,7 +340,10 @@ namespace Microsoft.AspNet.Mvc public void NormalizePath_ConvertsVirtualPaths_IntoRelativePaths(string path, string expectedPath) { // Arrange - var fileResult = new FilePathResult(path, "text/plain", Mock.Of()); + var fileResult = new FilePathResult(path, "text/plain") + { + FileSystem = Mock.Of(), + }; // Act var normalizedPath = fileResult.NormalizePath(path); @@ -334,7 +360,10 @@ namespace Microsoft.AspNet.Mvc public void NormalizePath_DoesNotConvert_InvalidVirtualPathsIntoRelativePaths(string path) { // Arrange - var fileResult = new FilePathResult(path, "text/plain", Mock.Of()); + var fileResult = new FilePathResult(path, "text/plain") + { + FileSystem = Mock.Of(), + }; // Act var normalizedPath = fileResult.NormalizePath(path); @@ -351,7 +380,10 @@ namespace Microsoft.AspNet.Mvc public void IsPathRooted_ReturnsTrue_ForAbsolutePaths(string path) { // Arrange - var fileResult = new FilePathResult(path, "text/plain", Mock.Of()); + var fileResult = new FilePathResult(path, "text/plain") + { + FileSystem = Mock.Of(), + }; // Act var isRooted = fileResult.IsPathRooted(path); @@ -393,7 +425,10 @@ namespace Microsoft.AspNet.Mvc public void IsPathRooted_ReturnsFalse_ForRelativePaths(string path) { // Arrange - var fileResult = new FilePathResult(path, "text/plain", Mock.Of()); + var fileResult = new FilePathResult(path, "text/plain") + { + FileSystem = Mock.Of(), + }; // Act var isRooted = fileResult.IsPathRooted(path); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/RedirectToActionResultTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/RedirectToActionResultTest.cs index 4dd5e8b509..903566da9d 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/RedirectToActionResultTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/RedirectToActionResultTest.cs @@ -25,8 +25,11 @@ namespace Microsoft.AspNet.Mvc.Core.Test.ActionResults var actionContext = new ActionContext(httpContext.Object, new RouteData(), new ActionDescriptor()); - IUrlHelper urlHelper = GetMockUrlHelper(expectedUrl); - RedirectToActionResult result = new RedirectToActionResult(urlHelper, "SampleAction", null, null); + var urlHelper = GetMockUrlHelper(expectedUrl); + var result = new RedirectToActionResult("SampleAction", null, null) + { + UrlHelper = urlHelper, + }; // Act await result.ExecuteResultAsync(actionContext); @@ -48,8 +51,11 @@ namespace Microsoft.AspNet.Mvc.Core.Test.ActionResults new RouteData(), new ActionDescriptor()); - IUrlHelper urlHelper = GetMockUrlHelper(returnValue: null); - RedirectToActionResult result = new RedirectToActionResult(urlHelper, null, null, null); + var urlHelper = GetMockUrlHelper(returnValue: null); + var result = new RedirectToActionResult(null, null, null) + { + UrlHelper = urlHelper, + }; // Act & Assert ExceptionAssert.ThrowsAsync( diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/RedirectToRouteResultTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/RedirectToRouteResultTest.cs index b2f20b7525..02d08e06e3 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/RedirectToRouteResultTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/RedirectToRouteResultTest.cs @@ -27,10 +27,12 @@ namespace Microsoft.AspNet.Mvc.Core var actionContext = new ActionContext(httpContext.Object, new RouteData(), new ActionDescriptor()); - IUrlHelper urlHelper = GetMockUrlHelper(expectedUrl); - RedirectToRouteResult result = new RedirectToRouteResult(urlHelper, - null, - TypeHelper.ObjectToDictionary(values)); + + var urlHelper = GetMockUrlHelper(expectedUrl); + var result = new RedirectToRouteResult(null, TypeHelper.ObjectToDictionary(values)) + { + UrlHelper = urlHelper, + }; // Act await result.ExecuteResultAsync(actionContext); @@ -52,10 +54,11 @@ namespace Microsoft.AspNet.Mvc.Core new RouteData(), new ActionDescriptor()); - IUrlHelper urlHelper = GetMockUrlHelper(returnValue: null); - RedirectToRouteResult result = new RedirectToRouteResult(urlHelper, - null, - new Dictionary()); + var urlHelper = GetMockUrlHelper(returnValue: null); + var result = new RedirectToRouteResult(null, new Dictionary()) + { + UrlHelper = urlHelper, + }; // Act & Assert ExceptionAssert.ThrowsAsync( diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ControllerUnitTestabilityTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ControllerUnitTestabilityTests.cs index ed8cafd014..9840ee7805 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ControllerUnitTestabilityTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ControllerUnitTestabilityTests.cs @@ -85,7 +85,7 @@ namespace Microsoft.AspNet.Mvc [Theory] [InlineData("/Created_1", "CreatedBody")] - [InlineData(null, null)] + [InlineData("/Created_2", null)] public void ControllerCreated_InvokedInUnitTests(string uri, string content) { // Arrange @@ -105,7 +105,7 @@ namespace Microsoft.AspNet.Mvc [Theory] [InlineData("CreatedBody", "text/html", "Created.html")] - [InlineData(null, null, null)] + [InlineData("CreatedBody", null, null)] public void ControllerFileContent_InvokedInUnitTests(string content, string contentType, string fileName) { // Arrange @@ -133,7 +133,7 @@ namespace Microsoft.AspNet.Mvc [Theory] [InlineData("CreatedBody", "text/html", "Created.html")] - [InlineData(null, null, null)] + [InlineData("CreatedBody", null, null)] public void ControllerFileStream_InvokedInUnitTests(string content, string contentType, string fileName) { // Arrange @@ -485,14 +485,13 @@ namespace Microsoft.AspNet.Mvc public IActionResult FileContent_Action(string content, string contentType, string fileName) { - var contentArray = string.IsNullOrEmpty(content) ? null : Encoding.UTF8.GetBytes(content); + var contentArray = Encoding.UTF8.GetBytes(content); return File(contentArray, contentType, fileName); } public IActionResult FileStream_Action(string content, string contentType, string fileName) { - var memoryStream = string.IsNullOrEmpty(content) ? null : - new MemoryStream(Encoding.UTF8.GetBytes(content)); + var memoryStream = new MemoryStream(Encoding.UTF8.GetBytes(content)); return File(memoryStream, contentType, fileName); }