longwa / grails-spring-batch

Grails plugin to add Spring Batch framework. It's intent is to minimize/eliminate the need for verbose XML files to configure Spring Batch jobs.
25 stars 18 forks source link

Plugin unusable if deployed within plugin #29

Open john-london opened 9 years ago

john-london commented 9 years ago

The plugin works by copying BatchConfig.groovy files to target/classes/batch as a result of the CompileStart event (https://github.com/johnrengelman/grails-spring-batch/blob/master/scripts/_Events.groovy). These files are then compiled and used as beans.

However, other plugins delete the target/classes directory when they are installed. As a result, depending on the order in which plugins are installed, these other plugins may delete the BatchConfig job files that have just been copied. The approach that the plugin follows only seems to work when the BatchConfig files are defined within the main application, not within a plugin that a main application imports.

Use case: a plugin that will parse files of a particular type and process them, and which uses this plugin as a dependency. The parsing plugin should be capable of being used within multiple other applications.

How to demonstrate: create a plugin that uses grails-spring-batch to execute a job defined in a BatchConfig.groovy file. Create an application that uses this new plugin and that uses a plugin that wipes the target/classes directory (e.g. tomcat).

This bug renders this plugin unusable if deployed within a plugin that is imported into a main application (I haven't yet discovered a viable workaround).

john-london commented 9 years ago

I've been looking at this, but am new to Grails/Groovy. We want to copy the *BatchConfig.groovy files to the target/classes/batch directory in order that the groovy files are in the class path for compilation. We then load the beans in the plugin's doWithSpring here. This calls the clone of a closure defined here which calls loadbeans.

Question: why do we even need to copy the files at all?

Why can't we use one of the methods described here (http://stackoverflow.com/questions/24028410/load-spring-beans-from-custom-groovy-files-in-grails-app) to ensure the beans are loaded from elsewhere?

Why can't we replace https://github.com/johnrengelman/grails-spring-batch/blob/master/SpringBatchGrailsPlugin.groovy#L156 with something like:

def loadBatchConfig = { ->
    loadBeans "file:./grails-app/batch/**/*BatchConfig.groovy"
    loadBeans "file:./plugins/*/grails-app/batch/**/*BatchConfig.groovy"
}

This would mean that the scripts/_Events.groovy file would be unnecessary.

danieldbower commented 9 years ago

That is a good question, feel free to try it. I didn't author that part of the plugin, so I'm not familiar with the mechanism. My hunch is that the copy happens because of the compilation process. Without that particular copy, the jobs might not make it into the Warfile. I'd have to test to verify exactly what happens, but I don't have the available cycles at the moment.

john-london commented 9 years ago

OK, will let you know how I get on. When I make the changes I mention, my plugin (that uses grails-spring-batch plugin) works correctly when I do run-app on the plugin, but I've yet to test with using my plugin within an application or within a WAR.

john-london commented 9 years ago

Unfortunately there were issues when I tried the solutions, the basis of which I couldn't get to the bottom of. Sometimes the code would work, then, without making any changes to any code, when tests were re-rerun, the plugin would refuse to load the beans that this plugin provides (e.g. JobRepository). The code would work when called using IntelliJ the first time, but never when using grails or gradle from the command line.

There's some nasty hacking going on somewhere when Grails is wiring up spring beans and installing plugins/this plugin - it's certainly not deterministic. It's a shame, but this means that this plugin is only able to be used when installed into a Grails application - it does not work when installed into a plugin that is then installed into an application.

danieldbower commented 9 years ago

Were you inlining the plugin? That can act oddly when you're working with the beans in the Plugin Descriptor. Doing a full deployment will make it behave a little more normally, though it takes more time to test.

john-london commented 9 years ago

Sorry, I'm not sure what you mean by inlining the plugin?

danieldbower commented 9 years ago

If you look at: https://github.com/johnrengelman/grails-spring-batch/blob/master/test/projects/simple-schedule/grails-app/conf/BuildConfig.groovy You'll see the the grails spring batch plugin is "Inlined" on line 66 vs the regular manner of installing a plugin. This can make plugin development quicker, but doesn't perform 100% in the same fashion as the regular installation.

john-london commented 9 years ago

OK, thanks, then no I wasn't. My plugin (that used SB) was compiled and installed onto a Nexus repository and it worked as it should when used alone. It was then used as a dependency within another plugin and was used in the normal manner. This other plugin also needed SB but did not work as the beans within the SB plugin were not being loaded.

I've had to abandon the idea of a plugin architecture as this plugin won't support it and will now look to implement my application as a monolithic structure where SB is used only within the base-level application.

johnrengelman commented 9 years ago

The loadBean change could work but it doesn't make the _Events.groovy obsolete. Without that, the .groovy files will not be copied into the target directory which is what is needed.

In the case of WAR files, there should probably be in additional section in _Events.groovy that ensures that the files are copied into the WAR staging directory during the WAR compilation.

john-london commented 9 years ago

I thought the purpose of the _Events.groovy script was to copy the BatchConfig files to the classpath, because grails-app/batch isn't in it, no? Sorry, I'm reasonably new to Grails.

But as I mentioned in the bug report, either Grails has a mode of behaviour in which it deletes the target directory contents when installing plugins, or certain plugins exhibit this behaviour. It's not possible to define the load order of all plugins that a user may install, and therefore we I don't think we can use this approach to installing the BatchConfig files into the class path: we'll have to take a different approach.

johnrengelman commented 9 years ago

That's exactly what it does....but it's doing it during the compile event callback. It should also do it during the war event callback. Otherwise it's relying on the notion that the files are in the compiled directory which as you have pointed out is not always the case.

This has always worked reasonably well. So, I'm curious what the flow is that you are doing that causes the compiled output to be deleted. The _Events callback is simply copying the source files to the output compile directory. Normally this doesn't occur in Grails. It normally only copies the compiled .class files to this directory.

john-london commented 9 years ago

I was trying to use the SB plugin within a plugin that I was writing, that would then be used within another plugin. The flow is described here: http://stackoverflow.com/questions/28814914/grails-spring-batch-failure-exception-to-create-parallel-job-bean-after-grai

I can simply see that when the SB plugin is used with another plugin, jobs are null, and this seems to be because the files have been deleted before they are compiled.

johnrengelman commented 9 years ago

so after a grails clean-all, when you do a grails run-app the files are missing.

It shouldn't have anything to do with plugin installs, because plugins are only installed once unless you explicitly delete them. What other plugins do you have installed? Is this project available somewhere?

johnrengelman commented 9 years ago

There's obviously some type of ordering issue.

Try this:

$ grails clean-all
$ find . -name batch
$ grails compile
$ find . -name batch
$ grails package
$ find . -name batch
john-london commented 9 years ago

I'm afraid the project isn't available, but the plugins I was using are here (spring-batch plugin not shown):

dependencies {
    test "org.grails:grails-datastore-test-support:1.0-grails-2.4"
    compile "cglib:cglib:2.2"
    runtime 'org.postgresql:postgresql:9.3-1102-jdbc41'
}

plugins {
    build(":release:3.0.1",
          ":rest-client-builder:1.0.3") {
        export = false
    }
    test("org.grails.plugins:code-coverage:2.0.3-2") {
        excludes 'xercesImpl', 'xml-apis'
    }
    compile ":scaffolding:2.1.2"
    runtime ":hibernate4:4.3.5.5"
    runtime ":database-migration:1.4.0"

            // 3 proprietary plugins, just implementing a series of business logic services (no spring-batch)

    runtime ":jquery:1.11.1"
    compile ":jquery-ui:1.10.4"
    runtime ":twitter-bootstrap:3.2.0.2"
    runtime ":font-awesome-resources:4.2.0.0"
    runtime ":modernizr:2.7.1.1"

    build ":tomcat:7.0.55"
    compile ":platform-core:1.0.0"

    compile ":asset-pipeline:1.9.6"
    runtime ":jquery:1.11.1"
    runtime ":twitter-bootstrap:3.2.0.2"
    runtime ":font-awesome-resources:4.2.0.0"
    runtime ":modernizr:2.7.1.1"

    runtime ":console:1.5.2"

    compile(":grails-melody:1.55.0") {
        export = false
    }

}
john-london commented 9 years ago

I'm travelling at the moment and so an not in a position to look at the code. However, the clean-all resulted in the all plugins being reinstalled. When the SB plugin was installed, the files are copied correctly and the "find" command showed that the files were there. When plugins that are loaded later than SB are loaded, some of them result in the target/classes directory being deleted - when I interrupted the compile with Ctrl C and then issued a find command, the files had been deleted.

When run-app is run for the second time (without a clean-all) the files are again copied to the directory and as the other plugins are already installed, the SB files are not deleted by the other plugins.

Why do we need to copy the files at all, is there not another way to add the batch directory to the classpath?

On 5 Mar 2015, at 18:48, John Engelman notifications@github.com wrote:

There's obviously some type of ordering issue.

Try this:

$ grails clean-all $ find . -name batch $ grails compile $ find . -name batch $ grails package $ find . -name batch — Reply to this email directly or view it on GitHub.

johnrengelman commented 9 years ago

Oh, I know what this is.

This is the problem. If the plugin is not installed, then it's _Events.groovy is not available to participate in the compile callback. So the files don't get copied.

This is the situation, you have a clean project. You run grails run-app and it determines that it needs to install the SB plugin, so it does. After installing the plugin, it continues on with compiling, but it doesn't register the newly installed plugin. So it's _Events.groovy isn't executed.

When you run grails run-app a second time, the plugin is already installed, so the callback is executed and the files are copied over.

This is a limitation of the Grails plugin mechanism. The concern here is why is the grails clean-all uninstalling your plugins (that shouldn't be happening). Do you have some special configuration in your BuildConfig.groovy that is placing the plugin source somewhere different than the default?

john-london commented 9 years ago

No, I don't think so, the BuildConfig.groovy section is:

grails.project.class.dir = "target/classes"
grails.project.test.class.dir = "target/test-classes"
grails.project.test.reports.dir = "target/test-reports"
grails.project.work.dir = "target/work
john-london commented 9 years ago

I'm also finding that the plugin doesn't seem to work when used in an application that is deployed as a War file on Tomcat. I'm seeing that the job beans are null, resulting in exceptions. @johnrengelman and @danieldbower, have either of you two please managed to get this to work in this manner?

danieldbower commented 9 years ago

You're using "grails war" and then copying the resultant war file to Tomcat? That is what I am doing. I'm deploying to Tomcat 7 on Java 7.

john-london commented 9 years ago

OK, the issue with the war file appeared to do with the fact that the war files for deployment were being created using the gradle-grails plugin. The war was fine if created using "grails war" and also if created using "gradle war". However, a custom gradle GrailsWarTask using a non-standard environment seemed to result in none of the job files being copied to the war, but they are copied after cleaning/re-running the tasks multiple times - I'm still looking into this, something isn't right here. It's just the original issue that remains now.

Sorry for the noise and for taking the time out to look at the issue, much appreciated!

john-london commented 9 years ago

I used the following _Events.groovy file to do some debugging. I can see that the BatchConfig files are copied across correctly by this plugin at CompileStart, but are then deleted when plugins are installed.

eventCompileStart = {
if (new File("${basedir}/grails-app/batch").exists()) {
    ant.copy(todir:"$classesDirPath/batch", preservelastmodified:true) {
        fileset(dir:"${basedir}/grails-app/batch", includes:"**/*BatchConfig.groovy")
    }
}
def process = "find . -name *BatchConfig.groovy".execute()
println "Found results: ${process.text}"
}

eventPluginInstalled = {pluginName ->
println "$pluginName installed"
def process = "find . -name *BatchConfig.groovy".execute()
println "Found results: ${process.text}"
}

Is there a reason why the files need to be copied at CompileStart? If the sole aim is to have the groovy files on the classpath, can they not also be copied at CompileEnd?

eventCompileEnd = {
if (new File("${basedir}/grails-app/batch").exists()) {
    ant.copy(todir:"$classesDirPath/batch", preservelastmodified:true) {
        fileset(dir:"${basedir}/grails-app/batch", includes:"**/*BatchConfig.groovy")
    }
}
def process = "find . -name *BatchConfig.groovy".execute()
println "Found results: ${process.text}"
}

Adding this event seems to get around the problem.