From cfa32b265a52510479ee6d97dbf3e8957c733c01 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Tue, 11 Nov 2025 06:59:25 +0100 Subject: [PATCH] Integration Tests: Avoid asserting on errors for permission tests (#20643) * Added integration tests for PropertyTypeUsageService and adjusted assert in management API permissions test. * Commented or fixed management API integration tests verifying permissions where we were asserting on an error response. --- .../IPropertyTypeUsageRepository.cs | 10 +++++ .../Services/IPropertyTypeUsageService.cs | 3 ++ .../Services/PropertyTypeUsageService.cs | 10 ++++- .../Implement/PropertyTypeUsageRepository.cs | 38 ++++++++----------- ...ExecuteActionHealthCheckControllerTests.cs | 4 +- .../LogViewer/AllLogViewerControllerTests.cs | 2 +- ...MessageTemplateLogViewerControllerTests.cs | 2 +- .../LogLevelCountLogViewerControllerTests.cs | 2 +- ...dateLogFileSizeLogViewerControllerTests.cs | 2 +- .../IsUsedPropertyTypeControllerTests.cs | 2 +- .../User/InviteUserControllerTests.cs | 2 +- .../UmbracoIntegrationTestWithContent.cs | 5 ++- .../Services/PropertyTypeUsageServiceTests.cs | 26 +++++++++++++ 13 files changed, 73 insertions(+), 35 deletions(-) create mode 100644 tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/PropertyTypeUsageServiceTests.cs diff --git a/src/Umbraco.Core/Persistence/Repositories/IPropertyTypeUsageRepository.cs b/src/Umbraco.Core/Persistence/Repositories/IPropertyTypeUsageRepository.cs index 7aa05fc1ff..d0e3eb356c 100644 --- a/src/Umbraco.Core/Persistence/Repositories/IPropertyTypeUsageRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/IPropertyTypeUsageRepository.cs @@ -1,7 +1,17 @@ namespace Umbraco.Cms.Core.Persistence.Repositories; +/// +/// Defines repository methods for querying property type usage. +/// public interface IPropertyTypeUsageRepository { + /// + /// Determines whether there are any saved property values for the specified content type and property alias. + /// Task HasSavedPropertyValuesAsync(Guid contentTypeKey, string propertyAlias); + + /// + /// Determines whether a content type with the specified unique identifier exists. + /// Task ContentTypeExistAsync(Guid contentTypeKey); } diff --git a/src/Umbraco.Core/Services/IPropertyTypeUsageService.cs b/src/Umbraco.Core/Services/IPropertyTypeUsageService.cs index df061ca628..5497e575c5 100644 --- a/src/Umbraco.Core/Services/IPropertyTypeUsageService.cs +++ b/src/Umbraco.Core/Services/IPropertyTypeUsageService.cs @@ -2,6 +2,9 @@ using Umbraco.Cms.Core.Services.OperationStatus; namespace Umbraco.Cms.Core.Services; +/// +/// Defines service methods for querying property type usage. +/// public interface IPropertyTypeUsageService { /// diff --git a/src/Umbraco.Core/Services/PropertyTypeUsageService.cs b/src/Umbraco.Core/Services/PropertyTypeUsageService.cs index d22bdeb440..579a338ecd 100644 --- a/src/Umbraco.Core/Services/PropertyTypeUsageService.cs +++ b/src/Umbraco.Core/Services/PropertyTypeUsageService.cs @@ -4,19 +4,25 @@ using Umbraco.Cms.Core.Services.OperationStatus; namespace Umbraco.Cms.Core.Services; +/// public class PropertyTypeUsageService : IPropertyTypeUsageService { private readonly IPropertyTypeUsageRepository _propertyTypeUsageRepository; - private readonly IContentTypeService _contentTypeService; private readonly ICoreScopeProvider _scopeProvider; + // TODO (V18): Remove IContentTypeService parameter from constructor. + + /// + /// Initializes a new instance of the class. + /// public PropertyTypeUsageService( IPropertyTypeUsageRepository propertyTypeUsageRepository, +#pragma warning disable IDE0060 // Remove unused parameter IContentTypeService contentTypeService, +#pragma warning restore IDE0060 // Remove unused parameter ICoreScopeProvider scopeProvider) { _propertyTypeUsageRepository = propertyTypeUsageRepository; - _contentTypeService = contentTypeService; _scopeProvider = scopeProvider; } diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/PropertyTypeUsageRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/PropertyTypeUsageRepository.cs index ab9a03bcef..64917f9af1 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/PropertyTypeUsageRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/PropertyTypeUsageRepository.cs @@ -1,4 +1,3 @@ - using NPoco; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Persistence.Repositories; @@ -8,28 +7,26 @@ using Umbraco.Extensions; namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement; +/// internal sealed class PropertyTypeUsageRepository : IPropertyTypeUsageRepository { - private static readonly Guid?[] NodeObjectTypes = new Guid?[] - { + private static readonly List _nodeObjectTypes = + [ Constants.ObjectTypes.DocumentType, Constants.ObjectTypes.MediaType, Constants.ObjectTypes.MemberType, - }; + ]; private readonly IScopeAccessor _scopeAccessor; - public PropertyTypeUsageRepository(IScopeAccessor scopeAccessor) - { - _scopeAccessor = scopeAccessor; - } + /// + /// Initializes a new instance of the class. + /// + public PropertyTypeUsageRepository(IScopeAccessor scopeAccessor) => _scopeAccessor = scopeAccessor; + /// public Task HasSavedPropertyValuesAsync(Guid contentTypeKey, string propertyAlias) { - IUmbracoDatabase? database = _scopeAccessor.AmbientScope?.Database; - - if (database is null) - { - throw new InvalidOperationException("A scope is required to query the database"); - } + IUmbracoDatabase? database = _scopeAccessor.AmbientScope?.Database + ?? throw new InvalidOperationException("A scope is required to query the database"); Sql selectQuery = database.SqlContext.Sql() .SelectAll() @@ -47,26 +44,21 @@ internal sealed class PropertyTypeUsageRepository : IPropertyTypeUsageRepository return Task.FromResult(database.ExecuteScalar(hasValuesQuery)); } + /// public Task ContentTypeExistAsync(Guid contentTypeKey) { - IUmbracoDatabase? database = _scopeAccessor.AmbientScope?.Database; - - if (database is null) - { - throw new InvalidOperationException("A scope is required to query the database"); - } + IUmbracoDatabase? database = _scopeAccessor.AmbientScope?.Database + ?? throw new InvalidOperationException("A scope is required to query the database"); Sql selectQuery = database.SqlContext.Sql() .SelectAll() .From("n") .Where(n => n.UniqueId == contentTypeKey, "n") - .Where(n => NodeObjectTypes.Contains(n.NodeObjectType), "n"); + .WhereIn(n => n.NodeObjectType, _nodeObjectTypes, "n"); Sql hasValuesQuery = database.SqlContext.Sql() .SelectAnyIfExists(selectQuery); return Task.FromResult(database.ExecuteScalar(hasValuesQuery)); } - - } diff --git a/tests/Umbraco.Tests.Integration/ManagementApi/HealthCheck/ExecuteActionHealthCheckControllerTests.cs b/tests/Umbraco.Tests.Integration/ManagementApi/HealthCheck/ExecuteActionHealthCheckControllerTests.cs index 249710ab1e..e4dd67d64d 100644 --- a/tests/Umbraco.Tests.Integration/ManagementApi/HealthCheck/ExecuteActionHealthCheckControllerTests.cs +++ b/tests/Umbraco.Tests.Integration/ManagementApi/HealthCheck/ExecuteActionHealthCheckControllerTests.cs @@ -27,7 +27,7 @@ public class ExecuteActionHealthCheckControllerTests : ManagementApiUserGroupTes protected override UserGroupAssertionModel AdminUserGroupAssertionModel => new() { - ExpectedStatusCode = HttpStatusCode.InternalServerError + ExpectedStatusCode = HttpStatusCode.OK }; protected override UserGroupAssertionModel EditorUserGroupAssertionModel => new() @@ -58,7 +58,7 @@ public class ExecuteActionHealthCheckControllerTests : ManagementApiUserGroupTes protected override async Task ClientRequest() { HealthCheckActionRequestModel healthCheckActionRequest = - new() { HealthCheck = new ReferenceByIdModel(_dataIntegrityHealthCheckId), ValueRequired = false }; + new() { HealthCheck = new ReferenceByIdModel(_dataIntegrityHealthCheckId), ValueRequired = false, Alias = "fixContentPaths" }; return await Client.PostAsync(Url, JsonContent.Create(healthCheckActionRequest)); } } diff --git a/tests/Umbraco.Tests.Integration/ManagementApi/LogViewer/AllLogViewerControllerTests.cs b/tests/Umbraco.Tests.Integration/ManagementApi/LogViewer/AllLogViewerControllerTests.cs index a5e4a2b4eb..9e8c1844ae 100644 --- a/tests/Umbraco.Tests.Integration/ManagementApi/LogViewer/AllLogViewerControllerTests.cs +++ b/tests/Umbraco.Tests.Integration/ManagementApi/LogViewer/AllLogViewerControllerTests.cs @@ -12,7 +12,7 @@ public class AllLogViewerControllerTests : ManagementApiUserGroupTestBase new() { - ExpectedStatusCode = HttpStatusCode.InternalServerError + ExpectedStatusCode = HttpStatusCode.OK }; protected override UserGroupAssertionModel EditorUserGroupAssertionModel => new() diff --git a/tests/Umbraco.Tests.Integration/ManagementApi/LogViewer/AllMessageTemplateLogViewerControllerTests.cs b/tests/Umbraco.Tests.Integration/ManagementApi/LogViewer/AllMessageTemplateLogViewerControllerTests.cs index 71c5d12ebf..71df002794 100644 --- a/tests/Umbraco.Tests.Integration/ManagementApi/LogViewer/AllMessageTemplateLogViewerControllerTests.cs +++ b/tests/Umbraco.Tests.Integration/ManagementApi/LogViewer/AllMessageTemplateLogViewerControllerTests.cs @@ -11,7 +11,7 @@ public class AllMessageTemplateLogViewerControllerTests : ManagementApiUserGroup // We get the InternalServerError for the admin because it has access, but there is no log file to view protected override UserGroupAssertionModel AdminUserGroupAssertionModel => new() { - ExpectedStatusCode = HttpStatusCode.InternalServerError + ExpectedStatusCode = HttpStatusCode.OK }; protected override UserGroupAssertionModel EditorUserGroupAssertionModel => new() diff --git a/tests/Umbraco.Tests.Integration/ManagementApi/LogViewer/LogLevelCountLogViewerControllerTests.cs b/tests/Umbraco.Tests.Integration/ManagementApi/LogViewer/LogLevelCountLogViewerControllerTests.cs index 96efcdb4ee..4fd8127281 100644 --- a/tests/Umbraco.Tests.Integration/ManagementApi/LogViewer/LogLevelCountLogViewerControllerTests.cs +++ b/tests/Umbraco.Tests.Integration/ManagementApi/LogViewer/LogLevelCountLogViewerControllerTests.cs @@ -11,7 +11,7 @@ public class LogLevelCountLogViewerControllerTests : ManagementApiUserGroupTestB // We get the InternalServerError for the admin because it has access, but there is no log file to view protected override UserGroupAssertionModel AdminUserGroupAssertionModel => new() { - ExpectedStatusCode = HttpStatusCode.InternalServerError + ExpectedStatusCode = HttpStatusCode.OK }; protected override UserGroupAssertionModel EditorUserGroupAssertionModel => new() diff --git a/tests/Umbraco.Tests.Integration/ManagementApi/LogViewer/ValidateLogFileSizeLogViewerControllerTests.cs b/tests/Umbraco.Tests.Integration/ManagementApi/LogViewer/ValidateLogFileSizeLogViewerControllerTests.cs index fcd26afd5a..56622b6db6 100644 --- a/tests/Umbraco.Tests.Integration/ManagementApi/LogViewer/ValidateLogFileSizeLogViewerControllerTests.cs +++ b/tests/Umbraco.Tests.Integration/ManagementApi/LogViewer/ValidateLogFileSizeLogViewerControllerTests.cs @@ -11,7 +11,7 @@ public class ValidateLogFileSizeLogViewerControllerTests: ManagementApiUserGroup // We get the InternalServerError for the admin because it has access, but there is no log file to view protected override UserGroupAssertionModel AdminUserGroupAssertionModel => new() { - ExpectedStatusCode = HttpStatusCode.InternalServerError + ExpectedStatusCode = HttpStatusCode.OK }; protected override UserGroupAssertionModel EditorUserGroupAssertionModel => new() diff --git a/tests/Umbraco.Tests.Integration/ManagementApi/PropertyType/IsUsedPropertyTypeControllerTests.cs b/tests/Umbraco.Tests.Integration/ManagementApi/PropertyType/IsUsedPropertyTypeControllerTests.cs index f9a7016019..c689ad242e 100644 --- a/tests/Umbraco.Tests.Integration/ManagementApi/PropertyType/IsUsedPropertyTypeControllerTests.cs +++ b/tests/Umbraco.Tests.Integration/ManagementApi/PropertyType/IsUsedPropertyTypeControllerTests.cs @@ -36,7 +36,7 @@ public class IsUsedPropertyTypeControllerTests : ManagementApiUserGroupTestBase< protected override UserGroupAssertionModel AdminUserGroupAssertionModel => new() { - ExpectedStatusCode = HttpStatusCode.InternalServerError + ExpectedStatusCode = HttpStatusCode.OK }; protected override UserGroupAssertionModel EditorUserGroupAssertionModel => new() diff --git a/tests/Umbraco.Tests.Integration/ManagementApi/User/InviteUserControllerTests.cs b/tests/Umbraco.Tests.Integration/ManagementApi/User/InviteUserControllerTests.cs index de91676ec3..827e958349 100644 --- a/tests/Umbraco.Tests.Integration/ManagementApi/User/InviteUserControllerTests.cs +++ b/tests/Umbraco.Tests.Integration/ManagementApi/User/InviteUserControllerTests.cs @@ -27,7 +27,7 @@ public class InviteUserControllerTests : ManagementApiUserGroupTestBase new() { - ExpectedStatusCode = HttpStatusCode.InternalServerError, + ExpectedStatusCode = HttpStatusCode.InternalServerError, // We expect an error here because email sending is not configured in these tests. }; protected override UserGroupAssertionModel EditorUserGroupAssertionModel => new() diff --git a/tests/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTestWithContent.cs b/tests/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTestWithContent.cs index 42aba90eb2..e8101d9414 100644 --- a/tests/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTestWithContent.cs +++ b/tests/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTestWithContent.cs @@ -1,7 +1,6 @@ // Copyright (c) Umbraco. // See LICENSE for more details. -using System; using NUnit.Framework; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Services; @@ -11,6 +10,8 @@ namespace Umbraco.Cms.Tests.Integration.Testing; public abstract class UmbracoIntegrationTestWithContent : UmbracoIntegrationTest { + protected const string TextpageContentTypeKey = "1D3A8E6E-2EA9-4CC1-B229-1AEE19821522"; + protected const string TextpageKey = "B58B3AD4-62C2-4E27-B1BE-837BD7C533E0"; protected const string SubPageKey = "07EABF4A-5C62-4662-9F2A-15BBB488BCA5"; protected const string SubPage2Key = "0EED78FC-A6A8-4587-AB18-D3AFE212B1C4"; @@ -48,7 +49,7 @@ public abstract class UmbracoIntegrationTestWithContent : UmbracoIntegrationTest // Create and Save ContentType "umbTextpage" -> 1051 (template), 1052 (content type) ContentType = ContentTypeBuilder.CreateSimpleContentType("umbTextpage", "Textpage", defaultTemplateId: template.Id); - ContentType.Key = new Guid("1D3A8E6E-2EA9-4CC1-B229-1AEE19821522"); + ContentType.Key = new Guid(TextpageContentTypeKey); ContentTypeService.Save(ContentType); // Create and Save Content "Homepage" based on "umbTextpage" -> 1053 diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/PropertyTypeUsageServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/PropertyTypeUsageServiceTests.cs new file mode 100644 index 0000000000..1905e23912 --- /dev/null +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/PropertyTypeUsageServiceTests.cs @@ -0,0 +1,26 @@ +using NUnit.Framework; +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Services.OperationStatus; +using Umbraco.Cms.Tests.Common.Testing; +using Umbraco.Cms.Tests.Integration.Testing; + +namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services; + +[TestFixture] +[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)] +internal sealed class PropertyTypeUsageServiceTests : UmbracoIntegrationTestWithContent +{ + private IPropertyTypeUsageService PropertyTypeUsageService => GetRequiredService(); + + [TestCase(TextpageContentTypeKey, "title", true, true, PropertyTypeOperationStatus.Success)] + [TestCase("1D3A8E6E-2EA9-4CC1-B229-1AEE19821523", "title", false, false, PropertyTypeOperationStatus.ContentTypeNotFound)] + [TestCase(TextpageContentTypeKey, "missingProperty", true, false, PropertyTypeOperationStatus.Success)] + public async Task Can_Check_For_Saved_Property_Values(Guid contentTypeKey, string propertyAlias, bool expectedSuccess, bool expectedResult, PropertyTypeOperationStatus expectedOperationStatus) + { + Attempt resultAttempt = await PropertyTypeUsageService.HasSavedPropertyValuesAsync(contentTypeKey, propertyAlias); + Assert.AreEqual(expectedSuccess, resultAttempt.Success); + Assert.AreEqual(expectedResult, resultAttempt.Result); + Assert.AreEqual(expectedOperationStatus, resultAttempt.Status); + } +}