GH-891 Introduced MultiRabbit to handle multiple brokers #1111

rwanderc · 2019-10-25T14:56:45Z

Hi @garyrussell and Spring team, it took a long time but finally happened. =)

I have tried to make it simpler avoiding name conventions, but didn't manage to follow your advice. Maybe you could also give me some direction on what to change and how to really test it since the instances are provided in the boot.

I created the test class MultiRabbitListenerAnnotationBeanPostProcessorCompatibilityTests as a copy of RabbitListenerAnnotationBeanPostProcessorCompatibilityTests to make sure nothing is broken from the default behavior if eventually the multirabbit BPP is loaded.

Resolves #891

garyrussell · 2019-10-25T15:00:26Z

@rwanderc Thanks!

Will take a look ASAP.

garyrussell · 2019-10-25T15:14:51Z

It will probably be next week before I can look at this in detail.

We will need an update to the reference manual (src/reference/asciidoc) before we merge, but it can wait until after the review if you prefer.

garyrussell

Looks good.

Some initial comments after a first pass review.

As previously mentioned; some doc updates would be good - particularly with respect to usage.

Perhaps an integration test or two (e.g. using 2 connection factories, which can be mocks) would be useful.

garyrussell · 2019-10-31T19:30:45Z

.../main/java/org/springframework/amqp/rabbit/annotation/MultiRabbitBootstrapConfiguration.java

+/**
+ * An {@link ImportBeanDefinitionRegistrar} class that registers a
+ * {@link MultiRabbitListenerAnnotationBeanPostProcessor} (overwriting the regular
+ * {@link RabbitListenerAnnotationBeanPostProcessor}) bean capable of processing Spring's @{@link RabbitListener}


We limit javadocs to 90 chars (and code to 120).

@rwanderc,

Please, stop sending Done and Fixed message.
We definitely get a notification about new commits in the PR.
Me personally follow the rule: if I don't agree or have some doubts, I comment on the review. Otherwise I just push the fix silently.

It really has just sounded as a spam on my phone 😄

Thanks for understanding

garyrussell · 2019-10-31T19:34:29Z

...ng-rabbit/src/main/java/org/springframework/amqp/rabbit/annotation/MultiRabbitConstants.java

+
+    private MultiRabbitConstants() {
+		throw new UnsupportedOperationException("Construction not supported.");
+	}


Tabs only, please, and a blank line between fields.

garyrussell · 2019-10-31T19:36:27Z

...g/springframework/amqp/rabbit/annotation/MultiRabbitListenerAnnotationBeanPostProcessor.java

+ * @see RabbitListenerAnnotationBeanPostProcessor
+ */
+public class MultiRabbitListenerAnnotationBeanPostProcessor extends RabbitListenerAnnotationBeanPostProcessor
+		implements ApplicationContextAware, BeanFactoryAware {


Redundant interface BFA.

garyrussell · 2019-10-31T19:36:48Z

...g/springframework/amqp/rabbit/annotation/MultiRabbitListenerAnnotationBeanPostProcessor.java

+		implements ApplicationContextAware, BeanFactoryAware {
+
+	private ApplicationContext applicationContext;
+	private BeanFactory beanFactory;


blank line between fields.

garyrussell · 2019-10-31T19:39:55Z

...g/springframework/amqp/rabbit/annotation/MultiRabbitListenerAnnotationBeanPostProcessor.java

+	private void enhanceBeansWithReferenceToRabbitAdmin(RabbitListener rabbitListener) {
+		RabbitAdmin rabbitAdmin = getRabbitAdminBean(rabbitListener);
+		// Enhance Exchanges
+		this.applicationContext.getBeansOfType(AbstractExchange.class).values().stream().filter(this::isNotProcessed)


Use getBeansOfType(clazz, false, false) - we don't want to prematurely load factory beans.

However, getting beans at all in a BPP is not generally a good thing to do. Instead implement SmartInitializingSingleton and defer these enhancements (e.g. store the RabbitAdmins in a field and process them in afterSingletonsInstantiated()).

Changed to getBeansOfType(clazz, false, false).
Thanks for the advice.

About plugging in BPP, the reason is that BPP runs for each @RabbitListener and creates the beans of the respective declarables (AbstractExchange, Queue and Binding) during this process.
Therefore, plugging to BPP makes it possible to associate the proper RabbitAdmin to the declarables just created. After they are enhanced with the RabbitAdmin, they are filtered out from next @RabbitListener processed (that's the reason of method isNotProcessed()).
Indeed the complexity of each of these statements is very bad (O(n2) for n @RabbitListeners), however I could not find a better solution to automate this process without the user's input (e.g. defining the RabbitAdmin's bean name in each declarable).

As far as I understand, implementing SmartInitializingSingleton will provide a processor after all beans are instantiated, which is much better in terms of complexity. But how could I know which declarable relates to each RabbitAdmin to make the association?

garyrussell · 2019-10-31T19:41:55Z

...g/springframework/amqp/rabbit/annotation/MultiRabbitListenerAnnotationBeanPostProcessor.java

+		RabbitAdmin rabbitAdmin = getRabbitAdminBean(rabbitListener);
+		// Enhance Exchanges
+		this.applicationContext.getBeansOfType(AbstractExchange.class).values().stream().filter(this::isNotProcessed)
+				.forEach(exchange -> exchange.setAdminsThatShouldDeclare(rabbitAdmin));


Do we really want to declare all exchanges on all servers?

Perhaps we could use some naming convention to determine which admin(s) should declare which declarables?

No. The declarables (AbstractExchange, Queue and Binding) are not declared on all servers.

That's the reason for the isNotProcessed() method.
Once the declarable beans are created by BPP for one @RabbitListener, they will not have any RabbitAdmin yet associated.
Then, they are enhanced with the corresponding RabbitAdmin.

Then, when BPP processes the 2nd, 3rd, etc @RabbitListeners, the previous declarables will be filtered out and not enhanced, because they already have a RabbitAdmin associated to them.

This way, only the last declarables created are associated to the current RabbitAdmin.

Oh; now I see the reason for my confusion; this is a bit brittle - you are using the fact that the BPP "just" added the beans for the current @RabbitListener.queueBindings() and the beans for other @QueueBindings don't exist yet.

The problem with this approach (using getBeansOfType()) is that all stand-alone queue/exchange/binding @Beans will be declared by the first admin.

We could use the names from the @QueueBindings and do getBean() for each individually to avoid picking up other unrelated beans.

However, this is complicated by the fact that the bean names are foo.n (where n is incremented with a common integer) so there is no way to know which beans were registered for the current annotation.

I can see a couple of possible solutions -

change processAmqpListener() to return Collection<Declarable> containing any beans that were registered for this listener in the super class

add a new @RabbitListener field to AbstractDeclarable and use that as the .filter.

garyrussell · 2019-10-31T19:48:26Z

...g/springframework/amqp/rabbit/annotation/MultiRabbitListenerAnnotationBeanPostProcessor.java

+	 */
+	private boolean isNotProcessed(Declarable declarable) {
+		return declarable.getDeclaringAdmins() == null || (
+				declarable.getDeclaringAdmins().stream().noneMatch(item -> item == this) && declarable


line break is a bit unusual

private boolean isNotProcessed(Declarable declarable) { return declarable.getDeclaringAdmins() == null || ( declarable.getDeclaringAdmins().stream().noneMatch(item -> item == this) && declarable.getDeclaringAdmins().stream().noneMatch(item -> item instanceof RabbitAdmin)); }

That said, how can item == this?

item can be an admin instance or an admin bean name (see declarableByMe in RabbitAdmin).

Indeed. Fixed line break.
The item == this was a mistake. Thanks for the heads-up. Removed.

garyrussell · 2019-10-31T19:49:27Z

...g/springframework/amqp/rabbit/annotation/MultiRabbitListenerAnnotationBeanPostProcessor.java

+	}
+
+	@Override
+	public void setApplicationContext(@NonNull ApplicationContext applicationContext) {


Convention is to put accessors before all other methods.

garyrussell · 2019-10-31T19:52:16Z

...c/test/java/org/springframework/amqp/rabbit/annotation/MultiRabbitAdminNameResolverTest.java

+/**
+ * @author Wander Costa
+ */
+class MultiRabbitAdminNameResolverTest {


Convention is ...Tests (even if only one).

garyrussell · 2019-10-31T20:30:51Z

Instead of duplicating that test, I think you can simply subclass it and subclass Config overloading the postProcessor bean.

rwanderc · 2019-11-02T14:45:29Z

Thank you so much @garyrussell for your comments.
I will probably start working on that next week. Will also try to come with the documentation so you can share your thoughts on that.

rwanderc · 2019-11-19T12:05:46Z

Sorry spamming you @garyrussell and @artembilan previously ...

About the tests, I followed the advice on subclassing and providing an overloading, but it didn't replace the bean. I guess it's because the context is initialized always with a specific class Config.class. I worked around it by changing the current test to provide the configuration class as a parameter.

About the integration tests, I created a couple of tests to make sure the declarables are declared by the proper RabbitAdmin, which is pretty much the main concern on the amqp side of the feature. I didn't find a meaningful way to test with the connection factories. Would you have a suggestion?
In the other hand, I also thought that would introduce a lot of complexity to test with @RabbitAvailable since it would require at least 2 servers running. Therefore, I didn't provide such test.

garyrussell

Regarding testing multiple CFs, you can either test with mocks or have multiple CFs using the same underlying connection factory. You can then spy() the createConnection() methods to verify the expected one was called. Also see the LocalizedQueueConnectionFactoryTests.

garyrussell · 2019-11-19T15:30:23Z

...g/springframework/amqp/rabbit/annotation/MultiRabbitListenerAnnotationBeanPostProcessor.java

+		RabbitAdmin rabbitAdmin = getRabbitAdminBean(rabbitListener);
+		// Enhance Exchanges
+		this.applicationContext.getBeansOfType(AbstractExchange.class).values().stream().filter(this::isNotProcessed)
+				.forEach(exchange -> exchange.setAdminsThatShouldDeclare(rabbitAdmin));


Oh; now I see the reason for my confusion; this is a bit brittle - you are using the fact that the BPP "just" added the beans for the current @RabbitListener.queueBindings() and the beans for other @QueueBindings don't exist yet.

The problem with this approach (using getBeansOfType()) is that all stand-alone queue/exchange/binding @Beans will be declared by the first admin.

We could use the names from the @QueueBindings and do getBean() for each individually to avoid picking up other unrelated beans.

However, this is complicated by the fact that the bean names are foo.n (where n is incremented with a common integer) so there is no way to know which beans were registered for the current annotation.

I can see a couple of possible solutions -

change processAmqpListener() to return Collection<Declarable> containing any beans that were registered for this listener in the super class

add a new @RabbitListener field to AbstractDeclarable and use that as the .filter.

rwanderc · 2020-01-20T15:55:52Z

Hi @garyrussell. As you recommended, I implemented the approach of returning the collection of Declarables in the BPP. Also, I created some integration tests that make sure the proper CF is touched as you advised.
I'm still lacking behind with the documentation, which I hope to work on soon.

artembilan

I think I'll come back for more review when we switch a version for possible breaking changes.

Thank you for your effort so far!

artembilan · 2020-01-21T18:46:46Z

...t/src/main/java/org/springframework/amqp/rabbit/annotation/MultiRabbitAdminNameResolver.java

+ *
+ * @author Wander Costa
+ */
+final class MultiRabbitAdminNameResolver {


I'm not sure that we need this functionality as a top-level class at all.
Looks like that resolve() method could be just private static in the MultiRabbitListenerAnnotationBeanPostProcessor

artembilan · 2020-01-21T18:48:08Z

.../main/java/org/springframework/amqp/rabbit/annotation/MultiRabbitBootstrapConfiguration.java

+ * An {@link ImportBeanDefinitionRegistrar} class that registers a
+ * {@link MultiRabbitListenerAnnotationBeanPostProcessor} (overwriting the regular
+ * {@link RabbitListenerAnnotationBeanPostProcessor}) bean capable of processing
+ * Spring's @{@link RabbitListener} annotations.


Typo: we don't need this extra @ before braces.

artembilan · 2020-01-21T18:56:15Z

...ng-rabbit/src/main/java/org/springframework/amqp/rabbit/annotation/MultiRabbitConstants.java

+ *
+ * @author Wander Costa
+ */
+public final class MultiRabbitConstants {


I'm not sure in the name and purpose of this class.
Why all of these constants can't go into a MultiRabbitListenerAnnotationBeanPostProcessor class?
Like we have already with its super one...

artembilan · 2020-01-21T19:00:47Z

...g/springframework/amqp/rabbit/annotation/MultiRabbitListenerAnnotationBeanPostProcessor.java

+ * configuration, preventing the server from automatic binding non-related structures.
+ *
+ * @author Wander Costa
+ * @see RabbitListenerAnnotationBeanPostProcessor


There is no reason in @see. You mention it in the JavaDocs and we have an extension from it anyway.

artembilan · 2020-01-21T19:02:06Z

...g/springframework/amqp/rabbit/annotation/MultiRabbitListenerAnnotationBeanPostProcessor.java

+			Object bean, String beanName) {
+		final Collection<Declarable> declarables = super.processAmqpListener(rabbitListener, method, bean, beanName);
+		final RabbitAdmin rabbitAdmin = resolveRabbitAdminBean(rabbitListener);
+		declarables.stream()


I would prefer regular loop with ifs instead of Stream objects.

artembilan · 2020-01-21T19:08:11Z

...it/src/test/java/org/springframework/amqp/rabbit/annotation/MultiRabbitIntegrationTests.java

+
+		static final SimpleRoutingConnectionFactory ROUTING_CONNECTION_FACTORY = new SimpleRoutingConnectionFactory();
+		static final ConnectionFactory DEFAULT_CONNECTION_FACTORY = Mockito.mock(ConnectionFactory.class);
+		static final ConnectionFactory CONNECTION_FACTORY_BROKER_B = Mockito.spy(ConnectionFactory.class);


Typo? There is nothing to spy on the interface.

Also: doesn't look like an integration test if you used mock.
Maybe better to call it MockMultiRabbitTests?

rwanderc · 2020-05-12T20:38:22Z

Hi @garyrussell and @artembilan. First of all, thank you for the previous review you've done and comments. I worked on them as advised.
I think the implementation in spring-amqp is probably done (missing docs, as I mention below). However, I have some follow-up questions:

A big concern about this PR is the change in the BPP so as to retrieve the Declarables returned from the processing. As @artembilan mentioned, this is possibly a breaking change (GH-891 Introduced MultiRabbit to handle multiple brokers #1111 (review)). How should I proceed? Should I write any sort of documentation about it (e.g. https://docs.spring.io/spring-amqp/reference/html/#_breaking_api_changes)?
Most of the implementation of Spring MultiRabbit will be in the spring-boot project. What sort of documentation do you recommend writing in spring-amqp then? I thought about introducing in What's new something with the titleSupport for multiple connection factories as well in the Reference a topic about it. Could you pls give some direction?
There are many commits. Before finishing, I suppose I'd rather squash them. We might come here later, of course.

garyrussell · 2020-05-12T21:01:48Z

Thanks @rwanderc - we will try to review later this week. We have a big release day tomorrow (this will go into 2.3 later this year; there is no new Spring AMQP for this week's Boot 2.3).

We'll build Milestone 1 soon after this is merged.

Don't worry about squashing - we always do that before merging to master.

Discrete commits makes it easier to review deltas during the review phase.

garyrussell · 2020-06-18T20:23:36Z

Finally getting around to looking at this; sorry for the delay...

garyrussell

Looks good! Just some minor comments.

@rwanderc - I have scheduled the first milestone of 2.3 next Wednesday; it would be great to get this into that release (even without documentation).

If you don't think you can address these concerns by then, I can address them.

Thanks

garyrussell · 2020-06-18T20:27:41Z

.../main/java/org/springframework/amqp/rabbit/annotation/MultiRabbitBootstrapConfiguration.java

@@ -0,0 +1,49 @@
+/*
+ * Copyright 2002-2020 the original author or authors.


Just 2020 for new files.

garyrussell · 2020-06-18T20:28:09Z

...g/springframework/amqp/rabbit/annotation/MultiRabbitListenerAnnotationBeanPostProcessor.java

@@ -0,0 +1,99 @@
+/*
+ * Copyright 2002-2020 the original author or authors.


garyrussell · 2020-06-18T20:31:18Z

...g/springframework/amqp/rabbit/annotation/MultiRabbitListenerAnnotationBeanPostProcessor.java

+		final Collection<Declarable> declarables = super.processAmqpListener(rabbitListener, method, bean, beanName);
+		final RabbitAdmin rabbitAdmin = resolveRabbitAdminBean(rabbitListener);
+		for (final Declarable declarable : declarables) {
+			if (declarable.getDeclaringAdmins().isEmpty() && declarable instanceof AbstractDeclarable) {


I think we can add setAdminsThatShouldDeclare as a default method on Declarable and avoid the cast.

I changed according to the suggestion.
Also, I suppose you also mean removing the condition if (... && declarable instanceof AbstractDeclarable) {...} to rely directly on the interface.

If that's the case, having a default method on Declarable might introduce space for a behavioral side-effect in MultiRabbit (declarables being processed by all admins, which I guess is not the main use-case). At spring-amqp, I see no impact, since all concrete classes eventually also extend AbstractDeclarable.

My thinking is that since these are breaking changes, why not introduce setAdminsThatShouldDeclare() as a regular signature (non-default) and force concrete classes to implement it?
Declarable's concretes must also implement getDeclaringAdmins() which makes me understand it's in the scope of its responsibilities.

What do you think about it?

Yes; that would be ok (non-default) - I doubt users implement their own Declarable s anyway.

garyrussell · 2020-06-18T20:35:04Z

...g/springframework/amqp/rabbit/annotation/MultiRabbitListenerAnnotationBeanPostProcessor.java

+	 */
+	private RabbitAdmin resolveRabbitAdminBean(RabbitListener rabbitListener) {
+		final String name = resolveAdminName(rabbitListener);
+		return this.beanFactory.getBean(name, RabbitAdmin.class);


We shouldn't call getBean() from a BPP - causes early instantiation.

That's why setAdminsThatShouldDeclare takes Object... - so we can just pass in the bean name.

Although I see that super.resolveAdmin() does that too; I need to fix that.

That's a big point! I'm changing it to provide only the name, and not prematurely instantiate the bean.

Also, regarding the logic here (and here), as I understand, it's intended to provide a fallback RabbitAdmin name.

At this moment, super.processAmqpListener() was already executed and resolved the name provided (if any) through the regular flow. Then, it's only important to assign a fallback bean name (the RabbitAdmin generated from MultiRabbit) if none was specified.

Therefore, I guess resolveExpressionAsString() wouldn't be necessary. Is that right?
Nevertheless, I provided it as suggested.

garyrussell · 2020-06-18T20:36:55Z

...g/springframework/amqp/rabbit/annotation/MultiRabbitListenerAnnotationBeanPostProcessor.java

+	 * @return The name of the RabbitAdmin bean.
+	 */
+	public static String resolveAdminName(RabbitListener rabbitListener) {
+		String admin = rabbitListener.admin();


Need to call resolveExpressionAsString() - see super.resolveAdmin() - we should make all those resolve* methods protected.

garyrussell · 2020-06-18T20:38:46Z

...ng-rabbit/src/test/java/org/springframework/amqp/rabbit/annotation/MockMultiRabbitTests.java

@@ -0,0 +1,332 @@
+/*
+ * Copyright 2002-2020 the original author or authors.


garyrussell · 2020-06-18T20:39:32Z

.../java/org/springframework/amqp/rabbit/annotation/MultiRabbitBootstrapConfigurationTests.java

@@ -0,0 +1,48 @@
+/*
+ * Copyright 2002-2019 the original author or authors.


garyrussell · 2020-06-18T20:39:41Z

...amqp/rabbit/annotation/MultiRabbitListenerAnnotationBeanPostProcessorCompatibilityTests.java

@@ -0,0 +1,59 @@
+/*
+ * Copyright 2002-2019 the original author or authors.


garyrussell · 2020-06-18T20:40:11Z

...st/java/org/springframework/amqp/rabbit/connection/ConnectionFactoryContextWrapperTests.java

@@ -0,0 +1,75 @@
+/*
+ * Copyright 2002-2020 the original author or authors.


garyrussell · 2020-06-18T20:49:26Z

.../main/java/org/springframework/amqp/rabbit/annotation/MultiRabbitBootstrapConfiguration.java

+ * @see RabbitListenerEndpointRegistry
+ * @see EnableRabbit
+ */
+public class MultiRabbitBootstrapConfiguration implements ImportBeanDefinitionRegistrar {


Don't we need an annotation or DeferredImportSelector to trigger this to ensure it's loaded before the RabbitBootstrapConfiguration?

cc/ @artembilan

Can't the user just declare a bean with the subclass and RABBIT_LISTENER_ANNOTATION_PROCESSOR_BEAN_NAME since the RabbitListenerConfigurationSelector won't run until all @Configuration classes have been processed; removing the need for this class altogether.

Well, need to understand how it is tested over here though.
Probably just EnableRabbit(multi = true) would be enough...

I'm not too sure how to proceed here. No changes.

I think we can just delete this; in order to override the default BPP, simply define the subclass as a bean with the default name, and the default one will be skipped here

spring-amqp/spring-rabbit/src/main/java/org/springframework/amqp/rabbit/annotation/RabbitBootstrapConfiguration.java

Lines 51 to 56 in 715f39f

if (!registry.containsBeanDefinition(

RabbitListenerConfigUtils.RABBIT_LISTENER_ANNOTATION_PROCESSOR_BEAN_NAME)) {

registry.registerBeanDefinition(RabbitListenerConfigUtils.RABBIT_LISTENER_ANNOTATION_PROCESSOR_BEAN_NAME,

new RootBeanDefinition(RabbitListenerAnnotationBeanPostProcessor.class));

}

Hi @garyrussell & @artembilan,
I'm writing this here to connect to the context of the issue.

I'm working on the boot part, and I tried to implement the instantiation of MultRabbitBPP the way suggested. But as overriding of beans is now disabled by default in Spring, it fails to register a bean with the same name as RabbitListenerConfigUtils.RABBIT_LISTENER_ANNOTATION_PROCESSOR_BEAN_NAME.

I'm trying to work around it by providing a MultiRabbit BPP bean with another name, via configuration. But when processing listeners, it still picks up the original one BPP and ALSO tries to process MultiRabbitBPP, leading to an error of multiple registrations of the same declarables.

Maybe it should never instantiate RabbitListenerAnnotationBeanPostProcessor since MultiRabbitListenerAnnotationBeanPostProcessor is provided, but I'm not figuring out how to make it work, or if that's related to some order of processing.

I got to make this work by directly implementing RabbitBootstrapConfiguration to instantiate new RootBeanDefinition(MultiRabbitListenerAnnotationBeanPostProcessor.class) whenever EnableRabbit(multi = true) (as per @artembilan's suggestion), but I feel this is not the best solution (very intrusive and not SOLID).

Would you have any suggestions on how to proceed with this?

Changes in spring-amqp.
Changes in spring-boot.

Another, perhaps less intrusive, option would be to add a new Boot property multiCluster. While we can't reference the RabbitProperties we can get a reference to the environment:

public class RabbitBootstrapConfiguration implements ImportBeanDefinitionRegistrar, EnvironmentAware { private Environment environment; @Override public void registerBeanDefinitions(@Nullable AnnotationMetadata importingClassMetadata, BeanDefinitionRegistry registry) { String multiS = this.environment.getProperty("spring.rabbitmq.multi-cluster"); boolean multi = multiS != null && Boolean.parseBoolean(multiS); if (multi) { ... } else if (!registry.containsBeanDefinition( RabbitListenerConfigUtils.RABBIT_LISTENER_ANNOTATION_PROCESSOR_BEAN_NAME)) { registry.registerBeanDefinition(RabbitListenerConfigUtils.RABBIT_LISTENER_ANNOTATION_PROCESSOR_BEAN_NAME, new RootBeanDefinition(RabbitListenerAnnotationBeanPostProcessor.class)); } if (!registry.containsBeanDefinition(RabbitListenerConfigUtils.RABBIT_LISTENER_ENDPOINT_REGISTRY_BEAN_NAME)) { registry.registerBeanDefinition(RabbitListenerConfigUtils.RABBIT_LISTENER_ENDPOINT_REGISTRY_BEAN_NAME, new RootBeanDefinition(RabbitListenerEndpointRegistry.class)); } } @Override public void setEnvironment(Environment environment) { this.environment = environment; } }

rwanderc · 2020-06-19T11:42:26Z

Looks good! Just some minor comments.

@rwanderc - I have scheduled the first milestone of 2.3 next Wednesday; it would be great to get this into that release (even without documentation).

If you don't think you can address these concerns by then, I can address them.

Thanks

@garyrussell I will look into the comments tomorrow, or later on Sunday. If none of them require other rounds, my guess is that we get this into 2.3. :)

Thanks

…he bean name instead

rwanderc · 2020-06-21T15:36:18Z

@garyrussell & @artembilan, just (rebased and) pushed the changes for your recent comments.
I have a couple of follow-up questions, and couldn't understand how to proceed here.

Pls let me know what you find, and I will of course jump in.
But to avoid another round and to be able to add it to the release (if you see no blockers), you can also make any changes you find necessary.

garyrussell

Great work; thanks for the contribution.

Next up: docs (if you have time - otherwise I'll take care of it).

KamilWitkowski7 · 2020-10-26T08:14:05Z

Great work; thanks for the contribution.

Next up: docs (if you have time - otherwise I'll take care of it).

Where can I find docs for this one?

Supplement to spring-projects#1111

garyrussell · 2020-10-27T20:29:56Z

@KamilWitkowski7 Thanks for the ping, I had forgotten about this; and timely too, we are releasing 2.3.0 tomorrow.

Pull request here: #1262

@rwanderc I would appreciate if you can give it a quick review, in case I missed anything.

Thanks.

Supplement to #1111

rwanderc changed the title ~~GH-891 Introduced MultiRabbit for handle multiple brokers~~ GH-891 Introduced MultiRabbit to handle multiple brokers Oct 25, 2019

rwanderc mentioned this pull request Oct 25, 2019

SpringMultirabbit for auto-configuration of multiple ConnectionFactories #891

Closed

garyrussell suggested changes Oct 31, 2019

View reviewed changes

garyrussell suggested changes Nov 19, 2019

View reviewed changes

artembilan requested changes Jan 21, 2020

View reviewed changes

rwanderc mentioned this pull request Feb 25, 2020

Autodeclaration of rabbit artifacts fails on non default connection when RabbitListener is used freenowtech/spring-multirabbit#7

Closed

rwanderc requested review from garyrussell and artembilan May 12, 2020 20:38

rwanderc mentioned this pull request Jun 16, 2020

How set PrefetchCount ? freenowtech/spring-multirabbit#19

Closed

garyrussell suggested changes Jun 18, 2020

View reviewed changes

Wander Costa and others added 9 commits June 21, 2020 16:36

GH-891 Introduced MultiRabbit for handle multiple brokers

f53ad9f

GH-891 Fixed formatting and conventions

7565017

GH-891 Avoid premature load of factory beans and some minor refactoring

2036950

GH-891 Introduced tests for SpringMultirabbit

dc98b3e

GH-891 Modified BPP to return list of Declarables created

0536151

GH-891 Minor refactoring

570041c

GH-891 Updated copyright year

a1f3239

GH-891 Introduced default setAdminsThatShouldDeclare() into Declarable

4ba998b

GH-891 Avoiding premature instantiation of RabbitAdmin by providing t…

6098f8c

…he bean name instead

rwanderc requested a review from garyrussell June 21, 2020 15:36

rwanderc added 2 commits June 24, 2020 07:05

GH-891 Changed to non-default

c40497a

GH-891 Removed MultiRabbitBootstrapConfiguration and its test

ccfef17

garyrussell approved these changes Jun 24, 2020

View reviewed changes

garyrussell merged commit c7897ed into spring-projects:master Jun 24, 2020

rwanderc mentioned this pull request Jun 25, 2020

GH-18 Introduced workaround to guarantee initialization of ConnectionFactory when processing listeners freenowtech/spring-multirabbit#20

Merged

garyrussell added a commit to garyrussell/spring-amqp that referenced this pull request Oct 27, 2020

spring-projectsGH-891: Docs for Multi RabbitMQ Support

58bfcdc

Supplement to spring-projects#1111

garyrussell added a commit to garyrussell/spring-amqp that referenced this pull request Oct 27, 2020

spring-projectsGH-891: Docs for Multi RabbitMQ Support

76b49b5

Supplement to spring-projects#1111

garyrussell mentioned this pull request Oct 27, 2020

GH-891: Docs for Multi RabbitMQ Support #1262

Merged

artembilan pushed a commit that referenced this pull request Oct 28, 2020

GH-891: Docs for Multi RabbitMQ Support

ef03b70

Supplement to #1111

rwanderc mentioned this pull request Jan 31, 2021

Introducing MultiRabbit Bootstrap #1303

Merged

rwanderc mentioned this pull request Feb 19, 2021

Introduced auto-configuration for MultiRabbit spring-projects/spring-boot#25369

Closed

		@@ -0,0 +1,49 @@
		/*
		* Copyright 2002-2020 the original author or authors.

		@@ -0,0 +1,99 @@
		/*
		* Copyright 2002-2020 the original author or authors.

		@@ -0,0 +1,332 @@
		/*
		* Copyright 2002-2020 the original author or authors.

		@@ -0,0 +1,48 @@
		/*
		* Copyright 2002-2019 the original author or authors.

		@@ -0,0 +1,59 @@
		/*
		* Copyright 2002-2019 the original author or authors.

		@@ -0,0 +1,75 @@
		/*
		* Copyright 2002-2020 the original author or authors.

	if (!registry.containsBeanDefinition(
	RabbitListenerConfigUtils.RABBIT_LISTENER_ANNOTATION_PROCESSOR_BEAN_NAME)) {

	registry.registerBeanDefinition(RabbitListenerConfigUtils.RABBIT_LISTENER_ANNOTATION_PROCESSOR_BEAN_NAME,
	new RootBeanDefinition(RabbitListenerAnnotationBeanPostProcessor.class));
	}

GH-891 Introduced MultiRabbit to handle multiple brokers #1111

GH-891 Introduced MultiRabbit to handle multiple brokers #1111

Uh oh!

Conversation

rwanderc commented Oct 25, 2019 • edited by garyrussell Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

garyrussell commented Oct 25, 2019

Uh oh!

garyrussell commented Oct 25, 2019

Uh oh!

garyrussell 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!

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!

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!

garyrussell commented Oct 31, 2019

Uh oh!

rwanderc commented Nov 2, 2019

Uh oh!

rwanderc commented Nov 19, 2019

Uh oh!

garyrussell left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rwanderc commented Jan 20, 2020

Uh oh!

artembilan 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!

rwanderc commented Oct 25, 2019 •

edited by garyrussell

Loading