Skip to content

Refactor InternalEntityEntry in preparation for complex collection support #36165

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,9 @@ If you are not sure, do not guess, just tell that you don't know or ask clarifyi

- Follow the existing test patterns in the corresponding test projects
- Create both unit tests and functional tests where appropriate
- Add or modify `SQL` and `C#` baselines for tests when necessary
- When running the tests specify the test project and let it be rebuilt.
- Fix `SQL` and `C#` baselines for tests when necessary by setting the `EF_TEST_REWRITE_BASELINES` env var to `1`
- Before building or running the tests execute `restore.cmd` or `restore.sh` and `activate.ps1` or `activate.sh` to set up the environment
- When running the tests specify the test project and let it be rebuilt by not adding `--no-build`

## Documentation

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
using Microsoft.EntityFrameworkCore.Design.Internal;
using Microsoft.EntityFrameworkCore.Internal;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
using Microsoft.EntityFrameworkCore.Query.Internal;

Expand Down Expand Up @@ -1498,8 +1497,10 @@ private void
var variableName = parameters.TargetName;
var mainBuilder = parameters.MainBuilder;
var unsafeAccessors = new HashSet<string>();
var isOnComplexCollection = property.DeclaringType is IReadOnlyComplexType complexType && complexType.ComplexProperty.IsCollection;

if (!property.IsShadowProperty()
&& !isOnComplexCollection
&& property is not IServiceProperty) // Service properties don't use property accessors
{
ClrPropertyGetterFactory.Instance.Create(
Expand Down Expand Up @@ -1559,7 +1560,8 @@ private void
.DecrementIndent();
}

if (property is not IServiceProperty)
if (property is not IServiceProperty
&& !isOnComplexCollection)
{
PropertyAccessorsFactory.Instance.Create(
property,
Expand Down Expand Up @@ -2783,6 +2785,21 @@ private void CreateAnnotations(
mainBuilder.AppendLine(";");
}

foreach (var navigation in entityType.GetSkipNavigations())
{
var variableName = _code.Identifier(navigation.Name, navigation, parameters.ScopeObjects, capitalize: false);

mainBuilder
.Append($"var {variableName} = ")
.Append($"{parameters.TargetName}.FindSkipNavigation({_code.Literal(navigation.Name)})");
if (nullable)
{
mainBuilder.Append("!");
}

mainBuilder.AppendLine(";");
}

var runtimeType = (IRuntimeEntityType)entityType;
var unsafeAccessors = new HashSet<string>();

Expand Down Expand Up @@ -2859,17 +2876,18 @@ private void CreateAnnotations(
.DecrementIndent();

AddNamespace(typeof(PropertyCounts), parameters.Namespaces);
var counts = runtimeType.Counts;
var counts = runtimeType.CalculateCounts();
mainBuilder
.Append(parameters.TargetName).AppendLine(".Counts = new PropertyCounts(")
.Append(parameters.TargetName).AppendLine(".SetCounts(new PropertyCounts(")
.IncrementIndent()
.Append("propertyCount: ").Append(_code.Literal(counts.PropertyCount)).AppendLine(",")
.Append("navigationCount: ").Append(_code.Literal(counts.NavigationCount)).AppendLine(",")
.Append("complexPropertyCount: ").Append(_code.Literal(counts.ComplexPropertyCount)).AppendLine(",")
.Append("complexCollectionCount: ").Append(_code.Literal(counts.ComplexCollectionCount)).AppendLine(",")
.Append("originalValueCount: ").Append(_code.Literal(counts.OriginalValueCount)).AppendLine(",")
.Append("shadowCount: ").Append(_code.Literal(counts.ShadowCount)).AppendLine(",")
.Append("relationshipCount: ").Append(_code.Literal(counts.RelationshipCount)).AppendLine(",")
.Append("storeGeneratedCount: ").Append(_code.Literal(counts.StoreGeneratedCount)).AppendLine(");")
.Append("storeGeneratedCount: ").Append(_code.Literal(counts.StoreGeneratedCount)).AppendLine("));")
.DecrementIndent();

Check.DebugAssert(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Reflection.Metadata;
using System.Text;
using System.Text.Json;

namespace Microsoft.EntityFrameworkCore.Metadata.Internal;

Expand Down Expand Up @@ -282,7 +280,7 @@ private static void AddDefaultMappings(
principalRootEntityType = ownership.PrincipalEntityType;
}

if (principalRootEntityType.FindDiscriminatorProperty() is not null) // tph
if (principalRootEntityType.FindDiscriminatorProperty() is not null)
{
principalRootEntityType = principalRootEntityType.GetRootType();
}
Expand Down
19 changes: 9 additions & 10 deletions src/EFCore/ChangeTracking/CollectionEntry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,7 @@ public override bool IsModified

if (Metadata is ISkipNavigation skipNavigation)
{
if (InternalEntry.EntityState != EntityState.Unchanged
&& InternalEntry.EntityState != EntityState.Detached)
if (InternalEntityEntry.EntityState is not EntityState.Unchanged and not EntityState.Detached)
{
return true;
}
Expand All @@ -122,11 +121,11 @@ public override bool IsModified
{
if (joinEntry.EntityType == joinEntityType
&& stateManager.FindPrincipal(joinEntry, foreignKey) == InternalEntry
&& (joinEntry.EntityState == EntityState.Added
|| joinEntry.EntityState == EntityState.Deleted
&& (joinEntry.EntityState is EntityState.Added
|| joinEntry.EntityState is EntityState.Deleted
|| foreignKey.Properties.Any(joinEntry.IsModified)
|| inverseForeignKey.Properties.Any(joinEntry.IsModified)
|| (stateManager.FindPrincipal(joinEntry, inverseForeignKey)?.EntityState == EntityState.Deleted)))
|| (stateManager.FindPrincipal(joinEntry, inverseForeignKey)?.EntityState is EntityState.Deleted)))
{
return true;
}
Expand Down Expand Up @@ -231,7 +230,7 @@ public override void Load(LoadOptions options)

if (!IsLoaded)
{
TargetLoader.Load(InternalEntry, options);
TargetLoader.Load(InternalEntityEntry, options);
}
}

Expand Down Expand Up @@ -279,7 +278,7 @@ public override Task LoadAsync(LoadOptions options, CancellationToken cancellati

return IsLoaded
? Task.CompletedTask
: TargetLoader.LoadAsync(InternalEntry, options, cancellationToken);
: TargetLoader.LoadAsync(InternalEntityEntry, options, cancellationToken);
}

/// <summary>
Expand All @@ -300,11 +299,11 @@ public override IQueryable Query()
{
EnsureInitialized();

return TargetLoader.Query(InternalEntry);
return TargetLoader.Query(InternalEntityEntry);
}

private void EnsureInitialized()
=> InternalEntry.GetOrCreateCollection(Metadata, forMaterialization: true);
=> InternalEntityEntry.GetOrCreateCollection(Metadata, forMaterialization: true);

/// <summary>
/// The <see cref="EntityEntry" /> of an entity this navigation targets.
Expand Down Expand Up @@ -332,7 +331,7 @@ private void EnsureInitialized()
[EntityFrameworkInternal]
protected virtual InternalEntityEntry? GetInternalTargetEntry(object entity)
=> CurrentValue == null
|| !InternalEntry.CollectionContains(Metadata, entity)
|| !InternalEntityEntry.CollectionContains(Metadata, entity)
? null
: InternalEntry.StateManager.GetOrCreateEntry(entity, Metadata.TargetEntityType);

Expand Down
6 changes: 3 additions & 3 deletions src/EFCore/ChangeTracking/CollectionEntry`.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public CollectionEntry(InternalEntityEntry internalEntry, INavigationBase naviga
/// </remarks>
/// <value> An entry for the entity that owns this member. </value>
public new virtual EntityEntry<TEntity> EntityEntry
=> new(InternalEntry);
=> new(InternalEntityEntry);

/// <summary>
/// Gets or sets the value currently assigned to this property. If the current value is set using this property,
Expand All @@ -73,7 +73,7 @@ public CollectionEntry(InternalEntityEntry internalEntry, INavigationBase naviga
/// </remarks>
public new virtual IEnumerable<TRelatedEntity>? CurrentValue
{
get => (IEnumerable<TRelatedEntity>?)this.GetInfrastructure().GetCurrentValue(Metadata);
get => (IEnumerable<TRelatedEntity>?)InternalEntry.GetCurrentValue(Metadata);
set => base.CurrentValue = value;
}

Expand All @@ -93,7 +93,7 @@ public CollectionEntry(InternalEntityEntry internalEntry, INavigationBase naviga
/// </remarks>
public new virtual IQueryable<TRelatedEntity> Query()
{
InternalEntry.GetOrCreateCollection(Metadata, forMaterialization: true);
InternalEntityEntry.GetOrCreateCollection(Metadata, forMaterialization: true);

return (IQueryable<TRelatedEntity>)base.Query();
}
Expand Down
4 changes: 2 additions & 2 deletions src/EFCore/ChangeTracking/ComplexPropertyEntry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public class ComplexPropertyEntry : MemberEntry
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
[EntityFrameworkInternal]
public ComplexPropertyEntry(InternalEntityEntry internalEntry, IComplexProperty complexProperty)
public ComplexPropertyEntry(IInternalEntry internalEntry, IComplexProperty complexProperty)
: base(internalEntry, complexProperty)
{
}
Expand All @@ -48,7 +48,7 @@ public ComplexPropertyEntry(InternalEntityEntry internalEntry, IComplexProperty
/// </remarks>
public override bool IsModified
{
get => Metadata.ComplexType.GetFlattenedProperties().Any(property => InternalEntry.IsModified(property));
get => Metadata.ComplexType.GetFlattenedProperties().Any(InternalEntry.IsModified);
set
{
foreach (var property in Metadata.ComplexType.GetFlattenedProperties())
Expand Down
4 changes: 2 additions & 2 deletions src/EFCore/ChangeTracking/ComplexPropertyEntry`.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public class ComplexPropertyEntry<TEntity, TComplexProperty> : ComplexPropertyEn
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
[EntityFrameworkInternal]
public ComplexPropertyEntry(InternalEntityEntry internalEntry, IComplexProperty complexProperty)
public ComplexPropertyEntry(IInternalEntry internalEntry, IComplexProperty complexProperty)
: base(internalEntry, complexProperty)
{
}
Expand All @@ -45,7 +45,7 @@ public ComplexPropertyEntry(InternalEntityEntry internalEntry, IComplexProperty
/// examples.
/// </remarks>
public new virtual EntityEntry<TEntity> EntityEntry
=> new(InternalEntry);
=> new(InternalEntry.EntityEntry);

/// <summary>
/// Gets or sets the value currently assigned to this property. If the current value is set using this property,
Expand Down
Loading
Loading