Retrieving scoped instances in OnCreation() creates a new instance

When using the supplied context in the overload of the OnCreation() function to retrieve a scoped instance, it seems to retrieve a new instance instead.

My use-case is wrapping a scoped NHibernate session in a Castle interceptor to manage transactions.

But the code below (adapted from lamar\src\LamarWithMinimalApiOnNet6\Program.cs) illustrates the same issue:

using System;
using System.Diagnostics;
using Lamar;
using Lamar.Microsoft.DependencyInjection;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.DependencyInjection;

var builder = WebApplication.CreateBuilder(args);

builder.Host.UseLamar();

builder.Host.ConfigureContainer<Lamar.ServiceRegistry>(services =>
{
    services.IncludeRegistry<MyRegistry>();

    services.AddControllers();
});

var app = builder.Build();

app.MapControllers();

app.Run();

[Route("api/[controller]")]
[ApiController]
public class HelloController : ControllerBase
{
    public HelloController(IWidget widget, IWidgetWrapper wrapper)
    {
        if (widget != wrapper.GetWidget())
        {
            throw new Exception("Different scoped widgets!");
        }
    }

    [HttpGet]
    public IActionResult Get()
    {

        return Ok("Hello");
    }
}

public interface IWidget
{
}

public class Widget : IWidget
{
    public Widget()
    {
       Debug.WriteLine("Constructing a Widget");
    }
}

public interface IWidgetWrapper
{
    IWidget GetWidget();
}

public class FakeWrapper : IWidgetWrapper
{
    public IWidget GetWidget()
    {
        throw new NotImplementedException();
    }
}

public class RealWrapper: IWidgetWrapper
{
    IWidget inner;

    public RealWrapper(IWidget inner)
    {
        this.inner = inner;
    }

    public IWidget GetWidget()
    {
        return inner;
    }
}

public class MyRegistry : ServiceRegistry
{
    public MyRegistry()
    {
        Scan(s =>
        {
            s.TheCallingAssembly();
            s.WithDefaultConventions();
        });

        For<IWidget>().Use<Widget>().Scoped();

        // This ignores the concrete FakeWrapper implementation.
        // It should return a RealWrapper containing the scoped widget from the container.
        // But it seems to create another Widget instance instead.
        For<IWidgetWrapper>().Use<FakeWrapper>()
                            .OnCreation((context, concrete) => new RealWrapper(context.GetInstance<IWidget>()));
    }
}

Can you bundle that up as a test in a PR for the repro steps? I'm a little dubious that you're really getting new instances of IWidget up above to be honest.

Hi @jeremydmiller

I'm not quite sure of how to create a unit test for using Lamar in a .Net 6 web app.

So I've added a bit more logging to my example code, and added a Guid to each Widget instance - see below.

On the first web request I am seeing the Widget constructor called twice - even though it is configured to be scoped.

The debug log shows e.g.:

  Constructing a Widget: 32f0f63f-44f9-4ee0-bc0e-0985e538296a
  Constructing a Widget: 9e9b6cc8-2e51-4c09-a499-8a23473a3b48

and the browser displays:

  Hello 
  widget=(32f0f63f-44f9-4ee0-bc0e-0985e538296a,04/02/2022 19:22:17) 
  wrapper.inner=(9e9b6cc8-2e51-4c09-a499-8a23473a3b48,04/02/2022 19:22:17)

But when I refresh the browser, on subsequent requests only one widget is created:

  Constructing a Widget: 2c53e87f-ea24-4612-a974-b18e72b4c3cc

and the browser returns:

  Hello 
  widget=(2c53e87f-ea24-4612-a974-b18e72b4c3cc,04/02/2022 19:24:12) 
  wrapper.inner=(9e9b6cc8-2e51-4c09-a499-8a23473a3b48,04/02/2022 19:22:17)

So it seems that the Widget returned from the context supplied to the OnCreation() lambda has some kind of Singleton lifecycle, and survives across multiple browser requests.

I did some debugging, and the context (Lamar.Container) passed to the Use() lambda for creating the Widget is not the same instance as the context passed to the OnCreation() lambda.

To see this, replace lamar\src\LamarWithMinimalApiOnNet6\Program.cs with the code below, then run it and refresh the browser:

using System.Diagnostics;
using Lamar;
using Lamar.Microsoft.DependencyInjection;
using Microsoft.AspNetCore.Mvc;

var builder = WebApplication.CreateBuilder(args);

// use Lamar as DI.
builder.Host.UseLamar((context, registry) =>
{
    // register services using Lamar
    registry.IncludeRegistry<MyRegistry>();
});

// add the controllers
builder.Services.AddControllers();

var app = builder.Build();

app.MapControllers();

app.Run();

[Route("api/[controller]")]
[ApiController]
public class HelloController : ControllerBase
{
    IWidget widget;
    IWidgetWrapper wrapper;

    public HelloController(IWidget widget, IWidgetWrapper wrapper)
    {
        this.widget = widget;
        this.wrapper = wrapper;
    }

    [HttpGet("/")]
    public IActionResult Get()
    {
        return Ok($"Hello \nwidget=({widget.Id},{widget.When}) \nwrapper.inner=({wrapper.GetWidget().Id},{wrapper.GetWidget().When})");
    }
}

public interface IWidget
{
    Guid Id { get; }
    DateTime When { get; }
}

public class Widget : IWidget
{
    public Guid Id { get; }

    public DateTime When { get; }

    public Widget(string message)
    {
       this.Id = Guid.NewGuid();
       this.When = DateTime.Now;

       Debug.WriteLine("Constructing a Widget: " + this.Id);
    }
}

public interface IWidgetWrapper
{
    IWidget GetWidget();
}

public class FakeWrapper : IWidgetWrapper
{
    public IWidget GetWidget()
    {
        throw new NotImplementedException();
    }
}

public class RealWrapper: IWidgetWrapper
{
    IWidget inner;

    public RealWrapper(IWidget inner)
    {
        this.inner = inner;
    }

    public IWidget GetWidget()
    {
        return inner;
    }
}

public class MyRegistry : ServiceRegistry
{
    public MyRegistry()
    {
        For<IWidget>().Use(context => new Widget("Hi")).Scoped();

        // This ignores the concrete FakeWrapper implementation.
        // It should return a RealWrapper containing the scoped widget from the container.
        // But it creates another Widget instance instead, which persists between web requests.
        For<IWidgetWrapper>().Use<FakeWrapper>()
                             .OnCreation((context, concrete) => new RealWrapper(context.GetInstance<IWidget>()));
    }
}

@tmc101 This test in master worked, and that's as intended behavior with the scoping:

using System;
using System.Diagnostics;
using Shouldly;
using Xunit;

namespace Lamar.Testing.Bugs
{
    public class Bug_324_scoping_within_on_creation
    {
        public interface IWidget
        {
        }

        public class Widget : IWidget
        {
            public Widget()
            {
                Debug.WriteLine("Constructing a Widget");
            }
        }

        public interface IWidgetWrapper
        {
            IWidget GetWidget();
        }

        public class FakeWrapper : IWidgetWrapper
        {
            public IWidget GetWidget()
            {
                throw new NotImplementedException();
            }
        }

        public class RealWrapper: IWidgetWrapper
        {
            IWidget inner;

            public RealWrapper(IWidget inner)
            {
                this.inner = inner;
            }

            public IWidget GetWidget()
            {
                return inner;
            }
        }

        [Fact]
        public void make_it_work()
        {
            var container = Container.For(x =>
            {
                x.For<IWidget>().Use<Widget>().Scoped();

                // This ignores the concrete FakeWrapper implementation.
                // It should return a RealWrapper containing the scoped widget from the container.
                // But it seems to create another Widget instance instead.
                x.For<IWidgetWrapper>().Use<FakeWrapper>()
                    .OnCreation((context, concrete) => new RealWrapper(context.GetInstance<IWidget>()));
            });

            var widget = container.GetInstance<IWidgetWrapper>()
                .ShouldBeOfType<RealWrapper>().GetWidget();

            widget.ShouldBeSameAs(container.GetInstance<IWidget>());
        }
    }
}

Re-open this if you have other questions, but Lamar seems to be behaving correctly here. Scoped services are scoped to the creating container, so the root container will always return the same widget, and that's exactly what's happening. The one thing I can't find from googling this morning is if the Minimal API usage does the automatic scoped container per request mechanism that you'd get from MVC Core controllers. Most likely, I think you're resolving this from the root container, and there's your trouble.

Hi @jeremydmiller

I think I've narrowed this down to the following question:

When Lamar constructs an instance using a nested container, should it pass the nested container to the OnCreation lambda?

Currently, Lamar passes the root container to the OnCreation lamba.

The unit test below fails on the last check - but I think it should pass.

using System;
using System.Diagnostics;
using Shouldly;
using Xunit;

namespace Lamar.Testing.Bugs
{
    public class Bug_324_scoping_within_on_creation
    {
        public interface IWidget
        {
        }

        public class Widget : IWidget
        {
            public Widget(){}
        }

        [Fact]
        public void oncreation_should_receive_the_constructing_container()
        {
            IServiceContext containerPassedToOnCreationLambda = null;

            var rootContainer = new Container(x =>
            {
                x.For<IWidget>().Use<Widget>()
                .OnCreation((context, concrete) => {
                    containerPassedToOnCreationLambda = context;
                    return concrete;
                })
                .Scoped();

            });

            var rootWidget = rootContainer.GetInstance<IWidget>();

            // The rootContainer should have been passed to the OnCreation lambda.
            containerPassedToOnCreationLambda.ShouldBeSameAs(rootContainer);

            var nestedContainer = rootContainer.GetNestedContainer();

            var nestedWidget = nestedContainer.GetInstance<IWidget>();

            // The nestedContainer should have been passed to the OnCreation lambda.
            containerPassedToOnCreationLambda.ShouldBeSameAs(nestedContainer);
        }
    }
}

JasperFx / lamar

Retrieving scoped instances in OnCreation() creates a new instance #324