-
Notifications
You must be signed in to change notification settings - Fork 13
[Spring Core] 김성현 과제 제출입니다. #5
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: main
Are you sure you want to change the base?
Conversation
20HyeonsuLee
left a comment
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.
전체적으로 잘해주셨네요~ 몇가지 코멘트 남겼습니다 확인해주세요!
| driver-class-name: com.mysql.cj.jdbc.Driver | ||
| url: jdbc:mysql://localhost:3306/bcsd?serverTimezone=Asia/Seoul | ||
| username: root | ||
| password: kk25781179@@ |
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.
비밀번호 확보!
gitignore에 추가해주세용
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.
clone한 프로젝트여서 gitignore도 다시 해줘야한다는 것을 깜빡했네요 다음부터는 놓치지 않게 조심하겠습니다
| @@ -0,0 +1,31 @@ | |||
| package com.example.demo.exception; | |||
|
|
|||
| public enum BoardExceptionType implements BaseExceptionType { | |||
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.
BaseExceptionType, BoardExceptionType, CommonExceptionType을 enum으로 나눠서 선언한 이유가 궁금합니다!
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.
프로젝트를 개발하는 과정에서 지속적으로 확장이 일어날 것이고, 이를 고려했을 때 도메인 별로 에러코드를 다르게하고 다른 enum에서 관리함으로써 유지보수성을 높일 수 있다고 생각했습니다.
| public Board findById(Long id) { | ||
| return jdbcTemplate.queryForObject(""" | ||
| try { | ||
| return jdbcTemplate.queryForObject(""" |
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.
queryForObject로 처리하고 예외를 잡은 이유가 있을까요?
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.
Spring JDBC를 사용했을 때, DB처리단에서 findById를 처리하지 못한 경우 EmptyResultDataAccessException 예외가 발생하게 되는데, 이 경우에 null을 리턴함으로써 Service단에서 예외처리를 해주기 위해서 catch했습니다.
| if (!isValid(request)) throw new CustomException(CommonExceptionType.BAD_REQUEST_NULL_VALUE); | ||
|
|
||
| Member existedMember = memberRepository.findById(request.authorId()); | ||
| if (existedMember == null) throw new CustomException(PostExceptionType.INVALID_MEMBER_REFERENCE); |
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.
PostExceptionType을 static import해주면 코드가 더 깔끔해진답니다~
맥기준 PostExceptionType위에 커서를 올려두고 옵션엔터를 누른 후 static import누르면 됩니당
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.
static import의 존재는 알고 있었는데 생각을 못했네요.
사용해보니까 훨씬 코드가 깔끔해지는 것 같습니다
| return ArticleResponse.of(saved, member, board); | ||
| } | ||
|
|
||
| private boolean isValid(ArticleCreateRequest request) { |
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.
isValid메소드를 ArticleCreateRequest내부에 만드는건 별로일까요?
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.
BoardService나 MemberService에서는 내부에 만들었었는데, ArticleService에서는 isValid 로직 코드의 가독성이 한눈에 알아보기 쉽지 않아서 isValid로 모듈화를 통해 함수명으로 한눈에 동작을 알아볼 수 있도록하여 가독성을 높였습니다.
20HyeonsuLee
left a comment
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.
Good~ 잘해주셨네요!
| this.createdAt = LocalDateTime.now(); | ||
| this.modifiedAt = LocalDateTime.now(); |
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에 위임 굳~
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.
감사합니다!
20HyeonsuLee
left a comment
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.
수고하셨습니다~!
| List<Article> findAllByBoard_Id(Long boardId); | ||
|
|
||
| List<Article> findAllByAuthor_Id(Long authorId); |
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.
쿼리 메서드 규칙을 사용한 것이라 저렇게 적용했던 것 같습니다.
Article 엔티티에서 boardId와 authorId가 아닌 Board와 Author로 관리하게되어서 저렇게 적용하게 되었습니다.
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.
헷갈릴 만한 여지가 있으니, 다음부터는 JPQL을 이용하여 작성하도록 하겠습니다.
| driver-class-name: com.mysql.cj.jdbc.Driver | ||
| url: jdbc:mysql://localhost:3306/bcsd?serverTimezone=Asia/Seoul | ||
| username: root | ||
| password: kk25781179@@ |
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.
clone한 프로젝트여서 gitignore도 다시 해줘야한다는 것을 깜빡했네요 다음부터는 놓치지 않게 조심하겠습니다
| @@ -0,0 +1,31 @@ | |||
| package com.example.demo.exception; | |||
|
|
|||
| public enum BoardExceptionType implements BaseExceptionType { | |||
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.
프로젝트를 개발하는 과정에서 지속적으로 확장이 일어날 것이고, 이를 고려했을 때 도메인 별로 에러코드를 다르게하고 다른 enum에서 관리함으로써 유지보수성을 높일 수 있다고 생각했습니다.
| public Board findById(Long id) { | ||
| return jdbcTemplate.queryForObject(""" | ||
| try { | ||
| return jdbcTemplate.queryForObject(""" |
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.
Spring JDBC를 사용했을 때, DB처리단에서 findById를 처리하지 못한 경우 EmptyResultDataAccessException 예외가 발생하게 되는데, 이 경우에 null을 리턴함으로써 Service단에서 예외처리를 해주기 위해서 catch했습니다.
| return ArticleResponse.of(saved, member, board); | ||
| } | ||
|
|
||
| private boolean isValid(ArticleCreateRequest request) { |
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.
BoardService나 MemberService에서는 내부에 만들었었는데, ArticleService에서는 isValid 로직 코드의 가독성이 한눈에 알아보기 쉽지 않아서 isValid로 모듈화를 통해 함수명으로 한눈에 동작을 알아볼 수 있도록하여 가독성을 높였습니다.
| if (!isValid(request)) throw new CustomException(CommonExceptionType.BAD_REQUEST_NULL_VALUE); | ||
|
|
||
| Member existedMember = memberRepository.findById(request.authorId()); | ||
| if (existedMember == null) throw new CustomException(PostExceptionType.INVALID_MEMBER_REFERENCE); |
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.
static import의 존재는 알고 있었는데 생각을 못했네요.
사용해보니까 훨씬 코드가 깔끔해지는 것 같습니다
| this.createdAt = LocalDateTime.now(); | ||
| this.modifiedAt = LocalDateTime.now(); |
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.
감사합니다!
| List<Article> findAllByBoard_Id(Long boardId); | ||
|
|
||
| List<Article> findAllByAuthor_Id(Long authorId); |
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.
쿼리 메서드 규칙을 사용한 것이라 저렇게 적용했던 것 같습니다.
Article 엔티티에서 boardId와 authorId가 아닌 Board와 Author로 관리하게되어서 저렇게 적용하게 되었습니다.
| List<Article> findAllByBoard_Id(Long boardId); | ||
|
|
||
| List<Article> findAllByAuthor_Id(Long authorId); |
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.
헷갈릴 만한 여지가 있으니, 다음부터는 JPQL을 이용하여 작성하도록 하겠습니다.
| format_sql: true | ||
|
|
||
| jwt: | ||
| secret: cf42d45e2e99cded4137f7307acec9b399b0de326f9b47a8115ca634bf7ed35113c33ae538a8a1f4bb7ff681726ad80b53509283a0e6e014e047b63ab662b352 |
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.
gitignore했는데 예전에 등록됐던 부분이어서 양해 부탁드립니다.
20HyeonsuLee
left a comment
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.
이번 과제도 수고하셨습니다~!
| @Value("${jwt.secret}") | ||
| private String secretKey; | ||
| private Key key; | ||
| private final long accessTokenValidTime = (60 * 1000) * 30; |
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.
적용하겠습니다 감사합니다.
| public class JwtTokenProvider { | ||
|
|
||
| @Value("${jwt.secret}") | ||
| private String secretKey; |
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이 있어야할것같네요
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.
secretKey를 @PostConstruct를 사용하여 init하기 위하여 final 키워드는 사용하지 않았습니다.
11주차 과제 제출입니다. (같은 PR, 변경사항은 commit) JWT를 선택해서 적용했습니다.