Extension method AddEnumAndValueTypeConverters is using ctors of the …

…value types for reading instead of factories because DB is "source of truth"
PawelGerr · Mar 14, 2021 · 116c95d · 116c95d
1 parent 17796d2
commit 116c95d
Show file tree

Hide file tree

Showing 10 changed files with 163 additions and 8 deletions.
diff --git a/src/Thinktecture.Runtime.Extensions.EntityFrameworkCore/Extensions/ModelBuilderExtensions.cs b/src/Thinktecture.Runtime.Extensions.EntityFrameworkCore/Extensions/ModelBuilderExtensions.cs
@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Reflection;
 using Microsoft.EntityFrameworkCore;
 using Microsoft.EntityFrameworkCore.Metadata;
 using Microsoft.EntityFrameworkCore.Metadata.Builders;
@@ -35,11 +36,29 @@ public static void AddEnumAndValueTypeConverters(
 
          foreach (var entity in modelBuilder.Model.GetEntityTypes())
          {
+            AddNonKeyedValueTypeMembers(entity);
+
             AddConverterForScalarProperties(entity, validateOnWrite, configure);
             AddConvertersForNavigations(entity, modelBuilder, validateOnWrite, configure);
          }
       }
 
+      private static void AddNonKeyedValueTypeMembers(IMutableEntityType entity)
+      {
+         if (entity.ClrType.GetCustomAttribute<KeyedValueTypeAttribute>() is not null)
+            return;
+
+         var ctorAttr = entity.ClrType.GetCustomAttribute<ValueTypeConstructorAttribute>();
+
+         if (ctorAttr is not null && ctorAttr.Members.Length != 0)
+         {
+            foreach (string memberName in ctorAttr.Members)
+            {
+               entity.AddProperty(memberName);
+            }
+         }
+      }
+
       private static void AddConvertersForNavigations(
          IMutableEntityType entity,
          ModelBuilder modelBuilder,

diff --git a/src/Thinktecture.Runtime.Extensions.SourceGenerator/CodeAnalysis/ValueTypeSourceGenerator.cs b/src/Thinktecture.Runtime.Extensions.SourceGenerator/CodeAnalysis/ValueTypeSourceGenerator.cs
@@ -131,7 +131,7 @@ private void GenerateValueType()
       internal static void ModuleInit()
       {{
          var convertFromKey = new Func<{keyMember.Type}, {_state.TypeIdentifier}>({_state.TypeIdentifier}.Create);
-         Expression<Func<{keyMember.Type}, {_state.TypeIdentifier}>> convertFromKeyExpression = {keyMember.ArgumentName} => {_state.TypeIdentifier}.Create({keyMember.ArgumentName});
+         Expression<Func<{keyMember.Type}, {_state.TypeIdentifier}>> convertFromKeyExpression = {keyMember.ArgumentName} => new {_state.TypeIdentifier}({keyMember.ArgumentName});
 
          var convertToKey = new Func<{_state.TypeIdentifier}, {keyMember.Type}>(item => item.{keyMember.Identifier});
          Expression<Func<{_state.TypeIdentifier}, {keyMember.Type}>> convertToKeyExpression = obj => obj.{keyMember.Identifier};

diff --git a/...workCore.Tests/EntityFrameworkCore/ValueConversion/ValueTypeValueConverterFactoryTests.cs b/...workCore.Tests/EntityFrameworkCore/ValueConversion/ValueTypeValueConverterFactoryTests.cs
@@ -0,0 +1,78 @@
+using System;
+using System.Threading.Tasks;
+using FluentAssertions;
+using Microsoft.EntityFrameworkCore;
+using Thinktecture.Runtime.Tests.TestEntities;
+using Thinktecture.Runtime.Tests.TestEnums;
+using Thinktecture.Runtime.Tests.TestValueTypes;
+using Xunit;
+
+namespace Thinktecture.Runtime.Tests.EntityFrameworkCore.ValueConversion
+{
+   public class ValueTypeValueConverterFactoryTests : IDisposable
+   {
+      private readonly TestDbContext _ctx;
+
+      public ValueTypeValueConverterFactoryTests()
+      {
+         _ctx = new();
+         _ctx.Database.OpenConnection();
+         _ctx.Database.EnsureCreated();
+      }
+
+      [Fact]
+      public async Task Should_write_and_read_enums_and_value_types()
+      {
+         var entity = new TestEntity_with_Enum_and_ValueTypes
+                      {
+                         Id = new Guid("A53F60CD-B53E-40E3-B16F-05E9A223E238"),
+                         TestEnum = TestEnum.Item1,
+                         IntBasedReferenceValueType = IntBasedReferenceValueType.Create(42),
+                         IntBasedStructValueType = IntBasedStructValueType.Create(43),
+                         StringBasedReferenceValueType = StringBasedReferenceValueType.Create("value 1"),
+                         StringBasedStructValueType = StringBasedStructValueType.Create("value 2"),
+                         Boundary = Boundary.Create(10, 20)
+                      };
+         _ctx.Add(entity);
+         await _ctx.SaveChangesAsync();
+
+         _ctx.ChangeTracker.Clear();
+         (await _ctx.TestEntities_with_Enum_and_ValueTypes.SingleAsync())
+            .Should().BeEquivalentTo(entity);
+      }
+
+      [Fact]
+      public async Task Should_use_ctor_of_value_types_instead_of_factory_because_EF_is_source_of_truth()
+      {
+         var entity = new TestEntity_with_Enum_and_ValueTypes
+                      {
+                         Id = new Guid("A53F60CD-B53E-40E3-B16F-05E9A223E238"),
+                         StringBasedReferenceValueType = StringBasedReferenceValueType.Create("value"),
+                         StringBasedStructValueType = StringBasedStructValueType.Create("other value"),
+                         Boundary = Boundary.Create(10, 20)
+                      };
+         _ctx.Add(entity);
+         await _ctx.SaveChangesAsync();
+
+         await using var command = _ctx.Database.GetDbConnection().CreateCommand();
+         command.CommandText = @"
+UPDATE TestEntities_with_Enum_and_ValueTypes
+SET
+    StringBasedStructValueType = '',
+    Boundary_Lower = 30
+";
+         await command.ExecuteNonQueryAsync();
+
+         _ctx.ChangeTracker.Clear();
+         var loadedEntity = await _ctx.TestEntities_with_Enum_and_ValueTypes.SingleAsync();
+         loadedEntity.StringBasedStructValueType.Property.Should().Be(String.Empty);
+         loadedEntity.Boundary.Lower.Should().Be(30);
+         loadedEntity.Boundary.Upper.Should().Be(20);
+      }
+
+      public void Dispose()
+      {
+         _ctx.Dispose();
+      }
+   }
+}
diff --git a/...ameworkCore.Tests/Extensions/ModelBuilderExtensionsTests/AddEnumAndValueTypeConverters.cs b/...ameworkCore.Tests/Extensions/ModelBuilderExtensionsTests/AddEnumAndValueTypeConverters.cs
@@ -11,17 +11,17 @@
 // ReSharper disable InconsistentNaming
 namespace Thinktecture.Runtime.Tests.Extensions.ModelBuilderExtensionsTests
 {
-   public class AddEnumAndValueTypeConverters
+   public class AddEnumAndValueTypeConverters : IDisposable
    {
       private static readonly Type _converterType = typeof(ValueTypeValueConverterFactory).GetNestedTypes(BindingFlags.NonPublic)
                                                                                           .Single(t => t.Name.StartsWith("ValidatableEnumValueConverter", StringComparison.Ordinal));
 
-      private readonly TestDbContext SUT = new();
+      private readonly TestDbContext _ctx = new();
 
       [Fact]
       public void Should_add_converters_for_owned_types()
       {
-         var entityType = SUT.Model.FindEntityType(typeof(TestEntity_with_OwnedTypes));
+         var entityType = _ctx.Model.FindEntityType(typeof(TestEntity_with_OwnedTypes));
          ValidateConverter(entityType, nameof(TestEntity_with_OwnedTypes.TestEnum));
 
          var inline_inline = entityType.FindNavigation(nameof(TestEntity_with_OwnedTypes.Inline_Inline)).TargetEntityType;
@@ -74,5 +74,10 @@ private static void ValidateConverter(IEntityType entityType, string propertyNam
       {
          entityType.FindProperty(propertyName).GetValueConverter().Should().BeOfType(_converterType);
       }
+
+      public void Dispose()
+      {
+         _ctx.Dispose();
+      }
    }
 }
diff --git a/test/Thinktecture.Runtime.Extensions.EntityFrameworkCore.Tests/TestEntities/TestDbContext.cs b/test/Thinktecture.Runtime.Extensions.EntityFrameworkCore.Tests/TestEntities/TestDbContext.cs
@@ -5,6 +5,7 @@ namespace Thinktecture.Runtime.Tests.TestEntities
    public class TestDbContext : DbContext
    {
       public DbSet<TestEntity_with_OwnedTypes> TestEntities_with_OwnedTypes { get; set; }
+      public DbSet<TestEntity_with_Enum_and_ValueTypes> TestEntities_with_Enum_and_ValueTypes { get; set; }
 
       public TestDbContext()
       {
@@ -27,6 +28,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
          base.OnModelCreating(modelBuilder);
 
          TestEntity_with_OwnedTypes.Configure(modelBuilder);
+         TestEntity_with_Enum_and_ValueTypes.Configure(modelBuilder);
 
          modelBuilder.AddEnumAndValueTypeConverters(true);
       }

diff --git a/....Extensions.EntityFrameworkCore.Tests/TestEntities/TestEntity_with_Enum_and_ValueTypes.cs b/....Extensions.EntityFrameworkCore.Tests/TestEntities/TestEntity_with_Enum_and_ValueTypes.cs
@@ -0,0 +1,31 @@
+using System;
+using Microsoft.EntityFrameworkCore;
+using Thinktecture.Runtime.Tests.TestEnums;
+using Thinktecture.Runtime.Tests.TestValueTypes;
+
+namespace Thinktecture.Runtime.Tests.TestEntities
+{
+   public class TestEntity_with_Enum_and_ValueTypes
+   {
+      public Guid Id { get; set; }
+
+      public TestEnum TestEnum { get; set; }
+      public IntBasedReferenceValueType IntBasedReferenceValueType { get; set; }
+      public IntBasedStructValueType IntBasedStructValueType { get; set; }
+      public StringBasedReferenceValueType StringBasedReferenceValueType { get; set; }
+      public StringBasedStructValueType StringBasedStructValueType { get; set; }
+      public Boundary Boundary { get; set; }
+
+      public static void Configure(ModelBuilder modelBuilder)
+      {
+         modelBuilder.Entity<TestEntity_with_Enum_and_ValueTypes>(builder =>
+                                                                  {
+                                                                     // struct are not added bey EF by default
+                                                                     builder.Property(p => p.StringBasedStructValueType);
+                                                                     builder.Property(p => p.IntBasedStructValueType);
+
+                                                                     builder.OwnsOne(e => e.Boundary);
+                                                                  });
+      }
+   }
+}
diff --git a/...me.Extensions.SourceGenerator.Tests/SourceGeneratorTests/ValueTypeSourceGeneratorTests.cs b/...me.Extensions.SourceGenerator.Tests/SourceGeneratorTests/ValueTypeSourceGeneratorTests.cs
@@ -445,7 +445,7 @@ partial class TestValueType : System.IEquatable<TestValueType?>, System.ICompara
       internal static void ModuleInit()
       {
          var convertFromKey = new Func<string, TestValueType>(TestValueType.Create);
-         Expression<Func<string, TestValueType>> convertFromKeyExpression = referenceField => TestValueType.Create(referenceField);
+         Expression<Func<string, TestValueType>> convertFromKeyExpression = referenceField => new TestValueType(referenceField);
 
          var convertToKey = new Func<TestValueType, string>(item => item.ReferenceField);
          Expression<Func<TestValueType, string>> convertToKeyExpression = obj => obj.ReferenceField;
@@ -666,7 +666,7 @@ partial class TestValueType : System.IEquatable<TestValueType?>, System.ICompara
       internal static void ModuleInit()
       {
          var convertFromKey = new Func<string, TestValueType>(TestValueType.Create);
-         Expression<Func<string, TestValueType>> convertFromKeyExpression = referenceField => TestValueType.Create(referenceField);
+         Expression<Func<string, TestValueType>> convertFromKeyExpression = referenceField => new TestValueType(referenceField);
 
          var convertToKey = new Func<TestValueType, string>(item => item.ReferenceField);
          Expression<Func<TestValueType, string>> convertToKeyExpression = obj => obj.ReferenceField;
@@ -880,7 +880,7 @@ partial class TestValueType : System.IEquatable<TestValueType?>, System.IFormatt
       internal static void ModuleInit()
       {
          var convertFromKey = new Func<int, TestValueType>(TestValueType.Create);
-         Expression<Func<int, TestValueType>> convertFromKeyExpression = referenceField => TestValueType.Create(referenceField);
+         Expression<Func<int, TestValueType>> convertFromKeyExpression = referenceField => new TestValueType(referenceField);
 
          var convertToKey = new Func<TestValueType, int>(item => item.ReferenceField);
          Expression<Func<TestValueType, int>> convertToKeyExpression = obj => obj.ReferenceField;

diff --git a/test/Thinktecture.Runtime.Extensions.Tests.Shared/TestValueTypes/Boundary.cs b/test/Thinktecture.Runtime.Extensions.Tests.Shared/TestValueTypes/Boundary.cs
@@ -0,0 +1,19 @@
+using System.ComponentModel.DataAnnotations;
+
+namespace Thinktecture.Runtime.Tests.TestValueTypes
+{
+   [ValueType]
+   public partial class Boundary
+   {
+      public decimal Lower { get; }
+      public decimal Upper { get; }
+
+      static partial void ValidateFactoryArguments(ref ValidationResult? validationResult, ref decimal lower, ref decimal upper)
+      {
+         if (lower <= upper)
+            return;
+
+         validationResult = new ValidationResult($"Lower boundary '{lower}' must be less than upper boundary '{upper}'");
+      }
+   }
+}
diff --git a/...nktecture.Runtime.Extensions.Tests.Shared/TestValueTypes/StringBasedReferenceValueType.cs b/...nktecture.Runtime.Extensions.Tests.Shared/TestValueTypes/StringBasedReferenceValueType.cs
@@ -8,7 +8,7 @@ public partial class StringBasedReferenceValueType
    {
       public string Property { get; }
 
-      static partial void ValidateFactoryArguments(ref ValidationResult validationResult, ref string property)
+      static partial void ValidateFactoryArguments(ref ValidationResult? validationResult, ref string property)
       {
          if (String.IsNullOrWhiteSpace(property))
          {

diff --git a/...cture.Runtime.Extensions.Tests.Shared/Thinktecture.Runtime.Extensions.Tests.Shared.csproj b/...cture.Runtime.Extensions.Tests.Shared/Thinktecture.Runtime.Extensions.Tests.Shared.csproj
@@ -1,6 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
    <PropertyGroup>
+      <Nullable>enable</Nullable>
       <NoWarn>$(NoWarn);CS1591;CA1052;CA1716;CA1801;CA1052;CA1707;CS1718;CA1062;CA1806;CA1822;CA1825;CA2000;CA2007;CA2234</NoWarn>
    </PropertyGroup>