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

Mark AggreateRoot as modified #19060

Closed
1 task done
Brandejs opened this issue Feb 16, 2024 · 9 comments
Closed
1 task done

Mark AggreateRoot as modified #19060

Brandejs opened this issue Feb 16, 2024 · 9 comments

Comments

@Brandejs
Copy link

Brandejs commented Feb 16, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe the problem.

According to AggregateRoot Class I would expect that if I am working with Order as a single unit, the modification in the OrderLines collection will cause a change to Order at the same time.

So if I add a new OrderLine to an Order. I would expect that there will be a LastModifierId and LastModificationTime change on the Order.

I would also expect that
ILocalEventHandler<EntityUpdatedEventData<Order>> should be triggered

Describe the solution you'd like

Would it be possible to add a MarkAsModified() method to BasicAggregateRoot? that would mark the entity as modified? So proper audition and event triggers be handled? Or with a repository.UpdateAsync() mark as changed even if there is no property change.

Additional context

image
image

@maliming maliming self-assigned this Feb 16, 2024
@maliming
Copy link
Member

hi

What is your ABP Framework version?

Can you share a test project? Thanks.

@Brandejs
Copy link
Author

Hi,

sure. ABP Framework version: 8.0.3

It's a newly generated project.

Code
src/Acme.BookStore_Eventing.Domain/Orders/Order.cs

Test
test/Acme.BookStore_Eventing.Domain.Tests/Samples/SampleDomainTests.cs
Acme.BookStore_Eventing.zip

@maliming maliming added this to the 8.2-preview milestone Feb 19, 2024
@Brandejs
Copy link
Author

Brandejs commented Feb 19, 2024

HI,

maybe not related... But I notice another thing.

If in DbContextModelCreatingExtensions we set HasDefaultValue for property.

b.Property(x => x.Name).HasDefaultValue("DefaultVlaue");

in AbpDbContext.cs

In the same method that is responsible for emitting events
`PublishEventsForTrackedEntity¨

There is condition
if (entry.Properties.Any(x => x.IsModified && x.Metadata.ValueGenerated == ValueGenerated.Never))

but with HasDefaultValue: x.Metadata.ValueGenerated == 'OnAdd'

I think that correct condition should be
if (entry.Properties.Any(x => x.IsModified && (x.Metadata.ValueGenerated == ValueGenerated.Never || x.Metadata.ValueGenerated == ValueGenerated.OnAdd)))

Screenshot 2024-02-19 at 11 17 13

@maliming
Copy link
Member

hi

The EF Core has an issue; We can't know whether the aggregate root has changed when deleting the child collection.

dotnet/efcore#24076

@Brandejs
Copy link
Author

Hi,

I think there may be more cases. For example, changing the Child Entity of Child entity. So I would suggest as a fallback to add some dummy property to BasicAggreageRoot, which we can set with this.MarkAsModified() As I know there is a similar problem with ValueObjects. So this could be used as a fallback.

I currently set LastModificatioTime = null as a fallback. Which will mark the entity as Modified.

@maliming
Copy link
Member

We should wait for EF Core to support it.

@maliming
Copy link
Member

Done by #19079

@JadynWong
Copy link
Contributor

JadynWong commented Apr 13, 2024

If you don't use Include to load the sub-navigation properties at the beginning, and use Repository.EnsureCollectionLoadedAsync in the middle of the process, it won't trigger EntityUpdatedEventData<TEntity>.

Such as:

public virtual async Task RemoveFromOrganizationUnitAsync(IdentityUser user, OrganizationUnit ou)
{
await UserRepository.EnsureCollectionLoadedAsync(user, u => u.OrganizationUnits,
CancellationTokenProvider.Token);
user.RemoveOrganizationUnit(ou.Id);
await UserRepository.UpdateAsync(user, cancellationToken: CancellationToken);
}

It does not trigger EntityChangedEventData<IdentityUser>.

@maliming
Copy link
Member

If you don't use Include to load the sub-navigation properties at the beginning, and use Repository.EnsureCollectionLoadedAsync in the middle of the process, it won't trigger EntityUpdatedEventData<TEntity>.

Such as:

public virtual async Task RemoveFromOrganizationUnitAsync(IdentityUser user, OrganizationUnit ou)
{
await UserRepository.EnsureCollectionLoadedAsync(user, u => u.OrganizationUnits,
CancellationTokenProvider.Token);
user.RemoveOrganizationUnit(ou.Id);
await UserRepository.UpdateAsync(user, cancellationToken: CancellationToken);
}

It does not trigger EntityChangedEventData<IdentityUser>.

Will be fixed by #19523 5f56d2b

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

No branches or pull requests

3 participants