Add change tracking support for shadow navigations to non-shadow entity types #9918
Conversation
| /// directly from your code. This API may change or be removed in future releases. | ||
| /// </summary> | ||
| public class ShadowCollectionAccessor<TCollection, TElement> : IShadowCollectionAccessor | ||
| where TCollection : class, IEnumerable<TElement> |
There was a problem hiding this comment.
Does this need to be generic? For the collection snapshot, we just use object.
There was a problem hiding this comment.
This class is mostly superfluous, I'll just inline it.
|
I think it would be good to discuss when we are in the office. I'd like to understand the need to move things to the state entry. |
|
The methods are moved to InternalStateEntry to abstract away the shadowness in the same way it's done for the properties, without adding any dependencies to the accessor implementation. |
AndriySvyryd
force-pushed
the
Issue749
branch
from
ef70f7c to
4eb1882
Compare
AndriySvyryd
changed the title
| public virtual IEnumerable GetOrCreateCollection([NotNull] INavigation navigation) | ||
| { | ||
| return navigation.GetCollectionAccessor().GetOrCreate(Entity); | ||
| } |
There was a problem hiding this comment.
Forgot to add Debug.Assert(!navigation.IsShadowProperty);
| @@ -12,7 +13,7 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Internal | |||
| /// This API supports the Entity Framework Core infrastructure and is not intended to be used | |||
| /// directly from your code. This API may change or be removed in future releases. | |||
| /// </summary> | |||
| public class ClrICollectionAccessor<TEntity, TCollection, TElement> : IClrCollectionAccessor | |||
| public class ClrCollectionAccessor<TEntity, TCollection, TElement> : IClrCollectionAccessor | |||
There was a problem hiding this comment.
ClrCollectionAccessor [](start = 17, length = 21)
The "I" is there for a reason. It means this is the implementation for ICollection, rather than just the gneral concept of some kind of collection of things on a thing.
There was a problem hiding this comment.
Still, it goes against our naming guidelines
| @@ -41,19 +42,19 @@ public interface IClrCollectionAccessor | |||
| /// This API supports the Entity Framework Core infrastructure and is not intended to be used | |||
| /// directly from your code. This API may change or be removed in future releases. | |||
| /// </summary> | |||
| object Create(); | |||
| IEnumerable Create(); | |||
There was a problem hiding this comment.
IEnumerable [](start = 8, length = 11)
Do we need to constrain this to only collections that implement IEnumerable?
There was a problem hiding this comment.
We already have this constraint, see https://github.com/aspnet/EntityFrameworkCore/blob/dev/src/EFCore/Metadata/Internal/ClrCollectionAccessorFactory.cs#L42
AndriySvyryd
force-pushed
the
Issue749
branch
from
4eb1882 to
7dc35f4
Compare
More tests will be added
Part of #749