Provide auto-configuration for spring-rabbit-stream #27480

garyrussell · 2021-07-23T18:11:50Z

new container factory (not in existing hierarchy)
also add ContainerCustomizer to other factories

wilkinsona · 2021-07-23T18:52:45Z

spring-boot-project/spring-boot-dependencies/build.gradle

@@ -1416,13 +1423,20 @@ bom {
 			]
 		}
 	}
-	library("Rabbit AMQP Client", "5.13.0") {
+	library("RabbitMQ AMQP Client", "5.13.0") {


This will result in a breaking change to the version property. It's rabbit-amqp-client.version at the moment and this will change it to rabbitmq-amqp-client.version. I'd prefer to leave it unchanged, but if we want to change it, it should be done in a separate issue.

That's fine; I just wanted to make it consistent; will revert.

wilkinsona · 2021-07-23T18:54:56Z

...src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitStreamAutoConfiguration.java

+@Configuration(proxyBeanMethods = false)
+@ConditionalOnClass(EnableRabbit.class)
+@ConditionalOnProperty(prefix = "spring.rabbitmq.listener", name = "type", havingValue = "stream")
+public class RabbitStreamAutoConfiguration {


We only name classes …AutoConfiguration if they're listed under the EnableAutoConfiguration key in spring.factories. This should be named RabbitStreamConfiguration or similar and should be package-private.

wilkinsona · 2021-07-23T18:55:58Z

...est/java/org/springframework/boot/autoconfigure/amqp/RabbitStreamAutoConfigurationTests.java

+ *
+ * @author Gary Russell
+ */
+@ExtendWith(OutputCaptureExtension.class)


Is this needed? I can't see the captured output being used anywhere.

Copy/paste artifact 😦

wilkinsona · 2021-07-23T18:57:43Z

spring-boot-project/spring-boot-dependencies/build.gradle

 		group("com.rabbitmq") {
 			modules = [
 				"amqp-client"
 			]
 		}
 	}
+	library("RabbitMQ Stream Client", "0.2.0-SNAPSHOT") {


Is there an ETA for this having a tagged release? We don't want to be on a snapshot for too long and also wouldn't want to have to revert if the release isn't available before our next milestone.

I will ask them when it is expected to be released, spring-rabbit-stream 2.4.0-M1 uses 0.1.0 (not a snapshot), but this needs a new feature in 0.2.0.

@wilkinsona It will be released sometime next week.

https://pivotal.slack.com/archives/CSC92F50B/p1627071597374400?thread_ts=1627070500.374200&cid=CSC92F50B

Excellent. Given that it's not long, let's wait till the release rather than adding the Sonatype snapshot repo.

wilkinsona · 2021-07-26T12:34:02Z

spring-boot-project/spring-boot-autoconfigure/build.gradle

@@ -58,6 +58,7 @@ dependencies {
 	optional("org.apache.commons:commons-dbcp2")
 	optional("org.apache.httpcomponents.client5:httpclient5")
 	optional("org.apache.kafka:kafka-streams")
+	optional("org.apache.qpid:proton-j")


I'm struggling to spot why this is needed so I tried removing it. Everything compiled cleanly. If it isn't needed we can get rid of the dependency management too.

We could move it to testImplementation but the stream-client is practically unusable without it; Spring Initializr would have to add it anyway - I don't know why it is not a transitive dependency of the client. I can start a discussion with them.

... Caused by: org.springframework.beans.BeanInstantiationException: Failed to instantiate [com.rabbitmq.stream.Environment]: Factory method 'rabbitStreamEnvironment' threw exception; nested exception is java.lang.NoClassDefFoundError: org/apache/qpid/proton/amqp/messaging/Section at org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:185) at org.springframework.beans.factory.support.ConstructorResolver.instantiate(ConstructorResolver.java:653) ... 128 more Caused by: java.lang.NoClassDefFoundError: org/apache/qpid/proton/amqp/messaging/Section

Yes, please. I'd rather it just comes in transitively than us having to manage the version and keep things in sync.

@acogoluegnes Agreed: rabbitmq/rabbitmq-stream-java-client#18

wilkinsona

Thanks for the updates, Gary. I've left a few more comments.

I'm not sure about the need for the changes to RabbitAnnotationDrivenConfiguration. They feel separate to adding spring-rabbit-stream support. If that's right, I'd prefer that we make them in a separate enhancement.

wilkinsona · 2021-08-10T09:19:13Z

...utoconfigure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitProperties.java

@@ -1128,4 +1165,48 @@ private boolean determineSslEnabled(boolean sslEnabled) {

 	}

+	public static final class Stream {
+
+		private String host = "localhost";


This property needs some javadoc that'll be captured in the configuration property metadata.

wilkinsona · 2021-08-10T09:19:20Z

...utoconfigure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitProperties.java

+
+		private String host = "localhost";
+
+		private int port = DEFAULT_STREAM_PORT;


This property needs some javadoc that'll be captured in the configuration property metadata.

wilkinsona · 2021-08-10T09:19:26Z

...utoconfigure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitProperties.java

+
+		private int port = DEFAULT_STREAM_PORT;
+
+		private String userName;


This property needs some javadoc that'll be captured in the configuration property metadata.

wilkinsona · 2021-08-10T09:19:32Z

...utoconfigure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitProperties.java

+
+		private String userName;
+
+		private String password;


This property needs some javadoc that'll be captured in the configuration property metadata.

wilkinsona · 2021-08-10T09:23:28Z

...ure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitStreamConfiguration.java

+	@Bean(name = "rabbitListenerContainerFactory")
+	@ConditionalOnMissingBean(name = "rabbitListenerContainerFactory")
+	StreamRabbitListenerContainerFactory streamRabbitListenerContainerFactory(Environment rabbitStreamEnvironment,
+			RabbitProperties properties, ObjectProvider<ConsumerCustomizer> consumerCustomizers,


Suggested change

RabbitProperties properties, ObjectProvider<ConsumerCustomizer> consumerCustomizers,

RabbitProperties properties, ObjectProvider<ConsumerCustomizer> consumerCustomizer,

I think this makes sense given that the setter is singular.

wilkinsona · 2021-08-10T09:25:15Z

...ure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitStreamConfiguration.java

+		if (containerCustomizers.getIfUnique() != null) {
+			factory.setContainerCustomizer(containerCustomizers.getIfUnique());
+		}


Suggested change

if (containerCustomizers.getIfUnique() != null) {

factory.setContainerCustomizer(containerCustomizers.getIfUnique());

}

containerCustomizer.ifUnique(factory::setContainerCustomizer);

wilkinsona · 2021-08-10T09:29:29Z

...utoconfigure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitProperties.java

@@ -871,6 +894,20 @@ public void setMissingQueuesFatal(boolean missingQueuesFatal) {

 	}

+	public static class StreamContainer extends BaseContainer {
+
+		boolean nativeListener;


This property needs some javadoc that'll be captured in the configuration property metadata.

wilkinsona · 2021-08-10T09:30:30Z

...ure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitStreamConfiguration.java

+	@ConditionalOnMissingBean(name = "rabbitListenerContainerFactory")
+	StreamRabbitListenerContainerFactory streamRabbitListenerContainerFactory(Environment rabbitStreamEnvironment,
+			RabbitProperties properties, ObjectProvider<ConsumerCustomizer> consumerCustomizers,
+			ObjectProvider<ContainerCustomizer<StreamListenerContainer>> containerCustomizers) {


Suggested change

ObjectProvider<ContainerCustomizer<StreamListenerContainer>> containerCustomizers) {

ObjectProvider<ContainerCustomizer<StreamListenerContainer>> containerCustomizer) {

I think this makes sense given that the setter is singular.

wilkinsona · 2021-08-10T09:31:53Z

...ure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitStreamConfiguration.java

+ * @since 2.6
+ */
+@Configuration(proxyBeanMethods = false)
+@ConditionalOnClass(EnableRabbit.class)
+@ConditionalOnProperty(prefix = "spring.rabbitmq.listener", name = "type", havingValue = "stream")
+public class RabbitStreamConfiguration {


Suggested change

* @since 2.6

*/

@Configuration(proxyBeanMethods = false)

@ConditionalOnClass(EnableRabbit.class)

@ConditionalOnProperty(prefix = "spring.rabbitmq.listener", name = "type", havingValue = "stream")

public class RabbitStreamConfiguration {

*/

@Configuration(proxyBeanMethods = false)

@ConditionalOnClass(EnableRabbit.class)

@ConditionalOnProperty(prefix = "spring.rabbitmq.listener", name = "type", havingValue = "stream")

class RabbitStreamConfiguration {

wilkinsona · 2021-08-10T09:32:47Z

...ure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitStreamConfiguration.java

+ * @since 2.6
+ */
+@Configuration(proxyBeanMethods = false)
+@ConditionalOnClass(EnableRabbit.class)


Does this need to be conditional on StreamRabbitListenerContainerFactory or similar as well?

wilkinsona · 2021-08-10T09:49:13Z

spring-boot-project/spring-boot-dependencies/build.gradle

@@ -1646,11 +1653,12 @@ bom {
 			]
 		}
 	}
-	library("Spring AMQP", "2.4.0-M1") {
+	library("Spring AMQP", "2.4.0-SNAPSHOT") {


When's 2.4.0-M2 due? I can't see it in the release calendar at the moment.

Aug 16 - scheduled.

- new container factory (not in existing hierarchy) - also add `ContainerCustomizer` to other factories

See gh-27480

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

wilkinsona reviewed Jul 23, 2021

View reviewed changes

wilkinsona reviewed Jul 26, 2021

View reviewed changes

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

wilkinsona added this to the 2.6.x milestone Jul 26, 2021

wilkinsona added the status: on-hold We can't start working on this issue yet label Jul 26, 2021

snicoll removed the status: on-hold We can't start working on this issue yet label Jul 28, 2021

snicoll changed the title ~~Initial Auto Config for spring-rabbit-stream~~ Auto-Configuration Support for spring-rabbit-stream Aug 6, 2021

wilkinsona self-assigned this Aug 10, 2021

wilkinsona requested changes Aug 10, 2021

View reviewed changes

wilkinsona added the status: waiting-for-feedback We need additional information before we can continue label Aug 10, 2021

wilkinsona reviewed Aug 10, 2021

View reviewed changes

garyrussell added 7 commits August 10, 2021 09:06

Initial Auto Config for spring-rabbit-stream

b6183bd

- new container factory (not in existing hierarchy) - also add `ContainerCustomizer` to other factories

Address PR Comments.

6323507

Upgrade stream-client to 0.2.0; remove ProtonJ dependency.

6741378

Remove Sonatype snapshot repo.

841c534

Upgrade stream-client to 0.3.0.

10a8156

Remove unused imports.

88ee436

Address PR Comments; move unrelated changes to a different PR.

9871270

garyrussell force-pushed the rabbitMQStream branch from 3032d9c to 9871270 Compare August 10, 2021 13:39

wilkinsona removed the status: waiting-for-feedback We need additional information before we can continue label Aug 11, 2021

wilkinsona pushed a commit that referenced this pull request Aug 11, 2021

Add auto-configuration for spring-rabbit-stream

9784838

See gh-27480

wilkinsona added a commit that referenced this pull request Aug 11, 2021

Polish "Add auto-configuration for spring-rabbit-stream"

7a0fe0f

See gh-27480

wilkinsona closed this in fdde40e Aug 11, 2021

wilkinsona changed the title ~~Auto-Configuration Support for spring-rabbit-stream~~ Provide auto-configuration for spring-rabbit-stream Aug 11, 2021

wilkinsona removed this from the 2.6.x milestone Aug 11, 2021

wilkinsona added this to the 2.6.0-M2 milestone Aug 11, 2021


		private String host = "localhost";

		private int port = DEFAULT_STREAM_PORT;


		private int port = DEFAULT_STREAM_PORT;

		private String userName;

	RabbitProperties properties, ObjectProvider<ConsumerCustomizer> consumerCustomizers,
	RabbitProperties properties, ObjectProvider<ConsumerCustomizer> consumerCustomizer,

	ObjectProvider<ContainerCustomizer<StreamListenerContainer>> containerCustomizers) {
	ObjectProvider<ContainerCustomizer<StreamListenerContainer>> containerCustomizer) {

Provide auto-configuration for spring-rabbit-stream #27480

Provide auto-configuration for spring-rabbit-stream #27480

Uh oh!

Conversation

garyrussell commented Jul 23, 2021

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!

wilkinsona Jul 23, 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!

wilkinsona Jul 23, 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!

wilkinsona Jul 26, 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!

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!

wilkinsona Aug 10, 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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wilkinsona Jul 23, 2021 •

edited

Loading

wilkinsona Jul 23, 2021 •

edited

Loading

wilkinsona Jul 26, 2021 •

edited

Loading

wilkinsona Aug 10, 2021 •

edited

Loading