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…

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...

Regards
Petri


> On 2 May 2020, at 6:16 PM, Michael Vorburger <m...@vorburger.ch> 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 <awa...@apache.org 
> <mailto:awa...@apache.org>> wrote:
> Hi Petri
> 
> Thanks for doing all this work. Its invaluable.
> 
> On Fri, May 1, 2020 at 10:24 AM Petri Tuomola <pe...@tuomola.org 
> <mailto:pe...@tuomola.org>> 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 <m...@vorburger.ch 
>> <mailto:m...@vorburger.ch>> 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, <petri.tuom...@gmail.com 
>> <mailto:petri.tuom...@gmail.com>> 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

Reply via email to