GH-2428: Manual Declarations Recovery

[EldarErel]

Issue link: #2428

Fixes an issue when manual declarations are not being redeclared upon connection recovery, when application context has not been set

[pivotal-cla]

@EldarErel Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

[pivotal-cla]

@EldarErel Thank you for signing the Contributor License Agreement!

[garyrussell]

@EldarErel Why is there no ApplicationContext?

This is a Spring project and is designed for use in a Spring application; there are probably many other features that won't work when used outside of Spring.

I am not inclined to accept a change that would support/promote such usage because it could be a slippery slope and become a support nightmare.

If you don't want to use Spring, consider using the amqp-client API directly.

If you are using it in a Spring app, then the admin must be declared as a bean so that all of its *Aware methods (and InitializingBean) interfaces are satisfied.

I'll leave this open for a short while, in case you can provide a compelling use case for us to accep this change.

[EldarErel]

@EldarErel Why is there no ApplicationContext?

This is a Spring project and is designed for use in a Spring application; there are probably many other features that won't work when used outside of Spring.

I am not inclined to accept a change that would support/promote such usage because it could be a slippery slope and become a support nightmare.

If you don't want to use Spring, consider using the amqp-client API directly.

If you are using it in a Spring app, then the admin must be declared as a bean so that all of its *Aware methods (and InitializingBean) interfaces are satisfied.

I'll leave this open for a short while, in case you can provide a compelling use case for us to accep this change.

Hi @garyrussell , thank you for your quick response.

I use spring, the use case is trying to create several listeners to several connections, I’ve created a connection, listener container and amqp admin for each connection.
I initialed them as spring beans.
I declare the queues once using @rabbitlistener
And using bean processor and some other parameters, I declare the queues and register listeners for the other connections as well.

As I created the beans myself, I skipped the setter of the application context in the amqp admin
RabbitAdmin.setApplicationContext(..)
(as it is not required by default, and didn’t use it in my case as im declaring the queues manually and dynamically for for other connections.

As you added manual declaration for non bean declarables, I thought it will be reasonable to use an AmqpAdmin bean for manual declarations only.

If application context is needed to use the manual declarations recovery, then maybe it should be enforced on initialization of a new RabbitAdmin instance.
So it would be explicit that we must set application context for the RabbitAdmin to work properly with manual redeclarations

[garyrussell]

RabbitAdmin.setApplicationContext(..). You don’t need to call it at all. If the admin is declared as a @Bean, Spring will set everything up for you. You should never create Spring beans manually. Or, if you do, you need to register them in the context and tell the context to fully initialize it.

[garyrussell]

It’s not a Spring bean if you just create it yourself, it’s just an instance of the class.

[EldarErel]

I created a bean for mapping the rabbitAdmins by the connection name: for example:

	@Bean
	public Map<String,AmqpAdmin> rabbitAdminsMap(Map<String,ConnectionFactory> connections) {
		Map<String, AmqpAdmin> rabbitAdmins = new HashMap<>();
		connections.forEach((key,connection) ->
		{
			RabbitAdmin admin = new RabbitAdmin(connection);
			admin.setRedeclareManualDeclarations(true);

                        // other settings

			rabbitAdmins.put(key, admin);

		});
		return rabbitAdmins;
	}

then, I use @Autowired to inject the map to the processor class and declare the queues by the given connections (on request)

I didn't register each of the admins as bean, just the whole map. (Maybe I should have done it.)
But as the documentation says: "Normally, when a connection is recovered, the admin only recovers auto-delete queues, etc, that are declared as beans in the application context. When this is true, it will also redeclare any manually declared Declarables via admin methods."

So I expected it to work.

[EldarErel]

With the spring support, upon connection recovery, the admin
initialze() method is being called, but the manual declarations are not being redeclared, because the RabbitAdmin.applicationContext == null.
This PR solves it , so when setRedeclareManualDeclarations(true) the functionally will work.

[garyrussell]

Adding a @Bean for the Map of admins does not register each admin as a bean (which is required for them to be beans). Here is some similar code to what you need...

@Bean
Map<String, RabbitAdmin> admins(ConfigurableApplicationContext context, ConnectionFactory cf) {
	RabbitAdmin admin = new RabbitAdmin(cf);
	context.getBeanFactory().registerSingleton("foo", admin);
	admin = (RabbitAdmin) context.getBeanFactory().initializeBean(admin, "foo");
	return Map.of("foo", admin);
}

[EldarErel]

Thank you very much for your time and response!
Managing dynamic creation and eviction of beans is a mission that I prefer to avoid when possible, as it required more maintenance in certain situations.

If the admin cannot functional without it being a bean, so ok I understand.
But as the manual declaration methods works without the admin being a been, the re declaration can be something to complete it. (As it doesn’t require the context to work, only re ordering the initialize method).

[garyrussell]

Ok; I can make an exception this time, but please be aware that other features won't work either (e.g. publication of DeclarationExceptionEvents), and I can make no guarantees that something else might break in the future. The admin is designed to be a Spring bean.

[EldarErel]

Well understood, (how can the event be sent without publisher and the context?)
thank you very much! I’m sure it will be helpful many people.

[garyrussell]

That's what I am saying; if it's not a bean (and you don't provide a publisher), there are no events published...

spring-amqp/spring-rabbit/src/main/java/org/springframework/amqp/rabbit/core/RabbitAdmin.java

Lines 909 to 911 in ba9f8a1

	if (this.applicationEventPublisher != null) {
	this.applicationEventPublisher.publishEvent(event);
	}

[EldarErel]

exactly, well understood and it make sense. We cannot expect publishing of events without a publisher. But we can expect manual re declaration without application context (as it is not necessary for the flow)