From abb5911b245873050a8237b620060eb8349c19b6 Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 26 Feb 2021 16:53:53 +1100 Subject: [PATCH] WIP fixing issue with macro forms --- .../Controllers/ProxyViewDataFeature.cs | 13 ++- .../UmbracoBuilderExtensions.cs | 1 + .../Macros/MacroRenderer.cs | 27 ++--- .../Macros/PartialViewMacroEngine.cs | 101 +++++------------- .../Macros/PartialViewMacroViewComponent.cs | 14 ++- .../Templates/TemplateRenderer.cs | 21 ++-- .../Controllers/SurfaceController.cs | 2 +- .../Controllers/UmbLoginController.cs | 6 +- .../Extensions/HtmlHelperRenderExtensions.cs | 13 +-- 9 files changed, 79 insertions(+), 119 deletions(-) diff --git a/src/Umbraco.Web.Common/Controllers/ProxyViewDataFeature.cs b/src/Umbraco.Web.Common/Controllers/ProxyViewDataFeature.cs index f926ccbfaa..e44f3270fe 100644 --- a/src/Umbraco.Web.Common/Controllers/ProxyViewDataFeature.cs +++ b/src/Umbraco.Web.Common/Controllers/ProxyViewDataFeature.cs @@ -1,4 +1,4 @@ -using Microsoft.AspNetCore.Mvc.ViewFeatures; +using Microsoft.AspNetCore.Mvc.ViewFeatures; namespace Umbraco.Cms.Web.Common.Controllers { @@ -10,11 +10,20 @@ namespace Umbraco.Cms.Web.Common.Controllers /// /// Initializes a new instance of the class. /// - public ProxyViewDataFeature(ViewDataDictionary viewData) => ViewData = viewData; + public ProxyViewDataFeature(ViewDataDictionary viewData, ITempDataDictionary tempData) + { + ViewData = viewData; + TempData = tempData; + } /// /// Gets the /// public ViewDataDictionary ViewData { get; } + + /// + /// Gets the + /// + public ITempDataDictionary TempData { get; } } } diff --git a/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilderExtensions.cs b/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilderExtensions.cs index 9dd375c9af..9e2e79cb9b 100644 --- a/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilderExtensions.cs +++ b/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilderExtensions.cs @@ -255,6 +255,7 @@ namespace Umbraco.Extensions builder.Services.AddUnique(); builder.Services.AddUnique(); + builder.Services.AddUnique(); builder.Services.AddUnique(); // register the umbraco context factory diff --git a/src/Umbraco.Web.Common/Macros/MacroRenderer.cs b/src/Umbraco.Web.Common/Macros/MacroRenderer.cs index f798199012..389267cd61 100644 --- a/src/Umbraco.Web.Common/Macros/MacroRenderer.cs +++ b/src/Umbraco.Web.Common/Macros/MacroRenderer.cs @@ -37,6 +37,7 @@ namespace Umbraco.Cms.Web.Common.Macros private readonly ISessionManager _sessionManager; private readonly IRequestAccessor _requestAccessor; private readonly IHttpContextAccessor _httpContextAccessor; + private readonly PartialViewMacroEngine _partialViewMacroEngine; public MacroRenderer( IProfilingLogger profilingLogger, @@ -52,7 +53,8 @@ namespace Umbraco.Cms.Web.Common.Macros IMemberUserKeyProvider memberUserKeyProvider, ISessionManager sessionManager, IRequestAccessor requestAccessor, - IHttpContextAccessor httpContextAccessor) + IHttpContextAccessor httpContextAccessor, + PartialViewMacroEngine partialViewMacroEngine) { _profilingLogger = profilingLogger ?? throw new ArgumentNullException(nameof(profilingLogger)); _logger = logger; @@ -68,6 +70,7 @@ namespace Umbraco.Cms.Web.Common.Macros _sessionManager = sessionManager; _requestAccessor = requestAccessor; _httpContextAccessor = httpContextAccessor; + _partialViewMacroEngine = partialViewMacroEngine; } #region MacroContent cache @@ -338,38 +341,28 @@ namespace Umbraco.Cms.Web.Common.Macros private Attempt ExecuteMacroOfType(MacroModel model, IPublishedContent content) { if (model == null) + { throw new ArgumentNullException(nameof(model)); + } // ensure that we are running against a published node (ie available in XML) // that may not be the case if the macro is embedded in a RTE of an unpublished document if (content == null) + { return Attempt.Fail(new MacroContent { Text = "[macro failed (no content)]" }); + } - var textService = _textService; + ILocalizedTextService textService = _textService; return ExecuteMacroWithErrorWrapper(model, $"Executing PartialView: MacroSource=\"{model.MacroSource}\".", "Executed PartialView.", - () => ExecutePartialView(model, content), + () => _partialViewMacroEngine.Execute(model, content), () => textService.Localize("errors/macroErrorLoadingPartialView", new[] { model.MacroSource })); } - #endregion - - #region Execute engines - - /// - /// Renders a PartialView Macro. - /// - /// The text output of the macro execution. - private MacroContent ExecutePartialView(MacroModel macro, IPublishedContent content) - { - var engine = new PartialViewMacroEngine(_httpContextAccessor, _hostingEnvironment); - return engine.Execute(macro, content); - } - #endregion #region Execution helpers diff --git a/src/Umbraco.Web.Common/Macros/PartialViewMacroEngine.cs b/src/Umbraco.Web.Common/Macros/PartialViewMacroEngine.cs index ef59c9f896..5b37557cd2 100644 --- a/src/Umbraco.Web.Common/Macros/PartialViewMacroEngine.cs +++ b/src/Umbraco.Web.Common/Macros/PartialViewMacroEngine.cs @@ -16,6 +16,7 @@ using Microsoft.Extensions.DependencyInjection; using Umbraco.Cms.Core.Hosting; using Umbraco.Cms.Core.Macros; using Umbraco.Cms.Core.Models.PublishedContent; +using Umbraco.Cms.Web.Common.Controllers; using Umbraco.Extensions; using static Umbraco.Cms.Core.Constants.Web.Routing; @@ -27,47 +28,19 @@ namespace Umbraco.Cms.Web.Common.Macros public class PartialViewMacroEngine { private readonly IHttpContextAccessor _httpContextAccessor; - private readonly IHostingEnvironment _hostingEnvironment; - //private readonly Func _getUmbracoContext; + private readonly IModelMetadataProvider _modelMetadataProvider; + private readonly ITempDataProvider _tempDataProvider; public PartialViewMacroEngine( - //IUmbracoContextAccessor umbracoContextAccessor, IHttpContextAccessor httpContextAccessor, - IHostingEnvironment hostingEnvironment) + IModelMetadataProvider modelMetadataProvider, + ITempDataProvider tempDataProvider) { _httpContextAccessor = httpContextAccessor; - _hostingEnvironment = hostingEnvironment; - - //_getUmbracoContext = () => - //{ - // var context = umbracoContextAccessor.UmbracoContext; - // if (context == null) - // { - // throw new InvalidOperationException( - // $"The {GetType()} cannot execute with a null UmbracoContext.Current reference."); - // } - - // return context; - //}; + _modelMetadataProvider = modelMetadataProvider; + _tempDataProvider = tempDataProvider; } - //public bool Validate(string code, string tempFileName, IPublishedContent currentPage, out string errorMessage) - //{ - // var temp = GetVirtualPathFromPhysicalPath(tempFileName); - // try - // { - // CompileAndInstantiate(temp); - // } - // catch (Exception exception) - // { - // errorMessage = exception.Message; - // return false; - // } - - // errorMessage = string.Empty; - // return true; - //} - public MacroContent Execute(MacroModel macro, IPublishedContent content) { if (macro == null) @@ -85,31 +58,32 @@ namespace Umbraco.Cms.Web.Common.Macros throw new ArgumentException("The MacroSource property of the macro object cannot be null or empty"); } - var httpContext = _httpContextAccessor.GetRequiredHttpContext(); - //var umbCtx = _getUmbracoContext(); - var routeVals = new RouteData(); - routeVals.Values.Add(ControllerToken, "PartialViewMacro"); - routeVals.Values.Add(ActionToken, "Index"); + HttpContext httpContext = _httpContextAccessor.GetRequiredHttpContext(); + //var routeVals = new RouteData(); + //routeVals.Values.Add(ControllerToken, "PartialViewMacro"); + //routeVals.Values.Add(ActionToken, "Index"); //TODO: Was required for UmbracoViewPage need to figure out if we still need that, i really don't think this is necessary //routeVals.DataTokens.Add(Core.Constants.Web.UmbracoContextDataToken, umbCtx); - var modelMetadataProvider = httpContext.RequestServices.GetRequiredService(); - var tempDataProvider = httpContext.RequestServices.GetRequiredService(); + RouteData currentRouteData = httpContext.GetRouteData(); + + // Check if there's proxied ViewData (i.e. returned from a SurfaceController) + ProxyViewDataFeature proxyViewDataFeature = httpContext.Features.Get(); + ViewDataDictionary viewData = proxyViewDataFeature?.ViewData ?? new ViewDataDictionary(_modelMetadataProvider, new ModelStateDictionary()); + ITempDataDictionary tempData = proxyViewDataFeature?.TempData ?? new TempDataDictionary(httpContext, _tempDataProvider); var viewContext = new ViewContext( - new ActionContext(httpContext, httpContext.GetRouteData(), new ControllerActionDescriptor()), + new ActionContext(httpContext, currentRouteData, new ControllerActionDescriptor()), new FakeView(), - new ViewDataDictionary(modelMetadataProvider, new ModelStateDictionary()), - new TempDataDictionary(httpContext, tempDataProvider), + viewData, + tempData, TextWriter.Null, new HtmlHelperOptions() ); - routeVals.DataTokens.Add("ParentActionViewContext", viewContext); - - var viewComponent = new PartialViewMacroViewComponent(macro, content); + //routeVals.DataTokens.Add("ParentActionViewContext", viewContext); var writer = new StringWriter(); var viewComponentContext = new ViewComponentContext( @@ -119,7 +93,9 @@ namespace Umbraco.Cms.Web.Common.Macros viewContext, writer); - viewComponent.InvokeAsync().GetAwaiter().GetResult().Execute(viewComponentContext); + var viewComponent = new PartialViewMacroViewComponent(macro, content, viewComponentContext); + + viewComponent.Invoke().Execute(viewComponentContext); var output = writer.GetStringBuilder().ToString(); @@ -129,38 +105,11 @@ namespace Umbraco.Cms.Web.Common.Macros private class FakeView : IView { /// - public Task RenderAsync(ViewContext context) - { - return Task.CompletedTask; - } + public Task RenderAsync(ViewContext context) => Task.CompletedTask; /// public string Path { get; } = "View"; } - - //private string GetVirtualPathFromPhysicalPath(string physicalPath) - //{ - // var rootpath = _hostingEnvironment.MapPathContentRoot("~/"); - // physicalPath = physicalPath.Replace(rootpath, ""); - // physicalPath = physicalPath.Replace("\\", "/"); - // return "~/" + physicalPath; - //} - - //private static PartialViewMacroPage CompileAndInstantiate(string virtualPath) - //{ - // // //Compile Razor - We Will Leave This To ASP.NET Compilation Engine & ASP.NET WebPages - // // //Security in medium trust is strict around here, so we can only pass a virtual file path - // // //ASP.NET Compilation Engine caches returned types - // // //Changed From BuildManager As Other Properties Are Attached Like Context Path/ - // // var webPageBase = WebPageBase.CreateInstanceFromVirtualPath(virtualPath); - // // var webPage = webPageBase as PartialViewMacroPage; - // // if (webPage == null) - // // throw new InvalidCastException("All Partial View Macro views must inherit from " + typeof(PartialViewMacroPage).FullName); - // // return webPage; - - // //TODO? How to check this - // return null; - //} } } diff --git a/src/Umbraco.Web.Common/Macros/PartialViewMacroViewComponent.cs b/src/Umbraco.Web.Common/Macros/PartialViewMacroViewComponent.cs index 2b317585b4..5cab93c00f 100644 --- a/src/Umbraco.Web.Common/Macros/PartialViewMacroViewComponent.cs +++ b/src/Umbraco.Web.Common/Macros/PartialViewMacroViewComponent.cs @@ -1,6 +1,7 @@ -using System.Linq; +using System.Linq; using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.ViewComponents; using Umbraco.Cms.Core.Composing; using Umbraco.Cms.Core.Macros; using Umbraco.Cms.Core.Models; @@ -20,13 +21,17 @@ namespace Umbraco.Cms.Web.Common.Macros public PartialViewMacroViewComponent( MacroModel macro, - IPublishedContent content) + IPublishedContent content, + ViewComponentContext viewComponentContext) { _macro = macro; _content = content; + // This must be set before Invoke is called else the call to View will end up + // using an empty ViewData instance because this hasn't been set yet. + ViewComponentContext = viewComponentContext; } - public async Task InvokeAsync() + public IViewComponentResult Invoke() { var model = new PartialViewMacroModel( _content, @@ -34,7 +39,8 @@ namespace Umbraco.Cms.Web.Common.Macros _macro.Alias, _macro.Name, _macro.Properties.ToDictionary(x => x.Key, x => (object)x.Value)); - var result = View(_macro.MacroSource, model); + + ViewViewComponentResult result = View(_macro.MacroSource, model); return result; } diff --git a/src/Umbraco.Web.Common/Templates/TemplateRenderer.cs b/src/Umbraco.Web.Common/Templates/TemplateRenderer.cs index 5818609aeb..81f034918a 100644 --- a/src/Umbraco.Web.Common/Templates/TemplateRenderer.cs +++ b/src/Umbraco.Web.Common/Templates/TemplateRenderer.cs @@ -36,9 +36,10 @@ namespace Umbraco.Cms.Web.Common.Templates private readonly IFileService _fileService; private readonly ILocalizationService _languageService; private readonly WebRoutingSettings _webRoutingSettings; - private readonly IShortStringHelper _shortStringHelper; private readonly IHttpContextAccessor _httpContextAccessor; private readonly ICompositeViewEngine _viewEngine; + private readonly IModelMetadataProvider _modelMetadataProvider; + private readonly ITempDataProvider _tempDataProvider; public TemplateRenderer( IUmbracoContextAccessor umbracoContextAccessor, @@ -46,18 +47,20 @@ namespace Umbraco.Cms.Web.Common.Templates IFileService fileService, ILocalizationService textService, IOptions webRoutingSettings, - IShortStringHelper shortStringHelper, IHttpContextAccessor httpContextAccessor, - ICompositeViewEngine viewEngine) + ICompositeViewEngine viewEngine, + IModelMetadataProvider modelMetadataProvider, + ITempDataProvider tempDataProvider) { _umbracoContextAccessor = umbracoContextAccessor ?? throw new ArgumentNullException(nameof(umbracoContextAccessor)); _publishedRouter = publishedRouter ?? throw new ArgumentNullException(nameof(publishedRouter)); _fileService = fileService ?? throw new ArgumentNullException(nameof(fileService)); _languageService = textService ?? throw new ArgumentNullException(nameof(textService)); _webRoutingSettings = webRoutingSettings.Value ?? throw new ArgumentNullException(nameof(webRoutingSettings)); - _shortStringHelper = shortStringHelper ?? throw new ArgumentNullException(nameof(shortStringHelper)); _httpContextAccessor = httpContextAccessor ?? throw new ArgumentNullException(nameof(httpContextAccessor)); _viewEngine = viewEngine ?? throw new ArgumentNullException(nameof(viewEngine)); + _modelMetadataProvider = modelMetadataProvider; + _tempDataProvider = tempDataProvider; } public async Task RenderAsync(int pageId, int? altTemplateId, StringWriter writer) @@ -156,10 +159,7 @@ namespace Umbraco.Cms.Web.Common.Templates throw new InvalidOperationException($"A view with the name {request.GetTemplateAlias()} could not be found"); } - var modelMetadataProvider = httpContext.RequestServices.GetRequiredService(); - var tempDataProvider = httpContext.RequestServices.GetRequiredService(); - - var viewData = new ViewDataDictionary(modelMetadataProvider, new ModelStateDictionary()) + var viewData = new ViewDataDictionary(_modelMetadataProvider, new ModelStateDictionary()) { Model = request.PublishedContent }; @@ -169,7 +169,7 @@ namespace Umbraco.Cms.Web.Common.Templates new ActionContext(httpContext, httpContext.GetRouteData(), new ControllerActionDescriptor()), viewResult.View, viewData, - new TempDataDictionary(httpContext, tempDataProvider), + new TempDataDictionary(httpContext, _tempDataProvider), writer, new HtmlHelperOptions() ); @@ -182,6 +182,9 @@ namespace Umbraco.Cms.Web.Common.Templates sw.Write(output); } + // TODO: I feel like we need to do more than this, pretty sure we need to replace the UmbracoRouteValues + // HttpRequest feature too while this renders. + private void SetNewItemsOnContextObjects(IPublishedRequest request) { // now, set the new ones for this page execution diff --git a/src/Umbraco.Web.Website/Controllers/SurfaceController.cs b/src/Umbraco.Web.Website/Controllers/SurfaceController.cs index a0798d2fd0..b86af23999 100644 --- a/src/Umbraco.Web.Website/Controllers/SurfaceController.cs +++ b/src/Umbraco.Web.Website/Controllers/SurfaceController.cs @@ -98,7 +98,7 @@ namespace Umbraco.Cms.Web.Website.Controllers /// protected UmbracoPageResult CurrentUmbracoPage() { - HttpContext.Features.Set(new ProxyViewDataFeature(ViewData)); + HttpContext.Features.Set(new ProxyViewDataFeature(ViewData, TempData)); return new UmbracoPageResult(ProfilingLogger); } } diff --git a/src/Umbraco.Web.Website/Controllers/UmbLoginController.cs b/src/Umbraco.Web.Website/Controllers/UmbLoginController.cs index 67c4cd9bf7..996103f9e5 100644 --- a/src/Umbraco.Web.Website/Controllers/UmbLoginController.cs +++ b/src/Umbraco.Web.Website/Controllers/UmbLoginController.cs @@ -1,4 +1,5 @@ -using System.Threading.Tasks; +using System; +using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc; using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Logging; @@ -35,6 +36,9 @@ namespace Umbraco.Cms.Web.Website.Controllers return CurrentUmbracoPage(); } + // TODO: This is supposed to be for members! not users + //throw new NotImplementedException("Implement this for members"); + if (await _websiteSecurityAccessor.WebsiteSecurity.LoginAsync(model.Username, model.Password) == false) { // Don't add a field level error, just model level. diff --git a/src/Umbraco.Web.Website/Extensions/HtmlHelperRenderExtensions.cs b/src/Umbraco.Web.Website/Extensions/HtmlHelperRenderExtensions.cs index 1502a51665..ddca7f37aa 100644 --- a/src/Umbraco.Web.Website/Extensions/HtmlHelperRenderExtensions.cs +++ b/src/Umbraco.Web.Website/Extensions/HtmlHelperRenderExtensions.cs @@ -165,16 +165,11 @@ namespace Umbraco.Extensions return htmlHelper.ValidationSummary(excludePropertyErrors, message, htmlAttributes); } - var htmlGenerator = GetRequiredService(htmlHelper); + IHtmlGenerator htmlGenerator = GetRequiredService(htmlHelper); - var viewContext = htmlHelper.ViewContext.Clone(); - foreach (var key in viewContext.ViewData.Keys.ToArray()) - { - if (!key.StartsWith(prefix)) - { - viewContext.ViewData.Remove(key); - } - } + ViewContext viewContext = htmlHelper.ViewContext.Clone(); + //change the HTML field name + viewContext.ViewData.TemplateInfo.HtmlFieldPrefix = prefix; var tagBuilder = htmlGenerator.GenerateValidationSummary( viewContext,