diff --git a/src/EFCore.Relational/Update/ModificationCommand.cs b/src/EFCore.Relational/Update/ModificationCommand.cs index 1497728e59b..d4a1fd522fd 100644 --- a/src/EFCore.Relational/Update/ModificationCommand.cs +++ b/src/EFCore.Relational/Update/ModificationCommand.cs @@ -5,6 +5,7 @@ using System.Data; using System.Text; using System.Text.Json; +using Microsoft.EntityFrameworkCore.ChangeTracking; using Microsoft.EntityFrameworkCore.ChangeTracking.Internal; using Microsoft.EntityFrameworkCore.Internal; using Microsoft.EntityFrameworkCore.Metadata; @@ -237,14 +238,7 @@ public virtual IColumnModification AddColumnModification(in ColumnModificationPa protected virtual IColumnModification CreateColumnModification(in ColumnModificationParameters columnModificationParameters) => new ColumnModification(columnModificationParameters); - private sealed class JsonPartialUpdateInfo - { - public List Path { get; } = []; - public IProperty? Property { get; set; } - public object? PropertyValue { get; set; } - } - - private record struct JsonPartialUpdatePathEntry(string PropertyName, int? Ordinal, IUpdateEntry ParentEntry, INavigation? Navigation, IComplexProperty? ComplexProperty = null); + private record struct JsonPartialUpdatePathEntry(string PropertyName, int? Ordinal, IUpdateEntry ParentEntry, IPropertyBase Property); private List GenerateColumnModifications() { @@ -254,13 +248,13 @@ private List GenerateColumnModifications() var deleting = state == EntityState.Deleted; var columnModifications = new List(); Dictionary? sharedTableColumnMap = null; - var jsonEntry = false; + // Detect table-splitting and populate shared columns to propagate values back to the sharing entries. if (_entries.Count > 1 - || _entries is [var singleEntry] - && (singleEntry.SharedIdentityEntry is not null - || singleEntry.EntityType.GetComplexProperties().Any() - || singleEntry.EntityType.GetNavigations().Any(e => e.IsCollection && e.TargetEntityType.IsMappedToJson()))) + || (_entries is [var singleEntry] + && (singleEntry.SharedIdentityEntry is not null + || singleEntry.EntityType.GetComplexProperties() + .Any(cp => !cp.IsCollection && !cp.ComplexType.IsMappedToJson())))) { Check.DebugAssert(StoreStoredProcedure is null, "Multiple entries/shared identity not supported with stored procedures"); @@ -294,30 +288,27 @@ private List GenerateColumnModifications() } HandleSharedColumns(entry.EntityType, entry, tableMapping, deleting, sharedTableColumnMap); - - if (!jsonEntry) - { - if (entry.EntityType.IsMappedToJson() - || entry.EntityType.GetFlattenedComplexProperties().Any(cp => cp.ComplexType.IsMappedToJson()) - || entry.EntityType.GetNavigations().Any(e => e.IsCollection && e.TargetEntityType.IsMappedToJson())) - { - jsonEntry = true; - } - } } } - if (jsonEntry) + if (_entries.Any(e => e.EntityType is IEntityType entityType + && (entityType.IsMappedToJson() + || entityType.GetFlattenedComplexProperties().Any(cp => cp.ComplexType.IsMappedToJson()) + || entityType.GetNavigations().Any(e => e.IsCollection && e.TargetEntityType.IsMappedToJson())))) { HandleJson(columnModifications); } - foreach (var entry in _entries.Where(x => !x.EntityType.IsMappedToJson())) + foreach (var entry in _entries) { + if (entry.EntityType.IsMappedToJson()) + { + continue; + } + var nonMainEntry = !_mainEntryAdded || entry != _entries[0]; var optionalDependentWithAllNull = false; - if (StoreStoredProcedure is null) { var tableMapping = GetTableMapping(entry.EntityType); @@ -331,99 +322,11 @@ entry.EntityState is EntityState.Modified or EntityState.Added && tableMapping.Table.IsOptional(entry.EntityType) && tableMapping.Table.GetRowInternalForeignKeys(entry.EntityType).Any(); - HandleNonJson(entry.EntityType, tableMapping); + HandleNonJson(entry, entry.EntityType, tableMapping, nonMainEntry, ref optionalDependentWithAllNull); } - else // Stored procedure mapping case + else { - var storedProcedureMapping = GetStoredProcedureMapping(entry.EntityType, EntityState); - Check.DebugAssert(storedProcedureMapping is not null, "No sproc mapping but StoredProcedure is not null"); - var storedProcedure = storedProcedureMapping.StoredProcedure; - - // Stored procedures may have an additional rows affected result column or return value, which does not have a - // property/column mapping but still needs to have be represented via a column modification. - // Note that for rows affected parameters/result columns, we add column modifications below along with regular parameters/ - // result columns; for return value we do that here. - if (storedProcedure.FindRowsAffectedParameter() is { } rowsAffectedParameter) - { - RowsAffectedColumn = rowsAffectedParameter.StoreParameter; - } - else if (storedProcedure.FindRowsAffectedResultColumn() is { } rowsAffectedResultColumn) - { - RowsAffectedColumn = rowsAffectedResultColumn.StoreResultColumn; - } - else if (storedProcedureMapping.StoreStoredProcedure.ReturnValue is { } rowsAffectedReturnValue) - { - RowsAffectedColumn = rowsAffectedReturnValue; - - columnModifications.Add( - CreateColumnModification( - new ColumnModificationParameters( - entry: null, - property: null, - rowsAffectedReturnValue, - _generateParameterName!, - rowsAffectedReturnValue.StoreTypeMapping, - valueIsRead: true, - valueIsWrite: false, - columnIsKey: false, - columnIsCondition: false, - _sensitiveLoggingEnabled))); - } - - // In TPH, the sproc has parameters for all entity types in the hierarchy; we must generate null column modifications - // for parameters for unrelated entity types. - // Enumerate over the sproc parameters in order, trying to match a corresponding parameter mapping. - // Note that we produce the column modifications in the same order as their sproc parameters; this is important and assumed - // later in the pipeline. - foreach (var parameter in StoreStoredProcedure.Parameters) - { - if (parameter.FindParameterMapping(entry.EntityType) is { } parameterMapping) - { - HandleColumn(parameterMapping); - continue; - } - - // The parameter has no corresponding mapping; this is either a sibling property in a TPH hierarchy or a rows affected - // output parameter. Note that we set IsRead to false since we don't propagate the output parameter. - columnModifications.Add( - CreateColumnModification( - new ColumnModificationParameters( - entry: null, - property: null, - parameter, - _generateParameterName!, - parameter.StoreTypeMapping, - valueIsRead: false, - valueIsWrite: parameter.Direction.HasFlag(ParameterDirection.Input), - columnIsKey: false, - columnIsCondition: false, - _sensitiveLoggingEnabled))); - } - - foreach (var resultColumn in StoreStoredProcedure.ResultColumns) - { - if (resultColumn.FindColumnMapping(entry.EntityType) is { } resultColumnMapping) - { - HandleColumn(resultColumnMapping); - continue; - } - - // The result column has no corresponding mapping; this is either a sibling property in a TPH hierarchy or a rows - // affected result column. Note that we set IsRead to false since we don't propagate the result column. - columnModifications.Add( - CreateColumnModification( - new ColumnModificationParameters( - entry: null, - property: null, - resultColumn, - _generateParameterName!, - resultColumn.StoreTypeMapping, - valueIsRead: false, - valueIsWrite: false, - columnIsKey: false, - columnIsCondition: false, - _sensitiveLoggingEnabled))); - } + HandleSprocs(entry, nonMainEntry, ref optionalDependentWithAllNull); } if (optionalDependentWithAllNull && _logger != null) @@ -437,125 +340,220 @@ entry.EntityState is EntityState.Modified or EntityState.Added _logger.OptionalDependentWithAllNullPropertiesWarning(entry); } } + } + + return columnModifications; + + void HandleNonJson( + IUpdateEntry entry, ITypeBase structuralType, ITableMapping tableMapping, bool nonMainEntry, ref bool optionalDependentWithAllNull) + { + foreach (var columnMapping in tableMapping.ColumnMappings) + { + HandleColumn(entry, columnMapping, nonMainEntry, ref optionalDependentWithAllNull); + } - void HandleNonJson(ITypeBase structuralType, ITableMapping tableMapping) + foreach (var complexProperty in structuralType.GetComplexProperties()) { - foreach (var columnMapping in tableMapping.ColumnMappings) + var complexTableMapping = GetTableMapping(complexProperty.ComplexType); + if (complexTableMapping != null) { - HandleColumn(columnMapping); + HandleNonJson(entry, complexProperty.ComplexType, complexTableMapping, nonMainEntry, ref optionalDependentWithAllNull); } + } + } - foreach (var complexProperty in structuralType.GetComplexProperties()) + void HandleColumn( + IUpdateEntry entry, IColumnMappingBase columnMapping, bool nonMainEntry, ref bool optionalDependentWithAllNull) + { + var property = columnMapping.Property; + var column = columnMapping.Column; + var storedProcedureParameter = columnMapping is IStoredProcedureParameterMapping parameterMapping + ? parameterMapping.Parameter + : null; + var isKey = property.IsPrimaryKey(); + var isCondition = !adding + && (isKey + || storedProcedureParameter is { ForOriginalValue: true } + || (property.IsConcurrencyToken && storedProcedureParameter is null)); + + // Store-generated properties generally need to be read back (unless we're deleting). + // One exception is if the property is mapped to a non-output parameter. + var readValue = state != EntityState.Deleted + && ColumnModification.IsStoreGenerated(entry, property) + && (storedProcedureParameter is null || storedProcedureParameter.Direction.HasFlag(ParameterDirection.Output)); + + ColumnValuePropagator? columnPropagator = null; + sharedTableColumnMap?.TryGetValue(column.Name, out columnPropagator); + + var writeValue = false; + if (!readValue) + { + if (adding) { - var complexTableMapping = GetTableMapping(complexProperty.ComplexType); - if (complexTableMapping != null) - { - HandleNonJson(complexProperty.ComplexType, complexTableMapping); - } + writeValue = property.GetBeforeSaveBehavior() == PropertySaveBehavior.Save + || entry.HasStoreGeneratedValue(property); + + columnPropagator?.TryPropagate(columnMapping, entry); + } + else if (storedProcedureParameter is not { ForOriginalValue: true } + && !deleting + && ((updating && property.GetAfterSaveBehavior() == PropertySaveBehavior.Save) + || (!isKey && nonMainEntry) + || entry.SharedIdentityEntry != null)) + { + // Note that for stored procedures we always need to send all parameters, regardless of whether the property + // actually changed. + writeValue = columnPropagator?.TryPropagate(columnMapping, entry) + ?? (entry.EntityState == EntityState.Added + || entry.EntityState == EntityState.Deleted + || ColumnModification.IsModified(entry, property) + || StoreStoredProcedure is not null); } } - void HandleColumn(IColumnMappingBase columnMapping) + if (readValue + || writeValue + || isCondition) { - var property = columnMapping.Property; - var column = columnMapping.Column; - var storedProcedureParameter = columnMapping is IStoredProcedureParameterMapping parameterMapping - ? parameterMapping.Parameter - : null; - var isKey = property.IsPrimaryKey(); - var isCondition = !adding - && (isKey - || storedProcedureParameter is { ForOriginalValue: true } - || (property.IsConcurrencyToken && storedProcedureParameter is null)); - - // Store-generated properties generally need to be read back (unless we're deleting). - // One exception is if the property is mapped to a non-output parameter. - var readValue = state != EntityState.Deleted - && ColumnModification.IsStoreGenerated(entry, property) - && (storedProcedureParameter is null || storedProcedureParameter.Direction.HasFlag(ParameterDirection.Output)); - - ColumnValuePropagator? columnPropagator = null; - sharedTableColumnMap?.TryGetValue(column.Name, out columnPropagator); - - var writeValue = false; - if (!readValue) + var columnModificationParameters = new ColumnModificationParameters( + entry, + property, + column, + _generateParameterName!, + columnMapping.TypeMapping, + readValue, + writeValue, + isKey, + isCondition, + _sensitiveLoggingEnabled); + + var columnModification = CreateColumnModification(columnModificationParameters); + + if (columnPropagator != null + && column.PropertyMappings.Count != 1) { - if (adding) + if (columnPropagator.ColumnModification != null) { - writeValue = property.GetBeforeSaveBehavior() == PropertySaveBehavior.Save - || entry.HasStoreGeneratedValue(property); + columnPropagator.ColumnModification.AddSharedColumnModification(columnModification); - columnPropagator?.TryPropagate(columnMapping, entry); - } - else if (storedProcedureParameter is not { ForOriginalValue: true } - && !deleting - && ((updating && property.GetAfterSaveBehavior() == PropertySaveBehavior.Save) - || (!isKey && nonMainEntry) - || entry.SharedIdentityEntry != null)) - { - // Note that for stored procedures we always need to send all parameters, regardless of whether the property - // actually changed. - writeValue = columnPropagator?.TryPropagate(columnMapping, entry) - ?? (entry.EntityState == EntityState.Added - || entry.EntityState == EntityState.Deleted - || ColumnModification.IsModified(entry, property) - || StoreStoredProcedure is not null); + return; } + + columnPropagator.ColumnModification = columnModification; } - if (readValue - || writeValue - || isCondition) + columnModifications.Add(columnModification); + + if (optionalDependentWithAllNull + && (columnModification.IsWrite + || (columnModification.IsCondition && !isKey)) + && columnModification.Value is not null) { - var columnModificationParameters = new ColumnModificationParameters( - entry, - property, - column, - _generateParameterName!, - columnMapping.TypeMapping, - readValue, - writeValue, - isKey, - isCondition, - _sensitiveLoggingEnabled); - - var columnModification = CreateColumnModification(columnModificationParameters); - - if (columnPropagator != null - && column.PropertyMappings.Count != 1) - { - if (columnPropagator.ColumnModification != null) - { - columnPropagator.ColumnModification.AddSharedColumnModification(columnModification); + optionalDependentWithAllNull = false; + } + } + else if (optionalDependentWithAllNull + && state == EntityState.Modified + && property.DeclaringType == entry.EntityType + && entry.GetCurrentValue(property) is not null) + { + optionalDependentWithAllNull = false; + } + } - return; - } + void HandleSprocs(IUpdateEntry entry, bool nonMainEntry, ref bool optionalDependentWithAllNull) + { + var storedProcedureMapping = GetStoredProcedureMapping(entry.EntityType, EntityState); + Check.DebugAssert(storedProcedureMapping is not null, "No sproc mapping but StoredProcedure is not null"); + var storedProcedure = storedProcedureMapping.StoredProcedure; + + // Stored procedures may have an additional rows affected result column or return value, which does not have a + // property/column mapping but still needs to have be represented via a column modification. + // Note that for rows affected parameters/result columns, we add column modifications below along with regular parameters/ + // result columns; for return value we do that here. + if (storedProcedure.FindRowsAffectedParameter() is { } rowsAffectedParameter) + { + RowsAffectedColumn = rowsAffectedParameter.StoreParameter; + } + else if (storedProcedure.FindRowsAffectedResultColumn() is { } rowsAffectedResultColumn) + { + RowsAffectedColumn = rowsAffectedResultColumn.StoreResultColumn; + } + else if (storedProcedureMapping.StoreStoredProcedure.ReturnValue is { } rowsAffectedReturnValue) + { + RowsAffectedColumn = rowsAffectedReturnValue; + + columnModifications.Add( + CreateColumnModification( + new ColumnModificationParameters( + entry: null, + property: null, + rowsAffectedReturnValue, + _generateParameterName!, + rowsAffectedReturnValue.StoreTypeMapping, + valueIsRead: true, + valueIsWrite: false, + columnIsKey: false, + columnIsCondition: false, + _sensitiveLoggingEnabled))); + } - columnPropagator.ColumnModification = columnModification; - } + // In TPH, the sproc has parameters for all entity types in the hierarchy; we must generate null column modifications + // for parameters for unrelated entity types. + // Enumerate over the sproc parameters in order, trying to match a corresponding parameter mapping. + // Note that we produce the column modifications in the same order as their sproc parameters; this is important and assumed + // later in the pipeline. + foreach (var parameter in StoreStoredProcedure.Parameters) + { + if (parameter.FindParameterMapping(entry.EntityType) is { } parameterMapping) + { + HandleColumn(entry, parameterMapping, nonMainEntry, ref optionalDependentWithAllNull); + continue; + } - columnModifications.Add(columnModification); + // The parameter has no corresponding mapping; this is either a sibling property in a TPH hierarchy or a rows affected + // output parameter. Note that we set IsRead to false since we don't propagate the output parameter. + columnModifications.Add( + CreateColumnModification( + new ColumnModificationParameters( + entry: null, + property: null, + parameter, + _generateParameterName!, + parameter.StoreTypeMapping, + valueIsRead: false, + valueIsWrite: parameter.Direction.HasFlag(ParameterDirection.Input), + columnIsKey: false, + columnIsCondition: false, + _sensitiveLoggingEnabled))); + } - if (optionalDependentWithAllNull - && (columnModification.IsWrite - || (columnModification.IsCondition && !isKey)) - && columnModification.Value is not null) - { - optionalDependentWithAllNull = false; - } - } - else if (optionalDependentWithAllNull - && state == EntityState.Modified - && property.DeclaringType == entry.EntityType - && entry.GetCurrentValue(property) is not null) + foreach (var resultColumn in StoreStoredProcedure.ResultColumns) + { + if (resultColumn.FindColumnMapping(entry.EntityType) is { } resultColumnMapping) { - optionalDependentWithAllNull = false; + HandleColumn(entry, resultColumnMapping, nonMainEntry, ref optionalDependentWithAllNull); + continue; } + + // The result column has no corresponding mapping; this is either a sibling property in a TPH hierarchy or a rows + // affected result column. Note that we set IsRead to false since we don't propagate the result column. + columnModifications.Add( + CreateColumnModification( + new ColumnModificationParameters( + entry: null, + property: null, + resultColumn, + _generateParameterName!, + resultColumn.StoreTypeMapping, + valueIsRead: false, + valueIsWrite: false, + columnIsKey: false, + columnIsCondition: false, + _sensitiveLoggingEnabled))); } } - return columnModifications; - void HandleSharedColumns( ITypeBase structuralType, IUpdateEntry entry, @@ -576,9 +574,9 @@ void HandleSharedColumns( } } - static JsonPartialUpdateInfo? FindJsonPartialUpdateInfo(IUpdateEntry entry, List processedEntries) + static List? FindJsonPartialUpdateInfo(IUpdateEntry entry, List processedEntries) { - var result = new JsonPartialUpdateInfo(); + var result = new List(); var currentEntry = entry; var currentOwnership = currentEntry.EntityType.FindOwnership()!; @@ -610,16 +608,19 @@ void HandleSharedColumns( currentOwnership.PrincipalEntityType.IsMappedToJson() ? jsonPropertyName : "$", ordinal, currentEntry, - currentOwnership.GetNavigation(pointsToPrincipal: false)!); + currentOwnership.GetNavigation(pointsToPrincipal: false)!); // TODO: Handle complex properties, Issue #36429 - result.Path.Insert(0, pathEntry); + result.Insert(0, pathEntry); } var modifiedMembers = entry.EntityType.GetFlattenedProperties().Where(entry.IsModified).ToList(); if (modifiedMembers.Count == 1) { - result.Property = modifiedMembers[0]; - result.PropertyValue = entry.GetCurrentValue(result.Property); + result.Add(new JsonPartialUpdatePathEntry( + modifiedMembers[0].GetJsonPropertyName()!, + null, + entry, + modifiedMembers[0])); } else { @@ -637,42 +638,43 @@ void HandleSharedColumns( return result; } - static JsonPartialUpdateInfo FindCommonJsonPartialUpdateInfo( - JsonPartialUpdateInfo first, - JsonPartialUpdateInfo second) + static List FindCommonJsonPartialUpdateInfo( + List first, + List second) { - var result = new JsonPartialUpdateInfo(); - for (var i = 0; i < Math.Min(first.Path.Count, second.Path.Count); i++) + var commonPath = new List(); + for (var i = 0; i < Math.Min(first.Count, second.Count); i++) { - if (first.Path[i].PropertyName == second.Path[i].PropertyName) + if (first[i].PropertyName != second[i].PropertyName) { - if (first.Path[i].Ordinal == second.Path[i].Ordinal) - { - result.Path.Add(first.Path[i]); - continue; - } + break; + } - var common = new JsonPartialUpdatePathEntry( - first.Path[i].PropertyName, - null, - first.Path[i].ParentEntry, - Navigation: first.Path[i].Navigation, - ComplexProperty: first.Path[i].ComplexProperty); + if (first[i].Ordinal == second[i].Ordinal) + { + commonPath.Add(first[i]); + continue; + } - result.Path.Add(common); + var common = new JsonPartialUpdatePathEntry( + first[i].PropertyName, + null, + first[i].ParentEntry, + Property: first[i].Property); - break; - } + commonPath.Add(common); + + break; } - Check.DebugAssert(result.Path.Count > 0, "Common denominator should always have at least one node - the root."); + Check.DebugAssert(commonPath.Count > 0, "Common denominator should always have at least one node - the root."); - return result; + return commonPath; } void HandleJson(List columnModifications) { - var jsonColumnsUpdateMap = new Dictionary(); + var jsonColumnsUpdateMap = new Dictionary>(); var processedEntries = new List(); foreach (var entry in _entries) { @@ -718,24 +720,32 @@ void HandleJson(List columnModifications) if (!jsonColumnsUpdateMap.ContainsKey(jsonCollectionColumn)) { - var jsonPartialUpdateInfo = new JsonPartialUpdateInfo(); - jsonPartialUpdateInfo.Path.Insert(0, new JsonPartialUpdatePathEntry("$", null, entry, jsonCollectionNavigation)); - jsonPartialUpdateInfo.PropertyValue = entry.GetCurrentValue(jsonCollectionNavigation); + var jsonPartialUpdateInfo = new List + { + new("$", null, entry, jsonCollectionNavigation) + }; jsonColumnsUpdateMap[jsonCollectionColumn] = jsonPartialUpdateInfo; } } - foreach (var complexProperty in entry.EntityType.GetFlattenedComplexProperties() - .Where(cp => cp.ComplexType.IsMappedToJson() && !cp.DeclaringType.IsMappedToJson())) + foreach (var complexProperty in entry.EntityType.GetFlattenedComplexProperties()) { var complexType = complexProperty.ComplexType; - var jsonColumn = GetTableMapping(entry.EntityType)!.Table.FindColumn(complexType.GetContainerColumnName()!)!; + if (!complexType.IsMappedToJson() + || complexProperty.DeclaringType.IsMappedToJson() + || (entry.EntityState != EntityState.Added + && !entry.IsModified(complexProperty))) + { + continue; + } + var jsonColumn = GetTableMapping(entry.EntityType)!.Table.FindColumn(complexType.GetContainerColumnName()!)!; if (!jsonColumnsUpdateMap.ContainsKey(jsonColumn)) { - var jsonPartialUpdateInfo = new JsonPartialUpdateInfo(); - jsonPartialUpdateInfo.Path.Insert(0, new JsonPartialUpdatePathEntry("$", null, entry, Navigation: null, ComplexProperty: complexProperty)); - jsonPartialUpdateInfo.PropertyValue = entry.GetCurrentValue(complexProperty); + var jsonPartialUpdateInfo = new List + { + new("$", null, entry, complexProperty) + }; jsonColumnsUpdateMap[jsonColumn] = jsonPartialUpdateInfo; } } @@ -743,24 +753,23 @@ void HandleJson(List columnModifications) foreach (var (jsonColumn, updateInfo) in jsonColumnsUpdateMap) { - var finalUpdatePathElement = updateInfo.Path.Last(); - var navigation = finalUpdatePathElement.Navigation; - var complexProperty = finalUpdatePathElement.ComplexProperty; + var finalUpdatePathElement = updateInfo.Last(); var jsonColumnTypeMapping = jsonColumn.StoreTypeMapping; - var jsonContainerProperty = (IPropertyBase?)navigation ?? complexProperty; - var navigationValue = finalUpdatePathElement.ParentEntry.GetCurrentValue(jsonContainerProperty!); + var jsonProperty = finalUpdatePathElement.Property; + var propertyValue = finalUpdatePathElement.ParentEntry.GetCurrentValue(jsonProperty); + // TODO: Change JSON path to be structured, issue #32185 var jsonPathString = string.Join( - ".", updateInfo.Path.Select(x => x.PropertyName + (x.Ordinal != null ? "[" + x.Ordinal + "]" : ""))); - if (updateInfo.Property is IProperty property) + ".", updateInfo.Select(x => x.PropertyName + (x.Ordinal != null ? "[" + x.Ordinal + "]" : ""))); + if (jsonProperty is IProperty property) { var columnModificationParameters = new ColumnModificationParameters( jsonColumn.Name, - value: updateInfo.PropertyValue, + value: propertyValue, property: property, columnType: jsonColumnTypeMapping.StoreType, jsonColumnTypeMapping, - jsonPath: jsonPathString + "." + updateInfo.Property.GetJsonPropertyName(), + jsonPath: jsonPathString, read: false, write: true, key: false, @@ -775,10 +784,10 @@ void HandleJson(List columnModifications) { var stream = new MemoryStream(); var writer = new Utf8JsonWriter(stream, new JsonWriterOptions { Indented = false }); - if (finalUpdatePathElement.Ordinal != null && navigationValue != null) + if (finalUpdatePathElement.Ordinal != null && propertyValue != null) { var i = 0; - foreach (var navigationValueElement in (IEnumerable)navigationValue) + foreach (var navigationValueElement in (IEnumerable)propertyValue) { if (i == finalUpdatePathElement.Ordinal) { @@ -786,7 +795,7 @@ void HandleJson(List columnModifications) writer, navigationValueElement, (IInternalEntry)finalUpdatePathElement.ParentEntry, - ((IPropertyBase?)navigation) ?? complexProperty!, + jsonProperty, ordinal: null, isCollection: false, isTopLevel: true); @@ -799,14 +808,13 @@ void HandleJson(List columnModifications) } else { - var propertyBase = ((IPropertyBase?)navigation) ?? complexProperty!; WriteJson( writer, - navigationValue, + propertyValue, (IInternalEntry)finalUpdatePathElement.ParentEntry, - ((IPropertyBase?)navigation) ?? complexProperty!, + jsonProperty, ordinal: null, - isCollection: propertyBase.IsCollection, + isCollection: jsonProperty.IsCollection, isTopLevel: true); } @@ -821,7 +829,7 @@ void HandleJson(List columnModifications) new ColumnModificationParameters( jsonColumn.Name, value: value, - property: updateInfo.Property, + property: null, columnType: jsonColumnTypeMapping.StoreType, jsonColumnTypeMapping, jsonPath: jsonPathString, @@ -871,11 +879,12 @@ protected virtual void ProcessSinglePropertyJsonUpdate(ref ColumnModificationPar } } -#pragma warning disable EF1001 // Internal EF Core API usage. private void WriteJson( Utf8JsonWriter writer, object? value, +#pragma warning disable EF1001 // Internal EF Core API usage. IInternalEntry parentEntry, +#pragma warning restore EF1001 // Internal EF Core API usage. IPropertyBase property, int? ordinal, bool isCollection, @@ -916,17 +925,21 @@ private void WriteJson( writer.WriteStartObject(); +#pragma warning disable EF1001 // Internal EF Core API usage. var entry = structuralType is IComplexType complexType ? complexType.ComplexProperty.IsCollection ? parentEntry.GetComplexCollectionEntry(complexType.ComplexProperty, ordinal!.Value) : parentEntry : ((InternalEntityEntry)parentEntry).StateManager.TryGetEntry(value, (IEntityType)structuralType)!; +#pragma warning restore EF1001 // Internal EF Core API usage. WriteJsonObject(writer, parentEntry, entry, structuralType, ordinal); writer.WriteEndObject(); } +#pragma warning disable EF1001 // Internal EF Core API usage. private void WriteJsonObject(Utf8JsonWriter writer, IInternalEntry parentEntry, IInternalEntry entry, ITypeBase structuralType, int? ordinal) +#pragma warning restore EF1001 // Internal EF Core API usage. { foreach (var property in structuralType.GetProperties()) { @@ -934,7 +947,9 @@ private void WriteJsonObject(Utf8JsonWriter writer, IInternalEntry parentEntry, { if (property.IsOrdinalKeyProperty() && ordinal != null) { +#pragma warning disable EF1001 // Internal EF Core API usage. entry.SetStoreGeneratedValue(property, ordinal.Value + 1, setModified: false); +#pragma warning disable EF1001 // Internal EF Core API usage. } continue; @@ -942,7 +957,9 @@ private void WriteJsonObject(Utf8JsonWriter writer, IInternalEntry parentEntry, // jsonPropertyName can only be null for key properties var jsonPropertyName = property.GetJsonPropertyName()!; +#pragma warning disable EF1001 // Internal EF Core API usage. var propertyValue = entry.GetCurrentValue(property); +#pragma warning disable EF1001 // Internal EF Core API usage. writer.WritePropertyName(jsonPropertyName); if (propertyValue is not null) @@ -960,7 +977,9 @@ private void WriteJsonObject(Utf8JsonWriter writer, IInternalEntry parentEntry, foreach (var complexProperty in structuralType.GetComplexProperties()) { var jsonPropertyName = complexProperty.GetJsonPropertyName()!; +#pragma warning disable EF1001 // Internal EF Core API usage. var complexPropertyValue = entry.GetCurrentValue(complexProperty); +#pragma warning disable EF1001 // Internal EF Core API usage. writer.WritePropertyName(jsonPropertyName); WriteJson( @@ -984,7 +1003,9 @@ private void WriteJsonObject(Utf8JsonWriter writer, IInternalEntry parentEntry, } var jsonPropertyName = navigation.TargetEntityType.GetJsonPropertyName()!; +#pragma warning disable EF1001 // Internal EF Core API usage. var ownedNavigationValue = entry.GetCurrentValue(navigation)!; +#pragma warning disable EF1001 // Internal EF Core API usage. writer.WritePropertyName(jsonPropertyName); WriteJson( @@ -1044,13 +1065,9 @@ private static void InitializeSharedColumns( { foreach (var columnMapping in tableMapping.ColumnMappings) { - if (columnMapping.Property.DeclaringType.IsMappedToJson()) - { - continue; - } - - if (columnMapping.Column.PropertyMappings.Select(p => p.Property).Distinct().Count() == 1 - && entry.SharedIdentityEntry == null) + if (columnMapping.Property.DeclaringType.IsMappedToJson() + || (columnMapping.Column.PropertyMappings.Select(p => p.Property).Distinct().Count() == 1 + && entry.SharedIdentityEntry == null)) { continue; } @@ -1093,7 +1110,7 @@ public virtual void PropagateResults(RelationalDataReader relationalReader) // listed in ColumnModifications. readerIndex++; #if DEBUG - Check.DebugAssert(!seenStoredProcedureResultColumn, "!seenStoredProcedureResultColumn"); + Check.DebugAssert(!seenStoredProcedureResultColumn); seenRegularResultColumn = true; #endif break; @@ -1107,7 +1124,7 @@ public virtual void PropagateResults(RelationalDataReader relationalReader) // For stored procedure result sets, we need to get the column ordering from metadata. readerIndex = resultColumn.Position; #if DEBUG - Check.DebugAssert(!seenRegularResultColumn, "!seenRegularResultColumn"); + Check.DebugAssert(!seenRegularResultColumn); seenStoredProcedureResultColumn = true; #endif break; diff --git a/src/EFCore/ChangeTracking/ComplexEntry`.cs b/src/EFCore/ChangeTracking/ComplexElementEntry`.cs similarity index 99% rename from src/EFCore/ChangeTracking/ComplexEntry`.cs rename to src/EFCore/ChangeTracking/ComplexElementEntry`.cs index 383db28c172..01b51169e63 100644 --- a/src/EFCore/ChangeTracking/ComplexEntry`.cs +++ b/src/EFCore/ChangeTracking/ComplexElementEntry`.cs @@ -81,7 +81,7 @@ public ComplexElementEntry(InternalComplexEntry internalEntry) /// /// An object that exposes change tracking information and operations for the given property. public virtual PropertyEntry Property( - Expression> propertyExpression) + Expression> propertyExpression) { Check.NotNull(propertyExpression, nameof(propertyExpression)); diff --git a/src/EFCore/ChangeTracking/ComplexPropertyEntry`.cs b/src/EFCore/ChangeTracking/ComplexPropertyEntry`.cs index 4875759ace8..0045f0a4c02 100644 --- a/src/EFCore/ChangeTracking/ComplexPropertyEntry`.cs +++ b/src/EFCore/ChangeTracking/ComplexPropertyEntry`.cs @@ -95,7 +95,7 @@ public virtual PropertyEntry Property( /// /// An object that exposes change tracking information and operations for the given property. public virtual ComplexPropertyEntry ComplexProperty( - Expression> propertyExpression) + Expression> propertyExpression) { Check.NotNull(propertyExpression); diff --git a/src/EFCore/ChangeTracking/EntityEntry`.cs b/src/EFCore/ChangeTracking/EntityEntry`.cs index b8676e2d8cc..8261d778ac3 100644 --- a/src/EFCore/ChangeTracking/EntityEntry`.cs +++ b/src/EFCore/ChangeTracking/EntityEntry`.cs @@ -74,7 +74,7 @@ public virtual PropertyEntry Property( /// /// An object that exposes change tracking information and operations for the given property. public virtual ComplexPropertyEntry ComplexProperty( - Expression> propertyExpression) + Expression> propertyExpression) { Check.NotNull(propertyExpression); diff --git a/src/EFCore/ChangeTracking/Internal/ChangeDetector.cs b/src/EFCore/ChangeTracking/Internal/ChangeDetector.cs index ae298a8714c..032f7d961b6 100644 --- a/src/EFCore/ChangeTracking/Internal/ChangeDetector.cs +++ b/src/EFCore/ChangeTracking/Internal/ChangeDetector.cs @@ -321,11 +321,11 @@ public bool DetectComplexCollectionChanges(InternalEntryBase entry, IComplexProp Check.DebugAssert(!complexProperty.ComplexType.ClrType.IsValueType, $"Expected {complexProperty.Name} to be a collection of reference types."); - var changesFound = false; var originalEntries = new Dictionary(ReferenceEqualityComparer.Instance); var currentCollection = (IList?)entry[complexProperty]; // The elements in the original collection might be the same instances as in the current collection, so their properties shouldn't be used for comparison. var originalCollection = (IList?)entry.GetOriginalValue(complexProperty); + var changesFound = (currentCollection == null) != (originalCollection == null); entry.EnsureComplexCollectionEntriesCapacity(complexProperty, currentCollection?.Count ?? 0, originalCollection?.Count ?? 0, trim: false); var originalNulls = new HashSet(); diff --git a/src/EFCore/ChangeTracking/Internal/InternalComplexEntry.cs b/src/EFCore/ChangeTracking/Internal/InternalComplexEntry.cs index 83ef4f75487..b460e303363 100644 --- a/src/EFCore/ChangeTracking/Internal/InternalComplexEntry.cs +++ b/src/EFCore/ChangeTracking/Internal/InternalComplexEntry.cs @@ -255,8 +255,9 @@ protected override void OnStateChanged(EntityState oldState) /// public override void AcceptChanges() { - if (EntityState == EntityState.Added - || ContainingEntry.GetComplexCollectionEntry(ComplexProperty, Ordinal) == this) + if (Ordinal != -1 + && (EntityState == EntityState.Added + || ContainingEntry.GetComplexCollectionOriginalEntry(ComplexProperty, Ordinal) == this)) { OriginalOrdinal = Ordinal; } diff --git a/src/EFCore/ChangeTracking/Internal/InternalEntryBase.InternalComplexCollectionEntry.cs b/src/EFCore/ChangeTracking/Internal/InternalEntryBase.InternalComplexCollectionEntry.cs index 7d1bc60e268..7ad99221396 100644 --- a/src/EFCore/ChangeTracking/Internal/InternalEntryBase.InternalComplexCollectionEntry.cs +++ b/src/EFCore/ChangeTracking/Internal/InternalEntryBase.InternalComplexCollectionEntry.cs @@ -149,24 +149,31 @@ public void AcceptChanges() if (_originalEntries != null) { - foreach (var entry in _originalEntries) + for (var i = 0; i < _originalEntries.Count; i++) { + var entry = _originalEntries[i]; if (entry == null || entry.EntityState != EntityState.Deleted) { continue; } entry.AcceptChanges(); + // The entry was deleted, so AcceptChanges removed it from the list + i--; } } if (_entries != null) { - _originalEntries = new List(_entries.Count); - foreach (var entry in _entries) + _originalEntries ??= new List(_entries.Count); + _originalEntries.Clear(); + for (var i = 0; i < _entries.Count; i++) + { + _originalEntries.Add(_entries[i]); + } + for (var i = 0; i < _originalEntries.Count; i++) { - _originalEntries.Add(entry); - entry?.AcceptChanges(); + _originalEntries[i]?.AcceptChanges(); } } else @@ -195,10 +202,14 @@ public void RejectChanges() if (_originalEntries != null) { - _entries = new List(_originalEntries.Count); - foreach (var entry in _originalEntries) + _entries ??= new List(_originalEntries.Count); + _entries.Clear(); + for (var i = 0; i < _originalEntries.Count; i++) + { + _entries.Add(_originalEntries[i]); + } + foreach (var entry in _entries) { - _entries.Add(entry); entry?.Ordinal = entry.OriginalOrdinal; } } diff --git a/src/EFCore/ChangeTracking/Internal/InternalEntryBase.cs b/src/EFCore/ChangeTracking/Internal/InternalEntryBase.cs index 0073348677a..d81686ad029 100644 --- a/src/EFCore/ChangeTracking/Internal/InternalEntryBase.cs +++ b/src/EFCore/ChangeTracking/Internal/InternalEntryBase.cs @@ -303,18 +303,18 @@ protected virtual void SetEntityState(EntityState oldState, EntityState newState { _originalValues.AcceptChanges(this); - foreach (var complexCollection in _complexCollectionEntries) + for (var i = 0; i < _complexCollectionEntries.Length; i++) { - complexCollection.AcceptChanges(); + _complexCollectionEntries[i].AcceptChanges(); } } else { _originalValues.RejectChanges(this); - foreach (var complexCollection in _complexCollectionEntries) + for (var i = 0; i < _complexCollectionEntries.Length; i++) { - complexCollection.RejectChanges(); + _complexCollectionEntries[i].RejectChanges(); } } } @@ -397,8 +397,7 @@ protected virtual void OnStateChanged(EntityState oldState) /// public bool IsModified(IProperty property) { - Check.DebugAssert(property.DeclaringType.IsAssignableFrom(StructuralType) || property.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + property.Name + " not contained under " + StructuralType.Name); + StructuralType.CheckContains(property); var propertyIndex = property.GetIndex(); @@ -415,8 +414,7 @@ public bool IsModified(IProperty property) /// public bool IsUnknown(IProperty property) { - Check.DebugAssert(property.DeclaringType.IsAssignableFrom(StructuralType) || property.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + property.Name + " not contained under " + StructuralType.Name); + StructuralType.CheckContains(property); return _stateData.IsPropertyFlagged(property.GetIndex(), PropertyFlag.Unknown); } @@ -434,8 +432,7 @@ public void SetPropertyModified( bool isConceptualNull = false, bool acceptChanges = false) { - Check.DebugAssert(property.DeclaringType.IsAssignableFrom(StructuralType) || property.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + property.Name + " not contained under " + StructuralType.Name); + StructuralType.CheckContains(property); var propertyIndex = property.GetIndex(); _stateData.FlagProperty(propertyIndex, PropertyFlag.Unknown, false); @@ -530,11 +527,12 @@ public void SetPropertyModified( /// public bool IsModified(IComplexProperty property) { - Check.DebugAssert(property.IsCollection, $"Property {property.Name} should be a collection"); - Check.DebugAssert(property.DeclaringType.IsAssignableFrom(StructuralType) || property.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + property.Name + " not contained under " + StructuralType.Name); + StructuralType.CheckContains(property); - return _complexCollectionEntries[property.GetIndex()].IsModified(); + return property.IsCollection + ? _complexCollectionEntries[property.GetIndex()].IsModified() + : property.ComplexType.GetFlattenedProperties().Any(IsModified) + || property.ComplexType.GetFlattenedComplexProperties().Any(cp => cp.IsCollection && IsModified(cp)); } /// @@ -549,8 +547,7 @@ public void SetPropertyModified( bool recurse = false) { Check.DebugAssert(property.IsCollection, $"Property {property.Name} should be a collection"); - Check.DebugAssert(property.DeclaringType.IsAssignableFrom(StructuralType) || property.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + property.Name + " not contained under " + StructuralType.Name); + StructuralType.CheckContains(property); var index = property.GetIndex(); if (_complexCollectionEntries[index].IsModified() == isModified) @@ -624,8 +621,7 @@ public bool HasConceptualNull /// public bool IsConceptualNull(IProperty property) { - Check.DebugAssert(property.DeclaringType.IsAssignableFrom(StructuralType) || property.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + property.Name + " not contained under " + StructuralType.Name); + StructuralType.CheckContains(property); return _stateData.IsPropertyFlagged(property.GetIndex(), PropertyFlag.Null); } @@ -638,8 +634,7 @@ public bool IsConceptualNull(IProperty property) /// public bool HasTemporaryValue(IProperty property) { - Check.DebugAssert(property.DeclaringType.IsAssignableFrom(StructuralType) || property.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + property.Name + " not contained under " + StructuralType.Name); + StructuralType.CheckContains(property); return GetValueType(property) == CurrentValueType.Temporary; } @@ -652,8 +647,7 @@ public bool HasTemporaryValue(IProperty property) /// protected virtual CurrentValueType GetValueType(IProperty property) { - Check.DebugAssert(property.DeclaringType.IsAssignableFrom(StructuralType) || property.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + property.Name + " not contained under " + StructuralType.Name); + StructuralType.CheckContains(property); return _stateData.IsPropertyFlagged(property.GetIndex(), PropertyFlag.IsStoreGenerated) ? CurrentValueType.StoreGenerated @@ -670,8 +664,7 @@ protected virtual CurrentValueType GetValueType(IProperty property) /// public void SetTemporaryValue(IProperty property, object? value, bool setModified = true) { - Check.DebugAssert(property.DeclaringType.IsAssignableFrom(StructuralType) || property.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + property.Name + " not contained under " + StructuralType.Name); + StructuralType.CheckContains(property); if (property.GetStoreGeneratedIndex() == -1) { @@ -691,8 +684,7 @@ public void SetTemporaryValue(IProperty property, object? value, bool setModifie /// public void MarkAsTemporary(IProperty property, bool temporary) { - Check.DebugAssert(property.DeclaringType.IsAssignableFrom(StructuralType) || property.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + property.Name + " not contained under " + StructuralType.Name); + StructuralType.CheckContains(property); _stateData.FlagProperty(property.GetIndex(), PropertyFlag.IsTemporary, temporary); } @@ -723,8 +715,7 @@ public static readonly MethodInfo FlaggedAsStoreGeneratedMethod /// public void SetStoreGeneratedValue(IProperty property, object? value, bool setModified = true) { - Check.DebugAssert(property.DeclaringType.IsAssignableFrom(StructuralType) || property.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + property.Name + " not contained under " + StructuralType.Name); + StructuralType.CheckContains(property); if (property.GetStoreGeneratedIndex() == -1) { @@ -749,8 +740,7 @@ public void SetStoreGeneratedValue(IProperty property, object? value, bool setMo /// public void MarkUnknown(IProperty property) { - Check.DebugAssert(property.DeclaringType.IsAssignableFrom(StructuralType) || property.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + property.Name + " not contained under " + StructuralType.Name); + StructuralType.CheckContains(property); _stateData.FlagProperty(property.GetIndex(), PropertyFlag.Unknown, true); } @@ -857,8 +847,7 @@ public TProperty GetOriginalValue(IProperty property) /// public virtual object? ReadPropertyValue(IPropertyBase propertyBase) { - Check.DebugAssert(propertyBase.DeclaringType.IsAssignableFrom(StructuralType) || propertyBase.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + propertyBase.Name + " not contained under " + StructuralType.Name); + StructuralType.CheckContains(propertyBase); return propertyBase.IsShadowProperty() ? _shadowValues[propertyBase.GetShadowIndex()] @@ -876,8 +865,7 @@ private void WritePropertyValue( object? value, bool forMaterialization) { - Check.DebugAssert(propertyBase.DeclaringType.IsAssignableFrom(StructuralType) || propertyBase.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + propertyBase.Name + " not contained under " + StructuralType.Name); + StructuralType.CheckContains(propertyBase); if (value == null && !propertyBase.ClrType.IsNullableType()) @@ -909,14 +897,9 @@ private void WritePropertyValue( /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public object? GetCurrentValue(IPropertyBase propertyBase) - { - Check.DebugAssert(propertyBase.DeclaringType.IsAssignableFrom(StructuralType) || propertyBase.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + propertyBase.Name + " not contained under " + StructuralType.Name); - - return propertyBase is not IProperty property || !IsConceptualNull(property) + => StructuralType.CheckContains(propertyBase) is not IProperty property || !IsConceptualNull(property) ? this[propertyBase] : null; - } /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -925,14 +908,9 @@ private void WritePropertyValue( /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public object? GetPreStoreGeneratedCurrentValue(IPropertyBase propertyBase) - { - Check.DebugAssert(propertyBase.DeclaringType.IsAssignableFrom(StructuralType) || propertyBase.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + propertyBase.Name + " not contained under " + StructuralType.Name); - - return propertyBase is not IProperty property || !IsConceptualNull(property) + => StructuralType.CheckContains(propertyBase) is not IProperty property || !IsConceptualNull(property) ? ReadPropertyValue(propertyBase) : null; - } /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -942,8 +920,7 @@ private void WritePropertyValue( /// public virtual object? GetOriginalValue(IPropertyBase propertyBase) { - Check.DebugAssert(propertyBase.DeclaringType.IsAssignableFrom(StructuralType) || propertyBase.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + propertyBase.Name + " not contained under " + StructuralType.Name); + StructuralType.CheckContains(propertyBase); return _originalValues.GetValue(this, propertyBase); } @@ -968,8 +945,7 @@ public void SetOriginalValue( object? value, int index = -1) { - Check.DebugAssert(propertyBase.DeclaringType.IsAssignableFrom(StructuralType) || propertyBase.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + propertyBase.Name + " not contained under " + StructuralType.Name); + StructuralType.CheckContains(propertyBase); EnsureOriginalValues(); @@ -1116,8 +1092,7 @@ public virtual bool HasOriginalValuesSnapshot /// public InternalComplexEntry GetComplexCollectionEntry(IComplexProperty property, int ordinal) { - Check.DebugAssert(property.DeclaringType.IsAssignableFrom(StructuralType) || property.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + property.Name + " not contained under " + StructuralType.Name); + StructuralType.CheckContains(property); Check.DebugAssert(property.IsCollection, $"Property {property.Name} should be a collection"); return _complexCollectionEntries[property.GetIndex()].GetEntry(ordinal); @@ -1131,8 +1106,7 @@ public InternalComplexEntry GetComplexCollectionEntry(IComplexProperty property, /// public IReadOnlyList GetComplexCollectionEntries(IComplexProperty property) { - Check.DebugAssert(property.DeclaringType.IsAssignableFrom(StructuralType) || property.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + property.Name + " not contained under " + StructuralType.Name); + StructuralType.CheckContains(property); Check.DebugAssert(property.IsCollection, $"Property {property.Name} should be a collection"); return _complexCollectionEntries[property.GetIndex()].GetOrCreateEntries(original: false)!; @@ -1146,8 +1120,7 @@ public InternalComplexEntry GetComplexCollectionEntry(IComplexProperty property, /// public InternalComplexEntry GetComplexCollectionOriginalEntry(IComplexProperty property, int ordinal) { - Check.DebugAssert(property.DeclaringType.IsAssignableFrom(StructuralType) || property.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + property.Name + " not contained under " + StructuralType.Name); + StructuralType.CheckContains(property); Check.DebugAssert(property.IsCollection, $"Property {property.Name} should be a collection"); return _complexCollectionEntries[property.GetIndex()].GetEntry(ordinal, original: true); @@ -1161,8 +1134,7 @@ public InternalComplexEntry GetComplexCollectionOriginalEntry(IComplexProperty p /// public IReadOnlyList GetComplexCollectionOriginalEntries(IComplexProperty property) { - Check.DebugAssert(property.DeclaringType.IsAssignableFrom(StructuralType) || property.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + property.Name + " not contained under " + StructuralType.Name); + StructuralType.CheckContains(property); Check.DebugAssert(property.IsCollection, $"Property {property.Name} should be a collection"); return _complexCollectionEntries[property.GetIndex()].GetOrCreateEntries(original: true)!; @@ -1185,8 +1157,7 @@ public IEnumerable GetFlattenedComplexEntries() /// public virtual void EnsureComplexCollectionEntriesCapacity(IComplexProperty property, int capacity, int originalCapacity, bool trim = true) { - Check.DebugAssert(property.DeclaringType.IsAssignableFrom(StructuralType) || property.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + property.Name + " not contained under " + StructuralType.Name); + StructuralType.CheckContains(property); Check.DebugAssert(property.IsCollection, $"Property {property.Name} should be a collection"); var index = property.GetIndex(); @@ -1202,8 +1173,7 @@ public virtual void EnsureComplexCollectionEntriesCapacity(IComplexProperty prop /// public virtual void MoveComplexCollectionEntry(IComplexProperty property, int fromOrdinal, int toOrdinal, bool original = false) { - Check.DebugAssert(property.DeclaringType.IsAssignableFrom(StructuralType) || property.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + property.Name + " not contained under " + StructuralType.Name); + StructuralType.CheckContains(property); Check.DebugAssert(property.IsCollection, $"Property {property.Name} should be a collection"); _complexCollectionEntries[property.GetIndex()].MoveEntry(fromOrdinal, toOrdinal, original); @@ -1218,8 +1188,7 @@ public virtual void MoveComplexCollectionEntry(IComplexProperty property, int fr public void OnComplexElementStateChange(InternalComplexEntry entry, EntityState oldState, EntityState newState) { var property = entry.ComplexProperty; - Check.DebugAssert(property.DeclaringType.IsAssignableFrom(StructuralType) || property.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + property.Name + " not contained under " + StructuralType.Name); + StructuralType.CheckContains(property); Check.DebugAssert(property.IsCollection, $"Property {property.Name} should be a collection"); _complexCollectionEntries[property.GetIndex()].HandleStateChange(entry, oldState, newState); @@ -1234,8 +1203,7 @@ public void OnComplexElementStateChange(InternalComplexEntry entry, EntityState public virtual int ValidateOrdinal(InternalComplexEntry entry, bool original) { var property = entry.ComplexProperty; - Check.DebugAssert(property.DeclaringType.IsAssignableFrom(StructuralType) || property.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + property.Name + " not contained under " + StructuralType.Name); + StructuralType.CheckContains(property); Check.DebugAssert(property.IsCollection, $"Property {property.Name} should be a collection"); return _complexCollectionEntries[entry.ComplexProperty.GetIndex()].ValidateOrdinal(entry, original); @@ -1517,9 +1485,9 @@ public virtual void AcceptChanges() { _originalValues.AcceptChanges(this); - foreach (var complexCollection in _complexCollectionEntries) + for (var i = 0; i < _complexCollectionEntries.Length; i++) { - complexCollection.AcceptChanges(); + _complexCollectionEntries[i].AcceptChanges(); } } @@ -1643,7 +1611,6 @@ void CheckForNullComplexProperties() foreach (var complexProperty in structuralType.GetFlattenedComplexProperties()) { if (!complexProperty.IsNullable - && !complexProperty.IsCollection && this[complexProperty] == null && complexProperty.ComplexType.GetProperties().Any(p => !p.IsNullable) && (complexProperty.DeclaringType is not IComplexType complexType @@ -1725,8 +1692,7 @@ public bool IsStoreGenerated(IProperty property) [MethodImpl(MethodImplOptions.AggressiveInlining)] public bool HasExplicitValue(IProperty property) { - Check.DebugAssert(property.DeclaringType.IsAssignableFrom(StructuralType) || property.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + property.Name + " not contained under " + StructuralType.Name); + StructuralType.CheckContains(property); return !HasSentinelValue(property) || _stateData.IsPropertyFlagged(property.GetIndex(), PropertyFlag.IsStoreGenerated) @@ -1735,8 +1701,7 @@ public bool HasExplicitValue(IProperty property) private bool HasSentinelValue(IProperty property) { - Check.DebugAssert(property.DeclaringType.IsAssignableFrom(StructuralType) || property.DeclaringType.ContainingType.IsAssignableFrom(StructuralType), - "Property " + property.Name + " not contained under " + StructuralType.Name); + StructuralType.CheckContains(property); return property.IsShadowProperty() ? AreEqual(_shadowValues[property.GetShadowIndex()], property.Sentinel, property) diff --git a/src/EFCore/Metadata/Builders/ComplexCollectionBuilder.cs b/src/EFCore/Metadata/Builders/ComplexCollectionBuilder.cs index f09fdea45e2..71be133c83e 100644 --- a/src/EFCore/Metadata/Builders/ComplexCollectionBuilder.cs +++ b/src/EFCore/Metadata/Builders/ComplexCollectionBuilder.cs @@ -94,6 +94,20 @@ public virtual ComplexCollectionBuilder HasTypeAnnotation(string annotation, obj return this; } + /// + /// Configures whether this property must have a value assigned or is a valid value. + /// A property can only be configured as non-required if it is based on a CLR type that can be + /// assigned . + /// + /// A value indicating whether the property is required. + /// The same builder instance so that multiple configuration calls can be chained. + public virtual ComplexCollectionBuilder IsRequired(bool required = true) + { + PropertyBuilder.IsRequired(required, ConfigurationSource.Explicit); + + return this; + } + /// /// Returns an object that can be used to configure a property of the complex type. /// If no property with the given name exists, then a new property will be added. diff --git a/src/EFCore/Metadata/Builders/ComplexCollectionBuilder`.cs b/src/EFCore/Metadata/Builders/ComplexCollectionBuilder`.cs index 785d2bb84af..b5c81a7fcc3 100644 --- a/src/EFCore/Metadata/Builders/ComplexCollectionBuilder`.cs +++ b/src/EFCore/Metadata/Builders/ComplexCollectionBuilder`.cs @@ -53,6 +53,16 @@ public ComplexCollectionBuilder(IMutableComplexProperty complexProperty) public new virtual ComplexCollectionBuilder HasTypeAnnotation(string annotation, object? value) => (ComplexCollectionBuilder)base.HasTypeAnnotation(annotation, value); + /// + /// Configures whether this property must have a value assigned or is a valid value. + /// A property can only be configured as non-required if it is based on a CLR type that can be + /// assigned . + /// + /// A value indicating whether the property is required. + /// The same builder instance so that multiple configuration calls can be chained. + public new virtual ComplexCollectionBuilder IsRequired(bool required = true) + => (ComplexCollectionBuilder)base.IsRequired(required); + /// /// Returns an object that can be used to configure a property of the complex type. /// If the specified property is not already part of the model, it will be added. diff --git a/src/EFCore/Metadata/Internal/ComplexProperty.cs b/src/EFCore/Metadata/Internal/ComplexProperty.cs index e458b1f627d..2e01fca8307 100644 --- a/src/EFCore/Metadata/Internal/ComplexProperty.cs +++ b/src/EFCore/Metadata/Internal/ComplexProperty.cs @@ -48,12 +48,6 @@ public ComplexProperty( targetTypeName ?? declaringType.GetOwnedName(targetType.ShortDisplayName(), name), targetType, this, configurationSource); _builder = new InternalComplexPropertyBuilder(this, declaringType.Model.Builder); - - if (collection) - { - _isNullable = false; - _isNullableConfigurationSource = configurationSource; - } } /// @@ -148,12 +142,6 @@ public virtual bool IsNullable throw new InvalidOperationException( CoreStrings.CannotBeNullable(Name, DeclaringType.DisplayName(), ClrType.ShortDisplayName())); } - - if (IsCollection) - { - throw new InvalidOperationException( - CoreStrings.ComplexPropertyOptional(DeclaringType.DisplayName(), Name)); - } } _isNullableConfigurationSource = configurationSource.Max(_isNullableConfigurationSource); diff --git a/src/EFCore/Metadata/Internal/TypeBase.cs b/src/EFCore/Metadata/Internal/TypeBase.cs index 1dda9946e09..cdc8dc6602c 100644 --- a/src/EFCore/Metadata/Internal/TypeBase.cs +++ b/src/EFCore/Metadata/Internal/TypeBase.cs @@ -1661,7 +1661,7 @@ public virtual IEnumerable GetFlattenedDeclaredProperties() { if (complexProperty.IsCollection) { - break; + continue; } foreach (var property in complexProperty.ComplexType.GetFlattenedDeclaredProperties()) diff --git a/src/EFCore/Metadata/Internal/TypeBaseExtensions.cs b/src/EFCore/Metadata/Internal/TypeBaseExtensions.cs index 4c1f31bbf8e..c2aef88f3fb 100644 --- a/src/EFCore/Metadata/Internal/TypeBaseExtensions.cs +++ b/src/EFCore/Metadata/Internal/TypeBaseExtensions.cs @@ -42,7 +42,10 @@ public static T CheckContains(this IReadOnlyTypeBase structuralType, T proper { Check.NotNull(property); - return !property.DeclaringType.IsAssignableFrom(structuralType) && !property.DeclaringType.ContainingType.IsAssignableFrom(structuralType) + return !property.DeclaringType.IsAssignableFrom(structuralType) + && (!property.DeclaringType.ContainingType.IsAssignableFrom(structuralType) + || (property.DeclaringType is IComplexType complexType + && complexType.ComplexProperty.IsCollection)) ? throw new InvalidOperationException( CoreStrings.PropertyDoesNotBelong(property.Name, property.DeclaringType.DisplayName(), structuralType.DisplayName())) : property; diff --git a/src/EFCore/Metadata/RuntimeTypeBase.cs b/src/EFCore/Metadata/RuntimeTypeBase.cs index 3e8d9c1dc42..04e754e9d89 100644 --- a/src/EFCore/Metadata/RuntimeTypeBase.cs +++ b/src/EFCore/Metadata/RuntimeTypeBase.cs @@ -655,7 +655,7 @@ static IEnumerable Create(RuntimeTypeBase type) { if (((IComplexProperty)complexProperty).IsCollection) { - break; + continue; } foreach (var property in complexProperty.ComplexType.GetFlattenedDeclaredProperties()) diff --git a/src/EFCore/Properties/CoreStrings.Designer.cs b/src/EFCore/Properties/CoreStrings.Designer.cs index 839c2ed4037..c76b5a13959 100644 --- a/src/EFCore/Properties/CoreStrings.Designer.cs +++ b/src/EFCore/Properties/CoreStrings.Designer.cs @@ -654,14 +654,6 @@ public static string ComplexPropertyNotFound(object? type, object? property) GetString("ComplexPropertyNotFound", nameof(type), nameof(property)), type, property); - /// - /// Configuring the collection complex property '{type}.{property}' as optional is invalid. - /// - public static string ComplexPropertyOptional(object? type, object? property) - => string.Format( - GetString("ComplexPropertyOptional", nameof(type), nameof(property)), - type, property); - /// /// Configuring the complex property '{type}.{property}' in shadow state isn't supported. See https://github.com/dotnet/efcore/issues/31243 for more information. /// diff --git a/src/EFCore/Properties/CoreStrings.resx b/src/EFCore/Properties/CoreStrings.resx index 5d7ec75be16..2a7ed7a0739 100644 --- a/src/EFCore/Properties/CoreStrings.resx +++ b/src/EFCore/Properties/CoreStrings.resx @@ -354,9 +354,6 @@ The complex property '{type}.{property}' could not be found. Ensure that the property exists and has been included in the model as a complex property. - - Configuring the collection complex property '{type}.{property}' as optional is invalid. - Configuring the complex property '{type}.{property}' in shadow state isn't supported. See https://github.com/dotnet/efcore/issues/31243 for more information. diff --git a/src/EFCore/Update/IUpdateEntry.cs b/src/EFCore/Update/IUpdateEntry.cs index ea03a1d6937..6a7482ab25b 100644 --- a/src/EFCore/Update/IUpdateEntry.cs +++ b/src/EFCore/Update/IUpdateEntry.cs @@ -52,13 +52,21 @@ public interface IUpdateEntry IUpdateEntry? SharedIdentityEntry { get; } /// - /// Gets a value indicating if the specified property is modified. If true, the current value assigned - /// to the property should be saved to the database. + /// Gets a value indicating if the specified property is modified. If , the current value + /// assigned to the property should be saved to the database. /// /// The property to be checked. /// if the property is modified, otherwise . bool IsModified(IProperty property); + /// + /// Gets a value indicating if the specified complex property is modified. If , + /// the current value assigned to the property should be saved to the database. + /// + /// The property to be checked. + /// if the property is modified, otherwise . + bool IsModified(IComplexProperty property); + /// /// Gets a value indicating if the specified property has a temporary value. /// diff --git a/src/EFCore/Update/UpdateEntryExtensions.cs b/src/EFCore/Update/UpdateEntryExtensions.cs index 204e2275bf7..c8b3db84874 100644 --- a/src/EFCore/Update/UpdateEntryExtensions.cs +++ b/src/EFCore/Update/UpdateEntryExtensions.cs @@ -4,6 +4,7 @@ using System.Collections; using System.Globalization; using System.Text; +using Microsoft.EntityFrameworkCore.ChangeTracking; using Microsoft.EntityFrameworkCore.ChangeTracking.Internal; using Microsoft.EntityFrameworkCore.Metadata.Internal; @@ -107,78 +108,7 @@ public static string ToDebugString( if ((options & ChangeTrackerDebugStringOptions.IncludeProperties) != 0) { - DumpProperties(entry.EntityType, indent + 2); - - void DumpProperties(ITypeBase structuralType, int tempIndent) - { - var tempIndentString = new string(' ', tempIndent); - foreach (var property in structuralType.GetProperties()) - { - builder.AppendLine().Append(tempIndentString); - - var currentValue = entry.GetCurrentValue(property); - builder - .Append(" ") - .Append(property.Name) - .Append(": "); - - AppendValue(currentValue); - - if (property.IsPrimaryKey()) - { - builder.Append(" PK"); - } - else if (property.IsKey()) - { - builder.Append(" AK"); - } - - if (property.IsForeignKey()) - { - builder.Append(" FK"); - } - - if (entry.IsModified(property)) - { - builder.Append(" Modified"); - } - - if (entry.HasTemporaryValue(property)) - { - builder.Append(" Temporary"); - } - - if (entry.IsUnknown(property)) - { - builder.Append(" Unknown"); - } - - if (entry.HasOriginalValuesSnapshot - && property.GetOriginalValueIndex() != -1) - { - var originalValue = entry.GetOriginalValue(property); - if (!Equals(originalValue, currentValue)) - { - builder.Append(" Originally "); - AppendValue(originalValue); - } - } - } - - foreach (var complexProperty in structuralType.GetComplexProperties()) - { - builder.AppendLine().Append(tempIndentString); - - builder - .Append(" ") - .Append(complexProperty.Name) - .Append(" (Complex: ") - .Append(complexProperty.ClrType.ShortDisplayName()) - .Append(")"); - - DumpProperties(complexProperty.ComplexType, tempIndent + 2); - } - } + DumpProperties(entry, entry.EntityType, indent + 2); } else { @@ -232,7 +162,7 @@ void DumpProperties(ITypeBase structuralType, int tempIndent) if (i < 32) { - AppendRelatedKey(targetType, relatedEntities[i]); + AppendRelatedKey(entry, targetType, relatedEntities[i]); } else { @@ -244,56 +174,130 @@ void DumpProperties(ITypeBase structuralType, int tempIndent) } else { - AppendRelatedKey(targetType, currentValue); + AppendRelatedKey(entry, targetType, currentValue); } } } + } + catch (Exception exception) + { + builder.AppendLine().AppendLine(CoreStrings.DebugViewError(exception.Message)); + } + + return builder.ToString(); - void AppendValue(object? value) + void DumpProperties(InternalEntityEntry entry, ITypeBase structuralType, int tempIndent) + { + var tempIndentString = new string(' ', tempIndent); + foreach (var property in structuralType.GetProperties()) { - if (value == null) + builder.AppendLine().Append(tempIndentString); + + var currentValue = entry.GetCurrentValue(property); + builder + .Append(" ") + .Append(property.Name) + .Append(": "); + + AppendValue(currentValue); + + if (property.IsPrimaryKey()) + { + builder.Append(" PK"); + } + else if (property.IsKey()) + { + builder.Append(" AK"); + } + + if (property.IsForeignKey()) { - builder.Append(""); + builder.Append(" FK"); } - else if (value.GetType().IsNumeric()) + + if (entry.IsModified(property)) { - builder.Append(value); + builder.Append(" Modified"); } - else if (value is byte[] bytes) + + if (entry.HasTemporaryValue(property)) + { + builder.Append(" Temporary"); + } + + if (entry.IsUnknown(property)) { - builder.AppendBytes(bytes); + builder.Append(" Unknown"); } - else + + if (entry.HasOriginalValuesSnapshot + && property.GetOriginalValueIndex() != -1) { - var stringValue = value.ToString(); - if (stringValue?.Length > 63) + var originalValue = entry.GetOriginalValue(property); + if (!Equals(originalValue, currentValue)) { - stringValue = string.Concat(stringValue.AsSpan(0, 60), "..."); + builder.Append(" Originally "); + AppendValue(originalValue); } - - builder - .Append('\'') - .Append(stringValue) - .Append('\''); } } - void AppendRelatedKey(IEntityType targetType, object value) + foreach (var complexProperty in structuralType.GetComplexProperties()) { - var otherEntry = entry.StateManager.TryGetEntry(value, targetType, throwOnTypeMismatch: false); + builder.AppendLine().Append(tempIndentString); - builder.Append( - otherEntry == null - ? "" - : otherEntry.BuildCurrentValuesString(targetType.FindPrimaryKey()!.Properties)); + builder + .Append(" ") + .Append(complexProperty.Name) + .Append(" (Complex: ") + .Append(complexProperty.ClrType.ShortDisplayName()) + .Append(")"); + + if (!complexProperty.IsCollection) + { + DumpProperties(entry, complexProperty.ComplexType, tempIndent + 2); + } } } - catch (Exception exception) + + void AppendValue(object? value) { - builder.AppendLine().AppendLine(CoreStrings.DebugViewError(exception.Message)); + if (value == null) + { + builder.Append(""); + } + else if (value.GetType().IsNumeric()) + { + builder.Append(value); + } + else if (value is byte[] bytes) + { + builder.AppendBytes(bytes); + } + else + { + var stringValue = value.ToString(); + if (stringValue?.Length > 63) + { + stringValue = string.Concat(stringValue.AsSpan(0, 60), "..."); + } + + builder + .Append('\'') + .Append(stringValue) + .Append('\''); + } } - return builder.ToString(); + void AppendRelatedKey(InternalEntityEntry entry, IEntityType targetType, object value) + { + var otherEntry = entry.StateManager.TryGetEntry(value, targetType, throwOnTypeMismatch: false); + + builder.Append( + otherEntry == null + ? "" + : otherEntry.BuildCurrentValuesString(targetType.FindPrimaryKey()!.Properties)); + } } /// diff --git a/test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosTestStore.cs b/test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosTestStore.cs index dc0b37d2fca..b088d2a608c 100644 --- a/test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosTestStore.cs +++ b/test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosTestStore.cs @@ -654,6 +654,7 @@ public object GetPreStoreGeneratedCurrentValue(IPropertyBase propertyBase) public bool IsConceptualNull(IProperty property) => throw new NotImplementedException(); + public bool IsModified(IComplexProperty property) => throw new NotImplementedException(); } public class FakeEntityType : Annotatable, IEntityType diff --git a/test/EFCore.Relational.Specification.Tests/Query/Relationships/ComplexJson/ComplexJsonRelationalFixtureBase.cs b/test/EFCore.Relational.Specification.Tests/Query/Relationships/ComplexJson/ComplexJsonRelationalFixtureBase.cs index 8aa6413d5d2..43f66403f94 100644 --- a/test/EFCore.Relational.Specification.Tests/Query/Relationships/ComplexJson/ComplexJsonRelationalFixtureBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Query/Relationships/ComplexJson/ComplexJsonRelationalFixtureBase.cs @@ -28,8 +28,6 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con orb.IsRequired(false); orb.ComplexProperty(r => r.OptionalNested).IsRequired(false); - - orb.ComplexProperty(r => r.RequiredNested); }); b.ComplexCollection(e => e.RelatedCollection,rcb => diff --git a/test/EFCore.Relational.Specification.Tests/Update/ComplexCollectionJsonUpdateTestBase.cs b/test/EFCore.Relational.Specification.Tests/Update/ComplexCollectionJsonUpdateTestBase.cs index af400117a87..247a5fb50c0 100644 --- a/test/EFCore.Relational.Specification.Tests/Update/ComplexCollectionJsonUpdateTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Update/ComplexCollectionJsonUpdateTestBase.cs @@ -11,7 +11,7 @@ public abstract class ComplexCollectionJsonUpdateTestBase(TFixture fix protected ComplexCollectionJsonContext CreateContext() => (ComplexCollectionJsonContext)Fixture.CreateContext(); - [ConditionalFact] + [ConditionalFact(Skip = "Issue #36433")] public virtual Task Add_element_to_complex_collection_mapped_to_json() => TestHelpers.ExecuteWithStrategyInTransactionAsync( CreateContext, @@ -19,18 +19,38 @@ public virtual Task Add_element_to_complex_collection_mapped_to_json() async context => { var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + + var companyEntry = context.Entry(company); + var budgetProperty = companyEntry.ComplexProperty(c => c.Department).Property(c => c.Budget); + Assert.Equal(budgetProperty.CurrentValue, budgetProperty.OriginalValue); + company.Contacts!.Add(new Contact { Name = "New Contact", PhoneNumbers = ["555-0000"] }); - + + Assert.Equal(""" +CompanyWithComplexCollections {Id: 1} Unchanged + Id: 1 PK + Name: 'Test Company' + Contacts (Complex: List) + Department (Complex: Department) + Budget: 10000.00 + Name: 'Initial Department' + Employees (Complex: List) + +""", context.ChangeTracker.DebugView.LongView); + ClearLog(); await context.SaveChangesAsync(); }, async context => { - var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); - Assert.Equal(3, company.Contacts!.Count); - Assert.Equal("New Contact", company.Contacts[2].Name); - Assert.Single(company.Contacts[2].PhoneNumbers); - Assert.Equal("555-0000", company.Contacts[2].PhoneNumbers[0]); + using (SuspendRecordingEvents()) + { + var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + Assert.Equal(3, company.Contacts!.Count); + Assert.Equal("New Contact", company.Contacts[2].Name); + Assert.Single(company.Contacts[2].PhoneNumbers); + Assert.Equal("555-0000", company.Contacts[2].PhoneNumbers[0]); + } }); [ConditionalFact] @@ -41,16 +61,20 @@ public virtual Task Remove_element_from_complex_collection_mapped_to_json() async context => { var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + company.Contacts!.RemoveAt(0); - + ClearLog(); await context.SaveChangesAsync(); }, async context => { - var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); - Assert.Single(company.Contacts!); - Assert.Equal("Second Contact", company.Contacts![0].Name); + using (SuspendRecordingEvents()) + { + var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + Assert.Single(company.Contacts!); + Assert.Equal("Second Contact", company.Contacts![0].Name); + } }); [ConditionalFact] @@ -61,15 +85,19 @@ public virtual Task Modify_element_in_complex_collection_mapped_to_json() async context => { var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + company.Contacts![0].Name = "First Contact - Modified"; - + ClearLog(); await context.SaveChangesAsync(); }, async context => { - var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); - Assert.Equal("First Contact - Modified", company.Contacts![0].Name); + using (SuspendRecordingEvents()) + { + var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + Assert.Equal("First Contact - Modified", company.Contacts![0].Name); + } }); [ConditionalFact] @@ -80,63 +108,56 @@ public virtual Task Move_elements_in_complex_collection_mapped_to_json() async context => { var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + var temp = company.Contacts![0]; company.Contacts[0] = company.Contacts[1]; company.Contacts[1] = temp; - + ClearLog(); await context.SaveChangesAsync(); }, async context => { - var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); - Assert.Equal("Second Contact", company.Contacts![0].Name); - Assert.Equal("First Contact", company.Contacts[1].Name); + using (SuspendRecordingEvents()) + { + var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + Assert.Equal("Second Contact", company.Contacts![0].Name); + Assert.Equal("First Contact", company.Contacts[1].Name); + } }); - [ConditionalFact] - public virtual Task Change_empty_complex_collection_to_null_mapped_to_json() + [ConditionalFact(Skip = "Issue #36433")] + public virtual Task Change_complex_collection_mapped_to_json_to_null_and_to_empty() => TestHelpers.ExecuteWithStrategyInTransactionAsync( CreateContext, UseTransaction, async context => { var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); - company.Contacts!.Clear(); - await context.SaveChangesAsync(); - - company.Contacts = null; - ClearLog(); + + company.Contacts!.Clear(); await context.SaveChangesAsync(); }, async context => { - var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); - Assert.Null(company.Contacts); - }); - - [ConditionalFact] - public virtual Task Change_null_complex_collection_to_empty_mapped_to_json() - => TestHelpers.ExecuteWithStrategyInTransactionAsync( - CreateContext, - UseTransaction, - async context => - { - var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); - company.Contacts = null; - await context.SaveChangesAsync(); + using (SuspendRecordingEvents()) + { + var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + Assert.NotNull(company.Contacts); + Assert.Empty(company.Contacts); + company.Contacts = null; + } - company.Contacts = []; - - ClearLog(); await context.SaveChangesAsync(); }, async context => { - var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); - Assert.NotNull(company.Contacts); - Assert.Empty(company.Contacts); + using (SuspendRecordingEvents()) + { + var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + Assert.Null(company.Contacts); + } }); [ConditionalFact] @@ -147,6 +168,7 @@ public virtual Task Complex_collection_with_nested_complex_type_mapped_to_json() async context => { var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + company.Employees = [ new Employee @@ -174,24 +196,27 @@ public virtual Task Complex_collection_with_nested_complex_type_mapped_to_json() } } ]; - + ClearLog(); await context.SaveChangesAsync(); }, async context => { - var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); - Assert.Equal(2, company.Employees!.Count); - - var john = company.Employees[0]; - Assert.Equal("John Doe", john.Name); - Assert.Equal("123 Main St", john.Address.Street); - Assert.Equal("Seattle", john.Address.City); - - var jane = company.Employees[1]; - Assert.Equal("Jane Smith", jane.Name); - Assert.Equal("456 Oak Ave", jane.Address.Street); - Assert.Equal("Portland", jane.Address.City); + using (SuspendRecordingEvents()) + { + var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + Assert.Equal(2, company.Employees!.Count); + + var john = company.Employees[0]; + Assert.Equal("John Doe", john.Name); + Assert.Equal("123 Main St", john.Address.Street); + Assert.Equal("Seattle", john.Address.City); + + var jane = company.Employees[1]; + Assert.Equal("Jane Smith", jane.Name); + Assert.Equal("456 Oak Ave", jane.Address.Street); + Assert.Equal("Portland", jane.Address.City); + } }); [ConditionalFact] @@ -202,21 +227,25 @@ public virtual Task Modify_multiple_complex_properties_mapped_to_json() async context => { var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + company.Contacts = [new Contact { Name = "Contact 1", PhoneNumbers = ["555-1111"] }]; company.Department = new Department { Name = "Department A", Budget = 50000.00m }; - + ClearLog(); await context.SaveChangesAsync(); }, async context => { - var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); - Assert.Single(company.Contacts!); - Assert.Equal("Contact 1", company.Contacts![0].Name); - - Assert.NotNull(company.Department); - Assert.Equal("Department A", company.Department.Name); - Assert.Equal(50000.00m, company.Department.Budget); + using (SuspendRecordingEvents()) + { + var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + Assert.Single(company.Contacts!); + Assert.Equal("Contact 1", company.Contacts![0].Name); + + Assert.NotNull(company.Department); + Assert.Equal("Department A", company.Department.Name); + Assert.Equal(50000.00m, company.Department.Budget); + } }); [ConditionalFact] @@ -227,15 +256,19 @@ public virtual Task Clear_complex_collection_mapped_to_json() async context => { var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + company.Contacts!.Clear(); - + ClearLog(); await context.SaveChangesAsync(); }, async context => { - var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); - Assert.Empty(company.Contacts!); + using (SuspendRecordingEvents()) + { + var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + Assert.Empty(company.Contacts!); + } }); [ConditionalFact] @@ -246,21 +279,25 @@ public virtual Task Replace_entire_complex_collection_mapped_to_json() async context => { var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + company.Contacts = [ new Contact { Name = "Replacement Contact 1", PhoneNumbers = ["999-1111"] }, new Contact { Name = "Replacement Contact 2", PhoneNumbers = ["999-2222", "999-3333"] } ]; - + ClearLog(); await context.SaveChangesAsync(); }, async context => { - var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); - Assert.Equal(2, company.Contacts!.Count); - Assert.Equal("Replacement Contact 1", company.Contacts[0].Name); - Assert.Equal("Replacement Contact 2", company.Contacts[1].Name); + using (SuspendRecordingEvents()) + { + var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + Assert.Equal(2, company.Contacts!.Count); + Assert.Equal("Replacement Contact 1", company.Contacts[0].Name); + Assert.Equal("Replacement Contact 2", company.Contacts[1].Name); + } }); [ConditionalFact] @@ -271,18 +308,22 @@ public virtual Task Add_element_to_nested_complex_collection_mapped_to_json() async context => { var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + company.Employees![0].PhoneNumbers.Add("555-9999"); - + ClearLog(); await context.SaveChangesAsync(); }, async context => { - var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); - var employee = company.Employees![0]; - Assert.Equal(2, employee.PhoneNumbers.Count); - Assert.Equal("555-0001", employee.PhoneNumbers[0]); - Assert.Equal("555-9999", employee.PhoneNumbers[1]); + using (SuspendRecordingEvents()) + { + var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + var employee = company.Employees![0]; + Assert.Equal(2, employee.PhoneNumbers.Count); + Assert.Equal("555-0001", employee.PhoneNumbers[0]); + Assert.Equal("555-9999", employee.PhoneNumbers[1]); + } }); [ConditionalFact] @@ -293,23 +334,27 @@ public virtual Task Modify_nested_complex_property_in_complex_collection_mapped_ async context => { var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + company.Employees![0].Address.City = "Modified City"; company.Employees[0].Address.PostalCode = "99999"; - + ClearLog(); await context.SaveChangesAsync(); }, async context => { - var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); - var employee = company.Employees![0]; - Assert.Equal("Modified City", employee.Address.City); - Assert.Equal("99999", employee.Address.PostalCode); - Assert.Equal("100 First St", employee.Address.Street); // Unchanged - Assert.Equal("USA", employee.Address.Country); // Unchanged + using (SuspendRecordingEvents()) + { + var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + var employee = company.Employees![0]; + Assert.Equal("Modified City", employee.Address.City); + Assert.Equal("99999", employee.Address.PostalCode); + Assert.Equal("100 First St", employee.Address.Street); // Unchanged + Assert.Equal("USA", employee.Address.Country); // Unchanged + } }); - [ConditionalFact] + [ConditionalFact(Skip = "Issue #36433")] public virtual Task Set_complex_collection_to_null_mapped_to_json() => TestHelpers.ExecuteWithStrategyInTransactionAsync( CreateContext, @@ -317,15 +362,23 @@ public virtual Task Set_complex_collection_to_null_mapped_to_json() async context => { var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + + var companyEntry = context.Entry(company); + var employeesProperty = companyEntry.ComplexCollection(c => c.Employees); + Assert.Equal(employeesProperty.CurrentValue, + employeesProperty.GetInfrastructure().GetOriginalValue(employeesProperty.Metadata)); company.Employees = null; - + ClearLog(); await context.SaveChangesAsync(); }, async context => { - var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); - Assert.Null(company.Employees); + using (SuspendRecordingEvents()) + { + var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + Assert.Null(company.Employees); + } }); [ConditionalFact] @@ -338,7 +391,7 @@ public virtual Task Set_null_complex_collection_to_non_empty_mapped_to_json() var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); company.Employees = null; await context.SaveChangesAsync(); - + company.Employees = [ new Employee @@ -354,16 +407,19 @@ public virtual Task Set_null_complex_collection_to_non_empty_mapped_to_json() } } ]; - + ClearLog(); await context.SaveChangesAsync(); }, async context => { - var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); - Assert.NotNull(company.Employees); - Assert.Single(company.Employees); - Assert.Equal("New Employee", company.Employees[0].Name); + using (SuspendRecordingEvents()) + { + var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + Assert.NotNull(company.Employees); + Assert.Single(company.Employees); + Assert.Equal("New Employee", company.Employees[0].Name); + } }); [ConditionalFact] @@ -374,6 +430,7 @@ public virtual Task Replace_complex_collection_element_mapped_to_json() async context => { var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + company.Employees![0] = new Employee { Name = "Replacement Employee", @@ -386,19 +443,22 @@ public virtual Task Replace_complex_collection_element_mapped_to_json() Country = "Canada" } }; - + ClearLog(); await context.SaveChangesAsync(); }, async context => { - var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); - var employee = company.Employees![0]; - Assert.Equal("Replacement Employee", employee.Name); - Assert.Equal(2, employee.PhoneNumbers.Count); - Assert.Equal("555-7777", employee.PhoneNumbers[0]); - Assert.Equal("789 Replace St", employee.Address.Street); - Assert.Equal("Canada", employee.Address.Country); + using (SuspendRecordingEvents()) + { + var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + var employee = company.Employees![0]; + Assert.Equal("Replacement Employee", employee.Name); + Assert.Equal(2, employee.PhoneNumbers.Count); + Assert.Equal("555-7777", employee.PhoneNumbers[0]); + Assert.Equal("789 Replace St", employee.Address.Street); + Assert.Equal("Canada", employee.Address.Country); + } }); [ConditionalFact] @@ -409,6 +469,7 @@ public virtual Task Complex_collection_with_empty_nested_collections_mapped_to_j async context => { var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + company.Employees!.Add(new Employee { Name = "Employee No Phone", @@ -421,43 +482,24 @@ public virtual Task Complex_collection_with_empty_nested_collections_mapped_to_j Country = "USA" } }); - - ClearLog(); - await context.SaveChangesAsync(); - }, - async context => - { - var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); - Assert.Equal(2, company.Employees!.Count); - var employeeWithoutPhone = company.Employees[1]; - Assert.Equal("Employee No Phone", employeeWithoutPhone.Name); - Assert.Empty(employeeWithoutPhone.PhoneNumbers); - Assert.Equal("Quiet City", employeeWithoutPhone.Address.City); - }); - [ConditionalFact] - public virtual Task Modify_complex_property_mapped_to_json() - => TestHelpers.ExecuteWithStrategyInTransactionAsync( - CreateContext, - UseTransaction, - async context => - { - var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); - company.Department!.Name = "Modified Department"; - company.Department.Budget = 75000.00m; - ClearLog(); await context.SaveChangesAsync(); }, async context => { - var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); - Assert.NotNull(company.Department); - Assert.Equal("Modified Department", company.Department.Name); - Assert.Equal(75000.00m, company.Department.Budget); + using (SuspendRecordingEvents()) + { + var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + Assert.Equal(2, company.Employees!.Count); + var employeeWithoutPhone = company.Employees[1]; + Assert.Equal("Employee No Phone", employeeWithoutPhone.Name); + Assert.Empty(employeeWithoutPhone.PhoneNumbers); + Assert.Equal("Quiet City", employeeWithoutPhone.Address.City); + } }); - [ConditionalFact] + [ConditionalFact(Skip = "Issue #36433")] public virtual Task Set_complex_property_mapped_to_json_to_null() => TestHelpers.ExecuteWithStrategyInTransactionAsync( CreateContext, @@ -466,14 +508,17 @@ public virtual Task Set_complex_property_mapped_to_json_to_null() { var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); company.Department = null; - + ClearLog(); await context.SaveChangesAsync(); }, async context => { - var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); - Assert.Null(company.Department); + using (SuspendRecordingEvents()) + { + var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + Assert.Null(company.Department); + } }); [ConditionalFact] @@ -486,18 +531,21 @@ public virtual Task Set_null_complex_property_to_non_null_mapped_to_json() var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); company.Department = null; await context.SaveChangesAsync(); - + company.Department = new Department { Name = "New Department", Budget = 25000.00m }; - + ClearLog(); await context.SaveChangesAsync(); }, async context => { - var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); - Assert.NotNull(company.Department); - Assert.Equal("New Department", company.Department.Name); - Assert.Equal(25000.00m, company.Department.Budget); + using (SuspendRecordingEvents()) + { + var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + Assert.NotNull(company.Department); + Assert.Equal("New Department", company.Department.Name); + Assert.Equal(25000.00m, company.Department.Budget); + } }); [ConditionalFact] @@ -508,26 +556,34 @@ public virtual Task Replace_complex_property_mapped_to_json() async context => { var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + company.Department = new Department { Name = "Replacement Department", Budget = 99999.99m }; - + ClearLog(); await context.SaveChangesAsync(); }, async context => { - var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); - Assert.NotNull(company.Department); - Assert.Equal("Replacement Department", company.Department.Name); - Assert.Equal(99999.99m, company.Department.Budget); + using (SuspendRecordingEvents()) + { + var company = await context.Companies.OrderBy(c => c.Id).FirstAsync(); + Assert.NotNull(company.Department); + Assert.Equal("Replacement Department", company.Department.Name); + Assert.Equal(99999.99m, company.Department.Budget); + } }); protected virtual void UseTransaction(DatabaseFacade facade, IDbContextTransaction transaction) - { - } + => facade.UseTransaction(transaction.GetDbTransaction()); protected virtual void ClearLog() - { - } + => Fixture.TestSqlLoggerFactory.Clear(); + + protected virtual void AssertSql(params string[] expected) + => Fixture.TestSqlLoggerFactory.AssertBaseline(expected); + + protected virtual IDisposable SuspendRecordingEvents() + => Fixture.TestSqlLoggerFactory.SuspendRecordingEvents(); protected class ComplexCollectionJsonContext(DbContextOptions options) : DbContext(options) { @@ -578,6 +634,9 @@ protected override string StoreName public TestSqlLoggerFactory TestSqlLoggerFactory => (TestSqlLoggerFactory)ListLoggerFactory; + protected override bool ShouldLogCategory(string logCategory) + => logCategory == DbLoggerCategory.Update.Name; + protected override Type ContextType => typeof(ComplexCollectionJsonContext); protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext context) @@ -615,30 +674,30 @@ protected override Task SeedAsync(DbContext context) Contacts = [ new Contact - { - Name = "First Contact", - PhoneNumbers = ["555-1234", "555-5678"] - }, - new Contact - { - Name = "Second Contact", - PhoneNumbers = ["555-9876", "555-5432"] - } + { + Name = "First Contact", + PhoneNumbers = ["555-1234", "555-5678"] + }, + new Contact + { + Name = "Second Contact", + PhoneNumbers = ["555-9876", "555-5432"] + } ], Employees = [ new Employee - { - Name = "Initial Employee", - PhoneNumbers = ["555-0001"], - Address = new Address { - Street = "100 First St", - City = "Initial City", - PostalCode = "00001", - Country = "USA" + Name = "Initial Employee", + PhoneNumbers = ["555-0001"], + Address = new Address + { + Street = "100 First St", + City = "Initial City", + PostalCode = "00001", + Country = "USA" + } } - } ], Department = new Department { @@ -651,5 +710,4 @@ protected override Task SeedAsync(DbContext context) return context.SaveChangesAsync(); } } - } diff --git a/test/EFCore.Specification.Tests/ModelBuilding/ModelBuilderTest.ComplexCollections.cs b/test/EFCore.Specification.Tests/ModelBuilding/ModelBuilderTest.ComplexCollections.cs index 42bcca12e1c..5e1203ad118 100644 --- a/test/EFCore.Specification.Tests/ModelBuilding/ModelBuilderTest.ComplexCollections.cs +++ b/test/EFCore.Specification.Tests/ModelBuilding/ModelBuilderTest.ComplexCollections.cs @@ -281,14 +281,17 @@ public virtual void Properties_can_be_made_optional() e => e.QuarksCollection, b => { + b.IsRequired(false); b.Property(e => e.Down).IsRequired(false); b.Property("Strange").IsRequired(false); b.Property("Bottom").IsRequired(false); }); var model = modelBuilder.FinalizeModel(); - var complexType = model.FindEntityType(typeof(ComplexProperties)).GetComplexProperties().Single().ComplexType; + var complexProperty = model.FindEntityType(typeof(ComplexProperties)).GetComplexProperties().Single(); + Assert.True(complexProperty.IsNullable); + var complexType = complexProperty.ComplexType; Assert.True(complexType.FindProperty("Down").IsNullable); Assert.True(complexType.FindProperty("Strange").IsNullable); Assert.True(complexType.FindProperty("Bottom").IsNullable); diff --git a/test/EFCore.Specification.Tests/ModelBuilding/ModelBuilderTest.Generic.cs b/test/EFCore.Specification.Tests/ModelBuilding/ModelBuilderTest.Generic.cs index 14b851378f9..332ec7fcbcd 100644 --- a/test/EFCore.Specification.Tests/ModelBuilding/ModelBuilderTest.Generic.cs +++ b/test/EFCore.Specification.Tests/ModelBuilding/ModelBuilderTest.Generic.cs @@ -714,6 +714,9 @@ public override TestComplexCollectionBuilder Ignore(Expression Ignore(string propertyName) => Wrap(PropertyBuilder.Ignore(propertyName)); + public override TestComplexCollectionBuilder IsRequired(bool isRequired = true) + => Wrap(PropertyBuilder.IsRequired(isRequired)); + public override TestComplexCollectionBuilder HasChangeTrackingStrategy(ChangeTrackingStrategy changeTrackingStrategy) => Wrap(PropertyBuilder.HasChangeTrackingStrategy(changeTrackingStrategy)); diff --git a/test/EFCore.Specification.Tests/ModelBuilding/ModelBuilderTest.NonGeneric.cs b/test/EFCore.Specification.Tests/ModelBuilding/ModelBuilderTest.NonGeneric.cs index e8e68b1c280..b4e5b0d82af 100644 --- a/test/EFCore.Specification.Tests/ModelBuilding/ModelBuilderTest.NonGeneric.cs +++ b/test/EFCore.Specification.Tests/ModelBuilding/ModelBuilderTest.NonGeneric.cs @@ -811,6 +811,9 @@ public override TestComplexCollectionBuilder Ignore(Expression Ignore(string propertyName) => Wrap(PropertyBuilder.Ignore(propertyName)); + public override TestComplexCollectionBuilder IsRequired(bool isRequired = true) + => Wrap(PropertyBuilder.IsRequired(isRequired)); + public override TestComplexCollectionBuilder HasChangeTrackingStrategy(ChangeTrackingStrategy changeTrackingStrategy) => Wrap(PropertyBuilder.HasChangeTrackingStrategy(changeTrackingStrategy)); diff --git a/test/EFCore.Specification.Tests/ModelBuilding/ModelBuilderTest.cs b/test/EFCore.Specification.Tests/ModelBuilding/ModelBuilderTest.cs index 26930625c46..5ca120eb6fd 100644 --- a/test/EFCore.Specification.Tests/ModelBuilding/ModelBuilderTest.cs +++ b/test/EFCore.Specification.Tests/ModelBuilding/ModelBuilderTest.cs @@ -574,6 +574,7 @@ public abstract TestComplexCollectionBuilder Ignore( Expression> propertyExpression); public abstract TestComplexCollectionBuilder Ignore(string propertyName); + public abstract TestComplexCollectionBuilder IsRequired(bool isRequired = true); public abstract TestComplexCollectionBuilder HasChangeTrackingStrategy(ChangeTrackingStrategy changeTrackingStrategy); public abstract TestComplexCollectionBuilder UsePropertyAccessMode(PropertyAccessMode propertyAccessMode); public abstract TestComplexCollectionBuilder UseDefaultPropertyAccessMode(PropertyAccessMode propertyAccessMode); diff --git a/test/EFCore.Specification.Tests/Scaffolding/CompiledModelTestBase.cs b/test/EFCore.Specification.Tests/Scaffolding/CompiledModelTestBase.cs index d5f1f603cd8..869ff2acc69 100644 --- a/test/EFCore.Specification.Tests/Scaffolding/CompiledModelTestBase.cs +++ b/test/EFCore.Specification.Tests/Scaffolding/CompiledModelTestBase.cs @@ -1848,7 +1848,7 @@ public class PrincipalDerived : PrincipalBase where TDependent : class { public TDependent? Dependent { get; set; } - protected IList ManyOwned = null!; + protected IList ManyOwned = []; public ICollection Principals { get; set; } = null!; } diff --git a/test/EFCore.SqlServer.FunctionalTests/SqlServerComplianceTest.cs b/test/EFCore.SqlServer.FunctionalTests/SqlServerComplianceTest.cs index c0758fde4f3..97609057392 100644 --- a/test/EFCore.SqlServer.FunctionalTests/SqlServerComplianceTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/SqlServerComplianceTest.cs @@ -10,7 +10,6 @@ public class SqlServerComplianceTest : RelationalComplianceTestBase { protected override ICollection IgnoredTestBases => new HashSet { - typeof(ComplexCollectionJsonUpdateTestBase<>) // issue #31252 }; protected override Assembly TargetAssembly { get; } = typeof(SqlServerComplianceTest).Assembly; diff --git a/test/EFCore.SqlServer.FunctionalTests/Update/ComplexCollectionJsonUpdateSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Update/ComplexCollectionJsonUpdateSqlServerTest.cs index 6ca6fd33154..8fc1b5fb1a8 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Update/ComplexCollectionJsonUpdateSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Update/ComplexCollectionJsonUpdateSqlServerTest.cs @@ -3,8 +3,7 @@ namespace Microsoft.EntityFrameworkCore.Update; -// TODO: Requires query support for complex collections mapped to JSON, issue #31252 -public abstract class ComplexCollectionJsonUpdateSqlServerTest : ComplexCollectionJsonUpdateTestBase +public class ComplexCollectionJsonUpdateSqlServerTest : ComplexCollectionJsonUpdateTestBase { public ComplexCollectionJsonUpdateSqlServerTest(ComplexCollectionJsonUpdateSqlServerFixture fixture) : base(fixture) @@ -16,12 +15,12 @@ public override async Task Add_element_to_complex_collection_mapped_to_json() AssertSql( """ -@p0='[{"Name":"First Contact","PhoneNumbers":["555-1234","555-5678"]},{"Name":"Second Contact","PhoneNumbers":["555-9876","555-5432"]},{"Name":"New Contact","PhoneNumbers":["555-0000"]}]' (Nullable = false) (Size = 200) +@p0='[{"Name":"First Contact","PhoneNumbers":["555-1234","555-5678"]},{"Name":"Second Contact","PhoneNumbers":["555-9876","555-5432"]},{"Name":"New Contact","PhoneNumbers":["555-0000"]}]' (Nullable = false) (Size = 181) @p1='1' SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; -UPDATE [CompanyWithComplexCollections] SET [Contacts] = @p0 +UPDATE [Companies] SET [Contacts] = @p0 OUTPUT 1 WHERE [Id] = @p1; """); @@ -33,14 +32,16 @@ public override async Task Remove_element_from_complex_collection_mapped_to_json AssertSql( """ -@p0='[{"Name":"Second Contact","PhoneNumbers":["555-9876","555-5432"]}]' (Nullable = false) (Size = 65) -@p1='1' +@p0='[{"Name":"Second Contact","PhoneNumbers":["555-9876","555-5432"]}]' (Nullable = false) (Size = 66) +@p1='{"Budget":10000.00,"Name":"Initial Department"}' (Nullable = false) (Size = 47) +@p2='[{"Name":"Initial Employee","PhoneNumbers":["555-0001"],"Address":{"City":"Initial City","Country":"USA","PostalCode":"00001","Street":"100 First St"}}]' (Nullable = false) (Size = 152) +@p3='1' SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; -UPDATE [CompanyWithComplexCollections] SET [Contacts] = @p0 +UPDATE [Companies] SET [Contacts] = @p0, [Department] = @p1, [Employees] = @p2 OUTPUT 1 -WHERE [Id] = @p1; +WHERE [Id] = @p3; """); } @@ -51,13 +52,15 @@ public override async Task Modify_element_in_complex_collection_mapped_to_json() AssertSql( """ @p0='[{"Name":"First Contact - Modified","PhoneNumbers":["555-1234","555-5678"]},{"Name":"Second Contact","PhoneNumbers":["555-9876","555-5432"]}]' (Nullable = false) (Size = 141) -@p1='1' +@p1='{"Budget":10000.00,"Name":"Initial Department"}' (Nullable = false) (Size = 47) +@p2='[{"Name":"Initial Employee","PhoneNumbers":["555-0001"],"Address":{"City":"Initial City","Country":"USA","PostalCode":"00001","Street":"100 First St"}}]' (Nullable = false) (Size = 152) +@p3='1' SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; -UPDATE [CompanyWithComplexCollections] SET [Contacts] = @p0 +UPDATE [Companies] SET [Contacts] = @p0, [Department] = @p1, [Employees] = @p2 OUTPUT 1 -WHERE [Id] = @p1; +WHERE [Id] = @p3; """); } @@ -67,125 +70,285 @@ public override async Task Move_elements_in_complex_collection_mapped_to_json() AssertSql( """ -@p0='[{"Name":"Second Contact","PhoneNumbers":["555-9876","555-5432"]},{"Name":"First Contact","PhoneNumbers":["555-1234","555-5678"]}]' (Nullable = false) (Size = 141) -@p1='1' +@p0='[{"Name":"Second Contact","PhoneNumbers":["555-9876","555-5432"]},{"Name":"First Contact","PhoneNumbers":["555-1234","555-5678"]}]' (Nullable = false) (Size = 130) +@p1='{"Budget":10000.00,"Name":"Initial Department"}' (Nullable = false) (Size = 47) +@p2='[{"Name":"Initial Employee","PhoneNumbers":["555-0001"],"Address":{"City":"Initial City","Country":"USA","PostalCode":"00001","Street":"100 First St"}}]' (Nullable = false) (Size = 152) +@p3='1' SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; -UPDATE [CompanyWithComplexCollections] SET [Contacts] = @p0 +UPDATE [Companies] SET [Contacts] = @p0, [Department] = @p1, [Employees] = @p2 OUTPUT 1 -WHERE [Id] = @p1; +WHERE [Id] = @p3; """); } - public override async Task Change_empty_complex_collection_to_null_mapped_to_json() + public override async Task Change_complex_collection_mapped_to_json_to_null_and_to_empty() { - await base.Change_empty_complex_collection_to_null_mapped_to_json(); + await base.Change_complex_collection_mapped_to_json_to_null_and_to_empty(); AssertSql( """ -@p0=NULL (Size = 4000) +@p0='[]' (Nullable = false) (Size = 2) @p1='1' SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; -UPDATE [CompanyWithComplexCollections] SET [Contacts] = @p0 +UPDATE [Companies] SET [Contacts] = @p0 OUTPUT 1 WHERE [Id] = @p1; +""", + // + """ +@p0=NULL (Nullable = false) +@p1='1' + +SET IMPLICIT_TRANSACTIONS OFF; +SET NOCOUNT ON; +UPDATE [Companies] SET [Contacts] = @p0 +OUTPUT 1 +WHERE [Id] = @p1; +"""); + } + + public override async Task Complex_collection_with_nested_complex_type_mapped_to_json() + { + await base.Complex_collection_with_nested_complex_type_mapped_to_json(); + + AssertSql( + """ +@p0='[{"Name":"First Contact","PhoneNumbers":["555-1234","555-5678"]},{"Name":"Second Contact","PhoneNumbers":["555-9876","555-5432"]}]' (Nullable = false) (Size = 130) +@p1='{"Budget":10000.00,"Name":"Initial Department"}' (Nullable = false) (Size = 47) +@p2='[{"Name":"John Doe","PhoneNumbers":["555-1234","555-5678"],"Address":{"City":"Seattle","Country":"USA","PostalCode":"98101","Street":"123 Main St"}},{"Name":"Jane Smith","PhoneNumbers":["555-9876"],"Address":{"City":"Portland","Country":"USA","PostalCode":"97201","Street":"456 Oak Ave"}}]' (Nullable = false) (Size = 289) +@p3='1' + +SET IMPLICIT_TRANSACTIONS OFF; +SET NOCOUNT ON; +UPDATE [Companies] SET [Contacts] = @p0, [Department] = @p1, [Employees] = @p2 +OUTPUT 1 +WHERE [Id] = @p3; """); } - public override async Task Change_null_complex_collection_to_empty_mapped_to_json() + public override async Task Modify_multiple_complex_properties_mapped_to_json() { - await base.Change_null_complex_collection_to_empty_mapped_to_json(); + await base.Modify_multiple_complex_properties_mapped_to_json(); + + AssertSql( + """ +@p0='[{"Name":"Contact 1","PhoneNumbers":["555-1111"]}]' (Nullable = false) (Size = 50) +@p1='{"Budget":50000.00,"Name":"Department A"}' (Nullable = false) (Size = 41) +@p2='[{"Name":"Initial Employee","PhoneNumbers":["555-0001"],"Address":{"City":"Initial City","Country":"USA","PostalCode":"00001","Street":"100 First St"}}]' (Nullable = false) (Size = 152) +@p3='1' + +SET IMPLICIT_TRANSACTIONS OFF; +SET NOCOUNT ON; +UPDATE [Companies] SET [Contacts] = @p0, [Department] = @p1, [Employees] = @p2 +OUTPUT 1 +WHERE [Id] = @p3; +"""); + } + + public override async Task Clear_complex_collection_mapped_to_json() + { + await base.Clear_complex_collection_mapped_to_json(); AssertSql( """ @p0='[]' (Nullable = false) (Size = 2) +@p1='{"Budget":10000.00,"Name":"Initial Department"}' (Nullable = false) (Size = 47) +@p2='[{"Name":"Initial Employee","PhoneNumbers":["555-0001"],"Address":{"City":"Initial City","Country":"USA","PostalCode":"00001","Street":"100 First St"}}]' (Nullable = false) (Size = 152) +@p3='1' + +SET IMPLICIT_TRANSACTIONS OFF; +SET NOCOUNT ON; +UPDATE [Companies] SET [Contacts] = @p0, [Department] = @p1, [Employees] = @p2 +OUTPUT 1 +WHERE [Id] = @p3; +"""); + } + + public override async Task Replace_entire_complex_collection_mapped_to_json() + { + await base.Replace_entire_complex_collection_mapped_to_json(); + + AssertSql( + """ +@p0='[{"Name":"Replacement Contact 1","PhoneNumbers":["999-1111"]},{"Name":"Replacement Contact 2","PhoneNumbers":["999-2222","999-3333"]}]' (Nullable = false) (Size = 134) +@p1='{"Budget":10000.00,"Name":"Initial Department"}' (Nullable = false) (Size = 47) +@p2='[{"Name":"Initial Employee","PhoneNumbers":["555-0001"],"Address":{"City":"Initial City","Country":"USA","PostalCode":"00001","Street":"100 First St"}}]' (Nullable = false) (Size = 152) +@p3='1' + +SET IMPLICIT_TRANSACTIONS OFF; +SET NOCOUNT ON; +UPDATE [Companies] SET [Contacts] = @p0, [Department] = @p1, [Employees] = @p2 +OUTPUT 1 +WHERE [Id] = @p3; +"""); + } + + public override async Task Add_element_to_nested_complex_collection_mapped_to_json() + { + await base.Add_element_to_nested_complex_collection_mapped_to_json(); + + AssertSql( + """ +@p0='[{"Name":"First Contact","PhoneNumbers":["555-1234","555-5678"]},{"Name":"Second Contact","PhoneNumbers":["555-9876","555-5432"]}]' (Nullable = false) (Size = 130) +@p1='{"Budget":10000.00,"Name":"Initial Department"}' (Nullable = false) (Size = 47) +@p2='[{"Name":"Initial Employee","PhoneNumbers":["555-0001","555-9999"],"Address":{"City":"Initial City","Country":"USA","PostalCode":"00001","Street":"100 First St"}}]' (Nullable = false) (Size = 163) +@p3='1' + +SET IMPLICIT_TRANSACTIONS OFF; +SET NOCOUNT ON; +UPDATE [Companies] SET [Contacts] = @p0, [Department] = @p1, [Employees] = @p2 +OUTPUT 1 +WHERE [Id] = @p3; +"""); + } + + public override async Task Modify_nested_complex_property_in_complex_collection_mapped_to_json() + { + await base.Modify_nested_complex_property_in_complex_collection_mapped_to_json(); + + AssertSql( + """ +@p0='[{"Name":"First Contact","PhoneNumbers":["555-1234","555-5678"]},{"Name":"Second Contact","PhoneNumbers":["555-9876","555-5432"]}]' (Nullable = false) (Size = 130) +@p1='{"Budget":10000.00,"Name":"Initial Department"}' (Nullable = false) (Size = 47) +@p2='[{"Name":"Initial Employee","PhoneNumbers":["555-0001"],"Address":{"City":"Modified City","Country":"USA","PostalCode":"99999","Street":"100 First St"}}]' (Nullable = false) (Size = 153) +@p3='1' + +SET IMPLICIT_TRANSACTIONS OFF; +SET NOCOUNT ON; +UPDATE [Companies] SET [Contacts] = @p0, [Department] = @p1, [Employees] = @p2 +OUTPUT 1 +WHERE [Id] = @p3; +"""); + } + + public override async Task Set_complex_collection_to_null_mapped_to_json() + { + await base.Set_complex_collection_to_null_mapped_to_json(); + + AssertSql( + """ +@p0=NULL (Nullable = false) @p1='1' SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; -UPDATE [CompanyWithComplexCollections] SET [Contacts] = @p0 +UPDATE [Companies] SET [Employees] = @p0 OUTPUT 1 WHERE [Id] = @p1; """); } - public override async Task Complex_collection_with_nested_complex_type_mapped_to_json() + public override async Task Set_null_complex_collection_to_non_empty_mapped_to_json() { - await base.Complex_collection_with_nested_complex_type_mapped_to_json(); + await base.Set_null_complex_collection_to_non_empty_mapped_to_json(); AssertSql( """ -@p0='[{"Address":{"City":"Seattle","Country":"USA","PostalCode":"98101","Street":"123 Main St"},"Name":"John Doe","PhoneNumbers":["555-1234","555-5678"]},{"Address":{"City":"Portland","Country":"USA","PostalCode":"97201","Street":"456 Oak Ave"},"Name":"Jane Smith","PhoneNumbers":["555-9876"]}]' (Nullable = false) (Size = 320) +@p0='[{"Name":"New Employee","PhoneNumbers":["555-1111"],"Address":{"City":"New City","Country":"USA","PostalCode":"12345","Street":"123 New St"}}]' (Nullable = false) (Size = 142) @p1='1' SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; -UPDATE [CompanyWithComplexCollections] SET [Employees] = @p0 +UPDATE [Companies] SET [Employees] = @p0 OUTPUT 1 WHERE [Id] = @p1; """); } - public override async Task Modify_multiple_complex_properties_mapped_to_json() + public override async Task Replace_complex_collection_element_mapped_to_json() { - await base.Modify_multiple_complex_properties_mapped_to_json(); + await base.Replace_complex_collection_element_mapped_to_json(); AssertSql( """ -@p0='[{"Name":"Contact 1","PhoneNumbers":["555-1111"]}]' (Nullable = false) (Size = 51) -@p1='[{"Name":"Department A","Budget":50000.00}]' (Nullable = false) (Size = 44) -@p2='1' +@p0='[{"Name":"First Contact","PhoneNumbers":["555-1234","555-5678"]},{"Name":"Second Contact","PhoneNumbers":["555-9876","555-5432"]}]' (Nullable = false) (Size = 130) +@p1='{"Budget":10000.00,"Name":"Initial Department"}' (Nullable = false) (Size = 47) +@p2='[{"Name":"Replacement Employee","PhoneNumbers":["555-7777","555-8888"],"Address":{"City":"Replace City","Country":"Canada","PostalCode":"54321","Street":"789 Replace St"}}]' (Nullable = false) (Size = 172) +@p3='1' SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; -UPDATE [CompanyWithComplexCollections] SET [Contacts] = @p0, [Departments] = @p1 +UPDATE [Companies] SET [Contacts] = @p0, [Department] = @p1, [Employees] = @p2 OUTPUT 1 -WHERE [Id] = @p2; +WHERE [Id] = @p3; """); } - public override async Task Clear_complex_collection_mapped_to_json() + public override async Task Complex_collection_with_empty_nested_collections_mapped_to_json() { - await base.Clear_complex_collection_mapped_to_json(); + await base.Complex_collection_with_empty_nested_collections_mapped_to_json(); AssertSql( """ -@p0='[]' (Nullable = false) (Size = 2) +@p0='[{"Name":"First Contact","PhoneNumbers":["555-1234","555-5678"]},{"Name":"Second Contact","PhoneNumbers":["555-9876","555-5432"]}]' (Nullable = false) (Size = 130) +@p1='{"Budget":10000.00,"Name":"Initial Department"}' (Nullable = false) (Size = 47) +@p2='[{"Name":"Initial Employee","PhoneNumbers":["555-0001"],"Address":{"City":"Initial City","Country":"USA","PostalCode":"00001","Street":"100 First St"}},{"Name":"Employee No Phone","PhoneNumbers":[],"Address":{"City":"Quiet City","Country":"USA","PostalCode":"00000","Street":"456 No Phone St"}}]' (Nullable = false) (Size = 295) +@p3='1' + +SET IMPLICIT_TRANSACTIONS OFF; +SET NOCOUNT ON; +UPDATE [Companies] SET [Contacts] = @p0, [Department] = @p1, [Employees] = @p2 +OUTPUT 1 +WHERE [Id] = @p3; +"""); + } + + public override async Task Set_complex_property_mapped_to_json_to_null() + { + await base.Set_complex_property_mapped_to_json_to_null(); + + AssertSql( + """ +@p0=NULL (Nullable = false) @p1='1' SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; -UPDATE [CompanyWithComplexCollections] SET [Contacts] = @p0 +UPDATE [Companies] SET [Department] = @p0 OUTPUT 1 WHERE [Id] = @p1; """); } - public override async Task Replace_entire_complex_collection_mapped_to_json() + public override async Task Set_null_complex_property_to_non_null_mapped_to_json() { - await base.Replace_entire_complex_collection_mapped_to_json(); + await base.Set_null_complex_property_to_non_null_mapped_to_json(); AssertSql( """ -@p0='[{"Name":"Replacement Contact 1","PhoneNumbers":["999-1111"]},{"Name":"Replacement Contact 2","PhoneNumbers":["999-2222","999-3333"]}]' (Nullable = false) (Size = 144) +@p0='{"Budget":25000.00,"Name":"New Department"}' (Nullable = false) (Size = 43) @p1='1' SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; -UPDATE [CompanyWithComplexCollections] SET [Contacts] = @p0 +UPDATE [Companies] SET [Department] = @p0 OUTPUT 1 WHERE [Id] = @p1; """); } - private void AssertSql(params string[] expected) - => Fixture.TestSqlLoggerFactory.AssertBaseline(expected); + public override async Task Replace_complex_property_mapped_to_json() + { + await base.Replace_complex_property_mapped_to_json(); + + AssertSql( + """ +@p0='[{"Name":"First Contact","PhoneNumbers":["555-1234","555-5678"]},{"Name":"Second Contact","PhoneNumbers":["555-9876","555-5432"]}]' (Nullable = false) (Size = 130) +@p1='{"Budget":99999.99,"Name":"Replacement Department"}' (Nullable = false) (Size = 51) +@p2='[{"Name":"Initial Employee","PhoneNumbers":["555-0001"],"Address":{"City":"Initial City","Country":"USA","PostalCode":"00001","Street":"100 First St"}}]' (Nullable = false) (Size = 152) +@p3='1' - protected override void ClearLog() - => Fixture.TestSqlLoggerFactory.Clear(); +SET IMPLICIT_TRANSACTIONS OFF; +SET NOCOUNT ON; +UPDATE [Companies] SET [Contacts] = @p0, [Department] = @p1, [Employees] = @p2 +OUTPUT 1 +WHERE [Id] = @p3; +"""); + } public class ComplexCollectionJsonUpdateSqlServerFixture : ComplexCollectionJsonUpdateFixtureBase { diff --git a/test/EFCore.Sqlite.FunctionalTests/SqliteComplianceTest.cs b/test/EFCore.Sqlite.FunctionalTests/SqliteComplianceTest.cs index 16c5458b672..1fec3cd333e 100644 --- a/test/EFCore.Sqlite.FunctionalTests/SqliteComplianceTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/SqliteComplianceTest.cs @@ -24,8 +24,7 @@ public class SqliteComplianceTest : RelationalComplianceTestBase typeof(OwnedNavigationsProjectionTestBase<>), typeof(OwnedNavigationsProjectionRelationalTestBase<>), typeof(OwnedJsonProjectionRelationalTestBase<>), - typeof(OwnedTableSplittingProjectionRelationalTestBase<>), - typeof(ComplexCollectionJsonUpdateTestBase<>) // issue #31252 + typeof(OwnedTableSplittingProjectionRelationalTestBase<>) }; protected override Assembly TargetAssembly { get; } = typeof(SqliteComplianceTest).Assembly; diff --git a/test/EFCore.Sqlite.FunctionalTests/Update/ComplexCollectionJsonUpdateSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Update/ComplexCollectionJsonUpdateSqliteTest.cs index 6a43bb8d4a9..db11e377884 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Update/ComplexCollectionJsonUpdateSqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Update/ComplexCollectionJsonUpdateSqliteTest.cs @@ -3,8 +3,7 @@ namespace Microsoft.EntityFrameworkCore.Update; -// TODO: Requires query support for complex collections mapped to JSON, issue #31252 -public abstract class ComplexCollectionJsonUpdateSqliteTest : ComplexCollectionJsonUpdateTestBase +public class ComplexCollectionJsonUpdateSqliteTest : ComplexCollectionJsonUpdateTestBase { public ComplexCollectionJsonUpdateSqliteTest(ComplexCollectionJsonUpdateSqliteFixture fixture) : base(fixture) @@ -16,20 +15,302 @@ public override async Task Add_element_to_complex_collection_mapped_to_json() AssertSql( """ -@p0='[{"Name":"First Contact","PhoneNumbers":["555-1234","555-5678"]},{"Name":"Second Contact","PhoneNumbers":["555-9876","555-5432"]},{"Name":"New Contact","PhoneNumbers":["555-0000"]}]' (Size = 200) -@p1='1' (DbType = String) +@p0='[{"Name":"First Contact","PhoneNumbers":["555-1234","555-5678"]},{"Name":"Second Contact","PhoneNumbers":["555-9876","555-5432"]},{"Name":"New Contact","PhoneNumbers":["555-0000"]}]' (Nullable = false) (Size = 181) +@p1='1' -UPDATE "CompanyWithComplexCollections" SET "Contacts" = @p0 +UPDATE "Companies" SET "Contacts" = @p0 WHERE "Id" = @p1 RETURNING 1; """); } - private void AssertSql(params string[] expected) - => Fixture.TestSqlLoggerFactory.AssertBaseline(expected); + public override async Task Remove_element_from_complex_collection_mapped_to_json() + { + await base.Remove_element_from_complex_collection_mapped_to_json(); + + AssertSql( + """ +@p0='[{"Name":"Second Contact","PhoneNumbers":["555-9876","555-5432"]}]' (Nullable = false) (Size = 66) +@p1='{"Budget":"10000.0","Name":"Initial Department"}' (Nullable = false) (Size = 48) +@p2='[{"Name":"Initial Employee","PhoneNumbers":["555-0001"],"Address":{"City":"Initial City","Country":"USA","PostalCode":"00001","Street":"100 First St"}}]' (Nullable = false) (Size = 152) +@p3='1' + +UPDATE "Companies" SET "Contacts" = @p0, "Department" = @p1, "Employees" = @p2 +WHERE "Id" = @p3 +RETURNING 1; +"""); + } + + public override async Task Modify_element_in_complex_collection_mapped_to_json() + { + await base.Modify_element_in_complex_collection_mapped_to_json(); + + AssertSql( + """ +@p0='[{"Name":"First Contact - Modified","PhoneNumbers":["555-1234","555-5678"]},{"Name":"Second Contact","PhoneNumbers":["555-9876","555-5432"]}]' (Nullable = false) (Size = 141) +@p1='{"Budget":"10000.0","Name":"Initial Department"}' (Nullable = false) (Size = 48) +@p2='[{"Name":"Initial Employee","PhoneNumbers":["555-0001"],"Address":{"City":"Initial City","Country":"USA","PostalCode":"00001","Street":"100 First St"}}]' (Nullable = false) (Size = 152) +@p3='1' + +UPDATE "Companies" SET "Contacts" = @p0, "Department" = @p1, "Employees" = @p2 +WHERE "Id" = @p3 +RETURNING 1; +"""); + } + + public override async Task Move_elements_in_complex_collection_mapped_to_json() + { + await base.Move_elements_in_complex_collection_mapped_to_json(); + + AssertSql( + """ +@p0='[{"Name":"Second Contact","PhoneNumbers":["555-9876","555-5432"]},{"Name":"First Contact","PhoneNumbers":["555-1234","555-5678"]}]' (Nullable = false) (Size = 130) +@p1='{"Budget":"10000.0","Name":"Initial Department"}' (Nullable = false) (Size = 48) +@p2='[{"Name":"Initial Employee","PhoneNumbers":["555-0001"],"Address":{"City":"Initial City","Country":"USA","PostalCode":"00001","Street":"100 First St"}}]' (Nullable = false) (Size = 152) +@p3='1' + +UPDATE "Companies" SET "Contacts" = @p0, "Department" = @p1, "Employees" = @p2 +WHERE "Id" = @p3 +RETURNING 1; +"""); + } + + public override async Task Change_complex_collection_mapped_to_json_to_null_and_to_empty() + { + await base.Change_complex_collection_mapped_to_json_to_null_and_to_empty(); + + AssertSql( + """ +@p0='[]' (Nullable = false) (Size = 2) +@p1='1' + +UPDATE "Companies" SET "Contacts" = @p0 +WHERE "Id" = @p1 +RETURNING 1; +""", + // + """ +@p0=NULL (Nullable = false) +@p1='1' + +UPDATE "Companies" SET "Contacts" = @p0 +WHERE "Id" = @p1 +RETURNING 1; +"""); + } + + public override async Task Complex_collection_with_nested_complex_type_mapped_to_json() + { + await base.Complex_collection_with_nested_complex_type_mapped_to_json(); + + AssertSql( + """ +@p0='[{"Name":"First Contact","PhoneNumbers":["555-1234","555-5678"]},{"Name":"Second Contact","PhoneNumbers":["555-9876","555-5432"]}]' (Nullable = false) (Size = 130) +@p1='{"Budget":"10000.0","Name":"Initial Department"}' (Nullable = false) (Size = 48) +@p2='[{"Name":"John Doe","PhoneNumbers":["555-1234","555-5678"],"Address":{"City":"Seattle","Country":"USA","PostalCode":"98101","Street":"123 Main St"}},{"Name":"Jane Smith","PhoneNumbers":["555-9876"],"Address":{"City":"Portland","Country":"USA","PostalCode":"97201","Street":"456 Oak Ave"}}]' (Nullable = false) (Size = 289) +@p3='1' + +UPDATE "Companies" SET "Contacts" = @p0, "Department" = @p1, "Employees" = @p2 +WHERE "Id" = @p3 +RETURNING 1; +"""); + } + + public override async Task Modify_multiple_complex_properties_mapped_to_json() + { + await base.Modify_multiple_complex_properties_mapped_to_json(); + + AssertSql( + """ +@p0='[{"Name":"Contact 1","PhoneNumbers":["555-1111"]}]' (Nullable = false) (Size = 50) +@p1='{"Budget":"50000.0","Name":"Department A"}' (Nullable = false) (Size = 42) +@p2='[{"Name":"Initial Employee","PhoneNumbers":["555-0001"],"Address":{"City":"Initial City","Country":"USA","PostalCode":"00001","Street":"100 First St"}}]' (Nullable = false) (Size = 152) +@p3='1' + +UPDATE "Companies" SET "Contacts" = @p0, "Department" = @p1, "Employees" = @p2 +WHERE "Id" = @p3 +RETURNING 1; +"""); + } + + public override async Task Clear_complex_collection_mapped_to_json() + { + await base.Clear_complex_collection_mapped_to_json(); + + AssertSql( + """ +@p0='[]' (Nullable = false) (Size = 2) +@p1='{"Budget":"10000.0","Name":"Initial Department"}' (Nullable = false) (Size = 48) +@p2='[{"Name":"Initial Employee","PhoneNumbers":["555-0001"],"Address":{"City":"Initial City","Country":"USA","PostalCode":"00001","Street":"100 First St"}}]' (Nullable = false) (Size = 152) +@p3='1' + +UPDATE "Companies" SET "Contacts" = @p0, "Department" = @p1, "Employees" = @p2 +WHERE "Id" = @p3 +RETURNING 1; +"""); + } + + public override async Task Replace_entire_complex_collection_mapped_to_json() + { + await base.Replace_entire_complex_collection_mapped_to_json(); + + AssertSql( + """ +@p0='[{"Name":"Replacement Contact 1","PhoneNumbers":["999-1111"]},{"Name":"Replacement Contact 2","PhoneNumbers":["999-2222","999-3333"]}]' (Nullable = false) (Size = 134) +@p1='{"Budget":"10000.0","Name":"Initial Department"}' (Nullable = false) (Size = 48) +@p2='[{"Name":"Initial Employee","PhoneNumbers":["555-0001"],"Address":{"City":"Initial City","Country":"USA","PostalCode":"00001","Street":"100 First St"}}]' (Nullable = false) (Size = 152) +@p3='1' + +UPDATE "Companies" SET "Contacts" = @p0, "Department" = @p1, "Employees" = @p2 +WHERE "Id" = @p3 +RETURNING 1; +"""); + } + + public override async Task Add_element_to_nested_complex_collection_mapped_to_json() + { + await base.Add_element_to_nested_complex_collection_mapped_to_json(); + + AssertSql( + """ +@p0='[{"Name":"First Contact","PhoneNumbers":["555-1234","555-5678"]},{"Name":"Second Contact","PhoneNumbers":["555-9876","555-5432"]}]' (Nullable = false) (Size = 130) +@p1='{"Budget":"10000.0","Name":"Initial Department"}' (Nullable = false) (Size = 48) +@p2='[{"Name":"Initial Employee","PhoneNumbers":["555-0001","555-9999"],"Address":{"City":"Initial City","Country":"USA","PostalCode":"00001","Street":"100 First St"}}]' (Nullable = false) (Size = 163) +@p3='1' + +UPDATE "Companies" SET "Contacts" = @p0, "Department" = @p1, "Employees" = @p2 +WHERE "Id" = @p3 +RETURNING 1; +"""); + } + + public override async Task Modify_nested_complex_property_in_complex_collection_mapped_to_json() + { + await base.Modify_nested_complex_property_in_complex_collection_mapped_to_json(); + + AssertSql( + """ +@p0='[{"Name":"First Contact","PhoneNumbers":["555-1234","555-5678"]},{"Name":"Second Contact","PhoneNumbers":["555-9876","555-5432"]}]' (Nullable = false) (Size = 130) +@p1='{"Budget":"10000.0","Name":"Initial Department"}' (Nullable = false) (Size = 48) +@p2='[{"Name":"Initial Employee","PhoneNumbers":["555-0001"],"Address":{"City":"Modified City","Country":"USA","PostalCode":"99999","Street":"100 First St"}}]' (Nullable = false) (Size = 153) +@p3='1' + +UPDATE "Companies" SET "Contacts" = @p0, "Department" = @p1, "Employees" = @p2 +WHERE "Id" = @p3 +RETURNING 1; +"""); + } + + public override async Task Set_complex_collection_to_null_mapped_to_json() + { + await base.Set_complex_collection_to_null_mapped_to_json(); + + AssertSql( + """ +@p0=NULL (Nullable = false) +@p1='1' + +UPDATE "Companies" SET "Employees" = @p0 +WHERE "Id" = @p1 +RETURNING 1; +"""); + } + + public override async Task Set_null_complex_collection_to_non_empty_mapped_to_json() + { + await base.Set_null_complex_collection_to_non_empty_mapped_to_json(); + + AssertSql( + """ +@p0='[{"Name":"New Employee","PhoneNumbers":["555-1111"],"Address":{"City":"New City","Country":"USA","PostalCode":"12345","Street":"123 New St"}}]' (Nullable = false) (Size = 142) +@p1='1' + +UPDATE "Companies" SET "Employees" = @p0 +WHERE "Id" = @p1 +RETURNING 1; +"""); + } + + public override async Task Replace_complex_collection_element_mapped_to_json() + { + await base.Replace_complex_collection_element_mapped_to_json(); + + AssertSql( + """ +@p0='[{"Name":"First Contact","PhoneNumbers":["555-1234","555-5678"]},{"Name":"Second Contact","PhoneNumbers":["555-9876","555-5432"]}]' (Nullable = false) (Size = 130) +@p1='{"Budget":"10000.0","Name":"Initial Department"}' (Nullable = false) (Size = 48) +@p2='[{"Name":"Replacement Employee","PhoneNumbers":["555-7777","555-8888"],"Address":{"City":"Replace City","Country":"Canada","PostalCode":"54321","Street":"789 Replace St"}}]' (Nullable = false) (Size = 172) +@p3='1' + +UPDATE "Companies" SET "Contacts" = @p0, "Department" = @p1, "Employees" = @p2 +WHERE "Id" = @p3 +RETURNING 1; +"""); + } + + public override async Task Complex_collection_with_empty_nested_collections_mapped_to_json() + { + await base.Complex_collection_with_empty_nested_collections_mapped_to_json(); + + AssertSql( + """ +@p0='[{"Name":"First Contact","PhoneNumbers":["555-1234","555-5678"]},{"Name":"Second Contact","PhoneNumbers":["555-9876","555-5432"]}]' (Nullable = false) (Size = 130) +@p1='{"Budget":"10000.0","Name":"Initial Department"}' (Nullable = false) (Size = 48) +@p2='[{"Name":"Initial Employee","PhoneNumbers":["555-0001"],"Address":{"City":"Initial City","Country":"USA","PostalCode":"00001","Street":"100 First St"}},{"Name":"Employee No Phone","PhoneNumbers":[],"Address":{"City":"Quiet City","Country":"USA","PostalCode":"00000","Street":"456 No Phone St"}}]' (Nullable = false) (Size = 295) +@p3='1' + +UPDATE "Companies" SET "Contacts" = @p0, "Department" = @p1, "Employees" = @p2 +WHERE "Id" = @p3 +RETURNING 1; +"""); + } + + public override async Task Set_complex_property_mapped_to_json_to_null() + { + await base.Set_complex_property_mapped_to_json_to_null(); + + AssertSql( + """ +@p0=NULL (Nullable = false) +@p1='1' + +UPDATE "Companies" SET "Department" = @p0 +WHERE "Id" = @p1 +RETURNING 1; +"""); + } + + public override async Task Set_null_complex_property_to_non_null_mapped_to_json() + { + await base.Set_null_complex_property_to_non_null_mapped_to_json(); + + AssertSql( + """ +@p0='{"Budget":"25000.0","Name":"New Department"}' (Nullable = false) (Size = 44) +@p1='1' + +UPDATE "Companies" SET "Department" = @p0 +WHERE "Id" = @p1 +RETURNING 1; +"""); + } + + public override async Task Replace_complex_property_mapped_to_json() + { + await base.Replace_complex_property_mapped_to_json(); + + AssertSql( + """ +@p0='[{"Name":"First Contact","PhoneNumbers":["555-1234","555-5678"]},{"Name":"Second Contact","PhoneNumbers":["555-9876","555-5432"]}]' (Nullable = false) (Size = 130) +@p1='{"Budget":"99999.99","Name":"Replacement Department"}' (Nullable = false) (Size = 53) +@p2='[{"Name":"Initial Employee","PhoneNumbers":["555-0001"],"Address":{"City":"Initial City","Country":"USA","PostalCode":"00001","Street":"100 First St"}}]' (Nullable = false) (Size = 152) +@p3='1' - protected override void ClearLog() - => Fixture.TestSqlLoggerFactory.Clear(); +UPDATE "Companies" SET "Contacts" = @p0, "Department" = @p1, "Employees" = @p2 +WHERE "Id" = @p3 +RETURNING 1; +"""); + } public class ComplexCollectionJsonUpdateSqliteFixture : ComplexCollectionJsonUpdateFixtureBase { diff --git a/test/EFCore.Tests/ExceptionTest.cs b/test/EFCore.Tests/ExceptionTest.cs index d06a5d65b54..7570521e2f2 100644 --- a/test/EFCore.Tests/ExceptionTest.cs +++ b/test/EFCore.Tests/ExceptionTest.cs @@ -156,6 +156,7 @@ public object GetPreStoreGeneratedCurrentValue(IPropertyBase propertyBase) public bool IsConceptualNull(IProperty property) => throw new NotImplementedException(); + public bool IsModified(IComplexProperty property) => throw new NotImplementedException(); } private static IEntityType CreateEntityType() diff --git a/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs b/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs index 4a41cd0f43c..057bd3d8285 100644 --- a/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs +++ b/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs @@ -1135,21 +1135,6 @@ public virtual void Detects_nonCollection_skip_navigations() modelBuilder); } - [ConditionalFact] - public virtual void Detects_optional_collection_complex_properties() - { - var modelBuilder = CreateConventionModelBuilder(); - modelBuilder.Ignore(typeof(Order)); - - var model = modelBuilder.Model; - var customerEntity = model.AddEntityType(typeof(Customer)); - var collection = customerEntity.AddComplexProperty(nameof(Customer.Orders), collection: true); - - Assert.Equal( - CoreStrings.ComplexPropertyOptional("Customer", "Orders"), - Assert.Throws(() => collection.IsNullable = true).Message); - } - [ConditionalFact] public virtual void Detects_value_type_complex_collection() { @@ -1183,7 +1168,8 @@ public virtual void Detects_non_list_complex_collection() eb => { eb.Property(e => e.Id); - eb.ComplexCollection(e => e.Tags); + eb.ComplexCollection(e => e.Tags, + cb => cb.Property(p => p.Key).IsRequired()); }); VerifyError(