code review comments

egovernments · Jun 21, 2024 · 391f82b · 391f82b
1 parent 992dc8e
commit 391f82b
Show file tree

Hide file tree

Showing 11 changed files with 163 additions and 119 deletions.
diff --git a/health-services/resource-estimation-service/README.md b/health-services/resource-estimation-service/README.md
@@ -4,13 +4,13 @@ Spring Boot Server
 
 
 ## Overview  
-This server was generated by the [swagger-codegen](https://github.com/swagger-api/swagger-codegen) project.  
+This server was generated by the [swagger-codegen](https://github.com/swagger-api/swagger-codegen) project.
 By using the [OpenAPI-Spec](https://github.com/swagger-api/swagger-core), you can easily generate a server stub.  
 This is an example of building a swagger-enabled server in Java using the SpringBoot framework.  
 
 The underlying library integrating swagger to SpringBoot is [springfox](https://github.com/springfox/springfox)  
 
-Start your server as an simple java application  
+Start your server as a simple java application  
 
 You can view the api documentation in swagger-ui by pointing to  
 http://localhost:8080/  

diff --git a/...resource-estimation-service/src/main/java/org/egov/processor/config/ServiceConstants.java b/...resource-estimation-service/src/main/java/org/egov/processor/config/ServiceConstants.java
@@ -44,17 +44,24 @@ public class ServiceConstants {
     public static final String FILE_TYPE = "boundaryWithTarget";
     public static final String FILE_TEMPLATE_IDENTIFIER = "Population";
     public static final String INPUT_IS_NOT_VALID = "File does not contain valid input for row ";
+
     public static final String MDMS_SCHEMA_TYPE =  "type";
     public static final String MDMS_SCHEMA_SECTION =  "section";
     public static final String MDMS_PLAN_MODULE_NAME = "hcm-microplanning";
     public static final String MDMS_MASTER_SCHEMAS = "Schemas";
     public static final String MDMS_CAMPAIGN_TYPE = "campaignType";
+
     public static final String ERROR_WHILE_UPDATING_PLAN_CONFIG = "Exception occurred while updating plan configuration.";
+
     public static final String VALIDATE_STRING_REGX = "^[a-zA-Z0-9 .,()_\\-`~!@#\\$%^&*\\+=\\\\|{}\\[\\]:;\"'<>,.?/]*$";    
     public static final String VALIDATE_NUMBER_REGX = "^[-+]?[0-9]*\\.?[0-9]+([eE][-+]?[0-9]+)?$";
     public static final String VALIDATE_BOOLEAN_REGX = "^(?i)(true|false)$";
+
     public static final String FILE_TEMPLATE = "Facilities";
     public static final String HIERARCHYTYPE_REPLACER = "{hierarchyType}";
     public static final String FILE_EXTENSION = "excel";
+
+    public static final String SCIENTIFIC_NOTATION_INDICATOR = "E";
+    public static final String ATTRIBUTE_IS_REQUIRED ="isRequired";
 
 }
diff --git a/...ices/resource-estimation-service/src/main/java/org/egov/processor/kafka/PlanConsumer.java b/...ices/resource-estimation-service/src/main/java/org/egov/processor/kafka/PlanConsumer.java
@@ -7,6 +7,8 @@
 import org.egov.processor.service.ResourceEstimationService;
 import org.egov.processor.web.models.PlanConfiguration;
 import org.egov.processor.web.models.PlanConfigurationRequest;
+import org.egov.tracer.model.CustomException;
+import org.springframework.http.HttpStatus;
 import org.springframework.kafka.annotation.KafkaListener;
 import org.springframework.kafka.support.KafkaHeaders;
 import org.springframework.messaging.handler.annotation.Header;
@@ -31,9 +33,12 @@ public void listen(Map<String, Object> consumerRecord, @Header(KafkaHeaders.RECE
             PlanConfigurationRequest planConfigurationRequest = objectMapper.convertValue(consumerRecord, PlanConfigurationRequest.class);
             if (planConfigurationRequest.getPlanConfiguration().getStatus().equals(PlanConfiguration.StatusEnum.GENERATED)) {
                 resourceEstimationService.estimateResources(planConfigurationRequest);
+                log.info("Successfully estimated resources for plan.");
             }
         } catch (Exception exception) {
-            log.error("Error processing record from topic {} : {} ",topic, exception);
+            log.error("Error processing record from topic "+topic+" with exception :"+exception);
+            throw new CustomException(Integer.toString(HttpStatus.INTERNAL_SERVER_ERROR.value()),
+					exception.toString());
         }
     }
 }
diff --git a/...ces/resource-estimation-service/src/main/java/org/egov/processor/service/ExcelParser.java b/...ces/resource-estimation-service/src/main/java/org/egov/processor/service/ExcelParser.java
@@ -101,12 +101,10 @@ public Object parseFileData(PlanConfigurationRequest planConfigurationRequest, S
 		PlanConfiguration planConfig = planConfigurationRequest.getPlanConfiguration();
 		byte[] byteArray = filestoreUtil.getFile(planConfig.getTenantId(), fileStoreId);
 		File file = parsingUtil.convertByteArrayToFile(byteArray, ServiceConstants.FILE_EXTENSION);
-
 		if (file == null || !file.exists()) {
 			log.info("FILE NOT FOUND");
 			return null;
 		}
-
 		return processExcelFile(planConfigurationRequest, file, fileStoreId, campaignResponse);
 	}
 
@@ -154,7 +152,7 @@ private String processExcelFile(PlanConfigurationRequest planConfigurationReques
 			}
 			return uploadedFileStoreId;
 		} catch (IOException | InvalidFormatException e) {
-			log.error("Error processing Excel file: {}", e.getMessage());
+			log.error("Error processing Excel file: {}", e);
 			throw new CustomException(Integer.toString(HttpStatus.INTERNAL_SERVER_ERROR.value()),
 					"Error processing Excel file");
 		}
@@ -196,8 +194,15 @@ private void processRows(PlanConfigurationRequest planConfigurationRequest, Shee
 		List<String> boundaryList = new ArrayList<>();
 		List<String> boundaryCodeList = getAllBoundaryPresentforHierarchyType(
 				boundarySearchResponse.getTenantBoundary().get(0).getBoundary(), boundaryList);
-		// log.info(list.toString());
 		Row firstRow = null;
+		performRowLevelCalculations(planConfigurationRequest, sheet, dataFormatter, fileStoreId, campaignBoundaryList,
+				planConfig, attributeNameVsDataTypeMap, boundaryCodeList, firstRow);
+	}
+
+	private void performRowLevelCalculations(PlanConfigurationRequest planConfigurationRequest, Sheet sheet,
+			DataFormatter dataFormatter, String fileStoreId, List<Boundary> campaignBoundaryList,
+			PlanConfiguration planConfig, Map<String, Object> attributeNameVsDataTypeMap, List<String> boundaryCodeList,
+			Row firstRow) throws IOException {
 		for (Row row : sheet) {
 			if (row.getRowNum() == 0) {
 				firstRow = row;
@@ -217,22 +222,8 @@ private void processRows(PlanConfigurationRequest planConfigurationRequest, Shee
 			validateRows(indexOfBoundaryCode, row, firstRow, attributeNameVsDataTypeMap, mappedValues, mapOfColumnNameAndIndex,
 					planConfigurationRequest, boundaryCodeList);
 			JsonNode feature = createFeatureNodeFromRow(row, dataFormatter, mapOfColumnNameAndIndex);
-			int columnIndex = row.getLastCellNum(); // Get the index of the last cell in the row
-
-			for (Operation operation : planConfig.getOperations()) {
-				BigDecimal result = calculationUtil.calculateResult(operation, feature, mappedValues,
-						assumptionValueMap, resultMap);
-				String output = operation.getOutput();
-				resultMap.put(output, result);
-
-				Cell cell = row.createCell(columnIndex++);
-				cell.setCellValue(result.doubleValue());
-
-				if (row.getRowNum() == 1) {
-					Cell headerCell = sheet.getRow(0).createCell(row.getLastCellNum() - 1);
-					headerCell.setCellValue(output);
-				}
-			}
+			performCalculationsOnOperations(sheet, planConfig, row, resultMap, mappedValues,
+					assumptionValueMap, feature);
 			if (config.isIntegrateWithAdminConsole())
 				campaignIntegrationUtil.updateCampaignBoundary(planConfig, feature, assumptionValueMap, mappedValues,
 						mapOfColumnNameAndIndex, campaignBoundaryList, resultMap);
@@ -242,6 +233,28 @@ private void processRows(PlanConfigurationRequest planConfigurationRequest, Shee
 		}
 	}
 
+	private void performCalculationsOnOperations(Sheet sheet, PlanConfiguration planConfig, Row row,
+			Map<String, BigDecimal> resultMap, Map<String, String> mappedValues,
+			Map<String, BigDecimal> assumptionValueMap, JsonNode feature) {
+		int columnIndex = row.getLastCellNum(); // Get the index of the last cell in the row
+
+		for (Operation operation : planConfig.getOperations()) {
+			BigDecimal result = calculationUtil.calculateResult(operation, feature, mappedValues,
+					assumptionValueMap, resultMap);
+			String output = operation.getOutput();
+			resultMap.put(output, result);
+
+			Cell cell = row.createCell(columnIndex++);
+			cell.setCellValue(result.doubleValue());
+
+			if (row.getRowNum() == 1) {
+				Cell headerCell = sheet.getRow(0).createCell(row.getLastCellNum() - 1);
+				headerCell.setCellValue(output);
+			}
+		}
+
+	}
+
 	/**
 	 * Uploads the converted XLS file to the file store.
 	 *
@@ -282,14 +295,14 @@ private File convertWorkbookToXls(Workbook workbook) {
 			// Write the XLS file
 			try (FileOutputStream fos = new FileOutputStream(outputFile)) {
 				workbook.write(fos);
-				System.out.println("XLS file saved successfully.");
+				log.info("XLS file saved successfully.");
 				return outputFile;
 			} catch (IOException e) {
-				System.err.println("Error saving XLS file: " + e.getMessage());
+				log.info("Error saving XLS file: " + e);
 				return null;
 			}
 		} catch (IOException e) {
-			System.err.println("Error converting workbook to XLS: " + e.getMessage());
+			log.info("Error converting workbook to XLS: " + e);
 			return null;
 		}
 	}
@@ -417,56 +430,75 @@ private void validateAttributes(Map<String, Object> attributeNameVsDataTypeMap,
 			Cell cell = row.getCell(j);
 			Cell columnName = columnHeaderRow.getCell(j);
 			String name = findByValue(mappedValues, columnName.getStringCellValue());
-			String value;
 			if (attributeNameVsDataTypeMap.containsKey(name)) {
-				value = attributeNameVsDataTypeMap.get(name).toString();
-				switch (cell.getCellType()) {
-				case STRING:
-					String cellValue = cell.getStringCellValue();
-					if (j == indexOfBoundaryCode && !boundaryCodeList.contains(cellValue)) {
-						log.info("Boundary Code "+ cellValue+" is not present in boundary search. Code for row " + (row.getRowNum() + 1)
-								+ " and cell/column " + columnName);
-						throw new CustomException(Integer.toString(HttpStatus.INTERNAL_SERVER_ERROR.value()),
-								"Boundary Code "+ cellValue+" is not present in boundary search. Code for row " + (row.getRowNum() + 1)
-								+ " and cell/column " + columnName);
-					}
-					// "^[a-zA-Z0-9 .,()-]+$"
-					if (cellValue != null && !cellValue.isEmpty()
-							&& cellValue.matches(ServiceConstants.VALIDATE_STRING_REGX)) {
-						continue;
-					} else {
-						log.info(ServiceConstants.INPUT_IS_NOT_VALID + (row.getRowNum() + 1) + " and cell/column "
-								+ columnName);
+				Map<String, Object> mapOfAttributes = (Map<String, Object>) attributeNameVsDataTypeMap.get(name);
+				boolean isRequired = (mapOfAttributes.containsKey(ServiceConstants.ATTRIBUTE_IS_REQUIRED)
+						? (boolean) mapOfAttributes.get(ServiceConstants.ATTRIBUTE_IS_REQUIRED)
+						: false);
+				if (cell != null) {
+					switch (cell.getCellType()) {
+					case STRING:
+						String cellValue = cell.getStringCellValue();
+						if (j == indexOfBoundaryCode && !boundaryCodeList.contains(cellValue)) {
+							log.info("Boundary Code " + cellValue + " is not present in boundary search. Code for row "
+									+ (row.getRowNum() + 1) + " and cell/column " + columnName);
+							throw new CustomException(Integer.toString(HttpStatus.INTERNAL_SERVER_ERROR.value()),
+									"Boundary Code " + cellValue + " is not present in boundary search. Code for row "
+											+ (row.getRowNum() + 1) + " and cell/column " + columnName);
+						}
+						// "^[a-zA-Z0-9 .,()-]+$"
+						if (cellValue != null && !cellValue.isEmpty()
+								&& cellValue.matches(ServiceConstants.VALIDATE_STRING_REGX)) {
+							continue;
+						} else {
+							log.info(ServiceConstants.INPUT_IS_NOT_VALID + (row.getRowNum() + 1) + " and cell/column "
+									+ columnName);
+							throw new CustomException(Integer.toString(HttpStatus.INTERNAL_SERVER_ERROR.value()),
+									ServiceConstants.INPUT_IS_NOT_VALID + row.getRowNum() + " and cell " + columnName);
+						}
+					case NUMERIC:
+						String numricValue = Double.toString(cell.getNumericCellValue());
+						// "^[-+]?[0-9]*\\.?[0-9]+([eE][-+]?[0-9]+)?$"
+						if (numricValue != null && !numricValue.isEmpty()
+								&& numricValue.matches(ServiceConstants.VALIDATE_NUMBER_REGX)) {
+							continue;
+						} else {
+							log.info(ServiceConstants.INPUT_IS_NOT_VALID + (row.getRowNum() + 1) + " and cell/column "
+									+ columnName);
+							throw new CustomException(Integer.toString(HttpStatus.INTERNAL_SERVER_ERROR.value()),
+									ServiceConstants.INPUT_IS_NOT_VALID + row.getRowNum() + " and cell " + columnName);
+						}
+					case BOOLEAN:
+						Boolean booleanvalue = cell.getBooleanCellValue();
+						// "^(?i)(true|false)$"
+						if (booleanvalue != null && !booleanvalue.toString().isEmpty()
+								&& booleanvalue.toString().matches(ServiceConstants.VALIDATE_BOOLEAN_REGX)) {
+							continue;
+						} else {
+							log.info(ServiceConstants.INPUT_IS_NOT_VALID + (row.getRowNum() + 1) + " and cell/column "
+									+ columnName);
+							throw new CustomException(Integer.toString(HttpStatus.INTERNAL_SERVER_ERROR.value()),
+									ServiceConstants.INPUT_IS_NOT_VALID + row.getRowNum() + " and cell " + columnName);
+						}
+					case BLANK:
+						if (!isRequired) {
+							continue;
+						}else {
+							throw new CustomException(Integer.toString(HttpStatus.INTERNAL_SERVER_ERROR.value()),
+									ServiceConstants.INPUT_IS_NOT_VALID + (row.getRowNum() + 1) + " and cell "
+											+ columnName);
+						}
+					default:
 						throw new CustomException(Integer.toString(HttpStatus.INTERNAL_SERVER_ERROR.value()),
-								ServiceConstants.INPUT_IS_NOT_VALID + row.getRowNum() + " and cell " + columnName);
+								ServiceConstants.INPUT_IS_NOT_VALID + (row.getRowNum() + 1) + " and cell "
+										+ columnName);
 					}
-				case NUMERIC:
-					String numricValue = Double.toString(cell.getNumericCellValue());
-					// "^[-+]?[0-9]*\\.?[0-9]+([eE][-+]?[0-9]+)?$"
-					if (numricValue != null && !numricValue.isEmpty()
-							&& numricValue.matches(ServiceConstants.VALIDATE_NUMBER_REGX)) {
-						continue;
-					} else {
-						log.info(ServiceConstants.INPUT_IS_NOT_VALID + (row.getRowNum() + 1) + " and cell/column "
-								+ columnName);
-						throw new CustomException(Integer.toString(HttpStatus.INTERNAL_SERVER_ERROR.value()),
-								ServiceConstants.INPUT_IS_NOT_VALID + row.getRowNum() + " and cell " + columnName);
-					}
-				case BOOLEAN:
-					Boolean booleanvalue = cell.getBooleanCellValue();
-					// "^(?i)(true|false)$"
-					if (booleanvalue != null && !booleanvalue.toString().isEmpty()
-							&& booleanvalue.toString().matches(ServiceConstants.VALIDATE_BOOLEAN_REGX)) {
-						continue;
-					} else {
-						log.info(ServiceConstants.INPUT_IS_NOT_VALID + (row.getRowNum() + 1) + " and cell/column "
-								+ columnName);
+				}else {
+					if(isRequired) {
 						throw new CustomException(Integer.toString(HttpStatus.INTERNAL_SERVER_ERROR.value()),
-								ServiceConstants.INPUT_IS_NOT_VALID + row.getRowNum() + " and cell " + columnName);
+								ServiceConstants.INPUT_IS_NOT_VALID + (row.getRowNum() + 1) + " and cell "
+										+ columnName);
 					}
-				default:
-					throw new CustomException(Integer.toString(HttpStatus.INTERNAL_SERVER_ERROR.value()),
-							ServiceConstants.INPUT_IS_NOT_VALID + (row.getRowNum() + 1) + " and cell " + columnName);
 				}
 			}
 		}

diff --git a/...s/resource-estimation-service/src/main/java/org/egov/processor/service/GeoJsonParser.java b/...s/resource-estimation-service/src/main/java/org/egov/processor/service/GeoJsonParser.java
@@ -56,7 +56,6 @@ public Object parseFileData(PlanConfigurationRequest planConfigurationRequest, S
     	PlanConfiguration planConfig = planConfigurationRequest.getPlanConfiguration();
         String geoJSON = parsingUtil.convertByteArrayToString(planConfig, fileStoreId);
 
-        ObjectMapper objectMapper = new ObjectMapper();
         JsonNode jsonNode = parsingUtil.parseJson(geoJSON, objectMapper);
 
         List<String> columnNamesList = parsingUtil.fetchAttributeNamesFromJson(jsonNode);

diff --git a/...stimation-service/src/main/java/org/egov/processor/service/ResourceEstimationService.java b/...stimation-service/src/main/java/org/egov/processor/service/ResourceEstimationService.java
@@ -50,7 +50,12 @@ public void estimateResources(PlanConfigurationRequest planConfigurationRequest)
         CampaignSearchRequest campaignRequest = campaignIntegrationUtil.buildCampaignRequestForSearch(planConfigurationRequest);
         Object campaignSearchResponse = serviceRequestRepository.fetchResult(new StringBuilder(config.getProjectFactoryHostEndPoint()+config.getCampaignIntegrationSearchEndPoint()),
 				campaignRequest);
-        for(File file:planConfiguration.getFiles())
+        processFiles(planConfigurationRequest, planConfiguration, parserMap, campaignSearchResponse);
+    }
+
+	private void processFiles(PlanConfigurationRequest planConfigurationRequest, PlanConfiguration planConfiguration,
+			Map<File.InputFileTypeEnum, FileParser> parserMap, Object campaignSearchResponse) {
+		for(File file:planConfiguration.getFiles())
         {
             if(file.getActive())
             {
@@ -62,7 +67,7 @@ public void estimateResources(PlanConfigurationRequest planConfigurationRequest)
                 parser.parseFileData(planConfigurationRequest, file.getFilestoreId(), campaignSearchResponse);
             }
         }
-    }
+	}
 
     /**
      * Retrieves a map of input file types to their respective parsers.

diff --git a/...resource-estimation-service/src/main/java/org/egov/processor/service/ShapeFileParser.java b/...resource-estimation-service/src/main/java/org/egov/processor/service/ShapeFileParser.java
@@ -1,18 +1,14 @@
 package org.egov.processor.service;
 
-import com.fasterxml.jackson.databind.JsonNode;
-import com.fasterxml.jackson.databind.ObjectMapper;
 import java.io.File;
-import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.math.BigDecimal;
-import java.net.MalformedURLException;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
-import lombok.extern.slf4j.Slf4j;
+
 import org.egov.processor.util.CalculationUtil;
 import org.egov.processor.util.FilestoreUtil;
 import org.egov.processor.util.ParsingUtil;
@@ -26,6 +22,11 @@
 import org.geotools.geojson.feature.FeatureJSON;
 import org.springframework.stereotype.Service;
 
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+
+import lombok.extern.slf4j.Slf4j;
+
 @Slf4j
 @Service
 public class ShapeFileParser implements FileParser {
@@ -103,7 +104,7 @@ public File convertShapefileToGeoJson(PlanConfiguration planConfig, String fileS
 
             writeFeaturesToGeoJson(featureSource, geojsonFile);
         } catch (IOException e) {
-            throw new CustomException(e.getMessage(), "");
+            throw new CustomException("CONVERSION_ERROR",e.getMessage());
         }
 
         return geojsonFile;
@@ -122,7 +123,7 @@ private DataStore getDataStore(File shapefile) {
             params.put("url", shapefile.toURI().toURL());
             return DataStoreFinder.getDataStore(params);
         } catch (IOException e) {
-            throw new CustomException(e.getMessage(),"");
+        	throw new CustomException("Exception accours while getting data store",e.getMessage());
         }
 
     }
@@ -138,7 +139,7 @@ private void writeFeaturesToGeoJson(SimpleFeatureSource featureSource, File geoj
         try (FileOutputStream geojsonStream = new FileOutputStream(geojsonFile)) {
             new FeatureJSON().writeFeatureCollection(featureSource.getFeatures(), geojsonStream);
         } catch (IOException e) {
-            throw new CustomException("","");
+            throw new CustomException("Failed to write feature to GeoJson",e.getMessage());
         }
     }