From 5660c6c369d41c6302cb03cd5b7b0114d4ce5322 Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Fri, 18 Jul 2025 10:47:34 +0200 Subject: [PATCH] Forward merge #19720 to V16 (#19735) * Add support for programmatic creation of property types providing the data type key (#19720) * Add support for programmatic creation of property types providing the data type key. * Add integration tests --------- Co-authored-by: kjac * Don't use Lazy --------- Co-authored-by: Andy Butland --- .../Implement/ContentTypeRepository.cs | 6 +- .../Implement/ContentTypeRepositoryBase.cs | 79 +++++++++++++------ .../Implement/MediaTypeRepository.cs | 6 +- .../Implement/MemberTypeRepository.cs | 8 +- .../Testing/UmbracoIntegrationTest.cs | 2 + .../Repositories/DocumentRepositoryTest.cs | 2 +- .../Repositories/MediaRepositoryTest.cs | 2 +- .../Repositories/MediaTypeRepositoryTest.cs | 3 +- .../Repositories/MemberTypeRepositoryTest.cs | 3 +- .../Repositories/TemplateRepositoryTest.cs | 2 +- .../Services/ContentTypeServiceTests.cs | 59 ++++++++++++++ 11 files changed, 135 insertions(+), 37 deletions(-) diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentTypeRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentTypeRepository.cs index f437c0cd0d..e1672beda9 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentTypeRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentTypeRepository.cs @@ -5,6 +5,7 @@ using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Persistence.Querying; using Umbraco.Cms.Core.Persistence.Repositories; +using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Strings; using Umbraco.Cms.Infrastructure.Persistence.Dtos; using Umbraco.Cms.Infrastructure.Persistence.Querying; @@ -25,8 +26,9 @@ internal class ContentTypeRepository : ContentTypeRepositoryBase, ILogger logger, IContentTypeCommonRepository commonRepository, ILanguageRepository languageRepository, - IShortStringHelper shortStringHelper) - : base(scopeAccessor, cache, logger, commonRepository, languageRepository, shortStringHelper) + IShortStringHelper shortStringHelper, + IIdKeyMap idKeyMap) + : base(scopeAccessor, cache, logger, commonRepository, languageRepository, shortStringHelper, idKeyMap) { } diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs index 0d68cd39bf..ae843ddfad 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs @@ -29,15 +29,22 @@ internal abstract class ContentTypeRepositoryBase : EntityRepositoryBas where TEntity : class, IContentTypeComposition { private readonly IShortStringHelper _shortStringHelper; + private readonly IIdKeyMap _idKeyMap; - protected ContentTypeRepositoryBase(IScopeAccessor scopeAccessor, AppCaches cache, - ILogger> logger, IContentTypeCommonRepository commonRepository, - ILanguageRepository languageRepository, IShortStringHelper shortStringHelper) + protected ContentTypeRepositoryBase( + IScopeAccessor scopeAccessor, + AppCaches cache, + ILogger> logger, + IContentTypeCommonRepository commonRepository, + ILanguageRepository languageRepository, + IShortStringHelper shortStringHelper, + IIdKeyMap idKeyMap) : base(scopeAccessor, cache, logger) { _shortStringHelper = shortStringHelper; CommonRepository = commonRepository; LanguageRepository = languageRepository; + _idKeyMap = idKeyMap; } protected IContentTypeCommonRepository CommonRepository { get; } @@ -291,7 +298,7 @@ AND umbracoNode.nodeObjectType = @objectType", // If the Id of the DataType is not set, we resolve it from the db by its PropertyEditorAlias if (propertyType.DataTypeId == 0 || propertyType.DataTypeId == default) { - AssignDataTypeFromPropertyEditor(propertyType); + AssignDataTypeIdFromProvidedKeyOrPropertyEditor(propertyType); } PropertyTypeDto propertyTypeDto = @@ -592,7 +599,7 @@ AND umbracoNode.id <> @id", // if the Id of the DataType is not set, we resolve it from the db by its PropertyEditorAlias if (propertyType.DataTypeId == 0 || propertyType.DataTypeId == default) { - AssignDataTypeFromPropertyEditor(propertyType); + AssignDataTypeIdFromProvidedKeyOrPropertyEditor(propertyType); } // validate the alias @@ -1434,37 +1441,59 @@ AND umbracoNode.id <> @id", protected abstract TEntity? PerformGet(Guid id); /// - /// Try to set the data type id based on its ControlId + /// Try to set the data type Id based on the provided key or property editor alias. /// /// - private void AssignDataTypeFromPropertyEditor(IPropertyType propertyType) + private void AssignDataTypeIdFromProvidedKeyOrPropertyEditor(IPropertyType propertyType) { - // we cannot try to assign a data type of it's empty - if (propertyType.PropertyEditorAlias.IsNullOrWhiteSpace() == false) + // If a key is provided, use that. + if (propertyType.DataTypeKey != Guid.Empty) { - Sql sql = Sql() - .Select(dt => dt.Select(x => x.NodeDto)) - .From() - .InnerJoin().On((dt, n) => dt.NodeId == n.NodeId) - .Where( - "propertyEditorAlias = @propertyEditorAlias", - new { propertyEditorAlias = propertyType.PropertyEditorAlias }) - .OrderBy(typeDto => typeDto.NodeId); - DataTypeDto? datatype = Database.FirstOrDefault(sql); - - // we cannot assign a data type if one was not found - if (datatype != null) + Attempt dataTypeIdAttempt = _idKeyMap.GetIdForKey(propertyType.DataTypeKey, UmbracoObjectTypes.DataType); + if (dataTypeIdAttempt.Success) { - propertyType.DataTypeId = datatype.NodeId; - propertyType.DataTypeKey = datatype.NodeDto.UniqueId; + propertyType.DataTypeId = dataTypeIdAttempt.Result; + return; } else { Logger.LogWarning( - "Could not assign a data type for the property type {PropertyTypeAlias} since no data type was found with a property editor {PropertyEditorAlias}", - propertyType.Alias, propertyType.PropertyEditorAlias); + "Could not assign a data type for the property type {PropertyTypeAlias} since no integer Id was found matching the key {DataTypeKey}. Falling back to look up via the property editor alias.", + propertyType.Alias, + propertyType.DataTypeKey); } } + + // Otherwise if a property editor alias is provided, try to find a data type that uses that alias. + if (propertyType.PropertyEditorAlias.IsNullOrWhiteSpace()) + { + // We cannot try to assign a data type if it's empty. + return; + } + + Sql sql = Sql() + .Select(dt => dt.Select(x => x.NodeDto)) + .From() + .InnerJoin().On((dt, n) => dt.NodeId == n.NodeId) + .Where( + "propertyEditorAlias = @propertyEditorAlias", + new { propertyEditorAlias = propertyType.PropertyEditorAlias }) + .OrderBy(typeDto => typeDto.NodeId); + DataTypeDto? datatype = Database.FirstOrDefault(sql); + + // we cannot assign a data type if one was not found + if (datatype != null) + { + propertyType.DataTypeId = datatype.NodeId; + propertyType.DataTypeKey = datatype.NodeDto.UniqueId; + } + else + { + Logger.LogWarning( + "Could not assign a data type for the property type {PropertyTypeAlias} since no data type was found with a property editor {PropertyEditorAlias}", + propertyType.Alias, + propertyType.PropertyEditorAlias); + } } protected abstract TEntity? PerformGet(string alias); diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MediaTypeRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MediaTypeRepository.cs index 51a4c36752..792e984387 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MediaTypeRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MediaTypeRepository.cs @@ -5,6 +5,7 @@ using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Persistence.Querying; using Umbraco.Cms.Core.Persistence.Repositories; +using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Strings; using Umbraco.Cms.Infrastructure.Persistence.Dtos; using Umbraco.Cms.Infrastructure.Persistence.Querying; @@ -24,8 +25,9 @@ internal class MediaTypeRepository : ContentTypeRepositoryBase, IMed ILogger logger, IContentTypeCommonRepository commonRepository, ILanguageRepository languageRepository, - IShortStringHelper shortStringHelper) - : base(scopeAccessor, cache, logger, commonRepository, languageRepository, shortStringHelper) + IShortStringHelper shortStringHelper, + IIdKeyMap idKeyMap) + : base(scopeAccessor, cache, logger, commonRepository, languageRepository, shortStringHelper, idKeyMap) { } diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberTypeRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberTypeRepository.cs index 61127b766b..e764f54090 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberTypeRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberTypeRepository.cs @@ -5,6 +5,7 @@ using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Persistence.Querying; using Umbraco.Cms.Core.Persistence.Repositories; +using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Strings; using Umbraco.Cms.Infrastructure.Persistence.Dtos; using Umbraco.Cms.Infrastructure.Persistence.Factories; @@ -27,9 +28,10 @@ internal class MemberTypeRepository : ContentTypeRepositoryBase, IM ILogger logger, IContentTypeCommonRepository commonRepository, ILanguageRepository languageRepository, - IShortStringHelper shortStringHelper) - : base(scopeAccessor, cache, logger, commonRepository, languageRepository, shortStringHelper) => - _shortStringHelper = shortStringHelper; + IShortStringHelper shortStringHelper, + IIdKeyMap idKeyMap) + : base(scopeAccessor, cache, logger, commonRepository, languageRepository, shortStringHelper, idKeyMap) + => _shortStringHelper = shortStringHelper; protected override bool SupportsPublishing => MemberType.SupportsPublishingConst; diff --git a/tests/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTest.cs b/tests/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTest.cs index d8fd5f48a4..ec92a40fe7 100644 --- a/tests/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTest.cs +++ b/tests/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTest.cs @@ -60,6 +60,8 @@ public abstract class UmbracoIntegrationTest : UmbracoIntegrationTestBase protected IShortStringHelper ShortStringHelper => Services.GetRequiredService(); + protected IIdKeyMap IdKeyMap => Services.GetRequiredService(); + protected GlobalSettings GlobalSettings => Services.GetRequiredService>().Value; protected IMapperCollection Mappers => Services.GetRequiredService(); diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/DocumentRepositoryTest.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/DocumentRepositoryTest.cs index 94a6c1884a..22cc35b424 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/DocumentRepositoryTest.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/DocumentRepositoryTest.cs @@ -123,7 +123,7 @@ internal sealed class DocumentRepositoryTest : UmbracoIntegrationTest new ContentTypeCommonRepository(scopeAccessor, templateRepository, appCaches, ShortStringHelper); var languageRepository = new LanguageRepository(scopeAccessor, appCaches, LoggerFactory.CreateLogger()); - contentTypeRepository = new ContentTypeRepository(scopeAccessor, appCaches, LoggerFactory.CreateLogger(), commonRepository, languageRepository, ShortStringHelper); + contentTypeRepository = new ContentTypeRepository(scopeAccessor, appCaches, LoggerFactory.CreateLogger(), commonRepository, languageRepository, ShortStringHelper, IdKeyMap); var relationTypeRepository = new RelationTypeRepository(scopeAccessor, AppCaches.Disabled, LoggerFactory.CreateLogger()); var entityRepository = new EntityRepository(scopeAccessor, AppCaches.Disabled); var relationRepository = new RelationRepository(scopeAccessor, LoggerFactory.CreateLogger(), relationTypeRepository, entityRepository); diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/MediaRepositoryTest.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/MediaRepositoryTest.cs index 3b21605d07..1532846fc4 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/MediaRepositoryTest.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/MediaRepositoryTest.cs @@ -56,7 +56,7 @@ internal sealed class MediaRepositoryTest : UmbracoIntegrationTest new ContentTypeCommonRepository(scopeAccessor, TemplateRepository, appCaches, ShortStringHelper); var languageRepository = new LanguageRepository(scopeAccessor, appCaches, LoggerFactory.CreateLogger()); - mediaTypeRepository = new MediaTypeRepository(scopeAccessor, appCaches, LoggerFactory.CreateLogger(), commonRepository, languageRepository, ShortStringHelper); + mediaTypeRepository = new MediaTypeRepository(scopeAccessor, appCaches, LoggerFactory.CreateLogger(), commonRepository, languageRepository, ShortStringHelper, IdKeyMap); var tagRepository = new TagRepository(scopeAccessor, appCaches, LoggerFactory.CreateLogger()); var relationTypeRepository = new RelationTypeRepository(scopeAccessor, AppCaches.Disabled, LoggerFactory.CreateLogger()); var entityRepository = new EntityRepository(scopeAccessor, AppCaches.Disabled); diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/MediaTypeRepositoryTest.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/MediaTypeRepositoryTest.cs index 34a9245636..a05d5b96c9 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/MediaTypeRepositoryTest.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/MediaTypeRepositoryTest.cs @@ -9,6 +9,7 @@ using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Persistence; using Umbraco.Cms.Core.Persistence.Repositories; +using Umbraco.Cms.Core.Services; using Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement; using Umbraco.Cms.Infrastructure.Scoping; using Umbraco.Cms.Tests.Common.Builders; @@ -411,7 +412,7 @@ internal sealed class MediaTypeRepositoryTest : UmbracoIntegrationTest } private MediaTypeRepository CreateRepository(IScopeProvider provider) => - new((IScopeAccessor)provider, AppCaches.Disabled, LoggerFactory.CreateLogger(), CommonRepository, LanguageRepository, ShortStringHelper); + new((IScopeAccessor)provider, AppCaches.Disabled, LoggerFactory.CreateLogger(), CommonRepository, LanguageRepository, ShortStringHelper, IdKeyMap); private EntityContainerRepository CreateContainerRepository(IScopeProvider provider) => new((IScopeAccessor)provider, AppCaches.Disabled, LoggerFactory.CreateLogger(), Constants.ObjectTypes.MediaTypeContainer); diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/MemberTypeRepositoryTest.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/MemberTypeRepositoryTest.cs index 0c1f071239..6392354a6f 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/MemberTypeRepositoryTest.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/MemberTypeRepositoryTest.cs @@ -10,6 +10,7 @@ using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Persistence; using Umbraco.Cms.Core.Persistence.Repositories; +using Umbraco.Cms.Core.Services; using Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement; using Umbraco.Cms.Infrastructure.Scoping; using Umbraco.Cms.Tests.Common.Builders; @@ -26,7 +27,7 @@ internal sealed class MemberTypeRepositoryTest : UmbracoIntegrationTest { var commonRepository = GetRequiredService(); var languageRepository = GetRequiredService(); - return new MemberTypeRepository((IScopeAccessor)provider, AppCaches.Disabled, Mock.Of>(), commonRepository, languageRepository, ShortStringHelper); + return new MemberTypeRepository((IScopeAccessor)provider, AppCaches.Disabled, Mock.Of>(), commonRepository, languageRepository, ShortStringHelper, IdKeyMap); } [Test] diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/TemplateRepositoryTest.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/TemplateRepositoryTest.cs index 973ea658dc..2350b1d1f5 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/TemplateRepositoryTest.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/TemplateRepositoryTest.cs @@ -266,7 +266,7 @@ internal sealed class TemplateRepositoryTest : UmbracoIntegrationTest var commonRepository = new ContentTypeCommonRepository(scopeAccessor, templateRepository, AppCaches, ShortStringHelper); var languageRepository = new LanguageRepository(scopeAccessor, AppCaches.Disabled, LoggerFactory.CreateLogger()); - var contentTypeRepository = new ContentTypeRepository(scopeAccessor, AppCaches.Disabled, LoggerFactory.CreateLogger(), commonRepository, languageRepository, ShortStringHelper); + var contentTypeRepository = new ContentTypeRepository(scopeAccessor, AppCaches.Disabled, LoggerFactory.CreateLogger(), commonRepository, languageRepository, ShortStringHelper, IdKeyMap); var relationTypeRepository = new RelationTypeRepository(scopeAccessor, AppCaches.Disabled, LoggerFactory.CreateLogger()); var entityRepository = new EntityRepository(scopeAccessor, AppCaches.Disabled); var relationRepository = new RelationRepository(scopeAccessor, LoggerFactory.CreateLogger(), relationTypeRepository, entityRepository); diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentTypeServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentTypeServiceTests.cs index 41064377a7..fd2c335112 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentTypeServiceTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentTypeServiceTests.cs @@ -1971,6 +1971,65 @@ internal sealed class ContentTypeServiceTests : UmbracoIntegrationTest .Variations); } + [Test] + public void Can_Create_Property_Type_Based_On_DataTypeKey() + { + // Arrange + var cts = ContentTypeService; + var dtdYesNo = DataTypeService.GetDataType(-49); + IContentType ctBase = new ContentType(ShortStringHelper, -1) + { + Name = "Base", + Alias = "Base", + Icon = "folder.gif", + Thumbnail = "folder.png" + }; + ctBase.AddPropertyType(new PropertyType(ShortStringHelper, "ShouldNotMatter", ValueStorageType.Nvarchar) + { + Name = "Hide From Navigation", + Alias = Constants.Conventions.Content.NaviHide, + DataTypeKey = dtdYesNo.Key + }); + cts.Save(ctBase); + + // Assert + ctBase = cts.Get(ctBase.Key); + Assert.That(ctBase, Is.Not.Null); + Assert.That(ctBase.HasIdentity, Is.True); + Assert.That(ctBase.PropertyTypes.Count(), Is.EqualTo(1)); + Assert.That(ctBase.PropertyTypes.First().DataTypeId, Is.EqualTo(dtdYesNo.Id)); + Assert.That(ctBase.PropertyTypes.First().PropertyEditorAlias, Is.EqualTo(dtdYesNo.EditorAlias)); + } + + [Test] + public void Can_Create_Property_Type_Based_On_PropertyEditorAlias() + { + // Arrange + var cts = ContentTypeService; + var dtdYesNo = DataTypeService.GetDataType(-49); + IContentType ctBase = new ContentType(ShortStringHelper, -1) + { + Name = "Base", + Alias = "Base", + Icon = "folder.gif", + Thumbnail = "folder.png" + }; + ctBase.AddPropertyType(new PropertyType(ShortStringHelper, "Umbraco.TrueFalse", ValueStorageType.Nvarchar) + { + Name = "Hide From Navigation", + Alias = Constants.Conventions.Content.NaviHide, + }); + cts.Save(ctBase); + + // Assert + ctBase = cts.Get(ctBase.Key); + Assert.That(ctBase, Is.Not.Null); + Assert.That(ctBase.HasIdentity, Is.True); + Assert.That(ctBase.PropertyTypes.Count(), Is.EqualTo(1)); + Assert.That(ctBase.PropertyTypes.First().DataTypeId, Is.EqualTo(dtdYesNo.Id)); + Assert.That(ctBase.PropertyTypes.First().PropertyEditorAlias, Is.EqualTo(dtdYesNo.EditorAlias)); + } + private ContentType CreateComponent() { var component = new ContentType(ShortStringHelper, -1)