From efc202bd563d79ba8dd476329644a199e848c3f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Halil=20=C4=B0brahim=20Kalkan?= Date: Fri, 20 Sep 2019 15:55:41 +0300 Subject: [PATCH] #1580 Removed setter for IEntity.Id --- .../Application/Services/CrudAppService.cs | 6 +- .../Entities/DisableIdGenerationAttribute.cs | 9 +++ .../Volo/Abp/Domain/Entities/EntityHelper.cs | 27 +++++++++ .../Volo/Abp/Domain/Entities/IEntity.cs | 2 +- .../Abp/EntityFrameworkCore/AbpDbContext.cs | 39 +++++++++---- .../MemoryDb/MemoryDbRepository.cs | 6 +- .../Repositories/MongoDB/MongoDbRepository.cs | 19 ++++++- .../Abp/Domain/Entities/EntityHelper_Tests.cs | 57 +++++++++++++++++++ 8 files changed, 148 insertions(+), 17 deletions(-) create mode 100644 framework/src/Volo.Abp.Ddd.Domain/Volo/Abp/Domain/Entities/DisableIdGenerationAttribute.cs create mode 100644 framework/test/Volo.Abp.Ddd.Tests/Volo/Abp/Domain/Entities/EntityHelper_Tests.cs 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 3a64760a0f..c8e89f6869 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 @@ -289,7 +289,11 @@ namespace Volo.Abp.Application.Services return; } - entityWithGuidId.Id = GuidGenerator.Create(); + EntityHelper.TrySetId( + entityWithGuidId, + () => GuidGenerator.Create(), + true + ); } /// diff --git a/framework/src/Volo.Abp.Ddd.Domain/Volo/Abp/Domain/Entities/DisableIdGenerationAttribute.cs b/framework/src/Volo.Abp.Ddd.Domain/Volo/Abp/Domain/Entities/DisableIdGenerationAttribute.cs new file mode 100644 index 0000000000..1d823ca177 --- /dev/null +++ b/framework/src/Volo.Abp.Ddd.Domain/Volo/Abp/Domain/Entities/DisableIdGenerationAttribute.cs @@ -0,0 +1,9 @@ +using System; + +namespace Volo.Abp.Domain.Entities +{ + public class DisableIdGenerationAttribute : Attribute + { + + } +} diff --git a/framework/src/Volo.Abp.Ddd.Domain/Volo/Abp/Domain/Entities/EntityHelper.cs b/framework/src/Volo.Abp.Ddd.Domain/Volo/Abp/Domain/Entities/EntityHelper.cs index b6558f34d1..dd483f5d7b 100644 --- a/framework/src/Volo.Abp.Ddd.Domain/Volo/Abp/Domain/Entities/EntityHelper.cs +++ b/framework/src/Volo.Abp.Ddd.Domain/Volo/Abp/Domain/Entities/EntityHelper.cs @@ -81,5 +81,32 @@ namespace Volo.Abp.Domain.Entities var lambdaBody = Expression.Equal(leftExpression, rightExpression); return Expression.Lambda>(lambdaBody, lambdaParam); } + + public static void TrySetId( + IEntity entity, + Func idFactory, + bool checkForDisableGuidGenerationAttribute = false) + { + //TODO: Can be optimized (by caching per entity type)? + var entityType = entity.GetType(); + var idProperty = entityType.GetProperty( + nameof(entity.Id) + ); + + if (idProperty == null) + { + return; + } + + if (checkForDisableGuidGenerationAttribute) + { + if (idProperty.IsDefined(typeof(DisableIdGenerationAttribute), true)) + { + return; + } + } + + idProperty.SetValue(entity, idFactory()); + } } } \ No newline at end of file diff --git a/framework/src/Volo.Abp.Ddd.Domain/Volo/Abp/Domain/Entities/IEntity.cs b/framework/src/Volo.Abp.Ddd.Domain/Volo/Abp/Domain/Entities/IEntity.cs index 58e607b120..890c5227bd 100644 --- a/framework/src/Volo.Abp.Ddd.Domain/Volo/Abp/Domain/Entities/IEntity.cs +++ b/framework/src/Volo.Abp.Ddd.Domain/Volo/Abp/Domain/Entities/IEntity.cs @@ -22,6 +22,6 @@ /// /// Unique identifier for this entity. /// - TKey Id { get; set; } + TKey Id { get; } } } diff --git a/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/AbpDbContext.cs b/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/AbpDbContext.cs index 63058c314f..3604913301 100644 --- a/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/AbpDbContext.cs +++ b/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/AbpDbContext.cs @@ -281,20 +281,37 @@ namespace Volo.Abp.EntityFrameworkCore protected virtual void CheckAndSetId(EntityEntry entry) { - //Set GUID Ids - var entity = entry.Entity as IEntity; - if (entity != null && entity.Id == Guid.Empty) + if (entry.Entity is IEntity entityWithGuidId) { - var dbGeneratedAttr = ReflectionHelper - .GetSingleAttributeOrDefault( - entry.Property("Id").Metadata.PropertyInfo - ); + TrySetGuidId(entry, entityWithGuidId); + } + } - if (dbGeneratedAttr == null || dbGeneratedAttr.DatabaseGeneratedOption == DatabaseGeneratedOption.None) - { - entity.Id = GuidGenerator.Create(); - } + protected virtual void TrySetGuidId(EntityEntry entry, IEntity entity) + { + if (entity.Id != default) + { + return; } + + var idProperty = entry.Property("Id").Metadata.PropertyInfo; + + //Check for DatabaseGeneratedAttribute + var dbGeneratedAttr = ReflectionHelper + .GetSingleAttributeOrDefault( + idProperty + ); + + if (dbGeneratedAttr != null && dbGeneratedAttr.DatabaseGeneratedOption != DatabaseGeneratedOption.None) + { + return; + } + + EntityHelper.TrySetId( + entity, + () => GuidGenerator.Create(), + true + ); } protected virtual void SetCreationAuditProperties(EntityEntry entry) diff --git a/framework/src/Volo.Abp.MemoryDb/Volo/Abp/Domain/Repositories/MemoryDb/MemoryDbRepository.cs b/framework/src/Volo.Abp.MemoryDb/Volo/Abp/Domain/Repositories/MemoryDb/MemoryDbRepository.cs index da71b1925d..68b9beeb6e 100644 --- a/framework/src/Volo.Abp.MemoryDb/Volo/Abp/Domain/Repositories/MemoryDb/MemoryDbRepository.cs +++ b/framework/src/Volo.Abp.MemoryDb/Volo/Abp/Domain/Repositories/MemoryDb/MemoryDbRepository.cs @@ -75,11 +75,13 @@ namespace Volo.Abp.Domain.Repositories.MemoryDb protected virtual void SetIdIfNeeded(TEntity entity) { - if (typeof(TKey) == typeof(int) || typeof(TKey) == typeof(long) || typeof(TKey) == typeof(Guid)) + if (typeof(TKey) == typeof(int) || + typeof(TKey) == typeof(long) || + typeof(TKey) == typeof(Guid)) { if (EntityHelper.HasDefaultId(entity)) { - entity.Id = Database.GenerateNextId(); + EntityHelper.TrySetId(entity, () => Database.GenerateNextId()); } } } diff --git a/framework/src/Volo.Abp.MongoDB/Volo/Abp/Domain/Repositories/MongoDB/MongoDbRepository.cs b/framework/src/Volo.Abp.MongoDB/Volo/Abp/Domain/Repositories/MongoDB/MongoDbRepository.cs index cddebd253d..deca912bec 100644 --- a/framework/src/Volo.Abp.MongoDB/Volo/Abp/Domain/Repositories/MongoDB/MongoDbRepository.cs +++ b/framework/src/Volo.Abp.MongoDB/Volo/Abp/Domain/Repositories/MongoDB/MongoDbRepository.cs @@ -15,6 +15,7 @@ using Volo.Abp.EventBus.Local; using Volo.Abp.Guids; using Volo.Abp.MongoDB; using Volo.Abp.MultiTenancy; +using Volo.Abp.Reflection; using Volo.Abp.Threading; namespace Volo.Abp.Domain.Repositories.MongoDB @@ -315,12 +316,26 @@ namespace Volo.Abp.Domain.Repositories.MongoDB protected virtual void CheckAndSetId(TEntity entity) { - if (entity is IEntity entityWithGuidId && entityWithGuidId.Id == default) + if (entity is IEntity entityWithGuidId) { - entityWithGuidId.Id = GuidGenerator.Create(); + TrySetGuidId(entityWithGuidId); } } + protected virtual void TrySetGuidId(IEntity entity) + { + if (entity.Id != default) + { + return; + } + + EntityHelper.TrySetId( + entity, + () => GuidGenerator.Create(), + true + ); + } + protected virtual void SetCreationAuditProperties(TEntity entity) { AuditPropertySetter.SetCreationProperties(entity); diff --git a/framework/test/Volo.Abp.Ddd.Tests/Volo/Abp/Domain/Entities/EntityHelper_Tests.cs b/framework/test/Volo.Abp.Ddd.Tests/Volo/Abp/Domain/Entities/EntityHelper_Tests.cs new file mode 100644 index 0000000000..7207085ee6 --- /dev/null +++ b/framework/test/Volo.Abp.Ddd.Tests/Volo/Abp/Domain/Entities/EntityHelper_Tests.cs @@ -0,0 +1,57 @@ +using System; +using Shouldly; +using Xunit; + +namespace Volo.Abp.Domain.Entities +{ + public class EntityHelper_Tests + { + [Fact] + public static void SetId_DerivedFromAggregateRoot() + { + var idValue = Guid.NewGuid(); + var myEntityDerivedFromAggregateRoot = new MyEntityDerivedFromAggregateRoot(); + EntityHelper.TrySetId(myEntityDerivedFromAggregateRoot, () => idValue, true); + myEntityDerivedFromAggregateRoot.Id.ShouldBe(idValue); + } + + [Fact] + public static void SetId_ImplementsIEntity() + { + var idValue = Guid.NewGuid(); + var myEntityImplementsIEntity = new MyEntityImplementsIEntity(); + EntityHelper.TrySetId(myEntityImplementsIEntity, () => idValue, true); + myEntityImplementsIEntity.Id.ShouldBe(idValue); + } + + [Fact] + public static void SetId_DisablesIdGeneration() + { + var idValue = Guid.NewGuid(); + var myEntityDisablesIdGeneration = new MyEntityDisablesIdGeneration(); + EntityHelper.TrySetId(myEntityDisablesIdGeneration, () => idValue, true); + myEntityDisablesIdGeneration.Id.ShouldBe(default); + } + + private class MyEntityDerivedFromAggregateRoot : AggregateRoot + { + + } + + private class MyEntityImplementsIEntity : IEntity + { + public Guid Id { get; set; } + + public object[] GetKeys() + { + return new object[] { Id }; + } + } + + private class MyEntityDisablesIdGeneration : Entity + { + [DisableIdGeneration] + public override Guid Id { get; set; } + } + } +}