9조 BE 코드리뷰 2회차 #36
Merged
9조 BE 코드리뷰 2회차 #36
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JwtProvider 구현. reissueAccessToken에 TODO가 존재한다.
JWT 토큰 관련 예외처리시 사용할 핸들러. Error 메시지에 대한 추가 구현이 남아있음.
JWT 로직을 처리할 서블릿 필터 구현. UserDetailsService 부분이 추가 구현사항으로 남아있다.
주문 도메인과 테이블명이 중복. 추후 협의후 최종 수정 예정.
JPA유저레포지토리 구현.
WebSecurityConfig 구현.
임시 컨트롤러. 서비스 로직 완성후 의존성 변경이 필요하며, 이후 data.sql를 통해 진행한다.
yaml 파일 gitignore에 추가.
yaml 파일 gitignore에 추가.
필요시 추후 추가 예정
ArtistInfo 관련코드는 추후 수정 예정
로그인 일부 구현
ArtistInfo 관련코드는 추후 수정 예정
테스트를 위한 data.sql추가
userId 추가.
String으로 관리하던 타입을 LocalDateTime으로 변경
FetchType을 Lazy로 변경.
JpaRepository 생성
ArtistInfo 관련코드는 추후 수정 예정, put 메서드 활용
그라파나, 프로메테우스 설정 및 관련 의존성 추가 #31
ArtistInfo와 StudentArtist 사이의 CascadeType.ALL 설정
변경 사항에 맞게 테스트 유저 변경
권한 설정이 가능하도록 추가 구현
ArtistInfo에 롬복 추가. about에 기본 값 추가. 추후 nullable = false를 고려
User에 사용할 UserDto및 artist의 response,request를 구현
테스트를 위한 security-test 추가
테스트를 위한 data.sql 별도 생성
테스트를 위한 yaml 별도 생성
토큰 테스트를 위한 별도의 유틸 클래스 생성
추후 필요한 내용 ToDo에 추가
ArtistService의 내용 추가. 에러 처리 구현이 현재 안되어있음.
JPA를 상속받는 StudentArtistRepositor 인터페이스 구현
ArtistController의 일부 기능 구현.
UserService 일부 구현
임시 로그인 E2E 테스트 구현
임시 Artist 테스트 구현
Weekly - 정기 병합
Develop - 정기 병합
leaf-upper
reviewed
Oct 6, 2024
|
|
||
| @SpringBootTest | ||
| @AutoConfigureMockMvc | ||
| public class ArtistE2Etest { |
There was a problem hiding this comment.
테스트 코드 잘작성해주신것 같습니다~ 다만 E2E 테스트는 정말 말그대로 EndPoint to EndPoint이기 때문에 유저레벨에서 해당 기능을 어떻게 사용하는지에 대한 테스트이기도하고, 1개의 API에 대해서만 테스트하는게 아니라, 연속적인 테스트가 필요할수도 있을것 같습니다. 그렇기에 코드에서 테스트하기 어려운 부분도 있을것 같아요. E2E Test 보다는 통합테스트 (integrationTest)가 조금더 적합할것 같습니다.
leaf-upper
reviewed
Oct 6, 2024
| @Component | ||
| public class JwtProvider implements InitializingBean { | ||
| @Value("${jwt.secret}") | ||
| private String secret; |
There was a problem hiding this comment.
요렇게 Value로 바인딩 받는것들은 프로퍼티로 묶어서 바인딩 받으면 조금더 코드가 깔끔해질것 같아요
leaf-upper
reviewed
Oct 6, 2024
| @Getter | ||
| @Builder | ||
| public class JwtToken { | ||
| private String accessToken; |
leaf-upper
reviewed
Oct 6, 2024
| private static final String IS_ACCESS_TOKEN = "isAccessToken"; | ||
| private static final String HEADER_PREFIX = "Bearer "; | ||
|
|
||
| public String parseHeader(String header) { |
There was a problem hiding this comment.
parseHeader는 JwtProvider와는 다른 ex HtthHeaderUtils 별도 클래스를 두고 해당 클래스에서 코드를 작성해도 좋을것 같아요.
leaf-upper
reviewed
Oct 6, 2024
| @Builder | ||
| @Getter | ||
| public class JwtUser implements UserDetails { | ||
| private Long id; |
leaf-upper
reviewed
Oct 6, 2024
| } | ||
|
|
||
| public static Category fromString(String name) { | ||
| System.out.println(name); |
There was a problem hiding this comment.
콘솔 출력보다는, Slf4j, Logback, Log4j2 같은 로깅툴로 로그를 남기시면 더 좋을것 같아요
leaf-upper
reviewed
Oct 6, 2024
| this.productRepository = productRepository; | ||
| } | ||
|
|
||
| public Product save(ProductRequest productSaveRequest) { |
There was a problem hiding this comment.
edit에는 @Transactional이 있는데 나머지 코드에는 없네요. DB 관련작업에는 추가해도 괜찮지 않을까요?
leaf-upper
approved these changes
Oct 6, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR
코드 리뷰 2회차 Pull Request 입니다!!! 잘 부탁드립니다: )
✨ 작업 내용
5주차 github 코드리뷰 질문
(윤재용) 몇몇 컨트롤러에 대한 E2E 테스트를 작성하였습니다. 처음에는 @WithMockUser 를 사용해서 테스트를 진행하려고 했는데, Header를 검증하다보니 불가능했습니다. 저희의 요구사항이 특정 url이 아니라면 헤더에 토큰이 필요하다보니 사용이 불가능하였기에 JwtTestUtils 클래스를 통해 테스트 유저를 사용하였습니다.
이런 전체 테스트를 처음 구현하다 보니 현재 작성한 테스트가 E2E 테스트라고 불려도 될 지 잘 모르겠습니다..! 추가로 테스트코드의 개선점이 있을지 궁금합니다.