Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@
@Transactional
public class ShopServiceImpl implements ShopService {

private static final String DISCOUNT_MATCHING_TICKET_CODE = "DISCOUNT_MATCHING_TICKET_1";

private static final Map<ItemType, Integer> PURCHASE_LIMITS = Map.of(
ItemType.MATCHING_TICKET, 30,
ItemType.OPTION_TICKET, 90
Expand All @@ -61,7 +63,8 @@ public List<ProductResponse> getActiveProducts(Long memberId, Boolean isBundle)
LocalDateTime now = LocalDateTime.now();
return products.stream()
.map(product -> toMemberProductResponse(memberId, product, now))
.filter(product -> Boolean.TRUE.equals(product.purchaseCountPurchasable()))
.filter(product -> Boolean.TRUE.equals(product.purchaseCountPurchasable())
|| DISCOUNT_MATCHING_TICKET_CODE.equals(product.code()))
.toList();
}

Expand All @@ -75,6 +78,10 @@ public void requestPurchase(Long memberId, Long productId, int quantity) {
throw new BusinessException(ItemErrorCode.PRODUCT_NOT_AVAILABLE);
}

if (DISCOUNT_MATCHING_TICKET_CODE.equals(resolveProductCode(product)) && quantity != 1) {
throw new BusinessException(PaymentErrorCode.INVALID_ORDER_QUANTITY);
}

Comment on lines 79 to +84

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

OCP 위반: 특정 상품 코드가 서비스 레이어에 하드코딩됨

DISCOUNT_MATCHING_TICKET_1 이라는 특정 상품 코드를 문자열 비교로 두 군데에 특례 처리하고 있습니다.

  • L39-L41 — 상수 선언
  • L65-L68 — 목록 노출 필터 (purchaseCountPurchasable() || DISCOUNT_MATCHING_TICKET_CODE.equals(product.code()))
  • 여기(L79-L84) — 요청 수량 검증 (DISCOUNT_MATCHING_TICKET_CODE.equals(...) && quantity != 1)

왜 문제인가: 이 서비스는 이미 "상품별 한정 판매 규칙"을 일반화해서 모델링하는 패턴이 있습니다 — Product 엔티티의 firstPurchaseOnly 플래그와, 그걸 소비하는 validatePurchaseCountLimit 의 제네릭 분기가 그 예입니다. 이번 변경은 그 패턴을 재사용하지 않고 특정 product code 문자열을 서비스 레이어에 직접 박아 넣는 새로운 특례를 추가했습니다. docs/code-review-guidelines.md 의 OCP 기준("조건문으로 분기되는 로직이 전략 패턴 등으로 분리 가능한지 검토합니다")에 해당하는 사례로, 다음에 "소진돼도 계속 노출", "1회 수량 제한" 같은 규칙을 가진 상품이 추가될 때마다 이 두 지점에 if (CODE.equals(...)) 분기가 계속 늘어나는 구조입니다.

개선 방향: Product 엔티티에 alwaysListedWhenExhausted(또는 유사한 이름) / maxQuantityPerOrder 같은 필드를 추가해 상품 데이터로 표현하고, ShopServiceImpl 은 특정 코드 문자열이 아니라 그 필드를 제네릭하게 검사하도록 바꾸는 것을 제안합니다. 필드 추가와 초기화 데이터(ShopDataInitializer) 변경이 함께 필요해 커밋 가능한 제안(suggestion block)으로 바로 반영하기는 어려워 설명으로만 남깁니다.

LocalDateTime now = LocalDateTime.now();
boolean hasPendingRequest = orderRepository.existsActivePendingOrder(memberId, now);
if (hasPendingRequest) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,196 @@
package com.comatching.item.global.init;

import java.util.List;

import org.springframework.boot.CommandLineRunner;
import org.springframework.stereotype.Component;
import org.springframework.transaction.annotation.Transactional;

import com.comatching.item.domain.roulette.entity.RouletteReward;
import com.comatching.item.domain.roulette.enums.RewardType;
import com.comatching.item.domain.roulette.enums.RouletteType;
import com.comatching.item.domain.roulette.repository.RouletteRewardRepository;

import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;

@Slf4j
@Component
@RequiredArgsConstructor
public class RouletteRewardDataInitializer implements CommandLineRunner {

private final RouletteRewardRepository rouletteRewardRepository;

@Override
@Transactional
public void run(String... args) throws Exception {
if (rouletteRewardRepository.count() > 0) {
log.info("[RouletteRewardDataInitializer] 이미 룰렛 보상 데이터가 존재하여 초기화를 건너뜁니다.");
return;
}

log.info("[RouletteRewardDataInitializer] 초기 룰렛 보상 데이터를 생성합니다...");

List<RouletteReward> rewards = List.of(

// FREE Roulette
reward(
RouletteType.FREE,
"옵션권 1장",
RewardType.OPTION_TICKET,
1,
1,
4500,
null
),

reward(
RouletteType.FREE,
"옵션권 2장",
RewardType.OPTION_TICKET,
2,
4501,
7000,
null
),

reward(
RouletteType.FREE,
"꽝",
RewardType.NONE,
0,
7001,
8500,
null
),

reward(
RouletteType.FREE,
"뽑기권 1장",
RewardType.MATCHING_TICKET,
1,
8501,
9700,
null
),

reward(
RouletteType.FREE,
"풀세트",
RewardType.FULL_SET,
0,
9701,
10000,
null
),

// SPECIAL Roulette
reward(
RouletteType.SPECIAL,
"옵션권 2장",
RewardType.OPTION_TICKET,
2,
1,
3900,
null
),

reward(
RouletteType.SPECIAL,
"옵션권 5장",
RewardType.OPTION_TICKET,
5,
3901,
6400,
null
),

reward(
RouletteType.SPECIAL,
"뽑기권 1장",
RewardType.MATCHING_TICKET,
1,
6401,
8400,
null
),

reward(
RouletteType.SPECIAL,
"풀세트",
RewardType.FULL_SET,
0,
8401,
9400,
null
),

reward(
RouletteType.SPECIAL,
"뽑기권 5장",
RewardType.MATCHING_TICKET,
5,
9401,
9600,
null
),

reward(
RouletteType.SPECIAL,
"뽑기권 10장",
RewardType.MATCHING_TICKET,
10,
9601,
9750,
null
),

reward(
RouletteType.SPECIAL,
"1만원권 상품권",
RewardType.GIFT_CARD,
0,
9751,
9900,
999
),

reward(
RouletteType.SPECIAL,
"2만원권 상품권",
RewardType.GIFT_CARD,
0,
9901,
10000,
999
)
);

rouletteRewardRepository.saveAll(rewards);

log.info(
"[RouletteRewardDataInitializer] 룰렛 보상 {}개 생성 완료.",
rewards.size()
);
}

private RouletteReward reward(
RouletteType rouletteType,
String rewardName,
RewardType rewardType,
int quantity,
int rangeStart,
int rangeEnd,
Integer remainingCount
) {
return RouletteReward.builder()
.rouletteType(rouletteType)
.rewardName(rewardName)
.rewardType(rewardType)
.quantity(quantity)
.rangeStart(rangeStart)
.rangeEnd(rangeEnd)
.remainingCount(remainingCount)
.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,25 @@ public void run(String... args) throws Exception {
addReward(firstPurchaseBundle, ItemType.MATCHING_TICKET, 3);
addReward(firstPurchaseBundle, ItemType.OPTION_TICKET, 6);

// 2026/9/14 pm 요구에 따른 추가
Product discountMatchingTicket = product(
"(할인) 뽑기권 1개",
"DISCOUNT_MATCHING_TICKET_1",
"",
800,
2,
true,
3,
false
);
addReward(discountMatchingTicket, ItemType.MATCHING_TICKET, 1);

Product miniBundle = product(
"미니 번들",
"MINI_BUNDLE",
"",
500,
2,
3,
true,
2,
false
Expand All @@ -61,7 +74,7 @@ public void run(String... args) throws Exception {
"VALUE_BUNDLE",
"",
5500,
3,
4,
true,
2,
false
Expand All @@ -74,7 +87,7 @@ public void run(String... args) throws Exception {
"FULL_OPTION_BUNDLE",
"",
7000,
4,
5,
true,
1,
false
Expand All @@ -87,7 +100,7 @@ public void run(String... args) throws Exception {
"SUPER_BUNDLE",
"",
9500,
5,
6,
true,
1,
false
Expand All @@ -100,7 +113,7 @@ public void run(String... args) throws Exception {
"HYPER_BUNDLE",
"",
18000,
6,
7,
true,
2,
false
Expand All @@ -113,7 +126,7 @@ public void run(String... args) throws Exception {
"MATCHING_TICKET_1",
"",
1000,
7,
8,
false,
null,
false
Expand All @@ -125,7 +138,7 @@ public void run(String... args) throws Exception {
"OPTION_TICKET_1",
"",
200,
8,
9,
false,
null,
false
Expand All @@ -134,6 +147,7 @@ public void run(String... args) throws Exception {

List<Product> products = List.of(
firstPurchaseBundle,
discountMatchingTicket,
miniBundle,
valueBundle,
fullOptionBundle,
Expand Down
Loading
Loading