[FIXED] My update function doesn't work : Spring CRUD API

Issue

I'm creating a CRUD API with Spring to manage students. Here is my service:

package com.example.demo.student;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service;

import javax.transaction.Transactional;
import java.util.List;
import java.util.Objects;
import java.util.Optional;

@Service
public class StudentService {

    private final StudentRepository studentRepository;
    @Autowired
    public StudentService(StudentRepository studentRepository) {
        this.studentRepository = studentRepository;
    }
    public List<Student> getStudents(){

        return studentRepository.findAll();
    }

    public void addNewStudent(Student student) {
        Optional<Student> studentByName = studentRepository.findStudentByName(student.getName());
        if(studentByName.isPresent()){
            try {
                throw new Exception("name taken");
            } catch (Exception e) {
                throw new RuntimeException(e);
            }

        }
        studentRepository.save(student);
    }

    public void deleteStudent(Long studentId) {
        boolean exists = studentRepository.existsById(studentId);
        if (!exists){
            throw new RuntimeException("student with id" + studentId + " does not exist.");
        }
        studentRepository.deleteById(studentId);

    }

    @Transactional
    public Student updateStudent(Long studentId, String name) {
        Student student = studentRepository.findById(studentId).orElseThrow(() -> new RuntimeException(
                "student with Id" + studentId + " does not exist"
                )
        );
        if(name != null && name.length() > 0 && !Objects.equals(student.getName(), name)){
            student.setName(name);
            return studentRepository.save(student);
        }
        return student;
    }


    public Optional<Student> getStudent(Long studentId) {
        Student student = studentRepository.findById(studentId).orElseThrow(() -> new RuntimeException(
                        "student with Id" + studentId + " does not exist"
                )
        );
        return studentRepository.findById(studentId);

    }
}

And my controller:

package com.example.demo.student;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.web.bind.annotation.*;

import java.util.List;
import java.util.Optional;

@RestController
@RequestMapping(path="api/student")
public class StudentController {

    private final StudentService studentService;

    @Autowired
    public StudentController(StudentService studentService) {
        this.studentService = studentService;
    }


    @GetMapping
    public List<Student> getStudents(){
        return studentService.getStudents();

    }

    @GetMapping(path="/{studentId}")
    public Optional<Student> getStudent(@PathVariable("studentId")Long studentId){
        return studentService.getStudent(studentId);

    }

    @PostMapping
    public void registerNewStudent(@RequestBody Student student){
        studentService.addNewStudent(student);
    }

    @DeleteMapping(path="{studentId}")
    public void deleteStudent(@PathVariable("studentId")Long studentId){
        studentService.deleteStudent(studentId);
    }

    // only update name
    @PutMapping(path="{studentId}")
    public void updateStudent(
            @PathVariable("studentId")Long studentId,
            @RequestParam(required = false) String name)

    {
        studentService.updateStudent(studentId, name );
    }
}
 

But when I go on postman, and try to update for example student 3 with this url: http://localhost:8080/api/student/3, it doesn't change anything, even if the response is 1.

Can anyone helps me ? Thanks in advance

Solution

[UPDATE: I have just been told that mixing @RequestParam AND @PathVariable is perfectly legit, so I guess I stand corrected. I hadn't seen that before. Use your best judgement.]

I would not mix @RequestParam AND @PathVariable in the same endpoint. Even so, try calling

PUT http://localhost:8080/api/student/3?name=Jones

Does that work?

Or change your controller method signature to

@PutMapping("/{studentId}/{name}")
    public void updateStudent( @PathVariable("studentId")Long studentId, @PathVariable("name") String name)

...and call

PUT http://localhost:8080/api/student/3/Jones

This may be tangental, but I would abstract out an interface IStudentService (or other naming convention) with just the method declarations, no implementations. Then update the class as

public class StudentService implements IStudentService;

Assuming that StudentRepository is a Jpa/CrudRepository interface, then in StudentService all you need is

@Autowired StudentRepository studentRepository; 

You don't need that verbose constructor injection. And in the controller all you need is

@Autowired IStudentService studentService; 

(the name of the interface not the implementation class).

Answered By - Howard007
Answer Checked By - Terry (JavaFixing Volunteer)