Skip to content

Commit

Permalink
Stop using similar-looking owned entity in deeply nested case
Browse files Browse the repository at this point in the history 
Fixes #15839

The issue was that both the immediate owner for `Rejection.FirstTest#FirstTest.Tester#SpecialistStaff` and `Attitude.FirstTest#FirstTest.Tester#SpecialistStaff` are the same resulting in the former being mistaken for the latter. The fix is to use the type rather than its name.
  • Loading branch information
@ajcvickers
ajcvickers committed Aug 5, 2019
1 parent 8744174 commit 1828f38
Show file tree
Hide file tree
Showing 3 changed files with 188 additions and 21 deletions.
4 changes: 2 additions & 2 deletions src/EFCore/Metadata/Internal/InternalRelationshipBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3154,7 +3154,7 @@ public virtual InternalRelationshipBuilder Attach([NotNull] InternalEntityTypeBu
principalEntityType = model.FindEntityType(
Metadata.PrincipalEntityType.Name,
Metadata.PrincipalEntityType.DefiningNavigationName,
Metadata.PrincipalEntityType.DefiningEntityType.Name);
Metadata.PrincipalEntityType.DefiningEntityType);
if (principalEntityType == null)
{
return null;
Expand Down Expand Up @@ -3209,7 +3209,7 @@ public virtual InternalRelationshipBuilder Attach([NotNull] InternalEntityTypeBu
dependentEntityType = model.FindEntityType(
Metadata.DeclaringEntityType.Name,
Metadata.DeclaringEntityType.DefiningNavigationName,
Metadata.DeclaringEntityType.DefiningEntityType.Name);
Metadata.DeclaringEntityType.DefiningEntityType);
}

if (dependentEntityType == null)
Expand Down
19 changes: 0 additions & 19 deletions src/EFCore/Metadata/Internal/Model.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -453,25 +453,6 @@ public virtual EntityType FindEntityType(
[NotNull] EntityType definingEntityType)
=> FindEntityType(GetDisplayName(type), definingNavigationName, definingEntityType);

/// <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
/// 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 EntityType FindEntityType(
[NotNull] string name,
[NotNull] string definingNavigationName,
[NotNull] string definingEntityTypeName)
{
return !_entityTypesWithDefiningNavigation.TryGetValue(name, out var entityTypesWithSameType)
? null
: entityTypesWithSameType
.FirstOrDefault(
e => e.DefiningNavigationName == definingNavigationName
&& e.DefiningEntityType.Name == definingEntityTypeName);
}

/// <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 Down
186 changes: 186 additions & 0 deletions test/EFCore.Tests/Metadata/Internal/EntityTypeTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2073,6 +2073,192 @@ public void Change_tracking_can_be_set_to_snapshot_only_for_non_notifying_entiti
.Message);
}

[ConditionalFact]
public void Entity_type_with_deeply_nested_owned_weak_types_builds_correctly()
{
using (var context = new RejectionContext(nameof(RejectionContext)))
{
var entityTypes = context.Model.GetEntityTypes();

Assert.Equal(new[]
{
"Application",
"ApplicationVersion",
"Rejection",
"Application.Attitude#Attitude",
"ApplicationVersion.Attitude#Attitude",
"Rejection.FirstTest#FirstTest",
"Application.Attitude#Attitude.FirstTest#FirstTest",
"ApplicationVersion.Attitude#Attitude.FirstTest#FirstTest",
"Rejection.FirstTest#FirstTest.Tester#SpecialistStaff",
"Application.Attitude#Attitude.FirstTest#FirstTest.Tester#SpecialistStaff",
"ApplicationVersion.Attitude#Attitude.FirstTest#FirstTest.Tester#SpecialistStaff"
}, entityTypes.Select(e => e.DisplayName()).ToList());
}
}

//
// ApplicationVersion Application
// | | |
// Attitude` Rejection Attitude``
// | | |
// FirstTest` FirstTest`` FirstTest```
// | | |
// SpecialistStaff` SpecialistStaff`` SpecialistStaff```
//
// ApplicationVersion = ApplicationVersion
// Attitude` = ApplicationVersion.Attitude#Attitude
// FirstTest` = Application.Attitude#Attitude.FirstTest#FirstTest
// SpecialistStaff` = ApplicationVersion.Attitude#Attitude.FirstTest#FirstTest.Tester#SpecialistStaff
//
// Rejection = Rejection
// FirstTest`` = Rejection.FirstTest#FirstTest
// SpecialistStaff`` = Rejection.FirstTest#FirstTest.Tester#SpecialistStaff
//
// Application = Application
// Attitude`` = Application.Attitude#Attitude
// FistTest``` = ApplicationVersion.Attitude#Attitude.FirstTest#FirstTest
// SpecialistStaff``` = Application.Attitude#Attitude.FirstTest#FirstTest.Tester#SpecialistStaff
//

private class Application
{
public Guid Id { get; protected set; }
public Attitude Attitude { get; set; }
public Rejection Rejection { get; set; }
}

private class ApplicationVersion
{
public Guid Id { get; protected set; }
public Attitude Attitude { get; set; }
}

private class Rejection
{
public FirstTest FirstTest { get; set; }
}

private class Attitude
{
public FirstTest FirstTest { get; set; }
}

private class FirstTest
{
public SpecialistStaff Tester { get; set; }
}

private class SpecialistStaff
{
}

private class RejectionContext : DbContext
{
private readonly string _databaseName;

public RejectionContext(string databaseName)
{
_databaseName = databaseName;
}

protected internal override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
=> optionsBuilder.UseInMemoryDatabase(_databaseName);

protected internal override void OnModelCreating(ModelBuilder modelBuilder)
{
List<string> GetTypeNames()
=> modelBuilder.Model.GetEntityTypes().Select(e => e.DisplayName()).ToList();

modelBuilder.Entity<Application>(entity =>
{
entity.OwnsOne(x => x.Attitude,
amb =>
{
amb.OwnsOne(x => x.FirstTest, mb =>
{
mb.OwnsOne(a => a.Tester);
});
});

entity.OwnsOne(x => x.Rejection,
amb =>
{
amb.OwnsOne(x => x.FirstTest, mb =>
{
mb.OwnsOne(a => a.Tester);
});
});
});

Assert.Equal(new[]
{
"Application",
"Attitude",
"Rejection",
"Attitude.FirstTest#FirstTest", // FirstTest is weak
"Rejection.FirstTest#FirstTest", // FirstTest is weak
"Attitude.FirstTest#FirstTest.Tester#SpecialistStaff", // SpecialistStaff is weak
"Rejection.FirstTest#FirstTest.Tester#SpecialistStaff" // SpecialistStaff is weak
}, GetTypeNames());

modelBuilder.Entity<ApplicationVersion>(entity =>
{
Assert.Equal(new[]
{
"Application",
"ApplicationVersion",
"Attitude",
"Rejection",
"Attitude.FirstTest#FirstTest",
"Rejection.FirstTest#FirstTest",
"Attitude.FirstTest#FirstTest.Tester#SpecialistStaff",
"Rejection.FirstTest#FirstTest.Tester#SpecialistStaff"
}, GetTypeNames());

entity.OwnsOne(x => x.Attitude,
amb =>
{
var typeNames = GetTypeNames();
Assert.Equal(new[]
{
"Application",
"ApplicationVersion",
"Rejection",
"Application.Attitude#Attitude", // Attitude becomes weak
"ApplicationVersion.Attitude#Attitude", // Attitude becomes weak
"Rejection.FirstTest#FirstTest",
"Application.Attitude#Attitude.FirstTest#FirstTest", // Attitude becomes weak
"ApplicationVersion.Attitude#Attitude.FirstTest#FirstTest", // Attitude becomes weak
"Rejection.FirstTest#FirstTest.Tester#SpecialistStaff",
"Application.Attitude#Attitude.FirstTest#FirstTest.Tester#SpecialistStaff", // Attitude becomes weak
"ApplicationVersion.Attitude#Attitude.FirstTest#FirstTest.Tester#SpecialistStaff" // Attitude becomes weak
}, typeNames);

amb.OwnsOne(x => x.FirstTest, mb =>
{
mb.OwnsOne(a => a.Tester);
});
});
});

Assert.Equal(new[]
{
"Application",
"ApplicationVersion",
"Rejection",
"Application.Attitude#Attitude",
"ApplicationVersion.Attitude#Attitude",
"Rejection.FirstTest#FirstTest",
"Application.Attitude#Attitude.FirstTest#FirstTest",
"ApplicationVersion.Attitude#Attitude.FirstTest#FirstTest",
"Rejection.FirstTest#FirstTest.Tester#SpecialistStaff",
"Application.Attitude#Attitude.FirstTest#FirstTest.Tester#SpecialistStaff",
"ApplicationVersion.Attitude#Attitude.FirstTest#FirstTest.Tester#SpecialistStaff"
}, GetTypeNames());
}
}

[ConditionalFact]
public void All_properties_have_original_value_indexes_when_using_snapshot_change_tracking()
{
Expand Down

0 comments on commit 1828f38

Please sign in to comment.