Fix TL enrichment null safety in plain search - #425
Conversation
WalkthroughReplaced strict null checks with tolerant extraction for locality data in BoundaryService and added guards to skip licenses lacking locality codes. EnrichmentService now logs missing user entries during owner enrichment instead of throwing exceptions and checks for null/empty owner lists before iterating. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@tradelicense/tl-services/src/main/java/org/egov/tl/service/BoundaryService.java`:
- Around line 90-100: In BoundaryService where you iterate
request.getLicenses(), you read context.read(jsonPath) into boundaryObject and
then call context.read(jsonPath) again for boundaryResponse; change the code to
reuse the already-read boundaryObject by casting it to the expected type (e.g.,
ArrayList) and assign that to boundaryResponse (e.g., ArrayList boundaryResponse
= (ArrayList) boundaryObject) after your instanceof and emptiness checks,
removing the second context.read(jsonPath) call; keep the existing checks using
boundaryObject and preserve variable names propertyIdToJsonPath, jsonPath,
context.read, boundaryObject, and boundaryResponse.
- Around line 53-62: Collecting localityCode into the localities set should be
followed by an early return when localities is empty to avoid calling the
location-service unnecessarily; in BoundaryService, after the block that adds
localityCode to localities (variables: localityCode, localities), check if
localities.isEmpty() and return the appropriate empty result for that method
(e.g., empty map/list) so the downstream location-service call is skipped.
In
`@tradelicense/tl-services/src/main/java/org/egov/tl/service/EnrichmentService.java`:
- Around line 316-326: The block in EnrichmentService that iterates owners can
NPE because license.getTradeLicenseDetail() may be null; update the logic in the
method containing this block to first null-check TradeLicenseDetail (e.g.,
assign TradeLicenseDetail detail = license.getTradeLicenseDetail()) and only if
detail != null and CollectionUtils.isNotEmpty(detail.getOwners()) iterate
owners, using detail.getId() for logging; ensure you never call getOwners() or
getId() on a null TradeLicenseDetail.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tradelicense/tl-services/src/main/java/org/egov/tl/service/BoundaryService.java (1)
77-84: Redundant empty check after early return.The condition
if(!CollectionUtils.isEmpty(localities))is now always true because the method returns early at line 66-68 when localities is empty. Consider removing the redundant check for clarity.♻️ Suggested cleanup
- if(!CollectionUtils.isEmpty(localities)) { - uri.append("&").append("codes="); - for (int i = 0; i < localities.size(); i++) { - uri.append(localities.get(i)); - if(i!=localities.size()-1) - uri.append(","); - } + uri.append("&").append("codes="); + for (int i = 0; i < localities.size(); i++) { + uri.append(localities.get(i)); + if(i!=localities.size()-1) + uri.append(","); }
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.