Add Spring Integration default poller auto-config #27992

artembilan · 2021-09-14T16:31:27Z

When polling consumers or source polling channel adapters
are used in Spring Integration applications, they require
some polling policy to be configured

Add some sensitive default bean PollerMetadata auto-configured
bean which can be overridden by end-user or customized via
respective newly added spring.integration.poller.* configuration
properties

For reference a recent StackOverflow question:
https://stackoverflow.com/questions/69175808

When polling consumers or source polling channel adapters are used in Spring Integration applications, they require some polling policy to be configured * Add some sensitive default bean `PollerMetadata` auto-configured bean which can be overridden by end-user or customized via respective newly added `spring.integration.poller.*` configuration properties For reference a recent StackOverflow question: https://stackoverflow.com/questions/69175808

wilkinsona

Thanks for the PR, @artembilan. I've left a few comments for your consideration when you have a moment.

wilkinsona · 2021-09-15T11:42:14Z

.../src/main/java/org/springframework/boot/autoconfigure/integration/IntegrationProperties.java

+		/**
+		 * How long to wait for messages on poll.
+		 */
+		private Duration receiveTimeout;


This field could have a default value that matches PollerMetadata.DEFAULT_RECEIVE_TIMEOUT. That would then make the default available in auto-complete when configuring the property.

Thank you Andy for feedback!
If I recall correctly, you or Stéphane pointed us in the past that it is better to keep configuration properties beans free from target libraries dependencies, so I decided to go with that sensitive defaults which comes with a PollerMetadata instance and what could be changed in the past if we would use values over here instead of those constant references. If that is still OK, I can change it as you suggest to point to those PollerMetadata constants.
Or I can use their values directly for time being. That's in the case if we don't go with nulls as I have done now.

We typically duplicate the default without referencing anything in the library. We can then test that our default matches the library's default, as you have already done in this case.

wilkinsona · 2021-09-15T11:44:12Z

.../src/main/java/org/springframework/boot/autoconfigure/integration/IntegrationProperties.java

+		/**
+		 * Maximum of messages to poll per polling cycle.
+		 */
+		private Long maxMessagesPerPoll;


This field could have a default value that matches PollerMetadata.MAX_MESSAGES_UNBOUNDED. That would then make the default available in auto-complete when configuring the property.

wilkinsona · 2021-09-15T11:46:34Z

.../src/main/java/org/springframework/boot/autoconfigure/integration/IntegrationProperties.java

+		private Duration initialDelay;
+
+		/**
+		 * Cron expression for polling.


It would be good to document that cron, fixedRate, and fixedDelay are mutually exclusive.

This is true.
In Stream Applications project we use @Validated feature to prevent ambiguity downstream and to avoid precedence as you pointed in the next comment: #27992 (comment).
The approach is like this: https://github.com/spring-cloud/stream-applications/blob/main/functions/consumer/log-consumer/src/main/java/org/springframework/cloud/fn/consumer/log/LogConsumerProperties.java#L57.
Since I don't see anything like that in Spring Boot by itself, I forgot to ask such a question when opened PR.

Thank you for reminder!
So, now let's discuss what is the best way to go.
I simply can validate exclusiveness in target code where I apply those properties, although it is still feels more nature to validate them on the configuration properties phase.
If you point me to something similar, I'll fix it ASAP

I've labeled this for discussion at a team meeting so that we can consider our options.

We're going to follow the precedent set by Framework's handling of @Scheduled and fail if more than one of cron, fixed-rate, and fixed-delay is set.

Good. I can do that. Any particular advice where to do validation?
I can just have an Assert.state() in that PollerMetadata bean definition.
Although it is not clear why you are talking about precedence if we are going to reject ambiguity anyway.

Thanks

Doing it in the PollerMetadata @Bean method sounds good to me. Generally, we try to keep the properties classes themselves as free of logic as possible.

Although it is not clear why you are talking about precedence

I was talking about the precedent set by Framework in how it handles similar properties on @Scheduled. Nothing to do with which property takes precedence over another. That was an option, but we've decided to fail now.

wilkinsona · 2021-09-15T11:46:42Z

.../src/main/java/org/springframework/boot/autoconfigure/integration/IntegrationProperties.java

+		private Duration fixedRate;
+
+		/**
+		 * Polling initial delay.


It would be good to document that cron, fixedRate, and fixedDelay are mutually exclusive.

wilkinsona · 2021-09-15T11:46:47Z

.../src/main/java/org/springframework/boot/autoconfigure/integration/IntegrationProperties.java

+		private Duration fixedDelay;
+
+		/**
+		 * Polling rate period.


It would be good to document that cron, fixedRate, and fixedDelay are mutually exclusive.

wilkinsona · 2021-09-15T11:48:34Z

...in/java/org/springframework/boot/autoconfigure/integration/IntegrationAutoConfiguration.java

+			map.from(poller::getCron).whenHasText().as(CronTrigger::new).to(pollerMetadata::setTrigger);
+			map.from(poller.getFixedDelay() != null ? poller.getFixedDelay() : poller.getFixedRate())
+					.as(Duration::toMillis).as(PeriodicTrigger::new).as((trigger) -> {
+						map.from(poller::getInitialDelay).as(Duration::toMillis).to(trigger::setInitialDelay);
+						trigger.setFixedRate(poller.getFixedRate() != null);
+						return trigger;
+					}).to(pollerMetadata::setTrigger);


I wonder how we should handle the user setting more than one of cron, fixedDelay, and fixedRate. As things stand it looks like the order of precedence is fixedDelay, fixedRate, cron. Should we document this ordering, fail when more than one is configured, or something else?

We've decided to fail.

* Document that `fixedDelay`, `fixedRate` and `cron` are mutually exclusive

`fixed-delay` & `fixed-rate` properties

When polling consumers or source polling channel adapters are used in Spring Integration applications, they require some polling policy to be configured. This comment auto-configures a PollerMetadata bean which customized via newly added `spring.integration.poller.*` configuration properties or overriden completely be user-defined bean. See spring-projectsgh-27992

See spring-projectsgh-27992

When polling consumers or source polling channel adapters are used in Spring Integration applications, they require some polling policy to be configured. This comment auto-configures a PollerMetadata bean which customized via newly added `spring.integration.poller.*` configuration properties or overriden completely be user-defined bean. See gh-27992

See gh-27992 Co-authored-by: Phillip Webb <pwebb@vmware.com>

Starting with Spring Boot `2.6`, the auto-configuration for Spring Integration introduces `poller` properties. The `DefaultPollerProperties` and its respective `ChannelBindingAutoConfiguration` are not needed any more. * Add `PollerConfigEnvironmentPostProcessor` to remap deprecated `spring.cloud.stream.poller` into respective `spring.integration.poller` properties * Mention in the docs a deprecation move and its replacement Related to: spring-projects/spring-boot#27992

Starting with Spring Boot `2.6`, the auto-configuration for Spring Integration introduces `poller` properties. The `DefaultPollerProperties` and its respective `ChannelBindingAutoConfiguration` are not needed any more. * Add `PollerConfigEnvironmentPostProcessor` to remap deprecated `spring.cloud.stream.poller` into respective `spring.integration.poller` properties * Mention in the docs a deprecation move and its replacement Related to: spring-projects/spring-boot#27992 Resolves #2233

spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 14, 2021

wilkinsona added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 15, 2021

wilkinsona added this to the 2.6.x milestone Sep 15, 2021

wilkinsona requested changes Sep 15, 2021

View reviewed changes

wilkinsona added the for: team-meeting An issue we'd like to discuss as a team to make progress label Sep 15, 2021

artembilan added 2 commits September 15, 2021 09:43

* Add defaults for maxMessagesPerPoll and receiveTimeout properties

874158d

* Document that `fixedDelay`, `fixedRate` and `cron` are mutually exclusive

* Fix Checkstyle violations

1800c1a

wilkinsona removed the for: team-meeting An issue we'd like to discuss as a team to make progress label Sep 17, 2021

* Add validation for exclusiveness of cron,

0244241

`fixed-delay` & `fixed-rate` properties

wilkinsona mentioned this pull request Sep 23, 2021

Provide a specific exception and failure analysis for situations where more than one mutually exclusive configuration property has been configured #28121

Closed

wilkinsona added a commit to wilkinsona/spring-boot that referenced this pull request Sep 23, 2021

Polish "Add Spring Integration default poller auto-config"

328374b

See spring-projectsgh-27992

philwebb added a commit that referenced this pull request Sep 23, 2021

Polish "Add Spring Integration default poller auto-config"

3274e24

See gh-27992 Co-authored-by: Phillip Webb <pwebb@vmware.com>

philwebb closed this in 5bd34be Sep 23, 2021

philwebb modified the milestones: 2.6.x, 2.6.0-M3 Sep 23, 2021

artembilan mentioned this pull request Oct 7, 2021

Remove poller props in favor of Spring Boot spring-cloud/spring-cloud-stream#2233

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add Spring Integration default poller auto-config #27992

Add Spring Integration default poller auto-config #27992

Uh oh!

artembilan commented Sep 14, 2021

Uh oh!

wilkinsona left a comment

Uh oh!

wilkinsona Sep 15, 2021

Uh oh!

artembilan Sep 15, 2021

Uh oh!

wilkinsona Sep 15, 2021

Uh oh!

wilkinsona Sep 15, 2021

Uh oh!

wilkinsona Sep 15, 2021

Uh oh!

artembilan Sep 15, 2021 •

edited

Loading

Uh oh!

wilkinsona Sep 15, 2021

Uh oh!

wilkinsona Sep 17, 2021

Uh oh!

artembilan Sep 17, 2021

Uh oh!

wilkinsona Sep 17, 2021

Uh oh!

wilkinsona Sep 15, 2021

Uh oh!

wilkinsona Sep 15, 2021

Uh oh!

wilkinsona Sep 15, 2021

Uh oh!

wilkinsona Sep 17, 2021

Uh oh!

Uh oh!

Add Spring Integration default poller auto-config #27992

Add Spring Integration default poller auto-config #27992

Uh oh!

Conversation

artembilan commented Sep 14, 2021

Uh oh!

wilkinsona left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

artembilan Sep 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

artembilan Sep 15, 2021 •

edited

Loading