Skip to content

New major version without Xerces#8

Merged
kuniss merged 49 commits intomasterfrom
pr-for-4.0-improvements+xerces-removal
Sep 13, 2024
Merged

New major version without Xerces#8
kuniss merged 49 commits intomasterfrom
pr-for-4.0-improvements+xerces-removal

Conversation

@kuniss
Copy link
Member

@kuniss kuniss commented Jul 4, 2024

This PR is intended to implement the new major version 4 of javapos-config-loader incorporating the following major changes

  • replacing the Xerces XML parser implementation by a Javax based one (contribution by @mjpcger )
  • a lot of source code improvements
    • generic types, code smell removals,
    • modernization of the 1999 code base,
    • internal data structure hiding
  • removal of deprecated methods and classes
  • solving long standing issue Class XmlHelper is not compatible with webstart environment #1 to enable javapos-config-loader for web-start environments

mjpcger and others added 30 commits May 27, 2024 17:53
is missing.

Xerces2RegPopularor did not work correctly whenever one of the optional
attributes are missing. Using an empty string in that case.
where possible as we are going to change it anyway. However, avoided
changes at public method signature to ensure backward compatibility.
by implementing all resources access using the try-with-resources code
schema where possible and meaningful.
if implicitly requested by usage of the DefaultCompositeRegPopulator.
Instead throwing a runtime exception .

Restricted the check from null pointer check to empty list check,
because in case of an empty list the subsequent code may cause the NPE
seen in the tests after having applied the try-with-resources code
schema strictly.
where possible. If not possible SuppressWarnig annotation had been
added, e.g. on public APIs.

Furthermore:
- clean up not done for Xerces code as it will be removed
- low hanging fruits taken, e.g. using StringBuilder instead of
StringBuffer
- improved initialization of singletons in jpos.profile
- further code improvements to get less code smell (no complains for
this old code...)

Tests passed locally.
by restructuring the control flow.
like
- bad constant referencing
- StringBuffer usage
- unnecessary casting
This is a backward incompatible change!
But OK for the new major number 4.0.

All deprecated since long time (1999)
- jpos.config.simple.SimpleEntryRegistry parameterless constructor
- jpos.loader.simple.SimpleServiceManager parameterless constructor
- jpos.util.Tracer internal class
The new class JavaxRegPopulator extends the config loader with an XML
populator that works with Javax instead of Xerces and thus enables
JavaPOS to be used without the external modules of the Xalan / Xerces
project.
according to discussion in
#7.

In addition to requested changes, string constant "jpos.xml" has been
replaced by constant DEFAULT_XML_FILE_NAME in method save.
- Constants for path and names of DTD and XSD file and for the DTD
document type value moved to XmlRegPopulator.
- Constants for XML tag and attribute values added to XmlRegPopulator.
- "Unchecked" warning suppressed for some methods.
- Method getSortedList removed. Use Collections.sort directly instead.
- Method notAttribute renamed to cannotSetAttributeOfTag for better
understanding.
- Comments for method cannotSetAttributeOfTag added.
- LinkedList replaced by ArrayList where meaningful and vice versa.
The default SimpleXmlRegPopulator has been changed to use
JavaxRegPopulator instead of XercesRegPoputator now. This allows to
remove Xerces completely later on.
instance a different file.
Solved by clearing the internal data structure, which was missing.
Detected by adding a copy of XercesRegPopulatorTestCase which
instantiates JavaxRegPopulator and fails for test
jpos.config.simple.xml.JavaxRegPopulatorTestCase.testXercesPopulator1().
Some other source formatting cleanups.
for a more readable implementation as discussed at
#7 (comment)
Note, there is still a compiler warning at this changed lines requesting
to use generic types. This is already fixed with branch fixes-from-pr7
and will work out when merging both branches together on the main.
Code cleanup: Defined a constant for that restriction rule attribute.
Affected methods are
- findFileInJarZipFiles(String fileName, Vector<String>
jarZipFilesVector) restored, forwarding to the new method
findFileInJarZipFiles( String fileName, List<String> jarZipFilesList )
- getJposEntries() conceptually substituted by methods
addJposEntry(String logicalName, JposEntry entry) and
clearAllJposEntries()

Both old reintroduced methods has been marked as deprecated, however,
pointing to the new methods.
DefaultProfileFactory.java is a copy of XercesProfilefactory from the
fixes-from-pr7 branch coming with a lot of code clean up.

At SimpleServiceManager.java the instantiation of the ProfileFactory has
been changed to the new DefaultProfileFactory.
Follow up to the previous commit.
@kuniss kuniss self-assigned this Jul 4, 2024
@github-actions
Copy link

github-actions bot commented Jul 4, 2024

Test Results

159 tests   - 6   159 ✅  - 6   0s ⏱️ ±0s
 22 suites  - 1     0 💤 ±0 
 22 files    - 1     0 ❌ ±0 

Results for commit 24aea5a. ± Comparison against base commit 72ee439.

♻️ This comment has been updated with latest results.

@kuniss
Copy link
Member Author

kuniss commented Jul 4, 2024

@mjpcger, do you want to review this pull request before I'm merging it? Most of the work is from your contribution...

I would, however, publish a pre-release of the original branch to Maven Central such that it can be verified in local build environments.

Copy link
Contributor

@mjpcger mjpcger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Se.ems to be good, only some JavaDoc comments should not remain empty

as requested by reviewer.
They had been added automatically by Eclipse when generating the serial
UID to solve compiler warnings. A comment would be meaningsless at the
end here...
@kuniss
Copy link
Member Author

kuniss commented Jul 5, 2024

@mjpcger could you please make a final review and approve the PR, please?
You should have something like the following in the GitHub GUI under the Review Changes button:
image

I could merge without your approval (because I'm the project admin 😄), however, I would like to try the "clean" process way...
Thanks!

@kuniss
Copy link
Member Author

kuniss commented Jul 5, 2024

A RC built from this PR has been published to Maven Central's snapshot repository at https://oss.sonatype.org/content/repositories/snapshots/org/javapos/javapos-config-loader/4.0.0-SNAPSHOT/.

It may be downloaded from there or resolved by dependency resolution if configured appropriately, e.g. for Gradle:

repositories {
    maven {
        url 'https://oss.sonatype.org/content/repositories/snapshots'
    }
}

dependencies {
    implementation 'org.javapos:javapos-config-loader:4.0.0+'
}

Who ever is interested, please integrate in your environment for testing purposes and report back any issue here in this PR.

Copy link
Contributor

@mjpcger mjpcger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good for me, no change necessary.

@mjpcger
Copy link
Contributor

mjpcger commented Jul 5, 2024

More approval is not possible for me. Looks as if I have no write access and therefore my approval is not enough to change the Review required status.

@kuniss
Copy link
Member Author

kuniss commented Jul 5, 2024

Thanks, @mjpcger!
I adjusted the branch rules. Now I can merge according to the rules. 😄

@kuniss
Copy link
Member Author

kuniss commented Jul 11, 2024

Who ever is interested: We just encountered in our off-side integration test that some of our JavaPOS XML configuration files are not read in correctly by the new javapos-config-loader-4.0.0-RC1, Those were read in without errors by the Xerces implementation, however.

We are investigating...

@kuniss
Copy link
Member Author

kuniss commented Jul 12, 2024

The reported regression was a misinterpretation. In fact, it new XML populator is just more accurate. If the DTD is given in the JavaPOS XML configuration file like
´´´

´´´
then file jpos/res/jcl.dtd must be in the classpath, otherwise the validation fails and no entries are read in.

@kuniss
Copy link
Member Author

kuniss commented Jul 12, 2024

I improved the tracing a bit. So, I'm going to publish a new SNAPSHOT on Maven Central's snapshot repository - RC2.

kuniss added 6 commits July 16, 2024 15:25
which is the same implementation, just another name to get rid of
"Xerces" in the name.
In fact, it was forgotten to remove the old file when renaming it.
in case there is a configuration error which can not be recovered from.
by refactoring out code to private methods for better readability.
@kuniss kuniss merged commit 24aea5a into master Sep 13, 2024
@kuniss kuniss deleted the pr-for-4.0-improvements+xerces-removal branch February 7, 2025 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants