-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: 구매자 로그인기능 추가 #14
base: feature/#11-sign-up
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.
고생하셨습니다. 리뷰 드린 내용 한번 확인부탁드립니다.
src/main/resources/application.yaml
Outdated
server: | ||
servlet: | ||
session: | ||
timeout: 30m |
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.
이 설정의 목적은 무엇인가요~?
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.
사용자 인증정보를 저장하는 세션의 유효시간을 설정하는 옵션입니다.
기본적인 단위는 초 이지만 뒤에 단위를 붙여서 사용할 수 있습니다.
하지만 클라이언트가 잘못된 요청을 보낸 경우 400 에러로 처리를 해야합니다. | ||
스프링 부트는 @RestControllerAdvice 애노테이션을 사용하여 JSON 응답 형태로 반환할 수 있도록 합니다. | ||
*/ | ||
@RestControllerAdvice |
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.
RestControllerAdvice
에 대해 불필요한 설명이 있습니다. 동작원리가 무엇인가요?
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.
스프링은 스프링 내부 로직에서 예외가 발생하는 경우 예외를 처리할 수 있는 ExceptionResolver를 찾습니다.
처리할 수 있는 Resolver가 있다면 해당 예외 객체와 요청, 응답 객체를 파라미터로 전달합니다.
ExceptionResolver를 구현하여 예외 로직 처리를 코드로 작성할 수 있습니다.
RestControllerAdvice는 그런 ExceptionResolver를 사용할 수 있도록 등록하는 전역 예외처리 빈입니다.
그리고 추가로 응답을 Json으로 반환할 수 있도록 특화된 예외처리 빈입니다.
@Transactional | ||
public BuyerResponse signUp(final BuyerRequest buyerRequest, final LocalDateTime createdAt) { | ||
Buyer buyer = Buyer.create(buyerRequest.email(), buyerRequest.password(), buyerRequest.name(), | ||
final Buyer buyer = Buyer.create(buyerRequest.email(), buyerRequest.password(), buyerRequest.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.
ffinal
을 붙이는 기준이 무엇인가요?
메소드의 파라미터에도 final 을 전부 붙이셨는데, 일관성이 있으면 더 좋을 것 같습니다.
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.
final을 사용하는 이유는 참조 값의 재할당을 방지하기 위해서 입니다.
코드의 안정성을 높이고 예기치 않은 참조 변경을 방지할 수 있습니다.
이후 다른 개발자가 보더라도 해당 buyer 객체는 참조가 변경되지 않는 다는 것을 명확하게 알 수 있으므로
유지보수할 때 참조 변경에 대한 생각을 하지 않아도 됩니다.
|
||
@Override | ||
public LoginResponse login(final LoginUser user) { | ||
final Buyer findBuyer = buyerRepository.findByEmail(user.email()) |
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.
findBuyer
는 동사형으로 변수이름으로는 적합하지 않습니다.
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.
맞습니다 !!
수정하도록 하겠습니다.
@@ -0,0 +1,7 @@ | |||
package shop.sendbox.sendbox.login; | |||
|
|||
public record LoginCreateRequest(String email, String password, UserType userType) { |
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.
LoginCreateRequest 와 LoginRequest 의 차이는 무엇인가요?
dto 클래스의 기준이 궁금합니다.
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를 나누려고 했습니다.
LoginService가 LoginControllerDto를 사용한다면 순환참조라고 생각했습니다.
참조는 단방향으로 하는 것이 유지보수 및 변경하기 자유롭기 때문입니다.
만약 해당 서비스를 다른 컨트롤러에서 사용한다면 기존에 사용하던 Controller Dto를 사용하게 되므로
의존성이 생기기 때문입니다.
@Transactional(readOnly = true)는 해당 메서드에서 데이터의 변경이 없음을 명시하는 것으로, | ||
스냅샷을 만들지 않아 성능을 최적화할 수 있습니다. | ||
*/ | ||
@Transactional(readOnly = true) |
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.
Transactional 어노테이션은 어떻게 동작하나요?
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.
@transactional은 AOP로 동작합니다.
스프링 컨테이너에서 빈을 등록하다가 AOP와 관련된 어노테이션을 확인하게 되면 프록시 객체를 생성합니다.
타겟 메소드가 실행하기 전에 트랜잭션 매니저에서 커넥션을 가져옵니다.
그리고 커넥션에서 트랜잭션을 시작하고 타겟 메서드를 실행합니다.
이후 타겟 메서드의 결과에 따라 커넥션 롤백, 커밋이 동작하게 됩니다.
이때 시작과 종료가 동일한 커넥션에서 이루어져야합니다.
동일한 커넥션을 가져오고 동시성 문제에서 벗어나는 방법을 스프링은 로컬 스레드를 사용하여 해결했습니다.
|
||
public interface LoginHandler { | ||
|
||
boolean supports(UserType userType); |
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.
- LoginService 코드를 제가 스프링 부트의 HandlerAapter 방식으로 구현하면 다른 판매자나 관리자가 생기더라도 유연하게 확장할 수 있지 않을까 싶어서 만들어봤는데
멘토님이 보시기에 불필요한 코드가 많이 들어갔는지 궁금합니다.
LoginHandler 인터페이스를 구현한 클래스는 supports와 login 추상 메서드를 구현해야합니다.
구매자, 판매자나 이후 확장이 되어 다른 로그인 대상이 생긴다고 하더라도
해당 인터페이스를 구현하면 동작하도록 구현해봤습니다.
아이디어는 HandlerAdapter 에서 supoorts()와 handler()에서 얻었습니다.
@@ -0,0 +1,7 @@ | |||
package shop.sendbox.sendbox.login; | |||
|
|||
public record LoginCreateRequest(String email, String password, UserType userType) { |
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를 나누려고 했습니다.
LoginService가 LoginControllerDto를 사용한다면 순환참조라고 생각했습니다.
참조는 단방향으로 하는 것이 유지보수 및 변경하기 자유롭기 때문입니다.
만약 해당 서비스를 다른 컨트롤러에서 사용한다면 기존에 사용하던 Controller Dto를 사용하게 되므로
의존성이 생기기 때문입니다.
|
||
@Override | ||
public LoginResponse login(final LoginUser user) { | ||
final Buyer findBuyer = buyerRepository.findByEmail(user.email()) |
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.
맞습니다 !!
수정하도록 하겠습니다.
@Transactional | ||
public BuyerResponse signUp(final BuyerRequest buyerRequest, final LocalDateTime createdAt) { | ||
Buyer buyer = Buyer.create(buyerRequest.email(), buyerRequest.password(), buyerRequest.name(), | ||
final Buyer buyer = Buyer.create(buyerRequest.email(), buyerRequest.password(), buyerRequest.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.
final을 사용하는 이유는 참조 값의 재할당을 방지하기 위해서 입니다.
코드의 안정성을 높이고 예기치 않은 참조 변경을 방지할 수 있습니다.
이후 다른 개발자가 보더라도 해당 buyer 객체는 참조가 변경되지 않는 다는 것을 명확하게 알 수 있으므로
유지보수할 때 참조 변경에 대한 생각을 하지 않아도 됩니다.
@Transactional(readOnly = true)는 해당 메서드에서 데이터의 변경이 없음을 명시하는 것으로, | ||
스냅샷을 만들지 않아 성능을 최적화할 수 있습니다. | ||
*/ | ||
@Transactional(readOnly = true) |
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.
@transactional은 AOP로 동작합니다.
스프링 컨테이너에서 빈을 등록하다가 AOP와 관련된 어노테이션을 확인하게 되면 프록시 객체를 생성합니다.
타겟 메소드가 실행하기 전에 트랜잭션 매니저에서 커넥션을 가져옵니다.
그리고 커넥션에서 트랜잭션을 시작하고 타겟 메서드를 실행합니다.
이후 타겟 메서드의 결과에 따라 커넥션 롤백, 커밋이 동작하게 됩니다.
이때 시작과 종료가 동일한 커넥션에서 이루어져야합니다.
동일한 커넥션을 가져오고 동시성 문제에서 벗어나는 방법을 스프링은 로컬 스레드를 사용하여 해결했습니다.
src/main/resources/application.yaml
Outdated
server: | ||
servlet: | ||
session: | ||
timeout: 30m |
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.
사용자 인증정보를 저장하는 세션의 유효시간을 설정하는 옵션입니다.
기본적인 단위는 초 이지만 뒤에 단위를 붙여서 사용할 수 있습니다.
하지만 클라이언트가 잘못된 요청을 보낸 경우 400 에러로 처리를 해야합니다. | ||
스프링 부트는 @RestControllerAdvice 애노테이션을 사용하여 JSON 응답 형태로 반환할 수 있도록 합니다. | ||
*/ | ||
@RestControllerAdvice |
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.
스프링은 스프링 내부 로직에서 예외가 발생하는 경우 예외를 처리할 수 있는 ExceptionResolver를 찾습니다.
처리할 수 있는 Resolver가 있다면 해당 예외 객체와 요청, 응답 객체를 파라미터로 전달합니다.
ExceptionResolver를 구현하여 예외 로직 처리를 코드로 작성할 수 있습니다.
RestControllerAdvice는 그런 ExceptionResolver를 사용할 수 있도록 등록하는 전역 예외처리 빈입니다.
그리고 추가로 응답을 Json으로 반환할 수 있도록 특화된 예외처리 빈입니다.
#️⃣연관된 이슈
📝작업 내용
✒️ 코드 변경 이유
빌더를 추가한 이유는 가독성과 불변성, 유연성입니다.
레코드 타입이 주는 가독성과 간결함,불변객체가 빌더 패턴보다 큰 장점이라고 생각했습니다.
💬리뷰 요구사항(선택)
LoginHandler를 추가해봤습니다.
로그인은 이메일과 비밀번호를 가지고 조회를 합니다. 만약 확장되어도 구매자, 판매자, 관리자라도 동일하다고 생각했습니다.
그래서 이걸 공통으로 인터페이스로 묶고, 스프링 부트의 HandlerAapter를 호출하는 것처럼 사용하면
코드도 간결해지고 유연하지 않을까 생각했습니다.
일급 컬렉션을 사용해봤습니다.
private final List 이 구조를 private final Handlers 로 일급 컬렉션을 만들어서 사용했는데
멘토님이 보시기에 적절하게 사용했는지 여쭤보고 싶습니다.