MOSIP-44737 Changed PARTNER_API_URL to use new api /admin-partner/v2 - #1641
Merged
Conversation
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughPartner-manager v2 integration expands partner metadata, updates endpoint configuration, adds cached issuer lookups, makes partner matching null-safe, and retrieves partner-type lists through paginated v2 responses transformed to the existing partner shape. ChangesPartner API v2 integration
Estimated code review effort: 4 (Complex) | ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@resident/resident-service/src/main/java/io/mosip/resident/service/impl/ResidentCredentialServiceImpl.java`:
- Line 175: The partner URL lookup in ResidentCredentialServiceImpl now depends
on ApiName.PARTNER_API_URL_V2, so update the checked-in runtime profile
configuration to define PARTNER_API_URL_V2 alongside the existing partner URL
entries. Make sure the new property is present in the committed profile files
used at runtime so env.getProperty(ApiName.PARTNER_API_URL_V2.name()) resolves
correctly before concatenating with dto.getIssuer().
In
`@resident/resident-service/src/main/java/io/mosip/resident/util/PartnersByPartnerType.java`:
- Around line 39-40: Validate the configured resident.partner.list.page.size
before it is used in PartnersByPartnerType, since a zero or negative pageSize
can be passed through to the partner-manager request and break pagination. Add a
guard in the flow that builds the query params (including the code paths around
the current pageSize usage and the related logic at the later referenced lines)
to ensure pageSize is positive, and fall back to a safe default or fail fast
with a clear error before calling the partner-manager API.
- Around line 80-90: Update testGetPartnersByPartnerTypeV2 in
ProxyPartnerManagementServiceTest to return a paginated Map<String, Object>
matching PartnersByPartnerType.getPartnersByPartnerType() expectations,
including totalResults and data, instead of a plain "Response" string. Make sure
the mocked responseWrapper.getResponse() now aligns with the new casting logic
in PartnersByPartnerType so the test exercises the map-based contract without
failing on the cast.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c4115674-0020-4fae-aa9e-b0deac9482aa
📒 Files selected for processing (3)
resident/resident-service/src/main/java/io/mosip/resident/constant/ApiName.javaresident/resident-service/src/main/java/io/mosip/resident/service/impl/ResidentCredentialServiceImpl.javaresident/resident-service/src/main/java/io/mosip/resident/util/PartnersByPartnerType.java
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
resident/resident-service/src/test/java/io/mosip/resident/service/impl/ProxyPartnerManagementServiceTest.java (1)
134-153: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a real multi-page merge case here.
This stub only returns one page, so it won't catch regressions in the new pagination loop. Please chain two
getApi(...)responses and assert that both partners are present in the mergeddatalist.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resident/resident-service/src/test/java/io/mosip/resident/service/impl/ProxyPartnerManagementServiceTest.java` around lines 134 - 153, The current test for ProxyPartnerManagementServiceTest only mocks a single page, so it does not exercise the pagination merge logic in getPartnersByPartnerType. Update the test to stub residentServiceRestClient.getApi(...) with two sequential ResponseWrapper responses representing separate pages, then verify the merged ResponseWrapper<?> contains both partner entries in the final response data list. Use the existing partnerByPartnerType setup and the getPartnersByPartnerType method to keep the test aligned with the new pagination loop.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@resident/resident-service/src/main/java/io/mosip/resident/service/impl/ProxyPartnerManagementServiceImpl.java`:
- Around line 47-58: The partner lookup in ProxyPartnerManagementServiceImpl
currently caches results without accounting for partnerType, which can return
stale or incorrect partner details across types. Update the cache key used by
the method that calls partnersByPartnerTypeCache.getPartnersByPartnerType so it
includes both partnerId and partnerType, and make sure any cache lookup/build
logic in this path uses the same combined key consistently.
---
Nitpick comments:
In
`@resident/resident-service/src/test/java/io/mosip/resident/service/impl/ProxyPartnerManagementServiceTest.java`:
- Around line 134-153: The current test for ProxyPartnerManagementServiceTest
only mocks a single page, so it does not exercise the pagination merge logic in
getPartnersByPartnerType. Update the test to stub
residentServiceRestClient.getApi(...) with two sequential ResponseWrapper
responses representing separate pages, then verify the merged ResponseWrapper<?>
contains both partner entries in the final response data list. Use the existing
partnerByPartnerType setup and the getPartnersByPartnerType method to keep the
test aligned with the new pagination loop.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0d839079-e463-4479-b263-76abd07406b0
📒 Files selected for processing (5)
resident/resident-service/src/main/java/io/mosip/resident/constant/ResidentConstants.javaresident/resident-service/src/main/java/io/mosip/resident/service/impl/PartnerServiceImpl.javaresident/resident-service/src/main/java/io/mosip/resident/service/impl/ProxyPartnerManagementServiceImpl.javaresident/resident-service/src/test/java/io/mosip/resident/service/impl/PartnerServiceImplTest.javaresident/resident-service/src/test/java/io/mosip/resident/service/impl/ProxyPartnerManagementServiceTest.java
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
resident/resident-service/src/main/resources/application-local.properties (1)
258-260: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInverted naming convention between
PARTNER_API_URLandPARTNER_API_URL_V2.
PARTNER_API_URLnow points to the v2 admin-partners endpoint (list/paginated), whilePARTNER_API_URL_V2points to the v1 partners endpoint (single-partner lookup). This is semantically inverted: the "V2" suffix denotes the older API version for single lookups, while the non-suffixed name denotes the newer v2 API. This naming mismatch is confusing and may lead to future maintenance errors.Consider renaming to align with semantic meaning, e.g.:
PARTNER_API_URL→PARTNER_ADMIN_API_URLorPARTNER_LIST_API_URLPARTNER_API_URL_V2→PARTNER_DETAIL_API_URLor keep asPARTNER_API_URL_V2but swap which endpoint each points toAt minimum, ensure the comment on Line 259-260 clearly explains this inversion so future maintainers don't misroute calls.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resident/resident-service/src/main/resources/application-local.properties` around lines 258 - 260, The partner API properties are semantically inverted: PARTNER_API_URL points to the admin-partners v2 endpoint while PARTNER_API_URL_V2 points to the single-partner v1 endpoint used by ResidentCredentialServiceImpl. Update the naming to match the actual usage, or swap the assignments so the suffixes align with the endpoint versions; if you keep the current names, clarify this inversion in the inline comment near PARTNER_API_URL_V2 so future changes in ResidentCredentialServiceImpl and related config references don’t misroute calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@resident/resident-service/src/main/resources/application-local.properties`:
- Around line 258-260: The partner API properties are semantically inverted:
PARTNER_API_URL points to the admin-partners v2 endpoint while
PARTNER_API_URL_V2 points to the single-partner v1 endpoint used by
ResidentCredentialServiceImpl. Update the naming to match the actual usage, or
swap the assignments so the suffixes align with the endpoint versions; if you
keep the current names, clarify this inversion in the inline comment near
PARTNER_API_URL_V2 so future changes in ResidentCredentialServiceImpl and
related config references don’t misroute calls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8c98c2cc-7d57-4db5-88d8-71bbaf638f86
📒 Files selected for processing (5)
resident/resident-service/src/main/java/io/mosip/resident/constant/ApiName.javaresident/resident-service/src/main/java/io/mosip/resident/service/impl/ProxyPartnerManagementServiceImpl.javaresident/resident-service/src/main/java/io/mosip/resident/util/PartnersByPartnerType.javaresident/resident-service/src/main/resources/application-local.propertiesresident/resident-service/src/test/resources/bootstrap.properties
💤 Files with no reviewable changes (1)
- resident/resident-service/src/main/java/io/mosip/resident/constant/ApiName.java
🚧 Files skipped from review as they are similar to previous changes (2)
- resident/resident-service/src/main/java/io/mosip/resident/service/impl/ProxyPartnerManagementServiceImpl.java
- resident/resident-service/src/main/java/io/mosip/resident/util/PartnersByPartnerType.java
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
resident/resident-service/src/main/java/io/mosip/resident/util/PartnersByPartnerType.java (1)
96-109: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAdd a fallback termination condition to prevent silent data truncation.
If
totalResultsis not aNumber(e.g., returned as a String or missing), it stays at the initial value of 0. The loop conditionmergedData.size() < totalResultsthen evaluates tomergedData.size() < 0, which is always false, so the loop terminates after the first page — silently returning incomplete partner data.Adding a
pageData.size() < effectivePageSizebreak detects the last page independently oftotalResults.🛡️ Proposed fix
mergedData.addAll(pageData); + if (pageData.size() < effectivePageSize) { + // Last page reached; don't rely solely on totalResults. + break; + } pageNo++; } while (mergedData.size() < totalResults);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resident/resident-service/src/main/java/io/mosip/resident/util/PartnersByPartnerType.java` around lines 96 - 109, Update the pagination loop in PartnersByPartnerType around totalResults, pageData, and mergedData so it also terminates when pageData.size() is smaller than the effective page size, regardless of totalResults validity. Preserve the existing empty-page break and ensure valid totalResults pagination continues while incomplete final pages stop without truncating data.
🧹 Nitpick comments (1)
resident/resident-service/src/test/java/io/mosip/resident/service/impl/ProxyPartnerManagementServiceTest.java (1)
132-162: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a multi-page pagination test.
testGetPartnersByPartnerTypeV2only covers a single-page scenario (totalResults=1). The core pagination loop — page accumulation, loop termination viatotalResults, and multi-page merging — is untested. A test withtotalResults > pageSizethat mocks multiple API responses and verifies all partners are merged would exercise this critical path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resident/resident-service/src/test/java/io/mosip/resident/service/impl/ProxyPartnerManagementServiceTest.java` around lines 132 - 162, The testGetPartnersByPartnerTypeV2 test only validates one response page; extend it to cover pagination by setting totalResults beyond the configured page size, mocking successive residentServiceRestClient.getApi responses for multiple pages, and verifying all partners are merged into the result. Also verify the client is invoked once per page and pagination terminates when the accumulated results reach totalResults.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@resident/resident-service/src/main/java/io/mosip/resident/service/impl/PartnerServiceImpl.java`:
- Around line 41-46: Update the partner ID handling in toV1Partners to retrieve
PMS_PARTNER_ID safely, check for null before converting it to a string, and skip
malformed partner records without adding an ID or aborting the entire list
retrieval.
---
Outside diff comments:
In
`@resident/resident-service/src/main/java/io/mosip/resident/util/PartnersByPartnerType.java`:
- Around line 96-109: Update the pagination loop in PartnersByPartnerType around
totalResults, pageData, and mergedData so it also terminates when
pageData.size() is smaller than the effective page size, regardless of
totalResults validity. Preserve the existing empty-page break and ensure valid
totalResults pagination continues while incomplete final pages stop without
truncating data.
---
Nitpick comments:
In
`@resident/resident-service/src/test/java/io/mosip/resident/service/impl/ProxyPartnerManagementServiceTest.java`:
- Around line 132-162: The testGetPartnersByPartnerTypeV2 test only validates
one response page; extend it to cover pagination by setting totalResults beyond
the configured page size, mocking successive residentServiceRestClient.getApi
responses for multiple pages, and verifying all partners are merged into the
result. Also verify the client is invoked once per page and pagination
terminates when the accumulated results reach totalResults.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f7b7e143-55be-42cf-acf3-f41196c9b12e
📒 Files selected for processing (5)
resident/resident-service/src/main/java/io/mosip/resident/constant/ResidentConstants.javaresident/resident-service/src/main/java/io/mosip/resident/service/impl/PartnerServiceImpl.javaresident/resident-service/src/main/java/io/mosip/resident/service/impl/ProxyPartnerManagementServiceImpl.javaresident/resident-service/src/main/java/io/mosip/resident/util/PartnersByPartnerType.javaresident/resident-service/src/test/java/io/mosip/resident/service/impl/ProxyPartnerManagementServiceTest.java
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Contributor
Author
|
@CodeRabbit please review |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
resident/resident-service/src/main/java/io/mosip/resident/util/PartnersByPartnerType.java (1)
48-124: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAdd an iteration cap to the pagination loop.
The loop is bounded by
pageDataemptiness, so it isn't truly infinite, but a corrupted/oversizedtotalResultsfrom partner-manager v2 combined with many non-empty pages could drive a long chain of blocking synchronous REST calls on the caller's thread with no upper bound. A small safety cap protects this resident-facing path from a slow/hanging request.♻️ Proposed fix
+ final int maxIterations = 1000; // safety cap; a legitimate partner list should never need this many pages + int iterations = 0; try { do { + if (++iterations > maxIterations) { + logger.error("Aborting partner list pagination after {} pages - possible corrupt totalResults", maxIterations); + break; + } List<String> pathsegements = null;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resident/resident-service/src/main/java/io/mosip/resident/util/PartnersByPartnerType.java` around lines 48 - 124, Update getPartnersByPartnerType pagination to enforce a small maximum page-iteration cap in addition to the existing totalResults and empty-page conditions. Track iterations using the existing pageNo or a dedicated counter, stop requesting further pages once the cap is reached, and preserve the current merge and response behavior for pages processed within the limit.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@resident/resident-service/src/main/java/io/mosip/resident/util/PartnerByIssuerCache.java`:
- Around line 31-37: Update the `@Cacheable` configuration on getPartnerByIssuer
so unsuccessful or empty partner lookups are excluded from partnerByIssuerCache.
Use an unless condition that checks the returned ResponseWrapper and prevents
caching when it is null or its response payload (getResponse()) is null, while
preserving caching for valid partner responses.
In
`@resident/resident-service/src/test/java/io/mosip/resident/util/PartnersByPartnerTypeTest.java`:
- Around line 108-366: Rename every test method in PartnersByPartnerTypeTest to
follow the should_<expectedBehavior>_when_<condition> convention, including
testV2FieldsMappedToV1Aliases, testAllPagesAreFetchedAndMerged, and
testInvalidPageSizeFallsBackToDefault. Preserve each test’s behavior and use
descriptive expected-behavior and condition segments for all mapping,
pagination, query-parameter, empty-response, and error-path cases.
---
Outside diff comments:
In
`@resident/resident-service/src/main/java/io/mosip/resident/util/PartnersByPartnerType.java`:
- Around line 48-124: Update getPartnersByPartnerType pagination to enforce a
small maximum page-iteration cap in addition to the existing totalResults and
empty-page conditions. Track iterations using the existing pageNo or a dedicated
counter, stop requesting further pages once the cap is reached, and preserve the
current merge and response behavior for pages processed within the limit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 841b6b0d-1424-4ff1-8d24-d5bf25f93f2a
📒 Files selected for processing (13)
resident/resident-service/src/main/java/io/mosip/resident/constant/ApiName.javaresident/resident-service/src/main/java/io/mosip/resident/constant/ResidentConstants.javaresident/resident-service/src/main/java/io/mosip/resident/service/impl/PartnerServiceImpl.javaresident/resident-service/src/main/java/io/mosip/resident/service/impl/ProxyPartnerManagementServiceImpl.javaresident/resident-service/src/main/java/io/mosip/resident/service/impl/ResidentCredentialServiceImpl.javaresident/resident-service/src/main/java/io/mosip/resident/util/PartnerByIssuerCache.javaresident/resident-service/src/main/java/io/mosip/resident/util/PartnersByPartnerType.javaresident/resident-service/src/main/java/io/mosip/resident/util/Utility.javaresident/resident-service/src/main/resources/application-local.propertiesresident/resident-service/src/test/java/io/mosip/resident/service/impl/ProxyPartnerManagementServiceTest.javaresident/resident-service/src/test/java/io/mosip/resident/service/impl/ResidentCredentialServiceTest.javaresident/resident-service/src/test/java/io/mosip/resident/util/PartnersByPartnerTypeTest.javaresident/resident-service/src/test/resources/bootstrap.properties
💤 Files with no reviewable changes (2)
- resident/resident-service/src/test/resources/bootstrap.properties
- resident/resident-service/src/main/java/io/mosip/resident/constant/ApiName.java
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
ashok-ksharma
suggested changes
Jul 28, 2026
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
ashok-ksharma
approved these changes
Jul 28, 2026
dhanendra06
approved these changes
Jul 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Tests