Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

how to determine current tenant in the distributedEventBus #4508

Closed
fenggaoyao opened this issue Jun 28, 2020 · 8 comments · Fixed by #4555
Closed

how to determine current tenant in the distributedEventBus #4508

fenggaoyao opened this issue Jun 28, 2020 · 8 comments · Fixed by #4555

Comments

@fenggaoyao
Copy link

  1. IDistributedEventHandler is different from mvc, it does not go through middleware, such as .UseMiddleware<MultiTenancyMiddleware>. So the DbContext uses the current host ,
    , Such as BlogUserSynchronizer in the Volo.Blogging module, it can only handle the host,
    If you need to handle tenant,do we need to add _currentTenant.Change(**),like this
  public async Task HandleEventAsync(EntityUpdatedEto<UserEto> eventData)
        {
            using (_currentTenant.Change(eventData.Entity.TenantId))
            {
                var user = await UserRepository.FindAsync(eventData.Entity.Id);
                if (user == null)
                {
                    user = await UserLookupService.FindByIdAsync(eventData.Entity.Id);
                    if (user == null)
                    {
                        return;
                    }
                }
                if (user.Update(eventData.Entity))
                {
                    await UserRepository.UpdateAsync(user);
                }
            }          
        }
  1. Volo.Abp.Domain.Entities.Events.Distributed.EntityEto, it does not include tenantId, do we noed to add tenantId?
@maliming maliming added this to the 3.1 milestone Jun 28, 2020
@maliming
Copy link
Member

maliming commented Jun 28, 2020

The entity's TenantId may be different from the current TenantId.
We can consider providing TenantId in the event. Developers can also customize this TenantId.

@hikalkan What do you think?

@hikalkan
Copy link
Member

  1. You're definitely right. We should store TenantId in the event data and switch to the tenant in the event handler. We can consider to handle it in the framework side (like if the ETO implements IMultiTenant, we can switch to the tenant before calling the handleevent method).
  2. I don't think so, because not all entities are multi-tenant. If your entity is multi-tenant, then implement IMultiTenant for the ETO.

@fenggaoyao
Copy link
Author

fenggaoyao commented Jun 28, 2020

thanks
1、EntityUpdatedEto<UserEto>, as the main synchronization example of distributedeventbus in the framework.but it is hard to use.UserEto should implements IMultiTenant.And when an update IdentityUser, more than 6 times events handle action will be triggered.and Each UserEto is different。I think it Caused by IdentityRoleStore's AutoSaveChanges。It should be as UnitofWork, can AutoSaveChanges be Changed to false and test this

2、Volo.Abp.Domain.Entities.Events.Distributed.EntityEto, as a general Eto, it has extensible Properties. If If your entity is multi-tenant. can add to this in the framework
EntityToEtoMapper.

public object Map(object entityObj)
        {
            ......
            var etoMappingItem = Options.EtoMappings.GetOrDefault(entityType);
            if (etoMappingItem == null)
            {
                var keys = entity.GetKeys().JoinAsString(",");
                var eto = new EntityEto(entityType.FullName, keys);
                 // add this
                if (entityObj is IMultiTenant multitenantEntity)
                {
                    eto.Properties.Add("tenantid",multitenantEntity.TenantId);
                }
                return eto;
            }  
          ....         
        }

@gdlcf88
Copy link
Contributor

gdlcf88 commented Jun 28, 2020

It will be a big change. I always inject the ICurrentTenant and change the current tenant to MyOwnEto.TenantId.

@hikalkan
Copy link
Member

1 was done with #4433
I didn't like the solution 2. Simply implement IMultiTenant for the ETO. For generic ETOs (like EntityUpdatedEto<T>) the framework can also check the T.

@gdlcf88
Copy link
Contributor

gdlcf88 commented Jun 28, 2020

Simply implement IMultiTenant for the ETO

Does it mean that If MyEto class implements IMultiTenant, the IDistributedEventHandler<EntityUpdatedEto<MyEto>> will automatically change the current tenant Id to MyEto.TenantId?

@hikalkan
Copy link
Member

hikalkan commented Jun 28, 2020

Does it mean that If MyEto class implements IMultiTenant, the IDistributedEventHandler<EntityUpdatedEto> will automatically change the current tenant Id to MyEto.TenantId?

Yes, it means that.

@hikalkan hikalkan removed their assignment Jun 30, 2020
@hikalkan
Copy link
Member

hikalkan commented Jun 30, 2020

@maliming can you work on that in this milestone

If the ETO implements IMultiTenant, we can switch to the tenant before calling the handleevent method.

Also consider this scenario:

Does it mean that If MyEto class implements IMultiTenant, the IDistributedEventHandler will automatically change the current tenant Id to MyEto.TenantId

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants