From 2a4c319081a0f95e8f437a932fb84299d2fcfc9b Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" Date: Tue, 25 Jun 2019 11:43:27 +0200 Subject: [PATCH 1/2] fix(multi-tenancy): correctly set TenantId on create in AppService The TenantId is now automatically set when creating new entities in an AsyncCrudAppService derived class. fixes #1360 --- .../Services/AsyncCrudAppService.cs | 2 +- .../MultiTenant_Creation_Tests.cs | 9 +++ .../DataFilters/MultiTenant_Creation_Tests.cs | 9 +++ .../MultiTenant_Creation_Tests.cs | 9 +++ .../Application/PersonAppService_Tests.cs | 43 ++++++++++- .../Abp/TestApp/Application/Dto/PersonDto.cs | 5 +- .../Testing/MultiTenant_Creation_Tests.cs | 77 +++++++++++++++++++ 7 files changed, 151 insertions(+), 3 deletions(-) create mode 100644 framework/test/Volo.Abp.EntityFrameworkCore.Tests/Volo/Abp/EntityFrameworkCore/DataFiltering/MultiTenant_Creation_Tests.cs create mode 100644 framework/test/Volo.Abp.MemoryDb.Tests/Volo/Abp/MemoryDb/DataFilters/MultiTenant_Creation_Tests.cs create mode 100644 framework/test/Volo.Abp.MongoDB.Tests/Volo/Abp/MongoDB/DataFiltering/MultiTenant_Creation_Tests.cs create mode 100644 framework/test/Volo.Abp.TestApp/Volo/Abp/TestApp/Testing/MultiTenant_Creation_Tests.cs diff --git a/framework/src/Volo.Abp.Ddd.Application/Volo/Abp/Application/Services/AsyncCrudAppService.cs b/framework/src/Volo.Abp.Ddd.Application/Volo/Abp/Application/Services/AsyncCrudAppService.cs index 7bc76d9923..f964ca3e8d 100644 --- a/framework/src/Volo.Abp.Ddd.Application/Volo/Abp/Application/Services/AsyncCrudAppService.cs +++ b/framework/src/Volo.Abp.Ddd.Application/Volo/Abp/Application/Services/AsyncCrudAppService.cs @@ -109,7 +109,7 @@ namespace Volo.Abp.Application.Services var entity = MapToEntity(input); - if (entity is IMultiTenant && !HasTenantIdProperty(entity)) + if (entity is IMultiTenant && HasTenantIdProperty(entity)) { TryToSetTenantId(entity); } diff --git a/framework/test/Volo.Abp.EntityFrameworkCore.Tests/Volo/Abp/EntityFrameworkCore/DataFiltering/MultiTenant_Creation_Tests.cs b/framework/test/Volo.Abp.EntityFrameworkCore.Tests/Volo/Abp/EntityFrameworkCore/DataFiltering/MultiTenant_Creation_Tests.cs new file mode 100644 index 0000000000..13ac63ead5 --- /dev/null +++ b/framework/test/Volo.Abp.EntityFrameworkCore.Tests/Volo/Abp/EntityFrameworkCore/DataFiltering/MultiTenant_Creation_Tests.cs @@ -0,0 +1,9 @@ +using Volo.Abp.TestApp.Testing; + +namespace Volo.Abp.EntityFrameworkCore.DataFiltering +{ + public class MultiTenant_Creation_Tests : MultiTenant_Creation_Tests + { + + } +} \ No newline at end of file diff --git a/framework/test/Volo.Abp.MemoryDb.Tests/Volo/Abp/MemoryDb/DataFilters/MultiTenant_Creation_Tests.cs b/framework/test/Volo.Abp.MemoryDb.Tests/Volo/Abp/MemoryDb/DataFilters/MultiTenant_Creation_Tests.cs new file mode 100644 index 0000000000..3058624ec7 --- /dev/null +++ b/framework/test/Volo.Abp.MemoryDb.Tests/Volo/Abp/MemoryDb/DataFilters/MultiTenant_Creation_Tests.cs @@ -0,0 +1,9 @@ +using Volo.Abp.TestApp.Testing; + +namespace Volo.Abp.MemoryDb.DataFilters +{ + public class MultiTenant_Creation_Tests : MultiTenant_Creation_Tests + { + + } +} \ No newline at end of file diff --git a/framework/test/Volo.Abp.MongoDB.Tests/Volo/Abp/MongoDB/DataFiltering/MultiTenant_Creation_Tests.cs b/framework/test/Volo.Abp.MongoDB.Tests/Volo/Abp/MongoDB/DataFiltering/MultiTenant_Creation_Tests.cs new file mode 100644 index 0000000000..a1ee94a943 --- /dev/null +++ b/framework/test/Volo.Abp.MongoDB.Tests/Volo/Abp/MongoDB/DataFiltering/MultiTenant_Creation_Tests.cs @@ -0,0 +1,9 @@ +using Volo.Abp.TestApp.Testing; + +namespace Volo.Abp.MongoDB.DataFiltering +{ + public class MultiTenant_Creation_Tests : MultiTenant_Creation_Tests + { + + } +} \ No newline at end of file diff --git a/framework/test/Volo.Abp.TestApp.Tests/Volo/Abp/TestApp/Application/PersonAppService_Tests.cs b/framework/test/Volo.Abp.TestApp.Tests/Volo/Abp/TestApp/Application/PersonAppService_Tests.cs index c58b1b5f85..0da6d4c8dc 100644 --- a/framework/test/Volo.Abp.TestApp.Tests/Volo/Abp/TestApp/Application/PersonAppService_Tests.cs +++ b/framework/test/Volo.Abp.TestApp.Tests/Volo/Abp/TestApp/Application/PersonAppService_Tests.cs @@ -1,7 +1,13 @@ -using Microsoft.Extensions.DependencyInjection; +using System; +using Microsoft.Extensions.DependencyInjection; using Shouldly; using System.Threading.Tasks; +using NSubstitute; using Volo.Abp.Application.Dtos; +using Volo.Abp.Domain.Repositories; +using Volo.Abp.MultiTenancy; +using Volo.Abp.TestApp.Application.Dto; +using Volo.Abp.TestApp.Domain; using Xunit; namespace Volo.Abp.TestApp.Application @@ -9,17 +15,52 @@ namespace Volo.Abp.TestApp.Application public class PersonAppService_Tests : TestAppTestBase { private readonly IPeopleAppService _peopleAppService; + private ICurrentTenant _fakeCurrentTenant; public PersonAppService_Tests() { _peopleAppService = ServiceProvider.GetRequiredService(); } + protected override void AfterAddApplication(IServiceCollection services) + { + _fakeCurrentTenant = Substitute.For(); + services.AddSingleton(_fakeCurrentTenant); + } + [Fact] public async Task GetList() { var people = await _peopleAppService.GetListAsync(new PagedAndSortedResultRequestDto()); people.Items.Count.ShouldBeGreaterThan(0); } + + [Fact] + public async Task Create() + { + var personDto = await _peopleAppService.CreateAsync(new PersonDto()); + + var repository = ServiceProvider.GetService>(); + var person = await repository.FindAsync(personDto.Id); + + person.ShouldNotBeNull(); + person.TenantId.ShouldBeNull(); + } + + [Fact] + public async Task Create_SetsTenantId() + { + _fakeCurrentTenant.Id.Returns(TestDataBuilder.TenantId1); + + var personDto = await _peopleAppService.CreateAsync(new PersonDto()); + + var repository = ServiceProvider.GetService>(); + var person = await repository.FindAsync(personDto.Id); + + person.ShouldNotBeNull(); + person.TenantId.ShouldNotBeNull(); + person.TenantId.ShouldNotBe(Guid.Empty); + person.TenantId.ShouldBe(TestDataBuilder.TenantId1); + } } } diff --git a/framework/test/Volo.Abp.TestApp/Volo/Abp/TestApp/Application/Dto/PersonDto.cs b/framework/test/Volo.Abp.TestApp/Volo/Abp/TestApp/Application/Dto/PersonDto.cs index 57204f260f..b878b4f9c4 100644 --- a/framework/test/Volo.Abp.TestApp/Volo/Abp/TestApp/Application/Dto/PersonDto.cs +++ b/framework/test/Volo.Abp.TestApp/Volo/Abp/TestApp/Application/Dto/PersonDto.cs @@ -1,12 +1,15 @@ using System; using Volo.Abp.Application.Dtos; +using Volo.Abp.MultiTenancy; namespace Volo.Abp.TestApp.Application.Dto { - public class PersonDto : EntityDto + public class PersonDto : EntityDto, IMultiTenant { public string Name { get; set; } public int Age { get; set; } + + public Guid? TenantId { get; set; } } } diff --git a/framework/test/Volo.Abp.TestApp/Volo/Abp/TestApp/Testing/MultiTenant_Creation_Tests.cs b/framework/test/Volo.Abp.TestApp/Volo/Abp/TestApp/Testing/MultiTenant_Creation_Tests.cs new file mode 100644 index 0000000000..c648fc66d4 --- /dev/null +++ b/framework/test/Volo.Abp.TestApp/Volo/Abp/TestApp/Testing/MultiTenant_Creation_Tests.cs @@ -0,0 +1,77 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using Microsoft.Extensions.DependencyInjection; +using NSubstitute; +using Shouldly; +using Volo.Abp.Data; +using Volo.Abp.Domain.Repositories; +using Volo.Abp.Modularity; +using Volo.Abp.MultiTenancy; +using Volo.Abp.TestApp.Application; +using Volo.Abp.TestApp.Application.Dto; +using Volo.Abp.TestApp.Domain; +using Xunit; + +namespace Volo.Abp.TestApp.Testing +{ + public abstract class MultiTenant_Creation_Tests : TestAppTestBase + where TStartupModule : IAbpModule + { + private ICurrentTenant _fakeCurrentTenant; + private readonly IRepository _personRepository; + private readonly IPeopleAppService _peopleAppService; + + protected MultiTenant_Creation_Tests() + { + _personRepository = GetRequiredService>(); + _peopleAppService = GetRequiredService(); + } + + protected override void AfterAddApplication(IServiceCollection services) + { + _fakeCurrentTenant = Substitute.For(); + services.AddSingleton(_fakeCurrentTenant); + } + + [Fact] + public async void Should_Set_TenantId_For_New_Person() + { + _fakeCurrentTenant.Id.Returns(TestDataBuilder.TenantId1); + + var personId = Guid.NewGuid(); + await _peopleAppService.CreateAsync(new PersonDto + { + Id = personId, + Name = "Person1", + Age = 21 + }); + + var person = await _personRepository.FindAsync(personId); + + person.ShouldNotBeNull(); + person.TenantId.ShouldNotBeNull(); + person.TenantId.ShouldNotBe(Guid.Empty); + person.TenantId.ShouldBe(TestDataBuilder.TenantId1); + } + + [Fact] + public async void Should_Set_Null_TenantId_For_Host_Tenant() + { + _fakeCurrentTenant.Id.Returns((Guid?)null); + + var personId = Guid.NewGuid(); + await _peopleAppService.CreateAsync(new PersonDto + { + Id = personId, + Name = "Person1", + Age = 21 + }); + + var person = await _personRepository.FindAsync(personId); + + person.ShouldNotBeNull(); + person.TenantId.ShouldBeNull(); + } + } +} From 70d334068b6decc80747a349f6a78d32770c20ab Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" Date: Tue, 25 Jun 2019 12:05:14 +0200 Subject: [PATCH 2/2] refactor(multi-tenancy): move TenantId check into BaseAppService Fix TenantId update in CrudAppService and moved TenantId check into base class. fixes #1360 --- .../Services/AsyncCrudAppService.cs | 5 +---- .../Application/Services/CrudAppService.cs | 5 +---- .../Services/CrudAppServiceBase.cs | 21 +++++++++++-------- 3 files changed, 14 insertions(+), 17 deletions(-) diff --git a/framework/src/Volo.Abp.Ddd.Application/Volo/Abp/Application/Services/AsyncCrudAppService.cs b/framework/src/Volo.Abp.Ddd.Application/Volo/Abp/Application/Services/AsyncCrudAppService.cs index f964ca3e8d..055de921ee 100644 --- a/framework/src/Volo.Abp.Ddd.Application/Volo/Abp/Application/Services/AsyncCrudAppService.cs +++ b/framework/src/Volo.Abp.Ddd.Application/Volo/Abp/Application/Services/AsyncCrudAppService.cs @@ -109,10 +109,7 @@ namespace Volo.Abp.Application.Services var entity = MapToEntity(input); - if (entity is IMultiTenant && HasTenantIdProperty(entity)) - { - TryToSetTenantId(entity); - } + TryToSetTenantId(entity); await Repository.InsertAsync(entity, autoSave: true); diff --git a/framework/src/Volo.Abp.Ddd.Application/Volo/Abp/Application/Services/CrudAppService.cs b/framework/src/Volo.Abp.Ddd.Application/Volo/Abp/Application/Services/CrudAppService.cs index 1d43ff084d..e11df56056 100644 --- a/framework/src/Volo.Abp.Ddd.Application/Volo/Abp/Application/Services/CrudAppService.cs +++ b/framework/src/Volo.Abp.Ddd.Application/Volo/Abp/Application/Services/CrudAppService.cs @@ -106,10 +106,7 @@ namespace Volo.Abp.Application.Services var entity = MapToEntity(input); - if (entity is IMultiTenant && !HasTenantIdProperty(entity)) - { - TryToSetTenantId(entity); - } + TryToSetTenantId(entity); Repository.Insert(entity, autoSave: true); diff --git a/framework/src/Volo.Abp.Ddd.Application/Volo/Abp/Application/Services/CrudAppServiceBase.cs b/framework/src/Volo.Abp.Ddd.Application/Volo/Abp/Application/Services/CrudAppServiceBase.cs index 21c19a9a4a..d504119a16 100644 --- a/framework/src/Volo.Abp.Ddd.Application/Volo/Abp/Application/Services/CrudAppServiceBase.cs +++ b/framework/src/Volo.Abp.Ddd.Application/Volo/Abp/Application/Services/CrudAppServiceBase.cs @@ -165,18 +165,21 @@ namespace Volo.Abp.Application.Services protected virtual void TryToSetTenantId(TEntity entity) { - var tenantId = CurrentTenant.Id; - - if (!tenantId.HasValue) + if (entity is IMultiTenant && HasTenantIdProperty(entity)) { - return; - } + var tenantId = CurrentTenant.Id; - var propertyInfo = entity.GetType().GetProperty(nameof(IMultiTenant.TenantId)); + if (!tenantId.HasValue) + { + return; + } - if (propertyInfo != null && propertyInfo.GetSetMethod() != null) - { - propertyInfo.SetValue(entity, tenantId, null); + var propertyInfo = entity.GetType().GetProperty(nameof(IMultiTenant.TenantId)); + + if (propertyInfo != null && propertyInfo.GetSetMethod() != null) + { + propertyInfo.SetValue(entity, tenantId, null); + } } }