Apache/struts
1
0
Fork 0
mirror of https://github.com/apache/struts.git synced 2026-08-05 14:47:09 +00:00
Code Issues Packages Projects Releases Wiki Activity

WW-3871 Derive ConversionRule prefixes for @TypeConversion keys (#1812)

Browse Source 
* WW-3871 docs: add design spec for @TypeConversion key derivation

Specifies deriving the ConversionRule prefix for @TypeConversion keys at
class, method and field level via a single resolver, adds ElementType.FIELD
as a target, and records the break/continue and empty-key fixes in the same
code block.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* WW-3871 docs: note interaction with the 7.3.0 converter mapping cache

Records that addConverterMapping runs inside the computeMappingIfAbsent
builder introduced by WW-5539, which executes outside any lock, so the new
field pass adds no deadlock risk but must stay side-effect free.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* WW-3871 docs: add implementation plan for @TypeConversion key derivation

Seven TDD tasks covering ConversionRule#prefix(), the shared resolveKey
helper, class- and field-level derivation, the break/continue and empty-key
fixes, an end-to-end binding proof and the Javadoc updates. Refines the
spec's resolveKey signature to take the two annotation attributes rather
than the annotation instance, so it can be unit tested directly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* WW-3871 feat(core): add ConversionRule#prefix() owning the rule-to-prefix table

* WW-3871 refactor(core): split addConverterMapping into per-source passes

* WW-3871 feat(core): derive conversion mapping keys through a single resolver

* WW-3871 fix(core): derive class level conversion keys and stop dropping later entries

* WW-3871 feat(core): support @TypeConversion on fields

* WW-3871 test(core): assert bare conversion keys bind through the action lifecycle

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* WW-3871 docs(core): document conversion key derivation and field level support

* WW-3871 docs(core): add deprecated Collection_ prefix to parameter table

* WW-3871 fix(core): widen resolveKey idempotence guard against any rule prefix

resolveKey only recognized a key as already-prefixed if it started with
its own declared rule's prefix. COLLECTION and ELEMENT are interchangeable
throughout the conversion pipeline (DefaultConversionAnnotationProcessor
handles them in the same branch, DefaultObjectTypeDeterminer.getElementClass
reads Element_ then falls back to the deprecated Collection_), so
key="Element_users" with rule=COLLECTION silently doubled to
Collection_Element_users instead of being left alone, losing the mapping.
Match against every known rule's prefix instead.

Also documents two related precedence subtleties surfaced during review:
processFieldAnnotations' Javadoc now notes that an inherited method can
claim a key before a subclass's own field annotation is considered, since
getMethods() includes inherited methods and runs first; and the
unresolvable-key WARN in processMethodAnnotations now names the method's
declaring class rather than the class being scanned, since getMethods()
can surface the same inherited method at every level of the hierarchy.

Design spec section 2 updated to match the implementation.

* WW-3871 docs(core): correct TypeConversion Javadoc property attribute and determiner package

Two pre-existing errors in the block this ticket's commits already touch:
the APPLICATION example used a non-existent "property" attribute where
"key" is the working form (see ConversionTestAction.java:97), and the
rule() Javadoc pointed at org.apache.struts2.util.DefaultObjectTypeDeterminer
instead of the actual org.apache.struts2.conversion.impl package.

* WW-3871 test(core): cover key-prefix crossover, empty class-level key, and KeyProperty_ end-to-end binding

- testResolveKeyLeavesAnAlreadyPrefixedKeyAlone: add the COLLECTION/ELEMENT
  crossover cases that demonstrate the resolveKey guard fix (fail before,
  pass after).
- New EmptyKeyConversionAction fixture plus
  testClassLevelEmptyKeyRegistersNoMapping: a class-level @TypeConversion
  with no key must be skipped, not registered under "". This was the one
  behavioural bullet in the spec's test plan with no coverage.
- MyBeanActionTest.testBareConversionKeysBindTheSameWayAsPrefixedOnes: add
  an assertion that the bare KeyProperty_ derivation actually binds the
  list index onto the created bean's id property end to end, not just that
  a converter mapping exists.

* WW-3871 fix(core): skip APPLICATION-scoped @TypeConversion with no explicit key

Method- and field-level @TypeConversion(type = APPLICATION) with no key
previously derived a member name (e.g. a setter's property name) and
registered it in the global default converter map via
addDefaultMapping. That map is only ever read by class name
(lookup(String, boolean) and lookup(Class)), so the entry was
permanently unreachable. Skip it before deriving a name, logging a WARN
naming the declaring class and member; the class-level pass already
handled this correctly via resolveKey returning null.

Adds a fixture and tests proving no default mapping is registered under
the derived member name in either pass.

* WW-3871 docs(core): fix broken TypeConversion Javadoc example and align spec

TypeConversion's example class declared `users` twice (once
unannotated, once again at its annotated field), so the sample no
longer compiled as written; drop the earlier, redundant declaration.

The same example's setConvertInt showed @TypeConversion(type =
APPLICATION) with no key - exactly the case the previous commit's
XWorkConverter fix now skips. Drop the type attribute so it reads as
a class-scoped conversion, matching the corrected ConversionTestAction
fixture. The correct APPLICATION example further down (execute(), key
= "java.util.Date") is untouched.

Also records the APPLICATION no-key skip rule in the design spec's
carve-out paragraph so spec and code agree.

* WW-3871 fix(core): dedupe method-pass WARN logging for inherited @TypeConversion

processMethodAnnotations iterates clazz.getMethods(), which includes inherited
public methods, and buildConverterMapping calls it once per class in the
hierarchy. A single misconfigured @TypeConversion on a base class method was
therefore logging its WARN once per subclass level. Gate both WARN call sites
on method.getDeclaringClass() == clazz so each fires exactly once, at the
level that owns the method; the derivation/registration logic keeps running
on every visit unchanged.

Adds a small permanent test proving the gate is logging-only: an inherited
annotated setter still resolves and registers through a subclass that
overrides nothing.

* WW-3871 docs(core): clarify field-name key default and dedicated-annotation precedence

Two gaps in the @TypeConversion Javadoc, both newly relevant now that the
annotation targets fields:

- The key() default on a field is the field name, not the JavaBean property
  name (processFieldAnnotations uses field.getName()). A field like _users
  backing property users would otherwise derive CreateIfNull__users, a key
  DefaultObjectTypeDeterminer never looks up.
- org.apache.struts2.util's dedicated field annotations (@Key, @Element,
  @KeyProperty, @CreateIfNull) are consulted by DefaultObjectTypeDeterminer
  before it falls back to the converter mapping @TypeConversion populates,
  so a dedicated annotation silently wins over an equivalent @TypeConversion
  on the same property. Verified against getAnnotation/getElementClass/
  getKeyProperty in DefaultObjectTypeDeterminer before documenting it.

* WW-3871 docs(core): note COLLECTION derives the deprecated Collection_ prefix

ConversionRule.COLLECTION.prefix() intentionally returns Collection_, the
spelling DefaultObjectTypeDeterminer treats as deprecated and logs an INFO
about on every fallback hit, kept for compatibility with existing
annotations. Document that the derivation is deliberate and point readers
at ELEMENT as the current form.

* WW-3871 refactor(core): extract shared annotation-registration pipeline

processMethodAnnotations and processFieldAnnotations were the same
five-step pipeline (skip non-@TypeConversion, skip APPLICATION-scoped
without a key, derive the name, resolve the key, register unless
already mapped) written twice, driving SonarCloud S3776 cognitive
complexity to 26 and 21 respectively and triggering three S135
multiple-break/continue findings.

Extract steps 2-5 into a private registerAnnotatedMember(mapping, tc,
Member, fallbackName, logSkips) helper that both passes delegate to.
Each pass is now just its loop plus one instanceof check. The method
pass keeps its per-declaring-class log gate (getMethods() revisits
inherited methods once per hierarchy level); the field pass always
logs, since getDeclaredFields() is visited once per class. The two
WARN wordings, which differed only in a trailing clause, are merged
into one message accurate for both a method and a field.

No change to the registered mapping, pass order, or precedence for
any class - verified via the existing XWorkConverterTest,
AnnotationXWorkConverterTest, MyBeanActionTest, and ConversionRuleTest
suites (92 tests, same count and same triggering warnings before and
after) plus the full core module suite (3043 tests).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* WW-3871 docs(core): fix inaccurate and self-contradicting TypeConversion key Javadoc

@Key, @Element, @KeyProperty and @CreateIfNull are @Target({FIELD, METHOD}), not
field-only, and the same paragraph already says they are read from the field,
setter and getter - drop "field" from "dedicated field annotations". Fold the
field-vs-property-name correction into key()'s opening sentence instead of
stating "defaults to the property name" and rebutting it three lines later, and
align the parameters table row for key with the same rule.

* WW-3871 docs(core): clarify XWorkConverter annotation-registration logging

Give the success DEBUG the same [declaringClass#member] shape the three skip
messages already use, instead of logging the bare member name that identifies
neither the class nor whether it was a method or a field. Reword the "already
mapped" DEBUG so it covers its commonest trigger - the same annotation seen one
hierarchy level down, not just a genuinely higher-precedence source. Note in the
logSkips comment that buildConverterMapping only visits each class' direct
interfaces, so a misconfigured annotation declared on a super-interface method
never gets logged at all, even though registration is unaffected. Also drop a
stray extra blank line.

No behavioural change: registration/derivation logic is untouched.

* WW-3871 test(core): make inherited-method-annotation test diagnostic

testInheritedMethodAnnotationStillRegistersThroughASubclass previously asserted
nothing the logSkips gate could break: the hierarchy walk always reaches
InheritedMethodConversionAction itself, where declaringClass == clazz, so the
key registers there regardless of whether registration is (wrongly) gated
alongside logging. The test passed identically with logSkips hardcoded true or
false.

Give InheritedMethodConversionSubAction a contesting field annotation for the
same property the inherited setter claims. The inherited method annotation
registers at the subclass level - before the subclass's own field pass runs -
so its value must keep winning; that is the invariant documented on
processFieldAnnotations, and it is exactly what gating registration would
break, since the subclass field would start winning over the inherited method
annotation instead.

Verified: temporarily wrapping the registerAnnotatedMember call in
processMethodAnnotations with `if (logSkips)` makes this test fail
(expected:<true> but was:<false>); reverting it passes again. Mutation was not
committed.

Corrected both Javadocs, which overclaimed what the old assertion proved.

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Lukasz Lenart
2026-07-29 07:55:46 +02:00
committed by GitHub
parent d906f23448
commit 6f802987f5
20 changed files with 2205 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/src/main/java/org/apache/struts2/conversion/annotations/ConversionRule.java
+26
View File
@@ -18,6 +18,8 @@
*/
package org.apache.struts2.conversion.annotations;
import org.apache.struts2.conversion.impl.DefaultObjectTypeDeterminer;
/**
* <code>ConversionRule</code>
*
@@ -28,6 +30,30 @@ public enum ConversionRule {
PROPERTY, COLLECTION, MAP, KEY, KEY_PROPERTY, ELEMENT, CREATE_IF_NULL;
/**
* The prefix a conversion mapping key carries for this rule, as read back by
* {@link DefaultObjectTypeDeterminer}. {@code PROPERTY} and {@code MAP} have no prefix of their
* own: map and collection metadata is read through the {@code Key_} and {@code Element_} keys.
*
* <p>{@code COLLECTION} deliberately derives {@code Collection_}, the deprecated spelling that
* {@link DefaultObjectTypeDeterminer} still falls back to (and logs an INFO about) for
* compatibility with existing annotations. Prefer {@link #ELEMENT}, whose {@code Element_}
* prefix is the current form.</p>
*
* @return the mapping key prefix, never null; an empty string when the rule has none
* @since 7.3.0
*/
public String prefix() {
return switch (this) {
case COLLECTION -> DefaultObjectTypeDeterminer.DEPRECATED_ELEMENT_PREFIX;
case CREATE_IF_NULL -> DefaultObjectTypeDeterminer.CREATE_IF_NULL_PREFIX;
case ELEMENT -> DefaultObjectTypeDeterminer.ELEMENT_PREFIX;
case KEY -> DefaultObjectTypeDeterminer.KEY_PREFIX;
case KEY_PROPERTY -> DefaultObjectTypeDeterminer.KEY_PROPERTY_PREFIX;
case PROPERTY, MAP -> "";
};
}
@Override
public String toString() {
return super.toString().toUpperCase();
core/src/main/java/org/apache/struts2/conversion/annotations/TypeConversion.java
+33 -10
View File
@@ -48,9 +48,16 @@ import java.lang.annotation.Target;
* <p><u>Annotation usage:</u></p>
*
* <!-- START SNIPPET: usage -->
* <p>The TypeConversion annotation can be applied at property and method level.</p>
* <p>The TypeConversion annotation can be applied at field and method level.</p>
* <!-- END SNIPPET: usage -->
*
* <p>The {@code org.apache.struts2.util} package also has dedicated {@code @Key},
* {@code @Element}, {@code @KeyProperty} and {@code @CreateIfNull} annotations that {@link
* org.apache.struts2.conversion.impl.DefaultObjectTypeDeterminer} consults, on the field then its
* setter then its getter, <em>before</em> falling back to the converter mapping this annotation
* populates. If both a dedicated annotation and an equivalent {@code @TypeConversion} are declared
* for the same property, the dedicated annotation wins silently.</p>
*
* <p><u>Annotation parameters:</u></p>
*
* <!-- START SNIPPET: parameters -->
@@ -67,8 +74,11 @@ import java.lang.annotation.Target;
* <tr>
* <td>key</td>
* <td>no</td>
* <td>The annotated property/key name</td>
* <td>The optional property name mostly used within TYPE level annotations.</td>
* <td>The resolved property name on a method; the field's own name on a field</td>
* <td>The property name the rule applies to. The matching prefix for the given rule
* (<code>Key_</code>, <code>Element_</code>, <code>KeyProperty_</code>, <code>CreateIfNull_</code>, or the deprecated
* <code>Collection_</code>) is prepended automatically unless the key already carries it. Required on TYPE level annotations,
* where there is no member name to derive it from.</td>
* </tr>
* <tr>
* <td>type</td>
@@ -115,11 +125,10 @@ import java.lang.annotation.Target;
* private String convertInt;
*
* private String convertDouble;
* private List users = null;
*
* private HashMap keyValues = null;
*
* &#64;TypeConversion(type = ConversionType.APPLICATION)
* &#64;TypeConversion()
* public void setConvertInt( String convertInt ) {
* this.convertInt = convertInt;
* }
@@ -129,6 +138,9 @@ import java.lang.annotation.Target;
* this.convertDouble = convertDouble;
* }
*
* &#64;TypeConversion(rule = ConversionRule.CREATE_IF_NULL, value = "true")
* private List users = null;
*
* &#64;TypeConversion(rule = ConversionRule.COLLECTION, converterClass = String.class)
* public void setUsers( List users ) {
* this.users = users;
@@ -139,7 +151,7 @@ import java.lang.annotation.Target;
* this.keyValues = keyValues;
* }
*
* &#64;TypeConversion(type = ConversionType.APPLICATION, property = "java.util.Date", converterClass = XWorkBasicConverter.class)
* &#64;TypeConversion(type = ConversionType.APPLICATION, key = "java.util.Date", converterClass = XWorkBasicConverter.class)
* public String execute() throws Exception {
* return SUCCESS;
* }
@@ -150,15 +162,26 @@ import java.lang.annotation.Target;
* @author Rainer Hermanns
* @version $Id$
*/
@Target({ ElementType.METHOD})
@Target({ElementType.METHOD, ElementType.FIELD})
@Retention(RetentionPolicy.RUNTIME)
public @interface TypeConversion {
/**
* The optional key name used within TYPE level annotations.
* Defaults to the property name.
* The property name this conversion applies to. Optional on a method, where it defaults to the
* resolved JavaBean property name; optional on a field, where it defaults to the <em>field's own
* name</em> instead - not necessarily the same thing. Required on TYPE level annotations.
*
* <p>The prefix matching the declared {@link ConversionRule} is prepended automatically, so
* {@code @TypeConversion(key = "users", rule = ConversionRule.CREATE_IF_NULL, value = "true")}
* and {@code @TypeConversion(key = "CreateIfNull_users", ...)} are equivalent.</p>
*
* <p>If a field's name does not match the property it backs (for example a field {@code _users}
* exposed as property {@code users}), give an explicit {@code key} of {@code "users"} - a derived
* key of {@code CreateIfNull__users} is never looked up, since conversion metadata is read by
* property name, not field name.</p>
*
* @return key
* @since 7.3.0 the rule prefix is derived; previously the full key had to be spelled out
*/
String key() default "";
@@ -174,7 +197,7 @@ public @interface TypeConversion {
/**
* The ConversionRule can be a PROPERTY, KEY, KEY_PROPERTY, ELEMENT, COLLECTION (deprecated) or a MAP.
* Note: Collection and Map conversion rules can be determined via org.apache.struts2.util.DefaultObjectTypeDeterminer.
* Note: Collection and Map conversion rules can be determined via org.apache.struts2.conversion.impl.DefaultObjectTypeDeterminer.
*
* @see DefaultObjectTypeDeterminer
*
core/src/main/java/org/apache/struts2/conversion/impl/XWorkConverter.java
+166 -37
View File
@@ -27,6 +27,8 @@ import org.apache.struts2.conversion.ConversionFileProcessor;
import org.apache.struts2.conversion.TypeConverter;
import org.apache.struts2.conversion.TypeConverterHolder;
import org.apache.struts2.conversion.annotations.Conversion;
import org.apache.struts2.conversion.annotations.ConversionRule;
import org.apache.struts2.conversion.annotations.ConversionType;
import org.apache.struts2.conversion.annotations.TypeConversion;
import org.apache.struts2.inject.Inject;
import org.apache.struts2.util.AnnotationUtils;
@@ -40,8 +42,10 @@ import org.apache.logging.log4j.Logger;
import org.apache.struts2.StrutsConstants;
import java.lang.annotation.Annotation;
import java.lang.reflect.Field;
import java.lang.reflect.Member;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.net.URL;
import java.text.MessageFormat;
import java.util.ArrayList;
@@ -486,6 +490,52 @@ public class XWorkConverter extends DefaultTypeConverter {
return (lastClass != null && lastProperty != null) ? new Object[] {lastClass, lastProperty} : null;
}
/**
* Resolves the conversion mapping key for an annotation: the given name carrying the
* {@link ConversionRule}'s prefix. A name that already starts with <em>any</em> known rule's
* prefix is returned unchanged, not only the prefix of the declared rule, so annotations that
* spell the prefix out keep working. This matters because {@link ConversionRule#COLLECTION} and
* {@link ConversionRule#ELEMENT} are interchangeable in both {@link DefaultConversionAnnotationProcessor}
* and {@link DefaultObjectTypeDeterminer}: a key such as {@code Element_users} declared with
* {@code rule = COLLECTION} must not become {@code Collection_Element_users}.
*
* @param type the annotation's {@link ConversionType}; APPLICATION keys are class names and are never prefixed
* @param rule the annotation's {@link ConversionRule}
* @param name an explicit key or a property name derived from a method or field
* @return the mapping key, or null when no name is available and the entry must be skipped
* @since 7.3.0
*/
static String resolveKey(ConversionType type, ConversionRule rule, String name) {
if (StringUtils.isEmpty(name)) {
return null;
}
if (type == ConversionType.APPLICATION) {
return name;
}
String prefix = rule.prefix();
if (name.startsWith(prefix)) {
return name;
}
for (ConversionRule other : ConversionRule.values()) {
String otherPrefix = other.prefix();
if (!otherPrefix.isEmpty() && name.startsWith(otherPrefix)) {
return name; // already carries a rule prefix; leave it exactly as written
}
}
return prefix + name;
}
/**
* True when a {@link TypeConversion} is APPLICATION-scoped but carries no explicit key.
* APPLICATION entries are stored in the global default converter map, which {@link
* #lookup(String, boolean)} and {@link #lookup(Class)} only ever read by <em>class name</em>.
* A member name derived from a method or field can never be looked up there, so such an entry
* must be skipped rather than registered under a key nothing can reach.
*/
private static boolean isApplicationScopedWithoutKey(TypeConversion tc) {
return tc.type() == ConversionType.APPLICATION && StringUtils.isEmpty(tc.key());
}
/**
* Looks for converter mappings for the specified class and adds it to an existing map. Only new converters are
* added. If a converter is defined on a key that already exists, the converter is ignored.
@@ -498,55 +548,134 @@ public class XWorkConverter extends DefaultTypeConverter {
String converterFilename = buildConverterFilename(clazz);
fileProcessor.process(mapping, clazz, converterFilename);
// Process annotations
Annotation[] annotations = clazz.getAnnotations();
processClassLevelAnnotations(mapping, clazz);
processMethodAnnotations(mapping, clazz);
processFieldAnnotations(mapping, clazz);
}
for (Annotation annotation : annotations) {
if (annotation instanceof Conversion conversion) {
for (TypeConversion tc : conversion.conversions()) {
if (mapping.containsKey(tc.key())) {
break;
}
if (LOG.isDebugEnabled()) {
if (StringUtils.isEmpty(tc.key())) {
LOG.debug("WARNING! key of @TypeConversion [{}/{}] applied to [{}] is empty!", tc.converter(), tc.converterClass(), clazz.getName());
} else {
LOG.debug("TypeConversion [{}/{}] with key: [{}]", tc.converter(), tc.converterClass(), tc.key());
}
}
annotationProcessor.process(mapping, tc, tc.key());
/**
* Registers the {@link TypeConversion} entries declared by a class level {@link Conversion}
* annotation.
*/
private void processClassLevelAnnotations(Map<String, Object> mapping, Class clazz) {
for (Annotation annotation : clazz.getAnnotations()) {
if (!(annotation instanceof Conversion conversion)) {
continue;
}
for (TypeConversion tc : conversion.conversions()) {
String key = resolveKey(tc.type(), tc.rule(), tc.key());
if (key == null) {
LOG.warn("Ignoring @TypeConversion [{}/{}] declared on [{}]: no key was given and a class level annotation has no property name to derive one from",
tc.converter(), tc.converterClass(), clazz.getName());
continue;
}
if (mapping.containsKey(key)) {
continue;
}
LOG.debug("TypeConversion [{}/{}] declared on [{}] resolved to key [{}]",
tc.converter(), tc.converterClass(), clazz.getName(), key);
annotationProcessor.process(mapping, tc, key);
}
}
}
// Process annotated methods
/**
* Registers {@link TypeConversion} annotations found on the class' methods.
*/
private void processMethodAnnotations(Map<String, Object> mapping, Class clazz) {
for (Method method : clazz.getMethods()) {
annotations = method.getAnnotations();
for (Annotation annotation : annotations) {
// clazz.getMethods() returns inherited public methods too, and buildConverterMapping
// calls this method once per class in the hierarchy, so a single annotation declared
// on a base class method is visited once per subclass level: for C extends B extends A,
// an annotation on a method declared in A is seen three times, all with the same
// method.getDeclaringClass(). Only log skips on the pass whose clazz is that declaring
// class, so a misconfigured annotation is reported exactly once instead of once per
// subclass. This gates logging only - the derivation/registration pipeline in
// registerAnnotatedMember still runs on every visit, which first-writer-wins relies on.
// Note buildConverterMapping only visits each class' direct interfaces, never
// super-interfaces, so for "class C implements B" where "interface B extends A" and A
// declares the annotated method, no visited level ever satisfies declaringClass == clazz
// and a misconfigured annotation there logs nothing at all. Registration is unaffected.
boolean logSkips = method.getDeclaringClass() == clazz;
for (Annotation annotation : method.getAnnotations()) {
if (annotation instanceof TypeConversion tc) {
String key = tc.key();
// Default to the property name with prefix
if (StringUtils.isEmpty(key)) {
key = AnnotationUtils.resolvePropertyName(method);
key = switch (tc.rule()) {
case COLLECTION -> DefaultObjectTypeDeterminer.DEPRECATED_ELEMENT_PREFIX + key;
case CREATE_IF_NULL -> DefaultObjectTypeDeterminer.CREATE_IF_NULL_PREFIX + key;
case ELEMENT -> DefaultObjectTypeDeterminer.ELEMENT_PREFIX + key;
case KEY -> DefaultObjectTypeDeterminer.KEY_PREFIX + key;
case KEY_PROPERTY -> DefaultObjectTypeDeterminer.KEY_PROPERTY_PREFIX + key;
default -> key;
};
LOG.debug("Retrieved key [{}] from method name [{}]", key, method.getName());
}
if (mapping.containsKey(key)) {
break;
}
annotationProcessor.process(mapping, tc, key);
registerAnnotatedMember(mapping, tc, method, AnnotationUtils.resolvePropertyName(method), logSkips);
}
}
}
}
/**
* Registers {@link TypeConversion} annotations found on the class' own fields. Only declared
* fields are read: {@link #buildConverterMapping(Class)} already walks the class hierarchy and
* calls this method once per class. Static and synthetic fields are skipped, which also makes
* this a no-op for interfaces.
*
* <p>The stated precedence "class &gt; method &gt; field" is per-class, not per-hierarchy-level:
* {@link #processMethodAnnotations(Map, Class)} sees {@link Class#getMethods()}, which includes
* inherited public methods, so a superclass's annotated setter claims its key before this pass
* ever looks at a subclass's field for that same class. A subclass field annotation only wins
* when no method anywhere in the hierarchy already claimed its key.</p>
*/
private void processFieldAnnotations(Map<String, Object> mapping, Class clazz) {
for (Field field : clazz.getDeclaredFields()) {
if (Modifier.isStatic(field.getModifiers()) || field.isSynthetic()) {
continue;
}
for (Annotation annotation : field.getAnnotations()) {
if (annotation instanceof TypeConversion tc) {
// getDeclaredFields() is visited once per class, never revisited from a
// subclass level, so there is no multi-visit noise to gate against here.
registerAnnotatedMember(mapping, tc, field, field.getName(), true);
}
}
}
}
/**
* Shared pipeline for a single {@link TypeConversion} annotation found by {@link
* #processMethodAnnotations(Map, Class)} or {@link #processFieldAnnotations(Map, Class)}: skip
* APPLICATION-scoped annotations with no explicit key, derive the name, resolve it to a mapping
* key, and register it unless something already claimed that key.
*
* @param mapping the map being built for the current class
* @param tc the annotation to register
* @param member the annotated {@link Method} or {@link Field}; used only for its name and
* declaring class, both needed for logging
* @param fallbackName the name to use when {@code tc.key()} is empty - a resolved property name
* for a method, or the field's own name for a field
* @param logSkips whether to log skipped entries; see {@link #processMethodAnnotations(Map, Class)}
* for why a method pass gates this and a field pass does not
*/
private void registerAnnotatedMember(Map<String, Object> mapping, TypeConversion tc, Member member, String fallbackName, boolean logSkips) {
if (isApplicationScopedWithoutKey(tc)) {
if (logSkips) {
LOG.warn("Ignoring @TypeConversion on [{}#{}]: an application-scoped conversion needs an explicit class-name key, not a derived property name",
member.getDeclaringClass().getName(), member.getName());
}
return;
}
String name = StringUtils.isEmpty(tc.key()) ? fallbackName : tc.key();
String key = resolveKey(tc.type(), tc.rule(), name);
if (key == null) {
if (logSkips) {
LOG.warn("Ignoring @TypeConversion on [{}#{}]: no key was given and no property name could be derived",
member.getDeclaringClass().getName(), member.getName());
}
return;
}
if (mapping.containsKey(key)) {
if (logSkips) {
LOG.debug("Skipping @TypeConversion on [{}#{}]: key [{}] is already mapped, either by a higher " +
"precedence source or by this same annotation seen at a lower level of the hierarchy",
member.getDeclaringClass().getName(), member.getName(), key);
}
return;
}
LOG.debug("TypeConversion [{}/{}] on [{}#{}] resolved to key [{}]",
tc.converter(), tc.converterClass(), member.getDeclaringClass().getName(), member.getName(), key);
annotationProcessor.process(mapping, tc, key);
}
/**
* Looks for converter mappings for the specified class, traversing up its class hierarchy and interfaces and adding
core/src/test/java/org/apache/struts2/conversion/ConversionTestAction.java
+1 -1
View File
@@ -53,7 +53,7 @@ public class ConversionTestAction implements Action {
return convertInt;
}
@TypeConversion(type = ConversionType.APPLICATION)
@TypeConversion()
public void setConvertInt( String convertInt ) {
this.convertInt = convertInt;
}
core/src/test/java/org/apache/struts2/conversion/annotations/ConversionRuleTest.java
+37
View File
@@ -0,0 +1,37 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.struts2.conversion.annotations;
import org.junit.Test;
import static org.junit.Assert.assertEquals;
public class ConversionRuleTest {
@Test
public void prefixIsDefinedForEveryRule() {
assertEquals("", ConversionRule.PROPERTY.prefix());
assertEquals("", ConversionRule.MAP.prefix());
assertEquals("Collection_", ConversionRule.COLLECTION.prefix());
assertEquals("CreateIfNull_", ConversionRule.CREATE_IF_NULL.prefix());
assertEquals("Element_", ConversionRule.ELEMENT.prefix());
assertEquals("Key_", ConversionRule.KEY.prefix());
assertEquals("KeyProperty_", ConversionRule.KEY_PROPERTY.prefix());
}
}
core/src/test/java/org/apache/struts2/conversion/impl/XWorkConverterTest.java
+179
View File
@@ -37,8 +37,21 @@ import org.apache.struts2.util.FurColor;
import org.apache.struts2.util.ValueStack;
import org.apache.struts2.util.reflection.ReflectionContextState;
import ognl.OgnlRuntime;
import org.apache.struts2.conversion.ConversionAnnotationProcessor;
import org.apache.struts2.conversion.TypeConverter;
import org.apache.struts2.conversion.TypeConverterHolder;
import org.apache.struts2.conversion.StrutsTypeConverterHolder;
import org.apache.struts2.conversion.annotations.ConversionRule;
import org.apache.struts2.conversion.annotations.ConversionType;
import org.apache.struts2.util.ApplicationScopedWithoutKeyConversionAction;
import org.apache.struts2.util.BareKeyConversionAction;
import org.apache.struts2.util.CollidingKeyConversionAction;
import org.apache.struts2.util.EmptyKeyConversionAction;
import org.apache.struts2.util.ExplicitKeyConversionAction;
import org.apache.struts2.util.FieldConversionAction;
import org.apache.struts2.util.InheritedMethodConversionSubAction;
import org.apache.struts2.util.MyBean;
import org.apache.struts2.util.MyBeanAction;
import java.io.IOException;
import java.io.InputStream;
@@ -814,6 +827,172 @@ public class XWorkConverterTest extends XWorkTestCase {
assertEquals(converted, Arrays.asList(1, 2, 3));
}
public void testResolveKeyPrependsTheRulePrefix() {
assertEquals("KeyProperty_annotatedBeanMap",
XWorkConverter.resolveKey(ConversionType.CLASS, ConversionRule.KEY_PROPERTY, "annotatedBeanMap"));
assertEquals("Element_annotatedBeanList",
XWorkConverter.resolveKey(ConversionType.CLASS, ConversionRule.ELEMENT, "annotatedBeanList"));
assertEquals("CreateIfNull_users",
XWorkConverter.resolveKey(ConversionType.CLASS, ConversionRule.CREATE_IF_NULL, "users"));
}
public void testResolveKeyLeavesAnAlreadyPrefixedKeyAlone() {
assertEquals("KeyProperty_annotatedBeanMap",
XWorkConverter.resolveKey(ConversionType.CLASS, ConversionRule.KEY_PROPERTY, "KeyProperty_annotatedBeanMap"));
assertEquals("Key_beanMap",
XWorkConverter.resolveKey(ConversionType.CLASS, ConversionRule.KEY, "Key_beanMap"));
// COLLECTION and ELEMENT are interchangeable in DefaultConversionAnnotationProcessor and
// DefaultObjectTypeDeterminer, so a key already carrying either prefix must be left alone
// regardless of which of the two rules is declared.
assertEquals("Element_users",
XWorkConverter.resolveKey(ConversionType.CLASS, ConversionRule.COLLECTION, "Element_users"));
assertEquals("Collection_users",
XWorkConverter.resolveKey(ConversionType.CLASS, ConversionRule.ELEMENT, "Collection_users"));
}
public void testResolveKeyDoesNotPrefixPropertyOrMapRules() {
assertEquals("someProperty",
XWorkConverter.resolveKey(ConversionType.CLASS, ConversionRule.PROPERTY, "someProperty"));
assertEquals("keyValues",
XWorkConverter.resolveKey(ConversionType.CLASS, ConversionRule.MAP, "keyValues"));
}
public void testResolveKeyNeverPrefixesApplicationScopedKeys() {
assertEquals("java.util.Date",
XWorkConverter.resolveKey(ConversionType.APPLICATION, ConversionRule.PROPERTY, "java.util.Date"));
assertEquals("java.util.Date",
XWorkConverter.resolveKey(ConversionType.APPLICATION, ConversionRule.ELEMENT, "java.util.Date"));
}
public void testResolveKeyReturnsNullWhenNoNameIsAvailable() {
assertNull(XWorkConverter.resolveKey(ConversionType.CLASS, ConversionRule.PROPERTY, null));
assertNull(XWorkConverter.resolveKey(ConversionType.CLASS, ConversionRule.KEY, ""));
}
public void testExplicitMethodKeyGetsTheRulePrefix() {
XWorkConverter freshConverter = container.inject(XWorkConverter.class);
freshConverter.setTypeConverterHolder(new StrutsTypeConverterHolder());
assertEquals("true", freshConverter.getConverter(ExplicitKeyConversionAction.class, "CreateIfNull_bareList"));
assertNull(freshConverter.getConverter(ExplicitKeyConversionAction.class, "bareList"));
}
public void testClassLevelBareKeysGetTheRulePrefix() {
XWorkConverter freshConverter = container.inject(XWorkConverter.class);
freshConverter.setTypeConverterHolder(new StrutsTypeConverterHolder());
assertEquals("id", freshConverter.getConverter(BareKeyConversionAction.class, "KeyProperty_annotatedBeanMap"));
assertEquals(MyBean.class, freshConverter.getConverter(BareKeyConversionAction.class, "Element_annotatedBeanMap"));
assertEquals("id", freshConverter.getConverter(BareKeyConversionAction.class, "KeyProperty_annotatedBeanList"));
assertEquals(MyBean.class, freshConverter.getConverter(BareKeyConversionAction.class, "Element_annotatedBeanList"));
}
public void testClassLevelBareKeysMatchTheSpelledOutForm() {
XWorkConverter freshConverter = container.inject(XWorkConverter.class);
freshConverter.setTypeConverterHolder(new StrutsTypeConverterHolder());
for (String key : new String[]{"KeyProperty_annotatedBeanMap", "Element_annotatedBeanMap",
"KeyProperty_annotatedBeanList", "Element_annotatedBeanList"}) {
assertEquals("mismatch for " + key,
freshConverter.getConverter(MyBeanAction.class, key),
freshConverter.getConverter(BareKeyConversionAction.class, key));
}
}
public void testClassLevelEntriesAfterAKeyCollisionAreStillRegistered() {
XWorkConverter freshConverter = container.inject(XWorkConverter.class);
freshConverter.setTypeConverterHolder(new StrutsTypeConverterHolder());
// supplied by the -conversion.properties file, so the annotation must not overwrite it
assertEquals("true", freshConverter.getConverter(CollidingKeyConversionAction.class, "CreateIfNull_fromProperties"));
// the entry after the collision used to be dropped by `break`
assertEquals("true", freshConverter.getConverter(CollidingKeyConversionAction.class, "CreateIfNull_afterTheCollision"));
}
public void testClassLevelEmptyKeyRegistersNoMapping() throws Exception {
XWorkConverter freshConverter = container.inject(XWorkConverter.class);
freshConverter.setTypeConverterHolder(new StrutsTypeConverterHolder());
Map<String, Object> mapping = freshConverter.buildConverterMapping(EmptyKeyConversionAction.class);
assertFalse("an empty class-level key must not register a \"\" mapping", mapping.containsKey(""));
assertTrue("no mapping should have been registered at all", mapping.isEmpty());
}
public void testFieldLevelAnnotationDerivesKeyFromTheFieldName() {
XWorkConverter freshConverter = container.inject(XWorkConverter.class);
freshConverter.setTypeConverterHolder(new StrutsTypeConverterHolder());
assertEquals("true", freshConverter.getConverter(FieldConversionAction.class, "CreateIfNull_fieldOnlyList"));
}
public void testMethodAnnotationWinsOverFieldAnnotation() {
XWorkConverter freshConverter = container.inject(XWorkConverter.class);
freshConverter.setTypeConverterHolder(new StrutsTypeConverterHolder());
assertEquals(Long.class, freshConverter.getConverter(FieldConversionAction.class, "Key_contestedMap"));
}
/**
* {@code processMethodAnnotations} gates its WARN logging to the hierarchy level whose {@code
* clazz} matches {@code method.getDeclaringClass()}, so a misconfigured annotation on an
* inherited method logs once instead of once per subclass. Gating logging alone is only safe
* because registration still happens at the subclass level - before that subclass's own field
* pass runs - as documented on {@code XWorkConverter#processFieldAnnotations}. {@link
* InheritedMethodConversionSubAction} declares a contesting field annotation for the same
* property the inherited setter claims; this asserts the inherited method annotation still wins,
* which would break if registration were mistakenly gated along with logging.
*/
public void testInheritedMethodAnnotationStillRegistersThroughASubclass() {
XWorkConverter freshConverter = container.inject(XWorkConverter.class);
freshConverter.setTypeConverterHolder(new StrutsTypeConverterHolder());
assertEquals("true", freshConverter.getConverter(InheritedMethodConversionSubAction.class, "CreateIfNull_inheritedList"));
}
/**
* Wires a fresh {@link StrutsTypeConverterHolder} into both {@code freshConverter} and its
* {@link ConversionAnnotationProcessor}. The two are injected as independent singletons: an
* {@code APPLICATION}-scoped {@link TypeConversion} is registered by the annotation processor
* calling {@code TypeConverterHolder.addDefaultMapping} directly (bypassing the {@code mapping}
* map {@code XWorkConverter} caches per-class), so swapping only {@code XWorkConverter}'s own
* holder - as the other {@code freshConverter.setTypeConverterHolder(...)} tests in this file do
* for class/method/field mappings - would silently observe the shared container-wide holder
* instead of the one the test controls.
*/
private XWorkConverter freshConverterWithIsolatedHolder(TypeConverterHolder holder) {
XWorkConverter freshConverter = container.inject(XWorkConverter.class);
DefaultConversionAnnotationProcessor freshProcessor = container.inject(DefaultConversionAnnotationProcessor.class);
freshProcessor.setTypeConverterHolder(holder);
freshConverter.setConversionAnnotationProcessor(freshProcessor);
freshConverter.setTypeConverterHolder(holder);
return freshConverter;
}
public void testApplicationScopedMethodWithoutKeyRegistersNoDefaultMapping() {
StrutsTypeConverterHolder holder = new StrutsTypeConverterHolder();
XWorkConverter freshConverter = freshConverterWithIsolatedHolder(holder);
// forces the mapping to be built; the property itself does not need to resolve to anything
freshConverter.getConverter(ApplicationScopedWithoutKeyConversionAction.class, "anything");
assertFalse("a method-level APPLICATION conversion with no key must not register a default "
+ "mapping under the derived member name, which lookup(String,boolean)/lookup(Class) never read",
holder.containsDefaultMapping("applicationScopedMethod"));
}
public void testApplicationScopedFieldWithoutKeyRegistersNoDefaultMapping() {
StrutsTypeConverterHolder holder = new StrutsTypeConverterHolder();
XWorkConverter freshConverter = freshConverterWithIsolatedHolder(holder);
freshConverter.getConverter(ApplicationScopedWithoutKeyConversionAction.class, "anything");
assertFalse("a field-level APPLICATION conversion with no key must not register a default "
+ "mapping under the derived member name, which lookup(String,boolean)/lookup(Class) never read",
holder.containsDefaultMapping("applicationScopedField"));
}
public static class CountingXWorkConverter extends XWorkConverter {
final AtomicInteger builds = new AtomicInteger();
core/src/test/java/org/apache/struts2/util/ApplicationScopedWithoutKeyConversionAction.java
+54
View File
@@ -0,0 +1,54 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.struts2.util;
import org.apache.struts2.conversion.annotations.ConversionType;
import org.apache.struts2.conversion.annotations.TypeConversion;
/**
* An {@code APPLICATION}-scoped {@link TypeConversion} is stored in the global default converter
* map, which is keyed by class name. With no explicit {@code key}, a method or field pass would
* otherwise derive a member name (e.g. {@code applicationScopedMethod}) and register it there,
* where nothing can ever look it up. Both the method-level and field-level annotation below must
* be skipped rather than registered under their derived member names.
*/
public class ApplicationScopedWithoutKeyConversionAction {
@TypeConversion(type = ConversionType.APPLICATION)
private String applicationScopedField;
private String applicationScopedMethod;
public String getApplicationScopedField() {
return applicationScopedField;
}
public void setApplicationScopedField(String applicationScopedField) {
this.applicationScopedField = applicationScopedField;
}
public String getApplicationScopedMethod() {
return applicationScopedMethod;
}
@TypeConversion(type = ConversionType.APPLICATION)
public void setApplicationScopedMethod(String applicationScopedMethod) {
this.applicationScopedMethod = applicationScopedMethod;
}
}
core/src/test/java/org/apache/struts2/util/BareKeyConversionAction.java
+61
View File
@@ -0,0 +1,61 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.struts2.util;
import org.apache.struts2.conversion.annotations.Conversion;
import org.apache.struts2.conversion.annotations.ConversionRule;
import org.apache.struts2.conversion.annotations.TypeConversion;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
/**
* The class level counterpart of {@link MyBeanAction}, declaring the same four conversions with
* bare property names instead of spelled-out prefixes.
*/
@Conversion(
conversions = {
@TypeConversion(key = "annotatedBeanMap", rule = ConversionRule.KEY_PROPERTY, value = "id"),
@TypeConversion(key = "annotatedBeanMap", rule = ConversionRule.ELEMENT, converterClass = MyBean.class),
@TypeConversion(key = "annotatedBeanList", rule = ConversionRule.KEY_PROPERTY, value = "id"),
@TypeConversion(key = "annotatedBeanList", rule = ConversionRule.ELEMENT, converterClass = MyBean.class)
})
public class BareKeyConversionAction {
private Map annotatedBeanMap = new HashMap();
private List annotatedBeanList = new ArrayList();
public Map getAnnotatedBeanMap() {
return annotatedBeanMap;
}
public void setAnnotatedBeanMap(Map annotatedBeanMap) {
this.annotatedBeanMap = annotatedBeanMap;
}
public List getAnnotatedBeanList() {
return annotatedBeanList;
}
public void setAnnotatedBeanList(List annotatedBeanList) {
this.annotatedBeanList = annotatedBeanList;
}
}
core/src/test/java/org/apache/struts2/util/CollidingKeyConversionAction.java
+48
View File
@@ -0,0 +1,48 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.struts2.util;
import org.apache.struts2.conversion.annotations.Conversion;
import org.apache.struts2.conversion.annotations.ConversionRule;
import org.apache.struts2.conversion.annotations.TypeConversion;
import java.util.ArrayList;
import java.util.List;
/**
* The first conversion entry collides with a key already supplied by
* {@code CollidingKeyConversionAction-conversion.properties}; the second must still be registered.
*/
@Conversion(
conversions = {
@TypeConversion(key = "fromProperties", rule = ConversionRule.CREATE_IF_NULL, value = "false"),
@TypeConversion(key = "afterTheCollision", rule = ConversionRule.CREATE_IF_NULL, value = "true")
})
public class CollidingKeyConversionAction {
private List afterTheCollision = new ArrayList();
public List getAfterTheCollision() {
return afterTheCollision;
}
public void setAfterTheCollision(List afterTheCollision) {
this.afterTheCollision = afterTheCollision;
}
}
core/src/test/java/org/apache/struts2/util/EmptyKeyConversionAction.java
+34
View File
@@ -0,0 +1,34 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.struts2.util;
import org.apache.struts2.conversion.annotations.Conversion;
import org.apache.struts2.conversion.annotations.ConversionRule;
import org.apache.struts2.conversion.annotations.TypeConversion;
/**
* A class level {@link TypeConversion} has no property name to derive a key from, so an unset
* (empty) {@code key} must be skipped rather than registered under {@code ""}.
*/
@Conversion(
conversions = {
@TypeConversion(rule = ConversionRule.CREATE_IF_NULL, value = "true")
})
public class EmptyKeyConversionAction {
}
core/src/test/java/org/apache/struts2/util/ExplicitKeyConversionAction.java
+43
View File
@@ -0,0 +1,43 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.struts2.util;
import org.apache.struts2.conversion.annotations.ConversionRule;
import org.apache.struts2.conversion.annotations.TypeConversion;
import java.util.ArrayList;
import java.util.List;
/**
* Declares a method level {@link TypeConversion} with an explicit, unprefixed key. Before WW-3871
* this registered a bare {@code bareList} mapping that nothing ever read.
*/
public class ExplicitKeyConversionAction {
private List bareList = new ArrayList();
public List getBareList() {
return bareList;
}
@TypeConversion(key = "bareList", rule = ConversionRule.CREATE_IF_NULL, value = "true")
public void setBareList(List bareList) {
this.bareList = bareList;
}
}
core/src/test/java/org/apache/struts2/util/FieldConversionAction.java
+58
View File
@@ -0,0 +1,58 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.struts2.util;
import org.apache.struts2.conversion.annotations.ConversionRule;
import org.apache.struts2.conversion.annotations.TypeConversion;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
/**
* Exercises field level {@link TypeConversion}: {@code fieldOnlyList} is annotated on the field
* alone, while {@code contestedMap} is annotated on both the field and its setter so the
* class &gt; method &gt; field precedence can be asserted.
*/
public class FieldConversionAction {
@TypeConversion(rule = ConversionRule.CREATE_IF_NULL, value = "true")
private List fieldOnlyList = new ArrayList();
@TypeConversion(rule = ConversionRule.KEY, converterClass = String.class)
private Map contestedMap = new HashMap();
public List getFieldOnlyList() {
return fieldOnlyList;
}
public void setFieldOnlyList(List fieldOnlyList) {
this.fieldOnlyList = fieldOnlyList;
}
public Map getContestedMap() {
return contestedMap;
}
@TypeConversion(rule = ConversionRule.KEY, converterClass = Long.class)
public void setContestedMap(Map contestedMap) {
this.contestedMap = contestedMap;
}
}
core/src/test/java/org/apache/struts2/util/InheritedMethodConversionAction.java
+48
View File
@@ -0,0 +1,48 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.struts2.util;
import org.apache.struts2.conversion.annotations.ConversionRule;
import org.apache.struts2.conversion.annotations.TypeConversion;
import java.util.ArrayList;
import java.util.List;
/**
* Declares a bare-key, method level {@link TypeConversion} on a setter that {@link
* InheritedMethodConversionSubAction} inherits without overriding. {@code Class#getMethods()} on the
* subclass still returns this method, so
* {@link org.apache.struts2.conversion.impl.XWorkConverter#buildConverterMapping} visits it once per
* class in the hierarchy, registering it at the subclass level before that subclass's own field pass
* runs - used together with {@link InheritedMethodConversionSubAction}'s contesting field annotation
* to assert that precedence, independent of how many times the (gated) WARN logging fires.
*/
public class InheritedMethodConversionAction {
private List inheritedList = new ArrayList();
public List getInheritedList() {
return inheritedList;
}
@TypeConversion(rule = ConversionRule.CREATE_IF_NULL, value = "true")
public void setInheritedList(List inheritedList) {
this.inheritedList = inheritedList;
}
}
core/src/test/java/org/apache/struts2/util/InheritedMethodConversionSubAction.java
+41
View File
@@ -0,0 +1,41 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.struts2.util;
import org.apache.struts2.conversion.annotations.ConversionRule;
import org.apache.struts2.conversion.annotations.TypeConversion;
import java.util.List;
/**
* Extends {@link InheritedMethodConversionAction} without overriding its annotated setter, so {@code
* XWorkConverter.buildConverterMapping}'s walk up the hierarchy - this class, then its parent, then
* stopping at {@code Object} - visits the inherited annotated setter once per class it processes.
*
* <p>Declares its own {@code inheritedList} field, annotated with a contesting {@code
* CREATE_IF_NULL} value, for the same property the inherited setter already claims. This is used to
* assert that the inherited method annotation - registered at this subclass level, before this
* class's own field pass ever runs - keeps winning over the field annotation declared here, per the
* precedence documented on {@code XWorkConverter#processFieldAnnotations}.</p>
*/
public class InheritedMethodConversionSubAction extends InheritedMethodConversionAction {
@TypeConversion(rule = ConversionRule.CREATE_IF_NULL, value = "false")
private List inheritedList;
}
core/src/test/java/org/apache/struts2/util/MyBeanActionTest.java
+28
View File
@@ -126,6 +126,34 @@ public class MyBeanActionTest extends XWorkTestCase {
}
}
public void testBareConversionKeysBindTheSameWayAsPrefixedOnes() throws Exception {
HashMap<String, Object> params = new HashMap<>();
params.put("annotatedBeanList(1234567890).name", "This is the bla bean by annotation");
params.put("annotatedBeanMap[1234567891].id", "1234567891");
params.put("annotatedBeanMap[1234567891].name", "This is the 2nd bla bean by annotation");
ActionContext extraContext = ActionContext.of().withParameters(HttpParameters.create(params).build());
ActionProxy proxy = actionProxyFactory.createActionProxy("", "MyBeanBareKey", null, extraContext.getContextMap());
proxy.execute();
MyBeanBareKeyAction action = (MyBeanBareKeyAction) proxy.getInvocation().getAction();
// CreateIfNull_annotatedBeanList + Element_annotatedBeanList
assertEquals(1, action.getAnnotatedBeanList().size());
assertEquals(MyBean.class, action.getAnnotatedBeanList().get(0).getClass());
assertEquals("This is the bla bean by annotation",
proxy.getInvocation().getStack().findValue("annotatedBeanList.get(0).name"));
// KeyProperty_annotatedBeanList (value = "id"): the index used to address the list,
// 1234567890, is bound onto the created bean's own "id" property.
assertEquals(Long.valueOf(1234567890L), ((MyBean) action.getAnnotatedBeanList().get(0)).getId());
// Key_annotatedBeanMap makes the key a Long, Element_annotatedBeanMap makes the value a MyBean
assertTrue(action.getAnnotatedBeanMap().containsKey(1234567891L));
assertEquals(MyBean.class, action.getAnnotatedBeanMap().get(1234567891L).getClass());
assertEquals("This is the 2nd bla bean by annotation",
proxy.getInvocation().getStack().findValue("annotatedBeanMap.get(1234567891L).name"));
}
@Override
protected void setUp() throws Exception {
super.setUp();
core/src/test/java/org/apache/struts2/util/MyBeanBareKeyAction.java
+68
View File
@@ -0,0 +1,68 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.struts2.util;
import org.apache.struts2.action.Action;
import org.apache.struts2.conversion.annotations.Conversion;
import org.apache.struts2.conversion.annotations.ConversionRule;
import org.apache.struts2.conversion.annotations.TypeConversion;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
/**
* {@link MyBeanAction} restated with bare property names as conversion keys. Both must bind
* identically; {@code MyBeanAction} keeps the spelled-out prefixes so the old form stays covered.
*/
@Conversion(
conversions = {
@TypeConversion(key = "annotatedBeanMap", rule = ConversionRule.KEY_PROPERTY, value = "id"),
@TypeConversion(key = "annotatedBeanMap", rule = ConversionRule.ELEMENT, converterClass = MyBean.class),
@TypeConversion(key = "annotatedBeanList", rule = ConversionRule.KEY_PROPERTY, value = "id"),
@TypeConversion(key = "annotatedBeanList", rule = ConversionRule.ELEMENT, converterClass = MyBean.class)
})
public class MyBeanBareKeyAction implements Action {
private Map annotatedBeanMap = new HashMap();
private List annotatedBeanList = new ArrayList();
public Map getAnnotatedBeanMap() {
return annotatedBeanMap;
}
@TypeConversion(rule = ConversionRule.KEY, converterClass = Long.class)
public void setAnnotatedBeanMap(Map annotatedBeanMap) {
this.annotatedBeanMap = annotatedBeanMap;
}
public List getAnnotatedBeanList() {
return annotatedBeanList;
}
@TypeConversion(rule = ConversionRule.CREATE_IF_NULL, value = "true")
public void setAnnotatedBeanList(List annotatedBeanList) {
this.annotatedBeanList = annotatedBeanList;
}
public String execute() throws Exception {
return SUCCESS;
}
}
core/src/test/resources/org/apache/struts2/util/CollidingKeyConversionAction-conversion.properties
+19
View File
@@ -0,0 +1,19 @@
#
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
#
CreateIfNull_fromProperties=true
core/src/test/resources/xwork-sample.xml
+6
View File
@@ -131,6 +131,12 @@
<result name="success" type="mock"/>
</action>
<action name="MyBeanBareKey" class="org.apache.struts2.util.MyBeanBareKeyAction">
<interceptor-ref name="debugStack"/>
<interceptor-ref name="defaultStack"/>
<result name="success" type="mock"/>
</action>
<action name="TestInterceptorParam" class="org.apache.struts2.SimpleAction">
<interceptor-ref name="test">
<param name="expectedFoo">expectedFoo</param>
docs/superpowers/plans/2026-07-25-WW-3871-typeconversion-key-derivation.md
+1026
View File
File diff suppressed because it is too large Load Diff
docs/superpowers/specs/2026-07-25-WW-3871-typeconversion-key-derivation-design.md
+229
View File
@@ -0,0 +1,229 @@
# WW-3871 — `@TypeConversion` key derivation
**Ticket:** [WW-3871](https://issues.apache.org/jira/browse/WW-3871) — TypeConversion annotation support improvement
**Target version:** 7.3.0
**Date:** 2026-07-25
## Problem
The reporter asked that `@TypeConversion` build its own key from the property name once a
`ConversionRule` is given, instead of forcing:
```java
@TypeConversion(key = "CreateIfNull_users", rule = ConversionRule.CreateIfNull, value = "true")
```
Half of this already works. Commit `77cbafb74` (2018) taught `XWorkConverter` to derive the key for
**method-level** annotations: with no `key`, the property name is resolved from the method and the
rule's prefix is prepended (`XWorkConverter.java:526-545`).
Three gaps remain:
1. **Class-level `@Conversion(conversions = {...})` has no derivation at all.** The `key` is used
verbatim (`XWorkConverter.java:504-519`), so callers still spell out prefixes — see
`core/src/test/java/org/apache/struts2/util/MyBeanAction.java:38-41`.
2. **`@TypeConversion` is `@Target({METHOD})`**, while its own Javadoc claims it "can be applied at
property and method level" (`TypeConversion.java:51`).
3. **Explicit keys are never normalised.** `@TypeConversion(key = "foo", rule = CREATE_IF_NULL)` on a
setter registers `foo`, a mapping nothing reads — `DefaultObjectTypeDeterminer` looks up
`CreateIfNull_foo`.
Two defects live in the same code block and are fixed here rather than left behind:
- `break` where `continue` is meant (`XWorkConverter.java:508` and `:542`). One already-mapped key
aborts the **remaining** `@TypeConversion` entries in a `@Conversion` array.
- An empty class-level `key` registers a mapping under `""`. `DefaultConversionAnnotationProcessor`
guards only `null` (`:58`).
## Goals
- A bare property name in `key` works at class, method and field level, for every `ConversionRule`.
- Existing annotations that spell out the prefix keep working byte-for-byte.
- `@TypeConversion` becomes usable on fields, matching its documentation.
- The rule-to-prefix table lives in one place.
## Non-goals
- `ConversionRule.COLLECTION` / the `Collection_` prefix stays deprecated-as-is.
- `ConversionRule.MAP` keeps having no prefix of its own.
- No changes to `struts-conversion.properties` or `<Class>-conversion.properties` parsing. This
ticket is annotations only.
## Design
### 1. Rule-to-prefix mapping moves onto `ConversionRule`
`org.apache.struts2.conversion.annotations.ConversionRule` gains:
```java
public String prefix() {
return switch (this) {
case COLLECTION -> DefaultObjectTypeDeterminer.DEPRECATED_ELEMENT_PREFIX; // "Collection_"
case CREATE_IF_NULL -> DefaultObjectTypeDeterminer.CREATE_IF_NULL_PREFIX; // "CreateIfNull_"
case ELEMENT -> DefaultObjectTypeDeterminer.ELEMENT_PREFIX; // "Element_"
case KEY -> DefaultObjectTypeDeterminer.KEY_PREFIX; // "Key_"
case KEY_PROPERTY -> DefaultObjectTypeDeterminer.KEY_PROPERTY_PREFIX; // "KeyProperty_"
case PROPERTY, MAP -> "";
};
}
```
`PROPERTY` and `MAP` returning `""` preserves today's behaviour: `DefaultObjectTypeDeterminer` reads
map and collection metadata through the `Key_` and `Element_` keys, never through a `Map_` key.
The `annotations` package already depends on `conversion.impl` (`TypeConversion.converterClass()`
defaults to `XWorkBasicConverter.class`), so referencing the prefix constants adds no new coupling.
An exhaustive `switch` over the enum means a future rule cannot silently miss a prefix.
### 2. One key resolver in `XWorkConverter`
```java
static String resolveKey(ConversionType type, ConversionRule rule, String name) {
if (name == null || name.isEmpty()) {
return null; // caller skips the entry and logs WARN
}
if (type == ConversionType.APPLICATION) {
return name; // key is a class name, never prefixed
}
String prefix = rule.prefix();
if (name.startsWith(prefix)) {
return name;
}
for (ConversionRule other : ConversionRule.values()) {
String otherPrefix = other.prefix();
if (!otherPrefix.isEmpty() && name.startsWith(otherPrefix)) {
return name; // already carries a rule prefix; leave it exactly as written
}
}
return prefix + name;
}
```
It takes the two annotation attributes rather than the `TypeConversion` instance so it can be unit
tested directly — annotation instances are awkward to construct in a test.
All three annotation passes route both explicit keys and derived property names through this
function, so the three call sites cannot drift apart.
**The `APPLICATION` carve-out** is belt-and-braces: application-scope entries use the default
`PROPERTY` rule in practice, where the prefix is `""` anyway. Without it,
`@TypeConversion(type = APPLICATION, key = "java.util.Date", rule = ELEMENT)` would register
`Element_java.util.Date` into the global converter map, which nothing reads.
`APPLICATION`-scoped entries are stored in the global default converter map, which is keyed by
**class name** (`DefaultConversionAnnotationProcessor` calls `converterHolder.addDefaultMapping(key,
converter)`, and the only readers, `XWorkConverter.lookup(String, boolean)` and `lookup(Class)`, both
key off a class name). A method or field annotation has no class name to offer, only a member name,
so `resolveKey` cannot derive one: `processMethodAnnotations` and `processFieldAnnotations` skip an
`APPLICATION`-typed entry with an empty `key` before deriving a name, logging a WARN that names the
declaring class and the member. The class-level pass needs no equivalent guard - `key` is required
there for an unrelated reason (no member to derive a property name from at all), and an empty key is
already skipped by the existing null-from-`resolveKey` path.
**The "already prefixed" guard checks every rule's prefix, not just the declared rule's.** An
already-prefixed key is returned untouched, so `key = "KeyProperty_annotatedBeanMap"` and
`key = "annotatedBeanMap"` both resolve to `KeyProperty_annotatedBeanMap` under `rule = KEY_PROPERTY`.
Checking only the declared rule's prefix is not enough: `COLLECTION` and `ELEMENT` are interchangeable
in both `DefaultConversionAnnotationProcessor` (same branch handles both) and
`DefaultObjectTypeDeterminer.getElementClass` (reads `Element_<prop>`, falls back to the deprecated
`Collection_<prop>`), so `key = "Element_users", rule = COLLECTION` must not become
`Collection_Element_users`. Matching against every rule's prefix keeps that crossover working. It
misfires only for a property literally named `KeyProperty_foo` (or another prefix), which is legal
Java but effectively nonexistent; such a property gets exactly today's behaviour.
### 3. Field-level support
`@TypeConversion` becomes `@Target({ElementType.METHOD, ElementType.FIELD})`.
`addConverterMapping` splits into four ordered passes, keeping today's first-writer-wins rule:
```java
protected void addConverterMapping(Map<String, Object> mapping, Class clazz) {
fileProcessor.process(mapping, clazz, buildConverterFilename(clazz)); // 1
processClassLevelAnnotations(mapping, clazz); // 2
processMethodAnnotations(mapping, clazz); // 3
processFieldAnnotations(mapping, clazz); // 4 new
}
```
This yields the precedence **class > method > field**, which preserves current behaviour exactly:
class-level `@Conversion` already outranks methods, and field annotations — none of which exist
today — only fill gaps.
Extracting the three passes into named private methods is the targeted cleanup this ticket earns.
The current single method is roughly 45 lines of nested loops concealing the `break` defect, and a
fourth pass would push it past readable.
The field pass iterates `clazz.getDeclaredFields()` — declared, not inherited, because
`buildConverterMapping` already walks the class hierarchy and calls `addConverterMapping` per class.
It skips `static` and synthetic fields, which also makes the interface case a no-op
(`getDeclaredFields()` on an interface returns its constants). A field's own name is the property
name; no getter/setter parsing is involved.
### 4. Interaction with the 7.3.0 mapping cache
`addConverterMapping` runs inside the builder that `XWorkConverter.buildConverterMapping` hands to
`TypeConverterHolder.computeMappingIfAbsent` (WW-5539). That builder deliberately executes **outside**
any lock — `StrutsTypeConverterHolder.java:171-188` documents why: it instantiates, and under
`SpringObjectFactory` autowires, arbitrary user-supplied `TypeConverter`s, so running it under a
`ConcurrentHashMap` bin lock would risk `IllegalStateException("Recursive update")` or self-deadlock
if any of that re-enters conversion.
The new field pass instantiates converters exactly as the existing method pass does, so it inherits
that safety and adds no new hazard. The one consequence to respect: under first-access contention the
builder may run more than once for the same class, so every pass must stay free of side effects
outside the `mapping` map it is handed.
### 5. Error handling
| Situation | Today | After |
|---|---|---|
| Class-level entry with `key = ""` | registers a `""` mapping | skipped, `WARN` naming class and annotation |
| `@TypeConversion` on a non-property method (`execute()`) with no key | silently dropped at DEBUG | skipped, `WARN` naming class and method |
| Field annotation whose key a method already claimed | n/a | skipped, `DEBUG` — the documented precedence, made visible |
| Second entry in a `@Conversion` array after a key collision | **dropped** (`break`) | processed (`continue`) |
The class-level `mapping.containsKey(...)` check moves **after** key resolution; it currently tests
the raw key, which post-change would be the wrong string. `DefaultConversionAnnotationProcessor.process`
keeps its `key == null` guard as defence in depth even though callers no longer pass null.
## Testing
Both affected test classes extend `XWorkTestCase`, i.e. JUnit 3 style — new tests use
`public void testXxx()` with no `@Test` annotation, which would silently never run there.
**`XWorkConverterTest` / `AnnotationXWorkConverterTest`:**
- bare key + rule resolves to the prefixed key
- already-prefixed key + same rule is unchanged (idempotence)
- `PROPERTY` and `MAP` rules leave the key untouched
- `type = APPLICATION` leaves the key untouched regardless of rule
- empty key and non-property method produce no mapping entry, and specifically no `""` key
**`MyBeanActionTest` — end-to-end:** `MyBeanAction` keeps its four spelled-out class-level prefixes
untouched, which is what proves existing applications don't break. A second fixture action declares
the same four conversions with **bare** keys; the test asserts both produce identical converter
mappings and identical bound results.
**Field support:** a fixture with `@TypeConversion` on a private field, asserting its derived key
matches the setter form's, plus a test that a method annotation wins when both a field and its
setter are annotated for the same key.
**`continue` regression:** a fixture whose `@Conversion` array has an early entry colliding with a
key already in the mapping, asserting the later entries still register. This regression is currently
invisible.
## Documentation
- `TypeConversion` Javadoc: the parameter table's `key` row gains the derivation rule (it currently
says only "Defaults to the property name", which understates it); the class-level example drops its
now-redundant prefixes; an `@since 7.3.0` note records that bare keys are accepted at class and
field level, and that fields are a supported target.
- `ConversionRule` Javadoc: document `prefix()` and which rules have none.
## Compatibility
Source- and binary-compatible. The only behavioural change to existing code is that an explicit
method-level key carrying a non-`PROPERTY` rule without its prefix now resolves to the prefixed key.
That mapping is unreachable today, so the change turns a silent no-op into the behaviour the author
intended.