This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch WW-3871-typeconversion-key-derivation in repository https://gitbox.apache.org/repos/asf/struts.git
commit b487d6640cef469a1cbb60385c34a5fdba9715a1 Author: Lukasz Lenart <[email protected]> AuthorDate: Sat Jul 25 15:26:50 2026 +0200 WW-3871 fix(core): derive class level conversion keys and stop dropping later entries --- .../struts2/conversion/impl/XWorkConverter.java | 28 +++++----- .../conversion/impl/XWorkConverterTest.java | 36 +++++++++++++ .../struts2/util/BareKeyConversionAction.java | 61 ++++++++++++++++++++++ .../struts2/util/CollidingKeyConversionAction.java | 48 +++++++++++++++++ ...lidingKeyConversionAction-conversion.properties | 19 +++++++ 5 files changed, 179 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/conversion/impl/XWorkConverter.java b/core/src/main/java/org/apache/struts2/conversion/impl/XWorkConverter.java index b3aa0e473..b0787735a 100644 --- a/core/src/main/java/org/apache/struts2/conversion/impl/XWorkConverter.java +++ b/core/src/main/java/org/apache/struts2/conversion/impl/XWorkConverter.java @@ -532,20 +532,22 @@ public class XWorkConverter extends DefaultTypeConverter { */ private void processClassLevelAnnotations(Map<String, Object> mapping, Class clazz) { for (Annotation annotation : clazz.getAnnotations()) { - 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()); + 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); } } } diff --git a/core/src/test/java/org/apache/struts2/conversion/impl/XWorkConverterTest.java b/core/src/test/java/org/apache/struts2/conversion/impl/XWorkConverterTest.java index 5b3fea454..b791fc941 100644 --- a/core/src/test/java/org/apache/struts2/conversion/impl/XWorkConverterTest.java +++ b/core/src/test/java/org/apache/struts2/conversion/impl/XWorkConverterTest.java @@ -41,7 +41,11 @@ import org.apache.struts2.conversion.TypeConverter; 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.BareKeyConversionAction; +import org.apache.struts2.util.CollidingKeyConversionAction; import org.apache.struts2.util.ExplicitKeyConversionAction; +import org.apache.struts2.util.MyBean; +import org.apache.struts2.util.MyBeanAction; import java.io.IOException; import java.io.InputStream; @@ -860,6 +864,38 @@ public class XWorkConverterTest extends XWorkTestCase { 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 static class CountingXWorkConverter extends XWorkConverter { final AtomicInteger builds = new AtomicInteger(); diff --git a/core/src/test/java/org/apache/struts2/util/BareKeyConversionAction.java b/core/src/test/java/org/apache/struts2/util/BareKeyConversionAction.java new file mode 100644 index 000000000..de8e3152c --- /dev/null +++ b/core/src/test/java/org/apache/struts2/util/BareKeyConversionAction.java @@ -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; + } +} diff --git a/core/src/test/java/org/apache/struts2/util/CollidingKeyConversionAction.java b/core/src/test/java/org/apache/struts2/util/CollidingKeyConversionAction.java new file mode 100644 index 000000000..e8d30f91d --- /dev/null +++ b/core/src/test/java/org/apache/struts2/util/CollidingKeyConversionAction.java @@ -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; + } +} diff --git a/core/src/test/resources/org/apache/struts2/util/CollidingKeyConversionAction-conversion.properties b/core/src/test/resources/org/apache/struts2/util/CollidingKeyConversionAction-conversion.properties new file mode 100644 index 000000000..f03453294 --- /dev/null +++ b/core/src/test/resources/org/apache/struts2/util/CollidingKeyConversionAction-conversion.properties @@ -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
