키워드별 추천 포스트 어드민 기능을 구현한다 #1470
키워드별 추천 포스트 어드민 기능을 구현한다 #1470
Conversation
| import org.springframework.data.jpa.repository.JpaRepository; | ||
| import wooteco.prolog.roadmap.domain.RecommendedPost; | ||
|
|
||
| public interface RecommendedRepository extends JpaRepository<RecommendedPost, Long> { |
There was a problem hiding this comment.
대상 엔티티가 RecommendedPost니까 repository의 네이밍을 RecommendedPostRespository라고 짓는 게 조금 더 명확할 것 같아요!
| return ResponseEntity.created( | ||
| URI.create("/keywords/" + keywordId + "/recommended-posts/" + id)).build(); |
There was a problem hiding this comment.
URI에 해당하는 경로에 대한 API가 없어서 단순히 status만 CREATED 하거나, 혹은 GET으로 받아오는 API가 필요할 것 같아요!
There was a problem hiding this comment.
굿굿 쓰이지 않는 API 추가보다는 201만 리턴하도록 할게요!
| @PathVariable("recommendedId") Long recommendedId, | ||
| @RequestBody RecommendedUpdateRequest request) { | ||
| recommendedService.update(recommendedId, request); | ||
| return ResponseEntity.ok().build(); |
There was a problem hiding this comment.
update의 경우에도 no_content를 적용할 수 있지 않을까요?
| @AllArgsConstructor | ||
| public class RecommendedUpdateRequest { | ||
|
|
||
| private String url; |
There was a problem hiding this comment.
@NotEmpty 같은 제약 조건이나 혹은 Url에 대한 패턴 매칭이 필요할 것 같아요!
현재는 null이 들어와도 그대로 db에 null이 들어갈 수 있어서...!
| @AllArgsConstructor | ||
| public class RecommendedRequest { | ||
|
|
||
| private String url; |
There was a problem hiding this comment.
여기도 마찬가지로 @NotEmpty 같은 제약 조건이 필요할 것 같습니다!
| recommendedPost.remove(); | ||
| recommendedRepository.delete(recommendedPost); |
There was a problem hiding this comment.
여기서 remove() 호출 후 delete를 하시는 이유가 따로 있을까요?!
There was a problem hiding this comment.
없습니다!
필요 없는 로직들을 많이 짜뒀네요...뇌 빼고 했던 것 같습니다.
모두 제거했어요!
| @CollectionTable(name = "keyword_reference") | ||
| @Column(name = "url") | ||
| private List<String> references; | ||
| @OneToMany(mappedBy = "keyword", cascade = CascadeType.ALL, orphanRemoval = true, fetch = FetchType.EAGER) |
There was a problem hiding this comment.
현재 Keyword 객체를 쓸 때는 항상 RecommendedPost들이 필요합니다!
따라서 조회할 때 한 번에 조회문을 다 날리려고 했습니다.
위와는 별개 내용으로,
처음에는 주체적으로 동작하는 상황이 생길 것 같아서 독립적인 비 식별 관계의 Entity로 만들었습니다.
사실 지금 생각해보면 Embedded로 해서 식별 객체로 만드는 것이 나았던 것 같습니다.
앞으로 바뀌는 로드맵에서 어떤 식으로 response를 주고 할 지를 보니 그럴 일이 없을 것 같다고 생각해요...
There was a problem hiding this comment.
EAGER를 사용하게 되면 나중에 연관관계가 복잡해질 때, 알 수 없는 쿼리들이 많이 나가는 것으로 알고 있어요.
저도 옛날에 EAGER 쓰는데 막 UNION쓰고, 제가 원하지 않는 쿼리들이 막 나가서 지양하고 있습니다.
혹시 나가는 쿼리들을 확인해보신건지 궁금해요.
말 그대로 EAGER는 "Keyword를 조회할 때 한번에 가져온다" 라는건 맞지만, 이 안에서 쿼리들이 어떻게 나갈지 모르기 때문에
직접 쿼리를 명시해줘서 가져오는게 좋지 않나 생각이 들어요.
There was a problem hiding this comment.
Lazy로 바꾸고 Fetch Join 하도록 변경했습니다.
다만 Bag 타입의 경우 여러 Fetch Join을 한 번에 할 수가 없기 때문에 Set를 가지도록해서 한 번에 모두 가져왔어요.
근데 Fetch Join이 많아지면서 Cartesian Product가 발생하는데, 앞으로 성능상의 이슈가 발생한다면 고도화를 해야할 것 같습니다.
지금 생각나는 방법은 Fetch Join을 한 번씩 해오면 더 빠를 수도 있을 것 같다고 생각해요.
Fetch Join을 한 번 해오고 나머지 Fetch Join은 In 절로 가져온다면 디비로 SQL을 플러시하는 횟수는 증가하지만 Cartesian Product는 발생하지 않으니까요!
| final Keyword keyword = findKeywordOrThrow(keywordId); | ||
|
|
||
| final RecommendedPost post = new RecommendedPost(request.getUrl()); | ||
| post.addKeyword(keyword); |
There was a problem hiding this comment.
RecommendedPost 를 생성할 때 keyword를 같이 넣어주고 save하면 될 것 같은데
Post에 add를 해주고나서 save하신 이유가 궁금해요
There was a problem hiding this comment.
꼼꼼한 리뷰 감사합니다!
생성자를 왜 url로만 했을까요...?
| } | ||
|
|
||
| @PostMapping | ||
| public ResponseEntity<Void> createRecommendedPost(@PathVariable("keywordId") Long keywordId, |
There was a problem hiding this comment.
컨벤션 때 final 붙이기로 한 것 같아요 ~
- id 필드 추가 #1397
nuyh99
force-pushed
the
feat/1397-recommended-posts
branch
from
bf1d8ea to
7124da0
Compare
|
SonarCloud Quality Gate failed.
|
#️⃣연관된 이슈
📝작업 내용