Skip to content

GH-3155: Add support for Java DSL extensions #3167

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

Merged
merged 5 commits into from
Feb 7, 2020

Conversation

artembilan
Copy link
Member

Fixes #3155

Provide an IntegrationFlowExtension for possible custom EI-operators
in the target project use-cases.

@artembilan
Copy link
Member Author

artembilan commented Feb 5, 2020

@sirimamilla,

Please, take a look into this.
That's about your request for Java DSL extensions.

Pay attention to the test class changes:

public static class CustomIntegrationFlowDefinition
			extends IntegrationFlowExtension<CustomIntegrationFlowDefinition> {

		public CustomIntegrationFlowDefinition customEip() {
			return split()
					.transform("payload.toUpperCase()");
		}

	}

This is how your write your extension with any possible custom operators.
And this is how you need to use it:

@Bean
public IntegrationFlow customFlowDefinition() {
	return
		new CustomIntegrationFlowDefinition()
			.customEip()
			.aggregate()
			.get();
}

I'm not sure that we can come up with something much simpler.

I intentionally made it as a separate IntegrationFlowExtension<B extends IntegrationFlowExtension<B>> extends IntegrationFlowDefinition<B> class to make the statement as clear as possible: if you want an extension, please, implement your specific class. And this one is an extension of existing IntegrationFlowDefinition so you get access to all its methods for build any possible custom EI-operators.

Let me know WDYT?

/CC @garyrussell

@sirimamilla
Copy link
Contributor

Hi @artembilan,
I will try this approach once and I will get back to you.

@garyrussell
Copy link
Contributor

LGTM; nice!

@artembilan
Copy link
Member Author

Thanks, Gary!

Let's wait for feedback from @sirimamilla anyway!

I see that some methods has to be moved to protected instead of private.
Plus might be the case that we need to make all the IntegrationComponentSpec implementation ctors as protected as well.
Let's imagine that we have missed some option to expose on the spec or we have a bug which is easy to overcome with an extension and override.
So, this way a custom operator against custom spect is going to be a good combination 😄

@garyrussell
Copy link
Contributor

Let's wait ...

Agreed; I was just saying the approach looks good.

@sirimamilla
Copy link
Contributor

Hi @artembilan,

This working as expected for the composite EIP patterns where in we need to extend the existing patterns. This is what I wanted to be able to do.

Let's imagine that we have missed some option to expose on the spec or we have a bug which is easy to overcome with an extension and override.

I agree, This would be good enhancement, as we may think some option is required for application but that may not fit good for framework itself. those features can be added using this

@sirimamilla
Copy link
Contributor

Overall I am amazed by the way @artembilan solved this complex problem with very simple and effective solution.

@sirimamilla
Copy link
Contributor

I was able to use the composite eip pattern and able to solve the requirement of selector on Gateway.

public <P> CustomIntegrationFlowDefinition selectingGateway(String channelName, Class<P> type, GenericSelector<P> selector) {
			MessageChannel aggregateChannel = new DirectChannel();
			return filter(type, selector, e -> e.discardChannel(aggregateChannel))
					.gateway(channelName)
					.channel(aggregateChannel);
		}

@artembilan
Copy link
Member Author

Thanks, @sirimamilla , for feedback!

So, stay tuned and I'll finish this PR today!

I'm not sure that it will address all the possible extension peculiarities, but we can address them eventually (if needed)

@artembilan artembilan changed the title [DO NOT MERGE YET] GH-3155: Add support for Java DSL extensions GH-3155: Add support for Java DSL extensions Feb 6, 2020
Fixes spring-projects#3155

Provide an `IntegrationFlowExtension` for possible custom EI-operators
in the target project use-cases.
* Make all the `IntegrationComponentSpec` ctors as `protected` for possible custom extensions
* Make some `BaseIntegrationFlowDefinition` methods and properties as `protected` to get them
access from the `IntegrationFlowExtension` implementations
* Document the feature
@artembilan
Copy link
Member Author

All docs fixed and rebased to master.

Thanks, Gary!

Copy link
Contributor

@sirimamilla sirimamilla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HI @artembilan,

GatewayEndpointSpec(String requestChannel) can be made protected as well

GatewayEndpointSpec(String requestChannel) {
		super(new GatewayMessageHandler());
		this.handler.setRequestChannelName(requestChannel);
	}

@artembilan
Copy link
Member Author

GatewayEndpointSpec(String requestChannel) can be made protected as well

Good catch! Thank you! Anything else ? 😉

@sirimamilla
Copy link
Contributor

Good catch! Thank you! Anything else ? 😉

super(new GatewayMessageHandler()); please expose a constructor with this argument as protected. So extension classes will be able to extend the handler.

@artembilan
Copy link
Member Author

So extension classes will be able to extend the handler.

I don't think so. That GatewayMessageHandler has a restricted responsibility to handle mid-flow requests. Nothing more.
I definitely not sure to what you can extend it.
But with the custom GatewayMessageHandler you are feel free to implement a custom ConsumerEndpointSpec on the matter similar to that GatewayEndpointSpec.

We talked about options to increase - the new component requires new contract and, therefore, new Spec. However with a new MessageHandler you might even don't need new spec - the .handle() fully covers you.

Please, elaborate more what is your goal to be able to extend that GatewayMessageHandler and therefore GatewayEndpointSpec?

* Add JavaDocs to `GatewayEndpointSpec` methods
@sirimamilla
Copy link
Contributor

Hi @artembilan,

Here is an example of the use of allowing GatewayMessageHandler be extended. This will act as Selecting Gateway. It will preserve all the gateway behaviors along with the additional filter capability.

public static class CustomIntegrationFlowDefinition
			extends IntegrationFlowExtension<CustomIntegrationFlowDefinition> {

		public CustomIntegrationFlowDefinition upperCaseAfterSplit() {
			return split()
					.transform("payload.toUpperCase()");
		}

		public CustomIntegrationFlowDefinition customAggregate(Consumer<CustomAggregatorSpec> aggregator) {
			return register(new CustomAggregatorSpec(), aggregator);
		}

		public CustomIntegrationFlowDefinition selectingGateway(MessageChannel channel,
														Consumer<CustomGatewayEndpointSpec> endpointConfigurer) {
			return register(new CustomGatewayEndpointSpec(channel), endpointConfigurer);

		}

	}

	public static class CustomGatewayEndpointSpec extends GatewayEndpointSpec  {

		protected CustomGatewayEndpointSpec(MessageChannel requestChannel) {
			super(requestChannel);
			this.handler = new CustomGatewayMessageHandler();
			handler.setRequestChannel(requestChannel);

		}

		public CustomGatewayEndpointSpec selector(Predicate<Message<?>> condition) {
			if (this.handler instanceof CustomGatewayMessageHandler) {
				((CustomGatewayMessageHandler) this.handler).setCondition(condition);
			}
			return this;
		}
	}


	public static class CustomGatewayMessageHandler extends GatewayMessageHandler {

		Predicate<Message<?>> condition;

		public void setCondition(Predicate<Message<?>> condition) {
			this.condition = condition;
		}

		@Override
		protected Object handleRequestMessage(Message<?> requestMessage) {

			return condition.test(requestMessage) ? super.handleRequestMessage(requestMessage) : requestMessage;
		}
	}

Selecting Gateway can be used as below.

@Bean
		public IntegrationFlow selectingGateway() {
			return new CustomIntegrationFlowDefinition()
					.selectingGateway(customFlowDefinition().getInputChannel(),
							g -> g.selector(m -> m.getHeaders().containsKey("pass")))
					.get();
		}

This will help in doing any further enhancements to the GatewayMessageHandler.

@sirimamilla
Copy link
Contributor

Hi @artembilan,

Protected actor can be added to below.

JmsPollableMessageChannelSpec(JmsChannelFactoryBean jmsChannelFactoryBean, ConnectionFactory connectionFactory) {
		this.jmsChannelFactoryBean = jmsChannelFactoryBean;
		this.jmsChannelFactoryBean.setConnectionFactory(connectionFactory);
		this.jmsChannelFactoryBean.setSingleton(false);
		this.jmsChannelFactoryBean.setBeanFactory(new DefaultListableBeanFactory());
	}

@artembilan
Copy link
Member Author

This will help in doing any further enhancements to the GatewayMessageHandler.

That's abusing a purpose of all the components we have out-of-the-box. It is not preserving, it is some hybrid I'd like to avoid.
You feel free to do such a "hell" in your own project, but I'm not going to encourage all users of our project to follow your idea on the matter.
Again: such a "selective gateway" can be implemented with existing components in some separate flow which could be referenced from other flows.

Sorry not accepting your request, but I'm fully against some hybrids when go beyond EIP catalog.

The JmsPollableMessageChannelSpec feedback is accepted 😄

@sirimamilla
Copy link
Contributor

That's abusing a purpose of all the components we have out-of-the-box. It is not preserving, it is some hybrid I'd like to avoid.

Even I agree with you, from EIP catalog this is an extension.

I will implement this in my application.

For this PR, I think we are almost ready to merge. No further comments from me.

Copy link
Contributor

@sirimamilla sirimamilla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @artembilan ,

As reviewed, PR is performing the tasks as expected and can be merged. :)

@artembilan
Copy link
Member Author

and can be merged

So, @garyrussell , the final decision is up to you 😉

@garyrussell garyrussell merged commit 867a8cf into spring-projects:master Feb 7, 2020
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

Successfully merging this pull request may close these issues.

Consider to Support Extentions with BaseIntegrationFlowDefinition/IntegrationFlowDefinition as base classes
3 participants