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

Completed service file parsing. #5205

Merged
merged 1 commit into from
Jan 26, 2023

Conversation

Master-Code-Programmer
Copy link
Contributor

Issue #5204.

According to the official documentation of Javas Service Loader it may contain comments. Everything which comes after the '#' is considered a comment. Reference: https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/ServiceLoader.html.
But Mojarras own Service Loader implementation doesn't support comments. It even doesn't skip blank lines. This leads to unexpected behaviour when using Mojarras Service Loader. When files under META-INF/services are loaded from Mojarras Service Loader it may lead to exceptions, while these service files under any other META-INF/services directory may perform perfect when they're loaded by the JDK Service Loader.

Therefore I implemented a Pull-Request which fixes that, so comments and blank lines don't lead to crashes when loading service files.

I also added unit tests to test the new functionality. To an area which had previously no test coverage at all.

I also replaced the magic string "UTF-8" with StandardCharsets.UTF_8 in the method I updated. It was introduced with Java 7 and can therefore also be used in a project where downwards compability is important.

P.S.: I read, understood and accepted the ECA.

Added tests for service file parsing.
@Master-Code-Programmer
Copy link
Contributor Author

Old PR (wasn't working, used another company e-mail): #5203

@BalusC BalusC merged commit 8d6e804 into eclipse-ee4j:4.0 Jan 26, 2023
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