From cb090353f4f037431c3f712f2175a366f20999c6 Mon Sep 17 00:00:00 2001 From: Josh Brown Date: Sat, 22 Jun 2024 10:20:12 +0100 Subject: [PATCH] Fix unguarded calls to ServiceDescriptor.ImplementationType for keyed services (#16604) * Update integration test base class to verify that calls to ServiceDescriptor.ImplementationType are guarded for keyed services * Fix unguarded calls to ServiceDescriptor.ImplementationType for keyed services --- .../DependencyInjection/UmbracoBuilderApiExtensions.cs | 2 +- .../DependencyInjection/UmbracoBuilderAuthExtensions.cs | 2 +- .../DependencyInjection/UmbracoBuilderExtensions.cs | 2 +- .../Testing/UmbracoIntegrationTest.cs | 6 ++++++ .../Umbraco.Infrastructure/Persistence/LocksTests.cs | 2 +- .../Umbraco.Persistence.EFCore/Scoping/EFCoreLockTests.cs | 2 +- 6 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/Umbraco.Cms.Api.Common/DependencyInjection/UmbracoBuilderApiExtensions.cs b/src/Umbraco.Cms.Api.Common/DependencyInjection/UmbracoBuilderApiExtensions.cs index b74727ff52..49fe1f233e 100644 --- a/src/Umbraco.Cms.Api.Common/DependencyInjection/UmbracoBuilderApiExtensions.cs +++ b/src/Umbraco.Cms.Api.Common/DependencyInjection/UmbracoBuilderApiExtensions.cs @@ -11,7 +11,7 @@ public static class UmbracoBuilderApiExtensions { public static IUmbracoBuilder AddUmbracoApiOpenApiUI(this IUmbracoBuilder builder) { - if (builder.Services.Any(x => x.ImplementationType == typeof(OperationIdSelector))) + if (builder.Services.Any(x => !x.IsKeyedService && x.ImplementationType == typeof(OperationIdSelector))) { return builder; } diff --git a/src/Umbraco.Cms.Api.Common/DependencyInjection/UmbracoBuilderAuthExtensions.cs b/src/Umbraco.Cms.Api.Common/DependencyInjection/UmbracoBuilderAuthExtensions.cs index 7e730695f3..d5556f63c2 100644 --- a/src/Umbraco.Cms.Api.Common/DependencyInjection/UmbracoBuilderAuthExtensions.cs +++ b/src/Umbraco.Cms.Api.Common/DependencyInjection/UmbracoBuilderAuthExtensions.cs @@ -17,7 +17,7 @@ public static class UmbracoBuilderAuthExtensions { public static IUmbracoBuilder AddUmbracoOpenIddict(this IUmbracoBuilder builder) { - if (builder.Services.Any(x=>x.ImplementationType == typeof(OpenIddictCleanupJob)) is false) + if (builder.Services.Any(x => !x.IsKeyedService && x.ImplementationType == typeof(OpenIddictCleanupJob)) is false) { ConfigureOpenIddict(builder); } diff --git a/src/Umbraco.Cms.Api.Management/DependencyInjection/UmbracoBuilderExtensions.cs b/src/Umbraco.Cms.Api.Management/DependencyInjection/UmbracoBuilderExtensions.cs index 9acd86381c..90993aa382 100644 --- a/src/Umbraco.Cms.Api.Management/DependencyInjection/UmbracoBuilderExtensions.cs +++ b/src/Umbraco.Cms.Api.Management/DependencyInjection/UmbracoBuilderExtensions.cs @@ -24,7 +24,7 @@ public static partial class UmbracoBuilderExtensions builder.Services.AddUnique(); builder.AddUmbracoApiOpenApiUI(); - if (!services.Any(x => x.ImplementationType == typeof(JsonPatchService))) + if (!services.Any(x => !x.IsKeyedService && x.ImplementationType == typeof(JsonPatchService))) { ModelsBuilderBuilderExtensions.AddModelsBuilder(builder) .AddJson() diff --git a/tests/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTest.cs b/tests/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTest.cs index 664fdb5493..43ade570e9 100644 --- a/tests/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTest.cs +++ b/tests/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTest.cs @@ -136,6 +136,12 @@ public abstract class UmbracoIntegrationTest : UmbracoIntegrationTestBase services.AddLogger(webHostEnvironment, Configuration); + // Register a keyed service to verify that all calls to ServiceDescriptor.ImplementationType + // are guarded by checking IsKeyedService first. + // Failure to check this when accessing a keyed service descriptor's ImplementationType property + // throws a InvalidOperationException. + services.AddKeyedSingleton("key"); + // Add it! var hostingEnvironment = TestHelper.GetHostingEnvironment(); var typeLoader = services.AddTypeLoader( diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/LocksTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/LocksTests.cs index 8a79cc78b8..93e4782195 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/LocksTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/LocksTests.cs @@ -36,7 +36,7 @@ public class LocksTests : UmbracoIntegrationTest protected override void ConfigureTestServices(IServiceCollection services) => // SQLite + retry policy makes tests fail, we retry before throwing distributed locking timeout. - services.RemoveAll(x => x.ImplementationType == typeof(SqliteAddRetryPolicyInterceptor)); + services.RemoveAll(x => !x.IsKeyedService && x.ImplementationType == typeof(SqliteAddRetryPolicyInterceptor)); [Test] public void SingleReadLockTest() diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Persistence.EFCore/Scoping/EFCoreLockTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Persistence.EFCore/Scoping/EFCoreLockTests.cs index 5103c2e2fa..200da2557b 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Persistence.EFCore/Scoping/EFCoreLockTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Persistence.EFCore/Scoping/EFCoreLockTests.cs @@ -23,7 +23,7 @@ public class EFCoreLockTests : UmbracoIntegrationTest protected override void ConfigureTestServices(IServiceCollection services) { // SQLite + retry policy makes tests fail, we retry before throwing distributed locking timeout. - services.RemoveAll(x => x.ImplementationType == typeof(SqliteAddRetryPolicyInterceptor)); + services.RemoveAll(x => !x.IsKeyedService && x.ImplementationType == typeof(SqliteAddRetryPolicyInterceptor)); // Remove all locking implementations to ensure we only use EFCoreDistributedLockingMechanisms services.RemoveAll(x => x.ServiceType == typeof(IDistributedLockingMechanism));