From ee9d4b2067f1a46d92d091e16bcb3ff3a3f639b7 Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Tue, 13 Aug 2024 18:35:55 +0100 Subject: [PATCH] More review updates. --- .../RelationalEntityTypeExtensions.cs | 10 +- .../RelationalModelValidator.cs | 2 +- .../Properties/RelationalStrings.Designer.cs | 10 +- .../Properties/RelationalStrings.resx | 5 +- .../SqlServerOpenJsonExpression.cs | 19 ++- .../Internal/SqlServerJsonPostprocessor.cs | 77 ++++++------ .../SqlServerJsonElementTypeMapping.cs | 31 ++--- .../Internal/SqlServerStringTypeMapping.cs | 6 +- .../Query/AdHocJsonQueryTestBase.cs | 8 +- .../Query/JsonQueryRelationalFixture.cs | 19 ++- .../RelationalModelValidatorTest.Json.cs | 2 +- .../AdHocJsonQuerySqlServerJsonTypeTest.cs | 22 +++- .../Query/JsonQueryJsonTypeSqlServerTest.cs | 114 +++++++++++++++--- ...veCollectionsQuerySqlServerJsonTypeTest.cs | 20 +-- .../SqlServerDatabaseModelFactoryTest.cs | 3 +- .../TestUtilities/SqlServerCondition.cs | 1 + .../SqlServerConditionAttribute.cs | 5 + .../TestUtilities/TestEnvironment.cs | 3 + .../Update/JsonUpdateJsonTypeSqlServerTest.cs | 4 +- .../SqlServerModelValidatorTest.cs | 6 +- 20 files changed, 250 insertions(+), 117 deletions(-) diff --git a/src/EFCore.Relational/Extensions/RelationalEntityTypeExtensions.cs b/src/EFCore.Relational/Extensions/RelationalEntityTypeExtensions.cs index bdd907f01b2..cb2e437c378 100644 --- a/src/EFCore.Relational/Extensions/RelationalEntityTypeExtensions.cs +++ b/src/EFCore.Relational/Extensions/RelationalEntityTypeExtensions.cs @@ -1606,9 +1606,8 @@ public static void SetContainerColumnName(this IMutableEntityType entityType, st /// The entity type to get the container column name for. /// The container column name to which the entity type is mapped. public static string? GetContainerColumnName(this IReadOnlyEntityType entityType) - => entityType.FindAnnotation(RelationalAnnotationNames.ContainerColumnName)?.Value is string columnName - ? columnName - : (entityType.FindOwnership()?.PrincipalEntityType.GetContainerColumnName()); + => entityType.FindAnnotation(RelationalAnnotationNames.ContainerColumnName)?.Value as string + ?? entityType.FindOwnership()?.PrincipalEntityType.GetContainerColumnName(); /// /// Sets the column type to use for the container column to which the entity type is mapped. @@ -1646,9 +1645,8 @@ public static void SetContainerColumnType(this IMutableEntityType entityType, st /// The entity type. /// The database column type. public static string? GetContainerColumnType(this IReadOnlyEntityType entityType) - => entityType.FindAnnotation(RelationalAnnotationNames.ContainerColumnType)?.Value is string columnType - ? columnType - : (entityType.FindOwnership()?.PrincipalEntityType.GetContainerColumnType()); + => entityType.FindAnnotation(RelationalAnnotationNames.ContainerColumnType)?.Value as string + ?? entityType.FindOwnership()?.PrincipalEntityType.GetContainerColumnType(); /// /// Sets the type mapping for the container column to which the entity type is mapped. diff --git a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs index cbc0cf877a4..4a33ba5f4ad 100644 --- a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs +++ b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs @@ -2598,7 +2598,7 @@ protected virtual void ValidateJsonEntities( { if (entityType.FindOwnership()?.PrincipalEntityType.IsOwned() == true) { - throw new InvalidOperationException(RelationalStrings.ContainerTypeOnNonRoot(entityType.DisplayName())); + throw new InvalidOperationException(RelationalStrings.ContainerTypeOnNestedOwnedEntityType(entityType.DisplayName())); } if (!entityType.IsOwned() diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index 0fcc8691b24..06b95ce7936 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -198,9 +198,9 @@ public static string ConflictingTypeMappingsInferredForColumn(object? column) /// /// The entity type '{entityType}' has a container column type configured, but is nested in another owned type. The container column type can only be specified on a top-level owned type mapped to a container. /// - public static string ContainerTypeOnNonRoot(object? entityType) + public static string ContainerTypeOnNestedOwnedEntityType(object? entityType) => string.Format( - GetString("ContainerTypeOnNonRoot", nameof(entityType)), + GetString("ContainerTypeOnNestedOwnedEntityType", nameof(entityType)), entityType); /// @@ -1069,6 +1069,12 @@ public static string JsonCantNavigateToParentEntity(object? jsonEntity, object? GetString("JsonCantNavigateToParentEntity", nameof(jsonEntity), nameof(parentEntity), nameof(navigation)), jsonEntity, parentEntity, navigation); + /// + /// The database returned the empty string when a JSON object was expected. + /// + public static string JsonEmptyString + => GetString("JsonEmptyString"); + /// /// Entity '{jsonType}' is mapped to JSON and also to a table or view '{tableOrViewName}', but its owner '{ownerType}' is mapped to a different table or view '{ownerTableOrViewName}'. Every entity mapped to JSON must also map to the same table or view as its owner. /// diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index 8ad50733a0d..883393063fb 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -187,7 +187,7 @@ Conflicting type mappings were inferred for column '{column}'. - + The entity type '{entityType}' has a container column type configured, but is nested in another owned type. The container column type can only be specified on a top-level owned type mapped to a container. @@ -520,6 +520,9 @@ Can't navigate from JSON-mapped entity '{jsonEntity}' to its parent entity '{parentEntity}' using navigation '{navigation}'. Entities mapped to JSON can only navigate to their children. + + The database returned the empty string when a JSON object was expected. + Entity '{jsonType}' is mapped to JSON and also to a table or view '{tableOrViewName}', but its owner '{ownerType}' is mapped to a different table or view '{ownerTableOrViewName}'. Every entity mapped to JSON must also map to the same table or view as its owner. diff --git a/src/EFCore.SqlServer/Query/Internal/SqlExpressions/SqlServerOpenJsonExpression.cs b/src/EFCore.SqlServer/Query/Internal/SqlExpressions/SqlServerOpenJsonExpression.cs index 11f0b64226a..cc213ae489c 100644 --- a/src/EFCore.SqlServer/Query/Internal/SqlExpressions/SqlServerOpenJsonExpression.cs +++ b/src/EFCore.SqlServer/Query/Internal/SqlExpressions/SqlServerOpenJsonExpression.cs @@ -152,6 +152,15 @@ public virtual SqlServerOpenJsonExpression Update( : new SqlServerOpenJsonExpression(Alias, jsonExpression, path, columnInfos); } + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public virtual SqlServerOpenJsonExpression Update(SqlExpression sqlExpression) + => new(Alias, sqlExpression, Path, ColumnInfos); + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in @@ -321,14 +330,4 @@ public readonly record struct ColumnInfo( RelationalTypeMapping TypeMapping, IReadOnlyList? Path = null, bool AsJson = false); - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - public virtual SqlServerOpenJsonExpression Update(SqlExpression sqlExpression) - => new(Alias, sqlExpression, Path, ColumnInfos); - } diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerJsonPostprocessor.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerJsonPostprocessor.cs index 1d6a007f43a..3744f827348 100644 --- a/src/EFCore.SqlServer/Query/Internal/SqlServerJsonPostprocessor.cs +++ b/src/EFCore.SqlServer/Query/Internal/SqlServerJsonPostprocessor.cs @@ -49,29 +49,6 @@ public Expression Process(Expression expression) return Visit(expression); } - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - protected override Expression VisitExtension(Expression expression) - => expression switch - { - SqlServerOpenJsonExpression openJsonExpression - => openJsonExpression is { JsonExpression.TypeMapping: SqlServerStringTypeMapping { StoreType: "json" } } or - { JsonExpression.TypeMapping: SqlServerJsonElementTypeMapping { StoreType: "json" } } - ? openJsonExpression.Update( - new SqlUnaryExpression( - ExpressionType.Convert, - (SqlExpression)Visit(openJsonExpression.JsonExpression), - typeof(string), - typeMappingSource.FindMapping(typeof(string))!)) - : openJsonExpression, - - _ => base.VisitExtension(expression) - }; - /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in @@ -100,14 +77,16 @@ SqlServerOpenJsonExpression openJsonExpression var table = selectExpression.Tables[i]; if (table.UnwrapJoin() is SqlServerOpenJsonExpression { ColumnInfos: { } columnInfos } openJsonExpression + // Condition 1: an ordering/projection still refers to the OPENJSON's [key] column - it needs to be preserved. && (selectExpression.Orderings.Select(o => o.Expression) - .Concat(selectExpression.Projection.Select(p => p.Expression)) - .Any(x => IsKeyColumn(x, openJsonExpression.Alias)) - || - // Condition 2: a column type in the WITH clause is a SQL Server "CLR type" (e.g. hierarchy id). - // These are not supported by OPENJSON with WITH. - columnInfos.Any(c => c.TypeMapping.StoreType is "hierarchyid"))) + .Concat(selectExpression.Projection.Select(p => p.Expression)) + .Any(x => IsKeyColumn(x, openJsonExpression.Alias)) + || + + // Condition 2: a column type in the WITH clause is a SQL Server "CLR type" (e.g. hierarchy id). + // These are not supported by OPENJSON with WITH. + columnInfos.Any(c => c.TypeMapping.StoreType is "hierarchyid"))) { // Remove the WITH clause from the OPENJSON expression var newOpenJsonExpression = openJsonExpression.Update( @@ -222,7 +201,11 @@ when _columnsToRewrite.TryGetValue((columnExpression.TableAlias, columnExpressio // The new OPENJSON (without WITH) always projects a `value` column, instead of a properly named column for individual // values inside; create a new ColumnExpression with that name. SqlExpression rewrittenColumn = new ColumnExpression( - "value", columnExpression.TableAlias, columnExpression.Type, _nvarcharMaxTypeMapping, columnExpression.IsNullable); + "value", + columnExpression.TableAlias, + columnExpression.Type, + _nvarcharMaxTypeMapping, + columnExpression.IsNullable); // Prepend the path from the OPENJSON/WITH to the path in the JsonScalarExpression var path = columnInfo.Path is null @@ -230,7 +213,10 @@ when _columnsToRewrite.TryGetValue((columnExpression.TableAlias, columnExpressio : columnInfo.Path.Concat(jsonScalarExpression.Path).ToList(); return new JsonScalarExpression( - rewrittenColumn, path, jsonScalarExpression.Type, jsonScalarExpression.TypeMapping, + rewrittenColumn, + path, + jsonScalarExpression.Type, + jsonScalarExpression.TypeMapping, jsonScalarExpression.IsNullable); } @@ -242,23 +228,44 @@ when _columnsToRewrite.TryGetValue((columnExpression.TableAlias, columnExpressio case JsonScalarExpression { TypeMapping.StoreTypeNameBase: "varbinary" or "binary" } jsonScalar: { var name = jsonScalar.Path.LastOrDefault(ps => ps.PropertyName is not null).PropertyName - ?? (jsonScalar.Json as ColumnExpression)?.Name - ?? "Json"; + ?? (jsonScalar.Json as ColumnExpression)?.Name + ?? "Json"; var tableAlias = sqlAliasManager.GenerateTableAlias(name); var join = new OuterApplyExpression( new SqlServerOpenJsonExpression( - tableAlias, jsonScalar.Json, path: null, + tableAlias, + jsonScalar.Json, + path: null, columnInfos: [new(name, jsonScalar.TypeMapping, jsonScalar.Path)])); // We record the new OUTER APPLY in _openWithOuterAppliesToAdd (it gets added after visiting the SelectExpression above), // and return a ColumnExpression referencing that new OUTER APPLY. _openjsonOuterAppliesToAdd.Add(join); - return new ColumnExpression(name, tableAlias, jsonScalar.Type, jsonScalar.TypeMapping, + return new ColumnExpression( + name, + tableAlias, + jsonScalar.Type, + jsonScalar.TypeMapping, jsonScalar.IsNullable); } + case SqlServerOpenJsonExpression openJsonExpression: + // Currently, OPEN_JSON does not accept a "json" type, so we must cast the value to a string. + // We do this for both the case where is a string type mapping for a top-level property with the store type + // of "json", and when there is an "element" type mapping to something in the document but is now being used + // with OPEN_JSON. + return openJsonExpression is { JsonExpression.TypeMapping: SqlServerStringTypeMapping { StoreType: "json" } } or + { JsonExpression.TypeMapping: SqlServerJsonElementTypeMapping { StoreType: "json" } } + ? openJsonExpression.Update( + new SqlUnaryExpression( + ExpressionType.Convert, + (SqlExpression)Visit(openJsonExpression.JsonExpression), + typeof(string), + typeMappingSource.FindMapping(typeof(string))!)) + : base.Visit(expression); + default: return base.Visit(expression); } diff --git a/src/EFCore.SqlServer/Storage/Internal/SqlServerJsonElementTypeMapping.cs b/src/EFCore.SqlServer/Storage/Internal/SqlServerJsonElementTypeMapping.cs index b855dd253c5..e2e2b3ae146 100644 --- a/src/EFCore.SqlServer/Storage/Internal/SqlServerJsonElementTypeMapping.cs +++ b/src/EFCore.SqlServer/Storage/Internal/SqlServerJsonElementTypeMapping.cs @@ -16,18 +16,12 @@ namespace Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal; /// public class SqlServerJsonElementTypeMapping : JsonTypeMapping { + private static readonly MethodInfo CreateUtf8StreamMethod + = typeof(SqlServerJsonElementTypeMapping).GetMethod(nameof(CreateUtf8Stream), [typeof(string)])!; + private static readonly MethodInfo GetStringMethod = typeof(DbDataReader).GetRuntimeMethod(nameof(DbDataReader.GetString), [typeof(int)])!; - private static readonly PropertyInfo UTF8Property - = typeof(Encoding).GetProperty(nameof(Encoding.UTF8))!; - - private static readonly MethodInfo EncodingGetBytesMethod - = typeof(Encoding).GetMethod(nameof(Encoding.GetBytes), [typeof(string)])!; - - private static readonly ConstructorInfo MemoryStreamConstructor - = typeof(MemoryStream).GetConstructor([typeof(byte[])])!; - /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in @@ -64,6 +58,17 @@ public SqlServerJsonElementTypeMapping(string storeType) public override MethodInfo GetDataReaderMethod() => GetStringMethod; + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public static MemoryStream CreateUtf8Stream(string json) + => json == "" + ? throw new InvalidOperationException(RelationalStrings.JsonEmptyString) + : new MemoryStream(Encoding.UTF8.GetBytes(json)); + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in @@ -71,12 +76,7 @@ public override MethodInfo GetDataReaderMethod() /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public override Expression CustomizeDataReaderExpression(Expression expression) - => Expression.New( - MemoryStreamConstructor, - Expression.Call( - Expression.Property(null, UTF8Property), - EncodingGetBytesMethod, - expression)); + => Expression.Call(CreateUtf8StreamMethod, expression); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -127,6 +127,7 @@ protected override void ConfigureParameter(DbParameter parameter) if (StoreType == "json" && parameter is SqlParameter sqlParameter) // To avoid crashing wrapping providers { + // TODO:SQLJSON Issue #34414 sqlParameter.SqlDbType = ((SqlDbType)35); } diff --git a/src/EFCore.SqlServer/Storage/Internal/SqlServerStringTypeMapping.cs b/src/EFCore.SqlServer/Storage/Internal/SqlServerStringTypeMapping.cs index c286786d787..b0d232d53e9 100644 --- a/src/EFCore.SqlServer/Storage/Internal/SqlServerStringTypeMapping.cs +++ b/src/EFCore.SqlServer/Storage/Internal/SqlServerStringTypeMapping.cs @@ -40,6 +40,7 @@ public class SqlServerStringTypeMapping : StringTypeMapping /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// + // TODO:SQLJSON Issue #34414 public static SqlServerStringTypeMapping JsonTypeDefault { get; } = new("json", sqlDbType: (SqlDbType)35); /// @@ -85,9 +86,7 @@ public SqlServerStringTypeMapping( private static string GetDefaultStoreName(bool unicode, bool fixedLength) => unicode ? fixedLength ? "nchar" : "nvarchar" - : fixedLength - ? "char" - : "varchar"; + : fixedLength ? "char" : "varchar"; private static DbType? GetDbType(bool unicode, bool fixedLength) => unicode @@ -157,6 +156,7 @@ protected override void ConfigureParameter(DbParameter parameter) var value = parameter.Value; var length = (value as string)?.Length; + // TODO:SQLJSON Issue #34414 var sqlDbType = _sqlDbType ?? (StoreType == "json" ? (SqlDbType)35 : null); diff --git a/test/EFCore.Relational.Specification.Tests/Query/AdHocJsonQueryTestBase.cs b/test/EFCore.Relational.Specification.Tests/Query/AdHocJsonQueryTestBase.cs index 888fe994cbb..04dd844badd 100644 --- a/test/EFCore.Relational.Specification.Tests/Query/AdHocJsonQueryTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Query/AdHocJsonQueryTestBase.cs @@ -214,7 +214,7 @@ public virtual async Task Accessing_missing_navigation_works(bool async) } } - [ConditionalTheory(Skip = "TODO:SQLJSON Returns empty (invalid) JSON (See BadJson.cs)")] + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual async Task Missing_navigation_works_with_deduplication(bool async) { @@ -225,7 +225,7 @@ public virtual async Task Missing_navigation_works_with_deduplication(bool async using (var context = contextFactory.CreateContext()) { - var result = context.Set().OrderBy(x => x.Id).Select( + var queryable = context.Set().OrderBy(x => x.Id).Select( x => new { x, @@ -235,7 +235,9 @@ public virtual async Task Missing_navigation_works_with_deduplication(bool async NestedOptional = x.Json.OptionalReference.Nested, NestedRequired = x.Json.RequiredReference.Nested, x.Json.Collection, - }).AsNoTracking().ToList(); + }).AsNoTracking(); + + var result = async ? await queryable.ToListAsync() : queryable.ToList(); Assert.Equal(4, result.Count); Assert.NotNull(result[0].OptionalReference); diff --git a/test/EFCore.Relational.Specification.Tests/Query/JsonQueryRelationalFixture.cs b/test/EFCore.Relational.Specification.Tests/Query/JsonQueryRelationalFixture.cs index 3c9e1d72cc7..8a054c59f57 100644 --- a/test/EFCore.Relational.Specification.Tests/Query/JsonQueryRelationalFixture.cs +++ b/test/EFCore.Relational.Specification.Tests/Query/JsonQueryRelationalFixture.cs @@ -7,7 +7,7 @@ namespace Microsoft.EntityFrameworkCore.Query; #nullable disable -public abstract class JsonQueryRelationalFixture: JsonQueryFixtureBase, ITestSqlLoggerFactory +public abstract class JsonQueryRelationalFixture : JsonQueryFixtureBase, ITestSqlLoggerFactory { public new RelationalTestStore TestStore => (RelationalTestStore)base.TestStore; @@ -22,8 +22,21 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con modelBuilder.Entity().OwnsOne(x => x.OwnedReferenceRoot).ToJson(); modelBuilder.Entity().OwnsMany(x => x.OwnedCollectionRoot).ToJson(); - modelBuilder.Entity().OwnsOne(x => x.OwnedReferenceRoot).ToJson("json_reference_custom_naming"); - modelBuilder.Entity().OwnsMany(x => x.OwnedCollectionRoot).ToJson("json_collection_custom_naming"); + modelBuilder.Entity().OwnsOne( + x => x.OwnedReferenceRoot, b => + { + b.ToJson("json_reference_custom_naming"); + b.OwnsOne(x => x.OwnedReferenceBranch); + b.OwnsMany(x => x.OwnedCollectionBranch); + }); + + modelBuilder.Entity().OwnsMany( + x => x.OwnedCollectionRoot, b => + { + b.ToJson("json_collection_custom_naming"); + b.OwnsOne(x => x.OwnedReferenceBranch); + b.OwnsMany(x => x.OwnedCollectionBranch); + }); modelBuilder.Entity().OwnsMany(x => x.OwnedCollection).ToJson(); diff --git a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.Json.cs b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.Json.cs index f713efd6f47..52cdd7793d2 100644 --- a/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.Json.cs +++ b/test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.Json.cs @@ -45,7 +45,7 @@ public void Throw_when_non_root_json_entity_has_column_type_set() }); VerifyError( - RelationalStrings.ContainerTypeOnNonRoot(nameof(ValidatorJsonOwnedBranch)), + RelationalStrings.ContainerTypeOnNestedOwnedEntityType(nameof(ValidatorJsonOwnedBranch)), modelBuilder); } diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/AdHocJsonQuerySqlServerJsonTypeTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/AdHocJsonQuerySqlServerJsonTypeTest.cs index 0f26fc47b16..fb0e7c2d8ec 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/AdHocJsonQuerySqlServerJsonTypeTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/AdHocJsonQuerySqlServerJsonTypeTest.cs @@ -6,9 +6,27 @@ namespace Microsoft.EntityFrameworkCore.Query; -// TODO:SQLJSON Enable tests -internal class AdHocJsonQuerySqlServerJsonTypeTest : AdHocJsonQuerySqlServerTestBase +[SqlServerCondition(SqlServerCondition.SupportsJsonType)] +public class AdHocJsonQuerySqlServerJsonTypeTest : AdHocJsonQuerySqlServerTestBase { + public override async Task Missing_navigation_works_with_deduplication(bool async) + { + // TODO:SQLJSON Returns empty (invalid) JSON (See BadJson.cs) + if (async) + { + Assert.Equal( + "Unable to cast object of type 'System.DBNull' to type 'System.String'.", + (await Assert.ThrowsAsync(() => base.Missing_navigation_works_with_deduplication(true))).Message); + } + else + { + Assert.Equal( + RelationalStrings.JsonEmptyString, + (await Assert.ThrowsAsync(() => base.Missing_navigation_works_with_deduplication(false))) + .Message); + } + } + public override async Task Contains_on_nested_collection_with_init_only_navigation(bool async) // TODO:SQLJSON (See JsonTypeToFunction.cs) => Assert.Equal( diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/JsonQueryJsonTypeSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/JsonQueryJsonTypeSqlServerTest.cs index bb5ceac88c2..95f302374ec 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/JsonQueryJsonTypeSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/JsonQueryJsonTypeSqlServerTest.cs @@ -7,8 +7,8 @@ namespace Microsoft.EntityFrameworkCore.Query; -// TODO:SQLJSON Enable tests -internal class JsonQueryJsonTypeSqlServerTest : JsonQueryRelationalTestBase +[SqlServerCondition(SqlServerCondition.SupportsJsonType)] +public class JsonQueryJsonTypeSqlServerTest : JsonQueryRelationalTestBase { public JsonQueryJsonTypeSqlServerTest(JsonQueryJsonTypeSqlServerFixture fixture, ITestOutputHelper testOutputHelper) : base(fixture) @@ -593,10 +593,22 @@ FROM [JsonEntitiesSingleOwned] AS [j] """); } - [ConditionalTheory(Skip = "TODO:SQLJSON Returns empty (invalid) JSON (See BadJson.cs)")] public override async Task Left_join_json_entities(bool async) { - await base.Left_join_json_entities(async); + // TODO:SQLJSON Returns empty (invalid) JSON (See BadJson.cs) + if (async) + { + Assert.Equal( + "Unable to cast object of type 'System.DBNull' to type 'System.String'.", + (await Assert.ThrowsAsync(() => base.Left_join_json_entities(true))).Message); + } + else + { + Assert.Equal( + RelationalStrings.JsonEmptyString, + (await Assert.ThrowsAsync(() => base.Left_join_json_entities(false))) + .Message); + } AssertSql( """ @@ -606,10 +618,22 @@ FROM [JsonEntitiesSingleOwned] AS [j] """); } - [ConditionalTheory(Skip = "TODO:SQLJSON Returns empty (invalid) JSON (See BadJson.cs)")] public override async Task Left_join_json_entities_complex_projection(bool async) { - await base.Left_join_json_entities_complex_projection(async); + // TODO:SQLJSON Returns empty (invalid) JSON (See BadJson.cs) + if (async) + { + Assert.Equal( + "Unable to cast object of type 'System.DBNull' to type 'System.String'.", + (await Assert.ThrowsAsync(() => base.Left_join_json_entities_complex_projection(true))).Message); + } + else + { + Assert.Equal( + RelationalStrings.JsonEmptyString, + (await Assert.ThrowsAsync(() => base.Left_join_json_entities_complex_projection(false))) + .Message); + } AssertSql( """ @@ -734,10 +758,22 @@ ORDER BY [j].[Id] """); } - [ConditionalTheory(Skip = "TODO:SQLJSON Returns empty (invalid) JSON (See BadJson.cs)")] public override async Task Json_entity_with_inheritance_basic_projection(bool async) { - await base.Json_entity_with_inheritance_basic_projection(async); + // TODO:SQLJSON Returns empty (invalid) JSON (See BadJson.cs) + if (async) + { + Assert.Equal( + "Unable to cast object of type 'System.DBNull' to type 'System.String'.", + (await Assert.ThrowsAsync(() => base.Json_entity_with_inheritance_basic_projection(true))).Message); + } + else + { + Assert.Equal( + RelationalStrings.JsonEmptyString, + (await Assert.ThrowsAsync(() => base.Json_entity_with_inheritance_basic_projection(false))) + .Message); + } AssertSql( """ @@ -905,10 +941,22 @@ public override async Task Json_collection_index_in_projection_using_untranslata message); } - [ConditionalTheory(Skip = "TODO:SQLJSON Returns empty (invalid) JSON (See BadJson.cs)")] public override async Task Json_collection_index_outside_bounds(bool async) { - await base.Json_collection_index_outside_bounds(async); + // TODO:SQLJSON Returns empty (invalid) JSON (See BadJson.cs) + if (async) + { + Assert.Equal( + "Unable to cast object of type 'System.DBNull' to type 'System.String'.", + (await Assert.ThrowsAsync(() => base.Json_collection_index_outside_bounds(true))).Message); + } + else + { + Assert.Equal( + RelationalStrings.JsonEmptyString, + (await Assert.ThrowsAsync(() => base.Json_collection_index_outside_bounds(false))) + .Message); + } AssertSql( """ @@ -917,10 +965,22 @@ FROM [JsonEntitiesBasic] AS [j] """); } - [ConditionalTheory(Skip = "TODO:SQLJSON Returns empty (invalid) JSON (See BadJson.cs)")] public override async Task Json_collection_index_outside_bounds2(bool async) { - await base.Json_collection_index_outside_bounds2(async); + // TODO:SQLJSON Returns empty (invalid) JSON (See BadJson.cs) + if (async) + { + Assert.Equal( + "Unable to cast object of type 'System.DBNull' to type 'System.String'.", + (await Assert.ThrowsAsync(() => base.Json_collection_index_outside_bounds2(true))).Message); + } + else + { + Assert.Equal( + RelationalStrings.JsonEmptyString, + (await Assert.ThrowsAsync(() => base.Json_collection_index_outside_bounds2(false))) + .Message); + } AssertSql( """ @@ -2042,7 +2102,6 @@ FROM [JsonEntitiesBasic] AS [j2] """); } - [ConditionalTheory(Skip = "TODO:SQLJSON Returns empty (invalid) JSON (See BadJson.cs)")] public override async Task Group_by_FirstOrDefault_on_json_scalar(bool async) { await base.Group_by_FirstOrDefault_on_json_scalar(async); @@ -2072,10 +2131,19 @@ FROM [JsonEntitiesBasic] AS [j2] """); } - [ConditionalTheory(Skip = "TODO:SQLJSON Returns empty (invalid) JSON (See BadJson.cs)")] public override async Task Group_by_Skip_Take_on_json_scalar(bool async) { - await base.Group_by_Skip_Take_on_json_scalar(async); + // TODO:SQLJSON Returns empty (invalid) JSON (See BadJson.cs) + if (async) + { + Assert.Equal( + "Unable to cast object of type 'System.DBNull' to type 'System.String'.", + (await Assert.ThrowsAsync(() => base.Group_by_Skip_Take_on_json_scalar(true))).Message); + } + else + { + await base.Group_by_Skip_Take_on_json_scalar(false); + } AssertSql( """ @@ -2934,10 +3002,22 @@ SELECT JSON_QUERY([m].[OwnedReferenceRoot], '$.OwnedCollectionBranch'), [m].[Id] """); } - [ConditionalTheory(Skip = "TODO:SQLJSON Returns empty (invalid) JSON (See BadJson.cs)")] public override async Task FromSql_on_entity_with_json_inheritance_on_base(bool async) { - await base.FromSql_on_entity_with_json_inheritance_on_base(async); + // TODO:SQLJSON Returns empty (invalid) JSON (See BadJson.cs) + if (async) + { + Assert.Equal( + "Unable to cast object of type 'System.DBNull' to type 'System.String'.", + (await Assert.ThrowsAsync(() => base.FromSql_on_entity_with_json_inheritance_on_base(true))).Message); + } + else + { + Assert.Equal( + RelationalStrings.JsonEmptyString, + (await Assert.ThrowsAsync(() => base.FromSql_on_entity_with_json_inheritance_on_base(false))) + .Message); + } AssertSql( """ diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/PrimitiveCollectionsQuerySqlServerJsonTypeTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/PrimitiveCollectionsQuerySqlServerJsonTypeTest.cs index ce821d8b95a..d83e60993a8 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/PrimitiveCollectionsQuerySqlServerJsonTypeTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/PrimitiveCollectionsQuerySqlServerJsonTypeTest.cs @@ -5,9 +5,8 @@ namespace Microsoft.EntityFrameworkCore.Query; -// TODO:SQLJSON Enable tests -[SqlServerCondition(SqlServerCondition.SupportsFunctions2022)] -internal class PrimitiveCollectionsQuerySqlServerJsonTypeTest : PrimitiveCollectionsQueryRelationalTestBase< +[SqlServerCondition(SqlServerCondition.SupportsFunctions2022 | SqlServerCondition.SupportsJsonType)] +public class PrimitiveCollectionsQuerySqlServerJsonTypeTest : PrimitiveCollectionsQueryRelationalTestBase< PrimitiveCollectionsQuerySqlServerJsonTypeTest.PrimitiveCollectionsQuerySqlServerFixture> { public PrimitiveCollectionsQuerySqlServerJsonTypeTest(PrimitiveCollectionsQuerySqlServerFixture fixture, ITestOutputHelper testOutputHelper) @@ -827,7 +826,7 @@ public override async Task Column_collection_of_nullable_ints_Contains(bool asyn FROM [PrimitiveCollectionsEntity] AS [p] WHERE 10 IN ( SELECT [n].[value] - FROM OPENJSON([p].[NullableInts]) WITH ([value] int '$') AS [n] + FROM OPENJSON(CAST([p].[NullableInts] AS nvarchar(max))) WITH ([value] int '$') AS [n] ) """); } @@ -842,7 +841,7 @@ public override async Task Column_collection_of_nullable_ints_Contains_null(bool FROM [PrimitiveCollectionsEntity] AS [p] WHERE EXISTS ( SELECT 1 - FROM OPENJSON([p].[NullableInts]) WITH ([value] int '$') AS [n] + FROM OPENJSON(CAST([p].[NullableInts] AS nvarchar(max))) WITH ([value] int '$') AS [n] WHERE [n].[value] IS NULL) """); } @@ -1816,7 +1815,7 @@ public override async Task Project_collection_of_nullable_ints_with_paging(bool FROM [PrimitiveCollectionsEntity] AS [p] OUTER APPLY ( SELECT TOP(20) CAST([n].[value] AS int) AS [value], [n].[key], CAST([n].[key] AS int) AS [c] - FROM OPENJSON([p].[NullableInts]) AS [n] + FROM OPENJSON(CAST([p].[NullableInts] AS nvarchar(max))) AS [n] ORDER BY CAST([n].[key] AS int) ) AS [n0] ORDER BY [p].[Id], [n0].[c] @@ -1833,7 +1832,7 @@ public override async Task Project_collection_of_nullable_ints_with_paging2(bool FROM [PrimitiveCollectionsEntity] AS [p] OUTER APPLY ( SELECT CAST([n].[value] AS int) AS [value], [n].[key] - FROM OPENJSON([p].[NullableInts]) AS [n] + FROM OPENJSON(CAST([p].[NullableInts] AS nvarchar(max))) AS [n] ORDER BY CAST([n].[value] AS int) OFFSET 1 ROWS ) AS [n0] @@ -1851,7 +1850,7 @@ public override async Task Project_collection_of_nullable_ints_with_paging3(bool FROM [PrimitiveCollectionsEntity] AS [p] OUTER APPLY ( SELECT CAST([n].[value] AS int) AS [value], [n].[key], CAST([n].[key] AS int) AS [c] - FROM OPENJSON([p].[NullableInts]) AS [n] + FROM OPENJSON(CAST([p].[NullableInts] AS nvarchar(max))) AS [n] ORDER BY CAST([n].[key] AS int) OFFSET 2 ROWS ) AS [n0] @@ -1909,12 +1908,12 @@ public override async Task Project_empty_collection_of_nullables_and_collection_ FROM [PrimitiveCollectionsEntity] AS [p] OUTER APPLY ( SELECT CAST([n].[value] AS int) AS [value], [n].[key], CAST([n].[key] AS int) AS [c] - FROM OPENJSON([p].[NullableInts]) AS [n] + FROM OPENJSON(CAST([p].[NullableInts] AS nvarchar(max))) AS [n] WHERE 0 = 1 ) AS [n1] OUTER APPLY ( SELECT CAST([n0].[value] AS int) AS [value], [n0].[key], CAST([n0].[key] AS int) AS [c] - FROM OPENJSON([p].[NullableInts]) AS [n0] + FROM OPENJSON(CAST([p].[NullableInts] AS nvarchar(max))) AS [n0] WHERE [n0].[value] IS NULL ) AS [n2] ORDER BY [p].[Id], [n1].[c], [n1].[key], [n2].[c] @@ -2081,6 +2080,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con b.PrimitiveCollection(e => e.Ints).HasColumnType("json"); b.PrimitiveCollection(e => e.Enums).HasColumnType("json"); b.PrimitiveCollection(e => e.NullableStrings).HasColumnType("json"); + b.PrimitiveCollection(e => e.NullableInts).HasColumnType("json"); }); } } diff --git a/test/EFCore.SqlServer.FunctionalTests/Scaffolding/SqlServerDatabaseModelFactoryTest.cs b/test/EFCore.SqlServer.FunctionalTests/Scaffolding/SqlServerDatabaseModelFactoryTest.cs index 20573fc9b3c..de336712544 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Scaffolding/SqlServerDatabaseModelFactoryTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Scaffolding/SqlServerDatabaseModelFactoryTest.cs @@ -2453,7 +2453,8 @@ nationalCharacterVaryingMaxColumn national char varying(max) NULL }, "DROP TABLE MaxColumns;"); - [ConditionalFact (Skip = "TODO:SQLJSON")] + [SqlServerCondition(SqlServerCondition.SupportsJsonType)] + [ConditionalFact] public void Handles_native_JSON_type() => Test( @" diff --git a/test/EFCore.SqlServer.FunctionalTests/TestUtilities/SqlServerCondition.cs b/test/EFCore.SqlServer.FunctionalTests/TestUtilities/SqlServerCondition.cs index 031ff85e9d6..2c3dd3d6104 100644 --- a/test/EFCore.SqlServer.FunctionalTests/TestUtilities/SqlServerCondition.cs +++ b/test/EFCore.SqlServer.FunctionalTests/TestUtilities/SqlServerCondition.cs @@ -21,4 +21,5 @@ public enum SqlServerCondition SupportsFunctions2017 = 1 << 12, SupportsFunctions2019 = 1 << 13, SupportsFunctions2022 = 1 << 14, + SupportsJsonType = 1 << 15, } diff --git a/test/EFCore.SqlServer.FunctionalTests/TestUtilities/SqlServerConditionAttribute.cs b/test/EFCore.SqlServer.FunctionalTests/TestUtilities/SqlServerConditionAttribute.cs index a28352fad6e..7f61d3a59d5 100644 --- a/test/EFCore.SqlServer.FunctionalTests/TestUtilities/SqlServerConditionAttribute.cs +++ b/test/EFCore.SqlServer.FunctionalTests/TestUtilities/SqlServerConditionAttribute.cs @@ -92,6 +92,11 @@ public ValueTask IsMetAsync() isMet &= TestEnvironment.IsFunctions2022Supported; } + if (Conditions.HasFlag(SqlServerCondition.SupportsJsonType)) + { + isMet &= TestEnvironment.IsJsonTypeSupported; + } + return ValueTask.FromResult(isMet); } diff --git a/test/EFCore.SqlServer.FunctionalTests/TestUtilities/TestEnvironment.cs b/test/EFCore.SqlServer.FunctionalTests/TestUtilities/TestEnvironment.cs index a93f0517a71..586583580f2 100644 --- a/test/EFCore.SqlServer.FunctionalTests/TestUtilities/TestEnvironment.cs +++ b/test/EFCore.SqlServer.FunctionalTests/TestUtilities/TestEnvironment.cs @@ -398,6 +398,9 @@ public static bool IsFunctions2022Supported } } + public static bool IsJsonTypeSupported + => false; + public static byte SqlServerMajorVersion => GetProductMajorVersion(); diff --git a/test/EFCore.SqlServer.FunctionalTests/Update/JsonUpdateJsonTypeSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Update/JsonUpdateJsonTypeSqlServerTest.cs index 851f6bf6bcc..3fcc3c283f9 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Update/JsonUpdateJsonTypeSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Update/JsonUpdateJsonTypeSqlServerTest.cs @@ -6,8 +6,8 @@ namespace Microsoft.EntityFrameworkCore.Update; -// TODO:SQLJSON Enable tests -internal class JsonUpdateJsonTypeSqlServerTest : JsonUpdateTestBase +[SqlServerCondition(SqlServerCondition.SupportsJsonType)] +public class JsonUpdateJsonTypeSqlServerTest : JsonUpdateTestBase { public JsonUpdateJsonTypeSqlServerTest(JsonUpdateJsonTypeSqlServerFixture fixture) : base(fixture) diff --git a/test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs b/test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs index e7459ba5e8b..53ceac990f7 100644 --- a/test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs +++ b/test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs @@ -15,11 +15,7 @@ public class SqlServerModelValidatorTest : RelationalModelValidatorTest public void Detects_use_of_json_column() { var modelBuilder = CreateConventionModelBuilder(); - modelBuilder.Entity( - b => - { - b.Property(e => e.Name).HasColumnType("json"); - }); + modelBuilder.Entity().Property(e => e.Name).HasColumnType("json"); VerifyWarning( SqlServerResources.LogJsonTypeExperimental(new TestLogger())