[URECA-50] Feat: 요약 북마크 제작 - #28
Conversation
📝 WalkthroughWalkthrough컨트롤러에 PATCH /api/summaries/{summaryId}/bookmark 엔드포인트가 추가되고, SummaryController의 create 요청 파라미터에 Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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
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: 3
🤖 Fix all issues with AI agents
In @.gitignore:
- Around line 44-45: 현재 .gitignore에서 "!**/src/main/resources/gcp/unity-stt.json"
예외가 뒤이어 나오는 "src/main/resources/gcp/" 무시 규칙에 의해 무효화됩니다; 수정하려면 디렉터리 무시
규칙("src/main/resources/gcp/")을 먼저 두고 그 다음에 디렉터리 자체를 예외
처리("!src/main/resources/gcp/")한 뒤 파일
예외("!**/src/main/resources/gcp/unity-stt.json" 또는
"!src/main/resources/gcp/unity-stt.json") 순으로 배치하여 unity-stt.json이 실제로 추적되도록
하세요.
In `@src/main/java/com/ureca/unity/domain/summary/mapper/SummaryMapper.java`:
- Around line 21-24: The issue is that SummaryMapper.updateBookmark declares
`@Param`("bookmarkId") for a boolean parameter named bookmarkId which doesn't
match the #{isBookmarked} used in Summary.xml and is misleading (implies an ID);
update the mapper so the boolean parameter and its `@Param` use the same clear
name (e.g., rename the method parameter to isBookmarked and change `@Param` to
`@Param`("isBookmarked")), or alternatively change only the `@Param` to
"isBookmarked" while keeping the boolean parameter name consistent, ensuring
SummaryMapper.updateBookmark and Summary.xml refer to the identical param name
and avoid using "bookmarkId" for a boolean.
In `@src/main/resources/mapper/summary/Summary.xml`:
- Around line 38-42: The MyBatis parameter name mismatch causes a
BindingException: the SQL update (update id="updateBookmark") references
#{isBookmarked} but SummaryMapper.java declares `@Param`("bookmarkId"); fix by
making the names consistent—either change the XML parameter to #{bookmarkId} or
(preferred) update the mapper method in SummaryMapper.java to use
`@Param`("isBookmarked") (and rename the method parameter if needed) so the `@Param`
name matches the XML placeholder.
🧹 Nitpick comments (2)
src/main/java/com/ureca/unity/domain/summary/controller/SummaryController.java (1)
27-30: 토글 결과를 클라이언트에 반환하는 것을 고려해보세요.현재
void반환으로 클라이언트가 토글 후 북마크 상태를 알 수 없습니다. 프론트엔드에서 별도 API 호출 없이 UI를 즉시 갱신하려면 변경된 상태를 반환하는 것이 좋습니다.♻️ 개선 제안
- `@PatchMapping`("/{summaryId}/bookmark") - public void toggleBookmark(`@PathVariable` Long summaryId) { - summaryService.toggleBookmark(summaryId); - } + `@PatchMapping`("/{summaryId}/bookmark") + public Map<String, Boolean> toggleBookmark(`@PathVariable` Long summaryId) { + boolean newStatus = summaryService.toggleBookmark(summaryId); + return Map.of("isBookmarked", newStatus); + }서비스 메서드도
boolean반환으로 변경이 필요합니다.src/main/java/com/ureca/unity/domain/summary/service/SummaryService.java (1)
72-86: 북마크 토글 로직이 잘 구현되었습니다.
@Transactional로 감싸 읽기-수정 작업의 원자성을 보장하고, 존재하지 않는 Summary에 대한 예외 처리도 되어 있네요.몇 가지 개선 포인트:
- 반환값 추가: 토글 후 새로운 상태를 반환하면 컨트롤러에서 클라이언트에 응답 가능
- 커스텀 예외 고려:
IllegalArgumentException대신SummaryNotFoundException같은 도메인 예외를 사용하면@ControllerAdvice에서 일관된 에러 응답 처리가 수월합니다♻️ 반환값 추가 제안
`@Transactional` - public void toggleBookmark(Long summaryId) { + public boolean toggleBookmark(Long summaryId) { Boolean isBookmarked = summaryMapper.findBookmarkStatus(summaryId); if (isBookmarked == null) { throw new IllegalArgumentException("Summary not found"); } + boolean newStatus = !isBookmarked; summaryMapper.updateBookmark( summaryId, - !isBookmarked + newStatus ); + return newStatus; }
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/main/java/com/ureca/unity/domain/summary/service/SummaryService.java`:
- Around line 23-37: The createSummary method currently performs
geminiSummaryService.summarize(...) inside the `@Transactional` scope and uses
summaryMapper.findLatestSummaryId(...) after insertSummary, which can hold DB
locks and return wrong IDs under concurrency; refactor by extracting the initial
insert into a short transactional helper that returns the generated summaryId
(use JDBC/mapper generated key instead of findLatestSummaryId), call
geminiSummaryService.summarize(...) outside any transaction, then persist the
summary result in a separate `@Transactional` method (e.g.,
updateSummaryResultTx(summaryId, ...)). Also replace the read-then-write
toggleBookmark flow with an atomic update on the mapper (e.g.,
summaryMapper.updateBookmarkAtomic(summaryId) executing UPDATE summary SET
bookmarked = NOT bookmarked WHERE id = ?) to avoid race conditions.
- Around line 29-34: The current insert-then-select pattern in SummaryService
(calling summaryMapper.insertSummary(...) then
summaryMapper.findLatestSummaryId(...)) is race-prone; update the MyBatis mapper
(Summary.xml) to return the generated key using useGeneratedKeys="true" (or a
<selectKey>) on insertSummary and adjust the SummaryMapper interface method
signature to capture that generated id (e.g., a property like generatedId or
returning Long), then change SummaryService to use the id returned from
summaryMapper.insertSummary(...) instead of calling findLatestSummaryId(...).
This removes the DESC-based lookup and prevents concurrent requests from reading
another request's ID.
🧹 Nitpick comments (1)
src/main/java/com/ureca/unity/domain/summary/service/SummaryService.java (1)
70-79: 토글은 한 번의 UPDATE로 원자화하는 편이 안전합니다.
현재 read-then-write는 레이스/왕복이 발생합니다.UPDATE summary SET is_bookmarked = NOT is_bookmarked WHERE summary_id = ?형태로 한 번에 토글하고, 영향 행 수로 존재 여부를 판단하세요.개선 예시(요지)
- Boolean isBookmarked = summaryMapper.findBookmarkStatus(summaryId); - if (isBookmarked == null) { ... } - summaryMapper.updateBookmark(summaryId, !isBookmarked); + int updated = summaryMapper.toggleBookmark(summaryId); + if (updated == 0) { throw new IllegalArgumentException("Summary not found"); }
Key Changes
북마크 제작
작업 내역
💬 공유사항 to 리뷰어
비고
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선
Chores
✏️ Tip: You can customize this high-level summary in your review settings.