Impetus / kundera

A JPA 2.1 compliant Polyglot Object-Datastore Mapping Library for NoSQL Datastores.Please subscribe to:
http://groups.google.com/group/kundera-discuss/subscribe
Apache License 2.0
903 stars 233 forks source link

Autodetect Feature #568

Closed ghost closed 10 years ago

ghost commented 10 years ago

Hello Guys,

We've started using Kundera and we really appreciate the polyglot persistence feature. One thing that's super annoying right now is that we have to declare all the entity classes in persistence.xml, well, that's ok if you have small number of entities but as soon as you grow past 50 odd entities it becomes a nightmare to maintain plus you have to maintain multiple PUs.

I know Kundera is geared more towards JPA first kind of design however there are more complicated scenarios that would appreciate some kind of convention over configuration or at least annotation with some lazy loading feature.

I am not sure if something like that already exists in Kundera and i don't know since it already scans the class at load time and builds the metadata.

Declaring all these classes (or scanning them upfront) also introduces some startup/load time issues which could be avoided if lazy loading of entites is introduced e.g. move process at the time of save/delete/find, this would of course make initial method call slow but subsequent operations wont have that problem and we can start the application faster.

I probably mixed couple of issues/features together here but i wanted to get this out there and know what y'all think about it and if there are plans already to add these enhancement.

mevivs commented 10 years ago

Hi, For polyglot persistence, other than specifying entities within persistence.xml, you may also hard code schema@pu with @table annotation. For more on this, please refer https://github.com/impetus-opensource/Kundera/issues/315. Lazy loading of entities at the time save/update operation would be an overhead and not desired with such operation, as per JPA metamodel must be readily available before such em level operations.

Hope it helps.

-Vivek

ghost commented 10 years ago

Well, i think i was more or less looking for <exclude-unlisted-classes>true</exclude-unlisted-classes> which resolved a lot of speed issues, not sure why kundera scans all the classes on classpath to select entities, its a real problem when you build fat jars (e.g. building services stanalone services with jetty).

So if you look at following two code blocks and the position of this statement

Class<?> clazz = this.getClass().getClassLoader().loadClass(className);

This one from 2.9.1

/**
 * The Metamodel configurer: a) Configure application meta data b) loads entity
 * metadata and maps metadata.
 * 
 * @author vivek.mishra
 */
public class MetamodelConfiguration extends AbstractSchemaConfiguration implements Configuration
{
    ....
private List<Class<?>> scanClassAndPutMetadata(InputStream bits, Reader reader,
        Map<String, EntityMetadata> entityMetadataMap, Map<String, Class<?>> entityNameToClassMap,
        String persistenceUnit, String client, Map<String, List<String>> clazzToPuMap,
        Map<String, IdDiscriptor> entityNameToKeyDiscriptorMap) throws IOException {
    .....
    try
    {
        cf = new ClassFile(dstream);
        className = cf.getName();
        ....
        // iterate through all valid annotations
        for (String validAnn : reader.getValidAnnotations())
        {
            // check if the current class has one?
            if (annotations.contains(validAnn))
            {
                Class<?> clazz = this.getClass().getClassLoader().loadClass(className);
                this.factory.validate(clazz);
                .....
}

And this one from 2.11

/**
 * The Metamodel configurer: a) Configure application meta data b) loads entity
 * metadata and maps metadata.
 * 
 * @author vivek.mishra
 */
public class MetamodelConfiguration extends AbstractSchemaConfiguration implements Configuration
{
    ....
private List<Class<?>> scanClassAndPutMetadata(InputStream bits, Reader reader,
        Map<String, EntityMetadata> entityMetadataMap, Map<String, Class<?>> entityNameToClassMap,
        String persistenceUnit, String client, Map<String, List<String>> clazzToPuMap,
        Map<String, IdDiscriptor> entityNameToKeyDiscriptorMap) throws IOException {
    .....
    try
    {
        cf = new ClassFile(dstream);
        className = cf.getName();
        ....

        Class<?> clazz = this.getClass().getClassLoader().loadClass(className);

        // iterate through all valid annotations
        for (String validAnn : reader.getValidAnnotations())
        {
            // check if the current class has one?
            if (annotations.contains(validAnn))
            {
                this.factory.validate(clazz);
                .....
}

for some reason it was moved before checking if the class contains annotations. This creates a different sort of havoc for someone who is building fat standalone jars.

Say for example: someone is using logback logging library, now , if you build a fat jar using maven-shade-plugin or something similar then it tries to load all the classes from logback library and guess what, those classes in turn have other dependencies on groovy-lang and other trillions of libraries. In an ideal situation you'd have to use that class and its dependency only if you need to BUT with this single statement every single class is being loaded at the time of loading persistence unit and would force you include whole world's dependencies.

You could possibly change your packaging style to something else but i see more more people using lightweight standalone containers like jetty combined with dropwizard for building their services and it's a norm to build fat jars like this in those kind of projects (we're in the age building lightweight REST services).

I do not understand the motivation for moving that statement out of the if statement but i think this a problem and should be changed back to the way it was.

BTW, i had to fix this issue before i could use the above mentioned attribute

Manish

mevivs commented 10 years ago

Hi, For loading explicit class and entities from specific jar files, you may map them as:

<class>....</class>
<jar-file>....</jar-file>
<exclude-unlisted-class>true</exclude-unlisted-classes>

If you don't specify above properties in persistence.xml, it would require to scan available classes for valid entity definitions.

Let me discuss this within team to find a reason for moving

        Class<?> clazz = this.getClass().getClassLoader().loadClass(className);

out of valid annotation check.

Thanks, -Vivek

chhavigangwal commented 10 years ago

Fix has been added. Releasing with 2.11.1