FINERACT-2511: Return 400 for invalid dateFormat on client create/up… by oluexpert99 · Pull Request #5558 · apache/fineract

oluexpert99 · 2026-02-28T05:35:53Z

Description

Return 400 for invalid dateFormat on client create/update

            - Validate dateFormat in ClientDataValidator (create and update) so invalid
              patterns (e.g. "02 February 2026") are rejected with 400 instead of 500.
            - Add @DateFormat constraint and DateFormatValidator in fineract-validation;
              ClientDataValidator uses DateFormatValidator.isValidPattern() for reuse.
            - Annotate BusinessDateUpdateRequest.dateFormat with @DateFormat.
            - Add ClientDataValidatorTest and DateFormatValidationTest;

Describe the changes made and why they were made. (Ignore if these details are present on the associated Apache Fineract JIRA ticket.)

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

Write the commit message as per our guidelines
Acknowledge that we will not review PRs that are not passing the build ("green") - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
Create/update unit or integration tests for verifying the changes made.
Follow our coding conventions.
Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
This PR must not be a "code dump". Large changes can be made in a branch, with assistance. Ask for help on the developer mailing list.

Your assigned reviewer(s) will follow our guidelines for code reviews.

IOhacker · 2026-02-28T08:39:32Z

@oluexpert99 sign your commit and make sure it is following the PR naming convention

IOhacker

Kindly review my comments

oluexpert99 · 2026-02-28T20:27:55Z

Kindly review my comments

@IOhacker done

IOhacker

LGTM

adamsaghy · 2026-03-02T09:35:14Z

+    @Override
+    public boolean isValid(String value, ConstraintValidatorContext context) {
+        if (StringUtils.isBlank(value)) {
+            return true; // blank is allowed; use @NotBlank if required
+        }
+        return isValidPattern(value);
+    }
+
+    /**
+     * Checks whether the given string is a valid {@link DateTimeFormatter} pattern. Can be used by validators that are
+     * not annotation-based (e.g. when validating JSON commands that do not bind to a DTO).
+     *
+     * @param pattern
+     *            the candidate pattern string
+     * @return {@code true} if the pattern is valid, {@code false} otherwise
+     */
+    public static boolean isValidPattern(String pattern) {
+        if (StringUtils.isBlank(pattern)) {
+            return true;
+        }
+        try {
+            DateTimeFormatter.ofPattern(pattern);
+            return true;
+        } catch (IllegalArgumentException e) {
+            return false;
+        }


Unnecessarily overcomplicated.
Below is doing the same:

try { DateTimeFormatter.ofPattern(pattern); return true; } catch (IllegalArgumentException e) { return false; }

@adamsaghy

Thanks for the review. The current implementation of DateFormatValidator#isValidPattern already uses that try { DateTimeFormatter.ofPattern(pattern); } catch (IllegalArgumentException e) { ... } pattern (with an additional early return for blank strings to preserve existing behavior), so we should be aligned here.
Please let me know if you’d like to change how blank/empty patterns are handled.

adamsaghy

Please kindly check my concern.

oluexpert99 · 2026-03-04T11:29:42Z

Please kindly check my concern.

@adamsaghy , concern addressed

adamsaghy · 2026-03-04T11:52:04Z

Please kindly check my concern.

@adamsaghy , concern addressed

I see no code changes, neither reasons why to leave it AS-IS. My concern was reflecting the fact it goes through multiple checks and multiple methods unnecessarily...

oluexpert99 · 2026-03-04T11:57:06Z

Please kindly check my concern.

@adamsaghy , concern addressed

I see no code changes, neither reasons why to leave it AS-IS. My concern was reflecting the fact it goes through multiple checks and multiple methods unnecessarily...

Hi @adamsaghy
There are two methods
one is used for the annotation @DateFormat on POJOs
isValidPattern was created for JSON commands that do not bind to a DTO ( where we cannot make use of the annotation)

oluexpert99 · 2026-03-04T12:10:45Z

Hi @adamsaghy , I update the code now , does this look better ?

adamsaghy · 2026-03-09T09:24:30Z

@oluexpert99 Please review and fix the failing checks!

oluexpert99 · 2026-03-17T14:11:47Z

Verify Liquibase Backward Compatibility / liquibase-backward-compatibility-check (pull_request)

Hi @adamsaghy , the failing tests are not connected to the fixes made. I guess it's a general issue as I saw migration mdsum failure

adamsaghy · 2026-03-17T14:13:05Z

@oluexpert99 Please rebase this PR with latest develop branch.

adamsaghy · 2026-03-17T14:26:29Z

@oluexpert99 Please make sure you are using rebase and squash your commits! ;)

IOhacker requested changes Feb 28, 2026

View reviewed changes

oluexpert99 force-pushed the bugfix/FINERACT-2511 branch from 3002148 to bab5504 Compare February 28, 2026 09:06

oluexpert99 requested a review from IOhacker February 28, 2026 13:41

oluexpert99 changed the title ~~FINERACT-2511 : Return 400 for invalid dateFormat on client create/up…~~ FINERACT-2511: Return 400 for invalid dateFormat on client create/up… Feb 28, 2026

IOhacker approved these changes Mar 1, 2026

View reviewed changes

adamsaghy reviewed Mar 2, 2026

View reviewed changes

adamsaghy requested changes Mar 2, 2026

View reviewed changes

oluexpert99 force-pushed the bugfix/FINERACT-2511 branch from 77f458a to 2364bc6 Compare March 17, 2026 10:47

oluexpert99 requested a review from adamsaghy March 17, 2026 11:12

oluexpert99 closed this Mar 17, 2026

oluexpert99 force-pushed the bugfix/FINERACT-2511 branch from f9074a6 to 1cdc97a Compare March 17, 2026 14:38

Conversation

oluexpert99 commented Feb 28, 2026

Description

Checklist

Uh oh!

IOhacker commented Feb 28, 2026

Uh oh!

IOhacker left a comment

Choose a reason for hiding this comment

Uh oh!

oluexpert99 commented Feb 28, 2026

Uh oh!

IOhacker left a comment

Choose a reason for hiding this comment

Uh oh!

adamsaghy Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

oluexpert99 Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

adamsaghy left a comment

Choose a reason for hiding this comment

Uh oh!

oluexpert99 commented Mar 4, 2026

Uh oh!

adamsaghy commented Mar 4, 2026

Uh oh!

oluexpert99 commented Mar 4, 2026

Uh oh!

oluexpert99 commented Mar 4, 2026

Uh oh!

adamsaghy commented Mar 9, 2026

Uh oh!

oluexpert99 commented Mar 17, 2026

Uh oh!

adamsaghy commented Mar 17, 2026

Uh oh!

adamsaghy commented Mar 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants