search

abpframework / abp

Open-source web application framework for ASP.NET Core! Offers an opinionated architecture to build enterprise software solutions with best practices on top of the .NET. Provides the fundamental infrastructure, cross-cutting-concern implementations, startup templates, application modules, UI themes, tooling and documentation.
https://abp.io
GNU Lesser General Public License v3.0
12.94k stars 3.45k forks source link

Should TenantId be set automatically if entity implements IMultiTenant interface? #1130

Closed Webdiyer closed 5 years ago

Webdiyer commented 5 years ago

If entity implements IMultiTenant interface, then property value of TenantId must be set manually within the constructor when the entity is created, it's tedious and also dangarous if someone forgot to set it, should it be better to set TenantId value automatically like creation properties in the code below? https://github.com/abpframework/abp/blob/8f2e19085700a55e5191254e2ec92abb132d49b7/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/AbpDbContext.cs#L133

maliming commented 5 years ago

see: https://aspnetboilerplate.com/Pages/Documents/Multi-Tenancy#additional-notes

Webdiyer commented 5 years ago

Sorry I'm not familiar with aspnetboilerplate and it's not easy to understand in which situation it's impossible to set TenantId value automatically, in abp vnext framework, there're no IMustHaveTenant and IMayHaveTenant interfaces but just one IMultiTenant interface. My problem is: my app service is simply inherited from AsyncCrudAppService, I have to rewrite CreateAsync method completely if I want to set the TenantId value of the entity object to be created, or should I add a TenantId property to my dto object and set this property value from within web layer and then mapped to entity? Which one is the recommended way? Thanks in advance.

maliming commented 5 years ago

The tenant ID and user ID can be modified within the scope.

https://github.com/abpframework/abp/blob/f0dd8cd1e8a1fbbe386b6a9a8376dbe28b61e87a/framework/test/Volo.Abp.MultiTenancy.Tests/Volo/Abp/MultiTenancy/CurrentTenant_Tests.cs#L48

If SaveChangesAsync is called somewhere, the wrong tenant ID and user ID may be used.

I personally think AsyncCrudAppService is not suitable for most scenarios.

hikalkan commented 5 years ago

@maliming explained it well.

Think these cases:

ApplyAbpConcepts see a new entity with tenant id is null and CurrentTenant.Id is X. Setting entity.TenantId to X may not be always correct. What if you intentionally wanted to set it to null (you wanted to create a host-side entity)?

It is similar if entity with tenant id is X and CurrentTenant.Id is null. Setting it to null is not correct.

You can create a new entity in tenant X scope and save your DbContext in tenant Y scope. So, only the code created the entity can know what is the correct tenantid.

Otherwise, it is pretty easy to set it in ApplyAbpConcepts for ABP framework.

BTW, if you problem is AsyncCrudAppService, then you are right. I created an issue to implement it: https://github.com/abpframework/abp/issues/1134

Webdiyer commented 5 years ago

Thanks for the detailed explanation and the great work you've done!