Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Relax backing field convention for collection properties #19447

Merged
merged 1 commit into from Jan 1, 2020
Merged

Relax backing field convention for collection properties #19447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 1 addition & 9 deletions src/EFCore/Metadata/Conventions/BackingFieldConvention.cs
View file
Open in desktop
Expand Up @@ -178,7 +178,7 @@ private static FieldInfo TryMatchFieldName(IConventionPropertyBase propertyBase,
{
var newMatch = typeInfo == null
? currentValue.Value
: (IsConvertible(typeInfo, currentValue.Value)
: (typeInfo.IsCompatibleWith(currentValue.Value.FieldType)
? currentValue.Value
: null);

Expand Down Expand Up @@ -240,14 +240,6 @@ private static int PrefixBinarySearch<T>(KeyValuePair<string, T>[] array, string
}
}

private static bool IsConvertible(Type typeInfo, FieldInfo fieldInfo)
{
var fieldTypeInfo = fieldInfo.FieldType;

return typeInfo.IsAssignableFrom(fieldTypeInfo)
|| fieldTypeInfo.IsAssignableFrom(typeInfo);
}

/// <summary>
/// Called after a model is finalized.
/// </summary>
Expand Down
5 changes: 2 additions & 3 deletions src/EFCore/Metadata/Internal/PropertyBase.cs
View file
Open in desktop
Expand Up @@ -202,10 +202,9 @@ public virtual void SetPropertyAccessMode(PropertyAccessMode? propertyAccessMode
return false;
}

var fieldTypeInfo = fieldInfo.FieldType;
var fieldType = fieldInfo.FieldType;
if (propertyType != null
&& !fieldTypeInfo.IsAssignableFrom(propertyType)
&& !propertyType.IsAssignableFrom(fieldTypeInfo))
&& !propertyType.IsCompatibleWith(fieldType))
{
if (shouldThrow)
{
Expand Down
16 changes: 16 additions & 0 deletions src/Shared/SharedTypeExtensions.cs
View file
Open in desktop
Expand Up @@ -165,6 +165,22 @@ public static Type TryGetElementType(this Type type, Type interfaceOrBaseType)
return singleImplementation?.GenericTypeArguments.FirstOrDefault();
}

public static bool IsCompatibleWith(this Type propertyType, Type fieldType)
{
if (propertyType.IsAssignableFrom(fieldType)
|| fieldType.IsAssignableFrom(propertyType))
{
return true;
}

var propertyElementType = propertyType.TryGetSequenceType();
var fieldElementType = fieldType.TryGetSequenceType();

return propertyElementType != null
&& fieldElementType != null
&& IsCompatibleWith(propertyElementType, fieldElementType);
}

public static IEnumerable<Type> GetGenericTypeImplementations(this Type type, Type interfaceOrBaseType)
{
var typeInfo = type.GetTypeInfo();
Expand Down
149 changes: 149 additions & 0 deletions test/EFCore.Specification.Tests/FieldMappingTestBase.cs
View file
Open in desktop
Expand Up @@ -378,6 +378,63 @@ public virtual void Projection_read_only_props(bool tracking)
public virtual void Update_read_only_props()
=> Update<BlogReadOnly>("Posts");

[ConditionalTheory]
[InlineData(false)]
[InlineData(true)]
public virtual void Simple_query_props_with_IReadOnlyCollection(bool tracking)
{
using var context = CreateContext();
AssertBlogs(context.Set<BlogWithReadOnlyCollection>().AsTracking(tracking).ToList());
}

[ConditionalTheory]
[InlineData(false)]
[InlineData(true)]
public virtual void Include_collection_props_with_IReadOnlyCollection(bool tracking)
{
using var context = CreateContext();
AssertGraph(context.Set<BlogWithReadOnlyCollection>().Include(e => e.Posts).AsTracking(tracking).ToList());
}

[ConditionalTheory]
[InlineData(false)]
[InlineData(true)]
public virtual void Include_reference_props_with_IReadOnlyCollection(bool tracking)
{
using var context = CreateContext();
AssertGraph(context.Set<PostWithReadOnlyCollection>().Include(e => e.Blog).AsTracking(tracking).ToList(), tracking);
}

[ConditionalFact]
public virtual void Load_collection_props_with_IReadOnlyCollection()
=> Load_collection<BlogWithReadOnlyCollection>("Posts");

[ConditionalFact]
public virtual void Load_reference_props_with_IReadOnlyCollection()
=> Load_reference<PostWithReadOnlyCollection>("Blog");

[ConditionalTheory]
[InlineData(false)]
[InlineData(true)]
public virtual void Query_with_conditional_constant_props_with_IReadOnlyCollection(bool tracking)
=> Query_with_conditional_constant<PostWithReadOnlyCollection>("BlogId", tracking);

[ConditionalTheory]
[InlineData(false)]
[InlineData(true)]
public virtual void Query_with_conditional_param_props_with_IReadOnlyCollection(bool tracking)
=> Query_with_conditional_param<PostWithReadOnlyCollection>("Title", tracking);

[ConditionalTheory]
[InlineData(false)]
[InlineData(true)]
public virtual void Projection_props_with_IReadOnlyCollection(bool tracking)
=> Projection<PostWithReadOnlyCollection>("Id", "Title", tracking);

[ConditionalFact]
public virtual void Update_props_with_IReadOnlyCollection()
=> Update<BlogWithReadOnlyCollection>("Posts");

[ConditionalTheory]
[InlineData(false)]
[InlineData(true)]
Expand Down Expand Up @@ -1279,6 +1336,79 @@ IBlogAccessor IPostAccessor.AccessBlog
}
}

protected class BlogWithReadOnlyCollection : IBlogAccessor
{
private int _id;
private string _title;
private ICollection<PostWithReadOnlyCollection> _posts;

[DatabaseGenerated(DatabaseGeneratedOption.None)]
public int Id => _id;

public string Title => _title;

public IReadOnlyCollection<PostWithReadOnlyCollection> Posts => (IReadOnlyCollection<PostWithReadOnlyCollection>)_posts;

int IBlogAccessor.AccessId
{
get => Id;
set => _id = value;
}

string IBlogAccessor.AccessTitle
{
get => Title;
set => _title = value;
}

IEnumerable<IPostAccessor> IBlogAccessor.AccessPosts
{
get => Posts;
set => _posts = (ICollection<PostWithReadOnlyCollection>)value;
}
}

protected class PostWithReadOnlyCollection : IPostAccessor
{
private int _id;
private string _title;
private int _blogId;
private BlogWithReadOnlyCollection _blog;

[DatabaseGenerated(DatabaseGeneratedOption.None)]
public int Id => _id;

public string Title => _title;

public int BlogId => _blogId;

public BlogWithReadOnlyCollection Blog => _blog;

int IPostAccessor.AccessId
{
get => Id;
set => _id = value;
}

string IPostAccessor.AccessTitle
{
get => Title;
set => _title = value;
}

int IPostAccessor.AccessBlogId
{
get => BlogId;
set => _blogId = value;
}

IBlogAccessor IPostAccessor.AccessBlog
{
get => Blog;
set => _blog = (BlogWithReadOnlyCollection)value;
}
}

protected class BlogReadOnlyExplicit : IBlogAccessor
{
private int _myid;
Expand Down Expand Up @@ -1826,6 +1956,22 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
b.HasMany(e => e.Posts).WithOne(e => e.Blog).HasForeignKey(e => e.BlogId);
});

modelBuilder.Entity<PostWithReadOnlyCollection>(
b =>
{
b.HasKey(e => e.Id);
b.Property(e => e.Title);
b.Property(e => e.BlogId);
});

modelBuilder.Entity<BlogWithReadOnlyCollection>(
b =>
{
b.HasKey(e => e.Id);
b.Property(e => e.Title);
b.HasMany(e => e.Posts).WithOne(e => e.Blog).HasForeignKey(e => e.BlogId);
});

modelBuilder.Entity<PostReadOnlyExplicit>(
b =>
{
Expand Down Expand Up @@ -1944,6 +2090,9 @@ protected override void Seed(PoolableDbContext context)
context.Add(CreateBlogAndPosts<BlogReadOnly, PostReadOnly>(new ObservableCollection<PostReadOnly>()));
context.AddRange(CreatePostsAndBlog<BlogReadOnly, PostReadOnly>());

context.Add(CreateBlogAndPosts<BlogWithReadOnlyCollection, PostWithReadOnlyCollection>(new ObservableCollection<PostWithReadOnlyCollection>()));
context.AddRange(CreatePostsAndBlog<BlogWithReadOnlyCollection, PostWithReadOnlyCollection>());

context.Add(CreateBlogAndPosts<BlogReadOnlyExplicit, PostReadOnlyExplicit>(new Collection<PostReadOnlyExplicit>()));
context.AddRange(CreatePostsAndBlog<BlogReadOnlyExplicit, PostReadOnlyExplicit>());

Expand Down
36 changes: 36 additions & 0 deletions test/EFCore.Sqlite.FunctionalTests/FieldMappingSqliteTest.cs
View file
Open in desktop
Expand Up @@ -324,6 +324,42 @@ public override void Update_full_props()
{
}

public override void Simple_query_props_with_IReadOnlyCollection(bool tracking)
{
}

public override void Include_collection_props_with_IReadOnlyCollection(bool tracking)
{
}

public override void Include_reference_props_with_IReadOnlyCollection(bool tracking)
{
}

public override void Load_collection_props_with_IReadOnlyCollection()
{
}

public override void Load_reference_props_with_IReadOnlyCollection()
{
}

public override void Query_with_conditional_constant_props_with_IReadOnlyCollection(bool tracking)
{
}

public override void Query_with_conditional_param_props_with_IReadOnlyCollection(bool tracking)
{
}

public override void Projection_props_with_IReadOnlyCollection(bool tracking)
{
}

public override void Update_props_with_IReadOnlyCollection()
{
}

public class EnforcePropertyFixture : FieldMappingSqliteFixtureBase
{
protected override string StoreName { get; } = "FieldMappingEnforcePropertyTest";
Expand Down