Skip to content

Commit

Permalink
Support mapping identity columns to enum properties (#25536)
Browse files Browse the repository at this point in the history
Fixes #11970

Also, add back in the provider-agnostic check for converters with value generators, but make it less restrictive.
  • Loading branch information
ajcvickers authored Aug 17, 2021
1 parent d4dde3c commit ab0feee
Show file tree
Hide file tree
Showing 7 changed files with 907 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,8 @@ public static bool IsCompatibleWithValueGeneration(IReadOnlyProperty property)
var type = (valueConverter?.ProviderClrType ?? property.ClrType).UnwrapNullableType();

return (type.IsInteger()
|| type == typeof(decimal));
|| type.IsEnum
|| type == typeof(decimal));
}

private static bool IsCompatibleWithValueGeneration(
Expand All @@ -555,7 +556,8 @@ private static bool IsCompatibleWithValueGeneration(
var type = (valueConverter?.ProviderClrType ?? property.ClrType).UnwrapNullableType();

return (type.IsInteger()
|| type == typeof(decimal));
|| type.IsEnum
|| type == typeof(decimal));
}

/// <summary>
Expand Down
8 changes: 8 additions & 0 deletions src/EFCore/Properties/CoreStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/EFCore/Properties/CoreStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -1527,6 +1527,9 @@
<data name="ValueCannotBeNull" xml:space="preserve">
<value>The value for property '{1_entityType}.{0_property}' cannot be set to null because its type is '{propertyType}' which is not a nullable type.</value>
</data>
<data name="ValueGenWithConversion" xml:space="preserve">
<value>Value generation is not supported for property '{entityType}.{property}' because it has a '{converter}' converter configured. Configure the property to not use value generation using 'ValueGenerated.Never' or 'DatabaseGeneratedOption.None' and specify explicit values instead.</value>
</data>
<data name="VisitIsNotAllowed" xml:space="preserve">
<value>Calling '{visitMethodName}' is not allowed. Visit the expression manually for the relevant part in the visitor.</value>
</data>
Expand Down
32 changes: 31 additions & 1 deletion src/EFCore/ValueGeneration/ValueGeneratorSelector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,37 @@ public virtual ValueGenerator Select(IProperty property, IEntityType entityType)
}

private static ValueGenerator? CreateFromFactory(IProperty property, IEntityType entityType)
=> (property.GetValueGeneratorFactory() ?? property.GetTypeMapping().ValueGeneratorFactory)?.Invoke(property, entityType);
{
var factory = property.GetValueGeneratorFactory();

if (factory == null)
{
var mapping = property.GetTypeMapping();
factory = mapping.ValueGeneratorFactory;

if (factory == null)
{
var converter = mapping.Converter;

if (converter != null)
{
var type = converter.ProviderClrType.UnwrapNullableType();
if (!type.IsInteger()
&& !type.IsEnum
&& type != typeof(decimal))
{
throw new NotSupportedException(
CoreStrings.ValueGenWithConversion(
property.DeclaringEntityType.DisplayName(),
property.Name,
converter.GetType().ShortDisplayName()));
}
}
}
}

return factory?.Invoke(property, entityType);
}

/// <summary>
/// Creates a new value generator for the given property.
Expand Down
19 changes: 19 additions & 0 deletions test/EFCore.Specification.Tests/StoreGeneratedTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,25 @@ protected StoreGeneratedTestBase(TFixture fixture)

protected TFixture Fixture { get; }

[ConditionalFact]
public virtual void Value_generation_throws_for_common_cases()
{
ValueGenerationNegative<int, IntToString, NumberToStringConverter<int>>();
ValueGenerationNegative<short, ShortToBytes, NumberToBytesConverter<short>>();
}

private void ValueGenerationNegative<TKey, TEntity, TConverter>()
where TEntity : WithConverter<TKey>, new()
{
using var context = CreateContext();
Assert.Equal(
CoreStrings.ValueGenWithConversion(
typeof(TEntity).ShortDisplayName(),
nameof(WithConverter<int>.Id),
typeof(TConverter).ShortDisplayName()),
Assert.Throws<NotSupportedException>(() => context.Add(new TEntity())).Message);
}

[ConditionalFact]
public virtual void Value_generation_works_for_common_GUID_conversions()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,112 @@ public class BlogWithUIntKey
public string Name { get; set; }
}

[ConditionalFact]
public void Insert_int_enum_to_Identity_column()
{
using var testStore = SqlServerTestStore.CreateInitialized(DatabaseName);
using (var context = new BlogContextIntEnumToIdentity(testStore.Name))
{
context.Database.EnsureCreatedResiliently();

context.AddRange(
new BlogWithIntEnumKey { Name = "One Unicorn" }, new BlogWithIntEnumKey { Name = "Two Unicorns" });

context.SaveChanges();
}

using (var context = new BlogContextIntEnumToIdentity(testStore.Name))
{
var blogs = context.EnumBlogs.OrderBy(e => e.Id).ToList();

Assert.Equal(1, (int)blogs[0].Id);
Assert.Equal(2, (int)blogs[1].Id);
}
}

public class BlogContextIntEnumToIdentity : ContextBase
{
public BlogContextIntEnumToIdentity(string databaseName)
: base(databaseName)
{
}

public DbSet<BlogWithIntEnumKey> EnumBlogs { get; set; }

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
base.OnModelCreating(modelBuilder);

modelBuilder
.Entity<BlogWithIntEnumKey>()
.Property(e => e.Id)
.ValueGeneratedOnAdd();
}
}

public class BlogWithIntEnumKey
{
public IntKey Id { get; set; }
public string Name { get; set; }
}

public enum IntKey
{
}

[ConditionalFact]
public void Insert_ulong_enum_to_Identity_column()
{
using var testStore = SqlServerTestStore.CreateInitialized(DatabaseName);
using (var context = new BlogContextULongEnumToIdentity(testStore.Name))
{
context.Database.EnsureCreatedResiliently();

context.AddRange(
new BlogWithULongEnumKey { Name = "One Unicorn" }, new BlogWithULongEnumKey { Name = "Two Unicorns" });

context.SaveChanges();
}

using (var context = new BlogContextULongEnumToIdentity(testStore.Name))
{
var blogs = context.EnumBlogs.OrderBy(e => e.Id).ToList();

Assert.Equal(1, (int)blogs[0].Id);
Assert.Equal(2, (int)blogs[1].Id);
}
}

public class BlogContextULongEnumToIdentity : ContextBase
{
public BlogContextULongEnumToIdentity(string databaseName)
: base(databaseName)
{
}

public DbSet<BlogWithULongEnumKey> EnumBlogs { get; set; }

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
base.OnModelCreating(modelBuilder);

modelBuilder
.Entity<BlogWithULongEnumKey>()
.Property(e => e.Id)
.ValueGeneratedOnAdd();
}
}

public class BlogWithULongEnumKey
{
public ULongKey Id { get; set; }
public string Name { get; set; }
}

public enum ULongKey : ulong
{
}

[ConditionalFact]
public void Insert_string_to_Identity_column_using_value_converter()
{
Expand Down
Loading

0 comments on commit ab0feee

Please sign in to comment.