LinuxForHealth / hl7v2-fhir-converter

Converts HL7 v2 Messages to FHIR Resources
Apache License 2.0
89 stars 36 forks source link

NullPointerException when setting `alternateResourceFolderFilePath` in >=1.0.14 #427

Closed CJStadler closed 2 years ago

CJStadler commented 2 years ago

Describe the bug Running FHIRConverterRunFile on versions 1.0.14 and greater I get the following error

java.lang.NullPointerException
    at java.util.Objects.requireNonNull (Objects.java:208)
    at sun.nio.fs.UnixFileSystem.getPath (UnixFileSystem.java:263)
    at java.nio.file.Path.of (Path.java:147)
    at java.nio.file.Paths.get (Paths.java:69)
    at io.github.linuxforhealth.hl7.resource.ResourceReader.getResource (ResourceReader.java:84)
    at io.github.linuxforhealth.hl7.resource.ResourceReader.getResourceInHl7Folder (ResourceReader.java:226)
    at io.github.linuxforhealth.hl7.resource.ResourceReader.getMessageModel (ResourceReader.java:167)
    at io.github.linuxforhealth.hl7.resource.ResourceReader.getMessageTemplates (ResourceReader.java:120)
    at io.github.linuxforhealth.hl7.HL7ToFHIRConverter.<init> (HL7ToFHIRConverter.java:56)
    at FHIRConverterRunFile.main (FHIRConverterRunFile.java:59)
    at org.codehaus.mojo.exec.ExecJavaMojo$1.run (ExecJavaMojo.java:254)
    at java.lang.Thread.run (Thread.java:833)

It looks to me like converterConfig.getAdditionalResourcesLocation() is returning null, which is then passed to Paths.get. I suspect the issue may be as simple as https://github.com/LinuxForHealth/hl7v2-fhir-converter/blob/58ad7c9/src/main/resources/config.properties missing the additional.resources.location key, but I haven't been able to get it to read a modified config file to confirm this.

It runs successfully on version 1.0.13.

To Reproduce I'm not sure how to run FHIRConverterRunFile directly from this repository (not very familiar with java) — I did it by setting up a minimal application with maven, with hl7v2-fhir-converter as a dependency.

Desktop (please complete the following information):

Thank you!

cragun47 commented 2 years ago

Hi @CJStadler . We'll look into it.

cragun47 commented 2 years ago

Hi @CJStadler . I regret I cannot reproduce the problem in my environment, which is running level 1.0.16.
I'm trying to figure out how I can reproduce the problem.

It surprises me that NULL from getAdditionalResourcesLocation() is causing a problem, because there are checks which only use the value if it is not null.

Your idea of adding additional.resources.location to the config.properties file is a good one. I assume you have downloaded source code? Can you change the existing config.properties located at src/main/resources/config.properties?

There is also a way to specify the config file from the command line. It is documented in the README.md.

CJStadler commented 2 years ago

Thanks so much for investigating @cragun47! Unfortunate that you can't reproduce ☹️

I will try to change the config.properties and test.

Here is my reading of the code, but maybe there is a null check I am missing. If the config field isn't present I believe it will default to null here: https://github.com/LinuxForHealth/hl7v2-fhir-converter/blob/11ef1aca2c1f5c0f4cb4adb737f164b2e3158c42/src/main/java/io/github/linuxforhealth/core/config/ConverterConfiguration.java#L93 That null would then be passed to Paths.get here, where the exception is raised: https://github.com/LinuxForHealth/hl7v2-fhir-converter/blob/11ef1aca2c1f5c0f4cb4adb737f164b2e3158c42/src/main/java/io/github/linuxforhealth/hl7/resource/ResourceReader.java#L84 Note that this is before the null check here: https://github.com/LinuxForHealth/hl7v2-fhir-converter/blob/11ef1aca2c1f5c0f4cb4adb737f164b2e3158c42/src/main/java/io/github/linuxforhealth/hl7/resource/ResourceReader.java#L91

cragun47 commented 2 years ago

Yet when I do my unit tests and run the converter with that config parameter missing, there is no exception.

I’d like to reproduce it before I make changes, so I can prove the fix works.

On Wed, Feb 2, 2022 at 10:12 AM Chris Stadler @.***> wrote:

If the config field isn't present I believe it will default to null here: https://github.com/LinuxForHealth/hl7v2-fhir-converter/blob/11ef1aca2c1f5c0f4cb4adb737f164b2e3158c42/src/main/java/io/github/linuxforhealth/core/config/ConverterConfiguration.java#L93 That null would then be passed to Paths.get here, where the exception is raised: https://github.com/LinuxForHealth/hl7v2-fhir-converter/blob/11ef1aca2c1f5c0f4cb4adb737f164b2e3158c42/src/main/java/io/github/linuxforhealth/hl7/resource/ResourceReader.java#L84 Note that this is before the null check here: https://github.com/LinuxForHealth/hl7v2-fhir-converter/blob/11ef1aca2c1f5c0f4cb4adb737f164b2e3158c42/src/main/java/io/github/linuxforhealth/hl7/resource/ResourceReader.java#L91

— Reply to this email directly, view it on GitHub https://github.com/LinuxForHealth/hl7v2-fhir-converter/issues/427#issuecomment-1028162216, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABFP5IPOAVYPX73CJJ7C5PLUZFQXXANCNFSM5NJU6RIA . You are receiving this because you commented.Message ID: @.***>

-- Brian Cragun LinkedIn http://www.linkedin.com/in/briancragun

CJStadler commented 2 years ago

Hi @cragun47, I've figured out the NPE is thrown when I run my application with openjdk 17, but not when I use openjdk 11. It looks like not throwing an NPE in this case was a bug in openjdk <16: https://bugs.openjdk.java.net/browse/JDK-8254876.

For my immediate purposes I can just use openjdk 11, but it still seems like there is something worth fixing here. Happy to open a PR if that makes sense to you.

As a side note, ./gradlew build fails for me with openjdk 17:


$ JAVA_HOME=/usr/local/opt/openjdk@17 ./gradlew build

> Configure project :
localDevEnv is true - Setting sourceset to the src directory

> Task :compileJava FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':compileJava'.
> java.lang.IllegalAccessError: class org.gradle.internal.compiler.java.ClassNameCollector (in unnamed module @0x436941f7) cannot access class com.sun.tools.javac.code.Symbol$TypeSymbol (in module jdk.compiler) because module jdk.compiler does not export com.sun.tools.javac.code to unnamed module @0x436941f7
cragun47 commented 2 years ago

@CJStadler If you'd like to open a PR, that would be great.