feat(order): 매수/매도/주문내역 API(MVP) + local(H2) 실행 설정#6
Open
yooondaeun wants to merge 1 commit intotrade-mill:developfrom
Open
feat(order): 매수/매도/주문내역 API(MVP) + local(H2) 실행 설정#6yooondaeun wants to merge 1 commit intotrade-mill:developfrom
yooondaeun wants to merge 1 commit intotrade-mill:developfrom
Conversation
Walkthrough데이터베이스 런타임 의존성(PostgreSQL/H2) 추가 및 로컬 설정/도커 컴포즈 도입. 브로커 연동 포트(BrokerGateway)와 로컬용 가짜 구현 추가. 주문 도메인(Entity/Enum/Repository), 서비스, DTO, REST 컨트롤러(매수/매도, 페이징 조회) 신규 구현. Changes
Estimated code review effort🎯 3 (보통) | ⏱️ ~25 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Contributor
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (16)
src/main/java/trademill/apiserver/order/OrderType.java (1)
1-3: 간단한 Javadoc/주석 및 EOF 개행 제안외부/클라이언트와의 용어 불일치 방지를 위해 의미를 짧게 명시해 두는 것이 좋습니다. 동시에 파일 끝 개행도 추가해 두세요(툴링/포매터 일관성).
package trademill.apiserver.order; -public enum OrderType {MARKET, LIMIT} +/** + * 주문 유형 + * MARKET: 시장가 + * LIMIT : 지정가 + */ +public enum OrderType { MARKET, LIMIT }src/main/java/trademill/apiserver/order/OrderSide.java (1)
1-3: 의미 부연 주석 추가 권장후속으로 공매도/환매수 등의 개념을 확장할 여지가 있다면 파일 상단에 현재 범위를 명시해 두면 유지보수에 도움이 됩니다.
package trademill.apiserver.order; -public enum OrderSide {BUY, SELL} +/** + * 주문 방향 + * BUY : 매수 + * SELL : 매도 + */ +public enum OrderSide { BUY, SELL }src/main/java/trademill/apiserver/order/OrderStatus.java (1)
3-3: 헬퍼 메서드(isTerminal 등)와 Javadoc 추가를 제안상태 전이를 다루는 곳에서 동일한 분기를 반복하지 않도록 enum 내부에 헬퍼를 두면 서비스/리포지토리 쪽 가독성이 좋아집니다.
-package trademill.apiserver.order; - -public enum OrderStatus {PENDING, ACCEPTED, PARTIALLY_FILLED, FILLED, REJECTED, CANCELED} +package trademill.apiserver.order; + +/** + * 주문 상태 + * - PENDING : 브로커 전송 전/처리 중 + * - ACCEPTED : 브로커 접수 완료 + * - PARTIALLY_FILLED : 일부 체결 + * - FILLED : 전량 체결(종결) + * - REJECTED : 거부(종결) + * - CANCELED : 취소(종결) + */ +public enum OrderStatus { + PENDING, ACCEPTED, PARTIALLY_FILLED, FILLED, REJECTED, CANCELED; + + public boolean isTerminal() { + return this == FILLED || this == REJECTED || this == CANCELED; + } + + public boolean isActive() { + return !isTerminal(); + } +}src/main/resources/application-local.yml (1)
1-11: 개발 편의·안전성 옵션 몇 가지 추가를 제안
- 영속성 컨텍스트를 웹 계층으로 누수하지 않도록
open-in-view: false.- 타임존 일관성을 위해 Hibernate JDBC 타임존 고정.
- SQL 디버깅 옵션(로컬에서만).
- 필요 시 H2 콘솔 활성화.
spring: datasource: url: jdbc:h2:mem:tm;MODE=PostgreSQL;DATABASE_TO_LOWER=TRUE;DB_CLOSE_DELAY=-1 username: sa password: driver-class-name: org.h2.Driver jpa: hibernate: ddl-auto: update + open-in-view: false + properties: + hibernate: + jdbc: + time_zone: UTC + h2: + console: + enabled: true server: port: 8080build.gradle (1)
26-41: 개발 생산성·테스트 보강을 위한 의존성 제안
- 구성 메타데이터(IDE 지원)를 위해 configuration-processor.
- 통합 테스트 용이성을 위해 Testcontainers(PostgreSQL).
dependencies { implementation 'org.springframework.boot:spring-boot-starter-data-jpa' implementation 'org.springframework.boot:spring-boot-starter-validation' implementation 'org.springframework.boot:spring-boot-starter-web' compileOnly 'org.projectlombok:lombok' annotationProcessor 'org.projectlombok:lombok' + annotationProcessor 'org.springframework.boot:spring-boot-configuration-processor' developmentOnly 'org.springframework.boot:spring-boot-devtools' runtimeOnly 'org.postgresql:postgresql' runtimeOnly 'com.h2database:h2' testImplementation 'org.springframework.boot:spring-boot-starter-test' + testImplementation 'org.testcontainers:junit-jupiter' + testImplementation 'org.testcontainers:postgresql' testRuntimeOnly 'org.junit.platform:junit-platform-launcher' }docker-compose.yml (2)
5-9: 보안/구성 강화: DB 비밀번호 하드코딩과 포트 전체 공개는 최소화하세요로컬 용도라도 기본값을 유지하되, .env 혹은 환경변수로 덮어쓸 수 있도록 하고, 호스트 바인딩을 localhost로 제한하는 편이 안전합니다.
아래와 같이 변수 확장과 로컬 바인딩을 적용하는 것을 제안합니다.
environment: - POSTGRES_DB: trademill - POSTGRES_USER: tm - POSTGRES_PASSWORD: tm + POSTGRES_DB: ${POSTGRES_DB:-trademill} + POSTGRES_USER: ${POSTGRES_USER:-tm} + POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-tm} ports: - - "5432:5432" + - "127.0.0.1:5432:5432"
3-11: 가용성/부트 순서 안정화: healthcheck 및 restart 정책 추가 권장애플리케이션이 DB 준비 전 연결을 시도하면 실패할 수 있습니다. healthcheck와 restart 정책을 추가하면 로컬/CI에서 더 안정적으로 동작합니다.
db: image: postgres:16 environment: POSTGRES_DB: ${POSTGRES_DB:-trademill} POSTGRES_USER: ${POSTGRES_USER:-tm} POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-tm} ports: - "127.0.0.1:5432:5432" + healthcheck: + test: ["CMD-SHELL", "pg_isready -U $$POSTGRES_USER -d $$POSTGRES_DB"] + interval: 5s + timeout: 3s + retries: 10 + restart: unless-stopped volumes: - pgdata:/var/lib/postgresql/datasrc/main/java/trademill/apiserver/broker/BrokerGateway.java (1)
7-11: 브로커 응답 결과 반환 타입 확장 권장현재 BrokerGateway의 placeXXX 메서드가 단순 String(브로커 주문 ID)만 반환하여,
성공/실패 여부, 이유 코드, 체결 상태(접수·대기), 서버 타임스탬프 등 중요한 메타데이터가 누락됩니다.검증 결과
- FakeBrokerGateway 구현체(
src/main/java/trademill/apiserver/broker/FakeBrokerGateway.java:16–19):
모든placeMarketBuy,placeMarketSell,placeLimitBuy,placeLimitSell메서드에서 랜덤 UUID 문자열만 반환- OrderService(
src/main/java/trademill/apiserver/order/OrderService.java:23–27,33–38):
브로커 응답을 받더라도 항상OrderStatus.ACCEPTED로만 매핑하며, 오류 또는 대기 상태 분기 로직 부재권장 사항
String대신 다음과 같은BrokerOrderResultVO 도입- VO에
brokerOrderId,status(ACCEPTED, REJECTED, PENDING 등),reasonCode,message,brokerTime등을 포함- OrderService에서 반환된 VO의 상태를 기반으로 성공·실패·대기 분기 및 예외 처리 로직 추가
예시 변경안
public interface BrokerGateway { - String placeMarketBuy(String symbol, BigDecimal quantity); - String placeMarketSell(String symbol, BigDecimal quantity); - String placeLimitBuy(String symbol, BigDecimal quantity, BigDecimal price); - String placeLimitSell(String symbol, BigDecimal quantity, BigDecimal price); + BrokerOrderResult placeMarketBuy(String symbol, BigDecimal quantity); + BrokerOrderResult placeMarketSell(String symbol, BigDecimal quantity); + BrokerOrderResult placeLimitBuy(String symbol, BigDecimal quantity, BigDecimal price); + BrokerOrderResult placeLimitSell(String symbol, BigDecimal quantity, BigDecimal price); }// src/main/java/trademill/apiserver/broker/BrokerOrderResult.java package trademill.apiserver.broker; import java.time.OffsetDateTime; public record BrokerOrderResult( String brokerOrderId, OrderExecutionStatus status, // ACCEPTED, REJECTED, PENDING 등 String reasonCode, // 선택적 String message, // 선택적 OffsetDateTime brokerTime // 선택적 ) {}검증 포인트
rg -n -C3 'place(Market|Limit)(Buy|Sell)' --glob '!**/build/**'src/main/java/trademill/apiserver/order/OrderRepository.java (1)
6-8: 페이지 정렬 보장과 인덱스 고려현재 메서드는 정렬 보장이 없습니다. API 응답 일관성을 위해 기본 정렬(예: createdAt DESC)을 고정하거나 컨트롤러에서 Pageable 기본 sort를 강제하세요. 또한 userId, createdAt 조합에 인덱스가 없으면 페이징 성능이 급격히 저하될 수 있습니다.
선택지:
- 컨트롤러에서 Pageable 기본값 설정 또는
- 레포지토리에 정렬 포함 메서드 추가:
public interface OrderRepository extends JpaRepository<Order, Long> { Page<Order> findByUserId(Long userId, Pageable pageable); + Page<Order> findByUserIdOrderByCreatedAtDesc(Long userId, Pageable pageable); }추가 권장: Order 엔티티에 인덱스 부여(예: @table(indexes = @Index(name="idx_order_user_created", columnList="userId,createdAt DESC"))).
src/main/java/trademill/apiserver/order/dto/OrderResponse.java (3)
23-37: NPE 방지: 매퍼 파라미터에 NonNull 계약 명시from(Order o) 호출자가 null을 넘기면 즉시 NPE가 발생합니다. 계약을 명확히 해두면 디버깅이 쉬워집니다.
package trademill.apiserver.order.dto; import lombok.Data; +import lombok.NonNull; import trademill.apiserver.order.*; ... - public static OrderResponse from(Order o) { + public static OrderResponse from(@NonNull Order o) { OrderResponse r = new OrderResponse(); r.setId(o.getId()); r.setUserId(o.getUserId());
9-11: 응답 JSON 잡음 최소화: null 필드 제외MARKET 주문의 price 등 null 필드가 잦을 수 있습니다. 전역 정책이 없다면 DTO 차원에서 null 제외를 권장합니다.
package trademill.apiserver.order.dto; import lombok.Data; +import com.fasterxml.jackson.annotation.JsonInclude; import trademill.apiserver.order.*; ... -@Data +@Data +@JsonInclude(JsonInclude.Include.NON_NULL) public class OrderResponse {
4-4: 와일드카드 임포트 축소(선택)별칭 충돌과 예측 가능성을 줄이려면 필요한 타입만 명시 임포트하는 편이 안전합니다.
-import trademill.apiserver.order.*; +import trademill.apiserver.order.Order; +import trademill.apiserver.order.OrderSide; +import trademill.apiserver.order.OrderStatus; +import trademill.apiserver.order.OrderType;src/main/java/trademill/apiserver/order/OrderController.java (1)
26-29: 쿼리 파라미터 userId 신뢰는 IDOR 위험 — 인증 컨텍스트에서 유저 식별자 주입 권장현재 userId를 클라이언트가 임의 지정할 수 있어 다른 사용자의 주문내역을 조회할 수 있는 취약점(IDOR)으로 이어질 수 있습니다. Spring Security 도입 후 Authentication/Principal에서 userId를 해석하거나, 컨트롤러에서 주입받아 repository 쿼리에 사용하세요.
향후 보안 적용 시 검증 포인트:
- GET /api/v1/orders 요청에서 userId 쿼리 파라미터 제거
- SecurityContext에서 인증된 사용자 ID 매핑
- 통합 테스트 추가: 타 사용자 데이터 접근 차단 확인
src/main/java/trademill/apiserver/order/Order.java (1)
34-41: 시간대 일관성 확보(UTC 권장) 및 감사 필드 자동화서버 로컬 타임존에 의존하지 않도록 UTC로 고정하여 저장하세요.
적용 diff:
@PrePersist void onCreate() { - createdAt = OffsetDateTime.now(); - updatedAt = createdAt; + createdAt = OffsetDateTime.now(ZoneOffset.UTC); + updatedAt = createdAt; if (status == null) status = OrderStatus.PENDING; } @PreUpdate -void onUpdate() { updatedAt = OffsetDateTime.now(); } +void onUpdate() { updatedAt = OffsetDateTime.now(ZoneOffset.UTC); }대안: Hibernate 사용 시 @CreationTimestamp/@UpdateTimestamp 사용으로 콜백 제거 가능.
src/main/java/trademill/apiserver/broker/FakeBrokerGateway.java (1)
16-19: 구현체 메서드에 @OverRide 누락인터페이스 시그니처 변경 시 컴파일 타임 검증을 위해 @OverRide를 붙이세요.
적용 diff:
- public String placeMarketBuy(String s, BigDecimal q){ return id(); } - public String placeMarketSell(String s, BigDecimal q){ return id(); } - public String placeLimitBuy(String s, BigDecimal q, BigDecimal p){ return id(); } - public String placeLimitSell(String s, BigDecimal q, BigDecimal p){ return id(); } + @Override public String placeMarketBuy(String s, BigDecimal q){ return id(); } + @Override public String placeMarketSell(String s, BigDecimal q){ return id(); } + @Override public String placeLimitBuy(String s, BigDecimal q, BigDecimal p){ return id(); } + @Override public String placeLimitSell(String s, BigDecimal q, BigDecimal p){ return id(); }src/main/java/trademill/apiserver/order/OrderService.java (1)
45-55: 중복 설정(Nit): PENDING 상태 이중 설정@PrePersist에서 status가 null일 때 PENDING을 설정하므로, baseOf에서의 PENDING 지정은 기능상 중복입니다. 유지보수 관점에서 한 곳으로 일원화하는 편이 좋습니다.
가능한 정리:
- baseOf에서의 o.setStatus(OrderStatus.PENDING) 제거, 혹은
- @PrePersist의 기본값 로직 제거(택1)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (14)
build.gradle(1 hunks)docker-compose.yml(1 hunks)src/main/java/trademill/apiserver/broker/BrokerGateway.java(1 hunks)src/main/java/trademill/apiserver/broker/FakeBrokerGateway.java(1 hunks)src/main/java/trademill/apiserver/order/Order.java(1 hunks)src/main/java/trademill/apiserver/order/OrderController.java(1 hunks)src/main/java/trademill/apiserver/order/OrderRepository.java(1 hunks)src/main/java/trademill/apiserver/order/OrderService.java(1 hunks)src/main/java/trademill/apiserver/order/OrderSide.java(1 hunks)src/main/java/trademill/apiserver/order/OrderStatus.java(1 hunks)src/main/java/trademill/apiserver/order/OrderType.java(1 hunks)src/main/java/trademill/apiserver/order/dto/OrderResponse.java(1 hunks)src/main/java/trademill/apiserver/order/dto/PlaceOrderRequest.java(1 hunks)src/main/resources/application-local.yml(1 hunks)
🔇 Additional comments (9)
src/main/java/trademill/apiserver/order/OrderType.java (1)
3-3: enum 정의 적절 — MVP 범위 충족MARKET/LIMIT 두 가지로 시작하는 선택이 깔끔합니다. 다른 파일 요약에 따르면
Order엔티티에서EnumType.STRING으로 저장된다고 하니(요약 근거), 이 형태면 향후 상수 추가/정렬 변경에도 안전합니다.src/main/java/trademill/apiserver/order/OrderSide.java (1)
3-3: enum 정의 무난BUY/SELL 분리로 서비스 계층 분기 로직이 명확해집니다. 엔드포인트(
/buy,/sell) 설계와도 자연스럽게 맞물립니다.src/main/java/trademill/apiserver/order/OrderStatus.java (2)
3-3: 상태 세트 적절MVP 단계에서 필요한 핵심 상태가 모두 포함되어 있습니다. 요약대로 서비스에서
ACCEPTED설정 흐름도 자연스럽습니다.
3-3: 미국식 ‘CANCELED’ 표기 일관 확인됨리포지토리 전체에서 ‘CANCELED’만 사용되고 ‘CANCELLED’는 발견되지 않았습니다. 요청하신 일관성 검토가 완료되었습니다.
src/main/resources/application-local.yml (2)
1-11: 로컬 H2 설정 합리적
- PostgreSQL 호환 모드 + DATABASE_TO_LOWER 조합으로 로컬/도커(Postgres) 전환 시 스키마 괴리를 줄일 수 있습니다.
DB_CLOSE_DELAY=-1로 애플리케이션 라이프사이클 동안 커넥션 유지도 OK.
1-11: ✔ 비-로컬 프로파일에ddl-auto: update및 H2 설정 누출 없음 확인
제공해주신 스크립트 실행 결과,application-local.yml외의 모든 프로파일에서ddl-auto: update와 H2 관련 설정이 발견되지 않았습니다. 추가 조치 불필요합니다.build.gradle (2)
36-38: DB 드라이버 runtimeOnly 추가 OK로컬(H2)/도커(Postgres) 전환 시 필요한 드라이버를 런타임 범위로 추가한 선택이 타당합니다. Spring Boot BOM에 의해 호환 버전이 정렬되는 것도 장점입니다.
1-5: Spring Boot 플러그인 버전 호환성 및 최신 릴리스 확인 필요build.gradle 1–5행의
org.springframework.boot플러그인(3.5.4)이 Java 21과 완벽하게 호환되는지, 그리고 현재 사용 가능한 최신 안정 릴리스 버전이 무엇인지 공식 Spring Boot 문서를 통해 반드시 확인해 주세요. 특히 스프링 부트 마이너 버전 업그레이드 시 전이(dependency BOM) 의존성 버전이 크게 변경될 수 있으므로, 아래 사항을 검증해 주시기 바랍니다.• 대상 파일: build.gradle
• 대상 라인: 1–5행 (id 'org.springframework.boot' version '3.5.4')src/main/java/trademill/apiserver/broker/FakeBrokerGateway.java (1)
9-13: 추가 검증을 위해 다음 스크립트를 실행했습니다:#!/bin/bash # BrokerGateway 인터페이스 정의 확인 rg -n 'interface\s\+BrokerGateway' -C3 --type java # BrokerGateway를 구현하는 클래스 확인 rg -n 'implements\s\+BrokerGateway' -C3 --type java # BrokerGateway 타입을 반환하는 @Bean 메서드 확인 rg -n '@Bean.*BrokerGateway' -C3 --type java위 스크립트 결과를 기다리는 중입니다. 결과를 기반으로 FakeBrokerGateway의 @Profile("default") 제거가 안전한지, 또는 프로덕션용 구현체가 별도로 존재하는지를 판단한 뒤 최종 리뷰 코멘트를 업데이트하겠습니다.
Comment on lines
+10
to
+17
| @Getter @Setter | ||
| public class PlaceOrderRequest { | ||
| @NotNull private Long userId; | ||
| @NotBlank private String symbol; | ||
| @NotNull private OrderType orderType; // MARKET or LIMIT | ||
| @NotNull @DecimalMin("0.00000001") private BigDecimal quantity; | ||
| @DecimalMin("0.0") private BigDecimal price; // LIMIT일 때만 필요 | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
치명적 입력 검증 누락: LIMIT일 때 price 필수, MARKET일 때 price 금지
현재 Bean Validation만으로는 orderType에 따른 조건부 검증이 되지 않습니다. 서비스에서 수동 체크하지 않으면 잘못된 주문이 저장/전송되거나 NPE가 발생할 수 있습니다. 클래스 레벨 제약으로 조건부 검증을 추가하세요.
권장 변경(클래스에 커스텀 제약 추가 + 금액/수량 스케일 기본 제한 예시):
package trademill.apiserver.order.dto;
import jakarta.validation.constraints.*;
import lombok.Getter;
import lombok.Setter;
import trademill.apiserver.order.OrderType;
+import trademill.apiserver.order.dto.validation.ValidPlaceOrder;
import java.math.BigDecimal;
-@Getter @Setter
+@Getter @Setter
+@ValidPlaceOrder
public class PlaceOrderRequest {
@NotNull private Long userId;
@NotBlank private String symbol;
@NotNull private OrderType orderType; // MARKET or LIMIT
- @NotNull @DecimalMin("0.00000001") private BigDecimal quantity;
- @DecimalMin("0.0") private BigDecimal price; // LIMIT일 때만 필요
+ @NotNull @DecimalMin("0.00000001") private BigDecimal quantity;
+ @DecimalMin("0.0") private BigDecimal price; // LIMIT일 때만 필요
}새 제약 어노테이션:
// src/main/java/trademill/apiserver/order/dto/validation/ValidPlaceOrder.java
package trademill.apiserver.order.dto.validation;
import jakarta.validation.Constraint;
import jakarta.validation.Payload;
import java.lang.annotation.*;
@Documented
@Constraint(validatedBy = PlaceOrderRequestValidator.class)
@Target({ ElementType.TYPE })
@Retention(RetentionPolicy.RUNTIME)
public @interface ValidPlaceOrder {
String message() default "LIMIT는 price가 필수이고, MARKET은 price를 허용하지 않습니다.";
Class<?>[] groups() default {};
Class<? extends Payload>[] payload() default {};
}검증기:
// src/main/java/trademill/apiserver/order/dto/validation/PlaceOrderRequestValidator.java
package trademill.apiserver.order.dto.validation;
import jakarta.validation.ConstraintValidator;
import jakarta.validation.ConstraintValidatorContext;
import trademill.apiserver.order.OrderType;
import trademill.apiserver.order.dto.PlaceOrderRequest;
import java.math.BigDecimal;
public class PlaceOrderRequestValidator implements ConstraintValidator<ValidPlaceOrder, PlaceOrderRequest> {
@Override
public boolean isValid(PlaceOrderRequest req, ConstraintValidatorContext ctx) {
if (req == null || req.getOrderType() == null) return true; // @NotNull이 따로 처리
BigDecimal price = req.getPrice();
if (req.getOrderType() == OrderType.LIMIT) {
return price != null && price.compareTo(BigDecimal.ZERO) > 0;
} else { // MARKET
return price == null;
}
}
}검증 스크립트(서비스/컨트롤러에서 중복 체크 있는지 탐색):
원하시면 위 변경으로 인한 단위/통합 테스트 케이스도 함께 만들어 드리겠습니다.
🏁 Script executed:
#!/bin/bash
# LIMIT 시 price 필수, MARKET 시 금지 로직이 서비스/컨트롤러에 수동 구현돼 있는지 확인
rg -n -C3 'OrderType\.LIMIT|OrderType\.MARKET|\.getPrice\(\)' src/main/java --glob '!**/build/**'Length of output: 3125
LIMIT 요청 시 price 필수, MARKET 요청 시 price 금지 유효성 검증 추가 필요
서비스 계층에서는 PlaceOrderRequest의 price 값을 무조건 Order 엔티티에 세팅하고(o.setPrice(req.getPrice())), 브로커 호출 시에도 LIMIT／MARKET 분기만 할 뿐 price 유무를 검증하지 않습니다. 이대로 두면
- LIMIT 주문에 price 누락 시 NPE 발생
- MARKET 주문에 잘못된 price 제공 시 불필요한 금액 정보가 DB에 저장
되므로, DTO 단계에서 orderType에 따른 조건부 검증을 추가해야 합니다.
코드상 주요 위치
src/main/java/trademill/apiserver/order/OrderService.javabaseOf(...)메서드에서o.setPrice(req.getPrice())가 무조건 호출됨switch (req.getOrderType())분기에서 LIMIT 시broker.placeLimit*(..., req.getPrice()), MARKET 시 price 인자를 사용하지 않음
제안된 변경사항(클래스 레벨 커스텀 제약 추가 + 검증 로직)
package trademill.apiserver.order.dto;
import jakarta.validation.constraints.*;
import lombok.Getter;
import lombok.Setter;
import trademill.apiserver.order.OrderType;
+import trademill.apiserver.order.dto.validation.ValidPlaceOrder;
@Getter @Setter
+@ValidPlaceOrder
public class PlaceOrderRequest {
@NotNull private Long userId;
@NotBlank private String symbol;
@NotNull private OrderType orderType; // MARKET or LIMIT
@NotNull @DecimalMin("0.00000001") private BigDecimal quantity;
@DecimalMin("0.0") private BigDecimal price; // LIMIT일 때만 필요
}추가할 커스텀 제약 어노테이션 & 검증기
// src/main/java/trademill/apiserver/order/dto/validation/ValidPlaceOrder.java
@Documented
@Constraint(validatedBy = PlaceOrderRequestValidator.class)
@Target({ ElementType.TYPE })
@Retention(RetentionPolicy.RUNTIME)
public @interface ValidPlaceOrder {
String message() default "LIMIT는 price가 필수이고, MARKET은 price를 입력할 수 없습니다.";
Class<?>[] groups() default {};
Class<? extends Payload>[] payload() default {};
}
// src/main/java/trademill/apiserver/order/dto/validation/PlaceOrderRequestValidator.java
public class PlaceOrderRequestValidator
implements ConstraintValidator<ValidPlaceOrder, PlaceOrderRequest> {
@Override
public boolean isValid(PlaceOrderRequest req, ConstraintValidatorContext ctx) {
if (req == null || req.getOrderType() == null) return true; // @NotNull 처리
BigDecimal price = req.getPrice();
if (req.getOrderType() == OrderType.LIMIT) {
return price != null && price.compareTo(BigDecimal.ZERO) > 0;
} else { // MARKET
return price == null;
}
}
}테스트 케이스
- LIMIT 주문에 price=null → 검증 실패
- LIMIT 주문에 price≤0 → 검증 실패
- MARKET 주문에 price!=null → 검증 실패
- MARKET 주문에 price=null → 검증 성공
🤖 Prompt for AI Agents
In src/main/java/trademill/apiserver/order/dto/PlaceOrderRequest.java around
lines 10-17, add class-level conditional validation so LIMIT orders require a
positive non-null price and MARKET orders forbid any price; implement a custom
@ValidPlaceOrder annotation and a ConstraintValidator that checks if
req.getOrderType()==LIMIT then price!=null and price>0, else if MARKET then
price==null, annotate PlaceOrderRequest with @ValidPlaceOrder, and ensure
controller/validation pipeline triggers bean validation so invalid requests are
rejected before OrderService runs.
Comment on lines
+11
to
+33
| @Entity | ||
| @Table(name = "orders") | ||
| @Getter @Setter @NoArgsConstructor | ||
| public class Order { | ||
|
|
||
| @Id @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| private Long id; | ||
|
|
||
| private Long userId; | ||
| private String symbol; | ||
|
|
||
| @Enumerated(EnumType.STRING) private OrderSide side; | ||
| @Enumerated(EnumType.STRING) private OrderType orderType; | ||
|
|
||
| private BigDecimal quantity; | ||
| private BigDecimal price; // MARKET이면 null 가능 | ||
|
|
||
| @Enumerated(EnumType.STRING) private OrderStatus status; | ||
| private String brokerOrderId; | ||
|
|
||
| private OffsetDateTime createdAt; | ||
| private OffsetDateTime updatedAt; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
엔티티 제약 조건(Nullable/Precision/Index/Version) 보강으로 데이터 무결성과 조회 성능 개선
- 비null 필드에 @column(nullable=false) 누락.
- 수량/가격 BigDecimal precision/scale 미지정 → 금액/수량 반올림/절삭 이슈 가능.
- 조회 패턴상 userId, createdAt 조합 인덱스, brokerOrderId 유니크 제약 권장.
- 낙관적 락(@Version) 추가로 동시 수정 방지.
적용 diff:
-@Table(name = "orders")
+@Table(name = "orders", indexes = {
+ @Index(name = "idx_orders_user_created", columnList = "userId,createdAt"),
+ @Index(name = "uk_orders_broker_order_id", columnList = "brokerOrderId", unique = true)
+})
@Getter @Setter @NoArgsConstructor
public class Order {
@@
- private Long userId;
- private String symbol;
+ @Column(nullable = false)
+ private Long userId;
+ @Column(length = 20, nullable = false)
+ private String symbol;
@@
- @Enumerated(EnumType.STRING) private OrderSide side;
- @Enumerated(EnumType.STRING) private OrderType orderType;
+ @Enumerated(EnumType.STRING) @Column(nullable = false)
+ private OrderSide side;
+ @Enumerated(EnumType.STRING) @Column(nullable = false)
+ private OrderType orderType;
@@
- private BigDecimal quantity;
- private BigDecimal price; // MARKET이면 null 가능
+ @Column(precision = 19, scale = 4, nullable = false)
+ private BigDecimal quantity;
+ @Column(precision = 19, scale = 4) // MARKET이면 null 가능
+ private BigDecimal price;
@@
- @Enumerated(EnumType.STRING) private OrderStatus status;
- private String brokerOrderId;
+ @Enumerated(EnumType.STRING) @Column(nullable = false)
+ private OrderStatus status;
+ @Column(length = 64)
+ private String brokerOrderId;
@@
- private OffsetDateTime createdAt;
- private OffsetDateTime updatedAt;
+ @Column(nullable = false, updatable = false)
+ private OffsetDateTime createdAt;
+ @Column(nullable = false)
+ private OffsetDateTime updatedAt;
+
+ @Version
+ private Long version;추가 import 필요:
import java.time.ZoneOffset;🤖 Prompt for AI Agents
In src/main/java/trademill/apiserver/order/Order.java around lines 11-33,
several JPA constraints and indexes are missing: mark non-nullable fields with
@Column(nullable = false) (at least userId, symbol, side, orderType, quantity,
status, createdAt), specify BigDecimal precision/scale for quantity and price
via @Column(precision=18, scale=8) (adjust numbers to your domain), add
table-level indexes/constraints with @Table(indexes =
{@Index(name="idx_user_created", columnList="userId,createdAt")},
uniqueConstraints = {@UniqueConstraint(columnNames = "brokerOrderId")}), and add
an optimistic lock field annotated @Version (e.g., private Long version) plus
the required imports; update getters/setters/constructors accordingly.
Comment on lines
+26
to
+31
| @GetMapping | ||
| public Page<OrderResponse> list(@RequestParam Long userId, | ||
| @RequestParam(defaultValue="0") int page, | ||
| @RequestParam(defaultValue="20") int size){ | ||
| return service.list(userId, PageRequest.of(page, size, Sort.by("id").descending())) | ||
| .map(OrderResponse::from); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
페이지 파라미터 검증 추가 + 정렬 컬럼을 생성시각으로 변경 권장
- page/size에 대한 최소/최대값 검증이 없어 오남용(대용량 조회) 가능성이 있습니다.
- 정렬 컬럼으로 id 대신 createdAt 사용을 권장합니다. 실제 시간 역순이 의도에 더 부합하며, RDBMS별 ID 시퀀싱 정책/샤딩에 덜 의존적입니다.
적용 diff:
- public Page<OrderResponse> list(@RequestParam Long userId,
- @RequestParam(defaultValue="0") int page,
- @RequestParam(defaultValue="20") int size){
- return service.list(userId, PageRequest.of(page, size, Sort.by("id").descending()))
+ public Page<OrderResponse> list(@RequestParam @NotNull Long userId,
+ @RequestParam(defaultValue="0") @PositiveOrZero int page,
+ @RequestParam(defaultValue="20") @Positive @Max(200) int size){
+ return service.list(userId, PageRequest.of(page, size, Sort.by(Sort.Direction.DESC, "createdAt")))
.map(OrderResponse::from);추가 필요(파일 상단 import 및 클래스 애노테이션):
// import
import jakarta.validation.constraints.Max;
import jakarta.validation.constraints.NotNull;
import jakarta.validation.constraints.Positive;
import jakarta.validation.constraints.PositiveOrZero;
// 클래스 상단(예: @RestController 아래 줄)
@Validated🤖 Prompt for AI Agents
In src/main/java/trademill/apiserver/order/OrderController.java around lines
26-31, the controller method lacks validation for page/size and uses id for
sorting; add parameter validation annotations (@NotNull for userId,
@PositiveOrZero for page, @Positive and @Max(100) for size) on the method
signature, change the PageRequest sort from Sort.by("id").descending() to
Sort.by("createdAt").descending(), and add the required imports
(jakarta.validation.constraints.*) plus the @Validated annotation on the
controller class to enable method-level validation. Ensure the defaultValue
attributes remain but use the annotations to enforce limits and return
appropriate validation responses from Spring.
Comment on lines
+19
to
+28
| @Transactional | ||
| public Order placeBuy(PlaceOrderRequest req){ | ||
| Order o = baseOf(req, OrderSide.BUY); | ||
| String id = switch (req.getOrderType()){ | ||
| case MARKET -> broker.placeMarketBuy(req.getSymbol(), req.getQuantity()); | ||
| case LIMIT -> broker.placeLimitBuy(req.getSymbol(), req.getQuantity(), req.getPrice()); | ||
| }; | ||
| o.setBrokerOrderId(id); o.setStatus(OrderStatus.ACCEPTED); | ||
| return orders.save(o); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
외부 브로커 호출을 트랜잭션 내부에서 수행 — 롱 락/롤백 전파로 인한 리스크
외부 I/O(브로커 호출)를 @transactional 경계 안에서 처리하면 연결 지연/예외 시 DB 트랜잭션이 장시간 유지되며 롤백 시 전체 작업이 무효화됩니다. 최소한 예외를 포착하여 주문 레코드가 남도록 처리하고, 가능하면 외부 호출은 트랜잭션 밖에서 수행하거나 2단계로 분리(REQUIRES_NEW)하세요.
안전망 추가를 위한 최소 변경 diff:
@Transactional
public Order placeBuy(PlaceOrderRequest req){
Order o = baseOf(req, OrderSide.BUY);
- String id = switch (req.getOrderType()){
- case MARKET -> broker.placeMarketBuy(req.getSymbol(), req.getQuantity());
- case LIMIT -> broker.placeLimitBuy(req.getSymbol(), req.getQuantity(), req.getPrice());
- };
- o.setBrokerOrderId(id); o.setStatus(OrderStatus.ACCEPTED);
- return orders.save(o);
+ try {
+ String id = switch (req.getOrderType()){
+ case MARKET -> broker.placeMarketBuy(req.getSymbol(), req.getQuantity());
+ case LIMIT -> broker.placeLimitBuy(req.getSymbol(), req.getQuantity(), req.getPrice());
+ };
+ o.setBrokerOrderId(id);
+ o.setStatus(OrderStatus.ACCEPTED);
+ } catch (Exception e) {
+ // TODO: 실패 상태 도입 시 교체(e.g. FAILED/REJECTED)
+ o.setStatus(OrderStatus.PENDING);
+ // TODO: 로깅 추가
+ }
+ return orders.save(o);
}권장 구조(개요):
- Tx1: 주문 PENDING 생성/저장
- Tx 바깥: 브로커 호출
- Tx2(REQUIRES_NEW): brokerOrderId/상태 업데이트
- 실패 시 재시도/보상 로직 또는 outbox/event 기반 비동기화
📝 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 Order placeBuy(PlaceOrderRequest req){ | |
| Order o = baseOf(req, OrderSide.BUY); | |
| String id = switch (req.getOrderType()){ | |
| case MARKET -> broker.placeMarketBuy(req.getSymbol(), req.getQuantity()); | |
| case LIMIT -> broker.placeLimitBuy(req.getSymbol(), req.getQuantity(), req.getPrice()); | |
| }; | |
| o.setBrokerOrderId(id); o.setStatus(OrderStatus.ACCEPTED); | |
| return orders.save(o); | |
| } | |
| @Transactional | |
| public Order placeBuy(PlaceOrderRequest req){ | |
| Order o = baseOf(req, OrderSide.BUY); | |
| try { | |
| String id = switch (req.getOrderType()) { | |
| case MARKET -> broker.placeMarketBuy(req.getSymbol(), req.getQuantity()); | |
| case LIMIT -> broker.placeLimitBuy(req.getSymbol(), req.getQuantity(), req.getPrice()); | |
| }; | |
| o.setBrokerOrderId(id); | |
| o.setStatus(OrderStatus.ACCEPTED); | |
| } catch (Exception e) { | |
| // TODO: 실패 상태 도입 시 교체(e.g. FAILED/REJECTED) | |
| o.setStatus(OrderStatus.PENDING); | |
| // TODO: 로깅 추가 | |
| } | |
| return orders.save(o); | |
| } |
Comment on lines
+30
to
+39
| @Transactional | ||
| public Order placeSell(PlaceOrderRequest req){ | ||
| Order o = baseOf(req, OrderSide.SELL); | ||
| String id = switch (req.getOrderType()){ | ||
| case MARKET -> broker.placeMarketSell(req.getSymbol(), req.getQuantity()); | ||
| case LIMIT -> broker.placeLimitSell(req.getSymbol(), req.getQuantity(), req.getPrice()); | ||
| }; | ||
| o.setBrokerOrderId(id); o.setStatus(OrderStatus.ACCEPTED); | ||
| return orders.save(o); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
매도 로직도 동일한 예외 처리 및 트랜잭션 리스크
위 매수와 동일하게 try/catch 및 저장 경로 보장 필요.
적용 diff:
@Transactional
public Order placeSell(PlaceOrderRequest req){
Order o = baseOf(req, OrderSide.SELL);
- String id = switch (req.getOrderType()){
- case MARKET -> broker.placeMarketSell(req.getSymbol(), req.getQuantity());
- case LIMIT -> broker.placeLimitSell(req.getSymbol(), req.getQuantity(), req.getPrice());
- };
- o.setBrokerOrderId(id); o.setStatus(OrderStatus.ACCEPTED);
- return orders.save(o);
+ try {
+ String id = switch (req.getOrderType()){
+ case MARKET -> broker.placeMarketSell(req.getSymbol(), req.getQuantity());
+ case LIMIT -> broker.placeLimitSell(req.getSymbol(), req.getQuantity(), req.getPrice());
+ };
+ o.setBrokerOrderId(id);
+ o.setStatus(OrderStatus.ACCEPTED);
+ } catch (Exception e) {
+ // TODO: 실패 상태 도입 시 교체(e.g. FAILED/REJECTED)
+ o.setStatus(OrderStatus.PENDING);
+ // TODO: 로깅 추가
+ }
+ return orders.save(o);
}📝 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 Order placeSell(PlaceOrderRequest req){ | |
| Order o = baseOf(req, OrderSide.SELL); | |
| String id = switch (req.getOrderType()){ | |
| case MARKET -> broker.placeMarketSell(req.getSymbol(), req.getQuantity()); | |
| case LIMIT -> broker.placeLimitSell(req.getSymbol(), req.getQuantity(), req.getPrice()); | |
| }; | |
| o.setBrokerOrderId(id); o.setStatus(OrderStatus.ACCEPTED); | |
| return orders.save(o); | |
| } | |
| @Transactional | |
| public Order placeSell(PlaceOrderRequest req){ | |
| Order o = baseOf(req, OrderSide.SELL); | |
| try { | |
| String id = switch (req.getOrderType()){ | |
| case MARKET -> broker.placeMarketSell(req.getSymbol(), req.getQuantity()); | |
| case LIMIT -> broker.placeLimitSell(req.getSymbol(), req.getQuantity(), req.getPrice()); | |
| }; | |
| o.setBrokerOrderId(id); | |
| o.setStatus(OrderStatus.ACCEPTED); | |
| } catch (Exception e) { | |
| // TODO: 실패 상태 도입 시 교체(e.g. FAILED/REJECTED) | |
| o.setStatus(OrderStatus.PENDING); | |
| // TODO: 로깅 추가 | |
| } | |
| return orders.save(o); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/trademill/apiserver/order/OrderService.java around lines 30 to
39, the sell flow lacks the same try/catch and guaranteed persistence path as
the buy flow, so wrap the broker calls and subsequent state updates in a
try/catch that mirrors the buy logic: create and persist the initial Order
before external broker call if your buy flow does that, call broker inside try,
on success set brokerOrderId and ACCEPTED and save, on broker exception set
order status to REJECTED (or appropriate failure state), record the exception
details, save the order in the catch to guarantee persistence, and rethrow or
handle transaction boundaries as the buy path does to avoid leaving the system
in an inconsistent state.
Comment on lines
+45
to
+55
| private Order baseOf(PlaceOrderRequest req, OrderSide side){ | ||
| Order o = new Order(); | ||
| o.setUserId(req.getUserId()); | ||
| o.setSymbol(req.getSymbol()); | ||
| o.setSide(side); | ||
| o.setOrderType(req.getOrderType()); | ||
| o.setQuantity(req.getQuantity()); | ||
| o.setPrice(req.getPrice()); | ||
| o.setStatus(OrderStatus.PENDING); | ||
| return o; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
비즈니스 규칙 검증(수량/가격) 강화 — LIMIT/MARKET별 가격 유효성
서비스 레벨에서 최소 검증을 추가하여 잘못된 입력이 브로커까지 전달되지 않도록 하세요. 특히 LIMIT는 price 필수/양수, MARKET은 price 무시(null).
적용 diff:
private Order baseOf(PlaceOrderRequest req, OrderSide side){
Order o = new Order();
o.setUserId(req.getUserId());
o.setSymbol(req.getSymbol());
o.setSide(side);
o.setOrderType(req.getOrderType());
- o.setQuantity(req.getQuantity());
- o.setPrice(req.getPrice());
+ if (req.getQuantity() == null || req.getQuantity().signum() <= 0) {
+ throw new IllegalArgumentException("quantity must be positive");
+ }
+ o.setQuantity(req.getQuantity());
+ switch (req.getOrderType()) {
+ case MARKET -> o.setPrice(null);
+ case LIMIT -> {
+ if (req.getPrice() == null || req.getPrice().signum() <= 0) {
+ throw new IllegalArgumentException("price must be positive for LIMIT order");
+ }
+ o.setPrice(req.getPrice());
+ }
+ }
o.setStatus(OrderStatus.PENDING);
return o;
}참고: DTO(PlaceOrderRequest)에도 Bean Validation(@NotNull, @positive 등)으로 1차 방어를 추가하면 더 견고해집니다.
📝 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
| private Order baseOf(PlaceOrderRequest req, OrderSide side){ | |
| Order o = new Order(); | |
| o.setUserId(req.getUserId()); | |
| o.setSymbol(req.getSymbol()); | |
| o.setSide(side); | |
| o.setOrderType(req.getOrderType()); | |
| o.setQuantity(req.getQuantity()); | |
| o.setPrice(req.getPrice()); | |
| o.setStatus(OrderStatus.PENDING); | |
| return o; | |
| } | |
| private Order baseOf(PlaceOrderRequest req, OrderSide side){ | |
| Order o = new Order(); | |
| o.setUserId(req.getUserId()); | |
| o.setSymbol(req.getSymbol()); | |
| o.setSide(side); | |
| o.setOrderType(req.getOrderType()); | |
| - o.setQuantity(req.getQuantity()); | |
| if (req.getQuantity() == null || req.getQuantity().signum() <= 0) { | |
| throw new IllegalArgumentException("quantity must be positive"); | |
| } | |
| o.setQuantity(req.getQuantity()); | |
| switch (req.getOrderType()) { | |
| case MARKET -> o.setPrice(null); | |
| case LIMIT -> { | |
| if (req.getPrice() == null || req.getPrice().signum() <= 0) { | |
| throw new IllegalArgumentException("price must be positive for LIMIT order"); | |
| } | |
| o.setPrice(req.getPrice()); | |
| } | |
| } | |
| o.setStatus(OrderStatus.PENDING); | |
| return o; | |
| } |
🤖 Prompt for AI Agents
In src/main/java/trademill/apiserver/order/OrderService.java around lines 45 to
55, strengthen business-rule validation in baseOf: for LIMIT orders ensure
req.getPrice() is non-null and > 0 (throw IllegalArgumentException or a
domain-specific ValidationException if violated); for MARKET orders ignore any
supplied price and set Order.price to null; also validate req.getQuantity() is >
0 and fail fast on invalid values; keep setting status to PENDING. Optionally
add or complement DTO Bean Validation annotations (@NotNull, @Positive) on
PlaceOrderRequest as a first-line defense.
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
2 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.
작업 내용 (MVP)
로컬 실행
./gradlew bootRun --args='--spring.profiles.active=local'
확인 예시
curl -X POST http://localhost:8080/api/v1/orders/buy
-H "Content-Type: application/json"
-d '{"userId":1,"symbol":"005930","orderType":"MARKET","quantity":10}'
curl -X POST http://localhost:8080/api/v1/orders/sell
-H "Content-Type: application/json"
-d '{"userId":1,"symbol":"005930","orderType":"LIMIT","quantity":5,"price":70000}'
curl "http://localhost:8080/api/v1/orders?userId=1&page=0&size=20"
다음 단계
Summary by CodeRabbit
새 기능
작업