From 235070cd92514f4763d9a650dd54970f44b9a1ef Mon Sep 17 00:00:00 2001 From: Augment Agent Date: Fri, 1 May 2026 21:53:55 +0000 Subject: [PATCH] polish(validation): replace requireNonNull with bean validation (#10, #15) - Drop Objects.requireNonNull from request-DTO canonical constructors (Crecust, Creacc, Dbcrfun, Updcust, Delcus and their nested Commarea/Key DTOs) so missing fields surface as 400 from MethodArgumentNotValidException instead of 500/UNEX from a Jackson NPE escape (issue #15). - Tighten CrecustCommareaRequestDto.commName / UpdcustCommareaRequestDto .commName/commAddress with @NotBlank and remove the bypass empty string from CrecustService.VALID_TITLES so blank/whitespace names cannot pass title validation (issue #10). - Add WebMvc tests covering missing-required-field and blank-name paths, and a CrecustService unit test for the empty-name rejection. - Document the rule in docs/translation-rules.md \xc2\xa76. --- docs/translation-rules.md | 9 ++++++++ .../augment/cbsa/service/CrecustService.java | 2 +- .../creacc/dto/CreaccCommareaRequestDto.java | 11 ---------- .../cbsa/web/creacc/dto/CreaccKeyDto.java | 6 ----- .../cbsa/web/creacc/dto/CreaccRequestDto.java | 5 ----- .../dto/CrecustCommareaRequestDto.java | 11 ++-------- .../cbsa/web/crecust/dto/CrecustKeyDto.java | 6 ----- .../web/crecust/dto/CrecustRequestDto.java | 5 ----- .../web/dbcrfun/dto/DbcrfunRequestDto.java | 5 ----- .../cbsa/web/delcus/dto/DelcusRequestDto.java | 5 ----- .../dto/UpdcustCommareaRequestDto.java | 16 +++----------- .../web/updcust/dto/UpdcustRequestDto.java | 5 ----- .../cbsa/service/CrecustServiceUnitTest.java | 9 ++++++++ .../crecust/CrecustControllerWebMvcTest.java | 22 +++++++++++++++++++ 14 files changed, 46 insertions(+), 71 deletions(-) diff --git a/docs/translation-rules.md b/docs/translation-rules.md index 7769ff8..fd02995 100644 --- a/docs/translation-rules.md +++ b/docs/translation-rules.md @@ -110,6 +110,15 @@ Never persist both forms. of the commarea (e.g. `INQCUST-CUSTNO` → `customerNumber`). - Validation via `jakarta.validation` annotations; constraint violations yield HTTP 400 from the `@ControllerAdvice`. +- Required fields on request-DTO records are expressed exclusively through + `@NotNull` (and `@NotBlank` for non-blank strings such as customer + name/address) on the record components, with `@Valid` on the controller + parameter and on every nested DTO/Key reference. Do **not** put + `Objects.requireNonNull(...)` checks inside the canonical constructor of + a request DTO: those throw `NullPointerException` during Jackson + deserialization, bypass `MethodArgumentNotValidException`, and surface + as a 500 (`UNEX`) instead of the intended 400. Issues #10 and #15 are + the empirical source for this rule. - A "not found" commarea result (e.g. `INQCUST-INQ-FAIL-CD = '1'`) maps to HTTP 404 with a `{ failCode, message }` body, **not** an exception. - Hard failures (commarea-style abend) map to HTTP 500 with a structured diff --git a/src/main/java/com/augment/cbsa/service/CrecustService.java b/src/main/java/com/augment/cbsa/service/CrecustService.java index 7942b9a..7e40694 100644 --- a/src/main/java/com/augment/cbsa/service/CrecustService.java +++ b/src/main/java/com/augment/cbsa/service/CrecustService.java @@ -37,7 +37,7 @@ public class CrecustService { private static final int REVIEW_DATE_BOUND = 20; private static final long CREDIT_AGENCY_REPLY_WINDOW_SECONDS = 3; private static final List VALID_TITLES = List.of( - "Professor", "Mr", "Mrs", "Miss", "Ms", "Dr", "Drs", "Lord", "Sir", "Lady", "" + "Professor", "Mr", "Mrs", "Miss", "Ms", "Dr", "Drs", "Lord", "Sir", "Lady" ); private final CrecustRepository crecustRepository; diff --git a/src/main/java/com/augment/cbsa/web/creacc/dto/CreaccCommareaRequestDto.java b/src/main/java/com/augment/cbsa/web/creacc/dto/CreaccCommareaRequestDto.java index 6e4b3de..8bb832d 100644 --- a/src/main/java/com/augment/cbsa/web/creacc/dto/CreaccCommareaRequestDto.java +++ b/src/main/java/com/augment/cbsa/web/creacc/dto/CreaccCommareaRequestDto.java @@ -10,7 +10,6 @@ import jakarta.validation.constraints.NotNull; import jakarta.validation.constraints.Size; import java.math.BigDecimal; -import java.util.Objects; public record CreaccCommareaRequestDto( @JsonProperty("CommEyecatcher") @@ -83,14 +82,4 @@ public record CreaccCommareaRequestDto( @Size(max = 1) String commFailCode ) { - - public CreaccCommareaRequestDto { - Objects.requireNonNull(commCustno, "commCustno must not be null"); - Objects.requireNonNull(commKey, "commKey must not be null"); - Objects.requireNonNull(commAccType, "commAccType must not be null"); - Objects.requireNonNull(commIntRt, "commIntRt must not be null"); - Objects.requireNonNull(commOverdrLim, "commOverdrLim must not be null"); - Objects.requireNonNull(commAvailBal, "commAvailBal must not be null"); - Objects.requireNonNull(commActBal, "commActBal must not be null"); - } } \ No newline at end of file diff --git a/src/main/java/com/augment/cbsa/web/creacc/dto/CreaccKeyDto.java b/src/main/java/com/augment/cbsa/web/creacc/dto/CreaccKeyDto.java index 81a2a3f..995b5f6 100644 --- a/src/main/java/com/augment/cbsa/web/creacc/dto/CreaccKeyDto.java +++ b/src/main/java/com/augment/cbsa/web/creacc/dto/CreaccKeyDto.java @@ -5,7 +5,6 @@ import jakarta.validation.constraints.Min; import jakarta.validation.constraints.NotNull; import jakarta.validation.constraints.Pattern; -import java.util.Objects; public record CreaccKeyDto( @JsonProperty("CommSortcode") @@ -19,9 +18,4 @@ public record CreaccKeyDto( @Max(99_999_999) Long commNumber ) { - - public CreaccKeyDto { - Objects.requireNonNull(commSortcode, "commSortcode must not be null"); - Objects.requireNonNull(commNumber, "commNumber must not be null"); - } } \ No newline at end of file diff --git a/src/main/java/com/augment/cbsa/web/creacc/dto/CreaccRequestDto.java b/src/main/java/com/augment/cbsa/web/creacc/dto/CreaccRequestDto.java index 3645161..dc458ab 100644 --- a/src/main/java/com/augment/cbsa/web/creacc/dto/CreaccRequestDto.java +++ b/src/main/java/com/augment/cbsa/web/creacc/dto/CreaccRequestDto.java @@ -3,7 +3,6 @@ import com.fasterxml.jackson.annotation.JsonProperty; import jakarta.validation.Valid; import jakarta.validation.constraints.NotNull; -import java.util.Objects; public record CreaccRequestDto( @JsonProperty("CreAcc") @@ -11,8 +10,4 @@ public record CreaccRequestDto( @NotNull CreaccCommareaRequestDto creAcc ) { - - public CreaccRequestDto { - Objects.requireNonNull(creAcc, "creAcc must not be null"); - } } \ No newline at end of file diff --git a/src/main/java/com/augment/cbsa/web/crecust/dto/CrecustCommareaRequestDto.java b/src/main/java/com/augment/cbsa/web/crecust/dto/CrecustCommareaRequestDto.java index 01ab75f..7aa4b16 100644 --- a/src/main/java/com/augment/cbsa/web/crecust/dto/CrecustCommareaRequestDto.java +++ b/src/main/java/com/augment/cbsa/web/crecust/dto/CrecustCommareaRequestDto.java @@ -4,9 +4,9 @@ import jakarta.validation.Valid; import jakarta.validation.constraints.Max; import jakarta.validation.constraints.Min; +import jakarta.validation.constraints.NotBlank; import jakarta.validation.constraints.NotNull; import jakarta.validation.constraints.Size; -import java.util.Objects; public record CrecustCommareaRequestDto( @JsonProperty("CommEyecatcher") @@ -19,7 +19,7 @@ public record CrecustCommareaRequestDto( CrecustKeyDto commKey, @JsonProperty("CommName") - @NotNull + @NotBlank @Size(max = 60) String commName, @@ -52,11 +52,4 @@ public record CrecustCommareaRequestDto( @Size(max = 1) String commFailCode ) { - - public CrecustCommareaRequestDto { - Objects.requireNonNull(commKey, "commKey must not be null"); - Objects.requireNonNull(commName, "commName must not be null"); - Objects.requireNonNull(commAddress, "commAddress must not be null"); - Objects.requireNonNull(commDateOfBirth, "commDateOfBirth must not be null"); - } } diff --git a/src/main/java/com/augment/cbsa/web/crecust/dto/CrecustKeyDto.java b/src/main/java/com/augment/cbsa/web/crecust/dto/CrecustKeyDto.java index 47597d2..f47e593 100644 --- a/src/main/java/com/augment/cbsa/web/crecust/dto/CrecustKeyDto.java +++ b/src/main/java/com/augment/cbsa/web/crecust/dto/CrecustKeyDto.java @@ -5,7 +5,6 @@ import jakarta.validation.constraints.Min; import jakarta.validation.constraints.NotNull; import jakarta.validation.constraints.Pattern; -import java.util.Objects; public record CrecustKeyDto( @JsonProperty("CommSortcode") @@ -19,9 +18,4 @@ public record CrecustKeyDto( @Max(9_999_999_999L) Long commNumber ) { - - public CrecustKeyDto { - Objects.requireNonNull(commSortcode, "commSortcode must not be null"); - Objects.requireNonNull(commNumber, "commNumber must not be null"); - } } diff --git a/src/main/java/com/augment/cbsa/web/crecust/dto/CrecustRequestDto.java b/src/main/java/com/augment/cbsa/web/crecust/dto/CrecustRequestDto.java index 6d3448b..1aae4a1 100644 --- a/src/main/java/com/augment/cbsa/web/crecust/dto/CrecustRequestDto.java +++ b/src/main/java/com/augment/cbsa/web/crecust/dto/CrecustRequestDto.java @@ -3,7 +3,6 @@ import com.fasterxml.jackson.annotation.JsonProperty; import jakarta.validation.Valid; import jakarta.validation.constraints.NotNull; -import java.util.Objects; public record CrecustRequestDto( @JsonProperty("CreCust") @@ -11,8 +10,4 @@ public record CrecustRequestDto( @NotNull CrecustCommareaRequestDto creCust ) { - - public CrecustRequestDto { - Objects.requireNonNull(creCust, "creCust must not be null"); - } } diff --git a/src/main/java/com/augment/cbsa/web/dbcrfun/dto/DbcrfunRequestDto.java b/src/main/java/com/augment/cbsa/web/dbcrfun/dto/DbcrfunRequestDto.java index 0b35cd2..42a6636 100644 --- a/src/main/java/com/augment/cbsa/web/dbcrfun/dto/DbcrfunRequestDto.java +++ b/src/main/java/com/augment/cbsa/web/dbcrfun/dto/DbcrfunRequestDto.java @@ -3,7 +3,6 @@ import com.fasterxml.jackson.annotation.JsonProperty; import jakarta.validation.Valid; import jakarta.validation.constraints.NotNull; -import java.util.Objects; public record DbcrfunRequestDto( @JsonProperty("PAYDBCR") @@ -11,8 +10,4 @@ public record DbcrfunRequestDto( @NotNull DbcrfunCommareaRequestDto paydbcr ) { - - public DbcrfunRequestDto { - Objects.requireNonNull(paydbcr, "paydbcr must not be null"); - } } \ No newline at end of file diff --git a/src/main/java/com/augment/cbsa/web/delcus/dto/DelcusRequestDto.java b/src/main/java/com/augment/cbsa/web/delcus/dto/DelcusRequestDto.java index eb739af..2e6fd53 100644 --- a/src/main/java/com/augment/cbsa/web/delcus/dto/DelcusRequestDto.java +++ b/src/main/java/com/augment/cbsa/web/delcus/dto/DelcusRequestDto.java @@ -3,7 +3,6 @@ import com.fasterxml.jackson.annotation.JsonProperty; import jakarta.validation.Valid; import jakarta.validation.constraints.NotNull; -import java.util.Objects; public record DelcusRequestDto( @JsonProperty("DelCus") @@ -11,8 +10,4 @@ public record DelcusRequestDto( @NotNull DelcusCommareaRequestDto delCus ) { - - public DelcusRequestDto { - Objects.requireNonNull(delCus, "delCus must not be null"); - } } diff --git a/src/main/java/com/augment/cbsa/web/updcust/dto/UpdcustCommareaRequestDto.java b/src/main/java/com/augment/cbsa/web/updcust/dto/UpdcustCommareaRequestDto.java index 81ab89b..34d105e 100644 --- a/src/main/java/com/augment/cbsa/web/updcust/dto/UpdcustCommareaRequestDto.java +++ b/src/main/java/com/augment/cbsa/web/updcust/dto/UpdcustCommareaRequestDto.java @@ -3,10 +3,10 @@ import com.fasterxml.jackson.annotation.JsonProperty; import jakarta.validation.constraints.Max; import jakarta.validation.constraints.Min; +import jakarta.validation.constraints.NotBlank; import jakarta.validation.constraints.NotNull; import jakarta.validation.constraints.Pattern; import jakarta.validation.constraints.Size; -import java.util.Objects; public record UpdcustCommareaRequestDto( @JsonProperty("CommEye") @@ -24,12 +24,12 @@ public record UpdcustCommareaRequestDto( String commCustno, @JsonProperty("CommName") - @NotNull + @NotBlank @Size(max = 60) String commName, @JsonProperty("CommAddress") - @NotNull + @NotBlank @Size(max = 160) String commAddress, @@ -59,14 +59,4 @@ public record UpdcustCommareaRequestDto( @Size(max = 1) String commUpdFailCd ) { - - public UpdcustCommareaRequestDto { - Objects.requireNonNull(commScode, "commScode must not be null"); - Objects.requireNonNull(commCustno, "commCustno must not be null"); - Objects.requireNonNull(commName, "commName must not be null"); - Objects.requireNonNull(commAddress, "commAddress must not be null"); - Objects.requireNonNull(commDob, "commDob must not be null"); - Objects.requireNonNull(commCreditScore, "commCreditScore must not be null"); - Objects.requireNonNull(commCsReviewDate, "commCsReviewDate must not be null"); - } } diff --git a/src/main/java/com/augment/cbsa/web/updcust/dto/UpdcustRequestDto.java b/src/main/java/com/augment/cbsa/web/updcust/dto/UpdcustRequestDto.java index 717dfd5..e2b9999 100644 --- a/src/main/java/com/augment/cbsa/web/updcust/dto/UpdcustRequestDto.java +++ b/src/main/java/com/augment/cbsa/web/updcust/dto/UpdcustRequestDto.java @@ -3,7 +3,6 @@ import com.fasterxml.jackson.annotation.JsonProperty; import jakarta.validation.Valid; import jakarta.validation.constraints.NotNull; -import java.util.Objects; public record UpdcustRequestDto( @JsonProperty("UpdCust") @@ -11,8 +10,4 @@ public record UpdcustRequestDto( @NotNull UpdcustCommareaRequestDto updCust ) { - - public UpdcustRequestDto { - Objects.requireNonNull(updCust, "updCust must not be null"); - } } diff --git a/src/test/java/com/augment/cbsa/service/CrecustServiceUnitTest.java b/src/test/java/com/augment/cbsa/service/CrecustServiceUnitTest.java index c8a05dd..2e27fdd 100644 --- a/src/test/java/com/augment/cbsa/service/CrecustServiceUnitTest.java +++ b/src/test/java/com/augment/cbsa/service/CrecustServiceUnitTest.java @@ -62,6 +62,15 @@ void rejectsInvalidCustomerTitles() { verifyNoInteractions(creditAgencyService, crecustRepository, reviewDateRandom); } + @Test + void rejectsBlankCustomerNamesAtTitleValidation() { + CrecustResult result = crecustService.create(new CrecustRequest(" ", "1 Main Street", 1012000)); + + assertThat(result.creationSuccess()).isFalse(); + assertThat(result.failCode()).isEqualTo("T"); + verifyNoInteractions(creditAgencyService, crecustRepository, reviewDateRandom); + } + @Test void rejectsDatesEarlierThan1601() { CrecustResult result = crecustService.create(new CrecustRequest("Dr Example", "1 Main Street", 1011500)); diff --git a/src/test/java/com/augment/cbsa/web/crecust/CrecustControllerWebMvcTest.java b/src/test/java/com/augment/cbsa/web/crecust/CrecustControllerWebMvcTest.java index bdf8dbf..859091d 100644 --- a/src/test/java/com/augment/cbsa/web/crecust/CrecustControllerWebMvcTest.java +++ b/src/test/java/com/augment/cbsa/web/crecust/CrecustControllerWebMvcTest.java @@ -96,6 +96,28 @@ void requestValidationFailuresRemainProblemDetails() throws Exception { .andExpect(jsonPath("$.title").value("Validation failed")); } + @Test + void missingRequiredFieldReturnsBadRequest() throws Exception { + // CommName omitted entirely; bean validation must reject with 400 rather + // than letting a constructor NPE escalate to a 500 abend. + mockMvc.perform(post("/api/v1/crecust/insert") + .contentType(APPLICATION_JSON) + .content(""" + {"CreCust":{"CommKey":{"CommSortcode":"000000","CommNumber":0},"CommAddress":"1 Main Street","CommDateOfBirth":10012000}} + """)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.title").value("Validation failed")); + } + + @Test + void blankCommNameReturnsBadRequest() throws Exception { + mockMvc.perform(post("/api/v1/crecust/insert") + .contentType(APPLICATION_JSON) + .content(requestJson(" ", "1 Main Street", 10_01_2000))) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.title").value("Validation failed")); + } + @Test void redactsAbendExceptionMessageFromResponseBody() throws Exception { when(crecustService.create(new CrecustRequest("Dr Alice Example", "1 Main Street", 10_01_2000)))