Merge pull request #62 from companieshouse/feature/lp-374-patch-valid…

…ation

Feature/lp 374 patch validation

src/main/java/uk/gov/companieshouse/limitedpartnershipsapi/controller/PartnershipController.java

@@ -73,7 +73,7 @@ public ResponseEntity<Object> createPartnership(
     public ResponseEntity<Object> updatePartnership(
             @RequestAttribute(TRANSACTION_KEY) Transaction transaction,
             @PathVariable(URL_PARAM_SUBMISSION_ID) String submissionId,
-            @RequestBody LimitedPartnershipPatchDto limitedPartnershipPatchDto,
+            @Valid @RequestBody LimitedPartnershipPatchDto limitedPartnershipPatchDto,
             @RequestHeader(value = ERIC_REQUEST_ID_KEY) String requestId,
             @RequestHeader(value = ERIC_IDENTITY) String userId) {
 

src/main/java/uk/gov/companieshouse/limitedpartnershipsapi/exception/GlobalExceptionHandler.java

@@ -50,7 +50,8 @@ private String truncate(String input) {
     }
 
     @ExceptionHandler(MethodArgumentNotValidException.class)
-    public ResponseEntity<Map<String, Map<String, String>>> handleValidationErrors(MethodArgumentNotValidException exception) {
+    public ResponseEntity<Map<String, Map<String, String>>> handleValidationErrors(MethodArgumentNotValidException exception, WebRequest webRequest) {
+        var context = webRequest.getHeader(ERIC_REQUEST_ID_KEY);
 
         List<FieldError> errors = exception.getBindingResult().getFieldErrors();
 
@@ -63,6 +64,8 @@ public ResponseEntity<Map<String, Map<String, String>>> handleValidationErrors(M
         Map<String, Map<String, String>> errorResponse = new HashMap<>();
         errorResponse.put("errors", errorFields);
 
+        ApiLogger.errorContext(context, errorResponse.toString(), null);
+
         return new ResponseEntity<>(errorResponse, HttpStatus.BAD_REQUEST);
     }
 }

src/main/java/uk/gov/companieshouse/limitedpartnershipsapi/model/dto/DataDto.java

@@ -13,12 +13,14 @@
 @NameSize
 public class DataDto {
     public static final int NAME_MIN_SIZE = 1;
+    public static final String NAME_MIN_SIZE_MESSAGE = "partnership name must be greater than {min}";
     public static final int NAME_MAX_SIZE = 160;
+    public static final String NAME_MAX_SIZE_MESSAGE = "partnership name must be less than {max}";
 
     @JsonInclude(NON_NULL)
     @JsonProperty("partnership_name")
-    @Size(min = NAME_MIN_SIZE, message = "partnership_name must be greater than {min}")
-    @Size(max = NAME_MAX_SIZE, message = "partnership_name must be less than {max}")
+    @Size(min = NAME_MIN_SIZE, message = NAME_MIN_SIZE_MESSAGE)
+    @Size(max = NAME_MAX_SIZE, message = NAME_MAX_SIZE_MESSAGE)
     private String partnershipName;
 
     @JsonInclude(NON_NULL)

src/main/java/uk/gov/companieshouse/limitedpartnershipsapi/model/dto/LimitedPartnershipPatchDto.java

@@ -1,52 +1,58 @@
 package uk.gov.companieshouse.limitedpartnershipsapi.model.dto;
 
 import com.fasterxml.jackson.annotation.JsonProperty;
-import org.openapitools.jackson.nullable.JsonNullable;
+import jakarta.validation.constraints.Email;
+import jakarta.validation.constraints.Size;
 import uk.gov.companieshouse.limitedpartnershipsapi.model.PartnershipNameEnding;
 import uk.gov.companieshouse.limitedpartnershipsapi.model.PartnershipType;
+import uk.gov.companieshouse.limitedpartnershipsapi.model.dto.validator.NameSize;
 
+@NameSize
 public class LimitedPartnershipPatchDto {
     @JsonProperty("partnership_name")
-    private JsonNullable<String> partnershipName;
+    @Size(min = DataDto.NAME_MIN_SIZE, message = DataDto.NAME_MIN_SIZE_MESSAGE)
+    @Size(max = DataDto.NAME_MAX_SIZE, message = DataDto.NAME_MAX_SIZE_MESSAGE)
+    private String partnershipName;
 
     @JsonProperty("name_ending")
-    private JsonNullable<PartnershipNameEnding> nameEnding;
+    private PartnershipNameEnding nameEnding;
 
     @JsonProperty("email")
-    private JsonNullable<String> email;
+    @Email
+    private String email;
 
     @JsonProperty("partnership_type")
-    private JsonNullable<PartnershipType> partnershipType;
+    private PartnershipType partnershipType;
 
-    public JsonNullable<String> getPartnershipName() {
+    public String getPartnershipName() {
         return partnershipName;
     }
 
-    public void setPartnershipName(JsonNullable<String> partnershipName) {
+    public void setPartnershipName(String partnershipName) {
         this.partnershipName = partnershipName;
     }
 
-    public JsonNullable<PartnershipNameEnding> getNameEnding() {
+    public PartnershipNameEnding getNameEnding() {
         return nameEnding;
     }
 
-    public void setNameEnding(JsonNullable<PartnershipNameEnding> nameEnding) {
+    public void setNameEnding(PartnershipNameEnding nameEnding) {
         this.nameEnding = nameEnding;
     }
 
-    public JsonNullable<String> getEmail() {
+    public String getEmail() {
         return email;
     }
 
-    public void setEmail(JsonNullable<String> email) {
+    public void setEmail(String email) {
         this.email = email;
     }
 
-    public JsonNullable<PartnershipType> getPartnershipType() {
+    public PartnershipType getPartnershipType() {
         return partnershipType;
     }
 
-    public void setPartnershipType(JsonNullable<PartnershipType> partnershipType) {
+    public void setPartnershipType(PartnershipType partnershipType) {
         this.partnershipType = partnershipType;
     }
 }

src/main/java/uk/gov/companieshouse/limitedpartnershipsapi/model/dto/validator/NameSize.java

@@ -13,7 +13,7 @@
 @Target(ElementType.TYPE)
 @Retention(RetentionPolicy.RUNTIME)
 public @interface NameSize {
-    String message() default "Max length 'data.partnership_name + data.name_ending' is " + DataDto.NAME_MAX_SIZE + " characters";
+    String message() default "Max length 'partnership name + name ending' is " + DataDto.NAME_MAX_SIZE + " characters";
 
     Class<?>[] groups() default {};
 

src/main/java/uk/gov/companieshouse/limitedpartnershipsapi/model/dto/validator/NameSizeValidator.java

@@ -3,15 +3,26 @@
 import jakarta.validation.ConstraintValidator;
 import jakarta.validation.ConstraintValidatorContext;
 import uk.gov.companieshouse.limitedpartnershipsapi.model.dto.DataDto;
+import uk.gov.companieshouse.limitedpartnershipsapi.model.dto.LimitedPartnershipPatchDto;
 
-public class NameSizeValidator implements ConstraintValidator<NameSize, DataDto> {
-    public boolean isValid(DataDto object, ConstraintValidatorContext context) {
-        if (!(object instanceof DataDto)) {
-            throw new IllegalArgumentException("@NameSize only applies to DataDto object");
+public class NameSizeValidator implements ConstraintValidator<NameSize, Object> {
+    public boolean isValid(Object object, ConstraintValidatorContext context) {
+        if (object instanceof DataDto dto) {
+            return isSizeCorrect(dto.getPartnershipName(), dto.getNameEnding());
+        } else if (object instanceof LimitedPartnershipPatchDto dto) {
+            String nameEnding = dto.getNameEnding() != null ? dto.getNameEnding().getDescription() : "";
+            return isSizeCorrect(dto.getPartnershipName(), nameEnding);
+        } else {
+            throw new IllegalArgumentException("@NameSize only applies to DataDto or LimitedPartnershipPatchDto object");
         }
+    }
 
-        var name = String.format("%s %s", object.getPartnershipName(), object.getNameEnding());
+    private boolean isSizeCorrect(String partnershipName, String nameEnding) {
+        if (partnershipName == null && nameEnding == null) {
+            return true;
+        }
 
+        var name = String.format("%s %s", partnershipName, nameEnding);
         return name.length() <= DataDto.NAME_MAX_SIZE;
     }
 }

src/test/java/uk/gov/companieshouse/limitedpartnershipsapi/controller/PartnershipControllerValidationTest.java

@@ -1,6 +1,7 @@
 package uk.gov.companieshouse.limitedpartnershipsapi.controller;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.commons.lang.StringUtils;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -22,7 +23,9 @@
 import uk.gov.companieshouse.limitedpartnershipsapi.model.dto.LimitedPartnershipSubmissionDto;
 import uk.gov.companieshouse.limitedpartnershipsapi.service.LimitedPartnershipService;
 
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.patch;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
+import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
 @ExtendWith(SpringExtension.class)
@@ -48,7 +51,6 @@ class PartnershipControllerValidationTest {
     @MockBean
     private LimitedPartnershipService service;
 
-
     @BeforeEach
     void setUp() {
         httpHeaders = new HttpHeaders();
@@ -79,6 +81,7 @@ void testCreatePartnershipShouldReturn201() throws Exception {
 
         mockMvc.perform(post("/transactions/863851-951242-143528/limited-partnership/partnership")
                         .contentType(MediaType.APPLICATION_JSON)
+                        .characterEncoding("utf-8")
                         .headers(httpHeaders)
                         .requestAttr("transaction", transaction)
                         .content(body))
@@ -100,10 +103,67 @@ void testCreatePartnershipShouldReturnBadRequestErrorIfPartnershipNameIsLessThan
 
         mockMvc.perform(post("/transactions/863851-951242-143528/limited-partnership/partnership")
                         .contentType(MediaType.APPLICATION_JSON)
+                        .characterEncoding("utf-8")
+                        .headers(httpHeaders)
+                        .requestAttr("transaction", transaction)
+                        .content(body))
+                .andExpect(status().isBadRequest());
+    }
+
+    @Test
+    void testUpdatePartnershipShouldReturn200() throws Exception {
+        String body = "{\"email\":\"[email protected]\"}";
+
+        mockMvc.perform(patch("/transactions/863851-951242-143528/limited-partnership/partnership/93702824-9062-4c63-a694-716acffccdd5")
+                        .contentType(MediaType.APPLICATION_JSON)
+                        .characterEncoding("utf-8")
+                        .headers(httpHeaders)
+                        .requestAttr("transaction", transaction)
+                        .content(body))
+                .andDo(print())
+                .andExpect(status().isOk());
+    }
+
+    @Test
+    void testUpdatePartnershipShouldReturnBadRequestErrorIfEmailBadlyFormated() throws Exception {
+        String body = "{\"email\":\"test@email.\"}";
+
+        mockMvc.perform(patch("/transactions/863851-951242-143528/limited-partnership/partnership/93702824-9062-4c63-a694-716acffccdd5")
+                        .contentType(MediaType.APPLICATION_JSON)
+                        .characterEncoding("utf-8")
                         .headers(httpHeaders)
                         .requestAttr("transaction", transaction)
                         .content(body))
+                .andDo(print())
                 .andExpect(status().isBadRequest());
     }
 
+    @Test
+    void testUpdatePartnershipShouldReturn200IfNameSizeIsCorrect() throws Exception {
+        String body = "{\"partnership_name\":\"Correct name size\",\"name_ending\":\"Limited Partnership\"}";
+
+        mockMvc.perform(patch("/transactions/863851-951242-143528/limited-partnership/partnership/93702824-9062-4c63-a694-716acffccdd5")
+                        .contentType(MediaType.APPLICATION_JSON)
+                        .characterEncoding("utf-8")
+                        .headers(httpHeaders)
+                        .requestAttr("transaction", transaction)
+                        .content(body))
+                .andDo(print())
+                .andExpect(status().isOk());
+    }
+
+    @Test
+    void testUpdatePartnershipShouldReturnBadRequestErrorIfNameSizeIsTooLong() throws Exception {
+        String longName = StringUtils.repeat("A", 160);
+        String body = "{\"partnership_name\":\"" + longName + "\",\"name_ending\":\"Limited Partnership\"}";
+
+        mockMvc.perform(patch("/transactions/863851-951242-143528/limited-partnership/partnership/93702824-9062-4c63-a694-716acffccdd5")
+                        .contentType(MediaType.APPLICATION_JSON)
+                        .characterEncoding("utf-8")
+                        .headers(httpHeaders)
+                        .requestAttr("transaction", transaction)
+                        .content(body))
+                .andDo(print())
+                .andExpect(status().isBadRequest());
+    }
 }

src/test/java/uk/gov/companieshouse/limitedpartnershipsapi/mapper/LimitedPartnershipPatchMapperTest.java

@@ -5,7 +5,6 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.CsvSource;
-import org.openapitools.jackson.nullable.JsonNullable;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.test.context.SpringBootTest;
 import uk.gov.companieshouse.limitedpartnershipsapi.model.PartnershipNameEnding;
@@ -31,23 +30,21 @@ class LimitedPartnershipPatchMapperTest {
 
     @Test
     void testObjectMapperCanHandleJsonNullableFields() throws JsonProcessingException {
-        assertEquals(JsonNullable.of("some description"), mapper.readValue("{\"partnership_name\":\"some description\"}",
+        assertEquals("some description", mapper.readValue("{\"partnership_name\":\"some description\"}",
                 LimitedPartnershipPatchDto.class).getPartnershipName());
-        assertEquals(JsonNullable.of(null), mapper.readValue("{\"partnership_name\":null}",
+        assertNull(mapper.readValue("{\"partnership_name\":null}",
                 LimitedPartnershipPatchDto.class).getPartnershipName());
         assertNull(mapper.readValue("{}", LimitedPartnershipPatchDto.class).getPartnershipName());
     }
 
     @ParameterizedTest
     @CsvSource(value = {
             // Field NOT present in the JSON - no update:
-            "{\"email\":\"[email protected]\"}$ Asset Strippers",
+            "{\"email\":\"[email protected]\"}$ Asset Strippers $ [email protected]",
             // Field IS present in the JSON - set the new string value:
-            "{\"partnership_name\":\"Asset Adders\", \"email\":\"[email protected]\"}$ Asset Adders",
-            // Field IS present in the JSON with value null - set to null:
-            "{\"partnership_name\":null, \"email\":\"[email protected]\"}$ NULL"
+            "{\"partnership_name\":\"Asset Adders\", \"email\":\"[email protected]\"}$ Asset Adders $ [email protected]"
     }, delimiter = '$')
-    void testMapStructMappingWhenEmailValueSentAndNameUnchanged(String incomingJson, String expectedPartnershipName)
+    void testMapStructMappingWhenEmailValueSentAndNameUnchanged(String incomingJson, String expectedPartnershipName, String expectedEmail)
             throws JsonProcessingException {
         // Given
         LimitedPartnershipPatchDto patchDto = mapper.readValue(incomingJson, LimitedPartnershipPatchDto.class);
@@ -58,35 +55,7 @@ void testMapStructMappingWhenEmailValueSentAndNameUnchanged(String incomingJson,
         patchMapper.update(patchDto, mongoDto);
 
         // Then
-        checkExpectedFieldValues(mongoDto, expectedPartnershipName.equals("NULL") ? null : expectedPartnershipName);
-    }
-
-    @Test
-    void testMapStructMappingWhenEmailValueSentAndNameSetToUndefined() {
-
-        // Field IS present in the JSON with value undefined - no update:
-
-        // Given
-
-        /* Need to create the patch DTO a bit differently, as this doesn't work::
-
-             String incomingJson = "{\"partnership_name\":undefined, \"email\":\"[email protected]\"}";
-             PatchDto patchDto = mapper.readValue(incomingJson, PatchDto.class);
-
-           Error - "JsonParseException: Unrecognized token 'undefined'")
-         */
-
-        LimitedPartnershipPatchDto patchDto = new LimitedPartnershipPatchDto();
-        patchDto.setPartnershipName(JsonNullable.undefined());
-        patchDto.setEmail(JsonNullable.of("[email protected]"));
-
-        DataDto mongoDto = createMongoDto();
-
-        // When
-        patchMapper.update(patchDto, mongoDto);
-
-        // Then
-        checkExpectedFieldValues(mongoDto, "Asset Strippers");
+        checkExpectedFieldValues(mongoDto, expectedPartnershipName, expectedEmail);
     }
 
     private DataDto createMongoDto() {
@@ -98,10 +67,10 @@ private DataDto createMongoDto() {
         return mongoDto;
     }
 
-    private void checkExpectedFieldValues(DataDto mongoDto, String expectedPartnershipName) {
+    private void checkExpectedFieldValues(DataDto mongoDto, String expectedPartnershipName, String expectedEmail) {
         assertEquals(expectedPartnershipName, mongoDto.getPartnershipName());
         assertEquals(PartnershipNameEnding.L_DOT_P_DOT.getDescription(), mongoDto.getNameEnding());
         assertEquals(PartnershipType.PFLP, mongoDto.getPartnershipType());
-        assertEquals("[email protected]", mongoDto.getEmail());
+        assertEquals(expectedEmail, mongoDto.getEmail());
     }
 }

src/test/java/uk/gov/companieshouse/limitedpartnershipsapi/model/LimitedPartnershipSubmissionDtoValidationTest.java

@@ -63,9 +63,8 @@ void testCreatePartnershipShouldReturnBadRequestErrorIfPartnershipNameIsMoreThan
         assertThat(violations)
                 .extracting(ConstraintViolation::getMessage)
                 .containsExactlyInAnyOrder(
-                        String.format("Max length 'data.partnership_name + data.name_ending' is %s characters", DataDto.NAME_MAX_SIZE),
-                        String.format("partnership_name must be less than %s", DataDto.NAME_MAX_SIZE),
+                        String.format("Max length 'partnership name + name ending' is %s characters", DataDto.NAME_MAX_SIZE),
+                        String.format("partnership name must be less than %s", DataDto.NAME_MAX_SIZE),
                         "must be a well-formed email address");
     }
-
 }