Remove dependency on commons-lang3

[Thomas Zimmermann]

Some general motivation/rant:

Keeping dependencies minimal is pretty crucial for Java libraries, because everything ultimately runs on the same class- or module-path, which doesn't support multiple versions. On each dependency update the application developer is forced to resolve version conflicts and pick a version of a shared dependency that works for every library that depends on it (which might not even be possible). So every (transitive) dependency increases the likelihood of conflicts.

Depending on general purpose libraries like commons-lang or commons-collections is especially bad because of their wide appeal. commons-lang3 is also 488 kB and oauth2-oidc-sdk only uses like 5 methods/classes.

I looked at the use of commons-lang3, summary and suggested solutions below.

Uses of org.apache.commons.lang3.tuple.Pair or org.apache.commons.lang3.tuple.ImmutablePair:

• com.nimbusds.openid.connect.sdk.id.PairwiseSubjectCodec (in API)
• com.nimbusds.openid.connect.sdk.id.AESBasedPairwiseSubjectCodec (in API)
• com.nimbusds.openid.connect.sdk.id.SIVAESBasedPairwiseSubjectCodec (in API)
• com.nimbusds.oauth2.sdk.auth.X509CertificateConfirmation (internal use)

Suggested solution:

• Replace Pair with Map.Entry, ImmutablePair with AbstractMap.SimpleImmutableEntry
• If PairwiseSubjectCodec and friends are supported API, consider introducing a new method and deprecating the old one first.

Uses of org.apache.commons.lang3.StringEscapeUtils::escapeJava:

• com.nimbusds.oauth2.sdk.token.BearerTokenError

Suggested solution:

• not sure. usage looks a bit fishy to me, the code tries to escape http header values, why use java escaping rules for this?

Uses of org.apache.commons.lang3.StringUtils::is(Not)Blank:

• Lots of places, most used methods

Suggested solution:

• Write own version

Test-Code only uses simple to replace stuff.

Thank you for developing this library, it's really handy!

[Vladimir Dzhuvinov]

Hi Thomas,

Thanks a lot for this comprehensive list of suggestions. The suggestions make sense. Some of them will be trivial to implements, other will perhaps require more thought.

I'm curious, do you use the library in std. Java or on Android?

[Thomas Zimmermann]

Hi Vladimir,

Thank you for looking into this! I'm integrating this library through pac4j-oidc and pac4j-undertow on the server side (so no Android). I'm using KeyCloak as the SSO-Server. If you need any more information, please let me know!

[Vladimir Dzhuvinov]

Do you reckon shadowing can work here, at least in some of those tickets?

Reuse it generally a good thing, and something I'd like to keep.

I've noticed that some of the large deps that we have in the Connect2id server, like Infinispan, practise shadowing.

[Thomas Zimmermann]

Shadowing would work. However, because the runtime does not care about jars, only about packages, the dependencies must be relocated to a different package. If not, two jars containing the same classes (but potentially different versions) can be on the classpath, making it hard to guess which version will be used (they could even be mixed). Jigsaw won't even start in this case.

If the relocated classes are supported public API however, this would break backward compatibility.

I think a possible plan of action could be:

1. Remove all unrelated third party classes from the public supported API (here: org.apache.commons.lang3.tuple.Pair) and replace them with classes from the JDK or own tiny wrapper classes (with deprecation phase, if applicable)
2. Relocate internally used dependencies (e.g. org.apache.commons.lang3.tuple.Pair -> com.nimbusds.oauth2.sdk.shaded.org.apache.commons.lang3.tuple.Pair) and
3. Remove dependencies from pom.xml

I'm not familiar with infinispan, but I looked at the source and they relocate their dependencies as well, so that approach looks good.

[Vladimir Dzhuvinov]

Commons collections was easy, let's see this one now :)

[Vladimir Dzhuvinov]

Replaced StringUtils with local version: 4c15045

[Vladimir Dzhuvinov]

Removed all deps on Commons Lang, by duplicating utils code or switching to Map.Entry. The pairwise subject codecs are likely used by us only, so the effect of the API change should be minimal (and not hard to adjust to).