Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ValueConverters not considered with Sequence generators #30749

Closed
Soundman32 opened this issue Apr 24, 2023 · 8 comments
Closed

ValueConverters not considered with Sequence generators #30749

Soundman32 opened this issue Apr 24, 2023 · 8 comments
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported

Comments

@Soundman32
Copy link

Soundman32 commented Apr 24, 2023

My code wraps primary key values in a type. I've configured the usual TypeConverter/ValueConverter and reading and writing to the database works perfectly. This project uses sequences to generate PK values, and when I configure it, I get this exception when a new record is created:

System.ArgumentException: The 'SqlServerSequenceValueGeneratorFactory' cannot create a value generator for property 'MyClass.MyIdType'. Only integer properties are supported.
   at Microsoft.EntityFrameworkCore.SqlServer.ValueGeneration.Internal.SqlServerSequenceValueGeneratorFactory.Create(IProperty property, SqlServerSequenceValueGeneratorState generatorState, ISqlServerConnection connection, IRawSqlCommandBuilder rawSqlCommandBuilder, IRelationalCommandDiagnosticsLogger commandLogger)
   at Microsoft.EntityFrameworkCore.SqlServer.ValueGeneration.Internal.SqlServerValueGeneratorSelector.Select(IProperty property, IEntityType entityType)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.ValueGenerationManager.Generate(InternalEntityEntry entry, Boolean includePrimaryKey)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.SetEntityState(EntityState entityState, Boolean acceptChanges, Boolean modifyProperties, Nullable`1 forceStateWhenUnknownKey)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityGraphAttacher.PaintAction(EntityEntryGraphNode`1 node)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityEntryGraphIterator.TraverseGraph[TState](EntityEntryGraphNode`1 node, Func`2 handleNode)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityGraphAttacher.AttachGraph(InternalEntityEntry rootEntry, EntityState targetState, EntityState storeGeneratedWithKeySetTargetState, Boolean forceStateWhenUnknownKey)
   at Microsoft.EntityFrameworkCore.Internal.InternalDbSet`1.Add(TEntity entity)
  ... MY CODE

I suspect this line in:

public virtual ValueGenerator Create(
            IProperty property,
            SqlServerSequenceValueGeneratorState generatorState,
            ISqlServerConnection connection,
            IRawSqlCommandBuilder rawSqlCommandBuilder,
            IRelationalCommandDiagnosticsLogger commandLogger)
        {
            var type = property.ClrType.UnwrapNullableType().UnwrapEnumType();

If that line is replaced with

      var type = property.FindTypeMapping()?.Converter?.ProviderClrType;

This will then run the sequence generator, but I then get a later InvalidCastException when it tries to assign the value to the type.

System.InvalidCastException: Unable to cast object of type 'System.Int64' to type MyAssembly.Types.MyScheduleId'.
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.SetProperty(IPropertyBase propertyBase, Object value, Boolean isMaterialization, Boolean setModified, Boolean isCascadeDelete, CurrentValueType valueType)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.ValueGenerationManager.Generate(InternalEntityEntry entry, Boolean includePrimaryKey)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.SetEntityState(EntityState entityState, Boolean acceptChanges, Boolean modifyProperties, Nullable`1 forceStateWhenUnknownKey)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityGraphAttacher.PaintAction(EntityEntryGraphNode`1 node)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityEntryGraphIterator.TraverseGraph[TState](EntityEntryGraphNode`1 node, Func`2 handleNode)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityGraphAttacher.AttachGraph(InternalEntityEntry rootEntry, EntityState targetState, EntityState storeGeneratedWithKeySetTargetState, Boolean forceStateWhenUnknownKey)
   at Microsoft.EntityFrameworkCore.Internal.InternalDbSet`1.Add(TEntity entity)

Include provider and version information

EF Core version: 6
Database provider: Oracle (but duplicated in Sql Server too)
Target framework: NET6
Operating system: Windows 10/11
IDE: Visual Studio 2022 17.5.4

@ajcvickers
Copy link
Member

Duplicate of #11597.

@Soundman32
Copy link
Author

I don't consider this a duplicate, as it specifically doesn't work with sequences, but the value converters DO work reading and writing ordinary properties.

@ajcvickers
Copy link
Member

@Soundman32 Does the issue still happen with EF Core 7.0? If so, then please attach a small, runnable project or post a small, runnable code listing that reproduces what you are seeing so that we can investigate.

@Soundman32
Copy link
Author

I've started creating a small example, but when I create a migration, I get:

SQL Server sequences cannot be used to generate values for the property 'Id' on entity type 'Order' because the property type is 'OrderId'. Sequences can only be used with integer properties.

Can you confirm that value converters with sequences are supported?

@ajcvickers
Copy link
Member

@Soundman32 Here's an example, and output showing the sequences, tables, and SQL.

using System;
using System.Collections.Generic;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
using Microsoft.Extensions.Logging;

using (var context = new SomeDbContext())
{
    await context.Database.EnsureDeletedAsync();
    await context.Database.EnsureCreatedAsync();

    await context.AddRangeAsync(
        new Category("Foods") { Products = { new("Marmite"), new("Toast"), new("Butter") } },
        new Category("Beverages") { Products = { new("Tea"), new("Milk") } });

    await context.SaveChangesAsync();
}

using (var context = new SomeDbContext())
{
    var categoriesAndProducts = await context.Categories.Include(category => category.Products).ToListAsync();

    Console.WriteLine();
    foreach (var category in categoriesAndProducts)
    {
        Console.WriteLine($"Category {category.Id.Value} is '{category.Name}'");
        foreach (var product in category.Products)
        {
            Console.WriteLine($"   Product {product.Id.Value} is '{product.Name}'");
        }
    }
    Console.WriteLine();
}

public readonly struct ProductId
{
    public ProductId(int value) => Value = value;
    public int Value { get; }
}

public readonly struct CategoryId
{
    public CategoryId(int value) => Value = value;
    public int Value { get; }
}

public class Product
{
    public Product(string name) => Name = name;
    public ProductId Id { get; set; }
    public string Name { get; set; }
    public CategoryId CategoryId { get; set; }
    public Category Category { get; set; } = null!;
}

public class Category
{
    public Category(string name) => Name = name;
    public CategoryId Id { get; set; }
    public string Name { get; set; }
    public List<Product> Products { get; } = new();
}

public class SomeDbContext : DbContext
{
    public DbSet<Product> Products => Set<Product>();
    public DbSet<Category> Categories => Set<Category>();

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder
            .UseSqlServer(@"Data Source=(LocalDb)\MSSQLLocalDB;Database=AllTogetherNow")
            .LogTo(Console.WriteLine, LogLevel.Information)
            .EnableSensitiveDataLogging();
    }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.UseKeySequences();
        modelBuilder.Entity<Product>().Property(product => product.Id).ValueGeneratedOnAdd();
        modelBuilder.Entity<Category>().Property(category => category.Id).ValueGeneratedOnAdd();
    }

    protected override void ConfigureConventions(ModelConfigurationBuilder configurationBuilder)
    {
        configurationBuilder.Properties<ProductId>().HaveConversion<ProductIdConverter>();
        configurationBuilder.Properties<CategoryId>().HaveConversion<CategoryIdConverter>();
    }

    private class ProductIdConverter : ValueConverter<ProductId, int>
    {
        public ProductIdConverter()
            : base(v => v.Value, v => new(v))
        {
        }
    }

    private class CategoryIdConverter : ValueConverter<CategoryId, int>
    {
        public CategoryIdConverter()
            : base(v => v.Value, v => new(v))
        {
        }
    }
}
warn: 4/25/2023 20:40:47.743 CoreEventId.SensitiveDataLoggingEnabledWarning[10400] (Microsoft.EntityFrameworkCore.Infrastructure)
      Sensitive data logging is enabled. Log entries and exception messages may include sensitive application data; this mode should only be enabled during development.
info: 4/25/2023 20:40:48.261 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command)
      Executed DbCommand (33ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      SELECT 1
info: 4/25/2023 20:40:48.330 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command)
      Executed DbCommand (3ms) [Parameters=[], CommandType='Text', CommandTimeout='60']
      IF SERVERPROPERTY('EngineEdition') <> 5
      BEGIN
          ALTER DATABASE [AllTogetherNow] SET SINGLE_USER WITH ROLLBACK IMMEDIATE;
      END;
info: 4/25/2023 20:40:48.341 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command)
      Executed DbCommand (11ms) [Parameters=[], CommandType='Text', CommandTimeout='60']
      DROP DATABASE [AllTogetherNow];
info: 4/25/2023 20:40:48.497 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command)
      Executed DbCommand (131ms) [Parameters=[], CommandType='Text', CommandTimeout='60']
      CREATE DATABASE [AllTogetherNow];
info: 4/25/2023 20:40:48.531 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command)
      Executed DbCommand (33ms) [Parameters=[], CommandType='Text', CommandTimeout='60']
      IF SERVERPROPERTY('EngineEdition') <> 5
      BEGIN
          ALTER DATABASE [AllTogetherNow] SET READ_COMMITTED_SNAPSHOT ON;
      END;
info: 4/25/2023 20:40:48.535 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command)
      Executed DbCommand (1ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      SELECT 1
info: 4/25/2023 20:40:48.652 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command)
      Executed DbCommand (4ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      CREATE SEQUENCE [CategorySequence] START WITH 1 INCREMENT BY 1 NO MINVALUE NO MAXVALUE NO CYCLE;
info: 4/25/2023 20:40:48.654 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command)
      Executed DbCommand (1ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      CREATE SEQUENCE [ProductSequence] START WITH 1 INCREMENT BY 1 NO MINVALUE NO MAXVALUE NO CYCLE;
info: 4/25/2023 20:40:48.659 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command)
      Executed DbCommand (4ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      CREATE TABLE [Categories] (
          [Id] int NOT NULL DEFAULT (NEXT VALUE FOR [CategorySequence]),
          [Name] nvarchar(max) NOT NULL,
          CONSTRAINT [PK_Categories] PRIMARY KEY ([Id])
      );
info: 4/25/2023 20:40:48.662 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command)
      Executed DbCommand (2ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      CREATE TABLE [Products] (
          [Id] int NOT NULL DEFAULT (NEXT VALUE FOR [ProductSequence]),
          [Name] nvarchar(max) NOT NULL,
          [CategoryId] int NOT NULL,
          CONSTRAINT [PK_Products] PRIMARY KEY ([Id]),
          CONSTRAINT [FK_Products_Categories_CategoryId] FOREIGN KEY ([CategoryId]) REFERENCES [Categories] ([Id]) ON DELETE CASCADE
      );
info: 4/25/2023 20:40:48.665 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command)
      Executed DbCommand (1ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      CREATE INDEX [IX_Products_CategoryId] ON [Products] ([CategoryId]);
info: 4/25/2023 20:40:48.976 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command)
      Executed DbCommand (32ms) [Parameters=[@p0='Foods' (Nullable = false) (Size = 4000), @p1='Beverages' (Nullable = false) (Size = 4000)], CommandType='Text', CommandTimeout='30']
      SET IMPLICIT_TRANSACTIONS OFF;
      SET NOCOUNT ON;
      MERGE [Categories] USING (
      VALUES (@p0, 0),
      (@p1, 1)) AS i ([Name], _Position) ON 1=0
      WHEN NOT MATCHED THEN
      INSERT ([Name])
      VALUES (i.[Name])
      OUTPUT INSERTED.[Id], i._Position;
info: 4/25/2023 20:40:49.034 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command)
      Executed DbCommand (6ms) [Parameters=[@p2='1', @p3='Marmite' (Nullable = false) (Size = 4000), @p4='1', @p5='Toast' (Nullable = false) (Size = 4000), @p6='1', @p7='Butter' (Nullable = false) (Size = 4000), @p8='2', @p9='Tea' (Nullable = false) (Size = 4000), @p10='2', @p11='Milk' (Nullable = false) (Size
= 4000)], CommandType='Text', CommandTimeout='30']
      SET IMPLICIT_TRANSACTIONS OFF;
      SET NOCOUNT ON;
      MERGE [Products] USING (
      VALUES (@p2, @p3, 0),
      (@p4, @p5, 1),
      (@p6, @p7, 2),
      (@p8, @p9, 3),
      (@p10, @p11, 4)) AS i ([CategoryId], [Name], _Position) ON 1=0
      WHEN NOT MATCHED THEN
      INSERT ([CategoryId], [Name])
      VALUES (i.[CategoryId], i.[Name])
      OUTPUT INSERTED.[Id], i._Position;
info: 4/25/2023 20:40:49.435 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command)
      Executed DbCommand (4ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      SELECT [c].[Id], [c].[Name], [p].[Id], [p].[CategoryId], [p].[Name]
      FROM [Categories] AS [c]
      LEFT JOIN [Products] AS [p] ON [c].[Id] = [p].[CategoryId]
      ORDER BY [c].[Id]

Category 1 is 'Foods'
   Product 1 is 'Marmite'
   Product 2 is 'Toast'
   Product 3 is 'Butter'
Category 2 is 'Beverages'
   Product 4 is 'Tea'
   Product 5 is 'Milk'

@Soundman32
Copy link
Author

Thanks @ajcvickers

My project has 100s of id types, so adding each one manually to ConfigureConventions would be difficult to manage.

I've taken your example and added a custom ValueConverterSelector, removed the explicit .HasConversion and this breaks the id generation. I'm not really sure why.

using (var context = new SomeDbContext())
{
    await context.Database.EnsureDeletedAsync();
    await context.Database.EnsureCreatedAsync();

    await context.AddRangeAsync(
        new Category("Foods") { Products = { new("Marmite"), new("Toast"), new("Butter") } },
        new Category("Beverages") { Products = { new("Tea"), new("Milk") } });

    await context.SaveChangesAsync();
}

using (var context = new SomeDbContext())
{
    var categoriesAndProducts = await context.Categories.Include(category => category.Products).ToListAsync();

    Console.WriteLine();
    foreach (var category in categoriesAndProducts)
    {
        Console.WriteLine($"Category {category.Id.Value} is '{category.Name}'");
        foreach (var product in category.Products)
        {
            Console.WriteLine($"   Product {product.Id.Value} is '{product.Name}'");
        }
    }
    Console.WriteLine();
}

public readonly struct ProductId
{
    public ProductId(long value) => Value = value;
    public long Value { get; }
}

public readonly struct CategoryId
{
    public CategoryId(int value) => Value = value;
    public int Value { get; }
}

public class Product
{
    public Product(string name) => Name = name;
    public ProductId Id { get; set; }
    public string Name { get; set; }
    public CategoryId CategoryId { get; set; }
    public Category Category { get; set; } = null!;
}

public class Category
{
    public Category(string name) => Name = name;
    public CategoryId Id { get; set; }
    public string Name { get; set; }
    public List<Product> Products { get; } = new();
}

public class SomeDbContext : DbContext
{
    public DbSet<Product> Products => Set<Product>();
    public DbSet<Category> Categories => Set<Category>();

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder
            .UseSqlServer(@"Server=.;Database=AllTogether;Trusted_Connection=True;TrustServerCertificate=True;MultipleActiveResultSets=true")
            .LogTo(Console.WriteLine, LogLevel.Information)
            .EnableSensitiveDataLogging()
            .ReplaceService<IValueConverterSelector, CustomValueConverterSelector>()
            ;
    }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<Product>()
            .Property(product => product.Id)
            .ValueGeneratedOnAdd()
            .UseHiLo();
        modelBuilder.Entity<Category>()
            .Property(category => category.Id)
            .ValueGeneratedOnAdd()
            .UseIdentityColumn();
    }

    protected override void ConfigureConventions(ModelConfigurationBuilder configurationBuilder)
    {
        //configurationBuilder.Properties<ProductId>().HaveConversion<ProductIdConverter>();
        //configurationBuilder.Properties<CategoryId>().HaveConversion<CategoryIdConverter>();
    }

    public class ProductIdConverter : ValueConverter<ProductId, long>
    {
        public ProductIdConverter()
            : base(v => v.Value, v => new(v))
        {
        }
    }

    public class CategoryIdConverter : ValueConverter<CategoryId, int>
    {
        public CategoryIdConverter()
            : base(v => v.Value, v => new(v))
        {
        }
    }
}

public class CustomValueConverterSelector : ValueConverterSelector
{
    private readonly ConcurrentDictionary<(Type ModelClrType, Type ProviderClrType), ValueConverterInfo> _converters
        = new();

    public CustomValueConverterSelector(ValueConverterSelectorDependencies dependencies)
        : base(dependencies)
    {
    }

    public override IEnumerable<ValueConverterInfo> Select(Type modelClrType, Type? providerClrType = null!)
    {
        _ = modelClrType ?? throw new ArgumentNullException(nameof(modelClrType));

        foreach (var converter in base.Select(modelClrType, providerClrType))
        {
            yield return converter;
        }

        var underlyingModelType = UnwrapNullableType(modelClrType);
        var underlyingProviderType = UnwrapNullableType(providerClrType);

        if (typeof(ProductId).IsAssignableFrom(underlyingModelType) && IsValidProviderType<ProductId>(underlyingProviderType!))
        {
            yield return GetOrAddValueConverter(modelClrType, underlyingModelType, typeof(ProductIdConverter), typeof(long));
        }

        if (typeof(CategoryId).IsAssignableFrom(underlyingModelType) && IsValidProviderType<CategoryId>(underlyingProviderType!))
        {
            yield return GetOrAddValueConverter(modelClrType, underlyingModelType, typeof(CategoryIdConverter), typeof(int));
        }
    }

    private static bool IsValidProviderType<T>(Type underlyingProviderType)
    {
        return underlyingProviderType is null || underlyingProviderType == typeof(T);
    }

    private static Type? UnwrapNullableType(Type? providerClrType)
    {
        return providerClrType is null ? null : Nullable.GetUnderlyingType(providerClrType) ?? providerClrType;
    }

    private ValueConverterInfo GetOrAddValueConverter(Type modelClrType, Type underlyingModelType, Type converterType, Type providerType)
    {
        return _converters.GetOrAdd(
            (underlyingModelType, providerType),
            k =>
            {
                Func<ValueConverterInfo, ValueConverter> factory = 
                    info => (ValueConverter)Activator.CreateInstance(converterType)!;

                return new ValueConverterInfo(modelClrType, providerType, factory);
            });
    }
}

@ajcvickers
Copy link
Member

Note for triage: validation in the model building process doesn't account for converters that will be selected later. We could probably fix this, but it looks like at least the ValueGeneratorSelector would also need to be replaced to make this work, and maybe other things.

@Soundman32 You might have more look doing this using some other form of bulk configuration.

@ajcvickers
Copy link
Member

Note from triage: we don't plan to change this behavior unless there is a case where doing so would result in a working solution. Currently, that doesn't appear to be the case.

@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale May 4, 2023
@ajcvickers ajcvickers added the closed-no-further-action The issue is closed and no further action is planned. label May 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported
Projects
None yet
Development

No branches or pull requests

2 participants