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 PersistenceUnitPostProcessor to be configured on EntityManagerFactoryBuilder #25443

Conversation

JohnNiang
Copy link
Contributor

Enable persistence unit post processors configurable in EntityManagerFactoryBuilder

This PR enables persistence unit post processors configurable by implementing EntityManagerFactoryBuilderCustomizer.java. It works only for org.springframework.orm.jpa.LocalContainerEntityManagerFactoryBean#internalPersistenceUnitManager.

@pivotal-issuemaster
Copy link

@JohnNiang 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-issuemaster
Copy link

@JohnNiang Thank you for signing the Contributor License Agreement!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 26, 2021
Copy link
Member

@snicoll snicoll left a comment

Choose a reason for hiding this comment

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

Thank you for the PR. Could you please provide a bit more context that led you to create this PR?

We can't consider merging this until the code that is changed is exercised by unit tests. Can you please add that as well?

* Such post-processors can, for example, register further entity classes and jar
* files, in addition to the metadata read from {@code persistence.xml}.
* <p>
* <b>NOTE: Only applied if no external PersistenceUnitManager specified.</b>
Copy link
Member

Choose a reason for hiding this comment

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

You wrote that in the PR description and I am not sure I understood what you're trying to convey here. In the context of this particular class, does it matter?

Copy link
Contributor Author

@JohnNiang JohnNiang Feb 26, 2021

Choose a reason for hiding this comment

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

I copied from org.springframework.orm.jpa.LocalContainerEntityManagerFactoryBean#setPersistenceUnitPostProcessors

Copy link
Member

Choose a reason for hiding this comment

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

There's no need to do that.

Copy link
Contributor Author

@JohnNiang JohnNiang Feb 26, 2021

Choose a reason for hiding this comment

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

So what should I do here? Remove or simplify them.

Copy link
Member

Choose a reason for hiding this comment

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

Simplify. You can look at other methods in this class for inspiration. More importantly, tests should be added.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I'll try hard.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are not tests related EntityManagerFactoryBuilder present, could you provide any suggestions for incoming tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe I should learn more about org.springframework.boot.autoconfigure.orm.jpa.AbstractJpaAutoConfigurationTests.

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Feb 26, 2021
@JohnNiang
Copy link
Contributor Author

@snicoll

If the changes are applied, so that we could customize MutablePersistenceUnitInfo as follow:

@Bean
EntityManagerFactoryBuilderCustomizer entityManagerFactoryBuilderCustomizer() {
	return builder -> {
		builder.setPersistenceUnitPostProcessors(pui -> {
			pui.addManagedClassName("customized.attribute.converter.class.name");
                        // other operations on MutablePersistenceUnitInfo
                        pui...
		});
	};
}

A scene is that we can register attribute converters into org.hibernate.jpa.boot.spi.EntityManagerFactoryBuilder programmatically. Because default fetching managed class names perform is scanning from class path only.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Feb 26, 2021
@snicoll snicoll added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Feb 26, 2021
@snicoll snicoll removed their request for review February 26, 2021 09:07
@snicoll snicoll added type: enhancement A general enhancement and removed status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged labels Feb 27, 2021
@snicoll snicoll added this to the 2.5.x milestone Feb 27, 2021
snicoll pushed a commit that referenced this pull request Feb 27, 2021
This commit updates EntityManagerFactoryBuilder so that persistence unit
post processors can be registered and applied when creating an
EntityManagerFactory.

See gh-25443
snicoll added a commit that referenced this pull request Feb 27, 2021
This commit updates EntityManagerFactoryBuilder so that persistence unit
post processors can be registered and applied when creating an
EntityManagerFactory.

See gh-25443
@snicoll snicoll closed this in 1bd603f Feb 27, 2021
@snicoll snicoll changed the title Enable persistence unit post processors configurable in EntityManager… Allow PersistenceUnitPostProcessor to be configured on EntityManagerFactoryBuilder Feb 27, 2021
@snicoll snicoll self-assigned this Feb 27, 2021
@snicoll snicoll modified the milestones: 2.5.x, 2.5.0-M3 Feb 27, 2021
@snicoll
Copy link
Member

snicoll commented Feb 27, 2021

@JohnNiang thank you for making your first contribution to Spring Boot.

@JohnNiang JohnNiang deleted the feat/more-control-in-entitymanagerfactorybuilder branch February 27, 2021 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants