WW-5685 Stop dropping the rest of a -conversion.properties file after the first already-mapped key

[lukaszlenart]

Fixes WW-5685

DefaultConversionFileProcessor.process skipped keys already present in the converter mapping with a break rather than a continue, so the first already-mapped entry ended the loop and every remaining entry in that -conversion.properties file was silently dropped — no warning, no error, the affected properties simply fell back to default conversion.

 if (mapping.containsKey(key)) {
-    break;
+    continue;
 }

Why it triggers in practice

XWorkConverter.buildConverterMapping walks the class, then its interfaces, then its superclass, passing one accumulating mapping into each addConverterMapping call. So a key claimed earlier in that walk aborts a later file outright:

• subclass Foo-conversion.properties declares bar
• superclass FooBase-conversion.properties declares bar, baz, qux
• when the superclass file is read, bar is already mapped → baz and qux are never registered

Annotation-derived entries land in the same map, so an @TypeConversion on the class could abort its properties file the same way.

Properties extends Hashtable and entrySet() has no defined order, so which entries survived depended on hash order rather than file order — which is what makes this hard to diagnose from the symptom.

WW-3871 fixed the identical defect in the annotation path (XWorkConverter, #1812, shipped 7.3.0). This is the properties-file path that change did not touch. The remaining break at XWorkConverter:761 is unrelated and correct — it exits an interface search after a hit.

Testing — and a vacuous test caught by mutation

DefaultConversionFileProcessorTest (new). The ticket suggested a fixture "whose first key is already mapped", but Hashtable ordering means no fixture can pin down which key is read first. So the test derives the colliding key from the actual iteration order at run time and pre-maps that one:

String firstKey = (String) fixture.entrySet().iterator().next().getKey();
mapping.put(firstKey, SENTINEL);
processor.process(mapping, PropertiesCollisionBaseAction.class, FILENAME);
// every key in the file must now be present

With break, the loop stops on that first entry and registers nothing at all — discriminating on any JDK, in any hash order, with no reliance on Hashtable layout. A second test asserts every entry registers when nothing is pre-mapped, guarding against a fix that skips too much.

XWorkConverterTest.testPropertiesEntriesAfterAKeyCollisionAreStillRegistered covers the realistic hierarchy trigger. Worth flagging: the first version of this test was vacuous. Its shared key hashed to the last position, so break dropped nothing and it passed against the unfixed code — only the mutation check exposed it. The key is renamed so it is read first (verified identical on Temurin 17.0.14, 21.0.7 and 25.0.1), and the test now asserts that precondition explicitly:

assertEquals("the fixture only discriminates while the shared key is read first; "
                + "Properties iteration order has changed and it must be renamed again",
        "CreateIfNull_overridden", firstKeyOf(PROPERTIES_COLLISION_BASE_FILE));

so a future reordering fails loudly instead of going quiet.

Both tests were mutation-checked by reverting continue to break; both fail.

mvn test -DskipAssembly -pl core — 3193 tests, 0 failures, 0 errors.

Compatibility

Behaviour changes as intended: entries previously dropped now register. That matches what WW-3871 shipped for the annotation path in 7.3.0. The containsKey precedence rule itself is untouched — first source to claim a key still wins, so nothing that already worked is overridden. Worth a Version Notes line; no Migration Guide entry needed.

🤖 Generated with Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud