Skip to content

Commit

Permalink
Add checks for concurrency tokens in Update and Delete methods of InM…
Browse files Browse the repository at this point in the history
…emoryTable. Fixes dotnet#10157 (dotnet#10158)
  • Loading branch information
shaynevanasperen authored and ajcvickers committed Nov 1, 2017
1 parent 1030467 commit e70d7da
Show file tree
Hide file tree
Showing 14 changed files with 228 additions and 13 deletions.
16 changes: 16 additions & 0 deletions src/EFCore.InMemory/Properties/InMemoryStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions src/EFCore.InMemory/Properties/InMemoryStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -128,4 +128,10 @@
<data name="UpdateConcurrencyException" xml:space="preserve">
<value>Attempted to update or delete an entity that does not exist in the store.</value>
</data>
<data name="UpdateConcurrencyTokenException" xml:space="preserve">
<value>Conflicts were detected for instance of entity type '{entityType}' on the concurrency token properties {properties}. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the conflicting values.</value>
</data>
<data name="UpdateConcurrencyTokenExceptionSensitive" xml:space="preserve">
<value>Conflicts were detected for instance of entity type '{entityType}' with the key value '{keyValue}' on the concurrency token property values {conflictingValues}, with corresponding database values {databaseValues}.</value>
</data>
</root>
64 changes: 63 additions & 1 deletion src/EFCore.InMemory/Storage/Internal/InMemoryTable.cs
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Linq;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
using Microsoft.EntityFrameworkCore.Internal;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
using Microsoft.EntityFrameworkCore.Update;
using Microsoft.EntityFrameworkCore.Update.Internal;
using Microsoft.EntityFrameworkCore.Utilities;

namespace Microsoft.EntityFrameworkCore.Storage.Internal
{
Expand All @@ -17,15 +22,17 @@ namespace Microsoft.EntityFrameworkCore.Storage.Internal
public class InMemoryTable<TKey> : IInMemoryTable
{
private readonly IPrincipalKeyValueFactory<TKey> _keyValueFactory;
private readonly bool _sensitiveLoggingEnabled;
private readonly Dictionary<TKey, object[]> _rows;

/// <summary>
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
public InMemoryTable([NotNull] IPrincipalKeyValueFactory<TKey> keyValueFactory)
public InMemoryTable([NotNull] IPrincipalKeyValueFactory<TKey> keyValueFactory, bool sensitiveLoggingEnabled)
{
_keyValueFactory = keyValueFactory;
_sensitiveLoggingEnabled = sensitiveLoggingEnabled;
_rows = new Dictionary<TKey, object[]>(keyValueFactory.EqualityComparer);
}

Expand Down Expand Up @@ -53,6 +60,22 @@ public virtual void Delete(IUpdateEntry entry)

if (_rows.ContainsKey(key))
{
var properties = entry.EntityType.GetProperties().ToList();
var concurrencyConflicts = new Dictionary<IProperty, object>();

for (var index = 0; index < properties.Count; index++)
{
if (properties[index].IsConcurrencyToken && !Equals(_rows[key][index], entry.GetOriginalValue(properties[index])))
{
concurrencyConflicts.Add(properties[index], _rows[key][index]);
}
}

if (concurrencyConflicts.Any())
{
ThrowUpdateConcurrencyException(entry, concurrencyConflicts);
}

_rows.Remove(key);
}
else
Expand All @@ -73,14 +96,25 @@ public virtual void Update(IUpdateEntry entry)
{
var properties = entry.EntityType.GetProperties().ToList();
var valueBuffer = new object[properties.Count];
var concurrencyConflicts = new Dictionary<IProperty, object>();

for (var index = 0; index < valueBuffer.Length; index++)
{
if (properties[index].IsConcurrencyToken && !Equals(_rows[key][index], entry.GetOriginalValue(properties[index])))
{
concurrencyConflicts.Add(properties[index], _rows[key][index]);
continue;
}
valueBuffer[index] = entry.IsModified(properties[index])
? entry.GetCurrentValue(properties[index])
: _rows[key][index];
}

if (concurrencyConflicts.Any())
{
ThrowUpdateConcurrencyException(entry, concurrencyConflicts);
}

_rows[key] = valueBuffer;
}
else
Expand All @@ -94,5 +128,33 @@ private TKey CreateKey(IUpdateEntry entry)

private static object[] CreateValueBuffer(IUpdateEntry entry)
=> entry.EntityType.GetProperties().Select(entry.GetCurrentValue).ToArray();

/// <summary>
/// Throws an exception indicating that concurrency conflicts were detected.
/// </summary>
/// <param name="entry"> The update entry which resulted in the conflict(s). </param>
/// <param name="concurrencyConflicts"> The conflicting properties with their associated database values. </param>
protected virtual void ThrowUpdateConcurrencyException([NotNull] IUpdateEntry entry, [NotNull] Dictionary<IProperty, object> concurrencyConflicts)
{
Check.NotNull(entry, nameof(entry));
Check.NotNull(concurrencyConflicts, nameof(concurrencyConflicts));

if (_sensitiveLoggingEnabled)
{
throw new DbUpdateConcurrencyException(
InMemoryStrings.UpdateConcurrencyTokenExceptionSensitive(
entry.EntityType.DisplayName(),
entry.BuildCurrentValuesString(entry.EntityType.FindPrimaryKey().Properties),
entry.BuildOriginalValuesString(concurrencyConflicts.Keys),
string.Join(", ", concurrencyConflicts.Select(c => c.Key.Name + ":" + c.Value))),
new[] { entry });
}

throw new DbUpdateConcurrencyException(
InMemoryStrings.UpdateConcurrencyTokenException(
entry.EntityType.DisplayName(),
Property.Format(concurrencyConflicts.Keys)),
new[] { entry });
}
}
}
21 changes: 18 additions & 3 deletions src/EFCore.InMemory/Storage/Internal/InMemoryTableFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
using System.Reflection;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
using Microsoft.EntityFrameworkCore.Utilities;

namespace Microsoft.EntityFrameworkCore.Storage.Internal
{
Expand All @@ -17,9 +19,22 @@ namespace Microsoft.EntityFrameworkCore.Storage.Internal
/// </summary>
public class InMemoryTableFactory : IdentityMapFactoryFactoryBase, IInMemoryTableFactory
{
private readonly bool _sensitiveLoggingEnabled = false;

private readonly ConcurrentDictionary<IKey, Func<IInMemoryTable>> _factories
= new ConcurrentDictionary<IKey, Func<IInMemoryTable>>();

/// <summary>
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
public InMemoryTableFactory([NotNull] ILoggingOptions loggingOptions)
{
Check.NotNull(loggingOptions, nameof(loggingOptions));

_sensitiveLoggingEnabled = loggingOptions.IsSensitiveDataLoggingEnabled;
}

/// <summary>
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
/// directly from your code. This API may change or be removed in future releases.
Expand All @@ -31,10 +46,10 @@ private Func<IInMemoryTable> Create([NotNull] IKey key)
=> (Func<IInMemoryTable>)typeof(InMemoryTableFactory).GetTypeInfo()
.GetDeclaredMethod(nameof(CreateFactory))
.MakeGenericMethod(GetKeyType(key))
.Invoke(null, new object[] { key });
.Invoke(null, new object[] { key, _sensitiveLoggingEnabled });

[UsedImplicitly]
private static Func<IInMemoryTable> CreateFactory<TKey>(IKey key)
=> () => new InMemoryTable<TKey>(key.GetPrincipalKeyValueFactory<TKey>());
private static Func<IInMemoryTable> CreateFactory<TKey>(IKey key, bool sensitiveLoggingEnabled)
=> () => new InMemoryTable<TKey>(key.GetPrincipalKeyValueFactory<TKey>(), sensitiveLoggingEnabled);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,8 @@ protected override void UseTransaction(DatabaseFacade facade, IDbContextTransact

protected override string UpdateConcurrencyMessage
=> RelationalStrings.UpdateConcurrencyException(1, 0);

protected override string UpdateConcurrencyTokenMessage
=> RelationalStrings.UpdateConcurrencyException(1, 0);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.ComponentModel.DataAnnotations;

namespace Microsoft.EntityFrameworkCore.TestModels.UpdatesModel
{
Expand All @@ -10,6 +11,7 @@ public class Product
public Guid Id { get; set; }
public int? DependentId { get; set; }
public string Name { get; set; }
[ConcurrencyCheck]
public decimal Price { get; set; }
}
}
62 changes: 56 additions & 6 deletions src/EFCore.Specification.Tests/UpdatesTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,10 @@ public virtual void Save_partial_update()
new Product
{
Id = productId,
Price = 1.99M
Price = 1.49M
});

entry.Property(c => c.Price).CurrentValue = 1.99M;
entry.Property(p => p.Price).IsModified = true;

Assert.False(entry.Property(p => p.DependentId).IsModified);
Expand Down Expand Up @@ -73,6 +74,31 @@ public virtual void Save_partial_update_on_missing_record_throws()
});
}

[Fact]
public virtual void Save_partial_update_on_concurrency_token_original_value_mismatch_throws()
{
var productId = new Guid("984ade3c-2f7b-4651-a351-642e92ab7146");

ExecuteWithStrategyInTransaction(
context =>
{
var entry = context.Products.Attach(
new Product
{
Id = productId,
Name = "Apple Fritter",
Price = 3.49M // Not the same as the value stored in the database
});

entry.Property(c => c.Name).IsModified = true;

Assert.Equal(
UpdateConcurrencyTokenMessage,
Assert.Throws<DbUpdateConcurrencyException>(
() => context.SaveChanges()).Message);
});
}

[Fact]
public virtual void Can_remove_partial()
{
Expand All @@ -84,7 +110,8 @@ public virtual void Can_remove_partial()
context.Products.Remove(
new Product
{
Id = productId
Id = productId,
Price = 1.49M
});

context.SaveChanges();
Expand Down Expand Up @@ -116,6 +143,28 @@ public virtual void Remove_partial_on_missing_record_throws()
});
}

[Fact]
public virtual void Remove_partial_on_concurrency_token_original_value_mismatch_throws()
{
var productId = new Guid("984ade3c-2f7b-4651-a351-642e92ab7146");

ExecuteWithStrategyInTransaction(
context =>
{
context.Products.Remove(
new Product
{
Id = productId,
Price = 3.49M // Not the same as the value stored in the database
});

Assert.Equal(
UpdateConcurrencyTokenMessage,
Assert.Throws<DbUpdateConcurrencyException>(
() => context.SaveChanges()).Message);
});
}

[Fact]
public virtual void Save_replaced_principal()
{
Expand Down Expand Up @@ -157,7 +206,7 @@ public virtual void SaveChanges_processes_all_tracked_entities()
var entry1 = stateManager.GetOrCreateEntry(new Category { Id = 77, PrincipalId = 777 });
var entry2 = stateManager.GetOrCreateEntry(new Category { Id = 78, PrincipalId = 778 });
var entry3 = stateManager.GetOrCreateEntry(new Product { Id = productId1 });
var entry4 = stateManager.GetOrCreateEntry(new Product { Id = productId2 });
var entry4 = stateManager.GetOrCreateEntry(new Product { Id = productId2, Price = 2.49M });

entry1.SetEntityState(EntityState.Added);
entry2.SetEntityState(EntityState.Modified);
Expand Down Expand Up @@ -192,7 +241,7 @@ public virtual void SaveChanges_false_processes_all_tracked_entities_without_cal
var entry1 = stateManager.GetOrCreateEntry(new Category { Id = 77, PrincipalId = 777 });
var entry2 = stateManager.GetOrCreateEntry(new Category { Id = 78, PrincipalId = 778 });
var entry3 = stateManager.GetOrCreateEntry(new Product { Id = productId1 });
var entry4 = stateManager.GetOrCreateEntry(new Product { Id = productId2 });
var entry4 = stateManager.GetOrCreateEntry(new Product { Id = productId2, Price = 2.49M });

entry1.SetEntityState(EntityState.Added);
entry2.SetEntityState(EntityState.Modified);
Expand Down Expand Up @@ -229,7 +278,7 @@ public Task SaveChangesAsync_processes_all_tracked_entities()
var entry1 = stateManager.GetOrCreateEntry(new Category { Id = 77, PrincipalId = 777 });
var entry2 = stateManager.GetOrCreateEntry(new Category { Id = 78, PrincipalId = 778 });
var entry3 = stateManager.GetOrCreateEntry(new Product { Id = productId1 });
var entry4 = stateManager.GetOrCreateEntry(new Product { Id = productId2 });
var entry4 = stateManager.GetOrCreateEntry(new Product { Id = productId2, Price = 2.49M });

entry1.SetEntityState(EntityState.Added);
entry2.SetEntityState(EntityState.Modified);
Expand Down Expand Up @@ -264,7 +313,7 @@ public Task SaveChangesAsync_false_processes_all_tracked_entities_without_callin
var entry1 = stateManager.GetOrCreateEntry(new Category { Id = 77, PrincipalId = 777 });
var entry2 = stateManager.GetOrCreateEntry(new Category { Id = 78, PrincipalId = 778 });
var entry3 = stateManager.GetOrCreateEntry(new Product { Id = productId1 });
var entry4 = stateManager.GetOrCreateEntry(new Product { Id = productId2 });
var entry4 = stateManager.GetOrCreateEntry(new Product { Id = productId2, Price = 2.49M });

entry1.SetEntityState(EntityState.Added);
entry2.SetEntityState(EntityState.Modified);
Expand All @@ -288,6 +337,7 @@ public Task SaveChangesAsync_false_processes_all_tracked_entities_without_callin
}

protected abstract string UpdateConcurrencyMessage { get; }
protected abstract string UpdateConcurrencyTokenMessage { get; }

protected virtual void ExecuteWithStrategyInTransaction(
Action<UpdatesContext> testOperation,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

namespace Microsoft.EntityFrameworkCore
{
public class UpdatesInMemoryFixture : UpdatesFixtureBase
public abstract class UpdatesInMemoryFixtureBase : UpdatesFixtureBase
{
protected override ITestStoreFactory TestStoreFactory => InMemoryTestStoreFactory.Instance;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@

namespace Microsoft.EntityFrameworkCore
{
public class UpdatesInMemoryTest : UpdatesTestBase<UpdatesInMemoryFixture>
public abstract class UpdatesInMemoryTestBase<TFixture> : UpdatesTestBase<TFixture>
where TFixture : UpdatesInMemoryFixtureBase
{
public UpdatesInMemoryTest(UpdatesInMemoryFixture fixture)
protected UpdatesInMemoryTestBase(TFixture fixture)
: base(fixture)
{
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
using Microsoft.EntityFrameworkCore.TestUtilities;

namespace Microsoft.EntityFrameworkCore
{
public class UpdatesInMemoryWithSensitiveDataLoggingFixture : UpdatesInMemoryFixtureBase
{
protected override ITestStoreFactory TestStoreFactory => InMemoryTestStoreFactory.Instance;

public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
=> base.AddOptions(builder).EnableSensitiveDataLogging();
}
}
Loading

0 comments on commit e70d7da

Please sign in to comment.