When possible, improve error messages on ENUM fields in request bodies (#101)

* Better error on JSON bodies
* Trim quotes off value
* Defensively code groups
* Assume value is quoted

Author: wied03

build.savant

@@ -29,7 +29,7 @@ logbackVersion = "1.5.13"
 slf4jVersion = "2.0.13"
 testngVersion = "7.8.0"
 
-project(group: "org.primeframework", name: "prime-mvc", version: "5.7.0", licenses: ["ApacheV2_0"]) {
+project(group: "org.primeframework", name: "prime-mvc", version: "5.8.0", licenses: ["ApacheV2_0"]) {
   workflow {
     fetch {
       // Dependency resolution order:

src/main/java/org/primeframework/mvc/content/json/BaseJacksonContentHandler.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013-2023, Inversoft Inc., All Rights Reserved
+ * Copyright (c) 2013-2025, Inversoft Inc., All Rights Reserved
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -20,6 +20,8 @@
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import com.fasterxml.jackson.core.JsonParseException;
 import com.fasterxml.jackson.core.JsonProcessingException;
@@ -53,6 +55,9 @@
  * @author Brian Pontarelli
  */
 public abstract class BaseJacksonContentHandler implements ContentHandler {
+  private static final Pattern invalidEnumerationValue = Pattern.compile("Cannot deserialize value of type `\\S+` from \\S+ \"(\\S+)\": not one of the values accepted for Enum class: \\[(.*)]$.*",
+                                                                         Pattern.DOTALL | Pattern.MULTILINE);
+
   private static final Logger logger = LoggerFactory.getLogger(BaseJacksonContentHandler.class);
 
   protected final ExpressionEvaluator expressionEvaluator;
@@ -194,6 +199,35 @@ private void addFieldError(JsonMappingException e) {
     }
   }
 
+  private void addFieldError(InvalidFormatException e) {
+    // Build the path so we can make the error
+    String field = buildField(e);
+    String code = "[invalidJSON]";
+
+    // If we cannot find the field, make this a general error.
+    if (field.equals("")) {
+      messageStore.add(new SimpleMessage(MessageType.ERROR, code, messageProvider.getMessage(code, "unknown", "Possible conversion error", e.getMessage())));
+    } else {
+      String messageText = e.getMessage();
+      Matcher matchesEnumNotValidValue = invalidEnumerationValue.matcher(messageText);
+      String message = null;
+      if (matchesEnumNotValidValue.matches() && matchesEnumNotValidValue.groupCount() == 2) {
+        code = "[invalid]%s".formatted(field);
+        // if we have an invalid enum value, provide a better message
+        String possibleValues = matchesEnumNotValidValue.group(2);
+        String valueUsed = matchesEnumNotValidValue.group(1);
+        message = messageProvider.getMessage(code,
+                                             valueUsed,
+                                             possibleValues);
+      }
+      // otherwise, fall back to what we know
+      if (message == null) {
+        message = messageProvider.getMessage(code, field, "Possible conversion error", messageText);
+      }
+      messageStore.add(new SimpleFieldMessage(MessageType.ERROR, field, code, message));
+    }
+  }
+
   private String buildField(JsonMappingException e) {
     StringBuilder fieldBuilder = new StringBuilder();
     List<JsonMappingException.Reference> references = e.getPath();

src/test/java/org/example/domain/UserField.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001-2017, Inversoft Inc., All Rights Reserved
+ * Copyright (c) 2001-2025, Inversoft Inc., All Rights Reserved
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -21,6 +21,8 @@
 import java.util.Locale;
 import java.util.Map;
 
+import org.example.action.ParameterHandlerAction.Fruit;
+
 /**
  * This is a test user with fields.
  *
@@ -39,6 +41,8 @@ public class UserField {
 
   public Integer favoriteYear;
 
+  public Fruit fruit;
+
   public Integer id;
 
   public Map<Integer, Integer> ids = new HashMap<>();
@@ -49,6 +53,8 @@ public class UserField {
 
   public String name;
 
+  public Nested nested;
+
   public String password;
 
   public String[] securityQuestions;
@@ -63,4 +69,8 @@ public UserField() {
   public UserField(String name) {
     this.name = name;
   }
+
+  public static class Nested {
+    public Fruit fruit;
+  }
 }

src/test/java/org/primeframework/mvc/content/json/JacksonContentHandlerTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001-2024, Inversoft Inc., All Rights Reserved
+ * Copyright (c) 2001-2025, Inversoft Inc., All Rights Reserved
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -41,6 +41,7 @@
 import org.primeframework.mvc.message.l10n.MessageProvider;
 import org.primeframework.mvc.parameter.el.ExpressionEvaluator;
 import org.primeframework.mvc.validation.ValidationException;
+import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 import static org.easymock.EasyMock.createNiceMock;
 import static org.easymock.EasyMock.createStrictMock;
@@ -62,6 +63,71 @@
 public class JacksonContentHandlerTest extends PrimeBaseTest {
   @Inject public ExpressionEvaluator expressionEvaluator;
 
+  @DataProvider(name = "trueFalse")
+  private static Object[][] getTrueFalse() {
+    return new Object[][]{
+        {true},
+        {false}
+    };
+  }
+
+  @Test(dataProvider = "trueFalse")
+  public void enum_values(boolean nested) throws IOException {
+    Map<Class<?>, Object> additionalConfig = new HashMap<>();
+    Map<HTTPMethod, RequestMember> requestMembers = new HashMap<>();
+    requestMembers.put(HTTPMethod.POST, new RequestMember("jsonRequest", UserField.class));
+    additionalConfig.put(JacksonActionConfiguration.class, new JacksonActionConfiguration(requestMembers, null, null));
+
+    KitchenSinkAction action = new KitchenSinkAction(null);
+    ActionConfiguration config = new ActionConfiguration(KitchenSinkAction.class, false, null, null, null, null, null, null, null, null, null, null, null, null, null, null, Collections.emptyList(), null, additionalConfig, null, null, null, null, null);
+    ActionInvocationStore store = createStrictMock(ActionInvocationStore.class);
+    expect(store.getCurrent()).andReturn(
+        new ActionInvocation(action, new ExecuteMethodConfiguration(HTTPMethod.POST, null, null), "/action", null, config));
+    replay(store);
+
+    String expected = nested ? """
+        {
+          "nested": {
+            "fruit": "bar"
+          }
+        }
+        """ : """
+        {
+          "fruit": "foo"
+        }
+        """;
+
+    HTTPRequest request = new HTTPRequest();
+    request.setInputStream(new ByteArrayInputStream(expected.getBytes()));
+    request.setContentLength((long) expected.getBytes().length);
+    request.setContentType("application/json");
+
+    MessageProvider messageProvider = createStrictMock(MessageProvider.class);
+    expect(messageProvider.getMessage(eq(nested ? "[invalid]nested.fruit" : "[invalid]fruit"),
+                                      eq(nested ? "bar" : "foo"),
+                                      eq("Apple, Orange"))).andReturn("Bad value");
+    replay(messageProvider);
+
+    MessageStore messageStore = createStrictMock(MessageStore.class);
+    messageStore.add(new SimpleFieldMessage(MessageType.ERROR,
+                                            nested ? "nested.fruit" : "fruit",
+                                            nested ? "[invalid]nested.fruit" : "[invalid]fruit",
+                                            "Bad value"));
+    replay(messageStore);
+
+    JacksonContentHandler handler = new JacksonContentHandler(request, store, new ObjectMapper(), expressionEvaluator, messageProvider, messageStore);
+    try {
+      handler.handle();
+      fail("Should have thrown");
+    } catch (ValidationException e) {
+      // Expected
+    }
+
+    assertNull(action.jsonRequest);
+
+    verify(store, messageProvider, messageStore);
+  }
+
   @Test
   public void handle() throws IOException {
     Map<Class<?>, Object> additionalConfig = new HashMap<>();