Ambika-Sony commented on PR #5510:
URL: https://github.com/apache/fineract/pull/5510#issuecomment-3983086624
Hi, I've addressed all the edge cases and refactored the logic as
discussed. I noticed the workflows are currently awaiting approval to run.
Please let me know if you'd like me to adjust anything else once the builds
finish.
On Thu, Feb 26, 2026 at 9:04 PM Aman-Mittal ***@***.***>
wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In
>
fineract-core/src/main/java/org/apache/fineract/infrastructure/core/api/ApiParameterHelper.java
> <https://github.com/apache/fineract/pull/5510#discussion_r2859693758>:
>
> > }
> }
> return fields;
> }
>
> public static Set<String>
extractAssociationsForResponseIfProvided(final MultivaluedMap<String, String>
queryParams) {
> - Set<String> fields = new HashSet<>();
> - String commaSeparatedParameters = "";
> - if (queryParams.getFirst("associations") != null) {
> - commaSeparatedParameters =
queryParams.getFirst("associations");
> - if (StringUtils.isNotBlank(commaSeparatedParameters)) {
> - fields = new
HashSet<>(Arrays.asList(commaSeparatedParameters.split("\\s*,\\s*"))); //
NOSONAR
> + Objects.requireNonNull(queryParams, "queryParams map cannot be
null");
> + Set<String> associations = new HashSet<>();
> + String commaSeparatedParameters =
queryParams.getFirst("associations");
> +
> + if (StringUtils.isNotBlank(commaSeparatedParameters)) {
>
> This seems duplication
> ------------------------------
>
> In
>
fineract-core/src/test/java/org/apache/fineract/infrastructure/core/api/ApiParameterHelperTest.java
> <https://github.com/apache/fineract/pull/5510#discussion_r2859724547>:
>
> > + */
> +package org.apache.fineract.infrastructure.core.api;
> +
> +import static org.junit.jupiter.api.Assertions.assertEquals;
> +import static org.junit.jupiter.api.Assertions.assertNull;
> +import static org.junit.jupiter.api.Assertions.assertThrows;
> +import static org.junit.jupiter.api.Assertions.assertTrue;
> +
> +import jakarta.ws.rs.core.MultivaluedHashMap;
> +import jakarta.ws.rs.core.MultivaluedMap;
> +import java.util.Set;
> +import org.junit.jupiter.api.Test;
> +
> +class ApiParameterHelperTest {
> +
> + @Test
>
> The current test coverage is a good start and covers the basic happy paths
> and invalid number parsing. However, I think we can strengthen this further
> by adding a few important edge cases to better protect API behavior.
>
> Some scenarios that are currently not covered:
>
> Multiple query parameters with the same key
>
> e.g. ?fields=id,name&fields=description
>
> Same for associations
>
> Ensures proper flattening across multi-valued params.
>
> Empty string values
>
> ?fields=
>
> ?associations=
>
> Prevents accidental inclusion of empty entries.
>
> Duplicate values
>
> fields=id,name,id
>
> Verifies deduplication behavior since the return type is Set.
>
> Partial pagination parameters
>
> Only offset present
>
> Only limit present
>
> Ensures default behavior is explicitly verified.
>
> Negative pagination values
>
> offset=-1, limit=-10
>
> Clarifies whether negatives are allowed or should fail.
>
> Overflow cases
>
> Very large numeric values exceeding Integer range.
>
> Important defensive test for parsing logic.
>
> Adding these would make the test suite more robust and better document the
> intended contract of ApiParameterHelper, especially since these helpers are
> reused across many endpoints.
>
> —
> Reply to this email directly, view it on GitHub
>
<https://github.com/apache/fineract/pull/5510#pullrequestreview-3861748897>,
> or unsubscribe
>
<https://github.com/notifications/unsubscribe-auth/BIN7S2E4RKJOU4A4TQESRVT4N4HB7AVCNFSM6AAAAACVQUWMQCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTQNRRG42DQOBZG4>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
--
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]
