Automatic table configuration via Annotation Processing

NCrouther commented 9 years ago

This pull request adds an annotation processor that examines annotations at compile time and generates a function that adds cached config information for all tables to the DAO Manager. This is intended to replace textual config files, with the advantage that it is generated automatically every build. All reflection is done at compile time, so there is no runtime cost. Simple inputs and outputs are included as automated unit tests.

stephanenicolas commented 9 years ago

Hi guys,

thanks @NCrouther for this. I am actually working on an OrmLite android Plugin. The plugin is ready but uses the ORMLite Config Util class to generate the ormlite configuration file. And this approach has several limitations that would all be removed by using an annotation processor. The android plugin : https://github.com/stephanenicolas/ormlite-android-gradle-plugin

The plugin works fine, but with several limitations. Nevertheless, it is production ready and we already use it at Groupon internally. We got large speed improvements but it twists a bit our build cycle and it creates some problem at the SCM tree level (basically the ormlite config file, which is now generated has still to be part of the tree). An annotation processor would get rid of those twists and SCM troubles.

The annotation processor is really the way to go but I didn't have time to look into it. I am glad someone could find some time and spent some effort on this.

I plan to review the PR this week, if you are fine with that @j256 .

Also, @j256 , I think this all should be merged in ORMLite : both the annotation processor and the android gradle plugin.

stephanenicolas commented 9 years ago

Generally speaking, a gradle / android example is missing. It should use aptplugin to configure the annotation processor.

stephanenicolas commented 9 years ago

I have a few comments, but I think this PR is really great ! I hope it can be made stable and merged soon to ORMLite.

NCrouther commented 9 years ago

Thanks for all of the detailed feedback. I knocked out all of the easy stuff today, I should have time to tackle the rest of it later this week. A couple of open questions:

How should we handle the new dependency on a java code generation library (Velocity or JavaPoet)?

I was hesitant to use a library since currently the Android JAR has no runtime dependencies. I know I have used ORMLite by just adding the JAR to an eclipse project, and I would guess other users do this as well; I didn't want to add too much complexity for those users. My research on the issue pointed me to the maven shade plugin which looks like will bundle the dependency into the single JAR. Is this a good way to go? Is there anybody familiar with it that can point me in the right direction?

Should the annotation processing code be in a separate JAR?

The gradle apt plugin looks like it is built around having a JAR with just the code for annotation processing so it can load it during compile but not add it to the output JAR. I can see the benefit, but since the annotation processing code is pretty small for now, I figured it would be simpler to keep it all one project. If we decide to split it, is that something I should do (i.e. I would create a project for it and the j256 repo would fork it), or should this repo split it out? My gut feel would be to keep it together for now and if the annotation processing grows (such as if we do more code generation to eliminate reflection when populating query result objects), @j256 would create the new project and migrate the annotation processing code there.

Thoughts?

NCrouther commented 9 years ago

I went ahead and added JavaPoet and used the maven shade plugin to eliminate the added dependency. It does bump the size of the annotation processing code from ~10KB to ~70KB which adds some more motivation to split the annotation processing into a separate JAR that can be discarded at runtime

TODOs:

investigate strategies for splitting into two JARs
create gradle example

Open Questions:

Should we split into two JARs, and how should it be handled?
Where should the gradle example go? I would guess that upon release it should be added to the main project website.

NCrouther commented 9 years ago

I looked at how Dagger handles creating two JARs and they use a maven multipke module project, which seems like the best way to go. There are a couple of areas that I could use some guidance:

I am pretty new to maven, so I'd like to have somebody experienced who fully understands the existing POM look over the 3 poms that I split it into to make sure I did it right.
Currently I have Javadoc links from some main JAR classes to annotation processor JAR classes, so javadoc is giving warnings (and I assume the links are broken). Is there a good way to handle this? Possibly by merging the Javadoc from the two maven subprojects somehow so the links properly resolve?

stephanenicolas commented 9 years ago

Hi @NCrouther ,

you are right, for the jar management. It's better to use modules. I would suggest the following : (but in a different PR as it involves changing orm lite android's pom)

a parent module (ormlite-android-parent) that contains stuff related to dependency and plugin management, release and quality plugins
a child (ormlite-android) that contains the lib
a child (ormlite-android-processor) that contains the processor
if necessary, to avoid a cycle : a child module (ormlite-android-core) that defines common data structures.

But this might be a little long and should maybe be addressed in a different PR. Already, this, which is quite a change, and has to be merged.

For Javadoc, I don't know.

@j256 @kpgalligan What do you think of this ?

NCrouther commented 9 years ago

Thanks for the confirmation. I solved the javadoc issue by using the aggregate-jar target for the javadoc plugin in the parent pom. I have the project split into the modules you describe and everything seems to be working properly. However, there are still sections of the POM (such as the sourceforge and sonatype profile sections) that I don't understand, so they will definitely need heavy review. Should I commit the changed pom (along with moving all source files to appropriate folders) to this PR or wait and create a new PR?

stephanenicolas commented 9 years ago

I don't know what the plan of @j256 is. The easiest would be a PR against your branch, not an additional commit and a link to follow up on the PR (which is gonna be in your repo).

Good work ! Sonatype and sourceforge stuff should not be modified. But they should bubble up to root level.

NCrouther commented 9 years ago

Thanks for the advice. I'm really new to Git/GitHub, so I appreciate the hand-holding, I'm learning a lot. I followed your recommendation and put the initial version of the module-split pom into a separate branch/PR in my repo: https://github.com/NCrouther/ormlite-android/pull/2. Based on the JARs created, functionally everything seems good, but I am sure the POMs can use some further refactoring.

koesie10 commented 9 years ago

I only saw this PR now, but it's really similar to a project I started last Friday: https://github.com/koesie10/ormlite-processor. It's basically the same, but without changing the project's code and it has some limitations.

kpgalligan commented 9 years ago

Getting a build error from the JavaPoet. I think the build needs to be 1.7, but changing to that throws a different issue. JavaPoet related issue:

[ERROR] /Users/kgalligan/Dropbox (Touch Lab)/devel_git/ormliterefactor/ormlite-android/src/main/java/com/j256/ormlite/android/processor/OrmLiteAnnotationProcessor.java:[453,18] cannot access java.nio.file.Path [ERROR] class file for java.nio.file.Path not found

Also, how do you add to a gradle project? Example? Working on this today.

kpgalligan commented 9 years ago

Didn't mention @NCrouther directly in last message. Not sure if you'll see it...

NCrouther commented 9 years ago

Sorry for the delay, I was away from email over the holiday weekend.

I didn't realize that JavaPoet would increase the required JRE/JDK version. Hopefully the impact will be small since I am splitting it into two separate JARs. The main JAR can retain 1.5 compatibility, and the annotation processor JAR can target the higher version that is necessary.

I am doing some experiments on the branch with the pom changes: https://github.com/NCrouther/ormlite-android/pull/2

I incorporated the animal sniffer plugin (http://mojo.codehaus.org/animal-sniffer/) to verify that the code is not using any newer APIs than are present in the declared java version. It seems like everything is fine on 1.6. This change should fix the java.nio.file.Path not found problem, the real issue will be figuring out the other error you ran into when changing that. Let me know what happens when you try to build the latest from that branch.

NCrouther commented 9 years ago

To add to gradle (for the branch with the annotation processing split into a separate JAR):

compile files('libs/ormlite-core.jar')
compile files('libs/ormlite-android.jar')
provided files('libs/ormlite-android-annotation-processor.jar')

Once this version is deployed to maven central, it will be:

compile 'com.j256.ormlite:ormlite-android:4.49'
provided 'com.j256.ormlite:ormlite-android-annotation-processor:4.49'

stephanenicolas commented 9 years ago

You should add a project type dependency it will handle both cases.

Let me know if I can review the code.

S. Le 2015-05-26 7:11 PM, "Nathan Crouther" notifications@github.com a écrit :

To add to gradle (for the branch with the annotation processing split into a separate JAR):

compile files('libs/ormlite-core.jar') compile files('libs/ormlite-android.jar') provided files('libs/ormlite-android-annotation-processor.jar')

Once this version is deployed to maven central, it will be:

compile 'com.j256.ormlite:ormlite-android:4.49' provided 'com.j256.ormlite:ormlite-android-annotation-processor:4.49'

— Reply to this email directly or view it on GitHub https://github.com/j256/ormlite-android/pull/45#issuecomment-105722531.

j256 / ormlite-android

Automatic table configuration via Annotation Processing #45