This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/struts.git
commit aaba012838148fc6addd98934d067eea9162d7f2 Author: Lukasz Lenart <[email protected]> AuthorDate: Mon Oct 26 09:19:44 2026 +0100 WW-5819 feat(validator): log a validator param that fails type conversion A validator param that cannot be converted to its property type is still left unset, but is now logged with the validator class, param, value and cause: WARN by default, ERROR in devMode. Params without a writable property (e.g. the annotation builder's methodName routing key) stay silent, as before. Co-Authored-By: Claude Opus 5.5 <[email protected]> --- .../struts2/factory/DefaultValidatorFactory.java | 41 ++++++++- .../factory/DefaultValidatorFactoryTest.java | 98 ++++++++++++++++++++++ .../java/org/apache/struts2/test/LogCapture.java | 11 +++ 3 files changed, 149 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/struts2/factory/DefaultValidatorFactory.java b/core/src/main/java/org/apache/struts2/factory/DefaultValidatorFactory.java index d781e72ad..9d4dd9517 100644 --- a/core/src/main/java/org/apache/struts2/factory/DefaultValidatorFactory.java +++ b/core/src/main/java/org/apache/struts2/factory/DefaultValidatorFactory.java @@ -18,11 +18,20 @@ */ package org.apache.struts2.factory; +import org.apache.commons.lang3.BooleanUtils; +import org.apache.commons.lang3.exception.ExceptionUtils; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.apache.struts2.ObjectFactory; +import org.apache.struts2.StrutsConstants; import org.apache.struts2.inject.Inject; +import org.apache.struts2.util.reflection.ReflectionException; import org.apache.struts2.util.reflection.ReflectionProvider; import org.apache.struts2.validator.Validator; +import java.beans.IntrospectionException; +import java.beans.PropertyDescriptor; import java.util.Map; /** @@ -30,8 +39,11 @@ import java.util.Map; */ public class DefaultValidatorFactory implements ValidatorFactory { + private static final Logger LOG = LogManager.getLogger(DefaultValidatorFactory.class); + private ObjectFactory objectFactory; private ReflectionProvider reflectionProvider; + private boolean devMode; @Inject public void setObjectFactory(ObjectFactory objectFactory) { @@ -43,10 +55,37 @@ public class DefaultValidatorFactory implements ValidatorFactory { this.reflectionProvider = reflectionProvider; } + @Inject(value = StrutsConstants.STRUTS_DEVMODE, required = false) + public void setDevMode(String devMode) { + this.devMode = BooleanUtils.toBoolean(devMode); + } + public Validator buildValidator(String className, Map<String, Object> params, Map<String, Object> extraContext) throws Exception { Validator validator = (Validator) objectFactory.buildBean(className, extraContext); - reflectionProvider.setProperties(params, validator, extraContext); + if (params != null) { + params.forEach((name, value) -> setParam(className, validator, name, value, extraContext)); + } return validator; } + + private void setParam(String className, Validator validator, String name, Object value, Map<String, Object> extraContext) { + try { + reflectionProvider.setProperty(name, value, validator, extraContext, true); + } catch (ReflectionException e) { + if (hasWritableProperty(validator, name)) { + LOG.log(devMode ? Level.ERROR : Level.WARN, "Validator [{}] ignores param [{}]: value [{}] cannot be converted ({})", + className, name, value, ExceptionUtils.getRootCauseMessage(e)); + } + } + } + + private boolean hasWritableProperty(Validator validator, String name) { + try { + PropertyDescriptor descriptor = reflectionProvider.getPropertyDescriptor(validator.getClass(), name); + return descriptor != null && descriptor.getWriteMethod() != null; + } catch (IntrospectionException | ReflectionException e) { + return false; + } + } } diff --git a/core/src/test/java/org/apache/struts2/factory/DefaultValidatorFactoryTest.java b/core/src/test/java/org/apache/struts2/factory/DefaultValidatorFactoryTest.java new file mode 100644 index 000000000..f9bdf3ff1 --- /dev/null +++ b/core/src/test/java/org/apache/struts2/factory/DefaultValidatorFactoryTest.java @@ -0,0 +1,98 @@ +/* + * 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.factory; + +import org.apache.logging.log4j.Level; +import org.apache.struts2.StrutsConstants; +import org.apache.struts2.StrutsInternalTestCase; +import org.apache.struts2.test.LogCapture; +import org.apache.struts2.validator.ValidatorConfig; +import org.apache.struts2.validator.ValidatorFactory; +import org.apache.struts2.validator.validators.LocalDateRangeFieldValidator; + +import java.time.LocalDate; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; + +public class DefaultValidatorFactoryTest extends StrutsInternalTestCase { + + private ValidatorFactory validatorFactory; + + @Override + protected void setUp() throws Exception { + super.setUp(); + validatorFactory = container.getInstance(ValidatorFactory.class); + } + + private LocalDateRangeFieldValidator localDateValidator(String min) { + ValidatorConfig config = new ValidatorConfig.Builder("localdate") + .addParam("fieldName", "birthday") + .addParam("min", min) + .build(); + return (LocalDateRangeFieldValidator) validatorFactory.getValidator(config); + } + + public void testUnconvertibleParamIsLoggedAsWarningAndValidatorIsStillBuilt() { + try (LogCapture logs = new LogCapture(DefaultValidatorFactory.class)) { + LocalDateRangeFieldValidator validator = localDateValidator("01/02/2026"); + + assertThat(validator.getMin()).isNull(); + assertThat(validator.getFieldName()).isEqualTo("birthday"); + assertThat(logs.messages()).hasSize(1); + assertThat(logs.messages(Level.WARN)).singleElement().asString().contains( + LocalDateRangeFieldValidator.class.getName(), "[min]", "[01/02/2026]", "cannot be converted"); + } + } + + public void testUnconvertibleParamIsLoggedAsErrorInDevMode() { + initDispatcher(Map.of(StrutsConstants.STRUTS_DEVMODE, "true")); + validatorFactory = container.getInstance(ValidatorFactory.class); + + try (LogCapture logs = new LogCapture(DefaultValidatorFactory.class)) { + LocalDateRangeFieldValidator validator = localDateValidator("01/02/2026"); + + assertThat(validator.getMin()).isNull(); + assertThat(logs.messages()).hasSize(1); + assertThat(logs.messages(Level.ERROR)).singleElement().asString().contains("[min]", "[01/02/2026]"); + } + } + + public void testConvertibleParamIsNotLogged() { + try (LogCapture logs = new LogCapture(DefaultValidatorFactory.class)) { + LocalDateRangeFieldValidator validator = localDateValidator("2026-02-01"); + + assertThat(validator.getMin()).isEqualTo(LocalDate.of(2026, 2, 1)); + assertThat(logs.messages()).isEmpty(); + } + } + + public void testParamWithoutPropertyIsNotLogged() { + ValidatorConfig config = new ValidatorConfig.Builder("required") + .addParam("fieldName", "name") + .addParam("methodName", "execute") + .build(); + + try (LogCapture logs = new LogCapture(DefaultValidatorFactory.class)) { + validatorFactory.getValidator(config); + + assertThat(logs.messages()).isEmpty(); + } + } +} diff --git a/core/src/test/java/org/apache/struts2/test/LogCapture.java b/core/src/test/java/org/apache/struts2/test/LogCapture.java index 58d021594..2fa33bca5 100644 --- a/core/src/test/java/org/apache/struts2/test/LogCapture.java +++ b/core/src/test/java/org/apache/struts2/test/LogCapture.java @@ -18,6 +18,7 @@ */ package org.apache.struts2.test; +import org.apache.logging.log4j.Level; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.core.LogEvent; import org.apache.logging.log4j.core.Logger; @@ -26,6 +27,7 @@ import org.apache.logging.log4j.core.config.Property; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.stream.IntStream; /** * Collects what a class logs while open, as the formatted message followed by the exception, if any. Only levels @@ -36,6 +38,7 @@ public class LogCapture implements AutoCloseable { private final Logger logger; private final AbstractAppender appender; private final List<String> messages = new CopyOnWriteArrayList<>(); + private final List<Level> levels = new CopyOnWriteArrayList<>(); public LogCapture(Class<?> source) { logger = (Logger) LogManager.getLogger(source); @@ -43,6 +46,7 @@ public class LogCapture implements AutoCloseable { @Override public void append(LogEvent event) { String message = event.getMessage().getFormattedMessage(); + levels.add(event.getLevel()); messages.add(event.getThrown() == null ? message : message + " " + event.getThrown()); } }; @@ -54,6 +58,13 @@ public class LogCapture implements AutoCloseable { return messages; } + public List<String> messages(Level level) { + return IntStream.range(0, messages.size()) + .filter(i -> levels.get(i) == level) + .mapToObj(messages::get) + .toList(); + } + @Override public void close() { logger.removeAppender(appender);
