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.
--
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]
