Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Block nested primitive collections in relational due to lack of query support #34325

Merged
merged 1 commit into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System.Data;
using Microsoft.EntityFrameworkCore.Internal;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Metadata.Internal;

namespace Microsoft.EntityFrameworkCore.Infrastructure;
Expand Down Expand Up @@ -64,6 +63,42 @@ public override void Validate(IModel model, IDiagnosticsLogger<DbLoggerCategory.
ValidateJsonEntities(model, logger);
}

/// <summary>
/// Validates the mapping of primitive collection properties the model.
/// </summary>
/// <param name="model">The model to validate.</param>
/// <param name="logger">The logger to use.</param>
protected override void ValidatePrimitiveCollections(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to do this for Cosmos too? Pragmatically, is it worth moving this to core given I don't think any provider supports it in 9 (though in theory one could, so some way to opt out)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We decided not to block it for Cosmos because nested collections of scalars where supported in EF8 on Cosmos, but not on relational. So this is not a breaking change for relational, but it would be for Cosmos, even though we don't know if any queries actually worked.

IModel model,
IDiagnosticsLogger<DbLoggerCategory.Model.Validation> logger)
{
base.ValidatePrimitiveCollections(model, logger);

foreach (var entityType in model.GetEntityTypes())
{
ValidateType(entityType);
}

static void ValidateType(ITypeBase typeBase)
{
foreach (var property in typeBase.GetDeclaredProperties())
{
if (property is { IsPrimitiveCollection: true }
&& property.GetTypeMapping().ElementTypeMapping?.ElementTypeMapping != null)
{
throw new InvalidOperationException(
RelationalStrings.NestedCollectionsNotSupported(
property.ClrType.ShortDisplayName(), typeBase.DisplayName(), property.Name));
}
}

foreach (var complexProperty in typeBase.GetDeclaredComplexProperties())
{
ValidateType(complexProperty.ComplexType);
}
}
}

/// <summary>
/// Validates the mapping/configuration of SQL queries in the model.
/// </summary>
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/EFCore.Relational/Properties/RelationalStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -950,6 +950,9 @@
<data name="NestedAmbientTransactionError" xml:space="preserve">
<value>A root ambient transaction was completed before the nested transaction. The nested transactions should be completed first.</value>
</data>
<data name="NestedCollectionsNotSupported" xml:space="preserve">
<value>The property '{propertyType} {type}.{property}' is a primitive collection of a primitive collection. Nested primitive collections are not yet supported with relational database providers.</value>
</data>
<data name="NoActiveTransaction" xml:space="preserve">
<value>The connection does not have any active transactions.</value>
</data>
Expand Down
6 changes: 3 additions & 3 deletions src/EFCore/Infrastructure/ModelValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1045,10 +1045,10 @@ protected virtual void ValidatePrimitiveCollections(
{
foreach (var entityType in model.GetEntityTypes())
{
Validate(entityType, logger);
ValidateType(entityType);
}

static void Validate(ITypeBase typeBase, IDiagnosticsLogger<DbLoggerCategory.Model.Validation> logger)
static void ValidateType(ITypeBase typeBase)
{
foreach (var property in typeBase.GetDeclaredProperties())
{
Expand All @@ -1067,7 +1067,7 @@ static void Validate(ITypeBase typeBase, IDiagnosticsLogger<DbLoggerCategory.Mod

foreach (var complexProperty in typeBase.GetDeclaredComplexProperties())
{
Validate(complexProperty.ComplexType, logger);
ValidateType(complexProperty.ComplexType);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,126 @@ namespace Microsoft.EntityFrameworkCore;

public abstract class JsonTypesRelationalTestBase : JsonTypesTestBase
{
public override Task Can_read_write_array_of_array_of_array_of_int_JSON_values()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I hope we'll clean up all these dedicated per-type tests via #33521.

=> NoNestedCollections(
"int[][][]", nameof(Int32ArrayArrayArrayType),
base.Can_read_write_array_of_array_of_array_of_int_JSON_values);

public override Task Can_read_write_array_of_list_of_array_of_IPAddress_JSON_values()
=> NoNestedCollections(
"List<IPAddress[]>[]", nameof(IpAddressArrayListArrayType),
base.Can_read_write_array_of_list_of_array_of_IPAddress_JSON_values);

public override Task Can_read_write_array_of_list_of_array_of_string_JSON_values()
=> NoNestedCollections(
"List<string[]>[]", nameof(StringArrayListArrayType),
base.Can_read_write_array_of_list_of_array_of_string_JSON_values);

public override Task Can_read_write_array_of_list_of_binary_JSON_values(string expected)
=> NoNestedCollections(
"List<byte[]>[]", nameof(BinaryListArrayType),
() => base.Can_read_write_array_of_list_of_binary_JSON_values(expected));

public override Task Can_read_write_array_of_list_of_GUID_JSON_values(string expected)
=> NoNestedCollections(
"ICollection<Guid>[]", nameof(GuidListArrayType),
() => base.Can_read_write_array_of_list_of_GUID_JSON_values(expected));

public override Task Can_read_write_array_of_list_of_int_JSON_values()
=> NoNestedCollections(
"IList<int>[]", nameof(Int32ListArrayType),
base.Can_read_write_array_of_list_of_int_JSON_values);

public override Task Can_read_write_array_of_list_of_IPAddress_JSON_values()
=> NoNestedCollections(
"Collection<IPAddress>[]", nameof(IpAddressListArrayType),
base.Can_read_write_array_of_list_of_IPAddress_JSON_values);

public override Task Can_read_write_array_of_list_of_string_JSON_values()
=> NoNestedCollections(
"List<string>[]", nameof(StringListArrayType),
base.Can_read_write_array_of_list_of_string_JSON_values);

public override Task Can_read_write_array_of_list_of_ulong_JSON_values()
=> NoNestedCollections(
"List<ulong>[]", nameof(ULongListArrayType),
base.Can_read_write_array_of_list_of_ulong_JSON_values);

public override Task Can_read_write_list_of_array_of_int_JSON_values()
=> NoNestedCollections(
"List<int[]>", nameof(Int32ArrayListType),
base.Can_read_write_list_of_array_of_int_JSON_values);

public override Task Can_read_write_list_of_array_of_IPAddress_JSON_values()
=> NoNestedCollections(
"ObservableCollection<IPAddress[]>", nameof(IpAddressArrayListType),
base.Can_read_write_list_of_array_of_IPAddress_JSON_values);

public override Task Can_read_write_list_of_array_of_list_of_IPAddress_JSON_values()
=> NoNestedCollections(
"List<List<IPAddress>[]>", nameof(IpAddressListArrayListType),
base.Can_read_write_list_of_array_of_list_of_IPAddress_JSON_values);

public override Task Can_read_write_list_of_array_of_list_of_string_JSON_values()
=> NoNestedCollections(
"List<List<string>[]>", nameof(StringListArrayListType),
base.Can_read_write_list_of_array_of_list_of_string_JSON_values);

public override Task Can_read_write_list_of_array_of_list_of_ulong_JSON_values()
=> NoNestedCollections(
"List<List<ulong[]>>", nameof(ULongListArrayListType),
base.Can_read_write_list_of_array_of_list_of_ulong_JSON_values);

public override Task Can_read_write_list_of_array_of_nullable_int_JSON_values()
=> NoNestedCollections(
"List<int?[]>", nameof(NullableInt32ArrayListType),
base.Can_read_write_list_of_array_of_nullable_int_JSON_values);

public override Task Can_read_write_list_of_array_of_nullable_ulong_JSON_values()
=> NoNestedCollections(
"List<ulong?[]>", nameof(NullableULongArrayListType),
base.Can_read_write_list_of_array_of_nullable_ulong_JSON_values);

public override Task Can_read_write_list_of_array_of_string_JSON_values()
=> NoNestedCollections(
"List<string[]>", nameof(StringArrayListType),
base.Can_read_write_list_of_array_of_string_JSON_values);

public override Task Can_read_write_list_of_array_of_ulong_JSON_values()
=> NoNestedCollections(
"List<ulong[]>", nameof(ULongArrayListType),
base.Can_read_write_list_of_array_of_ulong_JSON_values);

public override Task Can_read_write_list_of_list_of_list_of_int_JSON_values()
=> NoNestedCollections(
"List<List<List<int>>>", nameof(Int32ListListListType),
base.Can_read_write_list_of_list_of_list_of_int_JSON_values);

public override Task Can_read_write_list_of_array_of_binary_JSON_values(string expected)
=> NoNestedCollections(
"IEnumerable<byte[][]>", nameof(BinaryArrayListType),
() => base.Can_read_write_list_of_array_of_binary_JSON_values(expected));

public override Task Can_read_write_list_of_array_of_GUID_JSON_values(string expected)
=> NoNestedCollections(
"List<Guid[]>", nameof(GuidArrayListType),
() => base.Can_read_write_list_of_array_of_GUID_JSON_values(expected));

public override Task Can_read_write_list_of_array_of_list_of_array_of_binary_JSON_values(string expected)
=> NoNestedCollections(
"List<List<byte[][]>[]>", nameof(BinaryListArrayArrayListType),
() => base.Can_read_write_list_of_array_of_list_of_array_of_binary_JSON_values(expected));

public override Task Can_read_write_list_of_array_of_nullable_GUID_JSON_values(string expected)
=> NoNestedCollections(
"List<Guid?[]>", nameof(NullableGuidArrayListType),
() => base.Can_read_write_list_of_array_of_nullable_GUID_JSON_values(expected));

private async Task NoNestedCollections(string propertyType, string entityType, Func<Task> testCode)
=> Assert.Equal(
RelationalStrings.NestedCollectionsNotSupported(propertyType, entityType, "Prop"),
(await Assert.ThrowsAsync<InvalidOperationException>(testCode)).Message);

[ConditionalTheory]
[InlineData(null)]
public virtual Task Can_read_write_collection_of_fixed_length_string_JSON_values(object? storeType)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,33 @@ protected override void BuildBigModel(ModelBuilder modelBuilder, bool jsonColumn
eb.Property<string>("Data")
.IsFixedLength();
});

modelBuilder.Entity<ManyTypes>(
b =>
{
b.Ignore(e => e.BoolNestedCollection);
b.Ignore(e => e.UInt8NestedCollection);
b.Ignore(e => e.Int8NestedCollection);
b.Ignore(e => e.Int32NestedCollection);
b.Ignore(e => e.Int64NestedCollection);
b.Ignore(e => e.CharNestedCollection);
b.Ignore(e => e.GuidNestedCollection);
b.Ignore(e => e.StringNestedCollection);
b.Ignore(e => e.BytesNestedCollection);
b.Ignore(e => e.NullableUInt8NestedCollection);
b.Ignore(e => e.NullableInt32NestedCollection);
b.Ignore(e => e.NullableInt64NestedCollection);
b.Ignore(e => e.NullableGuidNestedCollection);
b.Ignore(e => e.NullableStringNestedCollection);
b.Ignore(e => e.NullableBytesNestedCollection);
b.Ignore(e => e.NullablePhysicalAddressNestedCollection);
b.Ignore(e => e.Enum8NestedCollection);
b.Ignore(e => e.Enum32NestedCollection);
b.Ignore(e => e.EnumU64NestedCollection);
b.Ignore(e => e.NullableEnum8NestedCollection);
b.Ignore(e => e.NullableEnum32NestedCollection);
b.Ignore(e => e.NullableEnumU64NestedCollection);
});
}

protected override void AssertBigModel(IModel model, bool jsonColumns)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3081,7 +3081,7 @@ public virtual Task Edit_single_property_collection_of_collection_of_bool()
entity.Collection[0].TestBooleanCollectionCollection = expected2;

ClearLog();
await context.SaveChangesAsync();
Assert.NotEqual(0, await context.SaveChangesAsync());
},
async context =>
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System.ComponentModel.DataAnnotations.Schema;
using Microsoft.EntityFrameworkCore.Diagnostics.Internal;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
using Xunit.Sdk;

Expand Down
2 changes: 0 additions & 2 deletions test/EFCore.Specification.Tests/JsonTypesTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System.Collections;
using System.Collections.ObjectModel;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Net;
using System.Net.NetworkInformation;
Expand Down Expand Up @@ -3713,7 +3712,6 @@ protected class Int32ListListListType
public List<List<List<int>>> Prop { get; set; } = null!;
}


[ConditionalFact]
public virtual Task Can_read_write_array_of_array_of_array_of_int_JSON_values()
=> Can_read_and_write_JSON_value<Int32ArrayArrayArrayType, int[][][]>(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.EntityFrameworkCore.TestModels.JsonQuery;

namespace Microsoft.EntityFrameworkCore.Query;

#nullable disable
Expand All @@ -9,4 +11,59 @@ public class JsonQuerySqlServerFixture : JsonQueryFixtureBase
{
protected override ITestStoreFactory TestStoreFactory
=> SqlServerTestStoreFactory.Instance;

protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext context)
{
base.OnModelCreating(modelBuilder, context);

modelBuilder.Entity<JsonEntityAllTypes>(
b =>
{
b.Ignore(e => e.TestInt64CollectionCollection);
b.Ignore(e => e.TestDoubleCollectionCollection);
b.Ignore(e => e.TestSingleCollectionCollection);
b.Ignore(e => e.TestBooleanCollectionCollection);
b.Ignore(e => e.TestCharacterCollectionCollection);
b.Ignore(e => e.TestDefaultStringCollectionCollection);
b.Ignore(e => e.TestMaxLengthStringCollectionCollection);
b.Ignore(e => e.TestInt16CollectionCollection);
b.Ignore(e => e.TestInt32CollectionCollection);
b.Ignore(e => e.TestNullableEnumWithIntConverterCollectionCollection);
b.Ignore(e => e.TestNullableInt32CollectionCollection);
b.Ignore(e => e.TestNullableEnumCollectionCollection);

b.OwnsOne(
e => e.Reference, b =>
{
b.Ignore(e => e.TestInt64CollectionCollection);
b.Ignore(e => e.TestDoubleCollectionCollection);
b.Ignore(e => e.TestSingleCollectionCollection);
b.Ignore(e => e.TestBooleanCollectionCollection);
b.Ignore(e => e.TestCharacterCollectionCollection);
b.Ignore(e => e.TestDefaultStringCollectionCollection);
b.Ignore(e => e.TestMaxLengthStringCollectionCollection);
b.Ignore(e => e.TestInt16CollectionCollection);
b.Ignore(e => e.TestInt32CollectionCollection);
b.Ignore(e => e.TestNullableEnumWithIntConverterCollectionCollection);
b.Ignore(e => e.TestNullableInt32CollectionCollection);
b.Ignore(e => e.TestNullableEnumCollectionCollection);
});
b.OwnsMany(
x => x.Collection, b =>
{
b.Ignore(e => e.TestInt64CollectionCollection);
b.Ignore(e => e.TestDoubleCollectionCollection);
b.Ignore(e => e.TestSingleCollectionCollection);
b.Ignore(e => e.TestBooleanCollectionCollection);
b.Ignore(e => e.TestCharacterCollectionCollection);
b.Ignore(e => e.TestDefaultStringCollectionCollection);
b.Ignore(e => e.TestMaxLengthStringCollectionCollection);
b.Ignore(e => e.TestInt16CollectionCollection);
b.Ignore(e => e.TestInt32CollectionCollection);
b.Ignore(e => e.TestNullableEnumWithIntConverterCollectionCollection);
b.Ignore(e => e.TestNullableInt32CollectionCollection);
b.Ignore(e => e.TestNullableEnumCollectionCollection);
});
});
}
}
Loading