AshharAhmadKhan opened a new pull request, #6403:
URL: https://github.com/apache/fineract/pull/6403
## Description
`ClientDataValidator` and the `@Pattern` annotations used by Staff
create/update requests independently hardcoded the same mobile number regex:
`^+?[0-9]{7,15}$`
This duplicated the validation logic across multiple locations, creating
multiple sources of truth and making future changes error-prone.
This PR centralizes the existing backend phone-number validation by
introducing a configurable phone-number regex and a shared validation service
used by both Client and Staff.
## Changes
- Added `fineract.phone.regex` under `FineractProperties` through
`FineractPhoneProperties`, with the existing regex as the default.
- Added `PhoneNumberValidationService` as the single source of truth for
phone-number regex validation.
- Added `@ValidPhoneNumber` and `PhoneNumberValidator` for Staff
create/update requests.
- Updated `ClientDataValidator` to receive `PhoneNumberValidationService`
through constructor injection.
- Removed the hardcoded `MOBILE_NUMBER_REGEX` from Client validation.
- Updated both Client create and update validation paths to use the shared
service.
- Updated `ClientDataValidatorTest` for the new constructor dependency.
- Added `PhoneNumberValidationServiceTest`.
- Added `PhoneNumberValidatorTest`.
- Added Client integration tests covering invalid mobile numbers during
create and update.
## Runtime Issue Identified During Verification
During end-to-end verification, a runtime issue was identified that was not
covered by the existing unit tests.
Spring's `@ConfigurationProperties` binding does not instantiate the nested
`FineractPhoneProperties` object when no matching `fineract.phone.*` property
exists in the environment. As a result, when `fineract.phone.regex` was absent,
`getPhone()` could return `null`, causing `getRegex()` to throw a
`NullPointerException`.
This was not detected by the unit tests because they manually constructed
`FineractProperties` rather than exercising the real Spring
configuration-binding path.
The issue was reproduced against a Docker-built Fineract server, where the
Staff integration test initially failed 3 of 17 tests with HTTP 500 responses.
The configuration was then corrected so that the default regex is available
through the actual application configuration, and integration coverage was
added to verify the real Spring binding path.
## Previous Attempts
### PR #5964
The first attempt introduced `PhoneNumberValidationService`, but it was not
wired into `ClientDataValidator`.
`ClientDataValidator` continued to use `FineractProperties` directly,
leaving the intended centralization incomplete.
The PR subsequently failed during `compileJava` after a force push.
### PR #6177
The second attempt correctly wired `PhoneNumberValidationService` into
`ClientDataValidator`, but introduced several issues:
- A duplicate `FineractPhoneValidationProperties` class definition in
`FineractProperties.java`.
- `@ValidPhoneNumber.message()` used the literal `"Invalid phone number"`
instead of the existing i18n message key.
- CI checks failed.
- The branch also encountered merge conflicts.
Neither previous PR identified the Spring configuration-binding runtime
issue because the necessary integration coverage was missing.
## Verification
The implementation was verified at multiple levels:
- `PhoneNumberValidationServiceTest`
- `PhoneNumberValidatorTest`
- Updated `ClientDataValidatorTest`
- Client create integration test with an invalid mobile number
- Client update integration test with an invalid mobile number
- Staff integration testing
- Real Spring `@ConfigurationProperties` binding
- Docker-built Fineract server
- End-to-end backend validation
The Client integration tests specifically verify that the configured regex
is correctly bound through Spring rather than only testing manually constructed
configuration objects.
## Scope
This PR covers the backend centralization of the existing phone-number regex
validation.
FINERACT-405 also includes work outside the scope of this PR:
- UI-side phone-number validation
- A dedicated `PhoneNumber` value object
## Result
Client and Staff backend phone-number validation now use the same
configurable validation mechanism instead of maintaining independent copies of
the regex.
The resulting flow is:
`fineract.phone.regex` → `FineractPhoneProperties` →
`PhoneNumberValidationService` → Client/Staff validation
This provides a single backend source of truth while preserving the existing
validation behaviour and validation messages.
## Related Issue
FINERACT-405: Centralize mobile number validation for Client and Staff
https://issues.apache.org/jira/browse/FINERACT-405
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
