-
Notifications
You must be signed in to change notification settings - Fork 78
[Spring Data JPA] 이예진 미션 제출합니다 #191
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: yaevrai
Are you sure you want to change the base?
Conversation
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.
안녕하세요 예진님, 어느덧 마지막 미션이네요!
기존에 JPA를 사용해 보셨다고 했는데, 이번 미션은 난이도가 어떠셨나요?
민지님 리뷰에서도 남겼지만, 이번 미션을 통해 도구의 사용법을 넘어, 현명하게 도구를 사용하는 법을 익히셨으면 좋겠어요.
최근에 이직해서, 다른 곳에 신경 쓸 곳이 많아, 코드 리뷰에 신경을 많이 못 쓴 것 같아 죄송하네요. 😭
요구사항은 모두 만족했으니, 생각할 곳에 대해 리뷰 남겼습니다!
반영하시고, 재요청 부탁드립니다!
public ResponseEntity<Void> login( | ||
@RequestBody LoginRequest loginRequest | ||
) { | ||
) throws NotFoundException { |
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.
NotFoundException
은 언체크 예외인데, 메서드에 명시적으로 적어주신 이유가 있을까요?
public static String getStatus(Reservation reservation, Integer rank) { | ||
if (reservation.isWaiting()) { | ||
return rank + "번째 예약대기"; | ||
} | ||
return "예약"; | ||
} |
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.
DTO의 메서드에 접근자가 public
인 정적 메서드가 있는 이유가 있을까요?
@Query("SELECT r FROM Reservation r JOIN FETCH r.theme JOIN FETCH r.time WHERE r.member.id = :memberId") | ||
List<Reservation> findByMemberIdWithThemeAndTime(@Param("memberId") Long memberId); |
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.
N+1 문제를 해결하기 위해 FetchJoin을 사용해 주셨네요!
지금은 연관 관계를 맺은 Theme
와 Time
이 @ManyToOne
관계를 가지고 있어서, 2개 이상 FetchJoin을 해도 문제가 발생하지 않는데, 만약 이 둘이 @OneToMany
관계였다면 어떤 문제가 발생했을까요?
이러한 문제는 어떤 방법으로 해결할 수 있을까요?
public void cancel(Long id) { | ||
Reservation reservation = reservationRepository.findById(id) | ||
.orElseThrow(() -> new IllegalArgumentException( | ||
"not found reservation with id:" + id)); | ||
boolean wasReserved = reservation.isReserved(); | ||
|
||
reservationRepository.deleteById(id); | ||
|
||
if (wasReserved) { | ||
promoteWaitingToReserved(reservation); | ||
} |
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.
reservationRepository.deleteById(id)
뒤 예외가 발생하면 어떤 문제가 발생하나요?promoteWaitingToReserved(reservation)
는Reservation
의 상태를 변경하는 메서드 같네요. 하지만 상태 변경이 DB까지 전파되나요?
이러한 문제를 해결하려면 어떤게 필요할까요?
public List<ReservationResponse> findAll() { | ||
return reservationDao.findAll().stream() | ||
.map(it -> new ReservationResponse(it.getId(), it.getName(), it.getTheme().getName(), it.getDate(), it.getTime().getValue())) | ||
.toList(); | ||
return reservationRepository.findAll().stream() | ||
.map(it -> new ReservationResponse( | ||
it.getId(), | ||
it.getName(), | ||
it.getTheme().getName(), | ||
it.getDate(), | ||
it.getTime().getValue() | ||
)).toList(); | ||
} |
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.
해당 메서드를 실행하면 어떤 예외가 발생하나요?
그렇다면 왜 예외가 발생할까요?
만약 예외가 발생하지 않고, DB에 저장된 Reservation
의 개수가 10개라 가정했을 때, reservationRepository.findAll()
에서 발생하는 쿼리를 포함한 쿼리는 총 몇 번 발생할까요?
public LoginService(MemberDao memberDao, JwtUtil jwtUtil) { | ||
this.memberDao = memberDao; | ||
public LoginService(MemberRepository memberRepository, JwtUtil jwtUtil) { | ||
this.memberRepository = memberRepository; | ||
this.jwtUtil = jwtUtil; | ||
} | ||
|
||
public String login(String email, String password) { | ||
Member member = memberDao.findByEmailAndPassword(email, password); | ||
Member member = memberRepository.findByEmailAndPassword(email, password) | ||
.orElseThrow( | ||
() -> new IllegalArgumentException("not found Member with email: " + email)); | ||
return jwtUtil.createToken(member); | ||
} | ||
|
||
public MemberInfo check(String token) { | ||
JwtPayload payload = jwtUtil.parseToken(token); | ||
Member member = memberDao.findByName(payload.name()); | ||
Member member = memberRepository.findByName(payload.name()) | ||
.orElseThrow(() -> new IllegalArgumentException( | ||
"not found Member with name: " + payload.name())); |
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.
@ManyToOne(fetch = FetchType.LAZY) | ||
@JoinColumn(name = "member_id") | ||
private Member member; |
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.
public boolean isReserved() { | ||
return this.status == ReservationStatus.RESERVED; | ||
} | ||
|
||
public boolean isWaiting() { | ||
return this.status == ReservationStatus.WAITING; | ||
} | ||
|
||
public void promoteToReserved() { | ||
this.status = ReservationStatus.RESERVED; | ||
} |
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.
상태를 직접 노출하지 않고, 명확한 이름으로 의미를 잘 전달하는군요!
글렌 안녕하세요 이번에도 잘 부탁드립니다!
이번 미션을 통해서 JPA는 물론이고 심화된 요구사항을 통해서
요구사항 및 전체적인 흐름에 대해 파악하는 능력을 길러야겠단 생각이 들었습니다 ^>^;;
6단계 미션을 진행하면서 제시된 사항과 약간 다르게 진행했는데
(Waiting이 Reservation은 성격이 비슷하여 분리하지 않아도 될 것같단 생각에 하나의 기능으로 개발했습니다)
그러면서 정상 동작을 위해 화면단 변화도 필요했고 요구사항에도 맞지 않는단 생각이 들었어요.
오히려 복잡하게 돌아가면서 시간을 많이 소비하기도 했고, 부족한 부분이 많을 것 같아 아쥬 가감없이 리뷰해주시면 감사하겠습니다..!!
🥹👍