[FEAT] outbox 패턴 적용#89
Merged
Merged
Conversation
hhjo96
added
documentation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
Walkthrough이 PR은 분산 작업 스케줄링을 위한 ShedLock과 신뢰할 수 있는 이벤트 발행을 위한 아웃박스 패턴을 구현합니다. 경매 생성 시 아웃박스 레코드를 저장하고, 백그라운드 워커가 실패한 항목을 재시도하며, 관리자 API를 통해 실패한 항목을 조회하고 수동으로 재시도할 수 있습니다. ChangesOutbox Pattern Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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: 8
🧹 Nitpick comments (5)
src/main/java/com/example/auction/domain/auction/eventBridge/service/AuctionEventBridgeService.java (1)
93-106: ⚡ Quick win현재 동작과 맞지 않는 주석/주석처리 코드를 정리해주세요
실제는 신규 스케줄만 등록하는데, “신규와 레거시 둘다 등록” 주석과 레거시 호출 주석 코드가 남아 있어 동작 해석을 혼동시킵니다.
🧹 제안 수정안
- // 신규와 레거시 둘다 등록 + // 신규 스케줄만 등록 registerSchedule(auctionId, startedAt.minusMinutes(5), startedAt, "START", lambdaArn, newArnSuffix); - //registerSchedule(auctionId, startedAt, startedAt,"START", legacyLambdaArn, legacyArnSuffix); @@ registerSchedule(auctionId, endedAt.minusMinutes(5), endedAt, "END", lambdaArn, newArnSuffix); - //registerSchedule(auctionId, endedAt, endedAt, "END", legacyLambdaArn, legacyArnSuffix);🤖 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/example/auction/domain/auction/eventBridge/service/AuctionEventBridgeService.java` around lines 93 - 106, Remove the misleading comment and commented-out legacy registerSchedule calls so the code reflects that only the new schedules are registered: delete the line "// 신규와 레거시 둘다 등록" and the two commented calls that reference legacyLambdaArn and legacyArnSuffix, and replace with a concise comment (or no comment) indicating only the new schedule is registered; update the registerStartSchedule/registerEndSchedule usages that call registerSchedule(auctionId, ..., "START"/"END", lambdaArn, newArnSuffix) to ensure no leftover references to legacyLambdaArn or legacyArnSuffix remain.src/main/java/com/example/auction/domain/auction/eventBridge/service/OutboxAdminService.java (2)
19-23: ⚡ Quick win상태 문자열을 상수로 관리하세요.
"FAILED"문자열이 하드코딩되어 있습니다. 다른 파일(예:AuctionOutboxWorker.java)에서도 동일한 상태 문자열을 사용하므로, 상수로 정의하여 일관성을 유지하고 오타 가능성을 줄이는 것이 좋습니다.♻️ 제안: 상수 정의 예시
별도의 상수 클래스 또는 엔티티에 정의:
public class OutboxStatus { public static final String PENDING = "PENDING"; public static final String FAILED = "FAILED"; public static final String PUBLISHED = "PUBLISHED"; }사용 예시:
public List<OutboxAdminResponse> getFailedOutbox() { - return outboxRepository.findByStatus("FAILED").stream() + return outboxRepository.findByStatus(OutboxStatus.FAILED).stream() .map(OutboxAdminResponse::from) .toList(); }🤖 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/example/auction/domain/auction/eventBridge/service/OutboxAdminService.java` around lines 19 - 23, getFailedOutbox in OutboxAdminService currently uses the hardcoded status string "FAILED"; define a shared constant (e.g., add OutboxStatus.FAILED to a new OutboxStatus class or the existing Outbox entity) and replace the literal in OutboxAdminService.getFailedOutbox with that constant; also update any other usages such as in AuctionOutboxWorker to use the same OutboxStatus.FAILED constant to ensure consistency and avoid typos.
26-29: ⚡ Quick win엔티티 조회 실패 시 예외 타입을 재고하세요.
IllegalArgumentException은 주로 메서드 인자의 유효성 검증 실패에 사용됩니다. 엔티티를 찾을 수 없는 경우에는EntityNotFoundException또는 커스텀 도메인 예외(예:OutboxNotFoundException)를 사용하는 것이 의미론적으로 더 명확합니다.현재 컨트롤러에서 이 예외를 400 Bad Request로 처리하고 있지만, 404 Not Found가 더 적절한 HTTP 상태 코드입니다.
🤖 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/example/auction/domain/auction/eventBridge/service/OutboxAdminService.java` around lines 26 - 29, Replace the inappropriate IllegalArgumentException in OutboxAdminService.retry with a semantically correct exception: either throw javax.persistence.EntityNotFoundException or create a custom runtime exception like OutboxNotFoundException (e.g., extends RuntimeException) and use it in the .orElseThrow call that wraps outboxRepository.findById(outboxId); update the import and constructor call (e.g., .orElseThrow(() -> new OutboxNotFoundException(outboxId))) and adjust any controller/exception-handler mappings if needed so the missing entity maps to HTTP 404.src/main/java/com/example/auction/domain/auction/eventBridge/service/AuctionOutboxWorker.java (2)
30-31: ⚡ Quick win상태 문자열을 상수로 관리하세요.
"PENDING","FAILED"문자열이 하드코딩되어 있습니다. Line 35의"START"도 마찬가지입니다.OutboxAdminService에서도 동일한 문제가 있으므로, 공통 상수 클래스를 만들어 관리하는 것이 좋습니다.🤖 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/example/auction/domain/auction/eventBridge/service/AuctionOutboxWorker.java` around lines 30 - 31, 하드코딩된 상태 문자열 ("PENDING", "FAILED", "START")를 공통 상수로 분리하세요: 새 상수 클래스(예: OutboxStatus) 를 만들고 PENDING, FAILED, START 같은 상태들을 public static final String 상수로 정의한 뒤 AuctionOutboxWorker의 findByStatusInAndAttemptCountLessThan 호출과 OutboxAdminService의 모든 상태 사용처를 해당 상수로 교체하세요; 참조 대상 식별자: AuctionOutboxWorker, OutboxAdminService, AuctionScheduleOutbox, findByStatusInAndAttemptCountLessThan.
29-54: ⚖️ Poor tradeoff대량 데이터 처리 시 배치 처리를 고려하세요.
현재 구현은 조건에 맞는 모든 outbox 항목을 메모리에 로드합니다. Outbox 테이블이 커지면 성능 문제가 발생할 수 있습니다.
예를 들어, FAILED 상태인 항목이 수천 개 쌓이면 30초마다 모두 로드하여 처리하게 됩니다. 페이징 또는 배치 크기 제한을 고려하세요.
예시:
// 한 번에 최대 100개씩 처리 List<AuctionScheduleOutbox> targets = outboxRepository .findTop100ByStatusInAndAttemptCountLessThan( List.of("PENDING", "FAILED"), MAX_ATTEMPTS);🤖 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/example/auction/domain/auction/eventBridge/service/AuctionOutboxWorker.java` around lines 29 - 54, The process() method in AuctionOutboxWorker currently loads all matching outbox rows into memory via outboxRepository.findByStatusInAndAttemptCountLessThan which will OOM or perform poorly at scale; change the implementation to iterate in bounded batches (e.g., use repository methods with limit like findTop100ByStatusInAndAttemptCountLessThan or use a Pageable/Slice query) and loop until no more results, processing and saving each batch before fetching the next one (retain existing logic in process() for calling eventBridgeService.registerStartSchedule/registerEndSchedule, outbox.markPublished(), outbox.incrementAttempt(), and outbox.markFailed()). Ensure the repository method name used (or a Pageable parameter) replaces the current findByStatusInAndAttemptCountLessThan call so only a fixed batch size is loaded each iteration.
🤖 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/docs/asciidoc/index.adoc`:
- Line 23: The include target in index.adoc uses include::outbox-admin.adoc[]
but the PR added a file named outbox-dmin.adoc, causing Asciidoctor to fail; fix
by either renaming the added file from outbox-dmin.adoc to outbox-admin.adoc to
match the existing include in index.adoc, or update the include line in
index.adoc to include::outbox-dmin.adoc[] so the filenames match.
In
`@src/main/java/com/example/auction/domain/auction/eventBridge/controller/OutboxAdminController.java`:
- Around line 27-33: The response message in OutboxAdminController.retry
currently returns "재시도 요청을 성공했습니다" which doesn't match the test expectation "재시도
요청 성공"; update the message string in the retry method's ResponseEntity (inside
BaseResponse.success call) to exactly "재시도 요청 성공" so the controller response
matches the test; ensure the change is made in the retry(`@PathVariable` Long
outboxId) method of the OutboxAdminController class.
- Around line 20-25: The response message in
OutboxAdminController.getFailedOutbox currently uses "FAILED outbox 조회에 성공했습니다"
which doesn't match the test expectation; update the BaseResponse.success call
in the getFailedOutbox method to use the exact expected message "FAILED outbox
조회 성공" so the returned ResponseEntity (from
OutboxAdminController.getFailedOutbox) matches the test assertion.
In
`@src/main/java/com/example/auction/domain/auction/eventBridge/entity/AuctionScheduleOutbox.java`:
- Around line 39-46: 팩토리 메서드 AuctionScheduleOutbox.of에서 입력값을 선검증하도록 수정하세요:
auctionId가 null이거나 <=0 인 경우, eventType이 null이거나 빈 문자열인 경우, scheduledAt이 null(또는
필요하면 과거 시간인 경우)에는 IllegalArgumentException을 던져 즉시 실패하도록 구현하고 기존 필드(status,
attemptCount) 설정은 유지하세요.
In
`@src/main/java/com/example/auction/domain/auction/eventBridge/repository/AuctionScheduleOutboxRepository.java`:
- Around line 20-21: The JPQL in markPublished (method markPublished in
AuctionScheduleOutboxRepository) only updates rows with o.status = 'PENDING', so
outboxes that were previously 'FAILED' and then succeed aren't flipped to
'PUBLISHED'; change the WHERE clause to include FAILED as well (e.g., allow
o.status IN ('PENDING','FAILED')) so successful retries are marked PUBLISHED for
the given auctionId and eventType, and keep the rest of the method signature
unchanged.
In `@src/main/resources/db/migration/V6__add_unique_constraints.sql`:
- Around line 2-6: The migration adds UNIQUE constraints uq_bids_auction_price
(bids.auction_id, price) and uq_auction_results_auction_id
(auction_results.auction_id) which will fail if duplicates exist; before adding
each constraint, remove or consolidate duplicates and/or add a validation check
in the same migration: for bids use a dedupe step that keeps a single row per
(auction_id, price) (e.g., retain MIN(id) or another canonical row) and delete
the rest, and for auction_results keep one row per auction_id similarly; after
deduping, optionally run a SELECT to assert zero duplicates for those key
combinations and only then execute ALTER TABLE ... ADD CONSTRAINT for
uq_bids_auction_price and uq_auction_results_auction_id.
In `@src/main/resources/db/migration/V7__create_outbox_table.sql`:
- Around line 16-17: Add a UNIQUE constraint/index to prevent duplicate outbox
events by creating a unique index on auction_schedule_outbox for the
(auction_id, event_type) column pair (next to the existing idx_outbox_status and
idx_outbox_auction_id). Reference the auction_schedule_outbox table and the
auction_id and event_type columns and give the index a clear name (e.g.,
idx_outbox_auction_event_unique) so duplicate (auction_id, event_type) inserts
will be rejected.
In `@src/main/resources/db/seed/R__add_seed_categories.sql`:
- Line 40: The seed SQL line contains a plaintext password hint appended as a
SQL comment ("--1234567890") after the bcrypt hash
('$2b$10$AJg6EA0ZI8UIXOXmnFnBu.ABlb88prNJhKXeZCg8C2paX4ks6p0vS'); remove the
plaintext comment so only the hash remains and scan the same file
(R__add_seed_categories.sql) for any other inline plaintext credential comments
and remove them as well.
---
Nitpick comments:
In
`@src/main/java/com/example/auction/domain/auction/eventBridge/service/AuctionEventBridgeService.java`:
- Around line 93-106: Remove the misleading comment and commented-out legacy
registerSchedule calls so the code reflects that only the new schedules are
registered: delete the line "// 신규와 레거시 둘다 등록" and the two commented calls that
reference legacyLambdaArn and legacyArnSuffix, and replace with a concise
comment (or no comment) indicating only the new schedule is registered; update
the registerStartSchedule/registerEndSchedule usages that call
registerSchedule(auctionId, ..., "START"/"END", lambdaArn, newArnSuffix) to
ensure no leftover references to legacyLambdaArn or legacyArnSuffix remain.
In
`@src/main/java/com/example/auction/domain/auction/eventBridge/service/AuctionOutboxWorker.java`:
- Around line 30-31: 하드코딩된 상태 문자열 ("PENDING", "FAILED", "START")를 공통 상수로 분리하세요:
새 상수 클래스(예: OutboxStatus) 를 만들고 PENDING, FAILED, START 같은 상태들을 public static
final String 상수로 정의한 뒤 AuctionOutboxWorker의
findByStatusInAndAttemptCountLessThan 호출과 OutboxAdminService의 모든 상태 사용처를 해당 상수로
교체하세요; 참조 대상 식별자: AuctionOutboxWorker, OutboxAdminService,
AuctionScheduleOutbox, findByStatusInAndAttemptCountLessThan.
- Around line 29-54: The process() method in AuctionOutboxWorker currently loads
all matching outbox rows into memory via
outboxRepository.findByStatusInAndAttemptCountLessThan which will OOM or perform
poorly at scale; change the implementation to iterate in bounded batches (e.g.,
use repository methods with limit like
findTop100ByStatusInAndAttemptCountLessThan or use a Pageable/Slice query) and
loop until no more results, processing and saving each batch before fetching the
next one (retain existing logic in process() for calling
eventBridgeService.registerStartSchedule/registerEndSchedule,
outbox.markPublished(), outbox.incrementAttempt(), and outbox.markFailed()).
Ensure the repository method name used (or a Pageable parameter) replaces the
current findByStatusInAndAttemptCountLessThan call so only a fixed batch size is
loaded each iteration.
In
`@src/main/java/com/example/auction/domain/auction/eventBridge/service/OutboxAdminService.java`:
- Around line 19-23: getFailedOutbox in OutboxAdminService currently uses the
hardcoded status string "FAILED"; define a shared constant (e.g., add
OutboxStatus.FAILED to a new OutboxStatus class or the existing Outbox entity)
and replace the literal in OutboxAdminService.getFailedOutbox with that
constant; also update any other usages such as in AuctionOutboxWorker to use the
same OutboxStatus.FAILED constant to ensure consistency and avoid typos.
- Around line 26-29: Replace the inappropriate IllegalArgumentException in
OutboxAdminService.retry with a semantically correct exception: either throw
javax.persistence.EntityNotFoundException or create a custom runtime exception
like OutboxNotFoundException (e.g., extends RuntimeException) and use it in the
.orElseThrow call that wraps outboxRepository.findById(outboxId); update the
import and constructor call (e.g., .orElseThrow(() -> new
OutboxNotFoundException(outboxId))) and adjust any controller/exception-handler
mappings if needed so the missing entity maps to HTTP 404.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d55a86e2-1788-4107-bce1-6e7d667e9379
📒 Files selected for processing (21)
build.gradledocker-compose.ymlsrc/docs/asciidoc/index.adocsrc/docs/asciidoc/outbox-dmin.adocsrc/main/java/com/example/auction/AuctionApplication.javasrc/main/java/com/example/auction/common/config/ShedLockConfig.javasrc/main/java/com/example/auction/domain/auction/eventBridge/controller/OutboxAdminController.javasrc/main/java/com/example/auction/domain/auction/eventBridge/dto/OutboxAdminResponse.javasrc/main/java/com/example/auction/domain/auction/eventBridge/entity/AuctionCancelledEventBridge.javasrc/main/java/com/example/auction/domain/auction/eventBridge/entity/AuctionCreatedEventBridge.javasrc/main/java/com/example/auction/domain/auction/eventBridge/entity/AuctionScheduleOutbox.javasrc/main/java/com/example/auction/domain/auction/eventBridge/repository/AuctionScheduleOutboxRepository.javasrc/main/java/com/example/auction/domain/auction/eventBridge/service/AuctionEventBridgeService.javasrc/main/java/com/example/auction/domain/auction/eventBridge/service/AuctionOutboxWorker.javasrc/main/java/com/example/auction/domain/auction/eventBridge/service/OutboxAdminService.javasrc/main/java/com/example/auction/domain/auction/service/AuctionService.javasrc/main/resources/db/migration/V6__add_unique_constraints.sqlsrc/main/resources/db/migration/V7__create_outbox_table.sqlsrc/main/resources/db/seed/R__add_seed_categories.sqlsrc/test/java/com/example/auction/domain/ai/service/RagComparisonDemoTest.javasrc/test/java/com/example/auction/domain/auction/controller/OutboxAdminControllerTest.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
3 participants
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.
📝 작업 내용
🔗 관련 이슈 (Related Issues)
Closes #
✅ 체크리스트 (Checklist)
💬 기타 사항
Summary by CodeRabbit
릴리스 노트
새로운 기능
문서
데이터베이스