Allow EntityManagerFactoryBuilder to also add PersistenceUnitPostProcessor instances

[Asanio06]

The goal of this commit is to enable the EntityManagerFactoryBuilder to automatically detect and apply PersistenceUnitPostProcessor beans.
Currently, it is possible to use an EntityManagerFactoryBuilderCustomizer, but since there is no getter providing access to the already registered PersistenceUnitPostProcessor instances, using setPersistenceUnitPostProcessors may unintentionally remove existing ones.

[snicoll]

Thanks for the PR. I've left a couple comments for your consideration.

[snicoll]

This should be an ObjectProvider as well.

[Asanio06]

Oh yes, The change has been made.

[snicoll]

This replaces an existing test. I think we should leave it as it is and create a new one that demonstrates the code you've changed.

[Asanio06]

Yes, absolutely, I have created a new test.

[snicoll]

We'll have to figure out what we want to do here. I am not 100% sure that registering them from beans is the right call as PersistenceUnitPostProcessor is from Spring Framework and has already a well-defined scope:

Implementations can be registered with a DefaultPersistenceUnitManager or via a org.springframework.orm.jpa.LocalContainerEntityManagerFactoryBean.

Perhaps a more narrowed improvement would be to change EntityManagerFactoryBuilder so that it can add a PersistenceUnitPostProcessor rather than always replacing the existing list. Once this is in place, then you can use EntityManagerFactoryBuilderCustomizer and set/add a PersistenceUnitPostProcessor as you see fit (including taking the ones from the context if that works for you).

I've flagged for team attention so that we can get more feedback before any further change is made. Thanks again.

[Asanio06]

@snicoll Indeed, adding an "add" would be very interesting. Maybe both approaches are worth adding.
Thanks for the comments, I’m waiting for your feedback on the next steps 🙂

[nosan]

Perhaps a more narrowed improvement would be to change EntityManagerFactoryBuilder so that it can add a PersistenceUnitPostProcessor rather than always replacing the existing list. Once this is in place, then you can use EntityManagerFactoryBuilderCustomizer and set/add a PersistenceUnitPostProcessor as you see fit (including taking the ones from the context if that works for you).

I think adding an addPersistenceUnitPostProcessors(...) or maybe additionalPersistenceUnitPostProcessors(...) method to EntityManagerFactoryBuilder looks much better to me than registering PersistenceUnitPostProcessor instances as @Bean and then setting them on the EntityManagerFactoryBuilder, with a chance that someone could later update them using customizers.

With the customizer approach, there's only one way to apply them; with beans, there would be two - and honestly, I don't see any advantage in that.

[Asanio06]

Thanks @nosan @snicoll for feedbacks. I make the change

[wilkinsona]

I'm increasingly not a fan of defining things as beans purely so that they can be registered with something else. With that in mind, I'd vote for methods on the builders and the use of a customizer.