diff --git a/CSF.Screenplay.Docs/docs/gettingStarted/reqnroll/BoDiWorkarounds.md b/CSF.Screenplay.Docs/docs/gettingStarted/reqnroll/BoDiWorkarounds.md new file mode 100644 index 00000000..d29268e2 --- /dev/null +++ b/CSF.Screenplay.Docs/docs/gettingStarted/reqnroll/BoDiWorkarounds.md @@ -0,0 +1,54 @@ +--- +uid: BoDiWorkaroundsArticle +--- + +# Workarounds for the BoDi DI container + +Reqnroll (and SpecFlow before it) use a custom dependency injection container named BoDi. +It does not implement the complete set of functionality of `IServiceCollection` or `IServiceProvider` and what's it does implement differs from them in some important ways. +However, the .NET Dependency Injection ecosystem fully expects the pair of `IServiceCollection` and `IServiceProvider`. +Many libraries expose extension methods for `IServiceCollection` which use that API to add themselves to a container. + +In an attempt to close this gap, Screenplay offers two limited **adapters** which allow some usage of BoDi's `IObjectContainer` as a service collection and as a service provider. +These are accessible via the extension methods [`ToServiceCollection`] and [`ToServiceProvider`]. +_Be warned that these are known to be imperfect_; they suffer from multiple limitations and will not work in all use-cases. +If you encounter showstopper problems using BoDi, _do not forget that Reqnroll provides support for swapping its DI container_ with another. +[This article shows you how to replace the BoDi container with Autofac], for example. + +The remainder of this page lists some of the particular difficulties and workarounds which have solutions and/or further documentation. + +[`ToServiceCollection`]: xref:CSF.Screenplay.ObjectContainerExtensions.ToServiceCollection(Reqnroll.BoDi.IObjectContainer) +[`ToServiceProvider`]: xref:CSF.Screenplay.ObjectContainerExtensions.ToServiceProvider(Reqnroll.BoDi.IObjectContainer) +[This article shows you how to replace the BoDi container with Autofac]: https://docs.reqnroll.net/latest/integrations/autofac.html + +## BoDi cannot natively resolve `IEnumerable` of registered services + +Say you were to separately register (add) three different implementations of `IMyCustomService` to a normal `ServiceCollection` instance. If you subsequently resolve `IEnumerable` from the resultant service provider then you would get a collection of custom service instances containing three items, one of each of the three implementations. This is built-in functionality for the standard Microsoft Service Collection. + +_BoDi cannot do this without some help_. The Screenplay Reqnroll plugin includes that help in the form of an extension method for `IServiceCollection`: [`AddEnumerableAdapter()`]. Use this method to enable enumerable resolution (as described above) for the specified generic type. You must use this method for each type T of `IEnumerable` your tests need to be capable of resolving. +There are further technical details about this limitation and the workaround available in the remarks for [`EnumerableResolutionAdapter`]. + +[`AddEnumerableAdapter()`]: xref:CSF.Screenplay.ServiceCollectionExtensions.AddEnumerableAdapter``1(Microsoft.Extensions.DependencyInjection.IServiceCollection) +[`EnumerableResolutionAdapter`]: xref:CSF.Screenplay.EnumerableResolutionAdapter`1 + +## BoDi does not natively support the Microsoft Options Pattern + +This is only relevant if you intend to make use of the [Microsoft Options Pattern] in your tests. The Selenium plugin does (via [CSF.WebDriver.Extensions]) and if using it, you must include `.AddOptionsAdapters()` in your DI setup. + +This limitation is a specific case of BoDi's inability to resolve enumerables of registered services (above). There is an extension method specifically crafted for this: [`AddOptionsAdapters()`]. This method must be used once for each type of TOptions you wish to use. It puts adapters/workarounds in place, to allow resolution of (for example) `IOptions` in your tests. + +[Microsoft Options Pattern]: https://learn.microsoft.com/en-us/dotnet/core/extensions/options +[CSF.WebDriver.Extensions]: https://github.com/csf-dev/CSF.Extensions.WebDriver +[`AddOptionsAdapters()`]: xref:CSF.Screenplay.ServiceCollectionExtensions.AddOptionsAdapters``1(Microsoft.Extensions.DependencyInjection.IServiceCollection) + +## Liskov Substitution Principle violations + +The adapters for using BoDi as an `IServiceCollection` and `IServiceProvider` are imperfect and limited. +In addition, they do not provide the complete range of functionality which is expected by the contract of these two interfaces. +This is [a violation of LSP]. + +There is extensive documentation about the differences in implementation details in the remarks for the classes [`ServiceCollectionAdapter`] and [`ServiceProviderAdapter`]. + +[a violation of LSP]: https://en.wikipedia.org/wiki/Liskov_substitution_principle +[`ServiceCollectionAdapter`]: xref:CSF.Screenplay.ServiceCollectionAdapter +[`ServiceProviderAdapter`]: xref:CSF.Screenplay.ServiceProviderAdapter diff --git a/CSF.Screenplay.Docs/docs/gettingStarted/reqnroll/index.md b/CSF.Screenplay.Docs/docs/gettingStarted/reqnroll/index.md index b53793c4..c3638e2a 100644 --- a/CSF.Screenplay.Docs/docs/gettingStarted/reqnroll/index.md +++ b/CSF.Screenplay.Docs/docs/gettingStarted/reqnroll/index.md @@ -24,6 +24,9 @@ Further detail is provided below. Reqnroll has a built-in mechanism which offers an opportunity to add services to its DI container; these are [the event hook attributes]. This is a viable technique because [Reqnroll's internal DI container "BoDi"] is unusual in that it permits retrospective addition of services. +> [!IMPORTANT] +> The Reqnroll BoDi DI container has a number of [quirks and limitations which require workarounds]. Read the linked documentation page for more info. + Use this technique by writing a small binding class like the following, replacing the comment with your DI registrations. There's no need to use `.AddScreenplay()` here, _the integration does that already_. For example, to add [the Screenplay/Selenium extension], add a line like `.AddSelenium()`. @@ -51,6 +54,7 @@ public class DependenciesSetup(IObjectContainer reqnrollContainer) [the event hook attributes]: https://docs.reqnroll.net/latest/automation/context-injection.html#advanced-options [Reqnroll's internal DI container "BoDi"]: https://github.com/reqnroll/Reqnroll/tree/main/Reqnroll/BoDi [the Screenplay/Selenium extension]: ../../extensions/selenium/index.md +[quirks and limitations which require workarounds]: BoDiWorkarounds.md ## Step 4: Writing step bindings diff --git a/CSF.Screenplay.Reqnroll/EnumerableResolutionAdapter.cs b/CSF.Screenplay.Reqnroll/EnumerableResolutionAdapter.cs index 3aff893a..1db3f814 100644 --- a/CSF.Screenplay.Reqnroll/EnumerableResolutionAdapter.cs +++ b/CSF.Screenplay.Reqnroll/EnumerableResolutionAdapter.cs @@ -30,7 +30,19 @@ namespace CSF.Screenplay /// The limitation of this type (as a workaround) is that this type must be added to the container manually for each /// type which could be resolved from the container. /// + /// + /// A developer might ask: Why not detect attempts to resolve IEnumerable<T> in , why do + /// we need this class? That is because whilst BoDi provides a Resolve All method, it does not use it internally for resolving generic enumerables. + /// That is, the internal resolution pipeline does not detect attempts to resolve and direct them to the resolve-all + /// functionality. Instead they would fail with on the basis of "cannot resolve an interface without a + /// registration". + /// This would mean, if we attempted to isolate this functionality into the service provider adapter we would be able to resolve generic enumerables + /// of services OK, but we would not be able to resolve anything which depended upon generic enumerables of services. + /// Registering this adapter class (adding it to the container) for the appropriate enumerable type forces that 'redirect away to the resolve-all + /// functionality' behaviour into the resolution pipeline. + /// /// + /// The type of enumerable service which this adapter covers public class EnumerableResolutionAdapter : IEnumerable where T : class { readonly IObjectContainer container; diff --git a/CSF.Screenplay.Reqnroll/ObjectContainerExtensions.cs b/CSF.Screenplay.Reqnroll/ObjectContainerExtensions.cs index b1453e7c..5b543ad2 100644 --- a/CSF.Screenplay.Reqnroll/ObjectContainerExtensions.cs +++ b/CSF.Screenplay.Reqnroll/ObjectContainerExtensions.cs @@ -19,14 +19,16 @@ public static class ObjectContainerExtensions /// /// /// - /// Note that this is an imperfect solution. The BoDi container shipped with Reqnroll/SpecFlow does not support all the functionality - /// which is expected from . Many methods of the returned object will throw - /// if attempts are made to use them (a known LSP violation). Additionally, not all service collection DI behaviour will operate in the same - /// manner when using this adapter. In short "your mileage may vary". + /// Note that this is an imperfect conversion and suffers from numerous + /// Liskov Substitution Principle violations. + /// In other words, the BoDi container used by Reqnroll/SpecFlow does not support the API defined for . + /// Using a BoDi container as a Service Collection requires workarounds for common scenarios and and comes with several + /// limitations which have no workaround. /// /// - /// However, for the most simple of usages, this enables the use of "Add to DI" logic which has been crafted for service collection, - /// in such a way that services may be added to the BoDi container without additional logic. + /// For more information, including suggestions to mitigate or avoid this, see + /// and the documentation for the class + /// . /// /// /// A Reqnroll/SpecFlow BoDi DI container. @@ -38,6 +40,20 @@ public static IServiceCollection ToServiceCollection(this IObjectContainer bodiC /// Gets an adapter object which permits the use of the BoDi as if it were an /// . /// + /// + /// + /// Note that this is an imperfect conversion and suffers from a + /// Liskov Substitution Principle violation. + /// In other words, the BoDi container used by Reqnroll/SpecFlow does not support the API defined for . + /// Using a BoDi container as a Service Provider requires workarounds for common scenarios and and comes with some + /// limitations which have no workaround. + /// + /// + /// For more information, including suggestions to mitigate or avoid this, see + /// and the documentation for the class + /// . + /// + /// /// A Reqnroll/SpecFlow BoDi DI container. /// An adapter object which implements the functionality of public static IServiceProvider ToServiceProvider(this IObjectContainer bodiContainer) diff --git a/CSF.Screenplay.Reqnroll/ServiceCollectionAdapter.cs b/CSF.Screenplay.Reqnroll/ServiceCollectionAdapter.cs index 6e7d8d64..d42add4e 100644 --- a/CSF.Screenplay.Reqnroll/ServiceCollectionAdapter.cs +++ b/CSF.Screenplay.Reqnroll/ServiceCollectionAdapter.cs @@ -20,10 +20,25 @@ namespace CSF.Screenplay /// many methods, and the indexer for , it will raise . /// The only functionality supported is and . /// + /// + /// Obviously, this is a serious Liskov Substitution Principle violation! + /// Some of these limitations , but some might have no workaround available. + /// This adapter class has been tested and found to be usable in simple testing scenarios though, which is why it is still provided. + /// It is a low-cost, low-effort workaround. If you find this insufficient then consider replacing Reqnroll's DI container. + /// An example of how to do this, using the Autofac container, is documented + /// on Reqnroll's website. + /// + /// + /// An additional limitation of this adapter is that adding multiple implementations of the same base type (for example, many classes implementing the same interface) + /// does not allow a consumer to resolve of that base type from the resulting service provider. + /// Whilst the Microsoft service collection/provider automatically interpret an attempt to resolve IEnumerable<T> as resolve every implementation of T, + /// BoDi does not. Use the extension method to enable resolution of enumerable + /// collections of service types. + /// /// public class ServiceCollectionAdapter : IServiceCollection { - const string NotSupportedMessage = "This method is not supported for ServiceCollectionAdapter; a BoDi object container is not a true service collection."; + const string NotSupportedMessage = "This is not supported for ServiceCollectionAdapter; a BoDi object container is not a true service collection."; // Suppress SonarQube issue S3011, I'm using BindingFlags.NonPublic here but only from internally within the same class. // The technique calling for reflection is to make an open generic method runtime-generic instead of compile-time. It's @@ -44,32 +59,33 @@ static readonly MethodInfo /// Always thrown public ServiceDescriptor this[int index] { - get => throw new NotSupportedException("This indexer is not supported for this adapter."); - set => throw new NotSupportedException("This indexer is not supported for this adapter."); + get => throw new NotSupportedException(NotSupportedMessage); + set => throw new NotSupportedException(NotSupportedMessage); } /// - /// Not supported, always returns zero. + /// Unsupported, this property always returns zero. /// public int Count => default; /// - /// Partially supported, always returns + /// Partially supported, always returns . /// public bool IsReadOnly => false; /// - /// Partially-supported, will add the specified service descriptor to the current object container. + /// Adds the specified service descriptor to the current object container. /// /// /// - /// This method supports only transient or singleton services. If the present on the item specified - /// is then the service descriptor will not be added and will be silently ignored. + /// This method supports only transient or singleton services. If the present on the item specified + /// is then the service descriptor will not be added and will be silently ignored. /// /// - /// In reality, the Reqnroll BoDi object container only really supports singleton services at this level. So, any services added as - /// will actually become singletons here. Whilst the BoDi container does support scoped services, they - /// must be added directly to the scope instance and cannot be added in advance. + /// In reality, the Reqnroll BoDi object container only really supports singleton services at this level. So, any services added as + /// will actually become singletons here. Whilst the BoDi container does support scoped services in a + /// manner of speaking, they must be added directly to the container from that scope. Services which are scoped must be added (as singletons) + /// directly into the scope; they cannot be added in advance. /// /// /// The service descriptor @@ -110,7 +126,7 @@ void RegisterFactory(ServiceDescriptor item) where T : class /// Not supported; always throws . /// /// Always thrown - public void Clear() => throw new NotSupportedException(); + public void Clear() => throw new NotSupportedException(NotSupportedMessage); /// /// Not supported; always throws . diff --git a/CSF.Screenplay.Reqnroll/ServiceProviderAdapter.cs b/CSF.Screenplay.Reqnroll/ServiceProviderAdapter.cs index f7e50b4c..c8b62b6f 100644 --- a/CSF.Screenplay.Reqnroll/ServiceProviderAdapter.cs +++ b/CSF.Screenplay.Reqnroll/ServiceProviderAdapter.cs @@ -13,6 +13,19 @@ namespace CSF.Screenplay /// /// Adapter class which allows a Reqnroll/BoDi IObjectContainer to be used as an . /// + /// + /// + /// This adapter class is limited and does not fully support the contract required by , despite being a one-method interface. + /// See the remarks on the method for more information about this limitation. + /// + /// + /// This is a Liskov Substitution Principle violation and it is recognised that this adapter + /// is of limited use. It is provided as a low-cost, low-effort workaround to making the BoDi DI container work alongside Microsoft Dependency Injection patterns. + /// If you find this insufficient then consider replacing Reqnroll's DI container. + /// An example of how to do this, using the Autofac container, is documented + /// on Reqnroll's website. + /// + /// public class ServiceProviderAdapter : IServiceProvider { readonly IObjectContainer wrapped; @@ -20,30 +33,45 @@ public class ServiceProviderAdapter : IServiceProvider /// /// /// - /// The design of BoDi means that the ObjectContainer.IsRegistered method (and variants) are unsafe to use here. - /// This means that we cannot reliably check whether a service type is already available in the container in a side-effect free manner. - /// This is because when using IsRegistered or its other overloads, can return for types which BoDi was capable - /// of resolving, due to its registration-free resolution functionality. That is - when resolving concrete types which are not registered, it - /// will do its best to create an instance regardless. - /// Thus, whilst IsRegistered is correctly named, it is not what we want here. What we wanted is "Can you resolve type X?" + /// This method includes two major departures from the contract which is expected from . /// + /// + /// This method should return a non-null resolved instance of the type requested by , + /// if the service was registered with (added to) the container. On the other hand, if the service was not registered with (added to) the container then this + /// method should return without throwing an exception. + /// This method should throw an exception if the service requested was registered with (added to) the container, but an exception + /// was encountered whilst resolving it. + /// /// - /// The alternative, used here, is to attempt to resolve the type, but contained within a try/catch which catched BoDi's - /// , which is thrown if BoDi cannot resolve the specified type. At that point, if it can't be resolved, - /// then we use the catch to revert to the behaviour, which is to return for a service - /// which cannot be resolved. + /// Unfortunately, this implementation does not quite behave that way. The BoDi container includes auto resolution of types which have not been registered + /// with (added to) the container. That is - in fact - a desired & intended feature for Reqnroll. The most common expected kind of resolved object within + /// a Reqnroll test is a simple model object which holds state for the duration of a scenario/feature. These are simply constructed and do not require prior + /// registration with the container. + /// A side-effect of that functionality is that attempting to resolve an object which is not registered - if that object is not + /// (or an ) - will result in an auto-resolution attempt. Thus this method could return a non-null object regardless. /// /// - /// Beware when debugging that this comes with an unfortunate trade-off! If BoDi encounters an error whilst resolving something, then we can't tell at this - /// point whether that was an expected or unexpected error. An example of an expected error would be that we're trying to resolve an interface which - /// is not registered. An unexpected error might be an exception encountered within the constructor of something we are resolving. - /// Because the contract of is to return null when attempting to resolve things which are not registered, then we must not - /// throw for those expected errors. The dilemma this offers us (because we can't tell expected and unexpected apart) is that we either have to throw for - /// every problem, or never throw. - /// I have picked the "never throw" route, but the trade-off is that this logic masks unexpected errors. That will make debugging more difficult, because unexpected - /// exceptions from BoDi will be squashed and replaced with null returns. What I have done to make that less painful is to integrate with MS Logging Abstractions here. - /// If you install and register a logging abstraction within DI, then an implementation of will be resolved and the original error - /// recorded in the log. + /// Note to developers: It has been considered to use ObjectContainer.IsRegistered here but unfortunately that does not tell us what we need to know. + /// This tells us only if the type is registered within the current container instance, not whether it could be resolved successfully. + /// Thus a process along the lines of "Is it registered ? If it is then resolve it, if not return null" would introduce false failures to resolve anything which + /// is expected to be auto-resolved (the inverse problem). + /// This means that this method always goes directly to . The consequence being + /// that it might cause an unwanted auto-resolution to occur where the developer would have wanted it to return null. + /// + /// + /// The other limitation is the BoDi uses the same exception type for some very different scenarios. is thrown if an attempt + /// is made to resolve an abstract/interface type which is not registered. It is thrown for circular dependencies and it is thrown when auto resolution fails + /// because a class' constructors are incompatible with that technique. + /// Notice that first scenario in which an attempt is made to resolve a service which is not registered. This is the prime scenario in which a service provider should + /// return null without throwing. Thus, this method uses a try/catch to intercept and convert them to null returns. + /// Unfortunately this means that other scenarios in which an object container exception is thrown will result in silently swallowing the exception and returning + /// null. This will make things like circular dependencies and auto-resolution failures difficult to diagnose, because they will not throw when the developer expects! + /// + /// + /// Developers should beware of this when debugging! + /// If you are able, add a logging package to DI which can provide an . If a logger is present in dependency injection and an + /// is caught then the full caught exception will be recorded to the log at the warning level. + /// This way, when attempting to diagnose unexpected behaviour, developers may at least read the log to find out what happened. /// /// public object GetService(Type serviceType) @@ -55,7 +83,7 @@ public object GetService(Type serviceType) catch(ObjectContainerException ex) { if(TryGetLogger(out var logger)) - logger.LogError(ex, "An error occured within BoDi's DI resolution. As we cannot differentiate between expected and unexpected errors, the service provider adapter logs it and returns null."); + logger.LogWarning(ex, "An error occured within BoDi's DI resolution. As we cannot differentiate between expected and unexpected errors, the service provider adapter logs it and returns null."); return null; } }