JavaPOSWorkingGroup / javapos-config-loader

JavaPOS Configuration Loader (aka JCL) for loading JavaPOS configurations and providing JavaPOS configuration data to applications
Other
0 stars 6 forks source link

Get rid of Xerces library as dependency #2

Closed kuniss closed 2 days ago

kuniss commented 4 years ago

The used Xerces librarary in version 1.2.3 is very old. So, it should either be replaced by a newer version or eliminated completely as dependency.

Eliminating is perferred as it would also solve one of the problems when moving to the a Java Module System (JMS) implementation as this analysis shows.

There also a solution had been drafted on how to solve this in general (independent of making it JMS ready).

Hackmancoltaire commented 9 months ago

Has there been any movement on this?

kuniss commented 9 months ago

At least I have done a short problem and solution analysis ...

Feel free, to make a contribution. 😃

Hackmancoltaire commented 9 months ago

So rather than fork this repo I took a stab at merging all of them into one, making it module compatible, and refactoring out the Xerces dependancy. This builds and runs for me, but the tests need to be refactored to use the newer JUnit. My company uses this heavily, so modern support is critical. Please feel free to take this repo and restructure it as you see fit. https://github.com/Hackmancoltaire/javapos

kuniss commented 9 months ago

Thanks for your contribution, @Hackmancoltaire!

Unfortunately, I cannot take this over that easily as it is not a PR to the javapos-config-loader project and makes me a lot of work which I have not and do not get the time for, unfortunately.

The split into different projects and libraries has been done for a reason and with intention. In fact, the device controls are often provided separately by device vendors or application vendors as their own implementation for different reasons. The one given here is only a reference implementation. Furthermore, the 3 libraries are typically changed for different reasons. E.g., javapos-contracts and javapos-controls are changed if a new UnifiedPOS version gets released. javapos-config-loader, however, is not a real part of the standard and only mentioned in the JavaPOS appendix of UnifiedPOS. So, it does not get change if a new UnifiedPOS version is release. And, even in regards to the SRP it is not good to blend them together! Versioning would becomes harder too.

The details for this design decision (separating the libraries) are described in the javapos project wiki, see chapter Flexibility in Dependency Management.

The javapos project builds an aggregated library which makes an uber JAR from all the 3 separate libraries, for users which does not use their own device control library. That way, we can satisfy both "kinds" of users.

So, how to proceed from here without losing your valuable contribution effort? May I just copy over the javapos-config-loader source part from your fork and see the diffs in the original project? Or, did your have applied structural changes to the sources too (renaming, moving, added files or packages, etc)?

Did you run the javapos-config-loader tests?

Did you follow the draft from here for the changes, or did you implement it "from scratch"?

Hackmancoltaire commented 9 months ago

I did not follow the draft. I just implemented a fix from scratch. You are welcome to use anything I did to improve the state of this codebase.

The primary issues I had were in using Jpos was within a module based project, and the dependency on Xerces.

Separating the concerns is logistically fine. You might use Git submodules to accomplish that while still having a central repository, but each project would need the proper module-info file.

Using the blended version I was able to compile it with my changes, and use it successfully with our DataLogic scale / scanner without any java module dependency issues.

If you want me to submit a more distinct PR against the Xerces changes I can.

kuniss commented 9 months ago

If you want me to submit a more distinct PR against the Xerces changes I can.

You mean, you could do a PR against javapos-config-loader with Xerces dependency removed?

That would be very appreciated!

mjpcger commented 9 months ago

May be the need for this issue has been removed by pull request Reduction of Dependencies and Deprecation Warnings: As default, a new Javax based populator, JavaxRegPopulator class will be used instead of XercesRegPopulator will be used. For cases where an application uses Xalan/Xerces anyway, either XercesRegPopulator or Xerces2RegPopulator can still be entered in jpos.properties to prevent unnecessary load of javax xml classes.

kuniss commented 2 months ago

This can be tested with release candidate RC2.

kuniss commented 2 days ago

This issue has been fixed in release 4.0.0.