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.

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

[BE] 7.07 거래 체결 기능 구현 (WebSocket) #53 #98

Merged
merged 22 commits into from
Nov 14, 2024

Conversation

sieunie
Copy link
Collaborator

@sieunie sieunie commented Nov 13, 2024

✅ 주요 작업

  • 현재 체결가 웹소켓 구독 등록 및 취소 로직 구현
  • 체결 가능한 주문인지 확인하는 로직 구현
  • 주문 체결 로직 구현 (테스트는 못해봤습니다. 내일 테스트 해보고 머지할게용)
  • 회원가입 시 asset 테이블에도 row 추가되도록 트랜잭션 구현
  • 린트 규칙 수정

💭 고민과 해결과정

  • stockOrderModule과 socketModule이 서로 의존하고 있어서 circular dependency 이슈가 발생했는데, forwardRef를 사용해서 해결하였습니다.
  • 주문이 등록되면 db에 주문 등록하면서 해당 종목 구독 시작하도록 로직 변경하였습니다. 추가로 주문 취소나 체결된 경우 db에 해당 종목에 관한 주문 존재하는지 확인하고 구독 취소하도록 구현했습니다.
  • circular dependency 때문에 린트 오류 나는데 지금 당장 해결 못할 것 같아서 일단 린트 규칙 껐습니다...ㅠㅠ 🥹

체결 로직 트랜잭션 구현
체결 로직 트랜잭션 구현할 때 @Transactional 하면 되는 줄 알았는데 데코레이터 방식은 권장되지 않는다고 합니다....
그래서 repository layer에서 private으로 dataSource 주입받은 후 queryRunner로 트랜잭션 구현했습니다.
추후에 커스텀 데코레이터도 만들어볼 수 있을 것 같습니당.

아래처럼 트랜잭션 구현했습니다.

// repository layer

async updateOrderAndAssetWhenBuy(order, realPrice) {
    const queryRunner = this.dataSource.createQueryRunner();
    await queryRunner.startTransaction();

    try {
      // DB 로직 수행
      await queryRunner.commitTransaction();
    } catch (err) {
      await queryRunner.rollbackTransaction();
      throw new InternalServerErrorException();
    } finally {
      await queryRunner.release();
    }
}

Circular Dependency
이거 관련해서는 일단 위에서 말했듯이 forwardRef로 임시 처치는 해놨는데... 아마 stockPriceSocketService 같은 각각 소켓 서비스들을 SocketModule에서 관리하면 안될 것 같고 각 도메인 모듈에서 관리해야할 것 같아요
지금 stockPriceSocketSerivice랑 stockOrderSerivice가 서로 의존하고 있어서 둘을 같은 모듈 내에 집어넣어야 할 것 같아요.
꼬일까봐 수정은 아직 안했습니다.
제 생각입니당...

@sieunie sieunie added BE 백엔드 SOCKET Socket 구현 labels Nov 13, 2024
@sieunie sieunie self-assigned this Nov 13, 2024
@sieunie sieunie linked an issue Nov 13, 2024 that may be closed by this pull request
@sieunie sieunie requested review from jinddings and uuuo3o November 13, 2024 09:34
@uuuo3o
Copy link
Collaborator

uuuo3o commented Nov 14, 2024

추후에 커스텀 데코레이터도 만들어볼 수 있을 것 같습니당.

🟢 Transactional 데코레이터가 권장사항이 아니라면, 시은님께서 말씀해주신 방식으로 하면 좋을 것 같아요ㅎㅎ! 저도 한번 알아보겠습니다.

@@ -18,6 +18,13 @@ export class StockPriceSocketService {
@Inject(forwardRef(() => StockOrderService))
private readonly stockOrderService: StockOrderService,
) {
baseSocketService.registerSocketOpenHandler(async () => {
const orders = await stockOrderService.findAllPendingOrder();
Copy link
Collaborator

Choose a reason for hiding this comment

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

🟢 데이터베이스에서 대기 상태인 거래 내역을 가져와서 소켓에 하나씩 등록하는 방식이군요ㅎㅎ
좋은 것 같습니다.

@Column({ nullable: false, default: INIT_ASSET })
cash_balance: number;

@Column({ nullable: false, default: INIT_ASSET })
Copy link
Collaborator

Choose a reason for hiding this comment

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

🟢 주식을 매수하지 않은 초기 상태에서는 주식 평가 금액이 0원 아닌가요?! 잘 생각이 나지 않네요.... 한번 찾아보고 다시 말씀드릴게요 ㅠㅠ

Copy link
Collaborator

Choose a reason for hiding this comment

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

생각해보니 밑에 노란색으로 단 거 생각하면 얘도 0원인게 정상일 것 같네요

@@ -21,6 +21,12 @@ export class StockOrderService {
private readonly stockPriceSocketService: StockPriceSocketService,
) {}

async findAllPendingOrder() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

🟢 이거 limit 41 이런 식으로 걸어두고 쓰거나 해야 하지 않을까요? 그 이상이 들어오면 구독이 안되니까요.
(추가) 산님이 말씀하신 것처럼 배열의 형태로 받아서 해결방안이 좀 되지 않을까도 싶네요!

@@ -11,6 +11,12 @@ export class StockOrderRepository extends Repository<Order> {
super(Order, dataSource.createEntityManager());
}

async findAllCodeByStatus() {
return this.createQueryBuilder('orders')
.select('DISTINCT orders.stock_code')
Copy link
Collaborator

Choose a reason for hiding this comment

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

🟢 이거 DISTINCT로 하는 이유가 삼성전자 구매한 사람이 여러명일 때 삼성전자 구독이 여러 개 되어서 그런거죠? 좋네요

.update(Asset)
.set({
cash_balance: () => 'cash_balance - :realPrice',
stock_balance: () => 'stock_balance - :realPrice',
Copy link
Collaborator

Choose a reason for hiding this comment

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

🟡 이건 확실한게 주식 평가 금액이 주식 샀다고 해서 떨어지는게 아니라 그 주식 금액만큼 올라가고, 주식 변동률에 따라 실시간으로 바뀌고 그런거라 이렇게 빼면 안될 거 같습니다.

.update(Asset)
.set({
cash_balance: () => 'cash_balance + :realPrice',
stock_balance: () => 'stock_balance + :realPrice',
Copy link
Collaborator

Choose a reason for hiding this comment

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

🟡 얘도 위에랑 똑같이.. 주식을 팔았는데 주식 평가 금액이 늘어나는건 이상한 거 같아요

Copy link
Collaborator

@uuuo3o uuuo3o left a comment

Choose a reason for hiding this comment

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

고생하셨습니다👍🏻

Copy link
Collaborator

@jinddings jinddings left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!

@sieunie sieunie merged commit d8280b7 into back/main Nov 14, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BE 백엔드 SOCKET Socket 구현
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BE] 7.07 거래 체결 기능 구현 (WebSocket)
3 participants