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 a1c2193af4f6f636289ce8e62bcd36257e6cea06 Author: Lukasz Lenart <[email protected]> AuthorDate: Sat Jul 25 17:11:36 2026 +0200 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. --- .../conversion/impl/XWorkConverterTest.java | 19 ++++++++++++ .../struts2/util/EmptyKeyConversionAction.java | 34 ++++++++++++++++++++++ .../org/apache/struts2/util/MyBeanActionTest.java | 3 ++ 3 files changed, 56 insertions(+) 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 1ca4bc161..db16433b1 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 @@ -43,6 +43,7 @@ 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.EmptyKeyConversionAction; import org.apache.struts2.util.ExplicitKeyConversionAction; import org.apache.struts2.util.FieldConversionAction; import org.apache.struts2.util.MyBean; @@ -836,6 +837,14 @@ public class XWorkConverterTest extends XWorkTestCase { 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() { @@ -897,6 +906,16 @@ public class XWorkConverterTest extends XWorkTestCase { 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()); diff --git a/core/src/test/java/org/apache/struts2/util/EmptyKeyConversionAction.java b/core/src/test/java/org/apache/struts2/util/EmptyKeyConversionAction.java new file mode 100644 index 000000000..267df549b --- /dev/null +++ b/core/src/test/java/org/apache/struts2/util/EmptyKeyConversionAction.java @@ -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 { +} diff --git a/core/src/test/java/org/apache/struts2/util/MyBeanActionTest.java b/core/src/test/java/org/apache/struts2/util/MyBeanActionTest.java index 3f19efbfe..bd9d1da44 100644 --- a/core/src/test/java/org/apache/struts2/util/MyBeanActionTest.java +++ b/core/src/test/java/org/apache/struts2/util/MyBeanActionTest.java @@ -143,6 +143,9 @@ public class MyBeanActionTest extends XWorkTestCase { 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));
