[
https://issues.apache.org/jira/browse/WW-3871?focusedWorklogId=1032237&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1032237
]
ASF GitHub Bot logged work on WW-3871:
--------------------------------------
Author: ASF GitHub Bot
Created on: 25/Jul/26 19:12
Start Date: 25/Jul/26 19:12
Worklog Time Spent: 10m
Work Description: lukaszlenart commented on code in PR #1812:
URL: https://github.com/apache/struts/pull/1812#discussion_r3650845617
##########
core/src/main/java/org/apache/struts2/conversion/impl/XWorkConverter.java:
##########
@@ -498,51 +537,103 @@ protected void addConverterMapping(Map<String, Object>
mapping, Class clazz) {
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) {
- 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);
+ for (Annotation annotation : method.getAnnotations()) {
+ if (!(annotation instanceof TypeConversion tc)) {
+ continue;
+ }
+ String name = StringUtils.isEmpty(tc.key()) ?
AnnotationUtils.resolvePropertyName(method) : tc.key();
+ String key = resolveKey(tc.type(), tc.rule(), name);
+ if (key == null) {
Review Comment:
Correct, and fixed in c3f9b0771 — the method pass now skips an
APPLICATION-scoped `@TypeConversion` with no explicit key and logs a WARN
naming the declaring class and method.
Worth recording that this path is not new: on `main` the method pass also
fell through to the unprefixed property name for `APPLICATION`, so the
unreachable entry predates this branch. Folding the fix in anyway, since this
PR is what made the key-resolution rules explicit and the spec already states
that APPLICATION keys are class names.
Verified the mechanism before changing anything: `lookup(String, boolean)`
is only ever reached with `clazz.getName()` (`XWorkConverter:315,318` →
`:404-405`), so a `convertInt`-style key in the default map is indeed
unreadable.
Issue Time Tracking
-------------------
Worklog Id: (was: 1032237)
Time Spent: 40m (was: 0.5h)
> TypeConversion annotation support improvement
> ---------------------------------------------
>
> Key: WW-3871
> URL: https://issues.apache.org/jira/browse/WW-3871
> Project: Struts 2
> Issue Type: Improvement
> Components: Plugin - Convention
> Affects Versions: 2.3.4.1
> Reporter: Pavan Ananth
> Assignee: Lukasz Lenart
> Priority: Major
> Fix For: 7.3.0
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> The annotation support for TypeConversion in Struts 2 is too literal an
> interpretation of the XML support. For instance, I am required to supply this
> if I have choose the CreateIfNull feature at a property level :
> @TypeConversion(key="CreateIfNull_users", rule=ConversionRule.CreateIfNull,
> value="true")
> List<User> users;
> Given that the rule is CreateIfNull, the key can be constructed implicitly
> using property name - why is it asked of an user to type in the full key.
> This holds good for the other supported ConversionRules as well
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
