Skip to content

refactor: 동기 주문 경로 완전 제거 (#121)#122

Merged
ohhalim merged 4 commits into
developfrom
refactor/remove-sync-order-path
Jun 16, 2026
Merged

refactor: 동기 주문 경로 완전 제거 (#121)#122
ohhalim merged 4 commits into
developfrom
refactor/remove-sync-order-path

Conversation

@ohhalim

@ohhalim ohhalim commented Jun 16, 2026

Copy link
Copy Markdown
Owner

배경

멘토 코드 리뷰에서 sync/async 경로가 공존하는 구조가 혼란스럽다는 피드백.
이제 async 경로만 남겨두고 동기 체결 관련 코드 전부 제거.

변경 사항

프로덕션

  • SyncOrderProcessor, CreateOrderResponse 삭제
  • OrderService에서 동기 체결 로직 제거
  • POST /orders/asyncPOST /orders 로 통합, 모든 주문 생성은 202 Accepted 반환
  • 오더북 가격 표시 오류 수정 (DB 조회 시 BigDecimal scale-18 → stripTrailingZeros() 적용)

테스트

  • 동기→비동기 전환 이후 워커 처리 완료 전에 assertion 하면서 터지던 테스트 전수 수정
  • Awaitility로 각 테스트에서 워커 완료 대기 추가 (160개 전부 통과)

커밋 구성

  • refactor: SyncOrderProcessor 제거
  • refactor: POST /orders 단일 경로로 통합
  • fix: 오더북 가격 표시 오류 수정
  • test: 비동기 워커 완료 대기 추가

Closes #121

Summary by CodeRabbit

  • API Changes

    • Order creation endpoint now returns HTTP 202 ACCEPTED status instead of 201 CREATED and processes asynchronously.
    • Removed the separate async order endpoint.
  • Bug Fixes

    • Improved order book price and quantity formatting by removing trailing zeros.
  • Tests

    • Updated integration tests to properly verify asynchronous order processing and eventual state consistency.

ohhalim added 4 commits June 16, 2026 14:07
동기 체결 경로에서만 쓰이던 SyncOrderProcessor, CreateOrderResponse 삭제.
OrderService에서 동기 처리 로직도 함께 정리.
/async 엔드포인트 제거하고 / 로 일원화.
이제 모든 주문 생성은 202 Accepted 반환.
DB에서 읽어온 BigDecimal이 scale-18로 들어오면서
100000000.000000000000000000 형태로 노출되던 문제 수정.
stripTrailingZeros().toPlainString() 적용.
동기→비동기 전환 이후 워커 처리 완료 전에 assertion 하면서 터지던 테스트 수정.
Awaitility로 각 테스트에서 워커 완료 대기 추가.
@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Removes the synchronous order creation path (SyncOrderProcessor, CreateOrderResponse) and consolidates POST /api/v1/orders/async into POST /api/v1/orders returning 202 ACCEPTED. OrderService is simplified to delegate directly to AcceptedOrderService. OrderBookResponse switches to stripTrailingZeros().toPlainString(). All tests are migrated to Awaitility-based async assertions.

Changes

Async Order Path Consolidation

Layer / File(s) Summary
Sync path removal: controller, service, processor, DTO
src/main/java/com/coinflow/order/api/OrderController.java, src/main/java/com/coinflow/order/service/OrderService.java, src/main/java/com/coinflow/order/service/processor/SyncOrderProcessor.java, src/main/java/com/coinflow/order/dto/CreateOrderResponse.java
OrderController merges /async into POST /api/v1/orders returning 202 ACCEPTED. OrderService constructor reduced to three dependencies and createOrder delegates to AcceptedOrderService.acceptOrder. SyncOrderProcessor (transactional matching + settlement + recovery) and CreateOrderResponse (record + TradeResult + factory) are deleted.
OrderBookResponse numeric formatting
src/main/java/com/coinflow/market/dto/OrderBookResponse.java
PriceLevel price and quantity strings switch from toPlainString() to stripTrailingZeros().toPlainString() in the aggregate method.
OrderApiTest and QueryApiTest async migration
src/test/java/com/coinflow/order/OrderApiTest.java, src/test/java/com/coinflow/query/QueryApiTest.java
OrderApiTest replaces 201 CREATED expectations with 202 ACCEPTED plus Awaitility polling on OrderRepository status; self-trade tests switch from immediate BAD_REQUEST to worker-driven REJECTED; createAsyncOrder path updated. QueryApiTest adds bounded await blocks on matchingEngine side sizes and tradeRepository.count() before REST assertions.
Integration test suite async migration
src/test/java/com/coinflow/integration/MatchingSettlementTest.java, src/test/java/com/coinflow/integration/ConcurrencyIntegrationTest.java, src/test/java/com/coinflow/integration/DomainEventTest.java, src/test/java/com/coinflow/integration/OrderBookRecoveryTest.java, src/test/java/com/coinflow/integration/KafkaPublishingIntegrationTest.java, src/test/java/com/coinflow/wallet/WalletApiTest.java, src/test/java/com/coinflow/integration/WebSocket*
All integration and WebSocket tests now assert 202 ACCEPTED, extract orderId from the response body, and use await().atMost(5s).untilAsserted(...) to poll orderRepository, tradeRepository, or matchingEngine before asserting settlement, domain event, or WebSocket snapshot outcomes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

  • #121 (refactor: 동기 주문 경로 제거) — This PR directly implements the issue: deletes SyncOrderProcessor, removes CreateOrderResponse, consolidates POST /api/v1/orders/asyncPOST /api/v1/orders with 202 ACCEPTED, and replaces all integration tests with async-path assertions.

Possibly related PRs

  • ohhalim/CoinFlow#118: Both PRs modify the order-processing pipeline around SyncOrderProcessor/AcceptedOrderService and the queued single-writer execution path.
  • ohhalim/CoinFlow#33: Directly overlaps in ConcurrencyIntegrationTest.java, updating concurrent order expectations from synchronous 201 CREATED to the async 202 ACCEPTED + Awaitility model.
  • ohhalim/CoinFlow#26: Both PRs modify OrderBookResponse aggregation and PriceLevel price/quantity string construction in the same class.

Poem

🐇 Hop, hop — the sync path is gone!
No more waiting for 201 at dawn,
202 ACCEPTED hops along,
Awaitility polls where orders belong.
The rabbit says: async all along! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 24.49% 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 title 'refactor: 동기 주문 경로 완전 제거' clearly and concisely summarizes the main change—complete removal of the synchronous order path—matching the primary objective of consolidating to a single async endpoint.
Linked Issues check ✅ Passed All key requirements from issue #121 are met: SyncOrderProcessor deleted, OrderService.createOrder refactored to async, endpoints consolidated to POST /orders returning 202 Accepted, CreateOrderResponse removed, and all integration tests updated to async patterns.
Out of Scope Changes check ✅ Passed The order book display fix (stripTrailingZeros in OrderBookResponse) is a related bug fix noted in the PR objectives and commit messages, not an out-of-scope change; all other modifications directly support the stated refactoring goal.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/remove-sync-order-path

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.

@ohhalim ohhalim merged commit db8b8a3 into develop Jun 16, 2026
0 of 2 checks passed
@ohhalim ohhalim deleted the refactor/remove-sync-order-path branch June 16, 2026 05:30
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.

refactor: 동기 주문 경로 제거

1 participant