Repository navigation
refactor: 소식지 단일 좋아요 여부 확인 API 제거 #455
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
Changes from 5 commits
c80d697
5260235
dbcde23
b036eb3
80634a1
dd1873e
452b4c7
c86ebac
214be6b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,9 @@ | ||
| package com.example.solidconnection.news.dto; | ||
|
|
||
| import static com.fasterxml.jackson.annotation.JsonInclude.Include.NON_NULL; | ||
|
|
||
| import com.example.solidconnection.news.domain.News; | ||
| import com.fasterxml.jackson.annotation.JsonInclude; | ||
| import java.time.ZonedDateTime; | ||
|
|
||
| public record NewsResponse( | ||
|
|
@@ -9,16 +12,21 @@ public record NewsResponse( | |
| String description, | ||
| String thumbnailUrl, | ||
| String url, | ||
|
|
||
| @JsonInclude(NON_NULL) | ||
| Boolean isLike, | ||
|
|
||
| ZonedDateTime updatedAt | ||
| ) { | ||
|
|
||
| public static NewsResponse from(News news) { | ||
| public static NewsResponse from(News news, Boolean isLike) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 인자가 2개 이상이 되었으므로, of 로 정적 팩터리 메서드 이름을 변경하는게 좋겠습니다~
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 반영했습니다! 214be6b |
||
| return new NewsResponse( | ||
| news.getId(), | ||
| news.getTitle(), | ||
| news.getDescription(), | ||
| news.getThumbnailUrl(), | ||
| news.getUrl(), | ||
| isLike, | ||
| news.getUpdatedAt() | ||
| ); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,23 @@ | ||
| package com.example.solidconnection.news.repository; | ||
|
|
||
| import com.example.solidconnection.news.domain.LikedNews; | ||
| import java.util.List; | ||
| import java.util.Optional; | ||
| import java.util.Set; | ||
| import org.springframework.data.jpa.repository.JpaRepository; | ||
| import org.springframework.data.jpa.repository.Query; | ||
| import org.springframework.data.repository.query.Param; | ||
|
|
||
| public interface LikedNewsRepository extends JpaRepository<LikedNews, Long> { | ||
|
|
||
| boolean existsByNewsIdAndSiteUserId(long newsId, long siteUserId); | ||
|
|
||
| Optional<LikedNews> findByNewsIdAndSiteUserId(long newsId, long siteUserId); | ||
|
|
||
| @Query(""" | ||
| SELECT l.newsId | ||
| FROM LikedNews l | ||
| WHERE l.newsId IN :newsIds AND l.siteUserId = :siteUserId | ||
| """) | ||
| Set<Long> findLikedNewsIdsByNewsIdsAndSiteUserId(@Param("newsIds") List<Long> newsIds, @Param("siteUserId") Long siteUserId); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,8 +3,10 @@ | |
| import com.example.solidconnection.news.domain.News; | ||
| import com.example.solidconnection.news.dto.NewsListResponse; | ||
| import com.example.solidconnection.news.dto.NewsResponse; | ||
| import com.example.solidconnection.news.repository.LikedNewsRepository; | ||
| import com.example.solidconnection.news.repository.NewsRepository; | ||
| import java.util.List; | ||
| import java.util.Set; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.stereotype.Service; | ||
| import org.springframework.transaction.annotation.Transactional; | ||
|
|
@@ -14,13 +16,33 @@ | |
| public class NewsQueryService { | ||
|
|
||
| private final NewsRepository newsRepository; | ||
| private final LikedNewsRepository likedNewsRepository; | ||
|
|
||
| @Transactional(readOnly = true) | ||
| public NewsListResponse findNewsBySiteUserId(long siteUserId) { | ||
| List<News> newsList = newsRepository.findAllBySiteUserIdOrderByUpdatedAtDesc(siteUserId); | ||
| public NewsListResponse findNewsByAuthorId(Long siteUserId, long authorId) { | ||
| List<News> newsList = newsRepository.findAllBySiteUserIdOrderByUpdatedAtDesc(authorId); | ||
|
Gyuhyeok99 marked this conversation as resolved.
Outdated
|
||
|
|
||
| // 로그인하지 않은 경우 | ||
| if (siteUserId == null) { | ||
| List<NewsResponse> newsResponseList = newsList.stream() | ||
| .map(news -> NewsResponse.from(news, null)) | ||
| .toList(); | ||
| return NewsListResponse.from(newsResponseList); | ||
| } | ||
|
|
||
| // 로그인한 경우 | ||
| List<Long> newsIds = newsList.stream() | ||
| .map(News::getId) | ||
| .toList(); | ||
|
|
||
| Set<Long> likedNewsIds = likedNewsRepository.findLikedNewsIdsByNewsIdsAndSiteUserId(newsIds, siteUserId); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 쿼리가 두 번만 발생하긴 하지만, JPQL을 사용하면 한 번으로 줄일 수 있을 것 같습니다. 어차피
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. DTO Projection을 사용해야할까요!? LikedNews랑 News를 한 번에 가져오는 걸 말씀하신거죠??
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 네 맞습니다 ! 아 그럼 QueryDSL을 사용해야겠네요
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 음 jpql이나 QueryDSL로 둘 다 가능할 거 같긴한데 쿼리 2개를 1개로 줄이려고 join과 함께 DTO Projection을 쓰는 게 맞을까란 생각도 들긴하네요
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 우선은 그렇게 바꿔보겠습니다~
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
저도 딱 이 생각입니다 ... ㅋㅋㅋ 한 번 구현해보신다면 해 보시고 배보다 배꼽이 더 큰 것 같다 하시면 그대로 가도 좋을 것 같습니다 ..!
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 저도 성혁님과 마찬가지로 규혁님의 판단에 맡기겠습니다 ㅎㅎ 참고로 '나라면 어떻게 했을까?'에 대해 생각해보자면,..
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 반영했습니다 ~ dd1873e |
||
| List<NewsResponse> newsResponseList = newsList.stream() | ||
| .map(NewsResponse::from) | ||
| .map(news -> { | ||
| Boolean isLike = likedNewsIds.contains(news.getId()); | ||
| return NewsResponse.from(news, isLike); | ||
| }) | ||
| .toList(); | ||
|
|
||
| return NewsListResponse.from(newsResponseList); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| package com.example.solidconnection.news.fixture; | ||
|
|
||
| import com.example.solidconnection.news.domain.LikedNews; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.boot.test.context.TestComponent; | ||
|
|
||
| @TestComponent | ||
| @RequiredArgsConstructor | ||
| public class LikedNewsFixture { | ||
|
|
||
| private final LikedNewsFixtureBuilder likedNewsFixtureBuilder; | ||
|
|
||
| public LikedNews 소식지_좋아요(long newsId, long siteUserId) { | ||
| return likedNewsFixtureBuilder.likedNews() | ||
| .newsId(newsId) | ||
| .siteUserId(siteUserId) | ||
| .create(); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| package com.example.solidconnection.news.fixture; | ||
|
|
||
| import com.example.solidconnection.news.domain.LikedNews; | ||
| import com.example.solidconnection.news.repository.LikedNewsRepository; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.boot.test.context.TestComponent; | ||
|
|
||
| @TestComponent | ||
| @RequiredArgsConstructor | ||
| public class LikedNewsFixtureBuilder { | ||
|
|
||
| private final LikedNewsRepository likedNewsRepository; | ||
|
|
||
| private long newsId; | ||
|
|
||
| private long siteUserId; | ||
|
|
||
| public LikedNewsFixtureBuilder likedNews() { | ||
| return new LikedNewsFixtureBuilder(likedNewsRepository); | ||
| } | ||
|
|
||
| public LikedNewsFixtureBuilder newsId(long newsId) { | ||
| this.newsId = newsId; | ||
| return this; | ||
| } | ||
|
|
||
| public LikedNewsFixtureBuilder siteUserId(long siteUserId) { | ||
| this.siteUserId = siteUserId; | ||
| return this; | ||
| } | ||
|
|
||
| public LikedNews create() { | ||
| LikedNews likedNews = new LikedNews(newsId, siteUserId); | ||
| return likedNewsRepository.save(likedNews); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,12 +6,15 @@ | |
| import com.example.solidconnection.news.domain.News; | ||
| import com.example.solidconnection.news.dto.NewsListResponse; | ||
| import com.example.solidconnection.news.dto.NewsResponse; | ||
| import com.example.solidconnection.news.fixture.LikedNewsFixture; | ||
| import com.example.solidconnection.news.fixture.NewsFixture; | ||
| import com.example.solidconnection.siteuser.domain.SiteUser; | ||
| import com.example.solidconnection.siteuser.fixture.SiteUserFixture; | ||
| import com.example.solidconnection.support.TestContainerSpringBootTest; | ||
| import java.util.Comparator; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.stream.Collectors; | ||
| import org.junit.jupiter.api.DisplayName; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.springframework.beans.factory.annotation.Autowired; | ||
|
|
@@ -29,25 +32,66 @@ class NewsQueryServiceTest { | |
| @Autowired | ||
| private NewsFixture newsFixture; | ||
|
|
||
| @Autowired | ||
| private LikedNewsFixture likedNewsFixture; | ||
|
|
||
| @Test | ||
| void 특정_사용자의_소식지_목록을_성공적으로_조회한다() { | ||
| void 로그인하지_않은_사용자가_특정_사용자의_소식지_목록을_성공적으로_조회한다() { | ||
| // given | ||
| SiteUser user1 = siteUserFixture.멘토(1, "mentor1"); | ||
| SiteUser user2 = siteUserFixture.멘토(2, "mentor2"); | ||
| News news1 = newsFixture.소식지(user1.getId()); | ||
| News news2 = newsFixture.소식지(user1.getId()); | ||
| newsFixture.소식지(user2.getId()); | ||
| SiteUser author = siteUserFixture.멘토(1, "author"); | ||
| SiteUser otherUser = siteUserFixture.멘토(2, "other"); | ||
|
|
||
| News news1 = newsFixture.소식지(author.getId()); | ||
| News news2 = newsFixture.소식지(author.getId()); | ||
| newsFixture.소식지(otherUser.getId()); | ||
| List<News> newsList = List.of(news1, news2); | ||
|
|
||
| // when | ||
| NewsListResponse response = newsQueryService.findNewsBySiteUserId(user1.getId()); | ||
| NewsListResponse response = newsQueryService.findNewsByAuthorId(null, author.getId()); | ||
|
|
||
| // then | ||
| assertAll( | ||
| () -> assertThat(response.newsResponseList()).hasSize(newsList.size()), | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. then 절에서 size 가 일치하는지 검증하는 것보다,
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 저도 영서님과 같은 생각입니다!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 반영했습니다! c86ebac |
||
| () -> assertThat(response.newsResponseList()) | ||
| .extracting(NewsResponse::updatedAt) | ||
| .isSortedAccordingTo(Comparator.reverseOrder()), | ||
| () -> assertThat(response.newsResponseList()) | ||
| .extracting(NewsResponse::isLike) | ||
| .containsOnly((Boolean) null) | ||
| ); | ||
| } | ||
|
|
||
| @Test | ||
| void 로그인한_사용자가_특정_사용자의_소식지_목록을_성공적으로_조회한다() { | ||
| // given | ||
| SiteUser author = siteUserFixture.멘토(1, "author"); | ||
| SiteUser loginUser = siteUserFixture.멘토(2, "loginUser"); | ||
|
|
||
| News news1 = newsFixture.소식지(author.getId()); | ||
| News news2 = newsFixture.소식지(author.getId()); | ||
| News news3 = newsFixture.소식지(author.getId()); | ||
|
|
||
| likedNewsFixture.소식지_좋아요(news1.getId(), loginUser.getId()); | ||
| likedNewsFixture.소식지_좋아요(news3.getId(), loginUser.getId()); | ||
|
|
||
| List<News> newsList = List.of(news1, news2, news3); | ||
|
|
||
| // when | ||
| NewsListResponse response = newsQueryService.findNewsByAuthorId(loginUser.getId(), author.getId()); | ||
|
|
||
| // then | ||
| assertAll( | ||
| () -> assertThat(response.newsResponseList()).hasSize(newsList.size()), | ||
| () -> assertThat(response.newsResponseList()) | ||
| .extracting(NewsResponse::updatedAt) | ||
| .isSortedAccordingTo(Comparator.reverseOrder()) | ||
| .isSortedAccordingTo(Comparator.reverseOrder()), | ||
| () -> { | ||
| Map<Long, Boolean> likeStatusMap = response.newsResponseList().stream() | ||
| .collect(Collectors.toMap(NewsResponse::id, NewsResponse::isLike)); | ||
| assertThat(likeStatusMap.get(news1.getId())).isTrue(); | ||
| assertThat(likeStatusMap.get(news2.getId())).isFalse(); | ||
| assertThat(likeStatusMap.get(news3.getId())).isTrue(); | ||
| } | ||
| ); | ||
| } | ||
| } | ||
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.
흠.. 지금 전체적인 코드에 '좋아요 했음'이
isLiked와isLike가 혼용되고 있네요🤔통일하는게 좋아보이는데, 둘 중 뭐가 더 좋다 생각하시나요?
(디스코드에도 질문 올려두겠습니다.)
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.
isLiked로 결정된 거 같아서 반영하였습니다~ 452b4c7