Efcore: Mechanism/API to specify a default conversion for any property of a given type in the model

[ajcvickers]

Value conversions were introduced by #242. Currently conversions are only set per-property, although bulk configuration can be used at the end of OnModelCreating in the normal way. This issue is about adding some kind of mechanism to set the conversion for all properties of a given type in the model. This could be through configuration on the model, or it could be via improved bulk config APIs.

[ajcvickers]

Note that the converter isn't really for "every property" even though it will also have that effect. It should also be for any time the CLR type is used in the query--that is, it is really a custom type mapping setup by the application.

[bugproof]

As a temporary workaround you can use this extension method(I think it works):

public static class ModelBuilderExtensions
{
    public static ModelBuilder UseValueConverterForType<T>(this ModelBuilder modelBuilder, ValueConverter converter)
    {
        return modelBuilder.UseValueConverterForType(typeof(T), converter);
    }

    public static ModelBuilder UseValueConverterForType(this ModelBuilder modelBuilder, Type type, ValueConverter converter)
    {
        foreach (var entityType in modelBuilder.Model.GetEntityTypes())
        {
            // note that entityType.GetProperties() will throw an exception, so we have to use reflection 
            var properties = entityType.ClrType.GetProperties().Where(p => p.PropertyType == type);
            foreach (var property in properties)
            {
                modelBuilder.Entity(entityType.Name).Property(property.Name)
                    .HasConversion(converter);
            }
        }

        return modelBuilder;
    }
}

[ajcvickers]

@ZeroNightzz What exception is thrown by entityType.GetProperties()?

[bugproof]

@ajcvickers

System.InvalidOperationException: The property 'CurrencyExchangeRate.Currency' could not be mapped, because it is of type 'Currency' which is not a supported primitive type or a valid entity type. Either explicitly map this property, or ignore it using the '[NotMapped]' attribute or by using 'EntityTypeBuilder.Ignore' in 'OnModelCreating'.
   at Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal.PropertyMappingValidationConvention.Apply(InternalModelBuilder modelBuilder)
   at Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal.ConventionDispatcher.ImmediateConventionScope.OnModelBuilt(InternalModelBuilder modelBuilder)
   at Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal.ConventionDispatcher.OnModelBuilt(InternalModelBuilder modelBuilder)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.Model.Validate()
   at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.CreateModel(DbContext context, IConventionSetBuilder conventionSetBuilder, IModelValidator validator)
   at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.<>c__DisplayClass5_0.<GetModel>b__1()
   at System.Lazy`1.ViaFactory(LazyThreadSafetyMode mode)
   at System.Lazy`1.ExecutionAndPublication(LazyHelper executionAndPublication, Boolean useDefaultConstructor)
   at System.Lazy`1.CreateValue()
   at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.GetModel(DbContext context, IConventionSetBuilder conventionSetBuilder, IModelValidator validator)
   at Microsoft.EntityFrameworkCore.Internal.DbContextServices.CreateModel()
   at Microsoft.EntityFrameworkCore.Internal.DbContextServices.get_Model()
   at Microsoft.EntityFrameworkCore.Infrastructure.EntityFrameworkServicesBuilder.<>c.<TryAddCoreServices>b__7_1(IServiceProvider p)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitFactory(FactoryCallSite factoryCallSite, ServiceProviderEngineScope scope)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(IServiceCallSite callSite, TArgument argument)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitScoped(ScopedCallSite scopedCallSite, ServiceProviderEngineScope scope)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(IServiceCallSite callSite, TArgument argument)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitConstructor(ConstructorCallSite constructorCallSite, ServiceProviderEngineScope scope)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(IServiceCallSite callSite, TArgument argument)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitScoped(ScopedCallSite scopedCallSite, ServiceProviderEngineScope scope)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(IServiceCallSite callSite, TArgument argument)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.DynamicServiceProviderEngine.<>c__DisplayClass1_0.<RealizeService>b__0(ServiceProviderEngineScope scope)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceProviderEngine.GetService(Type serviceType, ServiceProviderEngineScope serviceProviderEngineScope)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceProviderEngineScope.GetService(Type serviceType)
   at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService(IServiceProvider provider, Type serviceType)
   at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService[T](IServiceProvider provider)
   at Microsoft.EntityFrameworkCore.DbContext.get_DbContextDependencies()
   at Microsoft.EntityFrameworkCore.DbContext.get_InternalServiceProvider()
   at Microsoft.EntityFrameworkCore.DbContext.Microsoft.EntityFrameworkCore.Infrastructure.IInfrastructure<System.IServiceProvider>.get_Instance()
   at Microsoft.EntityFrameworkCore.Internal.InternalAccessorExtensions.GetService[TService](IInfrastructure`1 accessor)
   at Microsoft.EntityFrameworkCore.Infrastructure.AccessorExtensions.GetService[TService](IInfrastructure`1 accessor)
   at Microsoft.EntityFrameworkCore.Design.Internal.DbContextOperations.CreateContext(Func`1 factory)
   at Microsoft.EntityFrameworkCore.Design.Internal.DbContextOperations.CreateContext(String contextType)
   at Microsoft.EntityFrameworkCore.Design.Internal.MigrationsOperations.AddMigration(String name, String outputDir, String contextType)
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.AddMigrationImpl(String name, String outputDir, String contextType)
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.AddMigration.<>c__DisplayClass0_1.<.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)
The property 'CurrencyExchangeRate.Currency' could not be mapped, because it is of type 'Currency' which is not a supported primitive type or a valid entity type. Either explicitly map this property, or ignore it using the '[NotMapped]' attribute or by using 'EntityTypeBuilder.Ignore' in 'OnModelCreating'.

this is caused, so I have to use GetProperties() by accessing ClrType. I think the conversion must be known before using GetProperties() defined in EF.

[ajcvickers]

@Necronux Thanks. I thought you were indicating an exception when calling the GetProperties method itself.

[AndriySvyryd]

The bulk configuration should be performed before set discovery as it could affect whether a property would be considered a navigation. See https://github.com/dotnet/efcore/issues/12229

[ajcvickers]

Consider allowing converters to be registered only for a given provider, or only for cases where the provider doesn't have built-in support for the type being converted. See #14319

[kskalski]

Is there a way to use converters (or other mechanism) to map property (or in general its type) to a navigational property (basically what using ICollection<> would provide)?

My use-case is, I would like to have a property that is an IDictionary, which acts as navigational property containing related entities accessible by their primary key. The simple approach like below doesn't seem to work, I guess it would require re-discovering nagivational properties after their type was mapped by conversions:

    class Garden {
      public IDictionary<long, Flower> Flowers { get; set; }
    }
...
    protected override void OnModelCreating(ModelBuilder modelBuilder) {
      modelBuilder.Entity<Garden>()
        .Property(m => m.Flowers)
        .HasConversion(v => v.Values, v => v.ToDictionary(flower => flower.Id, flower => flower));
    }

A dedicated API to support IDictionary or other bags would probably allow for more efficient implementation, I guess this issue https://github.com/aspnet/EntityFrameworkCore/issues/2919 was created with that in mind. But I'm curious if there is any way available right now.

[ajcvickers]

@kskalski I'm not aware of any way to do this now; as you said, #2919 is tracking this.

[BjarkeMeier]

@backdoormanUC It works like a charm. Thx for sharing! I made a few small extensions to target my personal needs:

public static class ModelBuilderExtensions
  {
    /// <summary>
    ///   Based on BackDoorManUC's suggestion: https://github.com/aspnet/EntityFrameworkCore/issues/10784#issuecomment-415769754.
    /// </summary>
    public static ModelBuilder UseValueConverterForType<T>(this ModelBuilder modelBuilder, ValueConverter converter, string sqlType = null, bool useTypeNameSuffix = false)
    {
      return modelBuilder.UseValueConverterForType(typeof(T), converter, sqlType, useTypeNameSuffix);
    }

    public static ModelBuilder UseValueConverterForType(this ModelBuilder modelBuilder, Type type, ValueConverter converter, string sqlType = null, bool useTypeNameSuffix = false)
    {
      foreach (var entityType in modelBuilder.Model.GetEntityTypes())
      {
        // note that entityType.GetProperties() will throw an exception, so we have to use reflection 
        var properties = entityType.ClrType.GetProperties().Where(p => p.PropertyType == type);
        foreach (var property in properties)
        {
          var prop = modelBuilder.Entity(entityType.Name).Property(property.Name);
          prop.HasConversion(converter);
          if (sqlType != null)
            prop.HasColumnType(sqlType);
          if (useTypeNameSuffix)
            // Apparantly property.Name doesn't provide the current property name
            prop.HasColumnName($"{prop.Metadata.Relational().ColumnName}_{type.Name}");
        }
      }

      return modelBuilder;
    }
  }

[ghost]

@bugproof Solution works very well! But, It still needed a check for properties without setter and I changed it a bit to allow a even simpler syntax for decimal to double conversions. Besides note that it should be called twice one for the normal types and another for the nullable types:

This is the modified extension method if someone finds useful:

public static ModelBuilder UseGlobalConverter<T, B>(this ModelBuilder modelBuilder, ValueConverter? converter = null) {
            foreach (var entityType in modelBuilder.Model.GetEntityTypes()) {
                var propertiesInfo = entityType.ClrType.GetProperties().Where(p => p.PropertyType == typeof(T) && p.GetSetMethod() != null);
                foreach (var propertyInfo in propertiesInfo) {
                    if (converter == null) {
                    modelBuilder.Entity(entityType.Name).Property(propertyInfo.Name).HasConversion<B>();
                    } else {
                    modelBuilder.Entity(entityType.Name).Property(propertyInfo.Name).HasConversion(converter);
                    }
                }
            }
       return modelBuilder;
}

An you use it in OnModelCreating() like this:

modelBuilder.UseGlobalConverter<decimal?, double?>();
modelBuilder.UseGlobalConverter<decimal, double>();

[ajcvickers]

@dotnet/efcore and @AndriySvyryd in particular

API Proposal

The idea here is to create a new top-level method on ModelBuilder that will allow configuration of defaults for all properties of a given type. (As mentioned above, this will also apply to naked instances of the type in a query, so it's really about changing the type mapping, but conceptually it's easier to think about as setting defaults for a property type.)

The API mirrors the Property builder API since it is about setting defaults for properties. I went through all the property builder methods that we currently have and these seem to make sense for defaults:

```C#
protected override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.PropertyType(b =>
{
b.HasAnnotation("", "");
b.HasConversion b.IsRequired();
b.IsUnicode();
b.HasMaxLength(42);
b.HasValueGenerator();
b.IsConcurrencyToken();
b.IsRowVersion();
b.ValueGeneratedNever(); // etc.
b.UsePropertyAccessMode(PropertyAccessMode.Field);

    // Relational
    b.HasColumnType("");
    b.IsFixedLength();
    b.HasDefaultValueSql("");
    b.HasComputedColumnSql("");
    b.HasDefaultValue(0);

    // Cosmos
    b.IsEtagConcurrency();

    // Sqlite
    b.HasSrid(0);
    b.HasGeometricDimension("z");

    // SQL Server
    b.UseHiLo();
    b.HasHiLoSequence();
    b.UseIdentityColumn();
    b.HasIdentityColumnSeed();
    b.HasIdentityColumnIncrement();
});

}

I have not included
```C#
        b.ToJsonProperty("");
        b.HasField("");
        b.HasColumnName("");
        b.HasViewColumnName("");
        b.HasComment("");

because these seem very specific to individual properties.

Thoughts?

[AndriySvyryd]

This configuration can potentially impact a lot of conventions, so they would need to be rerun after this is applied. Also it itself doesn't depend on anything being in the model already, so this would be much better suited to pre-convention configration

[vslee]

If it's not too much trouble, maybe the property builders on the bottom can also be included. They might be used in some rare cases. Such as if someone wanted to put a comment on all DateTime columns, saying that all DateTimes are stored as UTC on the server. Or if they used views to combine columns for all instances of a particular type.

[ajcvickers]

@AndriySvyryd I was thinking the same thing. I was thinking we can document and enforce that these must be specified at the beginning of OnModelCreating.

[AndriySvyryd]

@ajcvickers That's too late, OnModelInitialized would've run already. We need a separate method.

[ajcvickers]

@vslee I think that's a valid option. You don't think it would be confusing to see, for example, HasField here?

[ajcvickers]

@AndriySvyryd Hmm. Good point.

[AndriySvyryd]

@ajcvickers Re: less-general configuration I agree with @vslee that there can be valid cases for all of them, but we don't need to implement everything in one go. We can start with just the most common ones.

[vslee]

I wouldn't personally be using HasField, but playing devil's advocate I could imagine, say, a large code generated set of classes which need a little bit of tweaking later. This might be done more easily with this than trying to go through to fix every little thing.
Deferring some implementations to later is fine!

[smitpatel]

Can put something hanging off OnConfiguring method? or OnModelInitializing?

[ajcvickers]

@smitpatel OnConfiguring runs too often.

[AndriySvyryd]

There's nothing bad about having an extra protected method on DbContext

[smitpatel]

OnModelInitializing

[sjb-sjb]

One problem with the workarounds specified above is that the ClrType may have properties that are not in the model. I just put in the following and so far it seems to be working for me (at least for the 10 minutes since I first compiled it).

    static public void SetValueConverters<TProperty>(this ModelBuilder modelBuilder, ValueConverter converter) where TProperty: struct
    {
        foreach (var entityType in modelBuilder.Model.GetEntityTypes()) {
            foreach (IMutableProperty property in entityType.GetProperties()) {
                if (property.ClrType == typeof(TProperty) || property.ClrType == typeof(Nullable<TProperty>)) {
                    property.SetValueConverter(converter);
                }
            }
        }
    }

I am calling this after calling base.OnModelCreating( modelBuilder).

[vslee]

A future enhancement might be to add overloads for even more configurability. For example:
```c#
IsRequired(Func predicate)
HasComment(Func selector)

Used as:
```c#
b.IsRequired(p => p.Name.StartsWith('a')) // only make properties starting with 'a' required
b.HasComment(p => $"{p.Name} is a special column") // ability to use the property name in the comment

[ajcvickers]

@vslee That's an interesting idea. Probably deserves its own issue as I doubt it will make it into the initial implementation.

[ajcvickers]

Spent some more time investigation this. I believe at this point we should punt this for 5.0 for two reasons:

• It requires significant new API for pre-initialization of the model. (Issue #12229) This isn't technically difficult, assuming we had a new protected method for it to DbContext. However, given that it's significant new API, it's probably better to do it near the start of a release when we can gather feedback and iterate on it. It could be something that comes back to bite us.
• Ideally, TypeMappingSource would return the correct mapping for a given type after this change. However, the correct mapping will now depend on the model, or at least some part of the model. This is problematic because TypeMappingSource is a singleton service and is used extensively by other singleton services. Changing these all to scoped is not feasible. Two ways I can think of that might work are:
  
  
  
  • Passing in the model whenever we ask for a type mapping from the source. This requires non-trivial flow of the model to places it isn't now.
  
  
  • Have the source return some kind of deferred mapping, which is only later resolved to the real mapping when the model is available. I've been experimenting with this and it seems promising, but it is again not a trivial change.
  
  

[AndriySvyryd]

Ideally, TypeMappingSource would return the correct mapping for a given type after this change.

There's of course another option, where the default conversion doesn't affect what TypeMappingSource returns, instead it's just a default for the properties in the model. There aren't many places where we need the TypeMapping of a type without having an associated property and for those scenarios we can consider adding alternative APIs to specify it (#4978. #21386).