From 358a8ec2aff6276c30456a64b4855172cb86b7c3 Mon Sep 17 00:00:00 2001 From: Elitsa Marinovska Date: Tue, 22 Dec 2020 16:36:07 +0100 Subject: [PATCH] Initial stage of getting rid of HttpResponseException and replacing it with the usage of ValidationErrorResult --- .../Controllers/UsersControllerUnitTests.cs | 7 +- .../Controllers/AuthenticationController.cs | 10 +-- .../Controllers/CodeFileController.cs | 22 ++++-- .../Controllers/ContentController.cs | 27 +++---- .../Controllers/ContentTypeController.cs | 36 +++++---- .../Controllers/CurrentUserController.cs | 10 +-- .../Controllers/DictionaryController.cs | 36 +++------ .../ExamineManagementController.cs | 17 ++-- .../Controllers/LanguageController.cs | 23 +++--- .../Controllers/LogViewerController.cs | 22 +++--- .../Controllers/MacrosController.cs | 44 +++++------ .../Controllers/MediaController.cs | 5 +- .../Controllers/MemberController.cs | 16 ++-- .../Controllers/PackageInstallController.cs | 18 ++--- .../Controllers/UsersController.cs | 79 ++++++++++--------- 15 files changed, 180 insertions(+), 192 deletions(-) diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/UsersControllerUnitTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/UsersControllerUnitTests.cs index 4f4db85e5e..bc1fc26a57 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/UsersControllerUnitTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/UsersControllerUnitTests.cs @@ -1,10 +1,11 @@ using AutoFixture.NUnit3; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; using Moq; using NUnit.Framework; using Umbraco.Core.Security; using Umbraco.Tests.UnitTests.AutoFixture; using Umbraco.Web.BackOffice.Controllers; -using Umbraco.Web.Common.Exceptions; namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers { @@ -23,8 +24,8 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers .Setup(x => x.FindByIdAsync(It.IsAny())) .ReturnsAsync(user); - Assert.ThrowsAsync(() => sut.PostUnlockUsers(userIds)); + var result = sut.PostUnlockUsers(userIds).Result as ObjectResult; + Assert.AreEqual(StatusCodes.Status400BadRequest, result.StatusCode); } - } } diff --git a/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs b/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs index 36e5c2b6fe..44eb747089 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs @@ -1,7 +1,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Net; using System.Security.Claims; using System.Threading.Tasks; using Microsoft.AspNetCore.Authorization; @@ -27,7 +26,6 @@ using Umbraco.Web.Common.ActionsResults; using Umbraco.Web.Common.Attributes; using Umbraco.Web.Common.Authorization; using Umbraco.Web.Common.Controllers; -using Umbraco.Web.Common.Exceptions; using Umbraco.Web.Common.Filters; using Umbraco.Web.Common.Security; using Umbraco.Web.Models; @@ -151,7 +149,7 @@ namespace Umbraco.Web.BackOffice.Controllers if (result.Succeeded == false) { - throw HttpResponseException.CreateNotificationValidationErrorResponse(result.Errors.ToErrorMessage()); + return ValidationErrorResult.CreateNotificationValidationErrorResult(result.Errors.ToErrorMessage()); } await _signInManager.SignOutAsync(); @@ -207,7 +205,7 @@ namespace Umbraco.Web.BackOffice.Controllers else { AddModelErrors(result); - throw HttpResponseException.CreateValidationErrorResponse(ModelState); + return new ValidationErrorResult(ModelState); } } @@ -350,7 +348,7 @@ namespace Umbraco.Web.BackOffice.Controllers // by our angular helper because it thinks that we need to re-perform the request once we are // authorized and we don't want to return a 403 because angular will show a warning message indicating // that the user doesn't have access to perform this function, we just want to return a normal invalid message. - throw new HttpResponseException(HttpStatusCode.BadRequest); + return new ValidationErrorResult(null); } /// @@ -468,7 +466,7 @@ namespace Umbraco.Web.BackOffice.Controllers { if (ModelState.IsValid == false) { - throw HttpResponseException.CreateValidationErrorResponse(ModelState); + return new ValidationErrorResult(ModelState); } var user = await _signInManager.GetTwoFactorAuthenticationUserAsync(); diff --git a/src/Umbraco.Web.BackOffice/Controllers/CodeFileController.cs b/src/Umbraco.Web.BackOffice/Controllers/CodeFileController.cs index d1feaf11e9..a945ad3b2b 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/CodeFileController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/CodeFileController.cs @@ -1,9 +1,10 @@ -using System; +using System; using System.Collections.Generic; using System.IO; using System.Linq; using System.Net; using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Options; using Umbraco.Core; @@ -21,7 +22,6 @@ using Umbraco.Web.BackOffice.Filters; using Umbraco.Web.Common.ActionsResults; using Umbraco.Web.Common.Attributes; using Umbraco.Web.Common.Authorization; -using Umbraco.Web.Common.Exceptions; using Umbraco.Web.Models.ContentEditing; using Stylesheet = Umbraco.Core.Models.Stylesheet; using StylesheetRule = Umbraco.Web.Models.ContentEditing.StylesheetRule; @@ -84,13 +84,19 @@ namespace Umbraco.Web.BackOffice.Controllers var view = new PartialView(PartialViewType.PartialView, display.VirtualPath); view.Content = display.Content; var result = _fileService.CreatePartialView(view, display.Snippet, currentUser.Id); - return result.Success == true ? Ok() : throw HttpResponseException.CreateNotificationValidationErrorResponse(result.Exception.Message); + if (result.Success) + return Ok(); + else + return ValidationErrorResult.CreateNotificationValidationErrorResult(result.Exception.Message); case Core.Constants.Trees.PartialViewMacros: var viewMacro = new PartialView(PartialViewType.PartialViewMacro, display.VirtualPath); viewMacro.Content = display.Content; var resultMacro = _fileService.CreatePartialViewMacro(viewMacro, display.Snippet, currentUser.Id); - return resultMacro.Success == true ? Ok() : throw HttpResponseException.CreateNotificationValidationErrorResponse(resultMacro.Exception.Message); + if (resultMacro.Success) + return Ok(); + else + return ValidationErrorResult.CreateNotificationValidationErrorResult(resultMacro.Exception.Message); case Core.Constants.Trees.Scripts: var script = new Script(display.VirtualPath); @@ -116,7 +122,7 @@ namespace Umbraco.Web.BackOffice.Controllers if (string.IsNullOrWhiteSpace(parentId)) throw new ArgumentException("Value cannot be null or whitespace.", "parentId"); if (string.IsNullOrWhiteSpace(name)) throw new ArgumentException("Value cannot be null or whitespace.", "name"); if (name.ContainsAny(Path.GetInvalidPathChars())) { - throw HttpResponseException.CreateNotificationValidationErrorResponse(_localizedTextService.Localize("codefile/createFolderIllegalChars")); + return ValidationErrorResult.CreateNotificationValidationErrorResult(_localizedTextService.Localize("codefile/createFolderIllegalChars")); } // if the parentId is root (-1) then we just need an empty string as we are @@ -233,7 +239,7 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// This is a string but will be 'partialViews', 'partialViewMacros' /// Returns a list of if a correct type is sent - public IEnumerable GetSnippets(string type) + public ActionResult> GetSnippets(string type) { if (string.IsNullOrWhiteSpace(type)) throw new ArgumentException("Value cannot be null or whitespace.", "type"); @@ -252,10 +258,10 @@ namespace Umbraco.Web.BackOffice.Controllers snippets = _fileService.GetPartialViewSnippetNames(); break; default: - throw new HttpResponseException(HttpStatusCode.NotFound); + return new ValidationErrorResult(type, StatusCodes.Status404NotFound); } - return snippets.Select(snippet => new SnippetDisplay() {Name = snippet.SplitPascalCasing(_shortStringHelper).ToFirstUpperInvariant(), FileName = snippet}); + return snippets.Select(snippet => new SnippetDisplay() {Name = snippet.SplitPascalCasing(_shortStringHelper).ToFirstUpperInvariant(), FileName = snippet}).ToList(); } /// diff --git a/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs b/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs index b6dcdce14d..4b464388c2 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs @@ -5,6 +5,8 @@ using System.Linq; using System.Net; using System.Net.Mime; using System.Text; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http.Extensions; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Logging; @@ -14,7 +16,6 @@ using Umbraco.Core.Events; using Umbraco.Core.Mapping; using Umbraco.Core.Models; using Umbraco.Core.Models.ContentEditing; -using Umbraco.Core.Models.Entities; using Umbraco.Core.Models.Membership; using Umbraco.Core.Models.Validation; using Umbraco.Core.Persistence; @@ -24,23 +25,21 @@ using Umbraco.Core.Security; using Umbraco.Core.Serialization; using Umbraco.Core.Services; using Umbraco.Core.Strings; -using Umbraco.Web.Actions; -using Umbraco.Web.ContentApps; -using Umbraco.Web.Models.ContentEditing; -using Umbraco.Web.Routing; -using Constants = Umbraco.Core.Constants; using Umbraco.Extensions; +using Umbraco.Web.Actions; +using Umbraco.Web.BackOffice.ActionResults; +using Umbraco.Web.BackOffice.Authorization; using Umbraco.Web.BackOffice.Filters; using Umbraco.Web.BackOffice.ModelBinders; -using Umbraco.Web.BackOffice.ActionResults; +using Umbraco.Web.Common.ActionsResults; using Umbraco.Web.Common.Attributes; -using Umbraco.Web.Common.Exceptions; -using Umbraco.Web.Common.Filters; -using Umbraco.Web.Models.Mapping; -using Microsoft.AspNetCore.Authorization; using Umbraco.Web.Common.Authorization; -using Umbraco.Web.BackOffice.Authorization; -using System.Threading.Tasks; +using Umbraco.Web.Common.Exceptions; +using Umbraco.Web.ContentApps; +using Umbraco.Web.Models.ContentEditing; +using Umbraco.Web.Models.Mapping; +using Umbraco.Web.Routing; +using Constants = Umbraco.Core.Constants; namespace Umbraco.Web.BackOffice.Controllers { @@ -1603,7 +1602,7 @@ namespace Umbraco.Web.BackOffice.Controllers { _logger.LogWarning("Content sorting failed, this was probably caused by an event being cancelled"); // TODO: Now you can cancel sorting, does the event messages bubble up automatically? - throw HttpResponseException.CreateValidationErrorResponse("Content sorting failed, this was probably caused by an event being cancelled"); + return new ValidationErrorResult("Content sorting failed, this was probably caused by an event being cancelled"); } return Ok(); diff --git a/src/Umbraco.Web.BackOffice/Controllers/ContentTypeController.cs b/src/Umbraco.Web.BackOffice/Controllers/ContentTypeController.cs index 05e4db5daa..e6fc284498 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/ContentTypeController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/ContentTypeController.cs @@ -21,9 +21,9 @@ using Umbraco.Core.PropertyEditors; using Umbraco.Core.Security; using Umbraco.Core.Services; using Umbraco.Core.Strings; +using Umbraco.Web.Common.ActionsResults; using Umbraco.Web.Common.Attributes; using Umbraco.Web.Common.Authorization; -using Umbraco.Web.Common.Exceptions; using Umbraco.Web.Editors; using Umbraco.Web.Models; using Umbraco.Web.Models.ContentEditing; @@ -118,12 +118,12 @@ namespace Umbraco.Web.BackOffice.Controllers /// [DetermineAmbiguousActionByPassingParameters] [Authorize(Policy = AuthorizationPolicies.TreeAccessDocumentTypes)] - public DocumentTypeDisplay GetById(int id) + public ActionResult GetById(int id) { var ct = _contentTypeService.Get(id); if (ct == null) { - throw new HttpResponseException(HttpStatusCode.NotFound); + return new ValidationErrorResult(ct, StatusCodes.Status404NotFound); } var dto = _umbracoMapper.Map(ct); @@ -137,12 +137,12 @@ namespace Umbraco.Web.BackOffice.Controllers /// [DetermineAmbiguousActionByPassingParameters] [Authorize(Policy = AuthorizationPolicies.TreeAccessDocumentTypes)] - public DocumentTypeDisplay GetById(Guid id) + public ActionResult GetById(Guid id) { var contentType = _contentTypeService.Get(id); if (contentType == null) { - throw new HttpResponseException(HttpStatusCode.NotFound); + return new ValidationErrorResult(contentType, StatusCodes.Status404NotFound); } var dto = _umbracoMapper.Map(contentType); @@ -156,16 +156,16 @@ namespace Umbraco.Web.BackOffice.Controllers /// [DetermineAmbiguousActionByPassingParameters] [Authorize(Policy = AuthorizationPolicies.TreeAccessDocumentTypes)] - public DocumentTypeDisplay GetById(Udi id) + public ActionResult GetById(Udi id) { var guidUdi = id as GuidUdi; if (guidUdi == null) - throw new HttpResponseException(HttpStatusCode.NotFound); + return new ValidationErrorResult(guidUdi, StatusCodes.Status404NotFound); var contentType = _contentTypeService.Get(guidUdi.Guid); if (contentType == null) { - throw new HttpResponseException(HttpStatusCode.NotFound); + return new ValidationErrorResult(contentType, StatusCodes.Status404NotFound); } var dto = _umbracoMapper.Map(contentType); @@ -185,7 +185,7 @@ namespace Umbraco.Web.BackOffice.Controllers var foundType = _contentTypeService.Get(id); if (foundType == null) { - throw new HttpResponseException(HttpStatusCode.NotFound); + return new ValidationErrorResult(foundType, StatusCodes.Status404NotFound); } _contentTypeService.Delete(foundType, _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.Id); @@ -263,13 +263,13 @@ namespace Umbraco.Web.BackOffice.Controllers } [Authorize(Policy = AuthorizationPolicies.TreeAccessAnyContentOrTypes)] - public ContentPropertyDisplay GetPropertyTypeScaffold(int id) + public ActionResult GetPropertyTypeScaffold(int id) { var dataTypeDiff = _dataTypeService.GetDataType(id); if (dataTypeDiff == null) { - throw new HttpResponseException(HttpStatusCode.NotFound); + return new ValidationErrorResult(dataTypeDiff, StatusCodes.Status404NotFound); } var configuration = _dataTypeService.GetDataType(id).Configuration; @@ -304,9 +304,10 @@ namespace Umbraco.Web.BackOffice.Controllers { var result = _contentTypeService.CreateContainer(parentId, name, _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.Id); - return result - ? Ok(result.Result) //return the id - : throw HttpResponseException.CreateNotificationValidationErrorResponse(result.Exception.Message); + if (result.Success) + return Ok(result.Result); //return the id + else + return ValidationErrorResult.CreateNotificationValidationErrorResult(result.Exception.Message); } [Authorize(Policy = AuthorizationPolicies.TreeAccessDocumentTypes)] @@ -314,9 +315,10 @@ namespace Umbraco.Web.BackOffice.Controllers { var result = _contentTypeService.RenameContainer(id, name, _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.Id); - return result - ? Ok(result.Result) //return the id - : throw HttpResponseException.CreateNotificationValidationErrorResponse(result.Exception.Message); + if (result.Success) + return Ok(result.Result); //return the id + else + return ValidationErrorResult.CreateNotificationValidationErrorResult(result.Exception.Message); } [Authorize(Policy = AuthorizationPolicies.TreeAccessDocumentTypes)] diff --git a/src/Umbraco.Web.BackOffice/Controllers/CurrentUserController.cs b/src/Umbraco.Web.BackOffice/Controllers/CurrentUserController.cs index d156551c26..2c64cdca49 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/CurrentUserController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/CurrentUserController.cs @@ -22,9 +22,9 @@ using Umbraco.Core.Strings; using Umbraco.Extensions; using Umbraco.Web.BackOffice.Filters; using Umbraco.Web.BackOffice.Security; +using Umbraco.Web.Common.ActionsResults; using Umbraco.Web.Common.Attributes; using Umbraco.Web.Common.Authorization; -using Umbraco.Web.Common.Exceptions; using Umbraco.Web.Models; using Umbraco.Web.Models.ContentEditing; @@ -171,7 +171,7 @@ namespace Umbraco.Web.BackOffice.Controllers /// This only works when the user is logged in (partially) /// [Authorize(Policy = AuthorizationPolicies.BackOfficeAccess)] // TODO: Why is this necessary? This inherits from UmbracoAuthorizedApiController - public async Task PostSetInvitedUserPassword([FromBody]string newPassword) + public async Task> PostSetInvitedUserPassword([FromBody]string newPassword) { var user = await _backOfficeUserManager.FindByIdAsync(_backofficeSecurityAccessor.BackOfficeSecurity.GetUserId().ResultOr(0).ToString()); if (user == null) throw new InvalidOperationException("Could not find user"); @@ -184,7 +184,7 @@ namespace Umbraco.Web.BackOffice.Controllers // so that is why it is being used here. ModelState.AddModelError("value", result.Errors.ToErrorMessage()); - throw HttpResponseException.CreateValidationErrorResponse(ModelState); + return new ValidationErrorResult(ModelState); } //They've successfully set their password, we can now update their user account to be approved @@ -214,7 +214,7 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// If the password is being reset it will return the newly reset password, otherwise will return an empty value /// - public async Task> PostChangePassword(ChangingPasswordModel data) + public async Task>> PostChangePassword(ChangingPasswordModel data) { // TODO: Why don't we inject this? Then we can just inject a logger var passwordChanger = new PasswordChanger(_loggerFactory.CreateLogger()); @@ -233,7 +233,7 @@ namespace Umbraco.Web.BackOffice.Controllers ModelState.AddModelError(memberName, passwordChangeResult.Result.ChangeError.ErrorMessage); } - throw HttpResponseException.CreateValidationErrorResponse(ModelState); + return new ValidationErrorResult(ModelState); } // TODO: Why is this necessary? This inherits from UmbracoAuthorizedApiController diff --git a/src/Umbraco.Web.BackOffice/Controllers/DictionaryController.cs b/src/Umbraco.Web.BackOffice/Controllers/DictionaryController.cs index c7f86e12a1..94ada7e3aa 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/DictionaryController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/DictionaryController.cs @@ -1,24 +1,21 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using System.Net.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Logging; using Umbraco.Core; -using Umbraco.Core.Configuration; using Umbraco.Core.Mapping; using Umbraco.Core.Models; using Umbraco.Core.Security; using Umbraco.Core.Services; -using Umbraco.Web.BackOffice.Filters; using Umbraco.Web.Common.Attributes; -using Umbraco.Web.Common.Exceptions; using Umbraco.Web.Models.ContentEditing; -using Umbraco.Web.Security; using Constants = Umbraco.Core.Constants; using Umbraco.Core.Configuration.Models; using Microsoft.Extensions.Options; using Microsoft.AspNetCore.Authorization; +using Umbraco.Web.Common.ActionsResults; using Umbraco.Web.Common.Authorization; namespace Umbraco.Web.BackOffice.Controllers @@ -101,7 +98,7 @@ namespace Umbraco.Web.BackOffice.Controllers public ActionResult Create(int parentId, string key) { if (string.IsNullOrEmpty(key)) - throw HttpResponseException.CreateNotificationValidationErrorResponse("Key can not be empty."); // TODO: translate + return ValidationErrorResult.CreateNotificationValidationErrorResult("Key can not be empty."); // TODO: translate if (_localizationService.DictionaryItemExists(key)) { @@ -109,7 +106,7 @@ namespace Umbraco.Web.BackOffice.Controllers "dictionaryItem/changeKeyError", _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.GetUserCulture(_localizedTextService, _globalSettings), new Dictionary { { "0", key } }); - throw HttpResponseException.CreateNotificationValidationErrorResponse(message); + return ValidationErrorResult.CreateNotificationValidationErrorResult(message); } try @@ -130,7 +127,7 @@ namespace Umbraco.Web.BackOffice.Controllers catch (Exception ex) { _logger.LogError(ex, "Error creating dictionary with {Name} under {ParentId}", key, parentId); - throw HttpResponseException.CreateNotificationValidationErrorResponse("Error creating dictionary item"); + return ValidationErrorResult.CreateNotificationValidationErrorResult("Error creating dictionary item"); } } @@ -141,11 +138,8 @@ namespace Umbraco.Web.BackOffice.Controllers /// The id. /// /// - /// The . + /// The . Returns a not found response when dictionary item does not exist /// - /// - /// Returns a not found response when dictionary item does not exist - /// [DetermineAmbiguousActionByPassingParameters] public ActionResult GetById(int id) { @@ -163,11 +157,8 @@ namespace Umbraco.Web.BackOffice.Controllers /// The id. /// /// - /// The . + /// The . Returns a not found response when dictionary item does not exist /// - /// - /// Returns a not found response when dictionary item does not exist - /// [DetermineAmbiguousActionByPassingParameters] public ActionResult GetById(Guid id) { @@ -185,11 +176,8 @@ namespace Umbraco.Web.BackOffice.Controllers /// The id. /// /// - /// The . + /// The . Returns a not found response when dictionary item does not exist /// - /// - /// Returns a not found response when dictionary item does not exist - /// [DetermineAmbiguousActionByPassingParameters] public ActionResult GetById(Udi id) { @@ -213,13 +201,13 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// The . /// - public DictionaryDisplay PostSave(DictionarySave dictionary) + public ActionResult PostSave(DictionarySave dictionary) { var dictionaryItem = _localizationService.GetDictionaryItemById(int.Parse(dictionary.Id.ToString())); if (dictionaryItem == null) - throw HttpResponseException.CreateNotificationValidationErrorResponse("Dictionary item does not exist"); + return ValidationErrorResult.CreateNotificationValidationErrorResult("Dictionary item does not exist"); var userCulture = _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.GetUserCulture(_localizedTextService, _globalSettings); @@ -236,7 +224,7 @@ namespace Umbraco.Web.BackOffice.Controllers userCulture, new Dictionary { { "0", dictionary.Name } }); ModelState.AddModelError("Name", message); - throw HttpResponseException.CreateValidationErrorResponse(ModelState); + return new ValidationErrorResult(ModelState); } dictionaryItem.ItemKey = dictionary.Name; @@ -263,7 +251,7 @@ namespace Umbraco.Web.BackOffice.Controllers catch (Exception ex) { _logger.LogError(ex, "Error saving dictionary with {Name} under {ParentId}", dictionary.Name, dictionary.ParentId); - throw HttpResponseException.CreateNotificationValidationErrorResponse("Something went wrong saving dictionary"); + return ValidationErrorResult.CreateNotificationValidationErrorResult("Something went wrong saving dictionary"); } } diff --git a/src/Umbraco.Web.BackOffice/Controllers/ExamineManagementController.cs b/src/Umbraco.Web.BackOffice/Controllers/ExamineManagementController.cs index 72f07c02f3..c692f45ac2 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/ExamineManagementController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/ExamineManagementController.cs @@ -1,8 +1,7 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using Examine; -using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Logging; using Umbraco.Core; @@ -10,8 +9,8 @@ using Umbraco.Core.Cache; using Umbraco.Core.IO; using Umbraco.Examine; using Umbraco.Extensions; +using Umbraco.Web.Common.ActionsResults; using Umbraco.Web.Common.Attributes; -using Umbraco.Web.Common.Exceptions; using Umbraco.Web.Models.ContentEditing; using Umbraco.Web.Search; using SearchResult = Umbraco.Web.Models.ContentEditing.SearchResult; @@ -62,14 +61,14 @@ namespace Umbraco.Web.BackOffice.Controllers return model; } - public SearchResults GetSearchResults(string searcherName, string query, int pageIndex = 0, int pageSize = 20) + public ActionResult GetSearchResults(string searcherName, string query, int pageIndex = 0, int pageSize = 20) { if (query.IsNullOrWhiteSpace()) return SearchResults.Empty(); var msg = ValidateSearcher(searcherName, out var searcher); if (!msg.IsSuccessStatusCode()) - throw new HttpResponseException(msg); + return new ValidationErrorResult(msg); // NativeQuery will work for a single word/phrase too (but depends on the implementation) the lucene one will work. var results = searcher.CreateQuery().NativeQuery(query).Execute(maxResults: pageSize * (pageIndex + 1)); @@ -105,11 +104,11 @@ namespace Umbraco.Web.BackOffice.Controllers var validate = ValidateIndex(indexName, out var index); if (!validate.IsSuccessStatusCode()) - throw new HttpResponseException(validate); + return new ValidationErrorResult(validate); validate = ValidatePopulator(index); if (!validate.IsSuccessStatusCode()) - throw new HttpResponseException(validate); + return new ValidationErrorResult(validate); var cacheKey = "temp_indexing_op_" + indexName; var found = _runtimeCache.Get(cacheKey); @@ -130,11 +129,11 @@ namespace Umbraco.Web.BackOffice.Controllers { var validate = ValidateIndex(indexName, out var index); if (!validate.IsSuccessStatusCode()) - throw new HttpResponseException(validate); + return new ValidationErrorResult(validate); validate = ValidatePopulator(index); if (!validate.IsSuccessStatusCode()) - throw new HttpResponseException(validate); + return new ValidationErrorResult(validate); _logger.LogInformation("Rebuilding index '{IndexName}'", indexName); diff --git a/src/Umbraco.Web.BackOffice/Controllers/LanguageController.cs b/src/Umbraco.Web.BackOffice/Controllers/LanguageController.cs index 21b205de0f..4ac9209dd9 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/LanguageController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/LanguageController.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Globalization; using System.Linq; @@ -6,16 +6,13 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Options; using Umbraco.Core; -using Umbraco.Core.Configuration; using Umbraco.Core.Configuration.Models; using Umbraco.Core.Mapping; using Umbraco.Core.Models; using Umbraco.Core.Services; -using Umbraco.Web.BackOffice.Filters; +using Umbraco.Web.Common.ActionsResults; using Umbraco.Web.Common.Attributes; using Umbraco.Web.Common.Authorization; -using Umbraco.Web.Common.Exceptions; -using Umbraco.Web.Editors; using Language = Umbraco.Web.Models.ContentEditing.Language; namespace Umbraco.Web.BackOffice.Controllers @@ -97,7 +94,7 @@ namespace Umbraco.Web.BackOffice.Controllers if (language.IsDefault) { var message = $"Language '{language.IsoCode}' is currently set to 'default' and can not be deleted."; - throw HttpResponseException.CreateNotificationValidationErrorResponse(message); + return ValidationErrorResult.CreateNotificationValidationErrorResult(message); } // service is happy deleting a language that's fallback for another language, @@ -113,10 +110,10 @@ namespace Umbraco.Web.BackOffice.Controllers /// [Authorize(Policy = AuthorizationPolicies.TreeAccessLanguages)] [HttpPost] - public Language SaveLanguage(Language language) + public ActionResult SaveLanguage(Language language) { if (!ModelState.IsValid) - throw HttpResponseException.CreateValidationErrorResponse(ModelState); + return new ValidationErrorResult(ModelState); // this is prone to race conditions but the service will not let us proceed anyways var existingByCulture = _localizationService.GetLanguageByIsoCode(language.IsoCode); @@ -132,7 +129,7 @@ namespace Umbraco.Web.BackOffice.Controllers { //someone is trying to create a language that already exist ModelState.AddModelError("IsoCode", "The language " + language.IsoCode + " already exists"); - throw HttpResponseException.CreateValidationErrorResponse(ModelState); + return new ValidationErrorResult(ModelState); } var existingById = language.Id != default ? _localizationService.GetLanguageById(language.Id) : null; @@ -149,7 +146,7 @@ namespace Umbraco.Web.BackOffice.Controllers catch (CultureNotFoundException) { ModelState.AddModelError("IsoCode", "No Culture found with name " + language.IsoCode); - throw HttpResponseException.CreateValidationErrorResponse(ModelState); + return new ValidationErrorResult(ModelState); } // create it (creating a new language cannot create a fallback cycle) @@ -172,7 +169,7 @@ namespace Umbraco.Web.BackOffice.Controllers if (existingById.IsDefault && !language.IsDefault) { ModelState.AddModelError("IsDefault", "Cannot un-default the default language."); - throw HttpResponseException.CreateValidationErrorResponse(ModelState); + return new ValidationErrorResult(ModelState); } existingById.IsDefault = language.IsDefault; @@ -187,12 +184,12 @@ namespace Umbraco.Web.BackOffice.Controllers if (!languages.ContainsKey(existingById.FallbackLanguageId.Value)) { ModelState.AddModelError("FallbackLanguage", "The selected fall back language does not exist."); - throw HttpResponseException.CreateValidationErrorResponse(ModelState); + return new ValidationErrorResult(ModelState); } if (CreatesCycle(existingById, languages)) { ModelState.AddModelError("FallbackLanguage", $"The selected fall back language {languages[existingById.FallbackLanguageId.Value].IsoCode} would create a circular path."); - throw HttpResponseException.CreateValidationErrorResponse(ModelState); + return new ValidationErrorResult(ModelState); } } diff --git a/src/Umbraco.Web.BackOffice/Controllers/LogViewerController.cs b/src/Umbraco.Web.BackOffice/Controllers/LogViewerController.cs index 5165d3a092..d77f76a4b2 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/LogViewerController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/LogViewerController.cs @@ -1,13 +1,13 @@ -using System; +using System; using System.Collections.Generic; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Umbraco.Core; using Umbraco.Core.Logging.Viewer; using Umbraco.Core.Models; +using Umbraco.Web.Common.ActionsResults; using Umbraco.Web.Common.Attributes; using Umbraco.Web.Common.Authorization; -using Umbraco.Web.Common.Exceptions; namespace Umbraco.Web.BackOffice.Controllers { @@ -44,53 +44,53 @@ namespace Umbraco.Web.BackOffice.Controllers } [HttpGet] - public int GetNumberOfErrors([FromQuery] DateTime? startDate = null,[FromQuery] DateTime? endDate = null) + public ActionResult GetNumberOfErrors([FromQuery] DateTime? startDate = null,[FromQuery] DateTime? endDate = null) { var logTimePeriod = GetTimePeriod(startDate, endDate); //We will need to stop the request if trying to do this on a 1GB file if (CanViewLogs(logTimePeriod) == false) { - throw HttpResponseException.CreateNotificationValidationErrorResponse("Unable to view logs, due to size"); + return ValidationErrorResult.CreateNotificationValidationErrorResult("Unable to view logs, due to size"); } return _logViewer.GetNumberOfErrors(logTimePeriod); } [HttpGet] - public LogLevelCounts GetLogLevelCounts([FromQuery] DateTime? startDate = null,[FromQuery] DateTime? endDate = null) + public ActionResult GetLogLevelCounts([FromQuery] DateTime? startDate = null,[FromQuery] DateTime? endDate = null) { var logTimePeriod = GetTimePeriod(startDate, endDate); //We will need to stop the request if trying to do this on a 1GB file if (CanViewLogs(logTimePeriod) == false) { - throw HttpResponseException.CreateNotificationValidationErrorResponse("Unable to view logs, due to size"); + return ValidationErrorResult.CreateNotificationValidationErrorResult("Unable to view logs, due to size"); } return _logViewer.GetLogLevelCounts(logTimePeriod); } [HttpGet] - public IEnumerable GetMessageTemplates([FromQuery] DateTime? startDate = null,[FromQuery] DateTime? endDate = null) + public ActionResult> GetMessageTemplates([FromQuery] DateTime? startDate = null,[FromQuery] DateTime? endDate = null) { var logTimePeriod = GetTimePeriod(startDate, endDate); //We will need to stop the request if trying to do this on a 1GB file if (CanViewLogs(logTimePeriod) == false) { - throw HttpResponseException.CreateNotificationValidationErrorResponse("Unable to view logs, due to size"); + return ValidationErrorResult.CreateNotificationValidationErrorResult("Unable to view logs, due to size"); } - return _logViewer.GetMessageTemplates(logTimePeriod); + return new ActionResult>(_logViewer.GetMessageTemplates(logTimePeriod)); } [HttpGet] - public PagedResult GetLogs(string orderDirection = "Descending", int pageNumber = 1, string filterExpression = null, [FromQuery(Name = "logLevels[]")]string[] logLevels = null, [FromQuery]DateTime? startDate = null, [FromQuery]DateTime? endDate = null) + public ActionResult> GetLogs(string orderDirection = "Descending", int pageNumber = 1, string filterExpression = null, [FromQuery(Name = "logLevels[]")]string[] logLevels = null, [FromQuery]DateTime? startDate = null, [FromQuery]DateTime? endDate = null) { var logTimePeriod = GetTimePeriod(startDate, endDate); //We will need to stop the request if trying to do this on a 1GB file if (CanViewLogs(logTimePeriod) == false) { - throw HttpResponseException.CreateNotificationValidationErrorResponse("Unable to view logs, due to size"); + return ValidationErrorResult.CreateNotificationValidationErrorResult("Unable to view logs, due to size"); } var direction = orderDirection == "Descending" ? Direction.Descending : Direction.Ascending; diff --git a/src/Umbraco.Web.BackOffice/Controllers/MacrosController.cs b/src/Umbraco.Web.BackOffice/Controllers/MacrosController.cs index 3ca89fa5ff..598fe15bf4 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/MacrosController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/MacrosController.cs @@ -1,4 +1,4 @@ -using Umbraco.Core.Services; +using Umbraco.Core.Services; using System; using System.Collections.Generic; using System.IO; @@ -12,14 +12,12 @@ using Umbraco.Core.Strings; using Umbraco.Web.Models.ContentEditing; using Constants = Umbraco.Core.Constants; using Umbraco.Core.PropertyEditors; -using Umbraco.Web.BackOffice.Filters; using Umbraco.Web.Common.Attributes; -using Umbraco.Web.Common.Exceptions; -using Umbraco.Web.Security; using Umbraco.Core; using Umbraco.Core.Mapping; using Umbraco.Core.Security; using Microsoft.AspNetCore.Authorization; +using Umbraco.Web.Common.ActionsResults; using Umbraco.Web.Common.Authorization; namespace Umbraco.Web.BackOffice.Controllers @@ -74,19 +72,19 @@ namespace Umbraco.Web.BackOffice.Controllers { if (string.IsNullOrWhiteSpace(name)) { - throw HttpResponseException.CreateNotificationValidationErrorResponse("Name can not be empty"); + return ValidationErrorResult.CreateNotificationValidationErrorResult("Name can not be empty"); } var alias = name.ToSafeAlias(_shortStringHelper); if (_macroService.GetByAlias(alias) != null) { - throw HttpResponseException.CreateNotificationValidationErrorResponse("Macro with this alias already exists"); + return ValidationErrorResult.CreateNotificationValidationErrorResult("Macro with this alias already exists"); } if (name == null || name.Length > 255) { - throw HttpResponseException.CreateNotificationValidationErrorResponse("Name cannnot be more than 255 characters in length."); + return ValidationErrorResult.CreateNotificationValidationErrorResult("Name cannnot be more than 255 characters in length."); } try @@ -106,19 +104,19 @@ namespace Umbraco.Web.BackOffice.Controllers { const string errorMessage = "Error creating macro"; _logger.LogError(exception, errorMessage); - throw HttpResponseException.CreateNotificationValidationErrorResponse(errorMessage); + return ValidationErrorResult.CreateNotificationValidationErrorResult(errorMessage); } } [HttpGet] [DetermineAmbiguousActionByPassingParameters] - public MacroDisplay GetById(int id) + public ActionResult GetById(int id) { var macro = _macroService.GetById(id); if (macro == null) { - throw HttpResponseException.CreateNotificationValidationErrorResponse($"Macro with id {id} does not exist"); + return ValidationErrorResult.CreateNotificationValidationErrorResult($"Macro with id {id} does not exist"); } var macroDisplay = MapToDisplay(macro); @@ -128,13 +126,13 @@ namespace Umbraco.Web.BackOffice.Controllers [HttpGet] [DetermineAmbiguousActionByPassingParameters] - public MacroDisplay GetById(Guid id) + public ActionResult GetById(Guid id) { var macro = _macroService.GetById(id); if (macro == null) { - throw HttpResponseException.CreateNotificationValidationErrorResponse($"Macro with id {id} does not exist"); + return ValidationErrorResult.CreateNotificationValidationErrorResult($"Macro with id {id} does not exist"); } var macroDisplay = MapToDisplay(macro); @@ -144,16 +142,16 @@ namespace Umbraco.Web.BackOffice.Controllers [HttpGet] [DetermineAmbiguousActionByPassingParameters] - public MacroDisplay GetById(Udi id) + public ActionResult GetById(Udi id) { var guidUdi = id as GuidUdi; if (guidUdi == null) - throw HttpResponseException.CreateNotificationValidationErrorResponse($"Macro with id {id} does not exist"); + return ValidationErrorResult.CreateNotificationValidationErrorResult($"Macro with id {id} does not exist"); var macro = _macroService.GetById(guidUdi.Guid); if (macro == null) { - throw HttpResponseException.CreateNotificationValidationErrorResponse($"Macro with id {id} does not exist"); + return ValidationErrorResult.CreateNotificationValidationErrorResult($"Macro with id {id} does not exist"); } var macroDisplay = MapToDisplay(macro); @@ -162,13 +160,13 @@ namespace Umbraco.Web.BackOffice.Controllers } [HttpPost] - public OkResult DeleteById(int id) + public ActionResult DeleteById(int id) { var macro = _macroService.GetById(id); if (macro == null) { - throw HttpResponseException.CreateNotificationValidationErrorResponse($"Macro with id {id} does not exist"); + return ValidationErrorResult.CreateNotificationValidationErrorResult($"Macro with id {id} does not exist"); } _macroService.Delete(macro); @@ -177,23 +175,23 @@ namespace Umbraco.Web.BackOffice.Controllers } [HttpPost] - public MacroDisplay Save(MacroDisplay macroDisplay) + public ActionResult Save(MacroDisplay macroDisplay) { if (macroDisplay == null) { - throw HttpResponseException.CreateNotificationValidationErrorResponse($"No macro data found in request"); + return ValidationErrorResult.CreateNotificationValidationErrorResult("No macro data found in request"); } if (macroDisplay.Name == null || macroDisplay.Name.Length > 255) { - throw HttpResponseException.CreateNotificationValidationErrorResponse($"Name cannnot be more than 255 characters in length."); + return ValidationErrorResult.CreateNotificationValidationErrorResult("Name cannnot be more than 255 characters in length."); } var macro = _macroService.GetById(int.Parse(macroDisplay.Id.ToString())); if (macro == null) { - throw HttpResponseException.CreateNotificationValidationErrorResponse($"Macro with id {macroDisplay.Id} does not exist"); + return ValidationErrorResult.CreateNotificationValidationErrorResult($"Macro with id {macroDisplay.Id} does not exist"); } if (macroDisplay.Alias != macro.Alias) @@ -202,7 +200,7 @@ namespace Umbraco.Web.BackOffice.Controllers if (macroByAlias != null) { - throw HttpResponseException.CreateNotificationValidationErrorResponse($"Macro with this alias already exists"); + return ValidationErrorResult.CreateNotificationValidationErrorResult("Macro with this alias already exists"); } } @@ -230,7 +228,7 @@ namespace Umbraco.Web.BackOffice.Controllers { const string errorMessage = "Error creating macro"; _logger.LogError(exception, errorMessage); - throw HttpResponseException.CreateNotificationValidationErrorResponse(errorMessage); + return ValidationErrorResult.CreateNotificationValidationErrorResult(errorMessage); } } diff --git a/src/Umbraco.Web.BackOffice/Controllers/MediaController.cs b/src/Umbraco.Web.BackOffice/Controllers/MediaController.cs index 67cd41ff29..465c1e06bf 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/MediaController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/MediaController.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.IO; using System.Linq; @@ -36,6 +36,7 @@ using Umbraco.Web.BackOffice.ActionResults; using Umbraco.Web.BackOffice.Authorization; using Umbraco.Web.BackOffice.Filters; using Umbraco.Web.BackOffice.ModelBinders; +using Umbraco.Web.Common.ActionsResults; using Umbraco.Web.Common.Attributes; using Umbraco.Web.Common.Authorization; using Umbraco.Web.Common.Exceptions; @@ -649,7 +650,7 @@ namespace Umbraco.Web.BackOffice.Controllers if (_mediaService.Sort(sortedMedia) == false) { _logger.LogWarning("Media sorting failed, this was probably caused by an event being cancelled"); - throw HttpResponseException.CreateValidationErrorResponse("Media sorting failed, this was probably caused by an event being cancelled"); + return new ValidationErrorResult("Media sorting failed, this was probably caused by an event being cancelled"); } return Ok(); } diff --git a/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs b/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs index 267539c97f..47487d6c5b 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.ComponentModel.DataAnnotations; using System.Linq; @@ -8,6 +8,7 @@ using System.Net.Mime; using System.Text; using System.Threading.Tasks; using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -27,13 +28,12 @@ using Umbraco.Core.Strings; using Umbraco.Extensions; using Umbraco.Web.BackOffice.Filters; using Umbraco.Web.BackOffice.ModelBinders; +using Umbraco.Web.Common.ActionsResults; using Umbraco.Web.Common.Attributes; using Umbraco.Web.Common.Authorization; -using Umbraco.Web.Common.Exceptions; using Umbraco.Web.Common.Filters; using Umbraco.Web.ContentApps; using Umbraco.Web.Models.ContentEditing; -using Umbraco.Web.Security; using Constants = Umbraco.Core.Constants; namespace Umbraco.Web.BackOffice.Controllers @@ -168,18 +168,18 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// [OutgoingEditorModelEvent] - public MemberDisplay GetEmpty(string contentTypeAlias = null) + public ActionResult GetEmpty(string contentTypeAlias = null) { IMember emptyContent; if (contentTypeAlias == null) { - throw new HttpResponseException(HttpStatusCode.NotFound); + return new ValidationErrorResult(contentTypeAlias, StatusCodes.Status404NotFound); } var contentType = _memberTypeService.Get(contentTypeAlias); if (contentType == null) { - throw new HttpResponseException(HttpStatusCode.NotFound); + return new ValidationErrorResult(contentType, StatusCodes.Status404NotFound); } var passwordGenerator = new PasswordGenerator(_passwordConfig); @@ -218,7 +218,7 @@ namespace Umbraco.Web.BackOffice.Controllers { var forDisplay = _umbracoMapper.Map(contentItem.PersistedContent); forDisplay.Errors = ModelState.ToErrorDictionary(); - throw HttpResponseException.CreateValidationErrorResponse(forDisplay); + return new ValidationErrorResult(forDisplay); } //We're gonna look up the current roles now because the below code can cause @@ -241,7 +241,7 @@ namespace Umbraco.Web.BackOffice.Controllers break; default: //we don't support anything else for members - throw new HttpResponseException(HttpStatusCode.NotFound); + return new ValidationErrorResult(contentItem.Action, StatusCodes.Status404NotFound); } //TODO: There's 3 things saved here and we should do this all in one transaction, which we can do here by wrapping in a scope diff --git a/src/Umbraco.Web.BackOffice/Controllers/PackageInstallController.cs b/src/Umbraco.Web.BackOffice/Controllers/PackageInstallController.cs index 961ec388f7..e9ed78cdb6 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/PackageInstallController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/PackageInstallController.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.IO; using System.Linq; @@ -16,14 +16,12 @@ using Umbraco.Core.Packaging; using Umbraco.Core.Security; using Umbraco.Core.Services; using Umbraco.Core.WebAssets; -using Umbraco.Web.BackOffice.Filters; using Umbraco.Web.Common.Attributes; -using Umbraco.Web.Common.Exceptions; using Umbraco.Web.Models; using Umbraco.Web.Models.ContentEditing; -using Umbraco.Web.Security; using Microsoft.AspNetCore.Authorization; using Umbraco.Web.Common.Authorization; +using Umbraco.Web.Common.ActionsResults; namespace Umbraco.Web.BackOffice.Controllers { @@ -190,7 +188,7 @@ namespace Umbraco.Web.BackOffice.Controllers if (installType == PackageInstallType.AlreadyInstalled) { //this package is already installed - throw HttpResponseException.CreateNotificationValidationErrorResponse( + return ValidationErrorResult.CreateNotificationValidationErrorResult( _localizedTextService.Localize("packager/packageAlreadyInstalled")); } @@ -217,7 +215,7 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// [HttpGet] - public async Task Fetch(string packageGuid) + public async Task> Fetch(string packageGuid) { //Default path string fileName = packageGuid + ".umb"; @@ -244,7 +242,7 @@ namespace Umbraco.Web.BackOffice.Controllers if (installType == PackageInstallType.AlreadyInstalled) { - throw HttpResponseException.CreateNotificationValidationErrorResponse( + return ValidationErrorResult.CreateNotificationValidationErrorResult( _localizedTextService.Localize("packager/packageAlreadyInstalled")); } @@ -259,7 +257,7 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// [HttpPost] - public PackageInstallModel Import(PackageInstallModel model) + public ActionResult Import(PackageInstallModel model) { var zipFile = new FileInfo(Path.Combine(_hostingEnvironment.MapPathContentRoot(Constants.SystemDirectories.Packages), model.ZipFileName)); @@ -270,7 +268,7 @@ namespace Umbraco.Web.BackOffice.Controllers { var packageMinVersion = packageInfo.UmbracoVersion; if (_umbracoVersion.Current < packageMinVersion) - throw HttpResponseException.CreateNotificationValidationErrorResponse( + return ValidationErrorResult.CreateNotificationValidationErrorResult( _localizedTextService.Localize("packager/targetVersionMismatch", new[] {packageMinVersion.ToString()})); } @@ -289,7 +287,7 @@ namespace Umbraco.Web.BackOffice.Controllers //save to the installedPackages.config, this will create a new entry with a new Id if (!_packagingService.SaveInstalledPackage(packageDefinition)) - throw HttpResponseException.CreateNotificationValidationErrorResponse("Could not save the package"); + return ValidationErrorResult.CreateNotificationValidationErrorResult("Could not save the package"); model.Id = packageDefinition.Id; break; diff --git a/src/Umbraco.Web.BackOffice/Controllers/UsersController.cs b/src/Umbraco.Web.BackOffice/Controllers/UsersController.cs index 9d7999b9f7..844db81c03 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/UsersController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/UsersController.cs @@ -34,7 +34,6 @@ using Umbraco.Web.BackOffice.Security; using Umbraco.Web.Common.ActionsResults; using Umbraco.Web.Common.Attributes; using Umbraco.Web.Common.Authorization; -using Umbraco.Web.Common.Exceptions; using Umbraco.Web.Editors; using Umbraco.Web.Models; using Umbraco.Web.Models.ContentEditing; @@ -121,11 +120,11 @@ namespace Umbraco.Web.BackOffice.Controllers /// Returns a list of the sizes of gravatar URLs for the user or null if the gravatar server cannot be reached /// /// - public string[] GetCurrentUserAvatarUrls() + public ActionResult GetCurrentUserAvatarUrls() { var urls = _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.GetUserAvatarUrls(_appCaches.RuntimeCache, _mediaFileSystem, _imageUrlGenerator); if (urls == null) - throw new HttpResponseException(HttpStatusCode.BadRequest, "Could not access Gravatar endpoint"); + return new ValidationErrorResult("Could not access Gravatar endpoint"); return urls; } @@ -141,7 +140,7 @@ namespace Umbraco.Web.BackOffice.Controllers { if (files is null) { - throw new HttpResponseException(HttpStatusCode.UnsupportedMediaType); + return new ValidationErrorResult(files, StatusCodes.Status415UnsupportedMediaType); } var root = hostingEnvironment.MapPathContentRoot(Constants.SystemDirectories.TempFileUploads); @@ -159,7 +158,7 @@ namespace Umbraco.Web.BackOffice.Controllers return new NotFoundResult(); if (files.Count > 1) - throw HttpResponseException.CreateValidationErrorResponse("The request was not formatted correctly, only one file can be attached to the request"); + return new ValidationErrorResult("The request was not formatted correctly, only one file can be attached to the request"); //get the file info var file = files.First(); @@ -224,12 +223,12 @@ namespace Umbraco.Web.BackOffice.Controllers /// [OutgoingEditorModelEvent] [Authorize(Policy = AuthorizationPolicies.AdminUserEditsRequireAdmin)] - public UserDisplay GetById(int id) + public ActionResult GetById(int id) { var user = _userService.GetUserById(id); if (user == null) { - throw new HttpResponseException(HttpStatusCode.NotFound); + return new ValidationErrorResult(user, StatusCodes.Status404NotFound); } var result = _umbracoMapper.Map(user); return result; @@ -242,20 +241,20 @@ namespace Umbraco.Web.BackOffice.Controllers /// [OutgoingEditorModelEvent] [Authorize(Policy = AuthorizationPolicies.AdminUserEditsRequireAdmin)] - public IEnumerable GetByIds([FromJsonPath]int[] ids) + public ActionResult> GetByIds([FromJsonPath]int[] ids) { if (ids == null) { - throw new HttpResponseException(HttpStatusCode.NotFound); + return new ValidationErrorResult(ids, StatusCodes.Status404NotFound); } if (ids.Length == 0) - return Enumerable.Empty(); + return Enumerable.Empty().ToList(); var users = _userService.GetUsersById(ids); if (users == null) { - throw new HttpResponseException(HttpStatusCode.NotFound); + return new ValidationErrorResult(users, StatusCodes.Status404NotFound); } var result = _umbracoMapper.MapEnumerable(users); @@ -336,13 +335,13 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// /// - public async Task PostCreateUser(UserInvite userSave) + public async Task> PostCreateUser(UserInvite userSave) { if (userSave == null) throw new ArgumentNullException("userSave"); if (ModelState.IsValid == false) { - throw new HttpResponseException(HttpStatusCode.BadRequest, ModelState); + return new ValidationErrorResult(ModelState); } if (_securitySettings.UsernameIsEmail) @@ -361,7 +360,7 @@ namespace Umbraco.Web.BackOffice.Controllers var canSaveUser = _userEditorAuthorizationHelper.IsAuthorized(_backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser, null, null, null, userSave.UserGroups); if (canSaveUser == false) { - throw new HttpResponseException(HttpStatusCode.Unauthorized, canSaveUser.Result); + return new ValidationErrorResult(canSaveUser.Result, StatusCodes.Status401Unauthorized); } //we want to create the user with the UserManager, this ensures the 'empty' (special) password @@ -372,7 +371,7 @@ namespace Umbraco.Web.BackOffice.Controllers var created = await _userManager.CreateAsync(identityUser); if (created.Succeeded == false) { - throw HttpResponseException.CreateNotificationValidationErrorResponse(created.Errors.ToErrorMessage()); + return ValidationErrorResult.CreateNotificationValidationErrorResult(created.Errors.ToErrorMessage()); } string resetPassword; @@ -381,7 +380,7 @@ namespace Umbraco.Web.BackOffice.Controllers var result = await _userManager.AddPasswordAsync(identityUser, password); if (result.Succeeded == false) { - throw HttpResponseException.CreateNotificationValidationErrorResponse(created.Errors.ToErrorMessage()); + return ValidationErrorResult.CreateNotificationValidationErrorResult(created.Errors.ToErrorMessage()); } resetPassword = password; @@ -431,9 +430,9 @@ namespace Umbraco.Web.BackOffice.Controllers else { //first validate the username if we're showing it - user = CheckUniqueUsername(userSave.Username, u => u.LastLoginDate != default || u.EmailConfirmedDate.HasValue); + user = CheckUniqueUsername(userSave.Username, u => u.LastLoginDate != default || u.EmailConfirmedDate.HasValue).Value; } - user = CheckUniqueEmail(userSave.Email, u => u.LastLoginDate != default || u.EmailConfirmedDate.HasValue); + user = CheckUniqueEmail(userSave.Email, u => u.LastLoginDate != default || u.EmailConfirmedDate.HasValue).Value; if (!EmailSender.CanSendRequiredEmail(_globalSettings) && !_userManager.HasSendingUserInviteEventHandler) { @@ -512,18 +511,19 @@ namespace Umbraco.Web.BackOffice.Controllers return display; } - private IUser CheckUniqueEmail(string email, Func extraCheck) + private ActionResult CheckUniqueEmail(string email, Func extraCheck) { var user = _userService.GetByEmail(email); if (user != null && (extraCheck == null || extraCheck(user))) { ModelState.AddModelError("Email", "A user with the email already exists"); - throw new HttpResponseException(HttpStatusCode.BadRequest, ModelState); + return new ValidationErrorResult(ModelState); } - return user; + + return new ActionResult(user); } - private IUser CheckUniqueUsername(string username, Func extraCheck) + private ActionResult CheckUniqueUsername(string username, Func extraCheck) { var user = _userService.GetByUsername(username); if (user != null && (extraCheck == null || extraCheck(user))) @@ -531,9 +531,10 @@ namespace Umbraco.Web.BackOffice.Controllers ModelState.AddModelError( _securitySettings.UsernameIsEmail ? "Email" : "Username", "A user with the username already exists"); - throw new HttpResponseException(HttpStatusCode.BadRequest, ModelState); + return new ValidationErrorResult(ModelState); } - return user; + + return new ActionResult(user); } private async Task SendUserInviteEmailAsync(UserBasic userDisplay, string from, string fromEmail, IUser to, string message) @@ -576,28 +577,29 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// [OutgoingEditorModelEvent] - public UserDisplay PostSaveUser(UserSave userSave) + public ActionResult PostSaveUser(UserSave userSave) { if (userSave == null) throw new ArgumentNullException(nameof(userSave)); if (ModelState.IsValid == false) { - throw new HttpResponseException(HttpStatusCode.BadRequest, ModelState); + return new ValidationErrorResult(ModelState); } var intId = userSave.Id.TryConvertTo(); if (intId.Success == false) - throw new HttpResponseException(HttpStatusCode.NotFound); + return new ValidationErrorResult(intId, StatusCodes.Status404NotFound); + var found = _userService.GetUserById(intId.Result); if (found == null) - throw new HttpResponseException(HttpStatusCode.NotFound); + return new ValidationErrorResult(found, StatusCodes.Status404NotFound); //Perform authorization here to see if the current user can actually save this user with the info being requested var canSaveUser = _userEditorAuthorizationHelper.IsAuthorized(_backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser, found, userSave.StartContentIds, userSave.StartMediaIds, userSave.UserGroups); if (canSaveUser == false) { - throw new HttpResponseException(HttpStatusCode.Unauthorized, canSaveUser.Result); + return new ValidationErrorResult(canSaveUser.Result, StatusCodes.Status401Unauthorized); } var hasErrors = false; @@ -644,7 +646,7 @@ namespace Umbraco.Web.BackOffice.Controllers } if (hasErrors) - throw new HttpResponseException(HttpStatusCode.BadRequest, ModelState); + return new ValidationErrorResult(ModelState); //merge the save data onto the user var user = _umbracoMapper.Map(userSave, found); @@ -662,25 +664,25 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// /// - public async Task> PostChangePassword(ChangingPasswordModel changingPasswordModel) + public async Task>> PostChangePassword(ChangingPasswordModel changingPasswordModel) { changingPasswordModel = changingPasswordModel ?? throw new ArgumentNullException(nameof(changingPasswordModel)); if (ModelState.IsValid == false) { - throw new HttpResponseException(HttpStatusCode.BadRequest, ModelState); + return new ValidationErrorResult(ModelState); } var intId = changingPasswordModel.Id.TryConvertTo(); if (intId.Success == false) { - throw new HttpResponseException(HttpStatusCode.NotFound); + return new ValidationErrorResult(intId, StatusCodes.Status404NotFound); } var found = _userService.GetUserById(intId.Result); if (found == null) { - throw new HttpResponseException(HttpStatusCode.NotFound); + return new ValidationErrorResult(found, StatusCodes.Status404NotFound); } // TODO: Why don't we inject this? Then we can just inject a logger @@ -699,7 +701,7 @@ namespace Umbraco.Web.BackOffice.Controllers ModelState.AddModelError(memberName, passwordChangeResult.Result.ChangeError.ErrorMessage); } - throw HttpResponseException.CreateValidationErrorResponse(ModelState); + return new ValidationErrorResult(ModelState); } @@ -713,7 +715,7 @@ namespace Umbraco.Web.BackOffice.Controllers var tryGetCurrentUserId = _backofficeSecurityAccessor.BackOfficeSecurity.GetUserId(); if (tryGetCurrentUserId && userIds.Contains(tryGetCurrentUserId.Result)) { - throw HttpResponseException.CreateNotificationValidationErrorResponse("The current user cannot disable itself"); + return ValidationErrorResult.CreateNotificationValidationErrorResult("The current user cannot disable itself"); } var users = _userService.GetUsersById(userIds).ToArray(); @@ -780,8 +782,8 @@ namespace Umbraco.Web.BackOffice.Controllers var unlockResult = await _userManager.SetLockoutEndDateAsync(user, DateTimeOffset.Now); if (unlockResult.Succeeded == false) { - throw HttpResponseException.CreateValidationErrorResponse( - string.Format("Could not unlock for user {0} - error {1}", u, unlockResult.Errors.ToErrorMessage())); + return new ValidationErrorResult( + $"Could not unlock for user {u} - error {unlockResult.Errors.ToErrorMessage()}"); } if (userIds.Length == 1) @@ -857,6 +859,5 @@ namespace Umbraco.Web.BackOffice.Controllers [DataMember(Name = "userStates")] public IDictionary UserStates { get; set; } } - } }