Open
Conversation
Walkthrough북마크 조회의 페이징 기준을 FavoriteContent.id로 통일하고, FavoriteContent 기반 응답 타입과 v2 엔드포인트를 추가해 기존 v1 응답을 유지하며 새로운 응답 흐름을 도입했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant ControllerV2 as "FavoriteContentController (v2)"
participant Service as "FavoriteContentService"
participant Repo as "FavoriteContentRepository"
participant DB as Database
Client->>ControllerV2: GET /api/v2/bookmarks?pageSize, lastId
ControllerV2->>Service: findMyFavoriteContents(account, pageSize, lastId)
Service->>Repo: findMyFavoriteContentsByAccountId(accountId, lastId, pageable)
Repo->>DB: execute JPQL (SELECT f ... WHERE f.id < :lastId ORDER BY f.id DESC)
DB-->>Repo: Slice<FavoriteContent>
Repo-->>Service: Slice<FavoriteContent>
Service->>Service: convertToFavoriteContentDetailResponses(favoriteContents)
Service-->>ControllerV2: FavoriteContentWithLoadableResult(bookmarks, loadable)
ControllerV2-->>Client: 200 OK + FavoriteContentDetailsWithLoadableResponse (JSON)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
backend/turip-app/src/main/java/turip/favorite/repository/FavoriteContentRepository.java (1)
49-52: 파라미터 명을lastId로 맞추면 추적성이 좋아집니다.
@Param("lastId")와 메서드 인자명(lastFavoriteContentId)이 달라 읽는 사람이 한 번 더 해석해야 합니다.✏️ 제안 변경안
Slice<FavoriteContent> findMyFavoriteContentsByAccountId( `@Param`("accountId") Long accountId, - `@Param`("lastId") Long lastFavoriteContentId, + `@Param`("lastId") Long lastId, Pageable pageable );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/main/java/turip/favorite/repository/FavoriteContentRepository.java` around lines 49 - 52, The repository method findMyFavoriteContentsByAccountId uses `@Param`("lastId") but the Java parameter is named lastFavoriteContentId which reduces traceability; rename the method parameter to lastId (or update the `@Param` to match, preferably rename the parameter) in FavoriteContentRepository's method signature so the `@Param`("lastId") and the parameter name are identical, ensuring clearer intent and easier maintenance.backend/turip-app/src/main/java/turip/favorite/service/dto/FavoriteContentWithLoadableResult.java (1)
3-9: 서비스 DTO에@JsonProperty주석을 제거해야 합니다.
FavoriteContentWithLoadableResult는 내부 변환용 서비스 DTO로, 컨트롤러에서 항상FavoriteContentDetailsWithLoadableResponse또는ContentsDetailWithLoadableResponse같은 응답 DTO로 변환된 후 직렬화됩니다. 서비스 DTO에서 JSON 필드 이름을 지정할 필요가 없으며, 이미 응답 DTO에@JsonProperty("bookmarks")가 있으므로 중복입니다.♻️ 제안 변경안
-import com.fasterxml.jackson.annotation.JsonProperty; import java.util.List; import turip.favorite.controller.dto.response.FavoriteContentDetailResponse; public record FavoriteContentWithLoadableResult( - `@JsonProperty`("bookmarks") List<FavoriteContentDetailResponse> favoriteContents, + List<FavoriteContentDetailResponse> favoriteContents, boolean loadable ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/main/java/turip/favorite/service/dto/FavoriteContentWithLoadableResult.java` around lines 3 - 9, Remove the unnecessary Jackson annotation from the service DTO: delete the `@JsonProperty` on the FavoriteContentWithLoadableResult record (the favoriteContents component) so the service-layer DTO does not declare JSON field names; leave JSON mapping to the response DTOs (e.g., FavoriteContentDetailsWithLoadableResponse / ContentsDetailWithLoadableResponse) which already define `@JsonProperty`("bookmarks").backend/turip-app/src/test/java/turip/favorite/service/FavoriteContentServiceTest.java (1)
123-155:lastFavoriteContentId != 0커서 케이스도 테스트로 고정해두는 것을 권장합니다.현재는 첫 페이지(0 →
Long.MAX_VALUE)만 검증합니다. 실제 페이지 이동 시 전달되는 커서 값이 그대로 repository에 전달되는 시나리오를 추가하면 회귀 방지에 도움이 됩니다.추가 테스트 예시
+ `@DisplayName`("내 찜 목록 조회 시 비-초기 커서는 그대로 사용한다") + `@Test` + void findMyFavoriteContents_withCursor() { + // given + int pageSize = 2; + long lastFavoriteContentId = 10L; + Account account = AccountFixture.createUser(); + + given(favoriteContentRepository.findMyFavoriteContentsByAccountId( + eq(account.getId()), eq(lastFavoriteContentId), any())) + .willReturn(new SliceImpl<>(List.of())); + + // when + FavoriteContentWithLoadableResult result = favoriteContentService.findMyFavoriteContents( + account, pageSize, lastFavoriteContentId); + + // then + assertAll( + () -> assertThat(result.favoriteContents()).isEmpty(), + () -> assertThat(result.loadable()).isFalse() + ); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/test/java/turip/favorite/service/FavoriteContentServiceTest.java` around lines 123 - 155, Add a test case that verifies the non-zero cursor path: create a new test (or extend FavoriteContentServiceTest) that calls favoriteContentService.findMyFavoriteContents with a non-zero lastContentId (e.g., 5L) and a pageSize, stub favoriteContentRepository.findMyFavoriteContentsByAccountId to expect eq(account.getId()) and eq(5L) (instead of Long.MAX_VALUE) and return a SliceImpl of FavoriteContent, stub the same contentPlaceService and contentRepository calls as in findMyFavoriteContents1, then assert the returned FavoriteContentWithLoadableResult is as expected and verify the repository was called with the exact cursor value to ensure the cursor is passed through correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@backend/turip-app/src/main/java/turip/favorite/controller/FavoriteContentController.java`:
- Around line 300-303: The Swagger v2 example JSON in FavoriteContentController
contains a syntax error and misleading domain data: add the missing comma after
the "content" block in the example JSON and update the bookmark example's
"isBookmarked" value in the example payload used by the controller (search for
the example JSON string or the method that builds the Swagger example in
FavoriteContentController) to reflect the correct domain state (set
"isBookmarked" appropriately, e.g., true for items in the bookmark list) so the
swagger sample is valid and semantically correct.
- Around line 255-263: The controller method readMyFavoriteContentsV1 currently
only accepts lastId which breaks backwards compatibility for clients sending
lastContentId; update the method signature in FavoriteContentController
(readMyFavoriteContentsV1) to accept both `@RequestParam`(required = false) Long
lastId and `@RequestParam`(name = "lastContentId", required = false) Long
lastContentId, compute a single effectiveLastId = (lastId != null) ? lastId :
lastContentId (and handle null if page/logic requires), then pass
effectiveLastId into favoriteContentService.findMyFavoriteContents so both old
and new clients are supported.
In
`@backend/turip-app/src/main/java/turip/favorite/service/FavoriteContentService.java`:
- Around line 91-95: The code maps favoriteContent and currently calls
calculateTripDuration(content) before verifying the content exists; move the
existence check validateContentExists(content.getId()) to run immediately after
obtaining Content (before calling calculateTripDuration and
contentPlaceService.countByContentId) so validation happens first and prevents
unnecessary duration computation or count operations when the content is missing
or invalid.
---
Nitpick comments:
In
`@backend/turip-app/src/main/java/turip/favorite/repository/FavoriteContentRepository.java`:
- Around line 49-52: The repository method findMyFavoriteContentsByAccountId
uses `@Param`("lastId") but the Java parameter is named lastFavoriteContentId
which reduces traceability; rename the method parameter to lastId (or update the
`@Param` to match, preferably rename the parameter) in FavoriteContentRepository's
method signature so the `@Param`("lastId") and the parameter name are identical,
ensuring clearer intent and easier maintenance.
In
`@backend/turip-app/src/main/java/turip/favorite/service/dto/FavoriteContentWithLoadableResult.java`:
- Around line 3-9: Remove the unnecessary Jackson annotation from the service
DTO: delete the `@JsonProperty` on the FavoriteContentWithLoadableResult record
(the favoriteContents component) so the service-layer DTO does not declare JSON
field names; leave JSON mapping to the response DTOs (e.g.,
FavoriteContentDetailsWithLoadableResponse / ContentsDetailWithLoadableResponse)
which already define `@JsonProperty`("bookmarks").
In
`@backend/turip-app/src/test/java/turip/favorite/service/FavoriteContentServiceTest.java`:
- Around line 123-155: Add a test case that verifies the non-zero cursor path:
create a new test (or extend FavoriteContentServiceTest) that calls
favoriteContentService.findMyFavoriteContents with a non-zero lastContentId
(e.g., 5L) and a pageSize, stub
favoriteContentRepository.findMyFavoriteContentsByAccountId to expect
eq(account.getId()) and eq(5L) (instead of Long.MAX_VALUE) and return a
SliceImpl of FavoriteContent, stub the same contentPlaceService and
contentRepository calls as in findMyFavoriteContents1, then assert the returned
FavoriteContentWithLoadableResult is as expected and verify the repository was
called with the exact cursor value to ensure the cursor is passed through
correctly.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
backend/turip-app/src/main/java/turip/content/controller/dto/response/content/ContentDetailResponse.javabackend/turip-app/src/main/java/turip/content/controller/dto/response/content/ContentsDetailWithLoadableResponse.javabackend/turip-app/src/main/java/turip/favorite/controller/FavoriteContentController.javabackend/turip-app/src/main/java/turip/favorite/controller/dto/response/FavoriteContentDetailResponse.javabackend/turip-app/src/main/java/turip/favorite/controller/dto/response/FavoriteContentDetailsWithLoadableResponse.javabackend/turip-app/src/main/java/turip/favorite/repository/FavoriteContentRepository.javabackend/turip-app/src/main/java/turip/favorite/service/FavoriteContentService.javabackend/turip-app/src/main/java/turip/favorite/service/dto/FavoriteContentWithLoadableResult.javabackend/turip-app/src/test/java/turip/favorite/repository/FavoriteContentRepositoryTest.javabackend/turip-app/src/test/java/turip/favorite/service/FavoriteContentServiceTest.java
backend/turip-app/src/main/java/turip/favorite/controller/FavoriteContentController.java
Show resolved
Hide resolved
backend/turip-app/src/main/java/turip/favorite/controller/FavoriteContentController.java
Outdated
Show resolved
Hide resolved
backend/turip-app/src/main/java/turip/favorite/service/FavoriteContentService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
🧹 Nitpick comments (2)
backend/turip-app/src/main/java/turip/favorite/service/FavoriteContentService.java (1)
56-57: 컬렉션 변수명을 복수형으로 맞추면 읽기성이 더 좋아집니다.현재 변수명은 컬렉션인데 단수형이라 의미가 약간 헷갈립니다.
가독성 개선 예시
- List<FavoriteContent> contents = favoriteContentSlice.getContent(); - List<FavoriteContentDetailResponse> favoriteContentDetailResponse = convertToFavoriteContentDetailResponses( - contents); + List<FavoriteContent> favoriteContents = favoriteContentSlice.getContent(); + List<FavoriteContentDetailResponse> favoriteContentDetailResponses = convertToFavoriteContentDetailResponses( + favoriteContents); ... - return FavoriteContentWithLoadableResult.of(favoriteContentDetailResponse, loadable); + return FavoriteContentWithLoadableResult.of(favoriteContentDetailResponses, loadable);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/main/java/turip/favorite/service/FavoriteContentService.java` around lines 56 - 57, The local variable holding the list of FavoriteContentDetailResponse is named in singular form (favoriteContentDetailResponse); rename it to a plural form (e.g., favoriteContentDetailResponses) and update all its usages in FavoriteContentService (including the assignment from convertToFavoriteContentDetailResponses and any subsequent references) so the collection variable name correctly reflects it contains multiple items and improves readability.backend/turip-app/src/main/java/turip/favorite/controller/FavoriteContentController.java (1)
254-264: 첫 페이지 요청 시lastId파라미터 처리 방식 문서화 필요
lastId가 필수 파라미터로 지정되어 있어, 클라이언트가 첫 페이지 요청 시 어떤 값을 전달해야 하는지 명확하지 않습니다. 커서 기반 페이지네이션에서 일반적으로 첫 요청은 커서 없이 호출하거나 sentinel 값(예:Long.MAX_VALUE)을 사용합니다.다음 중 하나를 권장합니다:
- API 설명(description)에 첫 페이지 호출 시 사용할 값 명시
lastId를 optional로 변경하고 기본값 설정♻️ Optional 파라미터로 변경하는 예시
`@GetMapping`("/api/v1/bookmarks") public ResponseEntity<ContentsDetailWithLoadableResponse> readMyFavoriteContentsV1( `@Parameter`(hidden = true) `@AuthAccount` Account account, `@RequestParam`(name = "size") Integer pageSize, - `@RequestParam`(name = "lastId") Long lastId + `@RequestParam`(name = "lastId", required = false) Long lastId ) { + Long cursor = lastId != null ? lastId : Long.MAX_VALUE; FavoriteContentWithLoadableResult result = favoriteContentService.findMyFavoriteContents(account, pageSize, - lastId); + cursor);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/main/java/turip/favorite/controller/FavoriteContentController.java` around lines 254 - 264, Make lastId optional and default to a sentinel for first-page requests: change the `@RequestParam` on the lastId parameter in readMyFavoriteContentsV1 to be optional (required=false) and accept a nullable Long, then when calling favoriteContentService.findMyFavoriteContents(account, pageSize, lastId) pass a default (e.g. Long.MAX_VALUE) when lastId is null. Also update the method's API description/@Parameter or JavaDoc to document that omitting lastId (or passing the sentinel) requests the first page. Use the symbols readMyFavoriteContentsV1, lastId, `@RequestParam`, and favoriteContentService.findMyFavoriteContents to locate and modify the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@backend/turip-app/src/main/java/turip/favorite/controller/FavoriteContentController.java`:
- Around line 254-264: Make lastId optional and default to a sentinel for
first-page requests: change the `@RequestParam` on the lastId parameter in
readMyFavoriteContentsV1 to be optional (required=false) and accept a nullable
Long, then when calling favoriteContentService.findMyFavoriteContents(account,
pageSize, lastId) pass a default (e.g. Long.MAX_VALUE) when lastId is null. Also
update the method's API description/@Parameter or JavaDoc to document that
omitting lastId (or passing the sentinel) requests the first page. Use the
symbols readMyFavoriteContentsV1, lastId, `@RequestParam`, and
favoriteContentService.findMyFavoriteContents to locate and modify the code.
In
`@backend/turip-app/src/main/java/turip/favorite/service/FavoriteContentService.java`:
- Around line 56-57: The local variable holding the list of
FavoriteContentDetailResponse is named in singular form
(favoriteContentDetailResponse); rename it to a plural form (e.g.,
favoriteContentDetailResponses) and update all its usages in
FavoriteContentService (including the assignment from
convertToFavoriteContentDetailResponses and any subsequent references) so the
collection variable name correctly reflects it contains multiple items and
improves readability.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/turip-app/src/main/java/turip/favorite/controller/FavoriteContentController.javabackend/turip-app/src/main/java/turip/favorite/service/FavoriteContentService.javabackend/turip-app/src/test/java/turip/favorite/service/FavoriteContentServiceTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issues
✔️ Check-list
🗒️ Work Description
V2 api 위치
디코에서 언급했던 방법들 중, 방법 1) 각 api 메서드별로 매핑 직접 해주기를 적용했어요. 버전별로 controller를 분리하면 버전 추가에 따라 클래스 수도 늘어나기에 관리 비용을 생각해 선택하지 않았고, 버전을 상수화하는 방법은 다른 컨트롤러들에도 일괄적으로 적용하면 좋을 것 같아서 보류했어요.
기존 서비스 메서드 재사용
현재 저희 구조상 service에서 controller의 response dto를 직접 생성해주고 있기에, v2 응답에 대한 서비스 메서드를 하나 더 추가해야 했어요. v2 응답을 위해 새롭게 변경한 repository 메서드를 이용해서 v1 응답도 만들 수 있어서, v1과 v2 모두 같은 서비스 메서드를 사용하도록 했어요. (새로운 서비스 메서드명 뒤에 v2를 붙이는 방법도 있었으나 가독성이 좋지 않다고 생각했고, 버전에 대한 내용을 서비스까지 가져오고 싶지 않았어요.)
그래서 공통적으로 사용하는 서비스 메서드는 FavoriteContentWithLoadableResult라는 서비스단의 dto를 응답하고, v1과 v2 컨트롤러 메서드에서 이를 이용하여 각각의 버전에서 사용하는 response dto를 조합하도록 했습니다!
📷 Screenshot
📚 Reference
Summary by CodeRabbit
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항
테스트