You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fineract.apache.org by "nidhiii128 (via GitHub)" <gi...@apache.org> on 2026/06/27 12:01:06 UTC
[PR] FINERACT-2293: New command processing - create client address [fineract]
nidhiii128 opened a new pull request, #6049:
URL: https://github.com/apache/fineract/pull/6049
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 (apache/fineract#5851).
## Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
- [ ] Write the commit message as per [our guidelines](https://github.com/apache/fineract/blob/develop/CONTRIBUTING.md#pull-requests)
- [ ] 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](https://fineract.apache.org/docs/current/#_testing) for verifying the changes made.
- [ ] Follow our [coding conventions](https://cwiki.apache.org/confluence/display/FINERACT/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"](https://cwiki.apache.org/confluence/display/FINERACT/Pull+Request+Size+Limit). Large changes can be made in a branch, with assistance. Ask for help on the [developer mailing list](https://fineract.apache.org/#contribute).
Your assigned reviewer(s) will follow our [guidelines for code reviews](https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide).
--
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: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2293: New command processing - create client address [fineract]
Posted by "vidakovic (via GitHub)" <gi...@apache.org>.
vidakovic commented on code in PR #6049:
URL: https://github.com/apache/fineract/pull/6049#discussion_r3538021764
##########
fineract-provider/src/test/java/org/apache/fineract/portfolio/address/handler/AddressCreateCommandHandlerTest.java:
##########
@@ -0,0 +1,61 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.address.handler;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import org.apache.fineract.portfolio.address.command.AddressCreateCommand;
+import org.apache.fineract.portfolio.address.data.AddressCreateRequest;
+import org.apache.fineract.portfolio.address.data.AddressCreateResponse;
+import org.apache.fineract.portfolio.address.service.AddressDomainService;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@ExtendWith(MockitoExtension.class)
Review Comment:
Same comment as in the PR #6085 concerning testing.
--
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: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2293: New command processing - create client address [fineract]
Posted by "Aman-Mittal (via GitHub)" <gi...@apache.org>.
Aman-Mittal commented on PR #6049:
URL: https://github.com/apache/fineract/pull/6049#issuecomment-4875008957
@adamsaghy @vidakovic please review
--
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: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2293: New command processing - create client address [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #6049:
URL: https://github.com/apache/fineract/pull/6049#issuecomment-4833033938
```
* 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!
--
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: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2293: New command processing - create client address [fineract]
Posted by "Aman-Mittal (via GitHub)" <gi...@apache.org>.
Aman-Mittal commented on PR #6049:
URL: https://github.com/apache/fineract/pull/6049#issuecomment-5057794859
@vidakovic Pls review
--
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: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2293: New command processing - create client address [fineract]
Posted by "nidhiii128 (via GitHub)" <gi...@apache.org>.
nidhiii128 commented on PR #6049:
URL: https://github.com/apache/fineract/pull/6049#issuecomment-4955259280
Thanks for the review @vidakovic , here's what I changed:
1. Deleted addClientAddress from CommandWrapperBuilder.
2. Deleted addClientAddress + addNewClientAddress from
3. AddressWritePlatformService (interface + impl), including the private
createAddress / createClientAddress helpers.
4. Migrated the caller in ClientWritePlatformServiceJpaRepositoryImpl.
createClient -> it now builds typed AddressCreateRequest objects and
calls AddressDomainService.create() directly. createClient itself is
still JsonCommand end-to-end (its full typed migration is a separate
PR since it's a large surface), so the request is populated via
fromApiJsonHelper matching the pattern since the rest of that method
already uses.
5. Removed context.authenticatedUser() from AddressDomainServiceImpl.create.
6. Reverted the HashMap regression in ClientHelper / ClientTest back to
typed AddressCreateResponse.
7. Moved the SecurityConfig client/addresses entry to the end of the
authorization list.
8. Kept the mock test in place per your comment.
updateClientAddress kept on the interface for now it's migrated in
the stacked PR https://github.com/apache/fineract/pull/6085.
AddressCommandFromApiJsonDeserializer kept in the impl because
updateClientAddress still uses it. Removed in https://github.com/apache/fineract/pull/6085 along with the
update migration.
--
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: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2293: New command processing - create client address [fineract]
Posted by "vidakovic (via GitHub)" <gi...@apache.org>.
vidakovic commented on code in PR #6049:
URL: https://github.com/apache/fineract/pull/6049#discussion_r3538013265
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/address/service/AddressWritePlatformService.java:
##########
@@ -24,8 +24,6 @@
public interface AddressWritePlatformService {
- CommandProcessingResult addClientAddress(Long clientId, Long addressTypeId, JsonCommand command);
-
CommandProcessingResult addNewClientAddress(Client client, JsonCommand command);
Review Comment:
Nope, belongs to legacy command processing.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/address/service/AddressWritePlatformService.java:
##########
@@ -24,8 +24,6 @@
public interface AddressWritePlatformService {
- CommandProcessingResult addClientAddress(Long clientId, Long addressTypeId, JsonCommand command);
-
CommandProcessingResult addNewClientAddress(Client client, JsonCommand command);
CommandProcessingResult updateClientAddress(Long clientId, JsonCommand command);
Review Comment:
Nope, belongs to legacy command processing.
--
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: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2293: New command processing - create client address [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #6049:
URL: https://github.com/apache/fineract/pull/6049#issuecomment-6238867231
@nidhiii128 Please rebase onto the latest `develop` and resolve the conflicts.
--
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: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2293: New command processing - create client address [fineract]
Posted by "vidakovic (via GitHub)" <gi...@apache.org>.
vidakovic commented on code in PR #6049:
URL: https://github.com/apache/fineract/pull/6049#discussion_r3538016531
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/address/service/AddressWritePlatformServiceImpl.java:
##########
@@ -53,27 +53,6 @@ public class AddressWritePlatformServiceImpl implements AddressWritePlatformServ
private final ClientAddressRepositoryWrapper clientAddressRepositoryWrapper;
private final AddressCommandFromApiJsonDeserializer fromApiJsonDeserializer;
Review Comment:
Nope, manual JSON parsing. We want to get away from this.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/address/service/AddressWritePlatformServiceImpl.java:
##########
@@ -53,27 +53,6 @@ public class AddressWritePlatformServiceImpl implements AddressWritePlatformServ
private final ClientAddressRepositoryWrapper clientAddressRepositoryWrapper;
private final AddressCommandFromApiJsonDeserializer fromApiJsonDeserializer;
- @Override
- public CommandProcessingResult addClientAddress(final Long clientId, final Long addressTypeId, final JsonCommand command) {
- JsonObject jsonObject = command.parsedJson().getAsJsonObject();
- context.authenticatedUser();
- fromApiJsonDeserializer.validateForCreate(jsonObject.toString(), false);
-
- final CodeValue addressTypeIdCodeValue = codeValueRepository.getReferenceById(addressTypeId);
- final Client client = clientRepositoryWrapper.findOneWithNotFoundDetection(clientId);
-
- final Address address = createAddress(jsonObject);
- addressRepository.save(address);
-
- final ClientAddress clientAddress = createClientAddress(client, jsonObject, addressTypeIdCodeValue, address);
- clientAddressRepository.saveAndFlush(clientAddress);
-
- return new CommandProcessingResultBuilder() //
- .withCommandId(command.commandId()) //
- .withEntityId(clientAddress.getId()) //
- .build();
- }
-
@Override
public CommandProcessingResult addNewClientAddress(final Client client, final JsonCommand command) {
Review Comment:
Nope, belongs to legacy command processing.
--
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: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2293: New command processing - create client address [fineract]
Posted by "vidakovic (via GitHub)" <gi...@apache.org>.
vidakovic commented on code in PR #6049:
URL: https://github.com/apache/fineract/pull/6049#discussion_r3538002379
##########
fineract-core/src/main/java/org/apache/fineract/commands/service/CommandWrapperBuilder.java:
##########
@@ -365,12 +364,12 @@ public CommandWrapperBuilder updateCreditBureauConfiguration(final long configur
return this;
}
- public CommandWrapperBuilder addClientAddress(final long clientId, final long addressTypeId) {
+ public CommandWrapperBuilder addClientAddress(final Long clientId, final Long addressTypeId) {
Review Comment:
Why do we need this when we migrate to new command processing?
--
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: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2293: New command processing - create client address [fineract]
Posted by "vidakovic (via GitHub)" <gi...@apache.org>.
vidakovic commented on code in PR #6049:
URL: https://github.com/apache/fineract/pull/6049#discussion_r3538028476
##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/common/ClientHelper.java:
##########
@@ -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,
Review Comment:
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!
##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/ClientTest.java:
##########
@@ -254,10 +253,10 @@ public void testClientAddressCreationWorksAfterClientIsCreated() {
// when
ClientAddressRequest request = new ClientAddressRequest().postalCode(postalCode).city(city).countryId(Long.valueOf(countryId))
.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,
Review Comment:
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!
--
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: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2293: New command processing - create client address [fineract]
Posted by "Aman-Mittal (via GitHub)" <gi...@apache.org>.
Aman-Mittal commented on PR #6049:
URL: https://github.com/apache/fineract/pull/6049#issuecomment-4954435227
@nidhiii128 Also resolve the comments that are addressed
--
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: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org