diff --git a/src/Umbraco.Core/Models/ISimpleContentType.cs b/src/Umbraco.Core/Models/ISimpleContentType.cs index 40862afd9e..7f6b2e68ea 100644 --- a/src/Umbraco.Core/Models/ISimpleContentType.cs +++ b/src/Umbraco.Core/Models/ISimpleContentType.cs @@ -5,8 +5,11 @@ namespace Umbraco.Core.Models /// /// Represents a simplified view of a content type. /// - public interface ISimpleContentType : IUmbracoEntity + public interface ISimpleContentType { + int Id { get; } + string Name { get; } + /// /// Gets the alias of the content type. /// diff --git a/src/Umbraco.Core/Models/PropertyCollection.cs b/src/Umbraco.Core/Models/PropertyCollection.cs index 5e71fe9f65..ed641080ea 100644 --- a/src/Umbraco.Core/Models/PropertyCollection.cs +++ b/src/Umbraco.Core/Models/PropertyCollection.cs @@ -93,7 +93,7 @@ namespace Umbraco.Core.Models } /// - /// Adds a property. + /// Adds or update a property. /// internal new void Add(Property property) { diff --git a/src/Umbraco.Core/Models/SimpleContentType.cs b/src/Umbraco.Core/Models/SimpleContentType.cs index c0e235e92d..86a1e56723 100644 --- a/src/Umbraco.Core/Models/SimpleContentType.cs +++ b/src/Umbraco.Core/Models/SimpleContentType.cs @@ -9,9 +9,6 @@ namespace Umbraco.Core.Models /// public class SimpleContentType : ISimpleContentType { - private readonly int _id; - private readonly string _name; - /// /// Initializes a new instance of the class. /// @@ -39,13 +36,12 @@ namespace Umbraco.Core.Models { if (contentType == null) throw new ArgumentNullException(nameof(contentType)); - _id = contentType.Id; + Id = contentType.Id; Alias = contentType.Alias; Variations = contentType.Variations; Icon = contentType.Icon; IsContainer = contentType.IsContainer; - Icon = contentType.Icon; - _name = contentType.Name; + Name = contentType.Name; AllowedAsRoot = contentType.AllowedAsRoot; IsElement = contentType.IsElement; } @@ -53,12 +49,7 @@ namespace Umbraco.Core.Models /// public string Alias { get; } - /// - public int Id - { - get => _id; - set => throw new NotSupportedException(); - } + public int Id { get; } /// public ITemplate DefaultTemplate { get; } @@ -71,13 +62,8 @@ namespace Umbraco.Core.Models /// public bool IsContainer { get; } - - /// - public string Name - { - get => _name; - set => throw new NotSupportedException(); - } + + public string Name { get; } /// public bool AllowedAsRoot { get; } @@ -120,30 +106,29 @@ namespace Umbraco.Core.Models // we have to have all this, because we're an IUmbracoEntity, because that is // required by the query expression visitor / SimpleContentTypeMapper - public object DeepClone() => throw new NotImplementedException(); - - public Guid Key { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } - public DateTime CreateDate { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } - public DateTime UpdateDate { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } - public DateTime? DeleteDate { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } - public bool HasIdentity => throw new NotImplementedException(); - - public int CreatorId { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } - public int ParentId { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } - public void SetParent(ITreeEntity parent) => throw new NotImplementedException(); - - public int Level { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } - public string Path { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } - public int SortOrder { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } - public bool Trashed => throw new NotImplementedException(); - public bool IsDirty() => throw new NotImplementedException(); - public bool IsPropertyDirty(string propName) => throw new NotImplementedException(); - public IEnumerable GetDirtyProperties() => throw new NotImplementedException(); - public void ResetDirtyProperties() => throw new NotImplementedException(); - public bool WasDirty() => throw new NotImplementedException(); - public bool WasPropertyDirty(string propertyName) => throw new NotImplementedException(); - public void ResetWereDirtyProperties() => throw new NotImplementedException(); - public void ResetDirtyProperties(bool rememberDirty) => throw new NotImplementedException(); - public IEnumerable GetWereDirtyProperties() => throw new NotImplementedException(); + //string ITreeEntity.Name { get => this.Name; set => throw new NotImplementedException(); } + //int IEntity.Id { get => this.Id; set => throw new NotImplementedException(); } + //bool IEntity.HasIdentity => this.Id != default; + //int ITreeEntity.CreatorId { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } + //int ITreeEntity.ParentId { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } + //int ITreeEntity.Level { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } + //string ITreeEntity.Path { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } + //int ITreeEntity.SortOrder { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } + //bool ITreeEntity.Trashed => throw new NotImplementedException(); + //Guid IEntity.Key { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } + //DateTime IEntity.CreateDate { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } + //DateTime IEntity.UpdateDate { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } + //DateTime? IEntity.DeleteDate { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } + //void ITreeEntity.SetParent(ITreeEntity parent) => throw new NotImplementedException(); + //object IDeepCloneable.DeepClone() => throw new NotImplementedException(); + //bool IRememberBeingDirty.WasDirty() => throw new NotImplementedException(); + //bool IRememberBeingDirty.WasPropertyDirty(string propertyName) => throw new NotImplementedException(); + //void IRememberBeingDirty.ResetWereDirtyProperties() => throw new NotImplementedException(); + //void IRememberBeingDirty.ResetDirtyProperties(bool rememberDirty) => throw new NotImplementedException(); + //IEnumerable IRememberBeingDirty.GetWereDirtyProperties() => throw new NotImplementedException(); + //bool ICanBeDirty.IsDirty() => throw new NotImplementedException(); + //bool ICanBeDirty.IsPropertyDirty(string propName) => throw new NotImplementedException(); + //IEnumerable ICanBeDirty.GetDirtyProperties() => throw new NotImplementedException(); + //void ICanBeDirty.ResetDirtyProperties() => throw new NotImplementedException(); } } diff --git a/src/Umbraco.Core/Persistence/Querying/ModelToSqlExpressionVisitor.cs b/src/Umbraco.Core/Persistence/Querying/ModelToSqlExpressionVisitor.cs index a353f01f5b..bf26523d9e 100644 --- a/src/Umbraco.Core/Persistence/Querying/ModelToSqlExpressionVisitor.cs +++ b/src/Umbraco.Core/Persistence/Querying/ModelToSqlExpressionVisitor.cs @@ -60,7 +60,7 @@ namespace Umbraco.Core.Persistence.Querying if (m.Expression != null && m.Expression.Type != typeof(T) - && TypeHelper.IsTypeAssignableFrom(m.Expression.Type) + && TypeHelper.IsTypeAssignableFrom(m.Expression.Type) //TODO: Could this just be `IEntity` ? why does it need to be IUmbracoEntity, we aren't even using the reference to that below && EndsWithConstant(m) == false) { //if this is the case, it means we have a sub expression / nested property access, such as: x.ContentType.Alias == "Test"; diff --git a/src/Umbraco.Tests/Models/ContentTests.cs b/src/Umbraco.Tests/Models/ContentTests.cs index a54b9c4e48..7d19c34167 100644 --- a/src/Umbraco.Tests/Models/ContentTests.cs +++ b/src/Umbraco.Tests/Models/ContentTests.cs @@ -30,6 +30,7 @@ namespace Umbraco.Tests.Models public class ContentTests : UmbracoTestBase { private IContentTypeService _contentTypeService; + private IContentTypeBaseServiceProvider _contentTypeServiceProvider; protected override void Compose() { @@ -56,6 +57,8 @@ namespace Umbraco.Tests.Models var mediaTypeService = Mock.Of(); var memberTypeService = Mock.Of(); Composition.Register(_ => ServiceContext.CreatePartial(dataTypeService: dataTypeService, contentTypeBaseServiceProvider: new ContentTypeBaseServiceProvider(_contentTypeService, mediaTypeService, memberTypeService))); + + _contentTypeServiceProvider = new ContentTypeBaseServiceProvider(_contentTypeService, Mock.Of(), Mock.Of()); } [Test] @@ -541,7 +544,7 @@ namespace Umbraco.Tests.Models var content = MockedContent.CreateTextpageContent(contentType, "Textpage", -1); // Act - content.PropertyValues(new {title = "This is the new title"}); + content.PropertyValues(_contentTypeServiceProvider, new { title = "This is the new title"}); // Assert Assert.That(content.Properties.Any(), Is.True); diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index 94cbf69b63..b5069e040b 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -1574,19 +1574,19 @@ namespace Umbraco.Tests.Services var contentType = MockedContentTypes.CreateAllTypesContentType("test", "test"); ServiceContext.ContentTypeService.Save(contentType, Constants.Security.SuperUserId); - + object obj = new { tags = "[\"Hello\",\"World\"]" }; var content1 = MockedContent.CreateBasicContent(contentType); - content1.PropertyValues(obj); + content1.PropertyValues(ServiceContext.ContentTypeBaseServices, obj); content1.ResetDirtyProperties(false); ServiceContext.ContentService.Save(content1, Constants.Security.SuperUserId); Assert.IsTrue(ServiceContext.ContentService.SaveAndPublish(content1, userId: 0).Success); var content2 = MockedContent.CreateBasicContent(contentType); - content2.PropertyValues(obj); + content2.PropertyValues(ServiceContext.ContentTypeBaseServices, obj); content2.ResetDirtyProperties(false); ServiceContext.ContentService.Save(content2, Constants.Security.SuperUserId); Assert.IsTrue(ServiceContext.ContentService.SaveAndPublish(content2, userId: 0).Success); diff --git a/src/Umbraco.Tests/Testing/ContentBaseExtensions.cs b/src/Umbraco.Tests/Testing/ContentBaseExtensions.cs index 85ecf03dee..883c7682f4 100644 --- a/src/Umbraco.Tests/Testing/ContentBaseExtensions.cs +++ b/src/Umbraco.Tests/Testing/ContentBaseExtensions.cs @@ -3,42 +3,48 @@ using System.Linq; using Umbraco.Core; using Umbraco.Core.Composing; using Umbraco.Core.Models; +using Umbraco.Core.Services; namespace Umbraco.Tests.Testing { public static class ContentBaseExtensions { + public static void PropertyValues(this IContentBase content, object value, string culture = null, string segment = null) + { + content.PropertyValues(Current.Services.ContentTypeBaseServices, value, culture, segment); + } + /// /// Set property values by alias with an anonymous object. /// /// Does not support variants. - public static void PropertyValues(this IContentBase content, object value, string culture = null, string segment = null) + public static void PropertyValues(this IContentBase content, IContentTypeBaseServiceProvider contentTypeServiceProvider, object value, string culture = null, string segment = null) { if (value == null) throw new Exception("No properties has been passed in"); - var contentType = Current.Services.ContentTypeBaseServices.GetContentTypeOf(content); - var propertyInfos = value.GetType().GetProperties(); foreach (var propertyInfo in propertyInfos) { - //Check if a PropertyType with alias exists thus being a valid property - var propertyType = contentType.CompositionPropertyTypes.FirstOrDefault(x => x.Alias == propertyInfo.Name); - if (propertyType == null) - throw new Exception($"The property alias {propertyInfo.Name} is not valid, because no PropertyType with this alias exists"); - - //Check if a Property with the alias already exists in the collection thus being updated or inserted - var item = content.Properties.FirstOrDefault(x => x.Alias == propertyInfo.Name); - if (item != null) + if (content.Properties.TryGetValue(propertyInfo.Name, out var property)) { - item.SetValue(propertyInfo.GetValue(value, null), culture, segment); + property.SetValue(propertyInfo.GetValue(value, null), culture, segment); //Update item with newly added value - content.Properties.Add(item); + content.Properties.Add(property); } else { + + //TODO: Will this ever happen?? In theory we don't need to lookup the content type here since we can just check if the content contains properties with the correct name, + // however, i think this may be needed in the case where the content type contains property types that do not exist yet as properties on the + // content item? But can that happen? AFAIK it can't/shouldn't because of how we create content items in ContentBase we do _properties.EnsurePropertyTypes! + + var contentType = contentTypeServiceProvider.GetContentTypeOf(content); + var propertyType = contentType.CompositionPropertyTypes.FirstOrDefault(x => x.Alias == propertyInfo.Name); + if (propertyType == null) + throw new Exception($"The property alias {propertyInfo.Name} is not valid, because no PropertyType with this alias exists"); //Create new Property to add to collection - var property = propertyType.CreateProperty(); + property = propertyType.CreateProperty(); property.SetValue(propertyInfo.GetValue(value, null), culture, segment); content.Properties.Add(property); } diff --git a/src/Umbraco.Web/Editors/Filters/MemberSaveValidationAttribute.cs b/src/Umbraco.Web/Editors/Filters/MemberSaveValidationAttribute.cs index 619bd76333..80d5f684fe 100644 --- a/src/Umbraco.Web/Editors/Filters/MemberSaveValidationAttribute.cs +++ b/src/Umbraco.Web/Editors/Filters/MemberSaveValidationAttribute.cs @@ -5,6 +5,7 @@ using System.Web.Http.Controllers; using System.Web.Http.Filters; using Umbraco.Core.Logging; using Umbraco.Core.Models; +using Umbraco.Core.Services; using Umbraco.Web.Composing; using Umbraco.Web.Models.ContentEditing; @@ -17,21 +18,23 @@ namespace Umbraco.Web.Editors.Filters { private readonly ILogger _logger; private readonly IUmbracoContextAccessor _umbracoContextAccessor; + private readonly IMemberTypeService _memberTypeService; - public MemberSaveValidationAttribute() : this(Current.Logger, Current.UmbracoContextAccessor) + public MemberSaveValidationAttribute() : this(Current.Logger, Current.UmbracoContextAccessor, Current.Services.MemberTypeService) { } - public MemberSaveValidationAttribute(ILogger logger, IUmbracoContextAccessor umbracoContextAccessor) + public MemberSaveValidationAttribute(ILogger logger, IUmbracoContextAccessor umbracoContextAccessor, IMemberTypeService memberTypeService) { _logger = logger; _umbracoContextAccessor = umbracoContextAccessor; + _memberTypeService = memberTypeService; } public override void OnActionExecuting(HttpActionContext actionContext) { var model = (MemberSave)actionContext.ActionArguments["contentItem"]; - var contentItemValidator = new MemberValidationHelper(_logger, _umbracoContextAccessor); + var contentItemValidator = new MemberValidationHelper(_logger, _umbracoContextAccessor, _memberTypeService); //now do each validation step if (contentItemValidator.ValidateExistingContent(model, actionContext)) if (contentItemValidator.ValidateProperties(model, model, actionContext)) diff --git a/src/Umbraco.Web/Editors/Filters/MemberValidationHelper.cs b/src/Umbraco.Web/Editors/Filters/MemberValidationHelper.cs index c97e35eb0d..8f98765c34 100644 --- a/src/Umbraco.Web/Editors/Filters/MemberValidationHelper.cs +++ b/src/Umbraco.Web/Editors/Filters/MemberValidationHelper.cs @@ -10,43 +10,22 @@ using Umbraco.Core; using Umbraco.Core.Composing; using Umbraco.Core.Logging; using Umbraco.Core.Models; +using Umbraco.Core.Services; using Umbraco.Web.Models.ContentEditing; namespace Umbraco.Web.Editors.Filters { - //internal class ContentValidationHelper : ContentItemValidationHelper - //{ - // public ContentValidationHelper(ILogger logger, IUmbracoContextAccessor umbracoContextAccessor) : base(logger, umbracoContextAccessor) - // { - // } - - // /// - // /// Validates that the correct information is in the request for saving a culture variant - // /// - // /// - // /// - // /// - // protected override bool ValidateCultureVariant(ContentItemSave postedItem, HttpActionContext actionContext) - // { - // var contentType = postedItem.PersistedContent.GetContentType(); - // if (contentType.VariesByCulture() && postedItem.Culture.IsNullOrWhiteSpace()) - // { - // //we cannot save a content item that is culture variant if no culture was specified in the request! - // actionContext.Response = actionContext.Request.CreateValidationErrorResponse($"No culture found in request. Cannot save a content item that varies by culture, without a specified culture."); - // return false; - // } - // return true; - // } - //} - /// /// Custom validation helper so that we can exclude the Member.StandardPropertyTypeStubs from being validating for existence /// internal class MemberValidationHelper : ContentItemValidationHelper { - public MemberValidationHelper(ILogger logger, IUmbracoContextAccessor umbracoContextAccessor) : base(logger, umbracoContextAccessor) + private readonly IMemberTypeService _memberTypeService; + + public MemberValidationHelper(ILogger logger, IUmbracoContextAccessor umbracoContextAccessor, IMemberTypeService memberTypeService) : base(logger, umbracoContextAccessor) { + _memberTypeService = memberTypeService; } /// @@ -118,7 +97,7 @@ namespace Umbraco.Web.Editors.Filters //if a sensitive value is being submitted. if (UmbracoContextAccessor.UmbracoContext.Security.CurrentUser.HasAccessToSensitiveData() == false) { - var contentType = Current.Services.MemberTypeService.Get(model.PersistedContent.ContentTypeId); + var contentType = _memberTypeService.Get(model.PersistedContent.ContentTypeId); var sensitiveProperties = contentType .PropertyTypes.Where(x => contentType.IsSensitiveProperty(x.Alias)) .ToList();