Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 7 additions & 5 deletions src/Trax.Effect.Data/Services/DataContext/DataContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -239,14 +239,16 @@ public async Task Track(IModel model)
{
var entry = Entry(model);

// Skip if already tracked — avoids base.Update() forcing Added → Modified
// Skip if already tracked — avoids forcing Added → Modified
// on entities that haven't been saved yet (breaks InMemory provider).
if (entry.State != EntityState.Detached)
return;

// base.Update uses EF's key heuristic: Id == 0 → Added, Id > 0 → Modified.
// This correctly handles both new entities and entities loaded from another context.
base.Update(model);
// Set state directly to avoid graph traversal — both base.Update() and Attach()
// walk ALL loaded navigation properties, which generates unnecessary UPDATE
// statements (e.g., Manifest) and can cause lock contention.
// Setting entry.State only affects this entity, not navigations.
entry.State = model.Id > 0 ? EntityState.Modified : EntityState.Added;
}

public async Task Update(IModel model)
Expand All @@ -256,7 +258,7 @@ public async Task Update(IModel model)
// Skip if already tracked — EF change tracking detects property mutations
// automatically via snapshot comparison.
if (entry.State == EntityState.Detached)
base.Update(model);
entry.State = model.Id > 0 ? EntityState.Modified : EntityState.Added;
}

/// <summary>
Expand Down
4 changes: 2 additions & 2 deletions src/Trax.Effect/Services/EffectRunner/EffectRunner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,13 @@ await ActiveEffectProviders.RunAllAsync(provider =>
/// </remarks>
public async Task Track(IModel model)
{
ActiveEffectProviders.RunAll(provider => provider.Track(model));
await ActiveEffectProviders.RunAllAsync(provider => provider.Track(model));
}

/// <inheritdoc />
public async Task Update(IModel model)
{
ActiveEffectProviders.RunAll(provider => provider.Update(model));
await ActiveEffectProviders.RunAllAsync(provider => provider.Update(model));
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
using Trax.Effect.Data.InMemory.Services.InMemoryContext;
using Trax.Effect.Data.Services.DataContext;
using Trax.Effect.Enums;
using Trax.Effect.Models.Manifest;
using Trax.Effect.Models.Manifest.DTOs;
using Trax.Effect.Models.Metadata.DTOs;
using Metadata = Trax.Effect.Models.Metadata.Metadata;

Expand Down Expand Up @@ -291,6 +293,100 @@ public async Task CrossContext_EntitySavedInOneContext_CanBeTrackedByAnother()

#endregion

#region Navigation Isolation Tests

[Test]
public async Task Track_EntityWithLoadedNavigation_DoesNotMarkNavigationAsModified()
{
var dbName = nameof(Track_EntityWithLoadedNavigation_DoesNotMarkNavigationAsModified);

// Context A: create manifest and metadata with FK relationship
var contextA = CreateContext(dbName);
var manifest = Manifest.Create(
new CreateManifest { Name = typeof(DataContextChangeTrackingTests) }
);
contextA.Manifests.Add(manifest);
await contextA.SaveChanges(CancellationToken.None);

var metadata = CreateMetadata("NavIsolation");
metadata.ManifestId = manifest.Id;
await contextA.Track(metadata);
await contextA.SaveChanges(CancellationToken.None);
var savedId = metadata.Id;

// Context B: load metadata WITH Manifest eagerly loaded (like LoadMetadataJunction)
var contextB = CreateContext(dbName);
var loaded = await contextB
.Metadatas.Include(x => x.Manifest)
.FirstAsync(x => x.Id == savedId);
loaded.Manifest.Should().NotBeNull();

// Context C: simulate inner train's EffectRunner context (cross-context Track)
var contextC = CreateContext(dbName);
contextC.Entry(loaded).State.Should().Be(EntityState.Detached);

await contextC.Track(loaded);

// Metadata should be tracked as Modified
contextC.Entry(loaded).State.Should().Be(EntityState.Modified);

// Manifest should NOT be tracked — this is the bug fix
contextC
.Entry(loaded.Manifest!)
.State.Should()
.Be(
EntityState.Detached,
"Navigation properties should not be cascaded into the change tracker"
);
}

[Test]
public async Task Update_EntityWithLoadedNavigation_DoesNotMarkNavigationAsModified()
{
var dbName = nameof(Update_EntityWithLoadedNavigation_DoesNotMarkNavigationAsModified);

// Context A: create manifest and metadata with FK relationship
var contextA = CreateContext(dbName);
var manifest = Manifest.Create(
new CreateManifest { Name = typeof(DataContextChangeTrackingTests) }
);
contextA.Manifests.Add(manifest);
await contextA.SaveChanges(CancellationToken.None);

var metadata = CreateMetadata("NavIsolationUpdate");
metadata.ManifestId = manifest.Id;
await contextA.Track(metadata);
await contextA.SaveChanges(CancellationToken.None);
var savedId = metadata.Id;

// Context B: load metadata WITH Manifest eagerly loaded
var contextB = CreateContext(dbName);
var loaded = await contextB
.Metadatas.Include(x => x.Manifest)
.FirstAsync(x => x.Id == savedId);
loaded.Manifest.Should().NotBeNull();

// Context C: simulate cross-context Update on detached entity
var contextC = CreateContext(dbName);
contextC.Entry(loaded).State.Should().Be(EntityState.Detached);

await contextC.Update(loaded);

// Metadata should be tracked as Modified
contextC.Entry(loaded).State.Should().Be(EntityState.Modified);

// Manifest should NOT be tracked
contextC
.Entry(loaded.Manifest!)
.State.Should()
.Be(
EntityState.Detached,
"Navigation properties should not be cascaded into the change tracker"
);
}

#endregion

#region Helpers

private static Metadata CreateMetadata(string name) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,32 @@ public async Task EffectRunner_Update_DelegatesToAllProviders()
factory2.Provider.UpdateCalled.Should().BeTrue();
}

[Test]
public async Task EffectRunner_Track_AwaitsAsyncProviders()
{
var registry = new EffectRegistry();
var factory = new AsyncTrackingEffectFactory();
registry.Register(typeof(AsyncTrackingEffectFactory), enabled: true);

using var runner = new EffectRunner([factory], registry);
await runner.Track(new FakeModel());

factory.Provider.TrackCompleted.Should().BeTrue();
}

[Test]
public async Task EffectRunner_Update_AwaitsAsyncProviders()
{
var registry = new EffectRegistry();
var factory = new AsyncTrackingEffectFactory();
registry.Register(typeof(AsyncTrackingEffectFactory), enabled: true);

using var runner = new EffectRunner([factory], registry);
await runner.Update(new FakeModel());

factory.Provider.UpdateCompleted.Should().BeTrue();
}

[Test]
public void EffectRunner_Dispose_HandlesProviderDisposalFailure()
{
Expand Down Expand Up @@ -460,5 +486,34 @@ private class ThrowingDisposeJunctionEffectFactory : IJunctionEffectProviderFact
public IJunctionEffectProvider Create() => Provider;
}

private class AsyncTrackingEffectProvider : IEffectProvider
{
public bool TrackCompleted { get; private set; }
public bool UpdateCompleted { get; private set; }

public Task SaveChanges(CancellationToken cancellationToken) => Task.CompletedTask;

public async Task Track(IModel model)
{
await Task.Yield();
TrackCompleted = true;
}

public async Task Update(IModel model)
{
await Task.Yield();
UpdateCompleted = true;
}

public void Dispose() { }
}

private class AsyncTrackingEffectFactory : IEffectProviderFactory
{
public AsyncTrackingEffectProvider Provider { get; } = new();

public IEffectProvider Create() => Provider;
}

#endregion
}
Loading