Skip to content

Commit 7f19dfa

Browse files
maumarmaumar
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 7f19dfa

File tree

8 files changed

+838
-12
lines changed

8 files changed

+838
-12
lines changed

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

+24-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,21 @@ 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+
isNonNullableScalar: !property.IsNullable && !property.IsKey()),
716725
type);
717726
}
718727

719728
private Expression CreateGetValueExpression(
720729
Expression jTokenExpression,
721730
string storeName,
722731
Type type,
723-
CoreTypeMapping typeMapping = null)
732+
CoreTypeMapping typeMapping = null,
733+
bool isNonNullableScalar = false)
724734
{
725735
Check.DebugAssert(type.IsNullableType(), "Must read nullable type from JObject.");
726736

@@ -763,6 +773,7 @@ var body
763773
Constant(CosmosClientWrapper.Serializer)),
764774
converter.ConvertFromProviderExpression.Body);
765775

776+
var originalBodyType = body.Type;
766777
if (body.Type != type)
767778
{
768779
body = Convert(body, type);
@@ -783,7 +794,11 @@ var body
783794
}
784795
else
785796
{
786-
replaceExpression = Default(type);
797+
replaceExpression = isNonNullableScalar
798+
? Expression.Convert(
799+
Default(originalBodyType),
800+
type)
801+
: Default(type);
787802
}
788803

789804
body = Condition(
@@ -799,7 +814,11 @@ var body
799814
}
800815
else
801816
{
802-
valueExpression = ConvertJTokenToType(jTokenExpression, typeMapping?.ClrType.MakeNullable() ?? type);
817+
valueExpression = ConvertJTokenToType(
818+
jTokenExpression,
819+
(isNonNullableScalar
820+
? typeMapping?.ClrType
821+
: typeMapping?.ClrType.MakeNullable()) ?? type);
803822

804823
if (valueExpression.Type != type)
805824
{

Diff for: ‎test/EFCore.Cosmos.FunctionalTests/Query/AdHocCosmosTestHelpers.cs

+43
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+
}

Diff for: ‎test/EFCore.Cosmos.FunctionalTests/Query/AdHocJsonQueryCosmosTest.cs

+269
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)