From 3551a62979b542e42ba02a2993c56d988a476e4c Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Tue, 27 Feb 2024 15:28:49 -0800 Subject: [PATCH] Reduce Constructur Injections (#15395) --- .../Controllers/AdminController.cs | 203 +++++++++++------- .../Controllers/ApiController.cs | 9 +- .../Controllers/ItemController.cs | 15 +- 3 files changed, 139 insertions(+), 88 deletions(-) diff --git a/src/OrchardCore.Modules/OrchardCore.Contents/Controllers/AdminController.cs b/src/OrchardCore.Modules/OrchardCore.Contents/Controllers/AdminController.cs index 33ffa0dd8f7..6a4796edce4 100644 --- a/src/OrchardCore.Modules/OrchardCore.Contents/Controllers/AdminController.cs +++ b/src/OrchardCore.Modules/OrchardCore.Contents/Controllers/AdminController.cs @@ -9,7 +9,6 @@ using Microsoft.AspNetCore.Mvc.Rendering; using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.Localization; -using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using OrchardCore.Admin; using OrchardCore.ContentManagement; @@ -31,20 +30,15 @@ namespace OrchardCore.Contents.Controllers { - public class AdminController : Controller + public class AdminController : Controller, IUpdateModel { + private readonly IAuthorizationService _authorizationService; private readonly IContentManager _contentManager; + private readonly IContentItemDisplayManager _contentItemDisplayManager; private readonly IContentDefinitionManager _contentDefinitionManager; - private readonly PagerOptions _pagerOptions; + private readonly IDisplayManager _contentOptionsDisplayManager; private readonly ISession _session; - private readonly IContentItemDisplayManager _contentItemDisplayManager; private readonly INotifier _notifier; - private readonly IAuthorizationService _authorizationService; - private readonly IDisplayManager _contentOptionsDisplayManager; - private readonly IContentsAdminListQueryService _contentsAdminListQueryService; - private readonly IUpdateModelAccessor _updateModelAccessor; - private readonly IShapeFactory _shapeFactory; - private readonly ILogger _logger; protected readonly IHtmlLocalizer H; protected readonly IStringLocalizer S; @@ -54,37 +48,29 @@ public AdminController( IContentManager contentManager, IContentItemDisplayManager contentItemDisplayManager, IContentDefinitionManager contentDefinitionManager, - IOptions pagerOptions, - INotifier notifier, - ISession session, - IShapeFactory shapeFactory, IDisplayManager contentOptionsDisplayManager, - IContentsAdminListQueryService contentsAdminListQueryService, - ILogger logger, + ISession session, + INotifier notifier, IHtmlLocalizer htmlLocalizer, - IStringLocalizer stringLocalizer, - IUpdateModelAccessor updateModelAccessor) + IStringLocalizer stringLocalizer) { _authorizationService = authorizationService; - _notifier = notifier; - _contentItemDisplayManager = contentItemDisplayManager; - _session = session; - _pagerOptions = pagerOptions.Value; _contentManager = contentManager; + _contentItemDisplayManager = contentItemDisplayManager; _contentDefinitionManager = contentDefinitionManager; - _updateModelAccessor = updateModelAccessor; _contentOptionsDisplayManager = contentOptionsDisplayManager; - _contentsAdminListQueryService = contentsAdminListQueryService; - _shapeFactory = shapeFactory; - _logger = logger; + _session = session; + _notifier = notifier; H = htmlLocalizer; S = stringLocalizer; } - [HttpGet] [Admin("Contents/ContentItems/{contentTypeId?}", "ListContentItems")] public async Task List( + [FromServices] IOptions pagerOptions, + [FromServices] IShapeFactory shapeFactory, + [FromServices] IContentsAdminListQueryService contentsAdminListQueryService, [ModelBinder(BinderType = typeof(ContentItemFilterEngineModelBinder), Name = "q")] QueryFilterResult queryFilterResult, ContentOptionsViewModel options, PagerParameters pagerParameters, @@ -93,7 +79,7 @@ public async Task List( { var contentTypeDefinitions = (await _contentDefinitionManager.ListTypeDefinitionsAsync()) .OrderBy(ctd => ctd.DisplayName) - .ToList(); + .ToArray(); if (!await _authorizationService.AuthorizeContentTypeDefinitionsAsync(User, CommonPermissions.ListContent, contentTypeDefinitions, _contentManager)) { @@ -155,23 +141,23 @@ public async Task List( // We populate the remaining SelectLists. options.ContentStatuses = [ - new SelectListItem(S["Latest"], nameof(ContentsStatus.Latest), options.ContentsStatus == ContentsStatus.Latest), - new SelectListItem(S["Published"], nameof(ContentsStatus.Published), options.ContentsStatus == ContentsStatus.Published), - new SelectListItem(S["Unpublished"], nameof(ContentsStatus.Draft), options.ContentsStatus == ContentsStatus.Draft), - new SelectListItem(S["All versions"], nameof(ContentsStatus.AllVersions), options.ContentsStatus == ContentsStatus.AllVersions), + new SelectListItem(S["Latest"], nameof(ContentsStatus.Latest)), + new SelectListItem(S["Published"], nameof(ContentsStatus.Published)), + new SelectListItem(S["Unpublished"], nameof(ContentsStatus.Draft)), + new SelectListItem(S["All versions"], nameof(ContentsStatus.AllVersions)), ]; if (await IsAuthorizedAsync(Permissions.ListContent)) { - options.ContentStatuses.Insert(1, new SelectListItem() { Text = S["Owned by me"], Value = nameof(ContentsStatus.Owner) }); + options.ContentStatuses.Insert(1, new SelectListItem(S["Owned by me"], nameof(ContentsStatus.Owner))); } options.ContentSorts = [ - new SelectListItem(S["Recently created"], nameof(ContentsOrder.Created), options.OrderBy == ContentsOrder.Created), - new SelectListItem(S["Recently modified"], nameof(ContentsOrder.Modified), options.OrderBy == ContentsOrder.Modified), - new SelectListItem(S["Recently published"], nameof(ContentsOrder.Published), options.OrderBy == ContentsOrder.Published), - new SelectListItem(S["Title"], nameof(ContentsOrder.Title), options.OrderBy == ContentsOrder.Title), + new SelectListItem(S["Recently created"], nameof(ContentsOrder.Created)), + new SelectListItem(S["Recently modified"], nameof(ContentsOrder.Modified)), + new SelectListItem(S["Recently published"], nameof(ContentsOrder.Published)), + new SelectListItem(S["Title"], nameof(ContentsOrder.Title)), ]; options.ContentsBulkAction = @@ -191,7 +177,7 @@ public async Task List( options.ContentTypeOptions ??= []; // With the populated options, filter the query allowing the filters to alter the options. - var query = await _contentsAdminListQueryService.QueryAsync(options, _updateModelAccessor.ModelUpdater); + var query = await contentsAdminListQueryService.QueryAsync(options, this); // The search text is provided back to the UI. options.SearchText = options.FilterResult.ToString(); @@ -200,17 +186,24 @@ public async Task List( // Populate route values to maintain previous route data when generating page links. options.RouteValues.TryAdd("q", options.FilterResult.ToString()); - var pager = new Pager(pagerParameters, _pagerOptions.GetPageSize()); - dynamic pagerShape = await _shapeFactory.PagerAsync(pager, _pagerOptions.MaxPagedCount > 0 ? _pagerOptions.MaxPagedCount : await query.CountAsync(), options.RouteValues); + var pager = new Pager(pagerParameters, pagerOptions.Value.GetPageSize()); + + var itemsPerPage = pagerOptions.Value.MaxPagedCount > 0 + ? pagerOptions.Value.MaxPagedCount + : await query.CountAsync(); + + dynamic pagerShape = await shapeFactory.PagerAsync(pager, itemsPerPage, options.RouteValues); // Load items so that loading handlers are invoked. - var pageOfContentItems = await query.Skip(pager.GetStartIndex()).Take(pager.PageSize).ListAsync(_contentManager); + var pageOfContentItems = await query.Skip(pager.GetStartIndex()) + .Take(pager.PageSize) + .ListAsync(_contentManager); // We prepare the content items SummaryAdmin shape. var contentItemSummaries = new List(); foreach (var contentItem in pageOfContentItems) { - contentItemSummaries.Add(await _contentItemDisplayManager.BuildDisplayAsync(contentItem, _updateModelAccessor.ModelUpdater, "SummaryAdmin")); + contentItemSummaries.Add(await _contentItemDisplayManager.BuildDisplayAsync(contentItem, this, "SummaryAdmin")); } // Populate options pager summary values. @@ -220,9 +213,9 @@ public async Task List( options.ContentItemsCount = contentItemSummaries.Count; options.TotalItemCount = pagerShape.TotalItemCount; - var header = await _contentOptionsDisplayManager.BuildEditorAsync(options, _updateModelAccessor.ModelUpdater, false, string.Empty, string.Empty); + var header = await _contentOptionsDisplayManager.BuildEditorAsync(options, this, false, string.Empty, string.Empty); - var shapeViewModel = await _shapeFactory.CreateAsync("ContentsAdminList", viewModel => + var shapeViewModel = await shapeFactory.CreateAsync("ContentsAdminList", viewModel => { viewModel.ContentItems = contentItemSummaries; viewModel.Pager = pagerShape; @@ -233,18 +226,22 @@ public async Task List( return View(shapeViewModel); } - [HttpPost, ActionName(nameof(List))] + [HttpPost] + [ActionName(nameof(List))] [FormValueRequired("submit.Filter")] public async Task ListFilterPOST(ContentOptionsViewModel options) { // When the user has typed something into the search input no further evaluation of the form post is required. if (!string.Equals(options.SearchText, options.OriginalSearchText, StringComparison.OrdinalIgnoreCase)) { - return RedirectToAction(nameof(List), new RouteValueDictionary { { "q", options.SearchText } }); + return RedirectToAction(nameof(List), new RouteValueDictionary + { + { "q", options.SearchText }, + }); } // Evaluate the values provided in the form post and map them to the filter result and route values. - await _contentOptionsDisplayManager.UpdateEditorAsync(options, _updateModelAccessor.ModelUpdater, false, string.Empty, string.Empty); + await _contentOptionsDisplayManager.UpdateEditorAsync(options, this, false, string.Empty, string.Empty); // The route value must always be added after the editors have updated the models. options.RouteValues.TryAdd("q", options.FilterResult.ToString()); @@ -252,14 +249,18 @@ public async Task ListFilterPOST(ContentOptionsViewModel options) return RedirectToAction(nameof(List), options.RouteValues); } - [HttpPost, ActionName(nameof(List))] + [HttpPost] + [ActionName(nameof(List))] [FormValueRequired("submit.BulkAction")] - public async Task ListPOST(ContentOptionsViewModel options, IEnumerable itemIds) + public async Task ListPOST(ContentOptionsViewModel options, long[] itemIds) { - if (itemIds?.Count() > 0) + if (itemIds.Length > 0) { // Load items so that loading handlers are invoked. - var checkedContentItems = await _session.Query().Where(x => x.DocumentId.IsIn(itemIds) && x.Latest).ListAsync(_contentManager); + var checkedContentItems = await _session.Query() + .Where(x => x.DocumentId.IsIn(itemIds) && x.Latest) + .ListAsync(_contentManager); + switch (options.BulkAction) { case ContentsBulkAction.None: @@ -307,7 +308,7 @@ public async Task ListPOST(ContentOptionsViewModel options, IEnume await _notifier.SuccessAsync(H["Content removed successfully."]); break; default: - throw new ArgumentOutOfRangeException(options.BulkAction.ToString(), "Invalid bulk action."); + return BadRequest(); } } @@ -329,14 +330,18 @@ public async Task Create(string id) return Forbid(); } - var model = await _contentItemDisplayManager.BuildEditorAsync(contentItem, _updateModelAccessor.ModelUpdater, true); + var model = await _contentItemDisplayManager.BuildEditorAsync(contentItem, this, true); return View(model); } - [HttpPost, ActionName(nameof(Create))] + [HttpPost] + [ActionName(nameof(Create))] [FormValueRequired("submit.Save")] - public Task CreatePOST(string id, [Bind(Prefix = "submit.Save")] string submitSave, string returnUrl) + public Task CreatePOST( + string id, + [Bind(Prefix = "submit.Save")] string submitSave, + string returnUrl) { var stayOnSamePage = submitSave == "submit.SaveAndContinue"; return CreatePOST(id, returnUrl, stayOnSamePage, async contentItem => @@ -351,9 +356,13 @@ await _notifier.SuccessAsync(string.IsNullOrWhiteSpace(typeDefinition?.DisplayNa }); } - [HttpPost, ActionName(nameof(Create))] + [HttpPost] + [ActionName(nameof(Create))] [FormValueRequired("submit.Publish")] - public async Task CreateAndPublishPOST(string id, [Bind(Prefix = "submit.Publish")] string submitPublish, string returnUrl) + public async Task CreateAndPublishPOST( + string id, + [Bind(Prefix = "submit.Publish")] string submitPublish, + string returnUrl) { if (string.IsNullOrEmpty(id)) { @@ -394,7 +403,7 @@ public async Task Display(string contentItemId) return Forbid(); } - var model = await _contentItemDisplayManager.BuildDisplayAsync(contentItem, _updateModelAccessor.ModelUpdater, "DetailAdmin"); + var model = await _contentItemDisplayManager.BuildDisplayAsync(contentItem, this, "DetailAdmin"); return View(model); } @@ -414,14 +423,18 @@ public async Task Edit(string contentItemId) return Forbid(); } - var model = await _contentItemDisplayManager.BuildEditorAsync(contentItem, _updateModelAccessor.ModelUpdater, false); + var model = await _contentItemDisplayManager.BuildEditorAsync(contentItem, this, false); return View(model); } - [HttpPost, ActionName(nameof(Edit))] + [HttpPost] + [ActionName(nameof(Edit))] [FormValueRequired("submit.Save")] - public Task EditPOST(string contentItemId, [Bind(Prefix = "submit.Save")] string submitSave, string returnUrl) + public Task EditPOST( + string contentItemId, + [Bind(Prefix = "submit.Save")] string submitSave, + string returnUrl) { var stayOnSamePage = submitSave == "submit.SaveAndContinue"; return EditPOST(contentItemId, returnUrl, stayOnSamePage, async contentItem => @@ -436,9 +449,13 @@ await _notifier.SuccessAsync(string.IsNullOrWhiteSpace(typeDefinition?.DisplayNa }); } - [HttpPost, ActionName(nameof(Edit))] + [HttpPost] + [ActionName(nameof(Edit))] [FormValueRequired("submit.Publish")] - public async Task EditAndPublishPOST(string contentItemId, [Bind(Prefix = "submit.Publish")] string submitPublish, string returnUrl) + public async Task EditAndPublishPOST( + string contentItemId, + [Bind(Prefix = "submit.Publish")] string submitPublish, + string returnUrl) { var stayOnSamePage = submitPublish == "submit.PublishAndContinue"; @@ -489,12 +506,16 @@ public async Task Clone(string contentItemId, string returnUrl) catch (InvalidOperationException) { await _notifier.WarningAsync(H["Could not clone the content item."]); - return Url.IsLocalUrl(returnUrl) ? (IActionResult)this.LocalRedirect(returnUrl, true) : RedirectToAction(nameof(List)); + return Url.IsLocalUrl(returnUrl) + ? this.LocalRedirect(returnUrl, true) + : RedirectToAction(nameof(List)); } await _notifier.InformationAsync(H["Successfully cloned. The clone was saved as a draft."]); - return Url.IsLocalUrl(returnUrl) ? (IActionResult)this.LocalRedirect(returnUrl, true) : RedirectToAction(nameof(List)); + return Url.IsLocalUrl(returnUrl) + ? this.LocalRedirect(returnUrl, true) + : RedirectToAction(nameof(List)); } [HttpPost] @@ -524,7 +545,9 @@ await _notifier.SuccessAsync(string.IsNullOrWhiteSpace(typeDefinition?.DisplayNa : H["The {0} draft has been removed.", typeDefinition.DisplayName]); } - return Url.IsLocalUrl(returnUrl) ? (IActionResult)this.LocalRedirect(returnUrl, true) : RedirectToAction(nameof(List)); + return Url.IsLocalUrl(returnUrl) + ? this.LocalRedirect(returnUrl, true) + : RedirectToAction(nameof(List)); } [HttpPost] @@ -549,7 +572,9 @@ await _notifier.SuccessAsync(string.IsNullOrWhiteSpace(typeDefinition?.DisplayNa : H["That {0} has been removed.", typeDefinition.DisplayName]); } - return Url.IsLocalUrl(returnUrl) ? (IActionResult)this.LocalRedirect(returnUrl, true) : RedirectToAction(nameof(List)); + return Url.IsLocalUrl(returnUrl) + ? this.LocalRedirect(returnUrl, true) + : RedirectToAction(nameof(List)); } [HttpPost] @@ -580,7 +605,9 @@ public async Task Publish(string contentItemId, string returnUrl) await _notifier.SuccessAsync(H["That {0} has been published.", typeDefinition.DisplayName]); } - return Url.IsLocalUrl(returnUrl) ? (IActionResult)this.LocalRedirect(returnUrl, true) : RedirectToAction(nameof(List)); + return Url.IsLocalUrl(returnUrl) + ? this.LocalRedirect(returnUrl, true) + : RedirectToAction(nameof(List)); } [HttpPost] @@ -611,10 +638,16 @@ public async Task Unpublish(string contentItemId, string returnUr await _notifier.SuccessAsync(H["The {0} has been unpublished.", typeDefinition.DisplayName]); } - return Url.IsLocalUrl(returnUrl) ? (IActionResult)this.LocalRedirect(returnUrl, true) : RedirectToAction(nameof(List)); + return Url.IsLocalUrl(returnUrl) + ? this.LocalRedirect(returnUrl, true) + : RedirectToAction(nameof(List)); } - private async Task CreatePOST(string id, string returnUrl, bool stayOnSamePage, Func conditionallyPublish) + private async Task CreatePOST( + string id, + string returnUrl, + bool stayOnSamePage, + Func conditionallyPublish) { var contentItem = await CreateContentItemOwnedByCurrentUserAsync(id); @@ -623,7 +656,7 @@ private async Task CreatePOST(string id, string returnUrl, bool s return Forbid(); } - var model = await _contentItemDisplayManager.UpdateEditorAsync(contentItem, _updateModelAccessor.ModelUpdater, true); + var model = await _contentItemDisplayManager.UpdateEditorAsync(contentItem, this, true); if (ModelState.IsValid) { @@ -653,7 +686,11 @@ private async Task CreatePOST(string id, string returnUrl, bool s return RedirectToRoute(adminRouteValues); } - private async Task EditPOST(string contentItemId, string returnUrl, bool stayOnSamePage, Func conditionallyPublish) + private async Task EditPOST( + string contentItemId, + string returnUrl, + bool stayOnSamePage, + Func conditionallyPublish) { var contentItem = await _contentManager.GetAsync(contentItemId, VersionOptions.DraftRequired); @@ -667,7 +704,7 @@ private async Task EditPOST(string contentItemId, string returnUr return Forbid(); } - var model = await _contentItemDisplayManager.UpdateEditorAsync(contentItem, _updateModelAccessor.ModelUpdater, false); + var model = await _contentItemDisplayManager.UpdateEditorAsync(contentItem, this, false); if (!ModelState.IsValid) { @@ -679,18 +716,27 @@ private async Task EditPOST(string contentItemId, string returnUr if (returnUrl == null) { - return RedirectToAction(nameof(Edit), new RouteValueDictionary { { "ContentItemId", contentItem.ContentItemId } }); + return RedirectToAction(nameof(Edit), new RouteValueDictionary + { + { "ContentItemId", contentItem.ContentItemId }, + }); } if (stayOnSamePage) { - return RedirectToAction(nameof(Edit), new RouteValueDictionary { { "ContentItemId", contentItem.ContentItemId }, { "returnUrl", returnUrl } }); + return RedirectToAction(nameof(Edit), new RouteValueDictionary + { + { "ContentItemId", contentItem.ContentItemId }, + { "returnUrl", returnUrl }, + }); } return this.LocalRedirect(returnUrl, true); } - private async Task> GetCreatableTypeOptionsAsync(bool canCreateSelectedContentType, params ContentTypeDefinition[] contentTypeDefinitions) + private async Task> GetCreatableTypeOptionsAsync( + bool canCreateSelectedContentType, + params ContentTypeDefinition[] contentTypeDefinitions) { var options = new List(); @@ -709,7 +755,10 @@ private async Task> GetCreatableTypeOptionsAsync(bool canCr return options; } - private async Task> GetListableContentTypeOptionsAsync(IEnumerable definitions, string selectedContentType, bool showSelectAll = true) + private async Task> GetListableContentTypeOptionsAsync( + IEnumerable definitions, + string selectedContentType, + bool showSelectAll = true) { var currentUserId = CurrentUserId(); diff --git a/src/OrchardCore.Modules/OrchardCore.Contents/Controllers/ApiController.cs b/src/OrchardCore.Modules/OrchardCore.Contents/Controllers/ApiController.cs index 8b0826f11c1..59e07313e6c 100644 --- a/src/OrchardCore.Modules/OrchardCore.Contents/Controllers/ApiController.cs +++ b/src/OrchardCore.Modules/OrchardCore.Contents/Controllers/ApiController.cs @@ -18,11 +18,15 @@ namespace OrchardCore.Contents.Controllers [Authorize(AuthenticationSchemes = "Api"), IgnoreAntiforgeryToken, AllowAnonymous] public class ApiController : Controller { - private static readonly JsonMergeSettings _updateJsonMergeSettings = new() { MergeArrayHandling = MergeArrayHandling.Replace }; + private static readonly JsonMergeSettings _updateJsonMergeSettings = new() + { + MergeArrayHandling = MergeArrayHandling.Replace + }; private readonly IContentManager _contentManager; private readonly IContentDefinitionManager _contentDefinitionManager; private readonly IAuthorizationService _authorizationService; + protected readonly IStringLocalizer S; public ApiController( @@ -37,7 +41,8 @@ public ApiController( S = stringLocalizer; } - [Route("{contentItemId}"), HttpGet] + [HttpGet] + [Route("{contentItemId}")] public async Task Get(string contentItemId) { if (!await _authorizationService.AuthorizeAsync(User, Permissions.AccessContentApi)) diff --git a/src/OrchardCore.Modules/OrchardCore.Contents/Controllers/ItemController.cs b/src/OrchardCore.Modules/OrchardCore.Contents/Controllers/ItemController.cs index 5f6c9cee5d8..66f5aaeb5c5 100644 --- a/src/OrchardCore.Modules/OrchardCore.Contents/Controllers/ItemController.cs +++ b/src/OrchardCore.Modules/OrchardCore.Contents/Controllers/ItemController.cs @@ -7,23 +7,20 @@ namespace OrchardCore.Contents.Controllers { - public class ItemController : Controller + public class ItemController : Controller, IUpdateModel { private readonly IContentManager _contentManager; private readonly IContentItemDisplayManager _contentItemDisplayManager; private readonly IAuthorizationService _authorizationService; - private readonly IUpdateModelAccessor _updateModelAccessor; public ItemController( IContentManager contentManager, IContentItemDisplayManager contentItemDisplayManager, - IAuthorizationService authorizationService, - IUpdateModelAccessor updateModelAccessor) + IAuthorizationService authorizationService) { - _authorizationService = authorizationService; - _contentItemDisplayManager = contentItemDisplayManager; _contentManager = contentManager; - _updateModelAccessor = updateModelAccessor; + _contentItemDisplayManager = contentItemDisplayManager; + _authorizationService = authorizationService; } public async Task Display(string contentItemId, string jsonPath) @@ -40,7 +37,7 @@ public async Task Display(string contentItemId, string jsonPath) return this.ChallengeOrForbid(); } - var model = await _contentItemDisplayManager.BuildDisplayAsync(contentItem, _updateModelAccessor.ModelUpdater); + var model = await _contentItemDisplayManager.BuildDisplayAsync(contentItem, this); return View(model); } @@ -66,7 +63,7 @@ public async Task Preview(string contentItemId) return this.ChallengeOrForbid(); } - var model = await _contentItemDisplayManager.BuildDisplayAsync(contentItem, _updateModelAccessor.ModelUpdater); + var model = await _contentItemDisplayManager.BuildDisplayAsync(contentItem, this); return View(model); }