feat: implement stock scheduler #12
Conversation
There was a problem hiding this comment.
@ConfigurationProperties 를 사용해서 값을 가져오는게 인상 깊네요!!
|
|
||
| @AfterEach | ||
| void afterEach() { | ||
| stockRepository.deleteAll(); |
There was a problem hiding this comment.
제 PR에서도 남겨둔 내용인데, @Transactional 애노테이션을 사용하면 테스트 수행 후 DB를 롤백하는 역할을 하는 것으로 알고 있어요!
There was a problem hiding this comment.
사실 이건 개인 취향일 수 있지만 저는 테스트코드에서 @Transactional을 지양하는 편이긴 해요!
예를 들어 실제 클래스에 @Transactional 처리가 빠져있을 때도 테스트코드에 의해 트랜잭션 처리가 될 수 있고 또는 트랜잭션 전파 레벨이 REQUIRES_NEW 일 경우에는 테스트코드에서 제대로 롤백이 되지 않을 수 있기도 하고..!!
테스트 실행 후에는 명시적으로 데이터를 삭제해주는 편입니다 👀
There was a problem hiding this comment.
앗 REQUIRES_NEW일 때 롤백이 되지 않는 경우가 있군요..!! 찾아봤는데 새로운 지식이 늘었습니다..!!! 저도 같은 방식으로 테스트 코드 짜볼게요!!
There was a problem hiding this comment.
테스트 용도의 객체를 생성하는 로직인 것 같은데, main 디레토리 안에 위치하신 이유가 있으실까요?! 메인 코드와 테스트 코드는 분리되면 좋을 것 같아서요!!
There was a problem hiding this comment.
앗 사실 저 위치는 testFixtures 패키지에 위치해요!! (다른 모듈에서 도메인 fixture 공통으로 사용하기 위한 패키지)
헷갈릴 수 있겠네요,,
There was a problem hiding this comment.
앗.. main 안에 들어있는 줄 알고 착각했어요!! 깃허브에서 보려니까 헷갈리는군요 🥲
| */ | ||
| @Transactional | ||
| @Scheduled(cron = "${schedules.stock}", zone = "UTC") | ||
| void run() { |
There was a problem hiding this comment.
public 이 아니어도 동작하긴 하는데 외부에 노출되는 것이 좋다고 생각하시나용?
There was a problem hiding this comment.
아하 저는 막연히 public, private, protected로 접근 제한자를 명시적으로 선언하는게 낫다고 생각했는데 다른 패키지에서 접근될 일이 없는 메서드이다보니 괜찮을거 같네요!!
|
코드 리뷰를 하면서 제가 더 많이 배워가네요 🥺 친절한 코멘트 감사해요!! |
|
추가적으로 stock screener API 살펴보니 그냥 호출했을 때는 전체 주식이 다 나오진 않는거 같아요! 그래서 sector별로 한번씩 호출하는 방식으로 조회하면 될 것 같습니다! 해당 PR merge 후 배당금 쪽 코드도 리팩토링 하는 작업 거칠게요! |
오옷 검토 감사합니다👍 섹터 별로 호출하도록 수정 한 후 pr merge 할게요~! +) conflict 난 부분들 대충 통일시켜놓긴 했는데 리팩터링 한번만 부탁드려요 🙌🏻 |
Issue Number
close: #10
작업 내역
변경사항
java-test-fixtures플러그인 추가PR 특이 사항
fmp API 중 stock-screener로 한번에 호출하면 될 것 같은데 검토 한번만 부탁드립니다 🙇🏻♀️
(+ stock 스케줄러가 UTC 기준 자정에 실행되니까 dividend 스케줄러 그 이후에 실행되도록 함께 변경 필요)