[🚀 Feature]: remove guava from client bindings

[joerg1985]

Feature and motivation

When using Java 11 we should not need the guava library in the client bindings not realy, we can do some small changes to remove them. Removing them from the dependencies should be done with Selenium 5, as client code could relay on it on the class path. We might be able to remove it from the server too, but this must be evaluated and could be done later.

Usage example

Less dependencies are in general better.

[github-actions]

@joerg1985, thank you for creating this issue. We will troubleshoot it as soon as we can.

---

Info for maintainers

Triage this issue by using labels.

If information is missing, add a helpful comment and then I-issue-template label.

If the issue is a question, add the I-question label.

If the issue is valid but there is no time to troubleshoot it, consider adding the help wanted label.

If the issue requires changes or fixes from an external project (e.g., ChromeDriver, GeckoDriver, MSEdgeDriver, W3C), add the applicable G-* label, and it will provide the correct link and auto-close the issue.

After troubleshooting the issue, please add the R-awaiting answer label.

Thank you!

[titusfortner]

Why would we wait for Selenium 5? Once we require Java 11, if we can update syntax and methods and dependencies we should just do it.

[diemol]

I understand the reasoning, a user might be relying on Selenium providing Guava as a dependency. But I think we can tell them what to do in the release notes.

[joerg1985]

Okay, this will be a breaking change:

selenium/java/src/org/openqa/selenium/support/ui/ExpectedCondition.java

Lines 34 to 36 in e518273

	// NB: this originally extended Guava's Function interface since Java didn't have one. To avoid code
	// such as "com.google.common.base.Function condition = ExpectedConditions.elementFound(By);"
	// breaking at compile time, we continue to extend Guava's Function interface.

[titusfortner]

Do we allow the native Java interface, or would we need to create a separate implementation for it. I'm hoping to pull it out of core Selenium code entirely for Selenium 5, so maybe we don't need to touch it and can spin it off at that time.

[joerg1985]

It should be working fine with the native java interface.

[joerg1985]

I just pushed a commit (42cc355) to remove the usage of guava from the browser bindings.

These are left areas are left:

• devtools -> [java] removed use of guava from devtools #12943
• bidi -> same changes as in the dev tools connection, i will wait for the devtools PR to get merged
• support -> open
• remote -> open
• os -> only referenced in build file without usage

[titusfortner]

@joerg1985 I'm going to remove the milestone on this one since there's no need for a specific timeframe on it. Are you planning to continue working on this? Thanks.

[diemol]

Having a single issue for this is helpful but not so actionable.

@joerg1985, if you have identified where we can make code changes, I suggest creating an issue for each place you found and adding instructions for the implementation details. With that, anyone could jump in and help. Then, closing this issue.

[joerg1985]

Yes i have planned to work on this as soon as possible. I think i will continue after christmas. I will split this in multiple tasks the next days.

[joerg1985]

Okay, i had a look and there is not much left, but there is one question how to remove the use of com.google.common.net.HttpHeaders and com.google.common.net.MediaType before i could split the issue or further implement it.

I would suggest to create two enums to replace them org.openqa.selenium.remote.http.HttpHeader and org.openqa.selenium.remote.http.MediaType. @diemol @titusfortner are you okay with that?

[titusfortner]

I'm pretty much ok with anything that doesn't require users to change their code. 😂

[joerg1985]

I just created the less-guava branch which removes all easy to replace guava uses.
I would merge this to main, after the release of 4.16.0, if the new class org.openqa.selenium.remote.http.HttpHeader
is okay.
After this is merged it should be easier to split this ticket, as there are much less areas left.

[diemol]

I just created the less-guava branch which removes all easy to replace guava uses. I would merge this to main, after the release of 4.16.0, if the new class org.openqa.selenium.remote.http.HttpHeader is okay. After this is merged it should be easier to split this ticket, as there are much less areas left.

I don't have a strong opinion. I believe we can have those enums, and hopefully documenting in the code why we have them. Thanks!

[joerg1985]

@diemol @titusfortner i have just pushed a commit to replace most of the usages, these tasks are left:

• update org.openqa.selenium.support.ui.ExpectedCondition, this is a breaking change so i did not do this
• update org.openqa.selenium.support.ui.FluentWait (i missed this one, just a single use of Throwables.throwIfUnchecked)
• update org.openqa.selenium.remote.http.Contents, replace the FileBackedOutputStream
• update org.openqa.selenium.remote.NewSessionPayload, replace the FileBackedOutputStream
• update org.openqa.selenium.remote.ProtocolHandshake, replace the FileBackedOutputStream and the CountingOutputStream

Have to create the tickets to replace these usages the next days.

[joerg1985]

Ok i was not able to describe all changes to remove the FileBackedOutputStream and CountingOutputStream, as this is not that trivial and i had to check a lot of things if it is possible to remove it.

So i ended in creating a PR with it, sorry for this ;)

[joerg1985]

Use of guava in org.openqa.selenium.support.ui.FluentWait has been removed.

[titusfortner]

Hey @joerg1985 what is the status?

@diemol should we need to announce that we will be removing guava support for a specific release? Not sure the right way to communicate this.

[diemol]

I don't think so, we should not be exposing Guava functionality.

[joerg1985]

@titusfortner still waiting for the review of #13308 by @shs96c

After the PR the is merged there is only one use of guava left (the breaking change in org.openqa.selenium.support.ui.ExpectedCondition).

[titusfortner]

Alternative option... We don't import Support package by default with selenium-java and keep the guava dependency in that package? Or is that a Selenium 5 level change? :)

[titusfortner]

I don't think so, we should not be exposing Guava functionality.

Yes, but if someone is using Selenium they have guava as a transitive dependency. They may be using it without having an explicit dependency, so updating Selenium will cause their code to break. I don't know if that should be called out ahead of time, or just part of the release notes.

[krmahadevan]

In my personal opinion transitive dependencies should not be something that end users should rely on because they are being brought in as transitive dependencies to solve some programming ask by selenium. Selenium would be free to make any changes to how those functionalities are delivered and in the process it should also be able to add/remove any dependencies that it needs.

An end user project that requires guava should explicitly depend on it and not rely on transitive dependencies (Which is always prone to issues especially if one has multiple libraries that bring in multiple versions of the same library, and thus causing dependency conflicts)

[joerg1985]

@diemol @titusfortner Now is only one use of guava left is the breaking change in org.openqa.selenium.support.ui.ExpectedCondition would be greate to have this completed in 4.19.0.

[diemol]

Is there a PR for it?

[joerg1985]

No, i did not know how to proper document this breaking change in the release notes.
It is just removing the guava interface from the selenium interface and remove the guava from the bazel file.

[diemol]

I remember now. We need to write a blog post about it, announce it, and then make the change because we cannot just deprecate the interface or do something similar.

[diemol]

In the end, this helps users because there have been countless issues of people reporting dependency conflicts.