Skip to content

Commit

Permalink
chore: UID does not need a PropertyEditor
Browse files Browse the repository at this point in the history 
as it has a contructor from String -> UID its automatically
picked up by Spring
  • Loading branch information
@teleivo
teleivo committed Mar 1, 2024
1 parent 26b8b2f commit e6b4a6a
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 78 deletions.
38 changes: 0 additions & 38 deletions dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/common/UIDParamEditor.java
View file

This file was deleted.

17 changes: 9 additions & 8 deletions dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/CrudControllerAdvice.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@
import org.hisp.dhis.security.spring2fa.TwoFactorAuthenticationException;
import org.hisp.dhis.tracker.imports.TrackerIdSchemeParam;
import org.hisp.dhis.util.DateUtils;
import org.hisp.dhis.webapi.common.UIDParamEditor;
import org.hisp.dhis.webapi.controller.exception.MetadataImportConflictException;
import org.hisp.dhis.webapi.controller.exception.MetadataSyncException;
import org.hisp.dhis.webapi.controller.exception.MetadataVersionException;
Expand Down Expand Up @@ -151,7 +150,6 @@ protected void initBinder(WebDataBinder binder) {
IdentifiableProperty.class, new FromTextPropertyEditor(String::toUpperCase));
this.enumClasses.forEach(c -> binder.registerCustomEditor(c, new ConvertEnum(c)));
binder.registerCustomEditor(TrackerIdSchemeParam.class, new IdSchemeParamEditor());
binder.registerCustomEditor(UID.class, new UIDParamEditor());
}

@ExceptionHandler
Expand Down Expand Up @@ -218,8 +216,8 @@ public WebMessage methodArgumentTypeMismatchException(MethodArgumentTypeMismatch
ex.getParameter().getParameterAnnotation(PathVariable.class);
String field = ex.getName();
Object value = ex.getValue();
String notValidValueMessage =
getNotValidValueMessage(value, field, pathVariableAnnotation != null);
boolean isPathVariable = pathVariableAnnotation != null;
String notValidValueMessage = getNotValidValueMessage(value, field, isPathVariable);

String customErrorMessage;
if (requiredType == null) {
Expand All @@ -231,7 +229,8 @@ public WebMessage methodArgumentTypeMismatchException(MethodArgumentTypeMismatch
} else if (ex.getCause() instanceof IllegalArgumentException) {
customErrorMessage = ex.getCause().getMessage();
} else if (ex.getCause() instanceof ConversionFailedException conversionException) {
notValidValueMessage = getConversionErrorMessage(value, field, conversionException);
notValidValueMessage =
getConversionErrorMessage(value, field, conversionException, isPathVariable);
customErrorMessage = "";
} else {
customErrorMessage = getGenericFieldErrorMessage(requiredType.getSimpleName());
Expand All @@ -258,7 +257,7 @@ public WebMessage handleTypeMismatchException(TypeMismatchException ex) {
} else if (ex.getCause() instanceof IllegalArgumentException) {
customErrorMessage = ex.getCause().getMessage();
} else if (ex.getCause() instanceof ConversionFailedException conversionException) {
notValidValueMessage = getConversionErrorMessage(value, field, conversionException);
notValidValueMessage = getConversionErrorMessage(value, field, conversionException, false);
customErrorMessage = "";
} else {
customErrorMessage = getGenericFieldErrorMessage(requiredType.getSimpleName());
Expand Down Expand Up @@ -308,7 +307,7 @@ private String getFormattedBadRequestMessage(String fieldErrorMessage, String cu
}

private static String getConversionErrorMessage(
Object rootValue, String field, ConversionFailedException ex) {
Object rootValue, String field, ConversionFailedException ex, boolean isPathVariable) {
Object invalidValue = ex.getValue();
if (TypeDescriptor.valueOf(String.class).equals(ex.getSourceType())
&& (invalidValue != null && ((String) invalidValue).contains(","))
Expand All @@ -319,7 +318,9 @@ private static String getConversionErrorMessage(
+ ex.getCause().getMessage();
}

return getNotValidValueMessage(invalidValue, field) + " " + ex.getCause().getMessage();
return getNotValidValueMessage(invalidValue, field, isPathVariable)
+ " "
+ ex.getCause().getMessage();
}

/**
Expand Down
93 changes: 61 additions & 32 deletions dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/common/UIDBindingTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,20 @@
*/
package org.hisp.dhis.webapi.common;

import static org.hamcrest.Matchers.containsString;
import static org.hisp.dhis.dxf2.webmessage.WebMessageUtils.ok;
import static org.hisp.dhis.utils.Assertions.assertStartsWith;
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 org.hisp.dhis.common.UID;
import org.hisp.dhis.dxf2.webmessage.WebMessage;
import org.hisp.dhis.jsontree.JsonMixed;
import org.hisp.dhis.webapi.controller.CrudControllerAdvice;
import org.hisp.dhis.webapi.json.domain.JsonWebMessage;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.mock.web.MockHttpServletResponse;
import org.springframework.stereotype.Controller;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.setup.MockMvcBuilders;
Expand All @@ -46,7 +51,8 @@

class UIDBindingTest {

private static final String VALID_UID = "bRNvL6NMQXb";
private static final String VALID_UID_STRING = "bRNvL6NMQXb";
private static final UID VALID_UID = UID.of("bRNvL6NMQXb");

private static final String INVALID_UID = "invalidUid";

Expand All @@ -57,10 +63,12 @@ class UIDBindingTest {
"Value 'invalidUid' is not valid for path parameter uid. UID must be an alphanumeric string of 11 characters";

private static final String BINDING_OBJECT_ERROR_MESSAGE =
"Value 'invalidUid' is not valid for parameter uidInRequestParams. UID must be an alphanumeric string of 11 characters";
"Value 'invalidUid' is not valid for parameter uid. UID must be an alphanumeric string of 11 characters";

private static final String ENDPOINT = "/uid";

private UID actual;

private MockMvc mockMvc;

@BeforeEach
Expand All @@ -73,66 +81,87 @@ public void setUp() {

@Test
void shouldReturnUIDValueWhenPassingValidUidAsPathVariable() throws Exception {
mockMvc
.perform(get(ENDPOINT + "/" + VALID_UID))
.andExpect(status().isOk())
.andExpect(content().string(VALID_UID));
mockMvc.perform(get(ENDPOINT + "/" + VALID_UID)).andExpect(status().isOk());

assertEquals(VALID_UID, actual);
}

@Test
void shouldReturnUIDValueWhenPassingValidUidAsRequestParam() throws Exception {
mockMvc
.perform(get(ENDPOINT).param("uid", VALID_UID))
.andExpect(status().isOk())
.andExpect(content().string(VALID_UID));
mockMvc.perform(get(ENDPOINT).param("uid", VALID_UID_STRING)).andExpect(status().isOk());

assertEquals(VALID_UID, actual);
}

@Test
void shouldReturnUIDValueWhenPassingValidUidAsRequestParamObject() throws Exception {
mockMvc
.perform(get(ENDPOINT + "/params").param("uidInRequestParams", VALID_UID))
.andExpect(status().isOk())
.andExpect(content().string(VALID_UID));
.perform(get(ENDPOINT + "/params").param("uid", VALID_UID_STRING))
.andExpect(status().isOk());

assertEquals(VALID_UID, actual);
}

@Test
void shouldReturnBadRequestResponseWhenPassingInvalidUidAsPathVariable() throws Exception {
mockMvc
.perform(get(ENDPOINT + "/" + INVALID_UID))
.andExpect(content().string(containsString(ERROR_MESSAGE_FOR_PATH_PARAM)));
MockHttpServletResponse response =
mockMvc
.perform(get(ENDPOINT + "/" + INVALID_UID).accept("application/json"))
.andReturn()
.getResponse();

JsonWebMessage message = JsonMixed.of(response.getContentAsString()).as(JsonWebMessage.class);
assertEquals(400, message.getHttpStatusCode());
assertStartsWith(ERROR_MESSAGE_FOR_PATH_PARAM, message.getMessage());
}

@Test
void shouldReturnBadRequestResponseWhenPassingInvalidUidAsRequestParam() throws Exception {
mockMvc
.perform(get(ENDPOINT).param("uid", INVALID_UID))
.andExpect(content().string(containsString(ERROR_MESSAGE)));
MockHttpServletResponse response =
mockMvc
.perform(get(ENDPOINT).accept("application/json").param("uid", INVALID_UID))
.andReturn()
.getResponse();

JsonWebMessage message = JsonMixed.of(response.getContentAsString()).as(JsonWebMessage.class);
assertEquals(400, message.getHttpStatusCode());
assertStartsWith(ERROR_MESSAGE, message.getMessage());
}

@Test
void shouldReturnBadRequestResponseWhenPassingInvalidUidAsRequestParamObject() throws Exception {
mockMvc
.perform(get(ENDPOINT + "/params").param("uidInRequestParams", INVALID_UID))
.andExpect(content().string(containsString(BINDING_OBJECT_ERROR_MESSAGE)));
MockHttpServletResponse response =
mockMvc
.perform(get(ENDPOINT + "/params").accept("application/json").param("uid", INVALID_UID))
.andReturn()
.getResponse();

JsonWebMessage message = JsonMixed.of(response.getContentAsString()).as(JsonWebMessage.class);
assertEquals(400, message.getHttpStatusCode());
assertStartsWith(BINDING_OBJECT_ERROR_MESSAGE, message.getMessage());
}

// TODO test binding collection
@Controller
private static class UIDController extends CrudControllerAdvice {
private class UIDController extends CrudControllerAdvice {
@GetMapping(value = ENDPOINT + "/{uid}")
public @ResponseBody String getUIDValueFromPath(@PathVariable UID uid) {
return uid.getValue();
public @ResponseBody WebMessage getUIDValueFromPath(@PathVariable UID uid) {
actual = uid;
return ok();
}

@GetMapping(value = ENDPOINT)
public @ResponseBody String getUIDValueFromRequestParam(@RequestParam UID uid) {
return uid.getValue();
public @ResponseBody WebMessage getUIDValueFromRequestParam(@RequestParam UID uid) {
actual = uid;
return ok();
}

@GetMapping(value = ENDPOINT + "/params")
public @ResponseBody String getUIDValueFromRequestObject(UIDRequestParams uidRequestParams) {
return uidRequestParams.uidInRequestParams().getValue();
public @ResponseBody WebMessage getUIDValueFromRequestObject(UIDRequestParams requestParams) {
actual = requestParams.uid;
return ok();
}
}

private record UIDRequestParams(UID uidInRequestParams) {}
private record UIDRequestParams(UID uid) {}
}

0 comments on commit e6b4a6a

Please sign in to comment.