From 41a003e20ebafe9e807ee21724d587ba14feb59c Mon Sep 17 00:00:00 2001 From: Paul Johnson Date: Tue, 24 May 2022 11:26:28 +0100 Subject: [PATCH] Ensure legacy scope returned when using legacy scope provider (#12465) * Separate legacy scope provider interface and explicitly implement. * Don't rely on legacy scope provider for existing tests. * Assert correct type returned when using legacy scope provider. --- .../Packaging/PackageDataInstallation.cs | 2 +- .../Scoping/LegacyIScopeProvider.cs | 85 ++++++++++++++++++- .../Scoping/ScopeProvider.cs | 36 ++++++++ .../Scoping/LegacyScopeProviderTests.cs | 21 +++++ .../PublishedSnapshotServiceTestBase.cs | 2 +- .../HealthCheckNotifierTests.cs | 2 +- .../Mapping/MappingTests.cs | 4 +- .../Security/MemberManagerTests.cs | 6 +- .../Security/MemberUserStoreTests.cs | 2 +- .../SnapDictionaryTests.cs | 4 +- .../Controllers/ContentControllerTests.cs | 2 +- 11 files changed, 152 insertions(+), 14 deletions(-) create mode 100644 tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/LegacyScopeProviderTests.cs diff --git a/src/Umbraco.Infrastructure/Packaging/PackageDataInstallation.cs b/src/Umbraco.Infrastructure/Packaging/PackageDataInstallation.cs index f0bca5f1ea..c870f2b7c6 100644 --- a/src/Umbraco.Infrastructure/Packaging/PackageDataInstallation.cs +++ b/src/Umbraco.Infrastructure/Packaging/PackageDataInstallation.cs @@ -104,7 +104,7 @@ namespace Umbraco.Cms.Infrastructure.Packaging contentTypeService, contentService, propertyEditors, - scopeProvider, + (Umbraco.Cms.Infrastructure.Scoping.IScopeProvider) scopeProvider, shortStringHelper, serializer, mediaService, diff --git a/src/Umbraco.Infrastructure/Scoping/LegacyIScopeProvider.cs b/src/Umbraco.Infrastructure/Scoping/LegacyIScopeProvider.cs index 371446e10c..b5dc069592 100644 --- a/src/Umbraco.Infrastructure/Scoping/LegacyIScopeProvider.cs +++ b/src/Umbraco.Infrastructure/Scoping/LegacyIScopeProvider.cs @@ -1,9 +1,90 @@ -using System; +using System.Data; +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Infrastructure.Persistence; // ReSharper disable once CheckNamespace namespace Umbraco.Cms.Core.Scoping; [Obsolete("Please use Umbraco.Cms.Infrastructure.Scoping.IScopeProvider or Umbraco.Cms.Core.Scoping.ICoreScopeProvider instead.")] -public interface IScopeProvider : Infrastructure.Scoping.IScopeProvider +public interface IScopeProvider { + /// + /// Creates an ambient scope. + /// + /// The transaction isolation level. + /// The repositories cache mode. + /// An optional events dispatcher. + /// An optional notification publisher. + /// A value indicating whether to scope the filesystems. + /// A value indicating whether this scope should always be registered in the call context. + /// A value indicating whether this scope is auto-completed. + /// The created ambient scope. + /// + /// The created scope becomes the ambient scope. + /// If an ambient scope already exists, it becomes the parent of the created scope. + /// When the created scope is disposed, the parent scope becomes the ambient scope again. + /// Parameters must be specified on the outermost scope, or must be compatible with the parents. + /// Auto-completed scopes should be used for read-only operations ONLY. Do not use them if you do not + /// understand the associated issues, such as the scope being completed even though an exception is thrown. + /// + IScope CreateScope( + IsolationLevel isolationLevel = IsolationLevel.Unspecified, + RepositoryCacheMode repositoryCacheMode = RepositoryCacheMode.Unspecified, + IEventDispatcher? eventDispatcher = null, + IScopedNotificationPublisher? scopedNotificationPublisher = null, + bool? scopeFileSystems = null, + bool callContext = false, + bool autoComplete = false); + + /// + /// Creates a detached scope. + /// + /// A detached scope. + /// The transaction isolation level. + /// The repositories cache mode. + /// An optional events dispatcher. + /// An option notification publisher. + /// A value indicating whether to scope the filesystems. + /// + /// A detached scope is not ambient and has no parent. + /// It is meant to be attached by . + /// + /// + /// This is not used by CMS but is used by Umbraco Deploy. + /// + IScope CreateDetachedScope( + IsolationLevel isolationLevel = IsolationLevel.Unspecified, + RepositoryCacheMode repositoryCacheMode = RepositoryCacheMode.Unspecified, + IEventDispatcher? eventDispatcher = null, + IScopedNotificationPublisher? scopedNotificationPublisher = null, + bool? scopeFileSystems = null); + + /// + /// Attaches a scope. + /// + /// The scope to attach. + /// A value indicating whether to force usage of call context. + /// + /// Only a scope created by can be attached. + /// + void AttachScope(IScope scope, bool callContext = false); + + /// + /// Detaches a scope. + /// + /// The detached scope. + /// + /// Only a scope previously attached by can be detached. + /// + IScope DetachScope(); + + /// + /// Gets the scope context. + /// + IScopeContext? Context { get; } + + /// + /// Gets the sql context. + /// + ISqlContext SqlContext { get; } } diff --git a/src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs b/src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs index 6fbfc170b0..4022f366e2 100644 --- a/src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs +++ b/src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs @@ -658,6 +658,42 @@ namespace Umbraco.Cms.Infrastructure.Scoping } } #endif + /// + Cms.Core.Scoping.IScope Cms.Core.Scoping.IScopeProvider.CreateScope( + IsolationLevel isolationLevel = IsolationLevel.Unspecified, + RepositoryCacheMode repositoryCacheMode = RepositoryCacheMode.Unspecified, + IEventDispatcher? eventDispatcher = null, + IScopedNotificationPublisher? notificationPublisher = null, + bool? scopeFileSystems = null, + bool callContext = false, + bool autoComplete = false) => + (Cms.Core.Scoping.IScope) CreateScope( + isolationLevel, + repositoryCacheMode, + eventDispatcher, + notificationPublisher, + scopeFileSystems, + callContext, + autoComplete); + + /// + Core.Scoping.IScope Core.Scoping.IScopeProvider.CreateDetachedScope(IsolationLevel isolationLevel, + RepositoryCacheMode repositoryCacheMode, IEventDispatcher? eventDispatcher, + IScopedNotificationPublisher? scopedNotificationPublisher, bool? scopeFileSystems) => + (Core.Scoping.IScope)CreateDetachedScope( + isolationLevel, + repositoryCacheMode, + eventDispatcher, + scopedNotificationPublisher, + scopeFileSystems); + + /// + void Core.Scoping.IScopeProvider.AttachScope(Core.Scoping.IScope scope, bool callContext) => + AttachScope(scope, callContext); + + /// + Core.Scoping.IScope Core.Scoping.IScopeProvider.DetachScope() => + (Core.Scoping.IScope)DetachScope(); } #if DEBUG_SCOPES diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/LegacyScopeProviderTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/LegacyScopeProviderTests.cs new file mode 100644 index 0000000000..ae7859b9cc --- /dev/null +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/LegacyScopeProviderTests.cs @@ -0,0 +1,21 @@ +using NUnit.Framework; +using Umbraco.Cms.Tests.Common.Testing; +using Umbraco.Cms.Tests.Integration.Testing; + +namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping; + +[TestFixture] +[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerFixture)] +public class LegacyScopeProviderTests : UmbracoIntegrationTest +{ + [Test] + public void CreateScope_Always_ReturnsLegacyIScope() + { + var scopeProvider = GetRequiredService(); + + using (var scope = scopeProvider.CreateScope()) + { + Assert.IsInstanceOf(scope); + } + } +} diff --git a/tests/Umbraco.Tests.UnitTests/TestHelpers/PublishedSnapshotServiceTestBase.cs b/tests/Umbraco.Tests.UnitTests/TestHelpers/PublishedSnapshotServiceTestBase.cs index 4cf6ffdc9e..a74f9e5726 100644 --- a/tests/Umbraco.Tests.UnitTests/TestHelpers/PublishedSnapshotServiceTestBase.cs +++ b/tests/Umbraco.Tests.UnitTests/TestHelpers/PublishedSnapshotServiceTestBase.cs @@ -225,7 +225,7 @@ namespace Umbraco.Cms.Tests.UnitTests.TestHelpers DomainService = serviceContext.DomainService; // create a scope provider - IScopeProvider scopeProvider = Mock.Of(); + Infrastructure.Scoping.IScopeProvider scopeProvider = Mock.Of(); Mock.Get(scopeProvider) .Setup(x => x.CreateScope( It.IsAny(), diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/HealthCheckNotifierTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/HealthCheckNotifierTests.cs index b6f94c17a8..b60eb9891d 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/HealthCheckNotifierTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/HealthCheckNotifierTests.cs @@ -145,7 +145,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.HostedServices var mockMainDom = new Mock(); mockMainDom.SetupGet(x => x.IsMainDom).Returns(isMainDom); - var mockScopeProvider = new Mock(); + var mockScopeProvider = new Mock(); var mockLogger = new Mock>(); var mockProfilingLogger = new Mock(); diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Mapping/MappingTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Mapping/MappingTests.cs index 9c8f9da75d..b3e761c5be 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Mapping/MappingTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Mapping/MappingTests.cs @@ -20,12 +20,12 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Mapping [TestFixture] public class MappingTests { - private IScopeProvider _scopeProvider; + private global::Umbraco.Cms.Infrastructure.Scoping.IScopeProvider _scopeProvider; [SetUp] public void MockScopeProvider() { - var scopeMock = new Mock(); + var scopeMock = new Mock(); scopeMock.Setup(x => x.CreateScope( It.IsAny(), It.IsAny(), diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberManagerTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberManagerTests.cs index 5fd34dae3f..c2081f2ce4 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberManagerTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberManagerTests.cs @@ -36,7 +36,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security public MemberManager CreateSut() { - IScopeProvider scopeProvider = new Mock().Object; + global::Umbraco.Cms.Infrastructure.Scoping.IScopeProvider scopeProvider = new Mock().Object; _mockMemberService = new Mock(); var mapDefinitions = new List() @@ -53,8 +53,8 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security new UmbracoMapper(new MapDefinitionCollection(() => mapDefinitions), scopeProvider), scopeProvider, new IdentityErrorDescriber(), - Mock.Of(), - Mock.Of(), + Mock.Of(), + Mock.Of(), Mock.Of()); _mockIdentityOptions = new Mock>(); diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberUserStoreTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberUserStoreTests.cs index 14261e34fb..0773da9c3f 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberUserStoreTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberUserStoreTests.cs @@ -27,7 +27,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security { _mockMemberService = new Mock(); var mockScope = new Mock(); - var mockScopeProvider = new Mock(); + var mockScopeProvider = new Mock(); mockScopeProvider .Setup(x => x.CreateScope(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) .Returns(mockScope.Object); diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.PublishedCache.NuCache/SnapDictionaryTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.PublishedCache.NuCache/SnapDictionaryTests.cs index 2b65cf81ef..b8e4953c50 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.PublishedCache.NuCache/SnapDictionaryTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.PublishedCache.NuCache/SnapDictionaryTests.cs @@ -1171,9 +1171,9 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.PublishedCache.NuCache } } - private static IScopeProvider GetScopeProvider() + private static ICoreScopeProvider GetScopeProvider() { - IScopeProvider scopeProvider = Mock.Of(); + ICoreScopeProvider scopeProvider = Mock.Of(); Mock.Get(scopeProvider) .Setup(x => x.Context).Returns(() => null); return scopeProvider; diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/ContentControllerTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/ContentControllerTests.cs index 2d703b8d0f..b2b14ceab2 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/ContentControllerTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/ContentControllerTests.cs @@ -260,7 +260,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers new ActionCollection(() => null), Mock.Of(), Mock.Of(), - Mock.Of(), + Mock.Of(), Mock.Of(), Mock.Of() );