From 3b1a5a9d00f96e081325f872dae654296d1824fb Mon Sep 17 00:00:00 2001 From: Rain Ramm Date: Wed, 14 Jan 2026 17:56:08 +0200 Subject: [PATCH] Fix FieldNameResolver bug dealing with @RequestParam --- .../ee/bitweb/core/api/FieldNameResolver.java | 35 ++++---- .../core/api/FieldNameResolverTest.java | 88 +++++++++++++++++++ 2 files changed, 108 insertions(+), 15 deletions(-) create mode 100644 src/test/java/ee/bitweb/core/api/FieldNameResolverTest.java diff --git a/src/main/java/ee/bitweb/core/api/FieldNameResolver.java b/src/main/java/ee/bitweb/core/api/FieldNameResolver.java index 07878b3..5e32f70 100644 --- a/src/main/java/ee/bitweb/core/api/FieldNameResolver.java +++ b/src/main/java/ee/bitweb/core/api/FieldNameResolver.java @@ -1,8 +1,6 @@ package ee.bitweb.core.api; - import jakarta.validation.ConstraintViolation; -import jakarta.validation.ElementKind; import jakarta.validation.Path; import lombok.AccessLevel; import lombok.NoArgsConstructor; @@ -10,15 +8,12 @@ import org.hibernate.validator.internal.engine.path.NodeImpl; import org.hibernate.validator.internal.engine.path.PathImpl; -import java.util.EnumSet; - @NoArgsConstructor(access = AccessLevel.PRIVATE) public class FieldNameResolver { private static final String INDEX_OPEN = "["; private static final String INDEX_CLOSE = "]"; private static final String FIELD_NAME_DELIMITER = "."; - private static final EnumSet IGNORED_ELEMENTS = EnumSet.of(ElementKind.METHOD, ElementKind.PARAMETER); public static String resolve(ConstraintViolation error) { if (error instanceof ConstraintViolationImpl violationImpl @@ -32,23 +27,33 @@ public static String resolve(ConstraintViolation error) { private static String resolveFieldName(PathImpl path) { StringBuilder builder = new StringBuilder(); + String parameterName = null; + for (Path.Node node : path) { - if (!(node instanceof NodeImpl nodeImpl) || IGNORED_ELEMENTS.contains(node.getKind())) { + if (!(node instanceof NodeImpl nodeImpl)) { continue; } - if (nodeImpl.isInIterable()) { - builder.append(INDEX_OPEN); - builder.append(nodeImpl.getIndex()); - builder.append(INDEX_CLOSE); - } - if (!builder.isEmpty()) { - builder.append(FIELD_NAME_DELIMITER); + switch (node.getKind()) { + case PARAMETER -> parameterName = nodeImpl.getName(); + case METHOD -> { + // Skip methods + } + default -> appendNode(builder, nodeImpl); } - builder.append(nodeImpl.getName()); } - return builder.toString(); + return builder.isEmpty() && parameterName != null ? parameterName : builder.toString(); + } + + private static void appendNode(StringBuilder builder, NodeImpl nodeImpl) { + if (nodeImpl.isInIterable()) { + builder.append(INDEX_OPEN).append(nodeImpl.getIndex()).append(INDEX_CLOSE); + } + if (!builder.isEmpty()) { + builder.append(FIELD_NAME_DELIMITER); + } + builder.append(nodeImpl.getName()); } public static String resolveWithRegex(ConstraintViolation error) { diff --git a/src/test/java/ee/bitweb/core/api/FieldNameResolverTest.java b/src/test/java/ee/bitweb/core/api/FieldNameResolverTest.java new file mode 100644 index 0000000..393f8ea --- /dev/null +++ b/src/test/java/ee/bitweb/core/api/FieldNameResolverTest.java @@ -0,0 +1,88 @@ +package ee.bitweb.core.api; + +import jakarta.validation.ConstraintViolation; +import jakarta.validation.Validation; +import jakarta.validation.Validator; +import jakarta.validation.ValidatorFactory; +import jakarta.validation.constraints.NotEmpty; +import jakarta.validation.executable.ExecutableValidator; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.lang.reflect.Method; +import java.util.Collections; +import java.util.List; +import java.util.Set; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; + +class FieldNameResolverTest { + + private static ExecutableValidator executableValidator; + + @BeforeAll + static void setUp() { + ValidatorFactory factory = Validation.buildDefaultValidatorFactory(); + Validator validator = factory.getValidator(); + executableValidator = validator.forExecutables(); + } + + @Test + @DisplayName("resolve() should return parameter name for @RequestParam validation errors") + void resolveShouldReturnParameterNameForRequestParamValidation() throws NoSuchMethodException { + // Simulate validation of a method parameter like @RequestParam @NotEmpty List items + Method method = TestController.class.getMethod("endpoint", List.class); + Object[] parameterValues = { Collections.emptyList() }; + + Set> violations = executableValidator.validateParameters( + new TestController(), + method, + parameterValues + ); + + assertFalse(violations.isEmpty(), "Should have validation violations"); + + ConstraintViolation violation = violations.iterator().next(); + + // The bug: FieldNameResolver.resolve() returns empty string for @RequestParam validation + // because the path only contains METHOD and PARAMETER nodes, both of which are ignored + String fieldName = FieldNameResolver.resolve(violation); + + // Expected: "items" (the parameter name) + // Actual (bug): "" (empty string) + assertEquals("items", fieldName, "Field name should be the parameter name 'items'"); + } + + @Test + @DisplayName("resolveWithRegex() should return parameter name for @RequestParam validation errors") + void resolveWithRegexShouldReturnParameterNameForRequestParamValidation() throws NoSuchMethodException { + Method method = TestController.class.getMethod("endpoint", List.class); + Object[] parameterValues = { Collections.emptyList() }; + + Set> violations = executableValidator.validateParameters( + new TestController(), + method, + parameterValues + ); + + assertFalse(violations.isEmpty(), "Should have validation violations"); + + ConstraintViolation violation = violations.iterator().next(); + + String fieldName = FieldNameResolver.resolveWithRegex(violation); + + assertEquals("items", fieldName, "Field name should be the parameter name 'items'"); + } + + /** + * Test controller class to simulate @RequestParam validation scenario + */ + static class TestController { + + public void endpoint(@NotEmpty List items) { + // Method simulating: @GetMapping public void endpoint(@RequestParam @NotEmpty List items) + } + } +}