Conversation
|
Note
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | PR 제목은 주요 변경사항인 '물풍선 동시 요청 문제' 해결을 명확하게 요약하고 있습니다. |
| Linked Issues check | ✅ Passed | 코드 변경사항이 이슈 #1027의 주요 목표인 동시성 문제 해결(INSERT...ON DUPLICATE KEY 사용) 및 자신에게 보내는 리마인더 에러 코드 명확화를 모두 충족합니다. |
| Out of Scope Changes check | ✅ Passed | 모든 변경사항이 이슈 #1027의 범위 내에 있으며, 동시성 문제 해결과 에러 코드 명확화에 직접 관련된 변경들입니다. |
| Description check | ✅ Passed | PR 설명이 템플릿 구조를 따르고 있으며, 관련 이슈, 문제점, 해결 방안, 주요 변경사항이 명확히 기술되어 있습니다. |
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
fix/1027
Tip
📝 Customizable high-level summaries are now available in beta!
You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
- Provide your own instructions using the
high_level_summary_instructionssetting. - Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
- Use
high_level_summary_in_walkthroughto move the summary from the description to the walkthrough section.
Example instruction:
"Divide the high-level summary into five sections:
- 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
- 📓 References — List relevant issues, discussions, documentation, or related PRs.
- 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
- 📊 Contributor Summary — Include a Markdown table showing contributions:
| Contributor | Lines Added | Lines Removed | Files Changed |- ✔️ Additional Notes — Add any extra reviewer context.
Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.
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 help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
backend/src/main/java/backend/mulkkam/friend/service/FriendReminderHistoryService.java (1)
3-4: 서비스 레벨 self-reminder 검증 추가는 방향성 좋습니다요청 초기에
senderId와friendId를 비교해서NOT_ALLOWED_SELF_REMINDER를 던지는 것은, 컨트롤러 레이어에서 바로 명시적인 에러 코드를 반환할 수 있어 사용자 입장에서 훨씬 이해하기 좋은 흐름이라고 생각합니다. 도메인 생성자에서 동일한 예외를 던지고 있지만, 현재getOrCreateDefault는 native insert를 통해 엔티티 생성자를 거치지 않기 때문에, 서비스 레벨에서 규칙을 한 번 더 방어하는 것도 합리적인 선택입니다.다만 두 가지 정도는 참고해 보시면 좋겠습니다.
규칙 중복 위치 정리
- 현재:
FriendReminderHistory생성자 +FriendReminderHistoryService.createAndSendReminder두 곳에서 같은 규칙을 검사.- 장점: 어떤 경로로 객체를 만들어도 self-reminder를 허용하지 않는 “이중 안전장치”.
- 단점: 나중에 비즈니스 규칙/에러 코드가 바뀌면 두 곳을 모두 수정해야 함.
- 대안: 예를 들어 아래처럼 도메인 정적 메서드나 별도 Validator로 공통화하면 규칙 변경 지점을 1곳으로 줄일 수 있습니다.
public static void validateSenderAndRecipient(Long senderId, Long recipientId) { if (senderId.equals(recipientId)) { throw new CommonException(NOT_ALLOWED_SELF_REMINDER); } }null-safe 비교 (선택)
- 현재 구현에서는
MemberDetails.id()와CreateFriendReminderRequest.memberId()가 null이 아닐 것이라 가정하고equals를 쓰고 있습니다.- 이 가정이 도메인상 항상 보장된다면 그대로 두어도 충분하지만, 방어적으로 가고 싶다면
Objects.equals(senderId, friendId)를 사용하는 것도 한 가지 옵션입니다.
(장점: NPE 리스크 제거, 단점: 의미상 “둘 다 null인 경우도 self-reminder로 본다”는 의미가 생김)현재 구현만으로도 요구사항은 잘 충족하고 있어 보이고, 위 내용은 향후 리팩터링 시 참고용 정도로 봐주시면 될 것 같습니다.
Also applies to: 8-8, 43-45
backend/src/main/java/backend/mulkkam/friend/repository/FriendReminderHistoryRepository.java (1)
24-35: ON DUPLICATE KEY 기반 createIfAbsent 동시성 처리 방향은 적절합니다
INSERT ... ON DUPLICATE KEY UPDATE를 이용해 “읽고-나서-삽입” 패턴의 경쟁 조건을 DB 레벨에서 해소한 점은, InnoDB Repeatable Read 환경에서 발생하던 고전적인 race를 깔끔하게 해결하는 선택으로 보입니다. 읽기 시점에 행을 못 보더라도, 최종적으로는 unique key 충돌이 update로 흡수되기 때문에 500 에러 없이 단일 row만 유지되는 장점이 있습니다.다만 몇 가지 부수 효과/트레이드오프도 같이 인지해 두시면 좋겠습니다.
DB 벤더 종속성
- 장점: MySQL/InnoDB에서는 구현이 간단하고 성능도 우수합니다.
- 단점:
ON DUPLICATE KEY는 MySQL 전용 문법이라, RDBMS를 변경할 경우 동일한 쿼리를 그대로 사용할 수 없습니다.- 대안:
- (현재처럼) MySQL에 고정된 서비스라면 유지비가 가장 적고 실용적인 선택.
- 향후 DB 변경 가능성이 크다면, JPA의 비관적 락(
select ... for update)이나 application 레벨 lock 등을 고려할 수 있으나, 구현 복잡도와 부하가 꽤 증가하는 단점이 있습니다.감사/공통 필드(BaseEntity)와의 정합성
- native insert는 JPA 엔티티 리스너나 auditing(@CreatedDate, @LastModifiedDate 등)을 타지 않는 경우가 많습니다.
- 만약
FriendReminderHistory의 생성/수정 시간 필드를 비즈니스 로직에서 활용한다면:
- DB 컬럼에 default/trigger를 두거나,
- upsert 이후
get(...)로 조회한 값을 사용한다는 점을 팀 내에서 명확히 공유해 두는 것이 좋습니다.- 현재
getOrCreateDefault는 insert 이후 즉시get(...)으로 다시 조회하므로, 이후 로직은 항상 JPA 엔티티를 사용한다는 점은 장점입니다.쿼리 표현 개선 (선택 사항)
ON DUPLICATE KEY UPDATE remaining = friend_reminder_history.remaining은 본질적으로 no-op update입니다.- 의도를 더 드러내고 싶다면
remaining = remaining처럼 단순하게 표현하는 것도 한 가지 방법입니다. 의미는 동일하지만, “기존 값을 그대로 유지한다”는 의도가 조금 더 직관적입니다.전반적으로 동시성 문제를 해결하려는 방향은 잘 잡혀 있고, 위 내용은 향후 유지보수/DB 전략을 세울 때 참고용으로 봐주시면 좋겠습니다.
backend/src/main/java/backend/mulkkam/friend/service/query/FriendReminderHistoryQueryService.java (1)
3-4: getOrCreateDefault의 upsert 흐름은 동시성 관점에서 적절하며, 예외 처리와 트랜잭션 설정만 약간 다듬으면 좋겠습니다현재 구현은 다음 면에서 잘 설계되어 있습니다.
findBy...로 우선 조회 후, 없을 때만createIfAbsent를 호출하되, unique 제약 +ON DUPLICATE KEY덕분에 동시 insert에서도 예외 없이 단일 row만 유지됩니다.- insert 이후 같은 트랜잭션 안에서
get(senderId, friendId, date)를 다시 호출해 JPA 엔티티를 확보하므로, 이후 로직은 항상 엔티티 기반으로 동작할 수 있습니다.- 클래스에
@Transactional(readOnly = true)가 걸려 있지만,getOrCreateDefault에 별도@Transactional이 있어 쓰기 트랜잭션으로 override된 점도 의도와 잘 맞습니다.조금만 다듬어 보면 더 좋을 부분은 아래 정도입니다.
예외 타입 명시 (get 메서드)
현재:
public FriendReminderHistory get(Long senderId, Long friendId, LocalDate date) { return friendReminderHistoryRepository.findBySenderIdAndRecipientIdAndQuotaDate(senderId, friendId, date) .orElseThrow(); }
orElseThrow()는NoSuchElementException을 던져서, 상위 계층(서비스/컨트롤러)에서 어떤 에러 코드로 매핑해야 하는지 다소 모호합니다.대안:
- 도메인/공통 예외(
CommonException)를 던지도록 바꾸면 에러 응답을 더 일관되게 매핑할 수 있습니다.- 장점: API 스펙과 에러 코드가 명시적, 로깅 시도 메인 흐름에서 바로 의도 파악 가능.
- 단점: 공통 예외 코드 정의/선택이 조금 필요합니다.
트랜잭션 속성 가독성 (선택)
현재도 동작에는 문제가 없지만, 클래스가 readOnly=true라서 메서드만 보고는 “여기서도 read-only인가?” 헷갈릴 수 있습니다.
필요하다면 다음처럼 명시적으로 적어두면, 팀원들이 코드를 읽을 때 의도가 더 분명해집니다.
@Transactional(readOnly = false) public FriendReminderHistory getOrCreateDefault(...) { ... }전반적으로 동시성 이슈를 해결하려는 구조 자체는 잘 짜여 있고, 위 내용은 향후 에러 처리 일관성 및 가독성을 높이기 위한 개선 포인트로 보시면 될 것 같습니다.
Also applies to: 24-25, 29-32
backend/src/test/java/backend/mulkkam/friend/service/FriendReminderHistoryServiceIntegrationTest.java (1)
1-90: 감소 로직 동시성은 잘 검증되지만, “초기 생성 시” 동시성 시나리오는 한 번 더 잡아두면 좋겠습니다이번 통합 테스트는 다음 면에서 의미 있게 동작하고 있습니다.
- 동일한
FriendReminderHistory(TODAY 기준, remaining=10)를 미리 만들어 둔 뒤,- 10개의 동시
createAndSendReminder호출을 통해tryReduceRemaining기반 감소 로직이 정확히 0까지 떨어지는지 검증하고 있습니다.- 만약 감소 로직에 race가 있었다면 remaining이 0이 아니거나 음수가 되는 등 비정상 상태가 나왔을 테니, 감소 부분에 대한 회귀 방지 역할은 잘 하고 있습니다.
다만 PR 설명에 나온 “동시 요청 시 unique key 충돌로 인한 500 에러”는 “FriendReminderHistory가 아직 없는 상태에서 동시에 생성될 때” 발생하는 문제라서, 그 시나리오를 직접적으로 커버하는 테스트가 하나 더 있으면 좋겠습니다.
동시 생성 시나리오 통합 테스트 제안
- 현재 테스트는
setup()에서 이미FriendReminderHistory를 하나 만들어 둡니다.- 별도의 테스트를 추가해, 초기 history를 만들지 않은 상태에서 다음을 검증하는 것을 추천드립니다.
- 동일한 (sender, recipient, date)에 대해 N개의 동시
createAndSendReminder호출을 수행.- 예외가 발생하지 않고 모든 요청이 정상 종료하는지 (예: Future 결과 확인이나 예외 수집).
- DB에 생성된
FriendReminderHistoryrow가 정확히 1건인지, 그리고 remaining 값이 기대한 값(INIT_REMAINING_VALUE - 성공한 호출 수)인지.간단한 예시 스케치는 아래와 같습니다.
@Test @DisplayName("초기 이력이 없는 상태에서 동시 요청 시, 단일 이력만 생성되고 예외가 발생하지 않는다") void createAndSendReminder_concurrentCreate() throws Exception { // given // setup에서는 FriendReminderHistory를 만들지 않도록 별도 설정 or 이 테스트만 다른 setup 사용 int concurrentRequests = 10; ExecutorService executor = Executors.newFixedThreadPool(concurrentRequests); CountDownLatch latch = new CountDownLatch(concurrentRequests); CreateFriendReminderRequest request = new CreateFriendReminderRequest(friend.getId()); MemberDetails memberDetails = new MemberDetails(sender); // when for (int i = 0; i < concurrentRequests; i++) { executor.submit(() -> { try { friendReminderHistoryService.createAndSendReminder(request, memberDetails); } finally { latch.countDown(); } }); } latch.await(); executor.shutdown(); // then List<FriendReminderHistory> histories = friendReminderHistoryRepository.findAll(); // 필요시 sender/recipient/date로 필터링 assertThat(histories).hasSize(1); }이렇게 해두면, **이번 PR의 핵심 목적이었던 “DB 레벨 upsert를 통한 중복 생성 방지”**가 통합 테스트 레벨에서 직접적으로 검증됩니다.
시간 소스 일관성 (선택 사항)
- 테스트에서는
LocalDate TODAY = LocalDate.now();를 사용하고, 서비스는CityDateTime.now(City.SEOUL).getLocalDate()를 사용합니다.- 대부분의 환경에서는 둘이 같겠지만, 서버 타임존 설정이나 자정 경계 상황에서 미묘하게 어긋날 가능성이 있습니다.
- 가능하다면 테스트에서도 동일한 유틸을 사용하는 편이 안전합니다.
private final LocalDate TODAY = CityDateTime.now(City.SEOUL).getLocalDate();현재 테스트만으로도 감소 로직의 동시성은 잘 검증되고 있으니, 위 내용은 “생성 시 동시성에 대한 방어막을 더 두텁게 하는 보완” 정도로 봐주시면 좋겠습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backend/src/main/java/backend/mulkkam/friend/domain/FriendReminderHistory.java(1 hunks)backend/src/main/java/backend/mulkkam/friend/repository/FriendReminderHistoryRepository.java(1 hunks)backend/src/main/java/backend/mulkkam/friend/service/FriendReminderHistoryService.java(2 hunks)backend/src/main/java/backend/mulkkam/friend/service/query/FriendReminderHistoryQueryService.java(2 hunks)backend/src/test/java/backend/mulkkam/friend/service/FriendReminderHistoryServiceIntegrationTest.java(1 hunks)backend/src/test/java/backend/mulkkam/friend/service/FriendReminderHistoryServiceUnitTest.java(0 hunks)
💤 Files with no reviewable changes (1)
- backend/src/test/java/backend/mulkkam/friend/service/FriendReminderHistoryServiceUnitTest.java
🧰 Additional context used
📓 Path-based instructions (1)
backend/**
⚙️ CodeRabbit configuration file
backend/**: - 1. 팀 및 공식 컨벤션, 가독성, 예외처리, 테스트/확장/유지보수성, 모듈화, API/DB/보안 설계 기준을 기반으로 리뷰해주세요.
- 최신 트렌드, 불필요한 로직, 클린코드, 리팩토링, 서비스/도메인 설계, 공통 예외 처리, 확장성도 함께 확인해주세요.
- 각 피드백은 문제점·대안·장단점을 짧고 논리적으로, 예시 코드가 있다면 간결히 포함해 주세요.
- 팀 내 스타일 통일성도 확인해주세요.
- 미작성한 테스트 코드 케이스가 있다면, 어떤 테스트가 필요한지 제안해주세요. (예: 컨트롤러는 인수 테스트, 나머지는 단위 테스트)
- 리뷰 남겨주는 부분은 해당 라인 범위의 코멘트에 작성해주세요.
Files:
backend/src/main/java/backend/mulkkam/friend/repository/FriendReminderHistoryRepository.javabackend/src/main/java/backend/mulkkam/friend/service/query/FriendReminderHistoryQueryService.javabackend/src/main/java/backend/mulkkam/friend/domain/FriendReminderHistory.javabackend/src/main/java/backend/mulkkam/friend/service/FriendReminderHistoryService.javabackend/src/test/java/backend/mulkkam/friend/service/FriendReminderHistoryServiceIntegrationTest.java
🧬 Code graph analysis (1)
backend/src/test/java/backend/mulkkam/friend/service/FriendReminderHistoryServiceIntegrationTest.java (1)
backend/src/test/java/backend/mulkkam/support/fixture/member/MemberFixtureBuilder.java (1)
MemberFixtureBuilder(9-100)
⏰ 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: test
🔇 Additional comments (1)
backend/src/main/java/backend/mulkkam/friend/domain/FriendReminderHistory.java (1)
27-27: INIT_REMAINING_VALUE를 public으로 노출한 설계에 대한 의견도메인 엔티티가 초기 quota 값을 단일 소스로 제공하도록 public 상수로 노출한 건, Repository/Service/테스트에서 모두 동일한 값을 참조할 수 있어서 유지보수 측면에서 장점이 있습니다.
다만 이 값이 “비즈니스 설정값(정책)”이라면,
- 현재처럼 엔티티 상수로 두면: 코드에서 추적은 쉽고 단순하지만, 나중에 운영 환경별로 값을 바꾸거나 A/B 테스트를 하기는 어렵고,
- 별도 설정/정책 컴포넌트(예: FriendReminderPolicy, 설정 프로퍼티)로 분리하면: 유연성은 커지지만 의존성이 늘고 주입/테스트 코드가 조금 복잡해지는 트레이드오프가 있습니다.
현 시점에서는 변경 목적에 잘 맞는 선택으로 보이고, 추후 값이 자주 바뀔 필요가 생기면 정책 클래스로의 분리 정도만 고려해 보시면 좋겠습니다.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/src/main/java/backend/mulkkam/friend/controller/FriendController.java(1 hunks)backend/src/test/java/backend/mulkkam/friend/service/FriendReminderHistoryServiceIntegrationTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/src/test/java/backend/mulkkam/friend/service/FriendReminderHistoryServiceIntegrationTest.java
🧰 Additional context used
📓 Path-based instructions (1)
backend/**
⚙️ CodeRabbit configuration file
backend/**: - 1. 팀 및 공식 컨벤션, 가독성, 예외처리, 테스트/확장/유지보수성, 모듈화, API/DB/보안 설계 기준을 기반으로 리뷰해주세요.
- 최신 트렌드, 불필요한 로직, 클린코드, 리팩토링, 서비스/도메인 설계, 공통 예외 처리, 확장성도 함께 확인해주세요.
- 각 피드백은 문제점·대안·장단점을 짧고 논리적으로, 예시 코드가 있다면 간결히 포함해 주세요.
- 팀 내 스타일 통일성도 확인해주세요.
- 미작성한 테스트 코드 케이스가 있다면, 어떤 테스트가 필요한지 제안해주세요. (예: 컨트롤러는 인수 테스트, 나머지는 단위 테스트)
- 리뷰 남겨주는 부분은 해당 라인 범위의 코멘트에 작성해주세요.
Files:
backend/src/main/java/backend/mulkkam/friend/controller/FriendController.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: test
backend/src/main/java/backend/mulkkam/friend/controller/FriendController.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
backend/src/main/java/backend/mulkkam/friend/service/query/FriendReminderHistoryQueryService.java (1)
18-21: 조회 실패 시orElseThrow()의 예외 타입을 도메인에 맞게 명시하는 것 고려현재는
findBySenderIdAndRecipientIdAndQuotaDate(...).orElseThrow()로Optional비어있는 경우NoSuchElementException이 그대로 올라가게 됩니다.
장점
- 코드가 매우 간단하고, “여기서 못 찾히면 진짜 버그”인 경우에는 그대로도 괜찮습니다.
단점
- 다른 서비스/컨트롤러에서 재사용될 경우, 도메인 의미가 “데이터 없음”임에도 불구하고 기술 예외(
NoSuchElementException)가 500으로 노출될 수 있습니다.- 프로젝트 전체가
CommonException+ 에러코드 방식으로 통일되어 있다면 이 메서드만 예외 스타일이 달라집니다.개선 방향 제안 (선택지)
- Optional 반환으로 노출
public Optional<FriendReminderHistory> get(...)로 바꾸고, 사용 측에서orElseThrow(new CommonException(...))등으로 도메인에 맞게 처리.- 장점: QueryService는 “조회”만 책임지고, 에러 정책은 상위 레이어에 위임.
- 단점: 기존 호출부 변경 필요.
- 도메인 예외 명시
return friendReminderHistoryRepository.findBySenderIdAndRecipientIdAndQuotaDate(senderId, friendId, date) .orElseThrow(() -> new CommonException(SOME_NOT_FOUND_ERROR_CODE));
- 장점: 호출부는 깔끔하게 유지하면서도 일관된 예외 정책.
- 단점: “없어도 되는” 조회에도 이 메서드를 재사용하기 애매해질 수 있음.
현재/향후 사용처가 “무조건 존재해야 하는 상황인지”, “없을 수도 있는 조회인지”에 따라 위 선택지 중 하나를 택해보시면 좋을 것 같습니다.
backend/src/test/java/backend/mulkkam/friend/service/FriendReminderHistoryServiceUnitTest.java (1)
74-91:LocalDate.now()직접 사용으로 인한 테스트 취약성과createIfAbsent호출 검증 누락이번 유닛 테스트 변경 방향 자체는 새 커맨드 서비스 시그니처에 잘 맞춰져 있는데, 두 가지 정도 보완 포인트가 있습니다.
LocalDate.now()를 그대로 검증 값에 사용하는 부분 (Line 78, 88, 115, 119, 127)
- 현재 서비스 내부도
LocalDate.now()를 사용하고, 테스트에서도LocalDate now = LocalDate.now();를 사용해 동일 값을 기대하고 있습니다.- 대부분 상황에서는 문제 없지만, 테스트 실행 시점이 자정 경계(23:59:59 → 00:00:00)를 넘는 극단적인 경우에는
- 테스트의
now와 서비스 내부의now가 다른 날짜가 될 수 있어, 매우 드물게 플래키 테스트가 될 여지가 있습니다.개선 방안 (선택지)
- (A) Clock 주입으로 서비스/테스트 모두 고정 시각 사용
- 서비스에
Clock을 주입해서LocalDate.now(clock)로 계산하고, 테스트에서는Clock.fixed(...)를 주입해 원하는 날짜를 고정.- 장점: 시간 관련 테스트가 완전히 결정적이며, 이후 시간/타임존 요구사항이 늘어나도 확장성이 좋습니다.
- 단점: 서비스 시그니처/구성이 조금 더 복잡해집니다.
- (B) 테스트에서 날짜 값 자체는 느슨하게 매칭
verify(friendReminderHistoryCommandService).reduceRemainingCount(eq(senderId), eq(friendId), any(LocalDate.class));
- 실패 케이스도 마찬가지로
any(LocalDate.class)를 사용할 수 있습니다.- 장점: 코드 변경이 최소이고, 자정 경계 플래키가 사라집니다.
- 단점: “정말 오늘 날짜를 쓰고 있는지”까지는 검증하지 못합니다.
- (C) 서비스에서 날짜를 인자로 받도록 바꾸고, 테스트에서 동일 값을 전달
createAndSendReminder(request, memberDetails, today)같은 형태로 확장.- 장점: 비즈니스 로직은 “전달받은 날짜”만 신뢰하게 되어 테스트 작성이 명확해집니다.
- 단점: 퍼블릭 API 변경 범위가 커집니다.
현재 요구사항을 보면 (B)만 적용해도 실질적인 플래키 위험은 거의 사라지므로, 비용 대비 효과는 (B) → (A) 순으로 보입니다.
FriendReminderHistoryCommandService.createIfAbsent호출에 대한 검증 부재새 설계 상
createAndSendReminder는
- 친구 관계 검증
createIfAbsent(senderId, friendId, today)reduceRemainingCount(senderId, friendId, today)
순으로 동작하는 것으로 보입니다(통합 테스트 및 커맨드 서비스 변경 맥락 기준).현재 유닛 테스트에서는:
- 성공 케이스 (Line 74~91):
reduceRemainingCount호출만 검증- 친구 아님 / 일일 한도 초과 케이스 (Line 93~129):
reduceRemainingCount의 호출 여부만 검증 (never()포함)이렇게 되면,
- “친구 아님” 시나리오에서
createIfAbsent가 호출되어도 테스트가 잡지 못합니다.- 반대로 성공 시나리오에서 누군가 실수로
createIfAbsent를 제거해도, 통합 테스트 외에는 바로 감지가 안 됩니다.보완 제안 (예시 코드)
- 성공 케이스에서:
verify(friendReminderHistoryCommandService).createIfAbsent(eq(senderId), eq(friendId), any(LocalDate.class)); verify(friendReminderHistoryCommandService).reduceRemainingCount(eq(senderId), eq(friendId), any(LocalDate.class));- 친구 아님 / self-reminder 등 실패 케이스에서:
verify(friendReminderHistoryCommandService, never()) .createIfAbsent(anyLong(), anyLong(), any(LocalDate.class)); verify(friendReminderHistoryCommandService, never()) .reduceRemainingCount(anyLong(), anyLong(), any(LocalDate.class));이렇게 하면
- 설계 상 “친구 검증에 실패하면 어떤 write도 발생하지 않는다”는 정책을 테스트 레벨에서 명시적으로 보장할 수 있고,
- 향후 리팩터링 시 플로우가 어긋나더라도 바로 깨지는 안전망이 됩니다.
요약하면, 현재 테스트는 새 설계와 잘 맞는 편이지만, 시간 처리와
createIfAbsent호출에 대한 검증을 조금만 더 강화하면 장기적으로 훨씬 안정적인 회귀 테스트가 될 것 같습니다.Also applies to: 95-109, 111-129
backend/src/main/java/backend/mulkkam/friend/repository/FriendReminderHistoryRepository.java (1)
16-29: 잔여 횟수 감소 쿼리와 MySQL 업서트의 전제 조건을 코드/스키마 레벨에서 명확히 하기
tryReduceRemaining와createIfAbsent조합으로 동시성 이슈를 DB 레벨에서 잘 해결하신 것 같습니다. 다만, 몇 가지 전제와 향후 유지보수를 위해 보완하면 좋은 부분들이 있습니다.
tryReduceRemaining쿼리 조건 (Line 18-24, 25-29)UPDATE FriendReminderHistory h SET h.remaining = h.remaining - 1 WHERE h.senderId = :senderId AND h.recipientId = :friendId AND h.quotaDate = :quotaDate AND h.remaining > 0
- 장점
remaining > 0조건으로 음수로 내려가는 것을 DB 레벨에서 한 번 더 방지.- 동시성 상황에서도 MySQL의 행 락 + 조건 평가로 인해 최대 한 번만 감소하게 되어, 로직과 잘 맞습니다.
- 단점/주의점
friendId파라미터 이름과 도메인 필드recipientId이름이 달라서 읽을 때 살짝 혼동될 수 있습니다.- 개선 제안
- 선택 1: 메서드 파라미터명을
recipientId로 맞추어 도메인 모델과 용어를 일치.- 선택 2: 혹은 주석으로 “friendId == recipientId”임을 한 줄 남겨 용어를 설명.
createIfAbsentnative 쿼리와 유니크 제약 전제 (Line 31-42)INSERT INTO friend_reminder_history(sender_id, recipient_id, quota_date, remaining) VALUES (:senderId, :friendId, :quotaDate, :initRemaining) ON DUPLICATE KEY UPDATE id = id
- 장점
INSERT ... ON DUPLICATE KEY UPDATE id = id패턴으로, 데이터 변경 없이 “이미 존재하면 그냥 통과”하는 업서트를 구현하셨습니다.- 동시 요청 10개가 동시에 들어와도 정확히 하나의 row만 생성된다는 점에서, 이번 PR 목적에 잘 맞는 선택입니다.
- 전제 조건 / 리스크
- 이 쿼리는
(sender_id, recipient_id, quota_date)조합에 유니크 제약(UNIQUE KEY)이 존재한다는 것을 강하게 전제합니다.- 만약 스키마에서 이 유니크 인덱스가 빠지거나 변경되면,
- 더 이상 “중복 시 no-op”이 아니라, 그냥 매번 insert가 되어 중복 row가 발생할 수 있고,
- 애플리케이션 레벨에서는 이를 감지할 방법이 거의 없습니다.
- 개선 제안
- 선택 1: 스키마 마이그레이션 (DDL) 쪽에
(sender_id, recipient_id, quota_date)UNIQUE INDEX가 명시되어 있는지 한 번 더 확인하고, 없다면 이번 PR에서 함께 추가.
- 장점: DB와 코드의 전제가 일치해 장기적으로 안전.
- 단점: 이미 운영 DB가 있다면 마이그레이션/배포 계획이 필요.
- 선택 2: 코드에 짧은 주석으로 “이 메서드는 (sender_id, recipient_id, quota_date) 유니크 인덱스를 전제로 동작함”을 남겨, 향후 스키마 변경 시 경각심을 줄 수 있습니다.
영속성 컨텍스트 동기화 관점 (선택적인 리팩터링)
- 현재
@Modifying에clearAutomatically = true를 사용하지 않고 있어서, 같은 트랜잭션 내에서 이미 로딩된FriendReminderHistory엔티티가 있다면 메모리 상의 값과 DB 값이 다를 수 있습니다.- 지금 플로우 상으로는 bulk update 전/후에
find를 섞어 쓰는 패턴은 안 보이지만, 향후 리팩터링 시 혼동을 줄이기 위해를 고려할 수 있습니다. (성능/캐시 히트 패턴과 트레이드오프가 있으니 필요 시에만)@Modifying(clearAutomatically = true)전반적으로 쿼리 자체는 의도에 잘 맞게 작성되어 있고, 위 전제(특히 유니크 인덱스)만 명확히 관리해주시면 동시성 측면에서 충분히 견고해 보입니다.
Also applies to: 31-42
backend/src/test/java/backend/mulkkam/friend/service/FriendReminderHistoryServiceIntegrationTest.java (1)
31-49: 동시성 통합 테스트 구조는 좋지만, 한도 상수·날짜 처리 의존성을 조금 더 느슨하게 만들면 더 안전합니다새 통합 테스트가 실제 DB + 스레드 풀을 이용해서 동시성 시나리오를 잘 검증하고 있어서, PR 목적(동시 요청 시 단일 row + 정확한 감소)을 잘 커버하고 있다고 보입니다. 여기에 몇 가지 개선 포인트를 더하면 테스트가 더 장기적으로 견고해질 것 같습니다.
CONCURRENT_REQUESTS = 10과remaining == 0이 비즈니스 상수에 강하게 결합 (Line 63, 111-113)
- 현재 테스트는
- 동시 요청 수
CONCURRENT_REQUESTS = 10- 최종
remaining == 0
로 검증하고 있어, 사실상INIT_REMAINING_VALUE가 10이라는 가정을 암묵적으로 포함합니다.- 향후 기획 변경으로 일일 리마인더 한도가 5나 20으로 바뀌면, 도메인 상수만 수정해도 이 테스트는 깨지게 됩니다.
개선 제안
- 도메인 상수를 직접 가져와서 테스트에도 동일하게 적용하는 방식이 더 안전합니다.
- 예시:
import static backend.mulkkam.friend.domain.FriendReminderHistory.INIT_REMAINING_VALUE; @Test void testCreateAndSendReminder_ConcurrentRequests() throws Exception { final int CONCURRENT_REQUESTS = INIT_REMAINING_VALUE; // 또는 INIT_REMAINING_VALUE / 2 등 명시적 // ... assertSoftly(softly -> { softly.assertThat(friendReminderHistoryRepository.findAll()).hasSize(1); softly.assertThat(resultHistory.getRemaining()).isEqualTo((short) (INIT_REMAINING_VALUE - CONCURRENT_REQUESTS)); }); }- 장점
- 한도 상수가 바뀌어도, “N번 요청하면 남은 횟수는 (초기값 - N)”이라는 비즈니스 규칙만 유지되면 테스트는 깨지지 않습니다.
- 테스트가 숫자 10에 묶이지 않고, 도메인 규칙에 묶이게 됩니다.
TODAY = LocalDate.now()필드와 서비스 내부 날짜 계산의 경계 조건 (Line 48, 107-108)
- 테스트 클래스 필드로
private final LocalDate TODAY = LocalDate.now();를 두고,
실제 저장 값 조회는quota_date = TODAY를 기준으로 하고 있습니다.- 서비스 내부 역시
LocalDate.now()로 오늘 날짜를 계산한다면,
- 대부분의 경우 문제는 없지만, 마찬가지로 테스트 인스턴스 생성 시점과 서비스 호출 시점이 자정을 넘길 경우 날짜가 달라질 수 있습니다.
- 해당 확률은 매우 낮지만, 장기적으로 보면 시간 관련 테스트의 대표적인 플래키 원인 중 하나입니다.
개선 제안 (선택지)
- (A) 이 통합 테스트에서도 도메인 상수와 함께
Clock을 사용하도록 확장 (테스트 전용 설정에서 고정 Clock 주입).- (B)
TODAY를 필드가 아닌 메서드 내부LocalDate today = LocalDate.now();로 두고,
테스트 시작 직전에 계산해서 서비스와 시간 차이를 최소화.- (C) 서비스에서 날짜를 인자로 받을 수 있게 확장한다면(유닛 테스트에서 논의한 방향과 동일), 여기서도 명시적으로 같은 값을 넘겨주는 방식.
현실적으로는 (B) 정도만으로도 자정 경계 이슈는 거의 사라지고, (A)는 장기적으로 시간/타임존 요구사항이 많아질 때 고려해볼 수 있겠습니다.
ExecutorService 자원 정리 (Line 65, 88-103)
- 현재도
future.get()호출 후executorService.shutdown()을 호출하고 있어, 실제로는 리소스 누수 가능성이 거의 없습니다.- 그래도 “테스트 중 예외가 발생해도 풀은 정리된다”는 보장을 더 명확히 하려면
try-finally패턴도 고려할 수 있습니다.- 예시:
ExecutorService executorService = Executors.newFixedThreadPool(CONCURRENT_REQUESTS); try { // submit, latch, future.get... } finally { executorService.shutdownNow(); }- 단, 지금 구조도 실질적인 문제는 없어 이 부분은 완전히 취향/팀 컨벤션 영역입니다.
전반적으로 이 통합 테스트는 이번 PR의 핵심(업서트 + 원자적 감소를 통한 동시성 해결)을 잘 커버하고 있고, 위와 같은 상수/날짜 의존성만 조금 정리해두면 이후 리밋 조정이나 시간 관련 정책 변경에도 더 유연하게 대응할 수 있을 것 같습니다.
Also applies to: 59-114
backend/src/main/java/backend/mulkkam/friend/service/command/FriendReminderHistoryCommandService.java (1)
21-30:reduceRemainingCount의 책임 범위와createIfAbsent호출 순서를 코드 레벨에서 더 안전하게 만들 수 있는 여지커맨드 서비스 레벨에서 조회/생성 로직을 걷어내고,
createIfAbsent(senderId, friendId, today)reduceRemainingCount(senderId, recipientId, date)
를 명시적으로 분리한 방향은 책임 분리 측면에서 좋습니다. 다만 몇 가지 설계 포인트를 한 번 더 생각해볼 만합니다.
updated == 0의 의미가 “row 없음”과 “remaining == 0”을 모두 포함하는 점 (Line 21-26)int updated = friendReminderHistoryRepository.tryReduceRemaining(senderId, recipientId, date); if (updated == 0) { throw new CommonException(EXCEED_FRIEND_REMINDER_LIMIT); }
- 현재 로직에서는
- (A) 해당
(senderId, recipientId, date)row가 아예 없거나- (B) row는 있지만
remaining == 0인 경우
모두EXCEED_FRIEND_REMINDER_LIMIT로 처리됩니다.- 이번 PR에서
createAndSendReminder가 항상 먼저createIfAbsent를 호출한다는 전제가 맞다면,
- 실질적으로 (A)는 발생하지 않고, (B)만
EXCEED_FRIEND_REMINDER_LIMIT가 되므로 문제는 없습니다.- 다만, 다른 서비스에서 이 메서드를 직접 호출할 수 있는 여지가 있다면
- “아직 한 번도 리마인더를 보낸 적이 없다(= row 없음)” 상태도 “한도를 초과했다”로 처리될 수 있습니다.
대안 제안 (선택지)
- 선택 1: 지금처럼 단순하게 유지하되, 주석으로 “항상
createIfAbsent이후에 호출된다는 전제”를 명시.- 선택 2: Repository 쿼리를 확장해 “row 없음”과 “remaining == 0”을 구분 리턴하고, 필요하다면 서로 다른 에러코드로 매핑.
- 장점: 의미 구분이 명확해짐.
- 단점: 현재 요구사항에서는 과한 복잡도일 수 있음.
createIfAbsent와reduceRemainingCount를 하나의 진입점으로 묶는 방안 (Line 28-30)
- 현재는 상위 서비스가
를 순서대로 호출하는 형태일 텐데, 이 순서가 깨지면(예: 실수로 reduce만 호출) 의미가 틀어질 수 있습니다.commandService.createIfAbsent(senderId, friendId, today); commandService.reduceRemainingCount(senderId, friendId, today);- 선택지:
- (A) 지금 구조 유지: 상위 서비스에서 순서를 보장하고, 단위/통합 테스트로 그 계약을 잡아둠.
- (B) 커맨드 서비스에 한 번에 처리하는 메서드를 노출:
@Transactional public void createIfAbsentAndReduce(Long senderId, Long friendId, LocalDate date) { friendReminderHistoryRepository.createIfAbsent(senderId, friendId, date, INIT_REMAINING_VALUE); int updated = friendReminderHistoryRepository.tryReduceRemaining(senderId, friendId, date); if (updated == 0) { throw new CommonException(EXCEED_FRIEND_REMINDER_LIMIT); } }
- 장점: 상위 서비스는 단일 메서드만 호출하면 되어, 순서/전제 조건이 캡슐화됩니다.
- 단점: 현재처럼 “생성만 미리 해두는” 케이스가 필요하다면 유연성이 줄어듭니다.
현재 요구사항이 “리마인더 전송 시점에만 생성+차감”이라면 (B) 쪽이 실수를 막는 데는 더 안전하고,
향후 다른 시나리오(예: 사전 초기화 배치 등)가 있을 수 있다면 지금 구조를 유지하면서 계약을 테스트로 강하게 검증하는 식이 합리적입니다.파라미터 네이밍 일관성 (
recipientIdvsfriendId)
- repository 쪽은
@Param("friendId") Long friendId를 받고, JPQL에서는h.recipientId를 사용하고 있습니다.- 서비스 쪽에서는 메서드 시그니처에
recipientId/friendId가 혼재하고 있어, 읽는 사람 입장에서 “friend == recipient” 매핑을 한 번 더 머리로 변환해야 합니다.- 가능하다면
- 도메인 필드 이름/파라미터 이름을
recipientId로 통일하거나,- 반대로 “friend” 라는 용어로 통일하는 등 하나로 맞추는 것이 가독성과 유지보수성에 좋습니다.
전체적으로는 이번 PR의 핵심 의도(조회+삽입 경쟁 제거, DB 원자 연산 사용)는 잘 반영되어 있고,
위와 같은 책임·계약에 대한 의도를 코드/주석/테스트로 조금만 더 명확히 해두면, 이후 기능 추가나 리팩터링 시 실수 여지를 더 줄일 수 있을 것 같습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backend/src/main/java/backend/mulkkam/friend/repository/FriendReminderHistoryRepository.java(1 hunks)backend/src/main/java/backend/mulkkam/friend/service/FriendReminderHistoryService.java(2 hunks)backend/src/main/java/backend/mulkkam/friend/service/command/FriendReminderHistoryCommandService.java(1 hunks)backend/src/main/java/backend/mulkkam/friend/service/query/FriendReminderHistoryQueryService.java(1 hunks)backend/src/test/java/backend/mulkkam/friend/service/FriendReminderHistoryServiceIntegrationTest.java(1 hunks)backend/src/test/java/backend/mulkkam/friend/service/FriendReminderHistoryServiceUnitTest.java(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/src/main/java/backend/mulkkam/friend/service/FriendReminderHistoryService.java
🧰 Additional context used
📓 Path-based instructions (1)
backend/**
⚙️ CodeRabbit configuration file
backend/**: - 1. 팀 및 공식 컨벤션, 가독성, 예외처리, 테스트/확장/유지보수성, 모듈화, API/DB/보안 설계 기준을 기반으로 리뷰해주세요.
- 최신 트렌드, 불필요한 로직, 클린코드, 리팩토링, 서비스/도메인 설계, 공통 예외 처리, 확장성도 함께 확인해주세요.
- 각 피드백은 문제점·대안·장단점을 짧고 논리적으로, 예시 코드가 있다면 간결히 포함해 주세요.
- 팀 내 스타일 통일성도 확인해주세요.
- 미작성한 테스트 코드 케이스가 있다면, 어떤 테스트가 필요한지 제안해주세요. (예: 컨트롤러는 인수 테스트, 나머지는 단위 테스트)
- 리뷰 남겨주는 부분은 해당 라인 범위의 코멘트에 작성해주세요.
Files:
backend/src/main/java/backend/mulkkam/friend/repository/FriendReminderHistoryRepository.javabackend/src/main/java/backend/mulkkam/friend/service/query/FriendReminderHistoryQueryService.javabackend/src/test/java/backend/mulkkam/friend/service/FriendReminderHistoryServiceIntegrationTest.javabackend/src/main/java/backend/mulkkam/friend/service/command/FriendReminderHistoryCommandService.javabackend/src/test/java/backend/mulkkam/friend/service/FriendReminderHistoryServiceUnitTest.java
🧬 Code graph analysis (1)
backend/src/test/java/backend/mulkkam/friend/service/FriendReminderHistoryServiceIntegrationTest.java (2)
backend/src/test/java/backend/mulkkam/support/fixture/member/MemberFixtureBuilder.java (1)
MemberFixtureBuilder(9-100)backend/src/test/java/backend/mulkkam/friend/service/FriendReminderHistoryServiceUnitTest.java (1)
DisplayName(70-142)
⏰ 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: test
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
backend/src/main/java/backend/mulkkam/friend/service/command/FriendReminderHistoryCommandService.java (2)
4-4: INIT_REMAINING_VALUE 직접 전달 +createIfAbsent업서트 동작을 한 번 더 점검해보면 좋겠습니다.
- 현재 구조(매 호출마다
createIfAbsent(..., INIT_REMAINING_VALUE)→tryReduceRemaining(...)) 자체는 동시성 관점에서 깔끔한 편인데, 업서트 SQL이 “존재할 때도remaining을INIT_REMAINING_VALUE로 재설정”하는 형태라면, 매 요청마다 쿼터가 리셋되어 한도 초과가 사실상 발생하지 않는 문제가 생길 수 있습니다.- 의도대로라면 “존재하지 않을 때만
INIT_REMAINING_VALUE초기화, 이미 존재하면remaining값은 그대로 유지”가 되어야 하므로, 실제INSERT ... ON DUPLICATE KEY UPDATE절이 어떻게 작성돼 있는지 한 번만 더 확인해보는 게 좋겠습니다.- 또 서비스가 도메인 상수(
INIT_REMAINING_VALUE)를 직접 알고 있는 구조라 기본값이 바뀔 때 변경 포인트가 퍼질 수 있습니다.
- 선택지 1: 지금처럼 상수를 외부에 공개해서 사용하는 방식(현재 구현, 변경 비용↓, 결합도↑).
- 선택지 2: 레포지토리 쪽에
createIfAbsent(senderId, recipientId, date)처럼 기본값을 내부에서 캡슐화한 오버로드를 추가(도메인 규칙 캡슐화↑, 초기 구현 약간↑).- 선택지 3: FriendReminderHistory 쪽에 “기본 쿼터”를 만드는 팩토리/정적 메서드를 두고 서비스는 그 메서드만 호출(DDD 관점에서 책임 분배가 더 자연스러움).
Also applies to: 21-23
21-25:updated변수 의미가 모호해 가독성이 떨어집니다. 이름만 정리해도 의도가 훨씬 분명해집니다.
- 현재는
int updated = friendReminderHistoryRepository.tryReduceRemaining(...)라고 되어 있어서, 이 값이 “업데이트된 행 수”인지 “감소 후 남은 횟수”인지 코드만 보고는 바로 파악하기 어렵습니다. 한 줄짜리 로직인데도 의도가 모호하면 리뷰/유지보수 비용이 올라갑니다.- 레포지토리 시그니처가 그대로라면, 최소한 이쪽에서는 의미가 드러나는 이름으로 바꿔주면 좋겠습니다. 예를 들면:
- int updated = friendReminderHistoryRepository.tryReduceRemaining(senderId, recipientId, date); - if (updated == 0) { + int updatedRowCount = friendReminderHistoryRepository.tryReduceRemaining(senderId, recipientId, date); + if (updatedRowCount == 0) { throw new CommonException(EXCEED_FRIEND_REMINDER_LIMIT); }
- 더 나아가 레포지토리에서 bool을 반환하도록 (
true면 차감 성공,false면 한도 초과) 바꾸면 서비스 레이어에서도 비즈니스 의도가 훨씬 자연스럽게 드러납니다(다만 그 경우 인터페이스 변경 범위가 커지니 후속 리팩토링 타이밍에 고려해볼 만한 정도입니다).backend/src/main/java/backend/mulkkam/friend/service/FriendReminderHistoryService.java (2)
40-42: 자기 자신에게 보내는 리마인더를 초기에 차단하는 구조는 명확하고 좋은 선택입니다.
senderId.equals(friendId)체크를 가장 앞단에서 처리해서, 친구 관계 검증과 섞이지 않고 “자기 자신 리마인드 금지”라는 비즈니스 규칙이 잘 드러납니다. 에러 코드도 전용NOT_ALLOWED_SELF_REMINDER를 쓰고 있어 클라이언트 입장에서도 해석이 명확합니다.- 다만 이 케이스는 클라이언트/서버 간 계약 측면에서 꽤 중요한 규칙이라, 서비스 단위 테스트나 (실제 API 레벨이라면) 인수 테스트에서 “자기 자신에게 보내면 반드시 해당 에러 코드가 내려온다”는 시나리오를 한 케이스로 고정해두면 회귀를 막는 데 도움이 될 것 같습니다.
45-47: 쿼터 차감 책임을 CommandService로 위임한 구조는 좋습니다. 경계/동시성 테스트 케이스만 조금 더 보강하면 좋겠습니다.
- 이전처럼 히스토리를 직접 조회해서 조작하는 대신,
consumeRemainingCount(senderId, friendId, today)로 캡슐화한 덕분에 이 서비스는 “친구 관계 검증 + 알림 발송”에만 집중하고, 쿼터/동시성 처리는 하위 계층에서 전담하게 되어 역할 분리가 잘 되었습니다.- 다만 한도 관련해서는 다음 시나리오들이 테스트로 명시돼 있으면 더 안심할 수 있을 것 같습니다.
- 이미 해당 (senderId, friendId, today) 행이 존재하는 상태에서 다시 호출했을 때,
createIfAbsent가 기존 남은 횟수를 망가뜨리지 않고 올바르게 차감되는지.- 마지막 허용 횟수에서 호출했을 때는 정상적으로 알림이 발송되고, 그 다음 호출부터는
EXCEED_FRIEND_REMINDER_LIMIT가 발생하는지(오프바이원 여부 확인).- 여러 스레드/요청이 동시에
createAndSendReminder를 호출할 때, 실제로 허용된 횟수 이상으로 알림이 발송되지 않는지(이 경우는 통합 테스트 + DB 기반으로 커버하는 쪽이 자연스러워 보입니다).- 이미 일부 통합/단위 테스트가 있으시겠지만, 위 세 가지 정도를 명시적으로 넣어두면 나중에 레포지토리 구현이 바뀌어도 의도한 동작을 안전하게 유지할 수 있습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/src/main/java/backend/mulkkam/friend/service/FriendReminderHistoryService.java(2 hunks)backend/src/main/java/backend/mulkkam/friend/service/command/FriendReminderHistoryCommandService.java(1 hunks)backend/src/test/java/backend/mulkkam/friend/service/FriendReminderHistoryServiceUnitTest.java(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/src/test/java/backend/mulkkam/friend/service/FriendReminderHistoryServiceUnitTest.java
🧰 Additional context used
📓 Path-based instructions (1)
backend/**
⚙️ CodeRabbit configuration file
backend/**: - 1. 팀 및 공식 컨벤션, 가독성, 예외처리, 테스트/확장/유지보수성, 모듈화, API/DB/보안 설계 기준을 기반으로 리뷰해주세요.
- 최신 트렌드, 불필요한 로직, 클린코드, 리팩토링, 서비스/도메인 설계, 공통 예외 처리, 확장성도 함께 확인해주세요.
- 각 피드백은 문제점·대안·장단점을 짧고 논리적으로, 예시 코드가 있다면 간결히 포함해 주세요.
- 팀 내 스타일 통일성도 확인해주세요.
- 미작성한 테스트 코드 케이스가 있다면, 어떤 테스트가 필요한지 제안해주세요. (예: 컨트롤러는 인수 테스트, 나머지는 단위 테스트)
- 리뷰 남겨주는 부분은 해당 라인 범위의 코멘트에 작성해주세요.
Files:
backend/src/main/java/backend/mulkkam/friend/service/command/FriendReminderHistoryCommandService.javabackend/src/main/java/backend/mulkkam/friend/service/FriendReminderHistoryService.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: test
1 participant
🔗 관련 이슈
close #1027
📝 작업 내용
기존 방식의 문제점
기존에
try-catch로 중복 데이터 생성을 막았던 로직은 실제 동시성 문제를 해결해주지 못함해결 방안
파일 변경 사항을 확인해보면
INSERT ... ON DUPLICATE KEY를 통한 조회+삽입을 DB에서의 원자적 연산으로 해결한 것을 볼 수 있음참고 자료
문제 해결 과정 문서
주요 변경사항
📸 스크린샷 (Optional)
Summary by CodeRabbit
버그 수정
테스트
문서