Skip to content

Commit ff114db

Browse files
rojiroji
authored
Fix ExecuteUpdate parameters for multiple properties with same name (#36792)
And validate against null required properties in JSON serialization. Fixes #36791
1 parent 0126da8 commit ff114db

File tree

7 files changed

+130
-23
lines changed

7 files changed

+130
-23
lines changed

‎src/EFCore.Relational/Properties/RelationalStrings.Designer.cs‎

Lines changed: 8 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/EFCore.Relational/Properties/RelationalStrings.resx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1069,6 +1069,9 @@
10691069
<data name="NullTypeMappingInSqlTree" xml:space="preserve">
10701070
<value>Expression '{sqlExpression}' in the SQL tree does not have a type mapping assigned.</value>
10711071
</data>
1072+
<data name="NullValueInRequiredJsonProperty" xml:space="preserve">
1073+
<value>Required JSON property '{property}' cannot contain null.</value>
1074+
</data>
10721075
<data name="OneOfThreeValuesMustBeSet" xml:space="preserve">
10731076
<value>Exactly one of '{param1}', '{param2}' or '{param3}' must be set.</value>
10741077
</data>

‎src/EFCore.Relational/Query/Internal/RelationalJsonUtilities.cs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,11 @@ void WriteJsonObject(Utf8JsonWriter writer, IComplexType complexType, object? ob
9292
var propertyValue = property.GetGetter().GetClrValue(objectValue);
9393
if (propertyValue is null)
9494
{
95+
if (!property.IsNullable)
96+
{
97+
throw new InvalidOperationException(RelationalStrings.NullValueInRequiredJsonProperty(property.Name));
98+
}
99+
95100
writer.WriteNullValue();
96101
}
97102
else
@@ -110,6 +115,11 @@ void WriteJsonObject(Utf8JsonWriter writer, IComplexType complexType, object? ob
110115

111116
var propertyValue = complexProperty.GetGetter().GetClrValue(objectValue);
112117

118+
if (propertyValue is null && !complexProperty.IsNullable)
119+
{
120+
throw new InvalidOperationException(RelationalStrings.NullValueInRequiredJsonProperty(complexProperty.Name));
121+
}
122+
113123
WriteJson(writer, complexProperty.ComplexType, propertyValue, complexProperty.IsCollection);
114124
}
115125

‎src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.ExecuteUpdate.cs‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.Diagnostics.CodeAnalysis;
5+
using System.Text;
56
using Microsoft.EntityFrameworkCore.Metadata.Internal;
67
using Microsoft.EntityFrameworkCore.Query.Internal;
78
using Microsoft.EntityFrameworkCore.Query.SqlExpressions;
@@ -636,10 +637,17 @@ constantExpression.Value is null
636637
Expression.Constant(property, typeof(IProperty))),
637638
QueryCompilationContext.QueryContextParameter);
638639

639-
var newParameterName =
640-
$"{ExecuteUpdateRuntimeParameterPrefix}{chainExpression.ParameterExpression.Name}_{property.Name}";
640+
var parameterNameBuilder = new StringBuilder(ExecuteUpdateRuntimeParameterPrefix)
641+
.Append(chainExpression.ParameterExpression.Name);
641642

642-
return _queryCompilationContext.RegisterRuntimeParameter(newParameterName, lambda);
643+
foreach (var complexProperty in chainExpression.ComplexPropertyChain)
644+
{
645+
parameterNameBuilder.Append('_').Append(complexProperty.Name);
646+
}
647+
648+
parameterNameBuilder.Append('_').Append(property.Name);
649+
650+
return _queryCompilationContext.RegisterRuntimeParameter(parameterNameBuilder.ToString(), lambda);
643651
}
644652

645653
case MemberInitExpression memberInitExpression

‎test/EFCore.Specification.Tests/Query/Associations/AssociationsBulkUpdateTestBase.cs‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,25 @@ public virtual Task Update_property_on_projected_association_with_OrderBy_Skip()
7979
s => s.SetProperty(c => c.String, "foo_updated"),
8080
rowsAffectedCount: 3);
8181

82+
[ConditionalFact]
83+
public virtual async Task Update_association_with_null_required_property()
84+
{
85+
using var context = Fixture.CreateContext();
86+
87+
var invalidRelated = Fixture.Data.RootEntities.Single(e => e.Id == 1).RequiredRelated;
88+
var originalValue = invalidRelated.String;
89+
invalidRelated.String = null!;
90+
91+
await Assert.ThrowsAnyAsync<Exception>(() =>
92+
context.Set<RootEntity>().ExecuteUpdateAsync(s => s.SetProperty(x => x.RequiredRelated, invalidRelated)));
93+
94+
// Make sure no update actually occurred in the database
95+
using (Fixture.ListLoggerFactory.SuspendRecordingEvents())
96+
{
97+
Assert.Equal(originalValue, (await context.Set<RootEntity>().SingleAsync(e => e.Id == 1)).RequiredRelated.String);
98+
}
99+
}
100+
82101
#endregion Update properties
83102

84103
#region Update association
@@ -249,6 +268,27 @@ public virtual Task Update_association_to_null_parameter()
249268
rowsAffectedCount: 7);
250269
}
251270

271+
[ConditionalFact]
272+
public virtual async Task Update_association_with_null_required_nested_association()
273+
{
274+
using var context = Fixture.CreateContext();
275+
276+
var invalidRelated = Fixture.Data.RootEntities.Single(e => e.Id == 1).RequiredRelated;
277+
var originalNested = invalidRelated.RequiredNested;
278+
invalidRelated.RequiredNested = null!;
279+
280+
await Assert.ThrowsAnyAsync<Exception>(() =>
281+
context.Set<RootEntity>().ExecuteUpdateAsync(s => s.SetProperty(x => x.RequiredRelated, invalidRelated)));
282+
283+
// Make sure no update actually occurred in the database
284+
using (Fixture.ListLoggerFactory.SuspendRecordingEvents())
285+
{
286+
Assert.Equal(
287+
originalNested.String,
288+
(await context.Set<RootEntity>().SingleAsync(e => e.Id == 1)).RequiredRelated.RequiredNested.String);
289+
}
290+
}
291+
252292
#endregion Update association
253293

254294
#region Update collection

‎test/EFCore.SqlServer.FunctionalTests/Query/Associations/ComplexJson/ComplexJsonBulkUpdateSqlServerTest.cs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,13 @@ public override async Task Update_property_on_projected_association_with_OrderBy
159159
AssertExecuteUpdateSql();
160160
}
161161

162+
public override async Task Update_association_with_null_required_property()
163+
{
164+
await base.Update_association_with_null_required_property();
165+
166+
AssertExecuteUpdateSql();
167+
}
168+
162169
#endregion Update properties
163170

164171
#region Update association
@@ -327,6 +334,13 @@ FROM [RootEntity] AS [r]
327334
""");
328335
}
329336

337+
public override async Task Update_association_with_null_required_nested_association()
338+
{
339+
await base.Update_association_with_null_required_nested_association();
340+
341+
AssertExecuteUpdateSql();
342+
}
343+
330344
#endregion Update association
331345

332346
#region Update collection

‎test/EFCore.SqlServer.FunctionalTests/Query/Associations/ComplexTableSplitting/ComplexTableSplittingBulkUpdateSqlServerTest.cs‎

Lines changed: 44 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,13 @@ public override async Task Update_property_on_projected_association_with_OrderBy
104104
AssertExecuteUpdateSql();
105105
}
106106

107+
public override async Task Update_association_with_null_required_property()
108+
{
109+
await base.Update_association_with_null_required_property();
110+
111+
AssertExecuteUpdateSql();
112+
}
113+
107114
#endregion Update properties
108115

109116
#region Update association
@@ -119,23 +126,28 @@ public override async Task Update_association_to_parameter()
119126
@complex_type_p_Ints='?' (Size = 4000)
120127
@complex_type_p_Name='?' (Size = 4000)
121128
@complex_type_p_String='?' (Size = 4000)
129+
@complex_type_p_RequiredNested_Id='?' (DbType = Int32)
130+
@complex_type_p_RequiredNested_Int='?' (DbType = Int32)
131+
@complex_type_p_RequiredNested_Ints='?' (Size = 4000)
132+
@complex_type_p_RequiredNested_Name='?' (Size = 4000)
133+
@complex_type_p_RequiredNested_String='?' (Size = 4000)
122134
123135
UPDATE [r]
124136
SET [r].[RequiredRelated_Id] = @complex_type_p_Id,
125137
[r].[RequiredRelated_Int] = @complex_type_p_Int,
126138
[r].[RequiredRelated_Ints] = @complex_type_p_Ints,
127139
[r].[RequiredRelated_Name] = @complex_type_p_Name,
128140
[r].[RequiredRelated_String] = @complex_type_p_String,
129-
[r].[RequiredRelated_OptionalNested_Id] = @complex_type_p_Id,
130-
[r].[RequiredRelated_OptionalNested_Int] = @complex_type_p_Int,
131-
[r].[RequiredRelated_OptionalNested_Ints] = @complex_type_p_Ints,
132-
[r].[RequiredRelated_OptionalNested_Name] = @complex_type_p_Name,
133-
[r].[RequiredRelated_OptionalNested_String] = @complex_type_p_String,
134-
[r].[RequiredRelated_RequiredNested_Id] = @complex_type_p_Id,
135-
[r].[RequiredRelated_RequiredNested_Int] = @complex_type_p_Int,
136-
[r].[RequiredRelated_RequiredNested_Ints] = @complex_type_p_Ints,
137-
[r].[RequiredRelated_RequiredNested_Name] = @complex_type_p_Name,
138-
[r].[RequiredRelated_RequiredNested_String] = @complex_type_p_String
141+
[r].[RequiredRelated_OptionalNested_Id] = NULL,
142+
[r].[RequiredRelated_OptionalNested_Int] = NULL,
143+
[r].[RequiredRelated_OptionalNested_Ints] = NULL,
144+
[r].[RequiredRelated_OptionalNested_Name] = NULL,
145+
[r].[RequiredRelated_OptionalNested_String] = NULL,
146+
[r].[RequiredRelated_RequiredNested_Id] = @complex_type_p_RequiredNested_Id,
147+
[r].[RequiredRelated_RequiredNested_Int] = @complex_type_p_RequiredNested_Int,
148+
[r].[RequiredRelated_RequiredNested_Ints] = @complex_type_p_RequiredNested_Ints,
149+
[r].[RequiredRelated_RequiredNested_Name] = @complex_type_p_RequiredNested_Name,
150+
[r].[RequiredRelated_RequiredNested_String] = @complex_type_p_RequiredNested_String
139151
FROM [RootEntity] AS [r]
140152
""");
141153
}
@@ -215,23 +227,28 @@ public override async Task Update_association_to_inline()
215227
@complex_type_p_Ints='?' (Size = 4000)
216228
@complex_type_p_Name='?' (Size = 4000)
217229
@complex_type_p_String='?' (Size = 4000)
230+
@complex_type_p_RequiredNested_Id='?' (DbType = Int32)
231+
@complex_type_p_RequiredNested_Int='?' (DbType = Int32)
232+
@complex_type_p_RequiredNested_Ints='?' (Size = 4000)
233+
@complex_type_p_RequiredNested_Name='?' (Size = 4000)
234+
@complex_type_p_RequiredNested_String='?' (Size = 4000)
218235
219236
UPDATE [r]
220237
SET [r].[RequiredRelated_Id] = @complex_type_p_Id,
221238
[r].[RequiredRelated_Int] = @complex_type_p_Int,
222239
[r].[RequiredRelated_Ints] = @complex_type_p_Ints,
223240
[r].[RequiredRelated_Name] = @complex_type_p_Name,
224241
[r].[RequiredRelated_String] = @complex_type_p_String,
225-
[r].[RequiredRelated_OptionalNested_Id] = @complex_type_p_Id,
226-
[r].[RequiredRelated_OptionalNested_Int] = @complex_type_p_Int,
227-
[r].[RequiredRelated_OptionalNested_Ints] = @complex_type_p_Ints,
228-
[r].[RequiredRelated_OptionalNested_Name] = @complex_type_p_Name,
229-
[r].[RequiredRelated_OptionalNested_String] = @complex_type_p_String,
230-
[r].[RequiredRelated_RequiredNested_Id] = @complex_type_p_Id,
231-
[r].[RequiredRelated_RequiredNested_Int] = @complex_type_p_Int,
232-
[r].[RequiredRelated_RequiredNested_Ints] = @complex_type_p_Ints,
233-
[r].[RequiredRelated_RequiredNested_Name] = @complex_type_p_Name,
234-
[r].[RequiredRelated_RequiredNested_String] = @complex_type_p_String
242+
[r].[RequiredRelated_OptionalNested_Id] = NULL,
243+
[r].[RequiredRelated_OptionalNested_Int] = NULL,
244+
[r].[RequiredRelated_OptionalNested_Ints] = NULL,
245+
[r].[RequiredRelated_OptionalNested_Name] = NULL,
246+
[r].[RequiredRelated_OptionalNested_String] = NULL,
247+
[r].[RequiredRelated_RequiredNested_Id] = @complex_type_p_RequiredNested_Id,
248+
[r].[RequiredRelated_RequiredNested_Int] = @complex_type_p_RequiredNested_Int,
249+
[r].[RequiredRelated_RequiredNested_Ints] = @complex_type_p_RequiredNested_Ints,
250+
[r].[RequiredRelated_RequiredNested_Name] = @complex_type_p_RequiredNested_Name,
251+
[r].[RequiredRelated_RequiredNested_String] = @complex_type_p_RequiredNested_String
235252
FROM [RootEntity] AS [r]
236253
""");
237254
}
@@ -356,6 +373,13 @@ FROM [RootEntity] AS [r]
356373
""");
357374
}
358375

376+
public override async Task Update_association_with_null_required_nested_association()
377+
{
378+
await base.Update_association_with_null_required_nested_association();
379+
380+
AssertExecuteUpdateSql();
381+
}
382+
359383
#endregion Update association
360384

361385
#region Update collection

0 commit comments

Comments
 (0)