BatooOrg / BatooJPA

Batoo JPA - Fastest JPA Implementation Ever! Over 15 times faster than the leading JPA implementations.
http://batoo.org
137 stars 32 forks source link

Entity classes are not recognized without explicitly enumeration in persistence.xml #56

Closed schueffi closed 12 years ago

schueffi commented 12 years ago

As per JPA-Spec, entity bean classes may be listed in persistence.xml either by class name, by "jar-name" (wich in turn adds all contained entity classes within the given jar files), some xml reference, or automatically by rules as defined in section 8.2.6.1.

According to section 8.2.1.6, bullet point one, all classes contained in the persistence unit itself are added automatically (except when "exclude unlisted classes" is true, which is false per default in JavaEE).

Batoo ignores any settings of "exclude unlisted classes", and just only adds all named "class"es.

Consequently, our ejb-jar does not work without listing all classes explicitly in persistence.xml (which works just fine in hibernate).

hceylan commented 12 years ago

Thank you for the bug report.

We will solve this ASAP.

hceylan commented 12 years ago

Fixed and the snapshot batoo-jpa-0.1.7-20121031.091028-12.jar deployed.

Please verify.

schueffi commented 12 years ago

Using the snapshot, regardles of exclude-unlisted-classes set to true or false, batoo does not load any of our entity classes contained in the same (ejb-)jar file as the persistence.xml file (aka the persistence unit root).

So, as before with 0.1.6, the only way to use batoo is to list all classes explicitly using "class" entries in persistence.xml

hceylan commented 12 years ago

Hey Stefan, could you try this with the latest snapshot? If this is solved I'd like too make a release.

schueffi commented 12 years ago

Again, using snapshot 17, batoo does not recognize any entity classes contained within the same jar file as persistence.xml when not listed explicitly in persistence.xml.

I tried this both without exclude-unlisted-classes and with exclude-unlisted-classes explicitly set to false.

Although within the persistence_2_0.xsd the default value for this node is "true", the jpa specification clearly states in section 8.2.1.6.1: in java EE deployments, the persistence provider has to load all classes unless the unlisted-excluded-classes is explicitly set to true - i.e. in java ee environments the default is to load all entity classes contained in the same jar file as persistence.xml. In Java-SE-deployments, this exclude-unlisted-classes element should not be used at all - here you have to use "class" entries for each of your entity classes.

hceylan commented 12 years ago

Thanks Stefan,

I am on this issue now.

hceylan commented 12 years ago

Thank you for your patience.

I think now we got it right.

On exclude-unlisted-classes, Although the xsd states that we are passed this info by the application server. At least JBoss respects that and passes false as the default.

Please try snapshot 22.

schueffi commented 12 years ago

I just tried snapshot 22 - and again, batoo does not load the classes...

schueffi commented 12 years ago

Looking in the source code of

parser/src/main/java/org/batoo/jpa/parser/impl/metadata/MetadataImpl.java

lmethod

public void parse(List<URL> jarFiles, ClassLoader classloader, List<String> managedClassNames, boolean excludeUnlistedClasses)

This method will be called with an empty jarFiles-list (which is fine, as i do not have jarFiles in persistence.xml), and an empty managedClassNames-list (which also is fine as i do not have classes in persistence.xml), and excludeUnlistedClasses = false (which again is fine).

Unfortunately, this.findClasses(classes, classloader) does not find all the contained classes.

hceylan commented 12 years ago

Is this an EAR or WAR project. Better yet, could you prepare a mock up please?

On Tue, Nov 6, 2012 at 8:49 AM, Stefan Schüffler notifications@github.comwrote:

I just tried snapshot 22 - and again, batoo does not load the classes...

— Reply to this email directly or view it on GitHubhttps://github.com/BatooOrg/BatooJPA/issues/56#issuecomment-10100843.

schueffi commented 12 years ago

It is an ear project. I think, the problem is inside findClasses(Set, ClassLoader). After determining all the "root" path segments in the classLoader, for each root a separate findClasses is called with the last argument "new File(root)"... This file just does not exists - at least not as a standard file. It only exists as a ressource within the containing classloader.

Example: i do have an ear-project like this:

myear.ear /lib/lib-a.jar /lib/lib-b.jar /myejb.jar

now, findClasses(Set, ClassLoader) gets called with a classloader containing those "root-paths"

ClassLoader: ModuleClassLoader for Module "deployment.myear.ear.myejb.jar:main" from Service Module Loader

Contained root paths when calling classloader.getRessources(""):
/home/jboss/jboss/modules/sun/jdk/main/service-loader-resources/
/content/myear.ear/myejb.jar/
/content/myear.ear/lib/lib-a.jar/
/content/myear.ear/lib/lib-b.jar/

Now, findClasses calls private findClasses for each of those paths e.g. with last argument new File("/content/myear.ear/myejb.jar/") which does not exist on my local drive.

schueffi commented 12 years ago

Beside the problem of non existing files, i am not sure if the routine should load all classes found by the classloader, or only the ones contained within the jar file where persistence.xml resides (i.e. in my example-ear, only the classes in myejb.jar, but not the ones in lib-a.jar and lib-b.jar, given that persistence.xml is in myejb.jar).

hceylan commented 12 years ago

Do you list the model jars in persistence.xml?

Following scenario works for me:

web.war /WEB-INF/lib/model-jar /WEB-INF/classes/META-INF/persistence.xml

<?xml version="1.0" encoding="UTF-8"?>
<persistence version="2.0"
    xmlns="http://java.sun.com/xml/ns/persistence" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xsi:schemaLocation="http://java.sun.com/xml/ns/persistence http://java.sun.com/xml/ns/persistence/persistence_2_0.xsd">
    <persistence-unit name="pu">
        <provider>org.batoo.jpa.core.BatooPersistenceProvider</provider>

        <jta-data-source>java:jboss/datasources/Benchmark</jta-data-source>
        <exclude-unlisted-classes>true</exclude-unlisted-classes>

        <jar-file>lib/model-0.0.1-SNAPSHOT.jar</jar-file>

        <properties>
            <property name="org.batoo.jpa.ddl" value="NONE" />
            <property name="org.batoo.jdbc.import_sql" value="import.sql"/>

            <!-- jboss7 specific -->
            <property name="jboss.as.jpa.providerModule" value="org.batoo" />
        </properties>
    </persistence-unit>
</persistence>

In container environment, we are provided the PersistenceUnitInfo, so, we do not look around for the persistence.xml file. That being the case, the classloader is the only way to search for classes unless `exclude-unlisted-classes, set to true.

As in your scenario, the persistence layer and the business layer may reside on different servers. Therefore you may have the following deployment:

mybusiness.ear /lib/lib-a.jar /lib/model.jar /myejb.jar

mypresentation.ear /lib/lib-a.jar /lib/lib-b.jar /mymodel.jar /mywar.war

In this scenario you do not want to deploy persistence unit into the presentation layer, you merely want to have the classes be available to reconstruct the data returned by the EJB server (Business server).

So I think it is required that

Therefore in you case, I believe you should place the persistence.xml in the EAR file with reference to JAR file in the persistence.xml

schueffi commented 12 years ago

Hi

i do not reference any classes or jar files in persistence.xml Although i do have an ear-deployment, my persistence.xml resides in myejb.jar.

To make things clear, i now list some example ear, and my expectations on what should be loaded and what not:

myapp.ear

    /lib/my-lib-with-utitlies.jar   // does not contain any entity classes or annotations.

    /lib/my-lib-with-entites.jar
           /B.class       // this class is annotated with @Entity

   /my-ejb.jar
       /META-INF/persistence.xml   // does not reference any classes or jar files
       /A.class  // this class is annotated with @Entity
       /ServiceBean.class // this is my stateless session bean

  /web.war  // does not contain any entity classes, but uses A through ServiceBean

Now, i think, the expected outcome after loading the entites through the jpa provider should be: A.class (and only A.class, not B.class) should be loaded automatically when there is no other class or jar file reference within my persistence.xml.

Why do i expect this outcome?

In section 8.2 of the spec, paragraph 3 (page 310), "the jar file or directory whose META-INF directory contains the persistence.xml file is termed the root of the persistence unit".

Furthermore, section 8.2.1.6.1 states "All classes contained in the root of the persistence unit are searched for annotated managed persistence classes [... unless exclude-unlisted-classes is explicitly set to true ...]"

Now, batoo (at least with snapshots from yesterday) loads the persistence unit, and scans everything contained in the classloader of the calling persistence unit (i.e. classloader.getResources("") ).

Problem one is now: Beside the fact (as of my comment https://github.com/BatooOrg/BatooJPA/issues/56#issuecomment-10101695 ) that this loading does not work as expected, batoo would end up having loaded A and B class when B is contained in the classloader (jboss default), or only A class if B is not contained (some class loader isolation like in more advanced deployments, perhaps other containers than jboss, ...).

And problem two: In jboss the search does not work at all as explained in the referenced comment. classloader.getRessources("") gives a list of URLs, but they do not refer actual files on the file system (at least not in my setup and deploying using the jboss-cli.sh

I am just working on a fix (possibly for both problems, but only for my environment linux and jboss - i am not aware if this works on windows or other containers than jboss). This will come in the next comment.

schueffi commented 12 years ago

First, a partial fix on how to locate the right jar file which has to be scanned (assuming that i am right with my thoughts about only A and not B

    private void findClasses(Set<Class<?>> classes, ClassLoader classPath) {
        try {
            // find the jars which have a persistence.xml
            final Enumeration<URL> resources = classPath.getResources("/META-INF/persistence.xml");
            while (resources.hasMoreElements()) {
                String root = resources.nextElement().toURI().toString();

                // TODO load the persistence.xml, and check if this actual persistence.xml is the one
                // belonging to the current persistence unit (i.e. the unitName == name of currentPersistenceUnit)
                // 
                // non matching xml files can happen if there are more than one persistence.xml on the
                // classpath of the current persistence unit. (e.g if in some lib or parent classloader 
                // also are some persistence.xml files. they have to be skipped, i think.)
                //
                // if (!matchingPersistenceXML(root, currentPersistenceUnitName)) {
                //  continue;
                // }

                // now we have the right persistence.xml and therefore the correct jar file
                // (persistence unit root) for the current persistence unit

                // navigate up to the root of the persistence unit 
                System.out.println(root);
                root = root.substring(0, root.lastIndexOf("/META-INF/persistence.xml"));
                System.out.println(root);
                this.findClasses(classPath, classes, root.length(), root);
            }
        } catch (final Exception e) {
            throw new PersistenceException("Cannot scan the classpath", e);
        }
    }                                                                                                                                                                       ```
schueffi commented 12 years ago

Now, the harder part is to scan the resulting URL for all classes and jpa annotations. Unfortunately, this is not as simple as new File(url), and then isDirectory() etc.

The protocol of the URL could be some

As a result, it unfortunately is not possible for me to give a quick fix. Some thoughts on how to implement this could be the Visitor as implemented in hibernate ( https://github.com/hibernate/hibernate-orm/blob/master/hibernate-entitymanager/src/main/java/org/hibernate/jpa/packaging/internal/JarVisitorFactory.java and possibly others), but i do not have a simple and quick idea on how to implement this in a clean way.

hceylan commented 12 years ago

First, a partial fix on how to locate the right jar file which has to be scanned (assuming that i am right with my thoughts about only A and not B

Very very good indeed. Never thought of that.

Jar visitor is easy, I just did not want to resort to complex visitor algorithms previously. As now it is a necessity, I think I must go down that path.

I really appreciate the input.

hceylan commented 12 years ago

OK, started to work on that to solve it once and for all.

First, a partial fix on how to locate the right jar file which has to be scanned (assuming that i am right with my thoughts about only A and not B

Fortunately, this has been cared of by the spec. PersistenceUnitInfo has a method called getPersistenceUnitRootUrl() that comes handy for locating the root of the persistence unit.

hceylan commented 12 years ago

For JBoss 7.x, Spring and standalone persistence, the bug has been fixed. The rest should be handled in each platform specific enhancement request.

Please check if batoo-jpa-2.0.0.RC2-20121113.034103-2.jar satisfies the spec and if it fails, re-open the issue.

schueffi commented 12 years ago

While testing the new snapshot, i now got new problems regarding parsing of orm.xml. So, unfortunately i now can not accept or reject this bugfix. I will now open a new bug report for the orm.xml, and test the current bugfix later on again.

schueffi commented 12 years ago

Unfortunately, the fix does not work with jboss-7 (7.1.3) - required classes are not found.

Caused by: java.lang.NoClassDefFoundError: org/jboss/jandex/Index
        at org.batoo.jpa.parser.impl.acl.JBoss7AnnotatedClassLocator.locateClasses(JBoss7AnnotatedClassLocator.java:105)
        at org.batoo.jpa.parser.impl.acl.BaseAnnotatedClassLocator.locateClasses(BaseAnnotatedClassLocator.java:69)
        at org.batoo.jpa.parser.impl.acl.BaseAnnotatedClassLocator.locatePersistentClasses(BaseAnnotatedClassLocator.java:106)
        at org.batoo.jpa.parser.impl.metadata.MetadataImpl.parse(MetadataImpl.java:337)
        at org.batoo.jpa.parser.PersistenceParserImpl.<init>(PersistenceParserImpl.java:100)
        at org.batoo.jpa.core.BatooPersistenceProvider.createContainerEntityManagerFactory(BatooPersistenceProvider.java:70)
        at org.batoo.jpa.core.BatooPersistenceProvider.createContainerEntityManagerFactory(BatooPersistenceProvider.java:43)
        at org.jboss.as.jpa.service.PersistenceUnitServiceImpl.createContainerEntityManagerFactory(PersistenceUnitServiceImpl.java:197) [jboss-as-jpa-7.1.3.Final.jar:7.1.3.Final]
        at org.jboss.as.jpa.service.PersistenceUnitServiceImpl.access$500(PersistenceUnitServiceImpl.java:57) [jboss-as-jpa-7.1.3.Final.jar:7.1.3.Final]
        at org.jboss.as.jpa.service.PersistenceUnitServiceImpl$1.run(PersistenceUnitServiceImpl.java:96) [jboss-as-jpa-7.1.3.Final.jar:7.1.3.Final]
        ... 4 more
Caused by: java.lang.ClassNotFoundException: org.jboss.jandex.Index from [Module "deployment.Taloomdevelnew.ear:main" from Service Module Loader]
        at org.jboss.modules.ModuleClassLoader.findClass(ModuleClassLoader.java:190) [jboss-modules.jar:1.1.3.GA]
        at org.jboss.modules.ConcurrentClassLoader.performLoadClassUnchecked(ConcurrentClassLoader.java:468) [jboss-modules.jar:1.1.3.GA]
hceylan commented 12 years ago

Excuse me for overlooking this detail.

The new deployment class scanner needs a module.xml change

<module xmlns="urn:jboss:module:1.1" name="org.batoo">
    <resources>
        <resource-root path="batoo-jpa-2.0.0.jar"/>
        <resource-root path="commons-dbutils-1.4.jar"/>
        <resource-root path="bonecp-7.1.jar"/>
        <resource-root path="antlr-runtime-3.4.jar"/>
        <resource-root path="asm-3.3.1.jar"/>
    </resources>

    <dependencies>
        <module name="com.google.guava"/>
        <module name="javax.persistence.api"/>
        <module name="javax.transaction.api"/>
        <module name="javax.validation.api"/>
        <module name="javax.xml.bind.api"/>
        <module name="org.apache.commons.collections"/>
        <module name="org.apache.commons.beanutils"/>
        <module name="org.apache.commons.lang"/>
        <module name="org.apache.commons.pool"/>
        <module name="org.apache.commons.io"/>
        <module name="org.slf4j"/>
        <module name="org.jboss.as.jpa.spi" />
        <module name="org.jboss.jandex" />
    </dependencies>
</module>

Specifically, org.jboss.as.jpa.spi and org.jboss.jandex dependencies must be added.

schueffi commented 12 years ago

batoo now finds all entity classes without explicitly enumerating them.

As a side remark: i could not get it working using a jboss module - i always have to include batoo.jar in my ear file. Do you have a hint on how i have to configure it for module usage? I tried with your module.xml (your last comment), and the corresponding files in the corresponding module directory ( /modules/org/batoo/main ). Then, i have an jboss-deployment-structure.xml inside my ear referencing this org.batoo-module, but with no success... Jboss always throws the exception

Caused by: javax.persistence.PersistenceException: JBAS011466: PersistenceProvider 'org.batoo.jpa.core.BatooPersistenceProvider' not found
        at org.jboss.as.jpa.processor.PersistenceUnitDeploymentProcessor.lookupProvider(PersistenceUnitDeploymentProcessor.java:560)
hceylan commented 12 years ago

Please refer to HelloJSF sample for Jboss 7 configuration.