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
    Commented Nov 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\$ Commented Nov 30, 2023 at 15:36
  • \$\begingroup\$ @Marvin, it's intentional \$\endgroup\$
    – J.Olufsen
    Commented Dec 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
    Commented Dec 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\$ Commented Dec 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.