Conversation
Walkthrough북마크 기능 추가: API 인터페이스 및 컨트롤러, 요청/응답 DTO, 비즈니스 서비스, 리포지토리 조회 확장, Media 엔티티 북마크 카운트 메서드, 에러 코드(B00*/BOOKMARK_NOT_FOUND) 추가 및 오타 수정이 도입됨. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant BC as BookmarkController
participant BS as BookmarkService
participant MR as MediaRepository
participant BR as BookmarkRepository
participant DB as Database
rect rgba(0, 100, 200, 0.5)
Note over Client,DB: 북마크 편집(토글) 흐름
Client->>BC: POST /bookmarks (memberId, mediaId)
BC->>BS: editBookmark(memberId, mediaId)
BS->>MR: findById(mediaId)
MR->>DB: SELECT media WHERE id=?
DB-->>MR: Media
MR-->>BS: Media
BS->>BR: findByMemberIdAndMediaId(memberId, targetMediaId)
BR->>DB: SELECT bookmark WHERE memberId=? AND mediaId=?
DB-->>BR: Bookmark? / null
BR-->>BS: Optional<Bookmark>
alt existing ACTIVE
BS->>BR: update status=DELETE
BS->>MR: decreaseBookmarkCount()
else existing DELETE
BS->>BR: update status=ACTIVE
BS->>MR: increaseBookmarkCount()
else not exists
BS->>BR: insert new ACTIVE bookmark
BS->>MR: increaseBookmarkCount()
end
BR->>DB: INSERT/UPDATE
DB-->>BR: OK
BS-->>BC: 완료
BC-->>Client: 200 OK
end
sequenceDiagram
actor Client
participant BC as BookmarkController
participant BS as BookmarkService
participant BR as BookmarkRepository
participant DB as Database
rect rgba(0, 150, 100, 0.5)
Note over Client,DB: 미디어 북마크 목록 조회
Client->>BC: GET /bookmarks/me/contents?page=&size=
BC->>BS: getBookmarkMediaList(memberId, page, size)
BS->>BR: findByMemberIdAndStatusAndMedia_MediaTypeIn(..., pageable)
BR->>DB: SELECT ... (페이징)
DB-->>BR: Page<Bookmark>
BR-->>BS: Page<Bookmark>
BS->>BS: Bookmark -> BookmarkMediaResponse 변환
BS-->>BC: PageResponse<BookmarkMediaResponse>
BC-->>Client: 200 OK
end
sequenceDiagram
actor Client
participant BC as BookmarkController
participant BS as BookmarkService
participant BR as BookmarkRepository
participant DB as Database
rect rgba(150, 100, 0, 0.5)
Note over Client,DB: 숏폼 북마크 목록 조회
Client->>BC: GET /bookmarks/me/short-form?page=&size=
BC->>BS: getBookmarkShortFormList(memberId, page, size)
BS->>BR: findByMemberIdAndStatusAndMedia_MediaType(..., pageable)
BR->>DB: SELECT ... (페이징)
DB-->>BR: Page<Bookmark>
BR-->>BS: Page<Bookmark>
BS->>BS: Bookmark -> BookmarkShortFormResponse 변환
BS-->>BC: PageResponse<BookmarkShortFormResponse>
BC-->>Client: 200 OK
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/api-user/src/main/java/com/ott/api_user/bookmark/controller/BookmarkAPI.java`:
- Around line 67-70: The getBookmarkShortFormList method in BookmarkAPI is
missing the authentication injection on the memberId parameter; annotate the
Long memberId parameter with `@AuthenticationPrincipal` (or the project’s custom
principal annotation/type) so the authenticated user id is injected for
authorization checks, and add the required import for the annotation or adjust
the parameter type to your custom principal (e.g., CustomUserDetails) if needed
to extract the memberId.
- Line 53: Change the page-size validation from `@Min`(0) to `@Min`(1) in the
BookmarkAPI controller so requests with size=0 no longer reach
PageRequest.of(...) and trigger IllegalArgumentException; update the
`@RequestParam` annotations for the size parameter (the occurrences in the
BookmarkAPI method signatures around the controller methods) to use `@Min`(1)
while keeping the defaultValue ("10") intact.
- Around line 13-38: Remove the Swagger RequestBody import and use Spring's
org.springframework.web.bind.annotation.RequestBody for the editBookmark method
parameter (look for method editBookmark and its `@RequestBody` usage), add the
missing `@AuthenticationPrincipal` annotation to the memberId parameter of
getBookmarkShortFormList (match the pattern used in editBookmark and other
methods), and change the `@Min` constraint on any size pagination parameters from
`@Min`(0) to `@Min`(1) (update size annotations in getBookmarkShortFormList and any
other methods using size to ensure pages start at 1).
In
`@apps/api-user/src/main/java/com/ott/api_user/bookmark/controller/BookmarkController.java`:
- Around line 42-44: The BookmarkController implementation is missing parameter
validation annotations so the interface constraints don't enforce request
validation; update the controller method signatures in BookmarkController to
annotate the page parameter with `@PositiveOrZero` and the size parameter with
`@Min`(1) `@Max`(100), and add `@Validated` to the BookmarkController class so Spring
will apply those validations at runtime (apply these to both occurrences of the
handler method that accept page and size).
- Around line 31-35: Add method-level role checks to the BookmarkController by
annotating the controller methods (e.g., editBookmark and the other methods at
the same spots) with a PreAuthorize expression that restricts access to allowed
roles; for example, add `@PreAuthorize`("hasAnyRole('USER','EDITOR','ADMIN')") on
the editBookmark method (and likewise on the methods at the other reported
locations) and import org.springframework.security.access.prepost.PreAuthorize
so the controller enforces role-based access in addition to authentication.
In
`@apps/api-user/src/main/java/com/ott/api_user/bookmark/dto/request/BookmarkRequest.java`:
- Around line 14-16: The mediaId field in BookmarkRequest only forbids null but
allows 0 or negative values; update the validation on the mediaId property in
the BookmarkRequest class by adding `@Positive` (or `@Min`(1)) alongside `@NotNull` so
only positive IDs are accepted, and adjust any import/validation messages
accordingly to reference mediaId.
In
`@apps/api-user/src/main/java/com/ott/api_user/bookmark/service/BookmarkService.java`:
- Around line 41-73: The editBookmark method currently toggles bookmark.status
and updates findMedia.bookmarkCount in separate read-modify steps which can race
under concurrent requests; change the flow to acquire a DB lock or perform
atomic updates: load the Bookmark (via
bookmarkRepository.findByMemberIdAndMediaId or a new repository method) using a
PESSIMISTIC_WRITE lock (or, alternatively, use a single repository update query
that flips status and adjusts media.bookmarkCount atomically), and if absent
create the Bookmark inside the same locked/transactional context after loading
Member (memberRepository.findById) and then update
findMedia.increaseBookmarkCount()/decreaseBookmarkCount() within that same
transaction/lock so the status toggle and media count change are applied
deterministically; ensure the method (editBookmark) runs in a transactional
boundary while using the locked repository methods or atomic update queries.
In
`@modules/domain/src/main/java/com/ott/domain/bookmark/repository/BookmarkRepository.java`:
- Around line 22-28: Both repository methods
(findByMemberIdAndStatusAndMedia_MediaTypeIn and
findByMemberIdAndStatusAndMedia_MediaType) return paged results without a
deterministic sort; update their signatures to enforce a consistent ordering
(e.g., newest first). Fix by either renaming the methods to include an explicit
ordering such as OrderByCreatedDateDesc or OrderByUpdatedAtDesc (e.g.,
findByMemberIdAndStatusAndMedia_MediaTypeInOrderByCreatedDateDesc) or add an
explicit Sort parameter and ensure callers provide PageRequest with
Sort.by(Sort.Direction.DESC, "createdDate" or "updatedAt"); apply the same
change to both methods and ensure the chosen timestamp field
(createdDate/updatedAt) matches the entity field name used in Bookmark/media.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/api-user/src/main/java/com/ott/api_user/bookmark/controller/.gitkeepapps/api-user/src/main/java/com/ott/api_user/bookmark/controller/BookmarkAPI.javaapps/api-user/src/main/java/com/ott/api_user/bookmark/controller/BookmarkController.javaapps/api-user/src/main/java/com/ott/api_user/bookmark/dto/.gitkeepapps/api-user/src/main/java/com/ott/api_user/bookmark/dto/request/BookmarkRequest.javaapps/api-user/src/main/java/com/ott/api_user/bookmark/dto/response/BookmarkMediaResponse.javaapps/api-user/src/main/java/com/ott/api_user/bookmark/dto/response/BookmarkShortFormResponse.javaapps/api-user/src/main/java/com/ott/api_user/bookmark/service/.gitkeepapps/api-user/src/main/java/com/ott/api_user/bookmark/service/BookmarkService.javamodules/common-web/src/main/java/com/ott/common/web/exception/ErrorCode.javamodules/domain/src/main/java/com/ott/domain/bookmark/repository/BookmarkRepository.javamodules/domain/src/main/java/com/ott/domain/media/domain/Media.java
|
이번 PR 리뷰에서 coderabbit/coderabbit-guidelines.md 읽었는지 알려줘. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
이번 PR 리뷰에서 🏁 Script executed: Length of output: 12253
가이드라인 파일을 읽었습니다! 이제 가이드라인에 따라 PR을 상세히 리뷰하겠습니다. 먼저 주요 파일들의 실제 코드를 확인하여 정확한 리뷰를 진행하겠습니다. 🏁 Script executed: Length of output: 11578 가이드라인을 바탕으로 코드를 검토했습니다. 발견된 이슈들을 하나씩 댓글로 남기겠습니다. [P1] 페이지 파라미터 타입 변경 필요 근거: 문제: 페이지/사이즈 파라미터가 가이드라인:
수정 제안: // 현재
public ResponseEntity<SuccessResponse<PageResponse<BookmarkMediaResponse>>> getBookmarkMediaList(
`@RequestParam`(defaultValue = "0") int page,
`@RequestParam`(defaultValue = "10") int size,
`@AuthenticationPrincipal` Long memberId)
// 수정
public ResponseEntity<SuccessResponse<PageResponse<BookmarkMediaResponse>>> getBookmarkMediaList(
`@RequestParam`(defaultValue = "0") Integer page,
`@RequestParam`(defaultValue = "10") Integer size,
`@AuthenticationPrincipal` Long memberId)동일하게 [P1] Repository에서 QueryDSL 사용 필요 근거: 문제: Spring Data JPA의 메서드 이름 기반 쿼리와 가이드라인:
수정 제안:
// BookmarkRepositoryCustom.java
public interface BookmarkRepositoryCustom {
Page<Bookmark> findBookmarkMediaList(Long memberId, Status status,
List<MediaType> mediaTypes, Pageable pageable);
Page<Bookmark> findBookmarkShortFormList(Long memberId, Status status,
MediaType mediaType, Pageable pageable);
}
// BookmarkRepositoryImpl.java에서 QueryDSL fetchJoin 사용[P2] Response DTO를 record로 변경 필요 근거:
문제: Response DTO가 가이드라인:
수정 제안: `@Schema`(description = "북마크한 콘텐츠 목록 응답 DTO")
public record BookmarkMediaResponse(
`@Schema`(type = "Long", description = "미디어 ID", example = "1")
Long mediaId,
`@Schema`(type = "String", description = "미디어 제목", example = "어서와요 김마루의 숲")
String title,
`@Schema`(type = "String", description = "미디어 설명", example = "허거덩의 숲에서 힐링을 즐겨봐요~")
String description,
`@Schema`(type = "String", description = "포스터 URL", example = "https://cdn.ott.com/posters/1.jpg")
String posterUrl
) {
public static BookmarkMediaResponse from(Bookmark bookmark) {
return new BookmarkMediaResponse(
bookmark.getMedia().getId(),
bookmark.getMedia().getTitle(),
bookmark.getMedia().getDescription(),
bookmark.getMedia().getPosterUrl()
);
}
}
[P2] 근거:
문제: 가이드라인:
수정 제안: `@Schema`(type = "Long", description = "미디어 ID", example = "1")
private Long mediaId;
`@Schema`(type = "String", description = "미디어 제목", example = "어서와요 김마루의 숲")
private String title;모든 필드에 [P2] Request DTO를 record로 변경 필요 근거: 문제: 단순한 요청 DTO가 가이드라인:
수정 제안: `@Schema`(description = "북마크 등록/취소 요청 DTO")
public record BookmarkRequest(
`@NotNull`(message = "mediaId는 필수입니다.")
`@Schema`(type = "Long", description = "북마크할 미디어 ID", example = "1")
Long mediaId
) {}[P2] 근거: 문제: 가이드라인:
수정 제안: public ResponseEntity<SuccessResponse<PageResponse<BookmarkMediaResponse>>> getBookmarkMediaList(
`@RequestParam`(value = "page", defaultValue = "0") Integer page,
`@RequestParam`(value = "size", defaultValue = "10") Integer size,
`@AuthenticationPrincipal` Long memberId)[P2] 근거: 문제: 클래스 레벨에 가이드라인:
수정 제안: `@Service`
`@RequiredArgsConstructor`
public class BookmarkService { // 클래스 레벨 `@Transactional` 제거
`@Transactional` // 각 메서드마다 명시
public void editBookmark(Long memberId, Long mediaId) {
// ...
}
`@Transactional`(readOnly = true)
public PageResponse<BookmarkMediaResponse> getBookmarkMediaList(...) {
// ...
}
`@Transactional`(readOnly = true)
public PageResponse<BookmarkShortFormResponse> getBookmarkShortFormList(...) {
// ...
}
}이상으로 가이드라인 기반 리뷰를 완료했습니다. 특히 P1 이슈(페이지 파라미터 타입, QueryDSL 사용)는 머지 전에 보완하시는 것을 권장드립니다! 🙂 |
There was a problem hiding this comment.
♻️ Duplicate comments (3)
apps/api-user/src/main/java/com/ott/api_user/bookmark/controller/BookmarkAPI.java (2)
67-71:⚠️ Potential issue | 🔴 Critical
getBookmarkShortFormList의memberId에@AuthenticationPrincipal어노테이션이 누락되었습니다.Line 70의
memberId파라미터에@AuthenticationPrincipal이 없어 인증된 사용자 ID가 주입되지 않습니다. 다른 메서드들(Line 38, 54)과 동일하게 어노테이션을 추가해야 합니다.As per coding guidelines
Strict USER/EDITOR/ADMIN authorization checks.🔒 수정 예시
ResponseEntity<SuccessResponse<PageResponse<BookmarkShortFormResponse>>> getBookmarkShortFormList( `@Parameter`(description = "페이지 번호 (0부터 시작)") `@RequestParam`(defaultValue = "0") `@Min`(0) Integer page, `@Parameter`(description = "페이지 크기") `@RequestParam`(defaultValue = "10") `@Min`(0) `@Max`(100) Integer size, - `@Parameter`(hidden = true) Long memberId + `@Parameter`(hidden = true) `@AuthenticationPrincipal` Long memberId );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-user/src/main/java/com/ott/api_user/bookmark/controller/BookmarkAPI.java` around lines 67 - 71, The parameter memberId in the controller method getBookmarkShortFormList is missing the `@AuthenticationPrincipal` annotation so the authenticated user ID won't be injected; update the method signature in BookmarkAPI to annotate the Long memberId parameter with `@AuthenticationPrincipal` (matching the other methods on the class) to ensure correct injection and downstream authorization checks.
52-53:⚠️ Potential issue | 🟠 Major페이지 크기 검증이
@Min(1)이어야 합니다.Line 53과 69의
size파라미터에@Min(0)이 적용되어 있어size=0이 허용됩니다. Spring Data의PageRequest.of(page, size)는size >= 1을 요구하므로size=0요청 시IllegalArgumentException이 발생합니다.🛠️ 수정 예시
ResponseEntity<SuccessResponse<PageResponse<BookmarkMediaResponse>>> getBookmarkMediaList( `@Parameter`(description = "페이지 번호 (0부터 시작)", required = true) `@RequestParam`(defaultValue = "0") `@Min`(0) Integer page, - `@Parameter`(description = "페이지 크기", required = true) `@RequestParam`(defaultValue = "10") `@Min`(0) `@Max`(100) Integer size, + `@Parameter`(description = "페이지 크기", required = true) `@RequestParam`(defaultValue = "10") `@Min`(1) `@Max`(100) Integer size, `@Parameter`(hidden = true) `@AuthenticationPrincipal` Long memberId );Line 69에도 동일하게 적용하세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-user/src/main/java/com/ott/api_user/bookmark/controller/BookmarkAPI.java` around lines 52 - 53, In BookmarkAPI update the request parameter validation for the page size: the handler methods that declare the Integer parameter named "size" currently use `@Min`(0) which allows size=0 and will cause PageRequest.of(page, size) to throw; change those annotations to `@Min`(1) (replace `@Min`(0) with `@Min`(1)) for the "size" parameter in the BookmarkAPI controller methods so only positive page sizes are accepted.apps/api-user/src/main/java/com/ott/api_user/bookmark/controller/BookmarkController.java (1)
44-51:⚠️ Potential issue | 🟠 Major컨트롤러 메서드에 페이지네이션 파라미터 검증 어노테이션이 적용되지 않았습니다.
검증 어노테이션 임포트(Line 6-8)는 추가되었지만 실제 파라미터에 적용되지 않았습니다. 인터페이스의 검증 제약은 문서용이며 런타임 검증을 강제하지 않으므로, 구현 클래스에서 직접 어노테이션을 적용해야 합니다.
🛠️ 수정 예시
public ResponseEntity<SuccessResponse<PageResponse<BookmarkMediaResponse>>> getBookmarkMediaList( - `@RequestParam`(defaultValue = "0") Integer page, - `@RequestParam`(defaultValue = "10") Integer size, + `@PositiveOrZero` `@RequestParam`(defaultValue = "0") Integer page, + `@Min`(1) `@Max`(100) `@RequestParam`(defaultValue = "10") Integer size, `@AuthenticationPrincipal` Long memberId) {
getBookmarkShortFormList메서드(Line 55-58)에도 동일하게 적용하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-user/src/main/java/com/ott/api_user/bookmark/controller/BookmarkController.java` around lines 44 - 51, The pagination parameters in BookmarkController's methods are missing runtime validation: add validation annotations to the method parameters in the implementation (e.g., annotate getBookmarkMediaList and getBookmarkShortFormList parameters: annotate page with `@PositiveOrZero` or `@Min`(0) and size with `@Min`(1) (and optionally `@Max`(100) if you want an upper bound)), and ensure the controller class is annotated with `@Validated` so those parameter constraints are enforced at runtime; apply these annotations to the method signatures for bookmarkService.getBookmarkMediaList(...) and the short form equivalent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@apps/api-user/src/main/java/com/ott/api_user/bookmark/controller/BookmarkAPI.java`:
- Around line 67-71: The parameter memberId in the controller method
getBookmarkShortFormList is missing the `@AuthenticationPrincipal` annotation so
the authenticated user ID won't be injected; update the method signature in
BookmarkAPI to annotate the Long memberId parameter with
`@AuthenticationPrincipal` (matching the other methods on the class) to ensure
correct injection and downstream authorization checks.
- Around line 52-53: In BookmarkAPI update the request parameter validation for
the page size: the handler methods that declare the Integer parameter named
"size" currently use `@Min`(0) which allows size=0 and will cause
PageRequest.of(page, size) to throw; change those annotations to `@Min`(1)
(replace `@Min`(0) with `@Min`(1)) for the "size" parameter in the BookmarkAPI
controller methods so only positive page sizes are accepted.
In
`@apps/api-user/src/main/java/com/ott/api_user/bookmark/controller/BookmarkController.java`:
- Around line 44-51: The pagination parameters in BookmarkController's methods
are missing runtime validation: add validation annotations to the method
parameters in the implementation (e.g., annotate getBookmarkMediaList and
getBookmarkShortFormList parameters: annotate page with `@PositiveOrZero` or
`@Min`(0) and size with `@Min`(1) (and optionally `@Max`(100) if you want an upper
bound)), and ensure the controller class is annotated with `@Validated` so those
parameter constraints are enforced at runtime; apply these annotations to the
method signatures for bookmarkService.getBookmarkMediaList(...) and the short
form equivalent.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/api-user/src/main/java/com/ott/api_user/bookmark/controller/BookmarkAPI.javaapps/api-user/src/main/java/com/ott/api_user/bookmark/controller/BookmarkController.javaapps/api-user/src/main/java/com/ott/api_user/bookmark/dto/request/BookmarkRequest.javaapps/api-user/src/main/java/com/ott/api_user/bookmark/dto/response/BookmarkMediaResponse.javaapps/api-user/src/main/java/com/ott/api_user/bookmark/dto/response/BookmarkShortFormResponse.javaapps/api-user/src/main/java/com/ott/api_user/bookmark/service/BookmarkService.javamodules/domain/src/main/java/com/ott/domain/bookmark/repository/BookmarkRepository.java
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api-user/src/main/java/com/ott/api_user/bookmark/dto/response/BookmarkMediaResponse.java
There was a problem hiding this comment.
♻️ Duplicate comments (3)
apps/api-user/src/main/java/com/ott/api_user/bookmark/controller/BookmarkController.java (2)
33-58:⚠️ Potential issue | 🟠 Major메서드 단위 역할 검증이 빠져 있습니다.
현재는 인증 주체만 주입되고, 엔드포인트별 USER/EDITOR/ADMIN 역할 제한이 코드에 명시되지 않습니다. 메서드에
@PreAuthorize(또는 동등한 정책)로 역할 체크를 명확히 추가해 주세요.As per coding guidelines, "Strict USER/EDITOR/ADMIN authorization checks".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-user/src/main/java/com/ott/api_user/bookmark/controller/BookmarkController.java` around lines 33 - 58, Add explicit method-level authorization to enforce USER/EDITOR/ADMIN roles on these endpoints: annotate editBookmark, getBookmarkMediaList, and getBookmarkShortFormList with a PreAuthorize expression (e.g. `@PreAuthorize`("hasAnyRole('USER','EDITOR','ADMIN')") or your project's equivalent) so the injected `@AuthenticationPrincipal` is validated against allowed roles; ensure you import the annotation and that method security is enabled in the application so the checks take effect.
45-57:⚠️ Potential issue | 🟠 Major페이지 파라미터 검증이 실제로 적용되지 않습니다.
page/size에 제약 애너테이션이 없고 클래스에@Validated도 없어, 음수/0/과대값이 그대로 서비스로 전달될 수 있습니다.제안 수정안
-@RestController +@RestController +@Validated `@RequiredArgsConstructor` `@RequestMapping`("/bookmarks") public class BookmarkController implements BookmarkAPI { @@ public ResponseEntity<SuccessResponse<PageResponse<BookmarkMediaResponse>>> getBookmarkMediaList( - `@RequestParam`(defaultValue = "0") Integer page, - `@RequestParam`(defaultValue = "10") Integer size, + `@PositiveOrZero` `@RequestParam`(defaultValue = "0") Integer page, + `@Min`(1) `@Max`(100) `@RequestParam`(defaultValue = "10") Integer size, `@AuthenticationPrincipal` Long memberId) { @@ public ResponseEntity<SuccessResponse<PageResponse<BookmarkShortFormResponse>>> getBookmarkShortFormList( - `@RequestParam`(defaultValue = "0") Integer page, - `@RequestParam`(defaultValue = "10") Integer size, + `@PositiveOrZero` `@RequestParam`(defaultValue = "0") Integer page, + `@Min`(1) `@Max`(100) `@RequestParam`(defaultValue = "10") Integer size, `@AuthenticationPrincipal` Long memberId) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-user/src/main/java/com/ott/api_user/bookmark/controller/BookmarkController.java` around lines 45 - 57, The page/size parameters in BookmarkController methods (e.g., getBookmarkShortFormList and getBookmarkMediaList) lack validation and `@Validated` on the controller, so invalid values can reach the service; add javax.validation annotations to the parameters (e.g., `@Min`(0) for page and `@Min`(1) or `@Positive` for size, and optionally `@Max`(100) for size) and annotate the controller class with `@Validated` so Spring enforces them before calling bookmarkService.getBookmarkMediaList / getBookmarkShortFormList; adjust parameter types if needed to support validation and update any API docs/tests accordingly.apps/api-user/src/main/java/com/ott/api_user/bookmark/service/BookmarkService.java (1)
55-80:⚠️ Potential issue | 🟠 Major동시 요청에서 북마크 상태/카운트 정합성이 깨질 수 있습니다.
현재 토글과 카운트 변경이 분리된 read-modify-write 흐름이라, 동일 사용자·미디어에 대한 동시 요청에서 결과가 비결정적으로 흔들릴 수 있습니다. 유니크 제약 + 비관적 락(또는 원자적 update)으로 한 트랜잭션 내에서 상태 전환과 카운트 반영을 묶어주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-user/src/main/java/com/ott/api_user/bookmark/service/BookmarkService.java` around lines 55 - 80, The toggle logic is vulnerable to race conditions because it reads and then writes bookmark and media counts separately; modify the flow to perform the read-modify-write inside a single transaction with a DB lock or atomic update: mark the service method as `@Transactional` and change the lookup to a locked read (e.g., use a repository method annotated with `@Lock`(PESSIMISTIC_WRITE) like findByMemberIdAndMediaId) so the block that calls bookmark.updateStatus(...) and targetMedia.increaseBookmarkCount()/decreaseBookmarkCount() executes under the same transaction and lock; for the insert-path, ensure you handle unique constraint violations (retry or load-with-lock after catching DataIntegrityViolation) instead of blind save, and use memberRepository.findById(...) inside the same transaction.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@apps/api-user/src/main/java/com/ott/api_user/bookmark/controller/BookmarkController.java`:
- Around line 33-58: Add explicit method-level authorization to enforce
USER/EDITOR/ADMIN roles on these endpoints: annotate editBookmark,
getBookmarkMediaList, and getBookmarkShortFormList with a PreAuthorize
expression (e.g. `@PreAuthorize`("hasAnyRole('USER','EDITOR','ADMIN')") or your
project's equivalent) so the injected `@AuthenticationPrincipal` is validated
against allowed roles; ensure you import the annotation and that method security
is enabled in the application so the checks take effect.
- Around line 45-57: The page/size parameters in BookmarkController methods
(e.g., getBookmarkShortFormList and getBookmarkMediaList) lack validation and
`@Validated` on the controller, so invalid values can reach the service; add
javax.validation annotations to the parameters (e.g., `@Min`(0) for page and
`@Min`(1) or `@Positive` for size, and optionally `@Max`(100) for size) and annotate
the controller class with `@Validated` so Spring enforces them before calling
bookmarkService.getBookmarkMediaList / getBookmarkShortFormList; adjust
parameter types if needed to support validation and update any API docs/tests
accordingly.
In
`@apps/api-user/src/main/java/com/ott/api_user/bookmark/service/BookmarkService.java`:
- Around line 55-80: The toggle logic is vulnerable to race conditions because
it reads and then writes bookmark and media counts separately; modify the flow
to perform the read-modify-write inside a single transaction with a DB lock or
atomic update: mark the service method as `@Transactional` and change the lookup
to a locked read (e.g., use a repository method annotated with
`@Lock`(PESSIMISTIC_WRITE) like findByMemberIdAndMediaId) so the block that calls
bookmark.updateStatus(...) and
targetMedia.increaseBookmarkCount()/decreaseBookmarkCount() executes under the
same transaction and lock; for the insert-path, ensure you handle unique
constraint violations (retry or load-with-lock after catching
DataIntegrityViolation) instead of blind save, and use
memberRepository.findById(...) inside the same transaction.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/api-user/src/main/java/com/ott/api_user/bookmark/controller/BookmarkAPI.javaapps/api-user/src/main/java/com/ott/api_user/bookmark/controller/BookmarkController.javaapps/api-user/src/main/java/com/ott/api_user/bookmark/service/BookmarkService.javaapps/api-user/src/main/java/com/ott/api_user/likes/service/LikesService.javamodules/common-web/src/main/java/com/ott/common/web/exception/ErrorCode.javamodules/domain/src/main/java/com/ott/domain/media/domain/Media.java
🚧 Files skipped from review as they are similar to previous changes (2)
- modules/domain/src/main/java/com/ott/domain/media/domain/Media.java
- apps/api-user/src/main/java/com/ott/api_user/bookmark/controller/BookmarkAPI.java
arlen02-01
left a comment
There was a problem hiding this comment.
확인 완료 했습니다! 고생하셨습니다~ ^^7
📝 작업 내용
📷 스크린샷
기존 상태 : memberId = 1, mediaId = 1, status = ACTIVE, bookmarkCount = 151
변경 후
변경 후 상태 : memberId = 1, mediaId = 1, status = DELETE , bookmarkCount = 150
☑️ 체크 리스트
#️⃣ 연관된 이슈
close #67
💬 리뷰 요구사항
쿼리쪽 자세히 봐주시면 감사하겠습니다.
Summary by CodeRabbit
새로운 기능
버그 수정 / 개선