-
Notifications
You must be signed in to change notification settings - Fork 3
[Board] 1차 제출입니다! #4
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
Conversation
회원 가입 시, jwt를 발급하여 바로 로그인 할 수 있도록 로직 구현
openAPI 를 이용한 swagger를 세팅하기 위해 버전 다운그레이드
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 사용과 매핑 관련해서 고민이 많으신 것 같아서 그 부분을 중점적으로 남겼고 추가적으로 구조에 대해서 리뷰를 남겼습니다.
그리고 N+1 문제에 대해서는 JPA의 동작 방식으로 어쩔 수 없이 생기는 문제인데요. 이를 없애기 위해서 매핑 방법(간접참조, 직접참조)을 바꾸는 건 좋지 않다고 생각합니다.
매핑을 하는 방식도 여러 방법이 있습니다. 각 방법을 혼용해서 쓰기도 하고 하나의 방식만 쓰기도 합니다. 개인적인 생각으로는 JPA를 사용하는 이유를 생각해보고 JPA에서 추구하는 방식은 무엇인지와 같은 등장 배경을 생각해보시면 JPA가 어떤 목적으로 사용되는지 이해를 할 수 있고, 그렇게 되면 JPA의 의도대로 사용하는 직접적인 연관관계 매핑도 이해할 수 있게 됩니다. domain과 entity 관점에서 고민해봐도 괜찮을 것 같습니다.
이 방식에 대해선 생각해보시고 저한테 설명 주시면 좋을 것 같아요~
JPA를 쓰면서 공부하다 보면 연관관계 매핑을 잘하라는 글도 보게되고 어떤 글은 간접참조가 좋다라는 글도 보고해서 혼동스러울 수 있는데요.
두 방식 당연히 모두 사용하기 때문에 장단점과 차이점을 아셔야합니다! 그리고 비즈니스 상황과 앞으로 프로젝트가 추구하는 방향에 따라서 방법을 선택하시는 게 중요하다고 생각합니다.
어쨋든 매핑을 하다보면 N+1이 발생하는데 이 경우 FetchJoin을 사용하셔도 되고 Querydsl 등등 다양하게 해결할 수 있는데, 차이점도 직접 공부해보시면 좋을 것 같습니다! 어쨋든 N+1 때문에 참조 방법을 바꾼다는 건 저는 개인적으로 좋지 않다고 생각하는 입장입니다.
수고 많으셨고 궁금하신 부분 있으면 언제든 편하게 연락주세요!
++ 오 그리고 테스트가 누락되어 있네요! 추가해주세요~
src/main/java/com/board/article/controller/ArticleController.java
Outdated
Show resolved
Hide resolved
src/main/java/com/board/article/exception/exceptions/ArticleException.java
Outdated
Show resolved
Hide resolved
src/main/java/com/board/member/controller/auth/AuthController.java
Outdated
Show resolved
Hide resolved
|
에고 실수로 Approve 눌렀네요! 이거 무시하시고 작업 쭉 해주시면 됩니다~ |
AuthService 에 있는 tokenProvider 로직 분리
sosow0212
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.
마지막 사소한 것들만 수정해주세요! 그리고 의견을 몇 가지 여쭤봤는데 답변 부탁드릴게요! 너무 수고 많으셨습니다 ㅎㅎ 정처기 sqld, 프로젝트 등등 아주 바쁘셨을텐데 고생하셨습니다 👍
docs/code_review.md
Outdated
| * 페이지 번호가 있는 프론트 구현 시, 잘 맞는다. | ||
|
|
||
| ## 단점 | ||
| * OFFSET 100000 같이 뒤 페이지로 갈수록 느려짐 → 인덱스가 있어도 느림 |
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.
다른 방법으로 개선할 수 있다고 하셔서 조금 찾아봤는데,
@Query 어노테이션을 통해 sql 문법을 작성하는 커서 기반 페이징이 맞을까요??
offset 은 처음부터 모든 column을 조회해서 속도가 느리지만 커서 기반 페이징은 특정 id 를 기준으로 해당 부분부터 조회하기에 빠르다는 것을 알았습니다..!
커서 기반 페이징이 개선할 수 있는 방식이라면, 다음 미션에서 해당 방식 적용해보겠습니다..!
| public List<Article> getAllArticles() { | ||
| return articleRepository.findAll(); | ||
| public List<Article> getAllArticles(Long lastId, int size) { | ||
| Pageable articlePageable = PageRequest.of(NO_OFFSET_PAGING_PAGE, size, Sort.by(PAGE_SORT_DELIMITER).descending()); |
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.
컨트롤러에서 pageable로 받으면 프론트측에서 사용할 수 있지 않나요?
프론트 정책에 따라 유연한 페이징 api를 사용하는 게 좋아 보일 것 같아서요!
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.
페이지 정보를 포함하여 반환하도록 로직 수정하였습니다..!
| Comment comment = new Comment(memberId, articleId, request.content()); | ||
| public Comment createComment(String content, Long memberId, Long articleId) { | ||
| Article article = articleService.getArticle(articleId); | ||
| Comment comment = new Comment(memberId, article, content); |
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.
pr 메시지에 Article과 Comment의 생명주기가 같다고 하셨는데, 삭제에 대해서만 같지 않을까요?
그러면 결국 제거 주기만 같을 뿐 생명주기가 같다고 할 순 없을 것 같아요! 결론적으론 Comment, Article 서로 다른 애그리거트라고 볼 수 있지 않을까요? 의존을 끊어낼 수 있어 보여서요! 혹시 다른 의도가 있다면 설명 부탁드릴게요~
| private final AuthArgumentResolver resolver; | ||
|
|
||
| @Override | ||
| public void addInterceptors(InterceptorRegistry registry) { |
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.
인터셉터에선 현재 GET 요청에 대해서만 인증/인가를 진행할 거 같은데요
GET은 모두에게 오픈하고 POST, PATCH 같은 건 로그인된 유저에게 노출하고 싶다면 인터셉터를 수정해야할 거 같아요
이 부분도 변경해주세요!
src/main/java/com/board/member/service/member/MemberService.java
Outdated
Show resolved
Hide resolved
sosow0212
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.
수고하셨습니다~ 여기서 머지하겠습니다!
다음에는 댓글, 게시글 작성시마다 포인트 정책을 도입해보는 건 어떨까요? 그리고 조회수와 추천도 도입해서 인기글 산정까지 해보면 좋을 것 같습니다~ 많은 고민을 하신게 느껴져서 좋았습니다 고생하셨습니다👍
| @Transactional | ||
| @EventListener | ||
| public void handleMemberDeletedEvent(ArticleDeletedEvent event) { | ||
| Long articleId = event.articleId(); | ||
|
|
||
| commentService.deleteCommentByArticle(articleId); | ||
| } |
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.
게시글이 제거되면 댓글은 비동기로 백단에서 제거되어도 괜찮지 않을까요?
- 그게 아니더라도 한 번 비동기로 처리해보시는 건 어떨까요? 처리하시면서 쓰레드풀 설정도 해보시고 쓰레드풀 설정은 어떤 방식으로 하는지, 어떤 원리인지 설명도 같이 해주시면 좋을 것 같아요~
EventListner, TransactionalEventlistener 차이도 학습해보시고요!
| int pageNumber, | ||
| int pageSize, | ||
| long totalElements, | ||
| int totalPages |
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.
int, long이면 db에서 꺼낼 때 NPE가 발생할 수 있지 않을까요? (여기선 사실 크게 그럴 일이 없어보이지만 혹시 모르니깐요 ㅎㅎ)
4/27
정말 오랜만에 PR 드리는 것 같습니다...!
4월 한 달 동안 자격증(sqld, 정처기 / 둘 다 붙은 거 같습니다 ㅋㅋ 개이득) 시험도 치루고 시험 공부도 하고 공모전 준비도 하고 작은 프로젝트도 했습니다.
제가 이 걸 말씀드리는 이유는.. 그냥 요새 이것 저것 하면서 형 생각 많이 났습니다 ㅋㅋ
형 덕분에 많은 것들을 배웠고, 항상 조언해주시고, 제가 막히는 부분도 잘 풀어주셨던 기억이 떠오르더라고요.
형 아니였다면, "갈피도 못 잡고 헤메고 있었을 텐데" 이런 생각이 가장 많이 들었습니다.
특히 공모전이나 작은 프로젝트에서 백엔드 파트를 혼자 맡아서 진행했는데, 헤메긴 해도 어찌 저찌 해냈습니다. 끝까지 해내고 나니까, 지금까지 배웠던 시간이 헛되지 않았다는 그 뿌듯함이 정말 컸고, 감사하다는 마음만 들었습니다.
써놓고 나니까 조금 오글거리긴 한 것 같은데, 형 덕분에 많이 성장했고, 정말 고마운 마음이 큽니다...!!
저도 형처럼 나눌 줄 아는 개발자가 되었으면 좋겠네요 ㅎㅎ
부담되지 않게 항상 감사하다는 말 전해드리고 싶었습니다!!..
항상 잘 되시길 기도하겠습니다. 그리고 항상 잘 되실겁니다...! 이재윤 파이팅!! 👍
[한 것]
ManyToOne,OneToMany,CasCade활용해서 기능 구현하였습니다.@Transaction을 통해 rollback 처리 되게 구현하였습니다.[해야할 것]
3/20
오랜만에 처음부터 만들어 보니, 헷갈리거나 까먹은 것들이 많아 정리하며 구현하였습니다.!
어느 정도 개념 잡혀서 더 빠르게 구현할 수 있었습니다.
[한 것]
[해야할 것]
@EntityGraph, 쿼리 사용, batchSize 설정 이 셋 중에 어떤 것으로 하는 것이 좋을까요!?!?