Audit entries rework (#19345)

* 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) * Apply suggestions from code review * Small improvement * Some adjustments following code review. Removed UnknownUserKey and used null instead. * Small adjustments * Better handle audits performed during the migration state * Update TODO comment
2025-06-06 13:12:35 +02:00
parent 321087c96e
commit 7fc2bc84de
24 changed files with 1040 additions and 394 deletions
--- a/tests/Umbraco.Tests.Common/Builders/AuditEntryBuilder.cs
+++ b/tests/Umbraco.Tests.Common/Builders/AuditEntryBuilder.cs
@@ -2,6 +2,7 @@
 // See LICENSE for more details.

 using System;
+using Umbraco.Cms.Core;
 using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Tests.Common.Builders.Interfaces;

@@ -25,6 +26,7 @@ public class AuditEntryBuilder<TParent>
 {
    private string _affectedDetails;
    private int? _affectedUserId;
+    private Guid? _affectedUserKey;
    private DateTime? _createDate;
    private DateTime? _deleteDate;
    private DateTime? _eventDateUtc;
@@ -34,6 +36,7 @@ public class AuditEntryBuilder<TParent>
    private Guid? _key;
    private string _performingDetails;
    private string _performingIp;
+    private Guid? _performingUserKey;
    private int? _performingUserId;
    private DateTime? _updateDate;

@@ -84,6 +87,12 @@ public class AuditEntryBuilder<TParent>
        return this;
    }

+    public AuditEntryBuilder<TParent> WithAffectedUserKey(Guid? affectedUserKey)
+    {
+        _affectedUserKey = affectedUserKey;
+        return this;
+    }
+
    public AuditEntryBuilder<TParent> WithEventDetails(string eventDetails)
    {
        _eventDetails = eventDetails;
@@ -120,6 +129,12 @@ public class AuditEntryBuilder<TParent>
        return this;
    }

+    public AuditEntryBuilder<TParent> WithPerformingUserKey(Guid? performingUserKey)
+    {
+        _performingUserKey = performingUserKey;
+        return this;
+    }
+
    public override IAuditEntry Build()
    {
        var id = _id ?? 0;
@@ -129,12 +144,14 @@ public class AuditEntryBuilder<TParent>
        var deleteDate = _deleteDate;
        var affectedDetails = _affectedDetails ?? Guid.NewGuid().ToString();
        var affectedUserId = _affectedUserId ?? -1;
+        var affectedUserKey = _affectedUserKey ?? Constants.Security.SuperUserKey;
        var eventDetails = _eventDetails ?? Guid.NewGuid().ToString();
        var eventType = _eventType ?? "umbraco/user";
        var performingDetails = _performingDetails ?? Guid.NewGuid().ToString();
        var performingIp = _performingIp ?? "127.0.0.1";
        var eventDateUtc = _eventDateUtc ?? DateTime.UtcNow;
        var performingUserId = _performingUserId ?? -1;
+        var performingUserKey = _performingUserKey ?? Constants.Security.SuperUserKey;

        return new AuditEntry
        {
@@ -145,12 +162,14 @@ public class AuditEntryBuilder<TParent>
            DeleteDate = deleteDate,
            AffectedDetails = affectedDetails,
            AffectedUserId = affectedUserId,
+            AffectedUserKey = affectedUserKey,
            EventDetails = eventDetails,
            EventType = eventType,
            PerformingDetails = performingDetails,
            PerformingIp = performingIp,
            EventDateUtc = eventDateUtc,
-            PerformingUserId = performingUserId
+            PerformingUserId = performingUserId,
+            PerformingUserKey = performingUserKey,
        };
    }
 }
--- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/AuditEntryServiceTests.cs
+++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/AuditEntryServiceTests.cs
@@ -0,0 +1,94 @@
+// Copyright (c) Umbraco.
+// See LICENSE for more details.
+
+using Microsoft.Extensions.DependencyInjection;
+using NUnit.Framework;
+using Umbraco.Cms.Core;
+using Umbraco.Cms.Core.Services;
+using Umbraco.Cms.Tests.Common.Builders;
+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 AuditEntryServiceTests : UmbracoIntegrationTest
+{
+    [Test]
+    public async Task Write_and_GetAll()
+    {
+        var sut = (AuditEntryService)Services.GetRequiredService<IAuditEntryService>();
+        var expected = new AuditEntryBuilder()
+            .Build();
+
+        var result = await sut.WriteAsync(
+            expected.PerformingUserKey.Value,
+            expected.PerformingDetails,
+            expected.PerformingIp,
+            expected.EventDateUtc,
+            expected.AffectedUserKey.Value,
+            expected.AffectedDetails,
+            expected.EventType,
+            expected.EventDetails);
+        Assert.NotNull(result);
+
+        var actual = result;
+
+        var entries = sut.GetAll().ToArray();
+
+        Assert.Multiple(() =>
+        {
+            Assert.AreEqual(expected.PerformingUserId, actual.PerformingUserId);
+            Assert.AreEqual(expected.PerformingUserKey, actual.PerformingUserKey);
+            Assert.AreEqual(expected.PerformingDetails, actual.PerformingDetails);
+            Assert.AreEqual(expected.EventDateUtc, actual.EventDateUtc);
+            Assert.AreEqual(expected.AffectedUserId, actual.AffectedUserId);
+            Assert.AreEqual(expected.AffectedUserKey, actual.AffectedUserKey);
+            Assert.AreEqual(expected.AffectedDetails, actual.AffectedDetails);
+            Assert.AreEqual(expected.EventType, actual.EventType);
+            Assert.AreEqual(expected.EventDetails, actual.EventDetails);
+            Assert.IsNotNull(entries);
+            Assert.AreEqual(1, entries.Length);
+            Assert.AreEqual(expected.PerformingUserKey, entries[0].PerformingUserKey);
+        });
+    }
+
+    [Test]
+    public async Task Write_and_GetAll_With_Keys()
+    {
+        var sut = (AuditEntryService)Services.GetRequiredService<IAuditEntryService>();
+        var eventDateUtc = DateTime.UtcNow;
+        var result = await sut.WriteAsync(
+            Constants.Security.SuperUserKey,
+            "performingDetails",
+            "performingIp",
+            eventDateUtc,
+            null,
+            "affectedDetails",
+            "umbraco/test",
+            "eventDetails");
+        Assert.NotNull(result);
+
+        var actual = result;
+
+        var entries = sut.GetAll().ToArray();
+
+        Assert.Multiple(() =>
+        {
+            Assert.AreEqual(Constants.Security.SuperUserId, actual.PerformingUserId);
+            Assert.AreEqual(Constants.Security.SuperUserKey, actual.PerformingUserKey);
+            Assert.AreEqual("performingDetails", actual.PerformingDetails);
+            Assert.AreEqual("performingIp", actual.PerformingIp);
+            Assert.AreEqual(eventDateUtc, actual.EventDateUtc);
+            Assert.AreEqual(Constants.Security.UnknownUserId, actual.AffectedUserId);
+            Assert.AreEqual(null, actual.AffectedUserKey);
+            Assert.AreEqual("affectedDetails", actual.AffectedDetails);
+            Assert.AreEqual("umbraco/test", actual.EventType);
+            Assert.AreEqual("eventDetails", actual.EventDetails);
+            Assert.IsNotNull(entries);
+            Assert.AreEqual(1, entries.Length);
+            Assert.AreEqual(Constants.Security.SuperUserId, entries[0].PerformingUserId);
+        });
+    }
+}
--- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/AuditServiceTests.cs
+++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/AuditServiceTests.cs
@@ -17,35 +17,6 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services;
 [UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)]
 internal sealed class AuditServiceTests : UmbracoIntegrationTest
 {
-    [Test]
-    public void GetPage()
-    {
-        var sut = (AuditService)GetRequiredService<IAuditService>();
-        var expected = new AuditEntryBuilder().Build();
-
-        for (var i = 0; i < 10; i++)
-        {
-            sut.Write(
-                expected.PerformingUserId + i,
-                expected.PerformingDetails,
-                expected.PerformingIp,
-                expected.EventDateUtc.AddMinutes(i),
-                expected.AffectedUserId + i,
-                expected.AffectedDetails,
-                expected.EventType,
-                expected.EventDetails);
-        }
-
-        var entries = sut.GetPage(2, 2, out var count).ToArray();
-
-        Assert.Multiple(() =>
-        {
-            Assert.AreEqual(2, entries.Length);
-            Assert.AreEqual(expected.PerformingUserId + 5, entries[0].PerformingUserId);
-            Assert.AreEqual(expected.PerformingUserId + 4, entries[1].PerformingUserId);
-        });
-    }
-
    [Test]
    public void GetUserLogs()
    {
@@ -72,37 +43,4 @@ internal sealed class AuditServiceTests : UmbracoIntegrationTest
            Assert.AreEqual(numberOfEntries, logs.Count(x => x.AuditType == AuditType.Unpublish));
        });
    }
-
-    [Test]
-    public void Write_and_GetAll()
-    {
-        var sut = (AuditService)Services.GetRequiredService<IAuditService>();
-        var expected = new AuditEntryBuilder().Build();
-
-        var actual = sut.Write(
-            expected.PerformingUserId,
-            expected.PerformingDetails,
-            expected.PerformingIp,
-            expected.EventDateUtc,
-            expected.AffectedUserId,
-            expected.AffectedDetails,
-            expected.EventType,
-            expected.EventDetails);
-
-        var entries = sut.GetAll().ToArray();
-
-        Assert.Multiple(() =>
-        {
-            Assert.AreEqual(expected.PerformingUserId, actual.PerformingUserId);
-            Assert.AreEqual(expected.PerformingDetails, actual.PerformingDetails);
-            Assert.AreEqual(expected.EventDateUtc, actual.EventDateUtc);
-            Assert.AreEqual(expected.AffectedUserId, actual.AffectedUserId);
-            Assert.AreEqual(expected.AffectedDetails, actual.AffectedDetails);
-            Assert.AreEqual(expected.EventType, actual.EventType);
-            Assert.AreEqual(expected.EventDetails, actual.EventDetails);
-            Assert.IsNotNull(entries);
-            Assert.AreEqual(1, entries.Length);
-            Assert.AreEqual(expected.PerformingUserId, entries[0].PerformingUserId);
-        });
-    }
 }
--- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/AuditEntryServiceTests.cs
+++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/AuditEntryServiceTests.cs
@@ -0,0 +1,154 @@
+using System.Data;
+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.Persistence.Repositories;
+using Umbraco.Cms.Core.Scoping;
+using Umbraco.Cms.Core.Services;
+using Umbraco.Cms.Core.Services.OperationStatus;
+
+namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Services;
+
+[TestFixture]
+public class AuditEntryServiceTests
+{
+    private static readonly Guid _testUserKey = Guid.NewGuid();
+    private IAuditEntryService _auditEntryService;
+    private Mock<ICoreScopeProvider> _scopeProviderMock;
+    private Mock<IAuditEntryRepository> _auditEntryRepositoryMock;
+    private Mock<IUserIdKeyResolver> _userIdKeyResolverMock;
+
+    [SetUp]
+    public void Setup()
+    {
+        _scopeProviderMock = new Mock<ICoreScopeProvider>(MockBehavior.Strict);
+        _auditEntryRepositoryMock = new Mock<IAuditEntryRepository>(MockBehavior.Strict);
+        _userIdKeyResolverMock = new Mock<IUserIdKeyResolver>(MockBehavior.Strict);
+
+        _auditEntryService = new AuditEntryService(
+            _auditEntryRepositoryMock.Object,
+            _userIdKeyResolverMock.Object,
+            _scopeProviderMock.Object,
+            Mock.Of<ILoggerFactory>(MockBehavior.Strict),
+            Mock.Of<IEventMessagesFactory>(MockBehavior.Strict));
+    }
+
+    [Test]
+    public async Task WriteAsync_Calls_Repository_With_Correct_Values()
+    {
+        SetupScopeProviderMock();
+
+        var date = DateTime.UtcNow;
+        _auditEntryRepositoryMock.Setup(x => x.Save(It.IsAny<IAuditEntry>()))
+            .Callback<IAuditEntry>(item =>
+            {
+                Assert.AreEqual(Constants.Security.SuperUserId, item.PerformingUserId);
+                Assert.AreEqual(Constants.Security.SuperUserKey, item.PerformingUserKey);
+                Assert.AreEqual("performingDetails", item.PerformingDetails);
+                Assert.AreEqual("performingIp", item.PerformingIp);
+                Assert.AreEqual(date, item.EventDateUtc);
+                Assert.AreEqual(Constants.Security.UnknownUserId, item.AffectedUserId);
+                Assert.AreEqual(null, item.AffectedUserKey);
+                Assert.AreEqual("affectedDetails", item.AffectedDetails);
+                Assert.AreEqual("umbraco/test", item.EventType);
+                Assert.AreEqual("eventDetails", item.EventDetails);
+            });
+        _userIdKeyResolverMock.Setup(x => x.TryGetAsync(Constants.Security.SuperUserKey))
+            .ReturnsAsync(Attempt.Succeed(Constants.Security.SuperUserId));
+
+        var result = await _auditEntryService.WriteAsync(
+            Constants.Security.SuperUserKey,
+            "performingDetails",
+            "performingIp",
+            date,
+            null,
+            "affectedDetails",
+            "umbraco/test",
+            "eventDetails");
+
+        _auditEntryRepositoryMock.Verify(x => x.Save(It.IsAny<IAuditEntry>()), Times.Once);
+
+        Assert.NotNull(result);
+        Assert.Multiple(() =>
+        {
+            Assert.AreEqual(Constants.Security.SuperUserId, result.PerformingUserId);
+            Assert.AreEqual("performingDetails", result.PerformingDetails);
+            Assert.AreEqual("performingIp", result.PerformingIp);
+            Assert.AreEqual(date, result.EventDateUtc);
+            Assert.AreEqual(Constants.Security.UnknownUserId, result.AffectedUserId);
+            Assert.AreEqual("affectedDetails", result.AffectedDetails);
+            Assert.AreEqual("umbraco/test", result.EventType);
+            Assert.AreEqual("eventDetails", result.EventDetails);
+        });
+    }
+
+    [Test]
+    public async Task GetUserId_UsingKey_Returns_Correct_Id()
+    {
+        SetupScopeProviderMock();
+
+        int userId = 12;
+        _userIdKeyResolverMock.Setup(x => x.TryGetAsync(_testUserKey))
+            .ReturnsAsync(Attempt.Succeed(userId));
+
+        var actualUserId = await ((AuditEntryService)_auditEntryService).GetUserId(_testUserKey);
+
+        Assert.AreEqual(actualUserId, userId);
+    }
+
+    [Test]
+    public async Task GetUserId_UsingNonExistingKey_Returns_Null()
+    {
+        SetupScopeProviderMock();
+
+        _userIdKeyResolverMock.Setup(x => x.TryGetAsync(_testUserKey))
+            .ReturnsAsync(Attempt.Fail<int>());
+
+        var actualUserId = await ((AuditEntryService)_auditEntryService).GetUserId(_testUserKey);
+
+        Assert.AreEqual(null, actualUserId);
+    }
+
+    [Test]
+    public async Task GetUserKey_UsingKey_Returns_Correct_Id()
+    {
+        SetupScopeProviderMock();
+
+        int userId = 12;
+        _userIdKeyResolverMock.Setup(x => x.TryGetAsync(userId))
+            .ReturnsAsync(Attempt.Succeed(_testUserKey));
+
+        var actualUserKey = await ((AuditEntryService)_auditEntryService).GetUserKey(userId);
+
+        Assert.AreEqual(actualUserKey, _testUserKey);
+    }
+
+    [Test]
+    public async Task GetUserKey_UsingNonExistingId_Returns_Null()
+    {
+        SetupScopeProviderMock();
+
+        int userId = 12;
+        _userIdKeyResolverMock.Setup(x => x.TryGetAsync(userId))
+            .ReturnsAsync(Attempt.Fail<Guid>());
+
+        var userKey = await ((AuditEntryService)_auditEntryService).GetUserKey(userId);
+
+        Assert.AreEqual(null, userKey);
+    }
+
+    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>());
+}