Skip to content

[java] Add field names to JsonException "Duplicate JSON field name..."#17225

Merged
asolntsev merged 1 commit intoSeleniumHQ:trunkfrom
asolntsev:fix/json/show-field-names-in-exception
Mar 14, 2026
Merged

[java] Add field names to JsonException "Duplicate JSON field name..."#17225
asolntsev merged 1 commit intoSeleniumHQ:trunkfrom
asolntsev:fix/json/show-field-names-in-exception

Conversation

@asolntsev
Copy link
Contributor

Exception message before:

Duplicate JSON field name detected while collecting field writers

Exception message after:

 Duplicate JSON field name detected while collecting field writers:
       FieldWriter(org.openqa.selenium.json.JsonTest$ChildFieldBean.value) vs
       FieldWriter(org.openqa.selenium.json.JsonTest$ParentFieldBean.value)

🔗 Related Issues

Continuation of #17187

💥 What does this PR do?

This PR extracts few anonymous lambdas in InstanceCoercer.java to named classes which now have toString() method. Thus, they look readable in exception messages.

🔄 Types of changes

  • New feature (non-breaking change which adds functionality and tests!)

@asolntsev asolntsev self-assigned this Mar 14, 2026
@asolntsev asolntsev added this to the 4.42.0 milestone Mar 14, 2026
@selenium-ci selenium-ci added the C-java Java Bindings label Mar 14, 2026
@qodo-code-review
Copy link
Contributor

Review Summary by Qodo

Improve JSON exception messages with field and property names

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Extract anonymous lambdas to named classes with toString() methods
• Improve exception messages with field/property names in duplicates
• Add class context to SimplePropertyDescriptor for better debugging
• Update test assertions to verify detailed exception messages

Grey Divider

File Changes

1. java/src/org/openqa/selenium/json/InstanceCoercer.java ✨ Enhancement +100/-40

Extract lambdas to named classes with toString methods

• Extract anonymous lambdas into named classes: FieldTypeAndWriter, FieldWriter,
 SimplePropertyTypeAndWriter, SimplePropertyWriter
• Add toString() method to TypeAndWriter that delegates to writer
• Implement toString() in writer classes to display field/property names
• Enhance duplicate field exception message to include both conflicting writers
• Change import from Collectors to static toMap import

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


2. java/src/org/openqa/selenium/json/SimplePropertyDescriptor.java ✨ Enhancement +30/-16

Extract lambda and add class context to descriptor

• Extract GET_CLASS_NAME anonymous lambda to named GetClassName class
• Add clazz field to store declaring class for better context
• Add toString() method returning formatted class and property name
• Update constructor calls to pass class parameter throughout

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


3. java/test/org/openqa/selenium/json/JsonTest.java 🧪 Tests +4/-1

Update test to verify detailed exception messages

• Update test assertion to verify detailed exception message format
• Check for specific field writer names in duplicate field error
• Verify both conflicting writers are shown in exception

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


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Mar 14, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Public constructor signature changed 📘 Rule violation ✓ Correctness
Description
SimplePropertyDescriptor's public constructor now requires an extra Class<?> clazz parameter,
which breaks source compatibility for any external callers. This violates the requirement to
maintain API/ABI compatibility for public interfaces.
Code

java/src/org/openqa/selenium/json/SimplePropertyDescriptor.java[R35-40]

  public SimplePropertyDescriptor(
-      String name, @Nullable Function<Object, @Nullable Object> read, @Nullable Method write) {
+      Class<?> clazz,
+      String name,
+      @Nullable Function<Object, @Nullable Object> read,
+      @Nullable Method write) {
+    this.clazz = clazz;
Evidence
PR Compliance ID 1 requires public APIs remain backward-compatible. The PR changes the `public
SimplePropertyDescriptor(...) constructor by adding a new required Class<?> clazz` parameter, so
previously compiling code that called the old constructor will now fail to compile.

AGENTS.md
java/src/org/openqa/selenium/json/SimplePropertyDescriptor.java[35-44]

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

## Issue description
`SimplePropertyDescriptor` is a `public` class and its `public` constructor signature was changed to require an extra `Class&lt;?&gt; clazz` argument, which is a source-breaking API change.

## Issue Context
To preserve API compatibility, existing external callers must still be able to construct `SimplePropertyDescriptor` with the prior argument list. The new `clazz` field can be derived when possible (e.g., from `write.getDeclaringClass()` when `write != null`) or defaulted safely.

## Fix Focus Areas
- java/src/org/openqa/selenium/json/SimplePropertyDescriptor.java[35-44]

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


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@asolntsev asolntsev changed the title Add field names to JsonException "Duplicate JSON field name..." [java] Add field names to JsonException "Duplicate JSON field name..." Mar 14, 2026
This PR extracts few anonymous lambdas in InstanceCoercer.java to named classes which now have "toString" method. Thus, they look readable in exception messages.

continuation of SeleniumHQ#17187
@asolntsev asolntsev force-pushed the fix/json/show-field-names-in-exception branch from c2b9e20 to b6a664c Compare March 14, 2026 10:06
@asolntsev asolntsev merged commit 7e9d2f7 into SeleniumHQ:trunk Mar 14, 2026
42 of 44 checks passed
@asolntsev asolntsev deleted the fix/json/show-field-names-in-exception branch March 14, 2026 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-java Java Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants