Remove guava dependency

ccoltx commented 2 years ago

Issue Description

As far as I can tell, the SDK odata-core does not really require guava. I found a single annotation being used import com.google.common.annotations.Beta;. IMO, that doesn't warrant a dependency on a very "controversial" library.

The s4hana-connectivity module generates code which uses guava, but that should be easily replaced with standard JDK usage.

Important information:

Excluding the transitive depenedency doesn't seem to have any side-effects on odata-core.

<dependency>
            <groupId>com.sap.cloud.sdk.datamodel</groupId>
            <artifactId>odata-core</artifactId>
            <exclusions>
                <exclusion>
                    <groupId>com.google.guava</groupId>
                    <artifactId>guava</artifactId>
                </exclusion>
            </exclusions>
        </dependency>

Impact / Priority

It's not uncommon to have version conflicts with guava, security findings, etc.

If you can reduce the number of unnecessary dependencies, you are helping your users maintain their code easier.

Thank you

Error Message

NA

Project Details

SAP Cloud SDK Version: 3.66.0

com.sap.cloud.sdk.datamodel odata-core com.google.guava guava

Checklist

[x] Checked out the documentation and Stack Overflow
[x] Description provided with all relevant information
[ ] Attached debug logs
[ ] Attached dependency tree
[x] Provided the SAP Cloud SDK version & link to relevant source code

newtork commented 2 years ago

Hello @ccoltx,

Thanks for your suggestion! Can you please motivate your statement guava is "very controversial"?

We will be looking into replacing Guava code with vanilla Java API, but to my experience this degrades code style and readability. Especially since we are currently leveraging lazy collections, which is a pain to implement manually.

If you can reduce the number of unnecessary dependencies, you are helping your users maintain their code easier.

That's one of our primary goals for the upcoming next major release. Your concerns are very valuable to us. It's just that we never had any complains for "Guava". Especially since their API remained stable throughout many releases for us. We are discussing removal / replacement / shading for all third-party dependency candidates.

Edit: For completeness, a colleague responded with one code-smell for guava: We currently need to manually exclude transitive checkerframework and errorprone dependencies. While I agree here, I don't see how this affects our consumers.

Thanks and kind regards Aleaxnder

MatKuhr commented 1 year ago

While we did remove quite some dependencies in the mean time, making the Cloud SDK more lightweight, guava is still in. This is, in part, due to the fact that we are using guavas @Beta annotations.

SAP / cloud-sdk