ConvertTriggerAnnotationsToSql can't execute at the same time for a cached model

[JustusGreiberORGADATA]

Hi,

we are using parallelized the test execution to run our integration tests and therefore use multiple DB Contexts in different threads.

I noticed that the build sometimes fails with the following exception:

Failed Execute_HasPersonsLinked_SendsEmails [5 s]
  Error Message:
   Initialization method Ofcas.Odid.IntegrationTests.InfrastructureTest.QuartzTest.JobsTest.NotifyAboutPrivacyPolicyUpdateJobTest.TestInitialize threw exception. System.InvalidOperationException: Collection was modified after the enumerator was instantiated..
  Stack Trace:
      at System.Collections.Generic.SortedSet`1.Enumerator.MoveNext()
   at System.Collections.Generic.SortedDictionary`2.Enumerator.MoveNext()
   at System.Collections.Generic.SortedDictionary`2.ValueCollection.Enumerator.MoveNext()
   at System.Linq.Enumerable.WhereEnumerableIterator`1.MoveNext()
   at Laraue.EfCoreTriggers.Common.Migrations.MigrationsModelDiffer.GetDifferences(IRelationalModel source, IRelationalModel target)
   at Microsoft.EntityFrameworkCore.Storage.RelationalDatabaseCreator.GetCreateTablesCommands(MigrationsSqlGenerationOptions options)
   at Microsoft.EntityFrameworkCore.Storage.RelationalDatabaseCreator.CreateTables()
   at Ofcas.Odid.IntegrationTests.SQLiteConnectionTestHelper.TestInitialize()

The failure is more likely the more parallel runners I add.

I investigated this and I think I found the problem. The library modifies the annotations of the model in the MigrationsModelDiffer:

Laraue.EfCoreTriggers/src/Laraue.EfCoreTriggers.Common/Migrations/MigrationsModelDiffer.cs

Lines 164 to 195 in 77b411e

	public static void ConvertTriggerAnnotationsToSql(this ITriggerVisitor triggerVisitor, IModel model)
	{
	foreach (var entityType in model?.GetEntityTypes() ?? Enumerable.Empty<IEntityType>())
	{
	var annotations = (SortedDictionary<string, Annotation>) AnnotationsField.GetValue(entityType);
	if (annotations is null)
	{
	return;
	}
	foreach (var key in annotations.Keys.ToArray())
	{
	if (!key.StartsWith(Constants.AnnotationKey))
	{
	continue;
	}
	var annotation = annotations[key];
	var value = annotation.Value;
	if (value is not ITrigger trigger)
	{
	continue;
	}
	var sql = triggerVisitor.GenerateCreateTriggerSql(trigger);
	annotations[key] = new ConventionAnnotation(key, sql, ConfigurationSource.DataAnnotation);
	}
	}
	}

This causes the SortedDict for the annotations to be modified:
https://github.com/dotnet/efcore/blob/ab0509d03fd93aecfe692348e2867257a5568d82/src/EFCore/Infrastructure/AnnotatableBase.cs#L24

The issue is caused by the fact, that EF Core build the model only once and caches the it for future calls: https://learn.microsoft.com/en-us/ef/core/modeling/dynamic-model. This means if CreateTables is called by multiple threads at the same time, one thread will modify the annotations, while the other threads holds an iterator for the annotations, by executing GetTriggerAnnotations() here:

Laraue.EfCoreTriggers/src/Laraue.EfCoreTriggers.Common/Migrations/MigrationsModelDiffer.cs

Line 70 in 77b411e

foreach (var annotation in targetModel?.FindEntityType(newTypeName).GetTriggerAnnotations() ?? Array.Empty<IAnnotation>())

I verified this by creating my own IModelCacheKeyFactory that essentially disables caching by generating a random cache key:

public class NoChachingModelCacheKeyFactory : IModelCacheKeyFactory
{
    public object Create(DbContext context, bool designTime)
        => Guid.NewGuid();
}

new DbContextOptionsBuilder<DbContext>()
                .UseSqlite(_connection)
                .UseSqlLiteTriggers(services => services.AddMethodCallConverter<TriggerMethodCallConverter>())
                .ReplaceService<IModelCacheKeyFactory, NoChachingModelCacheKeyFactory>()
                .Options;

This prevents the exception, but it is obviously not optimal, because it incurs a performance penalty.

Maybe there is a better way to solve this.