Created API for get count of any fields and with field value search #328
Conversation
|
Task linked: CU-86c0dgwg9 API query endpoint |
WalkthroughThe pull request introduces enhancements across multiple classes in the Changes
Possibly related PRs
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
CodeRabbit Configuration File (
|
…unt-of-Age-group CU-86c0h13xz - Create API for get count of Age group
…, alos created new api to get all list of fields
…hould-be-able-to-filter-by-optional-date-range CU-86c0h142v - Average patient age (should be able to filter by optio…
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 25
🧹 Outside diff range and nitpick comments (14)
JeMPI_Apps/JeMPI_LibMPI/src/main/java/org/jembi/jempi/libmpi/LibMPIClientInterface.java (1)
58-62: Overall assessment of new methods in LibMPIClientInterfaceThe addition of these three methods (
getFieldCount,getAgeGroupCount, andgetAllList) aligns with the PR objectives of creating an API for getting counts of fields and enabling field value searches. However, there are some areas that need attention:
- Consistency in return types: Ensure that return types accurately reflect the nature of the data being returned (e.g., numeric types for counts).
- Type safety: Consider using more appropriate types for parameters, especially for dates.
- Method naming: Ensure method names clearly indicate their purpose and functionality.
- Documentation: Add comprehensive Javadoc comments for all new methods, explaining their purpose, parameters, and return values.
These changes will significantly impact the API's usability and maintainability. Addressing the suggested improvements will make the interface more intuitive and less prone to misuse.
Consider the following architectural improvements:
- Group related methods together within the interface for better organization.
- If these new methods are part of a specific feature set, consider creating a separate interface for them, which can be extended by
LibMPIClientInterface.- Ensure that the
ApiModelsclass is well-documented and its usage is consistent across the codebase.JeMPI_Apps/JeMPI_LibShared/src/main/java/org/jembi/jempi/shared/models/ApiModels.java (3)
405-413: LGTM! Consider adding JavaDoc comments.The
CountFieldsrecord is well-structured and follows Java naming conventions. The use of@JsonInclude(JsonInclude.Include.NON_NULL)is appropriate for excluding null fields during JSON serialization.Consider adding JavaDoc comments to describe the purpose of this record and its fields, especially explaining the expected format for
startDateandendDate.
415-420: LGTM! Consider enhancing the record for more precise age-related searches.The
SearchAgeCountFieldsrecord is well-structured and follows Java naming conventions. The use of@JsonInclude(JsonInclude.Include.NON_NULL)is appropriate for excluding null fields during JSON serialization.Consider enhancing this record to support more precise age-related searches:
- Add JavaDoc comments to describe the purpose of this record and its fields.
- Consider adding fields like
minAgeandmaxAgefor more granular age filtering:@JsonInclude(JsonInclude.Include.NON_NULL) public record SearchAgeCountFields( String startDate, String endDate, Integer minAge, Integer maxAge ) { }This would allow for more flexible age-range queries in addition to date-based filtering.
Line range hint
405-449: Overall improvements for new records and file consistency.The new records (
CountFields,SearchAgeCountFields,AllList, andAverageAgeResponse) add useful structures for API interactions. However, there are some inconsistencies and areas for improvement across these additions.Consider the following general improvements for the entire file:
- Ensure all records have the
@JsonInclude(JsonInclude.Include.NON_NULL)annotation for consistency.- Add JavaDoc comments to all records, describing their purpose and the meaning of each field.
- Use a consistent multi-line format for all records to improve readability.
- Review the naming of records (especially
AllList) to ensure they clearly convey their purpose.- Consider grouping related records together within the file for better organization.
Implementing these suggestions will enhance the overall consistency, readability, and maintainability of the
ApiModelsclass.JeMPI_Apps/JeMPI_LibMPI/src/main/java/org/jembi/jempi/libmpi/LibMPI.java (1)
Line range hint
1-166: Consider refactoring for consistent error handling and connection managementThe
LibMPIclass contains numerous methods that follow a similar pattern of connecting to the client and then calling a corresponding client method. However, there's inconsistency in error handling across these methods.Consider the following improvements:
- Implement a consistent error handling strategy across all methods.
- Consider using a try-with-resources pattern or a similar mechanism to manage client connections.
- Create a private helper method to reduce code duplication for connecting to the client and handling exceptions.
Here's an example of how you might refactor the class:
public final class LibMPI { // ... existing fields ... private <T> T executeClientOperation(Function<LibMPIClientInterface, T> operation) throws ClientException { try { client.connect(); return operation.apply(client); } catch (Exception e) { throw new ClientException("Error executing client operation", e); } } public long getFieldCount(final ApiModels.CountFields countFields) throws ClientException { return executeClientOperation(client -> client.getFieldCount(countFields)); } public long getAgeGroupCount(final LocalDate startDate, final LocalDate endDate) throws ClientException { return executeClientOperation(client -> client.getAgeGroupCount(startDate, endDate)); } public List<String> getAllItemsList(final ApiModels.AllList allListRequest) throws ClientException { return executeClientOperation(client -> client.getAllList(allListRequest)); } // ... refactor other methods similarly ... }This refactoring would provide consistent error handling and connection management across all methods in the class.
JeMPI_Apps/JeMPI_LibAPI/src/main/java/org/jembi/jempi/libapi/Ask.java (1)
336-370: Consider adding method-level comments for the new methods.While the implementation of the new methods (
getAgeGroupCount,getAllList, andgetFieldCount) is correct and consistent with the class design, it would be beneficial to add brief method-level comments explaining the purpose of each method. This would improve code readability and make it easier for other developers to understand the functionality of these methods.documentation/JeMPI.postman_collection.json (5)
9-25: New "Count Golden Records" request added.This new GET request aligns with the PR objectives to create an API for getting counts of fields. The endpoint naming is clear and follows RESTful principles.
Consider adding a description to the request to provide more context about its purpose and any query parameters it might accept.
44-63: "Count Golden Records" request renamed and updated to "genderCount".The changes to this request align well with the PR objectives:
- Renaming to "genderCount" makes the purpose more specific.
- Changing to POST with a request body allows for more complex querying.
- The body structure provides flexibility for querying by field name, value, record type, and date range.
Consider generalizing the name to "fieldCount" instead of "genderCount", as the body allows counting any field, not just gender.
70-91: "Count Records" request renamed and updated to "get Age Group Count".The changes to this request improve its specificity and flexibility:
- Renaming to "get Age Group Count" clarifies the purpose.
- Adding a date range in the body allows for more flexible querying.
However, for consistency with other requests:
- Consider renaming to "ageGroupCount" to match the naming pattern of "genderCount".
- Add a "recordType" field in the body to match the structure of the "genderCount" request.
95-120: New "All List of Field" request added.This new POST request aligns with the PR objectives by allowing retrieval of all values for a specific field within a date range. The body structure is consistent with other requests, which is good.
Suggestions for improvement:
- Rename the request to "getFieldValues" or "listFieldValues" for clarity and consistency with other request names.
- Add a "recordType" field in the body to match the structure of the "genderCount" request.
- Consider using a more descriptive name for the "field" parameter, such as "fieldName".
849-849: Updated "CR Update fields" request body structure.The changes to the "CR Update fields" request body improve flexibility by allowing updates to multiple fields in a single request. This aligns well with the PR objectives of enhancing API functionality.
Consider adding a description to this request explaining the purpose of the "facility" field and any constraints on the fields that can be updated.
JeMPI_Apps/JeMPI_LibAPI/src/main/java/org/jembi/jempi/libapi/BackEnd.java (2)
556-557: Consider using 'calculateAllList' method to compute average ageThe
calculateAllListmethod is defined but not used. If the intention is to calculate the average age and utilize it, consider calling this method and handling its result.Uncomment the method call and adjust the response if needed:
- // double allList = calculateAllList(dobList); + double averageAge = calculateAllList(dobList);
692-692: Rename 'genderCount' to 'fieldCount' for clarityIn
GetFieldCountResponse, the field is namedgenderCount, which may be misleading if the count pertains to different fields. Renaming it tofieldCountmakes it more general and clear.Apply this diff:
- public record GetFieldCountResponse(String genderCount) implements EventResponse { } + public record GetFieldCountResponse(String fieldCount) implements EventResponse { }Don't forget to update the related variables and methods accordingly.
JeMPI_Apps/JeMPI_LibAPI/src/main/java/org/jembi/jempi/libapi/Routes.java (1)
713-718: Method names suggestGETrequests but are used inPOSTroutesThe methods
getFieldCount,getAgeGroupCount, andgetAllListare added under thePOSTroutes increateCoreAPIRoutes. However, their names start withget, which typically corresponds to HTTP GET requests. This could lead to confusion about the HTTP method being used.Consider renaming the methods to reflect that they handle POST requests:
- public static Route getFieldCount( + public static Route postFieldCount( - public static Route getAgeGroupCount( + public static Route postAgeGroupCount( - public static Route getAllList( + public static Route postAllList(Alternatively, if appropriate, consider moving these routes under the
GETroutes section if they fit RESTful conventions and do not require a request body.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (10)
- JeMPI_Apps/JeMPI_LibAPI/src/main/java/org/jembi/jempi/libapi/Ask.java (1 hunks)
- JeMPI_Apps/JeMPI_LibAPI/src/main/java/org/jembi/jempi/libapi/BackEnd.java (4 hunks)
- JeMPI_Apps/JeMPI_LibAPI/src/main/java/org/jembi/jempi/libapi/Routes.java (4 hunks)
- JeMPI_Apps/JeMPI_LibMPI/src/main/java/org/jembi/jempi/libmpi/LibMPI.java (1 hunks)
- JeMPI_Apps/JeMPI_LibMPI/src/main/java/org/jembi/jempi/libmpi/LibMPIClientInterface.java (1 hunks)
- JeMPI_Apps/JeMPI_LibMPI/src/main/java/org/jembi/jempi/libmpi/dgraph/DgraphQueries.java (4 hunks)
- JeMPI_Apps/JeMPI_LibMPI/src/main/java/org/jembi/jempi/libmpi/dgraph/LibDgraph.java (1 hunks)
- JeMPI_Apps/JeMPI_LibShared/src/main/java/org/jembi/jempi/shared/models/ApiModels.java (2 hunks)
- JeMPI_Apps/JeMPI_LibShared/src/main/java/org/jembi/jempi/shared/models/GlobalConstants.java (2 hunks)
- documentation/JeMPI.postman_collection.json (9 hunks)
🔇 Additional comments (18)
JeMPI_Apps/JeMPI_LibShared/src/main/java/org/jembi/jempi/shared/models/GlobalConstants.java (4)
28-28: LGTM: New constant SEGMENT_GET_FIELD_COUNT added.The new constant
SEGMENT_GET_FIELD_COUNTis consistent with the existing naming convention and aligns with the PR objective of creating an API for getting the count of fields.
66-66: LGTM: New constant SEGMENT_GET_AGE_GROUP_COUNT added. Please clarify its purpose.The new constant
SEGMENT_GET_AGE_GROUP_COUNTis consistent with the existing naming convention. However, it's not explicitly mentioned in the PR objectives. Could you please clarify the purpose of this constant and how it relates to the new API functionality?
67-67: LGTM: New constant SEGMENT_GET_ALL_LIST added. Please clarify its purpose and consider renaming.The new constant
SEGMENT_GET_ALL_LISTis consistent with the existing naming convention. However, it's not explicitly mentioned in the PR objectives. Could you please clarify the purpose of this constant and how it relates to the new API functionality?Additionally, consider renaming this constant to be more specific, e.g.,
SEGMENT_GET_ALL_FIELDS_LISTorSEGMENT_GET_ALL_RECORDS_LIST, depending on its intended use. This would make its purpose clearer and maintain consistency with the naming of other constants in this class.
28-28: Summary: New constants added for API endpoints. Please provide additional context.The three new constants (
SEGMENT_GET_FIELD_COUNT,SEGMENT_GET_AGE_GROUP_COUNT, andSEGMENT_GET_ALL_LIST) have been added consistently with the existing codebase. However, to ensure full alignment with the PR objectives:
- Could you provide more context on how
SEGMENT_GET_AGE_GROUP_COUNTandSEGMENT_GET_ALL_LISTrelate to the new API for getting count of fields and field value search?- Are there any additional constants needed to fully implement the described API functionality?
- Consider adding comments to these new constants to explain their specific purposes within the API.
To ensure these new endpoints are properly implemented, please run the following script:
Also applies to: 66-67
✅ Verification successful
Verified: New constants are correctly implemented and utilized in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of new API endpoints # Test: Search for the usage of new constants in the codebase echo "Searching for SEGMENT_GET_FIELD_COUNT usage:" rg --type java "SEGMENT_GET_FIELD_COUNT" echo "Searching for SEGMENT_GET_AGE_GROUP_COUNT usage:" rg --type java "SEGMENT_GET_AGE_GROUP_COUNT" echo "Searching for SEGMENT_GET_ALL_LIST usage:" rg --type java "SEGMENT_GET_ALL_LIST" # Test: Look for new API endpoint implementations echo "Searching for new API endpoint implementations:" rg --type java "getFieldCount|getAgeGroupCount|getAllList"Length of output: 7958
JeMPI_Apps/JeMPI_LibMPI/src/main/java/org/jembi/jempi/libmpi/dgraph/LibDgraph.java (4)
82-92: Summary of changes and alignment with PR objectivesThe additions to the
LibDgraphclass align well with the PR objectives of creating an API for getting counts of fields and enabling field value searches. The three new methods (getFieldCount,getAgeGroupCount, andgetAllList) expand the capabilities of the class, allowing for more flexible querying options.These methods serve as wrappers around corresponding methods in the
DgraphQueriesclass, which is a common pattern for maintaining separation of concerns and allowing for easier testing and maintenance.While the implementations look correct, there are a few areas where additional clarification or improvements could be made:
- The return type of
getFieldCount(String) could be explained or potentially changed to a more specific type.- The date parameters in
getAgeGroupCountcould be made more type-safe by usingLocalDateinstead ofString.- The purpose and usage of
getAllListcould be better documented.Overall, these additions enhance the API's functionality and align with the PR's goals. Adding comprehensive Javadoc comments to these methods would greatly improve the code's documentation and usability.
90-92: Approve changes and request clarification on method purpose.The implementation looks correct as it delegates the call to
DgraphQueries.getAllList(allList).To improve documentation and understanding of this method, please provide answers to the following questions:
- What is the purpose of this method? What kind of data does it retrieve?
- What does the
List<String>return value represent? Are these identifiers, names, or some other type of data?- Can you provide more information about the
ApiModels.AllListparameter? What fields or options does it contain?Consider adding a Javadoc comment to the method explaining its purpose, parameters, and return value.
To understand the usage of this method, let's search for its invocations:
rg "getAllList\(" --type java
86-88: Approve changes and suggest potential improvements.The implementation looks correct as it delegates the call to
DgraphQueries.getAgeGroupCount(startDate, endDate). The use oflongas the return type is appropriate for a count operation.Suggestions for improvement:
- Consider using
LocalDateorZonedDateTimeinstead ofStringfor date parameters to ensure type safety and avoid potential parsing errors.- If
Stringmust be used, consider adding input validation or documenting the expected date format in the method's Javadoc.Here's a potential refactor using
LocalDate:-public long getAgeGroupCount(final String startDate, final String endDate) { - return DgraphQueries.getAgeGroupCount(startDate, endDate); +public long getAgeGroupCount(final LocalDate startDate, final LocalDate endDate) { + return DgraphQueries.getAgeGroupCount(startDate, endDate); }Note: This refactor assumes that
DgraphQueries.getAgeGroupCountcan acceptLocalDateparameters. If not, you may need to convertLocalDatetoStringwithin this method.To understand the usage of this method and the format of date strings, let's search for its invocations:
✅ Verification successful
Changes Verified Successfully.
The
getAgeGroupCountmethod is correctly implemented and utilized across the codebase. All references and definitions have been confirmed.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
rg "getAgeGroupCount\(" --type javaLength of output: 1668
Script:
#!/bin/bash # Search for the method definition of getAgeGroupCount ast-grep --lang java --pattern $'public long getAgeGroupCount($_, $_) { $$$ }'Length of output: 79
Script:
#!/bin/bash # Search for any references to getAgeGroupCount rg "getAgeGroupCount\(" --type javaLength of output: 1675
82-84: Approve changes and request clarification on return type.The implementation looks correct as it delegates the call to
DgraphQueries.getFieldCount(countFields). However, the return typeStringis unusual for a count operation.Could you please clarify:
- What format does the returned String have? (e.g., JSON, plain number, formatted text)
- Why was String chosen as the return type instead of a numeric type like
longorint?To understand the usage of this method, let's search for its invocations:
JeMPI_Apps/JeMPI_LibAPI/src/main/java/org/jembi/jempi/libapi/Ask.java (3)
336-346: LGTM: Implementation ofgetAgeGroupCountis consistent and well-structured.The method follows the established pattern in the class for handling asynchronous requests. It correctly uses
AskPatternto send the request and handle the response, with an appropriate timeout setting.
348-358: LGTM: Implementation ofgetAllListis consistent and well-structured.The method adheres to the established pattern in the class for handling asynchronous requests. It correctly uses
AskPatternto send the request and handle the response, with an appropriate timeout setting.
360-370: LGTM: Implementation ofgetFieldCountis consistent and well-structured.The method follows the established pattern in the class for handling asynchronous requests. It correctly uses
AskPatternto send the request and handle the response, with an appropriate timeout setting.documentation/JeMPI.postman_collection.json (6)
3-3: Postman collection ID updated.The _postman_id has been changed, which is expected when updating or recreating a Postman collection. This change doesn't affect the functionality of the API requests.
29-29: Method changed to GET for "Count Interactions" request.Changing the method from POST to GET for the "Count Interactions" request is an improvement in RESTful API design. This change aligns well with the newly added "Count Golden Records" request and is more appropriate for a counting operation.
214-214: Updated example values in "Get Expanded Golden Record" and "Get Expanded Golden Records" requests.The "gid" and "uidList" values have been updated in these requests. These changes appear to be updates to example values rather than structural changes. Ensure that these new values are valid and representative of the data in the current system.
Also applies to: 240-240
910-938: New "Increase Lease" request added.This new GET request appears to be related to increasing the lease on UIDs, possibly for performance optimization. However, there are a few points to consider:
- The purpose of this request is not immediately clear. Consider adding a description to explain its function and when it should be used.
- The use of a localhost URL suggests this might be a development or internal operation. Verify if this is intended to be part of the public API.
- The "num" parameter value (50000000000000) is unusually large. Confirm if this is the correct value or if it should be adjusted.
To better understand the purpose and usage of this request, please provide more context or documentation about the "Increase Lease" functionality.
Line range hint
1-938: Overall assessment of changes to the Postman collection.The modifications to this Postman collection significantly enhance the API's functionality, aligning well with the PR objectives of creating an API for getting counts of fields and enabling field value searches. Key improvements include:
- New requests for counting and listing field values.
- Updates to existing requests to support more flexible querying.
- Addition of backup and restore functionality.
While the changes are generally positive, consider addressing the following points:
- Standardize naming conventions across requests (e.g., "genderCount" vs "get Age Group Count").
- Review and possibly enhance security measures for sensitive operations, especially in backup and restore requests.
- Provide more descriptive names and add descriptions to requests to improve clarity.
- Verify the purpose and correct usage of the "Increase Lease" request.
Overall, these changes represent a significant improvement in the API's capabilities and support for data analysis and management.
285-391: New backup and restore requests added.The addition of backup and restore functionality is a significant feature. These new requests use a different base URL (http://localhost:50010), suggesting they might be handled by a separate service.
Points to consider:
- Ensure that the backup and restore endpoints are properly secured, especially since they handle sensitive data.
- The large JSON payload in the "Restore Golden Record Backup-Restore" request contains personal information. Make sure this data is transmitted securely and consider minimizing the example data in the Postman collection.
- Verify that these localhost URLs are appropriate for the collection, or if they should use the {{LocalApi}} variable like other requests.
Run the following script to check for potential security issues:
JeMPI_Apps/JeMPI_LibMPI/src/main/java/org/jembi/jempi/libmpi/dgraph/DgraphQueries.java (1)
19-22: Imports are appropriateThe added imports for
JsonMappingException,JsonNode, andObjectMapperare necessary for JSON parsing in the newly added methods.
| @@ -55,6 +55,12 @@ List<ExpandedSourceId> findExpandedSourceIdList( | |||
|
|
|||
| PaginatedResultSet<ExpandedGoldenRecord> findExpandedGoldenRecords(List<String> goldenIds); | |||
|
|
|||
| String getFieldCount(ApiModels.CountFields countFields); | |||
There was a problem hiding this comment.
Consider revising the return type and adding documentation for getFieldCount.
The method getFieldCount returns a String, which seems inconsistent with its name suggesting a count operation. Consider the following points:
- If the method is intended to return a count, it would be more appropriate to use a numeric return type like
intorlong. - If a
Stringreturn type is necessary (e.g., for a formatted result), please add a comment explaining the expected format and why aStringis used. - Add Javadoc comments to describe the method's purpose, parameters, and return value.
Here's a suggested revision:
/**
* Counts the occurrences of specified fields.
*
* @param countFields The fields to be counted, specified using ApiModels.CountFields
* @return The count of the specified fields
*/
long getFieldCount(ApiModels.CountFields countFields);If a String return type is necessary, please explain why in a comment.
| @@ -55,6 +55,12 @@ List<ExpandedSourceId> findExpandedSourceIdList( | |||
|
|
|||
| PaginatedResultSet<ExpandedGoldenRecord> findExpandedGoldenRecords(List<String> goldenIds); | |||
|
|
|||
| String getFieldCount(ApiModels.CountFields countFields); | |||
|
|
|||
| long getAgeGroupCount(String startDate, String endDate); | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety and add documentation for getAgeGroupCount.
While the method name and return type are consistent, there are a few improvements that could be made:
- Consider using
LocalDateinstead ofStringfor the date parameters. This would improve type safety and prevent potential parsing errors. - Add Javadoc comments to describe the method's purpose, parameters, and return value.
- If
Stringmust be used for dates, specify the expected date format in the documentation.
Here's a suggested revision:
import java.time.LocalDate;
/**
* Counts the number of records within a specified age group.
*
* @param startDate The start date of the age range (inclusive)
* @param endDate The end date of the age range (inclusive)
* @return The count of records within the specified age range
*/
long getAgeGroupCount(LocalDate startDate, LocalDate endDate);If String must be used for dates, please add a comment specifying the expected date format:
/**
* Counts the number of records within a specified age group.
*
* @param startDate The start date of the age range (inclusive) in the format "yyyy-MM-dd"
* @param endDate The end date of the age range (inclusive) in the format "yyyy-MM-dd"
* @return The count of records within the specified age range
*/
long getAgeGroupCount(String startDate, String endDate);|
|
||
| long getAgeGroupCount(String startDate, String endDate); | ||
|
|
||
| List<String> getAllList(ApiModels.AllList allListRequest); |
There was a problem hiding this comment.
Clarify the purpose of getAllList and add documentation.
The method getAllList is currently vague and lacks clarity. Consider the following improvements:
- Rename the method to more accurately reflect its specific purpose. For example, if it's retrieving a list of all users, it could be named
getAllUsers. - Add Javadoc comments to describe the method's purpose, parameters, and return value.
- Consider using a more specific return type if possible, rather than
List<String>.
Here's a suggested revision (assuming it's retrieving a list of users as an example):
/**
* Retrieves a list of all users based on the specified criteria.
*
* @param allListRequest The criteria for retrieving the user list
* @return A list of user identifiers matching the specified criteria
*/
List<String> getAllUsers(ApiModels.AllList allListRequest);If the method is intended for a different purpose, please adjust the name and documentation accordingly to clearly reflect its functionality.
| public long getAgeGroupCount(final String startDate, final String endDate) { | ||
| client.connect(); | ||
| return client.getAgeGroupCount(startDate, endDate); | ||
| } |
There was a problem hiding this comment.
Add exception handling and consider using Date objects
While the return type is appropriate for a count operation, the method lacks exception handling. Additionally, using String for date parameters might lead to format inconsistencies.
Consider the following improvements:
- Add exception handling.
- Use
java.time.LocalDateinstead ofStringfor date parameters.
Here's a suggested implementation:
-public long getAgeGroupCount(final String startDate, final String endDate) {
+public long getAgeGroupCount(final LocalDate startDate, final LocalDate endDate) throws ClientException {
client.connect();
- return client.getAgeGroupCount(startDate, endDate);
+ try {
+ return client.getAgeGroupCount(startDate, endDate);
+ } catch (Exception e) {
+ throw new ClientException("Error getting age group count", e);
+ }
}This change assumes that ClientException is a custom exception class that you'd need to define, and that the client method can accept LocalDate objects.
Committable suggestion was skipped due to low confidence.
| totalAge += age; | ||
| count++; | ||
| } catch (DateTimeParseException e) { | ||
| LOGGER.error("Invalid date format for dob: " + dob + ". Skipping this record."); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use placeholders in logging instead of string concatenation
In the logging statement, string concatenation is used, which can lead to unnecessary object creation even if the log level is not enabled. It's better to use placeholders for efficiency.
Apply this diff:
- LOGGER.error("Invalid date format for dob: " + dob + ". Skipping this record.");
+ LOGGER.error("Invalid date format for dob: {}. Skipping this record.", dob);📝 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.
| LOGGER.error("Invalid date format for dob: " + dob + ". Skipping this record."); | |
| LOGGER.error("Invalid date format for dob: {}. Skipping this record.", dob); |
| /** | ||
| * Get age group count based on start and end dates for DOB. | ||
| * | ||
| * @param startDate The start date of the age range (inclusive) | ||
| * @param endDate The end date of the age range (inclusive) | ||
| * @return A string containing the JSON response with the count | ||
| */ | ||
| public static long getAgeGroupCount(final String startDate, final String endDate) { | ||
| // Constructing the query string | ||
| String query = String.format(Locale.ROOT, | ||
| """ | ||
| { | ||
| list(func: has(GoldenRecord.dob)) @filter(ge(GoldenRecord.dob, "%s") AND le(GoldenRecord.dob, "%s")) { | ||
| count(uid) | ||
| } | ||
| } | ||
| """, | ||
| startDate, endDate); // Order of %s matches the date range | ||
| try { | ||
| LOGGER.info("Query: {}", query); | ||
| return getCount(query); | ||
| } catch (Exception e) { | ||
| LOGGER.error("Error executing age group count query", e); | ||
| return 0L; // Return default count in case of error | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Sanitize inputs when constructing queries
In getAgeGroupCount, startDate and endDate are directly inserted into the query string. Without proper validation, this could lead to query injection vulnerabilities.
Ensure that inputs are validated and sanitized before use in the query.
| JsonNode peopleArray = jsonNode.get("peopleInDateRange"); | ||
|
|
||
| // Extract each `dob` and add to the list | ||
| for (JsonNode person : peopleArray) { | ||
| if (person.has("GoldenRecord.dob")) { | ||
| dobList.add(person.get("GoldenRecord.dob").asText()); | ||
| } | ||
| } | ||
| return dobList; |
There was a problem hiding this comment.
Check for null before iterating over peopleArray
In parseDobFromResponse, if peopleInDateRange is not present in the JSON, peopleArray will be null. Iterating over a null collection will cause a NullPointerException.
Add a null check before the loop:
JsonNode peopleArray = jsonNode.get("peopleInDateRange");
+ if (peopleArray != null) {
for (JsonNode person : peopleArray) {
if (person.has("GoldenRecord.dob")) {
dobList.add(person.get("GoldenRecord.dob").asText());
}
}
+ }📝 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.
| JsonNode peopleArray = jsonNode.get("peopleInDateRange"); | |
| // Extract each `dob` and add to the list | |
| for (JsonNode person : peopleArray) { | |
| if (person.has("GoldenRecord.dob")) { | |
| dobList.add(person.get("GoldenRecord.dob").asText()); | |
| } | |
| } | |
| return dobList; | |
| JsonNode peopleArray = jsonNode.get("peopleInDateRange"); | |
| if (peopleArray != null) { | |
| // Extract each `dob` and add to the list | |
| for (JsonNode person : peopleArray) { | |
| if (person.has("GoldenRecord.dob")) { | |
| dobList.add(person.get("GoldenRecord.dob").asText()); | |
| } | |
| } | |
| } | |
| return dobList; |
| private static String getFieldsCount(final String query) { | ||
| try { | ||
| return DgraphClient.getInstance().executeReadOnlyTransaction(query, null); | ||
| } catch (Exception e) { | ||
| LOGGER.error(e.getLocalizedMessage()); | ||
| return e.getLocalizedMessage(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Avoid returning exception messages as method results
In the getFieldsCount method, when an exception occurs, the method returns e.getLocalizedMessage() as the result. Returning exception messages as method outputs can lead to inconsistent behavior and may expose internal error details.
Consider throwing the exception or returning a consistent error response instead.
Apply this diff to adjust the error handling:
private static String getFieldsCount(final String query) {
try {
return DgraphClient.getInstance().executeReadOnlyTransaction(query, null);
} catch (Exception e) {
LOGGER.error(e.getLocalizedMessage());
- return e.getLocalizedMessage();
+ throw new RuntimeException("Error executing query", e);
}
}📝 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 static String getFieldsCount(final String query) { | |
| try { | |
| return DgraphClient.getInstance().executeReadOnlyTransaction(query, null); | |
| } catch (Exception e) { | |
| LOGGER.error(e.getLocalizedMessage()); | |
| return e.getLocalizedMessage(); | |
| } | |
| } | |
| private static String getFieldsCount(final String query) { | |
| try { | |
| return DgraphClient.getInstance().executeReadOnlyTransaction(query, null); | |
| } catch (Exception e) { | |
| LOGGER.error(e.getLocalizedMessage()); | |
| throw new RuntimeException("Error executing query", e); | |
| } | |
| } |
| e.printStackTrace(); | ||
| return List.of("Error: " + e.getMessage()); | ||
| } |
There was a problem hiding this comment.
Improve exception handling and logging
Using e.printStackTrace() is not suitable for production code as it can clutter the console and is not thread-safe. Additionally, returning an error message as a data result can confuse the caller.
Log the error properly and handle the exception appropriately:
} catch (Exception e) {
- e.printStackTrace();
- return List.of("Error: " + e.getMessage());
+ LOGGER.error("Error executing getAllList", e);
+ throw new RuntimeException("Error executing getAllList", e);
}📝 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.
| e.printStackTrace(); | |
| return List.of("Error: " + e.getMessage()); | |
| } | |
| LOGGER.error("Error executing getAllList", e); | |
| throw new RuntimeException("Error executing getAllList", e); | |
| } |
| public static List<String> getAllList(final ApiModels.AllList allListRequest) { | ||
| try { | ||
| // Assume these values come from a request | ||
| String field = allListRequest.field(); | ||
| String startDate = allListRequest.startDate(); // Empty means no start date | ||
| String endDate = allListRequest.endDate(); // Empty means no end date | ||
|
|
||
| // Build the query dynamically based on date range availability | ||
| String query; | ||
| if (!startDate.isEmpty() && !endDate.isEmpty()) { | ||
| query = String.format(""" | ||
| { | ||
| peopleInDateRange(func: has(GoldenRecord.%s)) | ||
| @filter(ge(GoldenRecord.aux_date_created, "%s") AND le(GoldenRecord.aux_date_created, "%s")) { | ||
| GoldenRecord.%s | ||
| } | ||
| } | ||
| """, field, startDate, endDate, field); | ||
| } else { | ||
| query = String.format(""" | ||
| { | ||
| peopleInDateRange(func: has(GoldenRecord.%s)) { | ||
| GoldenRecord.%s | ||
| } | ||
| } | ||
| """, field, field); | ||
| } | ||
| LOGGER.info("Query: {}", query); | ||
| // Assuming `DgraphClient` is set up to make requests | ||
| String responseJson = DgraphClient.getInstance().executeReadOnlyTransaction(query, null); | ||
| // Parse the JSON response to extract DOBs | ||
| List<String> dobList = parseDobFromResponse(responseJson); | ||
| return dobList; | ||
| } catch (Exception e) { | ||
| e.printStackTrace(); | ||
| return List.of("Error: " + e.getMessage()); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Add null checks for startDate and endDate to prevent NullPointerException
In the getAllList method, you check !startDate.isEmpty() and !endDate.isEmpty() without null checks. If either startDate or endDate is null, this will result in a NullPointerException.
Add null checks before checking if the strings are empty:
- if (!startDate.isEmpty() && !endDate.isEmpty()) {
+ if (startDate != null && endDate != null && !startDate.isEmpty() && !endDate.isEmpty()) {📝 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 static List<String> getAllList(final ApiModels.AllList allListRequest) { | |
| try { | |
| // Assume these values come from a request | |
| String field = allListRequest.field(); | |
| String startDate = allListRequest.startDate(); // Empty means no start date | |
| String endDate = allListRequest.endDate(); // Empty means no end date | |
| // Build the query dynamically based on date range availability | |
| String query; | |
| if (!startDate.isEmpty() && !endDate.isEmpty()) { | |
| query = String.format(""" | |
| { | |
| peopleInDateRange(func: has(GoldenRecord.%s)) | |
| @filter(ge(GoldenRecord.aux_date_created, "%s") AND le(GoldenRecord.aux_date_created, "%s")) { | |
| GoldenRecord.%s | |
| } | |
| } | |
| """, field, startDate, endDate, field); | |
| } else { | |
| query = String.format(""" | |
| { | |
| peopleInDateRange(func: has(GoldenRecord.%s)) { | |
| GoldenRecord.%s | |
| } | |
| } | |
| """, field, field); | |
| } | |
| LOGGER.info("Query: {}", query); | |
| // Assuming `DgraphClient` is set up to make requests | |
| String responseJson = DgraphClient.getInstance().executeReadOnlyTransaction(query, null); | |
| // Parse the JSON response to extract DOBs | |
| List<String> dobList = parseDobFromResponse(responseJson); | |
| return dobList; | |
| } catch (Exception e) { | |
| e.printStackTrace(); | |
| return List.of("Error: " + e.getMessage()); | |
| } | |
| } | |
| public static List<String> getAllList(final ApiModels.AllList allListRequest) { | |
| try { | |
| // Assume these values come from a request | |
| String field = allListRequest.field(); | |
| String startDate = allListRequest.startDate(); // Empty means no start date | |
| String endDate = allListRequest.endDate(); // Empty means no end date | |
| // Build the query dynamically based on date range availability | |
| String query; | |
| if (startDate != null && endDate != null && !startDate.isEmpty() && !endDate.isEmpty()) { | |
| query = String.format(""" | |
| { | |
| peopleInDateRange(func: has(GoldenRecord.%s)) | |
| @filter(ge(GoldenRecord.aux_date_created, "%s") AND le(GoldenRecord.aux_date_created, "%s")) { | |
| GoldenRecord.%s | |
| } | |
| } | |
| """, field, startDate, endDate, field); | |
| } else { | |
| query = String.format(""" | |
| { | |
| peopleInDateRange(func: has(GoldenRecord.%s)) { | |
| GoldenRecord.%s | |
| } | |
| } | |
| """, field, field); | |
| } | |
| LOGGER.info("Query: {}", query); | |
| // Assuming `DgraphClient` is set up to make requests | |
| String responseJson = DgraphClient.getInstance().executeReadOnlyTransaction(query, null); | |
| // Parse the JSON response to extract DOBs | |
| List<String> dobList = parseDobFromResponse(responseJson); | |
| return dobList; | |
| } catch (Exception e) { | |
| e.printStackTrace(); | |
| return List.of("Error: " + e.getMessage()); | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (5)
documentation/JeMPI.postman_collection.json (4)
44-56: LGTM: Enhanced gender count endpoint with flexible queryingThe changes to this endpoint (renamed to "genderCount") provide more flexibility in querying gender data. The use of POST with a detailed request body allows for complex queries, including filtering by date range and record type.
Consider renaming the endpoint to follow a more consistent naming convention, such as "getGenderCount" or "countByGender", to better reflect its purpose and align with other endpoint names.
70-82: LGTM: New age group count endpoint with date range filteringThe changes to this endpoint (renamed to "get Age Group Count") provide the ability to count age groups within a specified date range. The use of POST with a request body allows for flexible date filtering.
Consider renaming the endpoint to follow a more consistent naming convention, such as "getAgeGroupCount" or "countByAgeGroup", to better align with other endpoint names and RESTful practices.
95-121: LGTM: New endpoint for retrieving field data with filtering optionsThe addition of the "All Data List of Field" endpoint provides a flexible way to retrieve field data with date range filtering. The detailed description in the comments is helpful for understanding the endpoint's functionality.
Consider renaming the endpoint to follow a more consistent naming convention, such as "getFieldDataList" or "listFieldData", to better reflect its purpose and align with RESTful naming practices.
Line range hint
850-858: LGTM: Updated field update structure with facility exampleThe changes to the "CR Update fields" request body provide a clearer example of how to update fields, including the addition of a 'facility' field. This update improves the documentation and usability of the endpoint.
Consider adding a brief comment to explain the purpose and usage of the 'facility' field, as it may not be immediately clear to all users why this field is being updated.
JeMPI_Apps/JeMPI_LibAPI/src/main/java/org/jembi/jempi/libapi/BackEnd.java (1)
551-563: Consider implementing average age calculationThere's a commented out line that suggests an intention to calculate the average age from the list of DOBs. If this functionality is needed, it should be implemented.
If the average age calculation is required, consider uncommenting and implementing it:
- // double allList = calculateAllList(dobList); + double averageAge = calculateAllList(dobList); + LOGGER.info("Average age: {}", averageAge);If this functionality is not needed, consider removing the commented line to improve code clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- JeMPI_Apps/JeMPI_LibAPI/src/main/java/org/jembi/jempi/libapi/BackEnd.java (4 hunks)
- JeMPI_Apps/JeMPI_LibAPI/src/main/java/org/jembi/jempi/libapi/Routes.java (4 hunks)
- JeMPI_Apps/JeMPI_LibShared/src/main/java/org/jembi/jempi/shared/models/GlobalConstants.java (2 hunks)
- documentation/JeMPI.postman_collection.json (9 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- JeMPI_Apps/JeMPI_LibShared/src/main/java/org/jembi/jempi/shared/models/GlobalConstants.java
🔇 Additional comments (11)
documentation/JeMPI.postman_collection.json (7)
3-3: LGTM: Postman ID updateThe update to the
_postman_idis an expected change when modifying a Postman collection. This doesn't affect the functionality and is likely an automatic update.
9-25: LGTM: New endpoint for counting Golden RecordsThe addition of the "Count Golden Records" endpoint aligns with the PR objective of creating an API for getting counts of fields. The use of the GET method is appropriate for this read operation.
29-29: LGTM: Method change for Count InteractionsChanging the method from POST to GET for the "Count Interactions" endpoint is a good improvement. This aligns with RESTful API best practices, as counting is a read operation that doesn't modify server state.
Line range hint
400-404: LGTM: Updated example data in request bodyThe change in the uidList for the "Get Expanded Interaction Records Using CSV Copy" request is a simple update to the example data. This modification doesn't affect the endpoint's functionality and likely reflects a more relevant example for testing or documentation purposes.
426-426: LGTM: Updated example gid in request bodyThe change in the gid value for the "Get Golden Record Audit Trail" request is a simple update to the example data. This modification doesn't affect the endpoint's functionality and likely reflects a more relevant example for testing or documentation purposes.
Line range hint
1-939: Overall assessment of Postman collection changesThe changes to this Postman collection significantly enhance the API's functionality, aligning well with the PR objectives. New endpoints for field counts, data retrieval, backup/restore operations, and lease management have been added, providing more comprehensive capabilities for the JeMPI system.
Key points:
- The new endpoints for counting and retrieving field data offer flexible querying options.
- Backup and restore functionality has been introduced, which is crucial for data management.
- Most changes follow RESTful practices, with appropriate use of HTTP methods.
Please address the following points:
- Ensure consistent naming conventions across all endpoints for better readability and maintenance.
- Review and document the security measures for endpoints using localhost URLs, especially for backup/restore and lease management operations.
- Verify that all new endpoints are properly secured and documented for production use.
These enhancements greatly improve the API's capabilities, but careful attention to security and consistency will ensure a robust and maintainable system.
912-939: New utility endpoint added for lease managementThe addition of the "Increase Lease" endpoint appears to be a utility for managing resource allocation or permissions.
Please address the following concerns:
- The purpose of this endpoint is not immediately clear. Consider adding a comment explaining its functionality and when it should be used.
- The use of a localhost URL with a different port (6080) suggests this is a separate service. Ensure this is intentional and properly secured in production environments.
- The 'num' parameter value (50000000000000) is unusually large. Verify if this is correct and consider adding a comment explaining its significance.
Run the following script to check for other instances of this port usage:
#!/bin/bash # Search for other usages of port 6080 rg --type json 'localhost:6080'This will help identify any other endpoints using this port and ensure consistent documentation.
JeMPI_Apps/JeMPI_LibAPI/src/main/java/org/jembi/jempi/libapi/BackEnd.java (2)
690-701: New record definitions look goodThe new record definitions for GetFieldCountRequest, GetFieldCountResponse, GetAgeGroupCountRequest, GetAgeGroupCountResponse, GetAllListRequest, and GetAllListResponse are well-structured and consistent with the existing code style.
These new record definitions provide clear and concise representations of the request and response objects for the new functionality. Good job!
Line range hint
524-701: Overall assessment of new functionalityThe new functionality for field counts, age group counts, and retrieving lists has been implemented well. The code is generally clean and follows the existing patterns in the class. Here's a summary of the findings:
- Error messages in catch blocks for
getFieldCountHandlerandgetAgeGroupCountHandlerneed to be corrected.- The
getAllListHandlerhas a commented-out line that might indicate incomplete functionality.- The
calculateAllListmethod could benefit from improved encapsulation and use of constants.- New record definitions are well-structured and consistent.
Overall, the new functionality is a good addition to the
BackEndclass. After addressing the minor issues mentioned in the previous comments, this code will be ready for production.JeMPI_Apps/JeMPI_LibAPI/src/main/java/org/jembi/jempi/libapi/Routes.java (2)
574-585: LGTM: Well-structured and consistent implementationThe
getAgeGroupCountmethod is well-implemented and consistent with other methods in the class. It properly handles error cases and returns the result directly, which is efficient and clear.
587-598: Consider changing method visibility for consistencyThe
getDataListmethod is currently declared asprivate, while the other newly added methods (getFieldCountandgetAgeGroupCount) arepublic. This inconsistency might be intentional, but it's worth reviewing to ensure it aligns with the intended usage.If this method should be accessible from outside the class, consider changing its visibility to
public. Otherwise, please confirm that theprivatevisibility is intentional.✅ Verification successful
Method
getDataListis not used outside ofRoutes.javaBased on the analysis,
getDataListis not referenced outside of theRoutes.javafile aside from a string constant. Therefore, keeping itprivateis appropriate for encapsulation.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify if getDataList is used outside of this file rg --type java -g '!Routes.java' 'getDataList'Length of output: 207
| "protocol": "http", | ||
| "host": [ | ||
| "localhost" | ||
| ], | ||
| "port": "50010", | ||
| "path": [ | ||
| "JeMPI", | ||
| "gidsAll" | ||
| ] | ||
| } | ||
| }, | ||
| "response": [] | ||
| }, | ||
| { | ||
| "name": "Get Expanded Golden Record Backup-Restore", | ||
| "request": { | ||
| "method": "POST", | ||
| "header": [], | ||
| "body": { | ||
| "mode": "raw", | ||
| "raw": "{\n \"gid\": \"0x41\"\n}", | ||
| "options": { | ||
| "raw": { | ||
| "language": "json" | ||
| } | ||
| } | ||
| }, | ||
| "url": { | ||
| "raw": "http://localhost:50010/JeMPI/expandedGoldenRecord", | ||
| "protocol": "http", | ||
| "host": [ | ||
| "localhost" | ||
| ], | ||
| "port": "50010", | ||
| "path": [ | ||
| "JeMPI", | ||
| "expandedGoldenRecord" | ||
| ] | ||
| } | ||
| }, | ||
| "response": [] | ||
| }, | ||
| { | ||
| "name": "Get Expanded Golden Records Using UidList Backup-Restore'", | ||
| "request": { | ||
| "method": "POST", | ||
| "header": [], | ||
| "body": { | ||
| "mode": "raw", | ||
| "raw": "{\n \"uidList\": [\n \"0x940\",\n \"0x5\"\n ]\n}", | ||
| "options": { | ||
| "raw": { | ||
| "language": "json" | ||
| } | ||
| } | ||
| }, | ||
| "url": { | ||
| "raw": "http://localhost:50010/JeMPI/expandedGoldenRecords", | ||
| "protocol": "http", | ||
| "host": [ | ||
| "localhost" | ||
| ], | ||
| "port": "50010", | ||
| "path": [ | ||
| "JeMPI", | ||
| "expandedGoldenRecords" | ||
| ] | ||
| } | ||
| }, | ||
| "response": [] | ||
| }, | ||
| { | ||
| "name": "Restore Golden Record Backup-Restore", | ||
| "request": { | ||
| "method": "POST", | ||
| "header": [], | ||
| "body": { | ||
| "mode": "raw", | ||
| "raw": "{\n \"goldenRecord\": {\n \"uid\": \"0x4e28\",\n \"sourceId\": [\n {\n \"uid\": \"0x4e26\",\n \"facility\": \"Church & Dwight Co., Inc.\",\n \"patient\": \"672861\"\n },\n {\n \"uid\": \"0x4e29\",\n \"facility\": \"Jubilant HollisterStier LLC\",\n \"patient\": \"672861\"\n },\n {\n \"uid\": \"0x4e2b\",\n \"facility\": \"REMEDYREPACK INC.\",\n \"patient\": \"672861\"\n },\n {\n \"uid\": \"0x4e2d\",\n \"facility\": \"Amerisource Bergen\",\n \"patient\": \"672861\"\n },\n {\n \"uid\": \"0x4e2f\",\n \"facility\": \"GREENBRIER INTERNATIONAL\",\n \"patient\": \"672861\"\n },\n {\n \"uid\": \"0x4e31\",\n \"facility\": \"Sandoz Inc\",\n \"patient\": \"672861\"\n }\n ],\n \"uniqueGoldenRecordData\": {\n \"auxDateCreated\": \"2016-10-30T14:22:25.00Z\",\n \"auxAutoUpdateEnabled\": true,\n \"auxId\": \"672861\",\n \"auxGid\": \"1\"\n },\n \"demographicData\": {\n \"givenName\": \"felekech\",\n \"familyName\": \"eyob\",\n \"gender\": \"male\",\n \"dob\": \"19791014\",\n \"city\": \"granica\",\n \"phoneNumber\": \"0582212007\",\n \"nationalId\": \"19791014\"\n }\n },\n \"interactionsWithScore\": [\n {\n \"interaction\": {\n \"uid\": \"0x4e27\",\n \"sourceId\": {\n \"uid\": \"0x4e26\",\n \"facility\": \"Church & Dwight Co., Inc.\",\n \"patient\": \"672861\"\n },\n \"uniqueInteractionData\": {\n \"auxDateCreated\": \"2016-10-30T14:22:25.00Z\",\n \"auxId\": \"672861\",\n \"auxClinicalData\": \"672861\",\n \"auxIid\": \"1\"\n },\n \"demographicData\": {\n \"givenName\": \"felekech\",\n \"familyName\": \"eyob\",\n \"gender\": \"male\",\n \"dob\": \"19791014\",\n \"city\": \"granica\",\n \"phoneNumber\": \"0582212007\",\n \"nationalId\": \"19791014\"\n }\n },\n \"score\": 1.0\n },\n {\n \"interaction\": {\n \"uid\": \"0x4e2a\",\n \"sourceId\": {\n \"uid\": \"0x4e29\",\n \"facility\": \"Jubilant HollisterStier LLC\",\n \"patient\": \"672861\"\n },\n \"uniqueInteractionData\": {\n \"auxDateCreated\": \"2016-10-30T14:22:25.00Z\",\n \"auxId\": \"672861\",\n \"auxClinicalData\": \"672861\",\n \"auxIid\": \"2\"\n },\n \"demographicData\": {\n \"givenName\": \"felekecb\",\n \"familyName\": \"eyob\",\n \"gender\": \"male\",\n \"dob\": \"19791014\",\n \"city\": \"tangu\",\n \"phoneNumber\": \"0582213007\",\n \"nationalId\": \"19791014\"\n }\n },\n \"score\": 1.0\n },\n {\n \"interaction\": {\n \"uid\": \"0x4e2c\",\n \"sourceId\": {\n \"uid\": \"0x4e2b\",\n \"facility\": \"REMEDYREPACK INC.\",\n \"patient\": \"672861\"\n },\n \"uniqueInteractionData\": {\n \"auxDateCreated\": \"2016-10-30T14:22:25.00Z\",\n \"auxId\": \"672861\",\n \"auxClinicalData\": \"672861\",\n \"auxIid\": \"3\"\n },\n \"demographicData\": {\n \"givenName\": \"felekech\",\n \"familyName\": \"eyob\",\n \"gender\": \"male\",\n \"dob\": \"19791014\",\n \"city\": \"niebocko\",\n \"phoneNumber\": \"0582212007\",\n \"nationalId\": \"19791014\"\n }\n },\n \"score\": 1.0\n },\n {\n \"interaction\": {\n \"uid\": \"0x4e2e\",\n \"sourceId\": {\n \"uid\": \"0x4e2d\",\n \"facility\": \"Amerisource Bergen\",\n \"patient\": \"672861\"\n },\n \"uniqueInteractionData\": {\n \"auxDateCreated\": \"2016-10-30T14:22:25.00Z\",\n \"auxId\": \"672861\",\n \"auxClinicalData\": \"672861\",\n \"auxIid\": \"4\"\n },\n \"demographicData\": {\n \"givenName\": \"felekech\",\n \"familyName\": \"eyob\",\n \"gender\": \"male\",\n \"dob\": \"19791014\",\n \"city\": \"palca\",\n \"phoneNumber\": \"0582212007\",\n \"nationalId\": \"19791014\"\n }\n },\n \"score\": 1.0\n },\n {\n \"interaction\": {\n \"uid\": \"0x4e30\",\n \"sourceId\": {\n \"uid\": \"0x4e2f\",\n \"facility\": \"GREENBRIER INTERNATIONAL\",\n \"patient\": \"672861\"\n },\n \"uniqueInteractionData\": {\n \"auxDateCreated\": \"2016-10-30T14:22:25.00Z\",\n \"auxId\": \"672861\",\n \"auxClinicalData\": \"672861\",\n \"auxIid\": \"5\"\n },\n \"demographicData\": {\n \"givenName\": \"felekech\",\n \"familyName\": \"eyob\",\n \"gender\": \"female\",\n \"dob\": \"19791014\",\n \"city\": \"cinunjang\",\n \"phoneNumber\": \"0582212007\",\n \"nationalId\": \"19791014\"\n }\n },\n \"score\": 1.0\n },\n {\n \"interaction\": {\n \"uid\": \"0x4e32\",\n \"sourceId\": {\n \"uid\": \"0x4e31\",\n \"facility\": \"Sandoz Inc\",\n \"patient\": \"672861\"\n },\n \"uniqueInteractionData\": {\n \"auxDateCreated\": \"2016-10-30T14:22:25.00Z\",\n \"auxId\": \"672861\",\n \"auxClinicalData\": \"672861\",\n \"auxIid\": \"6\"\n },\n \"demographicData\": {\n \"givenName\": \"felekech\",\n \"familyName\": \"eyob\",\n \"gender\": \"female\",\n \"dob\": \"19791014\",\n \"city\": \"maceo\",\n \"phoneNumber\": \"0582212007\",\n \"nationalId\": \"19791014\"\n }\n },\n \"score\": 1.0\n }\n ]\n }", | ||
| "options": { | ||
| "raw": { | ||
| "language": "json" | ||
| } | ||
| } | ||
| }, | ||
| "url": { | ||
| "raw": "http://localhost:50010/JeMPI/restoreGoldenRecord", | ||
| "protocol": "http", | ||
| "host": [ | ||
| "localhost" | ||
| ], | ||
| "port": "50010", | ||
| "path": [ | ||
| "JeMPI", | ||
| "restoreGoldenRecord" | ||
| ] | ||
| } | ||
| }, | ||
| "response": [] | ||
| }, |
There was a problem hiding this comment.
💡 Codebase verification
Hardcoded localhost URLs found in Postman collection
Multiple hardcoded localhost URLs were found in documentation/JeMPI.postman_collection.json. It's recommended to replace these with environment variables to enhance security and flexibility.
documentation/JeMPI.postman_collection.json:"raw": "http://localhost:50010/JeMPI/gidsAll""raw": "http://localhost:50010/JeMPI/expandedGoldenRecord""raw": "http://localhost:50010/JeMPI/expandedGoldenRecords""raw": "http://localhost:50010/JeMPI/restoreGoldenRecord""raw": "http://localhost:6080/assign?what=uids&num=50000000000000""value": "http://localhost:50000/JeMPI"
🔗 Analysis chain
New backup and restore functionality added
The addition of backup and restore endpoints is a significant enhancement to the system's functionality. These new endpoints provide ways to retrieve and restore golden records, which can be crucial for data management and recovery processes.
Please verify the security implications of including localhost URLs in the Postman collection. While these are likely for testing purposes, ensure that:
- These URLs are not used in production environments.
- The backup and restore functionality is properly secured in production.
Run the following script to check for other instances of hardcoded localhost URLs:
This will help identify any other potential security concerns related to hardcoded URLs.
Action Required: Execute Corrected Script to Identify Hardcoded Localhost URLs
The previous script encountered a syntax error. Please run the following corrected script to search for all hardcoded localhost URLs in JSON files:
This will help ensure that no unintended localhost URLs are present in the codebase, addressing potential security concerns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for hardcoded localhost URLs
rg --type json 'http://localhost:[0-9]+'
Length of output: 242
Script:
#!/bin/bash
# Search for hardcoded localhost URLs in JSON files
rg --type json 'http://localhost:[0-9]+'
Length of output: 670
| private Behavior<Event> getAgeGroupCountHandler(final GetAgeGroupCountRequest request) { | ||
| long getCount = 0; | ||
| try { | ||
| String startDob = request.searchAgeCountFields.startDate(); | ||
| String endDob = request.searchAgeCountFields.endDate(); | ||
| LOGGER.info("startDob: {}, endDob: {}", startDob, endDob); | ||
| getCount = libMPI.getAgeGroupCount(startDob, endDob); | ||
| request.replyTo.tell(new GetAgeGroupCountResponse(getCount)); | ||
| } catch (Exception e) { | ||
| LOGGER.error(e.getLocalizedMessage(), e); | ||
| LOGGER.error("libMPI.findExpandedGoldenRecord failed for goldenId: {} with error: {}", e.getMessage()); | ||
| } | ||
| return Behaviors.same(); | ||
| } |
There was a problem hiding this comment.
Fix error message in catch block
The error message in the catch block is incorrect. It mentions "findExpandedGoldenRecord" which is not related to this method.
Please update the error message to reflect the correct method name:
- LOGGER.error("libMPI.findExpandedGoldenRecord failed for goldenId: {} with error: {}", e.getMessage());
+ LOGGER.error("libMPI.getAgeGroupCount failed with error: {}", e.getMessage());📝 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 Behavior<Event> getAgeGroupCountHandler(final GetAgeGroupCountRequest request) { | |
| long getCount = 0; | |
| try { | |
| String startDob = request.searchAgeCountFields.startDate(); | |
| String endDob = request.searchAgeCountFields.endDate(); | |
| LOGGER.info("startDob: {}, endDob: {}", startDob, endDob); | |
| getCount = libMPI.getAgeGroupCount(startDob, endDob); | |
| request.replyTo.tell(new GetAgeGroupCountResponse(getCount)); | |
| } catch (Exception e) { | |
| LOGGER.error(e.getLocalizedMessage(), e); | |
| LOGGER.error("libMPI.findExpandedGoldenRecord failed for goldenId: {} with error: {}", e.getMessage()); | |
| } | |
| return Behaviors.same(); | |
| } | |
| private Behavior<Event> getAgeGroupCountHandler(final GetAgeGroupCountRequest request) { | |
| long getCount = 0; | |
| try { | |
| String startDob = request.searchAgeCountFields.startDate(); | |
| String endDob = request.searchAgeCountFields.endDate(); | |
| LOGGER.info("startDob: {}, endDob: {}", startDob, endDob); | |
| getCount = libMPI.getAgeGroupCount(startDob, endDob); | |
| request.replyTo.tell(new GetAgeGroupCountResponse(getCount)); | |
| } catch (Exception e) { | |
| LOGGER.error(e.getLocalizedMessage(), e); | |
| LOGGER.error("libMPI.getAgeGroupCount failed with error: {}", e.getMessage()); | |
| } | |
| return Behaviors.same(); | |
| } |
| public static double calculateAllList(final List<String> dobList) { | ||
| LocalDate today = LocalDate.now(); // Get today's date | ||
| DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyyMMdd"); // DOB format in YYYYMMDD | ||
| int totalAge = 0; | ||
| int count = 0; | ||
| // Iterate through the list of DOBs and calculate the age for each | ||
| for (String dob : dobList) { | ||
| if (!dob.isEmpty()) { | ||
| try { | ||
| LocalDate birthDate = LocalDate.parse(dob, formatter); // Try to convert DOB to LocalDate | ||
| int age = Period.between(birthDate, today).getYears(); // Calculate age in years | ||
| totalAge += age; | ||
| count++; | ||
| } catch (DateTimeParseException e) { | ||
| LOGGER.error("Invalid date format for dob: " + dob + ". Skipping this record."); | ||
| } | ||
| } | ||
| } | ||
| // Calculate and return average age | ||
| return count > 0 ? (double) totalAge / count : 0; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider method visibility and use of constants
The calculateAllList method is currently public and static, which might expose it unnecessarily if it's only used within this class. Also, the date format string could be defined as a constant for better maintainability.
- If this method is only used within this class, consider making it private:
- public static double calculateAllList(final List<String> dobList) {
+ private static double calculateAllList(final List<String> dobList) {- Define the date format as a constant:
+ private static final DateTimeFormatter DOB_FORMATTER = DateTimeFormatter.ofPattern("yyyyMMdd");
public static double calculateAllList(final List<String> dobList) {
LocalDate today = LocalDate.now(); // Get today's date
- DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyyMMdd"); // DOB format in YYYYMMDD
int totalAge = 0;
int count = 0;
// Iterate through the list of DOBs and calculate the age for each
for (String dob : dobList) {
if (!dob.isEmpty()) {
try {
- LocalDate birthDate = LocalDate.parse(dob, formatter); // Try to convert DOB to LocalDate
+ LocalDate birthDate = LocalDate.parse(dob, DOB_FORMATTER); // Try to convert DOB to LocalDate
int age = Period.between(birthDate, today).getYears(); // Calculate age in years
totalAge += age;
count++;
} catch (DateTimeParseException e) {
LOGGER.error("Invalid date format for dob: " + dob + ". Skipping this record.");
}
}
}
// Calculate and return average age
return count > 0 ? (double) totalAge / count : 0;
}These changes will improve encapsulation and make the code more maintainable.
📝 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 static double calculateAllList(final List<String> dobList) { | |
| LocalDate today = LocalDate.now(); // Get today's date | |
| DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyyMMdd"); // DOB format in YYYYMMDD | |
| int totalAge = 0; | |
| int count = 0; | |
| // Iterate through the list of DOBs and calculate the age for each | |
| for (String dob : dobList) { | |
| if (!dob.isEmpty()) { | |
| try { | |
| LocalDate birthDate = LocalDate.parse(dob, formatter); // Try to convert DOB to LocalDate | |
| int age = Period.between(birthDate, today).getYears(); // Calculate age in years | |
| totalAge += age; | |
| count++; | |
| } catch (DateTimeParseException e) { | |
| LOGGER.error("Invalid date format for dob: " + dob + ". Skipping this record."); | |
| } | |
| } | |
| } | |
| // Calculate and return average age | |
| return count > 0 ? (double) totalAge / count : 0; | |
| } | |
| private static final DateTimeFormatter DOB_FORMATTER = DateTimeFormatter.ofPattern("yyyyMMdd"); | |
| private static double calculateAllList(final List<String> dobList) { | |
| LocalDate today = LocalDate.now(); // Get today's date | |
| int totalAge = 0; | |
| int count = 0; | |
| // Iterate through the list of DOBs and calculate the age for each | |
| for (String dob : dobList) { | |
| if (!dob.isEmpty()) { | |
| try { | |
| LocalDate birthDate = LocalDate.parse(dob, DOB_FORMATTER); // Try to convert DOB to LocalDate | |
| int age = Period.between(birthDate, today).getYears(); // Calculate age in years | |
| totalAge += age; | |
| count++; | |
| } catch (DateTimeParseException e) { | |
| LOGGER.error("Invalid date format for dob: " + dob + ". Skipping this record."); | |
| } | |
| } | |
| } | |
| // Calculate and return average age | |
| return count > 0 ? (double) totalAge / count : 0; | |
| } |
| path(GlobalConstants.SEGMENT_GET_FIELD_COUNT, | ||
| () -> Routes.getFieldCount(actorSystem, backEnd)), | ||
| path(GlobalConstants.SEGMENT_GET_AGE_GROUP_COUNT, | ||
| () -> Routes.getAgeGroupCount(actorSystem, backEnd)), | ||
| path(GlobalConstants.SEGMENT_GET_DATA_LIST, | ||
| () -> Routes.getDataList(actorSystem, backEnd)), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using GET method for retrieval operations
The new routes for getFieldCount, getAgeGroupCount, and getDataList are currently added to the POST route group. However, these operations are typically used for retrieving data rather than modifying it. According to RESTful principles, GET requests are more appropriate for such operations.
Consider moving these routes to the GET method group for better alignment with RESTful practices. Here's a suggested change:
-post(() -> concat(
+get(() -> concat(
// ... existing GET routes ...
path(GlobalConstants.SEGMENT_GET_FIELDS_CONFIGURATION,
() -> Routes.getFieldsConfiguration(actorSystem, backEnd)),
+ path(GlobalConstants.SEGMENT_GET_FIELD_COUNT,
+ () -> Routes.getFieldCount(actorSystem, backEnd)),
+ path(GlobalConstants.SEGMENT_GET_AGE_GROUP_COUNT,
+ () -> Routes.getAgeGroupCount(actorSystem, backEnd)),
+ path(GlobalConstants.SEGMENT_GET_DATA_LIST,
+ () -> Routes.getDataList(actorSystem, backEnd))
)))This change would make the API more intuitive and consistent with RESTful conventions.
Committable suggestion was skipped due to low confidence.
| ObjectMapper objectMapper = new ObjectMapper(); | ||
| JsonNode jsonResponse = null; | ||
| try { | ||
| jsonResponse = objectMapper.readTree(result.get().genderCount()); | ||
| } catch (JsonProcessingException e) { | ||
| throw new RuntimeException(e); | ||
| } | ||
| // Complete the request with JSON response | ||
| return complete(StatusCodes.OK, jsonResponse, JSON_MARSHALLER); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize ObjectMapper usage and improve exception handling
- Instead of creating a new
ObjectMapperinstance, reuse the existingOBJECT_MAPPERfromAppUtils. - Handle the
JsonProcessingExceptionmore gracefully instead of throwing aRuntimeException. - Consider returning the
genderCountdirectly without parsing it toJsonNodefor consistency with other methods.
Apply this diff to address these issues:
- ObjectMapper objectMapper = new ObjectMapper();
JsonNode jsonResponse = null;
try {
- jsonResponse = objectMapper.readTree(result.get().genderCount());
- } catch (JsonProcessingException e) {
- throw new RuntimeException(e);
- }
- // Complete the request with JSON response
- return complete(StatusCodes.OK, jsonResponse, JSON_MARSHALLER);
+ return complete(StatusCodes.OK, result.get().genderCount(), JSON_MARSHALLER);This change simplifies the code, improves efficiency by reusing the existing OBJECT_MAPPER, and maintains consistency with other methods in the class.
Committable suggestion was skipped due to low confidence.
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Outside diff range and nitpick comments (8)
JeMPI_Apps/JeMPI_LibMPI/src/main/java/org/jembi/jempi/libmpi/dgraph/LibDgraph.java (1)
90-92: Approve changes and suggest improving method name.The implementation looks correct as it delegates the call to
DgraphQueries.getAllList(allList). However, the method name "getAllList" is somewhat vague.Consider renaming the method to be more specific about what kind of list it's returning. For example, if it's returning a list of all entity IDs, you might name it
getAllEntityIdsorgetAllRecordIds.JeMPI_Apps/JeMPI_LibShared/src/main/java/org/jembi/jempi/shared/models/ApiModels.java (2)
405-413: LGTM! Consider adding JavaDoc comments for clarity.The
CountFieldsrecord is well-structured and consistent with other records in the file. The fields are appropriate for counting and filtering purposes.Consider adding JavaDoc comments to describe the purpose of this record and its fields. For example:
/** * Represents the parameters for counting fields with optional filtering. * * @param fieldName The name of the field to count * @param value List of values to filter by * @param recordType The type of record to count * @param startDate The start date for the count range (inclusive) * @param endDate The end date for the count range (inclusive) */ @JsonInclude(JsonInclude.Include.NON_NULL) public record CountFields( String fieldName, List<String> value, String recordType, String startDate, String endDate ) { }
415-421: LGTM! Consider adding JavaDoc comments and improving field naming.The
SearchAgeCountFieldsrecord is well-structured and consistent with other records in the file. The fields are appropriate for age-related searches with date range filtering.Consider the following improvements:
- Add JavaDoc comments to describe the purpose of this record and its fields.
- Rename the
fieldparameter toageFieldordateOfBirthFieldfor clarity.Example with improvements:
/** * Represents the parameters for searching and counting age-related fields within a date range. * * @param startDate The start date for the search range (inclusive) * @param endDate The end date for the search range (inclusive) * @param ageField The name of the field containing the date of birth or age information */ @JsonInclude(JsonInclude.Include.NON_NULL) public record SearchAgeCountFields( String startDate, String endDate, String ageField ) { }JeMPI_Apps/JeMPI_LibMPI/src/main/java/org/jembi/jempi/libmpi/LibMPI.java (1)
152-165: Overall improvements for new methodsThe new methods added to the
LibMPIclass align well with the PR objectives of creating an API for getting counts of fields and enabling field value searches. However, there are some common improvements that can be made across all three methods:
- Consistent exception handling: Implement
ClientExceptionhandling in all methods to improve error management.- Return type consistency: Ensure that return types are appropriate for the operation (e.g., numeric types for counts).
- Descriptive naming: Improve method names to be more specific about their functionality.
- Parameter types: Consider using more specific types (e.g.,
LocalDatefor date parameters) where applicable.Implementing these suggestions will enhance the robustness and maintainability of the code.
documentation/JeMPI.postman_collection.json (3)
44-63: Request updated for more specific gender counting.The changes to this request allow for more detailed counting based on gender, record type, and date range. The use of POST is appropriate for the complex query parameters.
However, please note that the comments in the JSON body (lines starting with
//) are not valid JSON and may cause issues when sending the request.Consider removing the comments from the JSON body or using a valid JSON format for including additional information.
95-121: New endpoint for retrieving data list added.The new POST request for retrieving a data list based on field and date range is well-structured and includes a helpful description.
However, please note that the comments in the JSON body (lines starting with
//) are not valid JSON and may cause issues when sending the request.Consider removing the comments from the JSON body or using a valid JSON format for including additional information.
Line range hint
1-939: Overall improvements to the API collection with some concerns.This update significantly enhances the API's functionality with new endpoints for counting, data retrieval, and system operations. The modifications to existing endpoints improve their specificity and adherence to RESTful principles.
However, there are two recurring issues that need to be addressed:
Hardcoded localhost URLs: Several new endpoints use hardcoded localhost URLs, which pose security risks and reduce portability. Replace these with environment variables.
Invalid JSON comments: Some request bodies contain comments that are not valid JSON. These should be removed or reformatted to ensure proper functionality.
Addressing these issues will greatly improve the robustness and maintainability of this Postman collection.
JeMPI_Apps/JeMPI_LibAPI/src/main/java/org/jembi/jempi/libapi/BackEnd.java (1)
551-563: Remove commented code and improve error loggingThere's a commented-out line that calls a
calculateAvarageAgemethod. If this functionality is not needed, it should be removed. Also, the error logging can be improved by using placeholders.Apply this diff to remove the commented code and improve error logging:
private Behavior<Event> getAllListHandler(final GetAllListRequest request) { List<String> dobList = new ArrayList<>(); try { dobList = libMPI.getAllList(request.allListRequest); LOGGER.info("dobList size: {}", dobList.size()); - // double allList = calculateAvarageAge(dobList); request.replyTo.tell(new GetAllListResponse(dobList)); } catch (Exception e) { LOGGER.error(e.getLocalizedMessage(), e); - LOGGER.error("libMPI.getAllList failed for allListRequest: {} with error: {}", request.allListRequest, e.getMessage()); + LOGGER.error("libMPI.getAllList failed with error: {}", e.getMessage()); } return Behaviors.same(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
- JeMPI_Apps/JeMPI_LibAPI/src/main/java/org/jembi/jempi/libapi/BackEnd.java (4 hunks)
- JeMPI_Apps/JeMPI_LibMPI/src/main/java/org/jembi/jempi/libmpi/LibMPI.java (1 hunks)
- JeMPI_Apps/JeMPI_LibMPI/src/main/java/org/jembi/jempi/libmpi/LibMPIClientInterface.java (1 hunks)
- JeMPI_Apps/JeMPI_LibMPI/src/main/java/org/jembi/jempi/libmpi/dgraph/DgraphQueries.java (4 hunks)
- JeMPI_Apps/JeMPI_LibMPI/src/main/java/org/jembi/jempi/libmpi/dgraph/LibDgraph.java (1 hunks)
- JeMPI_Apps/JeMPI_LibShared/src/main/java/org/jembi/jempi/shared/models/ApiModels.java (2 hunks)
- documentation/JeMPI.postman_collection.json (9 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- JeMPI_Apps/JeMPI_LibMPI/src/main/java/org/jembi/jempi/libmpi/LibMPIClientInterface.java
🔇 Additional comments (13)
JeMPI_Apps/JeMPI_LibMPI/src/main/java/org/jembi/jempi/libmpi/dgraph/LibDgraph.java (3)
82-84: Approve changes and request clarification on return type.The implementation looks correct as it delegates the call to
DgraphQueries.getFieldCount(countFields). However, the return typeStringis unusual for a count method.Could you please clarify:
- Why is the return type
Stringinstead of a numeric type likelongorint?- What format does the returned String have? Is it a JSON string or a formatted count?
86-88: LGTM! Appropriate implementation for age group count.The implementation looks correct. It delegates the call to
DgraphQueries.getAgeGroupCount(searchAgeCountFields)and returns along, which is appropriate for a count method.
82-92: Summary: New API methods align well with PR objectives.The three new methods (
getFieldCount,getAgeGroupCount, andgetAllList) have been successfully added to theLibDgraphclass. These additions align well with the PR objective of creating an API for getting counts of any fields and enabling field value searches.The implementations are consistent with the existing code style, delegating to
DgraphQueriesmethods. This approach maintains the separation of concerns between the API layer (LibDgraph) and the query execution layer (DgraphQueries).These changes effectively expand the querying capabilities of the API, allowing for more flexible and detailed data retrieval as intended.
documentation/JeMPI.postman_collection.json (4)
9-25: New endpoint for counting golden records added.The new GET request for counting golden records is well-structured and consistent with other count requests in the collection.
29-29: Method changed to GET for counting interactions.Changing the method from POST to GET for counting interactions aligns with RESTful principles for read operations and maintains consistency with other count requests.
70-94: Request updated for age group counting.The changes to this request allow for more detailed counting based on age groups and date range. The use of POST is appropriate for the query parameters.
286-392:⚠️ Potential issueNew backup and restore functionality added, but with security concerns.
The addition of backup and restore functionality is a valuable feature. However, the use of hardcoded localhost URLs in these requests is a security concern and reduces the portability of the collection.
Please replace the hardcoded localhost URLs with environment variables, similar to the {{LocalApi}} variable used in other requests. This will enhance security and flexibility.
This issue was previously identified in a past review. Please refer to the existing comment about hardcoded localhost URLs for more details.
JeMPI_Apps/JeMPI_LibAPI/src/main/java/org/jembi/jempi/libapi/BackEnd.java (2)
690-701: New record definitions look goodThe new record definitions for GetFieldCountRequest, GetFieldCountResponse, GetAgeGroupCountRequest, GetAgeGroupCountResponse, GetAllListRequest, and GetAllListResponse are well-structured and consistent with the existing code style.
Line range hint
1-701: Summary of changes and suggestionsThe new functionality for field counts, age group counts, and retrieving all records has been implemented correctly. However, there are a few areas that could be improved:
- Error messages and logging in the new handler methods need to be updated to reflect the correct method names and use proper placeholders.
- The
calculateAvarageAgemethod has a typo in its name and could benefit from using a constant for the date format.- Some minor code cleanup is suggested, such as removing commented-out code.
Overall, the changes are well-structured and consistent with the existing codebase. Addressing the suggested improvements will enhance the code's maintainability and error handling.
JeMPI_Apps/JeMPI_LibMPI/src/main/java/org/jembi/jempi/libmpi/dgraph/DgraphQueries.java (4)
19-21: LGTM: New imports added for JSON parsingThe new imports from the Jackson library are appropriate for the JSON parsing functionality introduced in the new methods.
428-435: Skipping comment ongetFieldsCountmethodThis issue has already been addressed in a previous review comment.
483-545: Skipping comments ongetFieldCountmethodThe issues related to potential SQL injection and unreachable code have already been addressed in previous review comments.
1286-1338: Skipping comments ongetAllListandparseDobFromResponsemethodsThe issues related to potential SQL injection, error handling, and null checks have already been addressed in previous review comments.
| @@ -426,4 +444,8 @@ public record ApiOffsetSearch( | |||
| Boolean sortAsc) { | |||
| } | |||
|
|
|||
| public record AllList(String field, String startDate, String endDate) { } | |||
There was a problem hiding this comment.
Improve consistency, clarity, and documentation for AllList record.
The AllList record structure can be improved for better consistency with other records in this file and enhanced clarity.
Consider the following improvements:
- Add the
@JsonInclude(JsonInclude.Include.NON_NULL)annotation for consistency. - Rename the record to better reflect its purpose, e.g.,
FieldListWithDateRange. - Expand the record to a multi-line format for better readability.
- Add JavaDoc comments to describe the purpose of this record and its fields.
Here's an example of how it could look:
/**
* Represents parameters for listing all entries of a field within a date range.
*
* @param field The name of the field to list
* @param startDate The start date of the range (inclusive)
* @param endDate The end date of the range (inclusive)
*/
@JsonInclude(JsonInclude.Include.NON_NULL)
public record FieldListWithDateRange(
String field,
String startDate,
String endDate
) { }| @@ -426,4 +444,8 @@ public record ApiOffsetSearch( | |||
| Boolean sortAsc) { | |||
| } | |||
|
|
|||
| public record AllList(String field, String startDate, String endDate) { } | |||
|
|
|||
| public record AverageAgeResponse(double averageAge) { } | |||
There was a problem hiding this comment.
Enhance AverageAgeResponse for consistency and optimization.
The AverageAgeResponse record can be improved for better consistency with other records in this file and potential optimization.
Consider the following improvements:
- Add the
@JsonInclude(JsonInclude.Include.NON_NULL)annotation for consistency. - Expand the record to a multi-line format for better readability.
- Consider using
floatinstead ofdoublefor theaverageAgefield, as it provides sufficient precision for age calculations while using less memory. - Add JavaDoc comments to describe the purpose of this record and its field.
Here's an example of how it could look:
/**
* Represents the response containing the average age.
*
* @param averageAge The calculated average age
*/
@JsonInclude(JsonInclude.Include.NON_NULL)
public record AverageAgeResponse(
float averageAge
) { }| public List<String> getAllList(final ApiModels.AllList allListRequest) { | ||
| client.connect(); | ||
| return client.getAllList(allListRequest); | ||
| } |
There was a problem hiding this comment.
Improve method naming and add exception handling
The current method name "getAllList" is somewhat redundant and not very descriptive. Additionally, the method lacks exception handling for potential errors from the client call.
Here are the suggested improvements:
- Rename the method to be more specific (e.g.,
getAllItemsListor a more descriptive name based on what it actually retrieves). - Add exception handling.
Here's a suggested implementation:
-public List<String> getAllList(final ApiModels.AllList allListRequest) {
+public List<String> getAllItemsList(final ApiModels.AllList allListRequest) throws ClientException {
client.connect();
- return client.getAllList(allListRequest);
+ try {
+ return client.getAllList(allListRequest);
+ } catch (Exception e) {
+ throw new ClientException("Error retrieving all items list", e);
+ }
}This change assumes that ClientException is a custom exception class that you'd need to define. Also, consider updating the client method name if you change this method name.
📝 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 List<String> getAllList(final ApiModels.AllList allListRequest) { | |
| client.connect(); | |
| return client.getAllList(allListRequest); | |
| } | |
| public List<String> getAllItemsList(final ApiModels.AllList allListRequest) throws ClientException { | |
| client.connect(); | |
| try { | |
| return client.getAllList(allListRequest); | |
| } catch (Exception e) { | |
| throw new ClientException("Error retrieving all items list", e); | |
| } | |
| } |
| public long getAgeGroupCount(final ApiModels.SearchAgeCountFields searchAgeCountFields) { | ||
| client.connect(); | ||
| return client.getAgeGroupCount(searchAgeCountFields); | ||
| } |
There was a problem hiding this comment.
Add exception handling and consider using more specific date types
While the return type is appropriate for a count operation, the method lacks exception handling. Additionally, consider using more specific date types if the SearchAgeCountFields includes date parameters.
Consider the following improvements:
- Add exception handling.
- If applicable, use
java.time.LocalDateinstead ofStringfor date parameters inSearchAgeCountFields.
Here's a suggested implementation:
-public long getAgeGroupCount(final ApiModels.SearchAgeCountFields searchAgeCountFields) {
+public long getAgeGroupCount(final ApiModels.SearchAgeCountFields searchAgeCountFields) throws ClientException {
client.connect();
- return client.getAgeGroupCount(searchAgeCountFields);
+ try {
+ return client.getAgeGroupCount(searchAgeCountFields);
+ } catch (Exception e) {
+ throw new ClientException("Error getting age group count", e);
+ }
}This change assumes that ClientException is a custom exception class that you'd need to define.
Committable suggestion was skipped due to low confidence.
| public String getFieldCount(final ApiModels.CountFields countFields) { | ||
| client.connect(); | ||
| return client.getFieldCount(countFields); | ||
| } |
There was a problem hiding this comment.
Consider revising the return type and adding exception handling
The getFieldCount method could be improved in the following ways:
- Change the return type from
Stringto a numeric type likelongorint, which is more appropriate for a count operation. - Add exception handling to manage potential errors from the client method call.
Here's a suggested improvement:
-public String getFieldCount(final ApiModels.CountFields countFields) {
+public long getFieldCount(final ApiModels.CountFields countFields) throws ClientException {
client.connect();
- return client.getFieldCount(countFields);
+ try {
+ return Long.parseLong(client.getFieldCount(countFields));
+ } catch (Exception e) {
+ throw new ClientException("Error getting field count", e);
+ }
}This change assumes that ClientException is a custom exception class that you'd need to define. Also, ensure that the client method returns a string that can be parsed into a long.
Committable suggestion was skipped due to low confidence.
| }, | ||
| { | ||
| "name": "Increase Lease", | ||
| "request": { | ||
| "method": "GET", | ||
| "header": [], | ||
| "url": { | ||
| "raw": "http://localhost:6080/assign?what=uids&num=50000000000000", | ||
| "protocol": "http", | ||
| "host": [ | ||
| "localhost" | ||
| ], | ||
| "port": "6080", | ||
| "path": [ | ||
| "assign" | ||
| ], | ||
| "query": [ | ||
| { | ||
| "key": "what", | ||
| "value": "uids" | ||
| }, | ||
| { | ||
| "key": "num", | ||
| "value": "50000000000000" | ||
| } | ||
| ] | ||
| } | ||
| }, | ||
| "response": [] |
There was a problem hiding this comment.
New "Increase Lease" functionality added, but with security concerns.
The addition of the "Increase Lease" functionality might be important for the system. However, the use of a hardcoded localhost URL in this request is a security concern and reduces the portability of the collection.
Please replace the hardcoded localhost URL with an environment variable, similar to the {{LocalApi}} variable used in other requests. This will enhance security and flexibility.
| private Behavior<Event> getFieldCountHandler(final GetFieldCountRequest request) { | ||
| String getCount = null; | ||
| try { | ||
| getCount = libMPI.getFieldCount(request.countFields); | ||
| request.replyTo.tell(new GetFieldCountResponse(getCount)); | ||
| } catch (Exception e) { | ||
| LOGGER.error(e.getLocalizedMessage(), e); | ||
| LOGGER.error("libMPI.findExpandedGoldenRecord failed for goldenId: {} with error: {}", e.getMessage()); | ||
| } | ||
| return Behaviors.same(); | ||
| } |
There was a problem hiding this comment.
Fix error message and improve error logging
The error message in the catch block is incorrect. It mentions "findExpandedGoldenRecord" which is not related to this method. Also, the error logging can be improved by using placeholders.
Apply this diff to fix the error message and improve logging:
private Behavior<Event> getFieldCountHandler(final GetFieldCountRequest request) {
String getCount = null;
try {
getCount = libMPI.getFieldCount(request.countFields);
request.replyTo.tell(new GetFieldCountResponse(getCount));
} catch (Exception e) {
LOGGER.error(e.getLocalizedMessage(), e);
- LOGGER.error("libMPI.findExpandedGoldenRecord failed for goldenId: {} with error: {}", e.getMessage());
+ LOGGER.error("libMPI.getFieldCount failed with error: {}", e.getMessage());
}
return Behaviors.same();
}📝 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 Behavior<Event> getFieldCountHandler(final GetFieldCountRequest request) { | |
| String getCount = null; | |
| try { | |
| getCount = libMPI.getFieldCount(request.countFields); | |
| request.replyTo.tell(new GetFieldCountResponse(getCount)); | |
| } catch (Exception e) { | |
| LOGGER.error(e.getLocalizedMessage(), e); | |
| LOGGER.error("libMPI.findExpandedGoldenRecord failed for goldenId: {} with error: {}", e.getMessage()); | |
| } | |
| return Behaviors.same(); | |
| } | |
| private Behavior<Event> getFieldCountHandler(final GetFieldCountRequest request) { | |
| String getCount = null; | |
| try { | |
| getCount = libMPI.getFieldCount(request.countFields); | |
| request.replyTo.tell(new GetFieldCountResponse(getCount)); | |
| } catch (Exception e) { | |
| LOGGER.error(e.getLocalizedMessage(), e); | |
| LOGGER.error("libMPI.getFieldCount failed with error: {}", e.getMessage()); | |
| } | |
| return Behaviors.same(); | |
| } |
| private Behavior<Event> getAgeGroupCountHandler(final GetAgeGroupCountRequest request) { | ||
| long getCount = 0; | ||
| try { | ||
| String startDob = request.searchAgeCountFields.startDate(); | ||
| String endDob = request.searchAgeCountFields.endDate(); | ||
| LOGGER.info("startDob: {}, endDob: {}", startDob, endDob); | ||
| getCount = libMPI.getAgeGroupCount(request.searchAgeCountFields); | ||
| request.replyTo.tell(new GetAgeGroupCountResponse(getCount)); | ||
| } catch (Exception e) { | ||
| LOGGER.error(e.getLocalizedMessage(), e); | ||
| LOGGER.error("libMPI.findExpandedGoldenRecord failed for goldenId: {} with error: {}", e.getMessage()); | ||
| } | ||
| return Behaviors.same(); | ||
| } |
There was a problem hiding this comment.
Fix error message and improve error logging
The error message in the catch block is incorrect. It mentions "findExpandedGoldenRecord" which is not related to this method. Also, the error logging can be improved by using placeholders.
Apply this diff to fix the error message and improve logging:
private Behavior<Event> getAgeGroupCountHandler(final GetAgeGroupCountRequest request) {
long getCount = 0;
try {
String startDob = request.searchAgeCountFields.startDate();
String endDob = request.searchAgeCountFields.endDate();
LOGGER.info("startDob: {}, endDob: {}", startDob, endDob);
getCount = libMPI.getAgeGroupCount(request.searchAgeCountFields);
request.replyTo.tell(new GetAgeGroupCountResponse(getCount));
} catch (Exception e) {
LOGGER.error(e.getLocalizedMessage(), e);
- LOGGER.error("libMPI.findExpandedGoldenRecord failed for goldenId: {} with error: {}", e.getMessage());
+ LOGGER.error("libMPI.getAgeGroupCount failed with error: {}", e.getMessage());
}
return Behaviors.same();
}📝 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 Behavior<Event> getAgeGroupCountHandler(final GetAgeGroupCountRequest request) { | |
| long getCount = 0; | |
| try { | |
| String startDob = request.searchAgeCountFields.startDate(); | |
| String endDob = request.searchAgeCountFields.endDate(); | |
| LOGGER.info("startDob: {}, endDob: {}", startDob, endDob); | |
| getCount = libMPI.getAgeGroupCount(request.searchAgeCountFields); | |
| request.replyTo.tell(new GetAgeGroupCountResponse(getCount)); | |
| } catch (Exception e) { | |
| LOGGER.error(e.getLocalizedMessage(), e); | |
| LOGGER.error("libMPI.findExpandedGoldenRecord failed for goldenId: {} with error: {}", e.getMessage()); | |
| } | |
| return Behaviors.same(); | |
| } | |
| private Behavior<Event> getAgeGroupCountHandler(final GetAgeGroupCountRequest request) { | |
| long getCount = 0; | |
| try { | |
| String startDob = request.searchAgeCountFields.startDate(); | |
| String endDob = request.searchAgeCountFields.endDate(); | |
| LOGGER.info("startDob: {}, endDob: {}", startDob, endDob); | |
| getCount = libMPI.getAgeGroupCount(request.searchAgeCountFields); | |
| request.replyTo.tell(new GetAgeGroupCountResponse(getCount)); | |
| } catch (Exception e) { | |
| LOGGER.error(e.getLocalizedMessage(), e); | |
| LOGGER.error("libMPI.getAgeGroupCount failed with error: {}", e.getMessage()); | |
| } | |
| return Behaviors.same(); | |
| } |
| public static double calculateAvarageAge(final List<String> dobList) { | ||
| LocalDate today = LocalDate.now(); // Get today's date | ||
| DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyyMMdd"); // DOB format in YYYYMMDD | ||
| int totalAge = 0; | ||
| int count = 0; | ||
| // Iterate through the list of DOBs and calculate the age for each | ||
| for (String dob : dobList) { | ||
| if (!dob.isEmpty()) { | ||
| try { | ||
| LocalDate birthDate = LocalDate.parse(dob, formatter); // Try to convert DOB to LocalDate | ||
| int age = Period.between(birthDate, today).getYears(); // Calculate age in years | ||
| totalAge += age; | ||
| count++; | ||
| } catch (DateTimeParseException e) { | ||
| LOGGER.error("Invalid date format for dob: " + dob + ". Skipping this record."); | ||
| } | ||
| } | ||
| } | ||
| // Calculate and return average age | ||
| return count > 0 ? (double) totalAge / count : 0; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve method name, use constant for date format, and enhance error logging
There are several improvements that can be made to this method:
- The method name has a typo: "Avarage" should be "Average".
- The date format string should be a constant.
- Error logging for invalid date formats can be improved.
Apply this diff to implement these improvements:
- public static double calculateAvarageAge(final List<String> dobList) {
+ private static final DateTimeFormatter DOB_FORMATTER = DateTimeFormatter.ofPattern("yyyyMMdd");
+
+ public static double calculateAverageAge(final List<String> dobList) {
LocalDate today = LocalDate.now(); // Get today's date
- DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyyMMdd"); // DOB format in YYYYMMDD
int totalAge = 0;
int count = 0;
// Iterate through the list of DOBs and calculate the age for each
for (String dob : dobList) {
if (!dob.isEmpty()) {
try {
- LocalDate birthDate = LocalDate.parse(dob, formatter); // Try to convert DOB to LocalDate
+ LocalDate birthDate = LocalDate.parse(dob, DOB_FORMATTER); // Try to convert DOB to LocalDate
int age = Period.between(birthDate, today).getYears(); // Calculate age in years
totalAge += age;
count++;
} catch (DateTimeParseException e) {
- LOGGER.error("Invalid date format for dob: " + dob + ". Skipping this record.");
+ LOGGER.error("Invalid date format for dob: {}. Skipping this record.", dob);
}
}
}
// Calculate and return average age
return count > 0 ? (double) totalAge / count : 0;
}📝 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 static double calculateAvarageAge(final List<String> dobList) { | |
| LocalDate today = LocalDate.now(); // Get today's date | |
| DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyyMMdd"); // DOB format in YYYYMMDD | |
| int totalAge = 0; | |
| int count = 0; | |
| // Iterate through the list of DOBs and calculate the age for each | |
| for (String dob : dobList) { | |
| if (!dob.isEmpty()) { | |
| try { | |
| LocalDate birthDate = LocalDate.parse(dob, formatter); // Try to convert DOB to LocalDate | |
| int age = Period.between(birthDate, today).getYears(); // Calculate age in years | |
| totalAge += age; | |
| count++; | |
| } catch (DateTimeParseException e) { | |
| LOGGER.error("Invalid date format for dob: " + dob + ". Skipping this record."); | |
| } | |
| } | |
| } | |
| // Calculate and return average age | |
| return count > 0 ? (double) totalAge / count : 0; | |
| } | |
| private static final DateTimeFormatter DOB_FORMATTER = DateTimeFormatter.ofPattern("yyyyMMdd"); | |
| public static double calculateAverageAge(final List<String> dobList) { | |
| LocalDate today = LocalDate.now(); // Get today's date | |
| int totalAge = 0; | |
| int count = 0; | |
| // Iterate through the list of DOBs and calculate the age for each | |
| for (String dob : dobList) { | |
| if (!dob.isEmpty()) { | |
| try { | |
| LocalDate birthDate = LocalDate.parse(dob, DOB_FORMATTER); // Try to convert DOB to LocalDate | |
| int age = Period.between(birthDate, today).getYears(); // Calculate age in years | |
| totalAge += age; | |
| count++; | |
| } catch (DateTimeParseException e) { | |
| LOGGER.error("Invalid date format for dob: {}. Skipping this record.", dob); | |
| } | |
| } | |
| } | |
| // Calculate and return average age | |
| return count > 0 ? (double) totalAge / count : 0; | |
| } |
| public static long getAgeGroupCount(final ApiModels.SearchAgeCountFields searchAgeCountFields) { | ||
| // Constructing the query string | ||
| String field = searchAgeCountFields.field(); | ||
| String startDate = searchAgeCountFields.startDate(); // Empty means no start date | ||
| String endDate = searchAgeCountFields.endDate(); | ||
| String query = String.format(Locale.ROOT, | ||
| """ | ||
| { | ||
| list(func: has(GoldenRecord.%s)) @filter(ge(GoldenRecord.%s, "%s") AND le(GoldenRecord.%s, "%s")) { | ||
| count(uid) | ||
| } | ||
| } | ||
| """, | ||
| field, field, startDate, field, endDate); // Order of %s matches the date range | ||
| try { | ||
| LOGGER.info("Query: {}", query); | ||
| return getCount(query); | ||
| } catch (Exception e) { | ||
| LOGGER.error("Error executing age group count query", e); | ||
| return 0L; // Return default count in case of error | ||
| } | ||
| } |
There was a problem hiding this comment.
Improve security and error handling in getAgeGroupCount
-
The method is vulnerable to SQL injection attacks. Consider using parameterized queries or sanitizing inputs before using them in the query string.
-
The error handling could be improved. Instead of returning a default value (0L) in case of an error, consider throwing a custom exception or returning an
Optional<Long>to indicate the possibility of failure.
Here's a suggested improvement for error handling:
public static long getAgeGroupCount(final ApiModels.SearchAgeCountFields searchAgeCountFields) {
// ... (existing code)
try {
LOGGER.info("Query: {}", query);
return getCount(query);
} catch (Exception e) {
LOGGER.error("Error executing age group count query", e);
- return 0L; // Return default count in case of error
+ throw new RuntimeException("Failed to execute age group count query", e);
}
}Would you like assistance in implementing parameterized queries to mitigate the SQL injection risk?
📝 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 static long getAgeGroupCount(final ApiModels.SearchAgeCountFields searchAgeCountFields) { | |
| // Constructing the query string | |
| String field = searchAgeCountFields.field(); | |
| String startDate = searchAgeCountFields.startDate(); // Empty means no start date | |
| String endDate = searchAgeCountFields.endDate(); | |
| String query = String.format(Locale.ROOT, | |
| """ | |
| { | |
| list(func: has(GoldenRecord.%s)) @filter(ge(GoldenRecord.%s, "%s") AND le(GoldenRecord.%s, "%s")) { | |
| count(uid) | |
| } | |
| } | |
| """, | |
| field, field, startDate, field, endDate); // Order of %s matches the date range | |
| try { | |
| LOGGER.info("Query: {}", query); | |
| return getCount(query); | |
| } catch (Exception e) { | |
| LOGGER.error("Error executing age group count query", e); | |
| return 0L; // Return default count in case of error | |
| } | |
| } | |
| public static long getAgeGroupCount(final ApiModels.SearchAgeCountFields searchAgeCountFields) { | |
| // Constructing the query string | |
| String field = searchAgeCountFields.field(); | |
| String startDate = searchAgeCountFields.startDate(); // Empty means no start date | |
| String endDate = searchAgeCountFields.endDate(); | |
| String query = String.format(Locale.ROOT, | |
| """ | |
| { | |
| list(func: has(GoldenRecord.%s)) @filter(ge(GoldenRecord.%s, "%s") AND le(GoldenRecord.%s, "%s")) { | |
| count(uid) | |
| } | |
| } | |
| """, | |
| field, field, startDate, field, endDate); // Order of %s matches the date range | |
| try { | |
| LOGGER.info("Query: {}", query); | |
| return getCount(query); | |
| } catch (Exception e) { | |
| LOGGER.error("Error executing age group count query", e); | |
| throw new RuntimeException("Failed to execute age group count query", e); | |
| } | |
| } |
Summary by CodeRabbit
Release Notes
New Features
Documentation
Bug Fixes