From 8bfaf8ad35c3fd5d194bcd37a90ca45bbd90d133 Mon Sep 17 00:00:00 2001 From: Alexander Fanat Date: Thu, 20 Aug 2020 08:46:57 -0700 Subject: [PATCH 1/6] Fix #18710 via proceeding with translation of non-persisted JObject property. --- .../Query/Internal/EntityProjectionExpression.cs | 1 + .../Storage/Internal/CosmosTypeMappingSource.cs | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/EFCore.Cosmos/Query/Internal/EntityProjectionExpression.cs b/src/EFCore.Cosmos/Query/Internal/EntityProjectionExpression.cs index fd7c02e182d..8a8a88adafd 100644 --- a/src/EFCore.Cosmos/Query/Internal/EntityProjectionExpression.cs +++ b/src/EFCore.Cosmos/Query/Internal/EntityProjectionExpression.cs @@ -128,6 +128,7 @@ public virtual Expression BindProperty([NotNull] IProperty property, bool client } if (!clientEval + && property.Name != EntityFrameworkCore.Metadata.Conventions.StoreKeyConvention.JObjectPropertyName && expression.Name.Length == 0) { // Non-persisted property can't be translated diff --git a/src/EFCore.Cosmos/Storage/Internal/CosmosTypeMappingSource.cs b/src/EFCore.Cosmos/Storage/Internal/CosmosTypeMappingSource.cs index 8ace7c1eab1..07fa2bada71 100644 --- a/src/EFCore.Cosmos/Storage/Internal/CosmosTypeMappingSource.cs +++ b/src/EFCore.Cosmos/Storage/Internal/CosmosTypeMappingSource.cs @@ -3,9 +3,13 @@ using System; using System.Collections.Generic; +using System.Collections.ObjectModel; +using System.Linq; +using System.Linq.Expressions; using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.ChangeTracking; using Microsoft.EntityFrameworkCore.Storage; +using Microsoft.EntityFrameworkCore.Storage.ValueConversion; using Microsoft.EntityFrameworkCore.Utilities; using Newtonsoft.Json.Linq; @@ -33,7 +37,8 @@ public CosmosTypeMappingSource([NotNull] TypeMappingSourceDependencies dependenc _clrTypeMappings = new Dictionary { - { typeof(byte[]), new CosmosTypeMapping(typeof(byte[]), keyComparer: new ArrayStructuralComparer()) } + { typeof(byte[]), new CosmosTypeMapping(typeof(byte[]), keyComparer: new ArrayStructuralComparer()) }, + { typeof(JObject), new CosmosTypeMapping(typeof(JObject)) } }; } From e75996402b53efeb4eb23b1e66b68a08448333b2 Mon Sep 17 00:00:00 2001 From: Alexander Fanat Date: Thu, 20 Aug 2020 11:26:56 -0700 Subject: [PATCH 2/6] Add test. --- .../ReloadTest.cs | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 test/EFCore.Cosmos.FunctionalTests/ReloadTest.cs diff --git a/test/EFCore.Cosmos.FunctionalTests/ReloadTest.cs b/test/EFCore.Cosmos.FunctionalTests/ReloadTest.cs new file mode 100644 index 00000000000..2e24ede0368 --- /dev/null +++ b/test/EFCore.Cosmos.FunctionalTests/ReloadTest.cs @@ -0,0 +1,45 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Microsoft.EntityFrameworkCore.Storage; +using Microsoft.EntityFrameworkCore.TestUtilities; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Xunit; +using static Microsoft.EntityFrameworkCore.Cosmos.Storage.Internal.CosmosDatabaseCreatorTest; + +namespace Microsoft.EntityFrameworkCore.Cosmos +{ + public class ReloadTest + { + public static IEnumerable IsAsyncData = new[] + { + new object[] { true }, + new object[] { false } + }; + + [Theory] + [MemberData(nameof(IsAsyncData))] + public async Task Entity_reference_can_be_reloaded(bool async) + { + await using var testDatabase = CosmosTestStore.CreateInitialized("Database"); + + using var context = new BloggingContext(testDatabase); + await context.Database.EnsureCreatedAsync(); + + var entry = context.Add(new Blog { Id = 1337 }); + + context.SaveChanges(); + + if (async) + { + await entry.ReloadAsync(); + } + else + { + entry.Reload(); + } + } + } +} From a3446ea5ee1a6aa25a90bffccdbc8b40d9666024 Mon Sep 17 00:00:00 2001 From: Alexander Fanat Date: Thu, 20 Aug 2020 11:34:21 -0700 Subject: [PATCH 3/6] Move test out of Internal folder, and use SaveChangesAsync. --- test/EFCore.Cosmos.FunctionalTests/ReloadTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/EFCore.Cosmos.FunctionalTests/ReloadTest.cs b/test/EFCore.Cosmos.FunctionalTests/ReloadTest.cs index 2e24ede0368..bff6fce7faa 100644 --- a/test/EFCore.Cosmos.FunctionalTests/ReloadTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/ReloadTest.cs @@ -30,7 +30,7 @@ public async Task Entity_reference_can_be_reloaded(bool async) var entry = context.Add(new Blog { Id = 1337 }); - context.SaveChanges(); + await context.SaveChangesAsync(); if (async) { From 9404878eef8c3cf1435ae0e2644fa943da5d525f Mon Sep 17 00:00:00 2001 From: Alexander Fanat Date: Fri, 21 Aug 2020 00:58:48 -0700 Subject: [PATCH 4/6] Make KeyAccessExpression translate to the document element if no name is provided, and update ReloadTest to verify that __jObject is updated after reloads. --- .../Query/Internal/KeyAccessExpression.cs | 4 +- .../ReloadTest.cs | 56 +++++++++++++++---- 2 files changed, 49 insertions(+), 11 deletions(-) diff --git a/src/EFCore.Cosmos/Query/Internal/KeyAccessExpression.cs b/src/EFCore.Cosmos/Query/Internal/KeyAccessExpression.cs index ff6c3c52bcc..5e87156901d 100644 --- a/src/EFCore.Cosmos/Query/Internal/KeyAccessExpression.cs +++ b/src/EFCore.Cosmos/Query/Internal/KeyAccessExpression.cs @@ -101,7 +101,9 @@ protected override void Print(ExpressionPrinter expressionPrinter) /// 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 string ToString() => $"{AccessExpression}[\"{Name}\"]"; + public override string ToString() => Name?.Length > 0 + ? $"{AccessExpression}[\"{Name}\"]" + : $"{AccessExpression}"; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to diff --git a/test/EFCore.Cosmos.FunctionalTests/ReloadTest.cs b/test/EFCore.Cosmos.FunctionalTests/ReloadTest.cs index bff6fce7faa..24510ce0fc8 100644 --- a/test/EFCore.Cosmos.FunctionalTests/ReloadTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/ReloadTest.cs @@ -1,13 +1,8 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; +using System.Collections.Generic; using System.Threading.Tasks; -using Microsoft.EntityFrameworkCore.Storage; using Microsoft.EntityFrameworkCore.TestUtilities; -using Microsoft.EntityFrameworkCore.Infrastructure; using Xunit; -using static Microsoft.EntityFrameworkCore.Cosmos.Storage.Internal.CosmosDatabaseCreatorTest; +using Newtonsoft.Json.Linq; namespace Microsoft.EntityFrameworkCore.Cosmos { @@ -19,19 +14,22 @@ public class ReloadTest new object[] { false } }; - [Theory] + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public async Task Entity_reference_can_be_reloaded(bool async) { await using var testDatabase = CosmosTestStore.CreateInitialized("Database"); - using var context = new BloggingContext(testDatabase); + using var context = new ReloadTestContext(testDatabase); await context.Database.EnsureCreatedAsync(); - var entry = context.Add(new Blog { Id = 1337 }); + var entry = context.Add(new Item { Id = 1337 }); await context.SaveChangesAsync(); + var itemJson = entry.Property("__jObject").CurrentValue; + itemJson["unmapped"] = 2; + if (async) { await entry.ReloadAsync(); @@ -40,6 +38,44 @@ public async Task Entity_reference_can_be_reloaded(bool async) { entry.Reload(); } + + itemJson = entry.Property("__jObject").CurrentValue; + Assert.Null(itemJson["unmapped"]); + } + + public class ReloadTestContext : DbContext + { + private readonly string _connectionUri; + private readonly string _authToken; + private readonly string _name; + + public ReloadTestContext(CosmosTestStore testStore) + { + _connectionUri = testStore.ConnectionUri; + _authToken = testStore.AuthToken; + _name = testStore.Name; + } + + protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) + { + optionsBuilder + .UseCosmos( + _connectionUri, + _authToken, + _name, + b => b.ApplyConfiguration()); + } + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + } + + public DbSet Items { get; set; } + } + + public class Item + { + public int Id { get; set; } } } } From b787607d5ef23883de47d24e9310a60e9a4487f3 Mon Sep 17 00:00:00 2001 From: Alex Fanat Date: Fri, 21 Aug 2020 15:57:08 -0700 Subject: [PATCH 5/6] Rename test database to ReloadTest. Co-authored-by: Andriy Svyryd --- test/EFCore.Cosmos.FunctionalTests/ReloadTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/EFCore.Cosmos.FunctionalTests/ReloadTest.cs b/test/EFCore.Cosmos.FunctionalTests/ReloadTest.cs index 24510ce0fc8..9534df5beaa 100644 --- a/test/EFCore.Cosmos.FunctionalTests/ReloadTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/ReloadTest.cs @@ -18,7 +18,7 @@ public class ReloadTest [MemberData(nameof(IsAsyncData))] public async Task Entity_reference_can_be_reloaded(bool async) { - await using var testDatabase = CosmosTestStore.CreateInitialized("Database"); + await using var testDatabase = CosmosTestStore.CreateInitialized("ReloadTest"); using var context = new ReloadTestContext(testDatabase); await context.Database.EnsureCreatedAsync(); From b37b46a4317bcbce7288de0c87b2493a9f8b24f7 Mon Sep 17 00:00:00 2001 From: Alexander Fanat Date: Sat, 22 Aug 2020 01:25:23 -0700 Subject: [PATCH 6/6] Add comments to describe condition for modification removal. --- src/EFCore.Cosmos/Query/Internal/EntityProjectionExpression.cs | 2 ++ src/EFCore.Cosmos/Query/Internal/KeyAccessExpression.cs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/EFCore.Cosmos/Query/Internal/EntityProjectionExpression.cs b/src/EFCore.Cosmos/Query/Internal/EntityProjectionExpression.cs index 8a8a88adafd..aba843e795a 100644 --- a/src/EFCore.Cosmos/Query/Internal/EntityProjectionExpression.cs +++ b/src/EFCore.Cosmos/Query/Internal/EntityProjectionExpression.cs @@ -128,6 +128,8 @@ public virtual Expression BindProperty([NotNull] IProperty property, bool client } if (!clientEval + // TODO: Remove once __jObject is translated to the access root in a better fashion and + // would not otherwise be found to be non-translatable. See issues #17670 and #14121. && property.Name != EntityFrameworkCore.Metadata.Conventions.StoreKeyConvention.JObjectPropertyName && expression.Name.Length == 0) { diff --git a/src/EFCore.Cosmos/Query/Internal/KeyAccessExpression.cs b/src/EFCore.Cosmos/Query/Internal/KeyAccessExpression.cs index 5e87156901d..097af897885 100644 --- a/src/EFCore.Cosmos/Query/Internal/KeyAccessExpression.cs +++ b/src/EFCore.Cosmos/Query/Internal/KeyAccessExpression.cs @@ -103,6 +103,8 @@ protected override void Print(ExpressionPrinter expressionPrinter) /// public override string ToString() => Name?.Length > 0 ? $"{AccessExpression}[\"{Name}\"]" + // TODO: Remove once __jObject is translated to the access root in a better fashion. + // See issue #17670 and related issue #14121. : $"{AccessExpression}"; ///