Manually set tenant for new scope after migration to 7.0.1 version

[lil-kita]

Hi, I updated the package to the 7.0.1 version and I have a question:
How do we set the tenant to the current scope?

in 6.. versions I use this approach:

using IServiceScope scope = _serviceScopeFactory.CreateScope(); // where _serviceScopeFactory is IServiceScopeFactory

MyTenantDbContext ctx = scope.ServiceProvider.GetRequiredService<MyTenantDbContext>();
MyTenantInfo tenant = await ctx.TenantInfo.FirstOrDefaultAsync(t => t.Identifier == tenantIdentifier);

IMultiTenantContextAccessor accessor = scope.ServiceProvider.GetRequiredService<IMultiTenantContextAccessor>();
accessor.MultiTenantContext = new MultiTenantContext<MyTenantInfo>() { TenantInfo = tenant };

// do smth

but now it has become read-only:

Property or indexer 'IMultiTenantContextAccessor.MultiTenantContext' cannot be assigned to -- it is read-only

[robinabganpat]

I'm currently looking into this as well. My use case is that when processing a message from a queue, I want to set the specific tenant based on a specific message property.

I've currently solved it as follows: Create your own strategy and put it before any other strategies. For example:

    public class MyCustomStrategy: IMultiTenantStrategy
    {
        public Task<string> GetIdentifierAsync(object context)
        {
            if (context is string identifier)
            {
                return Task.FromResult(identifier);
            }

            return Task.FromResult<string>(null);
        }
    }

Then add it to DI as follows:

.WithStrategy<MyCustomStrategy>(ServiceLifetime.Singleton)

Then wherever you want to manually set the tenant, use this:

var mtcSetter = scope.ServiceProvider.GetRequiredService<IMultiTenantContextSetter>();
var tenantResolver = scope.ServiceProvider.GetRequiredService<ITenantResolver>();
var multiTenantContext = await tenantResolver.ResolveAsync("IDENTIFIER HERE");
mtcSetter.MultiTenantContext = multiTenantContext;

Mind that this solution currently abuses the fact that the object context is of type object. Certain default strategies (like the HeaderStrategy) will fail because of this, since it expects to 'context' be of type HttpContext. Therefore make sure the .WithStrategy<MyCustomStrategy>(ServiceLifetime.Singleton) is called before any other strategies that are depending on the HttpContext.

Imho, the HttpContext dependent strategies should not throw an exception when the context is not a HttpContext. It should check if it is and otherwise just skip the strategy processing. Or at least make it configurable to explicity throw. Curious as to what others think of this.

[AndrewTriesToCode]

Edit: @robinabganpat beat me to the details above, thanks!

Hi, I abstracted setting it behind IMultiTenantContextSetter. Take a look in the middle where to see how it can be used:

var mtcSetter = context.RequestServices.GetRequiredService<IMultiTenantContextSetter>();
            
var resolver = context.RequestServices.GetRequiredService<ITenantResolver>();
            
var multiTenantContext = await resolver.ResolveAsync(context);

mtcSetter.MultiTenantContext = multiTenantContext;

[robinabganpat]

Edit: @robinabganpat beat me to the details above, thanks!

Hi, I abstracted setting it behind IMultiTenantContextSetter. Take a look in the middle where to see how it can be used:

var mtcSetter = context.RequestServices.GetRequiredService<IMultiTenantContextSetter>();
            
var resolver = context.RequestServices.GetRequiredService<ITenantResolver>();
            
var multiTenantContext = await resolver.ResolveAsync(context);

mtcSetter.MultiTenantContext = multiTenantContext;

No problem. Thank you for creating this library.

Given my comment:

Imho, the HttpContext dependent strategies should not throw an exception when the context is not a HttpContext. It should check if it is and otherwise just skip the strategy processing. Or at least make it configurable to explicity throw. Curious as to what others think of this.

What is your opinion on this and what is your idea behind the object context implementation? The choice for the object type makes it possible to use the context 'flexibly', which is nice, but then strategies should not always assume that the context is of type HttpContext, right?

[AndrewTriesToCode]

I like this change and it fits in with a new strategy I am adding that ‘HttpContextStrategy` that just takes a delegate taking http context and wraps the cast from object to http context so the user doesn’t need to worry about it. It could return null as you describe rather than an exception.

Although the current checks that return exceptions only make sense in http context use cases. Which strategy are you using with a non http context that is throwing the exception? I’d like to learn more about your use case.

[robinabganpat]

Although the current checks that return exceptions only make sense in http context use cases. Which strategy are you using with a non http context that is throwing the exception? I’d like to learn more about your use case.

I'm currently using the HeaderStrategy, which is a HttpContext use case, however in my use case I mix between a HttpContext use case and the use case mentioned above: A message is received from a queue, where a message property mentions the TenantIdentifier.

The TenantResolver just loops through the Strategies using the context that is given, so it doesn't know which Strategy is using the context as HttpContext or something else.

I've now tackled this issue by first registering my custom Strategy before any HttpContext related strategy to prevent an Exception to be thrown in case it tries to resolve the tenant when a message is received from a queue. Although, it would be nice if the HeaderStrategy would just check whether the given context is a HttpContext and would 'skip' the strategy if it isn't, instead of throwing an exception.

What also would be nice, if this behaviour would be configurable if you really want the strategy to throw an exception if it doesn't know what to do with the given context. This could be handy if you really want to force a strategy to be adhered to.

[AndrewTriesToCode]

Makes sense. I will prioritize this change. On Jun 26, 2024, at 1:21 AM, robinabganpat @.***> wrote:

Although the current checks that return exceptions only make sense in http context use cases. Which strategy are you using with a non http context that is throwing the exception? I’d like to learn more about your use case.

I'm currently using the HeaderStrategy, which is a HttpContext use case, however in my use case I mix between a HttpContext use case and the use case mentioned above: A message is received from a queue, where a message property mentions the TenantIdentifier. The TenantResolver just loops through the Strategies using the context that is given, so it doesn't know which Strategy is using the context as HttpContext or something else. I've now tackled this issue by first registering my custom Strategy before any HttpContext related strategy to prevent an Exception to be thrown in case it tries to resolve the tenant when a message is received from a queue. Although, it would be nice if the HeaderStrategy would just check whether the given context is a HttpContext and would 'skip' the strategy if it isn't, instead of throwing an exception. What also would be nice, if this behaviour would be configurable if you really want the strategy to throw an exception if it doesn't know what to do with the given context. This could be handy if you really want to force a strategy to be adhered to.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: @.***>

[github-actions[bot]]

This issue has been labeled inactive because it has been open 180 days with no activity. Please consider closing this issue if no further action is needed.

[tsutomi]

I personally see an issue with this implementation, with regards to the scenario of usage in background multi-threaded contexts, like in the case of the original poster (runners triggered by messages from queues).

The IMultiTenantContextAccessor and IMultiTenantContextSetter (practically the same object in the library implementation AsyncLocalMultiTenantContextAccessor) are registered in a Singleton scope. Although the ITenantResolver object is scoped, once the context is resolved, when the setting is done (using the IMultiTenantContextSetter), then the multi-tenant context is the same for all consuming contexts across all scopes.

For example

var services = new ServiceCollection();
services.AddMultiTenant<TenantInfo>()
    .WithDelegateStrategy(ctx => (string)ctx)
    .WithInMemoryStore(store => {
        store.Tenants.Add(new TenantInfo {
            Identifier = "tenant1",
            Id = "abc12345"
        }); 

        store.Tenants.Add(new TenantInfo {
            Identifier = "tenant2",
            Id = "54321cba"
        }); 
    });

services.AddDbContext<MyAppDbContext(options => {
   // ... assume this is a multi-tenant DbContext using the IMultiTenantContextAccessor object
});

... and then a job (e.g. a worker triggered by a queue message) like ...

public class SimpleJob {
    private IServiceProvider services;

    public SimpleJob(IServiceProvider services) {
        this.services = services;
    }

    public Task Execute(Message message) {
       var tenantId = message.TenantId
        using var scope = services.CreateScope().ServiceProvider;
        var setter = scope.GetRequiredService<IMultiTenantContextSetter>();
        var resolver = scope.GetRequiredService<ITenantResolver>();
        var context = resoliver.ResolveAsync(tenantId);
        // after this, all threads and scopes get the context for 'tenantId'
        setter.MultiTenantContext = context;

        // the IMultiTenantContextAccessor used by the MyAppDbContext options builder
        // is resolving the latest context from 'tenantId'
        using var dbContext = scope.GetRequiredService<MyAppDbContext>();
    }
}

If, for any reason, an application will run into a concurrent scenario where two SimpleJob will be run at the same time, the singleton stored at the instance of IMultiTenantContextAccessor will cause one of the two to re-use the connection string from another tenant to instantiate the DbContext.

Wouldn't it be preferable to register IMultiTenantContextAccessor as a scoped service? Do you see the same issue? Am I misunderstanding the role of the singleton implementation of the accessor?

[AndrewTriesToCode]

@tsutomi yes it gets tricky. The AsyncLocal<T> based implementation is modeled how IHttpContextAccessor works in ASP.NET Core... but obviously that is geared towards their async heavy web server design.

There is also StaticMultiTenantContextAccessor which you can use to just set it and leave it.

The reason for singleton scope was to support some issues in the per-tenant options support which assume related singleton options related services that would need to know the current tenant. I've been thinking of ways around this and plan to moved to all scoped based in v10.