Skip to content

Commit 25c7e12

Browse files
committed
Fix to #21006 - Support a default value for non-nullable properties
Description Problem was that we don't have a good story for schema evolution in Cosmos (e.g. adding a new non-nullable property). We start generating shaper code expecting the new schema, but the old documents are not being updated. Effectively, querying for those modified entities stops working because the document no longer matches the expected shape. This is a major pain point and adoption blocker for EF Core Cosmos users. While designing a compelling schema evolution story is a large task and out of scope for now, we believe we can solve majority of the problems with a targeted change - when generating shaper for Cosmos entities, if required value is not present, we generate the CLR default rather than throw. Customer impact EF Core customers using Cosmos experience exceptions when querying for an entity which had it's schema modified, e.g. by adding a non-nullable property. There is no good workaround, apart from manually adding missing data to the document or using nullable properties instead. How found Reported by multiple customers on 9. Regression No. Testing Added multiple tests for scalar and reference scenarios, all supported types, converters vs no. missing value vs explicit nulls. Risk Low - Targeted fix, when constructing non-nullable property of an entity type we produce default of that type, rather than null. This does not affect keys, just regular scalar properties. Potential negative side-effect is that we lose the validation for scenarios where legitimate required property is introduced, but the value hasn't been provided for it in the document by accident. Added a quirk as an escape hatch to revert to previous behavior. Fixes #21006
1 parent f718f7a commit 25c7e12

File tree

9 files changed

+1574
-11
lines changed

9 files changed

+1574
-11
lines changed

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

+29-5
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ private abstract class CosmosProjectionBindingRemovingExpressionVisitorBase(
2020
bool trackQueryResults)
2121
: ExpressionVisitor
2222
{
23+
private static readonly bool UseOldBehavior21006 =
24+
AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue21006", out var enabled21006) && enabled21006;
25+
2326
private static readonly MethodInfo GetItemMethodInfo
2427
= typeof(JObject).GetRuntimeProperties()
2528
.Single(pi => pi.Name == "Item" && pi.GetIndexParameters()[0].ParameterType == typeof(string))
@@ -691,7 +694,11 @@ private Expression CreateGetValueExpression(
691694
&& !property.IsShadowProperty())
692695
{
693696
var readExpression = CreateGetValueExpression(
694-
jTokenExpression, storeName, type.MakeNullable(), property.GetTypeMapping());
697+
jTokenExpression,
698+
storeName,
699+
type.MakeNullable(),
700+
property.GetTypeMapping(),
701+
isNonNullableScalar: false);
695702

696703
var nonNullReadExpression = readExpression;
697704
if (nonNullReadExpression.Type != type)
@@ -712,15 +719,23 @@ private Expression CreateGetValueExpression(
712719
}
713720

714721
return Convert(
715-
CreateGetValueExpression(jTokenExpression, storeName, type.MakeNullable(), property.GetTypeMapping()),
722+
CreateGetValueExpression(
723+
jTokenExpression,
724+
storeName,
725+
type.MakeNullable(),
726+
property.GetTypeMapping(),
727+
// 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
728+
// returning defaults is supposed to help with evolving the schema - so this doesn't concern keys anyway (they shouldn't evolve)
729+
isNonNullableScalar: !property.IsNullable && !property.IsKey()),
716730
type);
717731
}
718732

719733
private Expression CreateGetValueExpression(
720734
Expression jTokenExpression,
721735
string storeName,
722736
Type type,
723-
CoreTypeMapping typeMapping = null)
737+
CoreTypeMapping typeMapping = null,
738+
bool isNonNullableScalar = false)
724739
{
725740
Check.DebugAssert(type.IsNullableType(), "Must read nullable type from JObject.");
726741

@@ -763,6 +778,7 @@ var body
763778
Constant(CosmosClientWrapper.Serializer)),
764779
converter.ConvertFromProviderExpression.Body);
765780

781+
var originalBodyType = body.Type;
766782
if (body.Type != type)
767783
{
768784
body = Convert(body, type);
@@ -783,7 +799,11 @@ var body
783799
}
784800
else
785801
{
786-
replaceExpression = Default(type);
802+
replaceExpression = isNonNullableScalar && !UseOldBehavior21006
803+
? Expression.Convert(
804+
Default(originalBodyType),
805+
type)
806+
: Default(type);
787807
}
788808

789809
body = Condition(
@@ -799,7 +819,11 @@ var body
799819
}
800820
else
801821
{
802-
valueExpression = ConvertJTokenToType(jTokenExpression, typeMapping?.ClrType.MakeNullable() ?? type);
822+
valueExpression = ConvertJTokenToType(
823+
jTokenExpression,
824+
(isNonNullableScalar && !UseOldBehavior21006
825+
? typeMapping?.ClrType
826+
: typeMapping?.ClrType.MakeNullable()) ?? type);
803827

804828
if (valueExpression.Type != type)
805829
{
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 entity (status code: {response.StatusCode}) for json: {json}");
41+
}
42+
}
43+
}

0 commit comments

Comments
 (0)