-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Created API for get count of any fields and with field value search #328
base: dev
Are you sure you want to change the base?
Changes from all commits
7bf08ad
1318fd3
9621f5f
8c75c9a
5f8e409
d64123f
d8508b3
826aafd
c76e41f
532fc8c
d12ed69
41688ae
eda31f6
d0a0b0b
ac8cbfc
eafa50b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -29,7 +29,11 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import java.sql.SQLException; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import java.sql.Timestamp; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import java.time.LocalDateTime; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import java.time.Period; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import java.util.*; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import java.time.LocalDate; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import java.time.format.DateTimeFormatter; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import java.time.format.DateTimeParseException; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public final class BackEnd extends AbstractBehavior<BackEnd.Event> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -165,6 +169,9 @@ public Receive<Event> actor() { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.onMessage(PostUploadCsvFileRequest.class, this::postUploadCsvFileHandler) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.onMessage(SQLDashboardDataRequest.class, this::getSqlDashboardDataHandler) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.onMessage(GetConfigurationRequest.class, this::getConfigurationHandler) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.onMessage(GetFieldCountRequest.class, this::getFieldCountHandler) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.onMessage(GetAgeGroupCountRequest.class, this::getAgeGroupCountHandler) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.onMessage(GetAllListRequest.class, this::getAllListHandler) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.onMessage(PostConfigurationRequest.class, this::postConfigurationHandler) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.onMessage(GetFieldsConfigurationRequest.class, this::getFieldsConfigurationHandler) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.build(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -514,6 +521,69 @@ private Behavior<Event> getConfigurationHandler(final GetConfigurationRequest re | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.findExpandedGoldenRecord failed for goldenId: {} with error: {}", e.getMessage()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return Behaviors.same(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+524
to
+534
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+545
to
+546
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Incorrect error message and mismatched logging arguments in 'getAgeGroupCountHandler' The error message references Apply this diff to fix the error message and placeholders: - LOGGER.error("libMPI.findExpandedGoldenRecord failed for goldenId: {} with error: {}", e.getMessage());
+ LOGGER.error("libMPI.getAgeGroupCount failed with error: {}", e.getMessage()); 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return Behaviors.same(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+536
to
+549
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add null check for 'dob' before calling 'isEmpty()' There's a potential for a Apply this diff: - if (!dob.isEmpty()) {
+ if (dob != null && !dob.isEmpty()) { 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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."); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ 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
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Calculate and return average age | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return count > 0 ? (double) totalAge / count : 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+565
to
+585
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ 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:
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
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
private Behavior<Event> getFieldsConfigurationHandler(final GetFieldsConfigurationRequest request) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
final var separator = FileSystems.getDefault().getSeparator(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
final String configDir = System.getenv("SYSTEM_CONFIG_DIRS"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -617,6 +687,18 @@ public record GetConfigurationRequest(ActorRef<GetConfigurationResponse> replyTo | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public record GetConfigurationResponse(Configuration configuration) implements EventResponse { } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public record GetFieldCountRequest(ActorRef<GetFieldCountResponse> replyTo, ApiModels.CountFields countFields) implements Event { } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public record GetFieldCountResponse(String genderCount) implements EventResponse { } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public record GetAgeGroupCountRequest(ActorRef<GetAgeGroupCountResponse> replyTo, ApiModels.SearchAgeCountFields searchAgeCountFields) implements Event { } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public record GetAgeGroupCountResponse(long ageGroupCount) implements EventResponse { } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public record GetAllListRequest(ActorRef<GetAllListResponse> replyTo, ApiModels.AllList allListRequest) implements Event { } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public record GetAllListResponse(List<String> records) implements EventResponse { } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public record GetFieldsConfigurationRequest(ActorRef<GetFieldsConfigurationResponse> replyTo) implements Event { } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public record GetFieldsConfigurationResponse(List<FieldsConfiguration.Field> fields) implements EventResponse { } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -7,6 +7,8 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import akka.http.javadsl.marshalling.Marshaller; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import akka.http.javadsl.model.*; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import akka.http.javadsl.server.Route; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import com.fasterxml.jackson.databind.JsonNode; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import com.fasterxml.jackson.databind.ObjectMapper; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import org.apache.logging.log4j.LogManager; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import org.apache.logging.log4j.Logger; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import org.jembi.jempi.libmpi.MpiServiceError; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -548,6 +550,53 @@ public static Route getConfiguration( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public static Route getFieldCount( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
final ActorSystem<Void> actorSystem, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
final ActorRef<BackEnd.Event> backEnd) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return entity(Jackson.unmarshaller(ApiModels.CountFields.class), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
request -> onComplete(Ask.getFieldCount(actorSystem, backEnd, request), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
result -> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (!result.isSuccess()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return handleError(result.failed().get()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ObjectMapper objectMapper = new ObjectMapper(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
JsonNode jsonResponse = null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
jsonResponse = objectMapper.readTree(result.get().genderCount()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} catch (JsonProcessingException e) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
throw new RuntimeException(e); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+565
to
+568
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle Throwing a Apply this diff to handle the exception properly: } catch (JsonProcessingException e) {
- throw new RuntimeException(e);
+ LOGGER.error("Failed to parse JSON response in getFieldCount", e);
+ return handleError(e);
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Complete the request with JSON response | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return complete(StatusCodes.OK, jsonResponse, JSON_MARSHALLER); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+562
to
+570
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Optimize ObjectMapper usage and improve exception handling
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
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
})); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+553
to
+571
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Reuse existing In the Apply this diff to reuse the existing - ObjectMapper objectMapper = new ObjectMapper();
JsonNode jsonResponse = null;
try {
- jsonResponse = objectMapper.readTree(result.get().genderCount());
+ jsonResponse = OBJECT_MAPPER.readTree(result.get().genderCount());
} catch (JsonProcessingException e) {
throw new RuntimeException(e);
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public static Route getAgeGroupCount( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
final ActorSystem<Void> actorSystem, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
final ActorRef<BackEnd.Event> backEnd) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return entity(Jackson.unmarshaller(ApiModels.SearchAgeCountFields.class), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
request -> onComplete(Ask.getAgeGroupCount(actorSystem, backEnd, request), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
result -> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (!result.isSuccess()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return handleError(result.failed().get()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return complete(StatusCodes.OK, result.get().ageGroupCount(), JSON_MARSHALLER); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
})); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+574
to
+585
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure consistent exception handling across methods In the Consider adding exception handling similar to return entity(Jackson.unmarshaller(ApiModels.SearchAgeCountFields.class),
request -> onComplete(Ask.getAgeGroupCount(actorSystem, backEnd, request),
result -> {
if (!result.isSuccess()) {
return handleError(result.failed().get());
}
try {
return complete(StatusCodes.OK, result.get().ageGroupCount(), JSON_MARSHALLER);
} catch (Exception e) {
LOGGER.error("Error processing ageGroupCount response", e);
return handleError(e);
}
})); Alternatively, ensure that any potential exceptions are handled within the called methods or that appropriate error handling is in place. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
private static Route getDataList( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
final ActorSystem<Void> actorSystem, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
final ActorRef<BackEnd.Event> backEnd) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return entity(Jackson.unmarshaller(ApiModels.AllList.class), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
request -> onComplete(Ask.getAllList(actorSystem, backEnd, request), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
result -> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (!result.isSuccess()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return handleError(result.failed().get()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return complete(StatusCodes.OK, result.get(), JSON_MARSHALLER); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
})); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
private static Route getFieldsConfiguration( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
final ActorSystem<Void> actorSystem, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
final ActorRef<BackEnd.Event> backEnd) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -661,6 +710,12 @@ public static Route createCoreAPIRoutes( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
() -> Routes.getNotifications(actorSystem, backEnd)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
path(GlobalConstants.SEGMENT_POST_GOLDEN_RECORD, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
() -> Routes.updateGoldenRecord(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)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+713
to
+718
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider using GET method for retrieval operations The new routes for 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.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
path(GlobalConstants.SEGMENT_POST_FILTER_GIDS_WITH_INTERACTION_COUNT, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
() -> Routes.postFilterGidsWithInteractionCount(actorSystem, backEnd)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
path(GlobalConstants.SEGMENT_PROXY_POST_CR_UPDATE_FIELDS, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -677,7 +732,8 @@ public static Route createCoreAPIRoutes( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
path(GlobalConstants.SEGMENT_GET_CONFIGURATION, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
() -> Routes.getConfiguration(actorSystem, backEnd)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
path(GlobalConstants.SEGMENT_GET_FIELDS_CONFIGURATION, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
() -> Routes.getFieldsConfiguration(actorSystem, backEnd))))); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
() -> Routes.getFieldsConfiguration(actorSystem, backEnd)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
))); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -149,6 +149,21 @@ public Either<MpiGeneralError, List<GoldenRecord>> findGoldenRecords(final List< | |||||||||||||||||||||||||
return Either.right(results.get().data()); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
public String getFieldCount(final ApiModels.CountFields countFields) { | ||||||||||||||||||||||||||
client.connect(); | ||||||||||||||||||||||||||
return client.getFieldCount(countFields); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
Comment on lines
+152
to
+155
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider revising the return type and adding exception handling The
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
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
public long getAgeGroupCount(final ApiModels.SearchAgeCountFields searchAgeCountFields) { | ||||||||||||||||||||||||||
client.connect(); | ||||||||||||||||||||||||||
return client.getAgeGroupCount(searchAgeCountFields); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
Comment on lines
+157
to
+160
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 Consider the following improvements:
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
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
public List<String> getAllList(final ApiModels.AllList allListRequest) { | ||||||||||||||||||||||||||
client.connect(); | ||||||||||||||||||||||||||
return client.getAllList(allListRequest); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
Comment on lines
+162
to
+165
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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:
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 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
public ExpandedGoldenRecord findExpandedGoldenRecord(final String goldenId) { | ||||||||||||||||||||||||||
client.connect(); | ||||||||||||||||||||||||||
final var results = client.findExpandedGoldenRecords(List.of(goldenId)); | ||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider revising the return type and adding documentation for The method
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 |
||
|
||
long getAgeGroupCount(ApiModels.SearchAgeCountFields searchAgeCountFields); | ||
|
||
List<String> getAllList(ApiModels.AllList allListRequest); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Clarify the purpose of The method
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. |
||
|
||
List<String> findGoldenIds(); | ||
|
||
List<String> fetchGoldenIds( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect error message and mismatched logging arguments in 'getFieldCountHandler'
The error message references
libMPI.findExpandedGoldenRecord
and uses two placeholders{}
but only provides one argumente.getMessage()
. This is incorrect for this method and will cause logging issues.Apply this diff to fix the error message and placeholders:
📝 Committable suggestion