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 and the ASP.NET Core platforms. Provides the fundamental infrastructure, production-ready startup templates, application modules, UI themes, tooling, guides and documentation.
https://abp.io
GNU Lesser General Public License v3.0
12.31k stars 3.32k forks source link

After upgrade from 7.3 to 8.0.4 The local bus event is not working on update data #19266

Closed abdullahshaqaliah closed 1 month ago

abdullahshaqaliah commented 2 months ago

Hi I have an issue after upgrading to 8 with the local bus event the event update is not working when updating data with other events like create or delete is working fine `public class TagEventHandler : ILocalEventHandler<EntityChangedEventData>, ITransientDependency { private readonly IDistributedCache<IEnumerable> _tagCache;

public TagEventHandler(IDistributedCache<IEnumerable<TagListDto>> tagCache)
{
    _tagCache = tagCache;
}
public async Task HandleEventAsync(
    EntityChangedEventData<Tag> eventData)
{
    await _tagCache.RemoveAsync(CMSServiceDomainCache.Tags);
}

} public class SupportPhoneNumberHandler : ILocalEventHandler<EntityChangedEventData>, ILocalEventHandler<EntityUpdatedEventData>, ITransientDependency { private readonly IDistributedCache<List> _supportPhoneNumbersCache;

public SupportPhoneNumberHandler(IDistributedCache<List<SupportPhoneNumbersCacheItem>> supportPhoneNumbersCache)
{
    _supportPhoneNumbersCache = supportPhoneNumbersCache;
}

public async Task HandleEventAsync(EntityChangedEventData<SupportPhoneNumber> eventData)
{
    await _supportPhoneNumbersCache.RemoveAsync(CMSServiceDomainCache.SupportPhoneNumbers);
}

public async Task HandleEventAsync(EntityUpdatedEventData<SupportPhoneNumber> eventData)
{
    await _supportPhoneNumbersCache.RemoveAsync(CMSServiceDomainCache.SupportPhoneNumbers);
}

} `

@maliming Can check this issue for me??

abdullahshaqaliah commented 2 months ago

The code for update I make as generic ` public virtual async Task UpdateAsync(TKey id, TUpdateInput input) { await CheckUpdatePolicyAsync().ConfigureAwait(false);

    using (IsActiveFilter.Disable())
    {
        var entity = await GetEntityByIdAsync(id);

        await OnUpdateExecutingAsync(input, entity).ConfigureAwait(false);

        await ApplyUpdateValidatorAsync(input, id).ConfigureAwait(false);

        await TryRemoveStorageWhenUpdateAsync(input, entity);

        await MapToEntityAsync(input, entity).ConfigureAwait(false);

        await OnUpdateMappingAsync(entity).ConfigureAwait(false);

        await Repository.UpdateAsync(entity, autoSave: true);

        await OnUpdateExecutedAsync(entity).ConfigureAwait(false);
        await OnExecutedAsync(entity).ConfigureAwait(false);

        return await MapToGetOutputDtoAsync(entity);
    }

}`
maliming commented 2 months ago

hi

Can you share a minimal project?

abdullahshaqaliah commented 2 months ago

@maliming I have the class public class SupportPhoneNumber : FullAuditedEntity<Guid>, IIsActive { public int? CountryId { get; set; } public Country Country { get; set; } public bool IsActive { get; set; } public bool IsGlobal { get; set; } public ICollection<CountryPhoneNumber> CountryPhoneNumbers { get; set; } } When updating the CountryPhoneNumbers the trigger local events are not fired when updating another property the update events will be fired so I think the problem with a list when updating the system does not know the property is updated

maliming commented 2 months ago

hi

Please share a minimal project.

agustinsilvano commented 2 months ago

@maliming Hi, I'm facing the same issue as described on this thread.

I have a minimal sample project here.

Calling the endpoint shown below must trigger the local event bus handler called MyHandler. That class implements ILocalEventHandler<EntityChangedEventData<Order>>. image

You should uncomment the creation line on the TestAppService to create the first entity and then just use that entity ID.

I tested it with dotnet 7 and ABP 7.3.3, and everything worked as expected.

After upgrading to dotnet 8 and ABP 8.0.4 the event handler is not being called anymore.

maliming commented 2 months ago

hi @agustinsilvano

You can make your app service methods virtual. I tested in a unit test.

public virtual async Task<bool> AddItemAsync()

public virtual async Task<bool> ChangeOwnerAsync()
agustinsilvano commented 2 months ago

@maliming it partially worked. 1 - I'm using DDD and I have an Order(aggregate root) and OrderItems(child entities). By making these methods virtual it only triggers the local event bus by modifying directly the Order property called OwnerId (Guid), by adding a new item to the OrderItems collection property of the Order aggregate the local event bus is not being hit(This is the same behavior that I'm facing in my real application code). Are we ok that it should trigger an event that must be handled with EntityChangedEventData<Order>?

2 -Is required to make these methods virtual? Is there any workaround? I have an application with several AppServices methods (aka endpoints) and I'd like to avoid having to modify every single method with the virtual keyword on it.

Thanks in advance.

maliming commented 2 months ago
  1. I have a PR that can resolve this https://github.com/abpframework/abp/pull/19079 Will be done in this week.
  2. Yes, It is better to make all methods virtual, which has many benefits.
agustinsilvano commented 2 months ago

@maliming cool.

Could you describe or is somewhere in the doc the benefits of using virtual methods?

maliming commented 1 month ago

If you are not injecting the service over an interface (like IMyService), then the methods of the service must be virtual (otherwise, dynamic proxy / interception system can not work).

https://docs.abp.io/en/abp/latest/Unit-Of-Work#iunitofworkenabled-interface

agustinsilvano commented 1 month ago

Tried to navigate to the dynamic proxy/ interception and still in progress. Not sure what that dynamic proxy/interceptor does at this level.

What changed from the last version that now requires having that virtual keyword on the method signature? It was working fine without the virtual keyword on v7.

agustinsilvano commented 1 month ago

@maliming I've been checking the #19079 and seems like you plan to release that in v8.2, it might take a while to get it live.

Is there a chance to release that as a fix for the v8.0?

maliming commented 1 month ago

hi

I will cherry-pick it to rel-8.0

maliming commented 1 month ago

Done by https://github.com/abpframework/abp/pull/19079

agustinsilvano commented 1 month ago

@maliming thanks!

What's the release plan for that? Want to know the version that I should use to get that fix applied.

maliming commented 1 month ago

hi @agustinsilvano

We will release the 8.0.5 asap.

agustinsilvano commented 2 weeks ago

@maliming did it got released? Seems like the fix got released with v8.1.1 but I'm still facing the same behavior. If one of the props of the root is modified the event EntityChangedEventData<TEntity> is emitted, but if a entity is added to the collection of the root that event is not being fired. Am I right?

EDIT: I tested also with v8.1.0 because I saw the PR on these release notes but still not working as it is on v7.3.3

agustinsilvano commented 2 weeks ago

@maliming adding more context to this.

I've the entity Order(AggregateRoot<Guid>) with a collection of OrderItems where each OrderItem(Entity) has a ref to the Order and the Item (Entity<Guid>). The class OrderItem is a key-less entity with the method GetKeys() overriden as

public override object[] GetKeys()
{
    return [OrderId, ItemId];
}

The interesting thing about is: When the OrderItem had items and it's updated to have zero items, the event EntityChangedEventData<Order> is emmited. Otherwise (i.e: the collection had 1 item and it's updated to have 2 items), the event is NOT emmited.

Again, this code is working perfectly with 7.3.3.

maliming commented 2 weeks ago

hi @agustinsilvano

You can test your project by latest source code reference.

agustinsilvano commented 2 weeks ago

@maliming you mean by the latest code on dev branch? Or tag 8.1.1 ?

maliming commented 2 weeks ago

hi

Latest code on dev or rel-8.0 branch

agustinsilvano commented 2 weeks ago

@maliming is out there a nuget package that contains the latest code in rel-8.0?

maliming commented 2 weeks ago

8.0.5 has not released yet, you can add source code references to test.

agustinsilvano commented 2 weeks ago

Hi @maliming as far as I can see 8.0.5 has been released, but on the notes your PR is not referenced there. Either the code added on your PR is not on 8.0.5, but it is referenced on 8.1.0 (and so on).

image

image

I've tested on 8.1.0 and 8.1.1 and the error is still present. Am I missing something?

Checking the test for that, seems like it should alternate between true and false, right? image

If any update triggers the local event bus, shouldnt that change the value of the variable?

maliming commented 2 weeks ago

hi

I may have misremembered. It should be 8.0.6.

you can share a test application, I'll test it in the source code