Skip to content

Commit cce6585

Browse files
committed
Fix to #21006 - Support a default value for non-nullable properties
Only for scalar properties when projecting Json-mapped entity. Only need to change code for Cosmos - relational already works in the desired way after the change to streaming (properties that are not encountered maintain their default value) We still throw exception if JSON contains explicit null where non-nullable scalar is expected. Fixes #21006
1 parent 34ee81a commit cce6585

File tree

8 files changed

+840
-12
lines changed

8 files changed

+840
-12
lines changed

Diff for: src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.CosmosProjectionBindingRemovingExpressionVisitorBase.cs

+26-5
Original file line numberDiff line numberDiff line change
@@ -691,7 +691,11 @@ private Expression CreateGetValueExpression(
691691
&& !property.IsShadowProperty())
692692
{
693693
var readExpression = CreateGetValueExpression(
694-
jTokenExpression, storeName, type.MakeNullable(), property.GetTypeMapping());
694+
jTokenExpression,
695+
storeName,
696+
type.MakeNullable(),
697+
property.GetTypeMapping(),
698+
isNonNullableScalar: false);
695699

696700
var nonNullReadExpression = readExpression;
697701
if (nonNullReadExpression.Type != type)
@@ -712,15 +716,23 @@ private Expression CreateGetValueExpression(
712716
}
713717

714718
return Convert(
715-
CreateGetValueExpression(jTokenExpression, storeName, type.MakeNullable(), property.GetTypeMapping()),
719+
CreateGetValueExpression(
720+
jTokenExpression,
721+
storeName,
722+
type.MakeNullable(),
723+
property.GetTypeMapping(),
724+
// special case keys - we check them for null to see if the entity needs to be materialized, so we want to keep the null, rather than non-nullable default
725+
// returning defaults is supposed to help with evolving the schema - so this doesn't concern keys anyway (they shouldn't evolve)
726+
isNonNullableScalar: !property.IsNullable && !property.IsKey()),
716727
type);
717728
}
718729

719730
private Expression CreateGetValueExpression(
720731
Expression jTokenExpression,
721732
string storeName,
722733
Type type,
723-
CoreTypeMapping typeMapping = null)
734+
CoreTypeMapping typeMapping = null,
735+
bool isNonNullableScalar = false)
724736
{
725737
Check.DebugAssert(type.IsNullableType(), "Must read nullable type from JObject.");
726738

@@ -763,6 +775,7 @@ var body
763775
Constant(CosmosClientWrapper.Serializer)),
764776
converter.ConvertFromProviderExpression.Body);
765777

778+
var originalBodyType = body.Type;
766779
if (body.Type != type)
767780
{
768781
body = Convert(body, type);
@@ -783,7 +796,11 @@ var body
783796
}
784797
else
785798
{
786-
replaceExpression = Default(type);
799+
replaceExpression = isNonNullableScalar
800+
? Expression.Convert(
801+
Default(originalBodyType),
802+
type)
803+
: Default(type);
787804
}
788805

789806
body = Condition(
@@ -799,7 +816,11 @@ var body
799816
}
800817
else
801818
{
802-
valueExpression = ConvertJTokenToType(jTokenExpression, typeMapping?.ClrType.MakeNullable() ?? type);
819+
valueExpression = ConvertJTokenToType(
820+
jTokenExpression,
821+
(isNonNullableScalar
822+
? typeMapping?.ClrType
823+
: typeMapping?.ClrType.MakeNullable()) ?? type);
803824

804825
if (valueExpression.Type != type)
805826
{
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System.Net;
5+
using Microsoft.Azure.Cosmos;
6+
using Microsoft.EntityFrameworkCore.Cosmos.Storage.Internal;
7+
using Newtonsoft.Json;
8+
using Newtonsoft.Json.Linq;
9+
10+
namespace Microsoft.EntityFrameworkCore.Query;
11+
12+
public class AdHocCosmosTestHelpers
13+
{
14+
public static async Task CreateCustomEntityHelperAsync(
15+
Container container,
16+
string json,
17+
CancellationToken cancellationToken)
18+
{
19+
var document = JObject.Parse(json);
20+
21+
var stream = new MemoryStream();
22+
await using var __ = stream.ConfigureAwait(false);
23+
var writer = new StreamWriter(stream, new UTF8Encoding(), bufferSize: 1024, leaveOpen: false);
24+
await using var ___ = writer.ConfigureAwait(false);
25+
using var jsonWriter = new JsonTextWriter(writer);
26+
27+
CosmosClientWrapper.Serializer.Serialize(jsonWriter, document);
28+
await jsonWriter.FlushAsync(cancellationToken).ConfigureAwait(false);
29+
30+
var response = await container.CreateItemStreamAsync(
31+
stream,
32+
PartitionKey.None,
33+
requestOptions: null,
34+
cancellationToken)
35+
.ConfigureAwait(false);
36+
37+
38+
if (response.StatusCode != HttpStatusCode.Created)
39+
{
40+
throw new InvalidOperationException($"Failed to create entitty (status code: {response.StatusCode}) for json: {json}");
41+
}
42+
}
43+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,269 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using Microsoft.EntityFrameworkCore.Cosmos.Storage.Internal;
5+
6+
namespace Microsoft.EntityFrameworkCore.Query;
7+
8+
public class AdHocJsonQueryCosmosTest : AdHocJsonQueryTestBase
9+
{
10+
public override async Task Project_root_with_missing_scalars(bool async)
11+
{
12+
if (async)
13+
{
14+
await base.Project_root_with_missing_scalars(async);
15+
16+
AssertSql(
17+
"""
18+
SELECT VALUE c
19+
FROM root c
20+
""");
21+
}
22+
}
23+
24+
[ConditionalTheory(Skip = "issue #35702")]
25+
public override async Task Project_top_level_json_entity_with_missing_scalars(bool async)
26+
{
27+
if (async)
28+
{
29+
await base.Project_top_level_json_entity_with_missing_scalars(async);
30+
31+
AssertSql();
32+
}
33+
}
34+
35+
public override async Task Project_nested_json_entity_with_missing_scalars(bool async)
36+
{
37+
if (async)
38+
{
39+
await AssertTranslationFailed(
40+
() => base.Project_nested_json_entity_with_missing_scalars(async));
41+
42+
AssertSql();
43+
}
44+
}
45+
46+
protected override void OnModelCreating21006(ModelBuilder modelBuilder)
47+
{
48+
base.OnModelCreating21006(modelBuilder);
49+
50+
modelBuilder.Entity<Context21006.Entity>().ToContainer("Entities");
51+
}
52+
53+
protected override async Task Seed21006(Context21006 context)
54+
{
55+
await base.Seed21006(context);
56+
57+
var wrapper = (CosmosClientWrapper)context.GetService<ICosmosClientWrapper>();
58+
var singletonWrapper = context.GetService<ISingletonCosmosClientWrapper>();
59+
var entitiesContainer = singletonWrapper.Client.GetContainer(StoreName, containerId: "Entities");
60+
61+
var missingTopLevel =
62+
$$"""
63+
{
64+
"Id": 2,
65+
"$type": "Entity",
66+
"Name": "e2",
67+
"id": "2",
68+
"Collection": [
69+
{
70+
"Text": "e2 c1",
71+
"NestedCollection": [
72+
{
73+
"DoB": "2000-01-01T00:00:00",
74+
"Text": "e2 c1 c1"
75+
},
76+
{
77+
"DoB": "2000-01-01T00:00:00",
78+
"Text": "e2 c1 c2"
79+
}
80+
],
81+
"NestedOptionalReference": {
82+
"DoB": "2000-01-01T00:00:00",
83+
"Text": "e2 c1 nor"
84+
},
85+
"NestedRequiredReference": {
86+
"DoB": "2000-01-01T00:00:00",
87+
"Text": "e2 c1 nrr"
88+
}
89+
},
90+
{
91+
"Text": "e2 c2",
92+
"NestedCollection": [
93+
{
94+
"DoB": "2000-01-01T00:00:00",
95+
"Text": "e2 c2 c1"
96+
},
97+
{
98+
"DoB": "2000-01-01T00:00:00",
99+
"Text": "e2 c2 c2"
100+
}
101+
],
102+
"NestedOptionalReference": {
103+
"DoB": "2000-01-01T00:00:00",
104+
"Text": "e2 c2 nor"
105+
},
106+
"NestedRequiredReference": {
107+
"DoB": "2000-01-01T00:00:00",
108+
"Text": "e2 c2 nrr"
109+
}
110+
}
111+
],
112+
"OptionalReference": {
113+
"Text": "e2 or",
114+
"NestedCollection": [
115+
{
116+
"DoB": "2000-01-01T00:00:00",
117+
"Text": "e2 or c1"
118+
},
119+
{
120+
"DoB": "2000-01-01T00:00:00",
121+
"Text": "e2 or c2"
122+
}
123+
],
124+
"NestedOptionalReference": {
125+
"DoB": "2000-01-01T00:00:00",
126+
"Text": "e2 or nor"
127+
},
128+
"NestedRequiredReference": {
129+
"DoB": "2000-01-01T00:00:00",
130+
"Text": "e2 or nrr"
131+
}
132+
},
133+
"RequiredReference": {
134+
"Text": "e2 rr",
135+
"NestedCollection": [
136+
{
137+
"DoB": "2000-01-01T00:00:00",
138+
"Text": "e2 rr c1"
139+
},
140+
{
141+
"DoB": "2000-01-01T00:00:00",
142+
"Text": "e2 rr c2"
143+
}
144+
],
145+
"NestedOptionalReference": {
146+
"DoB": "2000-01-01T00:00:00",
147+
"Text": "e2 rr nor"
148+
},
149+
"NestedRequiredReference": {
150+
"DoB": "2000-01-01T00:00:00",
151+
"Text": "e2 rr nrr"
152+
}
153+
}
154+
}
155+
""";
156+
157+
await AdHocCosmosTestHelpers.CreateCustomEntityHelperAsync(
158+
entitiesContainer,
159+
missingTopLevel,
160+
CancellationToken.None);
161+
162+
var missingNested =
163+
$$"""
164+
{
165+
"Id": 3,
166+
"$type": "Entity",
167+
"Name": "e3",
168+
"id": "3",
169+
"Collection": [
170+
{
171+
"Number": 7.0,
172+
"Text": "e3 c1",
173+
"NestedCollection": [
174+
{
175+
"Text": "e3 c1 c1"
176+
},
177+
{
178+
"Text": "e3 c1 c2"
179+
}
180+
],
181+
"NestedOptionalReference": {
182+
"Text": "e3 c1 nor"
183+
},
184+
"NestedRequiredReference": {
185+
"Text": "e3 c1 nrr"
186+
}
187+
},
188+
{
189+
"Number": 7.0,
190+
"Text": "e3 c2",
191+
"NestedCollection": [
192+
{
193+
"Text": "e3 c2 c1"
194+
},
195+
{
196+
"Text": "e3 c2 c2"
197+
}
198+
],
199+
"NestedOptionalReference": {
200+
"Text": "e3 c2 nor"
201+
},
202+
"NestedRequiredReference": {
203+
"Text": "e3 c2 nrr"
204+
}
205+
}
206+
],
207+
"OptionalReference": {
208+
"Number": 7.0,
209+
"Text": "e3 or",
210+
"NestedCollection": [
211+
{
212+
"Text": "e3 or c1"
213+
},
214+
{
215+
"Text": "e3 or c2"
216+
}
217+
],
218+
"NestedOptionalReference": {
219+
"Text": "e3 or nor"
220+
},
221+
"NestedRequiredReference": {
222+
"Text": "e3 or nrr"
223+
}
224+
},
225+
"RequiredReference": {
226+
"Number": 7.0,
227+
"Text": "e3 rr",
228+
"NestedCollection": [
229+
{
230+
"Text": "e3 rr c1"
231+
},
232+
{
233+
"Text": "e3 rr c2"
234+
}
235+
],
236+
"NestedOptionalReference": {
237+
"Text": "e3 rr nor"
238+
},
239+
"NestedRequiredReference": {
240+
"Text": "e3 rr nrr"
241+
}
242+
}
243+
}
244+
""";
245+
246+
await AdHocCosmosTestHelpers.CreateCustomEntityHelperAsync(
247+
entitiesContainer,
248+
missingNested,
249+
CancellationToken.None);
250+
}
251+
252+
protected TestSqlLoggerFactory TestSqlLoggerFactory
253+
=> (TestSqlLoggerFactory)ListLoggerFactory;
254+
255+
private void AssertSql(params string[] expected)
256+
=> TestSqlLoggerFactory.AssertBaseline(expected);
257+
258+
protected static async Task AssertTranslationFailed(Func<Task> query)
259+
=> Assert.Contains(
260+
CoreStrings.TranslationFailed("")[48..],
261+
(await Assert.ThrowsAsync<InvalidOperationException>(query))
262+
.Message);
263+
264+
protected override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
265+
=> builder.ConfigureWarnings(b => b.Ignore(CosmosEventId.NoPartitionKeyDefined));
266+
267+
protected override ITestStoreFactory TestStoreFactory
268+
=> CosmosTestStoreFactory.Instance;
269+
}

0 commit comments

Comments
 (0)