[java] add deprecation notices to Grid logs and Java stdout

[titusfortner]

Update

I had to change the code quite a bit because what Java was building and what the Grid was receiving turned out to be more different than I realized. So most of the discussion in this initial post is out of date.

---

As discussed in TLC meeting today, to address #10374

Note: this is in DRAFT until we add the documentation (and update the URL as desired) and write the blog post.

This is what the output looks like: https://gist.github.com/titusfortner/f75a3bff9b1b9946687ae8069f112f7f
4 conditions:

1. Client sends Options with only w3c syntax (Se 4 Ruby/.NET/JS) — No warnings
2. Client sends Options with a valid handshake with both W3C & OSS and driver supports w3c (Se 3; Se 4 Java) — No warnings
3. Client sends Options a valid handshake with both W3C & OSS, but driver only supports or is set to use OSS — Warning that OSS mode deprecated
4. Client sends only OSS Capabilities (Se 2 or DesiredCapabilities class) — Warning to use W3C Capabilities

One issue is for use case 2, if the user is working with Selenium 3.0 - 3.8.1 (handshake, but not fully w3c compliant), Selenium reports w3c mode, but will attempt to convert any commands that aren't w3c compliant. So these people might likely wouldn't get warnings, but would have code stop working after we remove deprecations. The bright side is that only 6% of downloads are in this range for Java, and the likelihood that these people have not updated their client version, but *have updated their Grid version is likely an extremely small cross-section

[codecov-commenter]

Codecov Report

Merging #10408 (b33b53c) into trunk (1877b9d) will not change coverage.
The diff coverage is n/a.

❗ Current head b33b53c differs from pull request most recent head a14aae3. Consider uploading reports for the commit a14aae3 to get more accurate results

@@           Coverage Diff           @@
##            trunk   #10408   +/-   ##
=======================================
  Coverage   46.30%   46.30%           
=======================================
  Files          86       86           
  Lines        5799     5799           
  Branches      277      277           
=======================================
  Hits         2685     2685           
  Misses       2837     2837           
  Partials      277      277           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7e541ed...a14aae3. Read the comment docs.

[pujagani]

Thank you @titusfortner! Using parameterized logging is better performance-wise rather than string concatenation. I made a quick fix for that and now SonarLint should be happy.
Appreciate the effort, especially the data-backed details in the pull request template. The changes look good to me 👍

[sonarqubecloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

0.0% Coverage
0.0% Duplication

[titusfortner]

Well... This code isn't right :(
I built the grid from this PR, and this is what I'm getting:

        MutableCapabilities caps = new MutableCapabilities();
        caps.setCapability("browserName", "firefox");
        RemoteWebDriver driver = new RemoteWebDriver(caps);

INFO: Detected dialect: W3C
May 20, 2022 2:30:31 PM org.openqa.selenium.remote.ProtocolHandshake createSession
WARNING: Support for Legacy Capabilities is deprecated; Please update to W3C Syntax: https://www.selenium.dev/blog/2022/legacy-protocol-support/

[titusfortner]

So, this code is weird.
NewSessionPayload always sets desiredCapabilities, regardless

https://github.com/SeleniumHQ/selenium/blob/702b3aa/java/src/org/openqa/selenium/remote/NewSessionPayload.java#L64-L67

So, passing in an Options instance will always add OSS to the list of dialogs, and it adds W3C based on whether you have firstMatch/alwaysMatch, which I think means it only happens when using the WebDriverBuilder
So my idea of calling payload.getDownstreamDialects() and checking if OSS is in there, is absolutely useless.

Fortunately, command includes both capabilities & desiredCapabilities, so I think the answer is to get the keys from each, and if there are any of keys in desiredCapabilities that aren't in capabilities, throw the warning.
We can even list which keys aren't valid in the message.

This should work for the scenario where the Java bindings are sending invalid capabilities, but I need to run a bunch more tests to make sure.

I'm not certain how this applies to the scenario where something is sending commands to the grid, yet.

[titusfortner]

I tested this a number of ways including building the grid with this code and testing it with Selenium 3 Ruby & Python bindings.

This should do exactly what we want for local Java client code which always goes through this loop in an expected way. It will log results to console.

For grid it is trickier because the messages are in the grid logs not stdout. Also, because of grid weirdness (https://gist.github.com/titusfortner/1956ca0e863b5fa7baff9a61ebbe135b), it won't always log a problem, even when the user is doing the wrong thing.

Either way, I think this is the best we're going to get as far as warning users of the change, and the weirdness will get fixed when we start removing the extra stuff.

[titusfortner]

I moved this back to draft so we can figure out if we can log issues when people use setCapability() with non-w3c capabilities. It likely will be supper chatty, so if we can also figure out how to silence it, people might appreciate that.

[sonarqubecloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

0.0% Coverage
0.0% Duplication

[diemol]

Implemented via a297017