abp icon indicating copy to clipboard operation
abp copied to clipboard

Do not auto discover generic event handlers

Open PMExtra opened this issue 2 years ago • 10 comments

Description

Adding a GenericTypeDefinition (a generic type without arguments) to the AbpLocalEventBusOptions.Handlers or AbpDistributedEventBusOptions.Handlers makes no sense.

It cannot be determined which event should be subscribed.

And it makes KafkaDistributedEventBus broken:

Volo.Abp.AbpInitializationException: An error occurred during the initialize Volo.Abp.Modularity.OnApplicationInitializationModuleLifecycleContributor phase of the module Volo.Abp.EventBus.Kafka.AbpEventBusKafkaModule, Volo.Abp.EventBus.Kafka, Version=7.2.1.0, Culture=neutral, PublicKeyToken=null: Value cannot be null. (Parameter 'key'). See the inner exception for details.
 ---> System.ArgumentNullException: Value cannot be null. (Parameter 'key')
   at System.ThrowHelper.ThrowArgumentNullException(String name)
   at System.Collections.Concurrent.ConcurrentDictionary`2.set_Item(TKey key, TValue value)
   at Volo.Abp.EventBus.Kafka.KafkaDistributedEventBus.<GetOrCreateHandlerFactories>b__38_0(Type type)
   at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd(TKey key, Func`2 valueFactory)
   at Volo.Abp.EventBus.Kafka.KafkaDistributedEventBus.GetOrCreateHandlerFactories(Type eventType)
   at Volo.Abp.EventBus.Kafka.KafkaDistributedEventBus.Subscribe(Type eventType, IEventHandlerFactory factory)
   at Volo.Abp.EventBus.EventBusBase.SubscribeHandlers(ITypeList`1 handlers)
   at Volo.Abp.EventBus.Kafka.KafkaDistributedEventBus.Initialize()
   at Volo.Abp.EventBus.Kafka.AbpEventBusKafkaModule.OnApplicationInitialization(ApplicationInitializationContext context)
   at Volo.Abp.Modularity.AbpModule.OnApplicationInitializationAsync(ApplicationInitializationContext context)
   at Volo.Abp.Modularity.OnApplicationInitializationModuleLifecycleContributor.InitializeAsync(ApplicationInitializationContext context, IAbpModule module)
   at Volo.Abp.Modularity.ModuleManager.InitializeModulesAsync(ApplicationInitializationContext context)

Checklist

  • [x] I fully tested it as developer / designer and created unit / integration tests
  • [x] I documented it (or no need to document or I will create a separate documentation issue)

How to test it?

Take the MyGenericDistributedEventHandler class to your tests with Kafka Event Bus, then register it to your service collection like EventBusTestModule.

It would be broken without this patch.

PMExtra avatar Aug 24 '23 07:08 PMExtra

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Aug 24 '23 07:08 CLAassistant

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 53.29%. Comparing base (72d69d3) to head (e1e0342). Report is 2959 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev   #17465      +/-   ##
==========================================
- Coverage   53.34%   53.29%   -0.05%     
==========================================
  Files        3024     3024              
  Lines       94357    94362       +5     
==========================================
- Hits        50334    50292      -42     
- Misses      44023    44070      +47     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Aug 24 '23 08:08 codecov[bot]

Is there any actual reason to use such a class?

public class MyGenericDistributedEventHandler<TEvent> : IDistributedEventHandler<TEvent>

maliming avatar Aug 24 '23 08:08 maliming

Yes, we have an options to set which events should be handled by this handler.

PMExtra avatar Aug 24 '23 09:08 PMExtra

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Dec 15 '23 04:12 stale[bot]

This issue is still unresolved. Waiting for the staff's response.

PMExtra avatar Dec 15 '23 05:12 PMExtra

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Mar 17 '24 13:03 stale[bot]

@maliming

PMExtra avatar Mar 18 '24 10:03 PMExtra

https://github.com/abpframework/abp/pull/17465/files#r1305402874

maliming avatar Mar 19 '24 01:03 maliming

@maliming 非要在单元测试里复现的话,那需要ABP先提供一个KafkaEventBus的单元测试

PMExtra avatar Mar 19 '24 05:03 PMExtra