From ea296154ed3a86aef4ba83ed9d8f6ae26bf6fc38 Mon Sep 17 00:00:00 2001 From: AndriySvyryd Date: Mon, 16 Sep 2019 19:00:19 -0700 Subject: [PATCH] Don't add discriminators for embedded entities Allow to update entities without discriminators Don't try to add the default discriminator if a conflicting property already exists Fixes #17764 Fixes #17302 Fixes #17814 --- .editorconfig | 4 +- .../CosmosDiscriminatorConvention.cs | 64 +- .../Internal/CosmosConventionSetBuilder.cs | 4 + .../Conventions/StoreKeyConvention.cs | 18 + .../Storage/Internal/CosmosDatabaseWrapper.cs | 16 +- src/EFCore/Infrastructure/ModelSource.cs | 3 - .../Internal/InternalEntityTypeBuilder.cs | 46 +- .../CustomConvertersCosmosTest.cs | 8 +- .../EmbeddedDocumentsTest.cs | 2 +- .../EndToEndCosmosTest.cs | 589 ++++++++++-------- .../CustomConvertersTestBase.cs | 23 +- 11 files changed, 484 insertions(+), 293 deletions(-) diff --git a/.editorconfig b/.editorconfig index 446dc4e610c..e2415615cb5 100644 --- a/.editorconfig +++ b/.editorconfig @@ -197,8 +197,8 @@ dotnet_naming_rule.interface_naming.symbols = interface_symbol dotnet_naming_rule.interface_naming.style = interface_style dotnet_naming_rule.interface_naming.severity = suggestion -# Classes, Structs, Enums, Properties, Methods, Events, Namespaces -dotnet_naming_symbols.class_symbol.applicable_kinds = class, struct, enum, property, method, event, namespace +# Classes, Structs, Enums, Properties, Methods, Local Functions, Events, Namespaces +dotnet_naming_symbols.class_symbol.applicable_kinds = class, struct, enum, property, method, local_function, event, namespace dotnet_naming_symbols.class_symbol.applicable_accessibilities = * dotnet_naming_rule.class_naming.symbols = class_symbol diff --git a/src/EFCore.Cosmos/Metadata/Conventions/CosmosDiscriminatorConvention.cs b/src/EFCore.Cosmos/Metadata/Conventions/CosmosDiscriminatorConvention.cs index 3629af2d2b7..896b2986967 100644 --- a/src/EFCore.Cosmos/Metadata/Conventions/CosmosDiscriminatorConvention.cs +++ b/src/EFCore.Cosmos/Metadata/Conventions/CosmosDiscriminatorConvention.cs @@ -1,8 +1,9 @@ // 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.Collections; +using System.Linq; using JetBrains.Annotations; +using Microsoft.EntityFrameworkCore.Cosmos.Metadata.Internal; using Microsoft.EntityFrameworkCore.Metadata.Builders; using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure; using Microsoft.EntityFrameworkCore.Utilities; @@ -12,7 +13,11 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Conventions /// /// A convention that configures the discriminator value for entity types as the entity type name. /// - public class CosmosDiscriminatorConvention : DiscriminatorConvention, IEntityTypeAddedConvention + public class CosmosDiscriminatorConvention : + DiscriminatorConvention, + IForeignKeyOwnershipChangedConvention, + IForeignKeyRemovedConvention, + IEntityTypeAddedConvention { /// /// Creates a new instance of . @@ -36,22 +41,56 @@ public virtual void ProcessEntityTypeAdded( Check.NotNull(context, nameof(context)); var entityType = entityTypeBuilder.Metadata; - if (entityTypeBuilder.Metadata.BaseType == null - && !Any(entityTypeBuilder.Metadata.GetDerivedTypes())) + if (entityType.BaseType == null + && !entityType.GetDerivedTypes().Any() + && entityType.IsDocumentRoot()) { entityTypeBuilder.HasDiscriminator(typeof(string)) - .HasValue(entityType, entityType.ShortName()); + ?.HasValue(entityType, entityType.ShortName()); } } - private static bool Any([NotNull] IEnumerable source) + /// + /// Called after the ownership value for a foreign key is changed. + /// + /// The builder for the foreign key. + /// Additional information associated with convention execution. + public virtual void ProcessForeignKeyOwnershipChanged( + IConventionRelationshipBuilder relationshipBuilder, + IConventionContext context) { - foreach (var _ in source) + Check.NotNull(relationshipBuilder, nameof(relationshipBuilder)); + Check.NotNull(context, nameof(context)); + + var entityType = relationshipBuilder.Metadata.DeclaringEntityType; + if (relationshipBuilder.Metadata.IsOwnership + && !entityType.IsDocumentRoot() + && entityType.BaseType == null + && !entityType.GetDerivedTypes().Any()) { - return true; + entityType.Builder.HasNoDeclaredDiscriminator(); } + } - return false; + /// + /// Called after a foreign key is removed. + /// + /// The builder for the entity type. + /// The removed foreign key. + /// Additional information associated with convention execution. + public virtual void ProcessForeignKeyRemoved( + IConventionEntityTypeBuilder entityTypeBuilder, + IConventionForeignKey foreignKey, + IConventionContext context) + { + var entityType = foreignKey.DeclaringEntityType; + if (foreignKey.IsOwnership + && !entityType.IsDocumentRoot() + && entityType.BaseType == null + && !entityType.GetDerivedTypes().Any()) + { + entityType.Builder.HasNoDeclaredDiscriminator(); + } } /// @@ -72,11 +111,14 @@ public override void ProcessEntityTypeBaseTypeChanged( return; } - IConventionDiscriminatorBuilder discriminator; + IConventionDiscriminatorBuilder discriminator = null; var entityType = entityTypeBuilder.Metadata; if (newBaseType == null) { - discriminator = entityTypeBuilder.HasDiscriminator(typeof(string)); + if (entityType.IsDocumentRoot()) + { + discriminator = entityTypeBuilder.HasDiscriminator(typeof(string)); + } } else { diff --git a/src/EFCore.Cosmos/Metadata/Conventions/Internal/CosmosConventionSetBuilder.cs b/src/EFCore.Cosmos/Metadata/Conventions/Internal/CosmosConventionSetBuilder.cs index 24a06e779ba..be639c2c472 100644 --- a/src/EFCore.Cosmos/Metadata/Conventions/Internal/CosmosConventionSetBuilder.cs +++ b/src/EFCore.Cosmos/Metadata/Conventions/Internal/CosmosConventionSetBuilder.cs @@ -49,6 +49,10 @@ public override ConventionSet CreateConventionSet() conventionSet.EntityTypeBaseTypeChangedConventions.Add(storeKeyConvention); ReplaceConvention(conventionSet.EntityTypeBaseTypeChangedConventions, (DiscriminatorConvention)discriminatorConvention); + conventionSet.ForeignKeyRemovedConventions.Add(discriminatorConvention); + conventionSet.ForeignKeyRemovedConventions.Add(storeKeyConvention); + + conventionSet.ForeignKeyOwnershipChangedConventions.Add(discriminatorConvention); conventionSet.ForeignKeyOwnershipChangedConventions.Add(storeKeyConvention); conventionSet.EntityTypeAnnotationChangedConventions.Add(storeKeyConvention); diff --git a/src/EFCore.Cosmos/Metadata/Conventions/StoreKeyConvention.cs b/src/EFCore.Cosmos/Metadata/Conventions/StoreKeyConvention.cs index 7125a467138..5f5e0b82a03 100644 --- a/src/EFCore.Cosmos/Metadata/Conventions/StoreKeyConvention.cs +++ b/src/EFCore.Cosmos/Metadata/Conventions/StoreKeyConvention.cs @@ -23,6 +23,7 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Conventions public class StoreKeyConvention : IEntityTypeAddedConvention, IForeignKeyOwnershipChangedConvention, + IForeignKeyRemovedConvention, IEntityTypeAnnotationChangedConvention, IEntityTypeBaseTypeChangedConvention { @@ -113,6 +114,23 @@ public virtual void ProcessForeignKeyOwnershipChanged( Process(relationshipBuilder.Metadata.DeclaringEntityType.Builder); } + /// + /// Called after a foreign key is removed. + /// + /// The builder for the entity type. + /// The removed foreign key. + /// Additional information associated with convention execution. + public virtual void ProcessForeignKeyRemoved( + IConventionEntityTypeBuilder entityTypeBuilder, + IConventionForeignKey foreignKey, + IConventionContext context) + { + if (foreignKey.IsOwnership) + { + Process(foreignKey.DeclaringEntityType.Builder); + } + } + /// /// Called after an annotation is changed on an entity type. /// diff --git a/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs b/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs index 9c25eb1fe89..9e3761876a0 100644 --- a/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs +++ b/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs @@ -204,8 +204,12 @@ private bool Save(IUpdateEntry entry) { document = documentSource.CreateDocument(entry); - document[entityType.GetDiscriminatorProperty().GetPropertyName()] = - JToken.FromObject(entityType.GetDiscriminatorValue(), CosmosClientWrapper.Serializer); + var propertyName = entityType.GetDiscriminatorProperty()?.GetPropertyName(); + if (propertyName != null) + { + document[propertyName] = + JToken.FromObject(entityType.GetDiscriminatorValue(), CosmosClientWrapper.Serializer); + } } return _cosmosClient.ReplaceItem( @@ -255,8 +259,12 @@ private Task SaveAsync(IUpdateEntry entry, CancellationToken cancellationT { document = documentSource.CreateDocument(entry); - document[entityType.GetDiscriminatorProperty().GetPropertyName()] = - JToken.FromObject(entityType.GetDiscriminatorValue(), CosmosClientWrapper.Serializer); + var propertyName = entityType.GetDiscriminatorProperty()?.GetPropertyName(); + if (propertyName != null) + { + document[propertyName] = + JToken.FromObject(entityType.GetDiscriminatorValue(), CosmosClientWrapper.Serializer); + } } return _cosmosClient.ReplaceItemAsync( diff --git a/src/EFCore/Infrastructure/ModelSource.cs b/src/EFCore/Infrastructure/ModelSource.cs index 80fdc8282cb..57a6a961ed1 100644 --- a/src/EFCore/Infrastructure/ModelSource.cs +++ b/src/EFCore/Infrastructure/ModelSource.cs @@ -1,9 +1,6 @@ // 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.Concurrent; -using System.Threading; using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure; diff --git a/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs b/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs index bb47e8b839e..12554cce8fd 100644 --- a/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs +++ b/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs @@ -655,6 +655,41 @@ private bool CanRemoveProperty( && (canOverrideSameSource || (configurationSource != currentConfigurationSource)); } + /// + /// 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 virtual bool CanAddProperty(Type propertyType, string name,ConfigurationSource? typeConfigurationSource) + { + var conflictingMember = Metadata.FindPropertiesInHierarchy(name).FirstOrDefault(); + if (conflictingMember != null + && conflictingMember.IsShadowProperty() + && conflictingMember.ClrType != propertyType + && typeConfigurationSource != null + && !typeConfigurationSource.Overrides(conflictingMember.GetTypeConfigurationSource())) + { + return false; + } + + if (Metadata.ClrType == null) + { + return true; + } + + var memberInfo = Metadata.ClrType.GetMembersInHierarchy(name).FirstOrDefault(); + if (memberInfo != null + && propertyType != memberInfo.GetMemberType() + && (memberInfo as PropertyInfo)?.IsEFIndexerProperty() != true + && typeConfigurationSource != null) + { + return false; + } + + return true; + } + /// /// 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 @@ -3415,11 +3450,18 @@ private bool CanSetDiscriminator( string name, Type discriminatorType, bool fromDataAnnotation) - => (name == null && discriminatorType == null) + => ((name == null && discriminatorType == null) || ((name == null || discriminatorProperty?.Name == name) && (discriminatorType == null || discriminatorProperty?.ClrType == discriminatorType)) || (fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention) - .Overrides(Metadata.GetDiscriminatorPropertyConfigurationSource()); + .Overrides(Metadata.GetDiscriminatorPropertyConfigurationSource())) + && (discriminatorProperty != null + || Metadata.RootType().Builder.CanAddProperty( + discriminatorType ?? _defaultDiscriminatorType, + name ?? _defaultDiscriminatorName, + typeConfigurationSource: discriminatorType != null + ? fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention + : (ConfigurationSource?)null)); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to diff --git a/test/EFCore.Cosmos.FunctionalTests/CustomConvertersCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/CustomConvertersCosmosTest.cs index c4c6a1aa8ef..fb2495d77b6 100644 --- a/test/EFCore.Cosmos.FunctionalTests/CustomConvertersCosmosTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/CustomConvertersCosmosTest.cs @@ -113,12 +113,6 @@ public override void Can_insert_and_query_struct_to_string_converter_for_pk() base.Can_insert_and_query_struct_to_string_converter_for_pk(); } - [ConditionalTheory(Skip = "Issue #17814")] - public override Task Can_query_custom_type_not_mapped_by_default_equality(bool isAsync) - { - return base.Can_query_custom_type_not_mapped_by_default_equality(isAsync); - } - public class CustomConvertersCosmosFixture : CustomConvertersFixtureBase { protected override ITestStoreFactory TestStoreFactory => CosmosTestStoreFactory.Instance; @@ -147,6 +141,8 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con shadowJObject.SetConfigurationSource(ConfigurationSource.Convention); var nullableShadowJObject = (Property)modelBuilder.Entity().Property("__jObject").Metadata; nullableShadowJObject.SetConfigurationSource(ConfigurationSource.Convention); + + modelBuilder.Entity(b => b.ToContainer("SimpleCounters")); } } } diff --git a/test/EFCore.Cosmos.FunctionalTests/EmbeddedDocumentsTest.cs b/test/EFCore.Cosmos.FunctionalTests/EmbeddedDocumentsTest.cs index b2673d1363e..79180c4784e 100644 --- a/test/EFCore.Cosmos.FunctionalTests/EmbeddedDocumentsTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/EmbeddedDocumentsTest.cs @@ -182,7 +182,7 @@ public virtual async Task Can_add_collection_dependent_to_owner() var addressJson = existingAddressEntry.Property("__jObject").CurrentValue; Assert.Equal("Second", addressJson[nameof(Address.Street)]); - Assert.Equal(4, addressJson.Count); + Assert.Equal(3, addressJson.Count); Assert.Equal(2, addressJson["unmappedId"]); addresses = people[2].Addresses.ToList(); diff --git a/test/EFCore.Cosmos.FunctionalTests/EndToEndCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/EndToEndCosmosTest.cs index 22c834cb039..c3136459978 100644 --- a/test/EFCore.Cosmos.FunctionalTests/EndToEndCosmosTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/EndToEndCosmosTest.cs @@ -23,101 +23,163 @@ public EndToEndCosmosTest(CosmosFixture fixture) Fixture = fixture; } - [ConditionalFact(Skip = "Issue#16146")] + [ConditionalFact] public async Task Can_add_update_delete_end_to_end() { - await using (var testDatabase = CosmosTestStore.CreateInitialized(DatabaseName)) - { - var options = Fixture.CreateOptions(testDatabase); + await using var testDatabase = CosmosTestStore.CreateInitialized(DatabaseName); + var options = Fixture.CreateOptions(testDatabase); - var customer = new Customer { Id = 42, Name = "Theon" }; + var customer = new Customer { Id = 42, Name = "Theon" }; - using (var context = new CustomerContext(options)) - { - context.Database.EnsureCreated(); + using (var context = new CustomerContext(options)) + { + context.Database.EnsureCreated(); - context.Add(customer); + context.Add(customer); - context.SaveChanges(); - } + context.SaveChanges(); + } - using (var context = new CustomerContext(options)) - { - var customerFromStore = context.Set().Single(); + using (var context = new CustomerContext(options)) + { + var customerFromStore = context.Set().Single(); - Assert.Equal(42, customerFromStore.Id); - Assert.Equal("Theon", customerFromStore.Name); + Assert.Equal(42, customerFromStore.Id); + Assert.Equal("Theon", customerFromStore.Name); - customerFromStore.Name = "Theon Greyjoy"; + customerFromStore.Name = "Theon Greyjoy"; - context.SaveChanges(); - } + context.SaveChanges(); + } - using (var context = new CustomerContext(options)) - { - var customerFromStore = context.Set().Single(); + using (var context = new CustomerContext(options)) + { + var customerFromStore = context.Set().Single(); - Assert.Equal(42, customerFromStore.Id); - Assert.Equal("Theon Greyjoy", customerFromStore.Name); + Assert.Equal(42, customerFromStore.Id); + Assert.Equal("Theon Greyjoy", customerFromStore.Name); - context.Remove(customerFromStore); + context.Remove(customerFromStore); - context.SaveChanges(); - } + context.SaveChanges(); + } - using (var context = new CustomerContext(options)) - { - Assert.Equal(0, context.Set().Count()); - } + using (var context = new CustomerContext(options)) + { + Assert.Empty(context.Set().ToList()); } } - [ConditionalFact(Skip = "Issue#16146")] + [ConditionalFact] public async Task Can_add_update_delete_end_to_end_async() { - await using (var testDatabase = CosmosTestStore.CreateInitialized(DatabaseName)) + await using var testDatabase = CosmosTestStore.CreateInitialized(DatabaseName); + var options = Fixture.CreateOptions(testDatabase); + + var customer = new Customer { Id = 42, Name = "Theon" }; + + using (var context = new CustomerContext(options)) + { + await context.Database.EnsureCreatedAsync(); + + context.Add(customer); + + await context.SaveChangesAsync(); + } + + using (var context = new CustomerContext(options)) + { + var customerFromStore = await context.Set().SingleAsync(); + + Assert.Equal(42, customerFromStore.Id); + Assert.Equal("Theon", customerFromStore.Name); + + customerFromStore.Name = "Theon Greyjoy"; + + await context.SaveChangesAsync(); + } + + using (var context = new CustomerContext(options)) + { + var customerFromStore = await context.Set().SingleAsync(); + + Assert.Equal(42, customerFromStore.Id); + Assert.Equal("Theon Greyjoy", customerFromStore.Name); + + context.Remove(customerFromStore); + + await context.SaveChangesAsync(); + } + + using (var context = new CustomerContext(options)) + { + Assert.Empty(await context.Set().ToListAsync()); + } + } + + [ConditionalFact] + public async Task Can_add_update_delete_detached_entity_end_to_end_async() + { + await using var testDatabase = CosmosTestStore.CreateInitialized(DatabaseName); + var options = Fixture.CreateOptions(testDatabase); + + var customer = new Customer { Id = 42, Name = "Theon" }; + string storeId = null; + using (var context = new CustomerContext(options)) { - var options = Fixture.CreateOptions(testDatabase); + await context.Database.EnsureCreatedAsync(); - var customer = new Customer { Id = 42, Name = "Theon" }; + var entry = context.Add(customer); - using (var context = new CustomerContext(options)) - { - await context.Database.EnsureCreatedAsync(); + await context.SaveChangesAsync(); - context.Add(customer); + context.Add(customer); - await context.SaveChangesAsync(); - } + storeId = entry.Property("id").CurrentValue; + } - using (var context = new CustomerContext(options)) - { - var customerFromStore = context.Set().Single(); + Assert.NotNull(storeId); - Assert.Equal(42, customerFromStore.Id); - Assert.Equal("Theon", customerFromStore.Name); + using (var context = new CustomerContext(options)) + { + var customerFromStore = context.Set().Single(); - customerFromStore.Name = "Theon Greyjoy"; + Assert.Equal(42, customerFromStore.Id); + Assert.Equal("Theon", customerFromStore.Name); + } - await context.SaveChangesAsync(); - } + using (var context = new CustomerContext(options)) + { + customer.Name = "Theon Greyjoy"; + + var entry = context.Entry(customer); + entry.Property("id").CurrentValue = storeId; + + entry.State = EntityState.Modified; + + await context.SaveChangesAsync(); + } - using (var context = new CustomerContext(options)) - { - var customerFromStore = context.Set().Single(); + using (var context = new CustomerContext(options)) + { + var customerFromStore = context.Set().Single(); - Assert.Equal(42, customerFromStore.Id); - Assert.Equal("Theon Greyjoy", customerFromStore.Name); + Assert.Equal(42, customerFromStore.Id); + Assert.Equal("Theon Greyjoy", customerFromStore.Name); + } - context.Remove(customerFromStore); + using (var context = new CustomerContext(options)) + { + var entry = context.Entry(customer); + entry.Property("id").CurrentValue = storeId; + entry.State = EntityState.Deleted; - await context.SaveChangesAsync(); - } + await context.SaveChangesAsync(); + } - using (var context = new CustomerContext(options)) - { - Assert.Equal(0, await context.Set().CountAsync()); - } + using (var context = new CustomerContext(options)) + { + Assert.Empty(context.Set().ToList()); } } @@ -141,187 +203,177 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) } } - [ConditionalFact(Skip = "Issue #16146")] - public async Task Can_add_update_delete_detached_entity_end_to_end_async() + [ConditionalFact] + public async Task Can_add_update_delete_end_to_end_with_partition_key() { - await using (var testDatabase = CosmosTestStore.CreateInitialized(DatabaseName)) - { - var options = Fixture.CreateOptions(testDatabase); + await using var testDatabase = CosmosTestStore.CreateInitialized(DatabaseName); + var options = Fixture.CreateOptions(testDatabase); - var customer = new Customer { Id = 42, Name = "Theon" }; + var customer = new Customer { Id = 42, Name = "Theon", PartitionKey = 1 }; - string storeId = null; - using (var context = new CustomerContext(options)) - { - await context.Database.EnsureCreatedAsync(); - - var entry = context.Add(customer); - - await context.SaveChangesAsync(); - - context.Add(customer); - - storeId = entry.Property("id").CurrentValue; - } - - Assert.NotNull(storeId); + using (var context = new PartitionKeyContext(options)) + { + context.Database.EnsureCreated(); - using (var context = new CustomerContext(options)) - { - var customerFromStore = context.Set().Single(); + context.Add(customer); - Assert.Equal(42, customerFromStore.Id); - Assert.Equal("Theon", customerFromStore.Name); - } + context.SaveChanges(); + } - using (var context = new CustomerContext(options)) - { - customer.Name = "Theon Greyjoy"; + using (var context = new PartitionKeyContext(options)) + { + var customerFromStore = context.Set().Single(); - var entry = context.Entry(customer); - entry.Property("id").CurrentValue = storeId; + Assert.Equal(42, customerFromStore.Id); + Assert.Equal("Theon", customerFromStore.Name); + Assert.Equal(1, customerFromStore.PartitionKey); - entry.State = EntityState.Modified; + customerFromStore.Name = "Theon Greyjoy"; - await context.SaveChangesAsync(); - } + context.SaveChanges(); + } - using (var context = new CustomerContext(options)) - { - var customerFromStore = context.Set().Single(); + using (var context = new PartitionKeyContext(options)) + { + var customerFromStore = context.Set().Single(); - Assert.Equal(42, customerFromStore.Id); - Assert.Equal("Theon Greyjoy", customerFromStore.Name); - } + Assert.Equal(42, customerFromStore.Id); + Assert.Equal("Theon Greyjoy", customerFromStore.Name); + Assert.Equal(1, customerFromStore.PartitionKey); - using (var context = new CustomerContext(options)) - { - var entry = context.Entry(customer); - entry.Property("id").CurrentValue = storeId; - entry.State = EntityState.Deleted; + context.Remove(customerFromStore); - await context.SaveChangesAsync(); - } + context.SaveChanges(); + } - using (var context = new CustomerContext(options)) - { - Assert.Equal(0, context.Set().Count()); - } + using (var context = new PartitionKeyContext(options)) + { + Assert.Empty(context.Set().ToList()); } } - [ConditionalFact(Skip = "Issue#16146")] - public async Task Can_add_update_delete_end_to_end_with_partition_key() + private class PartitionKeyContext : DbContext { - await using (var testDatabase = CosmosTestStore.CreateInitialized(DatabaseName)) + public PartitionKeyContext(DbContextOptions dbContextOptions) + : base(dbContextOptions) + { + } + + protected override void OnModelCreating(ModelBuilder modelBuilder) { - var options = Fixture.CreateOptions(testDatabase); + modelBuilder.Entity().HasPartitionKey(c => c.PartitionKey); + modelBuilder.Entity().Property(c => c.PartitionKey).HasConversion(); + } + } - var customer = new Customer { Id = 42, Name = "Theon", PartitionKey = 1 }; + [ConditionalFact] + public async Task Can_use_detached_entities_without_discriminators() + { + await using var testDatabase = CosmosTestStore.CreateInitialized(DatabaseName); + var options = Fixture.CreateOptions(testDatabase); - using (var context = new PartitionKeyContext(options)) - { - context.Database.EnsureCreated(); + var customer = new Customer { Id = 42, Name = "Theon" }; - context.Add(customer); + using (var context = new NoDiscriminatorCustomerContext(options)) + { + context.Database.EnsureCreated(); - context.SaveChanges(); - } + context.Add(customer); - using (var context = new PartitionKeyContext(options)) - { - var customerFromStore = context.Set().Single(); + context.SaveChanges(); + } - Assert.Equal(42, customerFromStore.Id); - Assert.Equal("Theon", customerFromStore.Name); - Assert.Equal(1, customerFromStore.PartitionKey); + using (var context = new NoDiscriminatorCustomerContext(options)) + { + context.Add(customer).State = EntityState.Modified; - customerFromStore.Name = "Theon Greyjoy"; + customer.Name = "Theon Greyjoy"; - context.SaveChanges(); - } + context.SaveChanges(); + } - using (var context = new PartitionKeyContext(options)) - { - var customerFromStore = context.Set().Single(); + using (var context = new NoDiscriminatorCustomerContext(options)) + { + var customerFromStore = context.Set().AsNoTracking().Single(); - Assert.Equal(42, customerFromStore.Id); - Assert.Equal("Theon Greyjoy", customerFromStore.Name); - Assert.Equal(1, customerFromStore.PartitionKey); + Assert.Equal(42, customerFromStore.Id); + Assert.Equal("Theon Greyjoy", customerFromStore.Name); - context.Remove(customerFromStore); + context.Add(customer).State = EntityState.Deleted; - context.SaveChanges(); - } + context.SaveChanges(); + } - using (var context = new PartitionKeyContext(options)) - { - Assert.Equal(0, context.Set().Count()); - } + using (var context = new NoDiscriminatorCustomerContext(options)) + { + Assert.Empty(context.Set().ToList()); } } - private class PartitionKeyContext : DbContext + private class NoDiscriminatorCustomerContext : CustomerContext { - public PartitionKeyContext(DbContextOptions dbContextOptions) + public NoDiscriminatorCustomerContext(DbContextOptions dbContextOptions) : base(dbContextOptions) { } protected override void OnModelCreating(ModelBuilder modelBuilder) { - modelBuilder.Entity().HasPartitionKey(c => c.PartitionKey); + modelBuilder.Entity().HasNoDiscriminator(); } } [ConditionalFact] public async Task Can_update_unmapped_properties() { - await using (var testDatabase = CosmosTestStore.CreateInitialized(DatabaseName)) - { - var options = Fixture.CreateOptions(testDatabase); + await using var testDatabase = CosmosTestStore.CreateInitialized(DatabaseName); + var options = Fixture.CreateOptions(testDatabase); - var customer = new Customer { Id = 42, Name = "Theon" }; + var customer = new Customer { Id = 42, Name = "Theon" }; - using (var context = new ExtraCustomerContext(options)) - { - context.Database.EnsureCreated(); + using (var context = new ExtraCustomerContext(options)) + { + context.Database.EnsureCreated(); - var entry = context.Add(customer); - entry.Property("EMail").CurrentValue = "theon.g@winterfell.com"; + var entry = context.Add(customer); + entry.Property("EMail").CurrentValue = "theon.g@winterfell.com"; - context.SaveChanges(); - } + context.SaveChanges(); + } - using (var context = new CustomerContext(options)) - { - var customerFromStore = context.Set().Single(); + using (var context = new ExtraCustomerContext(options)) + { + var customerFromStore = context.Set().Single(); - Assert.Equal(42, customerFromStore.Id); - Assert.Equal("Theon", customerFromStore.Name); + Assert.Equal(42, customerFromStore.Id); + Assert.Equal("Theon", customerFromStore.Name); - customerFromStore.Name = "Theon Greyjoy"; + customerFromStore.Name = "Theon Greyjoy"; - context.SaveChanges(); - } + context.SaveChanges(); + } - using (var context = new ExtraCustomerContext(options)) - { - var customerFromStore = context.Set().Single(); + using (var context = new ExtraCustomerContext(options)) + { + var customerFromStore = context.Set().Single(); - Assert.Equal(42, customerFromStore.Id); - Assert.Equal("Theon Greyjoy", customerFromStore.Name); + Assert.Equal(42, customerFromStore.Id); + Assert.Equal("Theon Greyjoy", customerFromStore.Name); - var entry = context.Entry(customerFromStore); - Assert.Equal("theon.g@winterfell.com", entry.Property("EMail").CurrentValue); + var entry = context.Entry(customerFromStore); + Assert.Equal("theon.g@winterfell.com", entry.Property("EMail").CurrentValue); - var json = entry.Property("__jObject").CurrentValue; - Assert.Equal("theon.g@winterfell.com", json["e-mail"]); + var json = entry.Property("__jObject").CurrentValue; + Assert.Equal("theon.g@winterfell.com", json["e-mail"]); - context.Remove(customerFromStore); + context.Remove(customerFromStore); - context.SaveChanges(); - } + context.SaveChanges(); + } + + using (var context = new ExtraCustomerContext(options)) + { + Assert.Empty(context.Set().ToList()); } } @@ -339,36 +391,34 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) } } - [ConditionalFact(Skip = "Issue#16146")] + [ConditionalFact] public async Task Can_use_non_persisted_properties() { - await using (var testDatabase = CosmosTestStore.CreateInitialized(DatabaseName)) - { - var options = Fixture.CreateOptions(testDatabase); + await using var testDatabase = CosmosTestStore.CreateInitialized(DatabaseName); + var options = Fixture.CreateOptions(testDatabase); - var customer = new Customer { Id = 42, Name = "Theon" }; + var customer = new Customer { Id = 42, Name = "Theon" }; - using (var context = new UnmappedCustomerContext(options)) - { - context.Database.EnsureCreated(); + using (var context = new UnmappedCustomerContext(options)) + { + context.Database.EnsureCreated(); - context.Add(customer); + context.Add(customer); - context.SaveChanges(); - Assert.Equal("Theon", customer.Name); - } + context.SaveChanges(); + Assert.Equal("Theon", customer.Name); + } - using (var context = new UnmappedCustomerContext(options)) - { - var customerFromStore = context.Set().Single(); + using (var context = new UnmappedCustomerContext(options)) + { + var customerFromStore = context.Set().Single(); - Assert.Equal(42, customerFromStore.Id); - Assert.Null(customerFromStore.Name); + Assert.Equal(42, customerFromStore.Id); + Assert.Null(customerFromStore.Name); - customerFromStore.Name = "Theon Greyjoy"; + customerFromStore.Name = "Theon Greyjoy"; - Assert.Equal(0, context.SaveChanges()); - } + Assert.Equal(0, context.SaveChanges()); } } @@ -388,22 +438,20 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) [ConditionalFact] public async Task Using_a_conflicting_incompatible_id_throws() { - await using (var testDatabase = CosmosTestStore.CreateInitialized(DatabaseName)) - { - var options = Fixture.CreateOptions(testDatabase); + await using var testDatabase = CosmosTestStore.CreateInitialized(DatabaseName); + var options = Fixture.CreateOptions(testDatabase); - using (var context = new ConflictingIncompatibleIdContext(options)) - { - await Assert.ThrowsAnyAsync( - async () => - { - await context.Database.EnsureCreatedAsync(); + using (var context = new ConflictingIncompatibleIdContext(options)) + { + await Assert.ThrowsAnyAsync( + async () => + { + await context.Database.EnsureCreatedAsync(); - context.Add(new ConflictingIncompatibleId { id = 42 }); + context.Add(new ConflictingIncompatibleId { id = 42 }); - await context.SaveChangesAsync(); - }); - } + await context.SaveChangesAsync(); + }); } } @@ -427,60 +475,58 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) } } - [ConditionalFact(Skip = "Issue #16146")] + [ConditionalFact] public async Task Can_add_update_delete_end_to_end_with_conflicting_id() { - await using (var testDatabase = CosmosTestStore.CreateInitialized(DatabaseName)) - { - var options = Fixture.CreateOptions(testDatabase); + await using var testDatabase = CosmosTestStore.CreateInitialized(DatabaseName); + var options = Fixture.CreateOptions(testDatabase); - var entity = new ConflictingId { id = "42", Name = "Theon" }; + var entity = new ConflictingId { id = "42", Name = "Theon" }; - using (var context = new ConflictingIdContext(options)) - { - await context.Database.EnsureCreatedAsync(); + using (var context = new ConflictingIdContext(options)) + { + await context.Database.EnsureCreatedAsync(); - context.Add(entity); + context.Add(entity); - await context.SaveChangesAsync(); - } + await context.SaveChangesAsync(); + } - using (var context = new ConflictingIdContext(options)) - { - var entityFromStore = context.Set().Single(); + using (var context = new ConflictingIdContext(options)) + { + var entityFromStore = context.Set().Single(); - Assert.Equal("42", entityFromStore.id); - Assert.Equal("Theon", entityFromStore.Name); - } + Assert.Equal("42", entityFromStore.id); + Assert.Equal("Theon", entityFromStore.Name); + } - using (var context = new ConflictingIdContext(options)) - { - entity.Name = "Theon Greyjoy"; + using (var context = new ConflictingIdContext(options)) + { + entity.Name = "Theon Greyjoy"; - context.Update(entity); + context.Update(entity); - await context.SaveChangesAsync(); - } + await context.SaveChangesAsync(); + } - using (var context = new ConflictingIdContext(options)) - { - var entityFromStore = context.Set().Single(); + using (var context = new ConflictingIdContext(options)) + { + var entityFromStore = context.Set().Single(); - Assert.Equal("42", entityFromStore.id); - Assert.Equal("Theon Greyjoy", entityFromStore.Name); - } + Assert.Equal("42", entityFromStore.id); + Assert.Equal("Theon Greyjoy", entityFromStore.Name); + } - using (var context = new ConflictingIdContext(options)) - { - context.Remove(entity); + using (var context = new ConflictingIdContext(options)) + { + context.Remove(entity); - await context.SaveChangesAsync(); - } + await context.SaveChangesAsync(); + } - using (var context = new ConflictingIdContext(options)) - { - Assert.Equal(0, context.Set().Count()); - } + using (var context = new ConflictingIdContext(options)) + { + Assert.Empty(context.Set().ToList()); } } @@ -503,6 +549,47 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) } } + [ConditionalFact] + public async Task Can_have_non_string_property_named_Discriminator() + { + await using var testDatabase = CosmosTestStore.CreateInitialized(DatabaseName); + + using (var context = new NonStringDiscriminatorContext(Fixture.CreateOptions(testDatabase))) + { + context.Database.EnsureCreated(); + + context.Add(new NonStringDiscriminator { Id = 1 }); + await context.SaveChangesAsync(); + + Assert.NotNull(await context.Set().FirstOrDefaultAsync()); + } + } + + private class NonStringDiscriminator + { + public int Id { get; set; } + public EntityType Discriminator { get; set; } + } + + private enum EntityType + { + Base, + Derived + } + + public class NonStringDiscriminatorContext : DbContext + { + public NonStringDiscriminatorContext(DbContextOptions dbContextOptions) + : base(dbContextOptions) + { + } + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity(); + } + } + public class CosmosFixture : ServiceProviderFixtureBase { protected override ITestStoreFactory TestStoreFactory => CosmosTestStoreFactory.Instance; diff --git a/test/EFCore.Specification.Tests/CustomConvertersTestBase.cs b/test/EFCore.Specification.Tests/CustomConvertersTestBase.cs index a2318e448d6..cb79793b109 100644 --- a/test/EFCore.Specification.Tests/CustomConvertersTestBase.cs +++ b/test/EFCore.Specification.Tests/CustomConvertersTestBase.cs @@ -358,7 +358,7 @@ public virtual async Task Can_query_custom_type_not_mapped_by_default_equality(b { using (var context = CreateContext()) { - context.Set().Add(new SimpleCounter() { StyleKey = "Swag" }); + context.Set().Add(new SimpleCounter() { CounterId = 1, StyleKey = "Swag" }); context.SaveChanges(); } @@ -738,18 +738,15 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con b.Property(o => o.Id).HasConversion(new OrderIdEntityFrameworkValueConverter()); }); - // See issue#17814 - if (context.Database.ProviderName != "Microsoft.EntityFrameworkCore.Cosmos") - { - modelBuilder.Entity( - b => - { - b.HasKey(c => c.CounterId); - b.Property(c => c.Discriminator).HasConversion( - d => StringToDictionarySerializer.Serialize(d), - json => StringToDictionarySerializer.Deserialize(json)); - }); - } + modelBuilder.Entity( + b => + { + b.Property(e => e.CounterId).ValueGeneratedNever(); + b.HasKey(c => c.CounterId); + b.Property(c => c.Discriminator).HasConversion( + d => StringToDictionarySerializer.Serialize(d), + json => StringToDictionarySerializer.Deserialize(json)); + }); } public static class StringToDictionarySerializer