From 2927debb53aa8948f0799b536b3e038822425031 Mon Sep 17 00:00:00 2001 From: Halil ibrahim Kalkan Date: Mon, 16 Jul 2018 11:25:27 +0300 Subject: [PATCH] Resolved #366: Audit Logging Improvements --- ...pplicationConfigurationScriptController.cs | 2 + .../Mvc/Auditing/AbpAuditActionFilter.cs | 6 ++ .../AbpServiceProxyScriptController.cs | 4 +- .../Auditing/AuditingInterceptorRegistrar.cs | 2 +- .../Volo/Abp/Auditing/AuditingManager.cs | 60 ++++++++++++++++++- .../Volo/Abp/Auditing/EntityChangeInfo.cs | 9 +++ .../EntityHistory/EntityHistoryHelper.cs | 6 +- ...LoggingtDbContextModelBuilderExtensions.cs | 2 +- 8 files changed, 82 insertions(+), 9 deletions(-) diff --git a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ApplicationConfigurations/AbpApplicationConfigurationScriptController.cs b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ApplicationConfigurations/AbpApplicationConfigurationScriptController.cs index 0f61a87940..5772eceeb0 100644 --- a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ApplicationConfigurations/AbpApplicationConfigurationScriptController.cs +++ b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ApplicationConfigurations/AbpApplicationConfigurationScriptController.cs @@ -2,12 +2,14 @@ using System.Text; using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc; +using Volo.Abp.Auditing; using Volo.Abp.Json; namespace Volo.Abp.AspNetCore.Mvc.ApplicationConfigurations { [Area("Abp")] [Route("Abp/ApplicationConfigurationScript")] + [DisableAuditing] public class AbpApplicationConfigurationScriptController : AbpController { private readonly IApplicationConfigurationBuilder _configurationBuilder; diff --git a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Auditing/AbpAuditActionFilter.cs b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Auditing/AbpAuditActionFilter.cs index b3cb9f6f0a..6d31ba6542 100644 --- a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Auditing/AbpAuditActionFilter.cs +++ b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Auditing/AbpAuditActionFilter.cs @@ -84,6 +84,12 @@ namespace Volo.Abp.AspNetCore.Mvc.Auditing return false; } + //TODO: This is partially duplication of AuditHelper.ShouldSaveAudit method. Check why it does not work for controllers + if (!AuditingInterceptorRegistrar.ShouldAuditTypeByDefault(context.Controller.GetType())) + { + return false; + } + auditLog = auditLogScope.Log; auditLogAction = _auditingHelper.CreateAuditLogAction( context.ActionDescriptor.AsControllerActionDescriptor().ControllerTypeInfo.AsType(), diff --git a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ProxyScripting/AbpServiceProxyScriptController.cs b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ProxyScripting/AbpServiceProxyScriptController.cs index fa3100debb..9da8278764 100644 --- a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ProxyScripting/AbpServiceProxyScriptController.cs +++ b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ProxyScripting/AbpServiceProxyScriptController.cs @@ -1,12 +1,12 @@ using Microsoft.AspNetCore.Mvc; +using Volo.Abp.Auditing; using Volo.Abp.Http.ProxyScripting; namespace Volo.Abp.AspNetCore.Mvc.ProxyScripting { - //TODO: abp area? - //TODO: [DisableAuditing] [Area("Abp")] [Route("Abp/ServiceProxyScript")] + [DisableAuditing] public class AbpServiceProxyScriptController : AbpController { private readonly IProxyScriptManager _proxyScriptManager; diff --git a/framework/src/Volo.Abp.Auditing/Volo/Abp/Auditing/AuditingInterceptorRegistrar.cs b/framework/src/Volo.Abp.Auditing/Volo/Abp/Auditing/AuditingInterceptorRegistrar.cs index e901dbc037..546d54299c 100644 --- a/framework/src/Volo.Abp.Auditing/Volo/Abp/Auditing/AuditingInterceptorRegistrar.cs +++ b/framework/src/Volo.Abp.Auditing/Volo/Abp/Auditing/AuditingInterceptorRegistrar.cs @@ -30,7 +30,7 @@ namespace Volo.Abp.Auditing } //TODO: Move to a better place - internal static bool ShouldAuditTypeByDefault(Type type) + public static bool ShouldAuditTypeByDefault(Type type) { if (type.IsDefined(typeof(AuditedAttribute), true)) { diff --git a/framework/src/Volo.Abp.Auditing/Volo/Abp/Auditing/AuditingManager.cs b/framework/src/Volo.Abp.Auditing/Volo/Abp/Auditing/AuditingManager.cs index 1db7d6f00a..b875eaca37 100644 --- a/framework/src/Volo.Abp.Auditing/Volo/Abp/Auditing/AuditingManager.cs +++ b/framework/src/Volo.Abp.Auditing/Volo/Abp/Auditing/AuditingManager.cs @@ -1,5 +1,6 @@ using System; using System.Diagnostics; +using System.Linq; using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; @@ -76,18 +77,73 @@ namespace Volo.Abp.Auditing saveHandle.StopWatch.Stop(); saveHandle.AuditLog.ExecutionDuration = Convert.ToInt32(saveHandle.StopWatch.Elapsed.TotalMilliseconds); ExecutePostContributors(saveHandle.AuditLog); + MergeEntityChanges(saveHandle.AuditLog); + } + + protected virtual void MergeEntityChanges(AuditLogInfo auditLog) + { + var changeGroups = auditLog.EntityChanges + .Where(e => e.ChangeType == EntityChangeType.Updated) + .GroupBy(e => new {e.EntityTypeFullName, e.EntityId}) + .ToList(); + + foreach (var changeGroup in changeGroups) + { + if (changeGroup.Count() <= 1) + { + continue; + } + + var firstEntityChange = changeGroup.First(); + + foreach (var entityChangeInfo in changeGroup) + { + if (entityChangeInfo == firstEntityChange) + { + continue; + } + + firstEntityChange.Merge(entityChangeInfo); + + auditLog.EntityChanges.Remove(entityChangeInfo); + } + } } protected virtual async Task SaveAsync(DisposableSaveHandle saveHandle) { BeforeSave(saveHandle); - await _auditingStore.SaveAsync(saveHandle.AuditLog); + + if (ShouldSave(saveHandle.AuditLog)) + { + await _auditingStore.SaveAsync(saveHandle.AuditLog); + } } protected virtual void Save(DisposableSaveHandle saveHandle) { BeforeSave(saveHandle); - _auditingStore.Save(saveHandle.AuditLog); + + if (ShouldSave(saveHandle.AuditLog)) + { + _auditingStore.Save(saveHandle.AuditLog); + } + } + + protected bool ShouldSave(AuditLogInfo auditLog) + { + if (!auditLog.Actions.Any() && !auditLog.EntityChanges.Any()) + { + return false; + } + + if (!Options.IsEnabledForGetRequests && !auditLog.EntityChanges.Any()) + { + //TODO: We can create another option for that: IsEnabledIfNoChangesDone? + return false; + } + + return true; } protected class DisposableSaveHandle : IAuditLogSaveHandle diff --git a/framework/src/Volo.Abp.Auditing/Volo/Abp/Auditing/EntityChangeInfo.cs b/framework/src/Volo.Abp.Auditing/Volo/Abp/Auditing/EntityChangeInfo.cs index 40152f3372..ca59da3683 100644 --- a/framework/src/Volo.Abp.Auditing/Volo/Abp/Auditing/EntityChangeInfo.cs +++ b/framework/src/Volo.Abp.Auditing/Volo/Abp/Auditing/EntityChangeInfo.cs @@ -27,5 +27,14 @@ namespace Volo.Abp.Auditing { ExtraProperties = new Dictionary(); } + + public virtual void Merge(EntityChangeInfo changeInfo) + { + //TODO: Gracefully merge (add/update) and also for ExtraProperties + foreach (var propertyChange in changeInfo.PropertyChanges) + { + PropertyChanges.Add(propertyChange); + } + } } } diff --git a/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/EntityHistory/EntityHistoryHelper.cs b/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/EntityHistory/EntityHistoryHelper.cs index 1ea043fc77..1c48fffc53 100644 --- a/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/EntityHistory/EntityHistoryHelper.cs +++ b/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/EntityHistory/EntityHistoryHelper.cs @@ -219,12 +219,12 @@ namespace Volo.Abp.EntityFrameworkCore.EntityHistory return false; } - if (entityType.GetTypeInfo().IsDefined(typeof(AuditedAttribute), true)) + if (entityType.IsDefined(typeof(AuditedAttribute), true)) { return true; } - if (entityType.GetTypeInfo().IsDefined(typeof(DisableAuditingAttribute), true)) + if (entityType.IsDefined(typeof(DisableAuditingAttribute), true)) { return false; } @@ -257,7 +257,7 @@ namespace Volo.Abp.EntityFrameworkCore.EntityHistory } var entityType = propertyEntry.EntityEntry.Entity.GetType(); - if (entityType.GetTypeInfo().IsDefined(typeof(DisableAuditingAttribute), true)) + if (entityType.IsDefined(typeof(DisableAuditingAttribute), true)) { if (propertyInfo == null || !propertyInfo.IsDefined(typeof(AuditedAttribute), true)) { diff --git a/modules/audit-logging/src/Volo.Abp.AuditLogging.EntityFrameworkCore/Volo/Abp/AuditLogging/EntityFrameworkCore/AbpAuditLoggingtDbContextModelBuilderExtensions.cs b/modules/audit-logging/src/Volo.Abp.AuditLogging.EntityFrameworkCore/Volo/Abp/AuditLogging/EntityFrameworkCore/AbpAuditLoggingtDbContextModelBuilderExtensions.cs index 9d33567e20..967250d976 100644 --- a/modules/audit-logging/src/Volo.Abp.AuditLogging.EntityFrameworkCore/Volo/Abp/AuditLogging/EntityFrameworkCore/AbpAuditLoggingtDbContextModelBuilderExtensions.cs +++ b/modules/audit-logging/src/Volo.Abp.AuditLogging.EntityFrameworkCore/Volo/Abp/AuditLogging/EntityFrameworkCore/AbpAuditLoggingtDbContextModelBuilderExtensions.cs @@ -85,7 +85,7 @@ namespace Volo.Abp.AuditLogging.EntityFrameworkCore { b.ToTable(tablePrefix + "EntityPropertyChanges", schema); - b.Property(x => x.NewValue).HasMaxLength(EntityPropertyChangeConsts.MaxNewValueLength).IsRequired().HasColumnName(nameof(EntityPropertyChange.NewValue)); + b.Property(x => x.NewValue).HasMaxLength(EntityPropertyChangeConsts.MaxNewValueLength).HasColumnName(nameof(EntityPropertyChange.NewValue)); b.Property(x => x.PropertyName).HasMaxLength(EntityPropertyChangeConsts.MaxPropertyNameLength).IsRequired().HasColumnName(nameof(EntityPropertyChange.PropertyName)); b.Property(x => x.PropertyTypeFullName).HasMaxLength(EntityPropertyChangeConsts.MaxPropertyTypeFullNameLength).IsRequired().HasColumnName(nameof(EntityPropertyChange.PropertyTypeFullName)); b.Property(x => x.OriginalValue).HasMaxLength(EntityPropertyChangeConsts.MaxOriginalValueLength).HasColumnName(nameof(EntityPropertyChange.OriginalValue));