로그인 기능 구현 #3
로그인 기능 구현 #3
Conversation
- 로그인 기능 - 로그인 성공 시 jwt 토큰 발행
- @ConfigurationProperties 사용 시 기본 생성자 추가
doFilterInternal 메서드 적용되도록 FilterConfig 추가
- 필터 정상적으로 작동하도록 어노테이션 적용
- 기본 생성자 사용 부분 전부 Builder 사용하도록 변경
… into DongCaprio # Conflicts: # src/test/java/com/board/board/service/BlogServiceTest.java
- HandlerMethodArgument 이용하여 커스텀 어노테이션을 활용하여 컨트롤러 파라미터로 처리하도록 변경
sosow0212
left a comment
sosow0212
left a comment
There was a problem hiding this comment.
수고하셨습니다! 그만 Approve해도 될 거 같은데, 토큰 부분이 살~짝 아쉬워서 한 번만 더 리팩토링하고 갈게요!
좀 뭔가 귀찮을 것 같으신데, 하나라도 더 담고 가시면 좋을 것 같아서 리뷰를 빡빡하게 남겨봤습니다 ㅎㅎ
하시다가 이해가 안되는 부분 있으면 언제든지 연락 주세요!
수고하셨습니다~~
| @@ -0,0 +1,4 @@ | |||
| package com.board.board.domain; | |||
|
|
|||
| public class Article { | |||
| @Setter | ||
| public class ArticleCreateRequest { | ||
|
|
||
| @NotBlank |
There was a problem hiding this comment.
프론트엔드 측에서 실수로 blank 값을 넣었을 때 예외가 발생할 거 같은데, message로 예외를 추가해주면 좋을 것 같아요!
이 경우 에러 핸들링은 어떻게 할 수 있을까요? 기존 CustomException 같은 건 ControllerAdvice로 잡을 수 있는데 NotBlank와 같은 Validation은 어떻게 잡을지 학습해보면 좋을 것 같습니다
There was a problem hiding this comment.
message 추가했습니다!
@notblank 와 같은 validation도 ControllerAdvice에서 처리가 가능할 것 같습니다
예외를 찾아보니 valid annotation 에러는 MethodArgumentNotValidException 에러를 발생시킵니다.
즉 ControllerAdvice 에
@ExceptionHandler(MethodArgumentNotValidException.class) 를 추가하여 메서드를 생성하면
에러 처리가 가능합니다!
| public class BlogService { | ||
|
|
||
| private final BlogRepository blogRepository; | ||
| private final MemberService memberService; |
There was a problem hiding this comment.
MemberService와 MemberRepository를 가져다 쓰는 것과 어떤 차이가 있을까요? (사실 정답은 없지만) 동진님이 생각하시는 차이가 궁금합니다 ㅎㅎ
There was a problem hiding this comment.
MemberService 를 사용하는것이 좋다고 생각합니다.
Service : 기능 구현
Repository : DB접근 이라는 역할이 있으니
그에 맞게 Service에 기능을 활용하는것이 적절한 것 같습니다.
단순히 db에 접근하는것도 service에서 구현하여 service의 메서드를 이용하는것이 맞는 선택인 것 같습니다.
There was a problem hiding this comment.
여기 보시면 많은 분들이 토론하신 게 있는데 보시면 좋을 것 같아요~
의존성을 끊으면 더 좋긴합니다! (비즈니스적인 의존 관계가 없다고 쳤을 때) id값을 토큰으로 빼와서 가져오면서 이를 끊을 수도 있고 여러 가지 방법이 있겠네요. 다만 ManyToOne과 같이 연관관계 매핑을 했다면 힘들 수도 있겠네요
There was a problem hiding this comment.
의존성을 끊는다는 것이
BlogService 에서 MemberService 관련된 값이 없어도 작동하도록
즉 현재 save를 예를 들면 BlogService 자체적으로 저장이 가능하도록 하면 좋다는 말씀이시죠?
이것 참 어렵네요
현재 말씀해주신것처럼 @manytoone 연관관계 매핑이 있기 때문에
BlogService 를 통해 게시글에 작성자를 넣는것이 알맞다고 생각합니다!
@Transactional
public ArticleEntity save(ArticleCreateRequest request, Long memberId) {
return blogRepository.save(ArticleEntity.builder()
.title(request.getTitle())
.content(request.getContent())
.member(memberService.findById(memberId))
.build());
}저번에 실무에서는 @manytoone 을 별로 사용하지 않고
id만 저장하는 간접참조 방식을 많이 사용한다고 말씀해주셨었는데
이런 의존성을 줄이기 위해서 사용이 되는것 같다는 합리적 추측이...
하지만 현재 구조에서는 지금 방식도 괜찮은 방식이라고 생각합니다...!
There was a problem hiding this comment.
ㅎㅎ 네 모든 상황이 다 다르니깐요!
만약 저기서 간접참조를 썼다면 MemberService를 import할 필요는 없었을 것 같아서요
HandlerMethodArgumentResolver에서 토큰을 통해 로그인한 유저의 MemberId를 받아서 생성자에 넣어서 의존성을 분리할 수도 있습니다!
https://www.youtube.com/watch?v=dJ5C4qRqAgA
이 내용 관련해서 진짜 좋은 영상인데 좀 긴데 꼭 보셨으면 좋겠습니다! 어떤 내용인지 확 와닿으실거에요!
제가 말씀 드리는 것도 전부 정답은 아니고 하나의 방법이기 때문에 참고만 해주세요!
There was a problem hiding this comment.
영상도 찾아주시고 정말 정말 감사합니다!!
해당 영상을 봤는데 아직은...ㅎㅎ 사실 어렵네요
- 핵심은 시스템이 커질수록 직접참조보다 간접참조 방식을 권장
- 양방향 의존성을 줄이자
그 방법으로
의존성의 연결고리? 같은 중간 객체를 만드는것과
도메인 이벤트 등을 설명해주시는것 같은데
영상을 다시 돌려보고 모르는 부분(도메인 이벤트 등)을 더 검색해나가면서 이해해보도록 하겠습니다!!!
There was a problem hiding this comment.
ㅎㅎ 넵 영상에서 결국 참조관계는 양방향보다 단방향인 경우 유지보수도 그렇고 여러 장점이 있다는 것이고 그러기에 간접참조 얘기가 나온 거에요! 아마 지금은 힘들 수 있지만, 천천히 보시고 직접 느껴보면서 관련 자료 찾아보시다보면 금방 이해하실 것 같습니다!
|
|
||
| @RequiredArgsConstructor | ||
| @Service | ||
| @Transactional |
There was a problem hiding this comment.
이건 사소한 팁인데 여기에 readonly=true 속성을 붙이고 업서트 메서드에 @transactional 어노테이션을 선언하는 게 좋아보입니다 ㅎㅎ JPA를 사용하다보니, 실수로 데이터를 수정하는 것이 더 치명적이니깐요!
There was a problem hiding this comment.
팁 감사합니다!
클래스 상위 어노테이션에 readonly를 추가하고
개별 메서드에서 수정/ 삭제 등이 필요한 경우 @transactional 을 추가했습니다!
| String email = authUtil.getMemberEmail(); | ||
| return memberService.findByEmail(email); // MemberEntity를 반환하여 파라미터에 주입 |
There was a problem hiding this comment.
MemberEntity를 조회해서 줘도 될까요? 민감한 값이 있지 않을까요?
Long id(member_id pk)를 준다거나, dto를 씌워 필요한 값을 준다거나 다른 방법은 없을까요?
There was a problem hiding this comment.
맞습니다 비밀번호 같이 민감한 정보가 같이 민감한 값이 존재하고 있습니다.
dto로 반환하는 방식을 고민했다가 결국 id만 있어도 될 것 같아서
Long id (즉 memberService.findByEmail(email).getId )를 반환하는 방식으로 변경했습니다!
| return memberRepository.findByEmail(email).orElseThrow(() -> | ||
| EmailNotFoundException.from(email)); |
There was a problem hiding this comment.
| return memberRepository.findByEmail(email).orElseThrow(() -> | |
| EmailNotFoundException.from(email)); | |
| return memberRepository.findByEmail(email) | |
| .orElseThrow(() ->EmailNotFoundException.from(email)); |
There was a problem hiding this comment.
어떤 기준으로 줄바꿈을 해야되는지 아직 헷갈리네요ㅎㅎ
말씀하신대로 변경하니 더 깔끔해보이는것 같습니다!
변경완료했습니다!
There was a problem hiding this comment.
정답은 없는데 대부분의 경우 한줄에 점 하나 정도로 저는 맞춰서 쓰고 있어요 ㅎㅎ
이것 저것 해보시고 동진님이 편한 방법 찾는 것도 좋을 것 같습니다!
왜 줄바꿈을 해야하는지 해당 글 참고 해보시면 좋을 것 같아요~
There was a problem hiding this comment.
직접 글까지 찾아봐주시고 정말 너무 감사합니다!
글을 아직 전부 다 이해하지는 못했지만
사소하지만 줄바꿈으로 가독성이 좋아지는 경우도 많은 것 같습니다!
신경 써서 가독성이 조금이라도 좋은 형식으로 줄 바꿈을 진행해보겠습니다!
| @MockitoBean | ||
| private AuthenticatedMemberArgumentResolver authenticatedMemberArgumentResolver; // ✅ ArgumentResolver 추가 |
There was a problem hiding this comment.
ㅋㅋ 인증때문에 에러나서 gpt 사용하셨군요 Resolver를 Bean으로 달아줘야하는 이유는 무엇일까요?
BlogApiControllerTest에서 WebMvcTest로 스캔하는 빈의 범위는 어디까지일까요?
There was a problem hiding this comment.
- @WebMvcTest의 스캔 범위 : 컨트롤러 테스트 시 사용하고 지금같은 경우 @WebMvcTest(BlogApiController.class) 이므로 BlogApiController(서비스, 레포지토리 등 스캔X)와 ContorllerAdvice만 스캔합니다. 때문에 위 스캔에서 벗어난
service등은
@MockitoBean
private BlogService blogService;이런 식으로 스캔해야 합니다.
- Resolver를 Bean으로 달아줘야하는 이유
1번에서 언급했듯이 @WebMvcTest를 통해서는 controller 관련한 Bean만 스캔되는데
제가 만든 Resolver 또한 @component 를 활용해서 만들었으므로(즉 Bean이므로)
@MockitoBean 를 활용하여 추가를 해야 정상적으로 값이 null 이 아니라 주입되게 됩니다.
There was a problem hiding this comment.
아직은 어려우시죠?! bean 개념이 아직도 헷갈릴 수 있을 거 같아요~ 이쯤 스프링부트에서 빈 스캔은 어떻게 하는지, 기준이 있는지 한 번 찾아봐도 좋을 것 같습니다! (이걸 이해하면 테스트에서도 빈 스캔을 어떤식으로 해서 @WebMvcTest가 컨트롤러 관련 빈만 주입하는지 이해하기도 쉬워요)
There was a problem hiding this comment.
역시 바른길로 인도해주시는 멋진분!
다시 한번 찾아봤습니다!
스프링부트 빈 스캔은 애플리케이션 실행 시 모든 Bean을 다 스캔하고,
WebMvcTest에서는 말그대로 Web과 관련된 Bean만 띄워서 테스트를 빠르게 하려다보니 Web과 관련 즉 @controller 관련 Bean만 띄워서 테스트를 진행하게 됩니다!
그러다보니 @service 등은 Mock을 통해서 사용해야하고
저희가 만든 Resolver 도 @controller 가 아니므로 mock을 통해서 사용해야 되기 때문에 @MockitoBean으로 선언해주어야 되는것 같습니다!
There was a problem hiding this comment.
해당 내용과 조금 다르지만 추가적으로 생각하면 좋은 건
@service, @controller, @repository 같이 우리가 자주 사용하는 빈들을 모두 까보면 @component가 들어가있습니다. 사실 @component만 붙어도 빈으로 등록되는 것이죠 그리고 우리가 사용하는 빈들이 여기서 파생된 것도 유추할 수 있습니다.
그러면 굳이 왜 @controller, @service... 와 같이 따로 두냐? 라고 했을 땐 여러 가지 이유가 있겠지만 저는 명시적으로 알 수 있다는 점과 함께 테스트나 빈 스캔(ComponentScan)에서 구별지을 수 있기 때문이라고 생각합니다.
이를 이용하면 나중에 @service와 같은 커스텀 빈 등록 어노테이션을 만들고, 테스트에서 활성화와 비활성화도 할 수 있고 프로덕션 코드에서도 동적으로 활용할 수 있겠죠 ㅎㅎ
There was a problem hiding this comment.
말씀해주신 부분을 보니
커스텀 빈 등록을 할때, 커스텀 빈 어노테이션을 만들때
더욱 이해하고 코드를 작성할 수 있을 것 같습니다!
감사합니다!
|
|
||
| @SpringBootTest | ||
| @AutoConfigureMockMvc | ||
| @Transactional |
There was a problem hiding this comment.
Transactional은 데이터 롤백을 위해서 사용하신 것 같은데, 왜 통합테스트에선 롤백이 필요할까요? 롤백을 안하면 데이터가 쌓이는 원인은 무엇 때문일까요?!
There was a problem hiding this comment.
통합테스트에서는 실제와 똑같은 환경을 구성해주기 때문에
repository의 save 등 DB 관련 메서드가 발동되면
실제 DB에 값이 저장이 되게 됩니다.
하지만 통합테스트의 목적은 실제 DB에 값을 변경하는것이 아닌
기능이 실제와 똑같이 작동하는지를 확인하는 것이기 때문에
실제와 똑같이 작동 되는지 확인만 한 후에
변경된 값을 다시 원래대로 되돌리는것이 알맞다고 생각합니다.
때문에 값을 되돌리기 위한 어노테이션이 @transactional 이며 그것을 사용해서 롤백을 진행하여
데이터의 변경을 막았습니다!
There was a problem hiding this comment.
실제 DB에 값이 저장이 되게 됩니다.
-
테스트로 인해 실제 DB에 영향을 주면 안됩니다! 테스트에서만 사용할 수 있도록 db를 설정할 순 없을까요? 테스트 컨테이너나 mysql을 사용하면 테스트에서만 h2를 써도 좋을 것 같은데 참고 해보시면 좋을 것 같아요.
-
그리고 Transactional이 말씀하신대로 테스트 데이터를 롤백해주는데요. 아마 저기서 하나하나 개별 테스트 실행하면 다 작동은 되는데 전체 돌리면 안될거에요. 그 이유가 뭘까요? 스프링 공식문서를 보시면 힌트를 조금 얻으실 수 있을 것 같습니다!
-
마지막으로 Transactional 어노테이션 말고 다른 테스트 격리 방법은 없을까요?
There was a problem hiding this comment.
-
yml을 분리하여 application-test.yml을 만들고 통합테스트 클래스 위에 @activeprofiles("test")를 추가합니다 그 후
추가한 yml에 db 정보를 [ h2 ] 와 같이 원하는 정보를 적는다면 기존 yml과 다른 DB를 바라보게 할 수 있습니다! -
헉 공식문서.. 찾아주셔서 감사합니다 아직 이해가 잘 안되서 서칭을 더 해본 결과
전체 테스트를 돌리면 spring에서 ApplicationContext를 공유하기 때문에
@transactional을 통해 데이터는 롤백이 되도 다른것들(테이블 구조?) 등이 충돌을 일으킬수도 있다는 것 같습니다
그래서 해결 방법으로는 test.yml에 ddl-auto 속성을 create-drop 으로 설정하면
테이블 시작 시 테이블을 생성하고 테스트 종료시 테이블을 삭제하므로
전체 테스트 시 충돌로 인한 오류를 방지 할 수 있는것 같습니다!
There was a problem hiding this comment.
- 지금 새롭게 적용한 것 처럼 @activeprofiles("test") 을 추가하여
test.yml에 운영DB와 다른 testDB 정보를 설정하여 적용하면
@transactional 없이도 안전하게 테스트 할 수 있는것 같습니다!
There was a problem hiding this comment.
ㅎㅎ 넵 그것도 좋고요 여러 방법이 있는데
저는 주로 mysql을 쓰면 테스트용 db로 h2를 쓰고 있습니다. @beforeeach에 DatabaseCleaner.reset()을 실행시켜서 테스트 격리를 하는 걸 선호합니다! 더 나아가서 이를 어노테이션화 시키거나, extends로 @beforeeach를 테스트 클래스마다 안 쓸 수 있게 개선하고요
DatabaseCleaner는 테스트 헬퍼 클래스이고, 내부에서는 reset()메서드를 구현해서 현재 바인딩된 db의 테이블 정보를 긁어오고 모두 TRUNCATE 시켜주는 기능을 뒀습니다.
이렇게도 할 수 있습니다!
There was a problem hiding this comment.
재윤님 말씀이
@transactional을 쓰지않고
@beforeeach에 DatabaseCleaner.reset() 메서드를 넣고 클래스를 만들어 해당 클래스를 상속시켜
모든 클래스에서 테스트 시
모든 테이블의 데이터를 다 지우고 테스트를 안전하게 할 수 있다는 말씀이시죠?
해당 방법을 사용하는 이유는
전체 테스트를 돌리면 @transactional이 있어도 테스트끼리ApplicationContext를 공유하므로 예상치못한 결과가 나올수도 있기 때문에 테스트 안정성을 높이기 위함이고요!
제가 이해한 부분이 맞을까요?
There was a problem hiding this comment.
ㅎㅎ 네 얘기가 길어질 것 같은데 여기 향로님이 작성하신 글 보시면 너무 잘 나와있어서 읽어보시면 도움 많이되실 것 같아요
제 경우도 @transactional보다 DatabaseCleaner 같은 헬퍼 클래스를 두고 해당 클래스에선 현재의 경우 H2(mysql mode)를 테스트 디비로 사용하니 헬퍼 클래스에서 모든 테이블 서칭 후 TRUNCATE 쿼리를 날려주는 기능을 만들어주고 이걸 @IntegrationTest 와 같이 어노테이션을 만들어서 주입을 시켜주고 있습니다.
이를 만들고 Mock, Stub을 쓰는 테스트가 아닌 @SpringBootTest를 기반으로한 or @WebMvcTest와 같이 빈을 스캔하는 통합 테스트 환경에서 함께 붙여서 테스트 격리를 해주고 있습니다.
| import static org.mockito.ArgumentMatchers.any; | ||
| import static org.mockito.Mockito.when; | ||
|
|
||
| @ExtendWith(MockitoExtension.class) |
There was a problem hiding this comment.
맞아요~ Service 레이어 테스트는 통합테스트를 한다면 infra레벨까지 테스트 하게 될 수도 있습니다.
느린 것도 그렇고요 ㅎㅎ 그렇다고 무조건 서비스 레이어 레벨에서 mock, stub, fake를 이용한 테스트가 무조건 정답은 아닙니다! 가끔 통합 테스트가 필요할 수도 있어요! 이 부분 고민 한 번 해보시면 좋을 것 같아요.
Mock 방식도 다 좋지만, 실제 서비스에서는 실패하는 로직이지만, mocking으로 하는 테스트에선 성공하기 때문에 디버깅 하기 힘들어질 수도 있고요. (이를 깨지는 테스트라고 합니다!)모든 방법에는 장단점에 다 존재한다고 생각해서 이 부분 유의하시면서 테스트를 작성하면 도움이 되실 것 같습니다!
| @Nested | ||
| @DisplayName("로그인 테스트") |
There was a problem hiding this comment.
관심사 분리를 위해 inner class 좋습니다 ㅎㅎ
- 메서드 명 변경 - 클래스 어노테이션 @transactional(readOnly = true)로 변경
- jwt 커스텀 어노테이션에서 memberId만 반환하도록 변경(보안 이슈) - 로직 변경에 따른 테스트 코드 변경
- 파라미터가 Long 일때 @AuthenticatedMember가 주입되도록 변경
- test에서는 h2 testDB에 접근 - 통합테스트 시 @activeprofiles("test") 사용 - 통합 test시 ddl-auto : create-drop / jwt 값 변경 적용
- test에서는 h2 testDB에 접근 - 통합테스트 시 @activeprofiles("test") 사용 - 통합 test시 ddl-auto : create-drop / jwt 값 변경 적용
|
일단 현재 단계에서 이정도면 아주 충분한 것 같습니다! 좀 길어지긴 했는데 그만큼 배우신 것도 많으실 것 같아요~ |
|
approve 해주시면 뭔가 여태까지 질문 및 답변이 사라질것 같아서 |
충돌난 부분 해결했습니다! 이제 approve 가능하실 것 같습니다!! |
2 participants
rebase test two