Skip to content

FINERACT-2293: New command processing - create client address#6049

Draft
nidhiii128 wants to merge 1 commit into
apache:developfrom
nidhiii128:FINERACT-2293-create-client-address
Draft

FINERACT-2293: New command processing - create client address#6049
nidhiii128 wants to merge 1 commit into
apache:developfrom
nidhiii128:FINERACT-2293-create-client-address

Conversation

@nidhiii128

Copy link
Copy Markdown
Contributor

JIRA: https://issues.apache.org/jira/browse/FINERACT-2293

Migrates the create client address endpoint (POST /v1/client/{clientId}/addresses?type={addressTypeId})
from the legacy JsonCommand command-processing path to the new typed fineract-command framework.

Scope (PR 1 of 2 for FINERACT-2293)

  • New typed AddressCreateRequest / AddressCreateResponse POJOs with Jakarta Validation
  • New AddressMapper (MapStruct) — no JsonObject in the new code path
  • New AddressDomainService / AddressDomainServiceImpl — typed write service following the
    template module (FINERACT-2564) pattern
  • New AddressCreateCommand / AddressCreateCommandHandler using CommandHandler<REQ, RES>
  • Controller routes through CommandDispatcher and returns AddressCreateResponse directly
  • AntMatcher added in SecurityConfig for POST /api//client//addresses (CREATE_ADDRESS)
  • Resilience4j retry config for commandAddressCreate
  • Validation message bundle entries added
  • Legacy AddClientAddressCommandHandler deleted
  • Legacy addClientAddress(Long, Long, JsonCommand) method removed from
    AddressWritePlatformService (no remaining callers)
  • Legacy CommandWrapperBuilder.addClientAddress(Long, Long) builder method removed

addNewClientAddress(Client, JsonCommand) kept on AddressWritePlatformService since it is
still called by the client creation flow (ClientWritePlatformServiceJpaRepositoryImpl).
That migration is out of scope here.

Pattern follows the merged template-module reference (#5851).

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.

@nidhiii128 nidhiii128 force-pushed the FINERACT-2293-create-client-address branch 2 times, most recently from da0273f to 54d08b9 Compare June 29, 2026 07:30
@adamsaghy

Copy link
Copy Markdown
Contributor
* What went wrong:
Execution failed for task ':integration-tests:compileTestJava'.
> Compilation failed; see the compiler output below.
  /home/runner/work/fineract/fineract/integration-tests/src/test/java/org/apache/fineract/integrationtests/SmsCampaignIntegrationTest.java:66: warning: [removal] initializeRESTAssured() in Utils has been deprecated and marked for removal
          Utils.initializeRESTAssured();
               ^
  2 errors

Please run:
./gradlew spotlessApply spotbugsMain spotbugsTest checkstyleMain checkstyleTest
./gradlew --no-daemon build -x test -x cucumber -x doc

Before any PR or changes, please always run these two commands and make sure there is green build!

@nidhiii128 nidhiii128 force-pushed the FINERACT-2293-create-client-address branch from 54d08b9 to d064384 Compare June 30, 2026 12:37
@Aman-Mittal

Copy link
Copy Markdown
Member

@adamsaghy @vidakovic please review

}

public CommandWrapperBuilder addClientAddress(final long clientId, final long addressTypeId) {
public CommandWrapperBuilder addClientAddress(final Long clientId, final Long addressTypeId) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this when we migrate to new command processing?


CommandProcessingResult addClientAddress(Long clientId, Long addressTypeId, JsonCommand command);

CommandProcessingResult addNewClientAddress(Client client, JsonCommand command);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, belongs to legacy command processing.


CommandProcessingResult addNewClientAddress(Client client, JsonCommand command);

CommandProcessingResult updateClientAddress(Long clientId, JsonCommand command);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, belongs to legacy command processing.

@@ -53,27 +53,6 @@ public class AddressWritePlatformServiceImpl implements AddressWritePlatformServ
private final ClientAddressRepositoryWrapper clientAddressRepositoryWrapper;
private final AddressCommandFromApiJsonDeserializer fromApiJsonDeserializer;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, manual JSON parsing. We want to get away from this.

}

@Override
public CommandProcessingResult addNewClientAddress(final Client client, final JsonCommand command) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, belongs to legacy command processing.

import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;

@ExtendWith(MockitoExtension.class)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as in the PR #6085 concerning testing.

@@ -265,14 +264,14 @@ public static PostClientsResponse createClient(final RequestSpecification reques
// Example: org.apache.fineract.integrationtests.common.loans.LoanTransactionHelper.disburseLoan(java.lang.Long,
// org.apache.fineract.client.models.PostLoansLoanIdRequest)
@Deprecated(forRemoval = true)
public static PostClientClientIdAddressesResponse createClientAddress(final RequestSpecification requestSpec,
public static HashMap<String, Object> createClientAddress(final RequestSpecification requestSpec,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks wrong. We lost the type information and the whole idea of this code change is to have proper types in the first place. Something is wrong in the REST API resource class!

.stateProvinceId(Long.valueOf(stateId)).isActive(addressIsActive);
PostClientClientIdAddressesResponse response = ClientHelper.createClientAddress(requestSpec, responseSpec, clientId.longValue(),
addressTypeId, request);
HashMap<String, Object> response = ClientHelper.createClientAddress(requestSpec, responseSpec, clientId.longValue(), addressTypeId,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks wrong. We lost the type information and the whole idea of this code change is to have proper types in the first place. Something is wrong in the REST API resource class!

@nidhiii128 nidhiii128 marked this pull request as draft July 10, 2026 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants