Re: Code review for Issue 1777 createEMF fix incorrect: Should return null instead of throwing exception
Hi Wonseok,
The fix looks good.
-Tom
Wonseok Kim wrote:
> Hi Tom,
>
> The attached is the fix for following issue. Please review.
>
> Thanks,
> -Wonseok
>
> ---------- Forwarded message ----------
> From: * rancidfishbreath_at_dev.java.net
> <mailto:rancidfishbreath_at_dev.java.net>* <rancidfishbreath_at_dev.java.net
> <mailto:rancidfishbreath_at_dev.java.net>>
> Date: 16 Dec 2006 00:03:14 -0000
> Subject: [Issue 1777] New - createEMF fix incorrect: Should return
> null instead of throwing exception
> To: guruwons_at_dev.java.net <mailto:guruwons_at_dev.java.net>
>
> https://glassfish.dev.java.net/issues/show_bug.cgi?id=1777
> Issue #|1777
> Summary|createEMF fix incorrect: Should return null
> instead of
> | throwing exception
> Component|glassfish
> Version|9.0pe
> Platform|All
> OS/Version|All
> URL|
> Status|UNCONFIRMED
> Status whiteboard|
> Keywords|
> Resolution|
> Issue type|DEFECT
> Priority|P3
> Subcomponent|entity-persistence
> Assigned to|tware
> Reported by|rancidfishbreath
>
>
>
>
>
>
> ------- Additional comments from rancidfishbreath_at_dev.java.net
> <mailto:rancidfishbreath_at_dev.java.net> Sat Dec 16 00:03:13 +0000 2006
> -------
> <a href=https://glassfish.dev.java.net/issues/show_bug.cgi?id=854
> <https://glassfish.dev.java.net/issues/show_bug.cgi?id=854>>Issue
> 854</a> was not resolved
> correctly. The Javadoc for EntityManagerFactoryProvider's method
> createEntityManagerFactory(String
> emName, Map properties) states "@return EntityManagerFactory for the
> persistence unit, or null if the
> provider is not the right provider". The current fix:
>
> if (emSetupImpl == null) {
> throw new
> PersistenceException(EntityManagerSetupException.puNotExist(name));
> }
>
> should instead be:
>
> if (emSetupImpl == null) {
> return null;
> }
>
> This is important because the class Persistence's method
> createEntityManagerFactory(String
> persistenceUnitName, Map properties) has a loop that looks like this:
>
> for (PersistenceProvider provider : providers) {
> emf = provider.createEntityManagerFactory(persistenceUnitName,
> properties);
> if (emf != null){
> break;
> }
> }
>
> what EntityManagerFactoryProvider is doing is throwing an exception
> causing this loop to be dropped
> out of prematurely. If EntityManagerFactoryProvider behaved nicely
> and returned a null as explained
> above, this loop would give other PersistenceProviders a crack at
> finding the EntityManagerFactory.