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.

Sign up for GitHub

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

Jump to bottom

Replace usages of javax.xml.bind.DatatypeConverter #5083

Merged
merged 2 commits into from
Mar 11, 2019
Merged

Replace usages of javax.xml.bind.DatatypeConverter #5083

merged 2 commits into from
Mar 11, 2019

Conversation

wborn
Copy link
Member

@wborn wborn commented Mar 11, 2019 •
edited
Loading

The DatatypeConverter class is no longer provided by the default Java 11 modules.

See https://github.com/openhab/openhab2-addons/pull/5080#issuecomment-471343209

With these changes the Java 11 build now fails on the DenonMarantz Binding which uses even more javax.xml.bind classes. See this Travis Java 11 build of a branch with these fixes and https://github.com/openhab/openhab2-addons/pull/5080.

@wborn wborn requested a review from cweitkamp March 11, 2019 01:23
cweitkamp
cweitkamp requested changes Mar 11, 2019
View reviewed changes
Copy link
Contributor

@cweitkamp cweitkamp 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. Two additional improvements are possible. See comments inside.

now fails on the DenonMarantz Binding

We should open a new issue to refactor the binding and add the "help wanted" label.

...nding.helios/src/main/java/org/openhab/binding/helios/internal/handler/HeliosHandler221.java Outdated
@@ -1007,7 +1007,7 @@ public void filter(ClientRequestContext requestContext) throws IOException {
private String getBasicAuthentication() {
String token = this.user + ":" + this.password;
try {
return "Basic " + DatatypeConverter.printBase64Binary(token.getBytes("UTF-8"));
return "Basic " + Base64.getEncoder().encodeToString(token.getBytes("UTF-8"));
Copy link
Contributor

Choose a reason for hiding this comment

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

We could use StandardCharsets.UTF_8 instead of "UTF-8" to omit the check for UnsupportedEncodingException.

Suggested change
return "Basic " + Base64.getEncoder().encodeToString(token.getBytes("UTF-8"));
return "Basic " + Base64.getEncoder().encodeToString(token.getBytes(StandardCharsets.UTF_8));

...hab.binding.tesla/src/main/java/org/openhab/binding/tesla/internal/handler/TeslaHandler.java Outdated
@@ -1416,7 +1416,7 @@ public void filter(ClientRequestContext requestContext) throws IOException {
private String getBasicAuthentication() {
String token = this.user + ":" + this.password;
try {
return "Basic " + DatatypeConverter.printBase64Binary(token.getBytes("UTF-8"));
return "Basic " + Base64.getEncoder().encodeToString(token.getBytes("UTF-8"));
Copy link
Contributor

Choose a reason for hiding this comment

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

We could use StandardCharsets.UTF_8 instead of "UTF-8" to omit the check for UnsupportedEncodingException.

Suggested change
return "Basic " + Base64.getEncoder().encodeToString(token.getBytes("UTF-8"));
return "Basic " + Base64.getEncoder().encodeToString(token.getBytes(StandardCharsets.UTF_8));

@cweitkamp
Copy link
Contributor

Does it make sense to add the package javax.xml.bind to the list of forbidden packages or should we assume that people know that it has been deprecated / removed?

https://github.com/openhab/openhab2-addons/blob/772062fd37e5f18fe3a6c2148107da58d65f733a/tools/checkstyle.properties#L4

@wborn
Copy link
Member Author

wborn commented Mar 11, 2019

It's still possible for the framework to provide a bundle for the JAXB packages. The bindings could also add these libraries themselves e.g. using Maven dependencies.

See also this nice migration guide and stackoverflow post.

@wborn
Copy link
Member Author

wborn commented Mar 11, 2019 •
edited
Loading

We should open a new issue to refactor the binding and add the "help wanted" label.

We certainly should and maybe label them accordingly so we can easily see what needs to be done for supporting Java 11 (openhab/openhab-distro#768). There are already several issues for other bindings depending on full JREs. I'll run SAT again and see what other bindings depend on a full JRE.

martinvw
martinvw approved these changes Mar 11, 2019
View reviewed changes
Copy link
Member

@martinvw martinvw left a comment

Choose a reason for hiding this comment

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

Okay, given the remarks by @cweitkamp

wborn added 2 commits March 11, 2019 18:14
@wborn
Replace usages of javax.xml.bind.DatatypeConverter
5be6514 
The DatatypeConverter class is no longer provided by the default Java 11 modules

Signed-off-by: Wouter Born <[email protected]>
@wborn
Replace "UTF-8" with StandardCharsets.UTF_8
8c40ca1 
Signed-off-by: Wouter Born <[email protected]>
@wborn wborn force-pushed the replace-DatatypeConverter-usage branch from c59ee23 to 8c40ca1 Compare March 11, 2019 17:20
@wborn
Copy link
Member Author

wborn commented Mar 11, 2019

I've applied the two improvements @cweitkamp! 👍

@wborn wborn mentioned this pull request Mar 11, 2019
Merged
cweitkamp
cweitkamp approved these changes Mar 11, 2019
View reviewed changes
Copy link
Contributor

@cweitkamp cweitkamp left a comment

Choose a reason for hiding this comment

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

Great job. Thanks.

@cweitkamp cweitkamp merged commit db8ccc6 into openhab:master Mar 11, 2019
@wborn wborn deleted the replace-DatatypeConverter-usage branch March 11, 2019 20:47
@wborn wborn added this to the 2.5 milestone Mar 17, 2019
@cweitkamp cweitkamp mentioned this pull request Mar 19, 2019
Merged
Pshatsillo pushed a commit to Pshatsillo/openhab-addons that referenced this pull request Jun 19, 2019
@wborn @Pshatsillo
Replace usages of javax.xml.bind.DatatypeConverter (openhab#5083)
10b0d42 
* Replace usages of javax.xml.bind.DatatypeConverter

The DatatypeConverter class is no longer provided by the default Java 11 modules

* Replace "UTF-8" with StandardCharsets.UTF_8

Signed-off-by: Wouter Born <[email protected]>
Signed-off-by: Pshatsillo <[email protected]>
ne0h pushed a commit to ne0h/openhab-addons that referenced this pull request Sep 15, 2019
@wborn @ne0h
Replace usages of javax.xml.bind.DatatypeConverter (openhab#5083)
20686a3 
* Replace usages of javax.xml.bind.DatatypeConverter

The DatatypeConverter class is no longer provided by the default Java 11 modules

* Replace "UTF-8" with StandardCharsets.UTF_8

Signed-off-by: Wouter Born <[email protected]>
Signed-off-by: Maximilian Hess <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cweitkamp cweitkamp cweitkamp approved these changes

@martinvw martinvw martinvw approved these changes

@kgoderis kgoderis Awaiting requested review from kgoderis

@mgeramb mgeramb Awaiting requested review from mgeramb

@paulianttila paulianttila Awaiting requested review from paulianttila

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.5
Development

Successfully merging this pull request may close these issues.
3 participants
@wborn @cweitkamp @martinvw