diff --git a/src/Umbraco.Core/Models/IContentModel.cs b/src/Umbraco.Core/Models/IContentModel.cs index d0d4f175d7..692547aa3e 100644 --- a/src/Umbraco.Core/Models/IContentModel.cs +++ b/src/Umbraco.Core/Models/IContentModel.cs @@ -1,10 +1,30 @@ -using Umbraco.Core.Models; +using Umbraco.Core.Models; using Umbraco.Core.Models.PublishedContent; namespace Umbraco.Web.Models { + /// + /// The basic view model returned for front-end Umbraco controllers + /// + /// + /// + /// exists in order to unify all view models in Umbraco, whether it's a normal template view or a partial view macro, or + /// a user's custom model that they have created when doing route hijacking or custom routes. + /// + /// + /// By default all front-end template views inherit from UmbracoViewPage which has a model of but the model returned + /// from the controllers is which in normal circumstances would not work. This works with UmbracoViewPage because it + /// performs model binding between IContentModel and IPublishedContent. This offers a lot of flexibility when rendering views. In some cases if you + /// are route hijacking and returning a custom implementation of and your view is strongly typed to this model, you can still + /// render partial views created in the back office that have the default model of IPublishedContent without having to worry about explicitly passing + /// that model to the view. + /// + /// public interface IContentModel { + /// + /// Gets the + /// IPublishedContent Content { get; } } } diff --git a/src/Umbraco.Core/Models/PublishedContent/IPublishedContentType.cs b/src/Umbraco.Core/Models/PublishedContent/IPublishedContentType.cs index cfc789324a..f9330176aa 100644 --- a/src/Umbraco.Core/Models/PublishedContent/IPublishedContentType.cs +++ b/src/Umbraco.Core/Models/PublishedContent/IPublishedContentType.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; namespace Umbraco.Core.Models.PublishedContent @@ -8,21 +8,13 @@ namespace Umbraco.Core.Models.PublishedContent /// /// Instances implementing the interface should be /// immutable, ie if the content type changes, then a new instance needs to be created. - public interface IPublishedContentType2 : IPublishedContentType + public interface IPublishedContentType { /// /// Gets the unique key for the content type. /// Guid Key { get; } - } - /// - /// Represents an type. - /// - /// Instances implementing the interface should be - /// immutable, ie if the content type changes, then a new instance needs to be created. - public interface IPublishedContentType - { /// /// Gets the content type identifier. /// diff --git a/src/Umbraco.Core/Models/PublishedContent/PublishedContentType.cs b/src/Umbraco.Core/Models/PublishedContent/PublishedContentType.cs index 14c26442eb..daf75f5c50 100644 --- a/src/Umbraco.Core/Models/PublishedContent/PublishedContentType.cs +++ b/src/Umbraco.Core/Models/PublishedContent/PublishedContentType.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; @@ -9,7 +9,7 @@ namespace Umbraco.Core.Models.PublishedContent /// /// Instances of the class are immutable, ie /// if the content type changes, then a new class needs to be created. - public class PublishedContentType : IPublishedContentType2 + public class PublishedContentType : IPublishedContentType { private readonly IPublishedPropertyType[] _propertyTypes; diff --git a/src/Umbraco.Core/Models/PublishedContent/PublishedContentTypeExtensions.cs b/src/Umbraco.Core/Models/PublishedContent/PublishedContentTypeExtensions.cs deleted file mode 100644 index feab33c1d6..0000000000 --- a/src/Umbraco.Core/Models/PublishedContent/PublishedContentTypeExtensions.cs +++ /dev/null @@ -1,24 +0,0 @@ -using System; - -namespace Umbraco.Core.Models.PublishedContent -{ - public static class PublishedContentTypeExtensions - { - /// - /// Get the GUID key from an - /// - /// - /// - /// - public static bool TryGetKey(this IPublishedContentType publishedContentType, out Guid key) - { - if (publishedContentType is IPublishedContentType2 contentTypeWithKey) - { - key = contentTypeWithKey.Key; - return true; - } - key = Guid.Empty; - return false; - } - } -} diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs index f46c118174..f35f9b9469 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/BlockListPropertyValueConverter.cs @@ -1,4 +1,4 @@ -using Newtonsoft.Json; +using Newtonsoft.Json; using Newtonsoft.Json.Linq; using System; using System.Collections.Generic; @@ -103,8 +103,7 @@ namespace Umbraco.Web.PropertyEditors.ValueConverters if (settingGuidUdi != null) settingsPublishedElements.TryGetValue(settingGuidUdi.Guid, out settingsData); - if (!contentData.ContentType.TryGetKey(out var contentTypeKey)) - throw new InvalidOperationException("The content type was not of type " + typeof(IPublishedContentType2)); + var contentTypeKey = contentData.ContentType.Key; if (!blockConfigMap.TryGetValue(contentTypeKey, out var blockConfig)) continue; @@ -113,8 +112,7 @@ namespace Umbraco.Web.PropertyEditors.ValueConverters // we also ensure that the content type's match since maybe the settings type has been changed after this has been persisted. if (settingsData != null) { - if (!settingsData.ContentType.TryGetKey(out var settingsElementTypeKey)) - throw new InvalidOperationException("The settings element type was not of type " + typeof(IPublishedContentType2)); + var settingsElementTypeKey = settingsData.ContentType.Key; if (!blockConfig.SettingsElementTypeKey.HasValue || settingsElementTypeKey != blockConfig.SettingsElementTypeKey) settingsData = null; diff --git a/src/Umbraco.Infrastructure/PublishedCache/PublishedContentTypeCache.cs b/src/Umbraco.Infrastructure/PublishedCache/PublishedContentTypeCache.cs index 4c1482c82c..ae99243a2c 100644 --- a/src/Umbraco.Infrastructure/PublishedCache/PublishedContentTypeCache.cs +++ b/src/Umbraco.Infrastructure/PublishedCache/PublishedContentTypeCache.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using System.Threading; @@ -190,8 +190,7 @@ namespace Umbraco.Web.PublishedCache try { _lock.EnterWriteLock(); - if (type.TryGetKey(out var key)) - _keyToIdMap[key] = type.Id; + _keyToIdMap[type.Key] = type.Id; return _typesByAlias[aliasKey] = _typesById[type.Id] = type; } finally @@ -227,8 +226,7 @@ namespace Umbraco.Web.PublishedCache try { _lock.EnterWriteLock(); - if (type.TryGetKey(out var key)) - _keyToIdMap[key] = type.Id; + _keyToIdMap[type.Key] = type.Id; return _typesByAlias[GetAliasKey(type)] = _typesById[type.Id] = type; } finally diff --git a/src/Umbraco.PublishedCache.NuCache/ContentStore.cs b/src/Umbraco.PublishedCache.NuCache/ContentStore.cs index d41ca344dc..e79c195b46 100644 --- a/src/Umbraco.PublishedCache.NuCache/ContentStore.cs +++ b/src/Umbraco.PublishedCache.NuCache/ContentStore.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; @@ -1167,8 +1167,7 @@ namespace Umbraco.Web.PublishedCache.NuCache SetValueLocked(_contentTypesById, type.Id, type); SetValueLocked(_contentTypesByAlias, type.Alias, type); // ensure the key/id map is accurate - if (type.TryGetKey(out var key)) - _contentTypeKeyToIdMap[key] = type.Id; + _contentTypeKeyToIdMap[type.Key] = type.Id; } // set a node (just the node, not the tree) diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/BlockListPropertyValueConverterTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/BlockListPropertyValueConverterTests.cs index eb77ad2e1c..dc6d059a0a 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/BlockListPropertyValueConverterTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/BlockListPropertyValueConverterTests.cs @@ -1,4 +1,4 @@ -using Moq; +using Moq; using NUnit.Framework; using System; using System.Collections.Generic; @@ -33,19 +33,19 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.PropertyEditors /// private IPublishedSnapshotAccessor GetPublishedSnapshotAccessor() { - var test1ContentType = Mock.Of(x => + var test1ContentType = Mock.Of(x => x.IsElement == true && x.Key == ContentKey1 && x.Alias == ContentAlias1); - var test2ContentType = Mock.Of(x => + var test2ContentType = Mock.Of(x => x.IsElement == true && x.Key == ContentKey2 && x.Alias == ContentAlias2); - var test3ContentType = Mock.Of(x => + var test3ContentType = Mock.Of(x => x.IsElement == true && x.Key == SettingKey1 && x.Alias == SettingAlias1); - var test4ContentType = Mock.Of(x => + var test4ContentType = Mock.Of(x => x.IsElement == true && x.Key == SettingKey2 && x.Alias == SettingAlias2); diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ModelBinders/ContentModelBinderTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ModelBinders/ContentModelBinderTests.cs index ba5910da29..caac2f9207 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ModelBinders/ContentModelBinderTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ModelBinders/ContentModelBinderTests.cs @@ -1,4 +1,5 @@ using System; +using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Abstractions; @@ -11,57 +12,95 @@ using Umbraco.Core.Models.PublishedContent; using Umbraco.Web.Common.ModelBinders; using Umbraco.Web.Common.Routing; using Umbraco.Web.Models; -using Umbraco.Web.Website.Routing; namespace Umbraco.Tests.UnitTests.Umbraco.Web.Common.ModelBinders { [TestFixture] public class ContentModelBinderTests { + private ContentModelBinder _contentModelBinder; + + [SetUp] + public void SetUp() => _contentModelBinder = new ContentModelBinder(); + [Test] - public void Does_Not_Bind_Model_When_UmbracoDataToken_Not_In_Route_Data() + [TestCase(typeof(IPublishedContent), false)] + [TestCase(typeof(ContentModel), false)] + [TestCase(typeof(ContentType1), false)] + [TestCase(typeof(ContentModel), false)] + [TestCase(typeof(NonContentModel), true)] + [TestCase(typeof(MyCustomContentModel), true)] + [TestCase(typeof(IContentModel), true)] + public void Returns_Binder_For_IPublishedContent_And_IRenderModel(Type testType, bool expectNull) + { + var binderProvider = new ContentModelBinderProvider(); + var contextMock = new Mock(); + contextMock.Setup(x => x.Metadata).Returns(new EmptyModelMetadataProvider().GetMetadataForType(testType)); + + IModelBinder found = binderProvider.GetBinder(contextMock.Object); + if (expectNull) + { + Assert.IsNull(found); + } + else + { + Assert.IsNotNull(found); + } + } + + [Test] + public async Task Does_Not_Bind_Model_When_UmbracoToken_Not_In_Route_Values() { // Arrange IPublishedContent pc = CreatePublishedContent(); - var bindingContext = CreateBindingContext(typeof(ContentModel), pc, withUmbracoDataToken: false); - var binder = new ContentModelBinder(); + var bindingContext = CreateBindingContextForUmbracoRequest(typeof(ContentModel), pc); + bindingContext.ActionContext.RouteData.Values.Remove(Constants.Web.UmbracoRouteDefinitionDataToken); // Act - binder.BindModelAsync(bindingContext); + await _contentModelBinder.BindModelAsync(bindingContext); // Assert Assert.False(bindingContext.Result.IsModelSet); } [Test] - public void Does_Not_Bind_Model_When_Source_Not_Of_Expected_Type() + public async Task Does_Not_Bind_Model_When_UmbracoToken_Has_Incorrect_Model() { // Arrange IPublishedContent pc = CreatePublishedContent(); - var bindingContext = CreateBindingContext(typeof(ContentModel), pc, source: new NonContentModel()); - var binder = new ContentModelBinder(); + var bindingContext = CreateBindingContextForUmbracoRequest(typeof(ContentModel), pc); + bindingContext.ActionContext.RouteData.Values[Constants.Web.UmbracoRouteDefinitionDataToken] = new NonContentModel(); // Act - binder.BindModelAsync(bindingContext); + await _contentModelBinder.BindModelAsync(bindingContext); // Assert Assert.False(bindingContext.Result.IsModelSet); } [Test] - public void BindModel_Returns_If_Same_Type() + public async Task Bind_Model_When_UmbracoToken_Is_In_Route_Values() { // Arrange IPublishedContent pc = CreatePublishedContent(); - var content = new ContentModel(pc); - var bindingContext = CreateBindingContext(typeof(ContentModel), pc, source: content); - var binder = new ContentModelBinder(); + var bindingContext = CreateBindingContextForUmbracoRequest(typeof(ContentModel), pc); // Act - binder.BindModelAsync(bindingContext); + await _contentModelBinder.BindModelAsync(bindingContext); // Assert - Assert.AreSame(content, bindingContext.Result.Model); + Assert.True(bindingContext.Result.IsModelSet); + } + + [Test] + public void Throws_When_Source_Not_Of_Expected_Type() + { + // Arrange + IPublishedContent pc = CreatePublishedContent(); + var bindingContext = new DefaultModelBindingContext(); + + // Act/Assert + Assert.Throws(() => _contentModelBinder.BindModel(bindingContext, new NonContentModel(), typeof(ContentModel))); } [Test] @@ -69,11 +108,10 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.Common.ModelBinders { // Arrange IPublishedContent pc = CreatePublishedContent(); - var bindingContext = CreateBindingContext(typeof(ContentModel), pc, source: pc); - var binder = new ContentModelBinder(); + var bindingContext = new DefaultModelBindingContext(); // Act - binder.BindModelAsync(bindingContext); + _contentModelBinder.BindModel(bindingContext, pc, typeof(ContentModel)); // Assert Assert.True(bindingContext.Result.IsModelSet); @@ -84,24 +122,95 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.Common.ModelBinders { // Arrange IPublishedContent pc = CreatePublishedContent(); - var bindingContext = CreateBindingContext(typeof(ContentModel), pc, source: new ContentModel(new ContentType2(pc))); - var binder = new ContentModelBinder(); + var bindingContext = new DefaultModelBindingContext(); // Act - binder.BindModelAsync(bindingContext); + _contentModelBinder.BindModel(bindingContext, new ContentModel(new ContentType2(pc)), typeof(ContentModel)); // Assert Assert.True(bindingContext.Result.IsModelSet); } - private ModelBindingContext CreateBindingContext(Type modelType, IPublishedContent publishedContent, bool withUmbracoDataToken = true, object source = null) + [Test] + public void BindModel_Null_Source_Returns_Null() + { + var bindingContext = new DefaultModelBindingContext(); + _contentModelBinder.BindModel(bindingContext, null, typeof(ContentType1)); + Assert.IsNull(bindingContext.Result.Model); + } + + [Test] + public void BindModel_Returns_If_Same_Type() + { + var content = new ContentType1(Mock.Of()); + var bindingContext = new DefaultModelBindingContext(); + + _contentModelBinder.BindModel(bindingContext, content, typeof(ContentType1)); + + Assert.AreSame(content, bindingContext.Result.Model); + } + + [Test] + public void BindModel_RenderModel_To_IPublishedContent() + { + var content = new ContentType1(Mock.Of()); + var renderModel = new ContentModel(content); + + var bindingContext = new DefaultModelBindingContext(); + _contentModelBinder.BindModel(bindingContext, renderModel, typeof(IPublishedContent)); + + Assert.AreSame(content, bindingContext.Result.Model); + } + + [Test] + public void BindModel_IPublishedContent_To_RenderModel() + { + var content = new ContentType1(Mock.Of()); + var bindingContext = new DefaultModelBindingContext(); + + _contentModelBinder.BindModel(bindingContext, content, typeof(ContentModel)); + var bound = (IContentModel)bindingContext.Result.Model; + + Assert.AreSame(content, bound.Content); + } + + [Test] + public void BindModel_IPublishedContent_To_Generic_RenderModel() + { + var content = new ContentType1(Mock.Of()); + var bindingContext = new DefaultModelBindingContext(); + + _contentModelBinder.BindModel(bindingContext, content, typeof(ContentModel)); + var bound = (IContentModel)bindingContext.Result.Model; + + Assert.AreSame(content, bound.Content); + } + + [Test] + public void Null_Model_Binds_To_Null() + { + IPublishedContent pc = Mock.Of(); + var bindingContext = new DefaultModelBindingContext(); + _contentModelBinder.BindModel(bindingContext, null, typeof(ContentModel)); + Assert.IsNull(bindingContext.Result.Model); + } + + [Test] + public void Invalid_Model_Type_Throws_Exception() + { + IPublishedContent pc = Mock.Of(); + var bindingContext = new DefaultModelBindingContext(); + Assert.Throws(() => _contentModelBinder.BindModel(bindingContext, "Hello", typeof(IPublishedContent))); + } + + /// + /// Creates a binding context with the route values populated to similute an Umbraco dynamically routed request + /// + private ModelBindingContext CreateBindingContextForUmbracoRequest(Type modelType, IPublishedContent publishedContent) { var httpContext = new DefaultHttpContext(); var routeData = new RouteData(); - if (withUmbracoDataToken) - { - routeData.Values.Add(Constants.Web.UmbracoRouteDefinitionDataToken, new UmbracoRouteValues(publishedContent)); - } + routeData.Values.Add(Constants.Web.UmbracoRouteDefinitionDataToken, new UmbracoRouteValues(publishedContent)); var actionContext = new ActionContext(httpContext, routeData, new ActionDescriptor()); var metadataProvider = new EmptyModelMetadataProvider(); @@ -120,19 +229,25 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.Common.ModelBinders { } - private IPublishedContent CreatePublishedContent() - { - return new ContentType2(new Mock().Object); - } + private IPublishedContent CreatePublishedContent() => new ContentType2(new Mock().Object); public class ContentType1 : PublishedContentWrapped { - public ContentType1(IPublishedContent content) : base(content) { } + public ContentType1(IPublishedContent content) + : base(content) { } } public class ContentType2 : ContentType1 { - public ContentType2(IPublishedContent content) : base(content) { } + public ContentType2(IPublishedContent content) + : base(content) { } + } + + public class MyCustomContentModel : ContentModel + { + public MyCustomContentModel(IPublishedContent content) + : base(content) + { } } } } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ModelBinders/RenderModelBinderTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ModelBinders/RenderModelBinderTests.cs deleted file mode 100644 index 660a9b7bd1..0000000000 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ModelBinders/RenderModelBinderTests.cs +++ /dev/null @@ -1,182 +0,0 @@ -using System; -using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Mvc; -using Microsoft.AspNetCore.Mvc.Abstractions; -using Microsoft.AspNetCore.Mvc.ModelBinding; -using Microsoft.AspNetCore.Routing; -using Moq; -using NUnit.Framework; -using Umbraco.Core; -using Umbraco.Core.Models.PublishedContent; -using Umbraco.Web.Common.ModelBinders; -using Umbraco.Web.Common.Routing; -using Umbraco.Web.Models; -using Umbraco.Web.Website.Routing; - -namespace Umbraco.Tests.UnitTests.Umbraco.Web.Common.ModelBinders -{ - [TestFixture] - public class RenderModelBinderTests - { - private ContentModelBinder _contentModelBinder; - [SetUp] - public void SetUp() - { - _contentModelBinder = new ContentModelBinder(); - } - - [Test] - [TestCase(typeof(IPublishedContent), false)] - [TestCase(typeof(ContentModel), false)] - [TestCase(typeof(MyContent), false)] - [TestCase(typeof(ContentModel), false)] - [TestCase(typeof(MyOtherContent), true)] - [TestCase(typeof(MyCustomContentModel), true)] - [TestCase(typeof(IContentModel), true)] - public void Returns_Binder_For_IPublishedContent_And_IRenderModel(Type testType, bool expectNull) - { - var binderProvider = new ContentModelBinderProvider(); - var contextMock = new Mock(); - contextMock.Setup(x => x.Metadata).Returns(new EmptyModelMetadataProvider().GetMetadataForType(testType)); - - var found = binderProvider.GetBinder(contextMock.Object); - if (expectNull) - { - Assert.IsNull(found); - } - else - { - Assert.IsNotNull(found); - } - } - - [Test] - public void BindModel_Null_Source_Returns_Null() - { - var bindingContext = new DefaultModelBindingContext(); - _contentModelBinder.BindModelAsync(bindingContext, null, typeof(MyContent)); - Assert.IsNull(bindingContext.Result.Model); - } - - [Test] - public void BindModel_Returns_If_Same_Type() - { - var content = new MyContent(Mock.Of()); - var bindingContext = new DefaultModelBindingContext(); - - _contentModelBinder.BindModelAsync(bindingContext, content, typeof(MyContent)); - - Assert.AreSame(content, bindingContext.Result.Model); - } - - [Test] - public void BindModel_RenderModel_To_IPublishedContent() - { - var content = new MyContent(Mock.Of()); - var renderModel = new ContentModel(content); - - var bindingContext = new DefaultModelBindingContext(); - _contentModelBinder.BindModelAsync(bindingContext, renderModel, typeof(IPublishedContent)); - - Assert.AreSame(content, bindingContext.Result.Model); - } - - [Test] - public void BindModel_IPublishedContent_To_RenderModel() - { - var content = new MyContent(Mock.Of()); - var bindingContext = new DefaultModelBindingContext(); - - _contentModelBinder.BindModelAsync(bindingContext, content, typeof(ContentModel)); - var bound = (IContentModel) bindingContext.Result.Model; - - Assert.AreSame(content, bound.Content); - } - - [Test] - public void BindModel_IPublishedContent_To_Generic_RenderModel() - { - var content = new MyContent(Mock.Of()); - var bindingContext = new DefaultModelBindingContext(); - - _contentModelBinder.BindModelAsync(bindingContext, content, typeof(ContentModel)); - var bound = (IContentModel) bindingContext.Result.Model; - - Assert.AreSame(content, bound.Content); - } - - [Test] - public void No_DataToken_Returns_Null() - { - IPublishedContent pc = Mock.Of(); - var content = new MyContent(pc); - var bindingContext = CreateBindingContext(typeof(ContentModel), pc, false, content); - - _contentModelBinder.BindModelAsync(bindingContext); - - Assert.IsNull(bindingContext.Result.Model); - } - - [Test] - public void Invalid_DataToken_Model_Type_Returns_Null() - { - IPublishedContent pc = Mock.Of(); - var bindingContext = CreateBindingContext(typeof(IPublishedContent), pc, source: "Hello"); - _contentModelBinder.BindModelAsync(bindingContext); - Assert.IsNull(bindingContext.Result.Model); - } - - [Test] - public void IPublishedContent_DataToken_Model_Type_Uses_DefaultImplementation() - { - IPublishedContent pc = Mock.Of(); - var content = new MyContent(pc); - var bindingContext = CreateBindingContext(typeof(MyContent), pc, source: content); - - _contentModelBinder.BindModelAsync(bindingContext); - - Assert.AreEqual(content, bindingContext.Result.Model); - } - - private ModelBindingContext CreateBindingContext(Type modelType, IPublishedContent publishedContent, bool withUmbracoDataToken = true, object source = null) - { - var httpContext = new DefaultHttpContext(); - var routeData = new RouteData(); - if (withUmbracoDataToken) - { - routeData.Values.Add(Constants.Web.UmbracoRouteDefinitionDataToken, new UmbracoRouteValues(publishedContent)); - } - - var actionContext = new ActionContext(httpContext, routeData, new ActionDescriptor()); - var metadataProvider = new EmptyModelMetadataProvider(); - var routeValueDictionary = new RouteValueDictionary(); - var valueProvider = new RouteValueProvider(BindingSource.Path, routeValueDictionary); - return new DefaultModelBindingContext - { - ActionContext = actionContext, - ModelMetadata = metadataProvider.GetMetadataForType(modelType), - ModelName = modelType.Name, - ValueProvider = valueProvider, - }; - } - - public class MyCustomContentModel : ContentModel - { - public MyCustomContentModel(IPublishedContent content) - : base(content) - { } - } - - public class MyOtherContent - { - - } - - public class MyContent : PublishedContentWrapped - { - public MyContent(IPublishedContent content) : base(content) - { - } - } - } -} diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Views/UmbracoViewPageTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Views/UmbracoViewPageTests.cs index 3b52d0701e..03065d4bcb 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Views/UmbracoViewPageTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Views/UmbracoViewPageTests.cs @@ -1,6 +1,7 @@ using System; using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.AspNetCore.Mvc.Rendering; using Microsoft.AspNetCore.Mvc.ViewFeatures; using NUnit.Framework; using Umbraco.Core.Models.PublishedContent; @@ -13,7 +14,6 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.Common.Views [TestFixture] public class UmbracoViewPageTests { - #region RenderModel To ... [Test] public void RenderModel_To_RenderModel() { @@ -58,7 +58,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.Common.Views var view = new ContentType2TestPage(); var viewData = GetViewDataDictionary(model); - Assert.ThrowsAsync(async () => await view.SetViewDataAsyncX(viewData)); + Assert.Throws(() => view.SetViewData(viewData)); } [Test] @@ -96,12 +96,9 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.Common.Views var view = new RenderModelOfContentType2TestPage(); var viewData = GetViewDataDictionary(model); - Assert.ThrowsAsync(async () => await view.SetViewDataAsyncX(viewData)); + Assert.Throws(() => view.SetViewData(viewData)); } - #endregion - - #region RenderModelOf To ... [Test] public void RenderModelOf_ContentType1_To_RenderModel() @@ -117,20 +114,20 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.Common.Views } [Test] - public async Task RenderModelOf_ContentType1_To_ContentType1() + public void RenderModelOf_ContentType1_To_ContentType1() { var content = new ContentType1(null); var model = new ContentModel(content); var view = new ContentType1TestPage(); var viewData = GetViewDataDictionary>(model); - await view.SetViewDataAsyncX(viewData); + view.SetViewData(viewData); Assert.IsInstanceOf(view.Model); } [Test] - public async Task RenderModelOf_ContentType2_To_ContentType1() + public void RenderModelOf_ContentType2_To_ContentType1() { var content = new ContentType2(null); var model = new ContentModel(content); @@ -140,13 +137,13 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.Common.Views Model = model }; - await view.SetViewDataAsyncX(viewData); + view.SetViewData(viewData); Assert.IsInstanceOf(view.Model); } [Test] - public async Task RenderModelOf_ContentType1_To_ContentType2() + public void RenderModelOf_ContentType1_To_ContentType2() { var content = new ContentType1(null); @@ -154,7 +151,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.Common.Views var view = new ContentType2TestPage(); var viewData = GetViewDataDictionary(model); - Assert.ThrowsAsync(async () => await view.SetViewDataAsyncX(viewData)); + Assert.Throws(() => view.SetViewData(viewData)); } [Test] @@ -172,14 +169,14 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.Common.Views } [Test] - public async Task RenderModelOf_ContentType2_To_RenderModelOf_ContentType1() + public void RenderModelOf_ContentType2_To_RenderModelOf_ContentType1() { var content = new ContentType2(null); var model = new ContentModel(content); var view = new RenderModelOfContentType1TestPage(); var viewData = GetViewDataDictionary>(model); - await view.SetViewDataAsyncX(viewData); + view.SetViewData(viewData); Assert.IsInstanceOf>(view.Model); Assert.IsInstanceOf(view.Model.Content); @@ -193,48 +190,44 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.Common.Views var view = new RenderModelOfContentType2TestPage(); var viewData = GetViewDataDictionary(model); - Assert.ThrowsAsync(async () => await view.SetViewDataAsyncX(viewData)); + Assert.Throws(() => view.SetViewData(viewData)); } - #endregion - - #region ContentType To ... - [Test] - public async Task ContentType1_To_RenderModel() + public void ContentType1_To_RenderModel() { var content = new ContentType1(null); var view = new RenderModelTestPage(); var viewData = GetViewDataDictionary(content); - await view.SetViewDataAsyncX(viewData); + view.SetViewData(viewData); Assert.IsInstanceOf(view.Model); } [Test] - public async Task ContentType1_To_RenderModelOf_ContentType1() + public void ContentType1_To_RenderModelOf_ContentType1() { var content = new ContentType1(null); var view = new RenderModelOfContentType1TestPage(); var viewData = GetViewDataDictionary(content); - await view.SetViewDataAsyncX(viewData); + view.SetViewData(viewData); Assert.IsInstanceOf>(view.Model); Assert.IsInstanceOf(view.Model.Content); } [Test] - public async Task ContentType2_To_RenderModelOf_ContentType1() + public void ContentType2_To_RenderModelOf_ContentType1() { // Same as above but with ContentModel var content = new ContentType2(null); var view = new RenderModelOfContentType1TestPage(); var viewData = GetViewDataDictionary(content); - await view.SetViewDataAsyncX(viewData); + view.SetViewData(viewData); Assert.IsInstanceOf>(view.Model); Assert.IsInstanceOf(view.Model.Content); @@ -247,17 +240,17 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.Common.Views var view = new RenderModelOfContentType2TestPage(); var viewData = GetViewDataDictionary(content); - Assert.ThrowsAsync(async () => await view.SetViewDataAsyncX(viewData)); + Assert.Throws(() => view.SetViewData(viewData)); } [Test] - public async Task ContentType1_To_ContentType1() + public void ContentType1_To_ContentType1() { var content = new ContentType1(null); var view = new ContentType1TestPage(); var viewData = GetViewDataDictionary(content); - await view.SetViewDataAsyncX(viewData); + view.SetViewData(viewData); Assert.IsInstanceOf(view.Model); } @@ -269,23 +262,21 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.Common.Views var view = new ContentType2TestPage(); var viewData = GetViewDataDictionary(content); - Assert.ThrowsAsync(async () => await view.SetViewDataAsyncX(viewData)); + Assert.Throws(() => view.SetViewData(viewData)); } [Test] - public async Task ContentType2_To_ContentType1() + public void ContentType2_To_ContentType1() { var content = new ContentType2(null); var view = new ContentType1TestPage(); var viewData = GetViewDataDictionary(content); - await view.SetViewDataAsyncX(viewData); + view.SetViewData(viewData); Assert.IsInstanceOf(view.Model); } - #endregion - #region Test helpers methods private ViewDataDictionary GetViewDataDictionary(object model) @@ -324,10 +315,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.Common.Views throw new NotImplementedException(); } - public async Task SetViewDataAsyncX(ViewDataDictionary viewData) - { - await SetViewDataAsync(viewData); - } + public void SetViewData(ViewDataDictionary viewData) => ViewData = (ViewDataDictionary)BindViewData(viewData); } public class RenderModelTestPage : TestPage diff --git a/src/Umbraco.Web.Common/AspNetCore/UmbracoViewPage.cs b/src/Umbraco.Web.Common/AspNetCore/UmbracoViewPage.cs index a97b67a900..3afc8978b6 100644 --- a/src/Umbraco.Web.Common/AspNetCore/UmbracoViewPage.cs +++ b/src/Umbraco.Web.Common/AspNetCore/UmbracoViewPage.cs @@ -5,6 +5,7 @@ using Microsoft.AspNetCore.Html; using Microsoft.AspNetCore.Http.Extensions; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.Razor; +using Microsoft.AspNetCore.Mvc.Rendering; using Microsoft.AspNetCore.Mvc.ViewFeatures; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; @@ -16,6 +17,7 @@ using Umbraco.Core.Models.PublishedContent; using Umbraco.Core.Strings; using Umbraco.Extensions; using Umbraco.Web.Common.ModelBinders; +using Umbraco.Web.Models; namespace Umbraco.Web.Common.AspNetCore { @@ -28,23 +30,44 @@ namespace Umbraco.Web.Common.AspNetCore public abstract class UmbracoViewPage : RazorPage { - private IUmbracoContext _umbracoContext; + private IUmbracoContextAccessor UmbracoContextAccessor => Context.RequestServices.GetRequiredService(); + private GlobalSettings GlobalSettings => Context.RequestServices.GetRequiredService>().Value; + private ContentSettings ContentSettings => Context.RequestServices.GetRequiredService>().Value; + private IProfilerHtml ProfilerHtml => Context.RequestServices.GetRequiredService(); + private IIOHelper IOHelper => Context.RequestServices.GetRequiredService(); + private ContentModelBinder ContentModelBinder => new ContentModelBinder(); + /// + /// Gets the + /// protected IUmbracoContext UmbracoContext => _umbracoContext ??= UmbracoContextAccessor.UmbracoContext; + /// + public override ViewContext ViewContext + { + get => base.ViewContext; + set + { + // Here we do the magic model swap + ViewContext ctx = value; + ctx.ViewData = BindViewData(ctx.ViewData); + base.ViewContext = ctx; + } + } + /// public override void Write(object value) { if (value is IHtmlEncodedString htmlEncodedString) { - base.WriteLiteral(htmlEncodedString.ToHtmlString()); + WriteLiteral(htmlEncodedString.ToHtmlString()); } else { @@ -52,10 +75,12 @@ namespace Umbraco.Web.Common.AspNetCore } } + /// public override void WriteLiteral(object value) { // filter / add preview banner - if (Context.Response.ContentType.InvariantEquals("text/html")) // ASP.NET default value + // ASP.NET default value is text/html + if (Context.Response.ContentType.InvariantEquals("text/html")) { if (UmbracoContext.IsDebug || UmbracoContext.InPreviewMode) { @@ -70,7 +95,8 @@ namespace Umbraco.Web.Common.AspNetCore { // creating previewBadge markup markupToInject = - string.Format(ContentSettings.PreviewBadge, + string.Format( + ContentSettings.PreviewBadge, IOHelper.ResolveUrl(GlobalSettings.UmbracoPath), Context.Request.GetEncodedUrl(), UmbracoContext.PublishedRequest.PublishedContent.Id); @@ -84,7 +110,7 @@ namespace Umbraco.Web.Common.AspNetCore var sb = new StringBuilder(text); sb.Insert(pos, markupToInject); - base.WriteLiteral(sb.ToString()); + WriteLiteral(sb.ToString()); return; } } @@ -93,70 +119,93 @@ namespace Umbraco.Web.Common.AspNetCore base.WriteLiteral(value); } - // TODO: This trick doesn't work anymore, this method used to be an override. - // Now the model is bound in a different place - // maps model - protected async Task SetViewDataAsync(ViewDataDictionary viewData) + /// + /// Dynamically binds the incoming to the required + /// + /// + /// This is used in order to provide the ability for an Umbraco view to either have a model of type + /// or . This will use the to bind the models + /// to the correct output type. + /// + protected ViewDataDictionary BindViewData(ViewDataDictionary viewData) { + // check if it's already the correct type and continue if it is + if (viewData is ViewDataDictionary vdd) + { + return vdd; + } + + // Here we hand the default case where we know the incoming model is ContentModel and the + // outgoing model is IPublishedContent. This is a fast conversion that doesn't require doing the full + // model binding, allocating classes, etc... + if (viewData.ModelMetadata.ModelType == typeof(ContentModel) + && typeof(TModel) == typeof(IPublishedContent)) + { + var contentModel = (ContentModel)viewData.Model; + viewData.Model = contentModel.Content; + return viewData; + } + // capture the model before we tinker with the viewData var viewDataModel = viewData.Model; // map the view data (may change its type, may set model to null) - viewData = MapViewDataDictionary(viewData, typeof (TModel)); + viewData = MapViewDataDictionary(viewData, typeof(TModel)); // bind the model var bindingContext = new DefaultModelBindingContext(); - await ContentModelBinder.BindModelAsync(bindingContext, viewDataModel, typeof (TModel)); + ContentModelBinder.BindModel(bindingContext, viewDataModel, typeof(TModel)); viewData.Model = bindingContext.Result.Model; - // set the view data - ViewData = (ViewDataDictionary) viewData; + // return the new view data + return (ViewDataDictionary)viewData; } // viewData is the ViewDataDictionary (maybe ) that we have // modelType is the type of the model that we need to bind to - // // figure out whether viewData can accept modelType else replace it - // private static ViewDataDictionary MapViewDataDictionary(ViewDataDictionary viewData, Type modelType) { - var viewDataType = viewData.GetType(); - + Type viewDataType = viewData.GetType(); if (viewDataType.IsGenericType) { // ensure it is the proper generic type - var def = viewDataType.GetGenericTypeDefinition(); + Type def = viewDataType.GetGenericTypeDefinition(); if (def != typeof(ViewDataDictionary<>)) + { throw new Exception("Could not map viewData of type \"" + viewDataType.FullName + "\"."); + } // get the viewData model type and compare with the actual view model type: // viewData is ViewDataDictionary and we will want to assign an // object of type modelType to the Model property of type viewDataModelType, we // need to check whether that is possible - var viewDataModelType = viewDataType.GenericTypeArguments[0]; + Type viewDataModelType = viewDataType.GenericTypeArguments[0]; if (viewDataModelType.IsAssignableFrom(modelType)) + { return viewData; + } } // if not possible or it is not generic then we need to create a new ViewDataDictionary - var nViewDataType = typeof(ViewDataDictionary<>).MakeGenericType(modelType); + Type nViewDataType = typeof(ViewDataDictionary<>).MakeGenericType(modelType); var tViewData = new ViewDataDictionary(viewData) { Model = null }; // temp view data to copy values var nViewData = (ViewDataDictionary)Activator.CreateInstance(nViewDataType, tViewData); return nViewData; } - public HtmlString RenderSection(string name, HtmlString defaultContents) - { - return RazorPageExtensions.RenderSection(this, name, defaultContents); - } + /// + /// Renders a section with default content if the section isn't defined + /// + public HtmlString RenderSection(string name, HtmlString defaultContents) => RazorPageExtensions.RenderSection(this, name, defaultContents); - public HtmlString RenderSection(string name, string defaultContents) - { - return RazorPageExtensions.RenderSection(this, name, defaultContents); - } + /// + /// Renders a section with default content if the section isn't defined + /// + public HtmlString RenderSection(string name, string defaultContents) => RazorPageExtensions.RenderSection(this, name, defaultContents); } } diff --git a/src/Umbraco.Web.Common/Extensions/RazorPageExtensions.cs b/src/Umbraco.Web.Common/Extensions/RazorPageExtensions.cs index 884e2bbdbc..d6c3fb5715 100644 --- a/src/Umbraco.Web.Common/Extensions/RazorPageExtensions.cs +++ b/src/Umbraco.Web.Common/Extensions/RazorPageExtensions.cs @@ -1,19 +1,24 @@ -using Microsoft.AspNetCore.Html; +using Microsoft.AspNetCore.Html; using Microsoft.AspNetCore.Mvc.Razor; namespace Umbraco.Extensions { + /// + /// Extension methods for + /// public static class RazorPageExtensions { + /// + /// Renders a section with default content if the section isn't defined + /// public static HtmlString RenderSection(this RazorPage webPage, string name, HtmlString defaultContents) - { - return webPage.IsSectionDefined(name) ? webPage.RenderSection(name) : defaultContents; - } + => webPage.IsSectionDefined(name) ? webPage.RenderSection(name) : defaultContents; + /// + /// Renders a section with default content if the section isn't defined + /// public static HtmlString RenderSection(this RazorPage webPage, string name, string defaultContents) - { - return webPage.IsSectionDefined(name) ? webPage.RenderSection(name) : new HtmlString(defaultContents); - } + => webPage.IsSectionDefined(name) ? webPage.RenderSection(name) : new HtmlString(defaultContents); } } diff --git a/src/Umbraco.Web.Common/ModelBinders/ContentModelBinder.cs b/src/Umbraco.Web.Common/ModelBinders/ContentModelBinder.cs index d8178033c9..d747a4ff86 100644 --- a/src/Umbraco.Web.Common/ModelBinders/ContentModelBinder.cs +++ b/src/Umbraco.Web.Common/ModelBinders/ContentModelBinder.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Text; using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.ModelBinding; @@ -10,21 +10,24 @@ using Umbraco.Web.Models; namespace Umbraco.Web.Common.ModelBinders { /// - /// Maps view models, supporting mapping to and from any IPublishedContent or IContentModel. + /// Maps view models, supporting mapping to and from any or . /// public class ContentModelBinder : IModelBinder { + /// public Task BindModelAsync(ModelBindingContext bindingContext) { // Although this model binder is built to work both ways between IPublishedContent and IContentModel in reality - // only IPublishedContent will ever exist in the request. + // only IPublishedContent will ever exist in the request so when this model binder is used as an IModelBinder + // in the aspnet pipeline it will really only support converting from IPublishedContent which is contained + // in the UmbracoRouteValues --> IContentModel if (!bindingContext.ActionContext.RouteData.Values.TryGetValue(Core.Constants.Web.UmbracoRouteDefinitionDataToken, out var source) || !(source is UmbracoRouteValues umbracoRouteValues)) { return Task.CompletedTask; } - BindModelAsync(bindingContext, umbracoRouteValues.PublishedContent, bindingContext.ModelType); + BindModel(bindingContext, umbracoRouteValues.PublishedContent, bindingContext.ModelType); return Task.CompletedTask; } @@ -35,34 +38,42 @@ namespace Umbraco.Web.Common.ModelBinders // { ContentModel, ContentModel, IPublishedContent } // to // { ContentModel, ContentModel, IPublishedContent } - // - public Task BindModelAsync(ModelBindingContext bindingContext, object source, Type modelType) + + /// + /// Attempts to bind the model + /// + public void BindModel(ModelBindingContext bindingContext, object source, Type modelType) { // Null model, return if (source == null) { - return Task.CompletedTask; + return; } // If types already match, return - var sourceType = source.GetType(); - if (sourceType.Inherits(modelType)) // includes == + Type sourceType = source.GetType(); + if (sourceType.Inherits(modelType)) { bindingContext.Result = ModelBindingResult.Success(source); - return Task.CompletedTask; + return; } // Try to grab the content var sourceContent = source as IPublishedContent; // check if what we have is an IPublishedContent if (sourceContent == null && sourceType.Implements()) + { // else check if it's an IContentModel, and get the content sourceContent = ((IContentModel)source).Content; + } + if (sourceContent == null) { // else check if we can convert it to a content - var attempt1 = source.TryConvertTo(); + Attempt attempt1 = source.TryConvertTo(); if (attempt1.Success) + { sourceContent = attempt1.Result; + } } // If we have a content @@ -77,41 +88,41 @@ namespace Umbraco.Web.Common.ModelBinders } bindingContext.Result = ModelBindingResult.Success(sourceContent); - return Task.CompletedTask; + return; } // If model is ContentModel, create and return if (modelType == typeof(ContentModel)) { bindingContext.Result = ModelBindingResult.Success(new ContentModel(sourceContent)); - return Task.CompletedTask; + return; } // If model is ContentModel, check content type, then create and return if (modelType.IsGenericType && modelType.GetGenericTypeDefinition() == typeof(ContentModel<>)) { - var targetContentType = modelType.GetGenericArguments()[0]; + Type targetContentType = modelType.GetGenericArguments()[0]; if (sourceContent.GetType().Inherits(targetContentType) == false) { ThrowModelBindingException(true, true, sourceContent.GetType(), targetContentType); } bindingContext.Result = ModelBindingResult.Success(Activator.CreateInstance(modelType, sourceContent)); - return Task.CompletedTask; + return; } } // Last chance : try to convert - var attempt2 = source.TryConvertTo(modelType); + Attempt attempt2 = source.TryConvertTo(modelType); if (attempt2.Success) { bindingContext.Result = ModelBindingResult.Success(attempt2.Result); - return Task.CompletedTask; + return; } // Fail ThrowModelBindingException(false, false, sourceType, modelType); - return Task.CompletedTask; + return; } private void ThrowModelBindingException(bool sourceContent, bool modelContent, Type sourceType, Type modelType) @@ -121,12 +132,18 @@ namespace Umbraco.Web.Common.ModelBinders // prepare message msg.Append("Cannot bind source"); if (sourceContent) + { msg.Append(" content"); + } + msg.Append(" type "); msg.Append(sourceType.FullName); msg.Append(" to model"); if (modelContent) + { msg.Append(" content"); + } + msg.Append(" type "); msg.Append(modelType.FullName); msg.Append("."); @@ -134,7 +151,6 @@ namespace Umbraco.Web.Common.ModelBinders // raise event, to give model factories a chance at reporting // the error with more details, and optionally request that // the application restarts. - var args = new ModelBindingArgs(sourceType, modelType, msg); ModelBindingException?.Invoke(this, args);