Hi guys, a quick note on that commit (and if you have a better idea it would be very very welcomed).
Issue is JPA needs to know the BeanManager reference now. First idea was to start CDI before JPA (yes it breaks the class transformation lifecycle but let's assume it doesn't). Issue is then if CDI uses a startup event then JPA can be missing and the app will not boot. So we need to start JPA before CDI but hibernate for instance uses CDI when the EMF is created to lookup the listeners in CDI context...which fails cause CDI is not yet booted. I don't see a good solution to that so I basically made a proxy to the bean manager available to JPA and added a property to support the lazy creation of the EMF (see https://issues.apache.org/jira/browse/TOMEE-1951). It seems it works - I expect some test to break but will keep an eye on that - but it can break some tomee 7.0.[0|1] applications relying on hibernate (maybe eclipselink, didnt check yet but build will say us) and I'm not very happy of that state even if it is the best I found. Wanted to avoid to do some kind of reactive implementation popping up threads "on demand" which would make the application quite uncontrolled compared to what we do today which is quite predictable. Happy to get help on that topic if you see a small hole we can use to make it smoother. Romain Manni-Bucau @rmannibucau <https://twitter.com/rmannibucau> | Blog <https://blog-rmannibucau.rhcloud.com> | Old Wordpress Blog <http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> | LinkedIn <https://www.linkedin.com/in/rmannibucau> | Tomitriber <http://www.tomitribe.com> | JavaEE Factory <https://javaeefactory-rmannibucau.rhcloud.com> ---------- Forwarded message ---------- From: <rmannibu...@apache.org> Date: 2016-10-03 11:03 GMT+02:00 Subject: tomee git commit: TOMEE-1951 making JPA CDI aware To: comm...@tomee.apache.org Repository: tomee Updated Branches: refs/heads/master 63792008e -> 0737513ae TOMEE-1951 making JPA CDI aware Project: http://git-wip-us.apache.org/repos/asf/tomee/repo Commit: http://git-wip-us.apache.org/repos/asf/tomee/commit/0737513a Tree: http://git-wip-us.apache.org/repos/asf/tomee/tree/0737513a Diff: http://git-wip-us.apache.org/repos/asf/tomee/diff/0737513a Branch: refs/heads/master Commit: 0737513ae6e4755ac94699b3b8b09ee2371ece09 Parents: 6379200 Author: rmannibucau <rmannibu...@apache.org> Authored: Mon Oct 3 11:02:59 2016 +0200 Committer: rmannibucau <rmannibu...@apache.org> Committed: Mon Oct 3 11:02:59 2016 +0200 ---------------------------------------------------------------------- .../openejb/assembler/classic/Assembler.java | 6 +- .../classic/EntityManagerFactoryCallable.java | 53 +++++++- .../assembler/classic/PersistenceBuilder.java | 9 +- .../classic/ReloadableEntityManagerFactory.java | 47 ++++--- .../openejb/persistence/JtaEntityManager.java | 9 +- .../openejb/jpa/JPAHasBeanManagerTest.java | 121 +++++++++++++++++++ 6 files changed, 216 insertions(+), 29 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tomee/blob/0737513a/ container/openejb-core/src/main/java/org/apache/openejb/ assembler/classic/Assembler.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/main/java/org/apache/openejb/ assembler/classic/Assembler.java b/container/openejb-core/src/ main/java/org/apache/openejb/assembler/classic/Assembler.java index ae52f14..08eb1a3 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/ assembler/classic/Assembler.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/ assembler/classic/Assembler.java @@ -765,6 +765,8 @@ public class Assembler extends AssemblerTool implements org.apache.openejb.spi.A final Map<String, Object> appBindings = appBuilder.buildBindings(JndiEncBuilder.JndiScope.app); final Context appJndiContext = appBuilder.build(appBindings); + final boolean cdiActive = shouldStartCdi(appInfo); + try { // Generate the cmp2/cmp1 concrete subclasses final CmpJarBuilder cmpJarBuilder = new CmpJarBuilder(appInfo, classLoader); @@ -861,7 +863,7 @@ public class Assembler extends AssemblerTool implements org.apache.openejb.spi.A for (final PersistenceUnitInfo info : appInfo.persistenceUnits) { final ReloadableEntityManagerFactory factory; try { - factory = persistenceBuilder. createEntityManagerFactory(info, classLoader, validatorFactoriesByConfig); + factory = persistenceBuilder. createEntityManagerFactory(info, classLoader, validatorFactoriesByConfig, cdiActive); containerSystem.getJNDIContext().bind( PERSISTENCE_UNIT_NAMING_CONTEXT + info.id, factory); units.put(info.name, PERSISTENCE_UNIT_NAMING_CONTEXT + info.id); } catch (final NameAlreadyBoundException e) { @@ -907,7 +909,7 @@ public class Assembler extends AssemblerTool implements org.apache.openejb.spi.A propagateApplicationExceptions(appInfo, classLoader, allDeployments); } - if (shouldStartCdi(appInfo)) { + if (cdiActive) { new CdiBuilder().build(appInfo, appContext, allDeployments); ensureWebBeansContext(appContext); appJndiContext.bind("app/BeanManager", appContext.getBeanManager()); http://git-wip-us.apache.org/repos/asf/tomee/blob/0737513a/ container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/ EntityManagerFactoryCallable.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/main/java/org/apache/openejb/ assembler/classic/EntityManagerFactoryCallable.java b/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/ EntityManagerFactoryCallable.java index cb5706b..854c9c4 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/ assembler/classic/EntityManagerFactoryCallable.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/ assembler/classic/EntityManagerFactoryCallable.java @@ -19,11 +19,20 @@ package org.apache.openejb.assembler.classic; import org.apache.openejb.loader.SystemInstance; import org.apache.openejb.persistence.PersistenceUnitInfoImpl; +import org.apache.openejb.util.LogCategory; +import org.apache.openejb.util.Logger; +import org.apache.webbeans.config.WebBeansContext; +import org.apache.webbeans.container.InjectableBeanManager; +import javax.enterprise.inject.spi.BeanManager; import javax.persistence.EntityManagerFactory; import javax.persistence.ValidationMode; import javax.persistence.spi.PersistenceProvider; import javax.validation.ValidatorFactory; +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; import java.util.HashMap; import java.util.Map; import java.util.concurrent.Callable; @@ -35,14 +44,29 @@ public class EntityManagerFactoryCallable implements Callable<EntityManagerFacto private final String persistenceProviderClassName; private final PersistenceUnitInfoImpl unitInfo; private final Map<ComparableValidationConfig, ValidatorFactory> potentialValidators; + private final boolean cdi; private ClassLoader appClassLoader; public EntityManagerFactoryCallable(final String persistenceProviderClassName, final PersistenceUnitInfoImpl unitInfo, - final ClassLoader cl, final Map<ComparableValidationConfig, ValidatorFactory> validators) { + final ClassLoader cl, final Map<ComparableValidationConfig, ValidatorFactory> validators, + final boolean hasCdi) { this.persistenceProviderClassName = persistenceProviderClassName; this.unitInfo = unitInfo; this.appClassLoader = cl; this.potentialValidators = validators; + this.cdi = hasCdi; + } + + public Class<?> getProvider() { + final ClassLoader old = Thread.currentThread(). getContextClassLoader(); + Thread.currentThread().setContextClassLoader(appClassLoader); + try { + return appClassLoader.loadClass(persistenceProviderClassName); + } catch (final ClassNotFoundException e) { + throw new IllegalArgumentException(e); + } finally { + Thread.currentThread().setContextClassLoader(old); + } } @Override @@ -58,14 +82,37 @@ public class EntityManagerFactoryCallable implements Callable<EntityManagerFacto if (!ValidationMode.NONE.equals(unitInfo.getValidationMode())) { properties.put("javax.persistence.validation.factory", new ValidatorFactoryWrapper(potentialValidators)); } + if (cdi && "true".equalsIgnoreCase(unitInfo.getProperties().getProperty("tomee.jpa.cdi", "true")) + && "true".equalsIgnoreCase(SystemInstance.get().getProperty("tomee.jpa.cdi", "true"))) { + properties.put("javax.persistence.bean.manager", // TODO: impl a passthrough BM? + Proxy.newProxyInstance(appClassLoader, new Class<?>[]{BeanManager.class}, new InvocationHandler() { + private volatile BeanManager bm; + + @Override + public Object invoke(final Object proxy, final Method method, final Object[] args) throws Throwable { + try { + return method.invoke(findBm(), args); + } catch (final InvocationTargetException ite) { + Logger.getInstance(LogCategory.OPENEJB_JPA, EntityManagerFactoryCallable.class) + .warning("Exception calling CDI, if a lifecycle issue you should maybe set tomee.jpa.factory.lazy=true", ite.getCause()); + throw ite.getCause(); + } + } + + private Object findBm() { + return bm != null ? bm : (bm = new InjectableBeanManager(WebBeansContext.currentInstance(). getBeanManagerImpl())); + } + }) + ); + } customizeProperties(properties); final EntityManagerFactory emf = persistenceProvider. createContainerEntityManagerFactory(unitInfo, properties); if (unitInfo.getProperties() != null - && "true".equalsIgnoreCase(unitInfo.getProperties(). getProperty(OPENEJB_JPA_INIT_ENTITYMANAGER)) - || SystemInstance.get().getOptions().get(OPENEJB_JPA_INIT_ENTITYMANAGER, false)) { + && "true".equalsIgnoreCase(unitInfo.getProperties(). getProperty(OPENEJB_JPA_INIT_ENTITYMANAGER)) + || SystemInstance.get().getOptions().get(OPENEJB_JPA_INIT_ENTITYMANAGER, false)) { emf.createEntityManager().close(); } http://git-wip-us.apache.org/repos/asf/tomee/blob/0737513a/ container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/ PersistenceBuilder.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/main/java/org/apache/openejb/ assembler/classic/PersistenceBuilder.java b/container/openejb-core/src/ main/java/org/apache/openejb/assembler/classic/PersistenceBuilder.java index 992404e..5a54b21 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/ assembler/classic/PersistenceBuilder.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/ assembler/classic/PersistenceBuilder.java @@ -25,7 +25,6 @@ import org.apache.openejb.spi.ContainerSystem; import org.apache.openejb.util.LogCategory; import org.apache.openejb.util.Logger; -import java.util.Map; import javax.naming.Context; import javax.naming.InitialContext; import javax.naming.NamingException; @@ -35,6 +34,7 @@ import javax.persistence.spi. PersistenceUnitTransactionType; import javax.sql.CommonDataSource; import javax.sql.DataSource; import javax.validation.ValidatorFactory; +import java.util.Map; public class PersistenceBuilder { @@ -50,7 +50,8 @@ public class PersistenceBuilder { } public ReloadableEntityManagerFactory createEntityManagerFactory(final PersistenceUnitInfo info, final ClassLoader classLoader, - final Map<ComparableValidationConfig, ValidatorFactory> validators) throws Exception { + final Map<ComparableValidationConfig, ValidatorFactory> validators, + final boolean hasCdi) throws Exception { final PersistenceUnitInfoImpl unitInfo = new PersistenceUnitInfoImpl(persistenceClassLoaderHandler); // Persistence Unit Id @@ -68,7 +69,7 @@ public class PersistenceBuilder { // Exclude Unlisted Classes unitInfo.setExcludeUnlistedClasses(info.excludeUnlistedClasses); - unitInfo.setLazilyInitialized(info.webappName != null); + unitInfo.setLazilyInitialized(info.webappName != null || "true".equalsIgnoreCase(info.properties.getProperty("tomee.jpa.factory.lazy", "false"))); final Context context = SystemInstance.get(). getComponent(ContainerSystem.class).getJNDIContext(); @@ -151,7 +152,7 @@ public class PersistenceBuilder { final String persistenceProviderClassName = unitInfo. getPersistenceProviderClassName(); unitInfo.setPersistenceProviderClassName( persistenceProviderClassName); - final EntityManagerFactoryCallable callable = new EntityManagerFactoryCallable(persistenceProviderClassName, unitInfo, classLoader, validators); + final EntityManagerFactoryCallable callable = new EntityManagerFactoryCallable(persistenceProviderClassName, unitInfo, classLoader, validators, hasCdi); return new ReloadableEntityManagerFactory(classLoader, callable, unitInfo); } http://git-wip-us.apache.org/repos/asf/tomee/blob/0737513a/ container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/ ReloadableEntityManagerFactory.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/main/java/org/apache/openejb/ assembler/classic/ReloadableEntityManagerFactory.java b/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/ ReloadableEntityManagerFactory.java index c61b207..a9ba815 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/ assembler/classic/ReloadableEntityManagerFactory.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/ assembler/classic/ReloadableEntityManagerFactory.java @@ -86,7 +86,7 @@ public class ReloadableEntityManagerFactory implements EntityManagerFactory, Ser private final PersistenceUnitInfoImpl unitInfoImpl; private ClassLoader classLoader; - private EntityManagerFactory delegate; + private volatile EntityManagerFactory delegate; private final EntityManagerFactoryCallable entityManagerFactoryCallable; private ObjectName objectName; @@ -112,6 +112,21 @@ public class ReloadableEntityManagerFactory implements EntityManagerFactory, Ser unitInfoImpl.setClassLoader(loader); } + public EntityManagerFactoryCallable getEntityManagerFactoryCallable() { + return entityManagerFactoryCallable; + } + + private EntityManagerFactory delegate() { + if (delegate == null) { + synchronized (this) { + if (delegate == null) { + createDelegate(); + } + } + } + return delegate; + } + public void createDelegate() { JPAThreadContext.infos.put("properties", entityManagerFactoryCallable.getUnitInfo().getProperties()); final long start = System.nanoTime(); @@ -145,7 +160,7 @@ public class ReloadableEntityManagerFactory implements EntityManagerFactory, Ser public EntityManager createEntityManager() { EntityManager em; try { - em = delegate.createEntityManager(); + em = delegate().createEntityManager(); } catch (final LinkageError le) { em = delegate.createEntityManager(); } @@ -160,7 +175,7 @@ public class ReloadableEntityManagerFactory implements EntityManagerFactory, Ser public EntityManager createEntityManager(final Map map) { EntityManager em; try { - em = delegate.createEntityManager(map); + em = delegate().createEntityManager(map); } catch (final LinkageError le) { em = delegate.createEntityManager(map); } @@ -175,7 +190,7 @@ public class ReloadableEntityManagerFactory implements EntityManagerFactory, Ser public EntityManager createEntityManager(final SynchronizationType synchronizationType) { EntityManager em; try { - em = delegate.createEntityManager(synchronizationType); + em = delegate().createEntityManager(synchronizationType); } catch (final LinkageError le) { em = delegate.createEntityManager(synchronizationType); } @@ -190,7 +205,7 @@ public class ReloadableEntityManagerFactory implements EntityManagerFactory, Ser public EntityManager createEntityManager(final SynchronizationType synchronizationType, final Map map) { EntityManager em; try { - em = delegate.createEntityManager(synchronizationType, map); + em = delegate().createEntityManager(synchronizationType, map); } catch (final LinkageError le) { em = delegate.createEntityManager(synchronizationType, map); } @@ -206,56 +221,56 @@ public class ReloadableEntityManagerFactory implements EntityManagerFactory, Ser if (cls.isAssignableFrom(getClass())) { return cls.cast(this); } - return delegate.unwrap(cls); + return delegate().unwrap(cls); } @Override public void addNamedQuery(final String name, final Query query) { - delegate.addNamedQuery(name, query); + delegate().addNamedQuery(name, query); } @Override public <T> void addNamedEntityGraph(final String graphName, final EntityGraph<T> entityGraph) { - delegate.addNamedEntityGraph(graphName, entityGraph); + delegate().addNamedEntityGraph(graphName, entityGraph); } @Override public CriteriaBuilder getCriteriaBuilder() { - return delegate.getCriteriaBuilder(); + return delegate().getCriteriaBuilder(); } @Override public Metamodel getMetamodel() { - return delegate.getMetamodel(); + return delegate().getMetamodel(); } @Override public boolean isOpen() { - return delegate.isOpen(); + return delegate().isOpen(); } @Override public void close() { - delegate.close(); + delegate().close(); } @Override public Map<String, Object> getProperties() { - return delegate.getProperties(); + return delegate().getProperties(); } @Override public Cache getCache() { - return delegate.getCache(); + return delegate().getCache(); } @Override public PersistenceUnitUtil getPersistenceUnitUtil() { - return delegate.getPersistenceUnitUtil(); + return delegate().getPersistenceUnitUtil(); } public EntityManagerFactory getDelegate() { - return delegate; + return delegate(); } public void register() throws OpenEJBException { http://git-wip-us.apache.org/repos/asf/tomee/blob/0737513a/ container/openejb-core/src/main/java/org/apache/openejb/ persistence/JtaEntityManager.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/main/java/org/apache/openejb/ persistence/JtaEntityManager.java b/container/openejb-core/src/ main/java/org/apache/openejb/persistence/JtaEntityManager.java index 208f6d5..79d3154 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/ persistence/JtaEntityManager.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/ persistence/JtaEntityManager.java @@ -120,13 +120,14 @@ public class JtaEntityManager implements EntityManager, Serializable { public static boolean isJPA21(final EntityManagerFactory entityManagerFactory) { return ReloadableEntityManagerFactory.class.isInstance(entityManagerFactory) ? - isJPA21(ReloadableEntityManagerFactory.class.cast( entityManagerFactory).getDelegate()) - : hasMethod(entityManagerFactory, "createEntityManager", SynchronizationType.class); + hasMethod( + ReloadableEntityManagerFactory.class.cast( entityManagerFactory).getEntityManagerFactoryCallable().getProvider(), + "createEntityManager", SynchronizationType.class) + : hasMethod(entityManagerFactory.getClass(), "createEntityManager", SynchronizationType.class); } - private static boolean hasMethod(final Object object, final String name, final Class<?>... params) { + private static boolean hasMethod(final Class<?> objectClass, final String name, final Class<?>... params) { try { - final Class<?> objectClass = object.getClass(); Boolean result = IS_JPA21.get(objectClass); if (result == null) { result = !Modifier.isAbstract(objectClass.getMethod(name, params).getModifiers()); http://git-wip-us.apache.org/repos/asf/tomee/blob/0737513a/ container/openejb-core/src/test/java/org/apache/openejb/ jpa/JPAHasBeanManagerTest.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/test/java/org/apache/openejb/jpa/JPAHasBeanManagerTest.java b/container/openejb-core/src/test/java/org/apache/openejb/ jpa/JPAHasBeanManagerTest.java new file mode 100644 index 0000000..9e42db3 --- /dev/null +++ b/container/openejb-core/src/test/java/org/apache/openejb/ jpa/JPAHasBeanManagerTest.java @@ -0,0 +1,121 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.openejb.jpa; + +import org.apache.openejb.api.configuration.PersistenceUnitDefinition; +import org.apache.openejb.junit.ApplicationComposer; +import org.apache.openejb.testing.Classes; +import org.apache.openjpa.persistence.OpenJPAEntityManagerFactory; +import org.apache.openjpa.persistence.PersistenceProviderImpl; +import org.junit.Test; +import org.junit.runner.RunWith; + +import javax.annotation.PostConstruct; +import javax.ejb.Singleton; +import javax.ejb.Startup; +import javax.enterprise.context.ApplicationScoped; +import javax.enterprise.context.Initialized; +import javax.enterprise.event.Observes; +import javax.enterprise.inject.spi.BeanManager; +import javax.inject.Inject; +import javax.persistence.Entity; +import javax.persistence.EntityManager; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; +import javax.persistence.PersistenceContext; +import javax.persistence.spi.PersistenceUnitInfo; +import javax.transaction.Transactional; +import java.util.Map; + +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +@Classes(cdi = true, innerClassesAsBean = true) +@RunWith(ApplicationComposer.class) +public class JPAHasBeanManagerTest { + @Inject + private Dao dao; + + @Test + public void run() { + dao.doAsserts(); + } + + @Transactional + @ApplicationScoped + @PersistenceUnitDefinition( + entitiesPackage = "org.apache.openejb.jpa", + properties = {"openjpa.RuntimeUnenhancedClasses=supported", "tomee.jpa.factory.lazy=true"}, + provider = "org.apache.openejb.jpa.JPAHasBeanManagerTest$ TheTestProvider") + public static class Dao { + @PersistenceContext + private EntityManager em; + private TheTestEntity persisted; + + public void start(@Observes @Initialized(ApplicationScoped.class) final Object boot) { + TheTestEntity entity = new TheTestEntity(); + em.persist(entity); // ensure it works + persisted = entity; + } + + public void doAsserts() { + assertNotNull(TheTestProvider.MAP); + final Object bm = TheTestProvider.MAP.get(" javax.persistence.bean.manager"); + assertNotNull(bm); + assertTrue(BeanManager.class.isInstance(bm)); + assertNotNull(em.find(TheTestEntity.class, persisted.getId())); + } + } + + @Startup + @Singleton + public static class EJBDao { + @PersistenceContext + private EntityManager em; + + @PostConstruct + public void start() { + TheTestEntity entity = new TheTestEntity(); + em.persist(entity); // ensure it works + } + } + + public static class TheTestProvider extends PersistenceProviderImpl { + private static Map MAP; + + @Override + public OpenJPAEntityManagerFactory createContainerEntityManagerFactory(final PersistenceUnitInfo pui, final Map m) { + MAP = m; + // only works cause of lazy property + final BeanManager beanManager = BeanManager.class.cast(m.get(" javax.persistence.bean.manager")); + assertNotNull(beanManager.getReference(beanManager. resolve(beanManager.getBeans(Dao.class)), Dao.class, null)); + // just delegate to openjpa since we don't aim at reimplementing JPA in a test ;) + return super.createContainerEntityManagerFactory(pui, m); + } + } + + @Entity + public static class TheTestEntity { + @Id + @GeneratedValue + private long id; + + public long getId() { + return id; + } + } +}
