Ok - I implemented the Cargo plug-in last night. It seems to work fine, and also fixed issue with the broken integration test. Will send a pull request for your review.
Actually this setup with Cargo is a much nicer than using an embedded Tomcat in the Gradle JVM: now the integration tests are testing a “realistic” set-up i.e. taking the built WAR into a separate, standalone Tomcat environment. Might actually be preferable / more representative of real use than using the Spring Boot test harnesses. So option 2 might actually be preferable to option 1 even longer term. And yes, my understanding of the difference between (now obsolete) tomcatRun / tomcatRunWar was that tomcatRun was using the classes from the built class path when running the embedded Tomcat, whereas tomcatRunWar was taking the packaged WAR and deploying that to the embedded Tomcat. Regards Petri > On 2 May 2020, at 7:50 PM, Michael Vorburger <[email protected]> wrote: > > inline: > > On Sat, May 2, 2020 at 5:46 PM Petri Tuomola <[email protected] > <mailto:[email protected]>> wrote: > Michael, Awasum - many thanks for your prompt feedback - much appreciated! > > As you stated, it’s clear that integrationTest does currently rely on the > embedded (non-Spring Boot) Tomcat - and of course we do need to find a way to > run them somehow. > > My investigation yesterday went down exactly the same route as you describe > below as option 2. I was first going to launch Spring Boot Tomcat as a daemon > through a new plugin / task, but then thought that was a bit too much of a > hack. I then looked at Cargo, and was going to use that to solve the problem. > > However, then I thought of an even simpler solution: I upgraded the embedded > Tomcat dependency to point to the same version as is being pulled in through > Spring Boot. That means both the Gradle Tomcat plugin and Spring Boot are > referring to the same version, and avoids the classpath conflicts. It does of > course mean that the Spring Boot and embedded Tomcat references need to be > kept in synch. > > With this, both bootJar / bootRun and tomcatRun work. TomcatRunWar doesn’t > work for exactly the same reason as bootWar: it seems that in Tomcat 7 the > WebappLoader extracts all the class files into a temp directory allowing > OpenJPA to find them, whereas Tomcat 9 refers to the classes within the WAR > file and stops OpenJPA from working. > > But I changed integrationTest to use tomcatRun instead of tomcatRunWar, and > that seems to work - except that one test is now failing, and I need to check > why. > > So I can see three options: > > 1. Strategic solution: Rewrite the whole integrationTest to use a more > “Spring native” approach > > 2. Tactical solution: Use Cargo plugin to deploy the WAR into a separate > container, remove embedded Tomcat but continue using Tomcat 7 in the separate > container that is launched by Cargo > > 3. Tactical solution (my current one): Change integrationTest to use > tomcatRun and make the embedded Tomcat the same version as Spring Boot Tomcat > > Option 3 has the benefit that I’ve already implemented it and it works :-). > Option 2 would be better though, as it would remove the second embedded > Tomcat from the WAR and also the dependency to keep the two versions in synch > in the future. And it would mean that the integration tests are properly > “independent” of the build. So maybe I’ll look into that tonight… > > If you already have Option 3. working, and manage to fixt hat single failing > IT (which? FYI it may well just be one of our many flaky ones!) then please > do ignore me ;) re. Option 2 and raise a PR based on Option 3! Then either > Option 3. or directly Option 1. as a separate next follow-up step once we > have been able to merge PR based on Option 3. > > Btw the com.zaxxer.hikari.metrics.micrometer.MicrometerMetricsTrackerFactory > issue is because Quartz brings in a version of Hikari that is incompatible > with the version included by Spring Boot. That’s one of the things I had to > fix to get bootRun / tomcatRun to work... > > Oh, interesting. I have to admit that I don't fully get the difference > between tomcatRun VS tomcatRunWar.. do you? Does gradle-tomcat-plugin 's > tomcatRun use the project's classpath, whereas tomcatRunWAR deploys a WAR > into Tomcat more like gradle-cargo-plugin would (but within the same JVM, > instead of forking?). I guess it doesn't really matter, if we are about to > remove them... ;-) > > BTW, isn't tomcatRun missing keystoreFile and keystorePass? I don't suppose > that's the reason for your test failure? > > I'll hold off on https://github.com/apache/fineract/pull/795/files > <https://github.com/apache/fineract/pull/795/files> until your PR comes in! > > Regards > Petri > > >> On 2 May 2020, at 6:16 PM, Michael Vorburger <[email protected] >> <mailto:[email protected]>> wrote: >> >> Petri, I thought about this thread again while I was looking into >> https://issues.apache.org/jira/browse/FINERACT-914 >> <https://issues.apache.org/jira/browse/FINERACT-914> today, and regretted >> that we still use a standalone Tomcat and not one Spring can embed... ;-) So >> in order to support this to happen, here is a lengthier more technical reply >> to your (great!) questions inline: >> >> On Fri, May 1, 2020 at 2:15 PM Awasum Yannick <[email protected] >> <mailto:[email protected]>> wrote: >> Hi Petri >> >> Thanks for doing all this work. Its invaluable. >> >> On Fri, May 1, 2020 at 10:24 AM Petri Tuomola <[email protected] >> <mailto:[email protected]>> wrote: >> Great stuff! >> >> Having done some testing on this, what seems to work is: >> >> - bootRun starts Spring Boot embedded Tomcat and works as expected >> >> - bootJar creates a JAR that can be executed from command line and works as >> expected >> >> - bootWar creates a WAR that can be deployed to an external Tomcat and works >> as expected. However the WAR file can’t be started directly from command >> line, as OpenJPA doesn’t seem to be able to find persistence-enhanced >> classes from within a WAR (looking at the source, it seems to be hardcoded >> to only handle classes from filesystem or JAR). >> >> Ah, interesting, I see. I guess in theory there is probably a way to fix >> that, but I would just ignore it, it's super low priority; knowing that the >> JAR works. One thing we can do is just point this out in future >> documentation on the README. I wouldn't worry about it. >> >> What doesn’t seem to work anymore after my changes is the “other” embedded >> Tomcat - i.e. version 7.0.94 - that is included directly as a dependency in >> build.gradle and used for tasks tomcatRunWar / tomcatRun. I suppose that’s >> not surprising as there are two embedded Tomcats bundled that are likely to >> conflict, so getting both to work will require some thinking. >> >> To the best of my knowledge the tomcatRunWar task is used in development for >> quickly testing changes one makes as they solve a problem. Is it also used >> for integration tests? It might be Ok to just forget about this and use the >> "newly fixed" (PR still to be sent) bootRun Spring Boot gradle tasks. >> >> I think I see the problem you are facing. I totally agree with what Awasum >> as expressed. Basically, don't be shy to yank out things from the past to >> help this project get to tomorrow! ;-) >> >> Perhaps if we formulate the following "requirements", this may help you to >> zoom into the best solution: >> >> A. We want to be able to easily run Fineract during development. Whether >> that is by "tomcatRunWAR" as today or "bootRun" tomorrow doesn't matter at >> all, if they are basically fully equivalent for a developer. bootRun is >> obviously preferably - especially it can hot reload changes... ;-) Just >> remember to also update the documentation in the README in the PR, if you >> can. >> >> B. We do need those integration tests to run somehow - but we don't care at >> all how they run... :) Today, this works like this: The integrationTest >> Gradle task, see >> https://github.com/apache/fineract/blob/develop/fineract-provider/build.gradle#L495 >> >> <https://github.com/apache/fineract/blob/develop/fineract-provider/build.gradle#L495>, >> uses the tomcatRunWarAsDaemonForIntegrationTest Gradle task, which is based >> on https://github.com/bmuschko/gradle-tomcat-plugin >> <https://github.com/bmuschko/gradle-tomcat-plugin>. >> >> I think you have basically two ways forward here, you either: >> >> 1. go all the way in and do >> https://issues.apache.org/jira/browse/FINERACT-764 >> <https://issues.apache.org/jira/browse/FINERACT-764> together with >> https://issues.apache.org/jira/browse/FINERACT-730 >> <https://issues.apache.org/jira/browse/FINERACT-730> right away, meaning >> we'd run integration tests through Spring's support for such things, instead >> of starting an external Tomcat through Gradle, and totally remove the >> "other" (original, old) Tomcat dependencies and the gradle-tomcat-plugin, as >> in this scenario they won't be needed anymore. >> >> 2. stick, for now, to having a Tomcat launched for the ITs - but that won't, >> yet, be the "new" Tomcat which you'll embed for Spring Boot. I think what's >> important to realize here is that, as long as (quoting you) "bootWar creates >> a WAR that can be deployed to an external Tomcat and works as expected", >> that should still be totally possible, do you agree? I suspect the real >> problem you are facing is a classical classpath mess between the two >> Tomcats. In theory, if https://github.com/bmuschko/gradle-tomcat-plugin >> <https://github.com/bmuschko/gradle-tomcat-plugin> were to do classpath >> management well, this hypothetically should work (with different >> classloaders), but if you find that it doesn't, then just forget about that >> - and let's just find another way? I noticed on >> https://github.com/bmuschko/gradle-tomcat-plugin >> <https://github.com/bmuschko/gradle-tomcat-plugin> that, quote "Gradle >> starts the embedded container in the same JVM. Currently, the container >> cannot be forked as a separate process. If you are looking for this >> capability, please have a look at the Cargo plugin instead." and had a quick >> look at https://github.com/bmuschko/gradle-cargo-plugin >> <https://github.com/bmuschko/gradle-cargo-plugin>. I think if we just >> switched to using that gradle-cargo-plugin instead of the >> gradle-tomcat-plugin for the integration tests, for now, that would solve >> the problem you're hitting? It would likely be a little bit slower, as that >> approach will require the full WAR to be built, and deployed into a >> separately running Tomcat in another JVM (which is exactly what the >> gradle-cargo-plugin must be doing...), but given how slow our integration >> tests overall run, that's the least of our worries... ;-) Is that approach >> something you would like to try? I guess in theory it could even be done >> separately by someone else as a pre-requisite... anyone reading this up for >> that? >> >> I suspect 1. is more work, and because we seem to finally be almost there >> with 2. my recommendation would be to first go that way (2.), get that >> merged, and only then dig into 1, as a fully separate next follow-up step. >> >> As for tomcatRunWar VS tomcatRun, see >> https://github.com/apache/fineract/pull/795 >> <https://github.com/apache/fineract/pull/795>, which proposes we just remove >> tomcatRun! (I briefly tried it, but got a [strange!] error, and anyway >> don't quite see the point of the tomcatRun gradle task, as opposed to >> tomcatRunWar; Caused by: java.lang.AbstractMethodError: Receiver class >> com.zaxxer.hikari.metrics.micrometer.MicrometerMetricsTrackerFactory does >> not define or inherit an implementation of the resolved method 'abstract >> com.zaxxer.hikari.metrics.MetricsTracker create(java.lang.String, >> com.zaxxer.hikari.metrics.PoolStats)' of interface >> com.zaxxer.hikari.metrics.MetricsTrackerFactory.) >> >> One thing you can do is run the integrationTest tasks on the commandline and >> see if everything passes on your local Dev machine as that may use >> tomcatRunWar. >> >> I dont know if anyone has an Objection to that. Either way, Send your PR so >> we can also test while reviewing the code. Its not a problem as we can >> revert if we find that disable the tomcatRunWar tasks breaks something after >> merging. >> >> Before I try to find a way to get both of these to work, I wanted to check >> if these are both needed? I.e. is there a reason to include two different >> embedded Tomcat versions and two Gradle tasks to start them? Or would it be >> possible to just remove the older version and related Gradle tasks & plugin, >> and use the Spring Boot version only? >> >> Lets use the bootRun tasks for dev purposes except someone objects. Send >> your PR Petri >> >> Thanks. >> Awasum >> >> >>> On 30 Apr 2020, at 11:11 PM, Michael Vorburger <[email protected] >>> <mailto:[email protected]>> wrote: >>> >>> Petri, >>> >>> welcome to Apache Fineract. >>> >>> Yes, a PR for this would be *VERY* welcome... ;-) >>> >>> Tx, >>> M. >>> >>> >>> On Thu, 30 Apr 2020, 22:07 Petri Tuomola, <[email protected] >>> <mailto:[email protected]>> wrote: >>> Hi all >>> >>> I’m new to Fineract - so apologies in advance for anything I may have got >>> wrong with this: >>> >>> I had a bit of spare time today and downloaded the latest develop branch >>> for Fineract 1.x. Initially it did not start on my machine - ./gradlew >>> bootRun threw a lot of errors. >>> >>> But after a couple of fixes to dependencies, SSL config, main class etc, >>> ./gradlew bootRun now seems to work fine: the application starts and I can >>> call the API etc. >>> >>> My changes are in a branch at >>> https://github.com/ptuomola/fineract/tree/FINERACT-730 >>> <https://github.com/ptuomola/fineract/tree/FINERACT-730> and I was >>> wondering if these would be of interest to anyone? >>> >>> I think this should help to address the issue FINERACT-730 >>> >>> If yes, I can open a pull request - once I’ve done some more testing (e.g. >>> that the built WAR can also be started and that I haven’t broken the dev >>> profile build etc) >>> >>> Thanks in advance >>> >>> Regards >>> Petri
