Remove secondary caching in ContentDefinitionManager, improve error message in DataMigrationManager

src/OrchardCore/OrchardCore.ContentManagement/ContentDefinitionManager.cs

@@ -1,8 +1,7 @@
-using System;
-using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Linq;
 using System.Threading.Tasks;
+using System;
 using Microsoft.Extensions.Caching.Memory;
 using OrchardCore.ContentManagement.Metadata;
 using OrchardCore.ContentManagement.Metadata.Models;
@@ -19,21 +18,13 @@ public class ContentDefinitionManager : IContentDefinitionManager
         private readonly IContentDefinitionStore _contentDefinitionStore;
         private readonly IMemoryCache _memoryCache;
 
-        private readonly ConcurrentDictionary<string, ContentTypeDefinition> _cachedTypeDefinitions;
-        private readonly ConcurrentDictionary<string, ContentPartDefinition> _cachedPartDefinitions;
-
-        private readonly Dictionary<string, ContentTypeDefinition> _scopedTypeDefinitions = new(StringComparer.OrdinalIgnoreCase);
-        private readonly Dictionary<string, ContentPartDefinition> _scopedPartDefinitions = new(StringComparer.OrdinalIgnoreCase);
-
         public ContentDefinitionManager(
             IContentDefinitionStore contentDefinitionStore,
             IMemoryCache memoryCache)
         {
             _contentDefinitionStore = contentDefinitionStore;
             _memoryCache = memoryCache;
 
-            _cachedTypeDefinitions = _memoryCache.GetOrCreate("TypeDefinitions", entry => new ConcurrentDictionary<string, ContentTypeDefinition>(StringComparer.OrdinalIgnoreCase));
-            _cachedPartDefinitions = _memoryCache.GetOrCreate("PartDefinitions", entry => new ConcurrentDictionary<string, ContentPartDefinition>(StringComparer.OrdinalIgnoreCase));
         }
 
         public async Task<IEnumerable<ContentTypeDefinition>> LoadTypeDefinitionsAsync()
@@ -72,11 +63,6 @@ public async Task<ContentTypeDefinition> LoadTypeDefinitionAsync(string name)
         {
             ArgumentException.ThrowIfNullOrEmpty(name);
 
-            if (_scopedTypeDefinitions.TryGetValue(name, out var typeDefinition))
-            {
-                return typeDefinition;
-            }
-
             var document = await _contentDefinitionStore.LoadContentDefinitionAsync();
 
             return LoadTypeDefinition(document, name);
@@ -97,11 +83,6 @@ public async Task<ContentPartDefinition> LoadPartDefinitionAsync(string name)
         {
             ArgumentException.ThrowIfNullOrEmpty(name);
 
-            if (_scopedPartDefinitions.TryGetValue(name, out var partDefinition))
-            {
-                return partDefinition;
-            }
-
             var document = await _contentDefinitionStore.LoadContentDefinitionAsync();
 
             return LoadPartDefinition(document, name);
@@ -182,27 +163,20 @@ public async Task StorePartDefinitionAsync(ContentPartDefinition contentPartDefi
 
         public async Task<string> GetIdentifierAsync() => (await _contentDefinitionStore.GetContentDefinitionAsync()).Identifier;
 
-        private ContentTypeDefinition LoadTypeDefinition(ContentDefinitionRecord document, string name) =>
-            !_scopedTypeDefinitions.TryGetValue(name, out var typeDefinition)
-            ? _scopedTypeDefinitions[name] = Build(
+        private static ContentTypeDefinition LoadTypeDefinition(ContentDefinitionRecord document, string name) => Build(
                 document.ContentTypeDefinitionRecords.FirstOrDefault(type => type.Name.EqualsOrdinalIgnoreCase(name)),
-                document.ContentPartDefinitionRecords)
-            : typeDefinition;
+                document.ContentPartDefinitionRecords);
 
-        private ContentTypeDefinition GetTypeDefinition(ContentDefinitionRecord document, string name) =>
-            _cachedTypeDefinitions.GetOrAdd(name, name => Build(
+        private static ContentTypeDefinition GetTypeDefinition(ContentDefinitionRecord document, string name) =>
+           Build(
                 document.ContentTypeDefinitionRecords.FirstOrDefault(type => type.Name.EqualsOrdinalIgnoreCase(name)),
-                document.ContentPartDefinitionRecords));
+                document.ContentPartDefinitionRecords);
 
-        private ContentPartDefinition LoadPartDefinition(ContentDefinitionRecord document, string name) =>
-            !_scopedPartDefinitions.TryGetValue(name, out var partDefinition)
-            ? _scopedPartDefinitions[name] = Build(
-                document.ContentPartDefinitionRecords.FirstOrDefault(part => part.Name.EqualsOrdinalIgnoreCase(name)))
-            : partDefinition;
+        private static ContentPartDefinition LoadPartDefinition(ContentDefinitionRecord document, string name) =>
+           Build(document.ContentPartDefinitionRecords.FirstOrDefault(part => part.Name.EqualsOrdinalIgnoreCase(name)));
 
-        private ContentPartDefinition GetPartDefinition(ContentDefinitionRecord document, string name) =>
-            _cachedPartDefinitions.GetOrAdd(name, name => Build(
-                document.ContentPartDefinitionRecords.FirstOrDefault(record => record.Name.EqualsOrdinalIgnoreCase(name))));
+        private static ContentPartDefinition GetPartDefinition(ContentDefinitionRecord document, string name) => Build(
+                document.ContentPartDefinitionRecords.FirstOrDefault(record => record.Name.EqualsOrdinalIgnoreCase(name)));
 
         private static ContentTypeDefinitionRecord Acquire(
             ContentDefinitionRecord document,
@@ -376,10 +350,6 @@ private static ContentFieldDefinition Build(ContentFieldDefinitionRecord source)
         private async Task UpdateContentDefinitionRecordAsync(ContentDefinitionRecord document)
         {
             await _contentDefinitionStore.SaveContentDefinitionAsync(document);
-
-            // If multiple updates in the same scope, types and parts may need to be rebuilt.
-            _scopedTypeDefinitions.Clear();
-            _scopedPartDefinitions.Clear();
         }
 
         /// <summary>
@@ -393,10 +363,6 @@ private void CheckDocumentIdentifier(ContentDefinitionRecord document)
                 {
                     Identifier = document.Identifier,
                 };
-
-                _cachedTypeDefinitions.Clear();
-                _cachedPartDefinitions.Clear();
-
                 _memoryCache.Set(CacheKey, cacheEntry);
             }
         }

src/OrchardCore/OrchardCore.Data.YesSql/Migration/DataMigrationManager.cs

@@ -187,6 +187,8 @@ private async Task UpdateAsync(string featureId)
                 // Get current version for this migration.
                 var dataMigrationRecord = await GetDataMigrationRecordAsync(tempMigration);
 
+                var migrationName = migration.GetType().FullName;
+
                 var current = 0;
                 if (dataMigrationRecord != null)
                 {
@@ -195,7 +197,7 @@ private async Task UpdateAsync(string featureId)
                 }
                 else
                 {
-                    dataMigrationRecord = new Records.DataMigration { DataMigrationClass = migration.GetType().FullName };
+                    dataMigrationRecord = new Records.DataMigration { DataMigrationClass = migrationName };
                     _dataMigrationRecord.DataMigrations.Add(dataMigrationRecord);
                 }
 
@@ -209,7 +211,7 @@ private async Task UpdateAsync(string featureId)
 
                         if (createMethod == null)
                         {
-                            _logger.LogWarning("The migration '{name}' for '{FeatureName}' does not contain a proper Create or CreateAsync method.", migration.GetType().FullName, featureId);
+                            _logger.LogWarning("The migration '{MigrationName}' for '{FeatureName}' does not contain a proper Create or CreateAsync method.", migrationName, featureId);
                             continue;
                         }
 
@@ -220,7 +222,7 @@ private async Task UpdateAsync(string featureId)
 
                     while (lookupTable.TryGetValue(current, out var methodInfo))
                     {
-                        _logger.LogInformation("Applying migration for '{FeatureName}' from version {Version}.", featureId, current);
+                        _logger.LogInformation("Applying migration '{MigrationName}' for '{FeatureName}' from version {Version}.", migrationName, featureId, current);
 
                         current = await InvokeMethodAsync(methodInfo, migration);
                     }
@@ -235,7 +237,7 @@ private async Task UpdateAsync(string featureId)
                 }
                 catch (Exception ex)
                 {
-                    _logger.LogError(ex, "Error while running migration version {Version} for '{FeatureName}'.", current, featureId);
+                    _logger.LogError(ex, "Error while running migration '{MigrationName}' version {Version} for '{FeatureName}'.", migrationName, current, featureId);
 
                     await _session.CancelAsync();
                 }