Skip to content

Conversation

@danaggero
Copy link

동시성 & 결제 연동 미션

README 부족한 부분 추후에 보완하겠습니다.

Copy link

@ba2slk ba2slk left a comment

Choose a reason for hiding this comment

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

고생 많으셨습니다!

import org.springframework.cloud.openfeign.EnableFeignClients;
import org.springframework.data.jpa.repository.config.EnableJpaAuditing;

@EnableFeignClients(basePackages = "com.ceos22.springcgv.external.payment")
Copy link

Choose a reason for hiding this comment

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

저도 Feign 사용했어요! Spring Application 클래스 대신 OpenFeignConfig 설정 클래스를 별도로 만들어서 거기에 @EnableFeignClients를 달아주면 나중에 테스트할 때 더 유연하다는 내용을 본 것 같아요. 외부 API 호출이 필요 없는 코드를 테스트할 때에도 모든 Feign 클라이언트들을 로드하지 않기 때문에, 테스트가 무거워지는 상황을 피할 수 있다고 합니다!

Copy link
Author

Choose a reason for hiding this comment

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

혹시 스프링부트 버전 내리셨나요? 저는 기존 버전 3.5.5에서는 안 돌아가더라구요..

@Table(name = "concession_items")
public class ConcessionItem extends BaseEntity {
@Table(name = "snack_items")
public class SnackItem extends BaseEntity {
Copy link

Choose a reason for hiding this comment

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

Concession -> Snack 으로 변수명을 변경하신 이유가 궁금합니다!

Copy link
Author

Choose a reason for hiding this comment

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

그냥 최대한 직관적인 이름이 좋을거같아서.. 바꿨습니다..


@Getter
public enum ErrorCode {

Copy link

Choose a reason for hiding this comment

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

Error가 상세하게 분류되어 있어서 코드 읽으면서 많이 배웠습니다!

.customData("{\"cinema\":\"" + cinema.getName() + "\"}")
.build();

PaymentResponseDto paymentResponse = paymentClient.requestPayment(paymentId, paymentRequest);
Copy link

Choose a reason for hiding this comment

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

저도 하나의 트랜잭션 내에서 클라이언트가 결제 서버에 요청을 보내도록 코드를 짰었는데, 지금 생각해보니 성능에 문제가 있을 것 같습니다. DB가 락을 잡은 상태로 외부 API를 호출하면 실행 시간이 외부 API에 종속되는 문제가 생길 것 같아요. 그래서 1. (트랜잭션 O) 재고 확인 및 차감 2. (트랜잭션 x) 결제 요청 3. (트랜잭션 O) 결제 결과에 따른 DB 조작 (결제 상태 변경 또는 롤백) -> 이런 식으로 트랜잭션을 분리해야 병목이 안 생길 것 같습니다. 저도 바꿔보겠습니다..!

Copy link

@hayong39 hayong39 left a comment

Choose a reason for hiding this comment

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

주석을 잘 달아주셔서 코드 리뷰하기 수월했네요!! 수고하셨습니다 👍

Comment on lines +47 to +50
if (userDetails == null) {
return ResponseEntity.status(401)
.body(ApiResponse.failure(401, "인증되지 않은 사용자입니다."));
}

Choose a reason for hiding this comment

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

해당 경로를 authenticated()로 security config에서 설정하면 컨트롤러에 이 로직이 없어도 되지 않을까요? 추가하신 이유가 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

중복된 로직인 것 같습니다..! 감사합니다

Comment on lines +33 to +37
/**
* 매점 상품 구매 취소
* @param 상품 구매 ID
* @return
*/

Choose a reason for hiding this comment

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

주석으로 남기는 것도 좋지만 swagger annotation을 써보는 것도 좋을 것 같아요!! 그러면 Swagger UI에도 표시되서 나중에 확인할때 편하더라구요

Copy link
Author

Choose a reason for hiding this comment

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

넵.. 귀찮아서 안하고 있었는데 적용해보겠습니다..!

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.

3 participants