Named Singletons sub Singletons - Tenancy issue

[stephenlautier]

We had hit an issue when using multi-tenancy (see sample below), when being a singleton.

Basically, we have something like the following small sample:

• Service A - singleton per tenant (IHeroDataClient) - 2 different implementations per tenant
  • Service B - singleton per tenant (HerozClientSub) - same implementation but instance per tenant
  • Service C - singleton for all (IDataClientManager) - same instance for all tenants (shared)

In the above example if we remove Service B (HerozClientSub) it works, so basically when having nested singleton deps it doesn't resolve the tenant.

By changing Service A and Service B to scoped it will also work, but it's not ideal as it will create instances redundantly, and also not sure why it doesn't resolve or if it's intended or a bug.

This is the sample if you want to look at it:

Changes to cause the issue: sketch7/dotnet.multitenancy@feature/initial-imp...issue/nested-singleton-tenant-bug
Some interest points

• Register per services tenant https://github.com/sketch7/dotnet.multitenancy/blob/issue/nested-singleton-tenant-bug/samples/Sketch7.Multitenancy.Sample.Api/Startup.cs#L56
• Register implementation https://github.com/sketch7/dotnet.multitenancy/blob/issue%2Fnested-singleton-tenant-bug/src/Sketch7.Multitenancy.Grace/TenantContainerBuilder.cs#L34
• Locate for tenant https://github.com/sketch7/dotnet.multitenancy/blob/issue%2Fnested-singleton-tenant-bug/src/Sketch7.Multitenancy.Grace/GraceExtensions.cs#L121

Basically, the implementation of how tenancy works is how you had suggested.

Am I doing something wrong? should this work?

Thanks

[ipjohnson]

Sorry you ran into that let me take a look this weekend.

[stephenlautier]

No problem, thanks for your help as always

[ipjohnson]

So I took a look and I'm unclear when you say it doesn't work. What specifically is the error you are getting.

I will say that Singleton as a lifestyle is restrictive on what it can depend on and will not carry or use the injection context from the call (it starts fresh, no injection context).

[stephenlautier]

So by starting from fresh, I guess I'm using it wrong/unsupported. What we would like to achieve is to have singletons, and singleton dependencies per tenant as following:

Tenant A - IHeroDataClient (A) singleton -> HerozClientSub (A) singleton
Tenant B - IHeroDataClient (B) singleton -> HerozClientSub (B) singleton

Perhaps the ideal is to look at the sample

[ipjohnson]

It sounds very much like you need a custom lifestyle SingletonPerTenant. I don’t have a solution for you right at this moment but can you use SingletonPerScope for the next week while I come up with something?

Ultimately I think this is a use case worth supporting I’ve just never had someone ask for it.

[stephenlautier]

Yes, we are not blocked by this we found a way around it by using less DI and create a manual factory. Not the ideal but at least it's working. It would be much easier if it will be supported as a consumer would need to know about these quirks and there are certain scenarios unhandled.

[ipjohnson]

Sorry been swamped, I haven't forgotten about this. I'm hoping this weekend to finish the SingletonPerKey lifestyle.

[stephenlautier]

@ipjohnson I understand, thanks!

[ipjohnson]

I've released a new version with SingletonPerKey lifestlye. Let me know how it works

[ipjohnson]

You can see an example unit test here

[stephenlautier]

Great, I will try to give it a shot.

In which version is it? 7.1.0-beta768?

[ipjohnson]

Correct.

[stephenlautier]

I just tried it out quickly and it seems to be working fine, altho I had to do a change in the usage from your test, I used scope instead of context if i use the context i wont get the tenant

return configuration.Lifestyle.SingletonPerKey((scope, context) =>
{
	var tenant = scope.GetTenantContext();
	return tenant?.Key;
});

[ipjohnson]

That makes sense. The one change I would make is to provide a key if one doesn't exist otherwise it's going to throw an exception.