diff --git a/VirtoCommerce.OrdersModule.sln.DotSettings b/VirtoCommerce.OrdersModule.sln.DotSettings index c15dfd8b0..5ab6883d0 100644 --- a/VirtoCommerce.OrdersModule.sln.DotSettings +++ b/VirtoCommerce.OrdersModule.sln.DotSettings @@ -1,4 +1,5 @@ - + + <Policy><Descriptor Staticness="Static" AccessRightKinds="Private" Description="Static readonly fields (private)"><ElementKinds><Kind Name="READONLY_FIELD" /></ElementKinds></Descriptor><Policy Inspect="True" WarnAboutPrefixesAndSuffixes="False" Prefix="_" Suffix="" Style="aaBb" /></Policy> True True True diff --git a/src/VirtoCommerce.OrdersModule.Core/Model/Address.cs b/src/VirtoCommerce.OrdersModule.Core/Model/Address.cs index b84d1a3ee..884362f4b 100644 --- a/src/VirtoCommerce.OrdersModule.Core/Model/Address.cs +++ b/src/VirtoCommerce.OrdersModule.Core/Model/Address.cs @@ -1,3 +1,5 @@ +using System.Collections.Generic; +using System.Reflection; using VirtoCommerce.Platform.Core.Swagger; namespace VirtoCommerce.OrdersModule.Core.Model @@ -5,6 +7,9 @@ namespace VirtoCommerce.OrdersModule.Core.Model [SwaggerSchemaId("OrderAddress")] public class Address : CoreModule.Core.Common.Address { + public virtual IEnumerable GetAllProperties() + { + return GetProperties(); + } } - } diff --git a/src/VirtoCommerce.OrdersModule.Data/Handlers/IndexCustomerOrderChangedEventHandler.cs b/src/VirtoCommerce.OrdersModule.Data/Handlers/IndexCustomerOrderChangedEventHandler.cs index aaf7abd75..2e200af24 100644 --- a/src/VirtoCommerce.OrdersModule.Data/Handlers/IndexCustomerOrderChangedEventHandler.cs +++ b/src/VirtoCommerce.OrdersModule.Data/Handlers/IndexCustomerOrderChangedEventHandler.cs @@ -1,4 +1,3 @@ -using System; using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; @@ -35,20 +34,46 @@ public IndexCustomerOrderChangedEventHandler( _indexingConfigurations = indexingConfigurations; } - public async Task Handle(OrderChangedEvent message) + public virtual async Task Handle(OrderChangedEvent message) { - if (!_configuration.IsOrderFullTextSearchEnabled() || - !await _settingsManager.GetValueAsync(ModuleConstants.Settings.General.EventBasedIndexation)) + if (await ShouldIndexAsync()) { - return; + await IndexOrdersAsync(message); } + } + + protected virtual async Task ShouldIndexAsync() + { + return _configuration.IsOrderFullTextSearchEnabled() && + await _settingsManager.GetValueAsync(ModuleConstants.Settings.General.EventBasedIndexation); + } + + protected virtual Task IndexOrdersAsync(OrderChangedEvent message) + { + var indexEntries = GetOrderIndexEntries(message); - var indexEntries = message?.ChangedEntries - .Select(x => new IndexEntry { Id = x.OldEntry.Id, EntryState = x.EntryState, Type = ModuleConstants.OrderIndexDocumentType }) - .ToArray() ?? Array.Empty(); + if (indexEntries.Count > 0) + { + var documentBuilders = _indexingConfigurations + .GetDocumentBuilders(ModuleConstants.OrderIndexDocumentType, typeof(CustomerOrderChangesProvider)) + .ToList(); + + _indexingJobService.EnqueueIndexAndDeleteDocuments(indexEntries, JobPriority.Normal, documentBuilders); + } - _indexingJobService.EnqueueIndexAndDeleteDocuments(indexEntries, - JobPriority.Normal, _indexingConfigurations.GetDocumentBuilders(ModuleConstants.OrderIndexDocumentType, typeof(CustomerOrderChangesProvider)).ToList()); + return Task.CompletedTask; + } + + protected virtual IList GetOrderIndexEntries(OrderChangedEvent message) + { + return message?.ChangedEntries + .Select(x => new IndexEntry + { + Id = x.OldEntry.Id, + EntryState = x.EntryState, + Type = ModuleConstants.OrderIndexDocumentType, + }) + .ToList() ?? []; } } } diff --git a/src/VirtoCommerce.OrdersModule.Data/Handlers/LogChangesOrderChangedEventHandler.cs b/src/VirtoCommerce.OrdersModule.Data/Handlers/LogChangesOrderChangedEventHandler.cs index 44c22be3d..bb364199c 100644 --- a/src/VirtoCommerce.OrdersModule.Data/Handlers/LogChangesOrderChangedEventHandler.cs +++ b/src/VirtoCommerce.OrdersModule.Data/Handlers/LogChangesOrderChangedEventHandler.cs @@ -2,7 +2,6 @@ using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; -using System.Reflection; using System.Threading.Tasks; using AutoCompare; using Hangfire; @@ -25,7 +24,7 @@ public class LogChangesOrderChangedEventHandler : IEventHandler> _auditablePropertiesListByTypeDict = new(); + private static readonly ConcurrentDictionary> _auditablePropertiesListByTypeDict = new(); public LogChangesOrderChangedEventHandler(IChangeLogService changeLogService, IMemberService memberService, ISettingsManager settingsManager) { @@ -40,118 +39,133 @@ public virtual async Task Handle(OrderChangedEvent message) if (logOrderChangesEnabled && message.ChangedEntries.Any()) { - var operationLogs = new List(); + var operationLogs = GetOperationLogs(message.ChangedEntries); - foreach (var changedEntry in message.ChangedEntries) + if (!operationLogs.IsNullOrEmpty()) { - if (changedEntry.EntryState == EntryState.Modified) - { - var originalOperations = changedEntry.OldEntry.GetFlatObjectsListWithInterface().Distinct().ToList(); - var modifiedOperations = changedEntry.NewEntry.GetFlatObjectsListWithInterface().Distinct().ToList(); - - modifiedOperations.CompareTo(originalOperations, EqualityComparer.Default, - (state, modified, original) => operationLogs.AddRange(GetChangedEntryOperationLogs(new GenericChangedEntry(modified, original, state)))); - } - else if (changedEntry.EntryState == EntryState.Added || changedEntry.EntryState == EntryState.Deleted) - { - operationLogs.AddRange(GetChangedEntryOperationLogs(new GenericChangedEntry(changedEntry.NewEntry, changedEntry.OldEntry, changedEntry.EntryState))); - } + BackgroundJob.Enqueue(() => TryToLogChangesBackgroundJob(operationLogs.ToArray())); } + } + } - if (!operationLogs.IsNullOrEmpty()) + protected virtual IList GetOperationLogs(IEnumerable> changedEntries) + { + var operationLogs = new List(); + + foreach (var changedEntry in changedEntries) + { + switch (changedEntry.EntryState) { - BackgroundJob.Enqueue(() => TryToLogChangesBackgroundJob(operationLogs.ToArray())); + case EntryState.Modified: + { + var originalOperations = changedEntry.OldEntry.GetFlatObjectsListWithInterface().Distinct().ToList(); + var modifiedOperations = changedEntry.NewEntry.GetFlatObjectsListWithInterface().Distinct().ToList(); + + modifiedOperations.CompareTo(originalOperations, EqualityComparer.Default, + (state, modified, original) => operationLogs.AddRange(GetChangedEntryOperationLogs(new GenericChangedEntry(modified, original, state)))); + break; + } + case EntryState.Added or EntryState.Deleted: + operationLogs.AddRange(GetChangedEntryOperationLogs(new GenericChangedEntry(changedEntry.NewEntry, changedEntry.OldEntry, changedEntry.EntryState))); + break; } } + + return operationLogs; } - // (!) Do not make this method async, it causes improper user recorded into the log! It happens because the user stored in the current thread. If the thread switched, the user info will lost. - public void TryToLogChangesBackgroundJob(OperationLog[] operationLogs) + public Task TryToLogChangesBackgroundJob(OperationLog[] operationLogs) { - _changeLogService.SaveChangesAsync(operationLogs).GetAwaiter().GetResult(); + return _changeLogService.SaveChangesAsync(operationLogs); } protected virtual IEnumerable GetChangedEntryOperationLogs(GenericChangedEntry changedEntry) { var result = new List(); - if (changedEntry.EntryState == EntryState.Modified) + switch (changedEntry.EntryState) { - var logs = new List(); + case EntryState.Modified: + { + var logs = new List(); + var diff = GetOperationDifferences(changedEntry, logs); + var auditableProperties = GetAuditableProperties(changedEntry); - var diff = GetOperationDifferences(changedEntry, logs); + if (auditableProperties.Count > 0) + { + var observedDifferences = diff + .Where(x => auditableProperties.ContainsIgnoreCase(x.Name)) + .Distinct(new DifferenceComparer()); - var auditableProperties = GetAuditableProperties(changedEntry); + foreach (var difference in observedDifferences) + { + logs.Add($"The {changedEntry.OldEntry.OperationType} {changedEntry.NewEntry.Number} property '{difference.Name}' changed from '{difference.OldValue}' to '{difference.NewValue}'"); + } + } - if (auditableProperties.Count != 0) - { - var observedDifferences = diff.Join(auditableProperties, x => x.Name.ToLowerInvariant(), x => x.ToLowerInvariant(), (x, y) => x).ToArray(); - foreach (var difference in observedDifferences.Distinct(new DifferenceComparer())) + foreach (var log in logs) + { + result.Add(GetLogRecord(changedEntry.NewEntry, log)); + } + + break; + } + case EntryState.Deleted: { - logs.Add($"The {changedEntry.OldEntry.OperationType} {changedEntry.NewEntry.Number} property '{difference.Name}' changed from '{difference.OldValue}' to '{difference.NewValue}'"); + var record = GetLogRecord(changedEntry.NewEntry, + $"The {changedEntry.NewEntry.OperationType} {changedEntry.NewEntry.Number} deleted", + EntryState.Deleted); + result.Add(record); + break; + } + case EntryState.Added: + { + var record = GetLogRecord(changedEntry.NewEntry, + $"The new {changedEntry.NewEntry.OperationType} {changedEntry.NewEntry.Number} added", + EntryState.Added); + result.Add(record); + break; } - } - - result.AddRange(logs.Select(x => GetLogRecord(changedEntry.NewEntry, x))); - } - else if (changedEntry.EntryState == EntryState.Deleted) - { - var record = GetLogRecord(changedEntry.NewEntry, - $"The {changedEntry.NewEntry.OperationType} {changedEntry.NewEntry.Number} deleted", - EntryState.Deleted); - result.Add(record); - } - else if (changedEntry.EntryState == EntryState.Added) - { - var record = GetLogRecord(changedEntry.NewEntry, - $"The new {changedEntry.NewEntry.OperationType} {changedEntry.NewEntry.Number} added", - EntryState.Added); - result.Add(record); } return result; } - protected List GetAuditableProperties(GenericChangedEntry changedEntry) + protected static List GetAuditableProperties(GenericChangedEntry changedEntry) { var type = changedEntry.OldEntry.GetType(); - if (!_auditablePropertiesListByTypeDict.TryGetValue(type.Name, out var auditableProperties)) - { - auditableProperties = type.GetProperties().Where(prop => Attribute.IsDefined(prop, typeof(AuditableAttribute))) - .Select(x => x.Name) - .ToList(); - _auditablePropertiesListByTypeDict[type.Name] = auditableProperties; - } - return auditableProperties; + return _auditablePropertiesListByTypeDict.GetOrAdd(type, t => + t.GetProperties() + .Where(prop => Attribute.IsDefined(prop, typeof(AuditableAttribute))) + .Select(x => x.Name) + .ToList()); } protected virtual IList GetOperationDifferences(GenericChangedEntry changedEntry, List logs) { var diff = Comparer.Compare(changedEntry.OldEntry, changedEntry.NewEntry); - if (changedEntry.OldEntry is Shipment shipment) - { - logs.AddRange(GetShipmentChanges(shipment, changedEntry.NewEntry as Shipment)); - diff.AddRange(Comparer.Compare(shipment, changedEntry.NewEntry as Shipment)); - } - else if (changedEntry.OldEntry is PaymentIn payment) - { - logs.AddRange(GetPaymentChanges(payment, changedEntry.NewEntry as PaymentIn)); - diff.AddRange(Comparer.Compare(payment, changedEntry.NewEntry as PaymentIn)); - } - else if (changedEntry.OldEntry is CustomerOrder order) - { - logs.AddRange(GetCustomerOrderChanges(order, changedEntry.NewEntry as CustomerOrder)); - diff.AddRange(Comparer.Compare(order, changedEntry.NewEntry as CustomerOrder)); - } - else if (changedEntry.OldEntry is Capture capture) + switch (changedEntry.OldEntry) { - diff.AddRange(Comparer.Compare(capture, changedEntry.NewEntry as Capture)); - } - else if (changedEntry.OldEntry is Refund refund) - { - diff.AddRange(Comparer.Compare(refund, changedEntry.NewEntry as Refund)); + case Shipment shipment: + logs.AddRange(GetShipmentChanges(shipment, changedEntry.NewEntry as Shipment)); + diff.AddRange(Comparer.Compare(shipment, changedEntry.NewEntry as Shipment)); + break; + case PaymentIn payment: + logs.AddRange(GetPaymentChanges(payment, changedEntry.NewEntry as PaymentIn)); + diff.AddRange(Comparer.Compare(payment, changedEntry.NewEntry as PaymentIn)); + break; + case CustomerOrder order: + logs.AddRange(GetCustomerOrderChanges(order, changedEntry.NewEntry as CustomerOrder)); + diff.AddRange(Comparer.Compare(order, changedEntry.NewEntry as CustomerOrder)); + break; + case Capture capture: + diff.AddRange(Comparer.Compare(capture, changedEntry.NewEntry as Capture)); + break; + case Refund refund: + diff.AddRange(Comparer.Compare(refund, changedEntry.NewEntry as Refund)); + break; } return diff; @@ -160,63 +174,61 @@ protected virtual IList GetOperationDifferences(GenericChangedEntry< protected virtual IEnumerable GetCustomerOrderChanges(CustomerOrder originalOrder, CustomerOrder modifiedOrder) { var result = new List(); + if (originalOrder.EmployeeId != modifiedOrder.EmployeeId) { var employeeName = "none"; if (!string.IsNullOrEmpty(modifiedOrder.EmployeeId)) { var employee = _memberService.GetByIdAsync(modifiedOrder.EmployeeId).GetAwaiter().GetResult() as Employee; - employeeName = employee != null ? employee.FullName : employeeName; + employeeName = employee?.FullName ?? employeeName; } result.Add($"Order employee was changed to '{employeeName}'"); } + result.AddRange(GetAddressChanges(originalOrder, originalOrder.Addresses, modifiedOrder.Addresses)); - return result.ToArray(); + + return result; } protected virtual IEnumerable GetShipmentChanges(Shipment originalShipment, Shipment modifiedShipment) { - var retVal = new List(); - retVal.AddRange(GetAddressChanges(originalShipment, new[] { originalShipment.DeliveryAddress }, new[] { modifiedShipment.DeliveryAddress })); - return retVal.ToArray(); + return GetAddressChanges(originalShipment, [originalShipment.DeliveryAddress], [modifiedShipment.DeliveryAddress]); } protected virtual IEnumerable GetPaymentChanges(PaymentIn payment, PaymentIn modifiedPayment) { - var result = new List(); - result.AddRange(GetAddressChanges(payment, new[] { payment.BillingAddress }, new[] { modifiedPayment.BillingAddress })); - return result; + return GetAddressChanges(payment, [payment.BillingAddress], [modifiedPayment.BillingAddress]); } protected virtual IEnumerable GetAddressChanges(IOperation operation, IEnumerable
originalAddress, IEnumerable
modifiedAddress) { var result = new List(); - modifiedAddress.Where(x => x != null).ToList().CompareTo(originalAddress.Where(x => x != null).ToList(), EqualityComparer
.Default, - (state, source, target) => - { - if (state == EntryState.Added) - { - result.Add($"The address '{StringifyAddress(target)}' for {operation.OperationType} {operation.Number} added"); - } - else if (state == EntryState.Deleted) - { - result.Add($"The address '{StringifyAddress(target)}' for {operation.OperationType} {operation.Number} deleted"); - } - }); + + var modifiedAddressList = modifiedAddress?.Where(x => x != null).ToList() ?? []; + var originalAddressList = originalAddress?.Where(x => x != null).ToList() ?? []; + + modifiedAddressList.CompareTo(originalAddressList, EqualityComparer
.Default, (state, _, target) => + { + switch (state) + { + case EntryState.Added: + result.Add($"The address '{StringifyAddress(target)}' for {operation.OperationType} {operation.Number} added"); + break; + case EntryState.Deleted: + result.Add($"The address '{StringifyAddress(target)}' for {operation.OperationType} {operation.Number} deleted"); + break; + } + }); + return result; } protected virtual string StringifyAddress(Address address) { - var result = ""; - if (address != null) - { - return string.Join(", ", typeof(Address).GetProperties(BindingFlags.Instance | BindingFlags.Public) - .OrderBy(p => p.Name) - .Select(p => p.GetValue(address)) - .Where(x => x != null)); - } - return result; + return address is null + ? string.Empty + : string.Join(", ", address.GetAllProperties().Select(p => p.GetValue(address)).Where(x => x != null)); } protected virtual OperationLog GetLogRecord(IOperation operation, string template, EntryState operationType = EntryState.Modified) @@ -232,17 +244,28 @@ protected virtual OperationLog GetLogRecord(IOperation operation, string templat } } - internal class DifferenceComparer : EqualityComparer + internal sealed class DifferenceComparer : EqualityComparer { public override bool Equals(Difference x, Difference y) { - return GetHashCode(x) == GetHashCode(y); + if (ReferenceEquals(x, y)) + { + return true; + } + + if (x is null || y is null) + { + return false; + } + + return string.Equals(x.Name, y.Name, StringComparison.Ordinal) && + Equals(x.OldValue, y.OldValue) && + Equals(x.NewValue, y.NewValue); } public override int GetHashCode(Difference obj) { - var result = string.Join(":", obj.Name, obj.NewValue, obj.OldValue); - return result.GetHashCode(); + return HashCode.Combine(obj.Name, obj.OldValue, obj.NewValue); } } } diff --git a/src/VirtoCommerce.OrdersModule.Data/Search/Indexed/CustomerOrderChangesProvider.cs b/src/VirtoCommerce.OrdersModule.Data/Search/Indexed/CustomerOrderChangesProvider.cs index e38acab67..5da548680 100644 --- a/src/VirtoCommerce.OrdersModule.Data/Search/Indexed/CustomerOrderChangesProvider.cs +++ b/src/VirtoCommerce.OrdersModule.Data/Search/Indexed/CustomerOrderChangesProvider.cs @@ -2,8 +2,7 @@ using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; -using VirtoCommerce.CatalogModule.Core.Model; -using VirtoCommerce.CustomerModule.Core.Model; +using Microsoft.EntityFrameworkCore; using VirtoCommerce.OrdersModule.Core.Model; using VirtoCommerce.OrdersModule.Data.Repositories; using VirtoCommerce.Platform.Core.ChangeLog; @@ -26,28 +25,24 @@ public CustomerOrderChangesProvider(Func orderRepositoryFactor _changeLogSearchService = changeLogSearchService; } - public async Task> GetChangesAsync(DateTime? startDate, DateTime? endDate, long skip, long take) + public virtual Task> GetChangesAsync(DateTime? startDate, DateTime? endDate, long skip, long take) { - if (startDate == null && endDate == null) - { - return GetChangesFromRepository(skip, take); - } - - return await GetChangesFromOperaionLog(startDate, endDate, skip, take); + return startDate == null && endDate == null + ? GetChangesFromRepositoryAsync(skip, take) + : GetChangesFromOperationLogAsync(startDate, endDate, skip, take); } - public async Task GetTotalChangesCountAsync(DateTime? startDate, DateTime? endDate) + public virtual async Task GetTotalChangesCountAsync(DateTime? startDate, DateTime? endDate) { if (startDate == null && endDate == null) { // Get total products count - using (var repository = _orderRepositoryFactory()) - { - return repository.CustomerOrders.Count(); - } + using var repository = _orderRepositoryFactory(); + + return await repository.CustomerOrders.CountAsync(); } - var criteria = GetChangeLogSearchCriteria(startDate, endDate, 0, 0); + var criteria = GetChangeLogSearchCriteria(startDate, endDate, skip: 0, take: 0); // Get changes count from operation log return (await _changeLogSearchService.SearchAsync(criteria)).TotalCount; @@ -56,44 +51,45 @@ public async Task GetTotalChangesCountAsync(DateTime? startDate, DateTime? /// /// Get documents from repository and return them as changes /// - private IList GetChangesFromRepository(long skip, long take) + protected virtual async Task> GetChangesFromRepositoryAsync(long skip, long take) { - using (var repository = _orderRepositoryFactory()) - { - var productIds = repository.CustomerOrders - .OrderBy(i => i.CreatedDate) - .Select(i => i.Id) - .Skip((int)skip) - .Take((int)take) - .ToArray(); - - return productIds.Select(id => + using var repository = _orderRepositoryFactory(); + + var orders = await repository.CustomerOrders + .OrderBy(x => x.CreatedDate) + .Select(x => new { x.Id, ModifiedDate = x.ModifiedDate ?? x.CreatedDate }) + .Skip((int)skip) + .Take((int)take) + .ToArrayAsync(); + + return orders + .Select(x => new IndexDocumentChange { - DocumentId = id, + DocumentId = x.Id, ChangeType = IndexDocumentChangeType.Modified, - ChangeDate = DateTime.UtcNow - } - ).ToArray(); - } + ChangeDate = x.ModifiedDate, + }) + .ToList(); } /// /// Get changes from operation log /// - private async Task> GetChangesFromOperaionLog(DateTime? startDate, DateTime? endDate, long skip, long take) + protected virtual async Task> GetChangesFromOperationLogAsync(DateTime? startDate, DateTime? endDate, long skip, long take) { var criteria = GetChangeLogSearchCriteria(startDate, endDate, skip, take); var operations = (await _changeLogSearchService.SearchAsync(criteria)).Results; - return operations.Select(o => - new IndexDocumentChange - { - DocumentId = o.ObjectId, - ChangeType = o.OperationType == EntryState.Deleted ? IndexDocumentChangeType.Deleted : IndexDocumentChangeType.Modified, - ChangeDate = o.ModifiedDate ?? o.CreatedDate, - } - ).ToArray(); + return operations + .Select(x => + new IndexDocumentChange + { + DocumentId = x.ObjectId, + ChangeType = x.OperationType == EntryState.Deleted ? IndexDocumentChangeType.Deleted : IndexDocumentChangeType.Modified, + ChangeDate = x.ModifiedDate ?? x.CreatedDate, + }) + .ToList(); } protected virtual ChangeLogSearchCriteria GetChangeLogSearchCriteria(DateTime? startDate, DateTime? endDate, long skip, long take) @@ -102,14 +98,14 @@ protected virtual ChangeLogSearchCriteria GetChangeLogSearchCriteria(DateTime? s var types = AbstractTypeFactory.AllTypeInfos.Select(x => x.TypeName).ToList(); - if (types.Count != 0) + if (types.Count > 0) { - types.Add(nameof(CustomerOrder)); + types.Add(ChangeLogObjectType); criteria.ObjectTypes = types; } else { - criteria.ObjectType = nameof(CustomerOrder); + criteria.ObjectType = ChangeLogObjectType; } criteria.StartDate = startDate;