1
\$\begingroup\$

I'd like to have code review for backend of todo app.

It has 2 main functionalities:

  1. Authentication and authorization using Spring Security and JWT token.

  2. CRUD for tasks

In particular I'd like to focus on code quality and database design.

https://github.com/redshift-7/todo-app

REST controller for managing tasks:

@Slf4j @RequiredArgsConstructor @CrossOrigin @RestController @RequestMapping("/api") public class TaskController { private final TaskService taskService; @GetMapping("/tasks") public List<Task> all() { log.info("Request to get all tasks for current user"); return taskService.findAll(); } @GetMapping("/task/{id}") public ResponseEntity<Task> one(@PathVariable Long id) { log.info("Request to get tasks with id: {}", id); return taskService.getById(id).map(response -> ResponseEntity.ok().body(response)) .orElse(new ResponseEntity<>(HttpStatus.NOT_FOUND)); } @PostMapping("/tasks") public ResponseEntity<Task> newTask(@Valid @RequestBody Task task) throws URISyntaxException { log.info("Request to save new task item: {}", task); Task result = taskService.save(task); log.info("New task saved with id: {}", result.getId()); return ResponseEntity.created(new URI("/api/task/" + result.getId())).body(result); } @PutMapping("/tasks/{id}") public ResponseEntity<Task> updateTask(@Valid @RequestBody Task newTask, @PathVariable Long id) { log.info("Request to update task with id: {}", id); Optional<Task> result = taskService.update(newTask); return result.map(task -> ResponseEntity.ok().body(task)) .orElseGet(() -> ResponseEntity.notFound().build()); } @DeleteMapping("/tasks/{id}") public ResponseEntity<HttpStatus> deleteTask(@PathVariable Long id) { log.info("Request to delete task with id: {}", id); taskService.delete(id); return ResponseEntity.noContent().build(); } } 
\$\endgroup\$
5
  • 1
    \$\begingroup\$Is the /task/{id} mapping (with singular instead of plural) intentional or a typo?\$\endgroup\$
    – Marvin
    CommentedNov 30, 2023 at 13:29
  • \$\begingroup\$@Marvin it seems intentional as also the newTask method contains the generation of the Location link that is created as part of the 201 Created response that points to /task/{id}.\$\endgroup\$CommentedNov 30, 2023 at 15:36
  • \$\begingroup\$@Marvin, it's intentional\$\endgroup\$
    – J.Olufsen
    CommentedDec 6, 2023 at 10:57
  • 1
    \$\begingroup\$Interesting, as that doesn't match with your logging ("Request to get task_s_ with id") and, more importantly, your PUT or DELETE mappings.\$\endgroup\$
    – Marvin
    CommentedDec 6, 2023 at 11:42
  • 1
    \$\begingroup\$the repetition of "tasks" seems troubling. And the decision about "task without s" makes it typo-unreadable, so we would at the very least need some verbosity to that one character's being there or not\$\endgroup\$CommentedDec 9, 2023 at 15:51

0

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.