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.98k stars 3.45k forks source link

FullAuditedAggregateRootWithUser generic type wrong constraint #2624

Closed RayMMond closed 4 years ago

RayMMond commented 4 years ago

Anyone please tell me why FullAuditedAggregateRootWithUser need TUser to inherited from IEntity\<long> while IUser interface using IEntity\<Guid>? https://github.com/abpframework/abp/blob/654eeede81a733382066bfd00ab19b6b65ceec0e/framework/src/Volo.Abp.Ddd.Domain/Volo/Abp/Domain/Entities/Auditing/FullAuditedAggregateRootWithUser.cs#L11-L12

https://github.com/abpframework/abp/blob/654eeede81a733382066bfd00ab19b6b65ceec0e/framework/src/Volo.Abp.Ddd.Domain/Volo/Abp/Domain/Entities/Auditing/FullAuditedAggregateRootWithUser.cs#L30-L31

https://github.com/abpframework/abp/blob/42f37c5ff01ad853a5425d15539d4222cd0dab69/modules/users/src/Volo.Abp.Users.Domain/Volo/Abp/Users/IUser.cs#L8

maliming commented 4 years ago

I am also confused about this.

@hikalkan can you explain it. Thanks.

acjh commented 4 years ago

I suppose that is because FullAuditedAggregateRootWithUser was committed on 14 Mar 2018 (https://github.com/abpframework/abp/commit/2339fab33770417a0c80b873d3182fb5345a98f9#diff-88bc7789a4b0b95478b3bb69f9f1fc29), before @hikalkan decided to go with Guid for TKey of IUser on 20 Jun 2018 (https://github.com/abpframework/abp/commit/745d891134da81de8fa88e8851f085f4eeb03b91#diff-99e4bec2857b169bc53dbd6d309560c4).

FullAuditedAggregateRootWithUser may have been overlooked because it is not used anywhere (https://github.com/abpframework/abp/search?q=FullAuditedAggregateRootWithUser) and the two classes are in different projects — FullAuditedAggregateRootWithUser in the framework and IUser in the users module.

I believe we should update it in FullAuditedAggregateRootWithUser.

hikalkan commented 4 years ago

@acjh you are right. FullAuditedAggregateRootWithUser classes have never been used and forgotten :) I will update them.

hikalkan commented 4 years ago

BTW, we don't suggest to use this base class since it breaks one of the Aggregate design rules: "Don't add navigation properties between aggregates, use Id instead".

But you know not everyone has to perfectly design DDD based apps. If you are using EF Core, it is a common habit to add such navigation properties.

RayMMond commented 4 years ago

Thanks for the extra explanation about DDD :) @hikalkan

hikalkan commented 4 years ago

You're welcome :)

xkaede commented 4 years ago

hi @hikalkan AuditedAggregateRootWithUser and FullAuditedEntityWithUser are wrong too. https://github.com/abpframework/abp/blob/251a8d2de0a62d4e3ebda1c7f41a7408fad3259f/framework/src/Volo.Abp.Ddd.Domain/Volo/Abp/Domain/Entities/Auditing/AuditedAggregateRootWithUser.cs#L12 https://github.com/abpframework/abp/blob/251a8d2de0a62d4e3ebda1c7f41a7408fad3259f/framework/src/Volo.Abp.Ddd.Domain/Volo/Abp/Domain/Entities/Auditing/FullAuditedEntityWithUser.cs#L12

by the way, I am using CQRS pattern. and I find use *EntityWithUser in Query model will be convenient. so I still need these base classes.