From 56b9c7c03f8b6f8bde82081b4f6f9d90dd77fec4 Mon Sep 17 00:00:00 2001 From: Craig Fowler Date: Thu, 5 Nov 2026 20:13:25 +0000 Subject: [PATCH 1/2] Resolve #63 - Alter DI to avoid enumerating the container There's lots of docco on the `IGetsLogger` interface. --- .../WebDriverCreationConfigureOptionsTests.cs | 10 +-- .../WebDriverFactoryIntegrationTests.cs | 12 ++++ .../TestLoggerAttribute.cs | 20 ++++-- .../Factories/DriverTypeProvider.cs | 10 +-- .../Factories/OptionsTypeProvider.cs | 10 +-- .../ProxyWrappingWebDriverFactoryDecorator.cs | 8 ++- .../RemoteWebDriverFromOptionsFactory.cs | 8 ++- .../WebDriverConfigurationItemParser.cs | 10 +-- .../WebDriverCreationConfigureOptions.cs | 10 +-- .../Factories/WebDriverFromOptionsFactory.cs | 8 ++- .../WebDriverFromThirdPartyFactory.cs | 8 ++- CSF.Extensions.WebDriver/IGetsLogger.cs | 61 +++++++++++++++++++ .../ServiceCollectionExtensions.cs | 19 +----- 13 files changed, 140 insertions(+), 54 deletions(-) create mode 100644 CSF.Extensions.WebDriver/IGetsLogger.cs diff --git a/CSF.Extensions.WebDriver.Tests/Factories/WebDriverCreationConfigureOptionsTests.cs b/CSF.Extensions.WebDriver.Tests/Factories/WebDriverCreationConfigureOptionsTests.cs index a8a1115..211eb9b 100644 --- a/CSF.Extensions.WebDriver.Tests/Factories/WebDriverCreationConfigureOptionsTests.cs +++ b/CSF.Extensions.WebDriver.Tests/Factories/WebDriverCreationConfigureOptionsTests.cs @@ -203,7 +203,7 @@ public async Task ConfigureShouldNotThrowForARemoteDriverWithoutOptionsType([Sta [Test,AutoMoqData] public async Task ConfigureShouldBeAbleToSetupRemoteDriverWithSimpleOptionsFromJsonConfiguration([StandardTypes] IGetsDriverType driverTypeProvider, [StandardTypes] IGetsOptionsType optionsTypeProvider, - [TestLogger] ILogger logger) + [TestLogger(LoggerType = typeof(WebDriverCreationConfigureOptions))] IGetsLogger loggerProvider) { var type = typeof(SafariOptions); Mock.Get(optionsTypeProvider) @@ -214,7 +214,7 @@ public async Task ConfigureShouldBeAbleToSetupRemoteDriverWithSimpleOptionsFromJ ""DriverConfigurations"": { ""Test"": { ""DriverType"": ""RemoteWebDriver"", ""OptionsType"": ""SafariOptions"" } } -}", logger: logger); +}", loggerProvider: loggerProvider); Assert.That(options.DriverConfigurations, Is.Not.Empty); } @@ -230,16 +230,16 @@ public async Task ConfigureShouldBeAbleToSetupRemoteDriverWithSimpleOptionsFromJ static async Task GetOptionsAsync(IGetsDriverType driverTypeProvider, IGetsOptionsType optionsTypeProvider, string json, - ILogger? logger = null) + IGetsLogger? loggerProvider = null) { var options = new WebDriverCreationOptionsCollection(); var config = await ConfigurationFactory.GetConfigurationAsync(json); ICreatesDriverOptions optionsFactory = new ActivatorDriverOptionsFactory(); optionsFactory = new ConfigBindingDriverOptionsFactoryDecorator(optionsFactory); - var parser = new WebDriverConfigurationItemParser(driverTypeProvider, optionsTypeProvider, optionsFactory, Mock.Of>()); + var parser = new WebDriverConfigurationItemParser(driverTypeProvider, optionsTypeProvider, optionsFactory, Mock.Of()); var sut = new WebDriverCreationConfigureOptions(parser, config, - logger ?? Mock.Of>(), + loggerProvider ?? Mock.Of(), c => {}); sut.Configure(options); return options; diff --git a/CSF.Extensions.WebDriver.Tests/Factories/WebDriverFactoryIntegrationTests.cs b/CSF.Extensions.WebDriver.Tests/Factories/WebDriverFactoryIntegrationTests.cs index 4c1c6a5..63ccf17 100644 --- a/CSF.Extensions.WebDriver.Tests/Factories/WebDriverFactoryIntegrationTests.cs +++ b/CSF.Extensions.WebDriver.Tests/Factories/WebDriverFactoryIntegrationTests.cs @@ -80,6 +80,18 @@ public void GetWebDriverShouldReturnADriverWithTheCorrectQuirksForFirefox() }); } + [Test] + public void AddWebDriverFactoryWithoutOptionsPatternShouldNotEnumerateServiceCollection() + { + var serviceCollectionMock = new Mock(); + serviceCollectionMock.Setup(x => x.GetEnumerator()).Returns(Enumerable.Empty().GetEnumerator()); + var serviceCollection = serviceCollectionMock.Object; + + serviceCollection.AddWebDriverFactoryWithoutOptionsPattern(); + + Mock.Get(serviceCollection).Verify(x => x.GetEnumerator(), Times.Never); + } + Quirks.QuirksData GetCommonBrowserQuirks() { return new () diff --git a/CSF.Extensions.WebDriver.Tests/TestLoggerAttribute.cs b/CSF.Extensions.WebDriver.Tests/TestLoggerAttribute.cs index 2a9eb16..e8d2e63 100644 --- a/CSF.Extensions.WebDriver.Tests/TestLoggerAttribute.cs +++ b/CSF.Extensions.WebDriver.Tests/TestLoggerAttribute.cs @@ -18,13 +18,23 @@ namespace CSF.Extensions.WebDriver; /// public sealed class TestLoggerAttribute : CustomizeAttribute { + public Type? LoggerType { get; set; } + public override ICustomization GetCustomization(ParameterInfo parameter) { - if (!parameter.ParameterType.IsGenericType - || parameter.ParameterType.GetGenericTypeDefinition() != typeof(ILogger<>)) - throw new ArgumentException($"The parameter type must be an {nameof(ILogger)}", nameof(parameter)); - - return new TestLoggerCustomization(parameter.ParameterType.GenericTypeArguments.Single()); + var loggerType = GetLoggerType(parameter); + return new TestLoggerCustomization(loggerType); + } + + Type GetLoggerType(ParameterInfo parameter) + { + if(LoggerType is not null) return LoggerType; + + if (parameter.ParameterType.IsGenericType + && parameter.ParameterType.GetGenericTypeDefinition() == typeof(ILogger<>)) + return parameter.ParameterType.GenericTypeArguments.Single(); + + throw new ArgumentException($"Either {nameof(LoggerType)} must be set or the parameter type must be an {nameof(ILogger)}", nameof(parameter)); } } diff --git a/CSF.Extensions.WebDriver/Factories/DriverTypeProvider.cs b/CSF.Extensions.WebDriver/Factories/DriverTypeProvider.cs index 6b72e01..6e5eade 100644 --- a/CSF.Extensions.WebDriver/Factories/DriverTypeProvider.cs +++ b/CSF.Extensions.WebDriver/Factories/DriverTypeProvider.cs @@ -10,7 +10,7 @@ namespace CSF.Extensions.WebDriver.Factories /// public class DriverTypeProvider : IGetsDriverType { - readonly ILogger logger; + readonly ILogger logger; readonly IGetsWebDriverAndOptionsTypes typeProvider; /// @@ -50,12 +50,14 @@ public bool TryGetDriverType(WebDriverCreationOptions options, IConfigurationSec /// /// Initialises a new instance of . /// - /// A logger + /// A logger provider /// A provider for the concrete types of web driver and options /// If any parameter is - public DriverTypeProvider(ILogger logger, IGetsWebDriverAndOptionsTypes typeProvider) + public DriverTypeProvider(IGetsLogger loggerProvider, IGetsWebDriverAndOptionsTypes typeProvider) { - this.logger = logger ?? throw new ArgumentNullException(nameof(logger)); + if (loggerProvider is null) throw new ArgumentNullException(nameof(loggerProvider)); + + logger = loggerProvider.GetLogger(); this.typeProvider = typeProvider ?? throw new ArgumentNullException(nameof(typeProvider)); } } diff --git a/CSF.Extensions.WebDriver/Factories/OptionsTypeProvider.cs b/CSF.Extensions.WebDriver/Factories/OptionsTypeProvider.cs index afb2127..4347707 100644 --- a/CSF.Extensions.WebDriver/Factories/OptionsTypeProvider.cs +++ b/CSF.Extensions.WebDriver/Factories/OptionsTypeProvider.cs @@ -10,7 +10,7 @@ namespace CSF.Extensions.WebDriver.Factories /// public class OptionsTypeProvider : IGetsOptionsType { - readonly ILogger logger; + readonly ILogger logger; readonly IGetsWebDriverAndOptionsTypes typeProvider; readonly ICreatesDriverOptions optionsFactory; @@ -62,13 +62,15 @@ Func GetOptions(Type optionsType, IConfigurationSection config) /// /// Initialises a new instance of . /// - /// A logger + /// A logger provider /// A provider for the concrete types of web driver and options /// A factory for instances of /// If any parameter is - public OptionsTypeProvider(ILogger logger, IGetsWebDriverAndOptionsTypes typeProvider, ICreatesDriverOptions optionsFactory) + public OptionsTypeProvider(IGetsLogger loggerProvider, IGetsWebDriverAndOptionsTypes typeProvider, ICreatesDriverOptions optionsFactory) { - this.logger = logger ?? throw new ArgumentNullException(nameof(logger)); + if (loggerProvider is null) throw new ArgumentNullException(nameof(loggerProvider)); + + logger = loggerProvider.GetLogger(); this.typeProvider = typeProvider ?? throw new ArgumentNullException(nameof(typeProvider)); this.optionsFactory = optionsFactory ?? throw new ArgumentNullException(nameof(optionsFactory)); } diff --git a/CSF.Extensions.WebDriver/Factories/ProxyWrappingWebDriverFactoryDecorator.cs b/CSF.Extensions.WebDriver/Factories/ProxyWrappingWebDriverFactoryDecorator.cs index 5c311b8..d344383 100644 --- a/CSF.Extensions.WebDriver/Factories/ProxyWrappingWebDriverFactoryDecorator.cs +++ b/CSF.Extensions.WebDriver/Factories/ProxyWrappingWebDriverFactoryDecorator.cs @@ -42,15 +42,17 @@ public WebDriverAndOptions GetWebDriver(WebDriverCreationOptions options, Action /// /// The wrapped factory /// A proxy factory implementation - /// A logger + /// A logger provider /// If any parameter is . public ProxyWrappingWebDriverFactoryDecorator(ICreatesWebDriverFromOptions wrapped, IGetsProxyWebDriver proxyFactory, - ILogger logger) + IGetsLogger loggerProvider) { + if (loggerProvider is null) throw new ArgumentNullException(nameof(loggerProvider)); + + logger = loggerProvider.GetLogger(); this.wrapped = wrapped ?? throw new ArgumentNullException(nameof(wrapped)); this.proxyFactory = proxyFactory ?? throw new ArgumentNullException(nameof(proxyFactory)); - this.logger = logger ?? throw new ArgumentNullException(nameof(logger)); } } } \ No newline at end of file diff --git a/CSF.Extensions.WebDriver/Factories/RemoteWebDriverFromOptionsFactory.cs b/CSF.Extensions.WebDriver/Factories/RemoteWebDriverFromOptionsFactory.cs index 8984320..6ba3f8c 100644 --- a/CSF.Extensions.WebDriver/Factories/RemoteWebDriverFromOptionsFactory.cs +++ b/CSF.Extensions.WebDriver/Factories/RemoteWebDriverFromOptionsFactory.cs @@ -47,15 +47,17 @@ public WebDriverAndOptions GetWebDriver(WebDriverCreationOptions options, Action /// Initialises a new instance of . /// /// A type-providing service - /// A logger + /// A logger provider /// The next service in the chain of responsibility. /// If any parameters is . public RemoteWebDriverFromOptionsFactory(IGetsWebDriverAndOptionsTypes typeProvider, - ILogger logger, + IGetsLogger loggerProvider, ICreatesWebDriverFromOptions next) { + if (loggerProvider is null) throw new ArgumentNullException(nameof(loggerProvider)); + + logger = loggerProvider.GetLogger(); this.typeProvider = typeProvider ?? throw new ArgumentNullException(nameof(typeProvider)); - this.logger = logger ?? throw new ArgumentNullException(nameof(logger)); this.next = next ?? throw new ArgumentNullException(nameof(next)); } } diff --git a/CSF.Extensions.WebDriver/Factories/WebDriverConfigurationItemParser.cs b/CSF.Extensions.WebDriver/Factories/WebDriverConfigurationItemParser.cs index e07bedf..e54d518 100644 --- a/CSF.Extensions.WebDriver/Factories/WebDriverConfigurationItemParser.cs +++ b/CSF.Extensions.WebDriver/Factories/WebDriverConfigurationItemParser.cs @@ -13,7 +13,7 @@ public class WebDriverConfigurationItemParser : IParsesSingleWebDriverConfigurat readonly IGetsDriverType driverTypeProvider; readonly IGetsOptionsType optionsTypeProvider; readonly ICreatesDriverOptions optionsFactory; - readonly ILogger logger; + readonly ILogger logger; /// public WebDriverCreationOptions GetDriverConfiguration(IConfigurationSection configuration) @@ -86,16 +86,18 @@ static object GetOptionsCustomizer(Type optionsType, string customizerTypeName) /// A service to get the driver type /// A service to get the options type /// A service to get the driver options - /// The logger for this parser. + /// A logger provider public WebDriverConfigurationItemParser(IGetsDriverType driverTypeProvider, IGetsOptionsType optionsTypeProvider, ICreatesDriverOptions optionsFactory, - ILogger logger) + IGetsLogger loggerProvider) { + if (loggerProvider is null) throw new ArgumentNullException(nameof(loggerProvider)); + + logger = loggerProvider.GetLogger(); this.driverTypeProvider = driverTypeProvider ?? throw new ArgumentNullException(nameof(driverTypeProvider)); this.optionsTypeProvider = optionsTypeProvider ?? throw new ArgumentNullException(nameof(optionsTypeProvider)); this.optionsFactory = optionsFactory ?? throw new ArgumentNullException(nameof(optionsFactory)); - this.logger = logger ?? throw new ArgumentNullException(nameof(logger)); } } } diff --git a/CSF.Extensions.WebDriver/Factories/WebDriverCreationConfigureOptions.cs b/CSF.Extensions.WebDriver/Factories/WebDriverCreationConfigureOptions.cs index 07ab474..68faa7a 100644 --- a/CSF.Extensions.WebDriver/Factories/WebDriverCreationConfigureOptions.cs +++ b/CSF.Extensions.WebDriver/Factories/WebDriverCreationConfigureOptions.cs @@ -24,7 +24,7 @@ public sealed class WebDriverCreationConfigureOptions : IConfigureOptions logger; + readonly ILogger logger; readonly Action configureOptions; /// @@ -64,17 +64,19 @@ Dictionary GetDriverConfigurations(IConfigurat /// /// A parser for a single configuration item. /// The app configuration. - /// A logging implementation. + /// A logger provider /// An optional configuration action/callback to further configure the options /// If either parameter is . public WebDriverCreationConfigureOptions(IParsesSingleWebDriverConfigurationSection configParser, IConfiguration configuration, - ILogger logger, + IGetsLogger loggerProvider, Action configureOptions) { + if (loggerProvider is null) throw new ArgumentNullException(nameof(loggerProvider)); + + logger = loggerProvider.GetLogger(); this.configParser = configParser ?? throw new ArgumentNullException(nameof(configParser)); this.configuration = configuration; - this.logger = logger ?? throw new ArgumentNullException(nameof(logger)); this.configureOptions = configureOptions; } } diff --git a/CSF.Extensions.WebDriver/Factories/WebDriverFromOptionsFactory.cs b/CSF.Extensions.WebDriver/Factories/WebDriverFromOptionsFactory.cs index d5c2a9c..cdb169f 100644 --- a/CSF.Extensions.WebDriver/Factories/WebDriverFromOptionsFactory.cs +++ b/CSF.Extensions.WebDriver/Factories/WebDriverFromOptionsFactory.cs @@ -59,13 +59,15 @@ static void CuztomizeOptionsGeneric(TOptions options, object customize /// Initialises a new instance of . /// /// A type provider - /// A logger + /// A logger provider /// If any parameter is . public WebDriverFromOptionsFactory(IGetsWebDriverAndOptionsTypes typeProvider, - ILogger logger) + IGetsLogger loggerProvider) { + if (loggerProvider is null) throw new ArgumentNullException(nameof(loggerProvider)); + + logger = loggerProvider.GetLogger(); this.typeProvider = typeProvider ?? throw new ArgumentNullException(nameof(typeProvider)); - this.logger = logger ?? throw new ArgumentNullException(nameof(logger)); } } } \ No newline at end of file diff --git a/CSF.Extensions.WebDriver/Factories/WebDriverFromThirdPartyFactory.cs b/CSF.Extensions.WebDriver/Factories/WebDriverFromThirdPartyFactory.cs index ffe325d..6978cd7 100644 --- a/CSF.Extensions.WebDriver/Factories/WebDriverFromThirdPartyFactory.cs +++ b/CSF.Extensions.WebDriver/Factories/WebDriverFromThirdPartyFactory.cs @@ -54,17 +54,19 @@ ICreatesWebDriverFromOptions GetThirdPartyFactory(Type factoryType) /// The next implementation in the chain of responsibility. /// A type-providing service. /// The DI service provider. - /// A logger + /// A logger provider /// If any parameter is . public WebDriverFromThirdPartyFactory(ICreatesWebDriverFromOptions next, IGetsWebDriverAndOptionsTypes typeProvider, IServiceProvider services, - ILogger logger) + IGetsLogger loggerProvider) { + if (loggerProvider is null) throw new ArgumentNullException(nameof(loggerProvider)); + + logger = loggerProvider.GetLogger(); this.next = next ?? throw new ArgumentNullException(nameof(next)); this.typeProvider = typeProvider ?? throw new ArgumentNullException(nameof(typeProvider)); this.services = services ?? throw new ArgumentNullException(nameof(services)); - this.logger = logger ?? throw new ArgumentNullException(nameof(logger)); } } } \ No newline at end of file diff --git a/CSF.Extensions.WebDriver/IGetsLogger.cs b/CSF.Extensions.WebDriver/IGetsLogger.cs new file mode 100644 index 0000000..e76346f --- /dev/null +++ b/CSF.Extensions.WebDriver/IGetsLogger.cs @@ -0,0 +1,61 @@ + +using System; +using System.Collections.Generic; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; + +namespace CSF.Extensions.WebDriver +{ + /// + /// Small service which provides a method for obtaining loggers. + /// + /// + /// + /// This service is required in this library because we cannot use DI registration techniques to interrogate whether logging is + /// already registered into DI. This library is intended to be used alongside the BoDi DI container, which is part of the Reqnroll + /// BDD framework. In this scenario, BoDi is consumed via an adpater, to make it behave a little like . + /// + /// + /// Unfortunately, because of limitations in BoDi's design, this is far from perfect (an LSP violation) and the resulting service + /// collection throws from much of its functionality. One of these is the indexer and + /// implementation upon the service collection. Those functions are the only viable way to determine whether logging has already + /// been registered into DI (to avoid clobbering an existing DI registration), and they aren't available to us. + /// + /// + /// This interface and its internal visibility impl provide a workaround to that, which decides at resolution time whether a + /// logging implementation was formally registered into DI. If it was then it is resolved from the container as usual. + /// If it was not then a noop implementation of a logger is returned, to avoid throwing or returning . + /// + /// + public interface IGetsLogger + { + /// + /// Gets a logger for the specified type. + /// + /// The type which intends to record log messages. + /// A logger implementation + ILogger GetLogger(); + } + + /// + /// Internal-only implementation of . + /// + class LogProvider : IGetsLogger + { + readonly IServiceProvider services; + + /// + public ILogger GetLogger() => services.GetService>() ?? new NullLogger(); + + /// + /// Instantiates a new log provider instance. + /// + /// The service provider + /// If the service provider is + public LogProvider(IServiceProvider services) + { + this.services = services ?? throw new ArgumentNullException(nameof(services)); + } + } +} \ No newline at end of file diff --git a/CSF.Extensions.WebDriver/ServiceCollectionExtensions.cs b/CSF.Extensions.WebDriver/ServiceCollectionExtensions.cs index f576b08..c039bda 100644 --- a/CSF.Extensions.WebDriver/ServiceCollectionExtensions.cs +++ b/CSF.Extensions.WebDriver/ServiceCollectionExtensions.cs @@ -1,5 +1,4 @@ using System; -using System.Linq; using Castle.DynamicProxy; using CSF.Extensions.WebDriver.Factories; using CSF.Extensions.WebDriver.Identification; @@ -7,8 +6,6 @@ using CSF.Extensions.WebDriver.Quirks; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; namespace CSF.Extensions.WebDriver @@ -174,8 +171,7 @@ public static IServiceCollection AddWebDriverFactoryWithoutOptionsPattern(this I services.AddTransient(); services.AddTransient(); services.AddTransient(); - - AddLoggingIfNotAlreadyAdded(services); + services.AddTransient(); return services; } @@ -244,7 +240,7 @@ static Func().GetSection(configPath); return new WebDriverCreationConfigureOptions(services.GetRequiredService(), configSection, - services.GetRequiredService>(), + services.GetRequiredService(), configureOptions); }; } @@ -256,20 +252,11 @@ static Func(), configSection, - services.GetRequiredService>(), + services.GetRequiredService(), configureOptions); }; } - static void AddLoggingIfNotAlreadyAdded(IServiceCollection services) - { - if(services.Any(s => s.ServiceType == typeof(ILoggerFactory))) - return; - - services.AddTransient(); - services.AddTransient(typeof(ILogger<>), typeof(NullLogger<>)); - } - static void AddDriverOptionsFactory(IServiceCollection services) { services.AddTransient(); From e703cc50b34c0d867e24d44c1627bbb8eb5b8a84 Mon Sep 17 00:00:00 2001 From: Craig Fowler Date: Thu, 5 Nov 2026 21:17:36 +0000 Subject: [PATCH 2/2] Upgrade Java --- .github/workflows/dotnetCi.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/dotnetCi.yml b/.github/workflows/dotnetCi.yml index fee0793..3b113f9 100644 --- a/.github/workflows/dotnetCi.yml +++ b/.github/workflows/dotnetCi.yml @@ -56,6 +56,11 @@ jobs: run: dotnet tool install --global dotnet-sonarscanner - name: Install Coverlet console run: dotnet tool install --global coverlet.console --version 8.0.1 + - name: Install Java JDK for SonarScanner + uses: actions/setup-java@v5.1.0 + with: + java-version: 21 + distribution: zulu - name: Install DocFX run: dotnet tool install --global docfx # See https://chromium.googlesource.com/chromium/src/+/main/docs/security/apparmor-userns-restrictions.md