diff --git a/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs index 63c775e2a95..74b27a5ef49 100644 --- a/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs @@ -36,22 +36,22 @@ private static readonly MemberInfo SingleQueryResultCoordinatorResultReadyMember private static readonly MethodInfo CollectionAccessorAddMethodInfo = typeof(IClrCollectionAccessor).GetTypeInfo().GetDeclaredMethod(nameof(IClrCollectionAccessor.Add))!; - private static readonly MethodInfo JsonElementGetPropertyMethod - = typeof(JsonElement).GetMethod(nameof(JsonElement.GetProperty), new[] { typeof(string) })!; + private static readonly MethodInfo JsonElementTryGetPropertyMethod + = typeof(JsonElement).GetMethod(nameof(JsonElement.TryGetProperty), new[] { typeof(string), typeof(JsonElement).MakeByRefType() })!; private static readonly MethodInfo JsonElementGetItemMethodInfo = typeof(JsonElement).GetMethod("get_Item", new[] { typeof(int) })!; - private static readonly PropertyInfo _objectArrayIndexerPropertyInfo + private static readonly PropertyInfo ObjectArrayIndexerPropertyInfo = typeof(object[]).GetProperty("Item")!; - private static readonly PropertyInfo _nullableJsonElementHasValuePropertyInfo + private static readonly PropertyInfo NullableJsonElementHasValuePropertyInfo = typeof(JsonElement?).GetProperty(nameof(Nullable.HasValue))!; - private static readonly PropertyInfo _nullableJsonElementValuePropertyInfo + private static readonly PropertyInfo NullableJsonElementValuePropertyInfo = typeof(JsonElement?).GetProperty(nameof(Nullable.Value))!; - private static readonly MethodInfo _arrayCopyMethodInfo + private static readonly MethodInfo ArrayCopyMethodInfo = typeof(Array).GetMethod(nameof(Array.Copy), new[] { typeof(Array), typeof(Array), typeof(int) })!; private readonly RelationalShapedQueryCompilingExpressionVisitor _parentVisitor; @@ -1074,7 +1074,7 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp return property!.IsPrimaryKey() ? Expression.MakeIndex( keyPropertyValuesParameter, - _objectArrayIndexerPropertyInfo, + ObjectArrayIndexerPropertyInfo, new[] { Expression.Constant(index) }) : CreateExtractJsonPropertyExpression(jsonElementParameter, property); } @@ -1154,15 +1154,25 @@ private Expression CreateJsonShapers( shaperBlockVariables.Add(innerJsonElementParameter); - // TODO: do TryGetProperty and short circuit if failed instead + // JsonElement temp; + // JsonElement? innerJsonElement = jsonElement.TryGetProperty("PropertyName", temp) + // ? (JsonElement?)temp + // : null; + var tempParameter = Expression.Variable(typeof(JsonElement)); + shaperBlockVariables.Add(tempParameter); + var innerJsonElementAssignment = Expression.Assign( innerJsonElementParameter, - Expression.Convert( + Expression.Condition( Expression.Call( jsonElementShaperLambdaParameter, - JsonElementGetPropertyMethod, - Expression.Constant(ownedNavigation.TargetEntityType.GetJsonPropertyName())), - typeof(JsonElement?))); + JsonElementTryGetPropertyMethod, + Expression.Constant(ownedNavigation.TargetEntityType.GetJsonPropertyName()), + tempParameter), + Expression.Convert( + tempParameter, + typeof(JsonElement?)), + Expression.Constant(null, typeof(JsonElement?)))); shaperBlockExpressions.Add(innerJsonElementAssignment); @@ -1398,12 +1408,40 @@ private Expression CreateJsonShapers( Expression jsonElementAccessExpressionFragment; if (currentPath.JsonPropertyName is string stringPath) { - jsonElementAccessExpressionFragment = Expression.Call( + // JsonElement? jsonElement = (...) <- this is the previous one + // JsonElement temp; + // JsonElement? newJsonElement = jsonElement.HasValue && jsonElement.Value.TryGetProperty("PropertyName", temp) + // ? (JsonElement?)temp + // : null; + var tempParameter = Expression.Variable(typeof(JsonElement)); + _variables.Add(tempParameter); + + var tryGetPropertyCall = Expression.Call( Expression.MakeMemberAccess( currentJsonElementVariable!, - _nullableJsonElementValuePropertyInfo), - JsonElementGetPropertyMethod, - Expression.Constant(stringPath)); + NullableJsonElementValuePropertyInfo), + JsonElementTryGetPropertyMethod, + Expression.Constant(stringPath), + tempParameter); + + var newJsonElementVariable = Expression.Variable( + typeof(JsonElement?)); + + var newJsonElementAssignment = Expression.Assign( + newJsonElementVariable, + Expression.Condition( + Expression.AndAlso( + Expression.MakeMemberAccess( + currentJsonElementVariable!, + NullableJsonElementHasValuePropertyInfo), + tryGetPropertyCall), + Expression.Convert(tempParameter, typeof(JsonElement?)), + Expression.Constant(null, typeof(JsonElement?)))); + + _variables.Add(newJsonElementVariable); + _expressions.Add(newJsonElementAssignment); + + currentJsonElementVariable = newJsonElementVariable; } else { @@ -1416,7 +1454,7 @@ private Expression CreateJsonShapers( jsonElementAccessExpressionFragment = Expression.Call( Expression.MakeMemberAccess( currentJsonElementVariable!, - _nullableJsonElementValuePropertyInfo), + NullableJsonElementValuePropertyInfo), JsonElementGetItemMethodInfo, elementAccessExpression); @@ -1442,15 +1480,15 @@ private Expression CreateJsonShapers( Expression.Constant(currentKeyValuesCount))); var keyValuesArrayCopyFromPrevious = Expression.Call( - _arrayCopyMethodInfo, + ArrayCopyMethodInfo, previousKeyValuesVariable!, - currentKeyValuesVariable!, + currentKeyValuesVariable, Expression.Constant(currentKeyValuesCount - 1)); var missingKeyValueAssignment = Expression.Assign( Expression.MakeIndex( currentKeyValuesVariable, - _objectArrayIndexerPropertyInfo, + ObjectArrayIndexerPropertyInfo, new[] { Expression.Constant(currentKeyValuesCount - 1) }), Expression.Convert( Expression.Add(elementAccessExpression, Expression.Constant(1)), @@ -1461,26 +1499,26 @@ private Expression CreateJsonShapers( _expressions.Add(keyValuesArrayCopyFromPrevious); _expressions.Add(missingKeyValueAssignment); } - } - - var jsonElementValueExpression = Expression.Condition( - Expression.MakeMemberAccess( - currentJsonElementVariable!, - _nullableJsonElementHasValuePropertyInfo), - Expression.Convert( - jsonElementAccessExpressionFragment, - currentJsonElementVariable!.Type), - Expression.Default(currentJsonElementVariable!.Type)); - - currentJsonElementVariable = Expression.Variable( - typeof(JsonElement?)); - - var jsonElementAssignment = Expression.Assign( - currentJsonElementVariable, - jsonElementValueExpression); - _variables.Add(currentJsonElementVariable); - _expressions.Add(jsonElementAssignment); + var jsonElementValueExpression = Expression.Condition( + Expression.MakeMemberAccess( + currentJsonElementVariable, + NullableJsonElementHasValuePropertyInfo), + Expression.Convert( + jsonElementAccessExpressionFragment, + currentJsonElementVariable!.Type), + Expression.Default(currentJsonElementVariable.Type)); + + currentJsonElementVariable = Expression.Variable( + typeof(JsonElement?)); + + var jsonElementAssignment = Expression.Assign( + currentJsonElementVariable, + jsonElementValueExpression); + + _variables.Add(currentJsonElementVariable); + _expressions.Add(jsonElementAssignment); + } } } diff --git a/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.cs index e623a1e92e7..49eb8c4dd8a 100644 --- a/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.cs @@ -313,7 +313,6 @@ protected override Expression VisitShapedQuery(ShapedQueryExpression shapedQuery } else { - var nonComposedFromSql = selectExpression.IsNonComposedFromSql(); var shaper = new ShaperProcessingExpressionVisitor(this, selectExpression, _tags, splitQuery, nonComposedFromSql).ProcessShaper( shapedQueryExpression.ShaperExpression, diff --git a/test/EFCore.Relational.Specification.Tests/Query/JsonQueryAdHocTestBase.cs b/test/EFCore.Relational.Specification.Tests/Query/JsonQueryAdHocTestBase.cs index ce63c8289e2..5593965ff11 100644 --- a/test/EFCore.Relational.Specification.Tests/Query/JsonQueryAdHocTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Query/JsonQueryAdHocTestBase.cs @@ -88,6 +88,135 @@ public class MyJsonEntity29219 #endregion + #region 30028 + + protected abstract void Seed30028(MyContext30028 ctx); + + protected class MyContext30028 : DbContext + { + public MyContext30028(DbContextOptions options) + : base(options) + { + } + + public DbSet Entities { get; set; } + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity(b => + { + b.Property(x => x.Id).ValueGeneratedNever(); + b.OwnsOne(x => x.Json, nb => + { + nb.ToJson(); + nb.OwnsMany(x => x.Collection, nnb => nnb.OwnsOne(x => x.Nested)); + nb.OwnsOne(x => x.OptionalReference, nnb => nnb.OwnsOne(x => x.Nested)); + nb.OwnsOne(x => x.RequiredReference, nnb => nnb.OwnsOne(x => x.Nested)); + nb.Navigation(x => x.RequiredReference).IsRequired(); + }); + }); + } + } + + public class MyEntity30028 + { + public int Id { get; set; } + public MyJsonRootEntity30028 Json { get; set; } + } + + public class MyJsonRootEntity30028 + { + public string RootName { get; set; } + public MyJsonBranchEntity30028 RequiredReference { get; set; } + public MyJsonBranchEntity30028 OptionalReference { get; set; } + public List Collection { get; set; } + } + + public class MyJsonBranchEntity30028 + { + public string BranchName { get; set; } + public MyJsonLeafEntity30028 Nested { get; set; } + } + + public class MyJsonLeafEntity30028 + { + public string LeafName { get; set; } + } + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual async Task Accessing_missing_navigation_works(bool async) + { + var contextFactory = await InitializeAsync(seed: Seed30028); + using (var context = contextFactory.CreateContext()) + { + var result = context.Entities.OrderBy(x => x.Id).ToList(); + Assert.Equal(4, result.Count); + Assert.NotNull(result[0].Json.Collection); + Assert.NotNull(result[0].Json.OptionalReference); + Assert.NotNull(result[0].Json.RequiredReference); + + Assert.Null(result[1].Json.Collection); + Assert.NotNull(result[1].Json.OptionalReference); + Assert.NotNull(result[1].Json.RequiredReference); + + Assert.NotNull(result[2].Json.Collection); + Assert.Null(result[2].Json.OptionalReference); + Assert.NotNull(result[2].Json.RequiredReference); + + Assert.NotNull(result[3].Json.Collection); + Assert.NotNull(result[3].Json.OptionalReference); + Assert.Null(result[3].Json.RequiredReference); + } + } + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual async Task Missing_navigation_works_with_deduplication(bool async) + { + var contextFactory = await InitializeAsync(seed: Seed30028); + using (var context = contextFactory.CreateContext()) + { + var result = context.Entities.OrderBy(x => x.Id).Select(x => new + { + x, + x.Json, + x.Json.OptionalReference, + x.Json.RequiredReference, + NestedOptional = x.Json.OptionalReference.Nested, + NestedRequired = x.Json.RequiredReference.Nested, + x.Json.Collection, + }).ToList(); + + Assert.Equal(4, result.Count); + Assert.NotNull(result[0].OptionalReference); + Assert.NotNull(result[0].RequiredReference); + Assert.NotNull(result[0].NestedOptional); + Assert.NotNull(result[0].NestedRequired); + Assert.NotNull(result[0].Collection); + + Assert.NotNull(result[1].OptionalReference); + Assert.NotNull(result[1].RequiredReference); + Assert.NotNull(result[1].NestedOptional); + Assert.NotNull(result[1].NestedRequired); + Assert.Null(result[1].Collection); + + Assert.Null(result[2].OptionalReference); + Assert.NotNull(result[2].RequiredReference); + Assert.Null(result[2].NestedOptional); + Assert.NotNull(result[2].NestedRequired); + Assert.NotNull(result[2].Collection); + + Assert.NotNull(result[3].OptionalReference); + Assert.Null(result[3].RequiredReference); + Assert.NotNull(result[3].NestedOptional); + Assert.Null(result[3].NestedRequired); + Assert.NotNull(result[3].Collection); + } + } + + #endregion + #region ArrayOfPrimitives [ConditionalTheory] diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/JsonQueryAdHocSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/JsonQueryAdHocSqlServerTest.cs index b6e05e9bd97..60b2e09a9f1 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/JsonQueryAdHocSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/JsonQueryAdHocSqlServerTest.cs @@ -44,6 +44,33 @@ protected override void Seed29219(MyContext29219 ctx) VALUES(3, N'{{ ""NonNullableScalar"" : 30 }}', N'[{{ ""NonNullableScalar"" : 10001 }}]')"); } + protected override void Seed30028(MyContext30028 ctx) + { + // complete + ctx.Database.ExecuteSqlRaw(@"INSERT INTO [Entities] ([Id], [Json]) +VALUES( +1, +N'{{""RootName"":""e1"",""Collection"":[{{""BranchName"":""e1 c1"",""Nested"":{{""LeafName"":""e1 c1 l""}}}},{{""BranchName"":""e1 c2"",""Nested"":{{""LeafName"":""e1 c2 l""}}}}],""OptionalReference"":{{""BranchName"":""e1 or"",""Nested"":{{""LeafName"":""e1 or l""}}}},""RequiredReference"":{{""BranchName"":""e1 rr"",""Nested"":{{""LeafName"":""e1 rr l""}}}}}}')"); + + // missing collection + ctx.Database.ExecuteSqlRaw(@"INSERT INTO [Entities] ([Id], [Json]) +VALUES( +2, +N'{{""RootName"":""e2"",""OptionalReference"":{{""BranchName"":""e2 or"",""Nested"":{{""LeafName"":""e2 or l""}}}},""RequiredReference"":{{""BranchName"":""e2 rr"",""Nested"":{{""LeafName"":""e2 rr l""}}}}}}')"); + + // missing optional reference + ctx.Database.ExecuteSqlRaw(@"INSERT INTO [Entities] ([Id], [Json]) +VALUES( +3, +N'{{""RootName"":""e3"",""Collection"":[{{""BranchName"":""e3 c1"",""Nested"":{{""LeafName"":""e3 c1 l""}}}},{{""BranchName"":""e3 c2"",""Nested"":{{""LeafName"":""e3 c2 l""}}}}],""RequiredReference"":{{""BranchName"":""e3 rr"",""Nested"":{{""LeafName"":""e3 rr l""}}}}}}')"); + + // missing required reference + ctx.Database.ExecuteSqlRaw(@"INSERT INTO [Entities] ([Id], [Json]) +VALUES( +4, +N'{{""RootName"":""e4"",""Collection"":[{{""BranchName"":""e4 c1"",""Nested"":{{""LeafName"":""e4 c1 l""}}}},{{""BranchName"":""e4 c2"",""Nested"":{{""LeafName"":""e4 c2 l""}}}}],""OptionalReference"":{{""BranchName"":""e4 or"",""Nested"":{{""LeafName"":""e4 or l""}}}}}}')"); + } + protected override void SeedArrayOfPrimitives(MyContextArrayOfPrimitives ctx) { var entity1 = new MyEntityArrayOfPrimitives