feat: 응원 보내기/조회 API 구현 - #12
Merged
Merged
Conversation
📝 WalkthroughWalkthroughChangesCheer feature
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
actor User
participant CheerController
participant CheerService
participant CheerRepository
User->>CheerController: POST /api/cheers
CheerController->>CheerService: sendCheer(user, request)
CheerService->>CheerRepository: check daily cheer
CheerService->>CheerRepository: save Cheer
CheerController-->>User: send success response
User->>CheerController: GET /api/cheers
CheerController->>CheerService: getRecentCheers(user)
CheerService->>CheerRepository: find latest 10 cheers
CheerController-->>User: recent cheer response
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (2)
src/main/java/com/team4/hackerton/domain/cheer/repository/CheerRepository.java (1)
11-16: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winN+1 query on
senderwhen fetching recent cheers.
senderis lazily fetched onCheer(seeentity/Cheer.javalines 36-38), andCheerService.getRecentCheersaccessescheer.getSender().getName()for each of up to 10 results — this triggers a separate SELECT per cheer since this query performs no join/eager fetch.⚡ Proposed fix using `@EntityGraph`
package com.team4.hackerton.domain.cheer.repository; import com.team4.hackerton.domain.cheer.entity.Cheer; import com.team4.hackerton.domain.track.entity.Track; import com.team4.hackerton.domain.user.entity.User; +import org.springframework.data.jpa.repository.EntityGraph; import org.springframework.data.jpa.repository.JpaRepository; import java.time.LocalDate; import java.util.List; public interface CheerRepository extends JpaRepository<Cheer, Long> { boolean existsBySenderAndTargetDate(User sender, LocalDate targetDate); + `@EntityGraph`(attributePaths = "sender") List<Cheer> findTop10ByTrackOrderByCreatedAtDesc(Track track); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/team4/hackerton/domain/cheer/repository/CheerRepository.java` around lines 11 - 16, Update CheerRepository.findTop10ByTrackOrderByCreatedAtDesc to fetch the lazy sender association in the same query, preferably using an `@EntityGraph`(attributePaths = "sender"). Verify CheerService.getRecentCheers uses the repository result without triggering additional sender queries.src/main/java/com/team4/hackerton/domain/cheer/service/CheerService.java (1)
22-55: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftNo unit tests for the new duplicate-cheer / track-lookup logic.
sendCheer's duplicate-prevention andUSER_TRACK_NOT_FOUNDbranches are business-critical but untested in this PR.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/team4/hackerton/domain/cheer/service/CheerService.java` around lines 22 - 55, Add unit tests for CheerService.sendCheer covering successful cheer creation, duplicate detection via existsBySenderAndTargetDate resulting in ALREADY_CHEERED_TODAY, and missing current track resulting in USER_TRACK_NOT_FOUND. Mock CheerRepository and UserTrackRepository, and verify CheerRepository.save is called only for the successful path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/com/team4/hackerton/domain/cheer/service/CheerService.java`:
- Around line 27-39: Handle the race between existsBySenderAndTargetDate and
save in sendCheer by catching the database unique-constraint
DataIntegrityViolationException and translating it to
AppException(CheerErrorCode.ALREADY_CHEERED_TODAY), while retaining the existing
pre-check for normal cases. Ensure only the duplicate sender/target-date
violation is translated and unrelated persistence errors are rethrown.
---
Nitpick comments:
In
`@src/main/java/com/team4/hackerton/domain/cheer/repository/CheerRepository.java`:
- Around line 11-16: Update CheerRepository.findTop10ByTrackOrderByCreatedAtDesc
to fetch the lazy sender association in the same query, preferably using an
`@EntityGraph`(attributePaths = "sender"). Verify CheerService.getRecentCheers
uses the repository result without triggering additional sender queries.
In `@src/main/java/com/team4/hackerton/domain/cheer/service/CheerService.java`:
- Around line 22-55: Add unit tests for CheerService.sendCheer covering
successful cheer creation, duplicate detection via existsBySenderAndTargetDate
resulting in ALREADY_CHEERED_TODAY, and missing current track resulting in
USER_TRACK_NOT_FOUND. Mock CheerRepository and UserTrackRepository, and verify
CheerRepository.save is called only for the successful path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fca9264b-1152-48d8-ac8b-27676f355dff
📒 Files selected for processing (8)
src/main/java/com/team4/hackerton/domain/cheer/code/CheerErrorCode.javasrc/main/java/com/team4/hackerton/domain/cheer/code/CheerSuccessCode.javasrc/main/java/com/team4/hackerton/domain/cheer/controller/CheerController.javasrc/main/java/com/team4/hackerton/domain/cheer/dto/request/CheerRequest.javasrc/main/java/com/team4/hackerton/domain/cheer/dto/response/CheerResponse.javasrc/main/java/com/team4/hackerton/domain/cheer/entity/Cheer.javasrc/main/java/com/team4/hackerton/domain/cheer/repository/CheerRepository.javasrc/main/java/com/team4/hackerton/domain/cheer/service/CheerService.java
Comment on lines
+27
to
+39
| @Transactional | ||
| public void sendCheer(User sender, CheerRequest request) { | ||
| UserTrack senderTrack = userTrackRepository.findByUserAndIsCurrentTrue(sender) | ||
| .orElseThrow(() -> new AppException(CheerErrorCode.USER_TRACK_NOT_FOUND)); | ||
|
|
||
| LocalDate today = LocalDate.now(); | ||
|
|
||
| if (cheerRepository.existsBySenderAndTargetDate(sender, today)) { | ||
| throw new AppException(CheerErrorCode.ALREADY_CHEERED_TODAY); | ||
| } | ||
|
|
||
| cheerRepository.save(new Cheer(sender, senderTrack.getTrack(), request.getContent(), today)); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
TOCTOU race on duplicate-cheer check.
The existsBySenderAndTargetDate check and save() aren't atomic. Two concurrent requests from the same sender can both pass the check, and the second save() will violate the DB unique constraint (sender_id, target_date) and throw an unhandled DataIntegrityViolationException — surfacing as a 500 instead of the intended ALREADY_CHEERED_TODAY (409). The DB constraint still prevents duplicate rows, but the error contract breaks under this race.
🔒 Proposed fix: catch and translate the constraint violation
+import org.springframework.dao.DataIntegrityViolationException;
+
`@Transactional`
public void sendCheer(User sender, CheerRequest request) {
UserTrack senderTrack = userTrackRepository.findByUserAndIsCurrentTrue(sender)
.orElseThrow(() -> new AppException(CheerErrorCode.USER_TRACK_NOT_FOUND));
LocalDate today = LocalDate.now();
if (cheerRepository.existsBySenderAndTargetDate(sender, today)) {
throw new AppException(CheerErrorCode.ALREADY_CHEERED_TODAY);
}
- cheerRepository.save(new Cheer(sender, senderTrack.getTrack(), request.getContent(), today));
+ try {
+ cheerRepository.save(new Cheer(sender, senderTrack.getTrack(), request.getContent(), today));
+ } catch (DataIntegrityViolationException e) {
+ throw new AppException(CheerErrorCode.ALREADY_CHEERED_TODAY);
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @Transactional | |
| public void sendCheer(User sender, CheerRequest request) { | |
| UserTrack senderTrack = userTrackRepository.findByUserAndIsCurrentTrue(sender) | |
| .orElseThrow(() -> new AppException(CheerErrorCode.USER_TRACK_NOT_FOUND)); | |
| LocalDate today = LocalDate.now(); | |
| if (cheerRepository.existsBySenderAndTargetDate(sender, today)) { | |
| throw new AppException(CheerErrorCode.ALREADY_CHEERED_TODAY); | |
| } | |
| cheerRepository.save(new Cheer(sender, senderTrack.getTrack(), request.getContent(), today)); | |
| } | |
| import org.springframework.dao.DataIntegrityViolationException; | |
| `@Transactional` | |
| public void sendCheer(User sender, CheerRequest request) { | |
| UserTrack senderTrack = userTrackRepository.findByUserAndIsCurrentTrue(sender) | |
| .orElseThrow(() -> new AppException(CheerErrorCode.USER_TRACK_NOT_FOUND)); | |
| LocalDate today = LocalDate.now(); | |
| if (cheerRepository.existsBySenderAndTargetDate(sender, today)) { | |
| throw new AppException(CheerErrorCode.ALREADY_CHEERED_TODAY); | |
| } | |
| try { | |
| cheerRepository.save(new Cheer(sender, senderTrack.getTrack(), request.getContent(), today)); | |
| } catch (DataIntegrityViolationException e) { | |
| throw new AppException(CheerErrorCode.ALREADY_CHEERED_TODAY); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/team4/hackerton/domain/cheer/service/CheerService.java`
around lines 27 - 39, Handle the race between existsBySenderAndTargetDate and
save in sendCheer by catching the database unique-constraint
DataIntegrityViolationException and translating it to
AppException(CheerErrorCode.ALREADY_CHEERED_TODAY), while retaining the existing
pre-check for normal cases. Ensure only the duplicate sender/target-date
violation is translated and unrelated persistence errors are rethrown.
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.
Summary by CodeRabbit