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

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

Closed
abdullahshaqaliah opened this issue Mar 12, 2024 · 25 comments
Assignees
Milestone

Comments

@abdullahshaqaliah
Copy link

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
Copy link
Author

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
Copy link
Member

hi

Can you share a minimal project?

@maliming maliming self-assigned this Mar 13, 2024
@abdullahshaqaliah
Copy link
Author

@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
Copy link
Member

hi

Please share a minimal project.

@agustinsilvano
Copy link

@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
Copy link
Member

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
Copy link

@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
Copy link
Member

  1. I have a PR that can resolve this Publish EntityUpdatedEvent when navigation changes. #19079 Will be done in this week.
  2. Yes, It is better to make all methods virtual, which has many benefits.

@agustinsilvano
Copy link

@maliming cool.

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

@maliming
Copy link
Member

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
Copy link

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
Copy link

@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
Copy link
Member

hi

I will cherry-pick it to rel-8.0

@maliming
Copy link
Member

Done by #19079

@maliming maliming added this to the 8.0-patch milestone Apr 11, 2024
@agustinsilvano
Copy link

@maliming thanks!

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

@maliming
Copy link
Member

hi @agustinsilvano

We will release the 8.0.5 asap.

@agustinsilvano
Copy link

agustinsilvano commented Apr 29, 2024

@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
Copy link

@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
Copy link
Member

hi @agustinsilvano

You can test your project by latest source code reference.

@agustinsilvano
Copy link

agustinsilvano commented Apr 30, 2024

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

@maliming
Copy link
Member

hi

Latest code on dev or rel-8.0 branch

@agustinsilvano
Copy link

agustinsilvano commented Apr 30, 2024

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

@maliming
Copy link
Member

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

@agustinsilvano
Copy link

agustinsilvano commented Apr 30, 2024

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
Copy link
Member

maliming commented May 1, 2024

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

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

No branches or pull requests

3 participants