Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

HCMPRE-470 validation changes for assumptions #902

Merged
merged 8 commits into from
Oct 7, 2024
Merged

HCMPRE-470 validation changes for assumptions #902

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
00feaee
HCMPRE-470 adding changes for validating assumptions on the basis of …
Priyanka-eGov Sep 24, 2024
1048d45
HCMPRE-470 adding migration file from HCMPRE-499 for testing
Priyanka-eGov Sep 25, 2024
985fd63
HCMPRE-470 removing migration file from HCMPRE-499 after testing
Priyanka-eGov Sep 25, 2024
4a90d7a
HCMPRE-470 code review comments
Priyanka-eGov Sep 25, 2024
54538bc
HCMPRE-470 code review comments
Priyanka-eGov Sep 27, 2024
9fc5f1e
HCMPRE-470 code review comments
Priyanka-eGov Sep 27, 2024
d6bb7d3
HCMPRE-470 enhancing method comment
Priyanka-eGov Sep 27, 2024
dba72d4
Merge branch 'microplanning-dev' into HCMPRE-470
Priyanka-eGov Oct 7, 2024
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
20 changes: 18 additions & 2 deletions health-services/plan-service/src/main/java/digit/config/ServiceConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,26 @@ public class ServiceConstants {

public static final String BOUNDARY_CODE = "boundaryCode";

public static final String FILTER_ALL_ASSUMPTIONS = "[*].assumptionCategories[*].assumptions[*]";
public static final String FILTER_ALL_ASSUMPTIONS = ".assumptionCategories[*].assumptions[*]";

public static final String NAME_VALIDATION_DATA = "Data";

public static final String VEHICLE_ID_FIELD = "vehicleIds";

// Constants for operators and JSON fields
public static final String AND = " && ";
public static final String EQUALS = " == ";
public static final String SINGLE_QUOTE = "'";
Priyanka-eGov marked this conversation as resolved.
Show resolved Hide resolved
public static final String CAMPAIGN_TYPE = "campaignType";
public static final String DISTRIBUTION_PROCESS = "DistributionProcess";
public static final String REGISTRATION_PROCESS = "RegistrationProcess";
public static final String RESOURCE_DISTRIBUTION_STRATEGY_CODE = "resourceDistributionStrategyCode";
public static final String IS_REGISTRATION_AND_DISTRIBUTION_TOGETHER = "isRegistrationAndDistributionHappeningTogetherOrSeparately";
Priyanka-eGov marked this conversation as resolved.
Show resolved Hide resolved

public static final String JSON_FIELD_CAMPAIGN_TYPE = "@.campaignType";
public static final String JSON_FIELD_DISTRIBUTION_PROCESS = "@.DistributionProcess";
public static final String JSON_FIELD_REGISTRATION_PROCESS = "@.RegistrationProcess";
public static final String JSON_FIELD_RESOURCE_DISTRIBUTION_STRATEGY_CODE = "@.resourceDistributionStrategyCode";
public static final String JSON_FIELD_IS_REGISTRATION_AND_DISTRIBUTION_TOGETHER = "@.isRegistrationAndDistributionHappeningTogetherOrSeparately";
public static final String JSON_FIELD_VEHICLE_ID = "vehicleIds";
Priyanka-eGov marked this conversation as resolved.
Show resolved Hide resolved

}
14 changes: 9 additions & 5 deletions ...rvices/plan-service/src/main/java/digit/service/validator/PlanConfigurationValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,13 @@ public void validateAssumptionValue(PlanConfiguration planConfiguration) {
*/
public void validateAssumptionKeyAgainstMDMS(PlanConfigurationRequest request, Object mdmsData) {
PlanConfiguration planConfiguration = request.getPlanConfiguration();
final String jsonPathForAssumption = JSON_ROOT_PATH + MDMS_PLAN_MODULE_NAME + DOT_SEPARATOR + MDMS_MASTER_ASSUMPTION + FILTER_ALL_ASSUMPTIONS;
Object additionalDetails = request.getPlanConfiguration().getAdditionalDetails();

String jsonPathForAssumption = commonUtil.createJsonPathForAssumption((String) commonUtil.extractFieldsFromAdditionalDetails(additionalDetails,CAMPAIGN_TYPE),
(String) commonUtil.extractFieldsFromAdditionalDetails(additionalDetails,DISTRIBUTION_PROCESS),
(String) commonUtil.extractFieldsFromAdditionalDetails(additionalDetails,REGISTRATION_PROCESS),
(String) commonUtil.extractFieldsFromAdditionalDetails(additionalDetails,RESOURCE_DISTRIBUTION_STRATEGY_CODE),
(String) commonUtil.extractFieldsFromAdditionalDetails(additionalDetails,IS_REGISTRATION_AND_DISTRIBUTION_TOGETHER));
Priyanka-eGov marked this conversation as resolved.
Show resolved Hide resolved
List<Object> assumptionListFromMDMS = null;
try {
log.info(jsonPathForAssumption);
Expand Down Expand Up @@ -541,16 +546,15 @@ public void validateUserInfo(PlanConfigurationRequest request)
*/
public void validateVehicleIdsFromAdditionalDetailsAgainstMDMS(PlanConfigurationRequest request, List<Mdms> mdmsV2Data)
{
List<String> vehicleIdsLinkedWithPlanConfig = commonUtil.extractVehicleIdsFromAdditionalDetails(request.getPlanConfiguration().getAdditionalDetails());
List<String> vehicleIdsLinkedWithPlanConfig = (List<String>) commonUtil.extractFieldsFromAdditionalDetails(request.getPlanConfiguration().getAdditionalDetails(), JSON_FIELD_VEHICLE_ID);
Priyanka-eGov marked this conversation as resolved.
Show resolved Hide resolved

List<String> vehicleIdsFromMdms = mdmsV2Data.stream()
.map(Mdms::getId)
.collect(Collectors.toList());
.toList();
Priyanka-eGov marked this conversation as resolved.
Show resolved Hide resolved

List<String> finalVehicleIdsFromMdms = vehicleIdsFromMdms;
vehicleIdsLinkedWithPlanConfig.stream()
.forEach(vehicleId -> {
if(!finalVehicleIdsFromMdms.contains(vehicleId))
if(!vehicleIdsFromMdms.contains(vehicleId))
Priyanka-eGov marked this conversation as resolved.
Show resolved Hide resolved
{
log.error("Vehicle Id " + vehicleId + " is not present in MDMS");
throw new CustomException(VEHICLE_ID_NOT_FOUND_IN_MDMS_CODE, VEHICLE_ID_NOT_FOUND_IN_MDMS_MESSAGE);
Expand Down
57 changes: 49 additions & 8 deletions health-services/plan-service/src/main/java/digit/util/CommonUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,23 +44,64 @@ public Boolean validateStringAgainstRegex(String patternString, String inputStri
* @param additionalDetails the additionalDetails object from PlanConfigurationRequest
* @return a list of vehicle Ids from additional details
*/
public List<String> extractVehicleIdsFromAdditionalDetails(Object additionalDetails) {
public Object extractFieldsFromAdditionalDetails(Object additionalDetails, String fieldToExtract) {
try {
String jsonString = objectMapper.writeValueAsString(additionalDetails);
JsonNode rootNode = objectMapper.readTree(jsonString);

List<String> vehicleIds = new ArrayList<>();
JsonNode vehicleIdsNode = rootNode.get(VEHICLE_ID_FIELD);
if (vehicleIdsNode != null && vehicleIdsNode.isArray()) {
for (JsonNode idNode : vehicleIdsNode) {
vehicleIds.add(idNode.asText());
List<String> listFromAdditionalDetails = new ArrayList<>();
JsonNode node = rootNode.get(fieldToExtract);
if (node != null && node.isArray()) {
for (JsonNode idNode : node) {
listFromAdditionalDetails.add(idNode.asText());
}
return listFromAdditionalDetails;
Priyanka-eGov marked this conversation as resolved.
Show resolved Hide resolved
} else if (node != null) {
// Return the value in its original type based on its type
if (node.isInt()) {
return node.asInt();
} else if (node.isBoolean()) {
return node.asBoolean();
} else if (node.isTextual()) {
return node.asText();
Priyanka-eGov marked this conversation as resolved.
Show resolved Hide resolved
}
}

return vehicleIds;
// In case the node is of some other type (like object or binary), handle accordingly
return node;
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

Refactor to improve type safety by specifying a consistent return type

The method extractFieldsFromAdditionalDetails currently returns an Object, which could be a List<String>, Integer, Boolean, String, or even a JsonNode. This approach can lead to confusion and potential runtime errors for callers, as they need to perform type checks and casts based on the returned value.

Consider refactoring the method to return a more specific type or use generics to enhance type safety and clarity. For example, you could parameterize the method to return a specified type based on the expected field content.

Apply the following changes to specify the return type using generics:

-public Object extractFieldsFromAdditionalDetails(Object additionalDetails, String fieldToExtract) {
+public <T> T extractFieldsFromAdditionalDetails(Object additionalDetails, String fieldToExtract, Class<T> valueType) {
     try {
         String jsonString = objectMapper.writeValueAsString(additionalDetails);
         JsonNode rootNode = objectMapper.readTree(jsonString);

-        List<String> listFromAdditionalDetails = new ArrayList<>();
         JsonNode node = rootNode.get(fieldToExtract);
         if (node != null && node.isArray()) {
-            for (JsonNode idNode : node) {
-                listFromAdditionalDetails.add(idNode.asText());
-            }
-            return listFromAdditionalDetails;
+            return objectMapper.convertValue(node, objectMapper.getTypeFactory().constructCollectionType(List.class, valueType));
         } else if (node != null) {
-            // Return the value in its original type based on its type
-            if (node.isInt()) {
-                return node.asInt();
-            } else if (node.isBoolean()) {
-                return node.asBoolean();
-            } else if (node.isTextual()) {
-                return node.asText();
-            }
+            return objectMapper.convertValue(node, valueType);
         }
         // In case the node is of some other type (like object or binary), handle accordingly
         return null;
     } catch (Exception e) {
         log.error(e.getMessage());
         throw new CustomException(JSONPATH_ERROR_CODE, JSONPATH_ERROR_MESSAGE);
     }
 }

This change allows callers to specify the expected return type, enhancing usability and reducing the need for type checks.

📝 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
public Object extractFieldsFromAdditionalDetails(Object additionalDetails, String fieldToExtract) {
try {
String jsonString = objectMapper.writeValueAsString(additionalDetails);
JsonNode rootNode = objectMapper.readTree(jsonString);
List<String> vehicleIds = new ArrayList<>();
JsonNode vehicleIdsNode = rootNode.get(VEHICLE_ID_FIELD);
if (vehicleIdsNode != null && vehicleIdsNode.isArray()) {
for (JsonNode idNode : vehicleIdsNode) {
vehicleIds.add(idNode.asText());
List<String> listFromAdditionalDetails = new ArrayList<>();
JsonNode node = rootNode.get(fieldToExtract);
if (node != null && node.isArray()) {
for (JsonNode idNode : node) {
listFromAdditionalDetails.add(idNode.asText());
}
return listFromAdditionalDetails;
} else if (node != null) {
// Return the value in its original type based on its type
if (node.isInt()) {
return node.asInt();
} else if (node.isBoolean()) {
return node.asBoolean();
} else if (node.isTextual()) {
return node.asText();
}
}
return vehicleIds;
// In case the node is of some other type (like object or binary), handle accordingly
return node;
public <T> T extractFieldsFromAdditionalDetails(Object additionalDetails, String fieldToExtract, Class<T> valueType) {
try {
String jsonString = objectMapper.writeValueAsString(additionalDetails);
JsonNode rootNode = objectMapper.readTree(jsonString);
JsonNode node = rootNode.get(fieldToExtract);
if (node != null && node.isArray()) {
return objectMapper.convertValue(node, objectMapper.getTypeFactory().constructCollectionType(List.class, valueType));
} else if (node != null) {
return objectMapper.convertValue(node, valueType);
}
// In case the node is of some other type (like object or binary), handle accordingly
return null;
} catch (Exception e) {
log.error(e.getMessage());
throw new CustomException(JSONPATH_ERROR_CODE, JSONPATH_ERROR_MESSAGE);
}
}

Priyanka-eGov marked this conversation as resolved.
Show resolved Hide resolved
} catch (Exception e) {
log.error(e.getMessage());
throw new CustomException(JSONPATH_ERROR_CODE, JSONPATH_ERROR_MESSAGE);
}
}

/**
* Constructs a JSONPath expression used to filter assumptions based on the given parameters -
* campaign type, distribution process, registration process, resource distribution strategy,
* and whether registration and distribution are together match the provided values.
*
* @param campaignType The type of campaign to filter by (e.g., "Health", "Education").
* @param distributionProcess The process of distribution to filter by (e.g., "Central", "Decentralized").
* @param registrationProcess The registration process to filter by (e.g., "Online", "In-Person").
* @param resourceDistributionStrategyCode The strategy code for resource distribution to filter by (e.g., "Strategy1").
* @param isRegistrationAndDistributionTogether Whether registration and distribution are combined, to filter by ("true"/"false").
* @return A JSONPath expression string that filters assumptions based on the given criteria.
*/
public String createJsonPathForAssumption(
String campaignType,
String distributionProcess,
String registrationProcess,
String resourceDistributionStrategyCode,
String isRegistrationAndDistributionTogether
) {

StringBuilder jsonPathFilters = new StringBuilder("[?(");
jsonPathFilters.append(JSON_FIELD_CAMPAIGN_TYPE).append(EQUALS).append(SINGLE_QUOTE).append(campaignType).append(SINGLE_QUOTE)
.append(AND).append(JSON_FIELD_DISTRIBUTION_PROCESS).append(EQUALS).append(SINGLE_QUOTE).append(distributionProcess).append(SINGLE_QUOTE)
.append(AND).append(JSON_FIELD_REGISTRATION_PROCESS).append(EQUALS).append(SINGLE_QUOTE).append(registrationProcess).append(SINGLE_QUOTE)
.append(AND).append(JSON_FIELD_RESOURCE_DISTRIBUTION_STRATEGY_CODE).append(EQUALS).append(SINGLE_QUOTE).append(resourceDistributionStrategyCode).append(SINGLE_QUOTE)
.append(AND).append(JSON_FIELD_IS_REGISTRATION_AND_DISTRIBUTION_TOGETHER).append(EQUALS).append(SINGLE_QUOTE).append(isRegistrationAndDistributionTogether).append(SINGLE_QUOTE)
.append(")]");

return JSON_ROOT_PATH + MDMS_PLAN_MODULE_NAME + DOT_SEPARATOR + MDMS_MASTER_ASSUMPTION + jsonPathFilters + FILTER_ALL_ASSUMPTIONS;
}
Priyanka-eGov marked this conversation as resolved.
Show resolved Hide resolved
}