[java] increasing of properties scope for better appium compatibility

iampopovich commented 5 days ago

User description

Thanks for contributing to Selenium! A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines. Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

according to request in #13949 scopes of properties were increased here's a part of request

Increase the scope of private properties of the below classes to 'protected':
RemoteWebDriver -> capabilities

HttpCommandExecutor -> commandCodec

HttpCommandExecutor -> responseCodec

FluentWait -> clock
FluentWait -> timeout
FluentWait -> interval
FluentWait -> sleeper
FluentWait -> ignoredExceptions
FluentWait -> messageSupplier
FluentWait -> input

Make public accessor for the property:
HttpCommandExecutor -> client

DriverService.Builder -> exe

Motivation and Context

Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist

[x] I have read the contributing document.
[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.
[ ] I have added tests to cover my changes.
[x] All new and existing tests passed.

PR Type

Enhancement

Description

Changed the access level of several fields in HttpCommandExecutor to improve compatibility with Appium:
- client field changed to public.
- commandCodec and responseCodec fields changed to protected.
Changed the access level of the capabilities field in RemoteWebDriver to protected.
Changed the access level of the exe field in DriverService.Builder to public.
Changed the access level of multiple fields in FluentWait to protected:
- input, clock, sleeper, timeout, interval, messageSupplier, ignoredExceptions.

Changes walkthrough 📝

Relevant files

Enhancement

HttpCommandExecutor.java `Modify access levels for HttpCommandExecutor fields` java/src/org/openqa/selenium/remote/HttpCommandExecutor.java Changed `client` field to public. Changed `commandCodec` and `responseCodec` fields to protected.	+3/-3
RemoteWebDriver.java `Modify access level for RemoteWebDriver capabilities field` java/src/org/openqa/selenium/remote/RemoteWebDriver.java - Changed `capabilities` field to protected.	+1/-1
DriverService.java `Modify access level for DriverService.Builder exe field` java/src/org/openqa/selenium/remote/service/DriverService.java - Changed `exe` field to public.	+1/-1
FluentWait.java `Modify access levels for FluentWait fields` java/src/org/openqa/selenium/support/ui/FluentWait.java Changed multiple fields to protected: `input`, `clock`, `sleeper`, `timeout`, `interval`, `messageSupplier`, `ignoredExceptions`.	+7/-7

💡 PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

codiumai-pr-agent-pro[bot] commented 5 days ago

PR Reviewer Guide 🔍

⏱️ Estimated effort to review [1-5]	2
🧪 Relevant tests	No
🔒 Security concerns	No
⚡ Key issues to review	Access Level Changes: The PR changes the access levels of several fields in multiple classes. This could potentially expose internal implementation details and should be carefully reviewed to ensure that it does not violate encapsulation principles or introduce unintended side effects.

codiumai-pr-agent-pro[bot] commented 5 days ago

PR Code Suggestions ✨

Category	Suggestion	Score
Best practice	Change the access modifier of the `client` field from `public` to `protected` to improve encapsulation ___ Consider making the `client` field `protected` instead of `public` to restrict access to subclasses only, maintaining encapsulation and reducing the risk of unintended modifications. [java/src/org/openqa/selenium/remote/HttpCommandExecutor.java [47]](https://github.com/SeleniumHQ/selenium/pull/14183/files#diff-6482ca0b119c8bbec67761a22020a878117f6d9e558c25bba94837a6bd74bcf9R47-R47) ```diff -public final HttpClient client; +protected final HttpClient client; ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 8 Why: Changing the access modifier from `public` to `protected` for the `client` field enhances encapsulation and limits access to subclasses, which is a significant improvement in design.	8
Best practice	Make the `capabilities` field `final` to prevent it from being reassigned after initialization ___ Consider making the `capabilities` field `final` to indicate that it should not be reassigned after being initialized, which can help prevent bugs. [java/src/org/openqa/selenium/remote/RemoteWebDriver.java [117]](https://github.com/SeleniumHQ/selenium/pull/14183/files#diff-0cf141b01e48a733ec9037c51d8e610e7727d05a6b79aa0176def7c3a0fc311bR117-R117) ```diff -protected Capabilities capabilities; +protected final Capabilities capabilities; ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 7 Why: Making the `capabilities` field `final` is a good practice to ensure it is not modified after initialization, which can help in maintaining immutability and preventing bugs.	7
Enhancement	Initialize the `messageSupplier` field with a more meaningful default message ___ Consider initializing the `messageSupplier` field with a more meaningful default message to improve debugging and error reporting. [java/src/org/openqa/selenium/support/ui/FluentWait.java [75]](https://github.com/SeleniumHQ/selenium/pull/14183/files#diff-e33f54b8999af8e818a011f469e72eee95a3eb2b9bc69ffc5d496bb0da372838R75-R75) ```diff -protected Supplier messageSupplier = () -> null; +protected Supplier messageSupplier = () -> "Condition not met within timeout"; ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 6 Why: Providing a more meaningful default message for `messageSupplier` can enhance debugging and error reporting, which improves the usability of the class.	6

SeleniumHQ / selenium