From 447cb881bd881928584c19dfb236e3c7b7248931 Mon Sep 17 00:00:00 2001 From: Laura Neto <12862535+lauraneto@users.noreply.github.com> Date: Tue, 1 Jul 2025 09:12:37 +0200 Subject: [PATCH] Audit service rework (#19346) * Introduce new AuditEntryService - Moved logic related to the IAuditEntryRepository from the AuditService to the new service - Introduced new Async methods - Using ids (for easier transition from the previous Write method) - Using keys - Moved and updated integration tests related to the audit entries to a new test class `AuditEntryServiceTests` - Added unit tests class `AuditEntryServiceTests` and added a few unit tests - Added migration to add columns for `performingUserKey` and `affectedUserKey` and convert existing user ids - Adjusted usages of the old AuditService.Write method to use the new one (mostly notification handlers) * Audit service rework - Added new async and paged methods - Marked (now) redundant methods as obsolete - Updated all of the usages to use the non-obsolete methods - Added unit tests class `AuditServiceTests` and some unit tests - Updated existing integration test * Apply suggestions from code review * Small improvement * Update src/Umbraco.Core/Services/AuditService.cs * Some minor adjustments following the merge * Delete unnecessary file * Small cleanup on the tests --- .../Security/BackOfficeUserManagerAuditer.cs | 6 +- .../Events/RelateOnCopyNotificationHandler.cs | 39 +- src/Umbraco.Core/Services/AuditService.cs | 492 +++++++++++------- src/Umbraco.Core/Services/IAuditService.cs | 251 ++++++--- .../BackgroundJobs/Jobs/LogScrubberJob.cs | 19 +- .../UmbracoBuilder.CoreServices.cs | 6 +- .../RelateOnTrashNotificationHandler.cs | 52 +- .../Services/Implement/PackagingService.cs | 31 +- .../Services/ContentServiceTests.cs | 10 +- .../Services/AuditServiceTests.cs | 17 +- .../Services/ContentEditingServiceTests.cs | 2 +- .../Services/AuditServiceTests.cs | 232 +++++++++ .../Jobs/LogScrubberJobTests.cs | 2 +- 13 files changed, 836 insertions(+), 323 deletions(-) create mode 100644 tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/AuditServiceTests.cs diff --git a/src/Umbraco.Cms.Api.Management/Security/BackOfficeUserManagerAuditer.cs b/src/Umbraco.Cms.Api.Management/Security/BackOfficeUserManagerAuditer.cs index ab06c518ea..d844274afe 100644 --- a/src/Umbraco.Cms.Api.Management/Security/BackOfficeUserManagerAuditer.cs +++ b/src/Umbraco.Cms.Api.Management/Security/BackOfficeUserManagerAuditer.cs @@ -1,8 +1,4 @@ -using System.Globalization; -using System.Text; -using Microsoft.Extensions.DependencyInjection; -using Umbraco.Cms.Core; -using Umbraco.Cms.Core.DependencyInjection; +using System.Globalization; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Models.Membership; using Umbraco.Cms.Core.Notifications; diff --git a/src/Umbraco.Core/Events/RelateOnCopyNotificationHandler.cs b/src/Umbraco.Core/Events/RelateOnCopyNotificationHandler.cs index 3817f93f6f..ad0cdaac8c 100644 --- a/src/Umbraco.Core/Events/RelateOnCopyNotificationHandler.cs +++ b/src/Umbraco.Core/Events/RelateOnCopyNotificationHandler.cs @@ -1,31 +1,53 @@ // Copyright (c) Umbraco. // See LICENSE for more details. +using Microsoft.Extensions.DependencyInjection; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Notifications; using Umbraco.Cms.Core.Services; namespace Umbraco.Cms.Core.Events; -public class RelateOnCopyNotificationHandler : INotificationHandler +public class RelateOnCopyNotificationHandler : + INotificationHandler, + INotificationAsyncHandler { private readonly IAuditService _auditService; + private readonly IUserIdKeyResolver _userIdKeyResolver; private readonly IRelationService _relationService; - public RelateOnCopyNotificationHandler(IRelationService relationService, IAuditService auditService) + public RelateOnCopyNotificationHandler( + IRelationService relationService, + IAuditService auditService, + IUserIdKeyResolver userIdKeyResolver) { _relationService = relationService; _auditService = auditService; + _userIdKeyResolver = userIdKeyResolver; } - public void Handle(ContentCopiedNotification notification) + [Obsolete("Use the non-obsolete constructor instead. Scheduled for removal in V19.")] + public RelateOnCopyNotificationHandler( + IRelationService relationService, + IAuditService auditService) + : this( + relationService, + auditService, + StaticServiceProvider.Instance.GetRequiredService()) + { + } + + /// + public async Task HandleAsync(ContentCopiedNotification notification, CancellationToken cancellationToken) { if (notification.RelateToOriginal == false) { return; } - IRelationType? relationType = _relationService.GetRelationTypeByAlias(Constants.Conventions.RelationTypes.RelateDocumentOnCopyAlias); + IRelationType? relationType = + _relationService.GetRelationTypeByAlias(Constants.Conventions.RelationTypes.RelateDocumentOnCopyAlias); if (relationType == null) { @@ -43,11 +65,16 @@ public class RelateOnCopyNotificationHandler : INotificationHandler + HandleAsync(notification, CancellationToken.None).GetAwaiter().GetResult(); } diff --git a/src/Umbraco.Core/Services/AuditService.cs b/src/Umbraco.Core/Services/AuditService.cs index 4b71f6fd2a..342e62a004 100644 --- a/src/Umbraco.Core/Services/AuditService.cs +++ b/src/Umbraco.Core/Services/AuditService.cs @@ -7,15 +7,22 @@ using Umbraco.Cms.Core.Models.Membership; using Umbraco.Cms.Core.Persistence.Querying; using Umbraco.Cms.Core.Persistence.Repositories; using Umbraco.Cms.Core.Scoping; +using Umbraco.Cms.Core.Services.OperationStatus; namespace Umbraco.Cms.Core.Services.Implement; +/// +/// Audit service for logging and retrieving audit entries. +/// public sealed class AuditService : RepositoryService, IAuditService { - private readonly IUserService _userService; private readonly IAuditRepository _auditRepository; private readonly IEntityService _entityService; + private readonly IUserService _userService; + /// + /// Initializes a new instance of the class. + /// public AuditService( ICoreScopeProvider provider, ILoggerFactory loggerFactory, @@ -30,7 +37,10 @@ public sealed class AuditService : RepositoryService, IAuditService _entityService = entityService; } - [Obsolete("Use the non-obsolete constructor instead. Scheduled for removal in Umbraco 19.")] + /// + /// Initializes a new instance of the class. + /// + [Obsolete("Use the non-obsolete constructor. Scheduled for removal in Umbraco 19.")] public AuditService( ICoreScopeProvider provider, ILoggerFactory loggerFactory, @@ -49,79 +59,142 @@ public sealed class AuditService : RepositoryService, IAuditService { } - public void Add(AuditType type, int userId, int objectId, string? entityType, string comment, string? parameters = null) + /// + public async Task> AddAsync( + AuditType type, + Guid userKey, + int objectId, + string? entityType, + string? comment = null, + string? parameters = null) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope()) + var userId = (await _userService.GetAsync(userKey))?.Id; + if (userId is null) { - _auditRepository.Save(new AuditItem(objectId, type, userId, entityType, comment, parameters)); - scope.Complete(); + return Attempt.Fail(AuditLogOperationStatus.UserNotFound); } + + return AddInner(type, userId.Value, objectId, entityType, comment, parameters); } - public IEnumerable GetLogs(int objectId) + /// + [Obsolete("Use AddAsync() instead. Scheduled for removal in Umbraco 19.")] + public void Add( + AuditType type, + int userId, + int objectId, + string? entityType, + string comment, + string? parameters = null) => + AddInner(type, userId, objectId, entityType, comment, parameters); + + /// + public Task> GetItemsAsync( + int skip, + int take, + Direction orderDirection = Direction.Descending, + DateTimeOffset? sinceDate = null, + AuditType[]? auditTypeFilter = null) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope()) - { - IEnumerable result = _auditRepository.Get(Query().Where(x => x.Id == objectId)); - scope.Complete(); - return result; - } - } - - public IEnumerable GetUserLogs(int userId, AuditType type, DateTime? sinceDate = null) - { - using (ICoreScope scope = ScopeProvider.CreateCoreScope()) - { - IEnumerable result = sinceDate.HasValue == false - ? _auditRepository.Get(type, Query().Where(x => x.UserId == userId)) - : _auditRepository.Get( - type, - Query().Where(x => x.UserId == userId && x.CreateDate >= sinceDate.Value)); - scope.Complete(); - return result; - } + ArgumentOutOfRangeException.ThrowIfNegative(skip); + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(take); + + PaginationHelper.ConvertSkipTakeToPaging(skip, take, out var pageIndex, out var pageSize); + IQuery? customFilter = sinceDate.HasValue + ? Query().Where(x => x.CreateDate >= sinceDate) + : null; + + PagedModel result = GetItemsInner( + null, + pageIndex, + pageSize, + orderDirection, + auditTypeFilter, + customFilter); + return Task.FromResult(result); } + /// + [Obsolete("Use GetItemsAsync() instead. Scheduled for removal in Umbraco 19.")] public IEnumerable GetLogs(AuditType type, DateTime? sinceDate = null) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope()) - { - IEnumerable result = sinceDate.HasValue == false - ? _auditRepository.Get(type, Query()) - : _auditRepository.Get(type, Query().Where(x => x.CreateDate >= sinceDate.Value)); - scope.Complete(); - return result; - } + IQuery? customFilter = sinceDate.HasValue + ? Query().Where(x => x.CreateDate >= sinceDate) + : null; + + PagedModel result = GetItemsInner( + null, + 0, + int.MaxValue, + Direction.Ascending, + customFilter: customFilter); + return result.Items; } - public void CleanLogs(int maximumAgeOfLogsInMinutes) + /// + public Task> GetItemsByKeyAsync( + Guid entityKey, + UmbracoObjectTypes entityType, + int skip, + int take, + Direction orderDirection = Direction.Descending, + DateTimeOffset? sinceDate = null, + AuditType[]? auditTypeFilter = null) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope()) + ArgumentOutOfRangeException.ThrowIfNegative(skip); + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(take); + + Attempt keyToIdAttempt = _entityService.GetId(entityKey, entityType); + if (keyToIdAttempt.Success is false) { - _auditRepository.CleanLogs(maximumAgeOfLogsInMinutes); - scope.Complete(); + return Task.FromResult(new PagedModel { Items = [], Total = 0 }); } + + PaginationHelper.ConvertSkipTakeToPaging(skip, take, out var pageIndex, out var pageSize); + IQuery? customFilter = + sinceDate.HasValue ? Query().Where(x => x.CreateDate >= sinceDate) : null; + + PagedModel result = GetItemsByEntityInner( + keyToIdAttempt.Result, + pageIndex, + pageSize, + orderDirection, + auditTypeFilter, + customFilter); + return Task.FromResult(result); } - /// - /// Returns paged items in the audit trail for a given entity - /// - /// - /// - /// - /// - /// - /// By default this will always be ordered descending (newest first) - /// - /// - /// Since we currently do not have enum support with our expression parser, we cannot query on AuditType in the query - /// or the custom filter - /// so we need to do that here - /// - /// - /// Optional filter to be applied - /// - /// + /// + public Task> GetItemsByEntityAsync( + int entityId, + int skip, + int take, + Direction orderDirection = Direction.Descending, + AuditType[]? auditTypeFilter = null, + IQuery? customFilter = null) + { + ArgumentOutOfRangeException.ThrowIfNegative(skip); + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(take); + + if (entityId is Constants.System.Root or <= 0) + { + return Task.FromResult(new PagedModel { Items = [], Total = 0 }); + } + + PaginationHelper.ConvertSkipTakeToPaging(skip, take, out var pageIndex, out var pageSize); + PagedModel result = GetItemsByEntityInner( + entityId, + pageIndex, + pageSize, + orderDirection, + auditTypeFilter, + customFilter); + + return Task.FromResult(result); + } + + /// + [Obsolete("Use GetItemsByEntityAsync() instead. Scheduled for removal in Umbraco 19.")] public IEnumerable GetPagedItemsByEntity( int entityId, long pageIndex, @@ -131,49 +204,69 @@ public sealed class AuditService : RepositoryService, IAuditService AuditType[]? auditTypeFilter = null, IQuery? customFilter = null) { - if (pageIndex < 0) + ArgumentOutOfRangeException.ThrowIfNegative(pageIndex); + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(pageSize); + + if (entityId is Constants.System.Root or <= 0) { - throw new ArgumentOutOfRangeException(nameof(pageIndex)); + totalRecords = 0L; + return []; } - if (pageSize <= 0) - { - throw new ArgumentOutOfRangeException(nameof(pageSize)); - } + PagedModel result = GetItemsByEntityInner( + entityId, + pageIndex, + pageSize, + orderDirection, + auditTypeFilter, + customFilter); + totalRecords = result.Total; - if (entityId == Constants.System.Root || entityId <= 0) - { - totalRecords = 0; - return Enumerable.Empty(); - } - - using (ScopeProvider.CreateCoreScope(autoComplete: true)) - { - IQuery query = Query().Where(x => x.Id == entityId); - - return _auditRepository.GetPagedResultsByQuery(query, pageIndex, pageSize, out totalRecords, orderDirection, auditTypeFilter, customFilter); - } + return result.Items; } - /// - /// Returns paged items in the audit trail for a given user - /// - /// - /// - /// - /// - /// - /// By default this will always be ordered descending (newest first) - /// - /// - /// Since we currently do not have enum support with our expression parser, we cannot query on AuditType in the query - /// or the custom filter - /// so we need to do that here - /// - /// - /// Optional filter to be applied - /// - /// + /// + [Obsolete("Use GetItemsByEntityAsync() instead. Scheduled for removal in Umbraco 19.")] + public IEnumerable GetLogs(int objectId) + { + PagedModel result = GetItemsByEntityInner(objectId, 0, int.MaxValue, Direction.Ascending); + return result.Items; + } + + /// + public async Task> GetPagedItemsByUserAsync( + Guid userKey, + int skip, + int take, + Direction orderDirection = Direction.Descending, + AuditType[]? auditTypeFilter = null, + DateTime? sinceDate = null) + { + ArgumentOutOfRangeException.ThrowIfNegative(skip); + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(take); + + IUser? user = await _userService.GetAsync(userKey); + if (user is null) + { + return new PagedModel(); + } + + PaginationHelper.ConvertSkipTakeToPaging(skip, take, out var pageIndex, out var pageSize); + IQuery? customFilter = + sinceDate.HasValue ? Query().Where(x => x.CreateDate >= sinceDate) : null; + + PagedModel result = GetItemsByUserInner( + user.Id, + pageIndex, + pageSize, + orderDirection, + auditTypeFilter, + customFilter); + return result; + } + + /// + [Obsolete("Use GetPagedItemsByUserAsync() instead. Scheduled for removal in Umbraco 19.")] public IEnumerable GetPagedItemsByUser( int userId, long pageIndex, @@ -183,100 +276,42 @@ public sealed class AuditService : RepositoryService, IAuditService AuditType[]? auditTypeFilter = null, IQuery? customFilter = null) { - if (pageIndex < 0) + ArgumentOutOfRangeException.ThrowIfNegative(pageIndex); + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(pageSize); + + if (userId is Constants.System.Root or <= 0) { - throw new ArgumentOutOfRangeException(nameof(pageIndex)); + totalRecords = 0L; + return []; } - if (pageSize <= 0) - { - throw new ArgumentOutOfRangeException(nameof(pageSize)); - } + PagedModel items = GetItemsByUserInner( + userId, + pageIndex, + pageSize, + orderDirection, + auditTypeFilter, + customFilter); + totalRecords = items.Total; - if (userId < Constants.Security.SuperUserId) - { - totalRecords = 0; - return Enumerable.Empty(); - } - - using (ScopeProvider.CreateCoreScope(autoComplete: true)) - { - IQuery query = Query().Where(x => x.UserId == userId); - - return _auditRepository.GetPagedResultsByQuery(query, pageIndex, pageSize, out totalRecords, orderDirection, auditTypeFilter, customFilter); - } + return items.Items; } - public Task> GetItemsByKeyAsync( - Guid entityKey, - UmbracoObjectTypes entityType, - int skip, - int take, - Direction orderDirection = Direction.Descending, - DateTimeOffset? sinceDate = null, - AuditType[]? auditTypeFilter = null) - { - if (skip < 0) - { - throw new ArgumentOutOfRangeException(nameof(skip)); - } - - if (take <= 0) - { - throw new ArgumentOutOfRangeException(nameof(take)); - } - - Attempt keyToIdAttempt = _entityService.GetId(entityKey, entityType); - if (keyToIdAttempt.Success is false) - { - return Task.FromResult(new PagedModel { Items = Enumerable.Empty(), Total = 0 }); - } - - using (ScopeProvider.CreateCoreScope(autoComplete: true)) - { - IQuery query = Query().Where(x => x.Id == keyToIdAttempt.Result); - IQuery? customFilter = sinceDate.HasValue ? Query().Where(x => x.CreateDate >= sinceDate.Value.LocalDateTime) : null; - PaginationHelper.ConvertSkipTakeToPaging(skip, take, out var pageNumber, out var pageSize); - - IEnumerable auditItems = _auditRepository.GetPagedResultsByQuery(query, pageNumber, pageSize, out var totalRecords, orderDirection, auditTypeFilter, customFilter); - return Task.FromResult(new PagedModel { Items = auditItems, Total = totalRecords }); - } - } - - public async Task> GetPagedItemsByUserAsync( - Guid userKey, - int skip, - int take, - Direction orderDirection = Direction.Descending, - AuditType[]? auditTypeFilter = null, - DateTime? sinceDate = null) + /// + [Obsolete("Use GetPagedItemsByUserAsync() instead. Scheduled for removal in Umbraco 19.")] + public IEnumerable GetUserLogs(int userId, AuditType type, DateTime? sinceDate = null) { - if (skip < 0) - { - throw new ArgumentOutOfRangeException(nameof(skip)); - } - - if (take <= 0) - { - throw new ArgumentOutOfRangeException(nameof(take)); - } - - IUser? user = await _userService.GetAsync(userKey); - - if (user is null) - { - return new PagedModel(); - } - - using (ScopeProvider.CreateCoreScope(autoComplete: true)) - { - IQuery query = Query().Where(x => x.UserId == user.Id); - IQuery? customFilter = sinceDate.HasValue ? Query().Where(x => x.CreateDate >= sinceDate) : null; - PaginationHelper.ConvertSkipTakeToPaging(skip, take, out var pageNumber, out var pageSize); - - IEnumerable auditItems = _auditRepository.GetPagedResultsByQuery(query, pageNumber, pageSize, out var totalRecords, orderDirection, auditTypeFilter, customFilter); - return new PagedModel { Items = auditItems, Total = totalRecords }; - } + IQuery? customFilter = sinceDate.HasValue + ? Query().Where(x => x.AuditType == type && x.CreateDate >= sinceDate) + : null; + PagedModel result = GetItemsByUserInner( + userId, + 0, + int.MaxValue, + Direction.Ascending, + [type], + customFilter); + return result.Items; } /// @@ -305,4 +340,97 @@ public sealed class AuditService : RepositoryService, IAuditService eventType, eventDetails).GetAwaiter().GetResult(); } + + /// + public Task CleanLogsAsync(int maximumAgeOfLogsInMinutes) + { + CleanLogsInner(maximumAgeOfLogsInMinutes); + return Task.CompletedTask; + } + + /// + [Obsolete("Use CleanLogsAsync() instead. Scheduled for removal in Umbraco 19.")] + public void CleanLogs(int maximumAgeOfLogsInMinutes) + => CleanLogsInner(maximumAgeOfLogsInMinutes); + + private Attempt AddInner( + AuditType type, + int userId, + int objectId, + string? entityType, + string? comment = null, + string? parameters = null) + { + using ICoreScope scope = ScopeProvider.CreateCoreScope(); + _auditRepository.Save(new AuditItem(objectId, type, userId, entityType, comment, parameters)); + scope.Complete(); + + return Attempt.Succeed(AuditLogOperationStatus.Success); + } + + private PagedModel GetItemsInner( + IQuery? query, + long pageIndex, + int pageSize, + Direction orderDirection = Direction.Descending, + AuditType[]? auditTypeFilter = null, + IQuery? customFilter = null) + { + using (ScopeProvider.CreateCoreScope(autoComplete: true)) + { + IEnumerable auditItems = _auditRepository.GetPagedResultsByQuery( + query ?? Query(), + pageIndex, + pageSize, + out var totalRecords, + orderDirection, + auditTypeFilter, + customFilter); + return new PagedModel { Items = auditItems, Total = totalRecords }; + } + } + + private PagedModel GetItemsByEntityInner( + int entityId, + long pageIndex, + int pageSize, + Direction orderDirection = Direction.Descending, + AuditType[]? auditTypeFilter = null, + IQuery? customFilter = null) + { + IQuery query = Query().Where(x => x.Id == entityId); + + PagedModel result = GetItemsInner( + query, + pageIndex, + pageSize, + orderDirection, + auditTypeFilter, + customFilter); + return result; + } + + private PagedModel GetItemsByUserInner( + int userId, + long pageIndex, + int pageSize, + Direction orderDirection = Direction.Descending, + AuditType[]? auditTypeFilter = null, + IQuery? customFilter = null) + { + if (userId is < Constants.Security.SuperUserId) + { + return new PagedModel { Items = [], Total = 0 }; + } + + IQuery query = Query().Where(x => x.UserId == userId); + return GetItemsInner(query, pageIndex, pageSize, orderDirection, auditTypeFilter, customFilter); + } + + private void CleanLogsInner(int maximumAgeOfLogsInMinutes) + { + using ICoreScope scope = ScopeProvider.CreateCoreScope(); + _auditRepository.CleanLogs(maximumAgeOfLogsInMinutes); + scope.Complete(); + } } diff --git a/src/Umbraco.Core/Services/IAuditService.cs b/src/Umbraco.Core/Services/IAuditService.cs index 435b95bb17..eaca8dde69 100644 --- a/src/Umbraco.Core/Services/IAuditService.cs +++ b/src/Umbraco.Core/Services/IAuditService.cs @@ -1,5 +1,6 @@ using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Persistence.Querying; +using Umbraco.Cms.Core.Services.OperationStatus; namespace Umbraco.Cms.Core.Services; @@ -8,91 +9,71 @@ namespace Umbraco.Cms.Core.Services; /// public interface IAuditService : IService { + /// + /// Adds an audit entry. + /// + /// The type of the audit. + /// The key of the user triggering the event. + /// The identifier of the affected object. + /// The entity type of the affected object. + /// The comment associated with the audit entry. + /// The parameters associated with the audit entry. + /// Result of the add audit log operation. + public Task> AddAsync( + AuditType type, + Guid userKey, + int objectId, + string? entityType, + string? comment = null, + string? parameters = null) => throw new NotImplementedException(); + + [Obsolete("Use AddAsync() instead. Scheduled for removal in Umbraco 19.")] void Add(AuditType type, int userId, int objectId, string? entityType, string comment, string? parameters = null); - IEnumerable GetLogs(int objectId); - - IEnumerable GetUserLogs(int userId, AuditType type, DateTime? sinceDate = null); - - IEnumerable GetLogs(AuditType type, DateTime? sinceDate = null); - - void CleanLogs(int maximumAgeOfLogsInMinutes); - /// - /// Returns paged items in the audit trail for a given entity + /// Returns paged items in the audit trail. /// - /// - /// - /// - /// + /// The number of audit trail entries to skip. + /// The number of audit trail entries to take. /// - /// By default this will always be ordered descending (newest first) - /// - /// - /// Since we currently do not have enum support with our expression parser, we cannot query on AuditType in the query - /// or the custom filter - /// so we need to do that here - /// - /// - /// Optional filter to be applied - /// - /// - IEnumerable GetPagedItemsByEntity( - int entityId, - long pageIndex, - int pageSize, - out long totalRecords, - Direction orderDirection = Direction.Descending, - AuditType[]? auditTypeFilter = null, - IQuery? customFilter = null); - - /// - /// Returns paged items in the audit trail for a given user - /// - /// - /// - /// - /// - /// - /// By default this will always be ordered descending (newest first) - /// - /// - /// Since we currently do not have enum support with our expression parser, we cannot query on AuditType in the query - /// or the custom filter - /// so we need to do that here - /// - /// - /// Optional filter to be applied - /// - /// - IEnumerable GetPagedItemsByUser( - int userId, - long pageIndex, - int pageSize, - out long totalRecords, - Direction orderDirection = Direction.Descending, - AuditType[]? auditTypeFilter = null, - IQuery? customFilter = null); - - /// - /// Returns paged items in the audit trail for a given entity - /// - /// The key of the entity - /// The entity type - /// The amount of audit trail entries to skip - /// The amount of audit trail entries to take - /// - /// By default this will always be ordered descending (newest first) - /// - /// - /// Since we currently do not have enum support with our expression parser, we cannot query on AuditType in the query - /// or the custom filter - /// so we need to do that here + /// By default, this will always be ordered descending (newest first). /// /// /// If populated, will only return entries after this time. /// - /// + /// + /// Since we currently do not have enum support with our expression parser, we cannot query on AuditType in the query + /// or the custom filter, so we need to do that here. + /// + /// The paged audit logs. + public Task> GetItemsAsync( + int skip, + int take, + Direction orderDirection = Direction.Descending, + DateTimeOffset? sinceDate = null, + AuditType[]? auditTypeFilter = null) => throw new NotImplementedException(); + + [Obsolete("Use GetItemsAsync() instead. Scheduled for removal in Umbraco 19.")] + IEnumerable GetLogs(AuditType type, DateTime? sinceDate = null); + + /// + /// Returns paged items in the audit trail for a given entity. + /// + /// The key of the entity. + /// The entity type. + /// The number of audit trail entries to skip. + /// The number of audit trail entries to take. + /// + /// By default, this will always be ordered descending (newest first). + /// + /// + /// If populated, will only return entries after this time. + /// + /// + /// Since we currently do not have enum support with our expression parser, we cannot query on AuditType in the query + /// or the custom filter, so we need to do that here. + /// + /// The paged items in the audit trail for the specified entity. Task> GetItemsByKeyAsync( Guid entityKey, UmbracoObjectTypes entityType, @@ -103,21 +84,76 @@ public interface IAuditService : IService AuditType[]? auditTypeFilter = null) => throw new NotImplementedException(); /// - /// Returns paged items in the audit trail for a given user + /// Returns paged items in the audit trail for a given entity. /// - /// - /// - /// + /// The identifier of the entity. + /// The number of audit trail entries to skip. + /// The number of audit trail entries to take. /// - /// By default this will always be ordered descending (newest first) + /// By default, this will always be ordered descending (newest first). /// /// /// Since we currently do not have enum support with our expression parser, we cannot query on AuditType in the query - /// or the custom filter - /// so we need to do that here + /// or the custom filter, so we need to do that here. /// - /// - /// + /// + /// Optional filter to be applied. + /// + /// The paged items in the audit trail for the specified entity. + public Task> GetItemsByEntityAsync( + int entityId, + int skip, + int take, + Direction orderDirection = Direction.Descending, + AuditType[]? auditTypeFilter = null, + IQuery? customFilter = null) => throw new NotImplementedException(); + + /// + /// Returns paged items in the audit trail for a given entity. + /// + /// The identifier of the entity. + /// The index of tha page (pagination). + /// The number of results to return. + /// The total number of records. + /// + /// By default, this will always be ordered descending (newest first). + /// + /// + /// Since we currently do not have enum support with our expression parser, we cannot query on AuditType in the query + /// or the custom filter, so we need to do that here. + /// + /// + /// Optional filter to be applied. + /// + /// The paged items in the audit trail for the specified entity. + [Obsolete("Use GetItemsByEntityAsync() instead. Scheduled for removal in Umbraco 19.")] + IEnumerable GetPagedItemsByEntity( + int entityId, + long pageIndex, + int pageSize, + out long totalRecords, + Direction orderDirection = Direction.Descending, + AuditType[]? auditTypeFilter = null, + IQuery? customFilter = null); + + [Obsolete("Use GetItemsByEntityAsync() instead. Scheduled for removal in Umbraco 19.")] + IEnumerable GetLogs(int objectId); + + /// + /// Returns paged items in the audit trail for a given user. + /// + /// The key of the user. + /// The number of audit trail entries to skip. + /// The number of audit trail entries to take. + /// + /// By default, this will always be ordered descending (newest first). + /// + /// + /// Since we currently do not have enum support with our expression parser, we cannot query on AuditType in the query + /// or the custom filter, so we need to do that here. + /// + /// The date to filter the audit entries. + /// The paged items in the audit trail for the specified user. Task> GetPagedItemsByUserAsync( Guid userKey, int skip, @@ -126,6 +162,38 @@ public interface IAuditService : IService AuditType[]? auditTypeFilter = null, DateTime? sinceDate = null) => throw new NotImplementedException(); + /// + /// Returns paged items in the audit trail for a given user. + /// + /// + /// + /// + /// + /// + /// By default this will always be ordered descending (newest first). + /// + /// + /// Since we currently do not have enum support with our expression parser, we cannot query on AuditType in the query + /// or the custom filter + /// so we need to do that here. + /// + /// + /// Optional filter to be applied. + /// + /// + [Obsolete("Use GetPagedItemsByUserAsync() instead. Scheduled for removal in Umbraco 19.")] + IEnumerable GetPagedItemsByUser( + int userId, + long pageIndex, + int pageSize, + out long totalRecords, + Direction orderDirection = Direction.Descending, + AuditType[]? auditTypeFilter = null, + IQuery? customFilter = null); + + [Obsolete("Use GetPagedItemsByUserAsync() instead. Scheduled for removal in Umbraco 19.")] + IEnumerable GetUserLogs(int userId, AuditType type, DateTime? sinceDate = null); + /// /// Writes an audit entry for an audited event. /// @@ -144,7 +212,8 @@ public interface IAuditService : IService /// /// /// Free-form details about the audited event. - [Obsolete("Use AuditEntryService.WriteAsync() instead. Scheduled for removal in Umbraco 19.")] + /// The created audit entry. + [Obsolete("Use IAuditEntryService.WriteAsync() instead. Scheduled for removal in Umbraco 19.")] IAuditEntry Write( int performingUserId, string perfomingDetails, @@ -154,4 +223,14 @@ public interface IAuditService : IService string affectedDetails, string eventType, string eventDetails); + + /// + /// Cleans the audit logs older than the specified maximum age. + /// + /// The maximum age of logs in minutes. + /// Task representing the asynchronous operation. + public Task CleanLogsAsync(int maximumAgeOfLogsInMinutes) => throw new NotImplementedException(); + + [Obsolete("Use CleanLogsAsync() instead. Scheduled for removal in Umbraco 19.")] + void CleanLogs(int maximumAgeOfLogsInMinutes); } diff --git a/src/Umbraco.Infrastructure/BackgroundJobs/Jobs/LogScrubberJob.cs b/src/Umbraco.Infrastructure/BackgroundJobs/Jobs/LogScrubberJob.cs index 3fcbe8d3d6..725c66c2f8 100644 --- a/src/Umbraco.Infrastructure/BackgroundJobs/Jobs/LogScrubberJob.cs +++ b/src/Umbraco.Infrastructure/BackgroundJobs/Jobs/LogScrubberJob.cs @@ -21,18 +21,17 @@ namespace Umbraco.Cms.Infrastructure.BackgroundJobs.Jobs; /// public class LogScrubberJob : IRecurringBackgroundJob { - public TimeSpan Period { get => TimeSpan.FromHours(4); } - - // No-op event as the period never changes on this job - public event EventHandler PeriodChanged { add { } remove { } } - - private readonly IAuditService _auditService; private readonly ILogger _logger; private readonly IProfilingLogger _profilingLogger; private readonly ICoreScopeProvider _scopeProvider; private LoggingSettings _settings; + public TimeSpan Period => TimeSpan.FromHours(4); + + // No-op event as the period never changes on this job + public event EventHandler PeriodChanged { add { } remove { } } + /// /// Initializes a new instance of the class. /// @@ -48,7 +47,6 @@ public class LogScrubberJob : IRecurringBackgroundJob ILogger logger, IProfilingLogger profilingLogger) { - _auditService = auditService; _settings = settings.CurrentValue; _scopeProvider = scopeProvider; @@ -57,17 +55,14 @@ public class LogScrubberJob : IRecurringBackgroundJob settings.OnChange(x => _settings = x); } - public Task RunJobAsync() + public async Task RunJobAsync() { - // Ensure we use an explicit scope since we are running on a background thread. using (ICoreScope scope = _scopeProvider.CreateCoreScope()) using (_profilingLogger.DebugDuration("Log scrubbing executing", "Log scrubbing complete")) { - _auditService.CleanLogs((int)_settings.MaxLogAge.TotalMinutes); + await _auditService.CleanLogsAsync((int)_settings.MaxLogAge.TotalMinutes); _ = scope.Complete(); } - - return Task.CompletedTask; } } diff --git a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs index f593bab5c2..97478c435a 100644 --- a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs +++ b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs @@ -338,11 +338,11 @@ public static partial class UmbracoBuilderExtensions // add handlers for building content relations builder - .AddNotificationHandler() + .AddNotificationAsyncHandler() .AddNotificationHandler() - .AddNotificationHandler() + .AddNotificationAsyncHandler() .AddNotificationHandler() - .AddNotificationHandler(); + .AddNotificationAsyncHandler(); // add notification handlers for property editors builder diff --git a/src/Umbraco.Infrastructure/Events/RelateOnTrashNotificationHandler.cs b/src/Umbraco.Infrastructure/Events/RelateOnTrashNotificationHandler.cs index 1c2064a78f..266857d088 100644 --- a/src/Umbraco.Infrastructure/Events/RelateOnTrashNotificationHandler.cs +++ b/src/Umbraco.Infrastructure/Events/RelateOnTrashNotificationHandler.cs @@ -2,6 +2,8 @@ // See LICENSE for more details. using System.Globalization; +using Microsoft.Extensions.DependencyInjection; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Notifications; using Umbraco.Cms.Core.Scoping; @@ -15,14 +17,17 @@ namespace Umbraco.Cms.Core.Events; public sealed class RelateOnTrashNotificationHandler : INotificationHandler, INotificationHandler, + INotificationAsyncHandler, INotificationHandler, - INotificationHandler + INotificationHandler, + INotificationAsyncHandler { private readonly IAuditService _auditService; private readonly IEntityService _entityService; private readonly IRelationService _relationService; private readonly ICoreScopeProvider _scopeProvider; private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor; + private readonly IUserIdKeyResolver _userIdKeyResolver; private readonly ILocalizedTextService _textService; public RelateOnTrashNotificationHandler( @@ -31,7 +36,8 @@ public sealed class RelateOnTrashNotificationHandler : ILocalizedTextService textService, IAuditService auditService, IScopeProvider scopeProvider, - IBackOfficeSecurityAccessor backOfficeSecurityAccessor) + IBackOfficeSecurityAccessor backOfficeSecurityAccessor, + IUserIdKeyResolver userIdKeyResolver) { _relationService = relationService; _entityService = entityService; @@ -39,6 +45,26 @@ public sealed class RelateOnTrashNotificationHandler : _auditService = auditService; _scopeProvider = scopeProvider; _backOfficeSecurityAccessor = backOfficeSecurityAccessor; + _userIdKeyResolver = userIdKeyResolver; + } + + [Obsolete("Use the non-obsolete constructor instead. Scheduled for removal in V19.")] + public RelateOnTrashNotificationHandler( + IRelationService relationService, + IEntityService entityService, + ILocalizedTextService textService, + IAuditService auditService, + IScopeProvider scopeProvider, + IBackOfficeSecurityAccessor backOfficeSecurityAccessor) + : this( + relationService, + entityService, + textService, + auditService, + scopeProvider, + backOfficeSecurityAccessor, + StaticServiceProvider.Instance.GetRequiredService()) + { } public void Handle(ContentMovedNotification notification) @@ -57,7 +83,8 @@ public sealed class RelateOnTrashNotificationHandler : } } - public void Handle(ContentMovedToRecycleBinNotification notification) + /// + public async Task HandleAsync(ContentMovedToRecycleBinNotification notification, CancellationToken cancellationToken) { using (ICoreScope scope = _scopeProvider.CreateCoreScope()) { @@ -91,9 +118,9 @@ public sealed class RelateOnTrashNotificationHandler : new Relation(originalParentId, item.Entity.Id, relationType); _relationService.Save(relation); - _auditService.Add( + await _auditService.AddAsync( AuditType.Delete, - _backOfficeSecurityAccessor.BackOfficeSecurity?.CurrentUser?.Id ?? item.Entity.WriterId, + _backOfficeSecurityAccessor.BackOfficeSecurity?.CurrentUser?.Key ?? await _userIdKeyResolver.GetAsync(item.Entity.WriterId), item.Entity.Id, UmbracoObjectTypes.Document.GetName(), string.Format(_textService.Localize("recycleBin", "contentTrashed"), item.Entity.Id, originalParentId)); @@ -104,6 +131,10 @@ public sealed class RelateOnTrashNotificationHandler : } } + [Obsolete("Use the INotificationAsyncHandler.HandleAsync implementation instead. Scheduled for removal in V19.")] + public void Handle(ContentMovedToRecycleBinNotification notification) + => HandleAsync(notification, CancellationToken.None).GetAwaiter().GetResult(); + public void Handle(MediaMovedNotification notification) { foreach (MoveEventInfo item in notification.MoveInfoCollection.Where(x => @@ -119,7 +150,8 @@ public sealed class RelateOnTrashNotificationHandler : } } - public void Handle(MediaMovedToRecycleBinNotification notification) + /// + public async Task HandleAsync(MediaMovedToRecycleBinNotification notification, CancellationToken cancellationToken) { using (ICoreScope scope = _scopeProvider.CreateCoreScope()) { @@ -151,9 +183,9 @@ public sealed class RelateOnTrashNotificationHandler : _relationService.GetByParentAndChildId(originalParentId, item.Entity.Id, relationType) ?? new Relation(originalParentId, item.Entity.Id, relationType); _relationService.Save(relation); - _auditService.Add( + await _auditService.AddAsync( AuditType.Delete, - _backOfficeSecurityAccessor.BackOfficeSecurity?.CurrentUser?.Id ?? item.Entity.WriterId, + _backOfficeSecurityAccessor.BackOfficeSecurity?.CurrentUser?.Key ?? await _userIdKeyResolver.GetAsync(item.Entity.WriterId), item.Entity.Id, UmbracoObjectTypes.Media.GetName(), string.Format(_textService.Localize("recycleBin", "mediaTrashed"), item.Entity.Id, originalParentId)); @@ -163,4 +195,8 @@ public sealed class RelateOnTrashNotificationHandler : scope.Complete(); } } + + [Obsolete("Use the INotificationAsyncHandler.HandleAsync implementation instead. Scheduled for removal in V19.")] + public void Handle(MediaMovedToRecycleBinNotification notification) + => HandleAsync(notification, CancellationToken.None).GetAwaiter().GetResult(); } diff --git a/src/Umbraco.Infrastructure/Services/Implement/PackagingService.cs b/src/Umbraco.Infrastructure/Services/Implement/PackagingService.cs index 3d46df8f01..6ce2ee9d02 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/PackagingService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/PackagingService.cs @@ -8,6 +8,7 @@ using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Extensions; using Umbraco.Cms.Core.Manifest; using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Models.Membership; using Umbraco.Cms.Core.Models.Packaging; using Umbraco.Cms.Core.Notifications; using Umbraco.Cms.Core.Packaging; @@ -82,7 +83,13 @@ public class PackagingService : IPackagingService InstallationSummary summary = _packageInstallation.InstallPackageData(compiledPackage, userId, out _); - _auditService.Add(AuditType.PackagerInstall, userId, -1, "Package", $"Package data installed for package '{compiledPackage.Name}'."); + IUser? user = _userService.GetUserById(userId); + _auditService.AddAsync( + AuditType.PackagerInstall, + user?.Key ?? Constants.Security.SuperUserKey, + -1, + "Package", + $"Package data installed for package '{compiledPackage.Name}'.").GetAwaiter().GetResult(); // trigger the ImportedPackage event _eventAggregator.Publish(new ImportedPackageNotification(summary).WithStateFrom(importingPackageNotification)); @@ -115,8 +122,12 @@ public class PackagingService : IPackagingService return Attempt.FailWithStatus(PackageOperationStatus.NotFound, null); } - int currentUserId = (await _userService.GetRequiredUserAsync(userKey)).Id; - _auditService.Add(AuditType.Delete, currentUserId, -1, "Package", $"Created package '{package.Name}' deleted. Package key: {key}"); + Attempt result = await _auditService.AddAsync(AuditType.Delete, userKey, -1, "Package", $"Created package '{package.Name}' deleted. Package key: {key}"); + if (result is { Success: false, Result: AuditLogOperationStatus.UserNotFound }) + { + throw new InvalidOperationException($"Could not find user with key: {userKey}"); + } + _createdPackages.Delete(package.Id); scope.Complete(); @@ -163,8 +174,11 @@ public class PackagingService : IPackagingService return Attempt.FailWithStatus(PackageOperationStatus.DuplicateItemName, package); } - int currentUserId = (await _userService.GetRequiredUserAsync(userKey)).Id; - _auditService.Add(AuditType.New, currentUserId, -1, "Package", $"Created package '{package.Name}' created. Package key: {package.PackageId}"); + Attempt result = await _auditService.AddAsync(AuditType.New, userKey, -1, "Package", $"Created package '{package.Name}' created. Package key: {package.PackageId}"); + if (result is { Success: false, Result: AuditLogOperationStatus.UserNotFound }) + { + throw new InvalidOperationException($"Could not find user with key: {userKey}"); + } scope.Complete(); @@ -180,8 +194,11 @@ public class PackagingService : IPackagingService return Attempt.FailWithStatus(PackageOperationStatus.NotFound, package); } - int currentUserId = (await _userService.GetRequiredUserAsync(userKey)).Id; - _auditService.Add(AuditType.New, currentUserId, -1, "Package", $"Created package '{package.Name}' updated. Package key: {package.PackageId}"); + Attempt result = await _auditService.AddAsync(AuditType.New, userKey, -1, "Package", $"Created package '{package.Name}' updated. Package key: {package.PackageId}"); + if (result is { Success: false, Result: AuditLogOperationStatus.UserNotFound }) + { + throw new InvalidOperationException($"Could not find user with key: {userKey}"); + } scope.Complete(); diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentServiceTests.cs index 242be71557..0c883614ef 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentServiceTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentServiceTests.cs @@ -1018,7 +1018,7 @@ internal sealed class ContentServiceTests : UmbracoIntegrationTestWithContent } [Test] - public void Can_Publish_Content_Variation_And_Detect_Changed_Cultures() + public async Task Can_Publish_Content_Variation_And_Detect_Changed_Cultures() { CreateEnglishAndFrenchDocumentType(out var langUk, out var langFr, out var contentType); @@ -1028,7 +1028,7 @@ internal sealed class ContentServiceTests : UmbracoIntegrationTestWithContent var published = ContentService.Publish(content, new[] { langFr.IsoCode }); // audit log will only show that french was published - var lastLog = AuditService.GetLogs(content.Id).Last(); + var lastLog = (await AuditService.GetItemsByEntityAsync(content.Id, 0, 1)).Items.First(); Assert.AreEqual("Published languages: fr-FR", lastLog.Comment); // re-get @@ -1038,7 +1038,7 @@ internal sealed class ContentServiceTests : UmbracoIntegrationTestWithContent published = ContentService.Publish(content, new[] { langUk.IsoCode }); // audit log will only show that english was published - lastLog = AuditService.GetLogs(content.Id).Last(); + lastLog = (await AuditService.GetItemsByEntityAsync(content.Id, 0, 1)).Items.First(); Assert.AreEqual("Published languages: en-GB", lastLog.Comment); } @@ -1075,7 +1075,7 @@ internal sealed class ContentServiceTests : UmbracoIntegrationTestWithContent var unpublished = ContentService.Unpublish(content, langFr.IsoCode); // audit log will only show that french was unpublished - var lastLog = AuditService.GetLogs(content.Id).Last(); + var lastLog = (await AuditService.GetItemsByEntityAsync(content.Id, 0, 1)).Items.First(); Assert.AreEqual("Unpublished languages: fr-FR", lastLog.Comment); // re-get @@ -1084,7 +1084,7 @@ internal sealed class ContentServiceTests : UmbracoIntegrationTestWithContent unpublished = ContentService.Unpublish(content, langGb.IsoCode); // audit log will only show that english was published - var logs = AuditService.GetLogs(content.Id).ToList(); + var logs = (await AuditService.GetItemsByEntityAsync(content.Id, 0, int.MaxValue, Direction.Ascending)).Items.ToList(); Assert.AreEqual("Unpublished languages: en-GB", logs[^2].Comment); Assert.AreEqual("Unpublished (mandatory language unpublished)", logs[^1].Comment); } diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/AuditServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/AuditServiceTests.cs index 248fa38a05..21ece28328 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/AuditServiceTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/AuditServiceTests.cs @@ -1,13 +1,12 @@ // Copyright (c) Umbraco. // See LICENSE for more details. -using System.Linq; using Microsoft.Extensions.DependencyInjection; using NUnit.Framework; +using Umbraco.Cms.Core; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Services.Implement; -using Umbraco.Cms.Tests.Common.Builders; using Umbraco.Cms.Tests.Common.Testing; using Umbraco.Cms.Tests.Integration.Testing; @@ -18,22 +17,26 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services; internal sealed class AuditServiceTests : UmbracoIntegrationTest { [Test] - public void GetUserLogs() + public async Task GetUserLogs() { var sut = (AuditService)Services.GetRequiredService(); var eventDateUtc = DateTime.UtcNow.AddDays(-1); - var numberOfEntries = 10; + const int numberOfEntries = 10; for (var i = 0; i < numberOfEntries; i++) { eventDateUtc = eventDateUtc.AddMinutes(1); - sut.Add(AuditType.Unpublish, -1, 33, string.Empty, "blah"); + await sut.AddAsync(AuditType.Unpublish, Constants.Security.SuperUserKey, 33, string.Empty, "blah"); } - sut.Add(AuditType.Publish, -1, 33, string.Empty, "blah"); + await sut.AddAsync(AuditType.Publish, Constants.Security.SuperUserKey, 33, string.Empty, "blah"); - var logs = sut.GetUserLogs(-1, AuditType.Unpublish).ToArray(); + var logs = (await sut.GetPagedItemsByUserAsync( + Constants.Security.SuperUserKey, + 0, + int.MaxValue, + auditTypeFilter: [AuditType.Unpublish])).Items.ToArray(); Assert.Multiple(() => { diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.cs index 683ee8134b..4fca28f2ff 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.cs @@ -25,7 +25,7 @@ public partial class ContentEditingServiceTests : ContentEditingServiceTestsBase } protected override void CustomTestSetup(IUmbracoBuilder builder) - => builder.AddNotificationHandler(); + => builder.AddNotificationAsyncHandler(); private ITemplateService TemplateService => GetRequiredService(); diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/AuditServiceTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/AuditServiceTests.cs new file mode 100644 index 0000000000..d5fdaa7537 --- /dev/null +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/AuditServiceTests.cs @@ -0,0 +1,232 @@ +using System.Data; +using AutoFixture; +using Microsoft.Extensions.Logging; +using Moq; +using NUnit.Framework; +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Models.Membership; +using Umbraco.Cms.Core.Persistence.Querying; +using Umbraco.Cms.Core.Persistence.Repositories; +using Umbraco.Cms.Core.Scoping; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Services.Implement; +using Umbraco.Cms.Core.Services.OperationStatus; + +namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Services; + +[TestFixture] +public class AuditServiceTests +{ + private IAuditService _auditService; + private Mock _scopeProviderMock; + private Mock _auditRepositoryMock; + private Mock _entityServiceMock; + private Mock _userServiceMock; + + [SetUp] + public void Setup() + { + _scopeProviderMock = new Mock(MockBehavior.Strict); + _auditRepositoryMock = new Mock(MockBehavior.Strict); + _entityServiceMock = new Mock(MockBehavior.Strict); + _userServiceMock = new Mock(MockBehavior.Strict); + + _auditService = new AuditService( + _scopeProviderMock.Object, + Mock.Of(MockBehavior.Strict), + Mock.Of(MockBehavior.Strict), + _auditRepositoryMock.Object, + _userServiceMock.Object, + _entityServiceMock.Object); + } + + [TestCase(AuditType.Publish, 33, null, null, null)] + [TestCase(AuditType.Copy, 1, "entityType", "comment", "parameters")] + public async Task AddAsync_Calls_Repository_With_Correct_Values(AuditType type, int objectId, string? entityType, string? comment, string? parameters = null) + { + SetupScopeProviderMock(); + + _auditRepositoryMock.Setup(x => x.Save(It.IsAny())) + .Callback(item => + { + Assert.AreEqual(type, item.AuditType); + Assert.AreEqual(Constants.Security.SuperUserId, item.UserId); + Assert.AreEqual(objectId, item.Id); + Assert.AreEqual(entityType, item.EntityType); + Assert.AreEqual(comment, item.Comment); + Assert.AreEqual(parameters, item.Parameters); + }); + + Mock mockUser = new Mock(); + mockUser.Setup(x => x.Id).Returns(Constants.Security.SuperUserId); + + _userServiceMock.Setup(x => x.GetAsync(Constants.Security.SuperUserKey)).ReturnsAsync(mockUser.Object); + + var result = await _auditService.AddAsync( + type, + Constants.Security.SuperUserKey, + objectId, + entityType, + comment, + parameters); + _auditRepositoryMock.Verify(x => x.Save(It.IsAny()), Times.Once); + Assert.IsTrue(result.Success); + Assert.AreEqual(AuditLogOperationStatus.Success, result.Result); + } + + [Test] + public async Task AddAsync_Does_Not_Succeed_When_Non_Existing_User_Is_Provided() + { + _userServiceMock.Setup(x => x.GetAsync(It.IsAny())).ReturnsAsync((IUser?)null); + + var result = await _auditService.AddAsync( + AuditType.Publish, + Guid.Parse("00000000-0000-0000-0000-000000000001"), + 1, + "entityType", + "comment", + "parameters"); + Assert.IsFalse(result.Success); + Assert.AreEqual(AuditLogOperationStatus.UserNotFound, result.Result); + } + + [Test] + public void GetItemsAsync_Throws_When_Invalid_Pagination_Arguments_Are_Provided() + { + Assert.ThrowsAsync(async () => await _auditService.GetItemsAsync(-1, 10), "Skip must be greater than or equal to 0."); + Assert.ThrowsAsync(async () => await _auditService.GetItemsAsync(0, -1), "Take must be greater than 0."); + Assert.ThrowsAsync(async () => await _auditService.GetItemsAsync(0, 0), "Take must be greater than 0."); + } + + [Test] + public async Task GetItemsAsync_Returns_Correct_Total_And_Item_Count() + { + SetupScopeProviderMock(); + + Fixture fixture = new Fixture(); + long totalRecords = 12; + _auditRepositoryMock.Setup(x => x.GetPagedResultsByQuery( + It.IsAny>(), + 2, + 5, + out totalRecords, + Direction.Descending, + null, + null)) + .Returns(fixture.CreateMany(count: 2)); + + _scopeProviderMock.Setup(x => x.CreateQuery()).Returns(Mock.Of>()); + + var result = await _auditService.GetItemsAsync(10, 5); + Assert.AreEqual(totalRecords, result.Total); + Assert.AreEqual(2, result.Items.Count()); + } + + [Test] + public void GetItemsByKeyAsync_Throws_When_Invalid_Pagination_Arguments_Are_Provided() + { + Assert.ThrowsAsync(async () => await _auditService.GetItemsByKeyAsync(Guid.Empty, UmbracoObjectTypes.Document, -1, 10), "Skip must be greater than or equal to 0."); + Assert.ThrowsAsync(async () => await _auditService.GetItemsByKeyAsync(Guid.Empty, UmbracoObjectTypes.Document, 0, -1), "Take must be greater than 0."); + Assert.ThrowsAsync(async () => await _auditService.GetItemsByKeyAsync(Guid.Empty, UmbracoObjectTypes.Document, 0, 0), "Take must be greater than 0."); + } + + [Test] + public async Task GetItemsByKeyAsync_Returns_No_Results_When_Key_Is_Not_Found() + { + SetupScopeProviderMock(); + + _entityServiceMock.Setup(x => x.GetId(Guid.Empty, UmbracoObjectTypes.Document)).Returns(Attempt.Fail()); + + var result = await _auditService.GetItemsByKeyAsync(Guid.Empty, UmbracoObjectTypes.Document, 10, 10); + Assert.AreEqual(0, result.Total); + Assert.AreEqual(0, result.Items.Count()); + } + + [Test] + public async Task GetItemsByKeyAsync_Returns_Correct_Total_And_Item_Count() + { + SetupScopeProviderMock(); + + _entityServiceMock.Setup(x => x.GetId(Guid.Empty, UmbracoObjectTypes.Document)).Returns(Attempt.Succeed(2)); + + Fixture fixture = new Fixture(); + long totalRecords = 12; + + _auditRepositoryMock.Setup(x => x.GetPagedResultsByQuery( + It.IsAny>(), + 2, + 5, + out totalRecords, + Direction.Descending, + null, + null)) + .Returns(fixture.CreateMany(count: 2)); + + _scopeProviderMock.Setup(x => x.CreateQuery()) + .Returns(Mock.Of>()); + + var result = await _auditService.GetItemsByKeyAsync(Guid.Empty, UmbracoObjectTypes.Document, 10, 5); + Assert.AreEqual(totalRecords, result.Total); + Assert.AreEqual(2, result.Items.Count()); + } + + [TestCase(Constants.System.Root)] + [TestCase(-100)] + public async Task GetItemsByEntityAsync_Returns_No_Results_When_Id_Is_Root_Or_Lower(int userId) + { + var result = await _auditService.GetItemsByEntityAsync(userId, 10, 10); + Assert.AreEqual(0, result.Total); + Assert.AreEqual(0, result.Items.Count()); + } + + [Test] + public async Task GetItemsByEntityAsync_Returns_Correct_Total_And_Item_Count() + { + SetupScopeProviderMock(); + + _entityServiceMock.Setup(x => x.GetId(Guid.Empty, UmbracoObjectTypes.Document)).Returns(Attempt.Succeed(2)); + + Fixture fixture = new Fixture(); + long totalRecords = 12; + + _auditRepositoryMock.Setup(x => x.GetPagedResultsByQuery( + It.IsAny>(), + 2, + 5, + out totalRecords, + Direction.Descending, + null, + null)) + .Returns(fixture.CreateMany(count: 2)); + + _scopeProviderMock.Setup(x => x.CreateQuery()) + .Returns(Mock.Of>()); + + var result = await _auditService.GetItemsByEntityAsync(1, 10, 5); + Assert.AreEqual(totalRecords, result.Total); + Assert.AreEqual(2, result.Items.Count()); + } + + [Test] + public async Task CleanLogsAsync_Calls_Repository_With_Correct_Values() + { + SetupScopeProviderMock(); + _auditRepositoryMock.Setup(x => x.CleanLogs(100)); + await _auditService.CleanLogsAsync(100); + _auditRepositoryMock.Verify(x => x.CleanLogs(It.IsAny()), Times.Once); + } + + private void SetupScopeProviderMock() => + _scopeProviderMock + .Setup(x => x.CreateCoreScope( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .Returns(Mock.Of()); +} diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/BackgroundJobs/Jobs/LogScrubberJobTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/BackgroundJobs/Jobs/LogScrubberJobTests.cs index 6dd479364c..515111cd64 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/BackgroundJobs/Jobs/LogScrubberJobTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/BackgroundJobs/Jobs/LogScrubberJobTests.cs @@ -68,5 +68,5 @@ public class LogScrubberJobTests private void VerifyLogsScrubbed() => VerifyLogsScrubbed(Times.Once()); private void VerifyLogsScrubbed(Times times) => - _mockAuditService.Verify(x => x.CleanLogs(It.Is(y => y == MaxLogAgeInMinutes)), times); + _mockAuditService.Verify(x => x.CleanLogsAsync(It.Is(y => y == MaxLogAgeInMinutes)), times); }