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?

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.

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) {
  ...
}