Upgrade dependencies to a newer version

[lojzatran]

When working with this library, I noticed that Netty library used in this project is almost 3 years old (https://netty.io/news/2017/12/11/4-0-54-Final-4-1-18-Final.html) (https://github.com/commercetools/commercetools-jvm-sdk/blob/master/pom.xml#L95). And it's not the only library that is outdated. It would be great if the libraries are upgraded to a newer version.

This cause me a problem when I tried to use the SDK with micronaut framework. Micronaut also uses Netty, but newer version, and some classes in the old library was completely changed. But I cannot just replace one version with another, as both SDK and micronaut depends heavily on their implementation of Netty.

Example of a class where constructor has changed:
https://github.com/netty/netty/blob/4.0/transport/src/main/java/io/netty/channel/AbstractChannel.java#L85 vs https://github.com/netty/netty/blob/4.1/transport/src/main/java/io/netty/channel/AbstractChannel.java#L73

[ahmetoz]

cc: @jenschude @ashishhk

I also find out that when the other java versions > 8 are used it's triggering this warning, would be nice to have some update for this issue?
Any plan for managing dependencies for async-http-client ?

WARNING: An illegal reflective access operation has occurred

WARNING: Illegal reflective access by io.netty.util.internal.ReflectionUtil (file:/home/runner/.gradle/caches/modules-2/files-2.1/io.netty/netty-common/4.0.54.Final/f92dfcf8e3d0249510ab739573783b1d19e6b9dd/netty-common-4.0.54.Final.jar) to constructor java.nio.DirectByteBuffer(long,int)
WARNING: Please consider reporting this to the maintainers of io.netty.util.internal.ReflectionUtil
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release

Note: I resolved the issue like below by adding the latest version to the classpath.

implementation 'org.asynchttpclient:async-http-client:2.12.2'

[jenschude]

I see that there is a version defined for netty in the pom. But the value is never used. Only the AHC 2.0 client uses the specified version of netty internally.

What you can try is to use instead of commercetools-java-client the commercetools-java-client-ahc-2_5. This one uses a more recent version of netty

[ahmetoz]

I see, but most of the people are using commercetools-java-client directly and the main readme does not give details about commercetools-java-client-ahc-2_5.

Is there a reason to not upgrade the pom to the latest version?

commercetools-jvm-sdk/commercetools-java-client/pom.xml

Line 23 in efed3c2

<artifactId>commercetools-java-client-ahc-2_0</artifactId>

[jenschude]

Backwards compatibility of the SDK. As it may lead to incompatible configurations in existing projects. Also it's still possible that you could override the AsyncHttpClient client in your local config.

I just tried it:

<dependencies>
    <dependency>
        <groupId>com.commercetools.sdk.jvm.core</groupId>
        <artifactId>commercetools-models</artifactId>
        <version>1.58.0</version>
    </dependency>
    <dependency>
        <groupId>com.commercetools.sdk.jvm.core</groupId>
        <artifactId>commercetools-java-client-ahc-2_5</artifactId>
        <version>1.58.0</version>
        <exclusions>
            <exclusion>
                <groupId>org.asynchttpclient</groupId>
                <artifactId>async-http-client</artifactId>
            </exclusion>
        </exclusions>
    </dependency>
    <dependency>
        <groupId>org.asynchttpclient</groupId>
        <artifactId>async-http-client</artifactId>
        <version>2.12.2</version>
    </dependency>
</dependencies>

or in a gradle file:

dependencies {
    def jvmSdkVersion = "1.58.0"
    implementation "com.commercetools.sdk.jvm.core:commercetools-models:$jvmSdkVersion"
    implementation ("com.commercetools.sdk.jvm.core:commercetools-java-client-ahc-2_5:$jvmSdkVersion") {
        exclude group: "org.asynchttpclient"
    }
    implementation "org.asynchttpclient:async-http-client:2.12.2"
}

Beside that I had recently some issues while updating the jackson dependencies some other transient dependencies for the felix module had been updated and the integration tests for OSGi were running hours long and timing out by the CI instead of minutes.

[ahmetoz]

@jenschude
Thanks for the checks and the reason why a change is not done.

[ahmetoz]

Hi @jenschude

Changing the version caused some issues on our sync projects. On our end, some integration tests are failed on project-sync. We could not find the root cause but the assumption is there might be some changes that have a different implementation on the ahc 2.5.

Here the usages when it's running with ahc 2.0

Here the usages when it's running with ahc 2.5

TLDR; In the new version of ahc 2.5 we faced errors as below:

Note: the issue is flaky and happening in different tests but with a correlation with the client.

java.util.concurrent.CompletionException: io.sphere.sdk.http.HttpException: The underlying HTTP client detected a problem.
 Caused by: io.sphere.sdk.http.HttpException: The underlying HTTP client detected a problem.
 Caused by: java.lang.OutOfMemoryError: Java heap space

[jenschude]

I just compared the files for the 2_0 and 2_5 module is the naming and the different version dependencies.

Everything else is the same. So it's really hard to say anything here what could be the culprit. When you could pinpoint it to a code point in the SDK it would help. Else we can not provide support for 3rd party software.

[ahmetoz]

Thanks for checking it. from our side, we figured out that after the AHC version > 2_0 the library reacts differently, especially on the client management side because it has some changes based on pooling. We were recreating the client on integration test within java try with resources, now I changed it to the shared client between tests as it is not creating more thread pools and sharing the threads between tests to avoid resource leaks, with that there is no issue on the heap sizes, idling threads or on resource leak side.

Besides that, it would be nice to update/maintain outdated dependencies in SDK (not really sure what is the backward compatibility issue here), because it poses a potential security risk (reported also in snyk tool) mainly on the io.netty dependencies which AHC client uses.