Review feedback.

All.sln.DotSettings

@@ -32,6 +32,7 @@
  <s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=SuggestVarOrType_005FBuiltInTypes/@EntryIndexedValue">WARNING</s:String>
  <s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=SuggestVarOrType_005FElsewhere/@EntryIndexedValue">WARNING</s:String>
  <s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=SuggestVarOrType_005FSimpleTypes/@EntryIndexedValue">WARNING</s:String>
+ <s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=UnnecessaryWhitespace/@EntryIndexedValue">WARNING</s:String>
  <s:Boolean x:Key="/Default/CodeInspection/Highlighting/ReadSettingsFromFileLevel/@EntryValue">True</s:Boolean>
 
  <s:String x:Key="/Default/CodeStyle/CodeCleanup/Profiles/=EntityFramework/@EntryIndexedValue">&lt;?xml version="1.0" encoding="utf-16"?&gt;&lt;Profile name="EntityFramework"&gt;&lt;CSArrangeThisQualifier&gt;True&lt;/CSArrangeThisQualifier&gt;&lt;CSRemoveCodeRedundancies&gt;True&lt;/CSRemoveCodeRedundancies&gt;&lt;CSMakeFieldReadonly&gt;True&lt;/CSMakeFieldReadonly&gt;&lt;CSUseVar&gt;&lt;BehavourStyle&gt;CAN_CHANGE_TO_IMPLICIT&lt;/BehavourStyle&gt;&lt;LocalVariableStyle&gt;ALWAYS_IMPLICIT&lt;/LocalVariableStyle&gt;&lt;ForeachVariableStyle&gt;ALWAYS_IMPLICIT&lt;/ForeachVariableStyle&gt;&lt;/CSUseVar&gt;&lt;CSOptimizeUsings&gt;&lt;OptimizeUsings&gt;True&lt;/OptimizeUsings&gt;&lt;/CSOptimizeUsings&gt;&lt;CSShortenReferences&gt;True&lt;/CSShortenReferences&gt;&lt;CSReformatCode&gt;True&lt;/CSReformatCode&gt;&lt;XMLReformatCode&gt;True&lt;/XMLReformatCode&gt;&lt;CSUpdateFileHeader&gt;True&lt;/CSUpdateFileHeader&gt;&lt;CSharpFormatDocComments&gt;True&lt;/CSharpFormatDocComments&gt;&lt;RemoveCodeRedundancies&gt;True&lt;/RemoveCodeRedundancies&gt;&lt;CSMakeAutoPropertyGetOnly&gt;True&lt;/CSMakeAutoPropertyGetOnly&gt;&lt;CSArrangeQualifiers&gt;True&lt;/CSArrangeQualifiers&gt;&lt;CSEnforceVarKeywordUsageSettings&gt;True&lt;/CSEnforceVarKeywordUsageSettings&gt;&lt;CSFixBuiltinTypeReferences&gt;True&lt;/CSFixBuiltinTypeReferences&gt;&lt;CSCodeStyleAttributes ArrangeTypeAccessModifier="True" ArrangeTypeMemberAccessModifier="True" SortModifiers="True" ArrangeAttributes="True" ArrangeVarStyle="True" ArrangeBraces="True" ArrangeCodeBodyStyle="True" ArrangeObjectCreation="True" ArrangeDefaultValue="True" ArrangeNamespaces="True" /&gt;&lt;IDEA_SETTINGS&gt;&amp;lt;profile version="1.0"&amp;gt;&#xD;

src/EFCore.Relational/Update/Internal/CompositeRowForeignKeyValueFactory.cs

@@ -29,10 +29,11 @@ public CompositeRowForeignKeyValueFactory(
  : base(foreignKey.Columns)
  {
  _foreignKey = foreignKey;
- _principalKeyValueFactory = (IRowKeyValueFactory<object?[]>)((UniqueConstraint)foreignKey.PrincipalUniqueConstraint).GetRowKeyValueFactory();
+ _principalKeyValueFactory =
+ (IRowKeyValueFactory<object?[]>)((UniqueConstraint)foreignKey.PrincipalUniqueConstraint).GetRowKeyValueFactory();
 
  var columns = foreignKey.Columns;
- _valueConverters = new(columns.Count);
+ _valueConverters = new List<ValueConverter?>(columns.Count);
 
  for (var i = 0; i < columns.Count; i++)
  {

src/EFCore.Relational/Update/Internal/CompositeRowValueFactory.cs

@@ -23,7 +23,7 @@ protected CompositeRowValueFactory(IReadOnlyList<IColumn> columns)
  {
  Columns = columns;
  }
- 
+
  /// <summary>
  /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
  /// the same compatibility standards as public APIs. It may be changed or removed without notice in
@@ -38,7 +38,7 @@ protected CompositeRowValueFactory(IReadOnlyList<IColumn> columns)
  /// any release. You should only use it directly in your code with extreme caution and knowing that
  /// doing so can result in application failures when updating to a new Entity Framework Core release.
  /// </summary>
- public virtual IEqualityComparer<object?[]> EqualityComparer { get; set; } = null!;
+ public virtual IEqualityComparer<object?[]> EqualityComparer { get; protected set; } = null!;
 
  /// <summary>
  /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -92,8 +92,8 @@ public virtual bool TryCreateDependentKeyValue(IDictionary<string, object?> keyV
  /// doing so can result in application failures when updating to a new Entity Framework Core release.
  /// </summary>
  public virtual bool TryCreateDependentKeyValue(
- IReadOnlyModificationCommand command, 
- bool fromOriginalValues, 
+ IReadOnlyModificationCommand command,
+ bool fromOriginalValues,
  [NotNullWhen(true)] out object?[]? key)
  {
  var converters = ValueConverters;
@@ -124,7 +124,7 @@ public virtual bool TryCreateDependentKeyValue(
  {
  value = converter.ConvertFromProvider(value);
  }
- 
+
  if (!fromOriginalValues
  && (entry.EntityState == EntityState.Added
  || entry.EntityState == EntityState.Modified && entry.IsModified(property)))

src/EFCore.Relational/Update/Internal/RowForeignKeyValueFactory.cs

@@ -48,8 +48,9 @@ protected RowForeignKeyValueFactory(
  RelationalStrings.StoredKeyTypesNotConvertable(
  column.Name, column.StoreType, pkColumn.StoreType, pkColumn.Name));
  }
+
  _valueConverter = converterInfos.First().Create();
- 
+
  ColumnAccessors = new ColumnAccessors(
  ConvertAccessor((Func<IReadOnlyModificationCommand, (TForeignKey, bool)>)columnAccessors.CurrentValueGetter),
  ConvertAccessor((Func<IReadOnlyModificationCommand, (TForeignKey, bool)>)columnAccessors.OriginalValueGetter));
@@ -64,8 +65,8 @@ protected RowForeignKeyValueFactory(
  => command =>
  {
  var tuple = columnAccessor(command);
- return (tuple.Item1 == null 
- ? (default, tuple.Item2) 
+ return (tuple.Item1 == null
+ ? (default, tuple.Item2)
  : ((TKey)_valueConverter!.ConvertFromProvider(tuple.Item1)!, tuple.Item2))!;
  };
 

src/EFCore.Relational/Update/Internal/RowForeignKeyValueFactoryFactory.cs

@@ -35,20 +35,13 @@ public RowForeignKeyValueFactoryFactory(IValueConverterSelector valueConverterSe
  /// </summary>
  public virtual IRowForeignKeyValueFactory Create(IForeignKeyConstraint foreignKey)
  {
- try
- {
- return foreignKey.Columns.Count == 1
- ? (IRowForeignKeyValueFactory)CreateMethod
- .MakeGenericMethod(
- foreignKey.PrincipalColumns.First().ProviderClrType,
- foreignKey.Columns.First().ProviderClrType)
- .Invoke(null, new object[] { foreignKey, _valueConverterSelector })!
- : new CompositeRowForeignKeyValueFactory(foreignKey, _valueConverterSelector);
- }
- catch (TargetInvocationException exception)
- {
- throw exception.InnerException ?? exception;
- }
+ return foreignKey.Columns.Count == 1
+ ? (IRowForeignKeyValueFactory)CreateMethod
+ .MakeGenericMethod(
+ foreignKey.PrincipalColumns.First().ProviderClrType,
+ foreignKey.Columns.First().ProviderClrType)
+ .Invoke(null, new object[] { foreignKey, _valueConverterSelector })!
+ : new CompositeRowForeignKeyValueFactory(foreignKey, _valueConverterSelector);
  }
 
  private static readonly MethodInfo CreateMethod = typeof(RowForeignKeyValueFactoryFactory).GetTypeInfo()

src/EFCore.Relational/Update/Internal/SimpleNullablePrincipalRowForeignKeyValueFactory.cs

@@ -11,7 +11,7 @@ namespace Microsoft.EntityFrameworkCore.Update.Internal;
 /// any release. You should only use it directly in your code with extreme caution and knowing that
 /// doing so can result in application failures when updating to a new Entity Framework Core release.
 /// </summary>
-public class SimpleNullablePrincipalRowForeignKeyValueFactory<TKey, TNonNullableKey, TForeignKey> 
+public class SimpleNullablePrincipalRowForeignKeyValueFactory<TKey, TNonNullableKey, TForeignKey>
  : RowForeignKeyValueFactory<TKey, TForeignKey>
  where TNonNullableKey : struct
 {

test/EFCore.SqlServer.FunctionalTests/MismatchedKeyTypesSqlServerTest.cs → test/EFCore.SqlServer.FunctionalTests/Update/MismatchedKeyTypesSqlServerTest.cs

@@ -5,7 +5,7 @@
 
 using System.Transactions;
 
-namespace Microsoft.EntityFrameworkCore;
+namespace Microsoft.EntityFrameworkCore.Update;
 
 public class MismatchedKeyTypesSqlServerTest : IClassFixture<MismatchedKeyTypesSqlServerTest.MismatchedKeyTypesSqlServerFixture>
 {
@@ -27,10 +27,10 @@ public virtual void Can_update_and_delete_with_bigint_FK_and_int_PK()
 
  Assert.Equal(8, context.ChangeTracker.Entries().Count());
 
- principalEmpty.OptionalSingle = new();
- principalEmpty.RequiredSingle = new();
- principalEmpty.OptionalMany.Add(new());
- principalEmpty.RequiredMany.Add(new());
+ principalEmpty.OptionalSingle = new OptionalSingleIntLong();
+ principalEmpty.RequiredSingle = new RequiredSingleIntLong();
+ principalEmpty.OptionalMany.Add(new OptionalManyIntLong());
+ principalEmpty.RequiredMany.Add(new RequiredManyIntLong());
 
  principalPopulated.OptionalSingle = null;
  principalPopulated.RequiredSingle = null;
@@ -100,10 +100,10 @@ public virtual void Can_update_and_delete_with_tinyint_FK_and_smallint_PK()
 
  Assert.Equal(8, context.ChangeTracker.Entries().Count());
 
- principalEmpty.OptionalSingle = new();
- principalEmpty.RequiredSingle = new();
- principalEmpty.OptionalMany.Add(new());
- principalEmpty.RequiredMany.Add(new());
+ principalEmpty.OptionalSingle = new OptionalSingleShortByte();
+ principalEmpty.RequiredSingle = new RequiredSingleShortByte();
+ principalEmpty.OptionalMany.Add(new OptionalManyShortByte());
+ principalEmpty.RequiredMany.Add(new RequiredManyShortByte());
 
  principalPopulated.OptionalSingle = null;
  principalPopulated.RequiredSingle = null;
@@ -173,10 +173,10 @@ public virtual void Can_update_and_delete_with_string_FK_and_GUID_PK()
 
  Assert.Equal(8, context.ChangeTracker.Entries().Count());
 
- principalEmpty.OptionalSingle = new();
- principalEmpty.RequiredSingle = new();
- principalEmpty.OptionalMany.Add(new());
- principalEmpty.RequiredMany.Add(new());
+ principalEmpty.OptionalSingle = new OptionalSingleStringGuid();
+ principalEmpty.RequiredSingle = new RequiredSingleStringGuid();
+ principalEmpty.OptionalMany.Add(new OptionalManyStringGuid());
+ principalEmpty.RequiredMany.Add(new RequiredManyStringGuid());
 
  principalPopulated.OptionalSingle = null;
  principalPopulated.RequiredSingle = null;
@@ -246,10 +246,10 @@ public virtual void Can_update_and_delete_composite_keys_mismatched_in_store()
 
  Assert.Equal(8, context.ChangeTracker.Entries().Count());
 
- principalEmpty.OptionalSingle = new();
- principalEmpty.RequiredSingle = new();
- principalEmpty.OptionalMany.Add(new());
- principalEmpty.RequiredMany.Add(new());
+ principalEmpty.OptionalSingle = new OptionalSingleComposite();
+ principalEmpty.RequiredSingle = new RequiredSingleComposite();
+ principalEmpty.OptionalMany.Add(new OptionalManyComposite());
+ principalEmpty.RequiredMany.Add(new RequiredManyComposite());
 
  principalPopulated.OptionalSingle = null;
  principalPopulated.RequiredSingle = null;
@@ -361,7 +361,7 @@ public virtual void Queries_work_but_SaveChanges_fails_when_keys_incompatible_in
  Assert.Equal(
  RelationalStrings.StoredKeyTypesNotConvertable(
  nameof(OptionalSingleBad.PrincipalId), "uniqueidentifier", "bigint", nameof(PrincipalBad.Id)),
- Assert.Throws<InvalidOperationException>(() => context.SaveChanges()).Message);
+ Assert.Throws<TargetInvocationException>(() => context.SaveChanges()).InnerException!.Message);
  }
 
  protected class MismatchedKeyTypesContextNoFks : MismatchedKeyTypesContext
@@ -481,13 +481,24 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
  .HasValueGenerator<TemporaryByteValueGenerator>();
 
  modelBuilder.Entity<PrincipalComposite>()
- .HasKey(e => new { e.Id1, e.Id2, e.Id3 });
+ .HasKey(
+ e => new
+ {
+ e.Id1,
+ e.Id2,
+ e.Id3
+ });
 
  modelBuilder.Entity<PrincipalBadComposite>()
- .HasKey(e => new { e.Id1, e.Id2, e.Id3 });
+ .HasKey(
+ e => new
+ {
+ e.Id1,
+ e.Id2,
+ e.Id3
+ });
 
- modelBuilder.Entity<PrincipalBad>().
- Property(e => e.Id).ValueGeneratedNever();
+ modelBuilder.Entity<PrincipalBad>().Property(e => e.Id).ValueGeneratedNever();
 
  modelBuilder.Entity<OptionalSingleIntLong>().Property(e => e.PrincipalId).HasColumnType("bigint");
  modelBuilder.Entity<RequiredSingleIntLong>().Property(e => e.PrincipalId).HasColumnType("bigint");
@@ -518,11 +529,11 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
  });
 
  modelBuilder.Entity<OptionalSingleBadComposite>(
-  b =>
-  {
-  b.Property(e => e.Id).ValueGeneratedNever();
-  b.Property(e => e.PrincipalId3).HasConversion(v => new Guid(), v => 1);
-  });
+ b =>
+ {
+ b.Property(e => e.Id).ValueGeneratedNever();
+ b.Property(e => e.PrincipalId3).HasConversion(v => new Guid(), v => 1);
+ });
  }
  }