Fix JAR resource loading

[art-and-co]

Currently there are following issues with JCL v4+:

• JCL has reversed logic for populator value in properties file.
• JCL cannot open found JAR resource files

Reversed Populator Logic

This line is incorrect:

try (InputStream is = isPopulatorFileDefined() ? new FileInputStream(DEFAULT_XML_FILE_NAME) : getPopulatorFileIS())

Instead of loading the defined populator file it loads default and vice versa.

JCL Cannot Load Found JAR Resource Files

JCL v4 uses try-with-resources in findFileInJarZipFiles method:

try (ZipFile zipFile = new ZipFile( jarZipFileName ))

As result this statement is also closing returned zipEntry file handle too.

To correct this try and finally were used instead.

NB: It seems my Eclipse settings have screwed whitespace with tabs a little bit.

[kuniss]

Thanks @art-and-co, for your contribution!
It is definitely valuable!

See my comments for further improvements, maybe.
I really would like to know your opinion about my slight changes to your proposal.
If you agree, just take them over in your PR.

[kuniss]

@art-and-co , one further note:
Could you maybe correct the second code snippet in your PR comment. I guess, you had made an copy'n'paste error - it is the same code snippet as at the first issue you brought up.

[art-and-co]

I will try to add 3 test cases for JavaxRegPopulator.load() to cover current modifications:

1. Default jpos.xml not found
2. Custom file found
3. Custom file not found

Loading default jpos.xml is not that easy as it loads from working directory. In JUnit w/ IDE case it is project root.

It would be nice to cover JAR reading logic as well but this requires a bit of efforts: JAR generation and etc. So I would prefer to leave it out of scope.

Currently I got tests running locally. Now I need to understand tests structure better.

[art-and-co]

Added mentioned test cases. Please, review.

Now I copy-pasted some test logic. Maybe it could be extracted into separate method?

[art-and-co]

This was unintentional - I just wanted to add to my branch only.

[kuniss]

Thanks for taking the effort to add some tests.
I have one question, however. See my other comment.

[kuniss]

I will merge the changes shortly.

[kuniss]

A first release candidate version of 4.0.2 with this PR contained is available at Sonatype's snapshot repository https://oss.sonatype.org/content/repositories/snapshots/.

<dependency>
  <groupId>org.javapos</groupId>
  <artifactId>javapos-config-loader</artifactId>
  <version>4.0.2-SNAPSHOT</version>
</dependency>

@art-and-co, please test.

[art-and-co]

Thank you, @kuniss, it was a pleasure to have a code review with you.

[kuniss]

You are welcome! Me too.

[kuniss]

@art-and-co , what do you think, when you will be able to test the release candidate from above?

[art-and-co]

Yes, I will test it and will report if I would find any issues.

Thank you!

As well I have found that javapos version 1.15.x is using JCL v4+. So probably javapos version that includes these fixes may become part of the solution I am working with.

[kuniss]

As well I have found that javapos version 1.15.x is using JCL v4+. So probably javapos version that includes these fixes may become part of the solution I am working with.

I'm not sure whether I have understood you correctly, but when the version of the javapos-config-loader with your PR contained gets released I will push a javapos release with that javapos-config-loader release incorporated.

The javapos package is just an aggregation project which incorporates javapos-contracts, javapos-config-loader, and javapos-controls in particular versions. Finally, a convenience package...

[art-and-co]

Previously when I have looked at Maven javapos was still using JCL v3.2 only. Now I have found that newer versions are actually using v4 and dropped Xerces dependency.

[kuniss]

Previously when I have looked at Maven javapos was still using JCL v3.2 only. Now I have found that newer versions are actually using v4 and dropped Xerces dependency.

As I said, javapos is only a aggregation package. It incorporates all the other packages from the JavaPOS Working Group.
Therefore, your contribution and correction is important! It will finally appear in the javapos package too. I will take care of this.

[kuniss]

@art-and-co, it seems, the one issue you found and corrected was already tested successfully: See here: JavaPOSWorkingGroup/javapos#9 (comment).
You may concentrate on the second issue and on regressions. 😄

[art-and-co]

I am happy to hear that it was tested by somebody else too. It is not clear where jpos.properties file was stored. It could be that JAR resource reading logic was also tested.

I have tested locally the changes before creation of this PR.

There were some changes introduced during PR, which were not fully tested yet by me.

I plan to do some additional tests this week, which would cover the mentioned changes as well. As well I will use 4.0.2-SNAPSHOT version from Maven repository.

I will report results here.

I am looking forward to see JCL v4.0.2 used in javapos JAR so its version can be used as official supported reference version number in our projects.

[art-and-co]

@kuniss, finally, I have tested with the new JCL version using available online snapshot version.

I have build javapos 1.15.5 JAR locally using JCL 4.0.2-SNAPSHOT.

I have verified that generated source JAR contains correct modifications.

I have run this version on Windows 10 using Java 11. I was running in normal and debug mode.

I have tested both cases:

• Populator logic
• ZIP stream modifications.

In ZIP stream logic it was properly loading jpos/res/jcl.xsd and jpos/res/jcl.dtd files from javapos JAR file.

The testing has been successful.

[kuniss]

Great! I will build and publish it tomorrow then.
I guess, some people are waiting already. :-)

[art-and-co]

NB Today I have tested on Windows 10 Java 1.8. All run OK.

[kuniss]

javapos-config-loader-4.0.2 release is available on Maven Central: https://central.sonatype.com/artifact/org.javapos/javapos-config-loader/4.0.2

According javapos release incorporating too: https://central.sonatype.com/artifact/org.javapos/javapos/1.15.6