Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for Translator 3.7.1 #40

Merged
merged 2 commits into from
Mar 12, 2024
Merged

Conversation

dehall
Copy link

@dehall dehall commented Mar 8, 2024

This PR bumps the version of the CQL translator from 3.3.2 to 3.7.1. Thanks @elsaperelli for the initial pointers

Summary of changes:

  • At some point between 3.3.2 and 3.7.1 the translator migrated from javax.* to jakarta.* Accordingly, references within the code have to be updated as well. See changes to imports on most .java files
    • (TL;DR on the background of this change for posterity: a lot of what was under the javax namespace, such as XML binding and webservices, was originally part of "Java EE" (Enterprise Edition) which was run by Oracle. Eventually Oracle transferred this to the Eclipse Foundation but without allowing them to use the same javax namespace. Frustration ensues)
    • Also note that not all javax.* imports were updated. For example see TranslationResourceTest.java - the javax.xml.* imports were not changed. Why? Because these are part of "Java SE" (Standard Edition) and there is no jakarta version of these. https://stackoverflow.com/a/76364735
    • It's important to note that the javax versions of migrated classes may still exist, so missing an update wouldn't necessarily result in a compilation failure, it could produce weird results. (For example, I originally missed updating FormatFailureException.java and most things worked but the error page renderer would return an HTML page instead of the plain text error message in one of the unit tests. Fun to debug.) Please fully test all capabilities to make sure nothing else was missed
    • jaxb-runtime was updated to a version that supports Jakarta
    • Additional libraries needed to be included: jakarta.validation-api and jakarta.xml.bind-api
    • Jersey was updated to the latest release of v3, which targets Jakarta EE 9. There are newer versions of Jersey which target later versions of Jakarta, so they may or may not work
  • It seems like something changed in the translator serialization and optional arrays are now included if empty. Only code impact is one unit test needed to be updated

Also note: the Eclipse compiler complains because the xpp3 dependency includes a class in the javax.xml.namespace package, which duplicates a core java package and is no longer allowed. It shouldn't be a blocker since that dependency has been there for a while, but if you use Eclipse and see an error along those lines, that's why.

Copy link

@elsaperelli elsaperelli left a comment

Choose a reason for hiding this comment

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

Thank you for doing this!

I was able to successfully translate the CQL we have in some unit tests in fqm-execution as well as an entire measure bundle (CMS165) using both Java and Docker.

Copy link
Member

@cmoesel cmoesel left a comment

Choose a reason for hiding this comment

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

Thanks, @dehall! Figuring these types of things out is really annoying. Thanks for working it out for us!

I've tested this out w/ the CDS Authoring Tool and confirm it works there as well. Once I get the go ahead from the Abacus team to merge this, I will do that and then follow it up w/ a PR to bump the CQL Translation Service version number.

@cmoesel cmoesel merged commit ff83a90 into cqframework:master Mar 12, 2024
1 of 2 checks passed
@dehall dehall deleted the bump_translator branch March 12, 2024 13:34
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.

3 participants