diff --git a/src/Lamar.Testing/Bugs/Bug_descriptor_lifetime_for_fluent_use.cs b/src/Lamar.Testing/Bugs/Bug_descriptor_lifetime_for_fluent_use.cs new file mode 100644 index 00000000..12685f51 --- /dev/null +++ b/src/Lamar.Testing/Bugs/Bug_descriptor_lifetime_for_fluent_use.cs @@ -0,0 +1,126 @@ +using System.Linq; +using Microsoft.Extensions.DependencyInjection; +using Shouldly; +using Xunit; + +namespace Lamar.Testing.Bugs; + +// Repro for a bridge issue between Lamar's fluent registration API and the +// IServiceCollection snapshot Lamar exposes (since ServiceRegistry : IServiceCollection). +// +// When a Lamar Instance is added to the registry via the extension at +// src/Lamar/Scanning/Conventions/ServiceCollectionExtensions.cs:42-45: +// +// public static void Add(this IServiceCollection services, Instance instance) +// { +// services.Add(new ServiceDescriptor(instance.ServiceType, instance)); +// } +// +// the descriptor constructor used is ServiceDescriptor(Type, object) which +// hard-codes ServiceLifetime.Singleton (it's the only MS-DI overload that +// accepts a pre-existing instance). So every For().Use() / +// For().Use(factory) / For().Add() / ...Scoped() / ...Transient() +// call writes a ServiceDescriptor whose Lifetime is Singleton, regardless of +// what the underlying Lamar Instance says. +// +// Lamar itself still honors Instance.Lifetime when resolving, so apps that +// only ever go through Container.GetInstance() never see this. But anything +// that reads the IServiceCollection snapshot directly (e.g. Wolverine 5+'s +// HTTP endpoint code-gen, JasperFx's ServiceCollectionServerVariableSource) +// sees Singleton and treats the registration accordingly. That's the +// "Use existing HttpContext ServiceProvider for opaque scoped services" +// scenario tracked at JasperFx/wolverine#1610 — for users of Lamar via +// Wolverine HTTP it manifests as captive dependencies: a single resolved +// instance baked into a singleton-lifetime generated handler class. +// +// This file demonstrates the underlying mismatch as plain xUnit tests against +// Lamar alone, no Wolverine reference needed. +public interface ILifetimeBridgeThing { } +public class LifetimeBridgeThing : ILifetimeBridgeThing { } + +public class Bug_descriptor_lifetime_for_fluent_use +{ + // Convention-scan registrations (WithDefaultConventions()) are NOT affected: + // DefaultConventionScanner.ScanTypes calls the (serviceType, implType, lifetime) + // descriptor ctor directly at DefaultConventionScanner.cs:28, so the descriptor + // carries an honest lifetime. The bug below is specific to the fluent API paths + // that route through the Add(IServiceCollection, Instance) extension. + + // ------- Fluent paths: descriptor.Lifetime is silently Singleton ---------- + + [Fact] + public void For_Use_writes_Transient_lifetime_to_descriptor() + { + // ServiceRegistry.For() returns an InstanceExpression with + // ServiceLifetime.Transient as its default (ServiceRegistry.cs:96). + // The Instance Lamar adds also has Lifetime = Transient. But the + // ServiceDescriptor surfaced to IServiceCollection consumers will + // be Singleton. + var registry = new ServiceRegistry(); + registry.For().Use(); + + var descriptor = registry.LastOrDefault(d => d.ServiceType == typeof(ILifetimeBridgeThing)); + descriptor.ShouldNotBeNull(); + descriptor.Lifetime.ShouldBe(ServiceLifetime.Transient); + } + + [Fact] + public void For_Use_Scoped_writes_Scoped_lifetime_to_descriptor() + { + // .Scoped() updates instance.Lifetime to Scoped (see + // InstanceExtensions.Scoped in ServiceRegistry.cs:23). The descriptor + // still ends up Singleton. + var registry = new ServiceRegistry(); + registry.For().Use().Scoped(); + + var descriptor = registry.LastOrDefault(d => d.ServiceType == typeof(ILifetimeBridgeThing)); + descriptor.ShouldNotBeNull(); + descriptor.Lifetime.ShouldBe(ServiceLifetime.Scoped); + } + + [Fact] + public void For_Use_factory_Scoped_writes_Scoped_lifetime_to_descriptor() + { + // The lambda-factory overload — same bug. This is the exact shape + // that AMI uses to register IAsyncDocumentSession against RavenDB + // and that triggers captive sessions in Wolverine HTTP handlers. + var registry = new ServiceRegistry(); + registry.For().Use(_ => new LifetimeBridgeThing()).Scoped(); + + var descriptor = registry.LastOrDefault(d => d.ServiceType == typeof(ILifetimeBridgeThing)); + descriptor.ShouldNotBeNull(); + descriptor.Lifetime.ShouldBe(ServiceLifetime.Scoped); + } + + [Fact] + public void For_Add_writes_Transient_lifetime_to_descriptor() + { + // The multi-impl pipeline form. Same bridge path. + var registry = new ServiceRegistry(); + registry.For().Add(); + + var descriptor = registry.LastOrDefault(d => d.ServiceType == typeof(ILifetimeBridgeThing)); + descriptor.ShouldNotBeNull(); + descriptor.Lifetime.ShouldBe(ServiceLifetime.Transient); + } + + // ------- Sanity: container resolution honours intent (the consolation) --- + + [Fact] + public void Container_resolution_still_honours_intended_lifetime() + { + // Lamar's own container plan uses Instance.Lifetime correctly, so this + // passes today. Documenting it to make clear the bug is purely at the + // IServiceCollection-snapshot layer. + var container = new Container(x => x.For().Use().Scoped()); + + using var scope1 = container.GetNestedContainer(); + using var scope2 = container.GetNestedContainer(); + var a1 = scope1.GetInstance(); + var a2 = scope1.GetInstance(); + var b1 = scope2.GetInstance(); + + a1.ShouldBeSameAs(a2); // same scope -> same instance + a1.ShouldNotBeSameAs(b1); // different scope -> different instance + } +} diff --git a/src/Lamar.Testing/IoC/ServiceRegistryTests.cs b/src/Lamar.Testing/IoC/ServiceRegistryTests.cs index ee0589cf..ed4ced89 100644 --- a/src/Lamar.Testing/IoC/ServiceRegistryTests.cs +++ b/src/Lamar.Testing/IoC/ServiceRegistryTests.cs @@ -4,6 +4,7 @@ using Lamar.IoC; using Lamar.IoC.Frames; using Lamar.IoC.Instances; +using Lamar.Scanning.Conventions; using Microsoft.Extensions.DependencyInjection; using Shouldly; using StructureMap.Testing.Widget; @@ -22,7 +23,7 @@ public void can_add_custom_instance() registry.For().Use(custom); - registry.Single().ImplementationInstance.ShouldBe(custom); + registry.Single().LamarInstance().ShouldBe(custom); } } diff --git a/src/Lamar.Testing/Scanning/Conventions/ServiceCollectionExtensionsTests.cs b/src/Lamar.Testing/Scanning/Conventions/ServiceCollectionExtensionsTests.cs index 369f9a1b..9ebb09ec 100644 --- a/src/Lamar.Testing/Scanning/Conventions/ServiceCollectionExtensionsTests.cs +++ b/src/Lamar.Testing/Scanning/Conventions/ServiceCollectionExtensionsTests.cs @@ -29,7 +29,7 @@ public void add_type_on_an_empty_set() var widgetDescriptor = services.Single(x => x.ServiceType == typeof(IWidget)); widgetDescriptor.ServiceType.ShouldBe(typeof(IWidget)); - widgetDescriptor.ImplementationInstance.As().ImplementationType.ShouldBe(typeof(AWidget)); + widgetDescriptor.LamarInstance().ImplementationType.ShouldBe(typeof(AWidget)); } [Fact] @@ -42,7 +42,7 @@ public void add_type_on_new_implementation_type() services.AddType(typeof(IWidget), typeof(MoneyWidget)); services.Where(x => x.ServiceType == typeof(IWidget)) - .Select(x => x.ImplementationInstance).OfType().Select(x => x.ImplementationType) + .Select(x => x.LamarInstance()).Where(x => x != null).Select(x => x.ImplementationType) .ShouldHaveTheSameElementsAs(typeof(AWidget), typeof(MoneyWidget)); } @@ -58,7 +58,7 @@ public void do_not_add_twice() services.AddType(typeof(IWidget), typeof(MoneyWidget)); services.Where(x => x.ServiceType == typeof(IWidget)) - .Select(x => x.ImplementationInstance).OfType().Select(x => x.ImplementationType) + .Select(x => x.LamarInstance()).Where(x => x != null).Select(x => x.ImplementationType) .ShouldHaveTheSameElementsAs(typeof(AWidget), typeof(MoneyWidget)); } } \ No newline at end of file diff --git a/src/Lamar.Testing/ServiceRegistryTester.cs b/src/Lamar.Testing/ServiceRegistryTester.cs index 295f193c..e858dcce 100644 --- a/src/Lamar.Testing/ServiceRegistryTester.cs +++ b/src/Lamar.Testing/ServiceRegistryTester.cs @@ -1,5 +1,6 @@ using System.Linq; using Lamar.IoC.Instances; +using Lamar.Scanning.Conventions; using Microsoft.Extensions.DependencyInjection; using Shouldly; using StructureMap.Testing.Widget; @@ -17,7 +18,7 @@ public void for_use() var descriptor = registry.Single(); - var instance = descriptor.ImplementationInstance.ShouldBeOfType>(); + var instance = descriptor.LamarInstance().ShouldBeOfType>(); instance.ImplementationType.ShouldBe(typeof(AWidget)); descriptor.ServiceType.ShouldBe(typeof(IWidget)); @@ -31,7 +32,7 @@ public void forsingleton_use() var descriptor = registry.Single(); - var instance = descriptor.ImplementationInstance.ShouldBeOfType>(); + var instance = descriptor.LamarInstance().ShouldBeOfType>(); instance.ImplementationType.ShouldBe(typeof(AWidget)); instance.Lifetime.ShouldBe(ServiceLifetime.Singleton); diff --git a/src/Lamar/IoC/Instances/Instance.cs b/src/Lamar/IoC/Instances/Instance.cs index def41677..4157d8b5 100644 --- a/src/Lamar/IoC/Instances/Instance.cs +++ b/src/Lamar/IoC/Instances/Instance.cs @@ -7,6 +7,7 @@ using JasperFx.Core; using JasperFx.Core.Reflection; using Lamar.IoC.Frames; +using Lamar.Scanning.Conventions; using Microsoft.Extensions.DependencyInjection; namespace Lamar.IoC.Instances; @@ -34,13 +35,40 @@ protected Instance(Type serviceType, Type implementationType, ServiceLifetime li public bool IsExplicitlyNamed { get; set; } + // Set when ServiceCollectionExtensions.Add wraps the Instance in a + // LamarServiceDescriptor. Together they let the Lifetime setter (below) keep the + // descriptor's surface-area Lifetime in sync with this Instance — needed because + // the MS-DI ServiceDescriptor is immutable, so any Lifetime change requires + // replacing the descriptor in the registry. + internal IServiceCollection ServiceCollection { get; set; } + internal LamarServiceDescriptor LamarDescriptor { get; set; } + public Type ServiceType { get; } public Type ImplementationType { get; } public int Hash { get; set; } - public ServiceLifetime Lifetime { get; set; } = ServiceLifetime.Transient; + private ServiceLifetime _lifetime = ServiceLifetime.Transient; + + /// + /// Intended lifetime of this Instance. Mutating this value also refreshes + /// the backing LamarServiceDescriptor in the registry so IServiceCollection + /// consumers (Wolverine code-gen, MS-DI validation, etc.) see the current + /// lifetime. All Lifetime mutations — fluent .Scoped()/.Transient()/.Singleton(), + /// ReferencedInstance.createPlan's Lifetime = _inner.Lifetime, internal + /// adjustments — route through this single setter. + /// + public ServiceLifetime Lifetime + { + get => _lifetime; + set + { + if (_lifetime == value) return; + _lifetime = value; + this.ReplaceLamarDescriptor(); + } + } public string Name { @@ -147,9 +175,10 @@ public static Instance For(ServiceDescriptor service) #endif - if (service.ImplementationInstance is Instance i) + var lamarInstance = service.LamarInstance(); + if (lamarInstance != null) { - return i; + return lamarInstance; } if (service.ImplementationInstance != null) diff --git a/src/Lamar/LamarServiceDescriptor.cs b/src/Lamar/LamarServiceDescriptor.cs new file mode 100644 index 00000000..d5f12d78 --- /dev/null +++ b/src/Lamar/LamarServiceDescriptor.cs @@ -0,0 +1,51 @@ +using System; +using Lamar.IoC.Instances; +using Microsoft.Extensions.DependencyInjection; + +namespace Lamar; + +/// +/// ServiceDescriptor variant that carries a Lamar Instance alongside its +/// intended ServiceLifetime. This solves a long-standing mismatch: the +/// stock ServiceDescriptor(Type, object) constructor — which is the only +/// MS-DI overload that accepts a pre-existing object payload — hard-codes +/// Lifetime = Singleton, forcing every fluent registration (For().Use(), +/// .Use(factory), .Add(), and their .Scoped()/.Transient() variants) +/// to appear as a Singleton to anything inspecting the IServiceCollection +/// snapshot. That's the root cause behind Wolverine HTTP code-gen capturing +/// resolved Lamar registrations as singleton fields on generated handler +/// classes; see JasperFx/wolverine#1610 and #537 for visible symptoms. +/// +/// Descriptor shape: we delegate to the (Type, Func, ServiceLifetime) base +/// constructor, which makes the descriptor look like an "opaque lambda +/// factory" to external consumers. This is the shape Wolverine's docs say +/// triggers service-location at runtime — exactly what we want, because +/// service location calls back into Lamar's IServiceProvider, which can +/// see scan-only deps (whereas Wolverine's inline-construction codegen +/// cannot). The factory delegate itself is never invoked under Lamar +/// resolution: Lamar's checks for the subclass +/// first and returns the typed Instance directly, bypassing the factory +/// branch. The factory body throws to make any incorrect direct invocation +/// loud and obvious. +/// +public sealed class LamarServiceDescriptor : ServiceDescriptor +{ + public LamarServiceDescriptor(Instance instance) + : base( + instance.ServiceType, + FactoryShouldNotBeInvoked, + instance.Lifetime) + { + Instance = instance; + } + + public Instance Instance { get; } + + private static object FactoryShouldNotBeInvoked(IServiceProvider sp) => + throw new InvalidOperationException( + "LamarServiceDescriptor's ImplementationFactory is a Wolverine/MS-DI codegen marker " + + "and should not be invoked directly. Resolution under Lamar goes through Instance.For " + + "(which prefers the typed Instance on the subclass) and never through this delegate. " + + "If you see this, you are resolving the descriptor outside a Lamar container — build " + + "the container via UseLamar(...) instead of stock MS-DI ServiceProvider."); +} diff --git a/src/Lamar/Scanning/Conventions/ServiceCollectionExtensions.cs b/src/Lamar/Scanning/Conventions/ServiceCollectionExtensions.cs index 954c9892..229b7e69 100644 --- a/src/Lamar/Scanning/Conventions/ServiceCollectionExtensions.cs +++ b/src/Lamar/Scanning/Conventions/ServiceCollectionExtensions.cs @@ -35,13 +35,60 @@ public static ConnectedConcretions ConnectedConcretions(this IServiceCollection } /// - /// Add a registration via Lamar's intrinsic Instance type + /// Add a registration via Lamar's intrinsic Instance type. + /// + /// Lamar smuggles its rich Instance metadata through ServiceDescriptor by + /// placing the Instance object in the ImplementationInstance slot. The + /// built-in MS-DI ServiceDescriptor(Type, object) constructor forces + /// Lifetime = Singleton, which lies to any IServiceCollection consumer + /// (Wolverine code-gen in particular) that reads descriptor.Lifetime to + /// decide how to source the dependency. We use a LamarServiceDescriptor + /// subclass so the descriptor carries the Lamar Instance AND the intended + /// lifetime; Lamar's own self-discovery treats it the same as a legacy + /// ImplementationInstance round-trip via the helper below. /// /// /// public static void Add(this IServiceCollection services, Instance instance) { - services.Add(new ServiceDescriptor(instance.ServiceType, instance)); + var descriptor = new LamarServiceDescriptor(instance); + instance.ServiceCollection = services; + instance.LamarDescriptor = descriptor; + services.Add(descriptor); + } + + /// + /// Called by 's setter when the lifetime changes. + /// Locates the Instance's existing LamarServiceDescriptor in the registry by + /// reference and replaces it with a fresh one reflecting current Instance state. + /// Matches by descriptor reference (not by ServiceType, which is what MS-DI's + /// own IServiceCollection.Replace extension does — wrong semantics for us + /// when multiple registrations share a ServiceType, e.g. For<T>().Add<>() + /// pipelines). + /// + internal static void ReplaceLamarDescriptor(this Instance instance) + { + var services = instance.ServiceCollection; + var current = instance.LamarDescriptor; + if (services == null || current == null) return; + + var idx = services.IndexOf(current); + if (idx < 0) return; + + var refreshed = new LamarServiceDescriptor(instance); + services[idx] = refreshed; + instance.LamarDescriptor = refreshed; + } + + /// + /// Returns the Lamar Instance attached to , if any — + /// covering both the LamarServiceDescriptor subclass and the legacy + /// ImplementationInstance round-trip shape. + /// + internal static Instance LamarInstance(this ServiceDescriptor descriptor) + { + if (descriptor is LamarServiceDescriptor lsd) return lsd.Instance; + return descriptor.ImplementationInstance as Instance; } public static bool Matches(this ServiceDescriptor descriptor, Type serviceType, Type implementationType) @@ -56,9 +103,10 @@ public static bool Matches(this ServiceDescriptor descriptor, Type serviceType, return true; } - if (descriptor.ImplementationInstance is Instance) + var lamarInstance = descriptor.LamarInstance(); + if (lamarInstance != null) { - return descriptor.ImplementationInstance.As().ImplementationType == implementationType; + return lamarInstance.ImplementationType == implementationType; } return false; diff --git a/src/Lamar/ServiceRegistry.HashCollisionMitigation.cs b/src/Lamar/ServiceRegistry.HashCollisionMitigation.cs index 6d5f06c0..973c1177 100644 --- a/src/Lamar/ServiceRegistry.HashCollisionMitigation.cs +++ b/src/Lamar/ServiceRegistry.HashCollisionMitigation.cs @@ -1,5 +1,6 @@ using Lamar.IoC; using Lamar.IoC.Instances; +using Lamar.Scanning.Conventions; using System; using System.Collections.Generic; using System.Linq; @@ -74,8 +75,8 @@ internal void HandleInstanceHashCollisions(IGrouping collision, F internal IEnumerable> GetInstanceHashCollisions() { - return this.Select(x => x.ImplementationInstance) - .OfType() + return this.Select(x => x.LamarInstance()) + .Where(x => x != null) .GroupBy(x => x.Hash) .Where(x => x.Select(y => y.ServiceType).Distinct().Count() > 1); }