Skip to content

Commit

Permalink
Revert breaking change to shadow foreign key targeting generic types.
Browse files Browse the repository at this point in the history
Fixes #28962
  • Loading branch information
AndriySvyryd authored Sep 3, 2022
1 parent 823be49 commit 2543d4b
Show file tree
Hide file tree
Showing 6 changed files with 89 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ private void Create(IStoredProcedureParameter parameter, CSharpRuntimeAnnotation
{
var code = Dependencies.CSharpHelper;
var mainBuilder = parameters.MainBuilder;
var parameterVariable = code.Identifier(parameter.Name, parameters.ScopeVariables, capitalize: false);
var parameterVariable = code.Identifier(parameter.PropertyName ?? parameter.Name, parameters.ScopeVariables, capitalize: false);

mainBuilder
.Append("var ").Append(parameterVariable).Append(" = ")
Expand Down
27 changes: 25 additions & 2 deletions src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,30 @@ public static string GetDefaultColumnBaseName(this IReadOnlyProperty property)
/// <param name="property">The property.</param>
/// <returns>The default base column name to which the property would be mapped.</returns>
public static string GetDefaultColumnName(this IReadOnlyProperty property)
=> Uniquifier.Truncate(property.Name, property.DeclaringEntityType.Model.GetMaxIdentifierLength());
{
var name = property.Name;
if (property.IsShadowProperty()
&& property.GetContainingForeignKeys().Count() == 1)
{
var foreignKey = property.GetContainingForeignKeys().First();
var principalEntityType = foreignKey.PrincipalEntityType;
if (!principalEntityType.HasSharedClrType
&& principalEntityType.ClrType.IsConstructedGenericType
&& foreignKey.DependentToPrincipal == null)
{
var principalProperty = property.FindFirstPrincipal()!;
var principalName = principalEntityType.ShortName();
if (property.Name.Length == (principalName.Length + principalProperty.Name.Length)
&& property.Name.StartsWith(principalName, StringComparison.Ordinal)
&& property.Name.EndsWith(principalProperty.Name, StringComparison.Ordinal))
{
name = principalEntityType.ClrType.ShortDisplayName() + principalProperty.Name;
}
}
}

return Uniquifier.Truncate(name, property.DeclaringEntityType.Model.GetMaxIdentifierLength());
}

/// <summary>
/// Returns the default column name to which the property would be mapped.
Expand Down Expand Up @@ -213,7 +236,7 @@ public static string GetDefaultColumnName(this IReadOnlyProperty property)
entityType = ownerType;
}

var baseName = property.GetDefaultColumnName();
var baseName = storeObject.StoreObjectType == StoreObjectType.Table ? property.GetDefaultColumnName() : property.Name;
if (builder == null)
{
return baseName;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public InternalStoredProcedureParameterBuilder(
}

Metadata.SetName(name, configurationSource);

return this;
}

Expand All @@ -58,7 +58,7 @@ public virtual bool CanSetName(
ConfigurationSource configurationSource)
=> configurationSource.Overrides(Metadata.GetNameConfigurationSource())
|| Metadata.Name == name;

/// <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
Expand All @@ -75,7 +75,7 @@ public virtual bool CanSetName(
}

Metadata.SetDirection(direction, configurationSource);

return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2614,9 +2614,9 @@ public static void CreateAnnotations(RuntimeEntityType runtimeEntityType)
""PrincipalBaseId"", ParameterDirection.Input, false, ""PrincipalBaseId"", false);
var principalDerivedId = insertSproc.AddParameter(
""PrincipalDerivedId"", ParameterDirection.Input, false, ""PrincipalDerivedId"", false);
var baseId = insertSproc.AddParameter(
var id = insertSproc.AddParameter(
""BaseId"", ParameterDirection.Output, false, ""Id"", false);
baseId.AddAnnotation(""foo"", ""bar"");
id.AddAnnotation(""foo"", ""bar"");
insertSproc.AddAnnotation(""foo"", ""bar1"");
runtimeEntityType.AddAnnotation(""Relational:InsertStoredProcedure"", insertSproc);
Expand All @@ -2626,7 +2626,7 @@ public static void CreateAnnotations(RuntimeEntityType runtimeEntityType)
""TPC"",
true);
var id = deleteSproc.AddParameter(
var id0 = deleteSproc.AddParameter(
""Id"", ParameterDirection.Input, false, ""Id"", false);
runtimeEntityType.AddAnnotation(""Relational:DeleteStoredProcedure"", deleteSproc);
Expand All @@ -2640,7 +2640,7 @@ public static void CreateAnnotations(RuntimeEntityType runtimeEntityType)
""PrincipalBaseId"", ParameterDirection.Input, false, ""PrincipalBaseId"", false);
var principalDerivedId0 = updateSproc.AddParameter(
""PrincipalDerivedId"", ParameterDirection.Input, false, ""PrincipalDerivedId"", false);
var id0 = updateSproc.AddParameter(
var id1 = updateSproc.AddParameter(
""Id"", ParameterDirection.Input, false, ""Id"", false);
runtimeEntityType.AddAnnotation(""Relational:UpdateStoredProcedure"", updateSproc);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,62 @@ public class DisjointChildSubclass2 : Child

public abstract class SqlServerOneToMany : RelationalOneToManyTestBase
{
[ConditionalFact]
public virtual void Shadow_foreign_keys_to_generic_types_have_terrible_names_that_should_not_change()
{
var modelBuilder = CreateModelBuilder();

modelBuilder.Entity<EventBase>().ToTable("Events");
modelBuilder.Entity<Activity<Company>>().ToTable("CompanyActivities");
modelBuilder.Entity<Activity<User>>().ToTable("UserActivities");

var model = modelBuilder.FinalizeModel();

var companyActivityEventType = model.FindEntityType(typeof(ActivityEvent<Company>))!;
var eventTable = StoreObjectIdentifier.Create(companyActivityEventType, StoreObjectType.Table)!.Value;
var companyActivityEventFk = companyActivityEventType.GetForeignKeys().Single();
var companyActivityEventFkProperty = companyActivityEventFk.Properties.Single();
Assert.Equal("Activity<Company>Id", companyActivityEventFkProperty.GetColumnName(eventTable));
Assert.Equal("FK_Events_CompanyActivities_Activity<Company>Id", companyActivityEventFk.GetConstraintName());
Assert.Equal(
"FK_Events_CompanyActivities_Activity<Company>Id", companyActivityEventFk.GetConstraintName(
eventTable,
StoreObjectIdentifier.Create(companyActivityEventFk.PrincipalEntityType, StoreObjectType.Table)!.Value));

var userActivityEventType = model.FindEntityType(typeof(ActivityEvent<User>))!;
var userActivityEventFk = userActivityEventType.GetForeignKeys().Single();
var userActivityEventFkProperty = userActivityEventFk.Properties.Single();
Assert.Equal("Activity<User>Id", userActivityEventFkProperty.GetColumnName(eventTable));
Assert.Equal("FK_Events_UserActivities_Activity<User>Id", userActivityEventFk.GetConstraintName());
Assert.Equal(
"FK_Events_UserActivities_Activity<User>Id", userActivityEventFk.GetConstraintName(
eventTable,
StoreObjectIdentifier.Create(userActivityEventFk.PrincipalEntityType, StoreObjectType.Table)!.Value));
}

protected abstract class EventBase
{
public string? Id { get; set; }
}

protected class Activity<T>
{
public string? Id { get; set; }
public virtual List<ActivityEvent<T>> Events { get; private set; } = null!;
}

protected class ActivityEvent<TTarget> : EventBase
{
}

protected class Company
{
}

protected class User
{
}

protected override TestModelBuilder CreateModelBuilder(Action<ModelConfigurationBuilder>? configure = null)
=> CreateTestModelBuilder(SqlServerTestHelpers.Instance, configure);
}
Expand Down
2 changes: 0 additions & 2 deletions test/EFCore.Tests/ModelBuilding/OneToManyTestBase.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.



// ReSharper disable InconsistentNaming
namespace Microsoft.EntityFrameworkCore.ModelBuilding;

Expand Down

0 comments on commit 2543d4b

Please sign in to comment.