refactor: 소식지 단일 좋아요 여부 확인 API 제거 #455
Conversation
- 로그인하지 않은 사용자에게는 isLike를 응답하지 않도록
- 로그인한 사용자와 로그인하지 않은 사용자를 구분 - @Authorizeduser(required = false) 활용 - Set을 활용하여 n + 1 문제 해결
Walkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/main/java/com/example/solidconnection/news/service/NewsQueryService.java (2)
34-38: 빈 목록 최적화: IN () 쿼리 방지로 DB 호출을 단축하세요.
- 조회된 뉴스가 0건일 때도
IN (:ids)형태의 쿼리가 호출될 수 있습니다.
2) 일부 JPA 구현/DB에서는 빈 IN 절이 문제를 일으키거나 불필요한 호출이 됩니다.
3) newsIds가 비면 즉시 빈 응답을 반환하도록 단축하면, 안정성과 성능 모두 개선됩니다.아래와 같이 조기 반환을 추가해 주세요.
List<Long> newsIds = newsList.stream() .map(News::getId) .toList(); - Set<Long> likedNewsIds = likedNewsRepository.findLikedNewsIdsByNewsIdsAndSiteUserId(newsIds, siteUserId); + if (newsIds.isEmpty()) { + return NewsListResponse.from( + newsList.stream() + .map(news -> NewsResponse.from(news, Boolean.FALSE)) + .toList() + ); + } + + Set<Long> likedNewsIds = likedNewsRepository.findLikedNewsIdsByNewsIdsAndSiteUserId(newsIds, siteUserId);
22-23: siteUserId → authorId 전환 방향은 타당합니다. 다만 전체 일관성 점검을 권장합니다.
- 리소스 소유자 식별자(authorId)와 호출자 식별자(siteUserId)를 분리한 설계는 API 의미론 측면에서 명확합니다.
2) Controller, Repository, Entity 필드, API 문서(author-id 파라미터)까지 네이밍/유효성 제약이 일관되게 반영되었는지 최종 확인해 주세요.
3) 향후 권한 정책이 변경되더라도, 호출자와 리소스 소유자 분리는 확장성 측면에서 유리합니다.필요하시면 전체 호출경로를 스캔해 authorId/siteUserId 혼재 지점을 찾아 드릴게요.
src/test/java/com/example/solidconnection/news/fixture/LikedNewsFixtureBuilder.java (2)
18-21: likedNews()가 새 Builder를 반환하는 방식은 안전하지만, API 의도를 더 분명히 해봅시다.
- 현재도 상태 공유는 없지만, 메서드명이 상태 리셋을 직관적으로 드러내지 않습니다.
2) newBuilder() 같은 명칭 또는 static 팩토리(예: LikedNewsFixtureBuilder.newBuilder(repo))가 가독성을 더 높일 수 있습니다.- public LikedNewsFixtureBuilder likedNews() { - return new LikedNewsFixtureBuilder(likedNewsRepository); - } + public LikedNewsFixtureBuilder newBuilder() { + return new LikedNewsFixtureBuilder(likedNewsRepository); + }추가로, 사용하는 쪽에서도 호출을 다음처럼 정리하면 의도가 더 분명해집니다.
- before: likedNewsFixtureBuilder.likedNews().newsId(...).siteUserId(...).create()
- after: likedNewsFixtureBuilder.newBuilder().newsId(...).siteUserId(...).create()
32-35: 필수 필드 유효성 검증을 추가해 테스트 실수를 초기에 잡아냅시다.
- newsId/siteUserId가 설정되지 않은 채(기본값 0) 생성되면, 테스트가 잠재적 오염 상태로 진행될 수 있습니다.
2) create()에서 사전 검증을 추가하면, 문제를 즉시 드러내고 디버깅 비용을 절감합니다.public LikedNews create() { - LikedNews likedNews = new LikedNews(newsId, siteUserId); - return likedNewsRepository.save(likedNews); + if (newsId <= 0) { + throw new IllegalStateException("newsId must be set (> 0) before create()"); + } + if (siteUserId <= 0) { + throw new IllegalStateException("siteUserId must be set (> 0) before create()"); + } + LikedNews likedNews = new LikedNews(newsId, siteUserId); + return likedNewsRepository.save(likedNews); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
src/main/java/com/example/solidconnection/news/controller/NewsController.java(1 hunks)src/main/java/com/example/solidconnection/news/dto/LikedNewsResponse.java(0 hunks)src/main/java/com/example/solidconnection/news/dto/NewsResponse.java(2 hunks)src/main/java/com/example/solidconnection/news/repository/LikedNewsRepository.java(1 hunks)src/main/java/com/example/solidconnection/news/service/NewsLikeService.java(0 hunks)src/main/java/com/example/solidconnection/news/service/NewsQueryService.java(2 hunks)src/test/java/com/example/solidconnection/news/fixture/LikedNewsFixture.java(1 hunks)src/test/java/com/example/solidconnection/news/fixture/LikedNewsFixtureBuilder.java(1 hunks)src/test/java/com/example/solidconnection/news/service/NewsLikeServiceTest.java(0 hunks)src/test/java/com/example/solidconnection/news/service/NewsQueryServiceTest.java(2 hunks)
💤 Files with no reviewable changes (3)
- src/test/java/com/example/solidconnection/news/service/NewsLikeServiceTest.java
- src/main/java/com/example/solidconnection/news/service/NewsLikeService.java
- src/main/java/com/example/solidconnection/news/dto/LikedNewsResponse.java
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/test/java/com/example/solidconnection/news/fixture/LikedNewsFixture.java (1)
src/test/java/com/example/solidconnection/news/fixture/LikedNewsFixtureBuilder.java (1)
TestComponent(8-36)
src/test/java/com/example/solidconnection/news/fixture/LikedNewsFixtureBuilder.java (1)
src/test/java/com/example/solidconnection/news/fixture/LikedNewsFixture.java (1)
TestComponent(7-19)
🔇 Additional comments (10)
src/main/java/com/example/solidconnection/news/repository/LikedNewsRepository.java (1)
17-22: JPQL 쿼리가 올바르게 작성되었습니다!뉴스 ID 목록과 사용자 ID를 기반으로 좋아요한 뉴스 ID들을 효율적으로 조회하는 쿼리입니다. IN 절을 사용하여 여러 뉴스 ID를 한 번에 조회할 수 있어 성능상 이점이 있습니다.
src/test/java/com/example/solidconnection/news/fixture/LikedNewsFixture.java (1)
13-18: 테스트 픽스처가 간결하게 구현되었습니다!빌더 패턴을 활용하여 좋아요 데이터를 생성하는 코드가 깔끔합니다. 메서드명을 한글로 작성한 것도 테스트 코드의 가독성을 높이는 좋은 선택입니다.
src/test/java/com/example/solidconnection/news/service/NewsQueryServiceTest.java (2)
39-62: 비로그인 사용자 테스트가 명확하게 작성되었습니다!비로그인 사용자(null)의 경우 모든
isLike값이 null로 반환되는 것을 정확히 검증하고 있습니다. 또한 정렬 순서도 함께 확인하여 API 명세를 철저히 테스트하고 있습니다.
88-94: 좋아요 상태 검증 로직이 우수합니다!Map을 활용하여 각 뉴스별 좋아요 상태를 명확하게 검증하는 방식이 효과적입니다. news1과 news3는 좋아요가 true, news2는 false로 정확히 구분되어 테스트되고 있습니다.
src/main/java/com/example/solidconnection/news/controller/NewsController.java (1)
38-44: API 변경이 요구사항에 맞게 잘 구현되었습니다!로그인하지 않은 사용자도 소식지 목록을 조회할 수 있도록
@AuthorizedUser(required = false)를 사용한 것이 적절합니다.author-id파라미터를 통해 특정 작성자의 소식지를 조회하는 방식으로 변경된 것도 명확합니다.src/main/java/com/example/solidconnection/news/dto/NewsResponse.java (2)
16-17:@JsonInclude(NON_NULL)어노테이션 사용이 적절합니다!비로그인 사용자의 경우
isLike필드가 응답에서 제외되도록 하는 설계가 깔끔합니다. Boolean 타입을 사용하여 null 값을 허용한 것도 좋은 선택입니다.
22-32: 팩토리 메서드 시그니처 변경이 적절합니다!
from메서드에isLike파라미터를 추가하여 좋아요 상태를 외부에서 주입받도록 한 설계가 깨끗합니다. 이를 통해 서비스 레이어에서 좋아요 상태를 계산한 후 DTO로 변환할 수 있게 되었습니다.src/main/java/com/example/solidconnection/news/service/NewsQueryService.java (2)
22-31: 비로그인 분기 설계 적합합니다.
- 게스트 사용자의 경우 isLike를 null로 유지하는 결정이 요구사항과 정확히 일치합니다.
2) 스트림으로 원본 정렬(updatedAt desc)을 그대로 유지해 직렬화 순서 안정성도 보장됩니다.
3) DTO의 @JsonInclude(NON_NULL) 전략과도 합치되어, 응답 페이로드가 군더더기 없이 내려갑니다.
38-46: 좋아요 집합(Set)으로의 포함 여부 체크는 적절합니다.
- Set 기반 contains는 O(1) 접근으로 목록 매핑 시 효율적입니다.
2) 로그인 사용자의 경우 true/false를 명시적으로 내려주므로, 클라이언트 단 로직 단순화에도 도움이 됩니다.src/test/java/com/example/solidconnection/news/fixture/LikedNewsFixtureBuilder.java (1)
8-10: 테스트 빌더 컴포넌트 도입 좋습니다.
- @TestComponent + @requiredargsconstructor로 주입 간결성 확보가 좋습니다.
2) 픽스처 계층(LikedNewsFixture)에서 위임해 재사용성이 높아졌습니다.
src/main/java/com/example/solidconnection/news/service/NewsQueryService.java
Outdated
Show resolved
Hide resolved
whqtker
left a comment
whqtker
left a comment
There was a problem hiding this comment.
고생하셨습니다 ~ 사소한 부분만 코멘트 달았습니다 !
| .map(News::getId) | ||
| .toList(); | ||
|
|
||
| Set<Long> likedNewsIds = likedNewsRepository.findLikedNewsIdsByNewsIdsAndSiteUserId(newsIds, siteUserId); |
There was a problem hiding this comment.
쿼리가 두 번만 발생하긴 하지만, JPQL을 사용하면 한 번으로 줄일 수 있을 것 같습니다. 어차피 SELECT 쿼리니까 DB - 영속성 컨텍스트 불일치 문제도 신경 안 써도 되구요 !
There was a problem hiding this comment.
DTO Projection을 사용해야할까요!? LikedNews랑 News를 한 번에 가져오는 걸 말씀하신거죠??
There was a problem hiding this comment.
네 맞습니다 ! 아 그럼 QueryDSL을 사용해야겠네요
There was a problem hiding this comment.
음 jpql이나 QueryDSL로 둘 다 가능할 거 같긴한데 쿼리 2개를 1개로 줄이려고 join과 함께 DTO Projection을 쓰는 게 맞을까란 생각도 들긴하네요
There was a problem hiding this comment.
우선은 그렇게 바꿔보겠습니다~
There was a problem hiding this comment.
음 jpql이나 QueryDSL로 둘 다 가능할 거 같긴한데 쿼리 2개를 1개로 줄이려고 join과 함께 DTO Projection을 쓰는 게 맞을까란 생각도 들긴하네요
저도 딱 이 생각입니다 ... ㅋㅋㅋ 한 번 구현해보신다면 해 보시고 배보다 배꼽이 더 큰 것 같다 하시면 그대로 가도 좋을 것 같습니다 ..!
There was a problem hiding this comment.
저도 성혁님과 마찬가지로 규혁님의 판단에 맡기겠습니다 ㅎㅎ
참고로 '나라면 어떻게 했을까?'에 대해 생각해보자면,..
저라면 커스텀 레포지토리를 만들어서 querydsl은 사용하지 않고 jpql 프로젝션으로만 구현했을 것 같습니다!
querydsl을 사용할 정도의 복잡도는 아닌 것 같고,
join 해와서 나쁠게 없다고 생각하고,
"조회용"이라는 특정한 목적을 가졌으므로 별도의 클래스로 분리하는게 좋을 것 같다는 생각이 드네요😊
| ) { | ||
|
|
||
| public static NewsResponse from(News news) { | ||
| public static NewsResponse from(News news, Boolean isLike) { |
There was a problem hiding this comment.
인자가 2개 이상이 되었으므로, of 로 정적 팩터리 메서드 이름을 변경하는게 좋겠습니다~
| @JsonInclude(NON_NULL) | ||
| Boolean isLike, |
There was a problem hiding this comment.
흠.. 지금 전체적인 코드에 '좋아요 했음'이 isLiked와 isLike가 혼용되고 있네요🤔
통일하는게 좋아보이는데, 둘 중 뭐가 더 좋다 생각하시나요?
(디스코드에도 질문 올려두겠습니다.)
src/main/java/com/example/solidconnection/news/service/NewsQueryService.java
Outdated
Show resolved
Hide resolved
| .map(News::getId) | ||
| .toList(); | ||
|
|
||
| Set<Long> likedNewsIds = likedNewsRepository.findLikedNewsIdsByNewsIdsAndSiteUserId(newsIds, siteUserId); |
There was a problem hiding this comment.
저도 성혁님과 마찬가지로 규혁님의 판단에 맡기겠습니다 ㅎㅎ
참고로 '나라면 어떻게 했을까?'에 대해 생각해보자면,..
저라면 커스텀 레포지토리를 만들어서 querydsl은 사용하지 않고 jpql 프로젝션으로만 구현했을 것 같습니다!
querydsl을 사용할 정도의 복잡도는 아닌 것 같고,
join 해와서 나쁠게 없다고 생각하고,
"조회용"이라는 특정한 목적을 가졌으므로 별도의 클래스로 분리하는게 좋을 것 같다는 생각이 드네요😊
| // then | ||
| assertAll( | ||
| () -> assertThat(response.newsResponseList()).hasSize(newsList.size()), |
There was a problem hiding this comment.
then 절에서 size 가 일치하는지 검증하는 것보다,
news1.getId() 와 news2.getId()가 포함되는지 검증하는게
더 정확도가 높지 않을까요?
규혁님은 어떻게 생각하시나요? ㅎㅎ
lsy1307
left a comment
lsy1307
left a comment
There was a problem hiding this comment.
수고하셨습니다! 크게 문제되는 사항은 없어 보입니당
| // then | ||
| assertAll( | ||
| () -> assertThat(response.newsResponseList()).hasSize(newsList.size()), |
- jpql 프로젝션 활용
- 사이즈가 아닌 실제 id가 존재하는지 확인하도록
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/main/java/com/example/solidconnection/news/repository/NewsRepository.java (1)
10-10: [3] 메서드 네이밍에서 authorId/siteUserId 혼선 최소화
현재
findAllBySiteUserIdOrderByUpdatedAtDesc(long siteUserId)를 비로그인 흐름에서authorId로 호출합니다. 읽는 사람 입장에서 혼동 포인트입니다.도메인에서 siteUserId가 곧 작성자 ID여도, 의도를 드러내는 네이밍이 유지보수에 유리합니다.
제안 1. authorId 용도를 드러내도록 메서드명을 리네임합니다.
제안 2. 정렬 안정성을 위해 id로 2차 정렬을 추가하는 것도 고려해볼 만합니다.
적용 예시(diff):
- List<News> findAllBySiteUserIdOrderByUpdatedAtDesc(long siteUserId); + List<News> findAllByAuthorIdOrderByUpdatedAtDesc(long authorId);정렬 안정성(옵션):
- List<News> findAllBySiteUserIdOrderByUpdatedAtDesc(long siteUserId); + List<News> findAllBySiteUserIdOrderByUpdatedAtDescIdDesc(long siteUserId);src/main/java/com/example/solidconnection/news/service/NewsQueryService.java (1)
18-26: [8] 비로그인 흐름의 레포 호출 네이밍 혼선 보완
authorId를findAllBySiteUserId...에 전달하는 구조는 읽는 사람을 한 번 멈추게 합니다.- 레포 메서드명을
findAllByAuthorIdOrderByUpdatedAtDesc로 조정하면 의도가 즉시 드러납니다.적용 예시(diff):
- List<NewsResponse> newsResponseList = newsRepository.findAllBySiteUserIdOrderByUpdatedAtDesc(authorId) + List<NewsResponse> newsResponseList = newsRepository.findAllByAuthorIdOrderByUpdatedAtDesc(authorId) .stream() .map(news -> NewsResponse.from(news, null)) .toList();
🧹 Nitpick comments (7)
src/main/java/com/example/solidconnection/news/repository/custom/NewsCustomRepository.java (1)
6-9: [1] 파라미터 순서 일관성 정리로 가독성 향상 제안
서비스 시그니처는
(Long siteUserId, long authorId)이고, 커스텀 레포는(long authorId, Long siteUserId)로 반대입니다. 한 번 더 읽게 만드는 부분입니다.레이어 간 파라미터 순서를 통일하면 호출·구현부 모두 읽기 쉬워집니다.
제안 1. 파라미터 순서를 서비스와 동일하게 변경합니다.
제안 2. siteUserId가 null 가능하다는 점을 메서드 주석으로 명시합니다.
적용 예시(diff):
- List<NewsResponse> findNewsByAuthorIdWithLikeStatus(long authorId, Long siteUserId); + /** + * authorId 작성자의 소식지를 조회합니다. + * siteUserId는 로그인하지 않은 경우 null일 수 있습니다. + */ + List<NewsResponse> findNewsByAuthorIdWithLikeStatus(Long siteUserId, long authorId);src/test/java/com/example/solidconnection/news/service/NewsQueryServiceTest.java (2)
66-100: [5] 로그인 시나리오에서 isLiked 매핑 검증이 깔끔합니다
- Map으로 id→isLiked를 매핑해 개별 항목 검증하는 방식이 명료합니다.
- 추가 제안: 중복 like 레코드가 존재할 경우(데이터 정합성 이슈) 조인 결과가 중복될 여지가 있어 SELECT DISTINCT를 사용해 방어해두면 더 안전합니다. 이는 레포 구현부(JPQL)에서 처리 가능합니다.
38-64: 정렬 안정성 보강 제안
스크립트 실행 결과,NewsCustomRepositoryImpl.java의 쿼리에서ORDER BY n.updatedAt DESC만 사용 중임을 확인했습니다.
아래 변경 사항을 제안드립니다:
2차 정렬 키 추가
대상 파일 및 위치:src/main/java/com/example/solidconnection/news/repository/custom/NewsCustomRepositoryImpl.java30행
변경 내용:
ORDER BY n.updatedAt DESC, n.id DESC테스트 시나리오 강화
테스트 파일:src/test/java/com/example/solidconnection/news/service/NewsQueryServiceTest.java
변경 내용:
동일updatedAt타임스탬프 발생 시id DESC기준으로도 정렬 순서를 검증하는 케이스 추가src/main/java/com/example/solidconnection/news/repository/custom/NewsCustomRepositoryImpl.java (2)
15-36: [6] JPQL 개선: 정렬 안정성, 중복 방지, 게스트/로그인 단일 경로 통합 제안
- 정렬 안정성. updatedAt 동률 시 결과 순서가 비결정적일 수 있습니다. id DESC를 2차 정렬로 추가하면 테스트/운영 모두 안정적입니다.
- 중복 방지. 데이터 정합성 문제로 LikedNews에 동일 (newsId, siteUserId) 중복이 생기면 행 중복이 생길 수 있습니다. SELECT DISTINCT로 방어 가능합니다.
- 경로 통합(옵션). 게스트/로그인 경로를 하나의 쿼리로 처리하면 서비스 분기가 단순해집니다.
CASE WHEN :siteUserId IS NULL THEN NULL ELSE ... END로 isLiked를 산출하면 됩니다.적용 예시(diff):
- String jpql = """ - SELECT new com.example.solidconnection.news.dto.NewsResponse( + String jpql = """ + SELECT DISTINCT new com.example.solidconnection.news.dto.NewsResponse( n.id, n.title, n.description, n.thumbnailUrl, n.url, - CASE WHEN ln.id IS NOT NULL THEN true ELSE false END, + CASE + WHEN :siteUserId IS NULL THEN NULL + WHEN ln.id IS NOT NULL THEN true + ELSE false + END, n.updatedAt ) FROM News n LEFT JOIN LikedNews ln ON n.id = ln.newsId AND ln.siteUserId = :siteUserId WHERE n.siteUserId = :authorId - ORDER BY n.updatedAt DESC + ORDER BY n.updatedAt DESC, n.id DESC """;추가로, 대량 데이터 대비를 위해 페이징 도입(Pageable 인자 추가, setFirstResult/setMaxResults 사용)을 검토하면 좋습니다.
33-36: siteUserId null 바인딩 주의 및 LikedNews 중복 제약 확인 완료.
- siteUserId null 바인딩 주의.
현재 메서드는 로그인 시에만 호출돼 null 바인딩 이슈가 없습니다.
추후 호출 경로 확대에 대비해:siteUserId가 null일 때도 안전하게 동작하도록 통합 JPQL 혹은COALESCE등으로 조건을 처리해주세요.- LikedNews 유니크 제약 확인.
스크립트 실행 결과LikedNews엔티티에(site_user_id, news_id)유니크 제약이 선언돼 있음이 확인되었습니다.
DB 레벨에서 중복 좋아요가 방지됩니다.src/main/java/com/example/solidconnection/news/service/NewsQueryService.java (2)
28-31: [9] 로그인 흐름 위임은 적절합니다
- 커스텀 레포로 위임하여 isLiked까지 한 번에 가져오는 접근이 명확합니다.
- 위 JPQL 개선(정렬 안정성, DISTINCT)만 반영되면 더 견고해집니다.
18-31: 옵션 제안: 로그인/비로그인 처리 단일화
서비스 코드에 있던 두 갈래 길에 작별을 고하고, 한 번에 쓱 처리해볼까요?Walkthrough:
- 서비스 분기 제거
-if (siteUserId == null)부터 별도 호출부를 통째로 삭제합니다.
- 이제 항상newsRepository.findNewsByAuthorIdWithLikeStatus(siteUserId, authorId)만 불러옵니다.- JPQL 파라미터 확보
-siteUserId가 null일 때isLiked를 null로 내려주도록 쿼리 내 조건을 추가하면 완벽해집니다.- 호출 지점 점검
- 전역rg검색 결과, 해당 메서드 호출부는NewsQueryService단 한 곳뿐이므로 안전한 일괄 치환이 가능합니다.적용 예시(diff):
- if (siteUserId == null) { - List<NewsResponse> newsResponseList = newsRepository.findAllBySiteUserIdOrderByUpdatedAtDesc(authorId) - .stream() - .map(news -> NewsResponse.from(news, null)) - .toList(); - return NewsListResponse.from(newsResponseList); - } - List<NewsResponse> newsResponseList = newsRepository.findNewsByAuthorIdWithLikeStatus(authorId, siteUserId); + List<NewsResponse> newsResponseList = newsRepository.findNewsByAuthorIdWithLikeStatus(siteUserId, authorId); + return NewsListResponse.from(newsResponseList);선택 사항이지만, 코드가 한결 깔끔해지고 유지보수도 더 즐거워집니다!
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
src/main/java/com/example/solidconnection/news/dto/NewsResponse.java(2 hunks)src/main/java/com/example/solidconnection/news/repository/NewsRepository.java(1 hunks)src/main/java/com/example/solidconnection/news/repository/custom/NewsCustomRepository.java(1 hunks)src/main/java/com/example/solidconnection/news/repository/custom/NewsCustomRepositoryImpl.java(1 hunks)src/main/java/com/example/solidconnection/news/service/NewsQueryService.java(1 hunks)src/test/java/com/example/solidconnection/news/service/NewsQueryServiceTest.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/example/solidconnection/news/dto/NewsResponse.java
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-07-28T03:14:33.849Z
Learnt from: whqtker
PR: solid-connection/solid-connect-server#408
File: src/main/java/com/example/solidconnection/chat/service/ChatService.java:46-54
Timestamp: 2025-07-28T03:14:33.849Z
Learning: 페치 조인을 사용한 N+1 문제 해결에 대해 사용자 whqtker는 JPQL 쿼리 접근법을 선호하며, SELECT 쿼리의 경우 영속성 컨텍스트 간 불일치 문제를 고려하지 않아도 된다고 인식하고 있습니다.
Applied to files:
src/main/java/com/example/solidconnection/news/service/NewsQueryService.java
📚 Learning: 2025-07-28T18:36:56.462Z
Learnt from: nayonsoso
PR: solid-connection/solid-connect-server#408
File: src/main/java/com/example/solidconnection/chat/repository/ChatMessageRepository.java:21-27
Timestamp: 2025-07-28T18:36:56.462Z
Learning: MySQL + Hibernate 환경에서는 JPQL의 LIMIT 구문이 작동하지만, 이는 Hibernate 확장 문법이므로 다른 JPA 구현체나 데이터베이스에서는 동작하지 않을 수 있어 이식성 문제가 있다.
Applied to files:
src/main/java/com/example/solidconnection/news/service/NewsQueryService.java
📚 Learning: 2025-07-29T17:26:08.811Z
Learnt from: whqtker
PR: solid-connection/solid-connect-server#408
File: src/main/java/com/example/solidconnection/chat/repository/ChatReadStatusRepository.java:11-17
Timestamp: 2025-07-29T17:26:08.811Z
Learning: 사용자 whqtker는 JPA save() 메서드를 사용한 upsert 연산에서 레이스 컨디션 문제를 우려하고 있으며, 특히 채팅 읽음 상태 업데이트와 같이 동시성이 중요한 연산에서는 네이티브 쿼리의 원자적 연산이 더 안전하다고 인식하고 있습니다.
Applied to files:
src/main/java/com/example/solidconnection/news/service/NewsQueryService.java
🧬 Code graph analysis (1)
src/test/java/com/example/solidconnection/news/service/NewsQueryServiceTest.java (2)
src/test/java/com/example/solidconnection/security/aspect/RoleAuthorizationAspectTest.java (1)
TestContainerSpringBootTest(21-101)src/test/java/com/example/solidconnection/news/service/NewsCommandServiceTest.java (1)
TestContainerSpringBootTest(36-315)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
src/main/java/com/example/solidconnection/news/repository/NewsRepository.java (1)
8-8: [2] 커스텀 레포 확장 추가는 적절합니다
- 커스텀 조회를 레포지토리 인터페이스로 노출하여 서비스가 깔끔해졌습니다.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/main/java/com/example/solidconnection/news/dto/NewsResponse.java (2)
22-31: 정적 팩터리of(News, Boolean)변경은 적절합니다 + JPQL Constructor Expression 매개변수 순서 검증 요청
- 칭찬 한 줄: 과거 제안대로
from→of(News, Boolean)로 정리된 점, 시그니처가 의도를 잘 드러내서 좋습니다.
- 주의 한 줄: 본 레코드는 JPQL constructor expression으로도 생성됩니다. 매개변수 순서가
id, title, description, thumbnailUrl, url, isLiked, updatedAt로 정확히 일치해야 런타임 오류를 피할 수 있습니다.
- 확인 팁: 커스텀 리포지토리의
new com.example...NewsResponse(...)가 위 순서를 지키는지 점검해 주세요.다음 스크립트로 사용처/잔존 호출을 점검해 주세요.
#!/bin/bash set -euo pipefail # A) JPQL constructor expression 위치와 인자 순서 확인 rg -n -C2 --type=java -S 'new com\.example\.solidconnection\.news\.dto\.NewsResponse\s*\(' # B) 이전 팩터리 메서드 호출이 남아있는지 확인 rg -n -C2 --type=java -S '\bNewsResponse\.from\s*\(' || true # C) 새 팩터리 메서드 사용 현황 확인 rg -n -C2 --type=java -S '\bNewsResponse\.of\s*\('
16-18: API 필드명(isLiked vs isLike) 일관화 필요 + JSON 노출 키 고정 제안
- 무엇이 바뀌었나: 게스트 대응을 위해
Boolean isLiked가 추가되었고@JsonInclude(NON_NULL)로 null 시 미노출됩니다.
- 왜 중요한가: PR 설명과 과거 코멘트 기준 외부 응답 키는
isLike로 합의된 분위기인데, 현재 DTO 필드명이isLiked라 직렬화 시 JSON 키도 그대로isLiked가 됩니다. 클라이언트/문서와 불일치가 생기면 즉시 호환성 이슈입니다.
- 제안: 내부 필드명은
isLiked유지(가독성/관용), 외부 JSON 키는@JsonProperty("isLike")로 고정해 일관성을 확보합시다. 필요 시 입력 수용을 넓히려면@JsonAlias({"isLiked"})도 고려 가능합니다.
- 적용 diff: 아래와 같이 필드에 노출명을 고정하세요. 또한, 별도 코멘트(라인 3-4)에 import 추가를 제안해 두었습니다.
- @JsonInclude(NON_NULL) - Boolean isLiked, + @JsonInclude(NON_NULL) + @JsonProperty("isLike") + Boolean isLiked,
- 테스트: 비로그인(=null)일 때
isLike키가 빠지는지, 로그인 시true/false로 노출되는지 JSON 스냅샷/컨트롤러 통합 테스트를 추가하면 회귀를 막을 수 있습니다.다음 스크립트로 코드/문서의 키 사용을 훑어 불일치를 즉시 발견할 수 있습니다.
#!/bin/bash set -euo pipefail # 1) 응답/DTO/컨트롤러에서 isLike / isLiked 사용 흔적 비교 rg -n -C2 -g 'src/**' -S '\bisLike\b' rg -n -C2 -g 'src/**' -S '\bisLiked\b' # 2) API 문서/스니펫에서도 동일 키를 쓰는지 확인(레포에 문서가 함께 있을 경우) rg -n -C2 -S '\bisLike\b' docs api-doc openapi swagger || true
🧹 Nitpick comments (1)
src/main/java/com/example/solidconnection/news/dto/NewsResponse.java (1)
3-4:NON_NULL정적 임포트는 깔끔합니다 +@JsonProperty임포트 추가 제안(상기 코멘트 연동)
- 칭찬 한 줄:
Include.NON_NULL정적 임포트로 가독성이 좋아졌습니다.
- 연동 한 줄: 위에서 제안한
@JsonProperty("isLike")를 적용하려면 해당 임포트가 필요합니다. 함께 반영해 주세요.import static com.fasterxml.jackson.annotation.JsonInclude.Include.NON_NULL; import com.example.solidconnection.news.domain.News; +import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonInclude;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/main/java/com/example/solidconnection/news/dto/NewsResponse.java(2 hunks)src/main/java/com/example/solidconnection/news/service/NewsQueryService.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/example/solidconnection/news/service/NewsQueryService.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
4 participants
관련 이슈
작업 내용
지난 8월 14일 회의 때 요청한 내용을 반영한 pr입니다.
as-is
to-be
추가로 해당 api같은 경우는 로그인하지 않은 사용자도 요청할 수 있어야 하기에
@Authorizeduser(required = false) Long siteUserId로 설정하였습니다.
그렇기에 서비스 로직에서도 siteUserId는 null이 될 수 있기에 long이 아닌 Long으로 구현하였습니다.
또한, 로그인하지 않은 사용자가 요청했을 때에는 isLike를 응답할 필요가 없다고 생각하여 @JsonInclude(NON_NULL)를 추가하였습니다.
이거 또한 마찬가지로 null이 될 수 있기에 boolean이 아닌 Boolean으로 하였습니다.
bruno 관련
특이 사항
리뷰 요구사항 (선택)
기존 siteUserId로 소식지 작성자를 조회했었는데 현재 로그인 여부를 구분하게되어 authorId로 변경하였는데 괜찮나요?