Skip to content
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

[BE] feat: 회원의 학생 인증 정보 조회 API 구현 (#496) #531

Merged
merged 5 commits into from
Oct 19, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@

import com.festago.auth.annotation.Member;
import com.festago.student.application.StudentService;
import com.festago.student.dto.StudentResponse;
import com.festago.student.dto.StudentSendMailRequest;
import com.festago.student.dto.StudentVerificateRequest;
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.tags.Tag;
import jakarta.validation.Valid;
import lombok.RequiredArgsConstructor;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
Expand All @@ -33,10 +35,18 @@ public ResponseEntity<Void> sendEmail(@Member Long memberId,

@PostMapping("/verification")
@Operation(description = "학교 인증을 수행한다.", summary = "학생 인증 수행")
public ResponseEntity<Void> verificate(@Member Long memberId,
@RequestBody @Valid StudentVerificateRequest request) {
studentService.verificate(memberId, request);
public ResponseEntity<Void> verify(@Member Long memberId,
@RequestBody @Valid StudentVerificateRequest request) {
studentService.verify(memberId, request);
return ResponseEntity.ok()
.build();
}

@GetMapping
@Operation(description = "학생 인증 정보를 조회한다.", summary = "학생 인증 정보 조회")
public ResponseEntity<StudentResponse> findVerification(@Member Long memberId) {
StudentResponse response = studentService.findVerification(memberId);
return ResponseEntity.ok()
.body(response);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@
import com.festago.student.domain.StudentCode;
import com.festago.student.domain.VerificationCode;
import com.festago.student.domain.VerificationMailPayload;
import com.festago.student.dto.StudentResponse;
import com.festago.student.dto.StudentSendMailRequest;
import com.festago.student.dto.StudentVerificateRequest;
import com.festago.student.repository.StudentCodeRepository;
import com.festago.student.repository.StudentRepository;
import java.time.Clock;
import java.time.LocalDateTime;
import java.util.Optional;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
Expand Down Expand Up @@ -89,7 +91,7 @@ private void saveStudentCode(VerificationCode code, Member member, School school
);
}

public void verificate(Long memberId, StudentVerificateRequest request) {
public void verify(Long memberId, StudentVerificateRequest request) {
validateStudent(memberId);
Member member = findMember(memberId);
StudentCode studentCode = studentCodeRepository.findByCodeAndMember(new VerificationCode(request.code()),
Expand All @@ -98,4 +100,11 @@ public void verificate(Long memberId, StudentVerificateRequest request) {
studentRepository.save(new Student(member, studentCode.getSchool(), studentCode.getUsername()));
studentCodeRepository.deleteByMember(member);
}

public StudentResponse findVerification(Long memberId) {
Optional<Student> student = studentRepository.findByMemberIdWithFetch(memberId);
return student
.map(value -> StudentResponse.verified(value.getSchool()))
.orElseGet(StudentResponse::notVerified);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

다음과 같이 라인 수를 줄일 수 있을 것 같네요!
물론 선택입니다. 👍

Suggested change
public StudentResponse findVerification(Long memberId) {
Optional<Student> student = studentRepository.findByMemberIdWithFetch(memberId);
return student
.map(value -> StudentResponse.verified(value.getSchool()))
.orElseGet(StudentResponse::notVerified);
}
public StudentResponse findVerification(Long memberId) {
return studentRepository.findByMemberIdWithFetch(memberId)
.map(value -> StudentResponse.verified(value.getSchool()))
.orElseGet(StudentResponse::notVerified);
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

또한 지금 StudentService 클래스의 필드 수를 보고 있는데 의존하고 있는 객체가 어마무시하네요.
이 부분에 대해서는 추후 서비스를 작은 서비스로 분리시켜도 좋을 것 같아요!

}
23 changes: 23 additions & 0 deletions backend/src/main/java/com/festago/student/dto/StudentResponse.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package com.festago.student.dto;

import com.festago.school.domain.School;

public record StudentResponse(
boolean isVerified,
StudentSchoolResponse school
) {

public static StudentResponse verified(School school) {
return new StudentResponse(
true,
StudentSchoolResponse.from(school)
);
}

public static StudentResponse notVerified() {
return new StudentResponse(
false,
null
);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

인증이 되지 않은 사용자는 해당 정적 메서드를 호출하여 응답을 반환하는군요!
하지만 매번 새로운 인스턴스 값을 넘겨줄 필요가 있을 것 같지는 않아요! (record 클래스가 불변하므로)
따라서 다음과 같이 캐싱된 값을 넘겨줘도 좋을 것 같네요!

Suggested change
public static StudentResponse notVerified() {
return new StudentResponse(
false,
null
);
}
private static final StudentResponse NOT_VERIFIED = new StudentResponse(false, null);
...
public static StudentResponse notVerified() {
return NOT_VERIFIED;
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

글렌이 제안해주신 방법이 캐싱뿐만 아니라 상수 형식으로 빼놓기에 전 '가독성 측면에서도 더 좋을 수 있겠다' 라는 생각도 드네요

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오, 가독성과 캐싱 두가지 이점을 모두 챙길 수 있겠군요! 반영하겠습니다

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.festago.student.dto;

import com.festago.school.domain.School;

public record StudentSchoolResponse(
Long id,
String schoolName,
String domain
) {

public static StudentSchoolResponse from(School school) {
return new StudentSchoolResponse(
school.getId(),
school.getName(),
school.getDomain()
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@

import com.festago.member.domain.Member;
import com.festago.student.domain.Student;
import java.util.Optional;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Query;
import org.springframework.data.repository.query.Param;

public interface StudentRepository extends JpaRepository<Student, Long> {

Expand All @@ -11,4 +14,12 @@ public interface StudentRepository extends JpaRepository<Student, Long> {
boolean existsByUsernameAndSchoolId(String username, Long id);

boolean existsByMemberId(Long id);

@Query("""
SELECT st
FROM Student st
LEFT JOIN FETCH st.school sc
WHERE st.member.id = :memberId
""")
Optional<Student> findByMemberIdWithFetch(@Param("memberId") Long memberId);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦅👀

Suggested change
@Query("""
SELECT st
FROM Student st
LEFT JOIN FETCH st.school sc
WHERE st.member.id = :memberId
""")
Optional<Student> findByMemberIdWithFetch(@Param("memberId") Long memberId);
@Query("""
SELECT st
FROM Student st
LEFT JOIN FETCH st.school sc
WHERE st.member.id = :memberId
""")
Optional<Student> findByMemberIdWithFetch(@Param("memberId") Long memberId);

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

또한 School에 대해 left join을 하기보단 inner join을 해도 괜찮지 않을까요?
학생은 학교가 있어야만 존재하니까..

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inner join 좋은 아이디어인 것 같아요!!

}
Original file line number Diff line number Diff line change
@@ -1,14 +1,22 @@
package com.festago.presentation;

import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.BDDMockito.given;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.festago.student.application.StudentService;
import com.festago.student.dto.StudentResponse;
import com.festago.student.dto.StudentSchoolResponse;
import com.festago.student.dto.StudentSendMailRequest;
import com.festago.student.dto.StudentVerificateRequest;
import com.festago.support.CustomWebMvcTest;
import com.festago.support.WithMockAuth;
import java.nio.charset.StandardCharsets;
import org.junit.jupiter.api.DisplayNameGeneration;
import org.junit.jupiter.api.DisplayNameGenerator.ReplaceUnderscores;
import org.junit.jupiter.api.Nested;
Expand Down Expand Up @@ -93,4 +101,42 @@ class 학생_인증 {
.andExpect(status().isOk());
}
}

@Nested
class 학생_인증_정보_조회 {

@Test
void 인증이_되지_않으면_401() throws Exception {
// given
StudentVerificateRequest request = new StudentVerificateRequest("123456");

// when & then
mockMvc.perform(get("/students")
.contentType(MediaType.APPLICATION_JSON)
.content(objectMapper.writeValueAsString(request)))
.andExpect(status().isUnauthorized());
}

@Test
@WithMockAuth
void 학생_인증_조회() throws Exception {
// given
StudentResponse expected = new StudentResponse(true, new StudentSchoolResponse(1L, "테코대학교", "teco.ac.kr"));
given(studentService.findVerification(anyLong()))
.willReturn(expected);

// when & then
String content = mockMvc.perform(get("/students")
.contentType(MediaType.APPLICATION_JSON)
.header(HttpHeaders.AUTHORIZATION, "Bearer token")
)
.andExpect(status().isOk())
.andDo(print())
.andReturn()
.getResponse()
.getContentAsString(StandardCharsets.UTF_8);
StudentResponse actual = objectMapper.readValue(content, StudentResponse.class);
assertThat(actual).isEqualTo(expected);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import static com.festago.common.exception.ErrorCode.TOO_FREQUENT_REQUESTS;
import static org.assertj.core.api.Assertions.assertThatNoException;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.api.SoftAssertions.assertSoftly;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.anyString;
Expand All @@ -20,8 +21,10 @@
import com.festago.member.repository.MemberRepository;
import com.festago.school.domain.School;
import com.festago.school.repository.SchoolRepository;
import com.festago.student.domain.Student;
import com.festago.student.domain.StudentCode;
import com.festago.student.domain.VerificationCode;
import com.festago.student.dto.StudentResponse;
import com.festago.student.dto.StudentSendMailRequest;
import com.festago.student.dto.StudentVerificateRequest;
import com.festago.student.infrastructure.MockMailClient;
Expand All @@ -32,6 +35,7 @@
import com.festago.support.SchoolFixture;
import com.festago.support.SetUpMockito;
import com.festago.support.StudentCodeFixture;
import com.festago.support.StudentFixture;
import java.time.Clock;
import java.time.LocalDateTime;
import java.util.Optional;
Expand Down Expand Up @@ -186,7 +190,7 @@ class 학생_인증 {
.willReturn(true);

// when & then
assertThatThrownBy(() -> studentService.verificate(memberId, request))
assertThatThrownBy(() -> studentService.verify(memberId, request))
.isInstanceOf(BadRequestException.class)
.hasMessage(ALREADY_STUDENT_VERIFIED.getMessage());
}
Expand All @@ -202,7 +206,7 @@ class 학생_인증 {
.willReturn(Optional.empty());

// when & then
assertThatThrownBy(() -> studentService.verificate(memberId, request))
assertThatThrownBy(() -> studentService.verify(memberId, request))
.isInstanceOf(BadRequestException.class)
.hasMessage(INVALID_STUDENT_VERIFICATION_CODE.getMessage());
}
Expand All @@ -225,7 +229,47 @@ class 학생_인증 {

// when & then
assertThatNoException()
.isThrownBy(() -> studentService.verificate(memberId, request));
.isThrownBy(() -> studentService.verify(memberId, request));
}
}

@Nested
class 멤버_아이디로_인증정보_조회 {

@Test
void 학생_인증된_멤버의_경우() {
// given
Long memberId = 1L;
School school = SchoolFixture.school().id(2L).build();
Student student = StudentFixture.student().id(3L).school(school).build();
given(studentRepository.findByMemberIdWithFetch(memberId))
.willReturn(Optional.of(student));

// when
StudentResponse actual = studentService.findVerification(memberId);

// then
assertSoftly(softly -> {
softly.assertThat(actual.isVerified()).isTrue();
softly.assertThat(actual.school().id()).isEqualTo(school.getId());
});
}

@Test
void 학생_인증되지_않은_사용자의_경우() {
// given
Long memberId = 1L;
given(studentRepository.findByMemberIdWithFetch(memberId))
.willReturn(Optional.empty());

// when
StudentResponse actual = studentService.findVerification(memberId);

// then
assertSoftly(softly -> {
softly.assertThat(actual.isVerified()).isFalse();
softly.assertThat(actual.school()).isNull();
});
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package com.festago.student.repository;

import static org.assertj.core.api.SoftAssertions.assertSoftly;

import com.festago.member.domain.Member;
import com.festago.member.repository.MemberRepository;
import com.festago.school.domain.School;
import com.festago.school.repository.SchoolRepository;
import com.festago.student.domain.Student;
import com.festago.support.MemberFixture;
import com.festago.support.SchoolFixture;
import com.festago.support.StudentFixture;
import java.util.Optional;
import org.junit.jupiter.api.DisplayNameGeneration;
import org.junit.jupiter.api.DisplayNameGenerator.ReplaceUnderscores;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest;

@DisplayNameGeneration(ReplaceUnderscores.class)
@SuppressWarnings("NonAsciiCharacters")
@DataJpaTest
class StudentRepositoryTest {

@Autowired
StudentRepository studentRepository;

@Autowired
SchoolRepository schoolRepository;

@Autowired
MemberRepository memberRepository;

@Test
void 멤버id로_조회() {
// given
Member member = memberRepository.save(MemberFixture.member().build());
School school = schoolRepository.save(SchoolFixture.school().build());
Student student = studentRepository.save(StudentFixture.student().school(school).member(member).build());

// when
Optional<Student> actual = studentRepository.findByMemberIdWithFetch(member.getId());

// then
assertSoftly(softly -> {
softly.assertThat(actual).isNotEmpty();
softly.assertThat(actual.get().getId()).isEqualTo(student.getId());
});
}
}
44 changes: 44 additions & 0 deletions backend/src/test/java/com/festago/support/StudentFixture.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package com.festago.support;

import com.festago.member.domain.Member;
import com.festago.school.domain.School;
import com.festago.student.domain.Student;

public class StudentFixture {

private Long id;
private Member member = MemberFixture.member().build();
private School school = SchoolFixture.school().build();
private String username = "xxeol2";

private StudentFixture() {
}

public static StudentFixture student() {
return new StudentFixture();
}

public StudentFixture id(Long id) {
this.id = id;
return this;
}

public StudentFixture member(Member member) {
this.member = member;
return this;
}

public StudentFixture school(School school) {
this.school = school;
return this;
}

public StudentFixture username(String username) {
this.username = username;
return this;
}

public Student build() {
return new Student(id, member, school, username);
}
}
Loading