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

Fix java version parsing #253

Merged
merged 1 commit into from
Apr 15, 2024

Conversation

elkkhan
Copy link
Contributor

@elkkhan elkkhan commented Apr 14, 2024

Current implementation of parsing the Java version assumes that all Java versions returned by System.getProperty("java.version") will be in MAJOR.MINOR.PATCH format (e.g 21.0.2), while this is incorrect.
This breaks on versions that do not follow this format - for example, JDK 21+35 returns 21.
This is causing two issues:

  • Inconsistent date formats across different versions that are all > 1.6 - here
  • Inconsistent User-agent header - here

This PR changes version parsing to use a versioning abstraction that is Comparable - I chose to use the existing implementation from maven-artifacts

I've tried to write some tests for this but to be honest I don't find the MessageBirdServiceImpl class very unit-testable, so I ended up not writing a unit test for this.

@elkkhan
Copy link
Contributor Author

elkkhan commented Apr 15, 2024

hi @denizkilic! can i get a review on this please?

@denizkilic
Copy link
Member

Hi @elkkhan, thanks for the detailed description. The PR looks good to me. I will merge and prepare a new release.

@denizkilic denizkilic merged commit ec55bf7 into messagebird:master Apr 15, 2024
@denizkilic denizkilic mentioned this pull request Apr 15, 2024
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.

None yet

2 participants