Skip to content

Commit eacfc83

Browse files
author
Bart Koelman
committed
Explores support for concurrency tokens using PostgreSQL
*Update (2021-11-08): Rebased on latest changes in master branch* Because we fetch the row before update and apply changes on that, a concurrency violation is only reported when two concurrent requests update the same row in parallel. Instead, we want to produce an error if the token sent by the user does not match the stored token. To do that, we need to convince EF Core to use that as original version. That's not too hard. Now the problem is that there is no way to send the token for relationships or deleting a resource. Skipped tests have been added to demonstrate this. We could fetch such related rows upfront to work around that, but that kinda defeats the purpose of using concurrency tokens in the first place. It may be more correct to fail when a user is trying to add a related resource that has changed since it was fetched. This reasoning may be a bit too puristic and impractical, but at least that's how EF Core seems to handle it. Solutions considerations: - Add 'version' to resource identifier object, so the client can send it. The spec does not explicitly forbid adding custom fields, however 'meta' would probably be the recommended approach. Instead of extending the definition, we could encode it in the StringId. - Once we have access to that token value, we need to somehow map that to 'the' resource property. What if there are multiple concurrency token properties on a resource? And depending on the database used, this could be typed as numeric, guid, timestamp, binary or something else. - Given that PostgreSQL uses a number (uint xmin), should we obfuscate or even encrypt that? If the latter, we need to add an option for api developers to set the encryption key. See also: json-api/json-api#600 json-api/json-api#824
1 parent 15f5923 commit eacfc83

File tree

10 files changed

+857
-7
lines changed

10 files changed

+857
-7
lines changed

JsonApiDotNetCore.sln.DotSettings

+1
Original file line numberDiff line numberDiff line change
@@ -637,5 +637,6 @@ $left$ = $right$;</s:String>
637637
<s:Boolean x:Key="/Default/UserDictionary/Words/=subdirectory/@EntryIndexedValue">True</s:Boolean>
638638
<s:Boolean x:Key="/Default/UserDictionary/Words/=unarchive/@EntryIndexedValue">True</s:Boolean>
639639
<s:Boolean x:Key="/Default/UserDictionary/Words/=Workflows/@EntryIndexedValue">True</s:Boolean>
640+
<s:Boolean x:Key="/Default/UserDictionary/Words/=xmin/@EntryIndexedValue">True</s:Boolean>
640641
<s:Boolean x:Key="/Default/UserDictionary/Words/=xunit/@EntryIndexedValue">True</s:Boolean>
641642
</wpf:ResourceDictionary>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
using System;
2+
using System.Net;
3+
using JetBrains.Annotations;
4+
using JsonApiDotNetCore.Serialization.Objects;
5+
6+
namespace JsonApiDotNetCore.Errors
7+
{
8+
/// <summary>
9+
/// The error that is thrown when data has been modified on the server since the resource was retrieved.
10+
/// </summary>
11+
[PublicAPI]
12+
public sealed class DataConcurrencyException : JsonApiException
13+
{
14+
public DataConcurrencyException(Exception exception)
15+
: base(new ErrorObject(HttpStatusCode.Conflict)
16+
{
17+
Title = "The concurrency token is missing or does not match the server version. " +
18+
"This indicates that data has been modified since the resource was retrieved."
19+
}, exception)
20+
{
21+
}
22+
}
23+
}

src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs

+41-7
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System.Diagnostics.CodeAnalysis;
55
using System.Linq;
66
using System.Linq.Expressions;
7+
using System.Reflection;
78
using System.Threading;
89
using System.Threading.Tasks;
910
using JetBrains.Annotations;
@@ -207,7 +208,7 @@ public virtual async Task CreateAsync(TResource resourceFromRequest, TResource r
207208
DbSet<TResource> dbSet = _dbContext.Set<TResource>();
208209
await dbSet.AddAsync(resourceForDatabase, cancellationToken);
209210

210-
await SaveChangesAsync(cancellationToken);
211+
await SaveChangesAsync(cancellationToken, false);
211212

212213
await _resourceDefinitionAccessor.OnWriteSucceededAsync(resourceForDatabase, WriteOperationKind.CreateResource, cancellationToken);
213214

@@ -283,15 +284,43 @@ public virtual async Task UpdateAsync(TResource resourceFromRequest, TResource r
283284
attribute.SetValue(resourceFromDatabase, attribute.GetValue(resourceFromRequest));
284285
}
285286

287+
bool hasConcurrencyToken = RestoreConcurrencyToken(resourceFromRequest, resourceFromDatabase);
288+
286289
await _resourceDefinitionAccessor.OnWritingAsync(resourceFromDatabase, WriteOperationKind.UpdateResource, cancellationToken);
287290

288-
await SaveChangesAsync(cancellationToken);
291+
await SaveChangesAsync(cancellationToken, hasConcurrencyToken);
289292

290293
await _resourceDefinitionAccessor.OnWriteSucceededAsync(resourceFromDatabase, WriteOperationKind.UpdateResource, cancellationToken);
291294

292295
_dbContext.ResetChangeTracker();
293296
}
294297

298+
private bool RestoreConcurrencyToken(TResource resourceFromRequest, TResource resourceFromDatabase)
299+
{
300+
bool hasConcurrencyToken = false;
301+
302+
foreach (var propertyEntry in _dbContext.Entry(resourceFromDatabase).Properties)
303+
{
304+
if (propertyEntry.Metadata.IsConcurrencyToken)
305+
{
306+
// Overwrite the ConcurrencyToken coming from database with the one from the request body.
307+
// If they are different, EF Core throws a DbUpdateConcurrencyException on save.
308+
309+
PropertyInfo? concurrencyTokenProperty = typeof(TResource).GetProperty(propertyEntry.Metadata.PropertyInfo.Name);
310+
311+
if (concurrencyTokenProperty != null)
312+
{
313+
object? concurrencyTokenFromRequest = concurrencyTokenProperty.GetValue(resourceFromRequest);
314+
propertyEntry.OriginalValue = concurrencyTokenFromRequest;
315+
316+
hasConcurrencyToken = true;
317+
}
318+
}
319+
}
320+
321+
return hasConcurrencyToken;
322+
}
323+
295324
protected void AssertIsNotClearingRequiredToOneRelationship(RelationshipAttribute relationship, TResource leftResource, object? rightValue)
296325
{
297326
if (relationship is HasOneAttribute)
@@ -341,7 +370,7 @@ public virtual async Task DeleteAsync(TId id, CancellationToken cancellationToke
341370

342371
_dbContext.Remove(resourceTracked);
343372

344-
await SaveChangesAsync(cancellationToken);
373+
await SaveChangesAsync(cancellationToken, false);
345374

346375
await _resourceDefinitionAccessor.OnWriteSucceededAsync(resourceTracked, WriteOperationKind.DeleteResource, cancellationToken);
347376
}
@@ -412,7 +441,7 @@ public virtual async Task SetRelationshipAsync(TResource leftResource, object? r
412441

413442
await _resourceDefinitionAccessor.OnWritingAsync(leftResource, WriteOperationKind.SetRelationship, cancellationToken);
414443

415-
await SaveChangesAsync(cancellationToken);
444+
await SaveChangesAsync(cancellationToken, false);
416445

417446
await _resourceDefinitionAccessor.OnWriteSucceededAsync(leftResource, WriteOperationKind.SetRelationship, cancellationToken);
418447
}
@@ -445,7 +474,7 @@ public virtual async Task AddToToManyRelationshipAsync(TId leftId, ISet<IIdentif
445474

446475
await _resourceDefinitionAccessor.OnWritingAsync(leftResourceTracked, WriteOperationKind.AddToRelationship, cancellationToken);
447476

448-
await SaveChangesAsync(cancellationToken);
477+
await SaveChangesAsync(cancellationToken, false);
449478

450479
await _resourceDefinitionAccessor.OnWriteSucceededAsync(leftResourceTracked, WriteOperationKind.AddToRelationship, cancellationToken);
451480
}
@@ -508,7 +537,7 @@ public virtual async Task RemoveFromToManyRelationshipAsync(TResource leftResour
508537

509538
await _resourceDefinitionAccessor.OnWritingAsync(leftResourceTracked, WriteOperationKind.RemoveFromRelationship, cancellationToken);
510539

511-
await SaveChangesAsync(cancellationToken);
540+
await SaveChangesAsync(cancellationToken, false);
512541

513542
await _resourceDefinitionAccessor.OnWriteSucceededAsync(leftResourceTracked, WriteOperationKind.RemoveFromRelationship, cancellationToken);
514543
}
@@ -559,7 +588,7 @@ private bool RequireLoadOfInverseRelationship(RelationshipAttribute relationship
559588
return trackedValueToAssign != null && relationship is HasOneAttribute { IsOneToOne: true };
560589
}
561590

562-
protected virtual async Task SaveChangesAsync(CancellationToken cancellationToken)
591+
protected virtual async Task SaveChangesAsync(CancellationToken cancellationToken, bool hasConcurrencyToken)
563592
{
564593
cancellationToken.ThrowIfCancellationRequested();
565594

@@ -571,6 +600,11 @@ protected virtual async Task SaveChangesAsync(CancellationToken cancellationToke
571600
}
572601
catch (Exception exception) when (exception is DbUpdateException or InvalidOperationException)
573602
{
603+
if (hasConcurrencyToken && exception is DbUpdateConcurrencyException)
604+
{
605+
throw new DataConcurrencyException(exception);
606+
}
607+
574608
_dbContext.ResetChangeTracker();
575609

576610
throw new DataStoreUpdateException(exception);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
using JetBrains.Annotations;
2+
using Microsoft.EntityFrameworkCore;
3+
4+
// @formatter:wrap_chained_method_calls chop_always
5+
6+
namespace JsonApiDotNetCoreTests.IntegrationTests.ConcurrencyTokens
7+
{
8+
[UsedImplicitly(ImplicitUseTargetFlags.Members)]
9+
public sealed class ConcurrencyDbContext : DbContext
10+
{
11+
public DbSet<Disk> Disks => Set<Disk>();
12+
public DbSet<Partition> Partitions => Set<Partition>();
13+
14+
public ConcurrencyDbContext(DbContextOptions<ConcurrencyDbContext> options)
15+
: base(options)
16+
{
17+
}
18+
19+
protected override void OnModelCreating(ModelBuilder builder)
20+
{
21+
// https://www.npgsql.org/efcore/modeling/concurrency.html
22+
23+
builder.Entity<Disk>()
24+
.UseXminAsConcurrencyToken();
25+
26+
builder.Entity<Partition>()
27+
.UseXminAsConcurrencyToken();
28+
}
29+
}
30+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
using System;
2+
using Bogus;
3+
using JsonApiDotNetCore;
4+
using TestBuildingBlocks;
5+
6+
// @formatter:wrap_chained_method_calls chop_always
7+
// @formatter:keep_existing_linebreaks true
8+
9+
namespace JsonApiDotNetCoreTests.IntegrationTests.ConcurrencyTokens
10+
{
11+
internal sealed class ConcurrencyFakers : FakerContainer
12+
{
13+
private const ulong OneGigabyte = 1024 * 1024 * 1024;
14+
private static readonly string[] KnownFileSystems = ArrayFactory.Create("NTFS", "FAT32", "ext4", "XFS", "ZFS", "btrfs");
15+
16+
private readonly Lazy<Faker<Disk>> _lazyDiskFaker = new(() =>
17+
new Faker<Disk>().UseSeed(GetFakerSeed())
18+
.RuleFor(disk => disk.Manufacturer, faker => faker.Company.CompanyName())
19+
.RuleFor(disk => disk.SerialCode, faker => faker.System.ApplePushToken()));
20+
21+
private readonly Lazy<Faker<Partition>> _lazyPartitionFaker = new(() =>
22+
new Faker<Partition>().UseSeed(GetFakerSeed())
23+
.RuleFor(partition => partition.MountPoint, faker => faker.System.DirectoryPath())
24+
.RuleFor(partition => partition.FileSystem, faker => faker.PickRandom(KnownFileSystems))
25+
.RuleFor(partition => partition.CapacityInBytes, faker => faker.Random.ULong(OneGigabyte * 50, OneGigabyte * 100))
26+
.RuleFor(partition => partition.FreeSpaceInBytes, faker => faker.Random.ULong(OneGigabyte * 10, OneGigabyte * 40)));
27+
28+
public Faker<Disk> Disk => _lazyDiskFaker.Value;
29+
public Faker<Partition> Partition => _lazyPartitionFaker.Value;
30+
}
31+
}

0 commit comments

Comments
 (0)