Openhab-addons: Removal of dependency on 'org.apache.commons.*'

[cweitkamp]

In https://github.com/openhab/openhab-core/pull/1433 and https://github.com/openhab/openhab-core/pull/1441 (and more) we removed dependency on org.apache.commons.* libraries. They will be removed from OH3 Core API and thus not available anymore for OH add-ons too. Most of the methods which are used by several bindings are convenience helper methods and can be replaced easily by basic Java methods. Other methods are not yet available in Java 8 but in Java 11 and have to be replace later. It would be a good start to remove as much as we can before.

[lolodomo]

I already created 12 PRs to eliminate calls to IOUtils.closeQuietly.
As a first feedback in one of these PRs, @J-N-K said we should not close streams silently. So in additional to suppress the call to IOUtils.closeQuietly, we need to add a log.
I will update the already existing PRs.

[lolodomo]

I already created 12 PRs to eliminate calls to IOUtils.closeQuietly.
As a first feedback in one of these PRs, @J-N-K said we should not close streams silently. So in additional to suppress the call to IOUtils.closeQuietly, we need to add a log.
I will update the already existing PRs.

Is it oj for all mainteners that we should not close stuff (streams) silently ?
3 of my PRs include a log in case of error and are ready for a review/merge.
9 of my PRs are now tagged with "WIP" waiting to know if this is the way to go or not.

[cweitkamp]

Imo it should be okay to log the exception even if I do not really expect it to happen too often. But we should check two other things in advance:

1. Do we really need to close it? Or does the source of the stream takes care of it on its own (e.g. it is not necessary to close an InputStream from the Socket#getInputStream() method)
2. Can it maybe handled even better by using a try-with-resources statement

[lolodomo]

https://docs.oracle.com/javase/8/docs/api/java/net/Socket.html#close--
Closing this socket will also close the socket's InputStream and OutputStream.

Ok, you're right, this is useless for socket.
We have to check if it is required or not for serial port.

[lolodomo]

For serial port, it could depends on the framework implementation.
gnu.io.SerialPort does not seem to close automatically the in/out streams.
https://www.docjava.com/book/cgij/jdoc/gnu/io/CommPort.html#close()

[lolodomo]

In the Sonos binding, StringEscapeUtils.unescapeXml and StringEscapeUtils.escapeXml are used. What alternative do you propose ?

[cweitkamp]

That seems to be a hard one. I am not sure if there is a Java native equivalent available. At least not for Java 8. IIRC Java 13 added a method which does this: String#translateEscapes.

Even the methods from commons-lang have been deprecated. As of 3.6, it is recommended to use commons-text StringEscapeUtils instead.

That could be one option. Add commons-text dependency for this bundle. Otherwise we could add our own implementation in a util class (XMLUtils). For OH2.5.x inside the binding. For OH3 in org.openhab.core.util or in org.openhab.core.io.net.http.

[lolodomo]

That seems to be a hard one. I am not sure if there is a Java native equivalent available. At least not for Java 8. IIRC Java 13 added a method which does this: String#translateEscapes.

Even the methods from commons-lang have been deprecated. As of 3.6, it is recommended to use commons-text StringEscapeUtils instead.

That could be one option. Add commons-text dependency for this bundle. Otherwise we could add our own implementation in a util class (XMLUtils). For OH2.5.x inside the binding. For OH3 in org.openhab.core.util or in org.openhab.core.io.net.http.

We have 6 bindings using org.apache.commons.lang.StringEscapeUtils:

1. amazonechocontrol
2. bsblan
3. bosesoundtouch
4. homematic
5. sonos
6. wemo

[lolodomo]

To be honest, I have a big doubt this is really doable to remove ALL these dependencies. Too many are used in too many bindings.

[fwolter]

I filed a PR to remove Apache Commons from the Coding Guidelines default libraries: openhab/openhab-docs#1229

[Hilbrand]

@yfre please stop unpinning issues.

[mherwege]

That seems to be a hard one. I am not sure if there is a Java native equivalent available. At least not for Java 8. IIRC Java 13 added a method which does this: String#translateEscapes.
Even the methods from commons-lang have been deprecated. As of 3.6, it is recommended to use commons-text StringEscapeUtils instead.
That could be one option. Add commons-text dependency for this bundle. Otherwise we could add our own implementation in a util class (XMLUtils). For OH2.5.x inside the binding. For OH3 in org.openhab.core.util or in org.openhab.core.io.net.http.

We have 6 bindings using org.apache.commons.lang.StringEscapeUtils:

1. amazonechocontrol
2. bsblan
3. bosesoundtouch
4. homematic
5. sonos
6. wemo

You can add the upnpcontrol binding to this list.

@cweitkamp @lolodomo Would unbescape be a possible alternative dependency to cope with the functionality of StringEscapeUtils? See here. The jar is 169k and I don't see external dependencies for it.

[cweitkamp]

Short feedback - with the latest code we still have some occurrences (463 imports) of org.apache.commons left:

$ grep -iR "import org.apache.commons.io" | wc -l
24
$ grep -iR "import org.apache.commons.lang" | wc -l
386
$ grep -iR "import org.apache.commons" | grep -v "commons.io" | grep -v "commons.lang" | wc -l
53

I do not think it will be possible to eliminate all of them until code freeze for OH3. Thus I would suggest to postpone this to first OH3 milestone. For some bundles like o.a.c.l.StringEscapeUtils, o.a.c.mail or o.a.c.net and others we do not have an appropriate replacement. Those dependencies can eventually be added to the bindings POM / feature directly - if not already done. This would pave the way for cleaning leftovers from OHC.

[mherwege]

@cweitkamp Maybe it got lost in the history (https://github.com/openhab/openhab-addons/issues/7722#issuecomment-679997251), but did you check unbescape as an alternative for StringEscapeUtils?