Skip to content

refactor(#235): 경매 활성화/만료/카드동기화 배치를 메인앱 Internal API 위임으로 전환 - #12

Merged
cdkkyj123 merged 4 commits into
mainfrom
feat/auction-cardsync-internal-api/#235
Jun 16, 2026
Merged

refactor(#235): 경매 활성화/만료/카드동기화 배치를 메인앱 Internal API 위임으로 전환#12
cdkkyj123 merged 4 commits into
mainfrom
feat/auction-cardsync-internal-api/#235

Conversation

@cdkkyj123

@cdkkyj123 cdkkyj123 commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • MainAuctionLifecycleClient 신규: activate/closeExpired — POST /internal/auctions/{id}/activate|close-expired, ApiResponseEnvelope<Boolean> 언래핑, 3회 재시도(1s→2s→4s), X-Internal-Token + Idempotency-Key 헤더
  • MainCardSyncClient 신규: triggerSync — POST /internal/cards/sync, 202 정상 / 409(CARD_SYNC_IN_PROGRESS) 스킵 / 401 → step 실패
  • AuctionActivationTasklet/AuctionExpirationTasklet thin화: 자체 Redisson 락 + AuctionBatchService 제거, 건별 client 호출 + 집계
  • CardSyncTasklet thin화: TCGdex/CardRepository/OutboxEventWriter/RestTemplate 의존 전부 제거, triggerSync 1회 호출
  • AuctionBatchService 삭제 (경매 활성화기간 now.plusHours(7) CRITICAL 버그 포함), OutboxEventWriter 조건부 삭제 (참조 0건 확인), Auction.activate()/end() 死코드 삭제
  • 메인앱 POCAT PR#235와 병행 배포 필요 (배치 호출 전 메인앱 internal API 먼저 배포 권장)

Test plan

  • MainAuctionLifecycleClientTest (MockRestServiceServer) GREEN 확인
  • MainCardSyncClientTest (MockRestServiceServer) GREEN 확인
  • AuctionActivationTaskletTest / AuctionExpirationTaskletTest / CardSyncTaskletTest GREEN 확인
  • AuctionActivationJobConfigTest / AuctionExpirationJobConfigTest / CardSyncJobConfigTest 회귀 없음 확인
  • 스테이징 환경에서 auctionActivationJob / auctionExpirationJob / cardSyncJob 실제 실행 후 메인앱 로그에서 /internal/** 호출 확인
  • POCAT PR#235 함께 리뷰 권장

Summary by CodeRabbit

Release Notes

  • New Features

    • Implemented internal API delegation for auction lifecycle operations (activation and expiration) with retry and idempotency safeguards.
    • Restructured card synchronization to use delegated internal API with improved error handling.
  • Documentation

    • Added comprehensive documentation for scheduled batch job operations and monitoring points.
    • Provided deployment prerequisites and validation checklists for system configuration.
  • Refactor

    • Modernized auction and card processing architecture for improved reliability and maintainability.

@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@cdkkyj123, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 49 minutes and 24 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 447878c9-9f0f-4f20-bcc7-68eb578aa5c4

📥 Commits

Reviewing files that changed from the base of the PR and between 2997994 and 0412253.

📒 Files selected for processing (7)
  • docs/ARCHITECTURE.md
  • src/main/java/com/rocketcrew/pocatbatch/client/MainCardSyncClient.java
  • src/main/java/com/rocketcrew/pocatbatch/job/auctionactivation/AuctionActivationTasklet.java
  • src/main/java/com/rocketcrew/pocatbatch/job/auctionexpiration/AuctionExpirationTasklet.java
  • src/test/java/com/rocketcrew/pocatbatch/client/MainCardSyncClientTest.java
  • src/test/java/com/rocketcrew/pocatbatch/job/auctionactivation/AuctionActivationTaskletTest.java
  • src/test/java/com/rocketcrew/pocatbatch/job/auctionexpiration/AuctionExpirationTaskletTest.java
📝 Walkthrough

Walkthrough

Three batch tasklets (AuctionActivationTasklet, AuctionExpirationTasklet, CardSyncTasklet) are refactored to delegate to new HTTP clients (MainAuctionLifecycleClient, MainCardSyncClient) instead of performing in-process DB/outbox/Redisson operations. The AuctionBatchService, OutboxEventWriter, and Auction mutation methods are deleted. Tests and documentation are updated throughout.

Changes

Internal API Delegation Refactor

Layer / File(s) Summary
Remove in-batch domain logic
src/main/java/.../domain/auction/entity/Auction.java, src/main/java/.../domain/auction/service/AuctionBatchService.java, src/main/java/.../domain/outbox/service/OutboxEventWriter.java, src/main/java/.../job/auctionactivation/AuctionActivationTasklet.java, src/main/java/.../job/auctionexpiration/AuctionExpirationTasklet.java, src/main/java/.../job/cardsync/CardSyncTasklet.java
Deletes AuctionBatchService (transactional activate/end with outbox writes), OutboxEventWriter, and the Auction entity's activate()/end() methods; removes AuctionBatchService and RedissonClient injections from activation/expiration tasklets and strips the full TCGdex sync pipeline from CardSyncTasklet.
New internal API clients
src/main/java/.../client/MainAuctionLifecycleClient.java, src/main/java/.../client/MainCardSyncClient.java, src/test/java/.../client/MainAuctionLifecycleClientTest.java, src/test/java/.../client/MainCardSyncClientTest.java
Adds MainAuctionLifecycleClient (activate, closeExpired with idempotency keys and exponential-backoff retry on 5xx, immediate throw on 4xx) and MainCardSyncClient (triggerSync with throw-on-401, skip-on-409-in-progress, warn+skip on other 4xx); MockRestServiceServer-based tests verify headers, endpoints, retry count, and per-status-code exception behavior.
Tasklet delegation rewiring and tests
src/main/java/.../job/auctionactivation/AuctionActivationTasklet.java, src/main/java/.../job/auctionexpiration/AuctionExpirationTasklet.java, src/main/java/.../job/cardsync/CardSyncTasklet.java, src/test/java/.../job/auctionactivation/AuctionActivationTaskletTest.java, src/test/java/.../job/auctionexpiration/AuctionExpirationTaskletTest.java, src/test/java/.../job/cardsync/CardSyncTaskletTest.java
Rewires all three tasklets to call the new clients; activation/expiration tasklets add three-count (activated/skipped/failed) result tracking; unit tests cover empty-list early-exit, mixed boolean/exception outcomes, and all-success scenarios.
Architecture and runbook documentation
docs/ARCHITECTURE.md, docs/RUNBOOK.md
Updates the package tree with new client/ and job/ submodules; adds an internal API delegation architecture section (endpoints, retry, idempotency, X-Internal-Token auth, deployment ordering); extends the runbook with per-job schedule/ShedLock/monitoring entries and a post-deploy checklist for POCAT_INTERNAL_TOKEN and POCAT_API_BASE_URL.

Sequence Diagram

sequenceDiagram
    participant BatchScheduler
    participant AuctionActivationTasklet
    participant MainAuctionLifecycleClient
    participant MainApp as Main App Internal API

    BatchScheduler->>AuctionActivationTasklet: execute(StepContribution, ChunkContext)
    AuctionActivationTasklet->>AuctionActivationTasklet: fetch APPROVED auctions from AuctionRepository
    loop per auctionId
        AuctionActivationTasklet->>MainAuctionLifecycleClient: activate(auctionId, jobExecutionId)
        MainAuctionLifecycleClient->>MainApp: POST /internal/auctions/{id}/activate\nX-Internal-Token, Idempotency-Key
        alt 200 success=true, data=true
            MainApp-->>MainAuctionLifecycleClient: ApiResponseEnvelope{success:true, data:true}
            MainAuctionLifecycleClient-->>AuctionActivationTasklet: true → activatedCount++
        else 200 success=true, data=false
            MainApp-->>MainAuctionLifecycleClient: ApiResponseEnvelope{success:true, data:false}
            MainAuctionLifecycleClient-->>AuctionActivationTasklet: false → skippedCount++
        else 4xx
            MainApp-->>MainAuctionLifecycleClient: HttpClientErrorException
            MainAuctionLifecycleClient-->>AuctionActivationTasklet: RuntimeException → failedCount++
        else 5xx (retry ≤3 with backoff)
            MainApp-->>MainAuctionLifecycleClient: server error
            MainAuctionLifecycleClient-->>AuctionActivationTasklet: RuntimeException after retries → failedCount++
        end
    end
    AuctionActivationTasklet-->>BatchScheduler: RepeatStatus.FINISHED
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • POCAT-sparta/pocat-batch#3: Introduced the original batch scheduler migration that established the AuctionActivationTasklet, AuctionExpirationTasklet, and CardSyncTasklet implementations using AuctionBatchService and Redisson locks — the exact code this PR replaces with internal API delegation.

Poem

🐇 Hoppity-hop, no more direct DB calls,
The batch job now knocks on the main app's halls,
With idempotency keys and a token in hand,
Retry on 5xx — just as we planned!
activate, closeExpired, triggerSync with cheer,
The rabbit delegates, and the code is now clear! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.82% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main refactoring: delegating auction activation/expiration and card sync batch operations to the main application's Internal API instead of in-batch processing.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/auction-cardsync-internal-api/#235

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cdkkyj123 cdkkyj123 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai review

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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 `@docs/ARCHITECTURE.md`:
- Around line 280-282: The fenced code block at line 280 in the ARCHITECTURE.md
file is missing a language identifier, which violates the markdownlint MD040
rule. Add the language identifier "text" to the opening fence (change the three
backticks to backticks followed by "text") to properly label the code block and
resolve the linting error.

In `@src/main/java/com/rocketcrew/pocatbatch/client/MainCardSyncClient.java`:
- Around line 52-60: The code currently silently skips all 4xx errors after
handling the specific 409 CARD_SYNC_IN_PROGRESS case, which masks real contract
or deployment issues like 404 or 400 errors. After the if block that checks for
HttpStatus.CONFLICT with CARD_SYNC_IN_PROGRESS (which should continue to skip by
returning), change the handling for all other 4xx errors: instead of logging a
warning and returning, throw an exception to fail fast and prevent the step from
appearing successful when there are actual client errors that need attention.

In
`@src/main/java/com/rocketcrew/pocatbatch/job/auctionactivation/AuctionActivationTasklet.java`:
- Around line 48-56: Both AuctionActivationTasklet and AuctionExpirationTasklet
are masking failures by always returning RepeatStatus.FINISHED even when
exceptions occur during processing. In AuctionActivationTasklet (lines 48-56),
after the loop completes and the aggregated log.info statement executes, add a
check that throws an exception if failedCount is greater than 0 instead of
unconditionally returning RepeatStatus.FINISHED. Apply the identical post-loop
failure escalation fix in AuctionExpirationTasklet (lines 52-60), ensuring that
any accumulated failedCount > 0 causes the step to fail rather than complete
successfully. This preserves the per-item exception aggregation and logging
while properly escalating systemic failures.
🪄 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: 4493a56b-784d-49ee-a708-2df99189b793

📥 Commits

Reviewing files that changed from the base of the PR and between 2bc9893 and 2997994.

📒 Files selected for processing (15)
  • docs/ARCHITECTURE.md
  • docs/RUNBOOK.md
  • src/main/java/com/rocketcrew/pocatbatch/client/MainAuctionLifecycleClient.java
  • src/main/java/com/rocketcrew/pocatbatch/client/MainCardSyncClient.java
  • src/main/java/com/rocketcrew/pocatbatch/domain/auction/entity/Auction.java
  • src/main/java/com/rocketcrew/pocatbatch/domain/auction/service/AuctionBatchService.java
  • src/main/java/com/rocketcrew/pocatbatch/domain/outbox/service/OutboxEventWriter.java
  • src/main/java/com/rocketcrew/pocatbatch/job/auctionactivation/AuctionActivationTasklet.java
  • src/main/java/com/rocketcrew/pocatbatch/job/auctionexpiration/AuctionExpirationTasklet.java
  • src/main/java/com/rocketcrew/pocatbatch/job/cardsync/CardSyncTasklet.java
  • src/test/java/com/rocketcrew/pocatbatch/client/MainAuctionLifecycleClientTest.java
  • src/test/java/com/rocketcrew/pocatbatch/client/MainCardSyncClientTest.java
  • src/test/java/com/rocketcrew/pocatbatch/job/auctionactivation/AuctionActivationTaskletTest.java
  • src/test/java/com/rocketcrew/pocatbatch/job/auctionexpiration/AuctionExpirationTaskletTest.java
  • src/test/java/com/rocketcrew/pocatbatch/job/cardsync/CardSyncTaskletTest.java
💤 Files with no reviewable changes (3)
  • src/main/java/com/rocketcrew/pocatbatch/domain/outbox/service/OutboxEventWriter.java
  • src/main/java/com/rocketcrew/pocatbatch/domain/auction/service/AuctionBatchService.java
  • src/main/java/com/rocketcrew/pocatbatch/domain/auction/entity/Auction.java

Comment thread docs/ARCHITECTURE.md Outdated
@cdkkyj123
cdkkyj123 merged commit 9050332 into main Jun 16, 2026
2 checks passed
@cdkkyj123
cdkkyj123 deleted the feat/auction-cardsync-internal-api/#235 branch June 16, 2026 01:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant