Comsos Provider EntityEntry.ReloadAsync Non-Functional

[TheFanatr]

Using the Microsoft.EntityFrameworkCore.Cosmos nightly alpha of the master branch at 5.0.0-alpha1.19551.4 results in a bug when reloading data via EntityEntry.ReloadAsync. The following exception is thrown.

System.InvalidOperationException
  HResult=0x80131509
  Message=The model must be finalized before 'GetTypeMapping' can be used. Ensure that either 'OnModelCreating' has completed or, if using a stand-alone 'ModelBuilder', that 'FinalizeModel' has been called.
  Source=Microsoft.EntityFrameworkCore
  StackTrace:
   at Microsoft.EntityFrameworkCore.PropertyExtensions.GetTypeMapping(IProperty property)
   at Microsoft.EntityFrameworkCore.Cosmos.Query.Internal.KeyAccessExpression..ctor(IProperty property, Expression accessExpression)
   at Microsoft.EntityFrameworkCore.Cosmos.Query.Internal.EntityProjectionExpression.BindProperty(IProperty property, Boolean clientEval)
   at Microsoft.EntityFrameworkCore.Cosmos.Query.Internal.EntityProjectionExpression.BindMember(MemberIdentity member, Type entityClrType, Boolean clientEval, IPropertyBase& propertyBase)
   at Microsoft.EntityFrameworkCore.Cosmos.Query.Internal.EntityProjectionExpression.BindMember(String name, Type entityClrType, Boolean clientEval, IPropertyBase& propertyBase)
   at Microsoft.EntityFrameworkCore.Cosmos.Query.Internal.CosmosSqlTranslatingExpressionVisitor.TryBindMember(Expression source, MemberIdentity member, Expression& expression)
   at Microsoft.EntityFrameworkCore.Cosmos.Query.Internal.CosmosSqlTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
   at System.Linq.Expressions.MethodCallExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Cosmos.Query.Internal.CosmosSqlTranslatingExpressionVisitor.VisitUnary(UnaryExpression unaryExpression)
   at System.Linq.Expressions.UnaryExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Cosmos.Query.Internal.CosmosSqlTranslatingExpressionVisitor.VisitUnary(UnaryExpression unaryExpression)
   at System.Linq.Expressions.UnaryExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Cosmos.Query.Internal.CosmosSqlTranslatingExpressionVisitor.Translate(Expression expression)
   at Microsoft.EntityFrameworkCore.Cosmos.Query.Internal.CosmosProjectionBindingExpressionVisitor.Visit(Expression expression)
   at System.Linq.Expressions.ExpressionVisitor.Visit(ReadOnlyCollection`1 nodes)
   at System.Linq.Expressions.ExpressionVisitor.VisitNewArray(NewArrayExpression node)
   at System.Linq.Expressions.NewArrayExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Cosmos.Query.Internal.CosmosProjectionBindingExpressionVisitor.Visit(Expression expression)
   at Microsoft.EntityFrameworkCore.Cosmos.Query.Internal.CosmosProjectionBindingExpressionVisitor.Translate(SelectExpression selectExpression, Expression expression)
   at Microsoft.EntityFrameworkCore.Cosmos.Query.Internal.CosmosQueryableMethodTranslatingExpressionVisitor.TranslateSelect(ShapedQueryExpression source, LambdaExpression selector)
   at Microsoft.EntityFrameworkCore.Query.QueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
   at System.Linq.Expressions.MethodCallExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.QueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
   at System.Linq.Expressions.MethodCallExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.QueryCompilationContext.CreateQueryExecutor[TResult](Expression query)
   at Microsoft.EntityFrameworkCore.Storage.Database.CompileQuery[TResult](Expression query, Boolean async)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.CompileQueryCore[TResult](IDatabase database, Expression query, IModel model, Boolean async)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.<>c__DisplayClass12_0`1.<ExecuteAsync>b__0()
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQueryCore[TFunc](Object cacheKey, Func`1 compiler)
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQuery[TResult](Object cacheKey, Func`1 compiler)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.ExecuteAsync[TResult](Expression query, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryProvider.ExecuteAsync[TResult](Expression expression, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.ExecuteAsync[TSource,TResult](MethodInfo operatorMethodInfo, IQueryable`1 source, Expression expression, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.ExecuteAsync[TSource,TResult](MethodInfo operatorMethodInfo, IQueryable`1 source, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.FirstOrDefaultAsync[TSource](IQueryable`1 source, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Internal.EntityFinder`1.GetDatabaseValuesAsync(InternalEntityEntry entry, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.ChangeTracking.EntityEntry.<GetDatabaseValuesAsync>d__39.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
   at Microsoft.EntityFrameworkCore.ChangeTracking.EntityEntry.<ReloadAsync>d__41.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.GetResult()
   at EFCoreAzureCosmosDBExperiments.Program.<Main>d__3.MoveNext() in C:\Users\alexf\Source\Repos\EFCoreAzureCosmosDBExperiments\EFCoreAzureCosmosDBExperiments\Program.cs:line 44

Steps to reproduce

Database Context

public partial class CosmosContext : DbContext
{
    static LoggerFactory DebugLoggerFactory { get; } = new LoggerFactory(new[] { new DebugLoggerProvider { } });

    public virtual DbSet<Item> Items { get; set; }

    public virtual DbSet<User> Users { get; set; }

    public virtual DbSet<Heart> Hearts { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        if (!optionsBuilder.IsConfigured)
            optionsBuilder.UseLoggerFactory(DebugLoggerFactory).
                EnableSensitiveDataLogging().
                UseCosmos("https://localhost:8081", "C2y6yDjf5/R+ob0N8A7Cgv30VRDJIWEHLM+4QDU5DE2nQ9nDuVTqobD4b8mGGyPMbIZnqyMsEcaGQy67XIw/Jw==", "CosmosDBExperiments");
    }
}

Models

public abstract class Model
{
    [Key]
    public Guid ID { get; set; }
}

public partial class User : Model
{
    [Flags]
    public enum Characteristics
    {
        None,
        Administrator = 1 << 0,
        Resigned = 1 << 1
    }

    public string Name { get; set; }

    public string Handle { get; set; }

    public string Tagline { get; set; }

    public List<Item> Items { get; set; }

    public Characteristics Flags { get; set; }

    public Heart Heart { get; set; }
}

public partial class Item : Model
{
    public string Name { get; set; }

    public string Description { get; set; }

    public virtual User Owner { get; set; }
}

public class Heart : Model
{
    [Flags]
    public enum Colors
    {
        None = 0,
        Purple = 1 << 0,
        Blue = 1 << 1,
        Black = 1 << 2,
        Red = 1 << 3
    }

    [Flags]
    public enum Status
    {
        None = 0,
        Operational = 1 << 0,
        Failing = 1 << 1,
        Failed = 1 << 2,
        Detached = 1 << 3
    }

    public int Health { get; set; } = 100;

    public Status State { get; set; } = Status.Operational;

    public Colors Appearance { get; set; } = Colors.Red;
}

Console Program Class

using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.ChangeTracking;
using Microsoft.EntityFrameworkCore.Metadata.Conventions;
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Reflection;
using System.Text;
using System.Threading.Tasks;

namespace EFCoreAzureCosmosDBExperiments
{
    static class Program
    {
        static Random Generator { get; } = new Random { };

        async static Task Main()
        {
            User target = default;
            {
                using CosmosContext initializer = new CosmosContext { };
                await initializer.Database.EnsureCreatedAsync();

                target = await initializer.Users.FirstOrDefaultAsync() ?? await GenerateUser(initializer);

                if (initializer.Items.Count() == 0)
                {
                    await initializer.AddRangeAsync(Enumerable.Range(0, 100).Select(number => new Item { Name = $"Item #{number}", Owner = target, Description = Encoding.UTF8.GetString(Enumerable.Range(0, 250).Select(_ => (byte)Generator.Next(97, 123)).ToArray()) }));
                    await initializer.SaveChangesAsync();
                }
            }

            using CosmosContext context = new CosmosContext { };
            await context.AttachKeyed(target).ReloadAsync();

            Debug.Assert(target.Heart is { });

            static async Task<User> GenerateUser(CosmosContext context)
            {
                EntityEntry<User> entry = context.Add(new User
                {
                    Name = "Reginald Testerson",
                    Flags = User.Characteristics.Resigned | User.Characteristics.Administrator,
                    Tagline = "I eat hamburgers.",
                    Heart = new Heart
                    { 
                        Health = 50,
                        Appearance = Heart.Colors.Purple | Heart.Colors.Red,
                        State = Heart.Status.Failing
                    }
                });

                await context.SaveChangesAsync();
                return entry.Entity;
            }
        }

        /// <summary>
        /// Attaches an entity to a database context using a workaround for the shadow alternate key value generation bug reported in #15289.
        /// </summary>
        static EntityEntry<TItem> AttachKeyed<TContext, TItem>(this TContext context, TItem item) where TContext : DbContext where TItem : Model => context.Track(item, EntityState.Unchanged);

        /// <summary>
        /// Uses a workaround for the shadow alternate key value generation bug reported in #15289 to track an entity on a target database context.
        /// </summary>
        static EntityEntry<TItem> Track<TContext, TItem>(this TContext context, TItem item, EntityState targetState) where TContext : DbContext where TItem : Model
        {
            EntityEntry<TItem> itemEntry = context.Entry(item);
            (itemEntry.Property("id").CurrentValue, itemEntry.State) = ($"{nameof(Item)}|{item.ID}", targetState);
            return itemEntry;
        }
    }
}

Further technical details

Version: 5.0.0-alpha1.19551.4
Provider: Microsoft.EntityFrameworkCore.Cosmos
Target: .NET Core 5.0.100-alpha1-015521
Operating System: Microsoft Windows 1903 Build 18362.418
Development Environment: Visual Studio 2019 Community 16.3.7

[AndriySvyryd]

Note to implementor: the Reload query contains non-persisted properties, they can be filtered out in a Cosmos-specific IEntityFinder or in Core when #14121 is implemented

[alexeymarkov]

Is there any workaround I could use for handling optimistic concurrency?
I could set an entity to Detached and load it again by FindAsync but it corrupts changed navigation properties.

[ajcvickers]

@alexeymarkov You could try running a no-tracking query to get the database values and then apply these values to the already tracked entity.

[alexeymarkov]

@ajcvickers I ended up with the solution
`

var entry = context.Entry(entity);

foreach (var navigation in entry.Navigations)
{
	navigation.IsModified = false;
}

entry.State = EntityState.Detached;

var keys = entry.Metadata.FindPrimaryKey().Properties;
var keyValues = keys.Select(x => entry.CurrentValues[x]).ToArray();

entity = await context.FindAsync<TEntity>(keyValues, cancellationToken)

`

I'm not sure it will work for all cases but it works for me where I change a navigation property, try to call SaveChangesAsync, it fails and I need to reload the entity.

If there is a better workaround let me know (with a code snippet if possible).

BTW: for failed to save navigation properties I do not get DbUpdateConcurrencyException but CosmosException which seems to be a bug.

[AndriySvyryd]

We also need #17670 to be able to fully support this

[TheFanatr]

I made a workaround just around over a year ago where if you comment out the contents of ProcessJObjectProperty in KeyStoreConvention, this problem disappears. An equivalent fix that keeps __jObject as a shadow property is commenting out the parts in the expression visitors where it detects if a KeyAccessExpression has a blank key (c[“”]) and returns null instead of the expression or where it detects a similar thing for collections (unnecessary but for consistency) and also returns null. This fix is possible because c[“”] gets ignored or removed later anyways. But in order to apply this fix, you need to add a type mapping for the JObject type or for the __jObject property itself in StoreKeyConvention, because at some point something calls GetTypeMapping on the property, causing that model finalization error from the original stack trace if there is no type mapping to get.

[TheFanatr]

If option 2 is good enough for a PR, I can make it fairly trivially.

[AndriySvyryd]

@TheFanatr That would fix this particular issue, but it might also make other queries that use non-persisted properties like the one in #17670 appear to work, though they would return incorrect results.

We would accept a PR where you filter out just __jObject (StoreKeyConvention.JObjectPropertyName). You can provide the TypeMapping for it in CosmosTypeMappingSource.FindMapping

[TheFanatr]

@AndriySvyryd I will work on that then. I have the second option I suggested earlier fully implemented, type mapping and all, so this shouldn’t be too difficult.

[AndriySvyryd]

I meant filtering it out in the expression visitors. We definitely need the __jObject property to still exist

[TheFanatr]

I finished this a long time ago, but never got around to creating a pull request because the branch I am working in contains a lot of other fixes, and I really do not want to have to separate them out. This includes a fix for #15289, as well as a host of other issues I ran into, as well as support for entity members of type Dictionary<TKey, TValue>, IDictionary<TKey, TValue>, ICollection<T>, ISet<T>, HashSet<T>, IList<T>, List<T>, and Array<T> for every single combination of primitives there is, as well as Guids, TimeSpans, and DateTimes, including support for rich queries of these types (example: await context.Items.FirstOrDefaultAsync(item => item.GuidTimeSpanDictionary[myGuid] == myTimeSpan)). This also comes to virtually no runtime performance detriment because I didn't use reflection to generate the type maps nor convert the values. Can I just submit this as part of one large pull request?

[AndriySvyryd]

@TheFanatr Mixing fixes will make the PR process longer. I'd recommend separating them out as we'll stop accepting external PRs for 5.0 at some point this month.

[AndriySvyryd]

August 25th is effectively the last day for an external PR to be merged

[TheFanatr]

Ok, I'll have them in before then. A recent commit broke the indexer use translation, but I fixed it. Will separate them out now; individual branches for #18710 and #15289, and another the use of dictionaries and collections as children of entities as well as the translation of indexer uses on said children.

[TheFanatr]

What branch do I base these on? release/5.0 is the default, but this is labelled with consider-for-next-release.

[TheFanatr]

Will do release/5.0 for now.