Search/Query API cross parameter check

Question

Background

I was working on a coding challenge as part of an interview process. I had to create a REST API where the user can report sensor data and query derived metrics based on several filter criteria.

Ingest

In the sensor data ingest handler I could simply annotate the expected request body entity with validation rules.

@Data
@Builder
public class SensorIngestRequest {
 final static String DATE_FORMAT = "yyyy-MM-dd'T'HH:mm:ss";
 @NotNull(message = "must be provided in the following format: " + DATE_FORMAT)
 @JsonFormat(shape = JsonFormat.Shape.STRING, pattern = DATE_FORMAT, timezone = "GMT")
 Date observedAt;
 @NotNull(message = "must be provided and it should be a hyphened UUID v4")
 UUID deviceId;
 @NotNull(message = "must be provided")
 Integer measuredValue;
 @NotEmpty(message = "must be provided and should not be empty")
 String measureUnit;
}

then I could use @Valid to enforce validation

@PostMapping("/{sensor-name}")
ResponseEntity<?> ingestSensorData(
 @PathVariable(name="sensor-name") String sensorName,
 @Valid @RequestBody SensorIngestRequest ingestRequest) {
 //Compute
 var serviceModel = ingestService.ingest(sensorName, ingestRequest);
 //Generate Response << not important from the question perspective
 //Calculate surrounding days for observed at
 var calendar = Calendar.getInstance();
 calendar.setTime(serviceModel.getObservedAt());
 calendar.set(Calendar.HOUR_OF_DAY, 0);
 var from = calendar.getTime();
 calendar.add(Calendar.DATE, 1);
 var till = calendar.getTime();
 //Generate links << not important from the question perspective
 var linkSingle = createLinkForGetMetric(sensorName, from, till, Optional.of(ingestRequest.getDeviceId()), LinkRelation.of(RELATION_NAME_FOR_SINGLE_DEVICE_METRIC));
 var linkAll = createLinkForGetMetric(sensorName, from, till, Optional.empty(), LinkRelation.of(RELATION_NAME_FOR_ALL_DEVICES_METRIC));
 var responseModel = EntityModel.of(serviceModel, linkSingle, linkAll);
 return ResponseEntity
 .created(responseModel.getRequiredLink(RELATION_NAME_FOR_SINGLE_DEVICE_METRIC).toUri())
 .body(responseModel);
}

If the validation fails then it throws a MethodArgumentNotValidException against which I can define a ControllerAdvice

@ControllerAdvice
public class ValidationExceptionAdvice {
 final static String ERROR_FORMAT = "'%s' parameter %s";
 @ExceptionHandler(MethodArgumentNotValidException.class)
 public ResponseEntity<Problem> handleMANVE(MethodArgumentNotValidException ex) {
 var errors = ex.getBindingResult().getAllErrors().stream()
 .map(error -> String.format(ERROR_FORMAT,((FieldError) error).getField(), error.getDefaultMessage()))
 .sorted() //Make errors' ordering consistent
 .collect(Collectors.toList());
 return getResponseEntity(errors);
 }
 ResponseEntity<Problem> getResponseEntity(List<String> errors) {
 return ResponseEntity
 .status(HttpStatus.BAD_REQUEST)
 .body(Problem.create()
 .withTitle("Invalid data has been provided, please correct it")
 .withDetail(errors.toString()));
 }
}

Query

In the query/search handler I anticipate two dates (a from and a to). These could be used to narrow the date interval against which the metric calculation should be performed. A sample request looks like this

GET /api/v1/sensors/windSpeed/metrics/avg?from=2023年05月22日&till=2023年05月23日

@GetMapping("/{sensor-name}/metrics/{metric-type}")
ResponseEntity<?> getSensorMetricForADateRangeForADevice(
 @PathVariable(name="sensor-name") String sensorName,
 @PathVariable(name="metric-type") String metricType,
 @RequestParam("from") @Valid @DateTimeFormat(pattern="yyyy-MM-dd") Date from,
 @RequestParam("till") @Valid @DateTimeFormat(pattern="yyyy-MM-dd") Date till,
 @RequestParam(name="device-id",required = false) Optional<UUID> deviceId) {
 //Extra input data validation
 if (!"avg".equalsIgnoreCase(metricType)) {
 throw createCVE("metric-type", 1, metricType, String.class, "currently supports only 'avg'");
 }
 if (from.compareTo(till) > 0) {
 throw createCVE("till", 3, till, Date.class,"should be greater than from");
 }
 //Generate Response
 //... << omitted for the sake of brevity
 return ResponseEntity.ok().body(EntityModel.of(serviceModel, linkSingle, linkAll));
}

As you can see I had to compare from and to to make sure that from is smaller than to. I have found pretty hard to throw a hand-crafted MethodArgumentNotValidException. So, I've decided to use ConstraintViolationException instead. Well, that is also not an easy peasy task.

<T> ConstraintViolationException createCVE(
 String parameterName,
 int parameterIndex,
 T parameter,
 Class<T> parameterType,
 String violationMessage) {
 //NOTE: here the method name does not matter
 final var propertyPath = PathImpl.createPathFromString("");
 propertyPath.addParameterNode(parameterName, parameterIndex);
 var constraintViolation = ConstraintViolationImpl.forParameterValidation(
 violationMessage,
 Collections.emptyMap(),
 Collections.emptyMap(),
 violationMessage,
 parameterType,
 parameter,
 null,
 parameter,
 propertyPath,
 null,
 null,
 null);
 return new ConstraintViolationException(Set.of(constraintViolation));
}

Quite frankly I don't like approach either. I had to write another ExceptionHandler as well to create a proper Problem response.

@ExceptionHandler(ConstraintViolationException.class)
public ResponseEntity<Problem> handleCVE(ConstraintViolationException ex) {
 var errors = ex.getConstraintViolations().stream()
 .map(error -> String.format(ERROR_FORMAT, error.getPropertyPath(), error.getMessage()))
 .collect(Collectors.toList());
 return getResponseEntity(errors);
}

Question

• Is there any better way to perform custom cross parameter check for GET request?

• dariosicily: I saw you are using the Date that has been deprecated cause a lot of problems in favour of the LocalDate class , is this required from the interview? Because you are using streams so LocalDate is equally available.
• Peter Csala: @dariosicily Nice catch, thanks. I working on multiple projects with different programming languages (C#, Golang, TypeScript and Java) and sometimes I mix up things. ¯_(ツ)_/¯
• dariosicily: Glad to have helped :) , to simplify the validation code you could use the BindingResult class like here, its behaviour is still the same in the rest controller or mvc controller.
• Peter Csala: @dariosicily How does BindingResult help to perform cross parameter check (from points to an earlier date than till)? According to my understanding this collects the validation errors. Or shall I use the addError with a FieldError if my custom validation fails?
• dariosicily: I suggested BindingResult because you can intercept @NotNull and @DateTimeFormat validation errors and then create a new ResponseEntity(errormessage, HttpStatus.BAD_REQUEST)if BindingResult is not empty.In this case you should create a ResponseEntity for comparing at runtime the two dates with the error after checked BindingResults is empty. Alternatively you can create a new custom annotation for comparing dates like here but it seems me more complicated.

Answer 1

The first thing I noticed is the use of the deprecated Date class instead of the LocalDate class that has been introduced with Java 8. The code presented and the flow of the program are both correct, but some simplification can be introduced with the explicit use of the BindingResult class that is not directly used. Starting from the signature of the search method described in the question:

@GetMapping("/{sensor-name}/metrics/{metric-type}")
ResponseEntity<?> getSensorMetricForADateRangeForADevice(
 @PathVariable(name="sensor-name") String sensorName,
 @PathVariable(name="metric-type") String metricType,
 @RequestParam("from") @Valid @DateTimeFormat(pattern="yyyy-MM-dd") Date from,
 @RequestParam("till") @Valid @DateTimeFormat(pattern="yyyy-MM-dd") Date till) 

With a new helper class containing both the Date from and Date till fields and with the BindingResult class can be rewritten like below:

@Data
public class LocalDateChecker {
 @NotNull
 @DateTimeFormat(pattern="yyyy-MM-dd")
 private LocalDate from;
 @NotNull
 @DateTimeFormat(pattern="yyyy-MM-dd")
 private LocalDate till;
}
@RestController
public class SensorMetricController {
 @GetMapping("/api/v1/sensors/{sensor-name}/metrics/{metric-type}")
 public ResponseEntity getSensorMetricForADateRangeForADevice(
 @PathVariable(name = "sensor-name") String sensorName,
 @PathVariable(name = "metric-type") String metricType,
 @Valid LocalDateChecker localDateChecker,
 BindingResult bindingResult)
}

The advantage is that the bindingResult contains all the validation errors and relative messages erasing the propagation of the validation code outside of the relative controller. A possible way to rewrite the controller is below:

@RestController
public class SensorMetricController {
 @GetMapping("/api/v1/sensors/{sensor-name}/metrics/{metric-type}")
 public ResponseEntity getSensorMetricForADateRangeForADevice(
 @PathVariable(name = "sensor-name") String sensorName,
 @PathVariable(name = "metric-type") String metricType,
 @Valid LocalDateChecker localDateChecker,
 BindingResult bindingResult) {
 
 //I'm writing the first error, but all errors could be reported
 // in one time
 if (bindingResult.hasErrors()) {
 FieldError fieldError = bindingResult.getFieldErrors().get(0);
 throw new MetricException(fieldError.getField() + " " + fieldError.getDefaultMessage());
 }
 if (!"avg".equalsIgnoreCase(metricType)) {
 throw new MetricException("metric type currently supports only 'avg'");
 }
 if (localDateChecker.getFrom().isAfter(localDateChecker.getTill())) {
 throw new MetricException(("till shoud be greater or equal to from"));
 }
 return new ResponseEntity(List.of(
 sensorName,
 metricType,
 localDateChecker.getFrom(),
 localDateChecker.getTill()
 ), HttpStatus.OK);
 }
}

Because the generic user has to understand just which is the absent or malformed field not generating a correct output the only important thing is the propagation of a new generated MetricException exception containing the error message to the ControllerAdvice that will communicate the bad request error to the user:

public class MetricException extends RuntimeException {
 public MetricException(String message) {
 super(message);
 }
}
@ControllerAdvice
public class SensorMetricAdvice {
 @ResponseBody
 @ExceptionHandler(MetricException.class)
 @ResponseStatus(HttpStatus.BAD_REQUEST)
 String metricExceptionHandler(MetricException ex) {
 return ex.getMessage();
 }
}

Substantially all the original code presented in the question is absolutely correct, but with help of spring classes its size can be reduced, the code I wrote it's based on the validating-form-input (adapted to a rest controller) and Building REST services with Spring.

• Peter Csala: Thanks for the detailed review and suggestions. I will try it out tomorrow :)
• dariosicily: @PeterCsala Glad to have helped :)

Answer 2

You could wrap the request parameters in a class and then implement a custom validator that checks the dates.

That way, your controller's code would be reduced to:

@GetMapping("/{sensor-name}/metrics/{metric-type}")
ResponseEntity<?> getSensorMetricForADateRangeForADevice(
 @Valid SensorQuery sensorQuery) {
 ...
}

• Peter Csala: Hey Miguel thanks for the review. Let me check your suggested links.
• Peter Csala: Could you please show me how would you apply your first suggested link to request parameters that are annotated with @PathVariable?