-
Notifications
You must be signed in to change notification settings - Fork 3
Facade migration T4/ Migrate Student to Facade pattern #121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
8bbfef0
7d85e69
a212419
c164387
545c212
b653d38
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,33 +1,9 @@ | ||
| package org.unilab.uniplan.student; | ||
|
|
||
| import java.util.List; | ||
| import java.util.UUID; | ||
| import org.springframework.data.jpa.repository.JpaRepository; | ||
| import org.springframework.data.jpa.repository.Query; | ||
| import org.springframework.data.repository.query.Param; | ||
| import org.springframework.stereotype.Repository; | ||
| import org.unilab.uniplan.student.dto.StudentCourseMajorDto; | ||
|
|
||
| @Repository | ||
| public interface StudentRepository extends JpaRepository<Student, UUID> { | ||
| @Query(""" | ||
| select new org.unilab.uniplan.student.dto.StudentCourseMajorDto( | ||
| s.id, s.firstName, s.lastName, s.facultyNumber, | ||
| c.id, c.courseType, c.courseSubtype, c.courseYear, | ||
| m.id, m.majorName | ||
| ) | ||
| from Student s | ||
| join s.course c | ||
| join c.major m | ||
| where (:firstName is null or lower(s.firstName) like lower(concat('%', :firstName, '%'))) | ||
| and (:lastName is null or lower(s.lastName) like lower(concat('%', :lastName, '%'))) | ||
| and (:facultyNumber is null or s.facultyNumber like concat('%', :facultyNumber, '%')) | ||
| and (:majorName is null or lower(m.majorName) like lower(concat('%', :majorName, '%'))) | ||
| """) | ||
| List<StudentCourseMajorDto> searchStudents( | ||
| @Param("firstName") String firstName, | ||
| @Param("lastName") String lastName, | ||
| @Param("facultyNumber") String facultyNumber, | ||
| @Param("majorName") String majorName | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,72 +1,35 @@ | ||
| package org.unilab.uniplan.student; | ||
|
|
||
| import static org.unilab.uniplan.utils.ErrorConstants.STUDENT_NOT_FOUND; | ||
|
|
||
| import jakarta.transaction.Transactional; | ||
| import java.util.List; | ||
| import java.util.Optional; | ||
| import java.util.UUID; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.stereotype.Service; | ||
| import org.unilab.uniplan.exception.ResourceNotFoundException; | ||
| import org.unilab.uniplan.student.dto.StudentCourseMajorDto; | ||
| import org.unilab.uniplan.student.dto.StudentDto; | ||
| import org.unilab.uniplan.common.model.BaseService; | ||
|
|
||
| @Service | ||
| @RequiredArgsConstructor | ||
| public class StudentService { | ||
| public class StudentService implements BaseService<Student> { | ||
|
|
||
| private final StudentRepository studentRepository; | ||
| private final StudentMapper studentMapper; | ||
|
|
||
| @Transactional | ||
| public StudentDto createStudent(final StudentDto studentDTO) { | ||
| final Student student = studentMapper.toEntity(studentDTO); | ||
| return saveEntityAndConvertToDto(student); | ||
| } | ||
|
|
||
| public StudentDto findStudentById(final UUID id) { | ||
| return studentRepository.findById(id) | ||
| .map(studentMapper::toDto) | ||
| .orElseThrow(() -> new ResourceNotFoundException(STUDENT_NOT_FOUND.getMessage( | ||
| String.valueOf(id)))); | ||
| } | ||
|
|
||
| public List<StudentDto> findAll() { | ||
| return studentRepository.findAll() | ||
| .stream().map(studentMapper::toDto).toList(); | ||
| } | ||
|
|
||
| @Transactional | ||
| public StudentDto updateStudent(final UUID id, final StudentDto studentDTO) { | ||
| return studentRepository.findById(id) | ||
| .map(existingStudent -> updateEntityAndConvertToDto( | ||
| studentDTO, | ||
| existingStudent)) | ||
| .orElseThrow(() -> new ResourceNotFoundException(STUDENT_NOT_FOUND.getMessage( | ||
| String.valueOf(id)))); | ||
| } | ||
|
|
||
| @Transactional | ||
| public void deleteStudent(final UUID id) { | ||
| final Student student = studentRepository.findById(id) | ||
| .orElseThrow(() -> new ResourceNotFoundException( | ||
| STUDENT_NOT_FOUND.getMessage(String.valueOf(id)))); | ||
| studentRepository.delete(student); | ||
| @Override | ||
| public void save(final Student entity) { | ||
| studentRepository.save(entity); | ||
| } | ||
|
|
||
| public List<StudentCourseMajorDto> findStudentCourseMajorInfo(final String firstName, final String lastName, | ||
| final String facultyNumber, final String majorName){ | ||
| return studentRepository.searchStudents(firstName, lastName, facultyNumber, majorName); | ||
| @Override | ||
| public List<Student> getAll() { | ||
| return studentRepository.findAll(); | ||
| } | ||
|
|
||
| private StudentDto updateEntityAndConvertToDto(final StudentDto dto, | ||
| final Student entity) { | ||
| studentMapper.updateEntityFromDto(dto, entity); | ||
| return saveEntityAndConvertToDto(entity); | ||
| @Override | ||
| public Optional<Student> getById(final UUID id) { | ||
| return studentRepository.findById(id); | ||
| } | ||
|
|
||
| private StudentDto saveEntityAndConvertToDto(final Student entity) { | ||
| final Student savedEntity = studentRepository.save(entity); | ||
| return studentMapper.toDto(savedEntity); | ||
| @Override | ||
| public void delete(final Student entity) { | ||
| studentRepository.delete(entity); | ||
| } | ||
| } |
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't StudentValidator validate the StudentRequestDto instead of the Student entity? |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| package org.unilab.uniplan.student; | ||
|
|
||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.stereotype.Component; | ||
| import org.unilab.uniplan.course.CourseRepository; | ||
| import org.unilab.uniplan.exception.ResourceNotFoundException; | ||
| import org.unilab.uniplan.student.dto.StudentRequestDto; | ||
|
|
||
| import java.util.UUID; | ||
|
|
||
| import static org.unilab.uniplan.utils.ErrorConstants.COURSE_NOT_FOUND; | ||
|
|
||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class StudentValidator{ | ||
| private final CourseRepository courseRepository; | ||
|
|
||
| public void validate(final StudentRequestDto request) { | ||
| UUID id = request.courseId(); | ||
| if (!courseRepository.existsById(id)){ | ||
| throw new ResourceNotFoundException(COURSE_NOT_FOUND.getMessage(id.toString())); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| package org.unilab.uniplan.student; | ||
|
|
||
| import org.springframework.transaction.annotation.Transactional; | ||
| import lombok.RequiredArgsConstructor; | ||
| import lombok.extern.slf4j.Slf4j; | ||
| import org.springframework.stereotype.Component; | ||
| import org.unilab.uniplan.exception.ResourceNotFoundException; | ||
| import org.unilab.uniplan.student.dto.StudentRequestDto; | ||
| import org.unilab.uniplan.student.dto.StudentResponseDto; | ||
| import java.util.List; | ||
| import java.util.UUID; | ||
|
|
||
| import static org.unilab.uniplan.utils.ErrorConstants.STUDENT_NOT_FOUND; | ||
|
|
||
| @Component | ||
| @Slf4j | ||
| @RequiredArgsConstructor | ||
| public class StudentWebFacade { | ||
|
|
||
| private final StudentMapper studentMapper; | ||
| private final StudentService studentService; | ||
| private final StudentValidator studentValidator; | ||
|
|
||
| @Transactional | ||
| public void createStudent(final StudentRequestDto request){ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMPORTANT: createStudent returns void, breaking the REST API contract. Clients get 201 Created with no body and cannot learn the server-assigned UUID. Fix: change the return type to StudentResponseDto, return studentMapper.toResponseDto(student), and have the Controller return ResponseEntity.status(HttpStatus.CREATED).body(facade.createStudent(request)). Matches the reference pattern in backend.md Appendix A. |
||
| studentValidator.validate(request); | ||
| final Student student = studentMapper.toEntity(request); | ||
| studentService.save(student); | ||
|
constantine0621 marked this conversation as resolved.
|
||
| log.info("created student with ID: {}", student.getId()); | ||
| } | ||
|
|
||
| @Transactional(readOnly = true) | ||
| public List<StudentResponseDto> getAllStudents(){ | ||
| return studentMapper.toResponseDtoList(studentService.getAll()); | ||
| } | ||
|
|
||
| @Transactional(readOnly = true) | ||
| public StudentResponseDto getStudentById(final UUID id){ | ||
| final Student student = getStudentOrThrow(id); | ||
| return studentMapper.toResponseDto(student); | ||
| } | ||
|
|
||
| @Transactional | ||
| public void updateStudent(final UUID id, | ||
| final StudentRequestDto request){ | ||
| studentValidator.validate(request); | ||
| final Student student = getStudentOrThrow(id); | ||
| studentMapper.updateEntity(request, student); | ||
| studentService.save(student); | ||
| log.info("updated student with ID: {}", student.getId()); | ||
| } | ||
|
|
||
| @Transactional | ||
| public void deleteStudent(final UUID id){ | ||
| final Student student = getStudentOrThrow(id); | ||
| studentService.delete(student); | ||
| log.info("deleted student with ID: {}", id); | ||
| } | ||
|
|
||
| private Student getStudentOrThrow(final UUID id){ | ||
| return studentService.getById(id) | ||
| .orElseThrow(() -> new ResourceNotFoundException( | ||
| STUDENT_NOT_FOUND.getMessage(String.valueOf(id)) | ||
| )); | ||
| } | ||
| } | ||
This file was deleted.
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMPORTANT: @NotNull was removed from @PathVariable here (and on updateStudent line 50, deleteStudent line 58). The old controller carried @PathVariable @NotNull final UUID id on every path param. Per backend.md Appendix A, path variables must carry @PathVariable @NotNull. Please restore the annotation on all three methods.