fix: Ignore fixed settings in SaveObjectSettingsAsync() (#2835)

Co-authored-by: artem-dudarev <ad@virtoway.com>

Author: alexeyshibanov

src/VirtoCommerce.Platform.Core/Domain/IRepository.cs

@@ -6,8 +6,8 @@ namespace VirtoCommerce.Platform.Core.Common
     /// <summary>
     /// Repository interface. Provides base interface for all repositories used in the framework.
     /// </summary>
-	public interface IRepository : IDisposable
-	{
+    public interface IRepository : IDisposable
+    {
         /// <summary>
         /// Gets the unit of work. This class actually saves the data into underlying storage.
         /// </summary>
@@ -21,27 +21,27 @@ public interface IRepository : IDisposable
         /// </summary>
         /// <typeparam name="T"></typeparam>
         /// <param name="item">The item.</param>
-		void Attach<T>(T item) where T : class;
-      
+        void Attach<T>(T item) where T : class;
+
         /// <summary>
         /// Adds the specified item to the context in the Added state. Meaning item will be created in the underlying storage.
         /// </summary>
         /// <typeparam name="T"></typeparam>
         /// <param name="item">The item.</param>
-		void Add<T>(T item) where T : class;
+        void Add<T>(T item) where T : class;
 
         /// <summary>
         /// Updates the specified item. Marks the item for the update.
         /// </summary>
         /// <typeparam name="T"></typeparam>
         /// <param name="item">The item.</param>
-		void Update<T>(T item) where T : class;
+        void Update<T>(T item) where T : class;
 
         /// <summary>
         /// Removes the specified item. Item marked for deletion and will be removed from the underlying storage on save.
         /// </summary>
         /// <typeparam name="T"></typeparam>
         /// <param name="item">The item.</param>
-		void Remove<T>(T item) where T : class;
-	}
+        void Remove<T>(T item) where T : class;
+    }
 }

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;
 

src/VirtoCommerce.Platform.Data/Settings/SettingsManager.cs

@@ -21,7 +21,7 @@ namespace VirtoCommerce.Platform.Data.Settings
 {
     /// <summary>
     /// 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
     /// </summary>
@@ -32,7 +32,7 @@ public class SettingsManager : ISettingsManager
         private readonly IDictionary<string, SettingDescriptor> _registeredSettingsByNameDict = new Dictionary<string, SettingDescriptor>(StringComparer.OrdinalIgnoreCase).WithDefaultValue(null);
         private readonly IDictionary<string, IEnumerable<SettingDescriptor>> _registeredTypeSettingsByNameDict = new Dictionary<string, IEnumerable<SettingDescriptor>>(StringComparer.OrdinalIgnoreCase).WithDefaultValue(null);
         private readonly IEventPublisher _eventPublisher;
-        private readonly IDictionary<string, ObjectSettingEntry> _fixedSettingsDict;
+        private readonly Dictionary<string, ObjectSettingEntry> _fixedSettingsDict;
 
         public SettingsManager(Func<IPlatformRepository> repositoryFactory,
             IPlatformMemoryCache memoryCache,
@@ -51,10 +51,8 @@ public SettingsManager(Func<IPlatformRepository> repositoryFactory,
 
         public void RegisterSettingsForType(IEnumerable<SettingDescriptor> 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<SettingDescriptor> settings, str
 
         public IEnumerable<SettingDescriptor> GetSettingsForType(string typeName)
         {
-            return _registeredTypeSettingsByNameDict[typeName] ?? Enumerable.Empty<SettingDescriptor>();
+            return _registeredTypeSettingsByNameDict[typeName] ?? [];
         }
 
         public IEnumerable<SettingDescriptor> AllRegisteredSettings => _registeredSettingsByNameDict.Values;
 
         public void RegisterSettings(IEnumerable<SettingDescriptor> 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<SettingDescriptor> settings, string mod
 
         public virtual async Task<ObjectSettingEntry> 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<IEnumerable<ObjectSettingEntry>> GetObjectSettingsAsync(IEnumerable<string> 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<ObjectSettingEntry>();
                 var dbStoredSettings = new List<SettingEntity>();
@@ -113,87 +106,89 @@ public virtual async Task<IEnumerable<ObjectSettingEntry>> 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<ObjectSettingEntry> 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<ObjectSettingEntry> objectSettings)
         {
-            if (objectSettings == null)
-            {
-                throw new ArgumentNullException(nameof(objectSettings));
-            }
+            ArgumentNullException.ThrowIfNull(objectSettings);
 
             var changedEntries = new List<GenericChangedEntry<ObjectSettingEntry>>();
 
+            // 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<SettingEntity>.TryCreateInstance().FromModel(setting);
 
@@ -216,7 +211,7 @@ public virtual async Task SaveObjectSettingsAsync(IEnumerable<ObjectSettingEntry
                 await repository.UnitOfWork.CommitAsync();
             }
 
-            ClearCache(objectSettings);
+            ClearCache(settings);
 
             await _eventPublisher.Publish(new ObjectSettingChangedEvent(changedEntries));
         }
@@ -245,7 +240,7 @@ protected virtual ObjectSettingEntry GetRegularSetting(string name, List<Setting
                 ObjectType = objectType,
                 ObjectId = objectId
             };
-            var dbSetting = dbStoredSettings.FirstOrDefault(x => x.Name.EqualsInvariant(name));
+            var dbSetting = dbStoredSettings.FirstOrDefault(x => x.Name.EqualsIgnoreCase(name));
             if (dbSetting != null)
             {
                 objectSetting = dbSetting.ToModel(objectSetting);