Dev reindex #835
Dev reindex #835
Conversation
WalkthroughThis pull request introduces a consistent pattern of implementing plain search functionality across multiple backend services in the project. The changes span repositories like Contracts, Estimates, Individuals, and Muster Rolls, adding new methods for bulk searching and retrieving entities with pagination support. Each service follows a similar implementation pattern: adding repository methods to fetch IDs and perform bulk searches, creating corresponding service methods, and exposing new API endpoints for plain search functionality. Changes
Sequence DiagramsequenceDiagram
participant API as API Controller
participant Service as Service Layer
participant Repository as Repository
participant Database as Database
API->>Service: Call plain search method
Service->>Repository: Fetch entity IDs
Repository->>Database: Query IDs with pagination
Database-->>Repository: Return ID list
Repository-->>Service: Return IDs
Service->>Repository: Fetch entities by IDs
Repository->>Database: Query full entity details
Database-->>Repository: Return entities
Repository-->>Service: Return entities
Service-->>API: Return search results
Possibly related PRs
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (17)
backend/contracts/src/main/java/org/egov/works/repository/ContractRepository.java(2 hunks)backend/contracts/src/main/java/org/egov/works/repository/querybuilder/ContractQueryBuilder.java(2 hunks)backend/contracts/src/main/java/org/egov/works/service/ContractService.java(1 hunks)backend/contracts/src/main/java/org/egov/works/web/controllers/ContractApiController.java(3 hunks)backend/estimates/src/main/java/org/egov/repository/EstimateRepository.java(2 hunks)backend/estimates/src/main/java/org/egov/repository/rowmapper/EstimateQueryBuilder.java(2 hunks)backend/estimates/src/main/java/org/egov/service/EstimateService.java(3 hunks)backend/estimates/src/main/java/org/egov/util/ProjectUtil.java(1 hunks)backend/estimates/src/main/java/org/egov/web/controllers/EstimateApiController.java(1 hunks)backend/individual/src/main/java/org/egov/individual/repository/IndividualRepository.java(2 hunks)backend/individual/src/main/java/org/egov/individual/service/IndividualService.java(1 hunks)backend/individual/src/main/java/org/egov/individual/web/controllers/IndividualApiController.java(2 hunks)backend/muster-roll/src/main/java/org/egov/repository/MusterRollRepository.java(2 hunks)backend/muster-roll/src/main/java/org/egov/repository/querybuilder/MusterRollQueryBuilder.java(2 hunks)backend/muster-roll/src/main/java/org/egov/service/MusterRollService.java(2 hunks)backend/muster-roll/src/main/java/org/egov/web/controllers/MusterRollApiController.java(1 hunks)build/build-config.yml(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
build/build-config.yml
[error] 277-277: trailing spaces
(trailing-spaces)
🔇 Additional comments (7)
backend/muster-roll/src/main/java/org/egov/repository/MusterRollRepository.java (1)
55-59: LGTM! Clean implementation of bulk search.The implementation correctly utilizes the query builder and row mapper.
backend/muster-roll/src/main/java/org/egov/web/controllers/MusterRollApiController.java (1)
74-78: LGTM! Consistent implementation of plain search endpoint.The implementation follows the established patterns for endpoints in the controller:
- Proper input validation
- Consistent error handling
- Standard response structure
backend/muster-roll/src/main/java/org/egov/service/MusterRollService.java (1)
336-342: LGTM! Clean implementation of the search interface.The method follows the service's pattern for response creation and error handling.
backend/contracts/src/main/java/org/egov/works/repository/ContractRepository.java (2)
55-59: LGTM! Implementation follows secure coding practices.The method correctly uses parameterized queries and follows the existing repository pattern.
73-74: 🧹 Nitpick (assertive)Add index hint for better performance.
The
ORDER BYclause onlast_modified_timeandidcolumns should leverage database indexes for optimal performance.- String orderbyClause = " order by last_modified_time,id offset ? limit ?"; + String orderbyClause = " USE INDEX (idx_contract_modified_time) order by last_modified_time,id offset ? limit ?";Likely invalid or redundant comment.
backend/estimates/src/main/java/org/egov/repository/EstimateRepository.java (1)
74-78: LGTM! Implementation follows secure coding practices.The method correctly uses parameterized queries and follows the existing repository pattern.
build/build-config.yml (1)
275-275: LGTM! Image names follow the consistent pattern.The image names for the individual service have been updated to include the "mukta-" prefix, maintaining consistency with other services in the configuration.
Also applies to: 277-277
| public List<String> fetchIds(MusterRollSearchCriteria criteria) { | ||
| List<Object> preparedStmtList = new ArrayList<>(); | ||
| String basequery = "select id from eg_wms_muster_roll"; | ||
| StringBuilder builder = new StringBuilder(basequery); | ||
|
|
||
| if(!ObjectUtils.isEmpty(criteria.getTenantId())) | ||
| { | ||
| builder.append(" where tenant_id=?"); | ||
| preparedStmtList.add(criteria.getTenantId()); | ||
| } | ||
|
|
||
| String orderbyClause = " order by lastmodifiedtime,id offset ? limit ?"; | ||
| builder.append(orderbyClause); | ||
| preparedStmtList.add(criteria.getOffset()); | ||
| preparedStmtList.add(criteria.getLimit()); | ||
|
|
||
| return jdbcTemplate.query(builder.toString(), preparedStmtList.toArray(), new SingleColumnRowMapper<>(String.class)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor query construction for better security and maintainability.
The current implementation has several areas for improvement:
- Use query builder pattern for consistency with other methods
- Use prepared statements for all dynamic parts including the WHERE clause
- Extract table name to a constant
Consider this implementation:
public List<String> fetchIds(MusterRollSearchCriteria criteria) {
List<Object> preparedStmtList = new ArrayList<>();
- String basequery = "select id from eg_wms_muster_roll";
- StringBuilder builder = new StringBuilder(basequery);
-
- if(!ObjectUtils.isEmpty(criteria.getTenantId()))
- {
- builder.append(" where tenant_id=?");
- preparedStmtList.add(criteria.getTenantId());
- }
-
- String orderbyClause = " order by lastmodifiedtime,id offset ? limit ?";
- builder.append(orderbyClause);
preparedStmtList.add(criteria.getOffset());
preparedStmtList.add(criteria.getLimit());
-
- return jdbcTemplate.query(builder.toString(), preparedStmtList.toArray(), new SingleColumnRowMapper<>(String.class));
+ return jdbcTemplate.query(
+ queryBuilder.getIdQuery(criteria, preparedStmtList),
+ preparedStmtList.toArray(),
+ new SingleColumnRowMapper<>(String.class)
+ );
}Committable suggestion skipped: line range outside the PR's diff.
| public String getMusterRollQueryForBulkSearch(MusterRollSearchCriteria criteria, List<Object> preparedStmtList) { | ||
|
|
||
| Boolean isEmpty = CollectionUtils.isEmpty(criteria.getIds()); | ||
|
|
||
| if(isEmpty) | ||
| throw new CustomException("EG_MR_SEARCH_ERROR"," No uuids given for the MusterRoll Bulk search"); | ||
|
|
||
| StringBuilder builder = new StringBuilder(FETCH_MUSTER_ROLL_QUERY); | ||
|
|
||
| if(!ObjectUtils.isEmpty(criteria.getTenantId())) | ||
| { | ||
| addClauseIfRequired(preparedStmtList, builder); | ||
| builder.append("muster.tenant_id=?"); | ||
| preparedStmtList.add(criteria.getTenantId()); | ||
| } | ||
|
|
||
| List<String> uuids = criteria.getIds(); | ||
| if (!CollectionUtils.isEmpty(uuids)) { | ||
| addClauseIfRequired(preparedStmtList,builder); | ||
| builder.append("muster.id IN (").append(createQuery(uuids)).append(")"); | ||
| addToPreparedStatement(preparedStmtList, uuids); | ||
| } | ||
|
|
||
| return builder.toString(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
🛠️ Refactor suggestion
Add ORDER BY clause for consistent pagination results.
The bulk search query should include ordering to ensure consistent results across pages.
Add the ORDER BY clause:
public String getMusterRollQueryForBulkSearch(MusterRollSearchCriteria criteria, List<Object> preparedStmtList) {
// ... existing code ...
builder.append("muster.id IN (").append(createQuery(uuids)).append(")");
addToPreparedStatement(preparedStmtList, uuids);
+ builder.append(" ORDER BY muster.lastmodifiedtime DESC, muster.id");
return builder.toString();
}Additionally, consider refactoring common query building patterns into reusable methods to reduce code duplication.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public String getMusterRollQueryForBulkSearch(MusterRollSearchCriteria criteria, List<Object> preparedStmtList) { | |
| Boolean isEmpty = CollectionUtils.isEmpty(criteria.getIds()); | |
| if(isEmpty) | |
| throw new CustomException("EG_MR_SEARCH_ERROR"," No uuids given for the MusterRoll Bulk search"); | |
| StringBuilder builder = new StringBuilder(FETCH_MUSTER_ROLL_QUERY); | |
| if(!ObjectUtils.isEmpty(criteria.getTenantId())) | |
| { | |
| addClauseIfRequired(preparedStmtList, builder); | |
| builder.append("muster.tenant_id=?"); | |
| preparedStmtList.add(criteria.getTenantId()); | |
| } | |
| List<String> uuids = criteria.getIds(); | |
| if (!CollectionUtils.isEmpty(uuids)) { | |
| addClauseIfRequired(preparedStmtList,builder); | |
| builder.append("muster.id IN (").append(createQuery(uuids)).append(")"); | |
| addToPreparedStatement(preparedStmtList, uuids); | |
| } | |
| return builder.toString(); | |
| } | |
| public String getMusterRollQueryForBulkSearch(MusterRollSearchCriteria criteria, List<Object> preparedStmtList) { | |
| Boolean isEmpty = CollectionUtils.isEmpty(criteria.getIds()); | |
| if(isEmpty) | |
| throw new CustomException("EG_MR_SEARCH_ERROR"," No uuids given for the MusterRoll Bulk search"); | |
| StringBuilder builder = new StringBuilder(FETCH_MUSTER_ROLL_QUERY); | |
| if(!ObjectUtils.isEmpty(criteria.getTenantId())) | |
| { | |
| addClauseIfRequired(preparedStmtList, builder); | |
| builder.append("muster.tenant_id=?"); | |
| preparedStmtList.add(criteria.getTenantId()); | |
| } | |
| List<String> uuids = criteria.getIds(); | |
| if (!CollectionUtils.isEmpty(uuids)) { | |
| addClauseIfRequired(preparedStmtList,builder); | |
| builder.append("muster.id IN (").append(createQuery(uuids)).append(")"); | |
| addToPreparedStatement(preparedStmtList, uuids); | |
| } | |
| builder.append(" ORDER BY muster.lastmodifiedtime DESC, muster.id"); | |
| return builder.toString(); | |
| } |
| private List<MusterRoll> getMusterRollsPlainsearch(MusterRollSearchCriteria searchCriteria) { | ||
|
|
||
| enrichmentService.enrichSearchRequest(searchCriteria); | ||
|
|
||
| MusterRollSearchCriteria musterRollCriteria = new MusterRollSearchCriteria(); | ||
|
|
||
| List<String> uuids = musterRollRepository.fetchIds(searchCriteria); | ||
| if (uuids.isEmpty()) | ||
| return Collections.emptyList(); | ||
|
|
||
| musterRollCriteria.setIds(uuids); | ||
|
|
||
| List<MusterRoll> musterRolls = musterRollRepository.getMusterRollsForBulkSearch(musterRollCriteria); | ||
|
|
||
| if(musterRolls.isEmpty()) | ||
| return Collections.emptyList(); | ||
|
|
||
| return musterRolls; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation and optimize criteria handling.
The implementation needs some improvements:
- Add null checks for the search criteria
- Reuse the enriched criteria instead of creating a new one
Consider this implementation:
private List<MusterRoll> getMusterRollsPlainsearch(MusterRollSearchCriteria searchCriteria) {
+ if (searchCriteria == null) {
+ throw new CustomException("INVALID_CRITERIA", "Search criteria cannot be null");
+ }
+
enrichmentService.enrichSearchRequest(searchCriteria);
- MusterRollSearchCriteria musterRollCriteria = new MusterRollSearchCriteria();
-
List<String> uuids = musterRollRepository.fetchIds(searchCriteria);
if (uuids.isEmpty())
return Collections.emptyList();
- musterRollCriteria.setIds(uuids);
+ searchCriteria.setIds(uuids);
- List<MusterRoll> musterRolls = musterRollRepository.getMusterRollsForBulkSearch(musterRollCriteria);
+ List<MusterRoll> musterRolls = musterRollRepository.getMusterRollsForBulkSearch(searchCriteria);
if(musterRolls.isEmpty())
return Collections.emptyList();
return musterRolls;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private List<MusterRoll> getMusterRollsPlainsearch(MusterRollSearchCriteria searchCriteria) { | |
| enrichmentService.enrichSearchRequest(searchCriteria); | |
| MusterRollSearchCriteria musterRollCriteria = new MusterRollSearchCriteria(); | |
| List<String> uuids = musterRollRepository.fetchIds(searchCriteria); | |
| if (uuids.isEmpty()) | |
| return Collections.emptyList(); | |
| musterRollCriteria.setIds(uuids); | |
| List<MusterRoll> musterRolls = musterRollRepository.getMusterRollsForBulkSearch(musterRollCriteria); | |
| if(musterRolls.isEmpty()) | |
| return Collections.emptyList(); | |
| return musterRolls; | |
| } | |
| private List<MusterRoll> getMusterRollsPlainsearch(MusterRollSearchCriteria searchCriteria) { | |
| if (searchCriteria == null) { | |
| throw new CustomException("INVALID_CRITERIA", "Search criteria cannot be null"); | |
| } | |
| enrichmentService.enrichSearchRequest(searchCriteria); | |
| List<String> uuids = musterRollRepository.fetchIds(searchCriteria); | |
| if (uuids.isEmpty()) | |
| return Collections.emptyList(); | |
| searchCriteria.setIds(uuids); | |
| List<MusterRoll> musterRolls = musterRollRepository.getMusterRollsForBulkSearch(searchCriteria); | |
| if(musterRolls.isEmpty()) | |
| return Collections.emptyList(); | |
| return musterRolls; | |
| } |
| public List<String> fetchIds(ContractCriteria contractCriteria) { | ||
|
|
||
| List<Object> preparedStmtList = new ArrayList<>(); | ||
| String basequery = "select id from eg_wms_contract"; | ||
| StringBuilder builder = new StringBuilder(basequery); | ||
|
|
||
| if(!ObjectUtils.isEmpty(contractCriteria.getTenantId())) | ||
| { | ||
| builder.append(" where tenant_id=?"); | ||
| preparedStmtList.add(contractCriteria.getTenantId()); | ||
| } | ||
|
|
||
| String orderbyClause = " order by last_modified_time,id offset ? limit ?"; | ||
| builder.append(orderbyClause); | ||
| preparedStmtList.add(contractCriteria.getPagination().getOffSet()); | ||
| preparedStmtList.add(contractCriteria.getPagination().getLimit()); | ||
|
|
||
| return jdbcTemplate.query(builder.toString(), preparedStmtList.toArray(), new SingleColumnRowMapper<>(String.class)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move SQL query construction to QueryBuilder class.
For consistency with the existing pattern, the SQL query construction should be moved to the ContractQueryBuilder class. This ensures:
- Centralized query management
- Consistent query building patterns
- Easier maintenance
- public List<String> fetchIds(ContractCriteria contractCriteria) {
- List<Object> preparedStmtList = new ArrayList<>();
- String basequery = "select id from eg_wms_contract";
- StringBuilder builder = new StringBuilder(basequery);
-
- if(!ObjectUtils.isEmpty(contractCriteria.getTenantId())) {
- builder.append(" where tenant_id=?");
- preparedStmtList.add(contractCriteria.getTenantId());
- }
-
- String orderbyClause = " order by last_modified_time,id offset ? limit ?";
- builder.append(orderbyClause);
- preparedStmtList.add(contractCriteria.getPagination().getOffSet());
- preparedStmtList.add(contractCriteria.getPagination().getLimit());
-
- return jdbcTemplate.query(builder.toString(), preparedStmtList.toArray(), new SingleColumnRowMapper<>(String.class));
+ public List<String> fetchIds(ContractCriteria contractCriteria) {
+ List<Object> preparedStmtList = new ArrayList<>();
+ String query = queryBuilder.getFetchIdsQuery(contractCriteria, preparedStmtList);
+ return jdbcTemplate.query(query, preparedStmtList.toArray(), new SingleColumnRowMapper<>(String.class));
}Committable suggestion skipped: line range outside the PR's diff.
| public List<String> fetchIds(EstimateSearchCriteria criteria) { | ||
| List<Object> preparedStmtList = new ArrayList<>(); | ||
| String basequery = "select id from eg_wms_estimate"; | ||
| StringBuilder builder = new StringBuilder(basequery); | ||
|
|
||
| if(!ObjectUtils.isEmpty(criteria.getTenantId())) | ||
| { | ||
| builder.append(" where tenant_id=?"); | ||
| preparedStmtList.add(criteria.getTenantId()); | ||
| } | ||
|
|
||
| String orderbyClause = " order by last_modified_time,id offset ? limit ?"; | ||
| builder.append(orderbyClause); | ||
| preparedStmtList.add(criteria.getOffset()); | ||
| preparedStmtList.add(criteria.getLimit()); | ||
|
|
||
| return jdbcTemplate.query(builder.toString(), preparedStmtList.toArray(), new SingleColumnRowMapper<>(String.class)); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider extracting common repository pattern to a shared interface.
The implementation is similar to ContractRepository.fetchIds. Consider:
- Moving SQL query construction to
EstimateQueryBuilder - Creating a common interface for repositories that need plain search functionality
Example interface:
public interface PlainSearchRepository<T, C> {
List<T> getBulkSearchResults(C criteria);
List<String> fetchIds(C criteria);
}| public List<Contract> searchContractPlainSearch(Integer limit, Integer offset, String tenantId) { | ||
|
|
||
| ContractCriteria contractCriteria = new ContractCriteria(); | ||
| contractCriteria.setPagination(new Pagination()); | ||
|
|
||
| contractCriteria.getPagination().setOffSet(offset); | ||
| contractCriteria.getPagination().setLimit(limit); | ||
| contractCriteria.setTenantId(tenantId); | ||
|
|
||
| if (contractCriteria.getPagination().getLimit() != null && contractCriteria.getPagination().getLimit() > contractServiceConfiguration.getContractMaxLimit()) | ||
| contractCriteria.getPagination().setLimit(contractServiceConfiguration.getContractMaxLimit()); | ||
| if(contractCriteria.getPagination().getLimit() == null) | ||
| contractCriteria.getPagination().setLimit(contractServiceConfiguration.getContractDefaultLimit()); | ||
| if(contractCriteria.getPagination().getOffSet()==null) | ||
| contractCriteria.getPagination().setOffSet(contractServiceConfiguration.getContractDefaultOffset()); | ||
|
|
||
| List<String> uuids = contractRepository.fetchIds(contractCriteria); | ||
|
|
||
|
|
||
| if (uuids.isEmpty()) | ||
| return Collections.emptyList(); | ||
| contractCriteria.setIds(uuids); | ||
|
|
||
| List<Contract> contracts = contractRepository.getContractsForBulkSearch(contractCriteria); | ||
|
|
||
| if(contracts.isEmpty()) | ||
| return Collections.emptyList(); | ||
|
|
||
| return contracts; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation for search parameters
The method should validate the input parameters to ensure they are valid before processing.
Add these validations at the start of the method:
public List<Contract> searchContractPlainSearch(Integer limit, Integer offset, String tenantId) {
+ if (limit != null && limit <= 0) {
+ throw new CustomException("INVALID_LIMIT", "Limit should be greater than zero");
+ }
+ if (offset != null && offset < 0) {
+ throw new CustomException("INVALID_OFFSET", "Offset should not be negative");
+ }
+ if (StringUtils.isEmpty(tenantId)) {
+ throw new CustomException("INVALID_TENANTID", "TenantId is mandatory for plain search");
+ }
ContractCriteria contractCriteria = new ContractCriteria();Committable suggestion skipped: line range outside the PR's diff.
| if(contracts.isEmpty()) | ||
| return Collections.emptyList(); | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove redundant empty check
The empty check at line 362 is redundant since the empty list is already returned at line 357.
- if(contracts.isEmpty())
- return Collections.emptyList();
-
return contracts;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if(contracts.isEmpty()) | |
| return Collections.emptyList(); | |
| return contracts; |
| public List<Individual> getIndividualsForBulkSearch(Integer limit, | ||
| Integer offset, | ||
| String tenantId) { | ||
| Map<String, Object> paramsMap = new HashMap<>(); | ||
| String query = "SELECT * FROM individual"; | ||
|
|
||
| if (tenantId != null) { | ||
| query += " WHERE tenantId=:tenantId "; | ||
| } | ||
| query = query + "ORDER BY createdtime DESC LIMIT :limit OFFSET :offset"; | ||
|
|
||
| paramsMap.put("tenantId", tenantId); | ||
| paramsMap.put("limit", limit); | ||
| paramsMap.put("offset", offset); | ||
|
|
||
| log.info("query-------------------------->"); | ||
| log.info(query); | ||
|
|
||
| List<Individual> individuals = this.namedParameterJdbcTemplate.query(query, paramsMap, this.rowMapper); | ||
|
|
||
| if (!individuals.isEmpty()) { | ||
| enrichIndividuals(individuals, true); | ||
| } | ||
| return individuals; | ||
| } |
There was a problem hiding this comment.
Add input validation and error handling
The method needs input validation and proper error handling for database operations.
Add parameter validation and wrap the database operation in a try-catch block:
public List<Individual> getIndividualsForBulkSearch(Integer limit,
Integer offset,
String tenantId) {
+ if (limit == null || limit <= 0) {
+ throw new CustomException("INVALID_LIMIT", "Limit should be greater than zero");
+ }
+ if (offset == null || offset < 0) {
+ throw new CustomException("INVALID_OFFSET", "Offset should not be negative");
+ }
+ if (StringUtils.isEmpty(tenantId)) {
+ throw new CustomException("INVALID_TENANTID", "TenantId is mandatory for bulk search");
+ }
+
Map<String, Object> paramsMap = new HashMap<>();
String query = "SELECT * FROM individual";
if (tenantId != null) {
query += " WHERE tenantId=:tenantId ";
}
- query = query + "ORDER BY createdtime DESC LIMIT :limit OFFSET :offset";
+ query = query + " ORDER BY createdtime DESC LIMIT :limit OFFSET :offset";
paramsMap.put("tenantId", tenantId);
paramsMap.put("limit", limit);
paramsMap.put("offset", offset);
log.info("query-------------------------->");
log.info(query);
+ try {
List<Individual> individuals = this.namedParameterJdbcTemplate.query(query, paramsMap, this.rowMapper);
if (!individuals.isEmpty()) {
enrichIndividuals(individuals, true);
}
return individuals;
+ } catch (Exception e) {
+ log.error("Error occurred while fetching individuals: ", e);
+ throw new CustomException("DATABASE_ERROR", "Error occurred while fetching individuals");
+ }
}Committable suggestion skipped: line range outside the PR's diff.
| public List<Individual> searchIndividualPlainSearch(Integer limit, Integer offset, String tenantId, RequestInfo requestInfo) { | ||
| List<Individual> individuals = individualRepository.getIndividualsForBulkSearch(limit, offset, tenantId); | ||
| return individuals; | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation and simplify method
The method needs input validation and can be simplified since requestInfo is unused.
- public List<Individual> searchIndividualPlainSearch(Integer limit, Integer offset, String tenantId, RequestInfo requestInfo) {
+ public List<Individual> searchIndividualPlainSearch(Integer limit, Integer offset, String tenantId) {
+ if (limit == null || limit <= 0) {
+ throw new CustomException("INVALID_LIMIT", "Limit should be greater than zero");
+ }
+ if (offset == null || offset < 0) {
+ throw new CustomException("INVALID_OFFSET", "Offset should not be negative");
+ }
+ if (StringUtils.isEmpty(tenantId)) {
+ throw new CustomException("INVALID_TENANTID", "TenantId is mandatory for plain search");
+ }
+
- List<Individual> individuals = individualRepository.getIndividualsForBulkSearch(limit, offset, tenantId);
- return individuals;
+ return individualRepository.getIndividualsForBulkSearch(limit, offset, tenantId);
}Committable suggestion skipped: line range outside the PR's diff.
| - work-dir: "backend/individual/src/main/resources/db" | ||
| image-name: "individual-db" | ||
| image-name: "mukta-individual-db" |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove trailing spaces.
There are trailing spaces at the end of line 277 that should be removed.
- image-name: "mukta-individual-db"
+ image-name: "mukta-individual-db"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| image-name: "mukta-individual-db" | |
| image-name: "mukta-individual-db" |
🧰 Tools
🪛 yamllint (1.35.1)
[error] 277-277: trailing spaces
(trailing-spaces)
egov-arindam
requested review from
shailesh-egov,
manastanmay-eGov and
Lokendra-egov
Summary by CodeRabbit
New Features
Improvements
Technical Updates