Skip to content

Commit a2cc6a0

Browse files
MigaroezAndyButland
Migaroez
and
AndyButland
authored
Fix issue with use of EF Core scopes within notification handlers (take 2 - handling scopes with a base parent) (#19797)
* Add integration tests that shows the problem * Fix the problem and add explenation * Improved comments slightly to help when we come back here! Moved tests alongside existing ones related to scopes. Removed long running attribute from tests (they are quite fast). * Fixed casing in comment. --------- Co-authored-by: Andy Butland <[email protected]>
1 parent 5c57d03 commit a2cc6a0

File tree

3 files changed

+231
-1
lines changed

3 files changed

+231
-1
lines changed

‎src/Umbraco.Cms.Persistence.EFCore/Scoping/EFCoreScope.cs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,10 +127,16 @@ public override void Dispose()
127127

128128
Locks.ClearLocks(InstanceId);
129129

130-
if (ParentScope is null)
130+
// Since we can nest EFCoreScopes in other scopes derived from CoreScope, we should check whether our ParentScope OR the base ParentScope exists.
131+
// Only if neither do do we take responsibility for ensuring the locks are cleared.
132+
// Eventually the highest parent will clear the locks.
133+
// Further, these locks are a reference to the locks of the highest parent anyway (see the constructor of CoreScope).
134+
#pragma warning disable SA1100 // Do not prefix calls with base unless local implementation exists (justification: provides additional clarify here that this is defined on the base class).
135+
if (ParentScope is null && base.HasParentScope is false)
131136
{
132137
Locks.EnsureLocksCleared(InstanceId);
133138
}
139+
#pragma warning restore SA1100 // Do not prefix calls with base unless local implementation exists
134140

135141
_efCoreScopeProvider.PopAmbientScope();
136142

‎src/Umbraco.Core/Scoping/CoreScope.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,8 @@ protected void SetParentScope(ICoreScope coreScope)
250250
_parentScope = coreScope;
251251
}
252252

253+
protected bool HasParentScope => _parentScope is not null;
254+
253255
protected void HandleScopedNotifications() => _notificationPublisher?.ScopeExit(Completed.HasValue && Completed.Value);
254256

255257
private void EnsureNotDisposed()

‎tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/NestedScopeTests.cs

Lines changed: 222 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,222 @@
1+
using Microsoft.Extensions.DependencyInjection;
2+
using NUnit.Framework;
3+
using Umbraco.Cms.Core;
4+
using Umbraco.Cms.Core.Scoping;
5+
using Umbraco.Cms.Persistence.EFCore.Scoping;
6+
using Umbraco.Cms.Tests.Common.Testing;
7+
using Umbraco.Cms.Tests.Integration.Testing;
8+
using Umbraco.Cms.Tests.Integration.Umbraco.Persistence.EFCore.DbContext;
9+
using IScopeProvider = Umbraco.Cms.Infrastructure.Scoping.IScopeProvider;
10+
11+
namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping
12+
{
13+
/// <summary>
14+
/// These tests verify that the various types of scopes we have can be created and disposed within each other.
15+
/// </summary>
16+
/// <remarks>
17+
/// Scopes are:
18+
/// - "Normal" - created by <see cref="IScopeProvider"/>"/>.
19+
/// - "Core" - created by <see cref="ICoreScopeProvider"/>"/>.
20+
/// - "EFCore" - created by <see cref="IEFCoreScopeProvider{TDbContext}"/>"/>.
21+
/// </remarks>
22+
[TestFixture]
23+
[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)]
24+
internal sealed class NestedScopeTests : UmbracoIntegrationTest
25+
{
26+
private new IScopeProvider ScopeProvider => Services.GetRequiredService<IScopeProvider>();
27+
28+
private ICoreScopeProvider CoreScopeProvider => Services.GetRequiredService<ICoreScopeProvider>();
29+
30+
private IEFCoreScopeProvider<TestUmbracoDbContext> EfCoreScopeProvider =>
31+
Services.GetRequiredService<IEFCoreScopeProvider<TestUmbracoDbContext>>();
32+
33+
[Test]
34+
public void CanNestScopes_Normal_Core_EfCore()
35+
{
36+
using (var ambientScope = ScopeProvider.CreateScope())
37+
{
38+
ambientScope.WriteLock(Constants.Locks.ContentTree);
39+
40+
using (var outerScope = CoreScopeProvider.CreateCoreScope())
41+
{
42+
outerScope.WriteLock(Constants.Locks.ContentTree);
43+
44+
using (var innerScope = EfCoreScopeProvider.CreateScope())
45+
{
46+
innerScope.WriteLock(Constants.Locks.ContentTree);
47+
48+
innerScope.Complete();
49+
outerScope.Complete();
50+
ambientScope.Complete();
51+
}
52+
}
53+
}
54+
}
55+
56+
[Test]
57+
public void CanNestScopes_Normal_EfCore_Core()
58+
{
59+
using (var ambientScope = ScopeProvider.CreateScope())
60+
{
61+
ambientScope.WriteLock(Constants.Locks.ContentTree);
62+
63+
using (var outerScope = EfCoreScopeProvider.CreateScope())
64+
{
65+
outerScope.WriteLock(Constants.Locks.ContentTree);
66+
67+
using (var innerScope = CoreScopeProvider.CreateCoreScope())
68+
{
69+
innerScope.WriteLock(Constants.Locks.ContentTree);
70+
71+
innerScope.Complete();
72+
outerScope.Complete();
73+
ambientScope.Complete();
74+
}
75+
}
76+
}
77+
}
78+
79+
[Test]
80+
public void CanNestScopes_Core_Normal_Efcore()
81+
{
82+
using (var ambientScope = CoreScopeProvider.CreateCoreScope())
83+
{
84+
ambientScope.WriteLock(Constants.Locks.ContentTree);
85+
86+
using (var outerScope = ScopeProvider.CreateScope())
87+
{
88+
outerScope.WriteLock(Constants.Locks.ContentTree);
89+
90+
using (var innerScope = EfCoreScopeProvider.CreateScope())
91+
{
92+
innerScope.WriteLock(Constants.Locks.ContentTree);
93+
94+
innerScope.Complete();
95+
outerScope.Complete();
96+
ambientScope.Complete();
97+
}
98+
}
99+
}
100+
}
101+
102+
[Test]
103+
public void CanNestScopes_Core_EfCore_Normal()
104+
{
105+
using (var ambientScope = CoreScopeProvider.CreateCoreScope())
106+
{
107+
ambientScope.WriteLock(Constants.Locks.ContentTree);
108+
109+
using (var outerScope = EfCoreScopeProvider.CreateScope())
110+
{
111+
outerScope.WriteLock(Constants.Locks.ContentTree);
112+
113+
using (var innerScope = ScopeProvider.CreateScope())
114+
{
115+
innerScope.WriteLock(Constants.Locks.ContentTree);
116+
117+
innerScope.Complete();
118+
outerScope.Complete();
119+
ambientScope.Complete();
120+
}
121+
}
122+
}
123+
}
124+
125+
[Test]
126+
public void CanNestScopes_EfCore_Normal_Core()
127+
{
128+
using (var ambientScope = EfCoreScopeProvider.CreateScope())
129+
{
130+
ambientScope.WriteLock(Constants.Locks.ContentTree);
131+
132+
using (var outerScope = ScopeProvider.CreateScope())
133+
{
134+
outerScope.WriteLock(Constants.Locks.ContentTree);
135+
136+
using (var innerScope = CoreScopeProvider.CreateCoreScope())
137+
{
138+
innerScope.WriteLock(Constants.Locks.ContentTree);
139+
140+
innerScope.Complete();
141+
outerScope.Complete();
142+
ambientScope.Complete();
143+
}
144+
}
145+
}
146+
}
147+
148+
[Test]
149+
public void CanNestScopes_EfCore_Core_Normal()
150+
{
151+
using (var ambientScope = EfCoreScopeProvider.CreateScope())
152+
{
153+
ambientScope.WriteLock(Constants.Locks.ContentTree);
154+
155+
using (var outerScope = CoreScopeProvider.CreateCoreScope())
156+
{
157+
outerScope.WriteLock(Constants.Locks.ContentTree);
158+
159+
using (var innerScope = ScopeProvider.CreateScope())
160+
{
161+
innerScope.WriteLock(Constants.Locks.ContentTree);
162+
163+
innerScope.Complete();
164+
outerScope.Complete();
165+
ambientScope.Complete();
166+
}
167+
}
168+
}
169+
}
170+
171+
[Test]
172+
public void CanNestScopes_Normal_Normal()
173+
{
174+
using (var ambientScope = ScopeProvider.CreateScope())
175+
{
176+
ambientScope.WriteLock(Constants.Locks.ContentTree);
177+
178+
using (var inner = ScopeProvider.CreateScope())
179+
{
180+
inner.WriteLock(Constants.Locks.ContentTree);
181+
182+
inner.Complete();
183+
ambientScope.Complete();
184+
}
185+
}
186+
}
187+
188+
[Test]
189+
public void CanNestScopes_Core_Core()
190+
{
191+
using (var ambientScope = CoreScopeProvider.CreateCoreScope())
192+
{
193+
ambientScope.WriteLock(Constants.Locks.ContentTree);
194+
195+
using (var inner = CoreScopeProvider.CreateCoreScope())
196+
{
197+
inner.WriteLock(Constants.Locks.ContentTree);
198+
199+
inner.Complete();
200+
ambientScope.Complete();
201+
}
202+
}
203+
}
204+
205+
[Test]
206+
public void CanNestScopes_EfCore_EfCore()
207+
{
208+
using (var ambientScope = EfCoreScopeProvider.CreateScope())
209+
{
210+
ambientScope.WriteLock(Constants.Locks.ContentTree);
211+
212+
using (var inner = EfCoreScopeProvider.CreateScope())
213+
{
214+
inner.WriteLock(Constants.Locks.ContentTree);
215+
216+
inner.Complete();
217+
ambientScope.Complete();
218+
}
219+
}
220+
}
221+
}
222+
}

0 commit comments

Comments
 (0)