From c3adf2550a99af326e5e6b31ec50d64ad9eb408b Mon Sep 17 00:00:00 2001 From: Alexey Shibanov <83034617+alexeyshibanov@users.noreply.github.com> Date: Thu, 12 Sep 2024 17:28:30 +0300 Subject: [PATCH] fix: Ignore fixed settings in SaveObjectSettingsAsync() (#2835) Co-authored-by: artem-dudarev --- .../Domain/IRepository.cs | 16 +-- .../Infrastructure/DbContextRepositoryBase.cs | 2 +- .../Settings/SettingsManager.cs | 113 +++++++++--------- 3 files changed, 63 insertions(+), 68 deletions(-) diff --git a/src/VirtoCommerce.Platform.Core/Domain/IRepository.cs b/src/VirtoCommerce.Platform.Core/Domain/IRepository.cs index fcc8e36a057..c3fe052b8c0 100644 --- a/src/VirtoCommerce.Platform.Core/Domain/IRepository.cs +++ b/src/VirtoCommerce.Platform.Core/Domain/IRepository.cs @@ -6,8 +6,8 @@ namespace VirtoCommerce.Platform.Core.Common /// /// Repository interface. Provides base interface for all repositories used in the framework. /// - public interface IRepository : IDisposable - { + public interface IRepository : IDisposable + { /// /// Gets the unit of work. This class actually saves the data into underlying storage. /// @@ -21,27 +21,27 @@ public interface IRepository : IDisposable /// /// /// The item. - void Attach(T item) where T : class; - + void Attach(T item) where T : class; + /// /// Adds the specified item to the context in the Added state. Meaning item will be created in the underlying storage. /// /// /// The item. - void Add(T item) where T : class; + void Add(T item) where T : class; /// /// Updates the specified item. Marks the item for the update. /// /// /// The item. - void Update(T item) where T : class; + void Update(T item) where T : class; /// /// Removes the specified item. Item marked for deletion and will be removed from the underlying storage on save. /// /// /// The item. - void Remove(T item) where T : class; - } + void Remove(T item) where T : class; + } } diff --git a/src/VirtoCommerce.Platform.Data/Infrastructure/DbContextRepositoryBase.cs b/src/VirtoCommerce.Platform.Data/Infrastructure/DbContextRepositoryBase.cs index 486b59e975f..e63e6635c92 100644 --- a/src/VirtoCommerce.Platform.Data/Infrastructure/DbContextRepositoryBase.cs +++ b/src/VirtoCommerce.Platform.Data/Infrastructure/DbContextRepositoryBase.cs @@ -18,7 +18,7 @@ protected DbContextRepositoryBase(TContext dbContext, IUnitOfWork unitOfWork = n // Mitigations the breaking changes with cascade deletion introduced in EF Core 3.0 // https://docs.microsoft.com/en-us/ef/core/what-is-new/ef-core-3.0/breaking-changes#cascade // The new CascadeTiming.Immediate that is used by default in EF Core 3.0 is lead wrong track as Added for Deleted dependent/child entities during - // work of Patch method for data entities + // work of Patch method for data entities DbContext.ChangeTracker.CascadeDeleteTiming = CascadeTiming.OnSaveChanges; DbContext.ChangeTracker.DeleteOrphansTiming = CascadeTiming.OnSaveChanges; diff --git a/src/VirtoCommerce.Platform.Data/Settings/SettingsManager.cs b/src/VirtoCommerce.Platform.Data/Settings/SettingsManager.cs index d52a85aaefb..bdcec0eb054 100644 --- a/src/VirtoCommerce.Platform.Data/Settings/SettingsManager.cs +++ b/src/VirtoCommerce.Platform.Data/Settings/SettingsManager.cs @@ -21,7 +21,7 @@ namespace VirtoCommerce.Platform.Data.Settings { /// /// Provide next functionality to working with settings - /// - Load setting metainformation from module manifest and database + /// - Load settings meta information from module manifest and database /// - Deep load all settings for entity /// - Mass update all entity settings /// @@ -32,7 +32,7 @@ public class SettingsManager : ISettingsManager private readonly IDictionary _registeredSettingsByNameDict = new Dictionary(StringComparer.OrdinalIgnoreCase).WithDefaultValue(null); private readonly IDictionary> _registeredTypeSettingsByNameDict = new Dictionary>(StringComparer.OrdinalIgnoreCase).WithDefaultValue(null); private readonly IEventPublisher _eventPublisher; - private readonly IDictionary _fixedSettingsDict; + private readonly Dictionary _fixedSettingsDict; public SettingsManager(Func repositoryFactory, IPlatformMemoryCache memoryCache, @@ -51,10 +51,8 @@ public SettingsManager(Func repositoryFactory, public void RegisterSettingsForType(IEnumerable settings, string typeName) { - if (settings == null) - { - throw new ArgumentNullException(nameof(settings)); - } + ArgumentNullException.ThrowIfNull(settings); + var existTypeSettings = _registeredTypeSettingsByNameDict[typeName]; if (existTypeSettings != null) { @@ -65,17 +63,15 @@ public void RegisterSettingsForType(IEnumerable settings, str public IEnumerable GetSettingsForType(string typeName) { - return _registeredTypeSettingsByNameDict[typeName] ?? Enumerable.Empty(); + return _registeredTypeSettingsByNameDict[typeName] ?? []; } public IEnumerable AllRegisteredSettings => _registeredSettingsByNameDict.Values; public void RegisterSettings(IEnumerable settings, string moduleId = null) { - if (settings == null) - { - throw new ArgumentNullException(nameof(settings)); - } + ArgumentNullException.ThrowIfNull(settings); + foreach (var setting in settings) { setting.ModuleId = moduleId; @@ -89,21 +85,18 @@ public void RegisterSettings(IEnumerable settings, string mod public virtual async Task GetObjectSettingAsync(string name, string objectType = null, string objectId = null) { - if (name == null) - { - throw new ArgumentNullException(nameof(name)); - } - return (await GetObjectSettingsAsync(new[] { name }, objectType, objectId)).FirstOrDefault(); + ArgumentException.ThrowIfNullOrWhiteSpace(name); + + return (await GetObjectSettingsAsync([name], objectType, objectId)).FirstOrDefault(); } public virtual async Task> GetObjectSettingsAsync(IEnumerable names, string objectType = null, string objectId = null) { - if (names == null) - { - throw new ArgumentNullException(nameof(names)); - } - var cacheKey = CacheKey.With(GetType(), "GetSettingByNamesAsync", string.Join(";", names), objectType, objectId); - var result = await _memoryCache.GetOrCreateExclusiveAsync(cacheKey, async (cacheEntry) => + ArgumentNullException.ThrowIfNull(names); + + var settingNames = names as string[] ?? names.ToArray(); + var cacheKey = CacheKey.With(GetType(), "GetSettingByNamesAsync", string.Join(";", settingNames), objectType, objectId); + var result = await _memoryCache.GetOrCreateExclusiveAsync(cacheKey, async cacheEntry => { var resultObjectSettings = new List(); var dbStoredSettings = new List(); @@ -113,87 +106,89 @@ public virtual async Task> GetObjectSettingsAsyn { repository.DisableChangesTracking(); //try to load setting from db - dbStoredSettings.AddRange(await repository.GetObjectSettingsByNamesAsync(names.ToArray(), objectType, objectId)); + dbStoredSettings.AddRange(await repository.GetObjectSettingsByNamesAsync(settingNames, objectType, objectId)); } - foreach (var name in names) + foreach (var name in settingNames) { - var objectSetting = _fixedSettingsDict.ContainsKey(name) ? - GetFixedSetting(name) : - GetRegularSetting(name, dbStoredSettings, objectType, objectId); + var objectSetting = _fixedSettingsDict.ContainsKey(name) + ? GetFixedSetting(name) + : GetRegularSetting(name, dbStoredSettings, objectType, objectId); resultObjectSettings.Add(objectSetting); //Add cache expiration token for setting cacheEntry.AddExpirationToken(SettingsCacheRegion.CreateChangeToken(objectSetting)); } + return resultObjectSettings; }); + return result; } public virtual async Task RemoveObjectSettingsAsync(IEnumerable objectSettings) { - if (objectSettings == null) - { - throw new ArgumentNullException(nameof(objectSettings)); - } + ArgumentNullException.ThrowIfNull(objectSettings); + + var settingEntries = objectSettings as ObjectSettingEntry[] ?? objectSettings.ToArray(); using (var repository = _repositoryFactory()) { - foreach (var objectSetting in objectSettings) + foreach (var objectSetting in settingEntries) { - var dbSetting = repository.Settings.FirstOrDefault(x => x.Name == objectSetting.Name && x.ObjectType == objectSetting.ObjectType && x.ObjectId == objectSetting.ObjectId); + var dbSetting = repository.Settings.FirstOrDefault(x => + x.Name == objectSetting.Name && x.ObjectType == objectSetting.ObjectType && + x.ObjectId == objectSetting.ObjectId); if (dbSetting != null) { repository.Remove(dbSetting); } } + await repository.UnitOfWork.CommitAsync(); - ClearCache(objectSettings); } + + ClearCache(settingEntries); } public virtual async Task SaveObjectSettingsAsync(IEnumerable objectSettings) { - if (objectSettings == null) - { - throw new ArgumentNullException(nameof(objectSettings)); - } + ArgumentNullException.ThrowIfNull(objectSettings); var changedEntries = new List>(); + // Ignore unregistered settings, fixed settings, and settings without values + var settings = objectSettings + .Where(x => _registeredSettingsByNameDict.ContainsKey(x.Name) && + !_fixedSettingsDict.ContainsKey(x.Name) && + x.ItHasValues) + .ToArray(); + using (var repository = _repositoryFactory()) { - var settingNames = objectSettings.Select(x => x.Name).Distinct().ToArray(); - var alreadyExistDbSettings = (await repository.Settings + var settingNames = settings.Select(x => x.Name).Distinct().ToArray(); + + var alreadyExistDbSettings = await repository.Settings .Include(s => s.SettingValues) .Where(x => settingNames.Contains(x.Name)) .AsSplitQuery() - .ToListAsync()); + .ToListAsync(); var validator = new ObjectSettingEntryValidator(); - foreach (var setting in objectSettings.Where(x => x.ItHasValues)) - { - if (!validator.Validate(setting).IsValid) - { - throw new PlatformException($"Setting with name {setting.Name} is invalid"); - } - - if (_fixedSettingsDict.ContainsKey(setting.Name)) - { - throw new PlatformException($"Setting with name {setting.Name} is read only"); - } - // Skip when Setting is not registered + foreach (var setting in settings) + { var settingDescriptor = _registeredSettingsByNameDict[setting.Name]; - if (settingDescriptor == null) + + if (!(await validator.ValidateAsync(setting)).IsValid) { - continue; + throw new PlatformException($"Setting with name {setting.Name} is invalid"); } // We need to convert resulting DB entities to model. Use ValueObject.Equals to find already saved setting entity from passed setting - var originalEntity = alreadyExistDbSettings.Where(x => x.Name.EqualsInvariant(setting.Name)) - .FirstOrDefault(x => x.ToModel(new ObjectSettingEntry(settingDescriptor)).Equals(setting)); + var originalEntity = alreadyExistDbSettings.FirstOrDefault(x => + x.Name.EqualsIgnoreCase(setting.Name) && + x.ToModel(new ObjectSettingEntry(settingDescriptor)).Equals(setting)); var modifiedEntity = AbstractTypeFactory.TryCreateInstance().FromModel(setting); @@ -216,7 +211,7 @@ public virtual async Task SaveObjectSettingsAsync(IEnumerable x.Name.EqualsInvariant(name)); + var dbSetting = dbStoredSettings.FirstOrDefault(x => x.Name.EqualsIgnoreCase(name)); if (dbSetting != null) { objectSetting = dbSetting.ToModel(objectSetting);