Merge pull request #133 from simon-reynolds/fix-altercolumn-ops

AlterColumn operations no longer create unnecessary migrations

CHANGELOG.md

@@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+
+### Fixed
+- Prevent ManyServiceProvidersCreatedWarning exception - https://github.com/efcore/EFCore.FSharp/pull/130
+- Helper methods should allow composite keys - https://github.com/efcore/EFCore.FSharp/pull/131
+- AlterColumn operations no longer create unnecessary migrations - https://github.com/efcore/EFCore.FSharp/pull/133
+
 ## [6.0.5] - 2021-11-30
 
 ### Fixed

src/EFCore.FSharp/EFCore.FSharp.fsproj

@@ -37,6 +37,7 @@
     <Compile Include="Scaffolding\FSharpDbContextGenerator.fs" />
     <Compile Include="Scaffolding\FSharpEntityTypeGenerator.fs" />
     <Compile Include="Scaffolding\Internal\FSharpModelGenerator.fs" />
+    <Compile Include="Migrations\Internal\FSharpMigrationsModelDiffer.fs" />
     <Compile Include="Migrations\Design\FSharpSnapshotGenerator.fs" />
     <Compile Include="Migrations\Design\FSharpMigrationOperationGenerator.fs" />
     <Compile Include="Migrations\Design\FSharpMigrationsGenerator.fs" />

src/EFCore.FSharp/EFCoreFSharpServices.fs

@@ -11,6 +11,8 @@ open EntityFrameworkCore.FSharp.Scaffolding
 open EntityFrameworkCore.FSharp.Scaffolding.Internal
 open EntityFrameworkCore.FSharp.Migrations.Design
 open EntityFrameworkCore.FSharp.Internal
+open Microsoft.EntityFrameworkCore.Migrations
+open EntityFrameworkCore.FSharp.Migrations.Internal
 
 type EFCoreFSharpServices(scaffoldOptions: ScaffoldOptions) =
 
@@ -27,6 +29,7 @@ type EFCoreFSharpServices(scaffoldOptions: ScaffoldOptions) =
             services
                 .AddSingleton<ScaffoldOptions>(scaffoldOptions)
                 .AddSingleton<ICSharpHelper, FSharpHelper>()
+                .AddSingleton<IMigrationsModelDiffer, FSharpMigrationsModelDiffer>()
                 .AddSingleton<ICSharpEntityTypeGenerator, FSharpEntityTypeGenerator>()
                 .AddSingleton<ICSharpDbContextGenerator, FSharpDbContextGenerator>()
                 .AddSingleton<IModelCodeGenerator, FSharpModelGenerator>()

src/EFCore.FSharp/Migrations/Design/FSharpSnapshotGenerator.fs

@@ -135,8 +135,8 @@ type FSharpSnapshotGenerator
             None
 
     let generateFluentApiForDefaultValue (property: IProperty) =
-        match property.GetDefaultValue() |> Option.ofObj with
-        | Some defaultValue ->
+        match property.TryGetDefaultValue() with
+        | true, defaultValue ->
             let valueConverter =
                 if defaultValue <> (box DBNull.Value) then
                     let valueConverter =
@@ -169,7 +169,7 @@ type FSharpSnapshotGenerator
             $".HasDefaultValue({appendValueConverter})"
             |> Some
 
-        | None -> None
+        | _ -> None
 
     let genPropertyAnnotations propertyBuilderName (property: IProperty) =
         let annotations =

src/EFCore.FSharp/Migrations/Internal/FSharpMigrationsModelDiffer.fs

@@ -0,0 +1,68 @@
+namespace EntityFrameworkCore.FSharp.Migrations.Internal
+
+open System.Runtime.InteropServices
+open Microsoft.EntityFrameworkCore.Metadata
+open Microsoft.EntityFrameworkCore.Migrations.Internal
+open Microsoft.EntityFrameworkCore.Migrations.Operations
+open EntityFrameworkCore.FSharp.SharedTypeExtensions
+open Microsoft.EntityFrameworkCore.Metadata.Internal
+
+type FSharpMigrationsModelDiffer
+    (
+        typeMappingSource,
+        migrationsAnnotations,
+        changeDetector,
+        updateAdapterFactory,
+        commandBatchPreparerDependencies
+    ) =
+    inherit MigrationsModelDiffer
+        (
+            typeMappingSource,
+            migrationsAnnotations,
+            changeDetector,
+            updateAdapterFactory,
+            commandBatchPreparerDependencies
+        )
+
+    let isNullableType (p: IProperty) =
+        let clrType = p.ClrType
+        let isPrimaryKey = p.IsPrimaryKey()
+
+        let isNullable =
+            (isOptionType clrType || isNullableType clrType)
+
+        isNullable && not isPrimaryKey
+
+    override _.Diff
+        (
+            source: IColumn,
+            target: IColumn,
+            diffContext: MigrationsModelDiffer.DiffContext
+        ) : MigrationOperation seq =
+
+        System.Console.WriteLine("Diffing columns")
+
+        let sourceTypeProperty =
+            (source.PropertyMappings |> Seq.head).Property
+
+        let targetTypeProperty =
+            (target.PropertyMappings |> Seq.head).Property
+
+        (source :?> Column).IsNullable <- isNullableType sourceTypeProperty
+        (target :?> Column).IsNullable <- isNullableType targetTypeProperty
+
+        base.Diff(source, target, diffContext)
+
+    override _.Add
+        (
+            source: IColumn,
+            diffContext: MigrationsModelDiffer.DiffContext,
+            [<Optional; DefaultParameterValue(false)>] ``inline``: bool
+        ) : MigrationOperation seq =
+
+        let sourceTypeProperty =
+            (source.PropertyMappings |> Seq.head).Property
+
+        (source :?> Column).IsNullable <- isNullableType sourceTypeProperty
+
+        base.Add(source, diffContext, ``inline``)

tests/EFCore.FSharp.Tests/Migrations/Design/FSharpMigrationsGeneratorTest.fs

@@ -489,9 +489,9 @@ module FSharpMigrationsGeneratorTest =
                       |> HashSet
 
                   let columnMapping =
-                      $"{_nl}.HasColumnType(\"default_int_mapping\"){_nl}"
+                      $"{_nl}.HasColumnType(\"default_int_mapping\")"
 
-                  let columnMappingWithDefaultValue = $"{columnMapping}.HasDefaultValue(0)"
+                  let columnMappingWithDefaultValue = $"{columnMapping}"
 
                   let forProperty =
                       [ (CoreAnnotationNames.MaxLength,
@@ -504,19 +504,16 @@ module FSharpMigrationsGeneratorTest =
                         (CoreAnnotationNames.ValueConverter,
                          (box (ValueConverter<int, int64>((fun v -> v |> int64), (fun v -> v |> int), null)),
                           _nl
-                          + $".HasColumnType(\"default_long_mapping\"){_nl}.HasDefaultValue(0L){_nl}|> ignore"))
+                          + $".HasColumnType(\"default_long_mapping\"){_nl}|> ignore"))
                         (CoreAnnotationNames.ProviderClrType,
-                         (box typeof<int64>,
-                          $"{_nl}.HasColumnType(\"default_long_mapping\"){_nl}.HasDefaultValue(0L){_nl}|> ignore"))
+                         (box typeof<int64>, $"{_nl}.HasColumnType(\"default_long_mapping\"){_nl}|> ignore"))
                         (RelationalAnnotationNames.ColumnName,
                          (box "MyColumn",
                           columnMappingWithDefaultValue
                           + _nl
                           + $".HasColumnName(\"MyColumn\") |> ignore"))
                         (RelationalAnnotationNames.ColumnType,
-                         (box "int",
-                          _nl
-                          + $".HasColumnType(\"int\"){_nl}.HasDefaultValue(0){_nl}|> ignore"))
+                         (box "int", _nl + $".HasColumnType(\"int\"){_nl}|> ignore"))
                         (RelationalAnnotationNames.DefaultValueSql,
                          (box "some SQL",
                           columnMappingWithDefaultValue
@@ -534,7 +531,7 @@ module FSharpMigrationsGeneratorTest =
                         (RelationalAnnotationNames.DefaultValue,
                          (box 0,
                           columnMapping
-                          + $".HasDefaultValue(0){_nl}|> ignore"))
+                          + $"{_nl}.HasDefaultValue(0){_nl}|> ignore"))
                         (RelationalAnnotationNames.IsFixedLength,
                          (box true,
                           columnMappingWithDefaultValue
@@ -769,7 +766,6 @@ module FSharpMigrationsGeneratorTest =
                           "            b.Property<int>(\"Id\")"
                           "                .IsRequired(true)"
                           "                .HasColumnType(\"int\")"
-                          "                .HasDefaultValue(0)"
                           "                |> ignore"
                           ""
                           "            b.Property<string>(\"C2\")"
@@ -780,7 +776,6 @@ module FSharpMigrationsGeneratorTest =
                           "            b.Property<int>(\"C3\")"
                           "                .IsRequired(true)"
                           "                .HasColumnType(\"int\")"
-                          "                .HasDefaultValue(0)"
                           "                |> ignore"
                           ""
                           "            b.HasKey(\"Id\")"
@@ -916,7 +911,6 @@ module FSharpMigrationsGeneratorTest =
                           "            b.Property<string>(\"Ham\")"
                           "                .IsRequired(true)"
                           "                .HasColumnType(\"just_string(10)\")"
-                          "                .HasDefaultValue(\"A\")"
                           "                |> ignore"
                           ""
                           "            b.Property<string>(\"Pickle\")"
@@ -938,13 +932,11 @@ module FSharpMigrationsGeneratorTest =
                           "                .IsRequired(true)"
                           "                .ValueGeneratedOnAdd()"
                           "                .HasColumnType(\"default_int_mapping\")"
-                          "                .HasDefaultValue(0)"
                           "                |> ignore"
                           ""
                           "            b.Property<Guid>(\"PropertyWithValueGenerator\")"
                           "                .IsRequired(true)"
                           "                .HasColumnType(\"default_guid_mapping\")"
-                          "                .HasDefaultValue(Guid(\"00000000-0000-0000-0000-000000000000\"))"
                           "                |> ignore"
                           ""
                           "            b.HasKey(\"Id\")"