grid/TomlConfig: migrate TOML library to tomlj/tomlj

[Delta456]

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

This PR uses a new TOML parser as stated in https://github.com/SeleniumHQ/selenium/issues/13538

Motivation and Context

As this TOML library is more maintained than the one used so lesser chances of problems and vulnerabilities.

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [x] 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.
• [ ] All new and existing tests passed.

---

PR Type

enhancement, dependencies, tests

---

Description

• Migrated the TOML parsing library from jtoml to tomlj in the TomlConfig class to ensure better maintenance and reduce vulnerabilities.
• Implemented error handling for TOML parsing errors using the tomlj library.
• Updated tests to validate the new error handling and ensure compatibility with the tomlj library.
• Adjusted Bazel and Maven configurations to replace jtoml with tomlj.

---

Changes walkthrough 📝

	Relevant files
Enhancement	TomlConfig.java Migrate TOML parsing from `jtoml` to `tomlj`                          java/src/org/openqa/selenium/grid/config/TomlConfig.java Migrated from jtoml to tomlj for TOML parsing. Added error handling for TOML parsing errors. Updated methods to use tomlj API. +29/-13
Tests	TomlConfigTest.java Update tests for new TOML library `tomlj`                                java/test/org/openqa/selenium/grid/config/TomlConfigTest.java Added test for error handling in TOML parsing. Updated existing tests to accommodate new TOML library. +11/-4
Dependencies	MODULE.bazel Update Bazel dependencies for TOML library migration          MODULE.bazel Removed jtoml dependency. Added tomlj dependency. +1/-1      maven_install.json Update Maven artifacts for TOML library migration                java/maven_install.json Removed jtoml artifact. Added tomlj artifact. +37/-15  BUILD.bazel Update Bazel build file for TOML library change                    java/src/org/openqa/selenium/grid/config/BUILD.bazel Replaced jtoml with tomlj in dependencies. +1/-1      BUILD.bazel Update test dependencies for TOML library migration            java/test/org/openqa/selenium/grid/config/BUILD.bazel Removed jtoml from test dependencies. +0/-1

---

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

[Delta456]

As this TOML library uses a different approach compared to the old one. I would require help from the core maintainers to pass the tests and may have to change the tests and add more accordingly.

[qodo-code-review[bot]]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Key issues to review Error Handling The new implementation throws a ConfigException with all parsing errors concatenated. Consider handling each error separately or providing more context. Possible Performance Issue The getAll method now performs multiple checks and conversions. Consider optimizing for frequently accessed configurations.

[qodo-code-review[bot]]

PR Code Suggestions ✨

Category	Suggestion	Score
Best practice	Use try-with-resources to automatically close the reader Consider using a try-with-resources statement to ensure that the reader is properly closed after use. java/src/org/openqa/selenium/grid/config/TomlConfig.java [42-44] public TomlConfig(Reader reader) { - try { - toml = Toml.parse(reader); + try (Reader r = reader) { + toml = Toml.parse(r); [ ] Apply this suggestion Suggestion importance[1-10]: 8 Why: Using try-with-resources is a best practice for managing resources like readers, ensuring they are closed properly, which can prevent resource leaks.	8
	Use a null-safe method for converting values to strings Consider using a null-safe method like Objects.toString() instead of String.valueOf() to handle potential null values. java/src/org/openqa/selenium/grid/config/TomlConfig.java [125] -return Optional.of(List.of(String.valueOf(value))); +return Optional.of(List.of(Objects.toString(value))); [ ] Apply this suggestion Suggestion importance[1-10]: 6 Why: Using Objects.toString() is a null-safe way to convert objects to strings, which can prevent potential NullPointerExceptions.	6
Enhancement	Use a more specific exception type for TOML parsing errors Consider using a more specific exception type for TOML parsing errors instead of the generic ConfigException. java/src/org/openqa/selenium/grid/config/TomlConfig.java [46-51] if (toml.hasErrors()) { String error = toml.errors().stream().map(TomlParseError::toString).collect(Collectors.joining("\n")); - throw new ConfigException(error); + throw new TomlParseException(error); } [ ] Apply this suggestion Suggestion importance[1-10]: 7 Why: Using a more specific exception type like TomlParseException can improve error handling by providing more detailed information about the nature of the error.	7
	Add more specific assertions for error handling tests Consider adding more specific assertions to check the exact error message or exception details in the error handling test. java/test/org/openqa/selenium/grid/config/TomlConfigTest.java [42-43] assertThatThrownBy(() -> new TomlConfig(new StringReader(raw))) - .isInstanceOf(ConfigException.class); + .isInstanceOf(ConfigException.class) + .hasMessageContaining("bare keys cannot contain"); [ ] Apply this suggestion Suggestion importance[1-10]: 7 Why: Adding specific assertions for error messages can improve test robustness by ensuring that the correct exceptions are thrown with expected messages.	7

[pujagani]

Updating the TOML parser will lead to a breaking change for users.

Example: With the current TOML parser:

// This throws errors with the new parser tomlj
String raw = "[cheeses]\nselected=brie";
Config config = new TomlConfig(new StringReader(raw))

After the new TOML parser:

String raw = "[cheeses]\nselected=\"brie\"";
 Config config = new TomlConfig(new StringReader(raw));

The new parser requires quotes for the string values. Since our maven Grid package provides access to creating TomlConfig objects, it will be a breaking change for users.

We can add a warning with the current code base about the upcoming change, and maybe merge this PR after 2 versions so the end users get a chance to update the TOML.

@diemol What do you think?

[Delta456]

@pujagani I believe we should go with giving a warning for this as it is a breaking change. As said let's merge and introduce this change after next 2 versions 🚀

[pujagani]

@Delta456 Would like to create a separate PR displaying the warning?

[Delta456]

@Delta456 Would like to create a separate PR displaying the warning?

Sure

[Delta456]

https://github.com/SeleniumHQ/selenium/pull/14491 has been merged. Thanks @pujagani!

[pujagani]

Please resolve the conflicts when time permits. Thank you!

[pujagani]

@Delta456 Please help resolve the merge conflicts when time permits. Thank you!

[Delta456]

@Delta456 Please help resolve the merge conflicts when time permits. Thank you!

I can only fix the merge conflicts when the #14711 PR is merged

[Delta456]

@pujagani should be done now.