[bidi][java] Deprecate using builder for Locate Node parameters.

[pujagani]

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

Deprecated using Builder and added chaining methods.

Motivation and Context

Ensure consistency across BiDi parameters usage.

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

• 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.
• All new and existing tests passed.

---

Type

enhancement

---

Description

• Deprecated the use of Builder for constructing LocateNodeParameters and introduced chaining methods for setting optional parameters directly.
• Updated all instances in tests to utilize the new chaining methods, simplifying the instantiation process.
• This change aims to ensure consistency across BiDi parameters usage and improve the developer experience.

---

Changes walkthrough

	Relevant files
Enhancement	LocateNodeParameters.java Deprecate Builder and Introduce Chaining Methods for LocateNodeParameters java/src/org/openqa/selenium/bidi/browsingcontext/LocateNodeParameters.java Deprecated the Builder class for constructing LocateNodeParameters. Introduced chaining methods for setting optional parameters directly on LocateNodeParameters instances. Changed fields from final to non-final to support the new chaining methods. +40/-5
Tests	LocateNodesTest.java Update Tests to Use New Chaining Methods for LocateNodeParameters java/test/org/openqa/selenium/bidi/browsingcontext/LocateNodesTest.java Updated tests to use the new chaining methods instead of the deprecated Builder for LocateNodeParameters. Simplified the instantiation of LocateNodeParameters in tests. +10/-24

---

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

[qodo-merge-pro]

PR Description updated to latest commit (a60efb2)

• Copy walkthrough table to "Files Changed" Tab

[qodo-merge-pro]

PR Review

⏱️ Estimated effort to review [1-5]	2, because the changes are straightforward and localized to a specific functionality within the codebase. The modifications involve deprecating the use of a builder pattern and introducing method chaining for setting parameters, which is a common pattern in Java development. The changes are well-contained within the LocateNodeParameters class and its usage in tests, making it easier to review.
🧪 Relevant tests	Yes
🔍 Possible issues	Missing Validation: The new setters in LocateNodeParameters do not perform any validation on the input parameters. It might be beneficial to add null checks or other validations to ensure the integrity of the LocateNodeParameters instances.
	Deprecation Communication: While the Builder class is marked as deprecated, it might be helpful to include more information on the deprecation, such as recommended steps for migration or a link to documentation, in the deprecation message for better developer experience.
🔒 Security concerns	No

---

✨ Review tool usage guide:

---

Overview:
The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

• When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:

/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

• With a configuration file, use the following template:

[pr_reviewer]
some_config1=...
some_config2=...

See the review usage page for a comprehensive guide on using this tool.

[qodo-merge-pro]

PR Code Suggestions

Category	Suggestions
Enhancement	Add method overloads to accept Optional types for setter methods. Consider using method overloading for setMaxNodeCount, setOwnership, setSandbox, setSerializationOptions, and setStartNodes to accept both the current types and Optional types. This would provide flexibility for callers who might already have Optional instances of these parameters. java/src/org/openqa/selenium/bidi/browsingcontext/LocateNodeParameters.java [51-72] public LocateNodeParameters setMaxNodeCount(long maxNodeCount) { this.maxNodeCount = Optional.of(maxNodeCount); return this; } +public LocateNodeParameters setMaxNodeCount(Optional<Long> maxNodeCount) { + this.maxNodeCount = maxNodeCount.orElse(Optional.empty()); + return this; +} +
	Rename setter methods to follow the JavaBeans naming convention for clarity. To make the API more intuitive, consider renaming the setter methods to follow the JavaBeans naming convention (e.g., withMaxNodeCount instead of setMaxNodeCount). This can make it clearer that these methods are intended for chaining rather than traditional property setting. java/src/org/openqa/selenium/bidi/browsingcontext/LocateNodeParameters.java [51-53] -public LocateNodeParameters setMaxNodeCount(long maxNodeCount) { +public LocateNodeParameters withMaxNodeCount(long maxNodeCount) { this.maxNodeCount = Optional.of(maxNodeCount); return this; }
Bug	Initialize all optional fields in the constructor to avoid potential NullPointerExceptions. For the LocateNodeParameters constructor that takes a Locator parameter, initialize all optional fields to Optional.empty() to ensure consistency with the builder pattern and avoid potential NullPointerExceptions when accessing these fields. java/src/org/openqa/selenium/bidi/browsingcontext/LocateNodeParameters.java [47-49] public LocateNodeParameters(Locator locator) { this.locator = locator; + this.maxNodeCount = Optional.empty(); + this.ownership = Optional.empty(); + this.sandbox = Optional.empty(); + this.serializationOptions = Optional.empty(); + this.startNodes = Optional.empty(); }
Best practice	Validate input parameters in setter methods to ensure robustness. Instead of directly returning this in the setter methods, consider validating the input parameters to ensure they meet any expected constraints (e.g., non-negative values for maxNodeCount). This can prevent invalid states and make the API more robust. java/src/org/openqa/selenium/bidi/browsingcontext/LocateNodeParameters.java [51-53] public LocateNodeParameters setMaxNodeCount(long maxNodeCount) { + if (maxNodeCount < 0) { + throw new IllegalArgumentException("maxNodeCount must be non-negative"); + } this.maxNodeCount = Optional.of(maxNodeCount); return this; }
Maintainability	Make LocateNodeParameters a final class to prevent unintended subclassing. To ensure that the LocateNodeParameters class is easily extensible in the future, consider making it a final class or providing a clear extension policy. This can help prevent unintended subclassing which could complicate future API changes. java/src/org/openqa/selenium/bidi/browsingcontext/LocateNodeParameters.java [29] -public class LocateNodeParameters { +public final class LocateNodeParameters {

---

✨ Improve tool usage guide:

---

Overview:
The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

• When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:

/improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...

• With a configuration file, use the following template:

[pr_code_suggestions]
some_config1=...
some_config2=...

See the improve usage page for a comprehensive guide on using this tool.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.63%. Comparing base (37565af) to head (3cd9cd2).
Report is 1 commits behind head on trunk.

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

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff           @@
##            trunk   #13767   +/-   ##
=======================================
  Coverage   58.63%   58.63%           
=======================================
  Files          86       86           
  Lines        5272     5272           
  Branches      218      218           
=======================================
  Hits         3091     3091           
  Misses       1963     1963           
  Partials      218      218           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.