Skip to content
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
77 changes: 77 additions & 0 deletions EFCore.NamingConventions.Test/NameRewritingConventionTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,50 @@ public void Owned_json_entity_with_OnesOne_and_nested_OwnsMany()
Assert.Equal("owned", ownedEntityType.GetContainerColumnName());
}

[Fact]
public void Owned_json_entity_with_nested_owned_collection_can_compile_query()
{
// This test reproduces the issue from PR #347 where nested JSON owned entities
// caused an ArgumentNullException during query compilation in EF Core 10.
// The bug was that nested entities within a JSON structure were incorrectly
// having container column names set on them, which caused EF Core's query
// compilation to fail with: ArgumentNullException: Value cannot be null. (Parameter 'key')

var optionsBuilder = new DbContextOptionsBuilder<JsonOwnedContext>();
SqlServerTestHelpers.Instance.UseProviderOptions(optionsBuilder);
optionsBuilder.UseSnakeCaseNamingConvention();

using var context = new JsonOwnedContext(optionsBuilder.Options);

// Without the fix, this would throw ArgumentNullException during query compilation.
// The fix ensures nested JSON entities don't get processed incorrectly.
var exception = Record.Exception(() =>
{
// The compilation happens when we create the query expression
_ = context.JsonOwners.ToQueryString();
});

// The query should compile successfully
Assert.Null(exception);

// Verify the model is configured correctly
var ownerEntityType = context.Model.FindEntityType(typeof(JsonOwner))!;
var detailsEntityType = context.Model.FindEntityType(typeof(JsonDetails))!;
var subDetailsEntityType = context.Model.FindEntityType(typeof(JsonSubDetail))!;

// Root entity should have snake_case table name (based on DbSet name)
Assert.Equal("json_owners", ownerEntityType.GetTableName());

// JSON root should have snake_case container column name
Assert.Equal("details", detailsEntityType.GetContainerColumnName());
Assert.Equal("json_owners", detailsEntityType.GetTableName());

// The nested JSON entity is part of the JSON structure
// It should have the same container column as the root JSON entity
Assert.Equal("details", subDetailsEntityType.GetContainerColumnName());
Assert.Equal("json_owners", subDetailsEntityType.GetTableName());
}

[Fact]
public void Complex_property()
{
Expand Down Expand Up @@ -905,4 +949,37 @@ public class Card
public int Id { get; }
public required Board Board { get; set; }
}

public class JsonOwner
{
public int Id { get; set; }
public required JsonDetails Details { get; set; }
}

public class JsonDetails
{
public string? Name { get; set; }
public required List<JsonSubDetail> SubDetails { get; set; }
}

public class JsonSubDetail
{
public string? Value { get; set; }
}

public class JsonOwnedContext : DbContext
{
public JsonOwnedContext(DbContextOptions<JsonOwnedContext> options) : base(options) { }

public DbSet<JsonOwner> JsonOwners => Set<JsonOwner>();

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<JsonOwner>().OwnsOne(x => x.Details, details =>
{
details.ToJson();
details.OwnsMany(x => x.SubDetails);
});
}
}
}
34 changes: 26 additions & 8 deletions EFCore.NamingConventions/Internal/NameRewritingConvention.cs
Original file line number Diff line number Diff line change
Expand Up @@ -168,20 +168,38 @@ private void ProcessOwnershipChange(IConventionForeignKey foreignKey, IConventio
// does nothing.
ownedEntityType.FindPrimaryKey()?.Builder.HasNoAnnotation(RelationalAnnotationNames.Name);

// Check if this entity is part of a JSON structure - either directly mapped to JSON or nested within one.
if (foreignKey.PrincipalEntityType.IsMappedToJson())
{
// Nested JSON entities are part of the parent's JSON structure, not their own database column.
// Clear any relational annotations that may have been set before the entity became part of JSON.
ownedEntityType.Builder.HasNoAnnotation(RelationalAnnotationNames.TableName);
ownedEntityType.Builder.HasNoAnnotation(RelationalAnnotationNames.Schema);

foreach (var property in ownedEntityType.GetProperties())
{
property.Builder.HasNoAnnotation(RelationalAnnotationNames.ColumnName);
}

context.StopProcessing();
return;
}

if (ownedEntityType.IsMappedToJson())
{
ProcessJsonOwnedEntity(ownedEntityType, ownedEntityType.GetContainerColumnName());
if (ownedEntityType.GetContainerColumnName() is { } containerColumnName)
{
// Rewrite container column name only on the root JSON entity.
ownedEntityType.SetContainerColumnName(_namingNameRewriter.RewriteName(containerColumnName));
}

ProcessJsonOwnedEntity(ownedEntityType);

void ProcessJsonOwnedEntity(IConventionEntityType entityType, string? containerColumnName)
void ProcessJsonOwnedEntity(IConventionEntityType entityType)
{
entityType.Builder.HasNoAnnotation(RelationalAnnotationNames.TableName);
entityType.Builder.HasNoAnnotation(RelationalAnnotationNames.Schema);

if (containerColumnName is not null)
{
entityType.SetContainerColumnName(_namingNameRewriter.RewriteName(containerColumnName));
}

// TODO: Note that we do not rewrite names of JSON properties (which aren't relational columns).
// TODO: We could introduce an option for doing so, though that's probably not usually what people want when doing JSON
foreach (var property in entityType.GetProperties())
Expand All @@ -194,7 +212,7 @@ void ProcessJsonOwnedEntity(IConventionEntityType entityType, string? containerC
foreach (var navigation in entityType.GetNavigations()
.Where(n => n is { IsOnDependent: false, ForeignKey.IsOwnership: true }))
{
ProcessJsonOwnedEntity(navigation.TargetEntityType, containerColumnName);
ProcessJsonOwnedEntity(navigation.TargetEntityType);
}
}
}
Expand Down