Merge pull request #24519 from abpframework/salihozkara/improve-entity-changes

Improve entity history handling for shared types and JSON
9 months ago · 185048f5f8
5 changed files with 227 additions and 19 deletions
--- a/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/EntityHistory/EntityHistoryHelper.cs
+++ b/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/EntityHistory/EntityHistoryHelper.cs
@ -5,6 +5,7 @@ using System.Linq;
 using System.Reflection;
 using Microsoft.EntityFrameworkCore;
 using Microsoft.EntityFrameworkCore.ChangeTracking;
+using Microsoft.EntityFrameworkCore.Metadata;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Logging.Abstractions;
 using Microsoft.Extensions.Options;
@ -108,12 +109,17 @@ public class EntityHistoryHelper : IEntityHistoryHelper, ITransientDependency
        }

        var entityType = entity.GetType();
+        var entityFullName = entityType.FullName!;
+        if (entityEntry.Metadata.HasSharedClrType && !entityEntry.Metadata.IsOwned())
+        {
+            entityFullName = entityEntry.Metadata.Name;
+        }
        var entityChange = new EntityChangeInfo
        {
            ChangeType = changeType,
            EntityEntry = entityEntry,
            EntityId = entityId,
-            EntityTypeFullName = entityType.FullName,
+            EntityTypeFullName = entityFullName,
            PropertyChanges = GetPropertyChanges(entityEntry),
            EntityTenantId = GetTenantId(entity)
        };
@ -181,48 +187,102 @@ public class EntityHistoryHelper : IEntityHistoryHelper, ITransientDependency

        foreach (var property in properties)
        {
+            if (entityEntry.Metadata.IsMappedToJson() && property.GetJsonPropertyName() == null)
+            {
+                continue;
+            }
+
            var propertyEntry = entityEntry.Property(property.Name);
            if (ShouldSavePropertyHistory(propertyEntry, isCreated || isDeleted) && !IsSoftDeleted(entityEntry))
            {
+                var propertyType = DeterminePropertyTypeFromEntry(property, propertyEntry);
+
                propertyChanges.Add(new EntityPropertyChangeInfo
                {
                    NewValue = isDeleted ? null : JsonSerializer.Serialize(propertyEntry.CurrentValue!).TruncateWithPostfix(EntityPropertyChangeInfo.MaxValueLength),
                    OriginalValue = isCreated ? null : JsonSerializer.Serialize(propertyEntry.OriginalValue!).TruncateWithPostfix(EntityPropertyChangeInfo.MaxValueLength),
                    PropertyName = property.Name,
-                    PropertyTypeFullName = property.ClrType.GetFirstGenericArgumentIfNullable().FullName!
+                    PropertyTypeFullName = propertyType.FullName!
                });
            }
        }

-        if (AbpEfCoreNavigationHelper != null)
+        if (AbpEfCoreNavigationHelper == null)
+        {
+            return propertyChanges;
+        }
+
+        foreach (var (navigationEntry, index) in entityEntry.Navigations.Select((value, i) => ( value, i )))
        {
-            foreach (var (navigationEntry, index) in entityEntry.Navigations.Select((value, i) => ( value, i )))
+            var propertyInfo = navigationEntry.Metadata.PropertyInfo;
+            if (propertyInfo != null &&
+                propertyInfo.IsDefined(typeof(DisableAuditingAttribute), true))
            {
-                var propertyInfo = navigationEntry.Metadata.PropertyInfo;
-                if (propertyInfo != null &&
-                    propertyInfo.IsDefined(typeof(DisableAuditingAttribute), true))
+                continue;
+            }
+
+            if (navigationEntry.Metadata.TargetEntityType.IsMappedToJson() && navigationEntry is ReferenceEntry referenceEntry && referenceEntry.TargetEntry != null)
+            {
+                foreach (var propertyChange in GetPropertyChanges(referenceEntry.TargetEntry))
                {
-                    continue;
+                    propertyChange.PropertyName = $"{referenceEntry.Metadata.Name}.{propertyChange.PropertyName}";
+                    propertyChanges.Add(propertyChange);
                }

-                if (AbpEfCoreNavigationHelper.IsNavigationEntryModified(entityEntry, index))
+                continue;
+            }
+
+            if (AbpEfCoreNavigationHelper.IsNavigationEntryModified(entityEntry, index))
+            {
+                var abpNavigationEntry = AbpEfCoreNavigationHelper.GetNavigationEntry(entityEntry, index);
+
+                var isCollection = navigationEntry.Metadata.IsCollection;
+                propertyChanges.Add(new EntityPropertyChangeInfo
                {
-                    var abpNavigationEntry = AbpEfCoreNavigationHelper.GetNavigationEntry(entityEntry, index);
-                    var isCollection = navigationEntry.Metadata.IsCollection;
-                    propertyChanges.Add(new EntityPropertyChangeInfo
-                    {
-                        PropertyName = navigationEntry.Metadata.Name,
-                        PropertyTypeFullName = navigationEntry.Metadata.ClrType.GetFirstGenericArgumentIfNullable().FullName!,
-                        OriginalValue = GetNavigationPropertyValue(abpNavigationEntry?.OriginalValue, isCollection),
-                        NewValue = GetNavigationPropertyValue(abpNavigationEntry?.CurrentValue, isCollection)
-                    });
-                }
+                    PropertyName = navigationEntry.Metadata.Name,
+                    PropertyTypeFullName = navigationEntry.Metadata.ClrType.GetFirstGenericArgumentIfNullable().FullName!,
+                    OriginalValue = GetNavigationPropertyValue(abpNavigationEntry?.OriginalValue, isCollection),
+                    NewValue = GetNavigationPropertyValue(abpNavigationEntry?.CurrentValue, isCollection)
+                });
            }
        }

        return propertyChanges;
    }

+    /// <summary>
+    /// Determines the CLR type of a property based on its EF Core metadata and the values in the given <see cref="PropertyEntry"/>.
+    /// </summary>
+    /// <param name="property">The EF Core property metadata that provides the declared CLR type.</param>
+    /// <param name="propertyEntry">The property entry that contains the current and original values for the property.</param>
+    /// <returns>
+    /// The most specific CLR type inferred for the property. This is normally the property's declared CLR type (with
+    /// nullable wrappers removed). If the declared type is <see cref="object"/>, the type is inferred from the
+    /// runtime type of <see cref="PropertyEntry.CurrentValue"/> or, if that is <c>null</c>, from
+    /// <see cref="PropertyEntry.OriginalValue"/>. If both values are <c>null</c>, the declared CLR type
+    /// (which may remain <see cref="object"/>) is returned.
+    /// </returns>
+    protected virtual Type DeterminePropertyTypeFromEntry(IProperty property, PropertyEntry propertyEntry)
+    {
+        var propertyType = property.ClrType.GetFirstGenericArgumentIfNullable();
+
+        if (propertyType != typeof(object))
+        {
+            return propertyType;
+        }
+
+        if (propertyEntry.CurrentValue != null)
+        {
+            propertyType = propertyEntry.CurrentValue.GetType().GetFirstGenericArgumentIfNullable();
+        }
+        else if (propertyEntry.OriginalValue != null)
+        {
+            propertyType = propertyEntry.OriginalValue.GetType().GetFirstGenericArgumentIfNullable();
+        }
+
+        return propertyType;
+    }
+
    protected virtual string? GetNavigationPropertyValue(object? entity, bool isCollection)
    {
        switch (entity)
--- a/framework/test/Volo.Abp.Auditing.Tests/Volo/Abp/Auditing/AbpAuditingTestModule.cs
+++ b/framework/test/Volo.Abp.Auditing.Tests/Volo/Abp/Auditing/AbpAuditingTestModule.cs
@ -59,6 +59,8 @@ public class AbpAuditingTestModule : AbpModule
                    "AppEntityWithValueObject",
                    type => type == typeof(AppEntityWithValueObject) || type == typeof(AppEntityWithValueObjectAddress))
            );
+
+            options.EntityHistorySelectors.Add(new NamedTypeSelector(nameof(AppEntityWithJsonProperty), type => type == typeof(AppEntityWithJsonProperty)));
        });

        context.Services.AddType<Auditing_Tests.MyAuditedObject1>();
--- a/framework/test/Volo.Abp.Auditing.Tests/Volo/Abp/Auditing/App/Entities/AppEntityWithJsonProperty.cs
+++ b/framework/test/Volo.Abp.Auditing.Tests/Volo/Abp/Auditing/App/Entities/AppEntityWithJsonProperty.cs
@ -0,0 +1,27 @@
+using System;
+using System.Collections.Generic;
+using Volo.Abp.Domain.Entities.Auditing;
+
+namespace Volo.Abp.Auditing.App.Entities;
+
+public class AppEntityWithJsonProperty : FullAuditedAggregateRoot<Guid>
+{
+    public string Name { get; set; }
+
+    public JsonPropertyObject Data { get; set; }
+
+    public int Count { get; set; }
+
+    public AppEntityWithJsonProperty()
+    {
+    }
+
+    public AppEntityWithJsonProperty(Guid id, string name) : base(id)
+    {
+        Name = name;
+    }
+}
+
+public class JsonPropertyObject : Dictionary<string, object>
+{
+}
--- a/framework/test/Volo.Abp.Auditing.Tests/Volo/Abp/Auditing/App/EntityFrameworkCore/AbpAuditingTestDbContext.cs
+++ b/framework/test/Volo.Abp.Auditing.Tests/Volo/Abp/Auditing/App/EntityFrameworkCore/AbpAuditingTestDbContext.cs
@ -30,6 +30,7 @@ public class AbpAuditingTestDbContext : AbpDbContext<AbpAuditingTestDbContext>
    public DbSet<AppEntityWithNavigations> AppEntityWithNavigations { get; set; }
    public DbSet<AppEntityWithNavigationChildOneToMany> AppEntityWithNavigationChildOneToMany { get; set; }
    public DbSet<AppEntityWithNavigationsAndDisableAuditing> AppEntityWithNavigationsAndDisableAuditing { get; set; }
+    public DbSet<AppEntityWithJsonProperty> EntitiesWithObjectProperty { get; set; }

    public AbpAuditingTestDbContext(DbContextOptions<AbpAuditingTestDbContext> options)
        : base(options)
@ -56,5 +57,25 @@ public class AbpAuditingTestDbContext : AbpDbContext<AbpAuditingTestDbContext>
            b.HasMany(x => x.ManyToMany).WithMany(x => x.ManyToMany).UsingEntity<AppEntityWithNavigationsAndAppEntityWithNavigationChildManyToMany>();
        });

+        modelBuilder.Entity<AppEntityWithJsonProperty>(b =>
+        {
+            b.ConfigureByConvention();
+            b.OwnsOne(x => x.Data, b2 =>
+            {
+                b2.ToJson();
+
+                b2.Property<object>("Name")
+                    .HasConversion<string>(
+                        v => v.ToString(),
+                        v => v
+                    );
+
+                b2.Property<object>("Value")
+                    .HasConversion<string>(
+                        v => v.ToString(),
+                        v => v
+                    );
+            });
+        });
    }
 }
--- a/framework/test/Volo.Abp.Auditing.Tests/Volo/Abp/Auditing/Auditing_Tests.cs
+++ b/framework/test/Volo.Abp.Auditing.Tests/Volo/Abp/Auditing/Auditing_Tests.cs
@ -720,6 +720,104 @@ public class Auditing_Tests : AbpAuditingTestBase
                                                                     x.EntityChanges[1].PropertyChanges[0].PropertyName == nameof(AppEntityWithNavigationChildManyToMany.ManyToMany) &&
                                                                     x.EntityChanges[1].PropertyChanges[0].PropertyTypeFullName == typeof(List<AppEntityWithNavigations>).FullName));

+#pragma warning restore 4014
+    }
+
+    [Fact]
+    public async Task Should_Write_AuditLog_For_Json_Property_Changes()
+    {
+        var entityId = Guid.NewGuid();
+        var repository = ServiceProvider.GetRequiredService<IBasicRepository<AppEntityWithJsonProperty, Guid>>();
+
+        using (var scope = _auditingManager.BeginScope())
+        {
+            using (var uow = _unitOfWorkManager.Begin())
+            {
+                var entity = new AppEntityWithJsonProperty(entityId, "Test Entity")
+                {
+                    Data = new JsonPropertyObject()
+                    {
+                        { "Name", "String Name" },
+                        { "Value", "String Value"}
+                    },
+                    Count = 10
+                };
+
+                await repository.InsertAsync(entity);
+
+                await uow.CompleteAsync();
+                await scope.SaveAsync();
+            }
+        }
+
+#pragma warning disable 4014
+        AuditingStore.Received().SaveAsync(Arg.Is<AuditLogInfo>(x => x.EntityChanges.Count == 1 &&
+                                                                     x.EntityChanges[0].ChangeType == EntityChangeType.Created &&
+                                                                     x.EntityChanges[0].EntityTypeFullName == typeof(AppEntityWithJsonProperty).FullName &&
+                                                                     x.EntityChanges[0].PropertyChanges.Count == 4 &&
+
+                                                                     x.EntityChanges[0].PropertyChanges[0].OriginalValue == null &&
+                                                                     x.EntityChanges[0].PropertyChanges[0].NewValue == "10" &&
+                                                                     x.EntityChanges[0].PropertyChanges[0].PropertyName == nameof(AppEntityWithJsonProperty.Count) &&
+                                                                     x.EntityChanges[0].PropertyChanges[0].PropertyTypeFullName == typeof(int).FullName &&
+
+                                                                     x.EntityChanges[0].PropertyChanges[1].OriginalValue == null &&
+                                                                     x.EntityChanges[0].PropertyChanges[1].NewValue == "\"Test Entity\"" &&
+                                                                     x.EntityChanges[0].PropertyChanges[1].PropertyName == nameof(AppEntityWithJsonProperty.Name) &&
+                                                                     x.EntityChanges[0].PropertyChanges[1].PropertyTypeFullName == typeof(string).FullName &&
+
+                                                                     x.EntityChanges[0].PropertyChanges[2].OriginalValue == null &&
+                                                                     x.EntityChanges[0].PropertyChanges[2].NewValue == "\"String Name\"" &&
+                                                                     x.EntityChanges[0].PropertyChanges[2].PropertyName == "Data.Name" &&
+                                                                     x.EntityChanges[0].PropertyChanges[2].PropertyTypeFullName == typeof(string).FullName &&
+
+                                                                     x.EntityChanges[0].PropertyChanges[3].OriginalValue == null &&
+                                                                     x.EntityChanges[0].PropertyChanges[3].NewValue == "\"String Value\"" &&
+                                                                     x.EntityChanges[0].PropertyChanges[3].PropertyName == "Data.Value" &&
+                                                                     x.EntityChanges[0].PropertyChanges[3].PropertyTypeFullName == typeof(string).FullName));
+        AuditingStore.ClearReceivedCalls();
+#pragma warning restore 4014
+
+
+        using (var scope = _auditingManager.BeginScope())
+        {
+            using (var uow = _unitOfWorkManager.Begin())
+            {
+                var entity = await repository.GetAsync(entityId);
+
+                entity.Name = "Updated Test Entity";
+
+                entity.Data["Name"] = "Updated String Name";
+                entity.Data["Value"] = "Updated String Value";
+
+                await repository.UpdateAsync(entity);
+
+                await uow.CompleteAsync();
+                await scope.SaveAsync();
+            }
+        }
+
+#pragma warning disable 4014
+        AuditingStore.Received().SaveAsync(Arg.Is<AuditLogInfo>(x => x.EntityChanges.Count == 1 &&
+                                                                     x.EntityChanges[0].ChangeType == EntityChangeType.Updated &&
+                                                                     x.EntityChanges[0].EntityTypeFullName == typeof(AppEntityWithJsonProperty).FullName &&
+                                                                     x.EntityChanges[0].PropertyChanges.Count == 3 &&
+
+                                                                     x.EntityChanges[0].PropertyChanges[0].OriginalValue == "\"Test Entity\"" &&
+                                                                     x.EntityChanges[0].PropertyChanges[0].NewValue == "\"Updated Test Entity\"" &&
+                                                                     x.EntityChanges[0].PropertyChanges[0].PropertyName == nameof(AppEntityWithJsonProperty.Name) &&
+                                                                     x.EntityChanges[0].PropertyChanges[0].PropertyTypeFullName == typeof(string).FullName &&
+
+                                                                     x.EntityChanges[0].PropertyChanges[1].OriginalValue == "\"String Name\"" &&
+                                                                     x.EntityChanges[0].PropertyChanges[1].NewValue == "\"Updated String Name\"" &&
+                                                                     x.EntityChanges[0].PropertyChanges[1].PropertyName == "Data.Name" &&
+                                                                     x.EntityChanges[0].PropertyChanges[1].PropertyTypeFullName == typeof(string).FullName &&
+
+                                                                     x.EntityChanges[0].PropertyChanges[2].OriginalValue == "\"String Value\"" &&
+                                                                     x.EntityChanges[0].PropertyChanges[2].NewValue == "\"Updated String Value\"" &&
+                                                                     x.EntityChanges[0].PropertyChanges[2].PropertyName == "Data.Value" &&
+                                                                     x.EntityChanges[0].PropertyChanges[2].PropertyTypeFullName == typeof(string).FullName));
+        AuditingStore.ClearReceivedCalls();
 #pragma warning restore 4014
    }
 }