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

Allow injection of Spring-managed beans into Hazelcast components #28801

Conversation

jerrinot
Copy link
Contributor

Fixes #19487

When Spring Boot creates Hazelcast configuration and the Hazelcast-Spring
integration module is available on a classpath then inject SpringManagedContext
into Hazelcast configuration.

This change makes it possible to inject Spring managed beans into objects
instantiated by Hazelcast.

I'm not too familiar with Spring Boot so chances are the code is not idiomatic. It looks rather complex for what is basically an if-else condition. Any guidance is very much appreciated.

Fixes spring-projectsgh-19487
When Spring Boot creates Hazelcast configuration and the Hazelcast-Spring
integration module is available on a classpath then use SpringManagedContext.

This makes it possible to inject Spring managed beans into objects
instantiated by Hazelcast.
@jerrinot
Copy link
Contributor Author

the same change should be probably done in the client auto-configuration.

@jerrinot
Copy link
Contributor Author

jerrinot commented Nov 24, 2021

I was also considering not to introduce the concept of Customizers and keep it simple. Something along this lines:

try {
  var context = new SpringManagedContext(appContext);
  config.setManagedContext(context);
catch (ClassNotFoundError err) {
 // the Hazelcast-Spring integration JAR not available on a classpath, that's OK
}

but this felt dirty. From the other hand - the current approach feels unnecessary complex.

jerrinot and others added 2 commits November 25, 2021 14:22
Co-authored-by: Łukasz Dziedziul <l.dziedziul@gmail.com>
This eliminates the need for EMPTY_CUSTOMIZER.
@jerrinot
Copy link
Contributor Author

jerrinot commented Nov 25, 2021

@ldziedziul, @wilkinsona: thanks to both of you for the review. I pushed a change simplifying the code and I'm now quite happy with how it looks. Please let me know what you think.

edit: s/not/now

Copy link
Contributor

@ldziedziul ldziedziul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

@wilkinsona wilkinsona added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 29, 2021
@wilkinsona wilkinsona added this to the 2.7.x milestone Nov 29, 2021
@wilkinsona wilkinsona added the for: merge-with-amendments Needs some changes when we merge label Nov 29, 2021
@jerrinot
Copy link
Contributor Author

jerrinot commented Dec 2, 2021

Hi @wilkinsona,

may I kindly ask you what are the next steps with this PR? Is there anything else I could do to help merge it? I see you added a flag "merge-with-amendments". What are these amendments?

Thanks a lot!

@wilkinsona
Copy link
Member

Thanks for asking, @jerrinot. There's nothing more for you to do. The amendments are just cosmetic changes to align with the project's code style. We'll take care of those as part of merging the changes. Work on 2.7 is only just getting started (we're in a planning phase at the moment) with the first milestone not due until 20 January.

@jerrinot
Copy link
Contributor Author

jerrinot commented Dec 3, 2021

@wilkinsona thank you, that's very good to know!

@snicoll snicoll changed the title Inject SpringManagedContext into Hazelcast configuration Allow injection of Spring-managed beans into Hazelcast components Jan 4, 2022
@snicoll snicoll self-assigned this Jan 4, 2022
@snicoll snicoll modified the milestones: 2.7.x, 2.7.0-M1 Jan 4, 2022
snicoll pushed a commit that referenced this pull request Jan 4, 2022
This commit makes it possible to inject Spring managed beans into
objects instantiated by Hazelcast.

See gh-28801
@snicoll snicoll closed this in 69dbc34 Jan 4, 2022
@jerrinot jerrinot deleted the improvements/hazelcast-inject-managed-context branch January 4, 2022 09:10
@jerrinot
Copy link
Contributor Author

jerrinot commented Jan 4, 2022

@snicoll many thanks for taking this over the finish line.
Feel free to reach out to me if you need any help with anything Hazelcast-related!

snicoll added a commit that referenced this pull request Jan 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: merge-with-amendments Needs some changes when we merge type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create Hazelcast config with SpringManagedContext when available
5 participants