Skip to content
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package org.egov.project.service;

import com.fasterxml.jackson.databind.ObjectMapper;
import java.time.Duration;
import java.util.Collections;
import org.egov.common.contract.models.AuditDetails;
import jakarta.validation.Valid;
import java.util.Map;
Expand All @@ -19,6 +21,7 @@
import org.egov.project.validator.project.ProjectValidator;
import org.egov.tracer.model.CustomException;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.data.redis.core.RedisTemplate;
import org.springframework.stereotype.Service;

import java.util.ArrayList;
Expand All @@ -44,18 +47,22 @@ public class ProjectService {

private final ObjectMapper objectMapper;

@Autowired
public ProjectService(
ProjectRepository projectRepository,
ProjectValidator projectValidator, ProjectEnrichment projectEnrichment, ProjectConfiguration projectConfiguration, Producer producer,ProjectServiceUtil projectServiceUtil) {
this.projectRepository = projectRepository;
this.projectValidator = projectValidator;
this.projectEnrichment = projectEnrichment;
this.projectConfiguration = projectConfiguration;
this.producer = producer;
this.projectServiceUtil = projectServiceUtil;
this.objectMapper = new ObjectMapper();
}
@Autowired
private RedisTemplate<String, String> redisTemplate;


@Autowired
public ProjectService(
ProjectRepository projectRepository,
ProjectValidator projectValidator, ProjectEnrichment projectEnrichment, ProjectConfiguration projectConfiguration, Producer producer, ProjectServiceUtil projectServiceUtil) {
this.projectRepository = projectRepository;
this.projectValidator = projectValidator;
this.projectEnrichment = projectEnrichment;
this.projectConfiguration = projectConfiguration;
this.producer = producer;
this.projectServiceUtil = projectServiceUtil;
this.objectMapper = new ObjectMapper();
}

public List<String> validateProjectIds(List<String> productIds) {
return projectRepository.validateIds(productIds, "id");
Expand All @@ -65,19 +72,34 @@ public List<Project> findByIds(List<String> projectIds){
return projectRepository.findById(projectIds);
}

public ProjectRequest createProject(ProjectRequest projectRequest) {
projectValidator.validateCreateProjectRequest(projectRequest);
//Get parent projects if "parent" is present (For enrichment of projectHierarchy)
List<Project> parentProjects = getParentProjects(projectRequest);
//Validate Parent in request against projects fetched form database
if (parentProjects != null)
projectValidator.validateParentAgainstDB(projectRequest.getProjects(), parentProjects);
projectEnrichment.enrichProjectOnCreate(projectRequest, parentProjects);
log.info("Enriched with Project Number, Ids and AuditDetails");
producer.push(projectConfiguration.getSaveProjectTopic(), projectRequest);
log.info("Pushed to kafka");
return projectRequest;
public ProjectRequest createProject(ProjectRequest projectRequest) {
projectValidator.validateCreateProjectRequest(projectRequest);
//Get parent projects if "parent" is present (For enrichment of projectHierarchy)
List<Project> parentProjects = getParentProjects(projectRequest);
//Validate Parent in request against projects fetched form database
if (parentProjects != null) {
projectValidator.validateParentAgainstDB(projectRequest.getProjects(), parentProjects);
}
projectEnrichment.enrichProjectOnCreate(projectRequest, parentProjects);
log.info("Enriched with Project Number, Ids and AuditDetails");
producer.push(projectConfiguration.getSaveProjectTopic(), projectRequest);
log.info("Pushed to kafka");

// ✅ Save project IDs in Redis after Kafka push
for (Project project : projectRequest.getProjects()) {
String redisKey = "project-create-cache-" + project.getId();

if (StringUtils.isNotBlank(project.getProjectHierarchy())) {
redisTemplate.opsForValue()
.set(redisKey, project.getProjectHierarchy(), Duration.ofDays(1));
log.info("Cached projectHierarchy for project {} in Redis", project.getId());
} else {
log.warn("ProjectHierarchy is blank for project {}, not caching in Redis", project.getId());
}
}

return projectRequest;
}

/**
* Search for projects based on various criteria
Expand Down Expand Up @@ -304,17 +326,65 @@ private void checkAndEnrichCascadingProjectDates(ProjectRequest request, Project
}


/* Search for parent projects based on "parent" field and returns parent projects */
private List<Project> getParentProjects(ProjectRequest projectRequest) {
List<Project> parentProjects = null;
List<Project> projectsForSearchRequest = projectRequest.getProjects().stream().filter(p -> StringUtils.isNotBlank(p.getParent())).collect(Collectors.toList());
if (projectsForSearchRequest.size() > 0) {
parentProjects = searchProject(getSearchProjectRequest(projectsForSearchRequest, projectRequest.getRequestInfo(), true), projectConfiguration.getMaxLimit(), projectConfiguration.getDefaultOffset(), projectRequest.getProjects().get(0).getTenantId(), null, false, false, false, null, null, false);
}
log.info("Fetched parent projects from DB");
return parentProjects;
/* Search for parent projects based on "parent" field and returns parent projects */
private List<Project> getParentProjects(ProjectRequest projectRequest) {
List<Project> parentProjects = new ArrayList<>();

List<Project> projectsWithParent = projectRequest.getProjects().stream()
.filter(p -> StringUtils.isNotBlank(p.getParent()))
.collect(Collectors.toList());

if (projectsWithParent.isEmpty()) {
return Collections.emptyList();
}

List<String> missingParentIds = new ArrayList<>();

for (Project project : projectsWithParent) {
String parentId = project.getParent();
String redisKey = "project-create-cache-" + parentId;

String cachedHierarchy = redisTemplate.opsForValue().get(redisKey);
if (StringUtils.isNotBlank(cachedHierarchy)) {
log.info("Parent project hierarchy for {} fetched from Redis", parentId);

Project parent = new Project();
parent.setId(parentId);
parent.setProjectHierarchy(cachedHierarchy);
parentProjects.add(parent);
} else {
missingParentIds.add(parentId);
}
}

if (!missingParentIds.isEmpty()) {
List<Project> dbQueryProjects = missingParentIds.stream()
.map(id -> Project.builder().id(id)
.tenantId(projectRequest.getProjects().get(0).getTenantId()).build())
.collect(Collectors.toList());

ProjectRequest searchRequest = getSearchProjectRequest(dbQueryProjects,
projectRequest.getRequestInfo(), false);

List<Project> dbProjects = searchProject(
searchRequest,
projectConfiguration.getMaxLimit(),
projectConfiguration.getDefaultOffset(),
searchRequest.getProjects().get(0).getTenantId(),
null, false, false, false, null, null, false
);

for (Project parent : dbProjects) {
String redisKey = "project-create-cache-" + parent.getId();
redisTemplate.opsForValue().set(redisKey, parent.getProjectHierarchy(), Duration.ofDays(1));
parentProjects.add(parent);
}

}

return parentProjects;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Add error handling and consider refactoring for better maintainability.

The caching logic is well-designed but needs resilience and could be more maintainable.

Critical Issue: Add error handling for Redis operations

      String cachedHierarchy = redisTemplate.opsForValue().get(redisKey);

Should be wrapped in try-catch to handle Redis failures gracefully:

+     try {
        String cachedHierarchy = redisTemplate.opsForValue().get(redisKey);
        if (StringUtils.isNotBlank(cachedHierarchy)) {
          log.info("Parent project hierarchy for {} fetched from Redis", parentId);
          // ... existing logic
        } else {
          missingParentIds.add(parentId);
        }
+     } catch (Exception e) {
+       log.warn("Failed to fetch from Redis for parent {}: {}", parentId, e.getMessage());
+       missingParentIds.add(parentId);
+     }

Refactoring Suggestion: Break down the method

Consider extracting helper methods:

  • fetchFromCache(List<String> parentIds)
  • fetchFromDatabase(List<String> missingIds, ProjectRequest request)
  • cacheProjects(List<Project> projects)

This would improve readability and testability.

📝 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.

Suggested change
/* Search for parent projects based on "parent" field and returns parent projects */
private List<Project> getParentProjects(ProjectRequest projectRequest) {
List<Project> parentProjects = new ArrayList<>();
List<Project> projectsWithParent = projectRequest.getProjects().stream()
.filter(p -> StringUtils.isNotBlank(p.getParent()))
.collect(Collectors.toList());
if (projectsWithParent.isEmpty()) {
return Collections.emptyList();
}
List<String> missingParentIds = new ArrayList<>();
for (Project project : projectsWithParent) {
String parentId = project.getParent();
String redisKey = "project-create-cache-" + parentId;
String cachedHierarchy = redisTemplate.opsForValue().get(redisKey);
if (StringUtils.isNotBlank(cachedHierarchy)) {
log.info("Parent project hierarchy for {} fetched from Redis", parentId);
Project parent = new Project();
parent.setId(parentId);
parent.setProjectHierarchy(cachedHierarchy);
parentProjects.add(parent);
} else {
missingParentIds.add(parentId);
}
}
if (!missingParentIds.isEmpty()) {
List<Project> dbQueryProjects = missingParentIds.stream()
.map(id -> Project.builder().id(id)
.tenantId(projectRequest.getProjects().get(0).getTenantId()).build())
.collect(Collectors.toList());
ProjectRequest searchRequest = getSearchProjectRequest(dbQueryProjects,
projectRequest.getRequestInfo(), false);
List<Project> dbProjects = searchProject(
searchRequest,
projectConfiguration.getMaxLimit(),
projectConfiguration.getDefaultOffset(),
searchRequest.getProjects().get(0).getTenantId(),
null, false, false, false, null, null, false
);
for (Project parent : dbProjects) {
String redisKey = "project-create-cache-" + parent.getId();
redisTemplate.opsForValue().set(redisKey, parent.getProjectHierarchy(), Duration.ofDays(1));
parentProjects.add(parent);
}
}
return parentProjects;
}
/* Search for parent projects based on "parent" field and returns parent projects */
private List<Project> getParentProjects(ProjectRequest projectRequest) {
List<Project> parentProjects = new ArrayList<>();
List<Project> projectsWithParent = projectRequest.getProjects().stream()
.filter(p -> StringUtils.isNotBlank(p.getParent()))
.collect(Collectors.toList());
if (projectsWithParent.isEmpty()) {
return Collections.emptyList();
}
List<String> missingParentIds = new ArrayList<>();
for (Project project : projectsWithParent) {
String parentId = project.getParent();
String redisKey = "project-create-cache-" + parentId;
+ try {
String cachedHierarchy = redisTemplate.opsForValue().get(redisKey);
if (StringUtils.isNotBlank(cachedHierarchy)) {
log.info("Parent project hierarchy for {} fetched from Redis", parentId);
Project parent = new Project();
parent.setId(parentId);
parent.setProjectHierarchy(cachedHierarchy);
parentProjects.add(parent);
} else {
missingParentIds.add(parentId);
}
+ } catch (Exception e) {
+ log.warn("Failed to fetch from Redis for parent {}: {}", parentId, e.getMessage());
+ missingParentIds.add(parentId);
+ }
}
if (!missingParentIds.isEmpty()) {
List<Project> dbQueryProjects = missingParentIds.stream()
.map(id -> Project.builder().id(id)
.tenantId(projectRequest.getProjects().get(0).getTenantId()).build())
.collect(Collectors.toList());
ProjectRequest searchRequest = getSearchProjectRequest(dbQueryProjects,
projectRequest.getRequestInfo(), false);
List<Project> dbProjects = searchProject(
searchRequest,
projectConfiguration.getMaxLimit(),
projectConfiguration.getDefaultOffset(),
searchRequest.getProjects().get(0).getTenantId(),
null, false, false, false, null, null, false
);
for (Project parent : dbProjects) {
String redisKey = "project-create-cache-" + parent.getId();
redisTemplate.opsForValue().set(redisKey, parent.getProjectHierarchy(), Duration.ofDays(1));
parentProjects.add(parent);
}
}
return parentProjects;
}
🤖 Prompt for AI Agents
In
health-services/project/src/main/java/org/egov/project/service/ProjectService.java
between lines 329 and 386, add try-catch blocks around all Redis operations to
handle potential Redis failures gracefully without crashing the method. Refactor
the existing getParentProjects method by extracting three helper methods:
fetchFromCache to retrieve parent projects from Redis cache given a list of
parent IDs, fetchFromDatabase to query missing parent projects from the database
using the ProjectRequest, and cacheProjects to store retrieved projects back
into Redis with appropriate expiration. Replace the corresponding code blocks in
getParentProjects with calls to these helper methods to improve readability,
maintainability, and testability.


/* Construct Project Request object for search which contains project id and tenantId */
private ProjectRequest getSearchProjectRequest(List<Project> projects, RequestInfo requestInfo, Boolean isParentProjectSearch) {
List<Project> projectList = new ArrayList<>();
Expand Down
Loading