[json] Allow constructor args to be used when coercing from json

[shs96c]

No description provided.

[qodo-code-review]

PR Summary by Qodo

Enable JSON deserialization via constructor parameter names (-parameters)

✨ Enhancement 🧪 Tests ⚙️ Configuration changes 🕐 40+ Minutes

AI Description

• Add JSON coercion via constructors using runtime parameter names (javac -parameters).
• Add Optional and Collection coercion support to match constructor-based deserialization.
• Refactor BiDi/Grid model types to rely on constructor coercion, removing hand-written fromJson
 parsers.

Diagram

graph TD
  A["Bazel java_library.bzl"] --> B["javac -parameters"] --> C["JsonTypeCoercer"]
  C --> D["ConstructorCoercer"] --> F["Model constructors"]
  C --> E["OptionalCoercer"]
  H["ConstructorCoercerTest"] --> C

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Annotate constructor params for JSON names (explicit mapping)

• ➕ Does not require compiling with -parameters
• ➕ Supports renames without breaking JSON compatibility
• ➕ Allows aliases / versioned field names
• ➖ Requires adding/maintaining an annotation + reflection logic
• ➖ More verbosity in model classes than relying on parameter names

2. Keep/standardize static fromJson(JsonInput) parsers

• ➕ Maximum control over validation and backwards compatibility
• ➕ No reflection-based constructor selection ambiguity
• ➖ High boilerplate and easy-to-diverge parsing behavior
• ➖ Duplicated mapping logic across many types (harder to maintain)

Recommendation: The constructor-parameter approach is a good tradeoff for Selenium’s many immutable-ish DTOs, and the PR correctly backs it with Bazel -parameters to make it reliable. The main risk is accidental parameter renames breaking JSON mapping; mitigate by documenting the contract (done) and favoring stable parameter names for wire-facing models.

Files changed (28) +724 / -757

Enhancement (4) +309 / -2

BeforeRequestSent.javaUse JsonInput.readNonNull for Initiator construction +1/-1

Use JsonInput.readNonNull for Initiator construction

• Updates parsing to read Initiator via JsonInput.readNonNull(Initiator.class) instead of Initiator.fromJson. This exercises the new constructor-based coercion path for Initiator.

java/src/org/openqa/selenium/bidi/network/BeforeRequestSent.java

ConstructorCoercer.javaAdd constructor-based JSON coercer using runtime parameter names +229/-0

Add constructor-based JSON coercer using runtime parameter names

• Introduces a new TypeCoercer that selects a non-default constructor whose parameter names match JSON fields, prefers the longest matching overload, and treats Optional parameters as optional. Implements detailed error reporting for missing required fields, null required values, and ambiguous overload matches.

java/src/org/openqa/selenium/json/ConstructorCoercer.java

JsonTypeCoercer.javaRegister constructor/Optional coercers and relax null handling for Optional +21/-1

Register constructor/Optional coercers and relax null handling for Optional

• Adds OptionalCoercer and ConstructorCoercer to the coercer registry and supports Collection as a generic collection target. Updates null-handling so JSON null can be coerced into Optional without throwing.

java/src/org/openqa/selenium/json/JsonTypeCoercer.java

OptionalCoercer.javaAdd first-class Optional<T> JSON coercion +58/-0

Add first-class Optional<T> JSON coercion

• Introduces a dedicated coercer for Optional, mapping JSON null to Optional.empty and otherwise coercing the underlying generic type. Supports both raw Optional and parameterized Optional<T>.

java/src/org/openqa/selenium/json/OptionalCoercer.java

Refactor (19) +43 / -739

BiDiSessionStatus.javaRemove manual fromJson and rely on constructor coercion +0/-27

Remove manual fromJson and rely on constructor coercion

• Deletes the hand-written fromJson(JsonInput) parser. The type is now expected to be deserialized via the JSON coercion infrastructure (constructor-based or other existing coercers).

java/src/org/openqa/selenium/bidi/BiDiSessionStatus.java

HistoryUpdated.javaAlign constructor parameter names with BiDi JSON fields +5/-39

Align constructor parameter names with BiDi JSON fields

• Removes the dedicated fromJson(JsonInput) method and adjusts the constructor signature to use the JSON field name (context). Adds validation in the constructor to preserve prior behavior.

java/src/org/openqa/selenium/bidi/browsingcontext/HistoryUpdated.java

UserPromptClosed.javaUse constructor parameter names for JSON mapping (context) +3/-38

Use constructor parameter names for JSON mapping (context)

• Removes the manual fromJson(JsonInput) method and updates the constructor to accept the JSON field name (context). Keeps fields immutable and relies on JSON constructor coercion for parsing.

java/src/org/openqa/selenium/bidi/browsingcontext/UserPromptClosed.java

StackFrame.javaRename StackFrame constructor args to match JSON field names +4/-44

Rename StackFrame constructor args to match JSON field names

• Removes the fromJson(JsonInput) parser and updates constructor parameter names to url/functionName for JSON matching. Retains non-negative validation for numeric fields.

java/src/org/openqa/selenium/bidi/log/StackFrame.java

AuthChallenge.javaRemove custom JSON parsing for AuthChallenge +0/-25

Remove custom JSON parsing for AuthChallenge

• Deletes the fromJson(JsonInput) parser and related imports. AuthChallenge is now expected to be deserialized through the shared JSON coercion mechanisms.

java/src/org/openqa/selenium/bidi/network/AuthChallenge.java

FetchTimingInfo.javaRemove manual fromJson parsing for FetchTimingInfo +0/-96

Remove manual fromJson parsing for FetchTimingInfo

• Deletes the explicit fromJson(JsonInput) method and associated imports. FetchTimingInfo is now expected to be deserialized via constructor/instance coercion rather than bespoke parsing.

java/src/org/openqa/selenium/bidi/network/FetchTimingInfo.java

Header.javaRemove Header.fromJson and rely on standard coercion +0/-25

Remove Header.fromJson and rely on standard coercion

• Removes the custom fromJson(JsonInput) implementation and related imports. The object should now be constructed via JsonTypeCoercer pathways.

java/src/org/openqa/selenium/bidi/network/Header.java

Initiator.javaRemove Initiator.fromJson to enable constructor-based deserialization +0/-39

Remove Initiator.fromJson to enable constructor-based deserialization

• Deletes the manual parsing routine and its Require/JsonInput dependencies. Initiator is now created through the JSON coercer (notably ConstructorCoercer), including Optional fields.

java/src/org/openqa/selenium/bidi/network/Initiator.java

Message.javaDrop Message.fromJson in favor of constructor coercion +0/-36

Drop Message.fromJson in favor of constructor coercion

• Removes the fromJson(JsonInput) method and related imports. Message deserialization is now delegated to the shared JSON coercion infrastructure.

java/src/org/openqa/selenium/bidi/script/Message.java

NodeProperties.javaRemove custom NodeProperties JSON parsing +0/-74

Remove custom NodeProperties JSON parsing

• Deletes the hand-written fromJson(JsonInput) method that handled many Optional fields. NodeProperties is now expected to be populated via constructor/instance coercion and Optional support.

java/src/org/openqa/selenium/bidi/script/NodeProperties.java

Source.javaRename Source constructor parameter for JSON (context) +3/-30

Rename Source constructor parameter for JSON (context)

• Removes Source.fromJson and updates the constructor argument name from browsingContext to context to match JSON. Keeps the internal field name browsingContext unchanged.

java/src/org/openqa/selenium/bidi/script/Source.java

StackFrame.javaRemove StackFrame.fromJson and match ctor params to JSON +4/-44

Remove StackFrame.fromJson and match ctor params to JSON

• Drops the manual fromJson(JsonInput) implementation and renames constructor parameters to url/functionName to match JSON field names. Retains existing toJson behavior.

java/src/org/openqa/selenium/bidi/script/StackFrame.java

WindowProxyProperties.javaRely on ctor parameter name 'context' for JSON mapping +3/-25

Rely on ctor parameter name 'context' for JSON mapping

• Removes WindowProxyProperties.fromJson and switches constructor parameter name to context. This enables automatic mapping from the BiDi 'context' field.

java/src/org/openqa/selenium/bidi/script/WindowProxyProperties.java

CreateSessionRequest.javaMake constructor parameters match JSON field names (desiredCapabilities) +6/-35

Make constructor parameters match JSON field names (desiredCapabilities)

• Removes the private fromJson(JsonInput) method and updates constructor parameter names so JSON can map desiredCapabilities directly. Preserves validation and ImmutableCapabilities wrapping.

java/src/org/openqa/selenium/grid/data/CreateSessionRequest.java

DistributorStatus.javaRemove fromJson and standardize ctor parameter name to 'nodes' +3/-27

Remove fromJson and standardize ctor parameter name to 'nodes'

• Deletes the custom fromJson(JsonInput) path and renames the constructor parameter to nodes to match the serialized JSON key. Keeps existing toJson map structure.

java/src/org/openqa/selenium/grid/data/DistributorStatus.java

NewSessionErrorResponse.javaRemove NewSessionErrorResponse.fromJson +0/-27

Remove NewSessionErrorResponse.fromJson

• Drops the fromJson(JsonInput) implementation, leaving JSON serialization intact. Deserialization is expected to be handled by the updated JsonTypeCoercer logic.

java/src/org/openqa/selenium/grid/data/NewSessionErrorResponse.java

Session.javaAlign Session constructor arg names with JSON keys (sessionId/start) +5/-46

Align Session constructor arg names with JSON keys (sessionId/start)

• Removes Session.fromJson and renames constructor parameters to sessionId and start to match JSON fields. Maintains existing validation and capability copying behavior.

java/src/org/openqa/selenium/grid/data/Session.java

SessionRequestCapability.javaRemove fromJson and rename ctor parameter to JSON key (capabilities) +3/-32

Remove fromJson and rename ctor parameter to JSON key (capabilities)

• Deletes SessionRequestCapability.fromJson and updates the constructor parameter name from desiredCapabilities to capabilities to match the emitted JSON. Keeps toJson emitting requestId and capabilities.

java/src/org/openqa/selenium/grid/data/SessionRequestCapability.java

SlotId.javaRemove SlotId.fromJson and rename ctor args (hostId/id) +4/-30

Remove SlotId.fromJson and rename ctor args (hostId/id)

• Removes the custom fromJson(JsonInput) and updates constructor parameter names to hostId and id to match JSON fields. Maintains existing toJson output structure.

java/src/org/openqa/selenium/grid/data/SlotId.java

Tests (2) +336 / -9

ConstructorCoercerTest.javaAdd coverage for constructor matching, overload choice, and Optional rules +332/-0

Add coverage for constructor matching, overload choice, and Optional rules

• Adds unit tests verifying named-constructor population (including private ctors), nested coercion, Collection support, Optional missing/null semantics, overload selection, ambiguity errors, and fromJson precedence over constructors.

java/test/org/openqa/selenium/json/ConstructorCoercerTest.java

JsonTest.javaUpdate JsonTest to expect named-constructor deserialization to succeed +4/-9

Update JsonTest to expect named-constructor deserialization to succeed

• Replaces the previous expectation that classes without a default constructor fail to deserialize. Now asserts successful deserialization when constructor parameter names are available.

java/test/org/openqa/selenium/json/JsonTest.java

Documentation (1) +12 / -5

Json.javaDocument constructor parameter name deserialization and Optional/Collection support +12/-5

Document constructor parameter name deserialization and Optional/Collection support

• Updates Json Javadoc to include constructor-based deserialization and the requirement for javac -parameters. Expands documented supported types to include Collection and Optional.

java/src/org/openqa/selenium/json/Json.java

Other (2) +24 / -2

java_library.bzlCompile Java targets with -parameters to preserve constructor arg names +3/-1

Compile Java targets with -parameters to preserve constructor arg names

• Adds a default javac option (-parameters) to retain parameter names in class files. This is required for reflection-based JSON constructor coercion to match JSON fields to constructor arguments.

java/private/java_library.bzl

BUILD.bazelAdd ConstructorCoercer tests and compile JSON tests with -parameters +21/-1

Add ConstructorCoercer tests and compile JSON tests with -parameters

• Creates a dedicated ConstructorCoercerTests suite and excludes its source from the existing SmallTests glob. Ensures JSON tests compile with -parameters so reflection-based parameter name lookup works in test code.

java/test/org/openqa/selenium/json/BUILD.bazel

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (2) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 18 rules

1. BiDiSessionStatus.fromJson removed 📘 Rule violation ≡ Correctness

Description

The PR removes public static fromJson(JsonInput) factory methods (e.g., in BiDiSessionStatus and
other DTOs), which is a backward-incompatible API/ABI change for existing callers. Downstream code
that calls these methods will no longer compile against this version.

Code

java/src/org/openqa/selenium/bidi/BiDiSessionStatus.java[L38-62]

-  public static BiDiSessionStatus fromJson(JsonInput input) {
-    Boolean ready = null;
-    String message = null;
-
-    input.beginObject();
-    while (input.hasNext()) {
-      switch (input.nextName()) {
-        case "ready":
-          ready = input.read(Boolean.class);
-          break;
-
-        case "message":
-          message = input.read(String.class);
-          break;
-
-        default:
-          input.skipValue();
-          break;
-      }
-    }
-    input.endObject();
-
-    return new BiDiSessionStatus(
-        Require.nonNull("ready", ready), Require.nonNull("message", message));
-  }

Evidence

PR Compliance ID 389266 requires maintaining backward-compatible public API/ABI. These public
classes no longer contain fromJson(JsonInput) factories in the updated sources, indicating that
callers relying on those symbols would break.

Rule 389266: Maintain backward-compatible public API and ABI
java/src/org/openqa/selenium/bidi/BiDiSessionStatus.java[27-44]
java/src/org/openqa/selenium/bidi/browsingcontext/HistoryUpdated.java[23-50]
java/src/org/openqa/selenium/bidi/network/AuthChallenge.java[22-40]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Public static `fromJson(JsonInput)` methods were removed from multiple public types, creating a backward-incompatible API change.

## Issue Context
The compliance requirement is to maintain backward-compatible public API/ABI. If the new constructor-based coercion is the preferred mechanism, the old `fromJson(JsonInput)` entrypoints can remain as compatibility shims (optionally deprecated) delegating to the new implementation.

## Fix Focus Areas
- java/src/org/openqa/selenium/bidi/BiDiSessionStatus.java[32-44]
- java/src/org/openqa/selenium/bidi/browsingcontext/HistoryUpdated.java[23-50]
- java/src/org/openqa/selenium/bidi/network/AuthChallenge.java[22-40]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Session constructor lacks Javadoc 📘 Rule violation ✧ Quality

Description

A modified public constructor (Session(...)) is not preceded by a Javadoc block, so it lacks
required API documentation (including @param tags). This makes the public API harder to use
correctly and violates the Javadoc requirement for public API members.

Code

java/src/org/openqa/selenium/grid/data/Session.java[R46-56]

+  // Constructor parameter names are used as JSON field names.
  public Session(
-      SessionId id,
+      SessionId sessionId,
      URI uri,
      Capabilities stereotype,
      Capabilities capabilities,
-      Instant startTime) {
-    this.id = Require.nonNull("Session ID", id);
+      Instant start) {
+    this.id = Require.nonNull("Session ID", sessionId);
    this.uri = Require.nonNull("Where the session is running", uri);
-    this.startTime = Require.nonNull("Start time", startTime);
+    this.startTime = Require.nonNull("Start time", start);

Evidence

PR Compliance ID 330200 requires Javadoc for public API types/methods/constructors. The cited
constructors are public and are preceded by a line comment rather than a /** ... */ Javadoc block
with @param tags.

Rule 330200: Require Javadoc for all public API types and methods
java/src/org/openqa/selenium/grid/data/Session.java[46-60]
java/src/org/openqa/selenium/grid/data/CreateSessionRequest.java[30-46]
java/src/org/openqa/selenium/grid/data/DistributorStatus.java[29-36]
java/src/org/openqa/selenium/bidi/log/StackFrame.java[23-40]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Public constructors changed in this PR are missing required Javadoc blocks.

## Issue Context
The checklist requires Javadoc immediately preceding public API constructors/methods, and the Javadoc must include `@param` tags matching the constructor parameter names.

## Fix Focus Areas
- java/src/org/openqa/selenium/grid/data/Session.java[46-60]
- java/src/org/openqa/selenium/grid/data/CreateSessionRequest.java[36-46]
- java/src/org/openqa/selenium/grid/data/DistributorStatus.java[33-36]
- java/src/org/openqa/selenium/bidi/log/StackFrame.java[34-40]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Repeated JSON round-trips 🐞 Bug ➹ Performance

Description

ConstructorCoercer coerces each constructor argument by serializing the already-parsed value back
into JSON and reparsing it, multiplying parsing/allocation work per object. This is particularly
costly on hot deserialization paths (e.g., BiDi websocket event processing) now that multiple event
types rely on constructor-based coercion.

Code

java/src/org/openqa/selenium/json/ConstructorCoercer.java[R150-158]

+  private Object coerceValue(Object value, Type type, PropertySetting setting) {
+    StringWriter rawJson = new StringWriter();
+    try (JsonOutput output = new JsonOutput(rawJson)) {
+      output.write(value);
+    }
+
+    try (JsonInput input = new JsonInput(new StringReader(rawJson.toString()), coercer, setting)) {
+      return coercer.coerce(input, type, setting);
+    }

Evidence

The new coercer converts each property value into a JSON string and reparses it (coerceValue), and
BiDi message handling is a high-frequency deserialization path that maps websocket payloads into
Java objects. Several BiDi event types were modified to depend on constructor-parameter-name
deserialization, making this overhead newly relevant at scale.

java/src/org/openqa/selenium/json/ConstructorCoercer.java[150-159]
java/src/org/openqa/selenium/bidi/Connection.java[265-307]
java/src/org/openqa/selenium/bidi/browsingcontext/HistoryUpdated.java[32-37]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ConstructorCoercer.coerceValue` currently converts each field value via a StringWriter/StringReader JSON round-trip before coercing it to the constructor parameter type. This adds extra JSON serialization + parsing for every constructor parameter, increasing CPU and allocation/GC costs.

## Issue Context
This coercer is now in the default `JsonTypeCoercer` chain and multiple BiDi model types were updated to rely on constructor parameter names for deserialization, which occurs frequently in websocket message handling.

## Fix Focus Areas
- Implement object-to-type coercion without reparsing JSON (e.g., add a `coerce(Object value, Type targetType, PropertySetting setting)` path that can convert primitives/Map/List to the desired target type directly, or otherwise avoid per-parameter StringWriter/StringReader).
- Update constructor argument binding to use the new path and keep existing null/Optional semantics.

- java/src/org/openqa/selenium/json/ConstructorCoercer.java[57-65]
- java/src/org/openqa/selenium/json/ConstructorCoercer.java[150-158]
- java/src/org/openqa/selenium/json/ConstructorCoercer.java[184-207]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Unchecked reflective access failure 🐞 Bug ☼ Reliability

Description

ConstructorCoercer calls Constructor#setAccessible(true) while building coercers, but
access-denied failures (e.g., SecurityException/InaccessibleObjectException) will propagate as
unchecked runtime exceptions rather than JsonException. This bypasses Json's normal parse-error
wrapping (which only wraps JsonException) and can surface as unexpected runtime failures during
coercer construction.

Code

java/src/org/openqa/selenium/json/ConstructorCoercer.java[R166-170]

+    ConstructorCandidate(Constructor<?> constructor) {
+      this.constructor = constructor;
+      this.constructor.setAccessible(true);
+      this.parameters = constructor.getParameters();
+      this.parameterIndexes = getParameterIndexes(parameters);

Evidence

ConstructorCoercer sets accessibility during candidate construction with no guard; coercer
construction is triggered from JsonTypeCoercer’s computeIfAbsent path; and Json.toType only
wraps JsonException, so unchecked reflection exceptions are not normalized into parse errors.

java/src/org/openqa/selenium/json/ConstructorCoercer.java[166-171]
java/src/org/openqa/selenium/json/JsonTypeCoercer.java[142-178]
java/src/org/openqa/selenium/json/Json.java[174-180]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ConstructorCoercer` invokes `constructor.setAccessible(true)` without handling unchecked access exceptions. If reflective access is denied, the exception can escape as a non-`JsonException` (thrown during coercer construction), which is inconsistent with the rest of the JSON stack’s error model and can bypass `Json.toType` wrapping.

## Issue Context
`Json.toType(String, Type, PropertySetting)` wraps only `JsonException`, so unchecked reflection exceptions can escape as unexpected runtime errors.

## Fix Focus Areas
- Catch `RuntimeException` around `setAccessible(true)` (specifically `SecurityException` and `InaccessibleObjectException`) and rethrow as `JsonException` with a clear message including the target class/constructor.
- Ensure coercer construction errors consistently surface as `JsonException`.

- java/src/org/openqa/selenium/json/ConstructorCoercer.java[166-171]
- java/src/org/openqa/selenium/json/JsonTypeCoercer.java[142-178]
- java/src/org/openqa/selenium/json/Json.java[174-180]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---