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

EventProcessingConfigurer should expose registered configurations #2457

Open
nils-christian opened this issue Oct 27, 2022 · 3 comments
Open
Labels
Ideal for Contribution Priority 4: Would Lowest priority. Would-be-nice to include issues when time allows it. Type: Enhancement Use to signal an issue enhances an already existing feature of the project.

Comments

@nils-christian
Copy link
Contributor

Hi,

In our application we want to use a custom EventProcessorBuilder. However, the builder is not able to access the event processor configurations registered at the EventProcessingConfigurer, making it difficult to correctly configure the new event processor.

Enhancement Description

Either the EventProcessingConfigurer should somehow expose the registered configurations via methods (compatible API change) or the EventProcessorBuilder should get the configuration of the event processor to build as parameter (incompatible API change).

Current Behaviour

Please take a look at the following code. Note that MyEventProcessorBuilder cannot access the previously registered configuration for MyEventProcessor.

@Autowired
void configure( final EventProcessingConfigurer configurer ) {
	// This code is usually somewhere else in the application.
	configurer.registerTrackingEventProcessorConfiguration( "MyEventProcessor", c -> TrackingEventProcessorConfiguration.forParallelProcessing( 4 ) );
}

@Autowired
void registerEventProcessorFactory( final EventProcessingConfigurer configurer ) {
	configurer.registerEventProcessorFactory( new MyEventProcessorBuilder( ) );
}

private static final class MyEventProcessorBuilder implements EventProcessorBuilder {

	@Override
	public EventProcessor build( final String name, final Configuration configuration, final EventHandlerInvoker eventHandlerInvoker ) {
		final List<EventProcessingModule> eventProcessingModules = configuration.findModules( EventProcessingModule.class );
		final EventProcessingModule eventProcessingModule = eventProcessingModules.get( 0 );
		@SuppressWarnings( "unchecked" )
		final StreamableMessageSource<TrackedEventMessage<?>> messageSource = ( StreamableMessageSource<TrackedEventMessage<?>> ) configuration.eventBus( );

		// In out application we are actually building a more specific event processor
		return TrackingEventProcessor
				.builder( )
				.name( name )
				.eventHandlerInvoker( eventHandlerInvoker )
				.rollbackConfiguration( eventProcessingModule.rollbackConfiguration( name ) )
				.errorHandler( eventProcessingModule.errorHandler( name ) )
				.messageMonitor( eventProcessingModule.messageMonitor( TrackingEventProcessor.class, name ) )
				.messageSource( messageSource )
				.tokenStore( eventProcessingModule.tokenStore( name ) )
				.transactionManager( eventProcessingModule.transactionManager( name ) )
				.trackingEventProcessorConfiguration(
						// TODO: We cannot access the configuration from the EventProcessingModule
						TrackingEventProcessorConfiguration.forParallelProcessing( 4 ) )
				.build( );
	}

}

Wanted Behaviour

private static final class MyEventProcessorBuilder implements EventProcessorBuilder {

		@Override
		public EventProcessor build( final String name, final Configuration configuration, final TrackingEventProcessorConfiguration tepConfiguration, final EventHandlerInvoker eventHandlerInvoker ) {
			final List<EventProcessingModule> eventProcessingModules = configuration.findModules( EventProcessingModule.class );
			final EventProcessingModule eventProcessingModule = eventProcessingModules.get( 0 );
			@SuppressWarnings( "unchecked" )
			final StreamableMessageSource<TrackedEventMessage<?>> messageSource = ( StreamableMessageSource<TrackedEventMessage<?>> ) configuration.eventBus( );

			// In out application we are actually building a more specific event processor
			return TrackingEventProcessor
					.builder( )
					.name( name )
					.eventHandlerInvoker( eventHandlerInvoker )
					.rollbackConfiguration( eventProcessingModule.rollbackConfiguration( name ) )
					.errorHandler( eventProcessingModule.errorHandler( name ) )
					.messageMonitor( eventProcessingModule.messageMonitor( TrackingEventProcessor.class, name ) )
					.messageSource( messageSource )
					.tokenStore( eventProcessingModule.tokenStore( name ) )
					.transactionManager( eventProcessingModule.transactionManager( name ) )
					.trackingEventProcessorConfiguration( tepConfiguration )
					.build( );
		}

	}

Possible Workarounds

Developing a custom EventProcessingConfigurer which delegates to the original EventProcessingConfigurer, just to store the configuration.

@nils-christian nils-christian added the Type: Enhancement Use to signal an issue enhances an already existing feature of the project. label Oct 27, 2022
@smcvb
Copy link
Member

smcvb commented Oct 27, 2022

Hi @nils-christian! Just to be clear here, the sole thing you're missing from the Configuration -> EventProcessingConfiguration is the TrackingEventProcessorConfiguration?
Or are there other components you'd need access to?

@smcvb smcvb added the Status: Information Required Use to signal this issue is waiting for information to be provided in the issue's description. label Oct 27, 2022
@nils-christian
Copy link
Contributor Author

Hi @smcvb,

In our use case we are just missing the TrackingEventProcessorConfiguration, correct.

@smcvb
Copy link
Member

smcvb commented Oct 27, 2022

Awesome, thanks! That should be simple enough to expose through the EventProcessingConfiguration, in a similar fashion as the other components.
I would expect the JavaDoc to specify the intent for it to be used within the EventProcessorBuilder, by the way.

Nonetheless, fair request here, @nils-christian 👍
Would you perhaps be up for a PR for this?
As it's a enhancement, it will be part of 4.7.0, by the way.
And I just want to add, we are not planning to withhold a release of 4.7.0 for as long as we did with 4.6.0. 😅

@smcvb smcvb added Priority 4: Would Lowest priority. Would-be-nice to include issues when time allows it. Ideal for Contribution and removed Status: Information Required Use to signal this issue is waiting for information to be provided in the issue's description. labels Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ideal for Contribution Priority 4: Would Lowest priority. Would-be-nice to include issues when time allows it. Type: Enhancement Use to signal an issue enhances an already existing feature of the project.
Projects
None yet
Development

No branches or pull requests

2 participants