test: asssert on actual types and their order

instead of strings. Add tests for passing comma separated values in one
order param. Add test for the mixed case of comma separated values in one
order param with a repeated param

dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/CrudControllerAdvice.java

@@ -88,6 +88,8 @@
 import org.hisp.dhis.webapi.security.apikey.ApiTokenAuthenticationException;
 import org.hisp.dhis.webapi.security.apikey.ApiTokenError;
 import org.springframework.beans.TypeMismatchException;
+import org.springframework.core.convert.ConversionFailedException;
+import org.springframework.core.convert.TypeDescriptor;
 import org.springframework.dao.DataAccessResourceFailureException;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.converter.HttpMessageNotReadableException;
@@ -226,6 +228,8 @@ public WebMessage methodArgumentTypeMismatchException(MethodArgumentTypeMismatch
       customErrorMessage = getGenericFieldErrorMessage(requiredType.getSimpleName());
     } else if (ex.getCause() instanceof IllegalArgumentException) {
       customErrorMessage = ex.getCause().getMessage();
+    } else if (ex.getCause() instanceof ConversionFailedException conversionException) {
+      customErrorMessage = getConversionErrorMessage(ex, conversionException);
     } else {
       customErrorMessage = getGenericFieldErrorMessage(requiredType.getSimpleName());
     }
@@ -291,6 +295,22 @@ private String getFormattedBadRequestMessage(String fieldErrorMessage, String cu
     return fieldErrorMessage + " " + customMessage;
   }
 
+  private static String getConversionErrorMessage(
+      MethodArgumentTypeMismatchException ex, ConversionFailedException conversionException) {
+    String customErrorMessage;
+    if (TypeDescriptor.valueOf(String.class) == conversionException.getSourceType()
+        && ((String) conversionException.getValue()).contains(",")
+        && ex.getValue() != null
+        && ex.getValue().getClass().isArray()) {
+      customErrorMessage =
+          "You might have repeated a request parameter and used multiple comma-separated values within at least one of the parameters. Choose one of both possible approaches. "
+              + ex.getCause().getMessage();
+    } else {
+      customErrorMessage = ex.getCause().getMessage();
+    }
+    return customErrorMessage;
+  }
+
   /**
    * A BindException wraps possible errors happened trying to bind all the request parameters to a
    * binding object. Errors could be simple conversion failures (Trying to convert a 'TEXT' to an

dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/controller/OrderBindingTest.java

@@ -29,10 +29,13 @@
 
 import static org.hamcrest.core.StringContains.containsString;
 import static org.hisp.dhis.dxf2.webmessage.WebMessageUtils.ok;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
 import java.util.List;
+import org.hisp.dhis.common.SortDirection;
 import org.hisp.dhis.dxf2.webmessage.WebMessage;
 import org.hisp.dhis.webapi.controller.event.webrequest.OrderCriteria;
 import org.junit.jupiter.api.BeforeEach;
@@ -46,8 +49,8 @@
 
 class OrderBindingTest {
   private static final String ENDPOINT = "/ordering";
-
   private MockMvc mockMvc;
+  private List<OrderCriteria> actual;
 
   @BeforeEach
   public void setUp() {
@@ -59,35 +62,70 @@ public void setUp() {
 
   @Test
   void shouldReturnDefaultSortDirectionWhenNoSortDirectionIsPassedAsParameter() throws Exception {
-    mockMvc
-        .perform(get(ENDPOINT).param("order", "field"))
-        .andExpect(content().string(containsString("OK")))
-        .andExpect(content().string(containsString("field")))
-        .andExpect(content().string(containsString("ASC")));
+    mockMvc.perform(get(ENDPOINT).param("order", "field")).andExpect(status().isOk());
+
+    assertEquals(List.of(OrderCriteria.of("field", SortDirection.ASC)), actual);
   }
 
   @Test
   void shouldReturnAscSortDirectionWhenAscSortDirectionIsPassedAsParameter() throws Exception {
-    mockMvc
-        .perform(get(ENDPOINT).param("order", "field:asc"))
-        .andExpect(content().string(containsString("OK")))
-        .andExpect(content().string(containsString("field")))
-        .andExpect(content().string(containsString("ASC")));
+    mockMvc.perform(get(ENDPOINT).param("order", "field:asc")).andExpect(status().isOk());
+
+    assertEquals(List.of(OrderCriteria.of("field", SortDirection.ASC)), actual);
   }
 
   @Test
   void shouldReturnDescSortDirectionWhenDescSortDirectionIsPassedAsParameter() throws Exception {
+    mockMvc.perform(get(ENDPOINT).param("order", "field:desc")).andExpect(status().isOk());
+
+    assertEquals(List.of(OrderCriteria.of("field", SortDirection.DESC)), actual);
+  }
+
+  @Test
+  void shouldHandleMultipleOrderComponentsGivenViaOneParameter() throws Exception {
     mockMvc
-        .perform(get(ENDPOINT).param("order", "field:desc"))
-        .andExpect(content().string(containsString("OK")))
-        .andExpect(content().string(containsString("field")))
-        .andExpect(content().string(containsString("DESC")));
+        .perform(get(ENDPOINT).param("order", "field1:desc,field2:asc"))
+        .andExpect(status().isOk());
+
+    assertEquals(
+        List.of(
+            OrderCriteria.of("field1", SortDirection.DESC),
+            OrderCriteria.of("field2", SortDirection.ASC)),
+        actual);
+  }
+
+  @Test
+  void shouldHandleMultipleOrderComponentsGivenViaMultipleParameters() throws Exception {
+    mockMvc
+        .perform(get(ENDPOINT).param("order", "field1:desc").param("order", "field2:asc"))
+        .andExpect(status().isOk());
+
+    assertEquals(
+        List.of(
+            OrderCriteria.of("field1", SortDirection.DESC),
+            OrderCriteria.of("field2", SortDirection.ASC)),
+        actual);
+  }
+
+  @Test
+  void shouldReturnABadRequestWhenMixingRepeatedParameterAndCommaSeparatedValues()
+      throws Exception {
+    mockMvc
+        .perform(
+            get(ENDPOINT)
+                .accept("application/json")
+                .param("order", "field1:desc")
+                .param("order", "field2:asc,field3:desc"))
+        .andExpect(content().string(containsString("Bad Request")))
+        .andExpect(
+            content()
+                .string(containsString("You might have repeated a request parameter and used")));
   }
 
   @Test
   void shouldReturnABadRequestWhenInvalidSortDirectionIsPassedAsParameter() throws Exception {
     mockMvc
-        .perform(get(ENDPOINT).param("order", "field:wrong"))
+        .perform(get(ENDPOINT).accept("application/json").param("order", "field:wrong"))
         .andExpect(content().string(containsString("Bad Request")))
         .andExpect(
             content()
@@ -100,7 +138,11 @@ void shouldReturnABadRequestWhenInvalidSortDirectionIsPassedAsParameter() throws
   void shouldReturnABadRequestWhenInvalidSortDirectionIsPassedAsParameterInAListOfOrders()
       throws Exception {
     mockMvc
-        .perform(get(ENDPOINT).param("order", "field1:wrong").param("order", "field2:asc"))
+        .perform(
+            get(ENDPOINT)
+                .accept("application/json")
+                .param("order", "field1:wrong")
+                .param("order", "field2:asc"))
         .andExpect(content().string(containsString("Bad Request")))
         .andExpect(
             content()
@@ -110,9 +152,10 @@ void shouldReturnABadRequestWhenInvalidSortDirectionIsPassedAsParameterInAListOf
   }
 
   @Controller
-  private static class OrderingController extends CrudControllerAdvice {
+  private class OrderingController extends CrudControllerAdvice {
     @GetMapping(value = ENDPOINT)
     public @ResponseBody WebMessage getOrder(@RequestParam List<OrderCriteria> order) {
+      actual = order;
       return ok(order.toString());
     }
   }