Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(ticketItemOption) : 옵션 적용하기 구조 리펙토링 #215

Merged
merged 6 commits into from
Jan 29, 2023
Merged

refactor(ticketItemOption) : 옵션 적용하기 구조 리펙토링 #215

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a4974a5
refactor(itemOptionGroup) : 티켓상품 내 옵션그룹 관리 도메인 메소드 생성
kim-wonjin Jan 28, 2023
65d9953
refactor(itemOptionGroup) : 요청DTO 내 id검증 어노테이션 사용
kim-wonjin Jan 28, 2023
2ffd0af
refactor(itemOptionGroup) : 옵션적용 로직 구조 리펙토링 및 검증 추가
kim-wonjin Jan 28, 2023
d41f876
Merge branch 'dev' into refactor/211-apply-option
kim-wonjin Jan 28, 2023
f9b3903
refactor(itemOptionGroup) : 에러코드 컨플릭트 해결
kim-wonjin Jan 28, 2023
110ecfa
refactor(itemOptionGroup) : 분산락, 도메인 내부로 메소드 이, 에러코드 수정
kim-wonjin Jan 29, 2023
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
6 changes: 3 additions & 3 deletions ...g-Api/src/main/java/band/gosrock/api/ticketItem/dto/request/ApplyTicketOptionRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,19 @@


import io.swagger.v3.oas.annotations.media.Schema;
import javax.validation.constraints.NotNull;
import javax.validation.constraints.Positive;
import lombok.Getter;
import lombok.RequiredArgsConstructor;

@Getter
@RequiredArgsConstructor
public class ApplyTicketOptionRequest {

@NotNull
@Schema(nullable = false, example = "1")
@Positive(message = "올바른 티켓상품 고유 아이디를 입력해주세요")
private Long ticketItemId;

@NotNull
@Schema(nullable = false, example = "1")
@Positive(message = "올바른 옵션그룹 고유 아이디를 입력해주세요")
private Long optionGroupId;
}
2 changes: 1 addition & 1 deletion ...ng-Api/src/main/java/band/gosrock/api/ticketItem/dto/request/CreateTicketItemRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public class CreateTicketItemRequest {
private Long eventId;

@NotNull
@Schema(nullable = false, defaultValue = "FIRST_COME_FIRST_SERVED")
@Schema(nullable = false, defaultValue = "선착순")
private TicketType type;

@NotNull
Expand Down
16 changes: 7 additions & 9 deletions ...Api/src/main/java/band/gosrock/api/ticketItem/dto/response/ApplyTicketOptionResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,28 +1,26 @@
package band.gosrock.api.ticketItem.dto.response;


import band.gosrock.domain.domains.ticket_item.domain.ItemOptionGroup;
import band.gosrock.domain.domains.ticket_item.domain.TicketItem;
import io.swagger.v3.oas.annotations.media.Schema;
import java.util.List;
import lombok.Builder;
import lombok.Getter;

@Getter
@Builder
public class ApplyTicketOptionResponse {
@Schema(description = "상품-옵션그룹 id")
private final Long itemOptionGroupId;

@Schema(description = "티켓상품 id")
private final Long ticketItemId;

@Schema(description = "옵션그룹 id")
private final Long optionGroupId;
@Schema(description = "옵션그룹 id 리스트")
private final List<Long> optionGroupIds;
Comment on lines -13 to +18
Copy link
Member

Choose a reason for hiding this comment

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

내려주는 정보에서
id 정보보다는 티켓 정보랑
옵션 정보 뿌려주는게 프론트에서 편할것같긴해유


public static ApplyTicketOptionResponse from(ItemOptionGroup itemOptionGroup) {
public static ApplyTicketOptionResponse from(TicketItem ticketItem) {
return ApplyTicketOptionResponse.builder()
.itemOptionGroupId(itemOptionGroup.getId())
.ticketItemId(itemOptionGroup.getItem().getId())
.optionGroupId(itemOptionGroup.getOptionGroup().getId())
.ticketItemId(ticketItem.getId())
.optionGroupIds(ticketItem.getOptionGroupIds())
.build();
}
}
6 changes: 0 additions & 6 deletions DuDoong-Api/src/main/java/band/gosrock/api/ticketItem/mapper/TicketOptionMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@
import band.gosrock.api.ticketItem.dto.request.CreateTicketOptionRequest;
import band.gosrock.common.annotation.Mapper;
import band.gosrock.domain.domains.event.domain.Event;
import band.gosrock.domain.domains.ticket_item.domain.ItemOptionGroup;
import band.gosrock.domain.domains.ticket_item.domain.OptionGroup;
import band.gosrock.domain.domains.ticket_item.domain.TicketItem;
import java.util.ArrayList;
import lombok.RequiredArgsConstructor;

Expand All @@ -25,8 +23,4 @@ public OptionGroup toOptionGroup(
.options(new ArrayList<>())
.build();
}

public ItemOptionGroup toItemOptionGroup(TicketItem ticketItem, OptionGroup optionGroup) {
return ItemOptionGroup.builder().item(ticketItem).optionGroup(optionGroup).build();
}
}
29 changes: 8 additions & 21 deletions DuDoong-Api/src/main/java/band/gosrock/api/ticketItem/service/ApplyTicketOptionUseCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,12 @@
import band.gosrock.api.common.UserUtils;
import band.gosrock.api.ticketItem.dto.request.ApplyTicketOptionRequest;
import band.gosrock.api.ticketItem.dto.response.ApplyTicketOptionResponse;
import band.gosrock.api.ticketItem.mapper.TicketOptionMapper;
import band.gosrock.common.annotation.UseCase;
import band.gosrock.domain.domains.event.adaptor.EventAdaptor;
import band.gosrock.domain.domains.event.domain.Event;
import band.gosrock.domain.domains.host.adaptor.HostAdaptor;
import band.gosrock.domain.domains.host.domain.Host;
import band.gosrock.domain.domains.ticket_item.adaptor.OptionAdaptor;
import band.gosrock.domain.domains.ticket_item.adaptor.TicketItemAdaptor;
import band.gosrock.domain.domains.ticket_item.domain.ItemOptionGroup;
import band.gosrock.domain.domains.ticket_item.domain.OptionGroup;
import band.gosrock.domain.domains.host.service.HostService;
import band.gosrock.domain.domains.ticket_item.domain.TicketItem;
import band.gosrock.domain.domains.ticket_item.service.ItemOptionGroupService;
import band.gosrock.domain.domains.user.domain.User;
Expand All @@ -26,31 +22,22 @@ public class ApplyTicketOptionUseCase {
private final UserUtils userUtils;
private final EventAdaptor eventAdaptor;
private final HostAdaptor hostAdaptor;
private final TicketItemAdaptor ticketItemAdaptor;
private final OptionAdaptor optionAdaptor;
private final ItemOptionGroupService itemOptionGroupService;
private final TicketOptionMapper ticketOptionMapper;
private final HostService hostService;

public ApplyTicketOptionResponse execute(
ApplyTicketOptionRequest applyTicketOptionRequest, Long eventId) {
User user = userUtils.getCurrentUser();
Event event = eventAdaptor.findById(eventId);
Host host = hostAdaptor.findById(event.getHostId());
// 권한 체크 ( 해당 이벤트의 호스트인지 )
host.hasHostUserId(user.getId());
hostService.validateHostUser(host, user.getId());

TicketItem ticketItem =
ticketItemAdaptor.queryTicketItem(applyTicketOptionRequest.getTicketItemId());
OptionGroup optionGroup =
optionAdaptor.queryOptionGroup(applyTicketOptionRequest.getOptionGroupId());
// 티켓상품과 옵션이 해당 이벤트 소속인지 확인
itemOptionGroupService.checkEventId(eventId, ticketItem, optionGroup);
// 이미 적용된 옵션인지 확인
itemOptionGroupService.checkApply(ticketItem, optionGroup);
Long ticketItemId = applyTicketOptionRequest.getTicketItemId();
Long optionGroupId = applyTicketOptionRequest.getOptionGroupId();

ItemOptionGroup itemOptionGroup =
itemOptionGroupService.createItemOptionGroup(
ticketOptionMapper.toItemOptionGroup(ticketItem, optionGroup));
return ApplyTicketOptionResponse.from(itemOptionGroup);
TicketItem ticketItem =
itemOptionGroupService.createItemOptionGroup(ticketItemId, optionGroupId, eventId);
Copy link
Member

Choose a reason for hiding this comment

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

addItemOptionGroup 이 더 적절한 네이밍일듯!

Copy link
Member

Choose a reason for hiding this comment

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

eventId 는
티켓 아이템이랑 옵션그룹 엔티티에서 둘다 조회가능한 정보이니
인자로 받지말고 도메인 서비스 내부에서
티켓 아이템의 이벤트랑 옵션그룹의 이벤트가 같은지 검증하면 될것같긴하네유!
근데뭐.. 이건 자율! 의견입니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

세개가 모두 같은지 확인하는게 좋을거 같아서 요렇게 했숨니당

return ApplyTicketOptionResponse.from(ticketItem);
}
}
23 changes: 0 additions & 23 deletions ...src/main/java/band/gosrock/domain/domains/ticket_item/adaptor/ItemOptionGroupAdaptor.java
View file
Open in desktop

This file was deleted.

9 changes: 8 additions & 1 deletion DuDoong-Domain/src/main/java/band/gosrock/domain/domains/ticket_item/domain/OptionGroup.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import band.gosrock.domain.common.vo.Money;
import band.gosrock.domain.domains.event.domain.Event;
import band.gosrock.domain.domains.ticket_item.exception.InvalidOptionGroupException;
import java.util.ArrayList;
import java.util.List;
import javax.persistence.*;
Expand Down Expand Up @@ -43,7 +44,7 @@ public class OptionGroup {
private Boolean isEssential;

@OneToMany(cascade = CascadeType.ALL, mappedBy = "optionGroup")
private List<Option> options = new ArrayList<>();
private final List<Option> options = new ArrayList<>();

@Builder
public OptionGroup(
Expand All @@ -63,6 +64,12 @@ public OptionGroup(
options.forEach(option -> option.setOptionGroup(this));
}

public void checkEventId(Long eventId) {
if (!this.getEvent().getId().equals(eventId)) {
throw InvalidOptionGroupException.EXCEPTION;
}
}

public OptionGroup createTicketOption(Money additionalPrice) {
OptionGroupType type = this.getType();
if (type == TRUE_FALSE) {
Expand Down
37 changes: 27 additions & 10 deletions DuDoong-Domain/src/main/java/band/gosrock/domain/domains/ticket_item/domain/TicketItem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@
import band.gosrock.domain.common.vo.Money;
import band.gosrock.domain.common.vo.RefundInfoVo;
import band.gosrock.domain.domains.event.domain.Event;
import band.gosrock.domain.domains.ticket_item.exception.InvalidTicketItemException;
import band.gosrock.domain.domains.ticket_item.exception.TicketItemQuantityException;
import band.gosrock.domain.domains.ticket_item.exception.TicketItemQuantityLackException;
import band.gosrock.domain.domains.ticket_item.exception.TicketItemQuantityLargeException;
import java.time.LocalDateTime;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import javax.persistence.*;
import lombok.AccessLevel;
import lombok.Builder;
Expand Down Expand Up @@ -62,8 +64,8 @@ public class TicketItem extends BaseTimeEntity {
@JoinColumn(name = "event_id", nullable = false)
private Event event;

@OneToMany(mappedBy = "item")
private List<ItemOptionGroup> itemOptionGroups = new ArrayList<>();
@OneToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER)
private final Set<ItemOptionGroup> itemOptionGroups = new HashSet<>();

@Builder
public TicketItem(
Expand All @@ -77,8 +79,7 @@ public TicketItem(
Boolean isSellable,
LocalDateTime saleStartAt,
LocalDateTime saleEndAt,
Event event,
List<ItemOptionGroup> itemOptionGroups) {
Event event) {
this.type = type;
this.name = name;
this.description = description;
Expand All @@ -90,7 +91,25 @@ public TicketItem(
this.saleStartAt = saleStartAt;
this.saleEndAt = saleEndAt;
this.event = event;
this.itemOptionGroups = itemOptionGroups;
}

public void addItemOptionGroup(OptionGroup optionGroup) {
ItemOptionGroup itemOptionGroup =
ItemOptionGroup.builder().item(this).optionGroup(optionGroup).build();
this.itemOptionGroups.add(itemOptionGroup);
}

public Boolean hasItemOptionGroup(Long optionGroupId) {
return this.itemOptionGroups.stream()
.anyMatch(
itemOptionGroup ->
itemOptionGroup.getOptionGroup().getId().equals(optionGroupId));
}

public void checkEventId(Long eventId) {
if (!this.getEvent().getId().equals(eventId)) {
throw InvalidTicketItemException.EXCEPTION;
}
}

public RefundInfoVo getRefundInfoVo() {
Expand All @@ -113,10 +132,8 @@ public List<Long> getOptionGroupIds() {
.toList();
}

public void addOptionGroup(OptionGroup optionGroup) {
ItemOptionGroup itemOptionGroup =
ItemOptionGroup.builder().item(this).optionGroup(optionGroup).build();
this.itemOptionGroups.add(itemOptionGroup);
public Boolean isQuantityReduced() {
return !this.quantity.equals(this.supplyCount);
}

public void reduceQuantity(Long quantity) {
Expand Down
13 changes: 13 additions & 0 deletions ...ava/band/gosrock/domain/domains/ticket_item/exception/ForbiddenOptionChangeException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package band.gosrock.domain.domains.ticket_item.exception;


import band.gosrock.common.exception.DuDoongCodeException;

public class ForbiddenOptionChangeException extends DuDoongCodeException {

public static final DuDoongCodeException EXCEPTION = new ForbiddenOptionChangeException();

private ForbiddenOptionChangeException() {
super(TicketItemErrorCode.FORBIDDEN_OPTION_CHANGE);
}
}
7 changes: 3 additions & 4 deletions .../src/main/java/band/gosrock/domain/domains/ticket_item/exception/TicketItemErrorCode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package band.gosrock.domain.domains.ticket_item.exception;

import static band.gosrock.common.consts.DuDoongStatic.BAD_REQUEST;
import static band.gosrock.common.consts.DuDoongStatic.NOT_FOUND;
import static band.gosrock.common.consts.DuDoongStatic.*;

import band.gosrock.common.annotation.ExplainError;
import band.gosrock.common.dto.ErrorReason;
Expand Down Expand Up @@ -37,8 +36,8 @@ public enum TicketItemErrorCode implements BaseErrorCode {
@ExplainError("해당 티켓상품에 이미 적용된 옵션일 경우 발생하는 오류입니다.")
DUPLICATED_ITEM_OPTION_GROUP(BAD_REQUEST, "Item_Option_Group_400_1", "이미 적용된 옵션입니다."),
OPTION_ANSWER_NOT_CORRECT(
BAD_REQUEST, "Option_400_1", "옵션에 대한 답변이 올바르지 않습니다. T/F형일 경우 예 아니요 로 보내주세요.");

BAD_REQUEST, "Option_400_1", "옵션에 대한 답변이 올바르지 않습니다. T/F형일 경우 예 아니요 로 보내주세요."),
FORBIDDEN_OPTION_CHANGE(FORBIDDEN, "Item_Option_Group_403_1", "옵션 변경이 불가능한 상태입니다.");
Copy link
Member

Choose a reason for hiding this comment

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

FORBIDDEN 말고 400번대로 고쳐주세요
403 으로 응답주면 클라이언트에서 자동으로 로그아웃 됩니다!

private Integer status;
private String code;
private String reason;
Expand Down
10 changes: 0 additions & 10 deletions ...in/java/band/gosrock/domain/domains/ticket_item/repository/ItemOptionGroupRepository.java
View file
Open in desktop

This file was deleted.

45 changes: 25 additions & 20 deletions ...src/main/java/band/gosrock/domain/domains/ticket_item/service/ItemOptionGroupService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@


import band.gosrock.common.annotation.DomainService;
import band.gosrock.domain.domains.ticket_item.adaptor.ItemOptionGroupAdaptor;
import band.gosrock.domain.domains.ticket_item.domain.ItemOptionGroup;
import band.gosrock.domain.domains.ticket_item.adaptor.OptionAdaptor;
import band.gosrock.domain.domains.ticket_item.adaptor.TicketItemAdaptor;
import band.gosrock.domain.domains.ticket_item.domain.OptionGroup;
import band.gosrock.domain.domains.ticket_item.domain.TicketItem;
import band.gosrock.domain.domains.ticket_item.exception.DuplicatedItemOptionGroupException;
import band.gosrock.domain.domains.ticket_item.exception.InvalidOptionGroupException;
import band.gosrock.domain.domains.ticket_item.exception.InvalidTicketItemException;
import band.gosrock.domain.domains.ticket_item.exception.ForbiddenOptionChangeException;
import band.gosrock.domain.domains.ticket_item.repository.TicketItemRepository;
import lombok.RequiredArgsConstructor;
import org.springframework.transaction.annotation.Transactional;

Expand All @@ -17,26 +17,31 @@
@RequiredArgsConstructor
public class ItemOptionGroupService {

private final ItemOptionGroupAdaptor itemOptionGroupAdaptor;
private final TicketItemAdaptor ticketItemAdaptor;
private final OptionAdaptor optionAdaptor;
private final TicketItemRepository ticketItemRepository;

public void checkEventId(Long eventId, TicketItem ticketItem, OptionGroup optionGroup) {
if (!ticketItem.getEvent().getId().equals(eventId)) {
throw InvalidTicketItemException.EXCEPTION;
}
if (!optionGroup.getEvent().getId().equals(eventId)) {
throw InvalidOptionGroupException.EXCEPTION;
// @RedissonLock(LockName = "티켓 옵션적용", identifier = "ticketItemId")
@Transactional
Copy link
Member

Choose a reason for hiding this comment

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

여기에도 락을 걸긴해야해요
재고 감소

https://github.com/Gosrock/DuDoong-Backend/blob/refactor/112-order-validation-and-docs/DuDoong-Domain/src/main/java/band/gosrock/domain/domains/issuedTicket/service/IssuedTicketDomainService.java

@RedissonLock(LockName = "티켓재고관리", identifier = "itemId")
@Transactional
public void withDrawIssuedTicket(Long itemId, List<IssuedTicket> issuedTickets) {

위처럼요! 현재는 ticketItemId 가 identifier 이구
분산락 안에 트랜잭션 존재하니 트랜잭션 어노테이션은 빼도됩니당!

public TicketItem createItemOptionGroup(Long ticketItemId, Long optionGroupId, Long eventId) {

TicketItem ticketItem = ticketItemAdaptor.queryTicketItem(ticketItemId);
OptionGroup optionGroup = optionAdaptor.queryOptionGroup(optionGroupId);

// 해당 eventId에 속해 있는 티켓 아이템, 옵션그룹이 맞는지 확인
ticketItem.checkEventId(eventId);
optionGroup.checkEventId(eventId);

// 재고 감소된 티켓상품은 옵션적용 변경 불가
if (ticketItem.isQuantityReduced()) {
throw ForbiddenOptionChangeException.EXCEPTION;
}
Copy link
Member

Choose a reason for hiding this comment

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

티켓 아이템 도메인 내부로 이동해도 될것같습니다~!

}

public void checkApply(TicketItem ticketItem, OptionGroup optionGroup) {
if (itemOptionGroupAdaptor.existsQueryItemOptionGroup(
ticketItem.getId(), optionGroup.getId())) {
// 중복 체크
if (ticketItem.hasItemOptionGroup(optionGroup.getId())) {
throw DuplicatedItemOptionGroupException.EXCEPTION;
}
Copy link
Member

Choose a reason for hiding this comment

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

여기두요!

Copy link
Member Author

Choose a reason for hiding this comment

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

위에꺼랑 마찬가지로 주문내역이 있는지, 적용된 옵션인지 확인할일이 또 있을까봐 (무조건 쳐내지않고 확인) exception은 밖으로 뺀건데 그럴일 없을라나요?

Copy link
Member

Choose a reason for hiding this comment

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

hasItemOptionGroup 메서드 그대로 두시고
위 세줄을 또 메서드로 파심됩니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

오옹 exception은 도메인 내부 메소드에서 던지는게 더 좋나요?

Copy link
Member

Choose a reason for hiding this comment

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

그쵸
지금 우리가 코딩하니깐 그런거지
ItemOptionGroupService 가
지금소스보면 ticketItem.hasItemOptionGroup 메서드로
체크를 하잖아유

그말인즉슨 ItemOptionGroupService 가 ticketItem의 내부 정보까지 알아야 코딩이 가능한건데
ticketItem.validHasItemOptionGroup 메서드나

ticketItem.addOptionGroup() 메서드를 실행시키면
자동으로 재고 감소 되어있는지를 검증해준다거나

둘중하나의 방식으로 캡슐화를 잘 지켜야 된다고 봅니다
저는 후자의방식 추천드릴게유 그게 더 안전하잖아요?
addOptionGroup 전에 항상 validHasItemOptionGroup 를 호출해야한다라는걸
누구나 빼먹을수 있기때문에

}

@Transactional
public ItemOptionGroup createItemOptionGroup(ItemOptionGroup itemOptionGroup) {
return itemOptionGroupAdaptor.save(itemOptionGroup);
ticketItem.addItemOptionGroup(optionGroup);
return ticketItemRepository.save(ticketItem);
}
}