dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.77k stars 3.18k forks source link

ArgumentOutOfRange exception with custom IMigrationsIdGenerator #27502

Closed Danielku15 closed 10 months ago

Danielku15 commented 2 years ago

I am having a custom IMigrationsIdGenerator implementation which rather uses an incremented version number over the timestamp. Generally it is working fine, but I just came to notice that in case the migration name is too short, an ArgumentOutOfRange exception is thrown because my id generator is not used in all scenarios and the default implementation expects a minimum length and some other format.

I digged a bit deeper and found out that there might be a dependency issue that for some classes the IMigrationsIdGenerator is created too early and later not taken from the custom design time services.

Steps to reproduce

  1. Use the design time services from below and create a migration which is smaller than the usual one, e.g. dotnet ef migrations add Test (leading to a 001_Test migration)
  2. Try to create a second migration Test2 and notice the exception. Especially interesting is that not the custom id generator is used but the built-in one.
System.ArgumentOutOfRangeException: startIndex cannot be larger than length of string. (Parameter 'startIndex')
   at System.String.Substring(Int32 startIndex, Int32 length)
   at System.String.Substring(Int32 startIndex)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.MigrationsIdGenerator.GetName(String id)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.MigrationsAssembly.<>c__DisplayClass13_0.<FindMigrationId>b__1(String id)
   at System.Linq.Enumerable.WhereEnumerableIterator`1.MoveNext()
   at System.Linq.Enumerable.TryGetFirst[TSource](IEnumerable`1 source, Boolean& found)
   at System.Linq.Enumerable.FirstOrDefault[TSource](IEnumerable`1 source)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.MigrationsAssembly.FindMigrationId(String nameOrId)
   at Microsoft.EntityFrameworkCore.Migrations.Design.MigrationsScaffolder.ScaffoldMigration(String migrationName, String rootNamespace, String subNamespace, String language)
   at Microsoft.EntityFrameworkCore.Design.Internal.MigrationsOperations.AddMigration(String name, String outputDir, String contextType, String namespace)
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.AddMigrationImpl(String name, String outputDir, String contextType, String namespace)
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.AddMigration.<>c__DisplayClass0_0.<.ctor>b__0()
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.OperationBase.<>c__DisplayClass3_0`1.<Execute>b__0()
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.OperationBase.Execute(Action action)
CustomDesignTimeServices.cs ```cs using System.Diagnostics; using System.Text.RegularExpressions; using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Design; using Microsoft.EntityFrameworkCore.Migrations; using Microsoft.EntityFrameworkCore.Migrations.Design; using Microsoft.EntityFrameworkCore.Migrations.Operations; using Microsoft.Extensions.DependencyInjection.Extensions; [assembly: DesignTimeServicesReference("MyProject.CustomDesignTimeServices, MyProject")] namespace MyProject; public sealed class CustomDesignTimeServices : IDesignTimeServices { /// public void ConfigureDesignTimeServices(IServiceCollection serviceCollection) { serviceCollection.AddSingleton(); serviceCollection.Replace(ServiceDescriptor.Singleton(typeof(IMigrationsIdGenerator), typeof(MyMigrationsIdGenerator))); } } internal class MigrationSettings { public int LatestMigration { get; } public MigrationSettings(IMigrationsAssembly migrationsAssembly) { var existingMigrations = migrationsAssembly.Migrations.Keys; var currentLatestMigration = 0; foreach (var migration in existingMigrations) { var number = migration.Split('_').First(); if (number.Length != 3 || !int.TryParse(number, out var numberParsed)) { throw new InvalidOperationException( "There exists a migration that already has a malformed migration name, please fix it first. Migration: " + migration); } if (numberParsed > currentLatestMigration) { currentLatestMigration = numberParsed; } } LatestMigration = currentLatestMigration; } } internal class MyMigrationsIdGenerator : IMigrationsIdGenerator { private readonly MigrationSettings _migrationSettings; public MyMigrationsIdGenerator(MigrationSettings migrationSettings) { _migrationSettings = migrationSettings; } public string GenerateId(string name) { return $"{nextMigrationNumber:000}_{name}"; } public string GetName(string id) { return id.Substring(4); // 001_ is cut off } public bool IsValidId(string value) { return Regex.IsMatch( value, "[0-9]{{3}}_.+", default, TimeSpan.FromMilliseconds(1000.0)); } } ```

Insights on where the problem is caused

The default ID generator is created as part of this callstack:

MigrationsOperations.AddMigration
  \-> MigrationsOperations.EnsureServices
    \-> services.GetService<IMigrator> // Migrator : IMigrator has IMigrationsAssembly injected
     \-> services.GetService<IMigrationsAssembly> // MigrationsAssembly : IMigrationsAssembly has the ID generator injected
       \-> services.GetService<IMigrationsIdGenerator> resolves to the default MigrationsIdGenerator at this callstack

https://github.com/dotnet/efcore/blob/c1eeaff2c3c431343b691164c424d99cf89a2ec0/src/EFCore.Design/Design/Internal/MigrationsOperations.cs#L87

The custom ID generator is created as part of this callstack (which is a few lines after the callstack above):

MigrationsOperations.AddMigration
  \-> services.GetService<IMigrationsScaffolder>
    \-> services.GetService<MigrationsScaffolderDependencies>
     \-> services.GetService<IMigrationsIdGenerator> 
       \-> services.GetService<IMigrationsIdGenerator>

https://github.com/dotnet/efcore/blob/c1eeaff2c3c431343b691164c424d99cf89a2ec0/src/EFCore.Design/Design/Internal/MigrationsOperations.cs#L91

I'm not yet sure how a fix could look like, but at the end the correct IMigrationsIdGenerator needs to be used when calling the following method. https://github.com/dotnet/efcore/blob/c1eeaff2c3c431343b691164c424d99cf89a2ec0/src/EFCore.Design/Migrations/Design/MigrationsScaffolder.cs#L71

Include provider and version information

EF Core version: 6.0.1 Database provider Microsoft.EntityFrameworkCore.SqlServer Target framework: .NET 6.0 Operating system: Windows 11 Enterprise 21H2 22000.493 IDE: dotnet CLI for this case.

Danielku15 commented 2 years ago

Standalone project for testing.

Just run dotnet ef migrations add Test2 --project EFCore-Issue27502 and the exception appears. EFCore-Issue27502.zip

ajcvickers commented 10 months ago

Note for triage: verified that this works when the runtime service is replaced instead of the design-time service. We should document better when to replace a runtime verses design-time service.

ajcvickers commented 10 months ago

Docs covered by https://github.com/dotnet/EntityFramework.Docs/issues/4013

Danielku15 commented 4 months ago

@ajcvickers Sorry to revive this closed issue. What is your proposed solution to this problem? I just faced this problem again. We have a custom IDesignTimeServices registering a custom IMigrationsIdGenerator. Running the dotnet ef migraitons add fails within the built-in generator not using ours. Still happening in .net 8.

ajcvickers commented 4 months ago

@Danielku15 Use ReplaceService when building the DbContext options. This service is used at runtime as well as at design time, so it needs to replaced in the runtime configuration.

Danielku15 commented 4 months ago

Thanks a lot for the quick help. Got it running. Took a bit of code to reuse my current IDesignTimeServices also during runtime without negative impact but works good for my needs.

For future readers:

  1. I added custom IDesignTimeDbContextFactory<T> and register it in IDesignTimeServices.ConfigureDesignTimeServices for my DB context.
  2. I added a IDbContextOptionsExtension which I add to my options in IDesignTimeDbContextFactory.
  3. In IDbContextOptionsExtension.ApplyServices I call over to IDesignTimeServices.ConfigureDesignTimeServices

Maybe there is a nicer (built-in) way to tell a DB context to also load and register the IDesignTimeServices but I'm fine for now doing this on my own.

Disclaimer: We only do migrations manually via dotnet ef migrations and not by using the design time services and context in our code. There might be side effects in other toolings or design time services.