Yana P. #4
Conversation
…lify controller, implement CRUD and summary endpoint
…xception handling
| )); | ||
|
|
||
| public List<AnalyticsRecord> getAll(){ | ||
| return analyticsRecords; |
There was a problem hiding this comment.
This exposes the list so that the caller can mutate the list as a side-effect. Here a unmodifiable view on the list should be returned.
|
|
||
| service.add(newRecord); | ||
|
|
||
| URI location = URI.create("records/" + newTraceId); |
There was a problem hiding this comment.
Is this a relative path? Then relative to POST /records this would resolve to /records/records/{traceId}. It would be simpler to stick to the absolute path /records/{traceId}. Otherwise the path should become ../records/{traceId}.
I think best would be to let Spring generate the path like so:
URI location = ServletUriComponentsBuilder
.fromCurrentRequest()
.path("/{traceId}")
.buildAndExpand(newTraceId)
.toUri();
There was a problem hiding this comment.
This compiled class file should not be committed into Git.
|
|
||
| @ExceptionHandler(Exception.class) | ||
| public ResponseEntity<Map<String, String>> handleGeneralErrors(Exception ex) { | ||
| return ResponseEntity.badRequest().body(Map.of("error", ex.getMessage())); |
There was a problem hiding this comment.
General errors better give a HTTP response code like 500 instead of 400 Bad Request.
|
Nice work! I also noticed the Swagger-UI on http://localhost:8081/swagger-ui/index.html which helps to test the API manually. To make trying out requests even more easy you could add some examples for the input parameters. |
There was a problem hiding this comment.
This wrapper requires a configuration the download the right Maven en Java for building this project.
composix
left a comment
composix
left a comment
There was a problem hiding this comment.
My comments give some minor improvements that are possible. But overal this PR is good to go!
2 participants
This PR completes the core functionality and quality requirements for the Analytics API.
Implemented features include: