From 23b21789e5a5c883ec60b6a39b42abf3fde90da5 Mon Sep 17 00:00:00 2001 From: Jacob Viau Date: Sat, 11 Jan 2020 00:16:14 -0800 Subject: [PATCH 01/13] Add ItemRequestOptions to Cosmos ReplaceItem - Adds Cosmos concurrency token wrapper struct - Passes Entry's ConcurrencyToken as ItemRequestOptions.IfMatchEtag if present --- .../Storage/Internal/CosmosClientWrapper.cs | 30 +++++-- .../Internal/CosmosConcurrencyToken.cs | 90 +++++++++++++++++++ .../Storage/Internal/CosmosDatabaseWrapper.cs | 27 +++++- 3 files changed, 140 insertions(+), 7 deletions(-) create mode 100644 src/EFCore.Cosmos/Storage/Internal/CosmosConcurrencyToken.cs diff --git a/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs b/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs index 7635912ecb6..dd7d7452092 100644 --- a/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs +++ b/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs @@ -302,13 +302,16 @@ public virtual bool ReplaceItem( [NotNull] string collectionId, [NotNull] string documentId, [NotNull] JObject document, + [NotNull] CosmosConcurrencyToken concurrencyToken, [CanBeNull] string partitionKey) => _executionStrategyFactory.Create().Execute( - (collectionId, documentId, document, partitionKey), ReplaceItemOnce, null); + (collectionId, documentId, document, concurrencyToken, partitionKey), + ReplaceItemOnce, + null); private bool ReplaceItemOnce( DbContext context, - (string ContainerId, string ItemId, JObject Document, string PartitionKey) parameters) + (string ContainerId, string ItemId, JObject Document, CosmosConcurrencyToken concurrencyToken, string PartitionKey) parameters) => ReplaceItemOnceAsync(context, parameters).GetAwaiter().GetResult(); /// @@ -321,14 +324,18 @@ public virtual Task ReplaceItemAsync( [NotNull] string collectionId, [NotNull] string documentId, [NotNull] JObject document, + [NotNull] CosmosConcurrencyToken concurrencyToken, [CanBeNull] string partitionKey, CancellationToken cancellationToken = default) => _executionStrategyFactory.Create().ExecuteAsync( - (collectionId, documentId, document, partitionKey), ReplaceItemOnceAsync, null, cancellationToken); + (collectionId, documentId, document, concurrencyToken, partitionKey), + ReplaceItemOnceAsync, + null, + cancellationToken); private async Task ReplaceItemOnceAsync( DbContext _, - (string ContainerId, string ItemId, JObject Document, string PartitionKey) parameters, + (string ContainerId, string ItemId, JObject Document, CosmosConcurrencyToken ConcurrencyToken, string PartitionKey) parameters, CancellationToken cancellationToken = default) { using var stream = new MemoryStream(); @@ -338,9 +345,11 @@ private async Task ReplaceItemOnceAsync( await jsonWriter.FlushAsync(cancellationToken); var container = Client.GetDatabase(_databaseId).GetContainer(parameters.ContainerId); + var itemRequestOptions = CreateItemRequestOptions(parameters.ConcurrencyToken); var partitionKey = CreatePartitionKey(parameters.PartitionKey); + using var response = await container.ReplaceItemStreamAsync( - stream, parameters.ItemId, partitionKey, null, cancellationToken); + stream, parameters.ItemId, partitionKey, itemRequestOptions, cancellationToken); response.EnsureSuccessStatusCode(); return response.StatusCode == HttpStatusCode.OK; } @@ -407,6 +416,17 @@ private PartitionKey CreatePartitionKey(string partitionKey) ? PartitionKey.None : new PartitionKey(partitionKey); + private ItemRequestOptions CreateItemRequestOptions(CosmosConcurrencyToken concurrencyToken) + { + return concurrencyToken.Mode switch + { + CosmosConcurrencyMode.None => new ItemRequestOptions(), + CosmosConcurrencyMode.IfMatch => new ItemRequestOptions { IfMatchEtag = concurrencyToken.Value }, + CosmosConcurrencyMode.IfNoneMatch => new ItemRequestOptions { IfNoneMatchEtag = concurrencyToken.Value }, + _ => throw new InvalidOperationException(), + }; + } + /// /// 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/Storage/Internal/CosmosConcurrencyToken.cs b/src/EFCore.Cosmos/Storage/Internal/CosmosConcurrencyToken.cs new file mode 100644 index 00000000000..35c93f42ac7 --- /dev/null +++ b/src/EFCore.Cosmos/Storage/Internal/CosmosConcurrencyToken.cs @@ -0,0 +1,90 @@ +// 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. + +namespace Microsoft.EntityFrameworkCore.Cosmos.Storage.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 struct CosmosConcurrencyToken + { + private CosmosConcurrencyToken(string value, CosmosConcurrencyMode mode) + { + Value = value; + Mode = mode; + } + + /// + /// 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 CosmosConcurrencyToken None { get; } = new CosmosConcurrencyToken(null, CosmosConcurrencyMode.None); + + /// + /// 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 string Value { get; } + + /// + /// 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 CosmosConcurrencyMode Mode { get; } + + /// + /// 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 CosmosConcurrencyToken IfMatch(string value) + { + return new CosmosConcurrencyToken(value, CosmosConcurrencyMode.IfMatch); + } + + /// + /// 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 CosmosConcurrencyToken IfNotMatch(string value) + { + return new CosmosConcurrencyToken(value, CosmosConcurrencyMode.IfNoneMatch); + } + } + + /// + /// 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 enum CosmosConcurrencyMode + { + /// + /// No concurrency check. + /// + None, + + /// + /// Accept if token matches current value. + /// + IfMatch, + + /// + /// Accept if token does not match current value. + /// + IfNoneMatch = 2, + } +} diff --git a/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs b/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs index de84fd18367..5f45015322f 100644 --- a/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs +++ b/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs @@ -215,7 +215,12 @@ private bool Save(IUpdateEntry entry) } return _cosmosClient.ReplaceItem( - collectionId, documentSource.GetId(entry.SharedIdentityEntry ?? entry), document, GetPartitionKey(entry)); + collectionId, + documentSource.GetId(entry.SharedIdentityEntry ?? entry), + document, + GetConcurrencyToken(entry), + GetPartitionKey(entry)); + case EntityState.Deleted: return _cosmosClient.DeleteItem(collectionId, documentSource.GetId(entry), GetPartitionKey(entry)); default: @@ -270,8 +275,13 @@ private Task SaveAsync(IUpdateEntry entry, CancellationToken cancellationT } return _cosmosClient.ReplaceItemAsync( - collectionId, documentSource.GetId(entry.SharedIdentityEntry ?? entry), document, GetPartitionKey(entry), + collectionId, + documentSource.GetId(entry.SharedIdentityEntry ?? entry), + document, + GetConcurrencyToken(entry), + GetPartitionKey(entry), cancellationToken); + case EntityState.Deleted: return _cosmosClient.DeleteItemAsync( collectionId, documentSource.GetId(entry), GetPartitionKey(entry), cancellationToken); @@ -322,6 +332,19 @@ private IUpdateEntry GetRootDocument(InternalEntityEntry entry) return principal.EntityType.IsDocumentRoot() ? principal : GetRootDocument(principal); } + private static CosmosConcurrencyToken GetConcurrencyToken(IUpdateEntry entry) + { + foreach (var property in entry.EntityType.GetProperties()) + { + if (property.IsConcurrencyToken) + { + return CosmosConcurrencyToken.IfMatch((string)entry.GetCurrentValue(property)); + } + } + + return CosmosConcurrencyToken.None; + } + private static string GetPartitionKey(IUpdateEntry entry) { object partitionKey = null; From 150a93215b31f6cb78c79b7827f57a3b38daaa28 Mon Sep 17 00:00:00 2001 From: Jacob Viau Date: Sat, 11 Jan 2020 00:37:55 -0800 Subject: [PATCH 02/13] Add extension method for cosmos etag concurrency - Add 'UseEtagConcurrency' extension method for Cosmos provider. - Validate only '_etag' json property is concurrency token for cosmos. --- .../CosmosEntityTypeBuilderExtensions.cs | 23 ++++++++++++++++++ .../Internal/CosmosModelValidator.cs | 24 +++++++++++++++++++ .../Properties/CosmosStrings.Designer.cs | 8 +++++++ .../Properties/CosmosStrings.resx | 3 +++ 4 files changed, 58 insertions(+) diff --git a/src/EFCore.Cosmos/Extensions/CosmosEntityTypeBuilderExtensions.cs b/src/EFCore.Cosmos/Extensions/CosmosEntityTypeBuilderExtensions.cs index f34f95a1fc5..6684c382d66 100644 --- a/src/EFCore.Cosmos/Extensions/CosmosEntityTypeBuilderExtensions.cs +++ b/src/EFCore.Cosmos/Extensions/CosmosEntityTypeBuilderExtensions.cs @@ -255,5 +255,28 @@ public static bool CanSetPartitionKey( return entityTypeBuilder.CanSetAnnotation(CosmosAnnotationNames.PartitionKeyName, name, fromDataAnnotation); } + + /// + /// Configures this entity to use CosmosDb etag concurrency checks. + /// + /// The builder for the entity type being configured. + /// The same builder instance so that multiple calls can be chained. + public static EntityTypeBuilder UseEtagConcurrency(this EntityTypeBuilder entityTypeBuilder) + { + entityTypeBuilder.Property("_etag").ValueGeneratedOnAddOrUpdate().IsConcurrencyToken(); + return entityTypeBuilder; + } + + /// + /// Configures this entity to use CosmosDb etag concurrency checks. + /// + /// The builder for the entity type being configured. + /// The same builder instance so that multiple calls can be chained. + public static EntityTypeBuilder UseEtagConcurrency(this EntityTypeBuilder entityTypeBuilder) + where TEntity : class + { + UseEtagConcurrency((EntityTypeBuilder)entityTypeBuilder); + return entityTypeBuilder; + } } } diff --git a/src/EFCore.Cosmos/Internal/CosmosModelValidator.cs b/src/EFCore.Cosmos/Internal/CosmosModelValidator.cs index 12298ab23d9..cb04d0173d0 100644 --- a/src/EFCore.Cosmos/Internal/CosmosModelValidator.cs +++ b/src/EFCore.Cosmos/Internal/CosmosModelValidator.cs @@ -41,6 +41,7 @@ public override void Validate(IModel model, IDiagnosticsLogger @@ -173,5 +174,28 @@ protected virtual void ValidateSharedContainerCompatibility( } } } + + /// + /// 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. + /// + protected virtual void ValidateOnlyEtagConcurrencyToken( + [NotNull] IModel model, + [NotNull] IDiagnosticsLogger logger) + { + foreach (var entityType in model.GetEntityTypes()) + { + foreach (var property in entityType.GetProperties()) + { + if (property.IsConcurrencyToken && property.GetJsonPropertyName() != "_etag") + { + throw new InvalidOperationException( + CosmosStrings.NonEtagConcurrencyToken(entityType.DisplayName(), property.GetJsonPropertyName())); + } + } + } + } } } diff --git a/src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs b/src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs index c248f911a75..c6056a693ac 100644 --- a/src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs +++ b/src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs @@ -96,6 +96,14 @@ public static string PartitionKeyStoreNameMismatch([CanBeNull] object property1, GetString("PartitionKeyStoreNameMismatch", nameof(property1), nameof(entityType1), nameof(storeName1), nameof(property2), nameof(entityType2), nameof(storeName2)), property1, entityType1, storeName1, property2, entityType2, storeName2); + /// + /// The entity type '{entityType}' has property '(property}' as its concurrency token, but only '_etag' is supported. Consider using 'EntityTypeBuilder.UseEtagConcurrency'. + /// + public static string NonEtagConcurrencyToken([CanBeNull] object entityType, [CanBeNull] object property) + => string.Format( + GetString("NonEtagConcurrencyToken", nameof(entityType), nameof(property)), + entityType, property); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/EFCore.Cosmos/Properties/CosmosStrings.resx b/src/EFCore.Cosmos/Properties/CosmosStrings.resx index 429f6f25577..38e4cdce624 100644 --- a/src/EFCore.Cosmos/Properties/CosmosStrings.resx +++ b/src/EFCore.Cosmos/Properties/CosmosStrings.resx @@ -129,6 +129,9 @@ The entity type '{entityType}' is sharing the container '{container}' with other types, but does not have a discriminator value configured. + + The entity type '{entityType}' has property '(property}' as its concurrency token, but only '_etag' is supported. Consider using 'EntityTypeBuilder.UseEtagConcurrency'. + The entity type '{entityType}' does not have a partition key set, but it is mapped to the container '{container}' shared by entity types with partition keys. From b1a71805c83eb783b2c30b83b99a2e4c32c9bece Mon Sep 17 00:00:00 2001 From: Jacob Viau Date: Sat, 11 Jan 2020 19:24:16 -0800 Subject: [PATCH 03/13] Catch cosmos conflict and rethrow as db ex. - Catches cosmos conflicg exception and rethrows as efcore exception - Add arg checks to UseEtagConcurrency - Add and correct new cosmos strings --- .../CosmosEntityTypeBuilderExtensions.cs | 7 +++++- .../Properties/CosmosStrings.Designer.cs | 10 +++++++- .../Properties/CosmosStrings.resx | 5 +++- .../Storage/Internal/CosmosClientWrapper.cs | 23 +++++++++++++++---- .../Internal/CosmosConcurrencyToken.cs | 2 +- 5 files changed, 39 insertions(+), 8 deletions(-) diff --git a/src/EFCore.Cosmos/Extensions/CosmosEntityTypeBuilderExtensions.cs b/src/EFCore.Cosmos/Extensions/CosmosEntityTypeBuilderExtensions.cs index 6684c382d66..b907736055b 100644 --- a/src/EFCore.Cosmos/Extensions/CosmosEntityTypeBuilderExtensions.cs +++ b/src/EFCore.Cosmos/Extensions/CosmosEntityTypeBuilderExtensions.cs @@ -263,7 +263,11 @@ public static bool CanSetPartitionKey( /// The same builder instance so that multiple calls can be chained. public static EntityTypeBuilder UseEtagConcurrency(this EntityTypeBuilder entityTypeBuilder) { - entityTypeBuilder.Property("_etag").ValueGeneratedOnAddOrUpdate().IsConcurrencyToken(); + Check.NotNull(entityTypeBuilder, nameof(entityTypeBuilder)); + + entityTypeBuilder.Property("_etag") + .ValueGeneratedOnAddOrUpdate() + .IsConcurrencyToken(); return entityTypeBuilder; } @@ -275,6 +279,7 @@ public static EntityTypeBuilder UseEtagConcurrency(this EntityTypeBuilder entity public static EntityTypeBuilder UseEtagConcurrency(this EntityTypeBuilder entityTypeBuilder) where TEntity : class { + Check.NotNull(entityTypeBuilder, nameof(entityTypeBuilder)); UseEtagConcurrency((EntityTypeBuilder)entityTypeBuilder); return entityTypeBuilder; } diff --git a/src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs b/src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs index c6056a693ac..8f4a32680b9 100644 --- a/src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs +++ b/src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs @@ -97,13 +97,21 @@ public static string PartitionKeyStoreNameMismatch([CanBeNull] object property1, property1, entityType1, storeName1, property2, entityType2, storeName2); /// - /// The entity type '{entityType}' has property '(property}' as its concurrency token, but only '_etag' is supported. Consider using 'EntityTypeBuilder.UseEtagConcurrency'. + /// The entity type '{entityType}' has property '{property}' as its concurrency token, but only '_etag' is supported. Consider using 'EntityTypeBuilder.UseEtagConcurrency'. /// public static string NonEtagConcurrencyToken([CanBeNull] object entityType, [CanBeNull] object property) => string.Format( GetString("NonEtagConcurrencyToken", nameof(entityType), nameof(property)), entityType, property); + /// + /// Conflicts were detected for item with id '{itemId}'. + /// + public static string UpdateConcurrencyTokenException([CanBeNull] object itemId) + => string.Format( + GetString("UpdateConcurrencyTokenException", nameof(itemId)), + itemId); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/EFCore.Cosmos/Properties/CosmosStrings.resx b/src/EFCore.Cosmos/Properties/CosmosStrings.resx index 38e4cdce624..04b2f4e1b15 100644 --- a/src/EFCore.Cosmos/Properties/CosmosStrings.resx +++ b/src/EFCore.Cosmos/Properties/CosmosStrings.resx @@ -130,7 +130,7 @@ The entity type '{entityType}' is sharing the container '{container}' with other types, but does not have a discriminator value configured. - The entity type '{entityType}' has property '(property}' as its concurrency token, but only '_etag' is supported. Consider using 'EntityTypeBuilder.UseEtagConcurrency'. + The entity type '{entityType}' has property '{property}' as its concurrency token, but only '_etag' is supported. Consider using 'EntityTypeBuilder.UseEtagConcurrency'. The entity type '{entityType}' does not have a partition key set, but it is mapped to the container '{container}' shared by entity types with partition keys. @@ -150,4 +150,7 @@ The partition key property '{property1}' on '{entityType1}' is mapped as '{storeName1}', but the partition key property '{property2}' on '{entityType2}' is mapped as '{storeName2}'. All partition key properties need to be mapped to the same store property. + + Conflicts were detected for item with id '{itemId}'. + \ No newline at end of file diff --git a/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs b/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs index dd7d7452092..f09616a0786 100644 --- a/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs +++ b/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs @@ -14,6 +14,7 @@ using Microsoft.Azure.Cosmos; using Microsoft.EntityFrameworkCore.Cosmos.Diagnostics.Internal; using Microsoft.EntityFrameworkCore.Cosmos.Infrastructure.Internal; +using Microsoft.EntityFrameworkCore.Cosmos.Internal; using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Storage; @@ -287,6 +288,7 @@ private async Task CreateItemOnceAsync( var container = Client.GetDatabase(_databaseId).GetContainer(parameters.ContainerId); var partitionKey = CreatePartitionKey(parameters.PartitionKey); + using var response = await container.CreateItemStreamAsync(stream, partitionKey, null, cancellationToken); response.EnsureSuccessStatusCode(); return response.StatusCode == HttpStatusCode.Created; @@ -348,10 +350,17 @@ private async Task ReplaceItemOnceAsync( var itemRequestOptions = CreateItemRequestOptions(parameters.ConcurrencyToken); var partitionKey = CreatePartitionKey(parameters.PartitionKey); - using var response = await container.ReplaceItemStreamAsync( - stream, parameters.ItemId, partitionKey, itemRequestOptions, cancellationToken); - response.EnsureSuccessStatusCode(); - return response.StatusCode == HttpStatusCode.OK; + try + { + using var response = await container.ReplaceItemStreamAsync( + stream, parameters.ItemId, partitionKey, itemRequestOptions, cancellationToken); + response.EnsureSuccessStatusCode(); + return response.StatusCode == HttpStatusCode.OK; + } + catch (CosmosException cre) when (cre.StatusCode == HttpStatusCode.PreconditionFailed) + { + throw ThrowConcurrencyException(parameters.ItemId, cre); + } } /// @@ -411,6 +420,12 @@ public virtual async Task DeleteItemOnceAsync( return response.StatusCode == HttpStatusCode.NoContent; } + private static Exception ThrowConcurrencyException(string itemId, CosmosException cosmosException) + { + throw new DbUpdateConcurrencyException( + CosmosStrings.UpdateConcurrencyTokenException(itemId), cosmosException); + } + private PartitionKey CreatePartitionKey(string partitionKey) => partitionKey == null ? PartitionKey.None diff --git a/src/EFCore.Cosmos/Storage/Internal/CosmosConcurrencyToken.cs b/src/EFCore.Cosmos/Storage/Internal/CosmosConcurrencyToken.cs index 35c93f42ac7..d2834e5ab2d 100644 --- a/src/EFCore.Cosmos/Storage/Internal/CosmosConcurrencyToken.cs +++ b/src/EFCore.Cosmos/Storage/Internal/CosmosConcurrencyToken.cs @@ -9,7 +9,7 @@ namespace Microsoft.EntityFrameworkCore.Cosmos.Storage.Internal /// 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 struct CosmosConcurrencyToken + public readonly struct CosmosConcurrencyToken { private CosmosConcurrencyToken(string value, CosmosConcurrencyMode mode) { From 11c6e1ab93c2f35d5ae3c11fce64ac42a610d502 Mon Sep 17 00:00:00 2001 From: Jacob Viau Date: Sat, 11 Jan 2020 19:25:02 -0800 Subject: [PATCH 04/13] Add UseEtagConcurrency tests - Add model validation tests for etag concurrency - Test etag concurrency extension directly --- .../CosmosEntityTypeBuilderExtensionsTests.cs | 30 +++++++++++++++++++ .../CosmosModelValidatorTest.cs | 26 ++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 test/EFCore.Cosmos.Tests/Extensions/CosmosEntityTypeBuilderExtensionsTests.cs diff --git a/test/EFCore.Cosmos.Tests/Extensions/CosmosEntityTypeBuilderExtensionsTests.cs b/test/EFCore.Cosmos.Tests/Extensions/CosmosEntityTypeBuilderExtensionsTests.cs new file mode 100644 index 00000000000..debf4286967 --- /dev/null +++ b/test/EFCore.Cosmos.Tests/Extensions/CosmosEntityTypeBuilderExtensionsTests.cs @@ -0,0 +1,30 @@ +// 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 Microsoft.EntityFrameworkCore.Cosmos.TestUtilities; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Metadata; +using Microsoft.EntityFrameworkCore.TestUtilities; +using Xunit; + +// ReSharper disable once CheckNamespace +namespace Microsoft.EntityFrameworkCore +{ + public class CosmosEntityTypeBuilderExtensionsTests : ModelValidatorTestBase + { + protected override TestHelpers TestHelpers => CosmosTestHelpers.Instance; + + [ConditionalFact] + public void Can_set_etag_concurrency() + { + var modelBuilder = CreateConventionalModelBuilder(); + modelBuilder.Entity().UseEtagConcurrency(); + var model = modelBuilder.Model; + + var etagProperty = model.FindEntityType(typeof(Customer).FullName).FindProperty("_etag"); + Assert.NotNull(etagProperty); + Assert.Equal(ValueGenerated.OnAddOrUpdate, etagProperty.ValueGenerated); + Assert.True(etagProperty.IsConcurrencyToken); + } + } +} diff --git a/test/EFCore.Cosmos.Tests/Infrastructure/CosmosModelValidatorTest.cs b/test/EFCore.Cosmos.Tests/Infrastructure/CosmosModelValidatorTest.cs index 1526409dab4..de88043ec7d 100644 --- a/test/EFCore.Cosmos.Tests/Infrastructure/CosmosModelValidatorTest.cs +++ b/test/EFCore.Cosmos.Tests/Infrastructure/CosmosModelValidatorTest.cs @@ -126,6 +126,32 @@ public virtual void Detects_duplicate_discriminator_values() VerifyError(CosmosStrings.DuplicateDiscriminatorValue(typeof(Order).Name, "type", typeof(Customer).Name, "Orders"), model); } + [ConditionalFact] + public virtual void Passes_on_valid_concurrency_token() + { + var modelBuilder = CreateConventionalModelBuilder(); + modelBuilder.Entity() + .ToContainer("Orders") + .Property("_etag") + .IsConcurrencyToken(); + + var model = modelBuilder.Model; + Validate(model); + } + + [ConditionalFact] + public virtual void Detects_invalid_concurrency_token() + { + var modelBuilder = CreateConventionalModelBuilder(); + modelBuilder.Entity() + .ToContainer("Orders") + .Property("_not_etag") + .IsConcurrencyToken(); + + var model = modelBuilder.Model; + VerifyError(CosmosStrings.NonEtagConcurrencyToken(typeof(Customer).Name, "_not_etag"), model); + } + protected override TestHelpers TestHelpers => CosmosTestHelpers.Instance; } } From b1f37cdba70d1c7608602ff8b18a9da7b117455f Mon Sep 17 00:00:00 2001 From: Jacob Viau Date: Mon, 13 Jan 2020 20:05:18 -0800 Subject: [PATCH 05/13] Add [NotNull], return null for no etag - Add [NotNull] to UseEtagConcurrency - Return null for when there is no concurrency --- .../Extensions/CosmosEntityTypeBuilderExtensions.cs | 4 ++-- src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/EFCore.Cosmos/Extensions/CosmosEntityTypeBuilderExtensions.cs b/src/EFCore.Cosmos/Extensions/CosmosEntityTypeBuilderExtensions.cs index b907736055b..9966b3f2271 100644 --- a/src/EFCore.Cosmos/Extensions/CosmosEntityTypeBuilderExtensions.cs +++ b/src/EFCore.Cosmos/Extensions/CosmosEntityTypeBuilderExtensions.cs @@ -261,7 +261,7 @@ public static bool CanSetPartitionKey( /// /// The builder for the entity type being configured. /// The same builder instance so that multiple calls can be chained. - public static EntityTypeBuilder UseEtagConcurrency(this EntityTypeBuilder entityTypeBuilder) + public static EntityTypeBuilder UseEtagConcurrency([NotNull] this EntityTypeBuilder entityTypeBuilder) { Check.NotNull(entityTypeBuilder, nameof(entityTypeBuilder)); @@ -276,7 +276,7 @@ public static EntityTypeBuilder UseEtagConcurrency(this EntityTypeBuilder entity /// /// The builder for the entity type being configured. /// The same builder instance so that multiple calls can be chained. - public static EntityTypeBuilder UseEtagConcurrency(this EntityTypeBuilder entityTypeBuilder) + public static EntityTypeBuilder UseEtagConcurrency([NotNull] this EntityTypeBuilder entityTypeBuilder) where TEntity : class { Check.NotNull(entityTypeBuilder, nameof(entityTypeBuilder)); diff --git a/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs b/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs index f09616a0786..d3865e84575 100644 --- a/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs +++ b/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs @@ -435,7 +435,7 @@ private ItemRequestOptions CreateItemRequestOptions(CosmosConcurrencyToken concu { return concurrencyToken.Mode switch { - CosmosConcurrencyMode.None => new ItemRequestOptions(), + CosmosConcurrencyMode.None => null, // null to keep it consistent with previous behavior CosmosConcurrencyMode.IfMatch => new ItemRequestOptions { IfMatchEtag = concurrencyToken.Value }, CosmosConcurrencyMode.IfNoneMatch => new ItemRequestOptions { IfNoneMatchEtag = concurrencyToken.Value }, _ => throw new InvalidOperationException(), From e21a79e3d1d33fd11c8ff4a10765919c14f9d5b9 Mon Sep 17 00:00:00 2001 From: Jacob Viau Date: Mon, 13 Jan 2020 21:01:42 -0800 Subject: [PATCH 06/13] Fix API null-checks - Add [NotNull] to CosmosConcurrencyToken - Remove [NotNull] from CosmosClientWrapper --- .../Storage/Internal/CosmosClientWrapper.cs | 4 ++-- .../Storage/Internal/CosmosConcurrencyToken.cs | 15 +++++++-------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs b/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs index d3865e84575..d87563ef4ee 100644 --- a/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs +++ b/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs @@ -304,7 +304,7 @@ public virtual bool ReplaceItem( [NotNull] string collectionId, [NotNull] string documentId, [NotNull] JObject document, - [NotNull] CosmosConcurrencyToken concurrencyToken, + CosmosConcurrencyToken concurrencyToken, [CanBeNull] string partitionKey) => _executionStrategyFactory.Create().Execute( (collectionId, documentId, document, concurrencyToken, partitionKey), @@ -326,7 +326,7 @@ public virtual Task ReplaceItemAsync( [NotNull] string collectionId, [NotNull] string documentId, [NotNull] JObject document, - [NotNull] CosmosConcurrencyToken concurrencyToken, + CosmosConcurrencyToken concurrencyToken, [CanBeNull] string partitionKey, CancellationToken cancellationToken = default) => _executionStrategyFactory.Create().ExecuteAsync( diff --git a/src/EFCore.Cosmos/Storage/Internal/CosmosConcurrencyToken.cs b/src/EFCore.Cosmos/Storage/Internal/CosmosConcurrencyToken.cs index d2834e5ab2d..50977c53b87 100644 --- a/src/EFCore.Cosmos/Storage/Internal/CosmosConcurrencyToken.cs +++ b/src/EFCore.Cosmos/Storage/Internal/CosmosConcurrencyToken.cs @@ -1,6 +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.Diagnostics.CodeAnalysis; +using JetBrains.Annotations; + namespace Microsoft.EntityFrameworkCore.Cosmos.Storage.Internal { /// @@ -47,10 +50,8 @@ private CosmosConcurrencyToken(string value, CosmosConcurrencyMode mode) /// 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 CosmosConcurrencyToken IfMatch(string value) - { - return new CosmosConcurrencyToken(value, CosmosConcurrencyMode.IfMatch); - } + public static CosmosConcurrencyToken IfMatch([CanBeNull] string value) + => new CosmosConcurrencyToken(value, CosmosConcurrencyMode.IfMatch); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -58,10 +59,8 @@ public static CosmosConcurrencyToken IfMatch(string value) /// 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 CosmosConcurrencyToken IfNotMatch(string value) - { - return new CosmosConcurrencyToken(value, CosmosConcurrencyMode.IfNoneMatch); - } + public static CosmosConcurrencyToken IfNotMatch([CanBeNull] string value) + => new CosmosConcurrencyToken(value, CosmosConcurrencyMode.IfNoneMatch); } /// From c4d5e000e0567fae3558231e5d1ff4f87176eddd Mon Sep 17 00:00:00 2001 From: Jacob Viau Date: Wed, 15 Jan 2020 23:53:32 -0800 Subject: [PATCH 07/13] Further etag changes based on PR comments - Extract CosmosConcurrencyMode to own file - Validate etag provider type is string - Add etag entity annotation - Add PropertyBuilder SetEtagConcurrency extension method. --- .../Extensions/CosmosEntityTypeExtensions.cs | 52 +++++++++++++++++++ .../CosmosPropertyBuilderExtensions.cs | 22 ++++++++ .../Internal/CosmosModelValidator.cs | 20 +++++-- .../Conventions/ETagPropertyConvention.cs | 35 +++++++++++++ .../Internal/CosmosConventionSetBuilder.cs | 1 + .../Internal/CosmosAnnotationNames.cs | 8 +++ .../Properties/CosmosStrings.Designer.cs | 24 ++++++--- .../Properties/CosmosStrings.resx | 3 ++ .../Storage/Internal/CosmosConcurrencyMode.cs | 29 +++++++++++ .../Internal/CosmosConcurrencyToken.cs | 24 --------- .../Storage/Internal/CosmosDatabaseWrapper.cs | 17 +++--- 11 files changed, 193 insertions(+), 42 deletions(-) create mode 100644 src/EFCore.Cosmos/Metadata/Conventions/ETagPropertyConvention.cs create mode 100644 src/EFCore.Cosmos/Storage/Internal/CosmosConcurrencyMode.cs diff --git a/src/EFCore.Cosmos/Extensions/CosmosEntityTypeExtensions.cs b/src/EFCore.Cosmos/Extensions/CosmosEntityTypeExtensions.cs index c3c16d3cb48..48c3c9d1664 100644 --- a/src/EFCore.Cosmos/Extensions/CosmosEntityTypeExtensions.cs +++ b/src/EFCore.Cosmos/Extensions/CosmosEntityTypeExtensions.cs @@ -146,5 +146,57 @@ public static void SetPartitionKeyPropertyName( public static ConfigurationSource? GetPartitionKeyPropertyNameConfigurationSource([NotNull] this IConventionEntityType entityType) => entityType.FindAnnotation(CosmosAnnotationNames.PartitionKeyName) ?.GetConfigurationSource(); + + /// + /// Returns the name of the property that is used to store the etag. + /// + /// The entity type to get the etag property name for. + /// The name of the etag property. + public static string GetETagPropertyName([NotNull] this IEntityType entityType) + => entityType[CosmosAnnotationNames.ETagName] as string; + + /// + /// Sets the name of the property that is used to store the etag key. + /// + /// The entity type to set the etag property name for. + /// The name to set. + public static void SetETagPropertyName([NotNull] this IMutableEntityType entityType, [CanBeNull] string name) + => entityType.SetOrRemoveAnnotation( + CosmosAnnotationNames.ETagName, + Check.NullButNotEmpty(name, nameof(name))); + + /// + /// Sets the name of the property that is used to store the etag. + /// + /// The entity type to set the etag property name for. + /// The name to set. + /// Indicates whether the configuration was specified using a data annotation. + public static void SetETagPropertyName( + [NotNull] this IConventionEntityType entityType, [CanBeNull] string name, bool fromDataAnnotation = false) + => entityType.SetOrRemoveAnnotation( + CosmosAnnotationNames.ETagName, + Check.NullButNotEmpty(name, nameof(name)), + fromDataAnnotation); + + /// + /// Gets the for the property that is used to store the etag. + /// + /// The entity type to find configuration source for. + /// The for the etag property. + public static ConfigurationSource? GetETagPropertyNameConfigurationSource([NotNull] this IConventionEntityType entityType) + => entityType.FindAnnotation(CosmosAnnotationNames.ETagName) + ?.GetConfigurationSource(); + + /// + /// Gets the on this entity that is mapped to cosmos etag, if it exists. + /// + /// The entity type to get the etag property for. + /// The mapped to etag, or null if no property is mapped to etag. + public static IProperty GetETagProperty([NotNull] this IEntityType entityType) + { + Check.NotNull(entityType, nameof(entityType)); + var etagPropertyName = entityType.GetETagPropertyName(); + return !string.IsNullOrEmpty(etagPropertyName) ? entityType.FindProperty(etagPropertyName) : null; + } } } diff --git a/src/EFCore.Cosmos/Extensions/CosmosPropertyBuilderExtensions.cs b/src/EFCore.Cosmos/Extensions/CosmosPropertyBuilderExtensions.cs index d100bf553e0..1444161d756 100644 --- a/src/EFCore.Cosmos/Extensions/CosmosPropertyBuilderExtensions.cs +++ b/src/EFCore.Cosmos/Extensions/CosmosPropertyBuilderExtensions.cs @@ -87,5 +87,27 @@ public static bool CanSetJsonProperty( [CanBeNull] string name, bool fromDataAnnotation = false) => propertyBuilder.CanSetAnnotation(CosmosAnnotationNames.PropertyName, name, fromDataAnnotation); + + /// + /// Configures this property to be the etag concurrency token. + /// + /// The builder for the property being configured. + /// The same builder instance so that multiple calls can be chained. + public static PropertyBuilder SetEtagConcurrency([NotNull] this PropertyBuilder propertyBuilder) + { + Check.NotNull(propertyBuilder, nameof(propertyBuilder)); + propertyBuilder.IsConcurrencyToken().ToJsonProperty("_etag"); + return propertyBuilder; + } + + /// + /// Configures this property to be the etag concurrency token. + /// + /// The type of the property being configured. + /// The builder for the property being configured. + /// The same builder instance so that multiple calls can be chained. + public static PropertyBuilder SetEtagConcurrency( + [NotNull] this PropertyBuilder propertyBuilder) + => (PropertyBuilder)SetEtagConcurrency((PropertyBuilder)propertyBuilder); } } diff --git a/src/EFCore.Cosmos/Internal/CosmosModelValidator.cs b/src/EFCore.Cosmos/Internal/CosmosModelValidator.cs index cb04d0173d0..119736b53a5 100644 --- a/src/EFCore.Cosmos/Internal/CosmosModelValidator.cs +++ b/src/EFCore.Cosmos/Internal/CosmosModelValidator.cs @@ -187,12 +187,24 @@ protected virtual void ValidateOnlyEtagConcurrencyToken( { foreach (var entityType in model.GetEntityTypes()) { - foreach (var property in entityType.GetProperties()) + foreach (var property in entityType.GetDeclaredProperties()) { - if (property.IsConcurrencyToken && property.GetJsonPropertyName() != "_etag") + if (property.IsConcurrencyToken) { - throw new InvalidOperationException( - CosmosStrings.NonEtagConcurrencyToken(entityType.DisplayName(), property.GetJsonPropertyName())); + var storeName = property.GetJsonPropertyName(); + if (storeName != "_etag") + { + throw new InvalidOperationException( + CosmosStrings.NonEtagConcurrencyToken(entityType.DisplayName(), storeName)); + } + + var etagType = property.GetTypeMapping().Converter?.ProviderClrType ?? property.ClrType; + if (etagType != typeof(string)) + { + throw new InvalidOperationException( + CosmosStrings.ETagNonStringStoreType( + property.Name, entityType.DisplayName(), etagType.ShortDisplayName())); + } } } } diff --git a/src/EFCore.Cosmos/Metadata/Conventions/ETagPropertyConvention.cs b/src/EFCore.Cosmos/Metadata/Conventions/ETagPropertyConvention.cs new file mode 100644 index 00000000000..d12a6030cbb --- /dev/null +++ b/src/EFCore.Cosmos/Metadata/Conventions/ETagPropertyConvention.cs @@ -0,0 +1,35 @@ +// 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 Microsoft.EntityFrameworkCore.Metadata.Builders; +using Microsoft.EntityFrameworkCore.Metadata.Conventions; + +namespace Microsoft.EntityFrameworkCore.Cosmos.Metadata.Conventions +{ + /// + /// A convention that adds etag metadata on the concurrency token, if present. + /// + public class ETagPropertyConvention : IModelFinalizedConvention + { + /// + /// Called after a model is finalized. + /// + /// The builder for the model. + /// Additional information associated with convention execution. + public void ProcessModelFinalized( + IConventionModelBuilder modelBuilder, + IConventionContext context) + { + foreach (var entityType in modelBuilder.Metadata.GetEntityTypes()) + { + foreach (var property in entityType.GetDeclaredProperties()) + { + if (property.IsConcurrencyToken) + { + entityType.SetETagPropertyName(property.Name); + } + } + } + } + } +} diff --git a/src/EFCore.Cosmos/Metadata/Conventions/Internal/CosmosConventionSetBuilder.cs b/src/EFCore.Cosmos/Metadata/Conventions/Internal/CosmosConventionSetBuilder.cs index be639c2c472..be86b3bb604 100644 --- a/src/EFCore.Cosmos/Metadata/Conventions/Internal/CosmosConventionSetBuilder.cs +++ b/src/EFCore.Cosmos/Metadata/Conventions/Internal/CosmosConventionSetBuilder.cs @@ -38,6 +38,7 @@ public override ConventionSet CreateConventionSet() var conventionSet = base.CreateConventionSet(); conventionSet.ModelInitializedConventions.Add(new ContextContainerConvention(Dependencies)); + conventionSet.ModelFinalizedConventions.Add(new ETagPropertyConvention()); var discriminatorConvention = new CosmosDiscriminatorConvention(Dependencies); var storeKeyConvention = new StoreKeyConvention(Dependencies); diff --git a/src/EFCore.Cosmos/Metadata/Internal/CosmosAnnotationNames.cs b/src/EFCore.Cosmos/Metadata/Internal/CosmosAnnotationNames.cs index bb43cd56a15..1ec36a9b0a5 100644 --- a/src/EFCore.Cosmos/Metadata/Internal/CosmosAnnotationNames.cs +++ b/src/EFCore.Cosmos/Metadata/Internal/CosmosAnnotationNames.cs @@ -42,5 +42,13 @@ public static class CosmosAnnotationNames /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public const string PartitionKeyName = Prefix + "PartitionKeyName"; + + /// + /// 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 const string ETagName = Prefix + "EtagName"; } } diff --git a/src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs b/src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs index 8f4a32680b9..2ea506e5bde 100644 --- a/src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs +++ b/src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs @@ -32,6 +32,14 @@ public static string DuplicateDiscriminatorValue([CanBeNull] object entityType1, GetString("DuplicateDiscriminatorValue", nameof(entityType1), nameof(discriminatorValue), nameof(entityType2), nameof(container)), entityType1, discriminatorValue, entityType2, container); + /// + /// The type of the etag property '{property}' on '{entityType}' is '{propertyType}'. All etag properties need to be strings or have a string converter. + /// + public static string ETagNonStringStoreType([CanBeNull] object property, [CanBeNull] object entityType, [CanBeNull] object propertyType) + => string.Format( + GetString("ETagNonStringStoreType", nameof(property), nameof(entityType), nameof(propertyType)), + property, entityType, propertyType); + /// /// The entity type '{entityType}' is sharing the container '{container}' with other types, but does not have a discriminator property configured. /// @@ -48,6 +56,14 @@ public static string NoDiscriminatorValue([CanBeNull] object entityType, [CanBeN GetString("NoDiscriminatorValue", nameof(entityType), nameof(container)), entityType, container); + /// + /// The entity type '{entityType}' has property '{property}' as its concurrency token, but only '_etag' is supported. Consider using 'EntityTypeBuilder.UseEtagConcurrency'. + /// + public static string NonEtagConcurrencyToken([CanBeNull] object entityType, [CanBeNull] object property) + => string.Format( + GetString("NonEtagConcurrencyToken", nameof(entityType), nameof(property)), + entityType, property); + /// /// The entity type '{entityType}' does not have a partition key set, but it is mapped to the container '{container}' shared by entity types with partition keys. /// @@ -96,14 +112,6 @@ public static string PartitionKeyStoreNameMismatch([CanBeNull] object property1, GetString("PartitionKeyStoreNameMismatch", nameof(property1), nameof(entityType1), nameof(storeName1), nameof(property2), nameof(entityType2), nameof(storeName2)), property1, entityType1, storeName1, property2, entityType2, storeName2); - /// - /// The entity type '{entityType}' has property '{property}' as its concurrency token, but only '_etag' is supported. Consider using 'EntityTypeBuilder.UseEtagConcurrency'. - /// - public static string NonEtagConcurrencyToken([CanBeNull] object entityType, [CanBeNull] object property) - => string.Format( - GetString("NonEtagConcurrencyToken", nameof(entityType), nameof(property)), - entityType, property); - /// /// Conflicts were detected for item with id '{itemId}'. /// diff --git a/src/EFCore.Cosmos/Properties/CosmosStrings.resx b/src/EFCore.Cosmos/Properties/CosmosStrings.resx index 04b2f4e1b15..8dd23f4080e 100644 --- a/src/EFCore.Cosmos/Properties/CosmosStrings.resx +++ b/src/EFCore.Cosmos/Properties/CosmosStrings.resx @@ -123,6 +123,9 @@ The discriminator value for '{entityType1}' is '{discriminatorValue}' which is the same for '{entityType2}'. Every concrete entity type mapped to the container '{container}' needs to have a unique discriminator value. + + The type of the etag property '{property}' on '{entityType}' is '{propertyType}'. All etag properties need to be strings or have a string converter. + The entity type '{entityType}' is sharing the container '{container}' with other types, but does not have a discriminator property configured. diff --git a/src/EFCore.Cosmos/Storage/Internal/CosmosConcurrencyMode.cs b/src/EFCore.Cosmos/Storage/Internal/CosmosConcurrencyMode.cs new file mode 100644 index 00000000000..7a01a4916a0 --- /dev/null +++ b/src/EFCore.Cosmos/Storage/Internal/CosmosConcurrencyMode.cs @@ -0,0 +1,29 @@ +// 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. + +namespace Microsoft.EntityFrameworkCore.Cosmos.Storage.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 enum CosmosConcurrencyMode + { + /// + /// No concurrency check. + /// + None = 0, + + /// + /// Accept if token matches current value. + /// + IfMatch, + + /// + /// Accept if token does not match current value. + /// + IfNoneMatch = 2, + } +} diff --git a/src/EFCore.Cosmos/Storage/Internal/CosmosConcurrencyToken.cs b/src/EFCore.Cosmos/Storage/Internal/CosmosConcurrencyToken.cs index 50977c53b87..d1f9e518377 100644 --- a/src/EFCore.Cosmos/Storage/Internal/CosmosConcurrencyToken.cs +++ b/src/EFCore.Cosmos/Storage/Internal/CosmosConcurrencyToken.cs @@ -62,28 +62,4 @@ public static CosmosConcurrencyToken IfMatch([CanBeNull] string value) public static CosmosConcurrencyToken IfNotMatch([CanBeNull] string value) => new CosmosConcurrencyToken(value, CosmosConcurrencyMode.IfNoneMatch); } - - /// - /// 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 enum CosmosConcurrencyMode - { - /// - /// No concurrency check. - /// - None, - - /// - /// Accept if token matches current value. - /// - IfMatch, - - /// - /// Accept if token does not match current value. - /// - IfNoneMatch = 2, - } } diff --git a/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs b/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs index 5f45015322f..7ddf23f44c9 100644 --- a/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs +++ b/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs @@ -334,15 +334,20 @@ private IUpdateEntry GetRootDocument(InternalEntityEntry entry) private static CosmosConcurrencyToken GetConcurrencyToken(IUpdateEntry entry) { - foreach (var property in entry.EntityType.GetProperties()) + var etagProperty = entry.EntityType.GetETagProperty(); + if (etagProperty == null) { - if (property.IsConcurrencyToken) - { - return CosmosConcurrencyToken.IfMatch((string)entry.GetCurrentValue(property)); - } + return CosmosConcurrencyToken.None; + } + + var etag = entry.GetCurrentValue(etagProperty); + var converter = etagProperty.GetTypeMapping().Converter; + if (converter != null) + { + etag = converter.ConvertToProvider(etag); } - return CosmosConcurrencyToken.None; + return CosmosConcurrencyToken.IfMatch((string)etag); } private static string GetPartitionKey(IUpdateEntry entry) From 434eb2eba075232cbcec2aa53c50657e53fc779c Mon Sep 17 00:00:00 2001 From: Jacob Viau Date: Tue, 21 Jan 2020 16:54:45 -0800 Subject: [PATCH 08/13] Etag convention fix, IfMatchEtag on delete - Ensure ETag convention runs before validation - Add IfMatchEtag option on delete for cosmos --- .../Metadata/Conventions/ETagPropertyConvention.cs | 3 ++- .../Internal/CosmosConventionSetBuilder.cs | 5 ++++- .../Storage/Internal/CosmosClientWrapper.cs | 13 ++++++++----- .../Storage/Internal/CosmosDatabaseWrapper.cs | 3 ++- 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/EFCore.Cosmos/Metadata/Conventions/ETagPropertyConvention.cs b/src/EFCore.Cosmos/Metadata/Conventions/ETagPropertyConvention.cs index d12a6030cbb..d7c28581b13 100644 --- a/src/EFCore.Cosmos/Metadata/Conventions/ETagPropertyConvention.cs +++ b/src/EFCore.Cosmos/Metadata/Conventions/ETagPropertyConvention.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 Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Metadata.Builders; using Microsoft.EntityFrameworkCore.Metadata.Conventions; @@ -16,7 +17,7 @@ public class ETagPropertyConvention : IModelFinalizedConvention /// /// The builder for the model. /// Additional information associated with convention execution. - public void ProcessModelFinalized( + public virtual void ProcessModelFinalized( IConventionModelBuilder modelBuilder, IConventionContext context) { diff --git a/src/EFCore.Cosmos/Metadata/Conventions/Internal/CosmosConventionSetBuilder.cs b/src/EFCore.Cosmos/Metadata/Conventions/Internal/CosmosConventionSetBuilder.cs index be86b3bb604..526e2bffe88 100644 --- a/src/EFCore.Cosmos/Metadata/Conventions/Internal/CosmosConventionSetBuilder.cs +++ b/src/EFCore.Cosmos/Metadata/Conventions/Internal/CosmosConventionSetBuilder.cs @@ -38,7 +38,10 @@ public override ConventionSet CreateConventionSet() var conventionSet = base.CreateConventionSet(); conventionSet.ModelInitializedConventions.Add(new ContextContainerConvention(Dependencies)); - conventionSet.ModelFinalizedConventions.Add(new ETagPropertyConvention()); + ConventionSet.AddBefore( + conventionSet.ModelFinalizedConventions, + new ETagPropertyConvention(), + typeof(ValidatingConvention)); var discriminatorConvention = new CosmosDiscriminatorConvention(Dependencies); var storeKeyConvention = new StoreKeyConvention(Dependencies); diff --git a/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs b/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs index d87563ef4ee..f99d9f1b631 100644 --- a/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs +++ b/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs @@ -372,9 +372,10 @@ private async Task ReplaceItemOnceAsync( public virtual bool DeleteItem( [NotNull] string containerId, [NotNull] string documentId, + CosmosConcurrencyToken concurrencyToken, [CanBeNull] string partitionKey) => _executionStrategyFactory.Create().Execute( - (containerId, documentId, partitionKey), DeleteItemOnce, null); + (containerId, documentId, concurrencyToken, partitionKey), DeleteItemOnce, null); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -384,7 +385,7 @@ public virtual bool DeleteItem( /// public virtual bool DeleteItemOnce( [NotNull] DbContext context, - (string ContainerId, string DocumentId, string PartitionKey) parameters) + (string ContainerId, string DocumentId, CosmosConcurrencyToken concurrencyToken, string PartitionKey) parameters) => DeleteItemOnceAsync(context, parameters).GetAwaiter().GetResult(); /// @@ -396,10 +397,11 @@ public virtual bool DeleteItemOnce( public virtual Task DeleteItemAsync( [NotNull] string containerId, [NotNull] string documentId, + CosmosConcurrencyToken concurrencyToken, [CanBeNull] string partitionKey, CancellationToken cancellationToken = default) => _executionStrategyFactory.Create().ExecuteAsync( - (containerId, documentId, partitionKey), DeleteItemOnceAsync, null, cancellationToken); + (containerId, documentId, concurrencyToken, partitionKey), DeleteItemOnceAsync, null, cancellationToken); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -409,13 +411,14 @@ public virtual Task DeleteItemAsync( /// public virtual async Task DeleteItemOnceAsync( [CanBeNull] DbContext _, - (string ContainerId, string DocumentId, string PartitionKey) parameters, + (string ContainerId, string DocumentId, CosmosConcurrencyToken ConcurrencyToken, string PartitionKey) parameters, CancellationToken cancellationToken = default) { var items = Client.GetDatabase(_databaseId).GetContainer(parameters.ContainerId); + var itemRequestOptions = CreateItemRequestOptions(parameters.ConcurrencyToken); var partitionKey = CreatePartitionKey(parameters.PartitionKey); using var response = await items.DeleteItemStreamAsync( - parameters.DocumentId, partitionKey, cancellationToken: cancellationToken); + parameters.DocumentId, partitionKey, itemRequestOptions, cancellationToken: cancellationToken); response.EnsureSuccessStatusCode(); return response.StatusCode == HttpStatusCode.NoContent; } diff --git a/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs b/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs index 7ddf23f44c9..9d901937b54 100644 --- a/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs +++ b/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs @@ -222,7 +222,8 @@ private bool Save(IUpdateEntry entry) GetPartitionKey(entry)); case EntityState.Deleted: - return _cosmosClient.DeleteItem(collectionId, documentSource.GetId(entry), GetPartitionKey(entry)); + return _cosmosClient.DeleteItem( + collectionId, documentSource.GetId(entry), GetConcurrencyToken(entry), GetPartitionKey(entry)); default: return false; } From d0a4d2ad52568da52c10e1b1db3da3f0f4336927 Mon Sep 17 00:00:00 2001 From: Jacob Viau Date: Wed, 22 Jan 2020 10:03:03 -0800 Subject: [PATCH 09/13] Add/fix etag builder tests --- .../CosmosPropertyBuilderExtensions.cs | 5 +++- .../Conventions/ETagPropertyConvention.cs | 1 - .../Storage/Internal/CosmosDatabaseWrapper.cs | 3 +- .../CosmosEntityTypeBuilderExtensionsTests.cs | 30 ------------------- .../Metadata/CosmosBuilderExtensionsTest.cs | 28 +++++++++++++++++ .../Metadata/CosmosMetadataExtensionsTest.cs | 25 ++++++++++++++++ 6 files changed, 59 insertions(+), 33 deletions(-) delete mode 100644 test/EFCore.Cosmos.Tests/Extensions/CosmosEntityTypeBuilderExtensionsTests.cs diff --git a/src/EFCore.Cosmos/Extensions/CosmosPropertyBuilderExtensions.cs b/src/EFCore.Cosmos/Extensions/CosmosPropertyBuilderExtensions.cs index 1444161d756..5c64c94780d 100644 --- a/src/EFCore.Cosmos/Extensions/CosmosPropertyBuilderExtensions.cs +++ b/src/EFCore.Cosmos/Extensions/CosmosPropertyBuilderExtensions.cs @@ -96,7 +96,10 @@ public static bool CanSetJsonProperty( public static PropertyBuilder SetEtagConcurrency([NotNull] this PropertyBuilder propertyBuilder) { Check.NotNull(propertyBuilder, nameof(propertyBuilder)); - propertyBuilder.IsConcurrencyToken().ToJsonProperty("_etag"); + propertyBuilder + .IsConcurrencyToken() + .ToJsonProperty("_etag") + .ValueGeneratedOnAddOrUpdate(); return propertyBuilder; } diff --git a/src/EFCore.Cosmos/Metadata/Conventions/ETagPropertyConvention.cs b/src/EFCore.Cosmos/Metadata/Conventions/ETagPropertyConvention.cs index d7c28581b13..7a9ea66617c 100644 --- a/src/EFCore.Cosmos/Metadata/Conventions/ETagPropertyConvention.cs +++ b/src/EFCore.Cosmos/Metadata/Conventions/ETagPropertyConvention.cs @@ -1,7 +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 Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Metadata.Builders; using Microsoft.EntityFrameworkCore.Metadata.Conventions; diff --git a/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs b/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs index 9d901937b54..fbea0013137 100644 --- a/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs +++ b/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs @@ -285,7 +285,8 @@ private Task SaveAsync(IUpdateEntry entry, CancellationToken cancellationT case EntityState.Deleted: return _cosmosClient.DeleteItemAsync( - collectionId, documentSource.GetId(entry), GetPartitionKey(entry), cancellationToken); + collectionId, documentSource.GetId(entry), + GetConcurrencyToken(entry), GetPartitionKey(entry), cancellationToken); default: return Task.FromResult(false); } diff --git a/test/EFCore.Cosmos.Tests/Extensions/CosmosEntityTypeBuilderExtensionsTests.cs b/test/EFCore.Cosmos.Tests/Extensions/CosmosEntityTypeBuilderExtensionsTests.cs deleted file mode 100644 index debf4286967..00000000000 --- a/test/EFCore.Cosmos.Tests/Extensions/CosmosEntityTypeBuilderExtensionsTests.cs +++ /dev/null @@ -1,30 +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 Microsoft.EntityFrameworkCore.Cosmos.TestUtilities; -using Microsoft.EntityFrameworkCore.Infrastructure; -using Microsoft.EntityFrameworkCore.Metadata; -using Microsoft.EntityFrameworkCore.TestUtilities; -using Xunit; - -// ReSharper disable once CheckNamespace -namespace Microsoft.EntityFrameworkCore -{ - public class CosmosEntityTypeBuilderExtensionsTests : ModelValidatorTestBase - { - protected override TestHelpers TestHelpers => CosmosTestHelpers.Instance; - - [ConditionalFact] - public void Can_set_etag_concurrency() - { - var modelBuilder = CreateConventionalModelBuilder(); - modelBuilder.Entity().UseEtagConcurrency(); - var model = modelBuilder.Model; - - var etagProperty = model.FindEntityType(typeof(Customer).FullName).FindProperty("_etag"); - Assert.NotNull(etagProperty); - Assert.Equal(ValueGenerated.OnAddOrUpdate, etagProperty.ValueGenerated); - Assert.True(etagProperty.IsConcurrencyToken); - } - } -} diff --git a/test/EFCore.Cosmos.Tests/Metadata/CosmosBuilderExtensionsTest.cs b/test/EFCore.Cosmos.Tests/Metadata/CosmosBuilderExtensionsTest.cs index 4ec789f66cc..77723184caf 100644 --- a/test/EFCore.Cosmos.Tests/Metadata/CosmosBuilderExtensionsTest.cs +++ b/test/EFCore.Cosmos.Tests/Metadata/CosmosBuilderExtensionsTest.cs @@ -109,6 +109,33 @@ public void Default_discriminator_can_be_removed() Assert.Null(entityType.GetDiscriminatorProperty()); } + [ConditionalFact] + public void Can_set_etag_concurrency_entity() + { + var modelBuilder = CreateConventionModelBuilder(); + modelBuilder.Entity().UseEtagConcurrency(); + var model = modelBuilder.Model; + + var etagProperty = model.FindEntityType(typeof(Customer).FullName).FindProperty("_etag"); + Assert.NotNull(etagProperty); + Assert.Equal(ValueGenerated.OnAddOrUpdate, etagProperty.ValueGenerated); + Assert.True(etagProperty.IsConcurrencyToken); + } + + [ConditionalFact] + public void Can_set_etag_concurrency_property() + { + var modelBuilder = CreateConventionModelBuilder(); + modelBuilder.Entity().Property(x => x.ETag).SetEtagConcurrency(); + var model = modelBuilder.Model; + + var etagProperty = model.FindEntityType(typeof(Customer).FullName).FindProperty("ConcurrencyToken"); + Assert.NotNull(etagProperty); + Assert.Equal(ValueGenerated.OnAddOrUpdate, etagProperty.ValueGenerated); + Assert.True(etagProperty.IsConcurrencyToken); + Assert.Equal("_etag", etagProperty.GetJsonPropertyName()); + } + protected virtual ModelBuilder CreateConventionModelBuilder() => CosmosTestHelpers.Instance.CreateConventionBuilder(); private class Customer @@ -116,6 +143,7 @@ private class Customer public int Id { get; set; } public string Name { get; set; } public short SomeShort { get; set; } + public string ETag { get; set; } } } } diff --git a/test/EFCore.Cosmos.Tests/Metadata/CosmosMetadataExtensionsTest.cs b/test/EFCore.Cosmos.Tests/Metadata/CosmosMetadataExtensionsTest.cs index 90cd89a2a45..b93fcccd5e6 100644 --- a/test/EFCore.Cosmos.Tests/Metadata/CosmosMetadataExtensionsTest.cs +++ b/test/EFCore.Cosmos.Tests/Metadata/CosmosMetadataExtensionsTest.cs @@ -61,6 +61,31 @@ public void Can_get_and_set_partition_key_name() Assert.Null(((IConventionEntityType)entityType).GetPartitionKeyPropertyNameConfigurationSource()); } + [ConditionalFact] + public void Can_get_and_set_etag_name() + { + var modelBuilder = CreateModelBuilder(); + + var entityType = modelBuilder + .Entity().Metadata; + + Assert.Null(entityType.GetETagPropertyName()); + + ((IConventionEntityType)entityType).SetETagPropertyName("etag"); + Assert.Equal("etag", entityType.GetETagPropertyName()); + Assert.Equal( + ConfigurationSource.Convention, ((IConventionEntityType)entityType).GetETagPropertyNameConfigurationSource()); + + entityType.SetETagPropertyName("etag"); + Assert.Equal("etag", entityType.GetETagPropertyName()); + Assert.Equal( + ConfigurationSource.Explicit, ((IConventionEntityType)entityType).GetETagPropertyNameConfigurationSource()); + + entityType.SetETagPropertyName(null); + Assert.Null(entityType.GetETagPropertyName()); + Assert.Null(((IConventionEntityType)entityType).GetETagPropertyNameConfigurationSource()); + } + private static ModelBuilder CreateModelBuilder() => new ModelBuilder(new ConventionSet()); private class Customer From f98b609f1d6a7f119b13118b8b5d6aff05f0ba93 Mon Sep 17 00:00:00 2001 From: Jacob Viau Date: Thu, 30 Jan 2020 15:26:54 -0800 Subject: [PATCH 10/13] Fix Can_set_etag_concurrency_property --- .../EFCore.Cosmos.Tests/Metadata/CosmosBuilderExtensionsTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/EFCore.Cosmos.Tests/Metadata/CosmosBuilderExtensionsTest.cs b/test/EFCore.Cosmos.Tests/Metadata/CosmosBuilderExtensionsTest.cs index 77723184caf..35657632fac 100644 --- a/test/EFCore.Cosmos.Tests/Metadata/CosmosBuilderExtensionsTest.cs +++ b/test/EFCore.Cosmos.Tests/Metadata/CosmosBuilderExtensionsTest.cs @@ -129,7 +129,7 @@ public void Can_set_etag_concurrency_property() modelBuilder.Entity().Property(x => x.ETag).SetEtagConcurrency(); var model = modelBuilder.Model; - var etagProperty = model.FindEntityType(typeof(Customer).FullName).FindProperty("ConcurrencyToken"); + var etagProperty = model.FindEntityType(typeof(Customer).FullName).FindProperty("ETag"); Assert.NotNull(etagProperty); Assert.Equal(ValueGenerated.OnAddOrUpdate, etagProperty.ValueGenerated); Assert.True(etagProperty.IsConcurrencyToken); From b90406c1d76654cbf6b9547fbb88d9838dc97bd4 Mon Sep 17 00:00:00 2001 From: Jacob Viau Date: Mon, 3 Feb 2020 17:16:48 -0800 Subject: [PATCH 11/13] Add CosmosConcurrencyTests - Add CosmosConcurrencyTests - Capture and convert some CosmosException to DbUpdate(Concurrency)Exception --- .../CosmosPropertyBuilderExtensions.cs | 6 +- .../Properties/CosmosStrings.Designer.cs | 4 +- .../Properties/CosmosStrings.resx | 2 +- .../Storage/Internal/CosmosClientWrapper.cs | 35 ++--- .../Storage/Internal/CosmosDatabaseWrapper.cs | 52 ++++++- .../CosmosConcurrencyTest.cs | 145 ++++++++++++++++++ .../TestUtilities/CosmosTestStore.cs | 2 +- .../Metadata/CosmosBuilderExtensionsTest.cs | 2 +- 8 files changed, 212 insertions(+), 36 deletions(-) create mode 100644 test/EFCore.Cosmos.FunctionalTests/CosmosConcurrencyTest.cs diff --git a/src/EFCore.Cosmos/Extensions/CosmosPropertyBuilderExtensions.cs b/src/EFCore.Cosmos/Extensions/CosmosPropertyBuilderExtensions.cs index 5c64c94780d..4ccb130a41e 100644 --- a/src/EFCore.Cosmos/Extensions/CosmosPropertyBuilderExtensions.cs +++ b/src/EFCore.Cosmos/Extensions/CosmosPropertyBuilderExtensions.cs @@ -93,7 +93,7 @@ public static bool CanSetJsonProperty( /// /// The builder for the property being configured. /// The same builder instance so that multiple calls can be chained. - public static PropertyBuilder SetEtagConcurrency([NotNull] this PropertyBuilder propertyBuilder) + public static PropertyBuilder IsEtagConcurrency([NotNull] this PropertyBuilder propertyBuilder) { Check.NotNull(propertyBuilder, nameof(propertyBuilder)); propertyBuilder @@ -109,8 +109,8 @@ public static PropertyBuilder SetEtagConcurrency([NotNull] this PropertyBuilder /// The type of the property being configured. /// The builder for the property being configured. /// The same builder instance so that multiple calls can be chained. - public static PropertyBuilder SetEtagConcurrency( + public static PropertyBuilder IsEtagConcurrency( [NotNull] this PropertyBuilder propertyBuilder) - => (PropertyBuilder)SetEtagConcurrency((PropertyBuilder)propertyBuilder); + => (PropertyBuilder)IsEtagConcurrency((PropertyBuilder)propertyBuilder); } } diff --git a/src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs b/src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs index 2ea506e5bde..75f4decaab0 100644 --- a/src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs +++ b/src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs @@ -115,9 +115,9 @@ public static string PartitionKeyStoreNameMismatch([CanBeNull] object property1, /// /// Conflicts were detected for item with id '{itemId}'. /// - public static string UpdateConcurrencyTokenException([CanBeNull] object itemId) + public static string UpdateConflict([CanBeNull] object itemId) => string.Format( - GetString("UpdateConcurrencyTokenException", nameof(itemId)), + GetString("UpdateConflict", nameof(itemId)), itemId); private static string GetString(string name, params string[] formatterNames) diff --git a/src/EFCore.Cosmos/Properties/CosmosStrings.resx b/src/EFCore.Cosmos/Properties/CosmosStrings.resx index 8dd23f4080e..629497ae6bd 100644 --- a/src/EFCore.Cosmos/Properties/CosmosStrings.resx +++ b/src/EFCore.Cosmos/Properties/CosmosStrings.resx @@ -153,7 +153,7 @@ The partition key property '{property1}' on '{entityType1}' is mapped as '{storeName1}', but the partition key property '{property2}' on '{entityType2}' is mapped as '{storeName2}'. All partition key properties need to be mapped to the same store property. - + Conflicts were detected for item with id '{itemId}'. \ No newline at end of file diff --git a/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs b/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs index f99d9f1b631..6ccc1aeace5 100644 --- a/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs +++ b/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs @@ -252,13 +252,14 @@ private async Task CreateContainerIfNotExistsOnceAsync( public virtual bool CreateItem( [NotNull] string containerId, [NotNull] JToken document, + CosmosConcurrencyToken concurrencyToken, [CanBeNull] string partitionKey) => _executionStrategyFactory.Create().Execute( - (containerId, document, partitionKey), CreateItemOnce, null); + (containerId, document, concurrencyToken, partitionKey), CreateItemOnce, null); private bool CreateItemOnce( DbContext context, - (string ContainerId, JToken Document, string PartitionKey) parameters) + (string ContainerId, JToken Document, CosmosConcurrencyToken ConcurrencyToken, string PartitionKey) parameters) => CreateItemOnceAsync(context, parameters).GetAwaiter().GetResult(); /// @@ -270,14 +271,15 @@ private bool CreateItemOnce( public virtual Task CreateItemAsync( [NotNull] string containerId, [NotNull] JToken document, + CosmosConcurrencyToken concurrencyToken, [CanBeNull] string partitionKey, CancellationToken cancellationToken = default) => _executionStrategyFactory.Create().ExecuteAsync( - (containerId, document, partitionKey), CreateItemOnceAsync, null, cancellationToken); + (containerId, document, concurrencyToken, partitionKey), CreateItemOnceAsync, null, cancellationToken); private async Task CreateItemOnceAsync( DbContext _, - (string ContainerId, JToken Document, string PartitionKey) parameters, + (string ContainerId, JToken Document, CosmosConcurrencyToken ConcurrencyToken, string PartitionKey) parameters, CancellationToken cancellationToken = default) { await using var stream = new MemoryStream(); @@ -287,9 +289,10 @@ private async Task CreateItemOnceAsync( await jsonWriter.FlushAsync(cancellationToken); var container = Client.GetDatabase(_databaseId).GetContainer(parameters.ContainerId); + var itemRequestOptions = CreateItemRequestOptions(parameters.ConcurrencyToken); var partitionKey = CreatePartitionKey(parameters.PartitionKey); - using var response = await container.CreateItemStreamAsync(stream, partitionKey, null, cancellationToken); + using var response = await container.CreateItemStreamAsync(stream, partitionKey, itemRequestOptions, cancellationToken); response.EnsureSuccessStatusCode(); return response.StatusCode == HttpStatusCode.Created; } @@ -350,17 +353,10 @@ private async Task ReplaceItemOnceAsync( var itemRequestOptions = CreateItemRequestOptions(parameters.ConcurrencyToken); var partitionKey = CreatePartitionKey(parameters.PartitionKey); - try - { - using var response = await container.ReplaceItemStreamAsync( - stream, parameters.ItemId, partitionKey, itemRequestOptions, cancellationToken); - response.EnsureSuccessStatusCode(); - return response.StatusCode == HttpStatusCode.OK; - } - catch (CosmosException cre) when (cre.StatusCode == HttpStatusCode.PreconditionFailed) - { - throw ThrowConcurrencyException(parameters.ItemId, cre); - } + using var response = await container.ReplaceItemStreamAsync( + stream, parameters.ItemId, partitionKey, itemRequestOptions, cancellationToken); + response.EnsureSuccessStatusCode(); + return response.StatusCode == HttpStatusCode.OK; } /// @@ -417,18 +413,13 @@ public virtual async Task DeleteItemOnceAsync( var items = Client.GetDatabase(_databaseId).GetContainer(parameters.ContainerId); var itemRequestOptions = CreateItemRequestOptions(parameters.ConcurrencyToken); var partitionKey = CreatePartitionKey(parameters.PartitionKey); + using var response = await items.DeleteItemStreamAsync( parameters.DocumentId, partitionKey, itemRequestOptions, cancellationToken: cancellationToken); response.EnsureSuccessStatusCode(); return response.StatusCode == HttpStatusCode.NoContent; } - private static Exception ThrowConcurrencyException(string itemId, CosmosException cosmosException) - { - throw new DbUpdateConcurrencyException( - CosmosStrings.UpdateConcurrencyTokenException(itemId), cosmosException); - } - private PartitionKey CreatePartitionKey(string partitionKey) => partitionKey == null ? PartitionKey.None diff --git a/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs b/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs index fbea0013137..8644dbe0458 100644 --- a/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs +++ b/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs @@ -3,9 +3,11 @@ using System; using System.Collections.Generic; +using System.Net; using System.Threading; using System.Threading.Tasks; using JetBrains.Annotations; +using Microsoft.Azure.Cosmos; using Microsoft.EntityFrameworkCore.ChangeTracking.Internal; using Microsoft.EntityFrameworkCore.Cosmos.Internal; using Microsoft.EntityFrameworkCore.Cosmos.Metadata.Internal; @@ -18,6 +20,8 @@ using Microsoft.Extensions.DependencyInjection; using Newtonsoft.Json.Linq; +using Database = Microsoft.EntityFrameworkCore.Storage.Database; + namespace Microsoft.EntityFrameworkCore.Cosmos.Storage.Internal { /// @@ -96,9 +100,17 @@ public override int SaveChanges(IList entries) } entriesSaved.Add(entry); - if (Save(entry)) + + try { - rowsAffected++; + if (Save(entry)) + { + rowsAffected++; + } + } + catch (CosmosException ex) + { + throw ThrowUpdateException(ex, entry); } } @@ -149,9 +161,16 @@ public override async Task SaveChangesAsync( } entriesSaved.Add(entry); - if (await SaveAsync(entry, cancellationToken)) + try { - rowsAffected++; + if (await SaveAsync(entry, cancellationToken)) + { + rowsAffected++; + } + } + catch (CosmosException ex) + { + throw ThrowUpdateException(ex, entry); } } @@ -192,7 +211,8 @@ private bool Save(IUpdateEntry entry) case EntityState.Added: var newDocument = documentSource.CreateDocument(entry); - return _cosmosClient.CreateItem(collectionId, newDocument, GetPartitionKey(entry)); + return _cosmosClient.CreateItem( + collectionId, newDocument, GetConcurrencyToken(entry), GetPartitionKey(entry)); case EntityState.Modified: var document = documentSource.GetCurrentDocument(entry); if (document != null) @@ -253,7 +273,8 @@ private Task SaveAsync(IUpdateEntry entry, CancellationToken cancellationT { case EntityState.Added: var newDocument = documentSource.CreateDocument(entry); - return _cosmosClient.CreateItemAsync(collectionId, newDocument, GetPartitionKey(entry), cancellationToken); + return _cosmosClient.CreateItemAsync( + collectionId, newDocument, GetConcurrencyToken(entry), GetPartitionKey(entry), cancellationToken); case EntityState.Modified: var document = documentSource.GetCurrentDocument(entry); if (document != null) @@ -334,6 +355,18 @@ private IUpdateEntry GetRootDocument(InternalEntityEntry entry) return principal.EntityType.IsDocumentRoot() ? principal : GetRootDocument(principal); } + private Exception ThrowUpdateException(CosmosException exception, IUpdateEntry entry) + { + var documentSource = GetDocumentSource(entry.EntityType); + var id = documentSource.GetId(entry.SharedIdentityEntry ?? entry); + throw exception.StatusCode switch + { + HttpStatusCode.PreconditionFailed => new DbUpdateConcurrencyException(CosmosStrings.UpdateConflict(id), exception, new[] { entry }), + HttpStatusCode.Conflict => new DbUpdateException(CosmosStrings.UpdateConflict(id), exception, new[] { entry }), + _ => Rethrow(exception), + }; + } + private static CosmosConcurrencyToken GetConcurrencyToken(IUpdateEntry entry) { var etagProperty = entry.EntityType.GetETagProperty(); @@ -370,5 +403,12 @@ private static string GetPartitionKey(IUpdateEntry entry) return (string)partitionKey; } + + private static Exception Rethrow(Exception ex) + { + // Re-throw an exception, preserving the original stack and details, without being in the original "catch" block. + System.Runtime.ExceptionServices.ExceptionDispatchInfo.Capture(ex).Throw(); + return ex; + } } } diff --git a/test/EFCore.Cosmos.FunctionalTests/CosmosConcurrencyTest.cs b/test/EFCore.Cosmos.FunctionalTests/CosmosConcurrencyTest.cs new file mode 100644 index 00000000000..5a6c80bfc7e --- /dev/null +++ b/test/EFCore.Cosmos.FunctionalTests/CosmosConcurrencyTest.cs @@ -0,0 +1,145 @@ +using System; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.EntityFrameworkCore.Cosmos.TestUtilities; +using Microsoft.EntityFrameworkCore.TestUtilities; +using Xunit; + +namespace Microsoft.EntityFrameworkCore.Cosmos +{ + public class CosmosConcurrencyTest : IClassFixture + { + private const string DatabaseName = "CosmosConcurrencyTest"; + + protected CosmosFixture Fixture { get; } + + public CosmosConcurrencyTest(CosmosFixture fixture) + { + Fixture = fixture; + } + + [ConditionalFact] + public virtual Task Adding_the_same_entity_twice_results_in_DbUpdateException() + { + static void AddAction(ConcurrencyContext c) => + c.Customers.Add( + new Customer + { + Id = "1", + Name = "CreatedTwice", + }); + + return ConcurrencyTestAsync(AddAction); + } + + [ConditionalFact] + public virtual Task Updating_then_deleting_the_same_entity_results_in_DbUpdateConcurrencyException() + { + return ConcurrencyTestAsync( + ctx => ctx.Customers.Add(new Customer { Id = "2", Name = "Added", }), + ctx => ctx.Customers.Single(c => c.Id == "2").Name = "Updated", + ctx => ctx.Customers.Remove(ctx.Customers.Single(c => c.Id == "2"))); + } + + [ConditionalFact] + public virtual Task Updating_then_updating_the_same_entity_results_in_DbUpdateConcurrencyException() + { + return ConcurrencyTestAsync( + ctx => ctx.Customers.Add(new Customer { Id = "3", Name = "Added", }), + ctx => ctx.Customers.Single(c => c.Id == "3").Name = "Updated", + ctx => ctx.Customers.Single(c => c.Id == "3").Name = "Updated"); + } + + /// + /// Runs the two actions with two different contexts and calling + /// SaveChanges such that storeChange will succeed and the store will reflect this change, and + /// then clientChange will result in a concurrency exception. + /// After the exception is caught the resolver action is called, after which SaveChanges is called + /// again. Finally, a new context is created and the validator is called so that the state of + /// the database at the end of the process can be validated. + /// + protected virtual Task ConcurrencyTestAsync( + Action change) + where TException : DbUpdateException + => ConcurrencyTestAsync( + null, change, change); + + /// + /// Runs the two actions with two different contexts and calling + /// SaveChanges such that storeChange will succeed and the store will reflect this change, and + /// then clientChange will result in a concurrency exception. + /// After the exception is caught the resolver action is called, after which SaveChanges is called + /// again. Finally, a new context is created and the validator is called so that the state of + /// the database at the end of the process can be validated. + /// + protected virtual async Task ConcurrencyTestAsync( + Action seedAction, + Action storeChange, + Action clientChange) + where TException : DbUpdateException + { + // Issue: DB Generated values are not fetched when + // added, saved, and fetched in the same context. + // So we use a different context to "seed" the values. + using (var seedContext = CreateContext()) + { + await seedContext.Database.EnsureCreatedAsync(); + seedAction?.Invoke(seedContext); + await seedContext.SaveChangesAsync(); + } + + using var outerContext = CreateContext(); + clientChange?.Invoke(outerContext); + + using (var innerContext = CreateContext()) + { + storeChange?.Invoke(innerContext); + await innerContext.SaveChangesAsync(); + } + + var updateException = + await Assert.ThrowsAnyAsync(() => outerContext.SaveChangesAsync()); + + var entry = updateException.Entries.Single(); + Assert.IsAssignableFrom(entry.Entity); + } + + protected ConcurrencyContext CreateContext() => Fixture.CreateContext(); + + public class CosmosFixture : SharedStoreFixtureBase + { + protected override string StoreName => DatabaseName; + + protected override ITestStoreFactory TestStoreFactory => CosmosTestStoreFactory.Instance; + } + + public class ConcurrencyContext : PoolableDbContext + { + public ConcurrencyContext(DbContextOptions options) + : base(options) + { + } + + public DbSet Customers { get; set; } + + protected override void OnModelCreating(ModelBuilder builder) + { + builder.Entity( + b => + { + b.HasKey(c => c.Id); + b.Property(c => c.ETag).IsEtagConcurrency(); + }); + } + } + + public class Customer + { + public string Id { get; set; } + + public string Name { get; set; } + + public string ETag { get; set; } + } + } +} diff --git a/test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosTestStore.cs b/test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosTestStore.cs index 7c3939ab648..e66ed7520b9 100644 --- a/test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosTestStore.cs +++ b/test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosTestStore.cs @@ -129,7 +129,7 @@ private async Task CreateFromFile(DbContext context) document["id"] = $"{entityName}|{document["id"]}"; document["Discriminator"] = entityName; - await cosmosClient.CreateItemAsync("NorthwindContext", document, null); + await cosmosClient.CreateItemAsync("NorthwindContext", document, CosmosConcurrencyToken.None, null); } else if (reader.TokenType == JsonToken.EndObject) { diff --git a/test/EFCore.Cosmos.Tests/Metadata/CosmosBuilderExtensionsTest.cs b/test/EFCore.Cosmos.Tests/Metadata/CosmosBuilderExtensionsTest.cs index 35657632fac..0e9b842250b 100644 --- a/test/EFCore.Cosmos.Tests/Metadata/CosmosBuilderExtensionsTest.cs +++ b/test/EFCore.Cosmos.Tests/Metadata/CosmosBuilderExtensionsTest.cs @@ -126,7 +126,7 @@ public void Can_set_etag_concurrency_entity() public void Can_set_etag_concurrency_property() { var modelBuilder = CreateConventionModelBuilder(); - modelBuilder.Entity().Property(x => x.ETag).SetEtagConcurrency(); + modelBuilder.Entity().Property(x => x.ETag).IsEtagConcurrency(); var model = modelBuilder.Model; var etagProperty = model.FindEntityType(typeof(Customer).FullName).FindProperty("ETag"); From 30d0527640ffcd9d4baa1d38e81e9c2f2052cd57 Mon Sep 17 00:00:00 2001 From: Jacob Viau Date: Tue, 4 Feb 2020 19:02:11 -0800 Subject: [PATCH 12/13] Update ETag value for cosmos from response - Update etag value for cosmos updates & inserts - Fix concurrency test to use single context for seed and update --- .../Storage/Internal/CosmosClientWrapper.cs | 35 ++++++++++++++----- .../Storage/Internal/CosmosDatabaseWrapper.cs | 6 ++-- .../CosmosConcurrencyTest.cs | 25 ++++--------- .../TestUtilities/CosmosTestStore.cs | 24 ++++++++++++- 4 files changed, 60 insertions(+), 30 deletions(-) diff --git a/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs b/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs index 6ccc1aeace5..a173b8bb9fd 100644 --- a/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs +++ b/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs @@ -18,6 +18,7 @@ using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Storage; +using Microsoft.EntityFrameworkCore.Update; using Microsoft.EntityFrameworkCore.Utilities; using Microsoft.Extensions.DependencyInjection; using Newtonsoft.Json; @@ -252,14 +253,15 @@ private async Task CreateContainerIfNotExistsOnceAsync( public virtual bool CreateItem( [NotNull] string containerId, [NotNull] JToken document, + [NotNull] IUpdateEntry updateEntry, CosmosConcurrencyToken concurrencyToken, [CanBeNull] string partitionKey) => _executionStrategyFactory.Create().Execute( - (containerId, document, concurrencyToken, partitionKey), CreateItemOnce, null); + (containerId, document, updateEntry, concurrencyToken, partitionKey), CreateItemOnce, null); private bool CreateItemOnce( DbContext context, - (string ContainerId, JToken Document, CosmosConcurrencyToken ConcurrencyToken, string PartitionKey) parameters) + (string ContainerId, JToken Document, IUpdateEntry UpdateEntry, CosmosConcurrencyToken ConcurrencyToken, string PartitionKey) parameters) => CreateItemOnceAsync(context, parameters).GetAwaiter().GetResult(); /// @@ -271,15 +273,16 @@ private bool CreateItemOnce( public virtual Task CreateItemAsync( [NotNull] string containerId, [NotNull] JToken document, + [NotNull] IUpdateEntry updateEntry, CosmosConcurrencyToken concurrencyToken, [CanBeNull] string partitionKey, CancellationToken cancellationToken = default) => _executionStrategyFactory.Create().ExecuteAsync( - (containerId, document, concurrencyToken, partitionKey), CreateItemOnceAsync, null, cancellationToken); + (containerId, document, updateEntry, concurrencyToken, partitionKey), CreateItemOnceAsync, null, cancellationToken); private async Task CreateItemOnceAsync( DbContext _, - (string ContainerId, JToken Document, CosmosConcurrencyToken ConcurrencyToken, string PartitionKey) parameters, + (string ContainerId, JToken Document, IUpdateEntry UpdateEntry, CosmosConcurrencyToken ConcurrencyToken, string PartitionKey) parameters, CancellationToken cancellationToken = default) { await using var stream = new MemoryStream(); @@ -294,6 +297,13 @@ private async Task CreateItemOnceAsync( using var response = await container.CreateItemStreamAsync(stream, partitionKey, itemRequestOptions, cancellationToken); response.EnsureSuccessStatusCode(); + + if (parameters.ConcurrencyToken.Mode != CosmosConcurrencyMode.None) + { + var updateEntry = parameters.UpdateEntry; + updateEntry.SetStoreGeneratedValue(updateEntry.EntityType.GetETagProperty(), response.Headers.ETag); + } + return response.StatusCode == HttpStatusCode.Created; } @@ -307,16 +317,17 @@ public virtual bool ReplaceItem( [NotNull] string collectionId, [NotNull] string documentId, [NotNull] JObject document, + [NotNull] IUpdateEntry updateEntry, CosmosConcurrencyToken concurrencyToken, [CanBeNull] string partitionKey) => _executionStrategyFactory.Create().Execute( - (collectionId, documentId, document, concurrencyToken, partitionKey), + (collectionId, documentId, document, updateEntry, concurrencyToken, partitionKey), ReplaceItemOnce, null); private bool ReplaceItemOnce( DbContext context, - (string ContainerId, string ItemId, JObject Document, CosmosConcurrencyToken concurrencyToken, string PartitionKey) parameters) + (string ContainerId, string ItemId, JObject Document, IUpdateEntry UpdateEntry, CosmosConcurrencyToken concurrencyToken, string PartitionKey) parameters) => ReplaceItemOnceAsync(context, parameters).GetAwaiter().GetResult(); /// @@ -329,18 +340,19 @@ public virtual Task ReplaceItemAsync( [NotNull] string collectionId, [NotNull] string documentId, [NotNull] JObject document, + [NotNull] IUpdateEntry updateEntry, CosmosConcurrencyToken concurrencyToken, [CanBeNull] string partitionKey, CancellationToken cancellationToken = default) => _executionStrategyFactory.Create().ExecuteAsync( - (collectionId, documentId, document, concurrencyToken, partitionKey), + (collectionId, documentId, document, updateEntry, concurrencyToken, partitionKey), ReplaceItemOnceAsync, null, cancellationToken); private async Task ReplaceItemOnceAsync( DbContext _, - (string ContainerId, string ItemId, JObject Document, CosmosConcurrencyToken ConcurrencyToken, string PartitionKey) parameters, + (string ContainerId, string ItemId, JObject Document, IUpdateEntry UpdateEntry, CosmosConcurrencyToken ConcurrencyToken, string PartitionKey) parameters, CancellationToken cancellationToken = default) { using var stream = new MemoryStream(); @@ -356,6 +368,13 @@ private async Task ReplaceItemOnceAsync( using var response = await container.ReplaceItemStreamAsync( stream, parameters.ItemId, partitionKey, itemRequestOptions, cancellationToken); response.EnsureSuccessStatusCode(); + + if (parameters.ConcurrencyToken.Mode != CosmosConcurrencyMode.None) + { + var updateEntry = parameters.UpdateEntry; + updateEntry.SetStoreGeneratedValue(updateEntry.EntityType.GetETagProperty(), response.Headers.ETag); + } + return response.StatusCode == HttpStatusCode.OK; } diff --git a/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs b/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs index 8644dbe0458..e8be7a4f8bc 100644 --- a/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs +++ b/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs @@ -212,7 +212,7 @@ private bool Save(IUpdateEntry entry) var newDocument = documentSource.CreateDocument(entry); return _cosmosClient.CreateItem( - collectionId, newDocument, GetConcurrencyToken(entry), GetPartitionKey(entry)); + collectionId, newDocument, entry, GetConcurrencyToken(entry), GetPartitionKey(entry)); case EntityState.Modified: var document = documentSource.GetCurrentDocument(entry); if (document != null) @@ -238,6 +238,7 @@ private bool Save(IUpdateEntry entry) collectionId, documentSource.GetId(entry.SharedIdentityEntry ?? entry), document, + entry, GetConcurrencyToken(entry), GetPartitionKey(entry)); @@ -274,7 +275,7 @@ private Task SaveAsync(IUpdateEntry entry, CancellationToken cancellationT case EntityState.Added: var newDocument = documentSource.CreateDocument(entry); return _cosmosClient.CreateItemAsync( - collectionId, newDocument, GetConcurrencyToken(entry), GetPartitionKey(entry), cancellationToken); + collectionId, newDocument, entry, GetConcurrencyToken(entry), GetPartitionKey(entry), cancellationToken); case EntityState.Modified: var document = documentSource.GetCurrentDocument(entry); if (document != null) @@ -300,6 +301,7 @@ private Task SaveAsync(IUpdateEntry entry, CancellationToken cancellationT collectionId, documentSource.GetId(entry.SharedIdentityEntry ?? entry), document, + entry, GetConcurrencyToken(entry), GetPartitionKey(entry), cancellationToken); diff --git a/test/EFCore.Cosmos.FunctionalTests/CosmosConcurrencyTest.cs b/test/EFCore.Cosmos.FunctionalTests/CosmosConcurrencyTest.cs index 5a6c80bfc7e..3c41a194ff7 100644 --- a/test/EFCore.Cosmos.FunctionalTests/CosmosConcurrencyTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/CosmosConcurrencyTest.cs @@ -21,15 +21,8 @@ public CosmosConcurrencyTest(CosmosFixture fixture) [ConditionalFact] public virtual Task Adding_the_same_entity_twice_results_in_DbUpdateException() { - static void AddAction(ConcurrencyContext c) => - c.Customers.Add( - new Customer - { - Id = "1", - Name = "CreatedTwice", - }); - - return ConcurrencyTestAsync(AddAction); + return ConcurrencyTestAsync( + ctx => ctx.Customers.Add(new Customer { Id = "1", Name = "CreatedTwice", })); } [ConditionalFact] @@ -78,17 +71,11 @@ protected virtual async Task ConcurrencyTestAsync( Action clientChange) where TException : DbUpdateException { - // Issue: DB Generated values are not fetched when - // added, saved, and fetched in the same context. - // So we use a different context to "seed" the values. - using (var seedContext = CreateContext()) - { - await seedContext.Database.EnsureCreatedAsync(); - seedAction?.Invoke(seedContext); - await seedContext.SaveChangesAsync(); - } - using var outerContext = CreateContext(); + await outerContext.Database.EnsureCreatedAsync(); + seedAction?.Invoke(outerContext); + await outerContext.SaveChangesAsync(); + clientChange?.Invoke(outerContext); using (var innerContext = CreateContext()) diff --git a/test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosTestStore.cs b/test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosTestStore.cs index e66ed7520b9..6779eb1c681 100644 --- a/test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosTestStore.cs +++ b/test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosTestStore.cs @@ -6,10 +6,13 @@ using System.IO; using System.Threading.Tasks; using Microsoft.Azure.Cosmos; +using Microsoft.EntityFrameworkCore.ChangeTracking; using Microsoft.EntityFrameworkCore.Cosmos.Storage.Internal; using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Storage; using Microsoft.EntityFrameworkCore.TestUtilities; +using Microsoft.EntityFrameworkCore.Update; using Newtonsoft.Json; using Newtonsoft.Json.Linq; @@ -129,7 +132,8 @@ private async Task CreateFromFile(DbContext context) document["id"] = $"{entityName}|{document["id"]}"; document["Discriminator"] = entityName; - await cosmosClient.CreateItemAsync("NorthwindContext", document, CosmosConcurrencyToken.None, null); + await cosmosClient.CreateItemAsync( + "NorthwindContext", document, new FakeUpdateEntry(), CosmosConcurrencyToken.None, null); } else if (reader.TokenType == JsonToken.EndObject) { @@ -243,5 +247,23 @@ protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) optionsBuilder.UseCosmos(_testStore.ConnectionUri, _testStore.AuthToken, _testStore.Name, _testStore._configureCosmos); } } + + private class FakeUpdateEntry : IUpdateEntry + { + public IEntityType EntityType => throw new NotImplementedException(); + public EntityState EntityState { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } + public IUpdateEntry SharedIdentityEntry => throw new NotImplementedException(); + public object GetCurrentValue(IPropertyBase propertyBase) => throw new NotImplementedException(); + public TProperty GetCurrentValue(IPropertyBase propertyBase) => throw new NotImplementedException(); + public object GetOriginalValue(IPropertyBase propertyBase) => throw new NotImplementedException(); + public TProperty GetOriginalValue(IProperty property) => throw new NotImplementedException(); + public bool HasTemporaryValue(IProperty property) => throw new NotImplementedException(); + public bool IsModified(IProperty property) => throw new NotImplementedException(); + public bool IsStoreGenerated(IProperty property) => throw new NotImplementedException(); + public void SetOriginalValue(IProperty property, object value) => throw new NotImplementedException(); + public void SetPropertyModified(IProperty property) => throw new NotImplementedException(); + public void SetStoreGeneratedValue(IProperty property, object value) => throw new NotImplementedException(); + public EntityEntry ToEntityEntry() => throw new NotImplementedException(); + } } } From 09d1dc2c6322393de4cfea98c22b7cd808e2ee6c Mon Sep 17 00:00:00 2001 From: Jacob Viau Date: Tue, 4 Feb 2020 21:14:22 -0800 Subject: [PATCH 13/13] Simplify CosmosClientWrapper parameters - Reduce parameters in CosmosClientWrapper methods by using IUpdateEntry --- .../Storage/Internal/CosmosClientWrapper.cs | 130 ++++++++++-------- .../Storage/Internal/CosmosDatabaseWrapper.cs | 60 ++------ .../TestUtilities/CosmosTestStore.cs | 33 ++++- 3 files changed, 109 insertions(+), 114 deletions(-) diff --git a/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs b/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs index a173b8bb9fd..f3d61f89d7b 100644 --- a/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs +++ b/src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs @@ -14,7 +14,6 @@ using Microsoft.Azure.Cosmos; using Microsoft.EntityFrameworkCore.Cosmos.Diagnostics.Internal; using Microsoft.EntityFrameworkCore.Cosmos.Infrastructure.Internal; -using Microsoft.EntityFrameworkCore.Cosmos.Internal; using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Storage; @@ -253,15 +252,13 @@ private async Task CreateContainerIfNotExistsOnceAsync( public virtual bool CreateItem( [NotNull] string containerId, [NotNull] JToken document, - [NotNull] IUpdateEntry updateEntry, - CosmosConcurrencyToken concurrencyToken, - [CanBeNull] string partitionKey) + [NotNull] IUpdateEntry entry) => _executionStrategyFactory.Create().Execute( - (containerId, document, updateEntry, concurrencyToken, partitionKey), CreateItemOnce, null); + (containerId, document, entry), CreateItemOnce, null); private bool CreateItemOnce( DbContext context, - (string ContainerId, JToken Document, IUpdateEntry UpdateEntry, CosmosConcurrencyToken ConcurrencyToken, string PartitionKey) parameters) + (string ContainerId, JToken Document, IUpdateEntry Entry) parameters) => CreateItemOnceAsync(context, parameters).GetAwaiter().GetResult(); /// @@ -274,15 +271,13 @@ public virtual Task CreateItemAsync( [NotNull] string containerId, [NotNull] JToken document, [NotNull] IUpdateEntry updateEntry, - CosmosConcurrencyToken concurrencyToken, - [CanBeNull] string partitionKey, CancellationToken cancellationToken = default) => _executionStrategyFactory.Create().ExecuteAsync( - (containerId, document, updateEntry, concurrencyToken, partitionKey), CreateItemOnceAsync, null, cancellationToken); + (containerId, document, updateEntry), CreateItemOnceAsync, null, cancellationToken); private async Task CreateItemOnceAsync( DbContext _, - (string ContainerId, JToken Document, IUpdateEntry UpdateEntry, CosmosConcurrencyToken ConcurrencyToken, string PartitionKey) parameters, + (string ContainerId, JToken Document, IUpdateEntry Entry) parameters, CancellationToken cancellationToken = default) { await using var stream = new MemoryStream(); @@ -291,18 +286,13 @@ private async Task CreateItemOnceAsync( JsonSerializer.Create().Serialize(jsonWriter, parameters.Document); await jsonWriter.FlushAsync(cancellationToken); + var entry = parameters.Entry; var container = Client.GetDatabase(_databaseId).GetContainer(parameters.ContainerId); - var itemRequestOptions = CreateItemRequestOptions(parameters.ConcurrencyToken); - var partitionKey = CreatePartitionKey(parameters.PartitionKey); + var itemRequestOptions = CreateItemRequestOptions(entry); + var partitionKey = CreatePartitionKey(entry); using var response = await container.CreateItemStreamAsync(stream, partitionKey, itemRequestOptions, cancellationToken); - response.EnsureSuccessStatusCode(); - - if (parameters.ConcurrencyToken.Mode != CosmosConcurrencyMode.None) - { - var updateEntry = parameters.UpdateEntry; - updateEntry.SetStoreGeneratedValue(updateEntry.EntityType.GetETagProperty(), response.Headers.ETag); - } + ProcessResponse(response, entry); return response.StatusCode == HttpStatusCode.Created; } @@ -317,17 +307,15 @@ public virtual bool ReplaceItem( [NotNull] string collectionId, [NotNull] string documentId, [NotNull] JObject document, - [NotNull] IUpdateEntry updateEntry, - CosmosConcurrencyToken concurrencyToken, - [CanBeNull] string partitionKey) + [NotNull] IUpdateEntry entry) => _executionStrategyFactory.Create().Execute( - (collectionId, documentId, document, updateEntry, concurrencyToken, partitionKey), + (collectionId, documentId, document, entry), ReplaceItemOnce, null); private bool ReplaceItemOnce( DbContext context, - (string ContainerId, string ItemId, JObject Document, IUpdateEntry UpdateEntry, CosmosConcurrencyToken concurrencyToken, string PartitionKey) parameters) + (string ContainerId, string ItemId, JObject Document, IUpdateEntry Entry) parameters) => ReplaceItemOnceAsync(context, parameters).GetAwaiter().GetResult(); /// @@ -341,18 +329,16 @@ public virtual Task ReplaceItemAsync( [NotNull] string documentId, [NotNull] JObject document, [NotNull] IUpdateEntry updateEntry, - CosmosConcurrencyToken concurrencyToken, - [CanBeNull] string partitionKey, CancellationToken cancellationToken = default) => _executionStrategyFactory.Create().ExecuteAsync( - (collectionId, documentId, document, updateEntry, concurrencyToken, partitionKey), + (collectionId, documentId, document, updateEntry), ReplaceItemOnceAsync, null, cancellationToken); private async Task ReplaceItemOnceAsync( DbContext _, - (string ContainerId, string ItemId, JObject Document, IUpdateEntry UpdateEntry, CosmosConcurrencyToken ConcurrencyToken, string PartitionKey) parameters, + (string ContainerId, string ItemId, JObject Document, IUpdateEntry Entry) parameters, CancellationToken cancellationToken = default) { using var stream = new MemoryStream(); @@ -361,19 +347,14 @@ private async Task ReplaceItemOnceAsync( JsonSerializer.Create().Serialize(jsonWriter, parameters.Document); await jsonWriter.FlushAsync(cancellationToken); + var entry = parameters.Entry; var container = Client.GetDatabase(_databaseId).GetContainer(parameters.ContainerId); - var itemRequestOptions = CreateItemRequestOptions(parameters.ConcurrencyToken); - var partitionKey = CreatePartitionKey(parameters.PartitionKey); + var itemRequestOptions = CreateItemRequestOptions(entry); + var partitionKey = CreatePartitionKey(entry); using var response = await container.ReplaceItemStreamAsync( stream, parameters.ItemId, partitionKey, itemRequestOptions, cancellationToken); - response.EnsureSuccessStatusCode(); - - if (parameters.ConcurrencyToken.Mode != CosmosConcurrencyMode.None) - { - var updateEntry = parameters.UpdateEntry; - updateEntry.SetStoreGeneratedValue(updateEntry.EntityType.GetETagProperty(), response.Headers.ETag); - } + ProcessResponse(response, entry); return response.StatusCode == HttpStatusCode.OK; } @@ -387,10 +368,9 @@ private async Task ReplaceItemOnceAsync( public virtual bool DeleteItem( [NotNull] string containerId, [NotNull] string documentId, - CosmosConcurrencyToken concurrencyToken, - [CanBeNull] string partitionKey) + [NotNull] IUpdateEntry entry) => _executionStrategyFactory.Create().Execute( - (containerId, documentId, concurrencyToken, partitionKey), DeleteItemOnce, null); + (containerId, documentId, entry), DeleteItemOnce, null); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -400,7 +380,7 @@ public virtual bool DeleteItem( /// public virtual bool DeleteItemOnce( [NotNull] DbContext context, - (string ContainerId, string DocumentId, CosmosConcurrencyToken concurrencyToken, string PartitionKey) parameters) + (string ContainerId, string DocumentId, IUpdateEntry Entry) parameters) => DeleteItemOnceAsync(context, parameters).GetAwaiter().GetResult(); /// @@ -412,11 +392,10 @@ public virtual bool DeleteItemOnce( public virtual Task DeleteItemAsync( [NotNull] string containerId, [NotNull] string documentId, - CosmosConcurrencyToken concurrencyToken, - [CanBeNull] string partitionKey, + [NotNull] IUpdateEntry entry, CancellationToken cancellationToken = default) => _executionStrategyFactory.Create().ExecuteAsync( - (containerId, documentId, concurrencyToken, partitionKey), DeleteItemOnceAsync, null, cancellationToken); + (containerId, documentId, entry), DeleteItemOnceAsync, null, cancellationToken); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -426,33 +405,66 @@ public virtual Task DeleteItemAsync( /// public virtual async Task DeleteItemOnceAsync( [CanBeNull] DbContext _, - (string ContainerId, string DocumentId, CosmosConcurrencyToken ConcurrencyToken, string PartitionKey) parameters, + (string ContainerId, string DocumentId, IUpdateEntry Entry) parameters, CancellationToken cancellationToken = default) { + var entry = parameters.Entry; var items = Client.GetDatabase(_databaseId).GetContainer(parameters.ContainerId); - var itemRequestOptions = CreateItemRequestOptions(parameters.ConcurrencyToken); - var partitionKey = CreatePartitionKey(parameters.PartitionKey); + var itemRequestOptions = CreateItemRequestOptions(entry); + var partitionKey = CreatePartitionKey(entry); using var response = await items.DeleteItemStreamAsync( parameters.DocumentId, partitionKey, itemRequestOptions, cancellationToken: cancellationToken); - response.EnsureSuccessStatusCode(); + ProcessResponse(response, entry); + return response.StatusCode == HttpStatusCode.NoContent; } - private PartitionKey CreatePartitionKey(string partitionKey) - => partitionKey == null - ? PartitionKey.None - : new PartitionKey(partitionKey); + private static ItemRequestOptions CreateItemRequestOptions(IUpdateEntry entry) + { + var etagProperty = entry.EntityType.GetETagProperty(); + if (etagProperty == null) + { + return null; + } - private ItemRequestOptions CreateItemRequestOptions(CosmosConcurrencyToken concurrencyToken) + var etag = entry.GetCurrentValue(etagProperty); + var converter = etagProperty.GetTypeMapping().Converter; + if (converter != null) + { + etag = converter.ConvertToProvider(etag); + } + + return new ItemRequestOptions { IfMatchEtag = (string)etag }; + } + + private static PartitionKey CreatePartitionKey(IUpdateEntry entry) { - return concurrencyToken.Mode switch + object partitionKey = null; + var partitionKeyPropertyName = entry.EntityType.GetPartitionKeyPropertyName(); + if (partitionKeyPropertyName != null) { - CosmosConcurrencyMode.None => null, // null to keep it consistent with previous behavior - CosmosConcurrencyMode.IfMatch => new ItemRequestOptions { IfMatchEtag = concurrencyToken.Value }, - CosmosConcurrencyMode.IfNoneMatch => new ItemRequestOptions { IfNoneMatchEtag = concurrencyToken.Value }, - _ => throw new InvalidOperationException(), - }; + var partitionKeyProperty = entry.EntityType.FindProperty(partitionKeyPropertyName); + partitionKey = entry.GetCurrentValue(partitionKeyProperty); + + var converter = partitionKeyProperty.GetTypeMapping().Converter; + if (converter != null) + { + partitionKey = converter.ConvertToProvider(partitionKey); + } + } + + return partitionKey == null ? PartitionKey.None : new PartitionKey((string)partitionKey); + } + + private static void ProcessResponse(ResponseMessage response, IUpdateEntry entry) + { + response.EnsureSuccessStatusCode(); + var etagProperty = entry.EntityType.GetETagProperty(); + if (etagProperty != null && entry.EntityState != EntityState.Deleted) + { + entry.SetStoreGeneratedValue(etagProperty, response.Headers.ETag); + } } /// diff --git a/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs b/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs index e8be7a4f8bc..bde693d463b 100644 --- a/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs +++ b/src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs @@ -210,9 +210,8 @@ private bool Save(IUpdateEntry entry) { case EntityState.Added: var newDocument = documentSource.CreateDocument(entry); + return _cosmosClient.CreateItem(collectionId, newDocument, entry); - return _cosmosClient.CreateItem( - collectionId, newDocument, entry, GetConcurrencyToken(entry), GetPartitionKey(entry)); case EntityState.Modified: var document = documentSource.GetCurrentDocument(entry); if (document != null) @@ -235,16 +234,11 @@ private bool Save(IUpdateEntry entry) } return _cosmosClient.ReplaceItem( - collectionId, - documentSource.GetId(entry.SharedIdentityEntry ?? entry), - document, - entry, - GetConcurrencyToken(entry), - GetPartitionKey(entry)); + collectionId, documentSource.GetId(entry.SharedIdentityEntry ?? entry), document, entry); case EntityState.Deleted: - return _cosmosClient.DeleteItem( - collectionId, documentSource.GetId(entry), GetConcurrencyToken(entry), GetPartitionKey(entry)); + return _cosmosClient.DeleteItem(collectionId, documentSource.GetId(entry), entry); + default: return false; } @@ -275,7 +269,8 @@ private Task SaveAsync(IUpdateEntry entry, CancellationToken cancellationT case EntityState.Added: var newDocument = documentSource.CreateDocument(entry); return _cosmosClient.CreateItemAsync( - collectionId, newDocument, entry, GetConcurrencyToken(entry), GetPartitionKey(entry), cancellationToken); + collectionId, newDocument, entry, cancellationToken); + case EntityState.Modified: var document = documentSource.GetCurrentDocument(entry); if (document != null) @@ -302,14 +297,12 @@ private Task SaveAsync(IUpdateEntry entry, CancellationToken cancellationT documentSource.GetId(entry.SharedIdentityEntry ?? entry), document, entry, - GetConcurrencyToken(entry), - GetPartitionKey(entry), cancellationToken); case EntityState.Deleted: return _cosmosClient.DeleteItemAsync( - collectionId, documentSource.GetId(entry), - GetConcurrencyToken(entry), GetPartitionKey(entry), cancellationToken); + collectionId, documentSource.GetId(entry), entry, cancellationToken); + default: return Task.FromResult(false); } @@ -369,43 +362,6 @@ private Exception ThrowUpdateException(CosmosException exception, IUpdateEntry e }; } - private static CosmosConcurrencyToken GetConcurrencyToken(IUpdateEntry entry) - { - var etagProperty = entry.EntityType.GetETagProperty(); - if (etagProperty == null) - { - return CosmosConcurrencyToken.None; - } - - var etag = entry.GetCurrentValue(etagProperty); - var converter = etagProperty.GetTypeMapping().Converter; - if (converter != null) - { - etag = converter.ConvertToProvider(etag); - } - - return CosmosConcurrencyToken.IfMatch((string)etag); - } - - private static string GetPartitionKey(IUpdateEntry entry) - { - object partitionKey = null; - var partitionKeyPropertyName = entry.EntityType.GetPartitionKeyPropertyName(); - if (partitionKeyPropertyName != null) - { - var partitionKeyProperty = entry.EntityType.FindProperty(partitionKeyPropertyName); - partitionKey = entry.GetCurrentValue(partitionKeyProperty); - - var converter = partitionKeyProperty.GetTypeMapping().Converter; - if (converter != null) - { - partitionKey = converter.ConvertToProvider(partitionKey); - } - } - - return (string)partitionKey; - } - private static Exception Rethrow(Exception ex) { // Re-throw an exception, preserving the original stack and details, without being in the original "catch" block. diff --git a/test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosTestStore.cs b/test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosTestStore.cs index 6779eb1c681..dcf02ca0487 100644 --- a/test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosTestStore.cs +++ b/test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosTestStore.cs @@ -133,7 +133,7 @@ private async Task CreateFromFile(DbContext context) document["Discriminator"] = entityName; await cosmosClient.CreateItemAsync( - "NorthwindContext", document, new FakeUpdateEntry(), CosmosConcurrencyToken.None, null); + "NorthwindContext", document, new FakeUpdateEntry()); } else if (reader.TokenType == JsonToken.EndObject) { @@ -250,8 +250,8 @@ protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) private class FakeUpdateEntry : IUpdateEntry { - public IEntityType EntityType => throw new NotImplementedException(); - public EntityState EntityState { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } + public IEntityType EntityType => new FakeEntityType(); + public EntityState EntityState { get => EntityState.Added; set => throw new NotImplementedException(); } public IUpdateEntry SharedIdentityEntry => throw new NotImplementedException(); public object GetCurrentValue(IPropertyBase propertyBase) => throw new NotImplementedException(); public TProperty GetCurrentValue(IPropertyBase propertyBase) => throw new NotImplementedException(); @@ -265,5 +265,32 @@ private class FakeUpdateEntry : IUpdateEntry public void SetStoreGeneratedValue(IProperty property, object value) => throw new NotImplementedException(); public EntityEntry ToEntityEntry() => throw new NotImplementedException(); } + + public class FakeEntityType : IEntityType + { + public object this[string name] => null; + public IEntityType BaseType => throw new NotImplementedException(); + public string DefiningNavigationName => throw new NotImplementedException(); + public IEntityType DefiningEntityType => throw new NotImplementedException(); + public IModel Model => throw new NotImplementedException(); + public string Name => throw new NotImplementedException(); + public Type ClrType => throw new NotImplementedException(); + public bool IsSharedType => throw new NotImplementedException(); + public IAnnotation FindAnnotation(string name) => throw new NotImplementedException(); + public IForeignKey FindForeignKey(IReadOnlyList properties, IKey principalKey, IEntityType principalEntityType) => throw new NotImplementedException(); + public IIndex FindIndex(IReadOnlyList properties) => throw new NotImplementedException(); + public IKey FindKey(IReadOnlyList properties) => throw new NotImplementedException(); + public IKey FindPrimaryKey() => throw new NotImplementedException(); + public IProperty FindProperty(string name) => throw new NotImplementedException(); + public IServiceProperty FindServiceProperty(string name) => throw new NotImplementedException(); + public ISkipNavigation FindSkipNavigation(string name) => throw new NotImplementedException(); + public IEnumerable GetAnnotations() => throw new NotImplementedException(); + public IEnumerable GetForeignKeys() => throw new NotImplementedException(); + public IEnumerable GetIndexes() => throw new NotImplementedException(); + public IEnumerable GetKeys() => throw new NotImplementedException(); + public IEnumerable GetProperties() => throw new NotImplementedException(); + public IEnumerable GetServiceProperties() => throw new NotImplementedException(); + public IEnumerable GetSkipNavigations() => throw new NotImplementedException(); + } } }