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
This commit is contained in:
Laura Neto
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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<IAuditService>();
|
||||
|
||||
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(() =>
|
||||
{
|
||||
|
||||
@@ -25,7 +25,7 @@ public partial class ContentEditingServiceTests : ContentEditingServiceTestsBase
|
||||
}
|
||||
|
||||
protected override void CustomTestSetup(IUmbracoBuilder builder)
|
||||
=> builder.AddNotificationHandler<ContentCopiedNotification, RelateOnCopyNotificationHandler>();
|
||||
=> builder.AddNotificationAsyncHandler<ContentCopiedNotification, RelateOnCopyNotificationHandler>();
|
||||
|
||||
private ITemplateService TemplateService => GetRequiredService<ITemplateService>();
|
||||
|
||||
|
||||
@@ -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<ICoreScopeProvider> _scopeProviderMock;
|
||||
private Mock<IAuditRepository> _auditRepositoryMock;
|
||||
private Mock<IEntityService> _entityServiceMock;
|
||||
private Mock<IUserService> _userServiceMock;
|
||||
|
||||
[SetUp]
|
||||
public void Setup()
|
||||
{
|
||||
_scopeProviderMock = new Mock<ICoreScopeProvider>(MockBehavior.Strict);
|
||||
_auditRepositoryMock = new Mock<IAuditRepository>(MockBehavior.Strict);
|
||||
_entityServiceMock = new Mock<IEntityService>(MockBehavior.Strict);
|
||||
_userServiceMock = new Mock<IUserService>(MockBehavior.Strict);
|
||||
|
||||
_auditService = new AuditService(
|
||||
_scopeProviderMock.Object,
|
||||
Mock.Of<ILoggerFactory>(MockBehavior.Strict),
|
||||
Mock.Of<IEventMessagesFactory>(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<IAuditItem>()))
|
||||
.Callback<IAuditItem>(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<IUser> mockUser = new Mock<IUser>();
|
||||
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<IAuditItem>()), 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<Guid>())).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<ArgumentOutOfRangeException>(async () => await _auditService.GetItemsAsync(-1, 10), "Skip must be greater than or equal to 0.");
|
||||
Assert.ThrowsAsync<ArgumentOutOfRangeException>(async () => await _auditService.GetItemsAsync(0, -1), "Take must be greater than 0.");
|
||||
Assert.ThrowsAsync<ArgumentOutOfRangeException>(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<IQuery<IAuditItem>>(),
|
||||
2,
|
||||
5,
|
||||
out totalRecords,
|
||||
Direction.Descending,
|
||||
null,
|
||||
null))
|
||||
.Returns(fixture.CreateMany<AuditItem>(count: 2));
|
||||
|
||||
_scopeProviderMock.Setup(x => x.CreateQuery<IAuditItem>()).Returns(Mock.Of<IQuery<IAuditItem>>());
|
||||
|
||||
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<ArgumentOutOfRangeException>(async () => await _auditService.GetItemsByKeyAsync(Guid.Empty, UmbracoObjectTypes.Document, -1, 10), "Skip must be greater than or equal to 0.");
|
||||
Assert.ThrowsAsync<ArgumentOutOfRangeException>(async () => await _auditService.GetItemsByKeyAsync(Guid.Empty, UmbracoObjectTypes.Document, 0, -1), "Take must be greater than 0.");
|
||||
Assert.ThrowsAsync<ArgumentOutOfRangeException>(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<int>.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<IQuery<IAuditItem>>(),
|
||||
2,
|
||||
5,
|
||||
out totalRecords,
|
||||
Direction.Descending,
|
||||
null,
|
||||
null))
|
||||
.Returns(fixture.CreateMany<AuditItem>(count: 2));
|
||||
|
||||
_scopeProviderMock.Setup(x => x.CreateQuery<IAuditItem>())
|
||||
.Returns(Mock.Of<IQuery<IAuditItem>>());
|
||||
|
||||
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<IQuery<IAuditItem>>(),
|
||||
2,
|
||||
5,
|
||||
out totalRecords,
|
||||
Direction.Descending,
|
||||
null,
|
||||
null))
|
||||
.Returns(fixture.CreateMany<AuditItem>(count: 2));
|
||||
|
||||
_scopeProviderMock.Setup(x => x.CreateQuery<IAuditItem>())
|
||||
.Returns(Mock.Of<IQuery<IAuditItem>>());
|
||||
|
||||
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<int>()), Times.Once);
|
||||
}
|
||||
|
||||
private void SetupScopeProviderMock() =>
|
||||
_scopeProviderMock
|
||||
.Setup(x => x.CreateCoreScope(
|
||||
It.IsAny<IsolationLevel>(),
|
||||
It.IsAny<RepositoryCacheMode>(),
|
||||
It.IsAny<IEventDispatcher>(),
|
||||
It.IsAny<IScopedNotificationPublisher>(),
|
||||
It.IsAny<bool?>(),
|
||||
It.IsAny<bool>(),
|
||||
It.IsAny<bool>()))
|
||||
.Returns(Mock.Of<IScope>());
|
||||
}
|
||||
@@ -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<int>(y => y == MaxLogAgeInMinutes)), times);
|
||||
_mockAuditService.Verify(x => x.CleanLogsAsync(It.Is<int>(y => y == MaxLogAgeInMinutes)), times);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user