Skip to content
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

The DefaultBinderFactory.propagateSharedBeans() is looking for shared bean in wrong context #2728

Closed
mayur-solace opened this issue May 10, 2023 · 8 comments
Assignees
Milestone

Comments

@mayur-solace
Copy link

mayur-solace commented May 10, 2023

The call to the this.propagateSharedBeans(...) is passing the fromContext and toContext contexts in wrong order. The binderProducingContext should be toContext.

Also calling the binderProducingContext.refresh() before call to this.propagateSharedBeans(..) creates new beans (that depends on shared beans) before shared beans are available for auto-wiring.

Code snippet from org.springframework.cloud.stream.binder.DefaultBinderFactory

 if (refresh) {
	binderProducingContext.refresh();
	if (!useApplicationContextAsParent || "integration".equals(binderType.getDefaultName())) {
		this.propagateSharedBeans((GenericApplicationContext) this.context, binderProducingContext);
	}
}

should the call be instead....

if (refresh) {
	if (!useApplicationContextAsParent || "integration".equals(binderType.getDefaultName())) {
		this.propagateSharedBeans(binderProducingContext, (GenericApplicationContext) this.context);
	}
        binderProducingContext.refresh();
}

is there a tests that covers shared bean propagation ?

this.propagateSharedBeans((GenericApplicationContext) this.context, binderProducingContext);

@Nephery
Copy link

Nephery commented May 10, 2023

For context, we're getting this issue while trying to upgrade the Solace binder to Spring Cloud 2022.x since this binder needs to use shared.beans to share some beans from the parent context to binder contexts for the multiple systems setup.

@sobychacko
Copy link
Contributor

cc @olegz

@olegz
Copy link
Contributor

olegz commented Sep 28, 2023

@Nephery
Sorry, somehow this issue got lost.
Can you guys elaborate a bit more on this? Beans are propagated from parent context to child one (binder context) to ensure that certain beans are available to the binder context. So doing it the other way around is definitely wrong especially in the cases where you have multiple binders of the same type (i.e., two Solace binders connected to different solace clusters)

So I want to understand better what issue you are really experiencing and how to reproduce it,.

@olegz olegz removed this from the 4.0.4 milestone Sep 28, 2023
@mayur-solace
Copy link
Author

@olegz

Look at the call to the method and definition of the method.

this.propagateSharedBeans((GenericApplicationContext) this.context, binderProducingContext);

and

private void propagateSharedBeans(GenericApplicationContext toContext, GenericApplicationContext fromContext) { }

Here,
this.context - as per my understanding is the parent context and it is passed as toContext
binderProducingContext - is the child one (binder context) which is passed as fromContext

method propagateSharedBeans(toContext, fromContext) is trying to find the shared bean in binderProducingContext(i.e. fromContext) and inject it this.context (i.e. toContext). I think shared bean won't found in binderProducingContext

Let me know if my understanding is in correct.

@Nephery
Copy link

Nephery commented Sep 28, 2023

Can you guys elaborate a bit more on this? Beans are propagated from parent context to child one (binder context) to ensure that certain beans are available to the binder context. So doing it the other way around is definitely wrong especially in the cases where you have multiple binders of the same type (i.e., two Solace binders connected to different solace clusters)

@olegz the issue that we were facing was that shared.beans wasn't propagating beans from parent context to the child contexts. We aren't doing it the other way around.

To break down what we have:

  1. We have two beans, SolaceMeterAccessor and SolaceMessageMeterBinder, instantiated in this @Configuration, SolaceMeterConfiguration.
  2. SolaceMeterConfiguration is auto-configured in the parent context using the org.springframework.boot.autoconfigure.AutoConfiguration.imports file.
  3. The SolaceMeterAccessor and SolaceMessageMeterBinder beans are then forwarded to the child contexts using the shared.beans file.
  4. The beans are then autowired to the solace binder bean within the binder's context.
    • But instead of getting the shared bean, SolaceMeterAccessor, we're just getting null.

@olegz olegz added this to the 4.1.0 milestone Nov 7, 2023
@olegz olegz closed this as completed in 35964fc Dec 4, 2023
olegz added a commit that referenced this issue Dec 4, 2023
@Nephery
Copy link

Nephery commented Dec 21, 2023

@olegz this doesn't look like it works.

As @mayur-solace mentioned, we think this might be because binderProducingContext.refresh(); is being called before propagateSharedBeans() is invoked. The context might need to be refreshed after propagateSharedBeans() instead of before.

@olegz
Copy link
Contributor

olegz commented May 27, 2024

@Nephery Can you please try with the latest snapshot?

@Nephery
Copy link

Nephery commented May 27, 2024

@olegz I tested out the latest 4.1.2-SNAPSHOT, and it looks like this works now. Thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants