From 8f50ca574b3bceb34c13ace9daf2f7c3079b4025 Mon Sep 17 00:00:00 2001 From: AndriySvyryd Date: Thu, 18 Jul 2019 10:07:59 -0700 Subject: [PATCH] Don't persist keys for embedded entities Fixes #13578 Part of #12086 --- .../CosmosDbContextOptionsExtensions.cs | 4 +- .../Extensions/CosmosPropertyExtensions.cs | 23 +- .../CosmosServiceCollectionExtensions.cs | 3 + .../Conventions/StoreKeyConvention.cs | 19 +- ...osmosProjectionBindingExpressionVisitor.cs | 3 +- ...osShapedQueryCompilingExpressionVisitor.cs | 196 +++++++++++++----- .../CosmosSqlTranslatingExpressionVisitor.cs | 6 +- .../Internal/EntityProjectionExpression.cs | 60 ++++-- src/EFCore.Cosmos/Query/Internal/IShaper.cs | 33 --- .../Query/Internal/ObjectAccessExpression.cs | 10 +- .../Query/Internal/SqlExpressionFactory.cs | 8 +- .../Internal/ValueBufferFactoryFactory.cs | 157 -------------- .../Storage/Internal/CosmosDatabaseCreator.cs | 21 +- .../Storage/Internal/CosmosDatabaseWrapper.cs | 10 +- .../Update/Internal/DocumentSource.cs | 87 +++++--- .../Internal/CosmosValueGeneratorSelector.cs | 31 +++ .../Internal/IdValueGenerator.cs | 1 - .../{Internal => }/AsyncQueryingEnumerable.cs | 0 .../IncludeCompilingExpressionVisitor.cs | 38 ++-- ...omSqlParameterApplyingExpressionVisitor.cs | 8 +- ...eterValueBasedSelectExpressionOptimizer.cs | 1 - ...lationalSqlTranslatingExpressionVisitor.cs | 3 +- src/EFCore/Query/ExpressionPrinter.cs | 6 +- .../Internal/NavigationExpansionHelpers.cs | 5 +- ...umentsTest.cs => EmbeddedDocumentsTest.cs} | 37 ++-- .../BuiltInDataTypesSqlServerTest.cs | 1 - 26 files changed, 394 insertions(+), 377 deletions(-) delete mode 100644 src/EFCore.Cosmos/Query/Internal/IShaper.cs delete mode 100644 src/EFCore.Cosmos/Query/Internal/ValueBufferFactoryFactory.cs create mode 100644 src/EFCore.Cosmos/ValueGeneration/Internal/CosmosValueGeneratorSelector.cs rename src/EFCore.Relational/Query/{Internal => }/AsyncQueryingEnumerable.cs (100%) rename src/EFCore.Relational/Query/{Internal => }/IncludeCompilingExpressionVisitor.cs (94%) rename test/EFCore.Cosmos.FunctionalTests/{NestedDocumentsTest.cs => EmbeddedDocumentsTest.cs} (91%) diff --git a/src/EFCore.Cosmos/Extensions/CosmosDbContextOptionsExtensions.cs b/src/EFCore.Cosmos/Extensions/CosmosDbContextOptionsExtensions.cs index 278584d4faa..30a90f42958 100644 --- a/src/EFCore.Cosmos/Extensions/CosmosDbContextOptionsExtensions.cs +++ b/src/EFCore.Cosmos/Extensions/CosmosDbContextOptionsExtensions.cs @@ -23,7 +23,7 @@ public static class CosmosDbContextOptionsExtensions /// The account end-point to connect to. /// The account key. /// The database name. - /// An optional action to allow additional Cosmos-specific configuration. + /// An optional action to allow additional Cosmos-specific configuration. /// The options builder so that further configuration can be chained. public static DbContextOptionsBuilder UseCosmos( [NotNull] this DbContextOptionsBuilder optionsBuilder, @@ -46,7 +46,7 @@ public static class CosmosDbContextOptionsExtensions /// The account end-point to connect to. /// The account key. /// The database name. - /// An optional action to allow additional Cosmos-specific configuration. + /// An optional action to allow additional Cosmos-specific configuration. /// The options builder so that further configuration can be chained. public static DbContextOptionsBuilder UseCosmos( [NotNull] this DbContextOptionsBuilder optionsBuilder, diff --git a/src/EFCore.Cosmos/Extensions/CosmosPropertyExtensions.cs b/src/EFCore.Cosmos/Extensions/CosmosPropertyExtensions.cs index 39e156405a3..d09996dc4d5 100644 --- a/src/EFCore.Cosmos/Extensions/CosmosPropertyExtensions.cs +++ b/src/EFCore.Cosmos/Extensions/CosmosPropertyExtensions.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System.Linq; using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.Cosmos.Metadata.Internal; using Microsoft.EntityFrameworkCore.Metadata; @@ -20,7 +21,27 @@ public static class CosmosPropertyExtensions /// The property name used when targeting Cosmos. public static string GetCosmosPropertyName([NotNull] this IProperty property) => (string)property[CosmosAnnotationNames.PropertyName] - ?? property.Name; + ?? GetDefaultPropertyName(property); + + private static string GetDefaultPropertyName(IProperty property) + { + var entityType = property.DeclaringEntityType; + var ownership = entityType.FindOwnership(); + + if (ownership != null + && !entityType.IsDocumentRoot()) + { + var pk = property.FindContainingPrimaryKey(); + if (pk != null + && pk.Properties.Count == ownership.Properties.Count + (ownership.IsUnique ? 0 : 1) + && ownership.Properties.All(fkProperty => pk.Properties.Contains(fkProperty))) + { + return ""; + } + } + + return property.Name; + } /// /// Sets the property name used when targeting Cosmos. diff --git a/src/EFCore.Cosmos/Extensions/CosmosServiceCollectionExtensions.cs b/src/EFCore.Cosmos/Extensions/CosmosServiceCollectionExtensions.cs index b2cdd34dbd9..1b84cc4827b 100644 --- a/src/EFCore.Cosmos/Extensions/CosmosServiceCollectionExtensions.cs +++ b/src/EFCore.Cosmos/Extensions/CosmosServiceCollectionExtensions.cs @@ -9,12 +9,14 @@ using Microsoft.EntityFrameworkCore.Cosmos.Metadata.Conventions.Internal; using Microsoft.EntityFrameworkCore.Cosmos.Query.Internal; using Microsoft.EntityFrameworkCore.Cosmos.Storage.Internal; +using Microsoft.EntityFrameworkCore.Cosmos.ValueGeneration.Internal; using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure; using Microsoft.EntityFrameworkCore.Query; using Microsoft.EntityFrameworkCore.Storage; using Microsoft.EntityFrameworkCore.Utilities; +using Microsoft.EntityFrameworkCore.ValueGeneration; // ReSharper disable once CheckNamespace namespace Microsoft.Extensions.DependencyInjection @@ -53,6 +55,7 @@ public static IServiceCollection AddEntityFrameworkCosmos([NotNull] this IServic .TryAdd() .TryAdd() .TryAdd() + .TryAdd() .TryAdd() .TryAdd() .TryAdd() diff --git a/src/EFCore.Cosmos/Metadata/Conventions/StoreKeyConvention.cs b/src/EFCore.Cosmos/Metadata/Conventions/StoreKeyConvention.cs index 51f8af767cb..46a95560336 100644 --- a/src/EFCore.Cosmos/Metadata/Conventions/StoreKeyConvention.cs +++ b/src/EFCore.Cosmos/Metadata/Conventions/StoreKeyConvention.cs @@ -9,15 +9,16 @@ using Microsoft.EntityFrameworkCore.Utilities; using Newtonsoft.Json.Linq; +// ReSharper disable once CheckNamespace namespace Microsoft.EntityFrameworkCore.Metadata.Conventions { /// /// /// A convention that adds the 'id' property - a key required by Azure Cosmos. /// - /// This convention also add the '__jObject' containing the JSON object returned by the store. /// - /// + /// This convention also adds the '__jObject' containing the JSON object returned by the store. + /// /// public class StoreKeyConvention : IEntityTypeAddedConvention, @@ -52,10 +53,6 @@ private static void Process(IConventionEntityTypeBuilder entityTypeBuilder) var idProperty = entityTypeBuilder.Property(typeof(string), IdPropertyName); idProperty.HasValueGenerator((_, __) => new IdValueGenerator()); entityTypeBuilder.HasKey(new[] { idProperty.Metadata }); - - var jObjectProperty = entityTypeBuilder.Property(typeof(JObject), JObjectPropertyName); - jObjectProperty.ToJsonProperty(""); - jObjectProperty.ValueGenerated(ValueGenerated.OnAddOrUpdate); } else { @@ -68,7 +65,17 @@ private static void Process(IConventionEntityTypeBuilder entityTypeBuilder) entityType.Builder.HasNoKey(key); } } + } + if (entityType.BaseType == null + && !entityType.IsKeyless) + { + var jObjectProperty = entityTypeBuilder.Property(typeof(JObject), JObjectPropertyName); + jObjectProperty.ToJsonProperty(""); + jObjectProperty.ValueGenerated(ValueGenerated.OnAddOrUpdate); + } + else + { var jObjectProperty = entityType.FindDeclaredProperty(JObjectPropertyName); if (jObjectProperty != null) { diff --git a/src/EFCore.Cosmos/Query/Internal/CosmosProjectionBindingExpressionVisitor.cs b/src/EFCore.Cosmos/Query/Internal/CosmosProjectionBindingExpressionVisitor.cs index 1e5b06aa28a..f91ee0e40eb 100644 --- a/src/EFCore.Cosmos/Query/Internal/CosmosProjectionBindingExpressionVisitor.cs +++ b/src/EFCore.Cosmos/Query/Internal/CosmosProjectionBindingExpressionVisitor.cs @@ -200,7 +200,8 @@ protected override Expression VisitMember(MemberExpression memberExpression) throw new InvalidOperationException(); } - var navigationProjection = innerEntityProjection.BindMember(memberExpression.Member, innerExpression.Type, out var propertyBase); + var navigationProjection = innerEntityProjection.BindMember( + memberExpression.Member, innerExpression.Type, clientEval: true, out var propertyBase); if (!(propertyBase is INavigation navigation) || !navigation.IsEmbedded()) diff --git a/src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.cs b/src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.cs index 45bf36ab143..348cdf08266 100644 --- a/src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.cs +++ b/src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.cs @@ -4,6 +4,7 @@ using System; using System.Collections; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using System.Linq.Expressions; using System.Reflection; @@ -187,18 +188,25 @@ private class CosmosProjectionBindingRemovingExpressionVisitor : ExpressionVisit private static readonly MethodInfo _toObjectMethodInfo = typeof(CosmosProjectionBindingRemovingExpressionVisitor).GetTypeInfo().GetRuntimeMethods() .Single(mi => mi.Name == nameof(SafeToObject)); - private static readonly MethodInfo _isNullMethodInfo - = typeof(CosmosProjectionBindingRemovingExpressionVisitor).GetTypeInfo().GetRuntimeMethods() - .Single(mi => mi.Name == nameof(IsNull)); + private static readonly MethodInfo _collectionAccessorAddMethodInfo + = typeof(IClrCollectionAccessor).GetTypeInfo() + .GetDeclaredMethod(nameof(IClrCollectionAccessor.Add)); + private static readonly MethodInfo _collectionAccessorGetOrCreateMethodInfo + = typeof(IClrCollectionAccessor).GetTypeInfo() + .GetDeclaredMethod(nameof(IClrCollectionAccessor.GetOrCreate)); private readonly SelectExpression _selectExpression; private readonly ParameterExpression _jObjectParameter; private readonly bool _trackQueryResults; - private readonly IDictionary _materializationContextBindings - = new Dictionary(); + private readonly IDictionary _materializationContextBindings + = new Dictionary(); private readonly IDictionary _projectionBindings = new Dictionary(); + private readonly IDictionary _ownerMappings + = new Dictionary(); + private (IEntityType EntityType, ParameterExpression JObjectVariable) _ownerInfo; + private ParameterExpression _ordinalParameter; public CosmosProjectionBindingRemovingExpressionVisitor( SelectExpression selectExpression, @@ -231,32 +239,38 @@ protected override Expression VisitBinary(BinaryExpression binaryExpression) projectionExpression = ((UnaryExpression)convertExpression.Operand).Operand; } - Expression accessExpression; + Expression innerAccessExpression; if (projectionExpression is ObjectArrayProjectionExpression objectArrayProjectionExpression) { - accessExpression = objectArrayProjectionExpression.AccessExpression; + innerAccessExpression = objectArrayProjectionExpression.AccessExpression; _projectionBindings[objectArrayProjectionExpression] = parameterExpression; storeName ??= objectArrayProjectionExpression.Name; } else { var entityProjectionExpression = (EntityProjectionExpression)projectionExpression; - _projectionBindings[entityProjectionExpression.AccessExpression] = parameterExpression; + var accessExpression = entityProjectionExpression.AccessExpression; + _projectionBindings[accessExpression] = parameterExpression; storeName ??= entityProjectionExpression.Name; - switch (entityProjectionExpression.AccessExpression) + if (_ownerInfo.EntityType != null) + { + _ownerMappings[accessExpression] = _ownerInfo; + } + + switch (accessExpression) { case ObjectAccessExpression innerObjectAccessExpression: - accessExpression = innerObjectAccessExpression.AccessExpression; + innerAccessExpression = innerObjectAccessExpression.AccessExpression; break; case RootReferenceExpression _: - accessExpression = _jObjectParameter; + innerAccessExpression = _jObjectParameter; break; default: throw new InvalidOperationException(); } } - var valueExpression = CreateGetStoreValueExpression(accessExpression, storeName, parameterExpression.Type); + var valueExpression = CreateGetValueExpression(innerAccessExpression, storeName, parameterExpression.Type); return Expression.MakeBinary(ExpressionType.Assign, binaryExpression.Left, valueExpression); } @@ -277,8 +291,7 @@ protected override Expression VisitBinary(BinaryExpression binaryExpression) entityProjectionExpression = (EntityProjectionExpression)projection; } - _materializationContextBindings[parameterExpression] - = _projectionBindings[entityProjectionExpression.AccessExpression]; + _materializationContextBindings[parameterExpression] = entityProjectionExpression.AccessExpression; var updatedExpression = Expression.New(newExpression.Constructor, Expression.Constant(ValueBuffer.Empty), @@ -341,7 +354,7 @@ protected override Expression VisitExtension(Expression extensionExpression) { var projection = GetProjection(projectionBindingExpression); - return CreateGetStoreValueExpression( + return CreateGetValueExpression( _jObjectParameter, projection.Alias, projectionBindingExpression.Type, (projection.Expression as SqlExpression)?.TypeMapping); @@ -364,12 +377,20 @@ protected override Expression VisitExtension(Expression extensionExpression) } var jArray = _projectionBindings[objectArrayProjection]; - var jObjectParameter = Expression.Parameter(typeof(JObject), jArray.Name + "object"); - var ordinalParameter = Expression.Parameter(typeof(int), "ordinal"); + var jObjectParameter = Expression.Parameter(typeof(JObject), jArray.Name + "Object"); + var ordinalParameter = Expression.Parameter(typeof(int), jArray.Name + "Ordinal"); _projectionBindings[objectArrayProjection.InnerProjection.AccessExpression] = jObjectParameter; + if (_ownerInfo.EntityType != null) + { + _ownerMappings[objectArrayProjection.InnerProjection.AccessExpression] = _ownerInfo; + } + var previousOrdinalParameter = _ordinalParameter; + _ordinalParameter = ordinalParameter; var innerShaper = Visit(collectionShaperExpression.InnerShaper); + _ordinalParameter = previousOrdinalParameter; + return Expression.Call( _selectMethodInfo.MakeGenericMethod(typeof(JObject), innerShaper.Type), Expression.Call( @@ -389,6 +410,7 @@ protected override Expression VisitExtension(Expression extensionExpression) // These are the expressions added by JObjectInjectingExpressionVisitor var jObjectBlock = (BlockExpression)Visit(includeExpression.EntityExpression); + var jObjectVariable = jObjectBlock.Variables.Single(v => v.Type == typeof(JObject)); var jObjectCondition = (ConditionalExpression)jObjectBlock.Expressions[jObjectBlock.Expressions.Count - 1]; var shaperBlock = (BlockExpression)jObjectCondition.IfFalse; @@ -405,9 +427,13 @@ protected override Expression VisitExtension(Expression extensionExpression) var concreteEntityTypeVariable = shaperBlock.Variables.Single(v => v.Type == typeof(IEntityType)); var inverseNavigation = navigation.FindInverse(); var fixup = GenerateFixup( - includingClrType, relatedEntityClrType, navigation, inverseNavigation) - .Compile(); + includingClrType, relatedEntityClrType, navigation, inverseNavigation); + var initialize = GenerateInitialize(includingClrType, navigation); + + var previousOwner = _ownerInfo; + _ownerInfo = (navigation.DeclaringEntityType, jObjectVariable); var navigationExpression = Visit(includeExpression.NavigationExpression); + _ownerInfo = previousOwner; shaperExpressions.Add(Expression.Call( includeMethod.MakeGenericMethod(includingClrType, relatedEntityClrType), @@ -417,7 +443,8 @@ protected override Expression VisitExtension(Expression extensionExpression) navigationExpression, Expression.Constant(navigation), Expression.Constant(inverseNavigation, typeof(INavigation)), - Expression.Constant(fixup))); + Expression.Constant(fixup), + Expression.Constant(initialize, typeof(Action<>).MakeGenericType(includingClrType)))); shaperExpressions.Add(instanceVariable); shaperBlock = shaperBlock.Update(shaperBlock.Variables, shaperExpressions); @@ -445,7 +472,8 @@ protected override Expression VisitExtension(Expression extensionExpression) TIncludedEntity relatedEntity, INavigation navigation, INavigation inverseNavigation, - Action fixup) + Action fixup, + Action initialize) { if (entity == null || !navigation.DeclaringEntityType.IsAssignableFrom(entityType)) @@ -485,7 +513,8 @@ protected override Expression VisitExtension(Expression extensionExpression) IEnumerable relatedEntities, INavigation navigation, INavigation inverseNavigation, - Action fixup) + Action fixup, + Action initialize) { if (entity == null || !navigation.DeclaringEntityType.IsAssignableFrom(entityType)) @@ -498,24 +527,38 @@ protected override Expression VisitExtension(Expression extensionExpression) var includingEntity = (TIncludingEntity)entity; SetIsLoadedNoTracking(includingEntity, navigation); - foreach (var relatedEntity in relatedEntities) + if (relatedEntities != null) { - fixup(includingEntity, relatedEntity); - if (inverseNavigation != null) + foreach (var relatedEntity in relatedEntities) { - SetIsLoadedNoTracking(relatedEntity, inverseNavigation); + fixup(includingEntity, relatedEntity); + if (inverseNavigation != null) + { + SetIsLoadedNoTracking(relatedEntity, inverseNavigation); + } } } + else + { + initialize(includingEntity); + } } else { entry.SetIsLoaded(navigation); - using (var enumerator = relatedEntities.GetEnumerator()) + if (relatedEntities != null) { - while (enumerator.MoveNext()) + using (var enumerator = relatedEntities.GetEnumerator()) { + while (enumerator.MoveNext()) + { + } } } + else + { + initialize((TIncludingEntity)entity); + } } } @@ -527,7 +570,7 @@ private static void SetIsLoadedNoTracking(object entity, INavigation navigation) ?.Getter.GetClrValue(entity)) ?.SetLoaded(entity, navigation.Name); - private static LambdaExpression GenerateFixup( + private static Delegate GenerateFixup( Type entityType, Type relatedEntityType, INavigation navigation, @@ -551,7 +594,28 @@ private static void SetIsLoadedNoTracking(object entity, INavigation navigation) } - return Expression.Lambda(Expression.Block(typeof(void), expressions), entityParameter, relatedEntityParameter); + return Expression.Lambda(Expression.Block(typeof(void), expressions), entityParameter, relatedEntityParameter) + .Compile(); + } + + private static Delegate GenerateInitialize( + Type entityType, + INavigation navigation) + { + if (!navigation.IsCollection()) + { + return null; + } + + var entityParameter = Expression.Parameter(entityType); + + var getOrCreateExpression = Expression.Call( + Expression.Constant(navigation.GetCollectionAccessor()), + _collectionAccessorGetOrCreateMethodInfo, + entityParameter); + + return Expression.Lambda(Expression.Block(typeof(void), getOrCreateExpression), entityParameter) + .Compile(); } private static Expression AssignReferenceNavigation( @@ -571,10 +635,6 @@ private static void SetIsLoadedNoTracking(object entity, INavigation navigation) entity, relatedEntity); - private static readonly MethodInfo _collectionAccessorAddMethodInfo - = typeof(IClrCollectionAccessor).GetTypeInfo() - .GetDeclaredMethod(nameof(IClrCollectionAccessor.Add)); - private int GetProjectionIndex(ProjectionBindingExpression projectionBindingExpression) => projectionBindingExpression.ProjectionMember != null ? (int)((ConstantExpression)_selectExpression.GetMappedProjection(projectionBindingExpression.ProjectionMember)).Value @@ -595,19 +655,59 @@ private static Expression CreateReadJTokenExpression(Expression jObjectExpressio { if (property.Name == StoreKeyConvention.JObjectPropertyName) { - return jObjectExpression; + return _projectionBindings[jObjectExpression]; } var storeName = property.GetCosmosPropertyName(); if (storeName.Length == 0) { + var entityType = property.DeclaringEntityType; + if (!entityType.IsDocumentRoot()) + { + var ownership = entityType.FindOwnership(); + + if (ownership != null + && !ownership.IsUnique + && property.IsPrimaryKey() + && !property.IsForeignKey() + && property.ClrType == typeof(int)) + { + return _ordinalParameter; + } + + var principalProperty = property.FindFirstPrincipal(); + if (principalProperty != null) + { + Expression ownerJObjectExpression = null; + if (_ownerMappings.TryGetValue(jObjectExpression, out var ownerInfo)) + { + Debug.Assert(principalProperty.DeclaringEntityType.IsAssignableFrom(ownerInfo.EntityType)); + + ownerJObjectExpression = ownerInfo.JObjectVariable; + } + else if (jObjectExpression is RootReferenceExpression rootReferenceExpression) + { + ownerJObjectExpression = rootReferenceExpression; + } + else if (jObjectExpression is ObjectAccessExpression objectAccessExpression) + { + ownerJObjectExpression = objectAccessExpression.AccessExpression; + } + + if (ownerJObjectExpression != null) + { + return CreateGetValueExpression(ownerJObjectExpression, principalProperty); + } + } + } + return Expression.Default(property.ClrType); } - return CreateGetStoreValueExpression(jObjectExpression, storeName, property.ClrType, property.GetTypeMapping()); + return CreateGetValueExpression(jObjectExpression, storeName, property.ClrType, property.GetTypeMapping()); } - private Expression CreateGetStoreValueExpression( + private Expression CreateGetValueExpression( Expression jObjectExpression, string storeName, Type clrType, @@ -620,20 +720,20 @@ private static Expression CreateReadJTokenExpression(Expression jObjectExpressio } else if (jObjectExpression is RootReferenceExpression rootReferenceExpression) { - innerExpression = CreateGetStoreValueExpression( + innerExpression = CreateGetValueExpression( _jObjectParameter, rootReferenceExpression.Alias, typeof(JObject)); } else if (jObjectExpression is ObjectAccessExpression objectAccessExpression) { var innerAccessExpression = objectAccessExpression.AccessExpression; - innerExpression = CreateGetStoreValueExpression( + innerExpression = CreateGetValueExpression( innerAccessExpression, ((IAccessExpression)innerAccessExpression).Name, typeof(JObject)); } - var jTokenExpression = Expression.Call(innerExpression, _getItemMethodInfo, Expression.Constant(storeName)); - Expression valueExpression; + var jTokenExpression = CreateReadJTokenExpression(innerExpression, storeName); + Expression valueExpression; var converter = typeMapping?.Converter; if (converter != null) { @@ -654,15 +754,6 @@ private static Expression CreateReadJTokenExpression(Expression jObjectExpressio valueExpression = ConvertJTokenToType(jTokenExpression, clrType); } - if (clrType.IsNullableType()) - { - valueExpression = - Expression.Condition( - Expression.Call(_isNullMethodInfo, jTokenExpression), - Expression.Default(valueExpression.Type), - valueExpression); - } - return valueExpression; } @@ -674,10 +765,7 @@ private static Expression ConvertJTokenToType(Expression jTokenExpression, Type jTokenExpression); private static T SafeToObject(JToken token) - => token == null ? default : token.ToObject(); - - private static bool IsNull(JToken token) - => token == null || token.Type == JTokenType.Null; + => token == null || token.Type == JTokenType.Null ? default : token.ToObject(); } private class QueryingEnumerable : IEnumerable diff --git a/src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs b/src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs index f62f73852e1..e22e7ad1225 100644 --- a/src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs +++ b/src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs @@ -105,9 +105,9 @@ private bool TryBindMember(Expression source, MemberIdentity member, out Express if (source is EntityProjectionExpression entityProjectionExpression) { expression = member.MemberInfo != null - ? entityProjectionExpression.BindMember(member.MemberInfo, null, out _) - : entityProjectionExpression.BindMember(member.Name, null, out _); - return true; + ? entityProjectionExpression.BindMember(member.MemberInfo, null, clientEval: false, out _) + : entityProjectionExpression.BindMember(member.Name, null, clientEval: false, out _); + return expression != null; } if (source is MemberExpression innerMemberExpression diff --git a/src/EFCore.Cosmos/Query/Internal/EntityProjectionExpression.cs b/src/EFCore.Cosmos/Query/Internal/EntityProjectionExpression.cs index d4262aa885a..0162424df4a 100644 --- a/src/EFCore.Cosmos/Query/Internal/EntityProjectionExpression.cs +++ b/src/EFCore.Cosmos/Query/Internal/EntityProjectionExpression.cs @@ -19,10 +19,10 @@ namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal /// public class EntityProjectionExpression : Expression, IPrintable, IAccessExpression { - private readonly IDictionary _propertyExpressionsCache - = new Dictionary(); - private readonly IDictionary _navigationExpressionsCache - = new Dictionary(); + private readonly IDictionary _propertyExpressionsCache + = new Dictionary(); + private readonly IDictionary _navigationExpressionsCache + = new Dictionary(); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -98,7 +98,7 @@ protected override Expression VisitChildren(ExpressionVisitor visitor) /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public virtual SqlExpression BindProperty(IProperty property) + public virtual Expression BindProperty(IProperty property, bool clientEval) { if (!EntityType.IsAssignableFrom(property.DeclaringEntityType) && !property.DeclaringEntityType.IsAssignableFrom(EntityType)) @@ -113,7 +113,14 @@ public virtual SqlExpression BindProperty(IProperty property) _propertyExpressionsCache[property] = expression; } - return expression; + if (!clientEval + && expression.Name.Length == 0) + { + // Non-persisted property can't be translated + return null; + } + + return (Expression)expression; } /// @@ -122,7 +129,7 @@ public virtual SqlExpression BindProperty(IProperty property) /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public virtual Expression BindNavigation(INavigation navigation) + public virtual Expression BindNavigation(INavigation navigation, bool clientEval) { if (!EntityType.IsAssignableFrom(navigation.DeclaringEntityType) && !navigation.DeclaringEntityType.IsAssignableFrom(EntityType)) @@ -147,7 +154,14 @@ public virtual Expression BindNavigation(INavigation navigation) _navigationExpressionsCache[navigation] = expression; } - return expression; + if (!clientEval + && expression.Name.Length == 0) + { + // Non-persisted navigation can't be translated + return null; + } + + return (Expression)expression; } /// @@ -156,7 +170,7 @@ public virtual Expression BindNavigation(INavigation navigation) /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public virtual Expression BindMember(string name, Type entityClrType, out IPropertyBase propertyBase) + public virtual Expression BindMember(string name, Type entityClrType, bool clientEval, out IPropertyBase propertyBase) { var entityType = EntityType; if (entityClrType != null @@ -169,12 +183,19 @@ public virtual Expression BindMember(string name, Type entityClrType, out IPrope if (property != null) { propertyBase = property; - return BindProperty(property); + return BindProperty(property, clientEval); } var navigation = entityType.FindNavigation(name); - propertyBase = navigation; - return BindNavigation(navigation); + if (navigation != null) + { + propertyBase = navigation; + return BindNavigation(navigation, clientEval); + } + + // Entity member not found + propertyBase = null; + return null; } /// @@ -183,7 +204,7 @@ public virtual Expression BindMember(string name, Type entityClrType, out IPrope /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public virtual Expression BindMember(MemberInfo memberInfo, Type entityClrType, out IPropertyBase propertyBase) + public virtual Expression BindMember(MemberInfo memberInfo, Type entityClrType, bool clientEval, out IPropertyBase propertyBase) { var entityType = EntityType; if (entityClrType != null @@ -196,12 +217,19 @@ public virtual Expression BindMember(MemberInfo memberInfo, Type entityClrType, if (property != null) { propertyBase = property; - return BindProperty(property); + return BindProperty(property, clientEval); } var navigation = entityType.FindNavigation(memberInfo); - propertyBase = navigation; - return BindNavigation(navigation); + if (navigation != null) + { + propertyBase = navigation; + return BindNavigation(navigation, clientEval); + } + + // Entity member not found + propertyBase = null; + return null; } /// diff --git a/src/EFCore.Cosmos/Query/Internal/IShaper.cs b/src/EFCore.Cosmos/Query/Internal/IShaper.cs deleted file mode 100644 index 9ef33b0330b..00000000000 --- a/src/EFCore.Cosmos/Query/Internal/IShaper.cs +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; -using System.Linq.Expressions; - -namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal -{ - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - public interface IShaper - { - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - LambdaExpression CreateShaperLambda(); - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - Type Type { get; } - } -} diff --git a/src/EFCore.Cosmos/Query/Internal/ObjectAccessExpression.cs b/src/EFCore.Cosmos/Query/Internal/ObjectAccessExpression.cs index 6bb460e498a..0e1c5b54fd6 100644 --- a/src/EFCore.Cosmos/Query/Internal/ObjectAccessExpression.cs +++ b/src/EFCore.Cosmos/Query/Internal/ObjectAccessExpression.cs @@ -28,13 +28,21 @@ public ObjectAccessExpression(INavigation navigation, Expression accessExpressio if (Name == null) { throw new InvalidOperationException( - $"Navigation '{navigation.DeclaringEntityType.DisplayName()}.{navigation.Name}' doesn't point to a nested entity."); + $"Navigation '{navigation.DeclaringEntityType.DisplayName()}.{navigation.Name}' doesn't point to an embedded entity."); } Navigation = navigation; AccessExpression = accessExpression; } + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public override ExpressionType NodeType => ExpressionType.Extension; + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in diff --git a/src/EFCore.Cosmos/Query/Internal/SqlExpressionFactory.cs b/src/EFCore.Cosmos/Query/Internal/SqlExpressionFactory.cs index 8c313beae33..693c468687c 100644 --- a/src/EFCore.Cosmos/Query/Internal/SqlExpressionFactory.cs +++ b/src/EFCore.Cosmos/Query/Internal/SqlExpressionFactory.cs @@ -514,19 +514,19 @@ private void AddDiscriminator(SelectExpression selectExpression, IEntityType ent if (concreteEntityType.GetDiscriminatorProperty() != null) { var discriminatorColumn = ((EntityProjectionExpression)selectExpression.GetMappedProjection(new ProjectionMember())) - .BindProperty(concreteEntityType.GetDiscriminatorProperty()); + .BindProperty(concreteEntityType.GetDiscriminatorProperty(), clientEval: false); selectExpression.ApplyPredicate( - Equal(discriminatorColumn, Constant(concreteEntityType.GetDiscriminatorValue()))); + Equal((SqlExpression)discriminatorColumn, Constant(concreteEntityType.GetDiscriminatorValue()))); } } else { var discriminatorColumn = ((EntityProjectionExpression)selectExpression.GetMappedProjection(new ProjectionMember())) - .BindProperty(concreteEntityTypes[0].GetDiscriminatorProperty()); + .BindProperty(concreteEntityTypes[0].GetDiscriminatorProperty(), clientEval: false); selectExpression.ApplyPredicate( - In(discriminatorColumn, Constant(concreteEntityTypes.Select(et => et.GetDiscriminatorValue()).ToList()), negated: false)); + In((SqlExpression)discriminatorColumn, Constant(concreteEntityTypes.Select(et => et.GetDiscriminatorValue()).ToList()), negated: false)); } } } diff --git a/src/EFCore.Cosmos/Query/Internal/ValueBufferFactoryFactory.cs b/src/EFCore.Cosmos/Query/Internal/ValueBufferFactoryFactory.cs deleted file mode 100644 index e595d685901..00000000000 --- a/src/EFCore.Cosmos/Query/Internal/ValueBufferFactoryFactory.cs +++ /dev/null @@ -1,157 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; -using System.Collections.Generic; -using System.Linq; -using System.Linq.Expressions; -using System.Reflection; -using Microsoft.EntityFrameworkCore.Metadata; -using Microsoft.EntityFrameworkCore.Metadata.Conventions; -using Microsoft.EntityFrameworkCore.Query; -using Newtonsoft.Json.Linq; - -namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal -{ - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - public static class ValueBufferFactoryFactory - { - private static readonly ParameterExpression _jObjectParameter = Expression.Parameter(typeof(JObject), "jObject"); - - private static readonly MethodInfo _getItemMethodInfo - = typeof(JObject).GetTypeInfo().GetRuntimeProperties() - .Single(pi => pi.Name == "Item" && pi.GetIndexParameters()[0].ParameterType == typeof(string)) - .GetMethod; - - private static readonly MethodInfo _toObjectMethodInfo - = typeof(ValueBufferFactoryFactory).GetTypeInfo().GetRuntimeMethods() - .Single(mi => mi.Name == nameof(SafeToObject)); - - private static readonly MethodInfo _isNullMethodInfo - = typeof(ValueBufferFactoryFactory).GetTypeInfo().GetRuntimeMethods() - .Single(mi => mi.Name == nameof(IsNull)); - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - public static Expression> Create(List usedProperties) - => Expression.Lambda>( - Expression.NewArrayInit( - typeof(object), - usedProperties - .Select( - p => - CreateGetValueExpression( - _jObjectParameter, - p))), - _jObjectParameter); - - private static Expression CreateGetValueExpression( - Expression jObjectExpression, - IProperty property) - { - if (property.Name == StoreKeyConvention.JObjectPropertyName) - { - return jObjectExpression; - } - - var storeName = property.GetCosmosPropertyName(); - if (storeName.Length == 0) - { - var type = property.GetTypeMapping().Converter?.ProviderClrType - ?? property.ClrType; - - Expression calculatedExpression = Expression.Default(type); - return type.IsValueType - ? Expression.Convert(calculatedExpression, typeof(object)) - : calculatedExpression; - } - - var valueExpression = CreateGetStoreValueExpression(jObjectExpression, property, storeName); - if (valueExpression.Type.IsValueType) - { - valueExpression = Expression.Convert(valueExpression, typeof(object)); - } - - return valueExpression; - } - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - public static Expression CreateGetStoreValueExpression(Expression jObjectExpression, IProperty property, string storeName) - { - Expression valueExpression = Expression.Call( - jObjectExpression, - _getItemMethodInfo, - Expression.Constant(storeName)); - - var modelClrType = property.ClrType; - var converter = property.GetTypeMapping().Converter; - if (converter != null) - { - var nullableExpression = valueExpression; - - if (valueExpression.Type != converter.ProviderClrType) - { - valueExpression = - Expression.Call( - _toObjectMethodInfo.MakeGenericMethod(converter.ProviderClrType), - valueExpression); - - if (converter.ProviderClrType.IsNullableType()) - { - nullableExpression = valueExpression; - } - } - - valueExpression = ReplacingExpressionVisitor.Replace( - converter.ConvertFromProviderExpression.Parameters.Single(), - valueExpression, - converter.ConvertFromProviderExpression.Body); - - if (valueExpression.Type != modelClrType) - { - valueExpression = Expression.Convert(valueExpression, modelClrType); - } - - if (modelClrType.IsNullableType()) - { - valueExpression = - Expression.Condition( - nullableExpression.Type == typeof(JToken) - ? (Expression)Expression.Call(_isNullMethodInfo, nullableExpression) - : Expression.Equal(nullableExpression, Expression.Constant(null, nullableExpression.Type)), - Expression.Constant(null, modelClrType), - valueExpression); - } - } - else if (valueExpression.Type != modelClrType) - { - valueExpression = - Expression.Call( - _toObjectMethodInfo.MakeGenericMethod(modelClrType), - valueExpression); - } - - return valueExpression; - } - - private static T SafeToObject(JToken token) - => token == null ? default : token.ToObject(); - - private static bool IsNull(JToken token) - => token == null || token.Type == JTokenType.Null; - } -} diff --git a/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseCreator.cs b/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseCreator.cs index de846e0801b..8e4af21f01d 100644 --- a/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseCreator.cs +++ b/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseCreator.cs @@ -4,6 +4,7 @@ using System; using System.Threading; using System.Threading.Tasks; +using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.Cosmos.Metadata.Internal; using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Storage; @@ -55,7 +56,7 @@ public virtual bool EnsureCreated() { created |= _cosmosClient.CreateContainerIfNotExists( entityType.GetCosmosContainer(), - entityType.GetCosmosPartitionKeyStoreName()); + GetCosmosPartitionKeyStoreName(entityType)); } if (created) @@ -89,7 +90,7 @@ public virtual async Task EnsureCreatedAsync(CancellationToken cancellatio { created |= await _cosmosClient.CreateContainerIfNotExistsAsync( entityType.GetCosmosContainer(), - entityType.GetCosmosPartitionKeyStoreName(), + GetCosmosPartitionKeyStoreName(entityType), cancellationToken); } @@ -145,5 +146,21 @@ public virtual bool CanConnect() /// public virtual Task CanConnectAsync(CancellationToken cancellationToken = default) => throw new NotImplementedException(); + + /// + /// Returns the store name of the property that is used to store the partition key. + /// + /// The entity type to get the partition key property name for. + /// The name of the partition key property. + private static string GetCosmosPartitionKeyStoreName([NotNull] IEntityType entityType) + { + var name = entityType.GetCosmosPartitionKeyPropertyName(); + if (name != null) + { + return entityType.FindProperty(name).GetCosmosPropertyName(); + } + + return CosmosClientWrapper.DefaultPartitionKey; + } } } diff --git a/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs b/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs index 6ae1ef3d95f..95b4e2a154e 100644 --- a/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs +++ b/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs @@ -193,10 +193,7 @@ private bool Save(IUpdateEntry entry) return _cosmosClient.CreateItem(collectionId, newDocument, GetPartitionKey(entry)); case EntityState.Modified: - var jObjectProperty = entityType.FindProperty(StoreKeyConvention.JObjectPropertyName); - var document = jObjectProperty != null - ? (JObject)(entry.SharedIdentityEntry ?? entry).GetCurrentValue(jObjectProperty) - : null; + var document = documentSource.GetCurrentDocument(entry); if (document != null) { if (documentSource.UpdateDocument(document, entry) == null) @@ -247,10 +244,7 @@ private Task SaveAsync(IUpdateEntry entry, CancellationToken cancellationT var newDocument = documentSource.CreateDocument(entry); return _cosmosClient.CreateItemAsync(collectionId, newDocument, GetPartitionKey(entry), cancellationToken); case EntityState.Modified: - var jObjectProperty = entityType.FindProperty(StoreKeyConvention.JObjectPropertyName); - var document = jObjectProperty != null - ? (JObject)(entry.SharedIdentityEntry ?? entry).GetCurrentValue(jObjectProperty) - : null; + var document = documentSource.GetCurrentDocument(entry); if (document != null) { if (documentSource.UpdateDocument(document, entry) == null) diff --git a/src/EFCore.Cosmos/Update/Internal/DocumentSource.cs b/src/EFCore.Cosmos/Update/Internal/DocumentSource.cs index 8ce1803bb31..55eb47bea39 100644 --- a/src/EFCore.Cosmos/Update/Internal/DocumentSource.cs +++ b/src/EFCore.Cosmos/Update/Internal/DocumentSource.cs @@ -23,6 +23,7 @@ public class DocumentSource private readonly string _collectionId; private readonly CosmosDatabaseWrapper _database; private readonly IProperty _idProperty; + private readonly IProperty _jObjectProperty; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -35,6 +36,7 @@ public DocumentSource(IEntityType entityType, CosmosDatabaseWrapper database) _collectionId = entityType.GetCosmosContainer(); _database = database; _idProperty = entityType.FindProperty(StoreKeyConvention.IdPropertyName); + _jObjectProperty = entityType.FindProperty(StoreKeyConvention.JObjectPropertyName); } /// @@ -71,39 +73,43 @@ public virtual JObject CreateDocument(IUpdateEntry entry) { document[storeName] = ConvertPropertyValue(property, entry.GetCurrentValue(property)); } + else if (entry.HasTemporaryValue(property)) + { + ((InternalEntityEntry)entry)[property] = entry.GetCurrentValue(property); + } } - foreach (var ownedNavigation in entry.EntityType.GetNavigations()) + foreach (var embeddedNavigation in entry.EntityType.GetNavigations()) { - var fk = ownedNavigation.ForeignKey; + var fk = embeddedNavigation.ForeignKey; if (!fk.IsOwnership - || ownedNavigation.IsDependentToPrincipal() + || embeddedNavigation.IsDependentToPrincipal() || fk.DeclaringEntityType.IsDocumentRoot()) { continue; } - var nestedValue = entry.GetCurrentValue(ownedNavigation); - var nestedPropertyName = fk.DeclaringEntityType.GetCosmosContainingPropertyName(); - if (nestedValue == null) + var embeddedValue = entry.GetCurrentValue(embeddedNavigation); + var embeddedPropertyName = fk.DeclaringEntityType.GetCosmosContainingPropertyName(); + if (embeddedValue == null) { - document[nestedPropertyName] = null; + document[embeddedPropertyName] = null; } else if (fk.IsUnique) { - var dependentEntry = ((InternalEntityEntry)entry).StateManager.TryGetEntry(nestedValue, fk.DeclaringEntityType); - document[nestedPropertyName] = _database.GetDocumentSource(dependentEntry.EntityType).CreateDocument(dependentEntry); + var dependentEntry = ((InternalEntityEntry)entry).StateManager.TryGetEntry(embeddedValue, fk.DeclaringEntityType); + document[embeddedPropertyName] = _database.GetDocumentSource(dependentEntry.EntityType).CreateDocument(dependentEntry); } else { var array = new JArray(); - foreach (var dependent in (IEnumerable)nestedValue) + foreach (var dependent in (IEnumerable)embeddedValue) { var dependentEntry = ((InternalEntityEntry)entry).StateManager.TryGetEntry(dependent, fk.DeclaringEntityType); array.Add(_database.GetDocumentSource(dependentEntry.EntityType).CreateDocument(dependentEntry)); } - document[nestedPropertyName] = array; + document[embeddedPropertyName] = array; } } @@ -130,6 +136,10 @@ public virtual JObject UpdateDocument(JObject document, IUpdateEntry entry) document[storeName] = ConvertPropertyValue(property, entry.GetCurrentValue(property)); anyPropertyUpdated = true; } + else if (entry.HasTemporaryValue(property)) + { + ((InternalEntityEntry)entry)[property] = entry.GetCurrentValue(property); + } } } @@ -143,55 +153,56 @@ public virtual JObject UpdateDocument(JObject document, IUpdateEntry entry) continue; } - var nestedDocumentSource = _database.GetDocumentSource(fk.DeclaringEntityType); - var nestedValue = entry.GetCurrentValue(ownedNavigation); - var nestedPropertyName = fk.DeclaringEntityType.GetCosmosContainingPropertyName(); - if (nestedValue == null) + var embeddedDocumentSource = _database.GetDocumentSource(fk.DeclaringEntityType); + var embeddedValue = entry.GetCurrentValue(ownedNavigation); + var embeddedPropertyName = fk.DeclaringEntityType.GetCosmosContainingPropertyName(); + if (embeddedValue == null) { - if (document[nestedPropertyName] != null) + if (document[embeddedPropertyName] != null) { - document[nestedPropertyName] = null; + document[embeddedPropertyName] = null; anyPropertyUpdated = true; } } else if (fk.IsUnique) { - var nestedEntry = ((InternalEntityEntry)entry).StateManager.TryGetEntry(nestedValue, fk.DeclaringEntityType); - if (nestedEntry == null) + var embeddedEntry = ((InternalEntityEntry)entry).StateManager.TryGetEntry(embeddedValue, fk.DeclaringEntityType); + if (embeddedEntry == null) { return document; } - if (document[nestedPropertyName] is JObject nestedDocument) - { - nestedDocument = nestedDocumentSource.UpdateDocument(nestedDocument, nestedEntry); - } - else - { - nestedDocument = nestedDocumentSource.CreateDocument(nestedEntry); - } + var embeddedDocument = embeddedDocumentSource.GetCurrentDocument(embeddedEntry); + embeddedDocument = embeddedDocument != null + ? embeddedDocumentSource.UpdateDocument(embeddedDocument, embeddedEntry) + : embeddedDocumentSource.CreateDocument(embeddedEntry); - if (nestedDocument != null) + if (embeddedDocument != null) { - document[nestedPropertyName] = nestedDocument; + document[embeddedPropertyName] = embeddedDocument; anyPropertyUpdated = true; } } else { var array = new JArray(); - foreach (var dependent in (IEnumerable)nestedValue) + foreach (var dependent in (IEnumerable)embeddedValue) { - var dependentEntry = ((InternalEntityEntry)entry).StateManager.TryGetEntry(dependent, fk.DeclaringEntityType); - if (dependentEntry == null) + var embeddedEntry = ((InternalEntityEntry)entry).StateManager.TryGetEntry(dependent, fk.DeclaringEntityType); + if (embeddedEntry == null) { continue; } - array.Add(_database.GetDocumentSource(dependentEntry.EntityType).CreateDocument(dependentEntry)); + var embeddedDocument = embeddedDocumentSource.GetCurrentDocument(embeddedEntry); + embeddedDocument = embeddedDocument != null + ? embeddedDocumentSource.UpdateDocument(embeddedDocument, embeddedEntry) ?? embeddedDocument + : embeddedDocumentSource.CreateDocument(embeddedEntry); + + array.Add(embeddedDocument); } - document[nestedPropertyName] = array; + document[embeddedPropertyName] = array; anyPropertyUpdated = true; } } @@ -199,6 +210,14 @@ public virtual JObject UpdateDocument(JObject document, IUpdateEntry entry) return anyPropertyUpdated ? document : null; } + public virtual JObject GetCurrentDocument(IUpdateEntry entry) + { + var document = _jObjectProperty != null + ? (JObject)(entry.SharedIdentityEntry ?? entry).GetCurrentValue(_jObjectProperty) + : null; + return document; + } + private static JToken ConvertPropertyValue(IProperty property, object value) { if (value == null) diff --git a/src/EFCore.Cosmos/ValueGeneration/Internal/CosmosValueGeneratorSelector.cs b/src/EFCore.Cosmos/ValueGeneration/Internal/CosmosValueGeneratorSelector.cs new file mode 100644 index 00000000000..fdef196d1c0 --- /dev/null +++ b/src/EFCore.Cosmos/ValueGeneration/Internal/CosmosValueGeneratorSelector.cs @@ -0,0 +1,31 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using Microsoft.EntityFrameworkCore.Metadata; +using Microsoft.EntityFrameworkCore.ValueGeneration; +using Microsoft.EntityFrameworkCore.ValueGeneration.Internal; + +namespace Microsoft.EntityFrameworkCore.Cosmos.ValueGeneration.Internal +{ + public class CosmosValueGeneratorSelector : ValueGeneratorSelector + { + public CosmosValueGeneratorSelector(ValueGeneratorSelectorDependencies dependencies) + : base(dependencies) + { + } + + public override ValueGenerator Create(IProperty property, IEntityType entityType) + { + var type = property.ClrType.UnwrapNullableType().UnwrapEnumType(); + + if (property.GetCosmosPropertyName() == "" + && type == typeof(int)) + { + return new TemporaryIntValueGenerator(); + } + + return base.Create(property, entityType); + } + } +} diff --git a/src/EFCore.Cosmos/ValueGeneration/Internal/IdValueGenerator.cs b/src/EFCore.Cosmos/ValueGeneration/Internal/IdValueGenerator.cs index 68fe75bc8de..b0c3d7cc17d 100644 --- a/src/EFCore.Cosmos/ValueGeneration/Internal/IdValueGenerator.cs +++ b/src/EFCore.Cosmos/ValueGeneration/Internal/IdValueGenerator.cs @@ -4,7 +4,6 @@ using System.Collections; using System.Linq; using System.Text; -using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.ChangeTracking; using Microsoft.EntityFrameworkCore.Cosmos.Storage.Internal; using Microsoft.EntityFrameworkCore.ValueGeneration; diff --git a/src/EFCore.Relational/Query/Internal/AsyncQueryingEnumerable.cs b/src/EFCore.Relational/Query/AsyncQueryingEnumerable.cs similarity index 100% rename from src/EFCore.Relational/Query/Internal/AsyncQueryingEnumerable.cs rename to src/EFCore.Relational/Query/AsyncQueryingEnumerable.cs diff --git a/src/EFCore.Relational/Query/Internal/IncludeCompilingExpressionVisitor.cs b/src/EFCore.Relational/Query/IncludeCompilingExpressionVisitor.cs similarity index 94% rename from src/EFCore.Relational/Query/Internal/IncludeCompilingExpressionVisitor.cs rename to src/EFCore.Relational/Query/IncludeCompilingExpressionVisitor.cs index cc45e1abeeb..23b782eb681 100644 --- a/src/EFCore.Relational/Query/Internal/IncludeCompilingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/IncludeCompilingExpressionVisitor.cs @@ -13,7 +13,6 @@ using Microsoft.EntityFrameworkCore.Internal; using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Metadata.Internal; -using Microsoft.EntityFrameworkCore.Query.NavigationExpansion.Internal; namespace Microsoft.EntityFrameworkCore.Query { @@ -62,10 +61,11 @@ private class CustomShaperCompilingExpressionVisitor : ExpressionVisitor else { SetIsLoadedNoTracking(includingEntity, navigation); - if (relatedEntity is object) + if (relatedEntity != null) { fixup(includingEntity, relatedEntity); - if (inverseNavigation != null && !inverseNavigation.IsCollection()) + if (inverseNavigation != null + && !inverseNavigation.IsCollection()) { SetIsLoadedNoTracking(relatedEntity, inverseNavigation); } @@ -122,7 +122,7 @@ private class CustomShaperCompilingExpressionVisitor : ExpressionVisitor } if (StructuralComparisons.StructuralEqualityComparer.Equals( - innerKey, collectionMaterializationContext.SelfIdentifier)) + innerKey, collectionMaterializationContext.SelfIdentifier)) { // We don't need to materialize this entity but we may need to populate inner collections if any. innerShaper(queryContext, dbDataReader, (TRelatedEntity)collectionMaterializationContext.Current, resultCoordinator); @@ -184,10 +184,11 @@ private class CustomShaperCompilingExpressionVisitor : ExpressionVisitor } if (StructuralComparisons.StructuralEqualityComparer.Equals( - innerKey, collectionMaterializationContext.SelfIdentifier)) + innerKey, collectionMaterializationContext.SelfIdentifier)) { // We don't need to materialize this entity but we may need to populate inner collections if any. - innerShaper(queryContext, dbDataReader, (TIncludedEntity)collectionMaterializationContext.Current, resultCoordinator); + innerShaper( + queryContext, dbDataReader, (TIncludedEntity)collectionMaterializationContext.Current, resultCoordinator); return; } @@ -198,7 +199,8 @@ private class CustomShaperCompilingExpressionVisitor : ExpressionVisitor if (!trackingQuery) { fixup(entity, relatedEntity); - if (inverseNavigation != null && !inverseNavigation.IsCollection()) + if (inverseNavigation != null + && !inverseNavigation.IsCollection()) { SetIsLoadedNoTracking(relatedEntity, inverseNavigation); } @@ -260,7 +262,7 @@ private class CustomShaperCompilingExpressionVisitor : ExpressionVisitor Func parentIdentifier, Func outerIdentifier, IClrCollectionAccessor clrCollectionAccessor) - where TCollection :class, IEnumerable + where TCollection : class, IEnumerable { var collection = clrCollectionAccessor?.Create() ?? new List(); @@ -286,7 +288,8 @@ protected override Expression VisitExtension(Expression extensionExpression) { if (extensionExpression is IncludeExpression includeExpression) { - Debug.Assert(!includeExpression.Navigation.IsCollection(), + Debug.Assert( + !includeExpression.Navigation.IsCollection(), "Only reference include should be present in tree"); var entityClrType = includeExpression.EntityExpression.Type; var includingClrType = includeExpression.Navigation.DeclaringEntityType.ClrType; @@ -307,7 +310,8 @@ protected override Expression VisitExtension(Expression extensionExpression) Expression.Constant(includeExpression.Navigation), Expression.Constant(inverseNavigation, typeof(INavigation)), Expression.Constant( - GenerateFixup(includingClrType, relatedEntityClrType, includeExpression.Navigation, inverseNavigation).Compile()), + GenerateFixup( + includingClrType, relatedEntityClrType, includeExpression.Navigation, inverseNavigation).Compile()), Expression.Constant(_tracking)); } @@ -365,7 +369,8 @@ protected override Expression VisitExtension(Expression extensionExpression) collectionInitializingExpression.OuterIdentifier, QueryCompilationContext.QueryContextParameter, _dbDataReaderParameter).Compile()), - Expression.Constant(collectionInitializingExpression.Navigation?.GetCollectionAccessor(), + Expression.Constant( + collectionInitializingExpression.Navigation?.GetCollectionAccessor(), typeof(IClrCollectionAccessor))); } @@ -403,7 +408,8 @@ protected override Expression VisitExtension(Expression extensionExpression) Expression.Constant(((LambdaExpression)Visit(collectionShaper.InnerShaper)).Compile()), Expression.Constant(inverseNavigation, typeof(INavigation)), Expression.Constant( - GenerateFixup(entityClrType, relatedEntityClrType, collectionShaper.Navigation, inverseNavigation).Compile()), + GenerateFixup( + entityClrType, relatedEntityClrType, collectionShaper.Navigation, inverseNavigation).Compile()), Expression.Constant(_tracking)); } @@ -468,22 +474,18 @@ protected override Expression VisitExtension(Expression extensionExpression) ParameterExpression entity, ParameterExpression relatedEntity, INavigation navigation) - { - return entity.MakeMemberAccess(navigation.GetMemberInfo(forConstruction: false, forSet: true)) + => entity.MakeMemberAccess(navigation.GetMemberInfo(forConstruction: false, forSet: true)) .CreateAssignExpression(relatedEntity); - } private static Expression AddToCollectionNavigation( ParameterExpression entity, ParameterExpression relatedEntity, INavigation navigation) - { - return Expression.Call( + => Expression.Call( Expression.Constant(navigation.GetCollectionAccessor()), _collectionAccessorAddMethodInfo, entity, relatedEntity); - } private static readonly MethodInfo _collectionAccessorAddMethodInfo = typeof(IClrCollectionAccessor).GetTypeInfo() diff --git a/src/EFCore.Relational/Query/Internal/FromSqlParameterApplyingExpressionVisitor.cs b/src/EFCore.Relational/Query/Internal/FromSqlParameterApplyingExpressionVisitor.cs index 2a651205d30..9fa72e0f1df 100644 --- a/src/EFCore.Relational/Query/Internal/FromSqlParameterApplyingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/Internal/FromSqlParameterApplyingExpressionVisitor.cs @@ -19,16 +19,16 @@ private class FromSqlParameterApplyingExpressionVisitor : ExpressionVisitor private readonly IDictionary _visitedFromSqlExpressions = new Dictionary(ReferenceEqualityComparer.Instance); - private readonly ISqlExpressionFactory _SqlExpressionFactory; + private readonly ISqlExpressionFactory _sqlExpressionFactory; private readonly ParameterNameGenerator _parameterNameGenerator; private readonly IReadOnlyDictionary _parametersValues; public FromSqlParameterApplyingExpressionVisitor( - ISqlExpressionFactory _sqlExpressionFactory, + ISqlExpressionFactory sqlExpressionFactory, ParameterNameGenerator parameterNameGenerator, IReadOnlyDictionary parametersValues) { - _SqlExpressionFactory = _sqlExpressionFactory; + _sqlExpressionFactory = sqlExpressionFactory; _parameterNameGenerator = parameterNameGenerator; _parametersValues = parametersValues; } @@ -67,7 +67,7 @@ public override Expression Visit(Expression expression) new TypeMappedRelationalParameter( parameterName, parameterName, - _SqlExpressionFactory.GetTypeMappingForValue(parameterValues[i]), + _sqlExpressionFactory.GetTypeMappingForValue(parameterValues[i]), parameterValues[i]?.GetType().IsNullableType())); } } diff --git a/src/EFCore.Relational/Query/Internal/ParameterValueBasedSelectExpressionOptimizer.cs b/src/EFCore.Relational/Query/Internal/ParameterValueBasedSelectExpressionOptimizer.cs index c734ffce4d5..c76120f07bd 100644 --- a/src/EFCore.Relational/Query/Internal/ParameterValueBasedSelectExpressionOptimizer.cs +++ b/src/EFCore.Relational/Query/Internal/ParameterValueBasedSelectExpressionOptimizer.cs @@ -32,7 +32,6 @@ public SelectExpression Optimize(SelectExpression selectExpression, IReadOnlyDic _parameterNameGeneratorFactory.Create(), parametersValues).Visit(query); - return (SelectExpression)query; } } diff --git a/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs index f9b214adcd3..e4fa084e736 100644 --- a/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs @@ -5,6 +5,7 @@ using System.Diagnostics; using System.Linq; using System.Linq.Expressions; +using System.Reflection; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Internal; using Microsoft.EntityFrameworkCore.Metadata; @@ -86,7 +87,7 @@ public virtual SqlExpression TranslateAverage(Expression expression) return inputType == typeof(float) ? _sqlExpressionFactory.Convert( _sqlExpressionFactory.Function( - "AVG", new[] { sqlExpression }, typeof(double), null), + "AVG", new[] { sqlExpression }, typeof(double)), sqlExpression.Type, sqlExpression.TypeMapping) : (SqlExpression)_sqlExpressionFactory.Function( diff --git a/src/EFCore/Query/ExpressionPrinter.cs b/src/EFCore/Query/ExpressionPrinter.cs index 05d57675c5e..0aaa0f19b33 100644 --- a/src/EFCore/Query/ExpressionPrinter.cs +++ b/src/EFCore/Query/ExpressionPrinter.cs @@ -121,7 +121,7 @@ public virtual ExpressionPrinter DecrementIndent() private void Append([NotNull] string message) => _stringBuilder.Append(message); - private void AppendLine([NotNull] string message = "") + private void AppendLine([NotNull] string message) { if (RemoveFormatting) { @@ -467,7 +467,7 @@ protected override Expression VisitLambda(Expression lambdaExpression) foreach (var parameter in lambdaExpression.Parameters) { - var parameterName = parameter.Name ?? parameter.ToString(); + var parameterName = parameter.Name; if (!_parametersInScope.ContainsKey(parameter)) { @@ -478,7 +478,7 @@ protected override Expression VisitLambda(Expression lambdaExpression) if (parameter != lambdaExpression.Parameters.Last()) { - _stringBuilder.Append(" | "); + _stringBuilder.Append(", "); } } diff --git a/src/EFCore/Query/NavigationExpansion/Internal/NavigationExpansionHelpers.cs b/src/EFCore/Query/NavigationExpansion/Internal/NavigationExpansionHelpers.cs index e5e3ee3c91e..634652c860f 100644 --- a/src/EFCore/Query/NavigationExpansion/Internal/NavigationExpansionHelpers.cs +++ b/src/EFCore/Query/NavigationExpansion/Internal/NavigationExpansionHelpers.cs @@ -234,10 +234,7 @@ var transparentIdentifierCtorInfo AnonymousObject.AnonymousObjectCtor, Expression.NewArrayInit( typeof(object), - properties - .Select(p => Expression.Convert(CreatePropertyExpression(target, p, addNullCheck), typeof(object))) - .Cast() - .ToArray())); + properties.Select(p => Expression.Convert(CreatePropertyExpression(target, p, addNullCheck), typeof(object))))); private static Expression CreatePropertyExpression(Expression target, IProperty property, bool addNullCheck) { diff --git a/test/EFCore.Cosmos.FunctionalTests/NestedDocumentsTest.cs b/test/EFCore.Cosmos.FunctionalTests/EmbeddedDocumentsTest.cs similarity index 91% rename from test/EFCore.Cosmos.FunctionalTests/NestedDocumentsTest.cs rename to test/EFCore.Cosmos.FunctionalTests/EmbeddedDocumentsTest.cs index 264ea9982d0..60c98948cdd 100644 --- a/test/EFCore.Cosmos.FunctionalTests/NestedDocumentsTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/EmbeddedDocumentsTest.cs @@ -80,8 +80,7 @@ await using (var testDatabase = CreateTestStore()) } } - // #12086 - //[ConditionalFact] + [ConditionalFact] public virtual async Task Can_add_collection_dependent_to_owner() { await using (var testDatabase = CreateTestStore()) @@ -113,18 +112,14 @@ await using (var testDatabase = CreateTestStore()) addedAddress2 = new Address { Street = "Another", City = "Village" }; people[1].Addresses.Add(addedAddress2); - // Remove when issues #13578 and #13579 are fixed - var existingEntry = context.Attach(people[1].Addresses.First()); + var existingAddressEntry = context.Entry(people[1].Addresses.First()); - var secondPersonEntry = context.Entry(people[1]); - var json = secondPersonEntry.Property("__jObject").CurrentValue; + var addressJson = existingAddressEntry.Property("__jObject").CurrentValue; - var addresses = (JArray)json["Stored Addresses"]; - var jsonAddress = (JObject)addresses[0]; - Assert.Equal("Second", jsonAddress[nameof(Address.Street)]); - jsonAddress["unmappedId"] = 2; + Assert.Equal("Second", addressJson[nameof(Address.Street)]); + addressJson["unmappedId"] = 2; - secondPersonEntry.Property("__jObject").CurrentValue = json; + existingAddressEntry.Property("__jObject").CurrentValue = addressJson; addedAddress3 = new Address { Street = "Another", City = "City" }; var existingLastAddress = people[2].Addresses.Last(); @@ -132,10 +127,6 @@ await using (var testDatabase = CreateTestStore()) people[2].Addresses.Add(addedAddress3); people[2].Addresses.Add(existingLastAddress); - // Remove when issues #13578 and #13579 are fixed - context.Attach(people[2].Addresses.First()); - context.Attach(existingLastAddress); - context.SaveChanges(); } @@ -155,12 +146,13 @@ await using (var testDatabase = CreateTestStore()) Assert.Equal(addedAddress2.Street, addresses[1].Street); Assert.Equal(addedAddress2.City, addresses[1].City); - var json = context.Entry(people[1]).Property("__jObject").CurrentValue; - var jsonAddress = (JObject)((JArray)json["Stored Addresses"])[0]; - Assert.Equal("Second", jsonAddress[nameof(Address.Street)]); - // Uncomment when issue #13578 is fixed - //Assert.Equal(2, jsonAddress["unmappedId"]); - //Assert.Equal(2, jsonAddress.Count); + var existingAddressEntry = context.Entry(people[1].Addresses.First()); + + var addressJson = existingAddressEntry.Property("__jObject").CurrentValue; + + Assert.Equal("Second", addressJson[nameof(Address.Street)]); + Assert.Equal(4, addressJson.Count); + Assert.Equal(2, addressJson["unmappedId"]); addresses = people[2].Addresses.ToList(); Assert.Equal(3, addresses.Count); @@ -184,7 +176,8 @@ await using (var testDatabase = CreateTestStore()) { using (var context = CreateContext()) { - var firstOperator = context.Set().Select(v => v.Operator).OrderBy(o => o.VehicleName).AsNoTracking().First(); + var firstOperator = context.Set().OrderBy(o => o.Name).Select(v => v.Operator) + .AsNoTracking().First(); Assert.Equal("Albert Williams", firstOperator.Name); Assert.Null(firstOperator.Vehicle); diff --git a/test/EFCore.SqlServer.FunctionalTests/BuiltInDataTypesSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/BuiltInDataTypesSqlServerTest.cs index 03847fdb35c..f459f300d04 100644 --- a/test/EFCore.SqlServer.FunctionalTests/BuiltInDataTypesSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/BuiltInDataTypesSqlServerTest.cs @@ -11,7 +11,6 @@ using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Storage; using Microsoft.EntityFrameworkCore.TestUtilities; -using Microsoft.EntityFrameworkCore.TestUtilities.Xunit; using Xunit; using Xunit.Abstractions;