Core code should not know about specific modules - TRUNK-4739

Development
1

Regarding:

@dkayiwa can you please clarify why this is necessary? Were there some classloader rewrites in Platform 1.11 and master that break the uiframework module’s behavior?

Note that openmrs-core should not know about the specifics of any module, but this commit breaks the separation and adds uiframework-specific functionality to core. Rather it should be the uiframework module’s job to figure out how to re-enable this behavior against the classloader in openmrs-core 1.11+.

2

I simply wanted a VM argument telling core about the development directory of any module. My first attempt was to use something like “moduleid.development.directory”. An example of this would be “xforms.development.directory”. But i discovered that the existing VM argument in use is “uiFramework.development.moduleid”, where the above for the xforms module becomes " uiFramework.development.xforms". So i could rename the VM argument to whatever you suggest.

To make this clearer, this has nothing to do with the uiframework module. It is simply a VM argument which tells the platform to load classes of any module from a given development folder instead of the compiled omod. The motivation for this was that the current “uiFramework.development.moduleid” reloads only the new UI framework controllers. I was working on a module, where i was making changes to other non UI framework controllers and other classes and got fed up of having to redeploy the module for changes i made (even the simplest like a one line change). With this commit, i was able to take advantage of spring-loaded which monitors changes in my development folder and automatically reloads them whenever my IDE auto builds the class files.

In summary, after the above, all changes (whether new methods, classes are added or edited, etc) are automatically reloaded. And the only time that i will need to redeploy the omod is when am done with development and ready to release.

1 Like
3

Is there a documentation of how to use this feature?

4

Still looking for the best place where to put this. But in the meantime:

  1. Download the spring loaded jar from: https://github.com/spring-projects/spring-loaded

  2. If you are making changes to core, run with a VM argument like: javaagent:/Projects/springloaded-1.2.3.RELEASE.jar -noverify -Dspringloaded=inclusions=org.openmrs..*

OR

If you are changing a module, ensure that your core platform has the changes for TRUNK-4739. Then run with a VM argument like: -DuiFramework.development.xforms=/Projects/openmrs/xforms -javaagent:/Projects/springloaded-1.2.3.RELEASE.jar -noverify -Dspringloaded=inclusions=org.openmrs.module.xforms..*

Replace:

  • xforms with your moduleId.
  • /Projects/openmrs/xforms with the path to your module.
  • /Projects/springloaded-1.2.3.RELEASE.jar with where your spring-loaded jar is.

That should be all.

4 Likes
5

@dkayiwa, two things

First, this is pretty awesome. I had read about spring-loaded at some point, and then forgotten about it.

Second, I feel like any non-trivial change to the classloader needs to be discussed on OpenMRS Talk.

  • Modifying our core classloader to support a special development mode could have performance implications

  • And it introduces a difference between how code runs in development and production.

  • Is the reason we have to change code in openmrs-core because spring-loaded doesn’t rewrite classes loaded from JAR files, but only from .class files? Is it possible that we could make a PR against spring-loaded to support this from within their codebase, rather than changing the OpenMRS core classloader?

  • you should definitely not recycle “uiFramework” in the name of the VM argument. If we introduce this as a new core feature, it gets its own name…

6

Looking at the changes i had to make, they looked small and simple. But i fully agree that even what appears simple may introduce bugs or have performance implications. The good thing is that this change is only for developers. So we could revert the commit and instead have a pull request that a developer can apply locally to use a version of the platform for only development purposes. This is exactly what i was all along doing before i decided to actually commit it. :slight_smile:

As for the bullet point number 3, yes the current release of spring-loaded does not support reloading classes from JAR files. Even if it did, we would need some mechanism of compiling a new jar file for each change, and dump it into the temporary folder where our module engine extras omods. Creating a pull request against spring-loaded looked like a task which could take a reasonable amount of time. I did not try to do so because i assumed that their having not done this yet is because it was not obvious. But i could be wrong. :slight_smile:

Changing “uiFramework” to something else makes a lot of sense!

7

I have renamed the VM argument to “moduleid.development.directory”

2 Likes
8

@dkayiwa, to second @darius, this is amazing. To the point about reloading Jar files, I did find this one thread online which seems to imply that this is now supported with an argument like this:

-Dspringloaded=watchJars=foo.jar:bar.jar

See:

Not sure if this would change our solution or not, but thought I’d point it out…

9

By the time i implemented this, watching jars was not yet released. Will reevaluate the current release and test it out.