generator-jhipster
generator-jhipster copied to clipboard
make id param real optional in PUT/PATCH rest api
Currently, the id parameter in a POST/PUT API (e.g. /api/posts/:id) is claimed not required as in @PathVariable(required = false), but is not really optional, ignoring this parameter will produce a null pointer error.
Code generated by current template:
@PutMapping("/posts/{id}")
public ResponseEntity<Post> updatePost(@PathVariable(value = "id", required = false) final Long id, @Valid @RequestBody Post post)
throws URISyntaxException {
log.debug("REST request to update Post : {}, {}", id, post);
if (post.getId() == null) {
throw new BadRequestAlertException("Invalid id", ENTITY_NAME, "idnull");
}
if (!Objects.equals(id, post.getId())) {
throw new BadRequestAlertException("Invalid ID", ENTITY_NAME, "idinvalid");
}
if (!postRepository.existsById(id)) {
throw new BadRequestAlertException("Entity not found", ENTITY_NAME, "idnotfound");
}
Post result = postService.save(post);
return ResponseEntity
.ok()
.headers(HeaderUtil.createEntityUpdateAlert(applicationName, true, ENTITY_NAME, post.getId().toString()))
.body(result);
}
This merge request is to allow the id parameter to be really optional. This also benefits those legacy API consumers that do not pass an id param according to ancient versions.
Code generated after the change:
@PutMapping("/posts/{id}")
public ResponseEntity<Post> updatePost(@PathVariable(value = "id", required = false) final Long id, @Valid @RequestBody Post post)
throws URISyntaxException {
log.debug("REST request to update Post : {}, {}", id, post);
if (post.getId() == null) {
throw new BadRequestAlertException("Invalid id", ENTITY_NAME, "idnull");
}
if (id != null && !Objects.equals(id, post.getId())) {
throw new BadRequestAlertException("Invalid ID", ENTITY_NAME, "idinvalid");
}
if (!postRepository.existsById(post.getId())) {
throw new BadRequestAlertException("Entity not found", ENTITY_NAME, "idnotfound");
}
Post result = postService.save(post);
return ResponseEntity
.ok()
.headers(HeaderUtil.createEntityUpdateAlert(applicationName, true, ENTITY_NAME, post.getId().toString()))
.body(result);
}
Please make sure the below checklist is followed for Pull Requests.
- [ ] All continuous integration tests are green
- [ ] Tests are added where necessary
- [ ] The JDL part is updated if necessary
- [ ] jhipster-online is updated if necessary
- [ ] Documentation is added/updated where necessary
- [ ] Coding Rules & Commit Guidelines as per our CONTRIBUTING.md document are followed
When you are still working on the PR, consider converting it to Draft (below reviewers) and adding skip-ci label, you can still see CI build result at your branch.
This api changed at v7 beta cycle, we are about to start v8. IMO we should fix the null pointer problem, but keep the api. Unless there are some compelling reason.
The reason path variable id is nullable is to generate the correct error trough api, if we make sure the error is ok, we should set to required.