[java] Fix duplicate field name handling in InstanceCoercer#getFieldWriters

[seethinajayadileep]

💥 What does this PR do?

This PR fixes duplicate field name handling in InstanceCoercer#getFieldWriters.

Previously, field writers were collected using Collectors.toMap(...) without an explicit merge strategy. When duplicate field names were encountered (for example through field hiding in inheritance), the collector would fail with an implicit IllegalStateException caused by duplicate keys.

This change makes duplicate handling explicit by detecting duplicates during map collection and throwing a JsonException with a clear error message.

This prevents ambiguous field mappings and replaces the implicit collector failure with a controlled and descriptive exception.

---

🔧 Implementation Notes

The change modifies the Collectors.toMap(...) call in getFieldWriters.

Previous behavior

• Collectors.toMap was used without a merge function.
• Duplicate field names caused an implicit IllegalStateException from the collector.

New behavior

• A merge function is provided that throws a JsonException when duplicate field names are encountered.
• This makes the failure explicit and easier to diagnose.

Example scenario that previously caused an implicit collector failure:

class Parent {
  String value;
}

class Child extends Parent {
  String value;
} 

Both fields resolve to the same JSON key (value), producing a duplicate during field writer collection.

With this change, such cases now fail with a clear JsonException instead of a generic collector error.

💡 Additional Considerations

This change only affects cases where duplicate serialized field names exist.

Valid classes with unique field names continue to behave exactly as before.

A regression test was added to verify that duplicate field names detected during BY_FIELD deserialization result in a JsonException.

🔄 Types of changes

Bug fix (backwards compatible)

[qodo-code-review]

Review Summary by Qodo

Fix duplicate field name handling in InstanceCoercer#getFieldWriters

🐞 Bug fix

Walkthroughs

Description

• Fixes duplicate field name handling in InstanceCoercer#getFieldWriters
• Adds merge function to Collectors.toMap() to retain first entry
• Uses LinkedHashMap to preserve field processing order
• Prevents runtime IllegalStateException on duplicate keys

File Changes

1. java/src/org/openqa/selenium/json/InstanceCoercer.java 🐞 Bug fix +3/-1

Add merge function and LinkedHashMap to toMap collector

• Modified Collectors.toMap() call to include merge function (existing, ignored) -> existing
• Added LinkedHashMap::new as map supplier to preserve insertion order
• Prevents IllegalStateException: Duplicate key when multiple fields resolve to same serialized
 name
• Retains first discovered field writer and ignores subsequent duplicates

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

---

[qodo-code-review]

Code Review by Qodo

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

1. getFieldWriters duplicates untested ☑ 📘 Rule violation ⛯ Reliability

Description

The change alters runtime behavior for duplicate field names (now silently keeping the first writer)
without adding a unit test to prevent regressions. This makes it easy for future refactors to
reintroduce duplicate-key failures or change which field “wins” without detection.

Code

java/src/org/openqa/selenium/json/InstanceCoercer.java[R129-131]

+                },
+                (existing, ignored) -> existing,
+                java.util.LinkedHashMap::new));

Evidence

PR Compliance ID 4 requires adding/updating tests for behavior changes where feasible. The updated
collector explicitly changes behavior on duplicate keys, and the PR itself notes tests as follow-up
work rather than including them.

AGENTS.md
java/src/org/openqa/selenium/json/InstanceCoercer.java[129-131]

Agent prompt

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

## Issue description
`InstanceCoercer#getFieldWriters` now retains the first writer when duplicate field names occur, but this behavior change is not covered by tests.
## Issue Context
Duplicate field names can happen via field hiding (same-named fields across a class hierarchy). Historically this could throw `Duplicate key` during collection; now duplicates are ignored. This should be locked in with a unit test to prevent regressions and to ensure deterministic "winner" behavior.
## Fix Focus Areas
- java/src/org/openqa/selenium/json/InstanceCoercer.java[99-132]
- java/test/org/openqa/selenium/json/JsonTest.java[113-145]

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

---

2. Silent duplicate suppression ☑ 🐞 Bug ✓ Correctness

Description

getFieldWriters now silently ignores later fields that resolve to the same JSON key, which can
hide ambiguous class designs (field hiding) and lead to missing/incorrectly-applied data without any
signal. This trades a clear failure (duplicate key exception) for silent data loss and harder
debugging when duplicates exist.

Code

java/src/org/openqa/selenium/json/InstanceCoercer.java[R129-131]

+                },
+                (existing, ignored) -> existing,
+                java.util.LinkedHashMap::new));

Evidence

Fields are collected starting from the concrete class and then walking up the superclass chain, and
the collector now keeps the first entry and drops subsequent ones for the same key. During
deserialization only the retained writer is used for a given JSON key, so any dropped duplicate
field can never be set, with no warning/error emitted by this code path.

java/src/org/openqa/selenium/json/InstanceCoercer.java[99-105]
java/src/org/openqa/selenium/json/InstanceCoercer.java[107-132]
java/src/org/openqa/selenium/json/InstanceCoercer.java[77-88]

Agent prompt

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

## Issue description
`InstanceCoercer#getFieldWriters` now resolves duplicate field-name collisions by silently keeping the first writer and ignoring the rest. This can cause silent data loss / surprising field selection when a subclass hides a superclass field, making deserialization bugs very hard to detect.
### Issue Context
The stream collects fields across the class hierarchy and then builds a map keyed by `Field::getName`. With the new merge function, duplicates are no longer surfaced to the caller.
### Fix Focus Areas
- java/src/org/openqa/selenium/json/InstanceCoercer.java[99-132]

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

---

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

[seethinajayadileep]

Hi maintainers, could someone please assign a reviewer for this PR? Thank you.

[asolntsev]

It's slightly better than previously, so we can merge this PR. Thank you.

Before:

java.lang.IllegalStateException: Duplicate key value (attempted merging values org.openqa.selenium.json.InstanceCoercer$TypeAndWriter@4e70a728 and org.openqa.selenium.json.InstanceCoercer$TypeAndWriter@b7838a9)

After:

org.openqa.selenium.json.JsonException: Duplicate JSON field name detected while collecting field writers

P.S. But the name of the field is not still shown in the error message. Ideally, we should show the name as well. But I can it in a separate PR if you wish.