Skip to content

Commit

Permalink
Tweaks for by-convention mapping of gRPC model (#24553)
Browse files Browse the repository at this point in the history
Fixes #23703

At some point we started eagerly throwing when attempting to build a setter delegate. This should be lazy because we don't always need a setter.

Fixes #23901

Detects "propertyName_" as a backing field.
  • Loading branch information
ajcvickers authored Mar 31, 2021
1 parent 34fa402 commit 304219c
Show file tree
Hide file tree
Showing 9 changed files with 307 additions and 2 deletions.
2 changes: 2 additions & 0 deletions src/EFCore/Metadata/Conventions/BackingFieldConvention.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Conventions
/// * _[property name]
/// * m_[camel-cased property name]
/// * m_[property name]
/// * [property name]_
/// </para>
/// <para>
/// The field type must be of a type that's assignable to or from the property type.
Expand Down Expand Up @@ -181,6 +182,7 @@ private void DiscoverField(IConventionPropertyBaseBuilder conventionPropertyBase
match = TryMatch(sortedFields, "_", "", propertyName, propertyBase, match, entityClrType, propertyName);
match = TryMatch(sortedFields, "m_", camelPrefix, camelizedSuffix, propertyBase, match, entityClrType, propertyName);
match = TryMatch(sortedFields, "m_", "", propertyName, propertyBase, match, entityClrType, propertyName);
match = TryMatch(sortedFields, "", camelPrefix + camelizedSuffix, "_", propertyBase, match, entityClrType, propertyName);
}

return match;
Expand Down
4 changes: 2 additions & 2 deletions src/EFCore/Metadata/Internal/ClrCollectionAccessorFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,8 @@ private static IClrCollectionAccessor CreateGeneric<TEntity, TCollection, TEleme
var valueParameter = Expression.Parameter(typeof(TCollection), "collection");

var memberInfoForRead = navigation.GetMemberInfo(forMaterialization: false, forSet: false);
var memberInfoForWrite = navigation.GetMemberInfo(forMaterialization: false, forSet: true);
var memberInfoForMaterialization = navigation.GetMemberInfo(forMaterialization: true, forSet: true);
navigation.TryGetMemberInfo(forConstruction: false, forSet: true, out var memberInfoForWrite, out _);
navigation.TryGetMemberInfo(forConstruction: true, forSet: true, out var memberInfoForMaterialization, out _);

var memberAccessForRead = (Expression)Expression.MakeMemberAccess(entityParameter, memberInfoForRead);
if (memberAccessForRead.Type != typeof(TCollection))
Expand Down
21 changes: 21 additions & 0 deletions test/EFCore.AspNet.InMemory.FunctionalTests/GrpcInMemoryTest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using Microsoft.EntityFrameworkCore.TestUtilities;

namespace Microsoft.EntityFrameworkCore
{
public class GrpcInMemoryTest : GrpcTestBase<GrpcInMemoryTest.GrpcInMemoryFixture>
{
public GrpcInMemoryTest(GrpcInMemoryFixture fixture)
: base(fixture)
{
}

public class GrpcInMemoryFixture : GrpcFixtureBase
{
protected override ITestStoreFactory TestStoreFactory
=> InMemoryTestStoreFactory.Instance;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@
<ProjectReference Include="..\EFCore.Specification.Tests\EFCore.Specification.Tests.csproj" />
<PackageReference Include="Microsoft.AspNetCore.Identity.EntityFrameworkCore" Version="5.0.1" />
<PackageReference Include="IdentityServer4.EntityFramework" Version="4.1.1" />
<PackageReference Include="Grpc.AspNetCore" Version="2.35.0" />
</ItemGroup>

<ItemGroup>
<Protobuf Include="ProtoTest.proto" />
</ItemGroup>

</Project>
166 changes: 166 additions & 0 deletions test/EFCore.AspNet.Specification.Tests/GrpcTestBase.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Linq;
using Google.Protobuf.WellKnownTypes;
using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
using Microsoft.EntityFrameworkCore.TestUtilities;
using ProtoTest;
using Xunit;

namespace Microsoft.EntityFrameworkCore
{
public abstract class GrpcTestBase<TFixture> : IClassFixture<TFixture>
where TFixture : GrpcTestBase<TFixture>.GrpcFixtureBase
{
protected GrpcTestBase(TFixture fixture)
=> Fixture = fixture;

protected TFixture Fixture { get; }

protected List<EntityTypeMapping> ExpectedMappings
=> new()
{
new()
{
Name = "PostTag",
TableName = "PostTag",
PrimaryKey =
"Key: PostTag (Dictionary<string, object>).PostsInTagDataPostId, PostTag (Dictionary<string, object>).TagsInPostDataTagId PK",
Properties =
{
"Property: PostTag (Dictionary<string, object>).PostsInTagDataPostId (no field, int) Indexer Required PK FK AfterSave:Throw",
"Property: PostTag (Dictionary<string, object>).TagsInPostDataTagId (no field, int) Indexer Required PK FK Index AfterSave:Throw",
},
Indexes = { "{'TagsInPostDataTagId'} ", },
FKs =
{
"ForeignKey: PostTag (Dictionary<string, object>) {'PostsInTagDataPostId'} -> Post {'PostId'} Cascade",
"ForeignKey: PostTag (Dictionary<string, object>) {'TagsInPostDataTagId'} -> Tag {'TagId'} Cascade",
},
},
new()
{
Name = "ProtoTest.Author",
TableName = "Author",
PrimaryKey = "Key: Author.AuthorId PK",
Properties =
{
"Property: Author.AuthorId (authorId_, int) Required PK AfterSave:Throw ValueGenerated.OnAdd",
"Property: Author.DateCreated (dateCreated_, Timestamp)",
"Property: Author.Name (name_, string)",
},
},
new()
{
Name = "ProtoTest.Post",
TableName = "Post",
PrimaryKey = "Key: Post.PostId PK",
Properties =
{
"Property: Post.PostId (postId_, int) Required PK AfterSave:Throw ValueGenerated.OnAdd",
"Property: Post.AuthorId (authorId_, int) Required FK Index",
"Property: Post.DateCreated (dateCreated_, Timestamp)",
"Property: Post.PostStat (postStat_, PostStatus) Required",
"Property: Post.Title (title_, string)",
},
Indexes = { "{'AuthorId'} ", },
FKs = { "ForeignKey: Post {'AuthorId'} -> Author {'AuthorId'} ToPrincipal: PostAuthor Cascade", },
Navigations = { "Navigation: Post.PostAuthor (postAuthor_, Author) ToPrincipal Author", },
SkipNavigations =
{
"SkipNavigation: Post.TagsInPostData (tagsInPostData_, RepeatedField<Tag>) CollectionTag Inverse: PostsInTagData",
},
},
new()
{
Name = "ProtoTest.Tag",
TableName = "Tag",
PrimaryKey = "Key: Tag.TagId PK",
Properties =
{
"Property: Tag.TagId (tagId_, int) Required PK AfterSave:Throw ValueGenerated.OnAdd",
"Property: Tag.Name (name_, string)",
},
SkipNavigations =
{
"SkipNavigation: Tag.PostsInTagData (postsInTagData_, RepeatedField<Post>) CollectionPost Inverse: TagsInPostData",
},
},
};

[ConditionalFact]
public void Can_build_Grpc_model()
{
using var context = Fixture.CreateContext();

var entityTypeMappings = context.Model.GetEntityTypes().Select(e => new EntityTypeMapping(e)).ToList();
EntityTypeMapping.AssertEqual(ExpectedMappings, entityTypeMappings);
}

[ConditionalFact]
public void Can_query_Grpc_model()
{
using var context = Fixture.CreateContext();

var post = context.Set<Post>().Include(e => e.PostAuthor).Include(e => e.TagsInPostData).Single();

Assert.Equal("Arthur's post", post.Title);
Assert.Equal(new DateTime(2021, 9, 3, 12, 10, 0, DateTimeKind.Utc), post.DateCreated.ToDateTime());
Assert.Equal(PostStatus.Published, post.PostStat);
Assert.Equal("Arthur", post.PostAuthor.Name);
Assert.Equal(new DateTime(1973, 9, 3, 12, 10, 0, DateTimeKind.Utc), post.PostAuthor.DateCreated.ToDateTime());

Assert.Equal(2, post.TagsInPostData.Count);
Assert.Contains("Puppies", post.TagsInPostData.Select(e => e.Name).ToList());
Assert.Contains("Kittens", post.TagsInPostData.Select(e => e.Name).ToList());
Assert.Same(post, post.TagsInPostData.First().PostsInTagData.First());
Assert.Same(post, post.TagsInPostData.Skip(1).First().PostsInTagData.First());
}

public class GrpcContext : PoolableDbContext
{
public GrpcContext(DbContextOptions options)
: base(options)
{
}

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
var timeStampConverter = new ValueConverter<Timestamp, DateTime>(
v => v.ToDateTime(),
v => new DateTime(v.Ticks, DateTimeKind.Utc).ToTimestamp());

modelBuilder.Entity<Author>().Property(e => e.DateCreated).HasConversion(timeStampConverter);
modelBuilder.Entity<Post>().Property(e => e.DateCreated).HasConversion(timeStampConverter);
modelBuilder.Entity<Tag>();
}
}

public abstract class GrpcFixtureBase : SharedStoreFixtureBase<GrpcContext>
{
protected override string StoreName { get; } = "GrpcTest";

protected override void Seed(GrpcContext context)
{
var post = new Post
{
DateCreated = Timestamp.FromDateTime(new DateTime(2021, 9, 3, 12, 10, 0, DateTimeKind.Utc)),
Title = "Arthur's post",
PostAuthor = new Author
{
DateCreated = Timestamp.FromDateTime(new DateTime(1973, 9, 3, 12, 10, 0, DateTimeKind.Utc)), Name = "Arthur"
},
PostStat = PostStatus.Published,
TagsInPostData = { new Tag { Name = "Kittens" }, new Tag { Name = "Puppies" } }
};

context.Add(post);

context.SaveChanges();
}
}
}
}
57 changes: 57 additions & 0 deletions test/EFCore.AspNet.Specification.Tests/ProtoTest.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
syntax = "proto3";
option csharp_namespace = "ProtoTest";
import "google/protobuf/empty.proto";
import "google/protobuf/timestamp.proto";
package Test;

service ProtoTest{
rpc GetPosts(google.protobuf.Empty) returns (Posts);
rpc GetPost(GetPostQuery) returns (Post);
rpc GetAuthors(google.protobuf.Empty) returns (Authors);
rpc GetAuthor(GetAuthorQuery) returns (Author);
}

message Author {
int32 author_id = 1;
string name = 2;
google.protobuf.Timestamp date_created = 3;
}
message Authors {
repeated Author authors_data = 1;
}

message Post {
int32 post_id = 1;
int32 author_id = 2;
string title = 3;
google.protobuf.Timestamp date_created = 4;
PostStatus post_stat = 5;
Author post_author = 6;
repeated Tag tags_in_post_data = 7;
}
message Posts {
repeated Post posts_data = 1;
}

message Tag {
int32 tag_id = 1;
string name = 2;
repeated Post posts_in_tag_data = 3;
}
message Tags {
repeated Tag tags_data = 1;
}

enum PostStatus {
POST_STATUS_HIDDEN = 0;
POST_STATUS_PUBLISHED = 1;
POST_STATUS_DELETED = 2;
}

message GetPostQuery {
int32 id = 1;
}

message GetAuthorQuery {
int32 id = 1;
}
21 changes: 21 additions & 0 deletions test/EFCore.AspNet.SqlServer.FunctionalTests/GrpcSqlServerTest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using Microsoft.EntityFrameworkCore.TestUtilities;

namespace Microsoft.EntityFrameworkCore
{
public class GrpcSqlServerTest : GrpcTestBase<GrpcSqlServerTest.GrpcSqlServerFixture>
{
public GrpcSqlServerTest(GrpcSqlServerFixture fixture)
: base(fixture)
{
}

public class GrpcSqlServerFixture : GrpcFixtureBase
{
protected override ITestStoreFactory TestStoreFactory
=> SqlServerTestStoreFactory.Instance;
}
}
}
21 changes: 21 additions & 0 deletions test/EFCore.AspNet.Sqlite.FunctionalTests/GrpcSqliteTest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using Microsoft.EntityFrameworkCore.TestUtilities;

namespace Microsoft.EntityFrameworkCore
{
public class GrpcSqliteTest : GrpcTestBase<GrpcSqliteTest.GrpcSqliteFixture>
{
public GrpcSqliteTest(GrpcSqliteFixture fixture)
: base(fixture)
{
}

public class GrpcSqliteFixture : GrpcFixtureBase
{
protected override ITestStoreFactory TestStoreFactory
=> SqliteTestStoreFactory.Instance;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ public void Camel_case_matching_field_is_not_used_if_type_is_not_compatible()
public void Underscore_camel_case_matching_field_is_used_as_next_preference()
=> FieldMatchTest<TheDarkSideOfTheMoon>("Time", "_time");

[ConditionalFact]
public void Underscore_suffix_camel_case_matching_field_is_used_as_next_preference()
=> FieldMatchTest<TheDarkSideOfTheMoon>("Time", "_time");

[ConditionalFact]
public void Underscore_camel_case_matching_field_is_not_used_if_type_is_not_compatible()
=> FieldMatchTest<TheDarkSideOfTheMoon>("TheGreatGigInTheSky", "_TheGreatGigInTheSky");
Expand Down Expand Up @@ -330,6 +334,14 @@ public int Time
set { _time = value; }
}

private int? time2_;

public int Time2
{
get { return (int)_time; }
set { _time = value; }
}

private readonly string _theGreatGigInTheSky;
private int? _TheGreatGigInTheSky;

Expand Down

0 comments on commit 304219c

Please sign in to comment.