Skip to content

Optimized RemoveFromToManyRelationship endpoint #1039

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 13, 2021
Merged

Optimized RemoveFromToManyRelationship endpoint #1039

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
104ca5e
Corrected terminology: The attributes from the incoming request, not …
Aug 11, 2021
1179d86
Fix failing tests when EF Core change tracker is purged more often (s…
Aug 11, 2021
cde3954
Optimized DELETE Relationship endpoints (remove from to-many relation…
Aug 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/JsonApiDotNetCore/CollectionExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,5 +70,16 @@ public static bool DictionaryEqual<TKey, TValue>(this IReadOnlyDictionary<TKey,

return true;
}

public static void AddRange<T>(this ICollection<T> source, IEnumerable<T> itemsToAdd)
{
ArgumentGuard.NotNull(source, nameof(source));
ArgumentGuard.NotNull(itemsToAdd, nameof(itemsToAdd));

foreach (T item in itemsToAdd)
{
source.Add(item);
}
}
}
}
11 changes: 8 additions & 3 deletions src/JsonApiDotNetCore/Configuration/JsonApiApplicationBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,15 +284,20 @@ private void AddResourcesFromDbContext(DbContext dbContext, ResourceGraphBuilder
{
foreach (IEntityType entityType in dbContext.Model.GetEntityTypes())
{
#pragma warning disable EF1001 // Internal EF Core API usage.
if (entityType is not EntityType { IsImplicitlyCreatedJoinEntityType: true })
#pragma warning restore EF1001 // Internal EF Core API usage.
if (!IsImplicitManyToManyJoinEntity(entityType))
{
builder.Add(entityType.ClrType);
}
}
}

private static bool IsImplicitManyToManyJoinEntity(IEntityType entityType)
{
#pragma warning disable EF1001 // Internal EF Core API usage.
return entityType is EntityType { IsImplicitlyCreatedJoinEntityType: true };
#pragma warning restore EF1001 // Internal EF Core API usage.
}

public void Dispose()
{
_intermediateProvider.Dispose();
Expand Down
112 changes: 71 additions & 41 deletions src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,16 +182,14 @@ public virtual async Task CreateAsync(TResource resourceFromRequest, TResource r

using IDisposable _ = CodeTimingSessionManager.Current.Measure("Repository - Create resource");

using var collector = new PlaceholderResourceCollector(_resourceFactory, _dbContext);

foreach (RelationshipAttribute relationship in _targetedFields.Relationships)
{
object rightValue = relationship.GetValue(resourceFromRequest);

object rightValueEvaluated = await VisitSetRelationshipAsync(resourceForDatabase, relationship, rightValue, WriteOperationKind.CreateResource,
cancellationToken);

await UpdateRelationshipAsync(relationship, resourceForDatabase, rightValueEvaluated, collector, cancellationToken);
await UpdateRelationshipAsync(relationship, resourceForDatabase, rightValueEvaluated, cancellationToken);
}

foreach (AttrAttribute attribute in _targetedFields.Attributes)
Expand All @@ -207,6 +205,8 @@ public virtual async Task CreateAsync(TResource resourceFromRequest, TResource r
await SaveChangesAsync(cancellationToken);

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

_dbContext.ResetChangeTracker();
}

private async Task<object> VisitSetRelationshipAsync(TResource leftResource, RelationshipAttribute relationship, object rightValue,
Expand Down Expand Up @@ -254,8 +254,6 @@ public virtual async Task UpdateAsync(TResource resourceFromRequest, TResource r

using IDisposable _ = CodeTimingSessionManager.Current.Measure("Repository - Update resource");

using var collector = new PlaceholderResourceCollector(_resourceFactory, _dbContext);

foreach (RelationshipAttribute relationship in _targetedFields.Relationships)
{
object rightValue = relationship.GetValue(resourceFromRequest);
Expand All @@ -265,7 +263,7 @@ public virtual async Task UpdateAsync(TResource resourceFromRequest, TResource r

AssertIsNotClearingRequiredRelationship(relationship, resourceFromDatabase, rightValueEvaluated);

await UpdateRelationshipAsync(relationship, resourceFromDatabase, rightValueEvaluated, collector, cancellationToken);
await UpdateRelationshipAsync(relationship, resourceFromDatabase, rightValueEvaluated, cancellationToken);
}

foreach (AttrAttribute attribute in _targetedFields.Attributes)
Expand All @@ -278,18 +276,21 @@ public virtual async Task UpdateAsync(TResource resourceFromRequest, TResource r
await SaveChangesAsync(cancellationToken);

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

_dbContext.ResetChangeTracker();
}

protected void AssertIsNotClearingRequiredRelationship(RelationshipAttribute relationship, TResource leftResource, object rightValue)
{
bool relationshipIsRequired = false;

if (relationship is not HasManyAttribute { IsManyToMany: true })
if (relationship is HasManyAttribute { IsManyToMany: true })
{
INavigation navigation = TryGetNavigation(relationship);
relationshipIsRequired = navigation?.ForeignKey?.IsRequired ?? false;
// Many-to-many relationships cannot be required.
return;
}

INavigation navigation = TryGetNavigation(relationship);
bool relationshipIsRequired = navigation?.ForeignKey?.IsRequired ?? false;

bool relationshipIsBeingCleared = relationship is HasManyAttribute hasManyRelationship
? IsToManyRelationshipBeingCleared(hasManyRelationship, leftResource, rightValue)
: rightValue == null;
Expand Down Expand Up @@ -326,31 +327,30 @@ public virtual async Task DeleteAsync(TId id, CancellationToken cancellationToke
using IDisposable _ = CodeTimingSessionManager.Current.Measure("Repository - Delete resource");

// This enables OnWritingAsync() to fetch the resource, which adds it to the change tracker.
// If so, we'll reuse the tracked resource instead of a placeholder resource.
var emptyResource = _resourceFactory.CreateInstance<TResource>();
emptyResource.Id = id;
// If so, we'll reuse the tracked resource instead of this placeholder resource.
var placeholderResource = _resourceFactory.CreateInstance<TResource>();
placeholderResource.Id = id;

await _resourceDefinitionAccessor.OnWritingAsync(emptyResource, WriteOperationKind.DeleteResource, cancellationToken);
await _resourceDefinitionAccessor.OnWritingAsync(placeholderResource, WriteOperationKind.DeleteResource, cancellationToken);

using var collector = new PlaceholderResourceCollector(_resourceFactory, _dbContext);
TResource resource = collector.CreateForId<TResource, TId>(id);
var resourceTracked = (TResource)_dbContext.GetTrackedOrAttach(placeholderResource);

foreach (RelationshipAttribute relationship in _resourceGraph.GetResourceContext<TResource>().Relationships)
{
// Loads the data of the relationship, if in EF Core it is configured in such a way that loading the related
// entities into memory is required for successfully executing the selected deletion behavior.
if (RequiresLoadOfRelationshipForDeletion(relationship))
{
NavigationEntry navigation = GetNavigationEntry(resource, relationship);
NavigationEntry navigation = GetNavigationEntry(resourceTracked, relationship);
await navigation.LoadAsync(cancellationToken);
}
}

_dbContext.Remove(resource);
_dbContext.Remove(resourceTracked);

await SaveChangesAsync(cancellationToken);

await _resourceDefinitionAccessor.OnWriteSucceededAsync(resource, WriteOperationKind.DeleteResource, cancellationToken);
await _resourceDefinitionAccessor.OnWriteSucceededAsync(resourceTracked, WriteOperationKind.DeleteResource, cancellationToken);
}

private NavigationEntry GetNavigationEntry(TResource resource, RelationshipAttribute relationship)
Expand Down Expand Up @@ -413,8 +413,7 @@ public virtual async Task SetRelationshipAsync(TResource leftResource, object ri

AssertIsNotClearingRequiredRelationship(relationship, leftResource, rightValueEvaluated);

using var collector = new PlaceholderResourceCollector(_resourceFactory, _dbContext);
await UpdateRelationshipAsync(relationship, leftResource, rightValueEvaluated, collector, cancellationToken);
await UpdateRelationshipAsync(relationship, leftResource, rightValueEvaluated, cancellationToken);

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

Expand Down Expand Up @@ -442,16 +441,18 @@ public virtual async Task AddToToManyRelationshipAsync(TId leftId, ISet<IIdentif

if (rightResourceIds.Any())
{
using var collector = new PlaceholderResourceCollector(_resourceFactory, _dbContext);
TResource leftResource = collector.CreateForId<TResource, TId>(leftId);
var leftPlaceholderResource = _resourceFactory.CreateInstance<TResource>();
leftPlaceholderResource.Id = leftId;

await UpdateRelationshipAsync(relationship, leftResource, rightResourceIds, collector, cancellationToken);
var leftResourceTracked = (TResource)_dbContext.GetTrackedOrAttach(leftPlaceholderResource);

await _resourceDefinitionAccessor.OnWritingAsync(leftResource, WriteOperationKind.AddToRelationship, cancellationToken);
await UpdateRelationshipAsync(relationship, leftResourceTracked, rightResourceIds, cancellationToken);

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

await SaveChangesAsync(cancellationToken);

await _resourceDefinitionAccessor.OnWriteSucceededAsync(leftResource, WriteOperationKind.AddToRelationship, cancellationToken);
await _resourceDefinitionAccessor.OnWriteSucceededAsync(leftResourceTracked, WriteOperationKind.AddToRelationship, cancellationToken);
}
}

Expand All @@ -470,35 +471,62 @@ public virtual async Task RemoveFromToManyRelationshipAsync(TResource leftResour
using IDisposable _ = CodeTimingSessionManager.Current.Measure("Repository - Remove from to-many relationship");

var relationship = (HasManyAttribute)_targetedFields.Relationships.Single();
HashSet<IIdentifiable> rightResourceIdsToRemove = rightResourceIds.ToHashSet(IdentifiableComparer.Instance);

await _resourceDefinitionAccessor.OnRemoveFromRelationshipAsync(leftResource, relationship, rightResourceIds, cancellationToken);
await _resourceDefinitionAccessor.OnRemoveFromRelationshipAsync(leftResource, relationship, rightResourceIdsToRemove, cancellationToken);

if (rightResourceIds.Any())
if (rightResourceIdsToRemove.Any())
{
var leftResourceTracked = (TResource)_dbContext.GetTrackedOrAttach(leftResource);

// Make EF Core believe any additional resources added from ResourceDefinition already exist in database.
IIdentifiable[] extraResourceIdsToRemove = rightResourceIdsToRemove.Where(rightId => !rightResourceIds.Contains(rightId)).ToArray();

object rightValueStored = relationship.GetValue(leftResource);

HashSet<IIdentifiable> rightResourceIdsToStore =
_collectionConverter.ExtractResources(rightValueStored).ToHashSet(IdentifiableComparer.Instance);
// @formatter:wrap_chained_method_calls chop_always
// @formatter:keep_existing_linebreaks true

rightResourceIdsToStore.ExceptWith(rightResourceIds);
IIdentifiable[] rightResourceIdsStored = _collectionConverter
.ExtractResources(rightValueStored)
.Concat(extraResourceIdsToRemove)
.Select(rightResource => _dbContext.GetTrackedOrAttach(rightResource))
.ToArray();

// @formatter:keep_existing_linebreaks restore
// @formatter:wrap_chained_method_calls restore

rightValueStored = _collectionConverter.CopyToTypedCollection(rightResourceIdsStored, relationship.Property.PropertyType);
relationship.SetValue(leftResource, rightValueStored);

MarkRelationshipAsLoaded(leftResource, relationship);

AssertIsNotClearingRequiredRelationship(relationship, leftResource, rightResourceIdsToStore);
HashSet<IIdentifiable> rightResourceIdsToStore = rightResourceIdsStored.ToHashSet(IdentifiableComparer.Instance);
rightResourceIdsToStore.ExceptWith(rightResourceIdsToRemove);

using var collector = new PlaceholderResourceCollector(_resourceFactory, _dbContext);
await UpdateRelationshipAsync(relationship, leftResource, rightResourceIdsToStore, collector, cancellationToken);
AssertIsNotClearingRequiredRelationship(relationship, leftResourceTracked, rightResourceIdsToStore);

await _resourceDefinitionAccessor.OnWritingAsync(leftResource, WriteOperationKind.RemoveFromRelationship, cancellationToken);
await UpdateRelationshipAsync(relationship, leftResourceTracked, rightResourceIdsToStore, cancellationToken);

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

await SaveChangesAsync(cancellationToken);

await _resourceDefinitionAccessor.OnWriteSucceededAsync(leftResource, WriteOperationKind.RemoveFromRelationship, cancellationToken);
await _resourceDefinitionAccessor.OnWriteSucceededAsync(leftResourceTracked, WriteOperationKind.RemoveFromRelationship, cancellationToken);
}
}

private void MarkRelationshipAsLoaded(TResource leftResource, RelationshipAttribute relationship)
{
EntityEntry<TResource> leftEntry = _dbContext.Entry(leftResource);
CollectionEntry rightCollectionEntry = leftEntry.Collection(relationship.Property.Name);
rightCollectionEntry.IsLoaded = true;
}

protected async Task UpdateRelationshipAsync(RelationshipAttribute relationship, TResource leftResource, object valueToAssign,
PlaceholderResourceCollector collector, CancellationToken cancellationToken)
CancellationToken cancellationToken)
{
object trackedValueToAssign = EnsureRelationshipValueToAssignIsTracked(valueToAssign, relationship.Property.PropertyType, collector);
object trackedValueToAssign = EnsureRelationshipValueToAssignIsTracked(valueToAssign, relationship.Property.PropertyType);

if (RequireLoadOfInverseRelationship(relationship, trackedValueToAssign))
{
Expand All @@ -511,15 +539,15 @@ protected async Task UpdateRelationshipAsync(RelationshipAttribute relationship,
relationship.SetValue(leftResource, trackedValueToAssign);
}

private object EnsureRelationshipValueToAssignIsTracked(object rightValue, Type relationshipPropertyType, PlaceholderResourceCollector collector)
private object EnsureRelationshipValueToAssignIsTracked(object rightValue, Type relationshipPropertyType)
{
if (rightValue == null)
{
return null;
}

ICollection<IIdentifiable> rightResources = _collectionConverter.ExtractResources(rightValue);
IIdentifiable[] rightResourcesTracked = rightResources.Select(collector.CaptureExisting).ToArray();
IIdentifiable[] rightResourcesTracked = rightResources.Select(rightResource => _dbContext.GetTrackedOrAttach(rightResource)).ToArray();

return rightValue is IEnumerable
? _collectionConverter.CopyToTypedCollection(rightResourcesTracked, relationshipPropertyType)
Expand Down Expand Up @@ -551,6 +579,8 @@ protected virtual async Task SaveChangesAsync(CancellationToken cancellationToke
await _dbContext.Database.CurrentTransaction.RollbackAsync(cancellationToken);
}

_dbContext.ResetChangeTracker();

throw new DataStoreUpdateException(exception);
}
}
Expand Down
85 changes: 0 additions & 85 deletions src/JsonApiDotNetCore/Repositories/PlaceholderResourceCollector.cs
View file
Open in desktop

This file was deleted.

Loading