lukaszlenart opened a new pull request, #1812:
URL: https://github.com/apache/struts/pull/1812

   Fixes [WW-3871](https://issues.apache.org/jira/browse/WW-3871)
   
   ## What
   
   `@TypeConversion` now derives its conversion-mapping key prefix from the 
declared `ConversionRule`, so a bare property name works as `key` at class, 
method and field level:
   
   ```java
   // both forms are now equivalent
   @TypeConversion(key = "annotatedBeanMap", rule = 
ConversionRule.KEY_PROPERTY, value = "id")
   @TypeConversion(key = "KeyProperty_annotatedBeanMap", rule = 
ConversionRule.KEY_PROPERTY, value = "id")
   ```
   
   Half of the reporter's request was already implemented in 2018 (77cbafb74), 
which taught the *method-level* path to derive the key from the property name. 
This closes the remaining gaps.
   
   ## Changes
   
   - **`ConversionRule.prefix()`** owns the rule-to-prefix table, previously 
duplicated inline. The `switch` is exhaustive with no `default`, so a future 
rule cannot silently ship without a prefix.
   - **`XWorkConverter.resolveKey(type, rule, name)`** is the single resolver 
all annotation passes route through — explicit keys and derived property names 
alike. A key that already carries a rule prefix is returned untouched, so 
existing annotations keep working byte-for-byte. `ConversionType.APPLICATION` 
keys are class names and are never prefixed.
   - **`addConverterMapping` split into four ordered passes** — properties 
file, class-level, method, field — keeping the existing first-writer-wins rule. 
The previous single method was ~45 lines of nested loops.
   - **`@TypeConversion` is now `@Target({METHOD, FIELD})`**, which its Javadoc 
has always claimed. The field pass reads declared, non-static, non-synthetic 
fields only; `buildConverterMapping` already walks the hierarchy.
   
   Two defects in the same code path are fixed as a byproduct:
   
   - `break` where `continue` was meant: one already-mapped key aborted the 
**remaining** `@TypeConversion` entries in a `@Conversion` array.
   - An empty class-level `key` registered a mapping under `""`. It is now 
skipped with a WARN naming the class.
   
   ## Compatibility
   
   Source- and binary-compatible. `MyBeanAction` deliberately keeps its 
spelled-out prefixes and its tests are untouched — that is the 
backward-compatibility evidence, alongside a new bare-key twin asserted to 
produce an identical mapping.
   
   The one behavioural change to existing code: an explicit method-level key 
carrying a non-`PROPERTY` rule without its prefix now resolves to the prefixed 
key. That mapping was unreachable before, so this turns a silent no-op into the 
behaviour the author intended.
   
   ## Notes for review
   
   - The "already prefixed" guard matches against **every** rule's prefix, not 
just the declared rule's. `COLLECTION` and `ELEMENT` are interchangeable — 
`DefaultConversionAnnotationProcessor` handles both in one branch and 
`DefaultObjectTypeDeterminer.getElementClass` reads `Element_` then falls back 
to the deprecated `Collection_` — so `key = "Element_users", rule = COLLECTION` 
must not become `Collection_Element_users`.
   - Precedence is class > method > field per class. Because the method pass 
reads `getMethods()` (inherited) while the field pass reads 
`getDeclaredFields()`, a superclass's annotated setter beats a subclass's field 
annotation; this is documented on `processFieldAnnotations`.
   - Out of scope, but noticed: `DefaultConversionFileProcessor:69` has the 
identical `break`-instead-of-`continue` defect for `-conversion.properties` 
files. Happy to file a follow-up.
   
   ## Testing
   
   `mvn test -DskipAssembly -pl core` — 3040/3040 passing. `mvn javadoc:javadoc 
-pl core` clean.
   
   New coverage: `ConversionRuleTest`; `resolveKey` unit tests including prefix 
crossover and the `APPLICATION` carve-out; class-level bare keys asserted equal 
to the spelled-out form; a `-conversion.properties` key collision proving later 
entries still register; field derivation and method-over-field precedence; and 
an end-to-end binding test through the action lifecycle.
   
   Design notes and the implementation plan are included under 
`docs/superpowers/`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to